Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad

[Mauro Carvalho Chehab]

Em Mon, 19 Sep 2016 21:33:13 +0300
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> On Monday 19 Sep 2016 15:26:15 Mauro Carvalho Chehab wrote:
> > Em Mon, 19 Sep 2016 20:59:56 +0300 Laurent Pinchart escreveu:  
> > > On Monday 19 Sep 2016 14:55:43 Mauro Carvalho Chehab wrote:  
> > >> Em Wed, 14 Sep 2016 02:16:59 +0300 Laurent Pinchart escreveu:  
> > >>> Cropping on the WPF sink pad restricts the left and top coordinates to
> > >>> 0-255. The same result can be obtained by cropping on the RPF without
> > >>> any such restriction, this feature isn't useful. Disable it.
> > >>> 
> > >>> Signed-off-by: Laurent Pinchart
> > >>> 
> > >>> ---
> > >>> 
> > >>>  drivers/media/platform/vsp1/vsp1_rwpf.c | 37 
> > >>>  drivers/media/platform/vsp1/vsp1_wpf.c  | 18 +++-
> > >>>  2 files changed, 26 insertions(+), 29 deletions(-)
> > >>> 
> > >>> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > >>> b/drivers/media/platform/vsp1/vsp1_rwpf.c index
> > >>> 8cb87e96b78b..a3ace8df7f4d 100644
> > >>> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > >>> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c  
> 
> [snip]
> 
> > >>> @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct
> > >>> v4l2_subdev *subdev,
> > >>> struct v4l2_mbus_framefmt *format;
> > >>> int ret = 0;
> > >>> 
> > >>> -   /* Cropping is implemented on the sink pad. */
> > >>> -   if (sel->pad != RWPF_PAD_SINK)
> > >>> +   /* Cropping is only supported on the RPF and is implemented on
> > >>> the sink
> > >>> +* pad.
> > >>> +*/  
> > >> 
> > >> Please read CodingStyle and run checkpatch before sending stuff
> > >> upstream.
> > >> 
> > >> This violates the CodingStyle: it should be, instead:
> > >>  /*
> > >>   * foo
> > >>   * bar
> > >>   */  
> > > 
> > > But it's consistent with the coding style of this driver. I'm OK fixing
> > > it, but it should be done globally in that case.  
> > 
> > There are inconsistencies inside the driver too on multi-line
> > comments even without fixing the ones introduced on this series,
> > as, on several places, multi-line comments are correct:
> > 
> > drivers/media/platform/vsp1/vsp1_bru.c:/*
> > drivers/media/platform/vsp1/vsp1_bru.c- * The BRU can't perform format
> > conversion, all sink and source formats must be
> > drivers/media/platform/vsp1/vsp1_bru.c- * identical. We pick the format on
> > the first sink pad (pad 0) and propagate it
> > drivers/media/platform/vsp1/vsp1_bru.c- * to all other pads.
> > drivers/media/platform/vsp1/vsp1_bru.c- */
> > 
> > drivers/media/platform/vsp1/vsp1_dl.c:/*
> > drivers/media/platform/vsp1/vsp1_dl.c- * Initialize a display list body
> > object and allocate DMA memory for the body
> > drivers/media/platform/vsp1/vsp1_dl.c- * data. The display list body object
> > is expected to have been initialized to
> > drivers/media/platform/vsp1/vsp1_dl.c- * 0 when allocated.
> > drivers/media/platform/vsp1/vsp1_dl.c- */
> > 
> > ...
> > 
> > I'll address the ones only the CodingStyle violation introduced by this
> > series. I'll leave for the vsp1 maintainers/developers.  
> 
> OK, I'll address that.
> 
> > Btw, there are several kernel-doc tags that use just:
> > /*
> >  ...
> >  */
> > 
> > instead of:
> > 
> > /**
> >  ...
> >  */
> > 
> > I suggest you to add the files/headers with kernel-doc markups on
> > a Documentation/media/v4l-drivers/vsp1.rst file, to be created.
> > 
> > This way, you can validate that such documentation is correct,
> > and produce an auto-generated documentation for this driver.  
> 
> I've thought about it, but I don't think those comments should become part of 
> the kernel documentation. They're really about driver internals, and meant 
> for 
> the driver developers. In particular only a subset of the driver is 
> documented 
> that way, when I've considered that the code or structures were complex 
> enough 
> to need proper documentation. A generated doc would then be quite incomplete 
> and not very useful, the comments are meant to be read while working on the 
> code.

