Linux-Renesas-SoC Archive on lore.kernel.org
 help / color / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	"Benoit Parrot" <bparrot@ti.com>,
	linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH v2 02/30] media: entity: Use pads instead of entities in the media graph walk stack
Date: Wed, 16 Jan 2019 00:13:03 +0200
Message-ID: <20190115221302.eijpkvl7ivvijdg6@kekkonen.localdomain> (raw)
In-Reply-To: <20190115220313.GB28397@pendragon.ideasonboard.com>

Hi Laurent,

On Wed, Jan 16, 2019 at 12:03:13AM +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Fri, Nov 02, 2018 at 12:31:16AM +0100, Niklas Söderlund wrote:
> > From: Sakari Ailus <sakari.ailus@linux.intel.com>
> > 
> > Change the media graph walk stack structure to use media pads instead of
> > using media entities. In addition to the entity, the pad contains the
> > information which pad in the entity are being dealt with.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  drivers/media/media-entity.c | 53 ++++++++++++++++++------------------
> >  include/media/media-entity.h |  6 ++--
> >  2 files changed, 29 insertions(+), 30 deletions(-)
> > 
> > diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> > index 2bbc07de71aa5e6d..892e64a0a9d8ec42 100644
> > --- a/drivers/media/media-entity.c
> > +++ b/drivers/media/media-entity.c
> > @@ -237,40 +237,39 @@ EXPORT_SYMBOL_GPL(media_entity_pads_init);
> >   * Graph traversal
> >   */
> >  
> > -static struct media_entity *
> > -media_entity_other(struct media_entity *entity, struct media_link *link)
> > +static struct media_pad *
> > +media_entity_other(struct media_pad *pad, struct media_link *link)
> >  {
> > -	if (link->source->entity == entity)
> > -		return link->sink->entity;
> > +	if (link->source == pad)
> > +		return link->sink;
> >  	else
> > -		return link->source->entity;
> > +		return link->source;
> >  }
> >  
> >  /* push an entity to traversal stack */
> > -static void stack_push(struct media_graph *graph,
> > -		       struct media_entity *entity)
> > +static void stack_push(struct media_graph *graph, struct media_pad *pad)
> >  {
> >  	if (graph->top == MEDIA_ENTITY_ENUM_MAX_DEPTH - 1) {
> >  		WARN_ON(1);
> >  		return;
> >  	}
> >  	graph->top++;
> > -	graph->stack[graph->top].link = entity->links.next;
> > -	graph->stack[graph->top].entity = entity;
> > +	graph->stack[graph->top].link = pad->entity->links.next;
> > +	graph->stack[graph->top].pad = pad;
> >  }
> >  
> > -static struct media_entity *stack_pop(struct media_graph *graph)
> > +static struct media_pad *stack_pop(struct media_graph *graph)
> >  {
> > -	struct media_entity *entity;
> > +	struct media_pad *pad;
> >  
> > -	entity = graph->stack[graph->top].entity;
> > +	pad = graph->stack[graph->top].pad;
> >  	graph->top--;
> >  
> > -	return entity;
> > +	return pad;
> >  }
> >  
> >  #define link_top(en)	((en)->stack[(en)->top].link)
> > -#define stack_top(en)	((en)->stack[(en)->top].entity)
> > +#define stack_top(en)	((en)->stack[(en)->top].pad)
> >  
> >  /**
> >   * media_graph_walk_init - Allocate resources for graph walk
> > @@ -306,8 +305,8 @@ void media_graph_walk_start(struct media_graph *graph, struct media_pad *pad)
> >  	media_entity_enum_set(&graph->ent_enum, pad->entity);
> >  
> >  	graph->top = 0;
> > -	graph->stack[graph->top].entity = NULL;
> > -	stack_push(graph, pad->entity);
> > +	graph->stack[graph->top].pad = NULL;
> > +	stack_push(graph, pad);
> >  	dev_dbg(pad->graph_obj.mdev->dev,
> >  		"begin graph walk at '%s':%u\n", pad->entity->name, pad->index);
> >  }
> > @@ -315,16 +314,16 @@ EXPORT_SYMBOL_GPL(media_graph_walk_start);
> >  
> >  static void media_graph_walk_iter(struct media_graph *graph)
> >  {
> > -	struct media_entity *entity = stack_top(graph);
> > +	struct media_pad *pad = stack_top(graph);
> >  	struct media_link *link;
> > -	struct media_entity *next;
> > +	struct media_pad *next;
> >  
> >  	link = list_entry(link_top(graph), typeof(*link), list);
> >  
> >  	/* The link is not enabled so we do not follow. */
> >  	if (!(link->flags & MEDIA_LNK_FL_ENABLED)) {
> >  		link_top(graph) = link_top(graph)->next;
> > -		dev_dbg(entity->graph_obj.mdev->dev,
> > +		dev_dbg(pad->graph_obj.mdev->dev,
> >  			"walk: skipping disabled link '%s':%u -> '%s':%u\n",
> >  			link->source->entity->name, link->source->index,
> >  			link->sink->entity->name, link->sink->index);
> > @@ -332,22 +331,22 @@ static void media_graph_walk_iter(struct media_graph *graph)
> >  	}
> >  
> >  	/* Get the entity in the other end of the link . */
> 
> s/entity/pad/
> 
> > -	next = media_entity_other(entity, link);
> > +	next = media_entity_other(pad, link);
> >  
> >  	/* Has the entity already been visited? */
> 
> s/entity/pad's entity/
> 
> > -	if (media_entity_enum_test_and_set(&graph->ent_enum, next)) {
> > +	if (media_entity_enum_test_and_set(&graph->ent_enum, next->entity)) {
> >  		link_top(graph) = link_top(graph)->next;
> > -		dev_dbg(entity->graph_obj.mdev->dev,
> > +		dev_dbg(pad->graph_obj.mdev->dev,
> >  			"walk: skipping entity '%s' (already seen)\n",
> > -			next->name);
> > +			next->entity->name);
> >  		return;
> >  	}
> >  
> >  	/* Push the new entity to stack and start over. */
> 
> s/entity/pad/
> 
> >  	link_top(graph) = link_top(graph)->next;
> >  	stack_push(graph, next);
> > -	dev_dbg(entity->graph_obj.mdev->dev, "walk: pushing '%s' on stack\n",
> > -		next->name);
> > +	dev_dbg(next->graph_obj.mdev->dev, "walk: pushing '%s':%u on stack\n",
> > +		next->entity->name, next->index);
> >  }
> >  
> >  struct media_entity *media_graph_walk_next(struct media_graph *graph)
> > @@ -362,10 +361,10 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph)
> >  	 * top of the stack until no more entities on the level can be
> >  	 * found.
> >  	 */
> > -	while (link_top(graph) != &stack_top(graph)->links)
> > +	while (link_top(graph) != &stack_top(graph)->entity->links)
> >  		media_graph_walk_iter(graph);
> >  
> > -	entity = stack_pop(graph);
> > +	entity = stack_pop(graph)->entity;
> >  	dev_dbg(entity->graph_obj.mdev->dev,
> >  		"walk: returning entity '%s'\n", entity->name);
> >  
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index 07ab141e739ef5ff..99c7606f01317741 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -86,16 +86,16 @@ struct media_entity_enum {
> >   * struct media_graph - Media graph traversal state
> >   *
> >   * @stack:		Graph traversal stack; the stack contains information
> > - *			on the path the media entities to be walked and the
> > + *			on the path the media pads to be walked and the
> 
> This sentence doesn't make too much sense to me, are we missing
> something ?

I think this should have been "the stack contains information on the media
entities to be walked and the links through which they were reached". So
just replace "entities" by "pads" here.

> 
> The rest looks OK to me, so with this fixed,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> >   *			links through which they were reached.
> > - * @stack.entity:	pointer to &struct media_entity at the graph.
> > + * @stack.pad:		pointer to &struct media_pad at the graph.
> >   * @stack.link:		pointer to &struct list_head.
> >   * @ent_enum:		Visited entities
> >   * @top:		The top of the stack
> >   */
> >  struct media_graph {
> >  	struct {
> > -		struct media_entity *entity;
> > +		struct media_pad *pad;
> >  		struct list_head *link;
> >  	} stack[MEDIA_ENTITY_ENUM_MAX_DEPTH];
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

  reply index

Thread overview: 90+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-01 23:31 [PATCH v2 00/30] v4l: add support for multiplexed streams Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 01/30] media: entity: Use pad as a starting point for graph walk Niklas Söderlund
2019-01-15 21:43   ` Laurent Pinchart
2018-11-01 23:31 ` [PATCH v2 02/30] media: entity: Use pads instead of entities in the media graph walk stack Niklas Söderlund
2019-01-15 22:03   ` Laurent Pinchart
2019-01-15 22:13     ` Sakari Ailus [this message]
2019-01-15 22:07   ` Laurent Pinchart
2018-11-01 23:31 ` [PATCH v2 03/30] media: entity: Walk the graph based on pads Niklas Söderlund
2019-01-15 22:21   ` Laurent Pinchart
     [not found]     ` <20190115223406.mxgzl36cp54gb7nv@kekkonen.localdomain>
2019-01-15 23:28       ` Laurent Pinchart
2019-01-22 14:50         ` Sakari Ailus
2019-02-14 15:15   ` Jacopo Mondi
2018-11-01 23:31 ` [PATCH v2 04/30] v4l: mc: Start walk from a specific pad in use count calculation Niklas Söderlund
2019-01-15 22:24   ` Laurent Pinchart
2019-01-15 22:36     ` Sakari Ailus
2018-11-01 23:31 ` [PATCH v2 05/30] media: entity: Move the pipeline from entity to pads Niklas Söderlund
2019-01-15 22:38   ` Laurent Pinchart
2019-01-15 22:48     ` Sakari Ailus
2019-02-14 15:53   ` Jacopo Mondi
2018-11-01 23:31 ` [PATCH v2 06/30] media: entity: Use pad as the starting point for a pipeline Niklas Söderlund
2019-01-15 22:54   ` Laurent Pinchart
2019-01-22 15:31     ` Sakari Ailus
2019-01-22 15:37       ` Laurent Pinchart
2019-01-22 16:16         ` Sakari Ailus
2018-11-01 23:31 ` [PATCH v2 07/30] media: entity: Add has_route entity operation Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 08/30] media: entity: Add media_has_route() function Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 09/30] media: entity: Swap pads if route is checked from source to sink Niklas Söderlund
2019-01-15 22:57   ` Laurent Pinchart
2019-01-22 15:15     ` Sakari Ailus
2019-01-22 15:20       ` Laurent Pinchart
2019-02-18  9:21         ` Jacopo Mondi
2019-02-22 12:18           ` Laurent Pinchart
2019-03-04 12:35             ` Jacopo Mondi
2019-03-05 20:04               ` Laurent Pinchart
2019-03-06  8:29                 ` Jacopo Mondi
2018-11-01 23:31 ` [PATCH v2 10/30] media: entity: Use routing information during graph traversal Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 11/30] media: entity: Skip link validation for pads to which there is no route to Niklas Söderlund
2019-01-15 23:13   ` Laurent Pinchart
2018-11-01 23:31 ` [PATCH v2 12/30] media: entity: Add an iterator helper for connected pads Niklas Söderlund
2019-01-15 23:24   ` Laurent Pinchart
2019-01-22 15:36     ` Sakari Ailus
2019-01-22 15:38       ` Laurent Pinchart
2019-01-22 16:21         ` Sakari Ailus
2018-11-01 23:31 ` [PATCH v2 13/30] media: entity: Add only connected pads to the pipeline Niklas Söderlund
2019-01-15 23:33   ` Laurent Pinchart
2018-11-01 23:31 ` [PATCH v2 14/30] media: entity: Add debug information in graph walk route check Niklas Söderlund
2019-01-15 23:35   ` Laurent Pinchart
2019-01-22 15:38     ` Sakari Ailus
2018-11-01 23:31 ` [PATCH v2 15/30] media: entity: Look for indirect routes Niklas Söderlund
2019-01-15 23:41   ` Laurent Pinchart
2019-01-22 15:56     ` Sakari Ailus
2018-11-01 23:31 ` [PATCH v2 16/30] v4l: subdev: Add [GS]_ROUTING subdev ioctls and operations Niklas Söderlund
2019-01-15 23:51   ` Laurent Pinchart
2019-01-22 16:14     ` Sakari Ailus
2019-01-22 17:00       ` Laurent Pinchart
2019-02-21 14:59     ` Jacopo Mondi
2019-02-21 23:49       ` Sakari Ailus
2019-02-22  8:46         ` Jacopo Mondi
2019-02-21 14:39   ` Jacopo Mondi
2019-02-21 22:31     ` Sakari Ailus
2019-02-22  8:40       ` Jacopo Mondi
2019-02-22 11:04         ` Sakari Ailus
2019-02-22 11:17           ` Jacopo Mondi
2019-02-22 11:29             ` Sakari Ailus
2019-02-22 13:37               ` Ian Arkver
2019-02-22 13:50               ` Geert Uytterhoeven
2018-11-01 23:31 ` [PATCH v2 17/30] v4l: subdev: compat: Implement handling for VIDIOC_SUBDEV_[GS]_ROUTING Niklas Söderlund
2019-01-08 10:04   ` Geert Uytterhoeven
2019-01-15 23:53   ` Laurent Pinchart
2019-01-22 15:57     ` Sakari Ailus
2019-02-18 11:21     ` Jacopo Mondi
2019-02-21 23:50       ` Sakari Ailus
2018-11-01 23:31 ` [PATCH v2 18/30] v4l: subdev: Take routing information into account in link validation Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 19/30] v4l: subdev: Improve link format validation debug messages Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 20/30] v4l: mc: Add an S_ROUTING helper function for power state changes Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 21/30] v4l: Add bus type to frame descriptors Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 22/30] v4l: Add CSI-2 bus configuration " Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 23/30] v4l: Add stream to frame descriptor Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 24/30] adv748x: csi2: add translation from pixelcode to CSI-2 datatype Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 25/30] adv748x: csi2: only allow formats on sink pads Niklas Söderlund
2019-02-21 14:18   ` Jacopo Mondi
2018-11-01 23:31 ` [PATCH v2 26/30] adv748x: csi2: describe the multiplexed stream Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 27/30] adv748x: csi2: add internal routing configuration Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 28/30] adv748x: afe: add routing support Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 29/30] rcar-csi2: use frame description information to configure CSI-2 bus Niklas Söderlund
2018-11-01 23:31 ` [PATCH v2 30/30] rcar-csi2: expose the subdevice internal routing Niklas Söderlund
2018-11-14 13:10   ` Nikita Yushchenko
2018-11-14 19:45     ` Niklas Söderlund
2018-12-03 22:16 ` [PATCH v2 00/30] v4l: add support for multiplexed streams Sakari Ailus
2018-12-05 22:09   ` Niklas Söderlund

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190115221302.eijpkvl7ivvijdg6@kekkonen.localdomain \
    --to=sakari.ailus@linux.intel.com \
    --cc=bparrot@ti.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Renesas-SoC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-renesas-soc/0 linux-renesas-soc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-renesas-soc linux-renesas-soc/ https://lore.kernel.org/linux-renesas-soc \
		linux-renesas-soc@vger.kernel.org linux-renesas-soc@archiver.kernel.org
	public-inbox-index linux-renesas-soc


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-renesas-soc


AGPL code for this site: git clone https://public-inbox.org/ public-inbox