media: mt9p031: Implement crop bounds get selection

  Implement V4L2_SEL_TGT_CROP_BOUNDS query in get_selection subdev op
for this sensor. This is required e.g. to bind it to STM32MP15x DCMI.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Stefan Riedmueller <s.riedmueller@phytec.de>
---
 drivers/media/i2c/mt9p031.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
  

Hi Marek,

Thank you for the patch.

On Sun, Jun 19, 2022 at 12:21:08AM +0200, Marek Vasut wrote:
> Implement V4L2_SEL_TGT_CROP_BOUNDS query in get_selection subdev op
> for this sensor. This is required e.g. to bind it to STM32MP15x DCMI.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Stefan Riedmueller <s.riedmueller@phytec.de>
> ---
>  drivers/media/i2c/mt9p031.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index cbce8b88dbcf5..e73e814c60207 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -623,12 +623,20 @@ static int mt9p031_get_selection(struct v4l2_subdev *subdev,
>  {
>  	struct mt9p031 *mt9p031 = to_mt9p031(subdev);
>  
> -	if (sel->target != V4L2_SEL_TGT_CROP)
> +	switch (sel->target) {
> +	case V4L2_SEL_TGT_CROP_BOUNDS:
> +		sel->r.left = MT9P031_COLUMN_START_DEF;
> +		sel->r.top = MT9P031_ROW_START_DEF;

Shouldn't this be MT9P031_COLUMN_START_MIN and MT9P031_ROW_START_MIN ?

> +		sel->r.width = MT9P031_WINDOW_WIDTH_DEF;
> +		sel->r.height = MT9P031_WINDOW_HEIGHT_DEF;

And here MT9P031_WINDOW_WIDTH_MAX and MT9P031_WINDOW_HEIGHT_MAX ?

> +		return 0;
> +	case V4L2_SEL_TGT_CROP:
> +		sel->r = *__mt9p031_get_pad_crop(mt9p031, sd_state,
> +						 sel->pad, sel->which);
> +		return 0;
> +	default:
>  		return -EINVAL;
> -
> -	sel->r = *__mt9p031_get_pad_crop(mt9p031, sd_state, sel->pad,
> -					 sel->which);
> -	return 0;
> +	}
>  }
>  
>  static int mt9p031_set_selection(struct v4l2_subdev *subdev,
  

On 6/19/22 01:15, Laurent Pinchart wrote:
> Hi Marek,
> 
> Thank you for the patch.
> 
> On Sun, Jun 19, 2022 at 12:21:08AM +0200, Marek Vasut wrote:
>> Implement V4L2_SEL_TGT_CROP_BOUNDS query in get_selection subdev op
>> for this sensor. This is required e.g. to bind it to STM32MP15x DCMI.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Cc: Stefan Riedmueller <s.riedmueller@phytec.de>
>> ---
>>   drivers/media/i2c/mt9p031.c | 18 +++++++++++++-----
>>   1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
>> index cbce8b88dbcf5..e73e814c60207 100644
>> --- a/drivers/media/i2c/mt9p031.c
>> +++ b/drivers/media/i2c/mt9p031.c
>> @@ -623,12 +623,20 @@ static int mt9p031_get_selection(struct v4l2_subdev *subdev,
>>   {
>>   	struct mt9p031 *mt9p031 = to_mt9p031(subdev);
>>   
>> -	if (sel->target != V4L2_SEL_TGT_CROP)
>> +	switch (sel->target) {
>> +	case V4L2_SEL_TGT_CROP_BOUNDS:
>> +		sel->r.left = MT9P031_COLUMN_START_DEF;
>> +		sel->r.top = MT9P031_ROW_START_DEF;
> 
> Shouldn't this be MT9P031_COLUMN_START_MIN and MT9P031_ROW_START_MIN ?
> 
>> +		sel->r.width = MT9P031_WINDOW_WIDTH_DEF;
>> +		sel->r.height = MT9P031_WINDOW_HEIGHT_DEF;
> 
> And here MT9P031_WINDOW_WIDTH_MAX and MT9P031_WINDOW_HEIGHT_MAX ?

What makes you think that ?
  

On Sun, Jun 19, 2022 at 01:35:57AM +0200, Marek Vasut wrote:
> On 6/19/22 01:15, Laurent Pinchart wrote:
> > Hi Marek,
> > 
> > Thank you for the patch.
> > 
> > On Sun, Jun 19, 2022 at 12:21:08AM +0200, Marek Vasut wrote:
> >> Implement V4L2_SEL_TGT_CROP_BOUNDS query in get_selection subdev op
> >> for this sensor. This is required e.g. to bind it to STM32MP15x DCMI.
> >>
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> Cc: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> >> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> >> Cc: Stefan Riedmueller <s.riedmueller@phytec.de>
> >> ---
> >>   drivers/media/i2c/mt9p031.c | 18 +++++++++++++-----
> >>   1 file changed, 13 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> >> index cbce8b88dbcf5..e73e814c60207 100644
> >> --- a/drivers/media/i2c/mt9p031.c
> >> +++ b/drivers/media/i2c/mt9p031.c
> >> @@ -623,12 +623,20 @@ static int mt9p031_get_selection(struct v4l2_subdev *subdev,
> >>   {
> >>   	struct mt9p031 *mt9p031 = to_mt9p031(subdev);
> >>   
> >> -	if (sel->target != V4L2_SEL_TGT_CROP)
> >> +	switch (sel->target) {
> >> +	case V4L2_SEL_TGT_CROP_BOUNDS:
> >> +		sel->r.left = MT9P031_COLUMN_START_DEF;
> >> +		sel->r.top = MT9P031_ROW_START_DEF;
> > 
> > Shouldn't this be MT9P031_COLUMN_START_MIN and MT9P031_ROW_START_MIN ?
> > 
> >> +		sel->r.width = MT9P031_WINDOW_WIDTH_DEF;
> >> +		sel->r.height = MT9P031_WINDOW_HEIGHT_DEF;
> > 
> > And here MT9P031_WINDOW_WIDTH_MAX and MT9P031_WINDOW_HEIGHT_MAX ?
> 
> What makes you think that ?

https://linuxtv.org/downloads/v4l-dvb-apis/userspace-api/v4l/v4l2-selection-targets.html#v4l2-selection-targets

"Bounds of the crop rectangle. All valid crop rectangles fit inside the
crop bounds rectangle."