Commit 3056a8e9 authored by Laurent Pinchart's avatar Laurent Pinchart Committed by Sakari Ailus
Browse files

media: media-entity: Simplify media_pipeline_start() 



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: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
[Sakari Ailus: Drop redundant '!= NULL' as discussed]
Signed-off-by: default avatarSakari Ailus <sakari.ailus@linux.intel.com>
parent f17bc788

drivers/media/mc/mc-entity.c

+22 −30
Original line number Diff line number Diff line
@@ -396,11 +396,14 @@ __must_check int __media_pipeline_start(struct media_entity *entity, 
	struct media_link *link;

	int ret;



	if (!pipe->streaming_count++) {

	if (pipe->streaming_count) {

		pipe->streaming_count++;

		return 0;

	}



	ret = media_graph_walk_init(&pipe->graph, mdev);

	if (ret)

			goto error_graph_walk_start;

	}

		return ret;



	media_graph_walk_start(&pipe->graph, entity);
@@ -408,8 +411,6 @@ __must_check int __media_pipeline_start(struct media_entity *entity, 
		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,23 +492,16 @@ __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;

		}



		/*

		 * 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);



	return ret;
@@ -537,18 +533,14 @@ 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)

	while ((entity = media_graph_walk_next(graph)))

		entity->pipe = NULL;

		}

	}



	if (!--pipe->streaming_count)

	media_graph_walk_cleanup(graph);



}

include/media/media-entity.h

+4 −7
Original line number Diff line number Diff line
@@ -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;

}



/**