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@xxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxxx>
Thanks!
>
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> > ---
> > 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
