Hi Tomi,
On Thu, Dec 15, 2022 at 02:48:43PM +0200, Tomi Valkeinen wrote:
> On 12/12/2022 16:16, Laurent Pinchart wrote:
> > Add a media_pipeline_for_each_entity() macro to iterate over entities in
> > a pipeline. This should be used by driver as a replacement of the
> > media_graph_walk API, as iterating over the media_pipeline uses the
> > cached list of pads and is thus more efficient.
> >
> > Deprecate the media_graph_walk API to indicate it shouldn't be used in
> > new drivers.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> > drivers/media/mc/mc-entity.c | 37 +++++++++++++++++++
> > include/media/media-entity.h | 69 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 106 insertions(+)
> >
> > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > index 70df2050951c..f19bb98071b2 100644
> > --- a/drivers/media/mc/mc-entity.c
> > +++ b/drivers/media/mc/mc-entity.c
> > @@ -950,6 +950,43 @@ __media_pipeline_pad_iter_next(struct media_pipeline *pipe,
> > }
> > EXPORT_SYMBOL_GPL(__media_pipeline_pad_iter_next);
> >
> > +int media_pipeline_entity_iter_init(struct media_pipeline *pipe,
> > + struct media_pipeline_entity_iter *iter)
> > +{
> > + return media_entity_enum_init(&iter->ent_enum, pipe->mdev);
> > +}
> > +EXPORT_SYMBOL_GPL(media_pipeline_entity_iter_init);
> > +
> > +void media_pipeline_entity_iter_cleanup(struct media_pipeline_entity_iter *iter)
> > +{
> > + media_entity_enum_cleanup(&iter->ent_enum);
> > +}
> > +EXPORT_SYMBOL_GPL(media_pipeline_entity_iter_cleanup);
> > +
> > +struct media_entity *
> > +__media_pipeline_entity_iter_next(struct media_pipeline *pipe,
> > + struct media_pipeline_entity_iter *iter,
> > + struct media_entity *entity)
> > +{
> > + if (!entity)
> > + iter->cursor = pipe->pads.next;
> > +
> > + while (iter->cursor != &pipe->pads) {
> > + struct media_pipeline_pad *ppad;
> > + struct media_entity *entity;
> > +
> > + ppad = list_entry(iter->cursor, struct media_pipeline_pad, list);
> > + entity = ppad->pad->entity;
> > + iter->cursor = iter->cursor->next;
> > +
> > + if (media_entity_enum_test_and_set(&iter->ent_enum, entity))
> > + return entity;
>
> Shouldn't this be if (!media_entity...)? Doesn't
> media_entity_enum_test_and_set return true if the entity has already
> been seen, and thus we shouldn't return it?
You're absolutely right. I'll fix that. I wonder how it escaped my
tests.
> > + }
> > +
> > + return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(__media_pipeline_entity_iter_next);
> > +
> > /* -----------------------------------------------------------------------------
> > * Links management
> > */
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index e881e483b550..1b820cb6fed1 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -139,6 +139,17 @@ struct media_pipeline_pad_iter {
> > struct list_head *cursor;
> > };
> >
> > +/**
> > + * struct media_pipeline_entity_iter - Iterator for media_pipeline_for_each_entity
> > + *
> > + * @ent_enum: The entity enumeration tracker
> > + * @cursor: The current element
> > + */
> > +struct media_pipeline_entity_iter {
> > + struct media_entity_enum ent_enum;
> > + struct list_head *cursor;
> > +};
> > +
>
> Same question here as for the previous one: can this be in the mc-entity.c?
I'm afraid not, for the same reason as in the previous patch.
--
Regards,
Laurent Pinchart
