Hi Laurent,
On Fri, Jun 02, 2023 at 12:18:44PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Sat, May 06, 2023 at 12:52:51AM +0300, Sakari Ailus wrote:
> > Internal source pads will be used as routing endpoints in V4L2
> > [GS]_ROUTING IOCTLs, to indicate that the stream begins in the entity.
> >
> > Also prevent creating links to pads that have been flagged as internal.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> > .../userspace-api/media/mediactl/media-types.rst | 7 +++++++
> > drivers/media/mc/mc-entity.c | 8 +++++++-
> > include/uapi/linux/media.h | 1 +
> > 3 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst
> > index 0ffeece1e0c8..c724139ad46c 100644
> > --- a/Documentation/userspace-api/media/mediactl/media-types.rst
> > +++ b/Documentation/userspace-api/media/mediactl/media-types.rst
> > @@ -361,6 +361,7 @@ Types and flags used to represent the media graph elements
> > .. _MEDIA-PAD-FL-SINK:
> > .. _MEDIA-PAD-FL-SOURCE:
> > .. _MEDIA-PAD-FL-MUST-CONNECT:
> > +.. _MEDIA-PAD-FL-INTERNAL-SOURCE:
> >
> > .. flat-table:: Media pad flags
> > :header-rows: 0
> > @@ -382,6 +383,12 @@ Types and flags used to represent the media graph elements
> > when this flag isn't set; the absence of the flag doesn't imply
> > there is none.
> >
> > + * - ``MEDIA_PAD_FL_INTERNAL_SOURCE``
> > + - This flag indicates an internal pad that has no external
> > + connections. Such a pad may not be connected with a link. The internal
> > + flag indicates that the stream either starts or ends in the
>
> There's no mention of "stream" (with this meaning) anywhere in the MC
> API documentation, so you will need to explain streams first.
I'll add it to glossary and add a reference here.
>
> Apart from that, and addressing Tomi's comment, this patch seems OK.
>
> > + entity. For a given pad, the INTERNAL_SOURCE flag may not be set if
> > + either SINK or SOURCE flags is set.
> >
> > One and only one of ``MEDIA_PAD_FL_SINK`` and ``MEDIA_PAD_FL_SOURCE``
> > must be set for every pad.
> > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > index ef34ddd715c9..ed99160a2487 100644
> > --- a/drivers/media/mc/mc-entity.c
> > +++ b/drivers/media/mc/mc-entity.c
> > @@ -1062,7 +1062,8 @@ int media_get_pad_index(struct media_entity *entity, u32 pad_type,
> >
> > for (i = 0; i < entity->num_pads; i++) {
> > if ((entity->pads[i].flags &
> > - (MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE)) != pad_type)
> > + (MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE |
> > + MEDIA_PAD_FL_INTERNAL_SOURCE)) != pad_type)
> > continue;
> >
> > if (entity->pads[i].sig_type == sig_type)
> > @@ -1087,6 +1088,11 @@ media_create_pad_link(struct media_entity *source, u16 source_pad,
> > return -EINVAL;
> > if (WARN_ON(!(sink->pads[sink_pad].flags & MEDIA_PAD_FL_SINK)))
> > return -EINVAL;
> > + if (WARN_ON(sink->pads[sink_pad].flags &
> > + MEDIA_PAD_FL_INTERNAL_SOURCE) ||
> > + WARN_ON(source->pads[source_pad].flags &
> > + MEDIA_PAD_FL_INTERNAL_SOURCE))
> > + return -EINVAL;
> >
> > link = media_add_link(&source->links);
> > if (link == NULL)
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index edb8dfef9eba..0e2577e8b425 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -208,6 +208,7 @@ struct media_entity_desc {
> > #define MEDIA_PAD_FL_SINK (1U << 0)
> > #define MEDIA_PAD_FL_SOURCE (1U << 1)
> > #define MEDIA_PAD_FL_MUST_CONNECT (1U << 2)
> > +#define MEDIA_PAD_FL_INTERNAL_SOURCE (1U << 3)
> >
> > struct media_pad_desc {
> > __u32 entity; /* entity ID */
>
--
Regards,
Sakari Ailus