The v4l-drivers book is meant to have driver internals documentation,
and not the subsystem kAPI or uAPI.

I don't see any problems if you want to document just the more complex
functions/structs over the v4l-drivers/ book. Yet, as you already took
the time to write documentation for those functions, providing that the
kernel-doc markups are ok, a v4l-drivers/vsp1.rst file for it could be as
simple as (untested):


VSP1 driver
^^^

.. kernel-doc:: drivers/media/platform/vsp1/vsp1_dl.c

.. kernel-doc:: drivers/media/platform/vsp1/vsp1_drm.c

.. kernel-doc:: drivers/media/platform/vsp1/vsp1_drm.h

.. kernel-doc:: drivers/media/platform/vsp1/vsp1_entity.c

.. kernel-doc:: drivers/media/platform/vsp1/vsp1_entity.h

.. kernel-doc:: drivers/media/platform/vsp1/vsp1_pipe.c

.. kernel-doc:: 

Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad

[Laurent Pinchart]

Hi Mauro,

On Monday 19 Sep 2016 15:26:15 Mauro Carvalho Chehab wrote:
> Em Mon, 19 Sep 2016 20:59:56 +0300 Laurent Pinchart escreveu:
> > On Monday 19 Sep 2016 14:55:43 Mauro Carvalho Chehab wrote:
> >> Em Wed, 14 Sep 2016 02:16:59 +0300 Laurent Pinchart escreveu:
> >>> Cropping on the WPF sink pad restricts the left and top coordinates to
> >>> 0-255. The same result can be obtained by cropping on the RPF without
> >>> any such restriction, this feature isn't useful. Disable it.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> 
> >>> ---
> >>> 
> >>>  drivers/media/platform/vsp1/vsp1_rwpf.c | 37 
> >>>  drivers/media/platform/vsp1/vsp1_wpf.c  | 18 +++-
> >>>  2 files changed, 26 insertions(+), 29 deletions(-)
> >>> 
> >>> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> >>> b/drivers/media/platform/vsp1/vsp1_rwpf.c index
> >>> 8cb87e96b78b..a3ace8df7f4d 100644
> >>> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> >>> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c

[snip]

> >>> @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct
> >>> v4l2_subdev *subdev,
> >>>   struct v4l2_mbus_framefmt *format;
> >>>   int ret = 0;
> >>> 
> >>> - /* Cropping is implemented on the sink pad. */
> >>> - if (sel->pad != RWPF_PAD_SINK)
> >>> + /* Cropping is only supported on the RPF and is implemented on
> >>> the sink
> >>> +  * pad.
> >>> +  */
> >> 
> >> Please read CodingStyle and run checkpatch before sending stuff
> >> upstream.
> >> 
> >> This violates the CodingStyle: it should be, instead:
> >>/*
> >> * foo
> >> * bar
> >> */
> > 
> > But it's consistent with the coding style of this driver. I'm OK fixing
> > it, but it should be done globally in that case.
> 
> There are inconsistencies inside the driver too on multi-line
> comments even without fixing the ones introduced on this series,
> as, on several places, multi-line comments are correct:
> 
> drivers/media/platform/vsp1/vsp1_bru.c:/*
> drivers/media/platform/vsp1/vsp1_bru.c- * The BRU can't perform format
> conversion, all sink and source formats must be
> drivers/media/platform/vsp1/vsp1_bru.c- * identical. We pick the format on
> the first sink pad (pad 0) and propagate it
> drivers/media/platform/vsp1/vsp1_bru.c- * to all other pads.
> drivers/media/platform/vsp1/vsp1_bru.c- */
> 
> drivers/media/platform/vsp1/vsp1_dl.c:/*
> drivers/media/platform/vsp1/vsp1_dl.c- * Initialize a display list body
> object and allocate DMA memory for the body
> drivers/media/platform/vsp1/vsp1_dl.c- * data. The display list body object
> is expected to have been initialized to
> drivers/media/platform/vsp1/vsp1_dl.c- * 0 when allocated.
> drivers/media/platform/vsp1/vsp1_dl.c- */
> 
> ...
> 
> I'll address the ones only the CodingStyle violation introduced by this
> series. I'll leave for the vsp1 maintainers/developers.

