diff mbox series

[v2,04/30] media: atmel: atmel-isc: specialize max width and max height

Message ID 20210405155105.162529-5-eugen.hristev@microchip.com
State Superseded
Headers show
Series media: atmel: atmel-isc: add support for xisc | expand

Commit Message

Eugen Hristev April 5, 2021, 3:50 p.m. UTC
Move the max width and max height constants to the product specific driver
and have them in the device struct.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/media/platform/atmel/atmel-isc-base.c | 28 +++++++++----------
 drivers/media/platform/atmel/atmel-isc.h      |  9 ++++--
 .../media/platform/atmel/atmel-sama5d2-isc.c  |  7 +++--
 3 files changed, 25 insertions(+), 19 deletions(-)

Comments

Jacopo Mondi April 12, 2021, 9:53 a.m. UTC | #1
Ups,

On Mon, Apr 12, 2021 at 11:43:12AM +0200, Jacopo Mondi wrote:
> Hi Eugene,

>

> On Mon, Apr 05, 2021 at 06:50:39PM +0300, Eugen Hristev wrote:

> > Move the max width and max height constants to the product specific driver

> > and have them in the device struct.

> >

> > Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>

> > ---

> >  drivers/media/platform/atmel/atmel-isc-base.c | 28 +++++++++----------

> >  drivers/media/platform/atmel/atmel-isc.h      |  9 ++++--

> >  .../media/platform/atmel/atmel-sama5d2-isc.c  |  7 +++--

> >  3 files changed, 25 insertions(+), 19 deletions(-)

> >

> > diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c

> > index 45fc8dbb7943..350076dd029a 100644

> > --- a/drivers/media/platform/atmel/atmel-isc-base.c

> > +++ b/drivers/media/platform/atmel/atmel-isc-base.c

> > @@ -1204,8 +1204,8 @@ static void isc_try_fse(struct isc_device *isc,

> >  	 * just use the maximum ISC can receive.

> >  	 */

> >  	if (ret) {

> > -		pad_cfg->try_crop.width = ISC_MAX_SUPPORT_WIDTH;

> > -		pad_cfg->try_crop.height = ISC_MAX_SUPPORT_HEIGHT;

> > +		pad_cfg->try_crop.width = isc->max_width;

> > +		pad_cfg->try_crop.height = isc->max_height;

> >  	} else {

> >  		pad_cfg->try_crop.width = fse.max_width;

> >  		pad_cfg->try_crop.height = fse.max_height;

> > @@ -1282,10 +1282,10 @@ static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,

> >  	isc->try_config.sd_format = sd_fmt;

> >

> >  	/* Limit to Atmel ISC hardware capabilities */

> > -	if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)

> > -		pixfmt->width = ISC_MAX_SUPPORT_WIDTH;

> > -	if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)

> > -		pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;

> > +	if (pixfmt->width > isc->max_width)

> > +		pixfmt->width = isc->max_width;

> > +	if (pixfmt->height > isc->max_height)

> > +		pixfmt->height = isc->max_height;

> >

> >  	/*

> >  	 * The mbus format is the one the subdev outputs.

> > @@ -1327,10 +1327,10 @@ static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,

> >  	v4l2_fill_pix_format(pixfmt, &format.format);

> >

> >  	/* Limit to Atmel ISC hardware capabilities */

> > -	if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)

> > -		pixfmt->width = ISC_MAX_SUPPORT_WIDTH;

> > -	if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)

> > -		pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;

> > +	if (pixfmt->width > isc->max_width)

> > +		pixfmt->width = isc->max_width;

> > +	if (pixfmt->height > isc->max_height)

> > +		pixfmt->height = isc->max_height;

>

> What happens if the sensor sends a frame larger that the ISC max

> supported sizes ?

>


I meant to ask this question on the previous patch :/

> >

> >  	pixfmt->field = V4L2_FIELD_NONE;

> >  	pixfmt->bytesperline = (pixfmt->width * isc->try_config.bpp) >> 3;

> > @@ -1368,10 +1368,10 @@ static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)

> >  		return ret;

> >

> >  	/* Limit to Atmel ISC hardware capabilities */

> > -	if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)

> > -		pixfmt->width = ISC_MAX_SUPPORT_WIDTH;

> > -	if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)

> > -		pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;

> > +	if (f->fmt.pix.width > isc->max_width)

> > +		f->fmt.pix.width = isc->max_width;

> > +	if (f->fmt.pix.height > isc->max_height)

> > +		f->fmt.pix.height = isc->max_height;

> >

> >  	isc->fmt = *f;

> >

> > diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h

> > index 8d81d9967ad2..6becc6c3aaf0 100644

> > --- a/drivers/media/platform/atmel/atmel-isc.h

> > +++ b/drivers/media/platform/atmel/atmel-isc.h

> > @@ -10,9 +10,6 @@

> >   */

> >  #ifndef _ATMEL_ISC_H_

> >

> > -#define ISC_MAX_SUPPORT_WIDTH   2592

> > -#define ISC_MAX_SUPPORT_HEIGHT  1944

> > -

> >  #define ISC_CLK_MAX_DIV		255

> >

> >  enum isc_clk_id {

> > @@ -191,6 +188,9 @@ struct isc_ctrls {

> >   * @gamma_table:	pointer to the table with gamma values, has

> >   *			gamma_max sets of GAMMA_ENTRIES entries each

> >   * @gamma_max:		maximum number of sets of inside the gamma_table

> > + *

> > + * @max_width:		maximum frame width, dependent on the internal RAM

> > + * @max_height:		maximum frame height, dependent on the internal RAM

> >   */

> >  struct isc_device {

> >  	struct regmap		*regmap;

> > @@ -254,6 +254,9 @@ struct isc_device {

> >  	/* pointer to the defined gamma table */

> >  	const u32	(*gamma_table)[GAMMA_ENTRIES];

> >  	u32		gamma_max;

> > +

> > +	u32		max_width;

> > +	u32		max_height;

> >  };

> >

> >  extern struct isc_format formats_list[];

> > diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c

> > index f45d8b96bfb8..f8d1c8ba99b3 100644

> > --- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c

> > +++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c

> > @@ -49,8 +49,8 @@

> >  #include "atmel-isc-regs.h"

> >  #include "atmel-isc.h"

> >

> > -#define ISC_MAX_SUPPORT_WIDTH   2592

> > -#define ISC_MAX_SUPPORT_HEIGHT  1944

> > +#define ISC_SAMA5D2_MAX_SUPPORT_WIDTH   2592

> > +#define ISC_SAMA5D2_MAX_SUPPORT_HEIGHT  1944

> >

> >  #define ISC_CLK_MAX_DIV		255

> >

> > @@ -195,6 +195,9 @@ static int atmel_isc_probe(struct platform_device *pdev)

> >  	isc->gamma_table = isc_sama5d2_gamma_table;

> >  	isc->gamma_max = 2;

> >

> > +	isc->max_width = ISC_SAMA5D2_MAX_SUPPORT_WIDTH;

> > +	isc->max_height = ISC_SAMA5D2_MAX_SUPPORT_HEIGHT;

> > +

> >  	ret = isc_pipeline_init(isc);

> >  	if (ret)

> >  		return ret;

> > --

> > 2.25.1

> >
Eugen Hristev April 12, 2021, 10:04 a.m. UTC | #2
On 4/12/21 12:53 PM, Jacopo Mondi wrote:
> Ups,

> 

> On Mon, Apr 12, 2021 at 11:43:12AM +0200, Jacopo Mondi wrote:

>> Hi Eugene,

>>

>> On Mon, Apr 05, 2021 at 06:50:39PM +0300, Eugen Hristev wrote:

>>> Move the max width and max height constants to the product specific driver

>>> and have them in the device struct.

>>>

>>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>

>>> ---

>>>   drivers/media/platform/atmel/atmel-isc-base.c | 28 +++++++++----------

>>>   drivers/media/platform/atmel/atmel-isc.h      |  9 ++++--

>>>   .../media/platform/atmel/atmel-sama5d2-isc.c  |  7 +++--

>>>   3 files changed, 25 insertions(+), 19 deletions(-)

>>>

>>> diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c

>>> index 45fc8dbb7943..350076dd029a 100644

>>> --- a/drivers/media/platform/atmel/atmel-isc-base.c

>>> +++ b/drivers/media/platform/atmel/atmel-isc-base.c

>>> @@ -1204,8 +1204,8 @@ static void isc_try_fse(struct isc_device *isc,

>>>       * just use the maximum ISC can receive.

>>>       */

>>>      if (ret) {

>>> -           pad_cfg->try_crop.width = ISC_MAX_SUPPORT_WIDTH;

>>> -           pad_cfg->try_crop.height = ISC_MAX_SUPPORT_HEIGHT;

>>> +           pad_cfg->try_crop.width = isc->max_width;

>>> +           pad_cfg->try_crop.height = isc->max_height;

>>>      } else {

>>>              pad_cfg->try_crop.width = fse.max_width;

>>>              pad_cfg->try_crop.height = fse.max_height;

>>> @@ -1282,10 +1282,10 @@ static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,

>>>      isc->try_config.sd_format = sd_fmt;

>>>

>>>      /* Limit to Atmel ISC hardware capabilities */

>>> -   if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)

>>> -           pixfmt->width = ISC_MAX_SUPPORT_WIDTH;

>>> -   if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)

>>> -           pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;

>>> +   if (pixfmt->width > isc->max_width)

>>> +           pixfmt->width = isc->max_width;

>>> +   if (pixfmt->height > isc->max_height)

>>> +           pixfmt->height = isc->max_height;

>>>

>>>      /*

>>>       * The mbus format is the one the subdev outputs.

>>> @@ -1327,10 +1327,10 @@ static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,

>>>      v4l2_fill_pix_format(pixfmt, &format.format);

>>>

>>>      /* Limit to Atmel ISC hardware capabilities */

>>> -   if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)

>>> -           pixfmt->width = ISC_MAX_SUPPORT_WIDTH;

>>> -   if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)

>>> -           pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;

>>> +   if (pixfmt->width > isc->max_width)

>>> +           pixfmt->width = isc->max_width;

>>> +   if (pixfmt->height > isc->max_height)

>>> +           pixfmt->height = isc->max_height;

>>

>> What happens if the sensor sends a frame larger that the ISC max

>> supported sizes ?

>>

> 

> I meant to ask this question on the previous patch :/


Hi Jacopo,

The ISC has a feature in the PFE module (parallel front end), the first 
pixel capturing module, which will automatically crop the frame at the 
maximum size (or configured frame size).

here is the commit that implements this safety mechanism :

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=253ccf34232ae3b47497e5e55aef3ac48821425c

Eugen

> 

>>>

>>>      pixfmt->field = V4L2_FIELD_NONE;

>>>      pixfmt->bytesperline = (pixfmt->width * isc->try_config.bpp) >> 3;

>>> @@ -1368,10 +1368,10 @@ static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)

>>>              return ret;

>>>

>>>      /* Limit to Atmel ISC hardware capabilities */

>>> -   if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)

>>> -           pixfmt->width = ISC_MAX_SUPPORT_WIDTH;

>>> -   if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)

>>> -           pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;

>>> +   if (f->fmt.pix.width > isc->max_width)

>>> +           f->fmt.pix.width = isc->max_width;

>>> +   if (f->fmt.pix.height > isc->max_height)

>>> +           f->fmt.pix.height = isc->max_height;

>>>

>>>      isc->fmt = *f;

>>>

>>> diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h

>>> index 8d81d9967ad2..6becc6c3aaf0 100644

>>> --- a/drivers/media/platform/atmel/atmel-isc.h

>>> +++ b/drivers/media/platform/atmel/atmel-isc.h

>>> @@ -10,9 +10,6 @@

>>>    */

>>>   #ifndef _ATMEL_ISC_H_

>>>

>>> -#define ISC_MAX_SUPPORT_WIDTH   2592

>>> -#define ISC_MAX_SUPPORT_HEIGHT  1944

>>> -

>>>   #define ISC_CLK_MAX_DIV            255

>>>

>>>   enum isc_clk_id {

>>> @@ -191,6 +188,9 @@ struct isc_ctrls {

>>>    * @gamma_table:   pointer to the table with gamma values, has

>>>    *                 gamma_max sets of GAMMA_ENTRIES entries each

>>>    * @gamma_max:             maximum number of sets of inside the gamma_table

>>> + *

>>> + * @max_width:             maximum frame width, dependent on the internal RAM

>>> + * @max_height:            maximum frame height, dependent on the internal RAM

>>>    */

>>>   struct isc_device {

>>>      struct regmap           *regmap;

>>> @@ -254,6 +254,9 @@ struct isc_device {

>>>      /* pointer to the defined gamma table */

>>>      const u32       (*gamma_table)[GAMMA_ENTRIES];

>>>      u32             gamma_max;

>>> +

>>> +   u32             max_width;

>>> +   u32             max_height;

>>>   };

>>>

>>>   extern struct isc_format formats_list[];

>>> diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c

>>> index f45d8b96bfb8..f8d1c8ba99b3 100644

>>> --- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c

>>> +++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c

>>> @@ -49,8 +49,8 @@

>>>   #include "atmel-isc-regs.h"

>>>   #include "atmel-isc.h"

>>>

>>> -#define ISC_MAX_SUPPORT_WIDTH   2592

>>> -#define ISC_MAX_SUPPORT_HEIGHT  1944

>>> +#define ISC_SAMA5D2_MAX_SUPPORT_WIDTH   2592

>>> +#define ISC_SAMA5D2_MAX_SUPPORT_HEIGHT  1944

>>>

>>>   #define ISC_CLK_MAX_DIV            255

>>>

>>> @@ -195,6 +195,9 @@ static int atmel_isc_probe(struct platform_device *pdev)

>>>      isc->gamma_table = isc_sama5d2_gamma_table;

>>>      isc->gamma_max = 2;

>>>

>>> +   isc->max_width = ISC_SAMA5D2_MAX_SUPPORT_WIDTH;

>>> +   isc->max_height = ISC_SAMA5D2_MAX_SUPPORT_HEIGHT;

>>> +

>>>      ret = isc_pipeline_init(isc);

>>>      if (ret)

>>>              return ret;

>>> --

>>> 2.25.1

>>>
diff mbox series

Patch

diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
index 45fc8dbb7943..350076dd029a 100644
--- a/drivers/media/platform/atmel/atmel-isc-base.c
+++ b/drivers/media/platform/atmel/atmel-isc-base.c
@@ -1204,8 +1204,8 @@  static void isc_try_fse(struct isc_device *isc,
 	 * just use the maximum ISC can receive.
 	 */
 	if (ret) {
-		pad_cfg->try_crop.width = ISC_MAX_SUPPORT_WIDTH;
-		pad_cfg->try_crop.height = ISC_MAX_SUPPORT_HEIGHT;
+		pad_cfg->try_crop.width = isc->max_width;
+		pad_cfg->try_crop.height = isc->max_height;
 	} else {
 		pad_cfg->try_crop.width = fse.max_width;
 		pad_cfg->try_crop.height = fse.max_height;
@@ -1282,10 +1282,10 @@  static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,
 	isc->try_config.sd_format = sd_fmt;
 
 	/* Limit to Atmel ISC hardware capabilities */
-	if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)
-		pixfmt->width = ISC_MAX_SUPPORT_WIDTH;
-	if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)
-		pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;
+	if (pixfmt->width > isc->max_width)
+		pixfmt->width = isc->max_width;
+	if (pixfmt->height > isc->max_height)
+		pixfmt->height = isc->max_height;
 
 	/*
 	 * The mbus format is the one the subdev outputs.
@@ -1327,10 +1327,10 @@  static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,
 	v4l2_fill_pix_format(pixfmt, &format.format);
 
 	/* Limit to Atmel ISC hardware capabilities */
-	if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)
-		pixfmt->width = ISC_MAX_SUPPORT_WIDTH;
-	if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)
-		pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;
+	if (pixfmt->width > isc->max_width)
+		pixfmt->width = isc->max_width;
+	if (pixfmt->height > isc->max_height)
+		pixfmt->height = isc->max_height;
 
 	pixfmt->field = V4L2_FIELD_NONE;
 	pixfmt->bytesperline = (pixfmt->width * isc->try_config.bpp) >> 3;
@@ -1368,10 +1368,10 @@  static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)
 		return ret;
 
 	/* Limit to Atmel ISC hardware capabilities */
