diff mbox series

[RFC,v1,14/19] media: renesas: vsp1: Get configuration from partition instead of state

Message ID 20231122043009.2741-15-laurent.pinchart+renesas@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series media: renesas: vsp1: Conversion to subdev active state | expand

Commit Message

Laurent Pinchart Nov. 22, 2023, 4:30 a.m. UTC
Entities access various piece of information from the entity state when
configuring a partition. The same data is available through the
partition structure passed to the .configure_partition() operation. Use
it to avoid accessing the state, which will simplify moving to the V4L2
subdev active state API.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../media/platform/renesas/vsp1/vsp1_rpf.c    | 35 +++++++++----------
 .../media/platform/renesas/vsp1/vsp1_uds.c    |  6 +---
 .../media/platform/renesas/vsp1/vsp1_wpf.c    | 18 +++-------
 3 files changed, 23 insertions(+), 36 deletions(-)

Comments

Jacopo Mondi June 18, 2024, 11:23 a.m. UTC | #1
Hi Laurent

On Wed, Nov 22, 2023 at 06:30:04AM GMT, Laurent Pinchart wrote:
> Entities access various piece of information from the entity state when

s/entity state/subdev state/

> configuring a partition. The same data is available through the
> partition structure passed to the .configure_partition() operation. Use
> it to avoid accessing the state, which will simplify moving to the V4L2
> subdev active state API.

