| Message ID | 20240709132906.3198927-2-dan.scally@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add Arm Mali-C55 Image Signal Processor Driver | expand |
Hi Dan, Thank you for the patch. On Tue, Jul 09, 2024 at 02:28:49PM +0100, Daniel Scally wrote: > Record the number of video devices in a pipeline so that we can track > in a central location how many need to be started before drivers must > actually begin streaming. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > Changes in v6: > > - New patch. This is intended to support Sakari's requirement for the > driver not to start streaming before all of the video devices have > called streamon(). This was the cleanest way I could think to acheive > the goal, and lets us just check for start_count == required_count > before streaming. This violates the abstraction layers a bit, the MC code isn't supposed to handle video devices like that. What you can instead do is to use media_pipeline_for_each_entity() in your driver when starting the pipeline to iterate over all entities, and count the video devices there. > > drivers/media/mc/mc-entity.c | 5 +++++ > include/media/media-entity.h | 2 ++ > 2 files changed, 7 insertions(+) > > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c > index 96dd0f6ccd0d..1e8186b13b55 100644 > --- a/drivers/media/mc/mc-entity.c > +++ b/drivers/media/mc/mc-entity.c > @@ -596,6 +596,9 @@ static int media_pipeline_add_pad(struct media_pipeline *pipe, > > list_add_tail(&ppad->list, &pipe->pads); > > + if (pad->entity->obj_type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE) > + pipe->required_count++; > + > dev_dbg(pad->graph_obj.mdev->dev, > "media pipeline: added pad '%s':%u\n", > pad->entity->name, pad->index); > @@ -713,6 +716,8 @@ static void media_pipeline_cleanup(struct media_pipeline *pipe) > list_del(&ppad->list); > kfree(ppad); > } > + > + pipe->required_count = 0; > } > > static int media_pipeline_populate(struct media_pipeline *pipe, > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > index 0393b23129eb..ab84458b40dc 100644 > --- a/include/media/media-entity.h > +++ b/include/media/media-entity.h > @@ -104,12 +104,14 @@ struct media_graph { > * @mdev: The media device the pipeline is part of > * @pads: List of media_pipeline_pad > * @start_count: Media pipeline start - stop count > + * @required_count: Number of starts required to be "running" > */ > struct media_pipeline { > bool allocated; > struct media_device *mdev; > struct list_head pads; > int start_count; > + int required_count; > }; > > /**
Hi Laurent On 30/07/2024 16:09, Laurent Pinchart wrote: > Hi Dan, > > Thank you for the patch. > > On Tue, Jul 09, 2024 at 02:28:49PM +0100, Daniel Scally wrote: >> Record the number of video devices in a pipeline so that we can track >> in a central location how many need to be started before drivers must >> actually begin streaming. >> >> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> >> --- >> Changes in v6: >> >> - New patch. This is intended to support Sakari's requirement for the >> driver not to start streaming before all of the video devices have >> called streamon(). This was the cleanest way I could think to acheive >> the goal, and lets us just check for start_count == required_count >> before streaming. > This violates the abstraction layers a bit, the MC code isn't supposed > to handle video devices like that. > > What you can instead do is to use media_pipeline_for_each_entity() in > your driver when starting the pipeline to iterate over all entities, and > count the video devices there. That'll do! Thanks for the suggestion. > >> drivers/media/mc/mc-entity.c | 5 +++++ >> include/media/media-entity.h | 2 ++ >> 2 files changed, 7 insertions(+) >> >> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c >> index 96dd0f6ccd0d..1e8186b13b55 100644 >> --- a/drivers/media/mc/mc-entity.c >> +++ b/drivers/media/mc/mc-entity.c >> @@ -596,6 +596,9 @@ static int media_pipeline_add_pad(struct media_pipeline *pipe, >> >> list_add_tail(&ppad->list, &pipe->pads); >> >> + if (pad->entity->obj_type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE) >> + pipe->required_count++; >> + >> dev_dbg(pad->graph_obj.mdev->dev, >> "media pipeline: added pad '%s':%u\n", >> pad->entity->name, pad->index); >> @@ -713,6 +716,8 @@ static void media_pipeline_cleanup(struct media_pipeline *pipe) >> list_del(&ppad->list); >> kfree(ppad); >> } >> + >> + pipe->required_count = 0; >> } >> >> static int media_pipeline_populate(struct media_pipeline *pipe, >> diff --git a/include/media/media-entity.h b/include/media/media-entity.h >> index 0393b23129eb..ab84458b40dc 100644 >> --- a/include/media/media-entity.h >> +++ b/include/media/media-entity.h >> @@ -104,12 +104,14 @@ struct media_graph { >> * @mdev: The media device the pipeline is part of >> * @pads: List of media_pipeline_pad >> * @start_count: Media pipeline start - stop count >> + * @required_count: Number of starts required to be "running" >> */ >> struct media_pipeline { >> bool allocated; >> struct media_device *mdev; >> struct list_head pads; >> int start_count; >> + int required_count; >> }; >> >> /**
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c index 96dd0f6ccd0d..1e8186b13b55 100644 --- a/drivers/media/mc/mc-entity.c +++ b/drivers/media/mc/mc-entity.c @@ -596,6 +596,9 @@ static int media_pipeline_add_pad(struct media_pipeline *pipe, list_add_tail(&ppad->list, &pipe->pads); + if (pad->entity->obj_type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE) + pipe->required_count++; + dev_dbg(pad->graph_obj.mdev->dev, "media pipeline: added pad '%s':%u\n", pad->entity->name, pad->index); @@ -713,6 +716,8 @@ static void media_pipeline_cleanup(struct media_pipeline *pipe) list_del(&ppad->list); kfree(ppad); } + + pipe->required_count = 0; } static int media_pipeline_populate(struct media_pipeline *pipe, diff --git a/include/media/media-entity.h b/include/media/media-entity.h index 0393b23129eb..ab84458b40dc 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h @@ -104,12 +104,14 @@ struct media_graph { * @mdev: The media device the pipeline is part of * @pads: List of media_pipeline_pad * @start_count: Media pipeline start - stop count + * @required_count: Number of starts required to be "running" */ struct media_pipeline { bool allocated; struct media_device *mdev; struct list_head pads; int start_count; + int required_count; }; /**
Record the number of video devices in a pipeline so that we can track in a central location how many need to be started before drivers must actually begin streaming. Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> --- Changes in v6: - New patch. This is intended to support Sakari's requirement for the driver not to start streaming before all of the video devices have called streamon(). This was the cleanest way I could think to acheive the goal, and lets us just check for start_count == required_count before streaming. drivers/media/mc/mc-entity.c | 5 +++++ include/media/media-entity.h | 2 ++ 2 files changed, 7 insertions(+)