| Message ID | 20231122043009.2741-10-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 2d7e5d80f120b7b075c037df463cca6d6a20fe8a |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: renesas: vsp1: Conversion to subdev active state | expand |
Hi Laurent On Wed, Nov 22, 2023 at 06:29:59AM GMT, Laurent Pinchart wrote: > The entity .configure_partition() function operates on a partition, and > has to retrieve that partition from the pipeline's current partition > field. Pass the partition pointer to the function to make it clearer > what partition it operates on, and remove the vsp1_pipeline.partition > field. > > This change clearly shows that the DRM pipeline doesn't use partitions, > which makes entity implementation more complex and error-prone. This > will be addressed in a further cleanup. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > drivers/media/platform/renesas/vsp1/vsp1_drm.c | 2 +- > drivers/media/platform/renesas/vsp1/vsp1_entity.c | 4 +++- > drivers/media/platform/renesas/vsp1/vsp1_entity.h | 2 ++ > drivers/media/platform/renesas/vsp1/vsp1_pipe.h | 2 -- > drivers/media/platform/renesas/vsp1/vsp1_rpf.c | 5 +++-- > drivers/media/platform/renesas/vsp1/vsp1_uds.c | 2 +- > drivers/media/platform/renesas/vsp1/vsp1_video.c | 5 ++--- > drivers/media/platform/renesas/vsp1/vsp1_wpf.c | 5 +++-- > 8 files changed, 15 insertions(+), 12 deletions(-) > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > index 9b087bd8df7d..3954c138fa7b 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > @@ -569,7 +569,7 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe) > vsp1_entity_route_setup(entity, pipe, dlb); > vsp1_entity_configure_stream(entity, pipe, dl, dlb); > vsp1_entity_configure_frame(entity, pipe, dl, dlb); > - vsp1_entity_configure_partition(entity, pipe, dl, dlb); > + vsp1_entity_configure_partition(entity, pipe, NULL, dl, dlb); Are was passing a NULL pointer to rpf_configure_partition() and wpf_configure_partition() now ? Will this commit break bisection ? Can't you temporary keep 'struct vsp1_pipeline.partition' around and pass it down here ? > } > > vsp1_dl_list_commit(dl, dl_flags); > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c > index 8d39f1ee00ab..e9de75de8bde 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c > @@ -89,11 +89,13 @@ void vsp1_entity_configure_frame(struct vsp1_entity *entity, > > void vsp1_entity_configure_partition(struct vsp1_entity *entity, > struct vsp1_pipeline *pipe, > + const struct vsp1_partition *partition, > struct vsp1_dl_list *dl, > struct vsp1_dl_body *dlb) > { > if (entity->ops->configure_partition) > - entity->ops->configure_partition(entity, pipe, dl, dlb); > + entity->ops->configure_partition(entity, pipe, partition, > + dl, dlb); > } > > /* ----------------------------------------------------------------------------- > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h > index 802c0c2acab0..7b86b2fef3e5 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h > +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h > @@ -85,6 +85,7 @@ struct vsp1_entity_operations { > struct vsp1_dl_list *, struct vsp1_dl_body *); > void (*configure_partition)(struct vsp1_entity *, > struct vsp1_pipeline *, > + const struct vsp1_partition *, > struct vsp1_dl_list *, > struct vsp1_dl_body *); > unsigned int (*max_width)(struct vsp1_entity *, struct vsp1_pipeline *); > @@ -155,6 +156,7 @@ void vsp1_entity_configure_frame(struct vsp1_entity *entity, > > void vsp1_entity_configure_partition(struct vsp1_entity *entity, > struct vsp1_pipeline *pipe, > + const struct vsp1_partition *partition, > struct vsp1_dl_list *dl, > struct vsp1_dl_body *dlb); > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > index 840fd3288efb..3d2e35ac8fa0 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > @@ -106,7 +106,6 @@ struct vsp1_partition { > * @configured: when false the @stream_config shall be written to the hardware > * @interlaced: True when the pipeline is configured in interlaced mode > * @partitions: The number of partitions used to process one frame > - * @partition: The current partition for configuration to process > * @part_table: The pre-calculated partitions used by the pipeline > */ > struct vsp1_pipeline { > @@ -146,7 +145,6 @@ struct vsp1_pipeline { > bool interlaced; > > unsigned int partitions; > - struct vsp1_partition *partition; > struct vsp1_partition *part_table; > > u32 underrun_count; > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c > index 42b0c5655404..3b8a62299226 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c > @@ -280,6 +280,7 @@ static void rpf_configure_frame(struct vsp1_entity *entity, > > static void rpf_configure_partition(struct vsp1_entity *entity, > struct vsp1_pipeline *pipe, > + const struct vsp1_partition *partition, > struct vsp1_dl_list *dl, > struct vsp1_dl_body *dlb) > { > @@ -311,8 +312,8 @@ static void rpf_configure_partition(struct vsp1_entity *entity, > * 'width' need to be adjusted. > */ > if (pipe->partitions > 1) { > - crop.width = pipe->partition->rpf[rpf->entity.index].width; > - crop.left += pipe->partition->rpf[rpf->entity.index].left; > + crop.width = partition->rpf[rpf->entity.index].width; > + crop.left += partition->rpf[rpf->entity.index].left; > } > > if (pipe->interlaced) { > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c > index 887b1f70611a..737362ca2315 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c > @@ -300,11 +300,11 @@ static void uds_configure_stream(struct vsp1_entity *entity, > > static void uds_configure_partition(struct vsp1_entity *entity, > struct vsp1_pipeline *pipe, > + const struct vsp1_partition *partition, > struct vsp1_dl_list *dl, > struct vsp1_dl_body *dlb) > { > struct vsp1_uds *uds = to_uds(&entity->subdev); > - struct vsp1_partition *partition = pipe->partition; > const struct v4l2_mbus_framefmt *output; > > output = v4l2_subdev_state_get_format(uds->entity.state, > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c > index ea5773af54d6..6a8db541543a 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c > @@ -240,13 +240,12 @@ static void vsp1_video_pipeline_run_partition(struct vsp1_pipeline *pipe, > struct vsp1_dl_list *dl, > unsigned int partition) > { > + struct vsp1_partition *part = &pipe->part_table[partition]; > struct vsp1_dl_body *dlb = vsp1_dl_list_get_body0(dl); > struct vsp1_entity *entity; > > - pipe->partition = &pipe->part_table[partition]; > - > list_for_each_entry(entity, &pipe->entities, list_pipe) > - vsp1_entity_configure_partition(entity, pipe, dl, dlb); > + vsp1_entity_configure_partition(entity, pipe, part, dl, dlb); > } > > static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe) > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c > index 5129181b8217..80fe7571f4ff 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c > @@ -363,6 +363,7 @@ static void wpf_configure_frame(struct vsp1_entity *entity, > > static void wpf_configure_partition(struct vsp1_entity *entity, > struct vsp1_pipeline *pipe, > + const struct vsp1_partition *partition, > struct vsp1_dl_list *dl, > struct vsp1_dl_body *dlb) > { > @@ -390,8 +391,8 @@ static void wpf_configure_partition(struct vsp1_entity *entity, > * multiple slices. > */ > if (pipe->partitions > 1) { > - width = pipe->partition->wpf.width; > - left = pipe->partition->wpf.left; > + width = partition->wpf.width; > + left = partition->wpf.left; > } > > vsp1_wpf_write(wpf, dlb, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN | > -- > Regards, > > Laurent Pinchart > >
Hi Jacopo, On Tue, Jun 18, 2024 at 12:45:42PM +0200, Jacopo Mondi wrote: > On Wed, Nov 22, 2023 at 06:29:59AM GMT, Laurent Pinchart wrote: > > The entity .configure_partition() function operates on a partition, and > > has to retrieve that partition from the pipeline's current partition > > field. Pass the partition pointer to the function to make it clearer > > what partition it operates on, and remove the vsp1_pipeline.partition > > field. > > > > This change clearly shows that the DRM pipeline doesn't use partitions, > > which makes entity implementation more complex and error-prone. This > > will be addressed in a further cleanup. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > --- > > drivers/media/platform/renesas/vsp1/vsp1_drm.c | 2 +- > > drivers/media/platform/renesas/vsp1/vsp1_entity.c | 4 +++- > > drivers/media/platform/renesas/vsp1/vsp1_entity.h | 2 ++ > > drivers/media/platform/renesas/vsp1/vsp1_pipe.h | 2 -- > > drivers/media/platform/renesas/vsp1/vsp1_rpf.c | 5 +++-- > > drivers/media/platform/renesas/vsp1/vsp1_uds.c | 2 +- > > drivers/media/platform/renesas/vsp1/vsp1_video.c | 5 ++--- > > drivers/media/platform/renesas/vsp1/vsp1_wpf.c | 5 +++-- > > 8 files changed, 15 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > index 9b087bd8df7d..3954c138fa7b 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > @@ -569,7 +569,7 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe) > > vsp1_entity_route_setup(entity, pipe, dlb); > > vsp1_entity_configure_stream(entity, pipe, dl, dlb); > > vsp1_entity_configure_frame(entity, pipe, dl, dlb); > > - vsp1_entity_configure_partition(entity, pipe, dl, dlb); > > + vsp1_entity_configure_partition(entity, pipe, NULL, dl, dlb); > > Are was passing a NULL pointer to rpf_configure_partition() and > wpf_configure_partition() now ? Will this commit break bisection ? I don't think it will, becase pipe->partition is set in vsp1_video_pipeline_run_partition() only, which is not called for DRM pipelines. Entities already get a NULL pipe->partition in DRM pipelines, so this patch doesn't change the behaviour. > Can't you temporary keep 'struct vsp1_pipeline.partition' around and > pass it down here ? > > > } > > > > vsp1_dl_list_commit(dl, dl_flags); > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c > > index 8d39f1ee00ab..e9de75de8bde 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c > > @@ -89,11 +89,13 @@ void vsp1_entity_configure_frame(struct vsp1_entity *entity, > > > > void vsp1_entity_configure_partition(struct vsp1_entity *entity, > > struct vsp1_pipeline *pipe, > > + const struct vsp1_partition *partition, > > struct vsp1_dl_list *dl, > > struct vsp1_dl_body *dlb) > > { > > if (entity->ops->configure_partition) > > - entity->ops->configure_partition(entity, pipe, dl, dlb); > > + entity->ops->configure_partition(entity, pipe, partition, > > + dl, dlb); > > } > > > > /* ----------------------------------------------------------------------------- > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h > > index 802c0c2acab0..7b86b2fef3e5 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h > > @@ -85,6 +85,7 @@ struct vsp1_entity_operations { > > struct vsp1_dl_list *, struct vsp1_dl_body *); > > void (*configure_partition)(struct vsp1_entity *, > > struct vsp1_pipeline *, > > + const struct vsp1_partition *, > > struct vsp1_dl_list *, > > struct vsp1_dl_body *); > > unsigned int (*max_width)(struct vsp1_entity *, struct vsp1_pipeline *); > > @@ -155,6 +156,7 @@ void vsp1_entity_configure_frame(struct vsp1_entity *entity, > > > > void vsp1_entity_configure_partition(struct vsp1_entity *entity, > > struct vsp1_pipeline *pipe, > > + const struct vsp1_partition *partition, > > struct vsp1_dl_list *dl, > > struct vsp1_dl_body *dlb); > > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > > index 840fd3288efb..3d2e35ac8fa0 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > > @@ -106,7 +106,6 @@ struct vsp1_partition { > > * @configured: when false the @stream_config shall be written to the hardware > > * @interlaced: True when the pipeline is configured in interlaced mode > > * @partitions: The number of partitions used to process one frame > > - * @partition: The current partition for configuration to process > > * @part_table: The pre-calculated partitions used by the pipeline > > */ > > struct vsp1_pipeline { > > @@ -146,7 +145,6 @@ struct vsp1_pipeline { > > bool interlaced; > > > > unsigned int partitions; > > - struct vsp1_partition *partition; > > struct vsp1_partition *part_table; > > > > u32 underrun_count; > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c > > index 42b0c5655404..3b8a62299226 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c > > @@ -280,6 +280,7 @@ static void rpf_configure_frame(struct vsp1_entity *entity, > > > > static void rpf_configure_partition(struct vsp1_entity *entity, > > struct vsp1_pipeline *pipe, > > + const struct vsp1_partition *partition, > > struct vsp1_dl_list *dl, > > struct vsp1_dl_body *dlb) > > { > > @@ -311,8 +312,8 @@ static void rpf_configure_partition(struct vsp1_entity *entity, > > * 'width' need to be adjusted. > > */ > > if (pipe->partitions > 1) { > > - crop.width = pipe->partition->rpf[rpf->entity.index].width; > > - crop.left += pipe->partition->rpf[rpf->entity.index].left; > > + crop.width = partition->rpf[rpf->entity.index].width; > > + crop.left += partition->rpf[rpf->entity.index].left; > > } > > > > if (pipe->interlaced) { > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c > > index 887b1f70611a..737362ca2315 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c > > @@ -300,11 +300,11 @@ static void uds_configure_stream(struct vsp1_entity *entity, > > > > static void uds_configure_partition(struct vsp1_entity *entity, > > struct vsp1_pipeline *pipe, > > + const struct vsp1_partition *partition, > > struct vsp1_dl_list *dl, > > struct vsp1_dl_body *dlb) > > { > > struct vsp1_uds *uds = to_uds(&entity->subdev); > > - struct vsp1_partition *partition = pipe->partition; > > const struct v4l2_mbus_framefmt *output; > > > > output = v4l2_subdev_state_get_format(uds->entity.state, > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c > > index ea5773af54d6..6a8db541543a 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c > > @@ -240,13 +240,12 @@ static void vsp1_video_pipeline_run_partition(struct vsp1_pipeline *pipe, > > struct vsp1_dl_list *dl, > > unsigned int partition) > > { > > + struct vsp1_partition *part = &pipe->part_table[partition]; > > struct vsp1_dl_body *dlb = vsp1_dl_list_get_body0(dl); > > struct vsp1_entity *entity; > > > > - pipe->partition = &pipe->part_table[partition]; > > - > > list_for_each_entry(entity, &pipe->entities, list_pipe) > > - vsp1_entity_configure_partition(entity, pipe, dl, dlb); > > + vsp1_entity_configure_partition(entity, pipe, part, dl, dlb); > > } > > > > static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe) > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c > > index 5129181b8217..80fe7571f4ff 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c > > @@ -363,6 +363,7 @@ static void wpf_configure_frame(struct vsp1_entity *entity, > > > > static void wpf_configure_partition(struct vsp1_entity *entity, > > struct vsp1_pipeline *pipe, > > + const struct vsp1_partition *partition, > > struct vsp1_dl_list *dl, > > struct vsp1_dl_body *dlb) > > { > > @@ -390,8 +391,8 @@ static void wpf_configure_partition(struct vsp1_entity *entity, > > * multiple slices. > > */ > > if (pipe->partitions > 1) { > > - width = pipe->partition->wpf.width; > > - left = pipe->partition->wpf.left; > > + width = partition->wpf.width; > > + left = partition->wpf.left; > > } > > > > vsp1_wpf_write(wpf, dlb, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
Hi Laurent On Tue, Jun 18, 2024 at 07:13:30PM GMT, Laurent Pinchart wrote: > Hi Jacopo, > > On Tue, Jun 18, 2024 at 12:45:42PM +0200, Jacopo Mondi wrote: > > On Wed, Nov 22, 2023 at 06:29:59AM GMT, Laurent Pinchart wrote: > > > The entity .configure_partition() function operates on a partition, and > > > has to retrieve that partition from the pipeline's current partition > > > field. Pass the partition pointer to the function to make it clearer > > > what partition it operates on, and remove the vsp1_pipeline.partition > > > field. > > > > > > This change clearly shows that the DRM pipeline doesn't use partitions, > > > which makes entity implementation more complex and error-prone. This > > > will be addressed in a further cleanup. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > --- > > > drivers/media/platform/renesas/vsp1/vsp1_drm.c | 2 +- > > > drivers/media/platform/renesas/vsp1/vsp1_entity.c | 4 +++- > > > drivers/media/platform/renesas/vsp1/vsp1_entity.h | 2 ++ > > > drivers/media/platform/renesas/vsp1/vsp1_pipe.h | 2 -- > > > drivers/media/platform/renesas/vsp1/vsp1_rpf.c | 5 +++-- > > > drivers/media/platform/renesas/vsp1/vsp1_uds.c | 2 +- > > > drivers/media/platform/renesas/vsp1/vsp1_video.c | 5 ++--- > > > drivers/media/platform/renesas/vsp1/vsp1_wpf.c | 5 +++-- > > > 8 files changed, 15 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > > index 9b087bd8df7d..3954c138fa7b 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > > @@ -569,7 +569,7 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe) > > > vsp1_entity_route_setup(entity, pipe, dlb); > > > vsp1_entity_configure_stream(entity, pipe, dl, dlb); > > > vsp1_entity_configure_frame(entity, pipe, dl, dlb); > > > - vsp1_entity_configure_partition(entity, pipe, dl, dlb); > > > + vsp1_entity_configure_partition(entity, pipe, NULL, dl, dlb); > > > > Are was passing a NULL pointer to rpf_configure_partition() and > > wpf_configure_partition() now ? Will this commit break bisection ? > > I don't think it will, becase pipe->partition is set in > vsp1_video_pipeline_run_partition() only, which is not called for DRM > pipelines. Entities already get a NULL pipe->partition in DRM pipelines, > so this patch doesn't change the behaviour. > Good then, I was not aware of how the DRM pipe uses this Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Can't you temporary keep 'struct vsp1_pipeline.partition' around and > > pass it down here ? > > > > > } > > > > > > vsp1_dl_list_commit(dl, dl_flags); > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c > > > index 8d39f1ee00ab..e9de75de8bde 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c > > > @@ -89,11 +89,13 @@ void vsp1_entity_configure_frame(struct vsp1_entity *entity, > > > > > > void vsp1_entity_configure_partition(struct vsp1_entity *entity, > > > struct vsp1_pipeline *pipe, > > > + const struct vsp1_partition *partition, > > > struct vsp1_dl_list *dl, > > > struct vsp1_dl_body *dlb) > > > { > > > if (entity->ops->configure_partition) > > > - entity->ops->configure_partition(entity, pipe, dl, dlb); > > > + entity->ops->configure_partition(entity, pipe, partition, > > > + dl, dlb); > > > } > > > > > > /* ----------------------------------------------------------------------------- > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h > > > index 802c0c2acab0..7b86b2fef3e5 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h > > > @@ -85,6 +85,7 @@ struct vsp1_entity_operations { > > > struct vsp1_dl_list *, struct vsp1_dl_body *); > > > void (*configure_partition)(struct vsp1_entity *, > > > struct vsp1_pipeline *, > > > + const struct vsp1_partition *, > > > struct vsp1_dl_list *, > > > struct vsp1_dl_body *); > > > unsigned int (*max_width)(struct vsp1_entity *, struct vsp1_pipeline *); > > > @@ -155,6 +156,7 @@ void vsp1_entity_configure_frame(struct vsp1_entity *entity, > > > > > > void vsp1_entity_configure_partition(struct vsp1_entity *entity, > > > struct vsp1_pipeline *pipe, > > > + const struct vsp1_partition *partition, > > > struct vsp1_dl_list *dl, > > > struct vsp1_dl_body *dlb); > > > > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > > > index 840fd3288efb..3d2e35ac8fa0 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > > > @@ -106,7 +106,6 @@ struct vsp1_partition { > > > * @configured: when false the @stream_config shall be written to the hardware > > > * @interlaced: True when the pipeline is configured in interlaced mode > > > * @partitions: The number of partitions used to process one frame > > > - * @partition: The current partition for configuration to process > > > * @part_table: The pre-calculated partitions used by the pipeline > > > */ > > > struct vsp1_pipeline { > > > @@ -146,7 +145,6 @@ struct vsp1_pipeline { > > > bool interlaced; > > > > > > unsigned int partitions; > > > - struct vsp1_partition *partition; > > > struct vsp1_partition *part_table; > > > > > > u32 underrun_count; > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c > > > index 42b0c5655404..3b8a62299226 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c > > > @@ -280,6 +280,7 @@ static void rpf_configure_frame(struct vsp1_entity *entity, > > > > > > static void rpf_configure_partition(struct vsp1_entity *entity, > > > struct vsp1_pipeline *pipe, > > > + const struct vsp1_partition *partition, > > > struct vsp1_dl_list *dl, > > > struct vsp1_dl_body *dlb) > > > { > > > @@ -311,8 +312,8 @@ static void rpf_configure_partition(struct vsp1_entity *entity, > > > * 'width' need to be adjusted. > > > */ > > > if (pipe->partitions > 1) { > > > - crop.width = pipe->partition->rpf[rpf->entity.index].width; > > > - crop.left += pipe->partition->rpf[rpf->entity.index].left; > > > + crop.width = partition->rpf[rpf->entity.index].width; > > > + crop.left += partition->rpf[rpf->entity.index].left; > > > } > > > > > > if (pipe->interlaced) { > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c > > > index 887b1f70611a..737362ca2315 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c > > > @@ -300,11 +300,11 @@ static void uds_configure_stream(struct vsp1_entity *entity, > > > > > > static void uds_configure_partition(struct vsp1_entity *entity, > > > struct vsp1_pipeline *pipe, > > > + const struct vsp1_partition *partition, > > > struct vsp1_dl_list *dl, > > > struct vsp1_dl_body *dlb) > > > { > > > struct vsp1_uds *uds = to_uds(&entity->subdev); > > > - struct vsp1_partition *partition = pipe->partition; > > > const struct v4l2_mbus_framefmt *output; > > > > > > output = v4l2_subdev_state_get_format(uds->entity.state, > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c > > > index ea5773af54d6..6a8db541543a 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c > > > @@ -240,13 +240,12 @@ static void vsp1_video_pipeline_run_partition(struct vsp1_pipeline *pipe, > > > struct vsp1_dl_list *dl, > > > unsigned int partition) > > > { > > > + struct vsp1_partition *part = &pipe->part_table[partition]; > > > struct vsp1_dl_body *dlb = vsp1_dl_list_get_body0(dl); > > > struct vsp1_entity *entity; > > > > > > - pipe->partition = &pipe->part_table[partition]; > > > - > > > list_for_each_entry(entity, &pipe->entities, list_pipe) > > > - vsp1_entity_configure_partition(entity, pipe, dl, dlb); > > > + vsp1_entity_configure_partition(entity, pipe, part, dl, dlb); > > > } > > > > > > static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe) > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c > > > index 5129181b8217..80fe7571f4ff 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c > > > @@ -363,6 +363,7 @@ static void wpf_configure_frame(struct vsp1_entity *entity, > > > > > > static void wpf_configure_partition(struct vsp1_entity *entity, > > > struct vsp1_pipeline *pipe, > > > + const struct vsp1_partition *partition, > > > struct vsp1_dl_list *dl, > > > struct vsp1_dl_body *dlb) > > > { > > > @@ -390,8 +391,8 @@ static void wpf_configure_partition(struct vsp1_entity *entity, > > > * multiple slices. > > > */ > > > if (pipe->partitions > 1) { > > > - width = pipe->partition->wpf.width; > > > - left = pipe->partition->wpf.left; > > > + width = partition->wpf.width; > > > + left = partition->wpf.left; > > > } > > > > > > vsp1_wpf_write(wpf, dlb, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN | > > -- > Regards, > > Laurent Pinchart >
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c index 9b087bd8df7d..3954c138fa7b 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c @@ -569,7 +569,7 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe) vsp1_entity_route_setup(entity, pipe, dlb); vsp1_entity_configure_stream(entity, pipe, dl, dlb); vsp1_entity_configure_frame(entity, pipe, dl, dlb); - vsp1_entity_configure_partition(entity, pipe, dl, dlb); + vsp1_entity_configure_partition(entity, pipe, NULL, dl, dlb); } vsp1_dl_list_commit(dl, dl_flags); diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c index 8d39f1ee00ab..e9de75de8bde 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c @@ -89,11 +89,13 @@ void vsp1_entity_configure_frame(struct vsp1_entity *entity, void vsp1_entity_configure_partition(struct vsp1_entity *entity, struct vsp1_pipeline *pipe, + const struct vsp1_partition *partition, struct vsp1_dl_list *dl, struct vsp1_dl_body *dlb) { if (entity->ops->configure_partition) - entity->ops->configure_partition(entity, pipe, dl, dlb); + entity->ops->configure_partition(entity, pipe, partition, + dl, dlb); } /* ----------------------------------------------------------------------------- diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h index 802c0c2acab0..7b86b2fef3e5 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h @@ -85,6 +85,7 @@ struct vsp1_entity_operations { struct vsp1_dl_list *, struct vsp1_dl_body *); void (*configure_partition)(struct vsp1_entity *, struct vsp1_pipeline *, + const struct vsp1_partition *, struct vsp1_dl_list *, struct vsp1_dl_body *); unsigned int (*max_width)(struct vsp1_entity *, struct vsp1_pipeline *); @@ -155,6 +156,7 @@ void vsp1_entity_configure_frame(struct vsp1_entity *entity, void vsp1_entity_configure_partition(struct vsp1_entity *entity, struct vsp1_pipeline *pipe, + const struct vsp1_partition *partition, struct vsp1_dl_list *dl, struct vsp1_dl_body *dlb); diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h index 840fd3288efb..3d2e35ac8fa0 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h @@ -106,7 +106,6 @@ struct vsp1_partition { * @configured: when false the @stream_config shall be written to the hardware * @interlaced: True when the pipeline is configured in interlaced mode * @partitions: The number of partitions used to process one frame - * @partition: The current partition for configuration to process * @part_table: The pre-calculated partitions used by the pipeline */ struct vsp1_pipeline { @@ -146,7 +145,6 @@ struct vsp1_pipeline { bool interlaced; unsigned int partitions; - struct vsp1_partition *partition; struct vsp1_partition *part_table; u32 underrun_count; diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c index 42b0c5655404..3b8a62299226 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c @@ -280,6 +280,7 @@ static void rpf_configure_frame(struct vsp1_entity *entity, static void rpf_configure_partition(struct vsp1_entity *entity, struct vsp1_pipeline *pipe, + const struct vsp1_partition *partition, struct vsp1_dl_list *dl, struct vsp1_dl_body *dlb) { @@ -311,8 +312,8 @@ static void rpf_configure_partition(struct vsp1_entity *entity, * 'width' need to be adjusted. */ if (pipe->partitions > 1) { - crop.width = pipe->partition->rpf[rpf->entity.index].width; - crop.left += pipe->partition->rpf[rpf->entity.index].left; + crop.width = partition->rpf[rpf->entity.index].width; + crop.left += partition->rpf[rpf->entity.index].left; } if (pipe->interlaced) { diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c index 887b1f70611a..737362ca2315 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c @@ -300,11 +300,11 @@ static void uds_configure_stream(struct vsp1_entity *entity, static void uds_configure_partition(struct vsp1_entity *entity, struct vsp1_pipeline *pipe, + const struct vsp1_partition *partition, struct vsp1_dl_list *dl, struct vsp1_dl_body *dlb) { struct vsp1_uds *uds = to_uds(&entity->subdev); - struct vsp1_partition *partition = pipe->partition; const struct v4l2_mbus_framefmt *output; output = v4l2_subdev_state_get_format(uds->entity.state, diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c index ea5773af54d6..6a8db541543a 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c @@ -240,13 +240,12 @@ static void vsp1_video_pipeline_run_partition(struct vsp1_pipeline *pipe, struct vsp1_dl_list *dl, unsigned int partition) { + struct vsp1_partition *part = &pipe->part_table[partition]; struct vsp1_dl_body *dlb = vsp1_dl_list_get_body0(dl); struct vsp1_entity *entity; - pipe->partition = &pipe->part_table[partition]; - list_for_each_entry(entity, &pipe->entities, list_pipe) - vsp1_entity_configure_partition(entity, pipe, dl, dlb); + vsp1_entity_configure_partition(entity, pipe, part, dl, dlb); } static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe) diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c index 5129181b8217..80fe7571f4ff 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c @@ -363,6 +363,7 @@ static void wpf_configure_frame(struct vsp1_entity *entity, static void wpf_configure_partition(struct vsp1_entity *entity, struct vsp1_pipeline *pipe, + const struct vsp1_partition *partition, struct vsp1_dl_list *dl, struct vsp1_dl_body *dlb) { @@ -390,8 +391,8 @@ static void wpf_configure_partition(struct vsp1_entity *entity, * multiple slices. */ if (pipe->partitions > 1) { - width = pipe->partition->wpf.width; - left = pipe->partition->wpf.left; + width = partition->wpf.width; + left = partition->wpf.left; } vsp1_wpf_write(wpf, dlb, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
The entity .configure_partition() function operates on a partition, and has to retrieve that partition from the pipeline's current partition field. Pass the partition pointer to the function to make it clearer what partition it operates on, and remove the vsp1_pipeline.partition field. This change clearly shows that the DRM pipeline doesn't use partitions, which makes entity implementation more complex and error-prone. This will be addressed in a further cleanup. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/media/platform/renesas/vsp1/vsp1_drm.c | 2 +- drivers/media/platform/renesas/vsp1/vsp1_entity.c | 4 +++- drivers/media/platform/renesas/vsp1/vsp1_entity.h | 2 ++ drivers/media/platform/renesas/vsp1/vsp1_pipe.h | 2 -- drivers/media/platform/renesas/vsp1/vsp1_rpf.c | 5 +++-- drivers/media/platform/renesas/vsp1/vsp1_uds.c | 2 +- drivers/media/platform/renesas/vsp1/vsp1_video.c | 5 ++--- drivers/media/platform/renesas/vsp1/vsp1_wpf.c | 5 +++-- 8 files changed, 15 insertions(+), 12 deletions(-)