I would move this after 10/19 and possibily considering squashing the
two. The overall diff of the 2 patches combined is pretty understandable.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../media/platform/renesas/vsp1/vsp1_rpf.c    | 35 +++++++++----------
>  .../media/platform/renesas/vsp1/vsp1_uds.c    |  6 +---
>  .../media/platform/renesas/vsp1/vsp1_wpf.c    | 18 +++-------
>  3 files changed, 23 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> index 862751616646..b4558670b46f 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> @@ -289,7 +289,7 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
>  	struct vsp1_device *vsp1 = rpf->entity.vsp1;
>  	const struct vsp1_format_info *fmtinfo = rpf->fmtinfo;
>  	const struct v4l2_pix_format_mplane *format = &rpf->format;
> -	struct v4l2_rect crop;
> +	struct v4l2_rect crop = partition->rpf[rpf->entity.index];
>
>  	/*
>  	 * Source size and crop offsets.
> @@ -299,22 +299,6 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
>  	 * offsets are needed, as planes 2 and 3 always have identical
>  	 * strides.
>  	 */
> -	crop = *v4l2_subdev_state_get_crop(rpf->entity.state, RWPF_PAD_SINK);
> -
> -	/*
> -	 * Partition Algorithm Control
> -	 *
> -	 * The partition algorithm can split this frame into multiple
> -	 * slices. We must scale our partition window based on the pipe
> -	 * configuration to match the destination partition window.
> -	 * To achieve this, we adjust our crop to provide a 'sub-crop'
> -	 * matching the expected partition window. Only 'left' and
> -	 * 'width' need to be adjusted.
> -	 */
> -	if (pipe->partitions > 1) {
> -		crop.width = partition->rpf[rpf->entity.index].width;
> -		crop.left += partition->rpf[rpf->entity.index].left;
> -	}
>
>  	if (pipe->interlaced) {
>  		crop.height = round_down(crop.height / 2, fmtinfo->vsub);
> @@ -369,8 +353,23 @@ static void rpf_partition(struct vsp1_entity *entity,
>  			  struct v4l2_rect *window)
>  {
>  	struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
> +	struct v4l2_rect *rpf_rect = &partition->rpf[rpf->entity.index];
>
> -	partition->rpf[rpf->entity.index] = *window;
> +	/*
> +	 * Partition Algorithm Control
> +	 *
> +	 * The partition algorithm can split this frame into multiple slices. We
> +	 * must adjust our partition window based on the pipe configuration to
> +	 * match the destination partition window. To achieve this, we adjust
> +	 * our crop to provide a 'sub-crop' matching the expected partition
> +	 * window.
> +	 */
> +	*rpf_rect = *v4l2_subdev_state_get_crop(entity->state, RWPF_PAD_SINK);
> +
> +	if (pipe->partitions > 1) {
> +		rpf_rect->width = window->width;
> +		rpf_rect->left += window->left;
> +	}
>  }
>
>  static const struct vsp1_entity_operations rpf_entity_ops = {
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> index 4a14fd3baac1..e5953d86c17c 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> @@ -305,10 +305,6 @@ static void uds_configure_partition(struct vsp1_entity *entity,
>  				    struct vsp1_dl_body *dlb)
>  {
>  	struct vsp1_uds *uds = to_uds(&entity->subdev);
> -	const struct v4l2_mbus_framefmt *output;
> -
> -	output = v4l2_subdev_state_get_format(uds->entity.state,
> -					      UDS_PAD_SOURCE);
>
>  	/* Input size clipping. */
>  	vsp1_uds_write(uds, dlb, VI6_UDS_HSZCLIP, VI6_UDS_HSZCLIP_HCEN |
> @@ -320,7 +316,7 @@ static void uds_configure_partition(struct vsp1_entity *entity,
>  	vsp1_uds_write(uds, dlb, VI6_UDS_CLIP_SIZE,
>  		       (partition->uds_source.width
>  				<< VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) |
> -		       (output->height
> +		       (partition->uds_source.height

As I read this 'output' used to be the current format on the sink pad
and we use the height from there.

Now we use 'patrition->uds_source.height' which I read being
initialized in uds_partition() to:

	partition->uds_source = *window;

With 'window' being the parameter passed to uds_partition().

Is this correct ?

>  				<< VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
>  }
>
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> index f8d1e2f47691..5c363ff1d36c 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> @@ -370,7 +370,6 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
>  	struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
>  	struct vsp1_device *vsp1 = wpf->entity.vsp1;
>  	struct vsp1_rwpf_memory mem = wpf->mem;
> -	const struct v4l2_mbus_framefmt *sink_format;
>  	const struct v4l2_pix_format_mplane *format = &wpf->format;
>  	const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
>  	unsigned int width;
> @@ -380,20 +379,13 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
>  	unsigned int flip;
>  	unsigned int i;
>
> -	sink_format = v4l2_subdev_state_get_format(wpf->entity.state,
> -						   RWPF_PAD_SINK);
> -	width = sink_format->width;
> -	height = sink_format->height;
> -	left = 0;
> -
>  	/*
> -	 * Cropping. The partition algorithm can split the image into
> -	 * multiple slices.
> +	 * Cropping. The partition algorithm can split the image into multiple
> +	 * slices.
>  	 */
> -	if (pipe->partitions > 1) {
> -		width = partition->wpf.width;
> -		left = partition->wpf.left;
> -	}
> +	width = partition->wpf.width;
> +	left = partition->wpf.left;
> +	height = partition->wpf.height;

Same question as per the uds module

Thanks
  j

>
>  	vsp1_wpf_write(wpf, dlb, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
>  		       (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
> --
> Regards,
>
> Laurent Pinchart
>
>
Laurent Pinchart June 18, 2024, 4:38 p.m. UTC | #2
Hi Jacopo,

On Tue, Jun 18, 2024 at 01:23:33PM +0200, Jacopo Mondi wrote:
> On Wed, Nov 22, 2023 at 06:30:04AM GMT, Laurent Pinchart wrote:
> > Entities access various piece of information from the entity state when
> 
> s/entity state/subdev state/
> 
> > configuring a partition. The same data is available through the
> > partition structure passed to the .configure_partition() operation. Use
> > it to avoid accessing the state, which will simplify moving to the V4L2
> > subdev active state API.
> 
> I would move this after 10/19 and possibily considering squashing the
> two. The overall diff of the 2 patches combined is pretty understandable.

I can't do that, as this patch depends on 13/19 to calculatate
partitions for the DRM pipeline, otherwise the

	struct v4l2_rect crop = partition->rpf[rpf->entity.index];

line in rpf_configure_partition() will dereference a NULL pointer.

> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  .../media/platform/renesas/vsp1/vsp1_rpf.c    | 35 +++++++++----------
> >  .../media/platform/renesas/vsp1/vsp1_uds.c    |  6 +---
> >  .../media/platform/renesas/vsp1/vsp1_wpf.c    | 18 +++-------
> >  3 files changed, 23 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> > index 862751616646..b4558670b46f 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> > @@ -289,7 +289,7 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
> >  	struct vsp1_device *vsp1 = rpf->entity.vsp1;
> >  	const struct vsp1_format_info *fmtinfo = rpf->fmtinfo;
> >  	const struct v4l2_pix_format_mplane *format = &rpf->format;
> > -	struct v4l2_rect crop;
> > +	struct v4l2_rect crop = partition->rpf[rpf->entity.index];
> >
> >  	/*
> >  	 * Source size and crop offsets.
> > @@ -299,22 +299,6 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
> >  	 * offsets are needed, as planes 2 and 3 always have identical
> >  	 * strides.
> >  	 */
> > -	crop = *v4l2_subdev_state_get_crop(rpf->entity.state, RWPF_PAD_SINK);
> > -
> > -	/*
> > -	 * Partition Algorithm Control
> > -	 *
> > -	 * The partition algorithm can split this frame into multiple
> > -	 * slices. We must scale our partition window based on the pipe
> > -	 * configuration to match the destination partition window.
> > -	 * To achieve this, we adjust our crop to provide a 'sub-crop'
> > -	 * matching the expected partition window. Only 'left' and
> > -	 * 'width' need to be adjusted.
> > -	 */
> > -	if (pipe->partitions > 1) {
> > -		crop.width = partition->rpf[rpf->entity.index].width;
> > -		crop.left += partition->rpf[rpf->entity.index].left;
> > -	}
> >
> >  	if (pipe->interlaced) {
> >  		crop.height = round_down(crop.height / 2, fmtinfo->vsub);
> > @@ -369,8 +353,23 @@ static void rpf_partition(struct vsp1_entity *entity,
> >  			  struct v4l2_rect *window)
> >  {
> >  	struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
> > +	struct v4l2_rect *rpf_rect = &partition->rpf[rpf->entity.index];
> >
> > -	partition->rpf[rpf->entity.index] = *window;
> > +	/*
> > +	 * Partition Algorithm Control
> > +	 *
> > +	 * The partition algorithm can split this frame into multiple slices. We
> > +	 * must adjust our partition window based on the pipe configuration to
> > +	 * match the destination partition window. To achieve this, we adjust
> > +	 * our crop to provide a 'sub-crop' matching the expected partition
> > +	 * window.
> > +	 */
> > +	*rpf_rect = *v4l2_subdev_state_get_crop(entity->state, RWPF_PAD_SINK);
> > +
> > +	if (pipe->partitions > 1) {
> > +		rpf_rect->width = window->width;
> > +		rpf_rect->left += window->left;
> > +	}
> >  }
> >
> >  static const struct vsp1_entity_operations rpf_entity_ops = {
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> > index 4a14fd3baac1..e5953d86c17c 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> > @@ -305,10 +305,6 @@ static void uds_configure_partition(struct vsp1_entity *entity,
> >  				    struct vsp1_dl_body *dlb)
> >  {
> >  	struct vsp1_uds *uds = to_uds(&entity->subdev);
> > -	const struct v4l2_mbus_framefmt *output;
> > -
> > -	output = v4l2_subdev_state_get_format(uds->entity.state,
> > -					      UDS_PAD_SOURCE);
> >
> >  	/* Input size clipping. */
> >  	vsp1_uds_write(uds, dlb, VI6_UDS_HSZCLIP, VI6_UDS_HSZCLIP_HCEN |
> > @@ -320,7 +316,7 @@ static void uds_configure_partition(struct vsp1_entity *entity,
> >  	vsp1_uds_write(uds, dlb, VI6_UDS_CLIP_SIZE,
> >  		       (partition->uds_source.width
> >  				<< VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) |
> > -		       (output->height
> > +		       (partition->uds_source.height
> 
> As I read this 'output' used to be the current format on the sink pad

On the source pad.

> and we use the height from there.
> 
> Now we use 'patrition->uds_source.height' which I read being
> initialized in uds_partition() to:
> 
> 	partition->uds_source = *window;
> 
> With 'window' being the parameter passed to uds_partition().

And that's the window on the UDS output (source), as windows are
propagated from source to sink (see
vsp1_pipeline_propagate_partition()).

As partitions span the whole height of the image, output->height should
be equal to partition->uds_source.height as far as I can tell.

> Is this correct ?
> 
> >  				<< VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
> >  }
> >
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> > index f8d1e2f47691..5c363ff1d36c 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> > @@ -370,7 +370,6 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
> >  	struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
> >  	struct vsp1_device *vsp1 = wpf->entity.vsp1;
> >  	struct vsp1_rwpf_memory mem = wpf->mem;
> > -	const struct v4l2_mbus_framefmt *sink_format;
> >  	const struct v4l2_pix_format_mplane *format = &wpf->format;
> >  	const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
> >  	unsigned int width;
> > @@ -380,20 +379,13 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
> >  	unsigned int flip;
> >  	unsigned int i;
> >
> > -	sink_format = v4l2_subdev_state_get_format(wpf->entity.state,
> > -						   RWPF_PAD_SINK);
> > -	width = sink_format->width;
> > -	height = sink_format->height;
> > -	left = 0;
> > -
> >  	/*
> > -	 * Cropping. The partition algorithm can split the image into
> > -	 * multiple slices.
> > +	 * Cropping. The partition algorithm can split the image into multiple
> > +	 * slices.
> >  	 */
> > -	if (pipe->partitions > 1) {
> > -		width = partition->wpf.width;
> > -		left = partition->wpf.left;
> > -	}
> > +	width = partition->wpf.width;
> > +	left = partition->wpf.left;
> > +	height = partition->wpf.height;
> 
> Same question as per the uds module

Same answer :-)

> >
> >  	vsp1_wpf_write(wpf, dlb, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
> >  		       (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
Jacopo Mondi June 18, 2024, 4:46 p.m. UTC | #3
Hi Laurent

On Tue, Jun 18, 2024 at 07:38:31PM GMT, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Tue, Jun 18, 2024 at 01:23:33PM +0200, Jacopo Mondi wrote:
> > On Wed, Nov 22, 2023 at 06:30:04AM GMT, Laurent Pinchart wrote:
> > > Entities access various piece of information from the entity state when
> >
> > s/entity state/subdev state/
> >
> > > configuring a partition. The same data is available through the
> > > partition structure passed to the .configure_partition() operation. Use
> > > it to avoid accessing the state, which will simplify moving to the V4L2
> > > subdev active state API.
> >
> > I would move this after 10/19 and possibily considering squashing the
> > two. The overall diff of the 2 patches combined is pretty understandable.
>
> I can't do that, as this patch depends on 13/19 to calculatate
> partitions for the DRM pipeline, otherwise the
>
> 	struct v4l2_rect crop = partition->rpf[rpf->entity.index];
>
> line in rpf_configure_partition() will dereference a NULL pointer.
>

Ah, ok then, I only compile tested the re-ordering

> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > >  .../media/platform/renesas/vsp1/vsp1_rpf.c    | 35 +++++++++----------
> > >  .../media/platform/renesas/vsp1/vsp1_uds.c    |  6 +---
> > >  .../media/platform/renesas/vsp1/vsp1_wpf.c    | 18 +++-------
> > >  3 files changed, 23 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> > > index 862751616646..b4558670b46f 100644
> > > --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> > > @@ -289,7 +289,7 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
> > >  	struct vsp1_device *vsp1 = rpf->entity.vsp1;
> > >  	const struct vsp1_format_info *fmtinfo = rpf->fmtinfo;
> > >  	const struct v4l2_pix_format_mplane *format = &rpf->format;
> > > -	struct v4l2_rect crop;
> > > +	struct v4l2_rect crop = partition->rpf[rpf->entity.index];
> > >
> > >  	/*
> > >  	 * Source size and crop offsets.
> > > @@ -299,22 +299,6 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
> > >  	 * offsets are needed, as planes 2 and 3 always have identical
> > >  	 * strides.
> > >  	 */
> > > -	crop = *v4l2_subdev_state_get_crop(rpf->entity.state, RWPF_PAD_SINK);
> > > -
> > > -	/*
> > > -	 * Partition Algorithm Control
> > > -	 *
> > > -	 * The partition algorithm can split this frame into multiple
> > > -	 * slices. We must scale our partition window based on the pipe
> > > -	 * configuration to match the destination partition window.
> > > -	 * To achieve this, we adjust our crop to provide a 'sub-crop'
> > > -	 * matching the expected partition window. Only 'left' and
> > > -	 * 'width' need to be adjusted.
> > > -	 */
> > > -	if (pipe->partitions > 1) {
> > > -		crop.width = partition->rpf[rpf->entity.index].width;
> > > -		crop.left += partition->rpf[rpf->entity.index].left;
> > > -	}
> > >
> > >  	if (pipe->interlaced) {
> > >  		crop.height = round_down(crop.height / 2, fmtinfo->vsub);
> > > @@ -369,8 +353,23 @@ static void rpf_partition(struct vsp1_entity *entity,
> > >  			  struct v4l2_rect *window)
> > >  {
> > >  	struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
> > > +	struct v4l2_rect *rpf_rect = &partition->rpf[rpf->entity.index];
> > >
> > > -	partition->rpf[rpf->entity.index] = *window;
> > > +	/*
> > > +	 * Partition Algorithm Control
> > > +	 *
> > > +	 * The partition algorithm can split this frame into multiple slices. We
> > > +	 * must adjust our partition window based on the pipe configuration to
> > > +	 * match the destination partition window. To achieve this, we adjust
> > > +	 * our crop to provide a 'sub-crop' matching the expected partition
> > > +	 * window.
> > > +	 */
> > > +	*rpf_rect = *v4l2_subdev_state_get_crop(entity->state, RWPF_PAD_SINK);
> > > +
> > > +	if (pipe->partitions > 1) {
> > > +		rpf_rect->width = window->width;
> > > +		rpf_rect->left += window->left;
> > > +	}
> > >  }
> > >
> > >  static const struct vsp1_entity_operations rpf_entity_ops = {
> > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> > > index 4a14fd3baac1..e5953d86c17c 100644
> > > --- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> > > @@ -305,10 +305,6 @@ static void uds_configure_partition(struct vsp1_entity *entity,
> > >  				    struct vsp1_dl_body *dlb)
> > >  {
> > >  	struct vsp1_uds *uds = to_uds(&entity->subdev);
> > > -	const struct v4l2_mbus_framefmt *output;
> > > -
> > > -	output = v4l2_subdev_state_get_format(uds->entity.state,
> > > -					      UDS_PAD_SOURCE);
> > >
> > >  	/* Input size clipping. */
> > >  	vsp1_uds_write(uds, dlb, VI6_UDS_HSZCLIP, VI6_UDS_HSZCLIP_HCEN |
> > > @@ -320,7 +316,7 @@ static void uds_configure_partition(struct vsp1_entity *entity,
> > >  	vsp1_uds_write(uds, dlb, VI6_UDS_CLIP_SIZE,
> > >  		       (partition->uds_source.width
> > >  				<< VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) |
> > > -		       (output->height
> > > +		       (partition->uds_source.height
> >
> > As I read this 'output' used to be the current format on the sink pad
>
> On the source pad.
>
> > and we use the height from there.
> >
> > Now we use 'patrition->uds_source.height' which I read being
> > initialized in uds_partition() to:
> >
> > 	partition->uds_source = *window;
> >
> > With 'window' being the parameter passed to uds_partition().
>
> And that's the window on the UDS output (source), as windows are
> propagated from source to sink (see
> vsp1_pipeline_propagate_partition()).
>
> As partitions span the whole height of the image, output->height should
> be equal to partition->uds_source.height as far as I can tell.
>

Thanks for confirming
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> > Is this correct ?
> >
> > >  				<< VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
> > >  }
> > >
> > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> > > index f8d1e2f47691..5c363ff1d36c 100644
> > > --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> > > @@ -370,7 +370,6 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
> > >  	struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
> > >  	struct vsp1_device *vsp1 = wpf->entity.vsp1;
> > >  	struct vsp1_rwpf_memory mem = wpf->mem;
> > > -	const struct v4l2_mbus_framefmt *sink_format;
> > >  	const struct v4l2_pix_format_mplane *format = &wpf->format;
> > >  	const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
> > >  	unsigned int width;
> > > @@ -380,20 +379,13 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
> > >  	unsigned int flip;
> > >  	unsigned int i;
> > >
> > > -	sink_format = v4l2_subdev_state_get_format(wpf->entity.state,
> > > -						   RWPF_PAD_SINK);
> > > -	width = sink_format->width;
> > > -	height = sink_format->height;
> > > -	left = 0;
> > > -
> > >  	/*
> > > -	 * Cropping. The partition algorithm can split the image into
> > > -	 * multiple slices.
> > > +	 * Cropping. The partition algorithm can split the image into multiple
> > > +	 * slices.
> > >  	 */
> > > -	if (pipe->partitions > 1) {
> > > -		width = partition->wpf.width;
> > > -		left = partition->wpf.left;
> > > -	}
> > > +	width = partition->wpf.width;
> > > +	left = partition->wpf.left;
> > > +	height = partition->wpf.height;
> >
> > Same question as per the uds module
>
> Same answer :-)
>
> > >
> > >  	vsp1_wpf_write(wpf, dlb, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
> > >  		       (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
>
> --
> Regards,
>
> Laurent Pinchart
>
diff mbox series

Patch

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index 862751616646..b4558670b46f 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -289,7 +289,7 @@  static void rpf_configure_partition(struct vsp1_entity *entity,
 	struct vsp1_device *vsp1 = rpf->entity.vsp1;
 	const struct vsp1_format_info *fmtinfo = rpf->fmtinfo;
 	const struct v4l2_pix_format_mplane *format = &rpf->format;
-	struct v4l2_rect crop;
+	struct v4l2_rect crop = partition->rpf[rpf->entity.index];
 
 	/*
 	 * Source size and crop offsets.
@@ -299,22 +299,6 @@  static void rpf_configure_partition(struct vsp1_entity *entity,
 	 * offsets are needed, as planes 2 and 3 always have identical
 	 * strides.
 	 */
-	crop = *v4l2_subdev_state_get_crop(rpf->entity.state, RWPF_PAD_SINK);
-
-	/*
-	 * Partition Algorithm Control
-	 *
-	 * The partition algorithm can split this frame into multiple
-	 * slices. We must scale our partition window based on the pipe
-	 * configuration to match the destination partition window.
-	 * To achieve this, we adjust our crop to provide a 'sub-crop'
-	 * matching the expected partition window. Only 'left' and
-	 * 'width' need to be adjusted.
-	 */
-	if (pipe->partitions > 1) {
-		crop.width = partition->rpf[rpf->entity.index].width;
-		crop.left += partition->rpf[rpf->entity.index].left;
-	}
 
 	if (pipe->interlaced) {
 		crop.height = round_down(crop.height / 2, fmtinfo->vsub);
@@ -369,8 +353,23 @@  static void rpf_partition(struct vsp1_entity *entity,
 			  struct v4l2_rect *window)
 {
 	struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
+	struct v4l2_rect *rpf_rect = &partition->rpf[rpf->entity.index];
 
-	partition->rpf[rpf->entity.index] = *window;
+	/*
+	 * Partition Algorithm Control
+	 *
+	 * The partition algorithm can split this frame into multiple slices. We
+	 * must adjust our partition window based on the pipe configuration to
+	 * match the destination partition window. To achieve this, we adjust
+	 * our crop to provide a 'sub-crop' matching the expected partition
+	 * window.
+	 */
+	*rpf_rect = *v4l2_subdev_state_get_crop(entity->state, RWPF_PAD_SINK);
+
+	if (pipe->partitions > 1) {
+		rpf_rect->width = window->width;
+		rpf_rect->left += window->left;
+	}
 }
 
 static const struct vsp1_entity_operations rpf_entity_ops = {
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
index 4a14fd3baac1..e5953d86c17c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -305,10 +305,6 @@  static void uds_configure_partition(struct vsp1_entity *entity,
 				    struct vsp1_dl_body *dlb)
 {
 	struct vsp1_uds *uds = to_uds(&entity->subdev);
-	const struct v4l2_mbus_framefmt *output;
-
-	output = v4l2_subdev_state_get_format(uds->entity.state,
-					      UDS_PAD_SOURCE);
 
 	/* Input size clipping. */
 	vsp1_uds_write(uds, dlb, VI6_UDS_HSZCLIP, VI6_UDS_HSZCLIP_HCEN |
@@ -320,7 +316,7 @@  static void uds_configure_partition(struct vsp1_entity *entity,
 	vsp1_uds_write(uds, dlb, VI6_UDS_CLIP_SIZE,
 		       (partition->uds_source.width
 				<< VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) |
-		       (output->height
+		       (partition->uds_source.height
 				<< VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
 }
 
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
index f8d1e2f47691..5c363ff1d36c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
@@ -370,7 +370,6 @@  static void wpf_configure_partition(struct vsp1_entity *entity,
 	struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
 	struct vsp1_device *vsp1 = wpf->entity.vsp1;
 	struct vsp1_rwpf_memory mem = wpf->mem;
-	const struct v4l2_mbus_framefmt *sink_format;
 	const struct v4l2_pix_format_mplane *format = &wpf->format;
 	const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
 	unsigned int width;
@@ -380,20 +379,13 @@  static void wpf_configure_partition(struct vsp1_entity *entity,
 	unsigned int flip;
 	unsigned int i;
 
-	sink_format = v4l2_subdev_state_get_format(wpf->entity.state,
-						   RWPF_PAD_SINK);
-	width = sink_format->width;
-	height = sink_format->height;
-	left = 0;
-
 	/*
-	 * Cropping. The partition algorithm can split the image into
-	 * multiple slices.
+	 * Cropping. The partition algorithm can split the image into multiple
+	 * slices.
 	 */
-	if (pipe->partitions > 1) {
-		width = partition->wpf.width;
-		left = partition->wpf.left;
-	}
+	width = partition->wpf.width;
+	left = partition->wpf.left;
+	height = partition->wpf.height;
 
 	vsp1_wpf_write(wpf, dlb, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
 		       (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |