Re: [PATCH v2 07/14] media: mc: Add MEDIA_LNK_FL_VALIDATE_LATE

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

Hi Mirela,

On Mon, Feb 16, 2026 at 03:19:26PM +0200, Mirela Rabulea wrote:
> Hi Sakari,
> 
> On 2/11/26 11:09, Sakari Ailus wrote:
> > Add MEDIA_LNK_FL_VALIDATE_LATE flag to support late validation of links.
> > This is serving the use case where video devices are configured and
> > started streaming indepenently of each other but this sequence may be run
> > in series, in such a way that a video device in a pipeline starts
> > streaming before another one is configured.
> > 
> > Before this flag, drivers have resorted to implementing the link
> > validation separately for the video nodes as part of streaming start
> > sequence.
> > 
> > media_pipeline_start() shall be called on each leaf entity connected to
> > the graph with a link where MEDIA_LNK_FL_VALIDATE_LATE is set before
> > uphardware operation.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >   .../media/mediactl/media-ioc-setup-link.rst   |  4 ++
> >   .../media/mediactl/media-types.rst            |  5 ++
> >   drivers/media/mc/mc-entity.c                  | 58 ++++++++++++++++++-
> >   include/uapi/linux/media.h                    |  1 +
> >   4 files changed, 66 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/userspace-api/media/mediactl/media-ioc-setup-link.rst b/Documentation/userspace-api/media/mediactl/media-ioc-setup-link.rst
> > index 23208300cb61..7a9a43c71cde 100644
> > --- a/Documentation/userspace-api/media/mediactl/media-ioc-setup-link.rst
> > +++ b/Documentation/userspace-api/media/mediactl/media-ioc-setup-link.rst
> > @@ -49,6 +49,10 @@ Only links marked with the ``DYNAMIC`` link flag can be enabled/disabled
> >   while streaming media data. Attempting to enable or disable a streaming
> >   non-dynamic link will return an ``EBUSY`` error code.
> > 
> > +The ``VALIDATE_LATE`` flag is used to signal that the validation of the link may
> > +be delayed until actual hardware operation even if the rest of the pipeline
> > +would be validated at an earlier point of time.
> > +
> >   If the specified link can't be found the driver returns with an ``EINVAL``
> >   error code.
> > 
> > diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst
> > index 6332e8395263..d6a690655a01 100644
> > --- a/Documentation/userspace-api/media/mediactl/media-types.rst
> > +++ b/Documentation/userspace-api/media/mediactl/media-types.rst
> > @@ -391,6 +391,7 @@ must be set for every pad.
> >   .. _MEDIA-LNK-FL-ENABLED:
> >   .. _MEDIA-LNK-FL-IMMUTABLE:
> >   .. _MEDIA-LNK-FL-DYNAMIC:
> > +.. _MEDIA-LNK-FL-VALIDATE-LATE:
> >   .. _MEDIA-LNK-FL-LINK-TYPE:
> > 
> >   .. flat-table:: Media link flags
> > @@ -410,6 +411,10 @@ must be set for every pad.
> >          -  The link enabled state can be modified during streaming. This flag
> >            is set by drivers and is read-only for applications.
> > 
> > +    *  -  ``MEDIA_LNK_FL_VALIDATE_LATE``
> > +       -  The validation of the link may be delayed up to until the start of
> > +         hardware operation.
> > +
> >       *  -  ``MEDIA_LNK_FL_LINK_TYPE``
> >          -  This is a bitmask that defines the type of the link. The following
> >            link types are currently supported:
> > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > index ef959e9bb313..b29b519237d6 100644
> > --- a/drivers/media/mc/mc-entity.c
> > +++ b/drivers/media/mc/mc-entity.c
> > @@ -772,7 +772,7 @@ static int
> >   __media_pipeline_validate_one(struct media_pad *origin,
> >                                struct media_pipeline *pipe,
> >                                struct media_pad *pad, struct media_link *link,
> > -                             bool *has_enabled_link)
> > +                             bool *has_enabled_link, bool skip_validation)
> >   {
> >          struct media_device *mdev = origin->graph_obj.mdev;
> >          struct media_entity *entity = pad->entity;
> > @@ -782,6 +782,9 @@ __media_pipeline_validate_one(struct media_pad *origin,
> >          if (link->flags & MEDIA_LNK_FL_ENABLED && has_enabled_link)
> >                  *has_enabled_link = true;
> > 
> > +       if (skip_validation)
> > +               return 0;
> > +
> >          /*
> >           * Validate the link if it's enabled and has the
> >           * current pad as its sink.
> > @@ -833,7 +836,24 @@ __must_check int __media_pipeline_start(struct media_pad *origin,
> >           * valid, so just increase the start count.
> The above comment is no longer accurate.

I'll see how to reword this for v3.

> >           */
> >          if (pipe->start_count) {
> > +               struct media_link *link;
> > +
> > +               link = __media_entity_next_link(origin->entity, NULL,
> > +                                               MEDIA_LNK_FL_DATA_LINK);
> This __media_entity_next_link() will retrieve the first data link. Is this
> guaranteed to be connected to origin pad? Is this intended, or is a check
> for (link->source == origin || link->sink == origin) also necessary?

I'd say so, when it comes to video nodes, but theoretically the flag could
be set on other entities as well, albeit probably erroneously. I think we
could add a check there's exacly one pad in the entity and that pad has a
single link.

> > +               if (link && link->flags & MEDIA_LNK_FL_VALIDATE_LATE) {
> > +                       dev_dbg(mdev->dev,
> > +                               "Validating pad '%s':%u late\n",
> > +                               origin->entity->name, origin->index);
> > +
> > +                       ret = __media_pipeline_validate_one(link->sink, pipe,
> > +                                                           link->sink, link,
> > +                                                           NULL, false);
> > +                       if (ret)
> > +                               return ret;
> > +               }
> > +
> >                  pipe->start_count++;
> > +
> >                  return 0;
> >          }
> > 
> > @@ -881,9 +901,16 @@ __must_check int __media_pipeline_start(struct media_pad *origin,
> >                          if (link->sink != pad && link->source != pad)
> >                                  continue;
> > 
> > +                       /* Skip late-validated links not connected to origin. */
> > +                       bool skip_validation =
> 
> Move skip_validation declaration at beginning of the block.

I'd think it's fine to declare it here, too, but I can move it a few lines
up as well.

> 
> Regards,
> 
> Mirela
> 
> > +                               link->flags & MEDIA_LNK_FL_VALIDATE_LATE &&
> > +                               (link->sink == origin ||
> > +                                link->source == origin);
> > +
> >                          ret = __media_pipeline_validate_one(origin, pipe,
> >                                                              origin, link,
> > -                                                           &has_enabled_link);
> > +                                                           &has_enabled_link,
> > +                                                           skip_validation);
> >                          if (ret)
> >                                  goto error;
> >                  }

-- 
Regards,

Sakari Ailus