| Message ID | 20220113150042.15630-3-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: mc: Simplify media pipeline start/stop | expand |
Moi, Thanks for the set. On Thu, Jan 13, 2022 at 05:00:42PM +0200, Laurent Pinchart wrote: > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > The media_pipeline_start() function has two purposes: it constructs a > pipeline by recording the entities that are part of it, gathered from a > graph walk, and validate the media links. The pipeline pointer is stored > in the media_entity structure as part of this process, and the entity's > stream count is increased, to record that the entity is streaming. > > When multiple video nodes are present in a pipeline, > media_pipeline_start() is typically called on all of them, with the same > pipeline pointer. This is taken into account in media_pipeline_start() > by skipping validation for entities that are already part of the > pipeline, while returning an error if an entity is part of a different > pipeline. > > It turns out that this process is overly complicated. When > media_pipeline_start() is called for the first time, it constructs the > full pipeline, adding all entities and validating all the links. > Subsequent calls to media_pipeline_start() are then nearly no-ops, they > only increase the stream count on the pipeline and on all entities. > > The media_entity stream_count field is used for two purposes: checking > if the entity is streaming, and detecting when a call to > media_pipeline_stop() balances needs to reset the entity pipe pointer to > NULL. The former can easily be replaced by a check of the pipe pointer. > > Simplify media_pipeline_start() by avoiding the pipeline walk on all > calls but the first one, and drop the media_entity stream_count field. > media_pipeline_stop() is updated accordingly. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/mc/mc-entity.c | 52 +++++++++++++++--------------------- > include/media/media-entity.h | 11 +++----- > 2 files changed, 26 insertions(+), 37 deletions(-) > > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c > index f83e043f0f3b..8ab0913d8d82 100644 > --- a/drivers/media/mc/mc-entity.c > +++ b/drivers/media/mc/mc-entity.c > @@ -396,20 +396,21 @@ __must_check int __media_pipeline_start(struct media_entity *entity, > struct media_link *link; > int ret; > > - if (!pipe->streaming_count++) { > - ret = media_graph_walk_init(&pipe->graph, mdev); > - if (ret) > - goto error_graph_walk_start; > + if (pipe->streaming_count) { > + pipe->streaming_count++; > + return 0; > } > > + ret = media_graph_walk_init(&pipe->graph, mdev); > + if (ret) > + return ret; > + > media_graph_walk_start(&pipe->graph, entity); > > while ((entity = media_graph_walk_next(graph))) { > DECLARE_BITMAP(active, MEDIA_ENTITY_MAX_PADS); > DECLARE_BITMAP(has_no_links, MEDIA_ENTITY_MAX_PADS); > > - entity->stream_count++; > - > if (entity->pipe && entity->pipe != pipe) { > pr_err("Pipe active for %s. Can't start for %s\n", > entity->name, > @@ -418,12 +419,12 @@ __must_check int __media_pipeline_start(struct media_entity *entity, > goto error; > } > > - entity->pipe = pipe; > - > /* Already streaming --- no need to check. */ > - if (entity->stream_count > 1) > + if (entity->pipe) > continue; > > + entity->pipe = pipe; > + > if (!entity->ops || !entity->ops->link_validate) > continue; > > @@ -479,6 +480,8 @@ __must_check int __media_pipeline_start(struct media_entity *entity, > } > } > > + pipe->streaming_count++; > + > return 0; > > error: > @@ -489,24 +492,17 @@ __must_check int __media_pipeline_start(struct media_entity *entity, > media_graph_walk_start(graph, entity_err); > > while ((entity_err = media_graph_walk_next(graph))) { > - /* Sanity check for negative stream_count */ > - if (!WARN_ON_ONCE(entity_err->stream_count <= 0)) { > - entity_err->stream_count--; > - if (entity_err->stream_count == 0) > - entity_err->pipe = NULL; > - } > + entity_err->pipe = NULL; > > /* > - * We haven't increased stream_count further than this > - * so we quit here. > + * We haven't started entities further than this so we quit > + * here. > */ > if (entity_err == entity) > break; > } > > -error_graph_walk_start: > - if (!--pipe->streaming_count) > - media_graph_walk_cleanup(graph); > + media_graph_walk_cleanup(graph); > > return ret; > } > @@ -537,19 +533,15 @@ void __media_pipeline_stop(struct media_entity *entity) > if (WARN_ON(!pipe)) > return; > > + if (--pipe->streaming_count) > + return; > + > media_graph_walk_start(graph, entity); > > - while ((entity = media_graph_walk_next(graph))) { > - /* Sanity check for negative stream_count */ > - if (!WARN_ON_ONCE(entity->stream_count <= 0)) { > - entity->stream_count--; > - if (entity->stream_count == 0) > - entity->pipe = NULL; > - } > - } > + while ((entity = media_graph_walk_next(graph))) > + entity->pipe = NULL; > > - if (!--pipe->streaming_count) > - media_graph_walk_cleanup(graph); > + media_graph_walk_cleanup(graph); > > } > EXPORT_SYMBOL_GPL(__media_pipeline_stop); > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > index 8546f13c42a9..e3c4fd1e3623 100644 > --- a/include/media/media-entity.h > +++ b/include/media/media-entity.h > @@ -268,7 +268,6 @@ enum media_entity_type { > * @pads: Pads array with the size defined by @num_pads. > * @links: List of data links. > * @ops: Entity operations. > - * @stream_count: Stream count for the entity. > * @use_count: Use count for the entity. > * @pipe: Pipeline this entity belongs to. > * @info: Union with devnode information. Kept just for backward > @@ -283,10 +282,9 @@ enum media_entity_type { > * > * .. note:: > * > - * @stream_count and @use_count reference counts must never be > - * negative, but are signed integers on purpose: a simple ``WARN_ON(<0)`` > - * check can be used to detect reference count bugs that would make them > - * negative. > + * The @use_count reference count must never be negative, but is a signed > + * integer on purpose: a simple ``WARN_ON(<0)`` check can be used to detect > + * reference count bugs that would make it negative. > */ > struct media_entity { > struct media_gobj graph_obj; /* must be first field in struct */ > @@ -305,7 +303,6 @@ struct media_entity { > > const struct media_entity_operations *ops; > > - int stream_count; > int use_count; > > struct media_pipeline *pipe; > @@ -867,7 +864,7 @@ struct media_pad *media_entity_remote_pad(const struct media_pad *pad); > */ > static inline bool media_entity_is_streaming(const struct media_entity *entity) > { > - return entity->stream_count > 0; > + return entity->pipe != NULL; I'd drop "!= NULL" part; it's redundant. I'll do that when applying if that's fine. > } > > /**
Hi Sakari, On Sat, Jan 15, 2022 at 07:39:33PM +0200, Sakari Ailus wrote: > Moi, > > Thanks for the set. Ole hyvä. > On Thu, Jan 13, 2022 at 05:00:42PM +0200, Laurent Pinchart wrote: > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > The media_pipeline_start() function has two purposes: it constructs a > > pipeline by recording the entities that are part of it, gathered from a > > graph walk, and validate the media links. The pipeline pointer is stored > > in the media_entity structure as part of this process, and the entity's > > stream count is increased, to record that the entity is streaming. > > > > When multiple video nodes are present in a pipeline, > > media_pipeline_start() is typically called on all of them, with the same > > pipeline pointer. This is taken into account in media_pipeline_start() > > by skipping validation for entities that are already part of the > > pipeline, while returning an error if an entity is part of a different > > pipeline. > > > > It turns out that this process is overly complicated. When > > media_pipeline_start() is called for the first time, it constructs the > > full pipeline, adding all entities and validating all the links. > > Subsequent calls to media_pipeline_start() are then nearly no-ops, they > > only increase the stream count on the pipeline and on all entities. > > > > The media_entity stream_count field is used for two purposes: checking > > if the entity is streaming, and detecting when a call to > > media_pipeline_stop() balances needs to reset the entity pipe pointer to > > NULL. The former can easily be replaced by a check of the pipe pointer. > > > > Simplify media_pipeline_start() by avoiding the pipeline walk on all > > calls but the first one, and drop the media_entity stream_count field. > > media_pipeline_stop() is updated accordingly. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > drivers/media/mc/mc-entity.c | 52 +++++++++++++++--------------------- > > include/media/media-entity.h | 11 +++----- > > 2 files changed, 26 insertions(+), 37 deletions(-) > > > > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c > > index f83e043f0f3b..8ab0913d8d82 100644 > > --- a/drivers/media/mc/mc-entity.c > > +++ b/drivers/media/mc/mc-entity.c > > @@ -396,20 +396,21 @@ __must_check int __media_pipeline_start(struct media_entity *entity, > > struct media_link *link; > > int ret; > > > > - if (!pipe->streaming_count++) { > > - ret = media_graph_walk_init(&pipe->graph, mdev); > > - if (ret) > > - goto error_graph_walk_start; > > + if (pipe->streaming_count) { > > + pipe->streaming_count++; > > + return 0; > > } > > > > + ret = media_graph_walk_init(&pipe->graph, mdev); > > + if (ret) > > + return ret; > > + > > media_graph_walk_start(&pipe->graph, entity); > > > > while ((entity = media_graph_walk_next(graph))) { > > DECLARE_BITMAP(active, MEDIA_ENTITY_MAX_PADS); > > DECLARE_BITMAP(has_no_links, MEDIA_ENTITY_MAX_PADS); > > > > - entity->stream_count++; > > - > > if (entity->pipe && entity->pipe != pipe) { > > pr_err("Pipe active for %s. Can't start for %s\n", > > entity->name, > > @@ -418,12 +419,12 @@ __must_check int __media_pipeline_start(struct media_entity *entity, > > goto error; > > } > > > > - entity->pipe = pipe; > > - > > /* Already streaming --- no need to check. */ > > - if (entity->stream_count > 1) > > + if (entity->pipe) > > continue; > > > > + entity->pipe = pipe; > > + > > if (!entity->ops || !entity->ops->link_validate) > > continue; > > > > @@ -479,6 +480,8 @@ __must_check int __media_pipeline_start(struct media_entity *entity, > > } > > } > > > > + pipe->streaming_count++; > > + > > return 0; > > > > error: > > @@ -489,24 +492,17 @@ __must_check int __media_pipeline_start(struct media_entity *entity, > > media_graph_walk_start(graph, entity_err); > > > > while ((entity_err = media_graph_walk_next(graph))) { > > - /* Sanity check for negative stream_count */ > > - if (!WARN_ON_ONCE(entity_err->stream_count <= 0)) { > > - entity_err->stream_count--; > > - if (entity_err->stream_count == 0) > > - entity_err->pipe = NULL; > > - } > > + entity_err->pipe = NULL; > > > > /* > > - * We haven't increased stream_count further than this > > - * so we quit here. > > + * We haven't started entities further than this so we quit > > + * here. > > */ > > if (entity_err == entity) > > break; > > } > > > > -error_graph_walk_start: > > - if (!--pipe->streaming_count) > > - media_graph_walk_cleanup(graph); > > + media_graph_walk_cleanup(graph); > > > > return ret; > > } > > @@ -537,19 +533,15 @@ void __media_pipeline_stop(struct media_entity *entity) > > if (WARN_ON(!pipe)) > > return; > > > > + if (--pipe->streaming_count) > > + return; > > + > > media_graph_walk_start(graph, entity); > > > > - while ((entity = media_graph_walk_next(graph))) { > > - /* Sanity check for negative stream_count */ > > - if (!WARN_ON_ONCE(entity->stream_count <= 0)) { > > - entity->stream_count--; > > - if (entity->stream_count == 0) > > - entity->pipe = NULL; > > - } > > - } > > + while ((entity = media_graph_walk_next(graph))) > > + entity->pipe = NULL; > > > > - if (!--pipe->streaming_count) > > - media_graph_walk_cleanup(graph); > > + media_graph_walk_cleanup(graph); > > > > } > > EXPORT_SYMBOL_GPL(__media_pipeline_stop); > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > > index 8546f13c42a9..e3c4fd1e3623 100644 > > --- a/include/media/media-entity.h > > +++ b/include/media/media-entity.h > > @@ -268,7 +268,6 @@ enum media_entity_type { > > * @pads: Pads array with the size defined by @num_pads. > > * @links: List of data links. > > * @ops: Entity operations. > > - * @stream_count: Stream count for the entity. > > * @use_count: Use count for the entity. > > * @pipe: Pipeline this entity belongs to. > > * @info: Union with devnode information. Kept just for backward > > @@ -283,10 +282,9 @@ enum media_entity_type { > > * > > * .. note:: > > * > > - * @stream_count and @use_count reference counts must never be > > - * negative, but are signed integers on purpose: a simple ``WARN_ON(<0)`` > > - * check can be used to detect reference count bugs that would make them > > - * negative. > > + * The @use_count reference count must never be negative, but is a signed > > + * integer on purpose: a simple ``WARN_ON(<0)`` check can be used to detect > > + * reference count bugs that would make it negative. > > */ > > struct media_entity { > > struct media_gobj graph_obj; /* must be first field in struct */ > > @@ -305,7 +303,6 @@ struct media_entity { > > > > const struct media_entity_operations *ops; > > > > - int stream_count; > > int use_count; > > > > struct media_pipeline *pipe; > > @@ -867,7 +864,7 @@ struct media_pad *media_entity_remote_pad(const struct media_pad *pad); > > */ > > static inline bool media_entity_is_streaming(const struct media_entity *entity) > > { > > - return entity->stream_count > 0; > > + return entity->pipe != NULL; > > I'd drop "!= NULL" part; it's redundant. I usually use "if (pointer)" or "if (!pointer)" without comparing to NULL, but for some reason, when returning a bool, it feels more explicit to me to use a comparison. I'm not sure why, maybe because, unlike with if (), the implicit cast is only apparent when you read the signature of the function ? Not that it's far away in this case, it's only two lines up. > I'll do that when applying if that's fine. Fine with me, I don't mind either way. Thanks. > > } > > > > /**
Hi Sakari, On Sat, Jan 15, 2022 at 10:02:21PM +0200, Laurent Pinchart wrote: > On Sat, Jan 15, 2022 at 07:39:33PM +0200, Sakari Ailus wrote: > > On Thu, Jan 13, 2022 at 05:00:42PM +0200, Laurent Pinchart wrote: > > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > > The media_pipeline_start() function has two purposes: it constructs a > > > pipeline by recording the entities that are part of it, gathered from a > > > graph walk, and validate the media links. The pipeline pointer is stored > > > in the media_entity structure as part of this process, and the entity's > > > stream count is increased, to record that the entity is streaming. > > > > > > When multiple video nodes are present in a pipeline, > > > media_pipeline_start() is typically called on all of them, with the same > > > pipeline pointer. This is taken into account in media_pipeline_start() > > > by skipping validation for entities that are already part of the > > > pipeline, while returning an error if an entity is part of a different > > > pipeline. > > > > > > It turns out that this process is overly complicated. When > > > media_pipeline_start() is called for the first time, it constructs the > > > full pipeline, adding all entities and validating all the links. > > > Subsequent calls to media_pipeline_start() are then nearly no-ops, they > > > only increase the stream count on the pipeline and on all entities. > > > > > > The media_entity stream_count field is used for two purposes: checking > > > if the entity is streaming, and detecting when a call to > > > media_pipeline_stop() balances needs to reset the entity pipe pointer to > > > NULL. The former can easily be replaced by a check of the pipe pointer. > > > > > > Simplify media_pipeline_start() by avoiding the pipeline walk on all > > > calls but the first one, and drop the media_entity stream_count field. > > > media_pipeline_stop() is updated accordingly. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > drivers/media/mc/mc-entity.c | 52 +++++++++++++++--------------------- > > > include/media/media-entity.h | 11 +++----- > > > 2 files changed, 26 insertions(+), 37 deletions(-) > > > > > > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c > > > index f83e043f0f3b..8ab0913d8d82 100644 > > > --- a/drivers/media/mc/mc-entity.c > > > +++ b/drivers/media/mc/mc-entity.c > > > @@ -396,20 +396,21 @@ __must_check int __media_pipeline_start(struct media_entity *entity, > > > struct media_link *link; > > > int ret; > > > > > > - if (!pipe->streaming_count++) { > > > - ret = media_graph_walk_init(&pipe->graph, mdev); > > > - if (ret) > > > - goto error_graph_walk_start; > > > + if (pipe->streaming_count) { > > > + pipe->streaming_count++; > > > + return 0; > > > } > > > > > > + ret = media_graph_walk_init(&pipe->graph, mdev); > > > + if (ret) > > > + return ret; > > > + > > > media_graph_walk_start(&pipe->graph, entity); > > > > > > while ((entity = media_graph_walk_next(graph))) { > > > DECLARE_BITMAP(active, MEDIA_ENTITY_MAX_PADS); > > > DECLARE_BITMAP(has_no_links, MEDIA_ENTITY_MAX_PADS); > > > > > > - entity->stream_count++; > > > - > > > if (entity->pipe && entity->pipe != pipe) { > > > pr_err("Pipe active for %s. Can't start for %s\n", > > > entity->name, > > > @@ -418,12 +419,12 @@ __must_check int __media_pipeline_start(struct media_entity *entity, > > > goto error; > > > } > > > > > > - entity->pipe = pipe; > > > - > > > /* Already streaming --- no need to check. */ > > > - if (entity->stream_count > 1) > > > + if (entity->pipe) > > > continue; > > > > > > + entity->pipe = pipe; > > > + > > > if (!entity->ops || !entity->ops->link_validate) > > > continue; > > > > > > @@ -479,6 +480,8 @@ __must_check int __media_pipeline_start(struct media_entity *entity, > > > } > > > } > > > > > > + pipe->streaming_count++; > > > + > > > return 0; > > > > > > error: > > > @@ -489,24 +492,17 @@ __must_check int __media_pipeline_start(struct media_entity *entity, > > > media_graph_walk_start(graph, entity_err); > > > > > > while ((entity_err = media_graph_walk_next(graph))) { > > > - /* Sanity check for negative stream_count */ > > > - if (!WARN_ON_ONCE(entity_err->stream_count <= 0)) { > > > - entity_err->stream_count--; > > > - if (entity_err->stream_count == 0) > > > - entity_err->pipe = NULL; > > > - } > > > + entity_err->pipe = NULL; > > > > > > /* > > > - * We haven't increased stream_count further than this > > > - * so we quit here. > > > + * We haven't started entities further than this so we quit > > > + * here. > > > */ > > > if (entity_err == entity) > > > break; > > > } > > > > > > -error_graph_walk_start: > > > - if (!--pipe->streaming_count) > > > - media_graph_walk_cleanup(graph); > > > + media_graph_walk_cleanup(graph); > > > > > > return ret; > > > } > > > @@ -537,19 +533,15 @@ void __media_pipeline_stop(struct media_entity *entity) > > > if (WARN_ON(!pipe)) > > > return; > > > > > > + if (--pipe->streaming_count) > > > + return; > > > + > > > media_graph_walk_start(graph, entity); > > > > > > - while ((entity = media_graph_walk_next(graph))) { > > > - /* Sanity check for negative stream_count */ > > > - if (!WARN_ON_ONCE(entity->stream_count <= 0)) { > > > - entity->stream_count--; > > > - if (entity->stream_count == 0) > > > - entity->pipe = NULL; > > > - } > > > - } > > > + while ((entity = media_graph_walk_next(graph))) > > > + entity->pipe = NULL; > > > > > > - if (!--pipe->streaming_count) > > > - media_graph_walk_cleanup(graph); > > > + media_graph_walk_cleanup(graph); > > > > > > } > > > EXPORT_SYMBOL_GPL(__media_pipeline_stop); > > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > > > index 8546f13c42a9..e3c4fd1e3623 100644 > > > --- a/include/media/media-entity.h > > > +++ b/include/media/media-entity.h > > > @@ -268,7 +268,6 @@ enum media_entity_type { > > > * @pads: Pads array with the size defined by @num_pads. > > > * @links: List of data links. > > > * @ops: Entity operations. > > > - * @stream_count: Stream count for the entity. > > > * @use_count: Use count for the entity. > > > * @pipe: Pipeline this entity belongs to. > > > * @info: Union with devnode information. Kept just for backward > > > @@ -283,10 +282,9 @@ enum media_entity_type { > > > * > > > * .. note:: > > > * > > > - * @stream_count and @use_count reference counts must never be > > > - * negative, but are signed integers on purpose: a simple ``WARN_ON(<0)`` > > > - * check can be used to detect reference count bugs that would make them > > > - * negative. > > > + * The @use_count reference count must never be negative, but is a signed > > > + * integer on purpose: a simple ``WARN_ON(<0)`` check can be used to detect > > > + * reference count bugs that would make it negative. > > > */ > > > struct media_entity { > > > struct media_gobj graph_obj; /* must be first field in struct */ > > > @@ -305,7 +303,6 @@ struct media_entity { > > > > > > const struct media_entity_operations *ops; > > > > > > - int stream_count; > > > int use_count; > > > > > > struct media_pipeline *pipe; > > > @@ -867,7 +864,7 @@ struct media_pad *media_entity_remote_pad(const struct media_pad *pad); > > > */ > > > static inline bool media_entity_is_streaming(const struct media_entity *entity) > > > { > > > - return entity->stream_count > 0; > > > + return entity->pipe != NULL; > > > > I'd drop "!= NULL" part; it's redundant. > > I usually use "if (pointer)" or "if (!pointer)" without comparing to > NULL, but for some reason, when returning a bool, it feels more explicit > to me to use a comparison. I'm not sure why, maybe because, unlike with > if (), the implicit cast is only apparent when you read the signature of > the function ? Not that it's far away in this case, it's only two lines > up. > > > I'll do that when applying if that's fine. > > Fine with me, I don't mind either way. Thanks. Will you take this series for v5.18 ? > > > } > > > > > > /**
On Fri, Feb 11, 2022 at 11:27:22PM +0200, Laurent Pinchart wrote: > > > I'll do that when applying if that's fine. > > > > Fine with me, I don't mind either way. Thanks. > > Will you take this series for v5.18 ? Yes.
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c index f83e043f0f3b..8ab0913d8d82 100644 --- a/drivers/media/mc/mc-entity.c +++ b/drivers/media/mc/mc-entity.c @@ -396,20 +396,21 @@ __must_check int __media_pipeline_start(struct media_entity *entity, struct media_link *link; int ret; - if (!pipe->streaming_count++) { - ret = media_graph_walk_init(&pipe->graph, mdev); - if (ret) - goto error_graph_walk_start; + if (pipe->streaming_count) { + pipe->streaming_count++; + return 0; } + ret = media_graph_walk_init(&pipe->graph, mdev); + if (ret) + return ret; + media_graph_walk_start(&pipe->graph, entity); while ((entity = media_graph_walk_next(graph))) { DECLARE_BITMAP(active, MEDIA_ENTITY_MAX_PADS); DECLARE_BITMAP(has_no_links, MEDIA_ENTITY_MAX_PADS); - entity->stream_count++; - if (entity->pipe && entity->pipe != pipe) { pr_err("Pipe active for %s. Can't start for %s\n", entity->name, @@ -418,12 +419,12 @@ __must_check int __media_pipeline_start(struct media_entity *entity, goto error; } - entity->pipe = pipe; - /* Already streaming --- no need to check. */ - if (entity->stream_count > 1) + if (entity->pipe) continue; + entity->pipe = pipe; + if (!entity->ops || !entity->ops->link_validate) continue; @@ -479,6 +480,8 @@ __must_check int __media_pipeline_start(struct media_entity *entity, } } + pipe->streaming_count++; + return 0; error: @@ -489,24 +492,17 @@ __must_check int __media_pipeline_start(struct media_entity *entity, media_graph_walk_start(graph, entity_err); while ((entity_err = media_graph_walk_next(graph))) { - /* Sanity check for negative stream_count */ - if (!WARN_ON_ONCE(entity_err->stream_count <= 0)) { - entity_err->stream_count--; - if (entity_err->stream_count == 0) - entity_err->pipe = NULL; - } + entity_err->pipe = NULL; /* - * We haven't increased stream_count further than this - * so we quit here. + * We haven't started entities further than this so we quit + * here. */ if (entity_err == entity) break; } -error_graph_walk_start: - if (!--pipe->streaming_count) - media_graph_walk_cleanup(graph); + media_graph_walk_cleanup(graph); return ret; } @@ -537,19 +533,15 @@ void __media_pipeline_stop(struct media_entity *entity) if (WARN_ON(!pipe)) return; + if (--pipe->streaming_count) + return; + media_graph_walk_start(graph, entity); - while ((entity = media_graph_walk_next(graph))) { - /* Sanity check for negative stream_count */ - if (!WARN_ON_ONCE(entity->stream_count <= 0)) { - entity->stream_count--; - if (entity->stream_count == 0) - entity->pipe = NULL; - } - } + while ((entity = media_graph_walk_next(graph))) + entity->pipe = NULL; - if (!--pipe->streaming_count) - media_graph_walk_cleanup(graph); + media_graph_walk_cleanup(graph); } EXPORT_SYMBOL_GPL(__media_pipeline_stop); diff --git a/include/media/media-entity.h b/include/media/media-entity.h index 8546f13c42a9..e3c4fd1e3623 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h @@ -268,7 +268,6 @@ enum media_entity_type { * @pads: Pads array with the size defined by @num_pads. * @links: List of data links. * @ops: Entity operations. - * @stream_count: Stream count for the entity. * @use_count: Use count for the entity. * @pipe: Pipeline this entity belongs to. * @info: Union with devnode information. Kept just for backward @@ -283,10 +282,9 @@ enum media_entity_type { * * .. note:: * - * @stream_count and @use_count reference counts must never be - * negative, but are signed integers on purpose: a simple ``WARN_ON(<0)`` - * check can be used to detect reference count bugs that would make them - * negative. + * The @use_count reference count must never be negative, but is a signed + * integer on purpose: a simple ``WARN_ON(<0)`` check can be used to detect + * reference count bugs that would make it negative. */ struct media_entity { struct media_gobj graph_obj; /* must be first field in struct */ @@ -305,7 +303,6 @@ struct media_entity { const struct media_entity_operations *ops; - int stream_count; int use_count; struct media_pipeline *pipe; @@ -867,7 +864,7 @@ struct media_pad *media_entity_remote_pad(const struct media_pad *pad); */ static inline bool media_entity_is_streaming(const struct media_entity *entity) { - return entity->stream_count > 0; + return entity->pipe != NULL; } /**