On 08/07/2021 16:11, Jacopo Mondi wrote:

Hello again,

On Mon, May 24, 2021 at 01:43:47PM +0300, Tomi Valkeinen wrote:

quoted

From: Sakari Ailus <sakari.ailus@linux.intel.com>

This moves the pipe and stream_count fields from struct media_entity to
struct media_pad. Effectively streams become pad-specific rather than
being entity specific, allowing several independent streams to traverse a
single entity and an entity to be part of several streams.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

- Update documentation to use 'pads'
- Use the media pad iterator in media_entity.c
- Update rcar-dma.c to use the new per-pad stream count
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

- Fix cleanup in the error path of __media_pipeline_start()
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
  drivers/media/mc/mc-entity.c                  | 68 +++++++++++--------
  drivers/media/platform/exynos4-is/fimc-isp.c  |  2 +-
  drivers/media/platform/exynos4-is/fimc-lite.c |  2 +-
  drivers/media/platform/omap3isp/isp.c         |  2 +-
  drivers/media/platform/omap3isp/ispvideo.c    |  2 +-
  drivers/media/platform/omap3isp/ispvideo.h    |  2 +-
  drivers/media/platform/rcar-vin/rcar-core.c   | 16 +++--
  drivers/media/platform/rcar-vin/rcar-dma.c    |  2 +-
  drivers/media/platform/xilinx/xilinx-dma.c    |  2 +-
  drivers/media/platform/xilinx/xilinx-dma.h    |  2 +-
  drivers/staging/media/imx/imx-media-utils.c   |  2 +-
  drivers/staging/media/omap4iss/iss.c          |  2 +-
  drivers/staging/media/omap4iss/iss_video.c    |  2 +-
  drivers/staging/media/omap4iss/iss_video.h    |  2 +-
  include/media/media-entity.h                  | 21 +++---
  15 files changed, 73 insertions(+), 56 deletions(-)

diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 40ae9b6bac47..ea1cf7f63ae8 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c

@@ -424,24 +424,28 @@ __must_check int __media_pipeline_start(struct media_entity *entity,

  	while ((pad = media_graph_walk_next(graph))) {
  		struct media_entity *entity = pad->entity;
+		bool skip_validation = pad->pipe != NULL;
+		struct media_pad *iter;

  		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,
-				pad_err->entity->name);
-			ret = -EBUSY;
-			goto error;
+		media_entity_for_each_pad(entity, iter) {
+			if (iter->pipe && iter->pipe != pipe) {
+				pr_err("Pipe active for %s. Can't start for %s\n",
+				       entity->name, iter->entity->name);
+				ret = -EBUSY;
+			} else {
+				iter->pipe = pipe;
+			}
+			iter->stream_count++;
  		}

-		entity->pipe = pipe;
+		if (ret)

ret is not initialized when declared and there is a code path that
could lead here without assigning it. I would initialize it to 0 when
declaring it, or even re-assign it to 0 at the beginning of this while
loop.

Good catch! I'll fix that.

  Tomi