OK, I'll address that.

> Btw, there are several kernel-doc tags that use just:
>   /*
>...
>*/
> 
> instead of:
> 
>   /**
>...
>*/
> 
> I suggest you to add the files/headers with kernel-doc markups on
> a Documentation/media/v4l-drivers/vsp1.rst file, to be created.
> 
> This way, you can validate that such documentation is correct,
> and produce an auto-generated documentation for this driver.

I've thought about it, but I don't think those comments should become part of 
the kernel documentation. They're really about driver internals, and meant for 
the driver developers. In particular only a subset of the driver is documented 
that way, when I've considered that the code or structures were complex enough 
to need proper documentation. A generated doc would then be quite incomplete 
and not very useful, the comments are meant to be read while working on the 
code.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad

[Mauro Carvalho Chehab]

Em Mon, 19 Sep 2016 20:59:56 +0300
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> On Monday 19 Sep 2016 14:55:43 Mauro Carvalho Chehab wrote:
> > Em Wed, 14 Sep 2016 02:16:59 +0300 Laurent Pinchart escreveu:  
> > > Cropping on the WPF sink pad restricts the left and top coordinates to
> > > 0-255. The same result can be obtained by cropping on the RPF without
> > > any such restriction, this feature isn't useful. Disable it.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > 
> > > ---
> > > 
> > >  drivers/media/platform/vsp1/vsp1_rwpf.c | 37
> > >  + drivers/media/platform/vsp1/vsp1_wpf.c
> > >   | 18 +++-
> > >  2 files changed, 26 insertions(+), 29 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > > b/drivers/media/platform/vsp1/vsp1_rwpf.c index
> > > 8cb87e96b78b..a3ace8df7f4d 100644
> > > --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > > +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> > > @@ -66,7 +66,6 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev  
> > > *subdev,>   
> > >   struct vsp1_rwpf *rwpf = to_rwpf(subdev);
> > >   struct v4l2_subdev_pad_config *config;
> > >   struct v4l2_mbus_framefmt *format;
> > > 
> > > - struct v4l2_rect *crop;
> > > 
> > >   int ret = 0;
> > >   
> > >   mutex_lock(>entity.lock);
> > > 
> > > @@ -103,12 +102,16 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev  
> > > *subdev,>   
> > >   fmt->format = *format;
> > > 
> > > - /* Update the sink crop rectangle. */
> > > - crop = vsp1_rwpf_get_crop(rwpf, config);
> > > - crop->left = 0;
> > > - crop->top = 0;
> > > - crop->width = fmt->format.width;
> > > - crop->height = fmt->format.height;
> > > + if (rwpf->entity.type == VSP1_ENTITY_RPF) {
> > > + struct v4l2_rect *crop;
> > > +
> > > + /* Update the sink crop rectangle. */
> > > + crop = vsp1_rwpf_get_crop(rwpf, config);
> > > + crop->left = 0;
> > > + crop->top = 0;
> > > + crop->width = fmt->format.width;
> > > + crop->height = fmt->format.height;
> > > + }
> > > 
> > >   /* Propagate the format to the source pad. */
> > >   format = vsp1_entity_get_pad_format(>entity, config,
> > > 
> > > @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct 
> > > v4l2_subdev  
> > > *subdev,>   
> > >   struct v4l2_mbus_framefmt *format;
> > >   int ret = 0;
> > > 
> > > - /* Cropping is implemented on the sink pad. */
> > > - if (sel->pad != RWPF_PAD_SINK)
> > > + /* Cropping is only supported on the RPF and is implemented on the   
> sink
> > > +  * pad.
> > > +  */  
> > 
> > Please read CodingStyle and run checkpatch before sending stuff upstream.
> > 
> > This violates the CodingStyle: it should be, instead:
> > /*
> >  * foo
> >  * bar
> >  */  
> 
> But it's consistent with the coding style of this driver. I'm OK fixing it, 
> but it should be done globally in that case.

There are inconsistencies inside the driver too on multi-line
comments even without fixing the ones introduced on this series,
as, on several places, multi-line comments are correct:

drivers/media/platform/vsp1/vsp1_bru.c:/*
drivers/media/platform/vsp1/vsp1_bru.c- * The BRU can't perform format 
conversion, all sink and source formats must be
drivers/media/platform/vsp1/vsp1_bru.c- * identical. We pick the format on the 
first sink pad (pad 0) and propagate it
drivers/media/platform/vsp1/vsp1_bru.c- * to all other pads.
drivers/media/platform/vsp1/vsp1_bru.c- */

drivers/media/platform/vsp1/vsp1_dl.c:/*
drivers/media/platform/vsp1/vsp1_dl.c- * Initialize a display list body object 
and allocate DMA memory for the body
drivers/media/platform/vsp1/vsp1_dl.c- * data. The display list body object is 
expected to have been initialized to
drivers/media/platform/vsp1/vsp1_dl.c- * 0 when allocated.
drivers/media/platform/vsp1/vsp1_dl.c- */

...

I'll address the ones only the CodingStyle violation introduced by this
series. I'll leave for the vsp1 maintainers/developers.

Btw, there are several kernel-doc tags that use just:
/*
 ...
 */

instead of:

/**
 ...
 */

I suggest you to add the files/headers with kernel-doc markups on
a Documentation/media/v4l-drivers/vsp1.rst file, to be created.

This way, you can validate that such documentation is correct,
and produce an auto-generated documentation for this driver.

Regards,
Mauro

Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad

[Laurent Pinchart]

Hi Mauro,

On Monday 19 Sep 2016 14:55:43 Mauro Carvalho Chehab wrote:
> Em Wed, 14 Sep 2016 02:16:59 +0300 Laurent Pinchart escreveu:
> > Cropping on the WPF sink pad restricts the left and top coordinates to
> > 0-255. The same result can be obtained by cropping on the RPF without
> > any such restriction, this feature isn't useful. Disable it.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/media/platform/vsp1/vsp1_rwpf.c | 37
> >  + drivers/media/platform/vsp1/vsp1_wpf.c
> >   | 18 +++-
> >  2 files changed, 26 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > b/drivers/media/platform/vsp1/vsp1_rwpf.c index
> > 8cb87e96b78b..a3ace8df7f4d 100644
> > --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> > @@ -66,7 +66,6 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev
> > *subdev,> 
> > struct vsp1_rwpf *rwpf = to_rwpf(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > 
> > -   struct v4l2_rect *crop;
> > 
> > int ret = 0;
> > 
> > mutex_lock(>entity.lock);
> > 
> > @@ -103,12 +102,16 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev
> > *subdev,> 
> > fmt->format = *format;
> > 
> > -   /* Update the sink crop rectangle. */
> > -   crop = vsp1_rwpf_get_crop(rwpf, config);
> > -   crop->left = 0;
> > -   crop->top = 0;
> > -   crop->width = fmt->format.width;
> > -   crop->height = fmt->format.height;
> > +   if (rwpf->entity.type == VSP1_ENTITY_RPF) {
> > +   struct v4l2_rect *crop;
> > +
> > +   /* Update the sink crop rectangle. */
> > +   crop = vsp1_rwpf_get_crop(rwpf, config);
> > +   crop->left = 0;
> > +   crop->top = 0;
> > +   crop->width = fmt->format.width;
> > +   crop->height = fmt->format.height;
> > +   }
> > 
> > /* Propagate the format to the source pad. */
> > format = vsp1_entity_get_pad_format(>entity, config,
> > 
> > @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev
> > *subdev,> 
> > struct v4l2_mbus_framefmt *format;
> > int ret = 0;
> > 
> > -   /* Cropping is implemented on the sink pad. */
> > -   if (sel->pad != RWPF_PAD_SINK)
> > +   /* Cropping is only supported on the RPF and is implemented on the 
sink
> > +* pad.
> > +*/
> 
> Please read CodingStyle and run checkpatch before sending stuff upstream.
> 
> This violates the CodingStyle: it should be, instead:
>   /*
>* foo
>* bar
>*/

But it's consistent with the coding style of this driver. I'm OK fixing it, 
but it should be done globally in that case.

> This time, I'll fix it, but next time I might not have enough time, and
> need to reject the patch series.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad

[Mauro Carvalho Chehab]

Em Wed, 14 Sep 2016 02:16:59 +0300
Laurent Pinchart  escreveu:

> Cropping on the WPF sink pad restricts the left and top coordinates to
> 0-255. The same result can be obtained by cropping on the RPF without
> any such restriction, this feature isn't useful. Disable it.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/media/platform/vsp1/vsp1_rwpf.c | 37 
> +
>  drivers/media/platform/vsp1/vsp1_wpf.c  | 18 +++-
>  2 files changed, 26 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c 
> b/drivers/media/platform/vsp1/vsp1_rwpf.c
> index 8cb87e96b78b..a3ace8df7f4d 100644
> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> @@ -66,7 +66,6 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
>   struct vsp1_rwpf *rwpf = to_rwpf(subdev);
>   struct v4l2_subdev_pad_config *config;
>   struct v4l2_mbus_framefmt *format;
> - struct v4l2_rect *crop;
>   int ret = 0;
>  
>   mutex_lock(>entity.lock);
> @@ -103,12 +102,16 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev 
> *subdev,
>  
>   fmt->format = *format;
>  
> - /* Update the sink crop rectangle. */
> - crop = vsp1_rwpf_get_crop(rwpf, config);
> - crop->left = 0;
> - crop->top = 0;
> - crop->width = fmt->format.width;
> - crop->height = fmt->format.height;
> + if (rwpf->entity.type == VSP1_ENTITY_RPF) {
> + struct v4l2_rect *crop;
> +
> + /* Update the sink crop rectangle. */
> + crop = vsp1_rwpf_get_crop(rwpf, config);
> + crop->left = 0;
> + crop->top = 0;
> + crop->width = fmt->format.width;
> + crop->height = fmt->format.height;
> + }
>  
>   /* Propagate the format to the source pad. */
>   format = vsp1_entity_get_pad_format(>entity, config,
> @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev 
> *subdev,
>   struct v4l2_mbus_framefmt *format;
>   int ret = 0;
>  
> - /* Cropping is implemented on the sink pad. */
> - if (sel->pad != RWPF_PAD_SINK)
> + /* Cropping is only supported on the RPF and is implemented on the sink
> +  * pad.
> +  */

Please read CodingStyle and run checkpatch before sending stuff upstream.

This violates the CodingStyle: it should be, instead:
/*
 * foo
 * bar
 */

This time, I'll fix it, but next time I might not have enough time, and
need to reject the patch series.

Thanks,
Mauro

Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad

[Niklas Söderlund]

On 2016-09-14 02:16:59 +0300, Laurent Pinchart wrote:
> Cropping on the WPF sink pad restricts the left and top coordinates to
> 0-255. The same result can be obtained by cropping on the RPF without
> any such restriction, this feature isn't useful. Disable it.
> 
> Signed-off-by: Laurent Pinchart 

Acked-by: Niklas Söderlund 

> ---
>  drivers/media/platform/vsp1/vsp1_rwpf.c | 37 
> +
>  drivers/media/platform/vsp1/vsp1_wpf.c  | 18 +++-
>  2 files changed, 26 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c 
> b/drivers/media/platform/vsp1/vsp1_rwpf.c
> index 8cb87e96b78b..a3ace8df7f4d 100644
> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> @@ -66,7 +66,6 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
>   struct vsp1_rwpf *rwpf = to_rwpf(subdev);
>   struct v4l2_subdev_pad_config *config;
>   struct v4l2_mbus_framefmt *format;
> - struct v4l2_rect *crop;
>   int ret = 0;
>  
>   mutex_lock(>entity.lock);
> @@ -103,12 +102,16 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev 
> *subdev,
>  
>   fmt->format = *format;
>  
> - /* Update the sink crop rectangle. */
> - crop = vsp1_rwpf_get_crop(rwpf, config);
> - crop->left = 0;
> - crop->top = 0;
> - crop->width = fmt->format.width;
> - crop->height = fmt->format.height;
> + if (rwpf->entity.type == VSP1_ENTITY_RPF) {
> + struct v4l2_rect *crop;
> +
> + /* Update the sink crop rectangle. */
> + crop = vsp1_rwpf_get_crop(rwpf, config);
> + crop->left = 0;
> + crop->top = 0;
> + crop->width = fmt->format.width;
> + crop->height = fmt->format.height;
> + }
>  
>   /* Propagate the format to the source pad. */
>   format = vsp1_entity_get_pad_format(>entity, config,
> @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev 
> *subdev,
>   struct v4l2_mbus_framefmt *format;
>   int ret = 0;
>  
> - /* Cropping is implemented on the sink pad. */
> - if (sel->pad != RWPF_PAD_SINK)
> + /* Cropping is only supported on the RPF and is implemented on the sink
> +  * pad.
> +  */
> + if (rwpf->entity.type == VSP1_ENTITY_WPF || sel->pad != RWPF_PAD_SINK)
>   return -EINVAL;
>  
>   mutex_lock(>entity.lock);
> @@ -175,8 +180,10 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev 
> *subdev,
>   struct v4l2_rect *crop;
>   int ret = 0;
>  
> - /* Cropping is implemented on the sink pad. */
> - if (sel->pad != RWPF_PAD_SINK)
> + /* Cropping is only supported on the RPF and is implemented on the sink
> +  * pad.
> +  */
> + if (rwpf->entity.type == VSP1_ENTITY_WPF || sel->pad != RWPF_PAD_SINK)
>   return -EINVAL;
>  
>   if (sel->target != V4L2_SEL_TGT_CROP)
> @@ -190,9 +197,7 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev 
> *subdev,
>   goto done;
>   }
>  
> - /* Make sure the crop rectangle is entirely contained in the image. The
> -  * WPF top and left offsets are limited to 255.
> -  */
> + /* Make sure the crop rectangle is entirely contained in the image. */
>   format = vsp1_entity_get_pad_format(>entity, config,
>   RWPF_PAD_SINK);
>  
> @@ -208,10 +213,6 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev 
> *subdev,
>  
>   sel->r.left = min_t(unsigned int, sel->r.left, format->width - 2);
>   sel->r.top = min_t(unsigned int, sel->r.top, format->height - 2);
> - if (rwpf->entity.type == VSP1_ENTITY_WPF) {
> - sel->r.left = min_t(unsigned int, sel->r.left, 255);
> - sel->r.top = min_t(unsigned int, sel->r.top, 255);
> - }
>   sel->r.width = min_t(unsigned int, sel->r.width,
>format->width - sel->r.left);
>   sel->r.height = min_t(unsigned int, sel->r.height,
> diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c 
> b/drivers/media/platform/vsp1/vsp1_wpf.c
> index 748f5af90b7e..f3a593196282 100644
> --- a/drivers/media/platform/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_wpf.c
> @@ -212,7 +212,6 @@ static void wpf_configure(struct vsp1_entity *entity,
>   struct vsp1_device *vsp1 = wpf->entity.vsp1;
>   const struct v4l2_mbus_framefmt *source_format;
>   const struct v4l2_mbus_framefmt *sink_format;
> - const struct v4l2_rect *crop;
>   unsigned int i;
>   u32 outfmt = 0;
>   u32 srcrpf = 0;
> @@ -237,16 +236,6 @@ static void wpf_configure(struct vsp1_entity *entity,
>   return;
>   }
>  
> - /* Cropping */
> - crop = vsp1_rwpf_get_crop(wpf, wpf->entity.config);
> -
> - vsp1_wpf_write(wpf, dl, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
> -