Re: [PATCH v5 11/24] media: entity: Skip link validation for pads to which there is no route to

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Laurent,

On Sun, Apr 18, 2021 at 09:06:11PM +0300, Laurent Pinchart wrote:
> Hi Tomi and Sakari,
> 
> Thank you for the patch.
> 
> There's an extra "to" in the subject line.
> 
> On Thu, Apr 15, 2021 at 04:04:37PM +0300, Tomi Valkeinen wrote:
> > From: Sakari Ailus <sakari.ailus@linux.intel.com>
> > 
> > Links are validated along the pipeline which is about to start streaming.
> > Not all the pads in entities that are traversed along that pipeline are
> > part of the pipeline, however. Skip the link validation for such pads,
> > and while at there rename "other_pad" to "local_pad" to convey the fact
> > the route to be checked is internal to the entity.
> 
> Both "pad" and "local_pad" are local. I would have kept the "other_pad"

The pad that in the remote entity is not local. The other one could be
called remote_pad though.

> variable name.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

> 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  drivers/media/mc/mc-entity.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > index 28d7fd254c77..fe6cb743c85c 100644
> > --- a/drivers/media/mc/mc-entity.c
> > +++ b/drivers/media/mc/mc-entity.c
> > @@ -482,12 +482,17 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
> >  		bitmap_fill(has_no_links, entity->num_pads);
> >  
> >  		list_for_each_entry(link, &entity->links, list) {
> > -			struct media_pad *other_pad =
> > +			struct media_pad *local_pad =
> >  				link->sink->entity == entity ?
> >  				link->sink : link->source;
> >  
> > +			/* Ignore pads to which there is no route. */
> > +			if (!media_entity_has_route(entity, pad->index,
> > +						    local_pad->index))
> > +				continue;
> > +
> >  			/* Mark that a pad is connected by a link. */
> > -			bitmap_clear(has_no_links, other_pad->index, 1);
> > +			bitmap_clear(has_no_links, local_pad->index, 1);
> >  
> >  			/*
> >  			 * Pads that either do not need to connect or
> > @@ -496,13 +501,13 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
> >  			 */
> >  			if (!(pad->flags & MEDIA_PAD_FL_MUST_CONNECT) ||
> >  			    link->flags & MEDIA_LNK_FL_ENABLED)
> > -				bitmap_set(active, other_pad->index, 1);
> > +				bitmap_set(active, local_pad->index, 1);
> >  
> >  			/*
> >  			 * Link validation will only take place for
> >  			 * sink ends of the link that are enabled.
> >  			 */
> > -			if (link->sink != other_pad ||
> > +			if (link->sink != local_pad ||
> >  			    !(link->flags & MEDIA_LNK_FL_ENABLED))
> >  				continue;
> >  
> 

-- 
Sakari Ailus