Hi Laurent On Wed, Nov 22, 2023 at 06:30:00AM GMT, Laurent Pinchart wrote: > The vsp1_partition_window structure is used to store the horizontal size > of a partition window. This is all that is currently needed, as all > partitions span the whole image vertically. The horizontal window size > is retrieved in the .configure_partition() handler from the > vsp1_partition_window structure, and the vertical window size from the > subdev state. > > Accessing the subdev state in the .configure_partition() handler is > problematic in the context of moving to the V4L2 subdev active state > API, as .configure_partition() is called in non-interruptable context, > and the state lock can't be taken. To avoid this, start by storing the > vertical size in the window, replacing the custom vsp1_partition_window > structure with a v4l2_rect. Retrieving the vertical size from the window > in .configure_partition() will be done in a subsequent change. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > --- > .../media/platform/renesas/vsp1/vsp1_entity.h | 3 +-- > .../media/platform/renesas/vsp1/vsp1_pipe.c | 6 ++++-- > .../media/platform/renesas/vsp1/vsp1_pipe.h | 21 +++++-------------- > .../media/platform/renesas/vsp1/vsp1_rpf.c | 2 +- > .../media/platform/renesas/vsp1/vsp1_sru.c | 4 +++- > .../media/platform/renesas/vsp1/vsp1_uds.c | 10 ++++----- > .../media/platform/renesas/vsp1/vsp1_wpf.c | 2 +- > 7 files changed, 20 insertions(+), 28 deletions(-) > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h > index 7b86b2fef3e5..f67f60677644 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h > +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h > @@ -19,7 +19,6 @@ struct vsp1_dl_body; > struct vsp1_dl_list; > struct vsp1_pipeline; > struct vsp1_partition; > -struct vsp1_partition_window; > > enum vsp1_entity_type { > VSP1_ENTITY_BRS, > @@ -91,7 +90,7 @@ struct vsp1_entity_operations { > unsigned int (*max_width)(struct vsp1_entity *, struct vsp1_pipeline *); > void (*partition)(struct vsp1_entity *, struct vsp1_pipeline *, > struct vsp1_partition *, unsigned int, > - struct vsp1_partition_window *); > + struct v4l2_rect *); > }; > > struct vsp1_entity { > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c > index ca6817f45dd2..8eba3cda1e3d 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c > @@ -459,7 +459,7 @@ void vsp1_pipeline_propagate_alpha(struct vsp1_pipeline *pipe, > static void vsp1_pipeline_propagate_partition(struct vsp1_pipeline *pipe, > struct vsp1_partition *partition, > unsigned int index, > - struct vsp1_partition_window *window) > + struct v4l2_rect *window) > { > struct vsp1_entity *entity; > > @@ -485,7 +485,7 @@ void vsp1_pipeline_calculate_partition(struct vsp1_pipeline *pipe, > unsigned int index) > { > const struct v4l2_mbus_framefmt *format; > - struct vsp1_partition_window window; > + struct v4l2_rect window; > unsigned int modulus; > > /* > @@ -498,6 +498,8 @@ void vsp1_pipeline_calculate_partition(struct vsp1_pipeline *pipe, > /* Initialise the partition with sane starting conditions. */ > window.left = index * div_size; > window.width = div_size; > + window.top = 0; > + window.height = format->height; > > modulus = format->width % div_size; > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > index 3d2e35ac8fa0..c1f411227de7 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > @@ -53,17 +53,6 @@ enum vsp1_pipeline_state { > VSP1_PIPELINE_STOPPING, > }; > > -/* > - * struct vsp1_partition_window - Partition window coordinates > - * @left: horizontal coordinate of the partition start in pixels relative to the > - * left edge of the image > - * @width: partition width in pixels > - */ > -struct vsp1_partition_window { > - unsigned int left; > - unsigned int width; > -}; > - > /* > * struct vsp1_partition - A description of a slice for the partition algorithm > * @rpf: The RPF partition window configuration > @@ -73,11 +62,11 @@ struct vsp1_partition_window { > * @wpf: The WPF partition window configuration > */ > struct vsp1_partition { > - struct vsp1_partition_window rpf[VSP1_MAX_RPF]; > - struct vsp1_partition_window uds_sink; > - struct vsp1_partition_window uds_source; > - struct vsp1_partition_window sru; > - struct vsp1_partition_window wpf; > + struct v4l2_rect rpf[VSP1_MAX_RPF]; > + struct v4l2_rect uds_sink; > + struct v4l2_rect uds_source; > + struct v4l2_rect sru; > + struct v4l2_rect wpf; > }; > > /* > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c > index 3b8a62299226..862751616646 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c > @@ -366,7 +366,7 @@ static void rpf_partition(struct vsp1_entity *entity, > struct vsp1_pipeline *pipe, > struct vsp1_partition *partition, > unsigned int partition_idx, > - struct vsp1_partition_window *window) > + struct v4l2_rect *window) > { > struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev); > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_sru.c b/drivers/media/platform/renesas/vsp1/vsp1_sru.c > index 2c67aae0d5fd..f35187daa643 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c > @@ -324,7 +324,7 @@ static void sru_partition(struct vsp1_entity *entity, > struct vsp1_pipeline *pipe, > struct vsp1_partition *partition, > unsigned int partition_idx, > - struct vsp1_partition_window *window) > + struct v4l2_rect *window) > { > struct vsp1_sru *sru = to_sru(&entity->subdev); > struct v4l2_mbus_framefmt *input; > @@ -339,6 +339,8 @@ static void sru_partition(struct vsp1_entity *entity, > if (input->width != output->width) { > window->width /= 2; > window->left /= 2; > + window->height /= 2; > + window->top /= 2; If partitions vertically span the whole image, is this required ? Thanks j > } > > partition->sru = *window; > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c > index 737362ca2315..4a14fd3baac1 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c > @@ -363,16 +363,12 @@ static void uds_partition(struct vsp1_entity *entity, > struct vsp1_pipeline *pipe, > struct vsp1_partition *partition, > unsigned int partition_idx, > - struct vsp1_partition_window *window) > + struct v4l2_rect *window) > { > struct vsp1_uds *uds = to_uds(&entity->subdev); > const struct v4l2_mbus_framefmt *output; > const struct v4l2_mbus_framefmt *input; > > - /* Initialise the partition state. */ > - partition->uds_sink = *window; > - partition->uds_source = *window; > - > input = v4l2_subdev_state_get_format(uds->entity.state, UDS_PAD_SINK); > output = v4l2_subdev_state_get_format(uds->entity.state, > UDS_PAD_SOURCE); > @@ -381,6 +377,10 @@ static void uds_partition(struct vsp1_entity *entity, > / output->width; > partition->uds_sink.left = window->left * input->width > / output->width; > + partition->uds_sink.height = input->height; > + partition->uds_sink.top = 0; > + > + partition->uds_source = *window; > > *window = partition->uds_sink; > } > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c > index 80fe7571f4ff..f8d1e2f47691 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c > @@ -515,7 +515,7 @@ static void wpf_partition(struct vsp1_entity *entity, > struct vsp1_pipeline *pipe, > struct vsp1_partition *partition, > unsigned int partition_idx, > - struct vsp1_partition_window *window) > + struct v4l2_rect *window) > { > partition->wpf = *window; > } > -- > Regards, > > Laurent Pinchart > >