Re: [PATCH v2 4/6] rcar-vin: Do not reset the crop and compose rectangles in s_fmt

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

Hi Laurent,

Thanks for your feedback.

On 2019-08-08 11:37:51 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Thu, Aug 08, 2019 at 03:18:48AM +0200, Niklas Söderlund wrote:
> > The crop and compose rectangles are reset when s_fmt is called
> > resulting in potentially valid rectangles being lost when updating the
> > format.
> 
> Isn't this the expected behaviour ?

I though so to at first but I had a nagging feeling as I was not sure.  
So I went and looked at vivid and it does do map the old crop/compose 
rectangles inside the new limits vivid_s_fmt_vid_cap().

Please note that the variable names in this patch are horrible and are 
fixed later in this series. In essence the current crop rectangle is 
mapped inside the size of the video coming from the sensor and the 
compose rectangle is mapped inside the new format set on the VIN video 
device.

I'm open to dropping this patch, I just want this driver to get this 
part right so I mimic vivid.

> 
> > Fix this by mapping the rectangles inside the new format.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> >  drivers/media/platform/rcar-vin/rcar-v4l2.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > index 8b30267f1636aaf1..5dcd787a9cf96ac9 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > @@ -279,8 +279,8 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv,
> >  		return ret;
> >  
> >  	vin->format = f->fmt.pix;
> > -	vin->crop = crop;
> > -	vin->compose = compose;
> > +	v4l2_rect_map_inside(&vin->crop, &crop);
> > +	v4l2_rect_map_inside(&vin->compose, &compose);
> >  	vin->src_rect = crop;
> >  
> >  	return 0;
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Niklas Söderlund