-	if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)
-		pixfmt->width = ISC_MAX_SUPPORT_WIDTH;
-	if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)
-		pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;
+	if (f->fmt.pix.width > isc->max_width)
+		f->fmt.pix.width = isc->max_width;
+	if (f->fmt.pix.height > isc->max_height)
+		f->fmt.pix.height = isc->max_height;
 
 	isc->fmt = *f;
 
diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h
index 8d81d9967ad2..6becc6c3aaf0 100644
--- a/drivers/media/platform/atmel/atmel-isc.h
+++ b/drivers/media/platform/atmel/atmel-isc.h
@@ -10,9 +10,6 @@ 
  */
 #ifndef _ATMEL_ISC_H_
 
-#define ISC_MAX_SUPPORT_WIDTH   2592
-#define ISC_MAX_SUPPORT_HEIGHT  1944
-
 #define ISC_CLK_MAX_DIV		255
 
 enum isc_clk_id {
@@ -191,6 +188,9 @@  struct isc_ctrls {
  * @gamma_table:	pointer to the table with gamma values, has
  *			gamma_max sets of GAMMA_ENTRIES entries each
  * @gamma_max:		maximum number of sets of inside the gamma_table
+ *
+ * @max_width:		maximum frame width, dependent on the internal RAM
+ * @max_height:		maximum frame height, dependent on the internal RAM
  */
 struct isc_device {
 	struct regmap		*regmap;
@@ -254,6 +254,9 @@  struct isc_device {
 	/* pointer to the defined gamma table */
 	const u32	(*gamma_table)[GAMMA_ENTRIES];
 	u32		gamma_max;
+
+	u32		max_width;
+	u32		max_height;
 };
 
 extern struct isc_format formats_list[];
diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c
index f45d8b96bfb8..f8d1c8ba99b3 100644
--- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c
+++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c
@@ -49,8 +49,8 @@ 
 #include "atmel-isc-regs.h"
 #include "atmel-isc.h"
 
-#define ISC_MAX_SUPPORT_WIDTH   2592
-#define ISC_MAX_SUPPORT_HEIGHT  1944
+#define ISC_SAMA5D2_MAX_SUPPORT_WIDTH   2592
+#define ISC_SAMA5D2_MAX_SUPPORT_HEIGHT  1944
 
 #define ISC_CLK_MAX_DIV		255
 
@@ -195,6 +195,9 @@  static int atmel_isc_probe(struct platform_device *pdev)
 	isc->gamma_table = isc_sama5d2_gamma_table;
 	isc->gamma_max = 2;
 
+	isc->max_width = ISC_SAMA5D2_MAX_SUPPORT_WIDTH;
+	isc->max_height = ISC_SAMA5D2_MAX_SUPPORT_HEIGHT;
+
 	ret = isc_pipeline_init(isc);
 	if (ret)
 		return ret;