Re: [PATCH v1 5/5] media: xilinx: dma: Use media_pipeline_for_each_pad()

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

On Thu, Dec 15, 2022 at 03:29:13PM +0200, Tomi Valkeinen wrote:
> On 12/12/2022 16:16, Laurent Pinchart wrote:
> > Replace usage of the deprecated media graph walk API with the new
> > media_pipeline_for_each_pad() macro.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >   drivers/media/platform/xilinx/xilinx-dma.c | 28 +++++-----------------
> >   1 file changed, 6 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
> > index 0a7fd8642a65..fee02c8c85fd 100644
> > --- a/drivers/media/platform/xilinx/xilinx-dma.c
> > +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> > @@ -173,31 +173,19 @@ static int xvip_pipeline_set_stream(struct xvip_pipeline *pipe, bool on)
> >   static int xvip_pipeline_validate(struct xvip_pipeline *pipe,
> >   				  struct xvip_dma *start)
> >   {
> > -	struct media_graph graph;
> > -	struct media_entity *entity = &start->video.entity;
> > -	struct media_device *mdev = entity->graph_obj.mdev;
> > +	struct media_pipeline_pad_iter iter;
> >   	unsigned int num_inputs = 0;
> >   	unsigned int num_outputs = 0;
> > -	int ret;
> > +	struct media_pad *pad;
> >   
> > -	mutex_lock(&mdev->graph_mutex);
> > -
> > -	/* Walk the graph to locate the video nodes. */
> > -	ret = media_graph_walk_init(&graph, mdev);
> > -	if (ret) {
> > -		mutex_unlock(&mdev->graph_mutex);
> > -		return ret;
> > -	}
> > -
> > -	media_graph_walk_start(&graph, entity);
> > -
> > -	while ((entity = media_graph_walk_next(&graph))) {
> > +	/* Locate the video nodes in the pipeline. */
> > +	media_pipeline_for_each_pad(&pipe->pipe, &iter, pad) {
> >   		struct xvip_dma *dma;
> >   
> > -		if (entity->function != MEDIA_ENT_F_IO_V4L)
> > +		if (pad->entity->function != MEDIA_ENT_F_IO_V4L)
> >   			continue;
> 
> Why do you iterate the pads here, instead of using 
> media_pipeline_for_each_entity()?

The entity iterator still iterates over all pads internally, so it's not
more efficient. It also requires explicit initialization and cleanup, so
is more complicated to use. In this specific case, the driver ignores
all pads belonging to entities that are not V4L2 video nodes
(MEDIA_ENT_F_IO_V4L). As such entities have a single pad, there's a
guarantee they will not be visited twice.

If you think this is too fragile due to the assumption that
MEDIA_ENT_F_IO_V4L entities have a single pad, I can use the entity
iterator, it's not a big deal.

-- 
Regards,

Laurent Pinchart