| Message ID | 20230802214556.180589-4-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Generic line based metadata support, internal pads | expand |
On 03/08/2023 00:45, 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@linux.intel.com> > --- > Documentation/userspace-api/media/glossary.rst | 6 ++++++ > Documentation/userspace-api/media/mediactl/media-types.rst | 6 ++++++ > drivers/media/mc/mc-entity.c | 6 +++++- > include/uapi/linux/media.h | 1 + > 4 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/Documentation/userspace-api/media/glossary.rst b/Documentation/userspace-api/media/glossary.rst > index 96a360edbf3b..f7b99a4527c7 100644 > --- a/Documentation/userspace-api/media/glossary.rst > +++ b/Documentation/userspace-api/media/glossary.rst > @@ -173,6 +173,12 @@ Glossary > An integrated circuit that integrates all components of a computer > or other electronic systems. > > +_media-glossary-stream: > + Stream > + A distinct flow of data (image data or metadata) over a media pipeline > + from source to sink. A source may be e.g. an image sensor and a sink > + e.g. a memory buffer. > + > V4L2 API > **V4L2 userspace API** > > diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst > index 0ffeece1e0c8..28941da27790 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: > > .. flat-table:: Media pad flags > :header-rows: 0 > @@ -382,6 +383,11 @@ 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`` > + - The internal flag indicates an internal pad that has no external > + connections. Such a pad shall not be connected with a link. The > + internal flag indicates that the :ref:``stream > + <media-glossary-stream>`` either starts or ends in the entity. > > 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 4991281dcccc..03a9e0b8ebab 100644 > --- a/drivers/media/mc/mc-entity.c > +++ b/drivers/media/mc/mc-entity.c > @@ -1071,7 +1071,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)) != pad_type) > continue; > > if (entity->pads[i].sig_type == sig_type) > @@ -1094,6 +1095,9 @@ media_create_pad_link(struct media_entity *source, u16 source_pad, > return -EINVAL; > if (WARN_ON(!(source->pads[source_pad].flags & MEDIA_PAD_FL_SOURCE))) > return -EINVAL; > + if (WARN_ON(source->pads[source_pad].flags & MEDIA_PAD_FL_SOURCE && > + source->pads[source_pad].flags & MEDIA_PAD_FL_INTERNAL)) > + return -EINVAL; What does this check? Shouldn't we check here if either sink or source is INTERNAL, then fail? Tomi
Moi, Thanks for the review. On Thu, Aug 03, 2023 at 10:56:35AM +0300, Tomi Valkeinen wrote: > On 03/08/2023 00:45, 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@linux.intel.com> > > --- > > Documentation/userspace-api/media/glossary.rst | 6 ++++++ > > Documentation/userspace-api/media/mediactl/media-types.rst | 6 ++++++ > > drivers/media/mc/mc-entity.c | 6 +++++- > > include/uapi/linux/media.h | 1 + > > 4 files changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/userspace-api/media/glossary.rst b/Documentation/userspace-api/media/glossary.rst > > index 96a360edbf3b..f7b99a4527c7 100644 > > --- a/Documentation/userspace-api/media/glossary.rst > > +++ b/Documentation/userspace-api/media/glossary.rst > > @@ -173,6 +173,12 @@ Glossary > > An integrated circuit that integrates all components of a computer > > or other electronic systems. > > +_media-glossary-stream: > > + Stream > > + A distinct flow of data (image data or metadata) over a media pipeline > > + from source to sink. A source may be e.g. an image sensor and a sink > > + e.g. a memory buffer. > > + > > V4L2 API > > **V4L2 userspace API** > > diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst > > index 0ffeece1e0c8..28941da27790 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: > > .. flat-table:: Media pad flags > > :header-rows: 0 > > @@ -382,6 +383,11 @@ 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`` > > + - The internal flag indicates an internal pad that has no external > > + connections. Such a pad shall not be connected with a link. The > > + internal flag indicates that the :ref:``stream > > + <media-glossary-stream>`` either starts or ends in the entity. > > 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 4991281dcccc..03a9e0b8ebab 100644 > > --- a/drivers/media/mc/mc-entity.c > > +++ b/drivers/media/mc/mc-entity.c > > @@ -1071,7 +1071,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)) != pad_type) > > continue; > > if (entity->pads[i].sig_type == sig_type) > > @@ -1094,6 +1095,9 @@ media_create_pad_link(struct media_entity *source, u16 source_pad, > > return -EINVAL; > > if (WARN_ON(!(source->pads[source_pad].flags & MEDIA_PAD_FL_SOURCE))) > > return -EINVAL; > > + if (WARN_ON(source->pads[source_pad].flags & MEDIA_PAD_FL_SOURCE && > > + source->pads[source_pad].flags & MEDIA_PAD_FL_INTERNAL)) > > + return -EINVAL; > > What does this check? > > Shouldn't we check here if either sink or source is INTERNAL, then fail? We shouldn't have any source pads that have the internal flag set, that was the intention here (but you can drop the source flag check). Also both pads can't have the internal flag, I'll add that for v3.
On 03/08/2023 11:19, Sakari Ailus wrote: > Moi, > > Thanks for the review. > > On Thu, Aug 03, 2023 at 10:56:35AM +0300, Tomi Valkeinen wrote: >> On 03/08/2023 00:45, 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@linux.intel.com> >>> --- >>> Documentation/userspace-api/media/glossary.rst | 6 ++++++ >>> Documentation/userspace-api/media/mediactl/media-types.rst | 6 ++++++ >>> drivers/media/mc/mc-entity.c | 6 +++++- >>> include/uapi/linux/media.h | 1 + >>> 4 files changed, 18 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/userspace-api/media/glossary.rst b/Documentation/userspace-api/media/glossary.rst >>> index 96a360edbf3b..f7b99a4527c7 100644 >>> --- a/Documentation/userspace-api/media/glossary.rst >>> +++ b/Documentation/userspace-api/media/glossary.rst >>> @@ -173,6 +173,12 @@ Glossary >>> An integrated circuit that integrates all components of a computer >>> or other electronic systems. >>> +_media-glossary-stream: >>> + Stream >>> + A distinct flow of data (image data or metadata) over a media pipeline >>> + from source to sink. A source may be e.g. an image sensor and a sink >>> + e.g. a memory buffer. >>> + >>> V4L2 API >>> **V4L2 userspace API** >>> diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst >>> index 0ffeece1e0c8..28941da27790 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: >>> .. flat-table:: Media pad flags >>> :header-rows: 0 >>> @@ -382,6 +383,11 @@ 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`` >>> + - The internal flag indicates an internal pad that has no external >>> + connections. Such a pad shall not be connected with a link. The >>> + internal flag indicates that the :ref:``stream >>> + <media-glossary-stream>`` either starts or ends in the entity. >>> 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 4991281dcccc..03a9e0b8ebab 100644 >>> --- a/drivers/media/mc/mc-entity.c >>> +++ b/drivers/media/mc/mc-entity.c >>> @@ -1071,7 +1071,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)) != pad_type) >>> continue; >>> if (entity->pads[i].sig_type == sig_type) >>> @@ -1094,6 +1095,9 @@ media_create_pad_link(struct media_entity *source, u16 source_pad, >>> return -EINVAL; >>> if (WARN_ON(!(source->pads[source_pad].flags & MEDIA_PAD_FL_SOURCE))) >>> return -EINVAL; >>> + if (WARN_ON(source->pads[source_pad].flags & MEDIA_PAD_FL_SOURCE && >>> + source->pads[source_pad].flags & MEDIA_PAD_FL_INTERNAL)) >>> + return -EINVAL; >> >> What does this check? >> >> Shouldn't we check here if either sink or source is INTERNAL, then fail? > > We shouldn't have any source pads that have the internal flag set, that was > the intention here (but you can drop the source flag check). Also both > pads can't have the internal flag, I'll add that for v3. The function is media_create_pad_link(), isn't that about creating links between entities? Internal pads shouldn't be allowed on either side. Tomi
On Thu, Aug 03, 2023 at 11:24:46AM +0300, Tomi Valkeinen wrote: > On 03/08/2023 11:19, Sakari Ailus wrote: > > Moi, > > > > Thanks for the review. > > > > On Thu, Aug 03, 2023 at 10:56:35AM +0300, Tomi Valkeinen wrote: > > > On 03/08/2023 00:45, 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@linux.intel.com> > > > > --- > > > > Documentation/userspace-api/media/glossary.rst | 6 ++++++ > > > > Documentation/userspace-api/media/mediactl/media-types.rst | 6 ++++++ > > > > drivers/media/mc/mc-entity.c | 6 +++++- > > > > include/uapi/linux/media.h | 1 + > > > > 4 files changed, 18 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/Documentation/userspace-api/media/glossary.rst b/Documentation/userspace-api/media/glossary.rst > > > > index 96a360edbf3b..f7b99a4527c7 100644 > > > > --- a/Documentation/userspace-api/media/glossary.rst > > > > +++ b/Documentation/userspace-api/media/glossary.rst > > > > @@ -173,6 +173,12 @@ Glossary > > > > An integrated circuit that integrates all components of a computer > > > > or other electronic systems. > > > > +_media-glossary-stream: > > > > + Stream > > > > + A distinct flow of data (image data or metadata) over a media pipeline > > > > + from source to sink. A source may be e.g. an image sensor and a sink > > > > + e.g. a memory buffer. > > > > + > > > > V4L2 API > > > > **V4L2 userspace API** > > > > diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst > > > > index 0ffeece1e0c8..28941da27790 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: > > > > .. flat-table:: Media pad flags > > > > :header-rows: 0 > > > > @@ -382,6 +383,11 @@ 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`` > > > > + - The internal flag indicates an internal pad that has no external > > > > + connections. Such a pad shall not be connected with a link. The > > > > + internal flag indicates that the :ref:``stream > > > > + <media-glossary-stream>`` either starts or ends in the entity. > > > > 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 4991281dcccc..03a9e0b8ebab 100644 > > > > --- a/drivers/media/mc/mc-entity.c > > > > +++ b/drivers/media/mc/mc-entity.c > > > > @@ -1071,7 +1071,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)) != pad_type) > > > > continue; > > > > if (entity->pads[i].sig_type == sig_type) > > > > @@ -1094,6 +1095,9 @@ media_create_pad_link(struct media_entity *source, u16 source_pad, > > > > return -EINVAL; > > > > if (WARN_ON(!(source->pads[source_pad].flags & MEDIA_PAD_FL_SOURCE))) > > > > return -EINVAL; > > > > + if (WARN_ON(source->pads[source_pad].flags & MEDIA_PAD_FL_SOURCE && > > > > + source->pads[source_pad].flags & MEDIA_PAD_FL_INTERNAL)) > > > > + return -EINVAL; > > > > > > What does this check? > > > > > > Shouldn't we check here if either sink or source is INTERNAL, then fail? > > > > We shouldn't have any source pads that have the internal flag set, that was > > the intention here (but you can drop the source flag check). Also both > > pads can't have the internal flag, I'll add that for v3. > > The function is media_create_pad_link(), isn't that about creating links > between entities? Internal pads shouldn't be allowed on either side. Uh, yes, indeed. I'll address this in v3.
diff --git a/Documentation/userspace-api/media/glossary.rst b/Documentation/userspace-api/media/glossary.rst index 96a360edbf3b..f7b99a4527c7 100644 --- a/Documentation/userspace-api/media/glossary.rst +++ b/Documentation/userspace-api/media/glossary.rst @@ -173,6 +173,12 @@ Glossary An integrated circuit that integrates all components of a computer or other electronic systems. +_media-glossary-stream: + Stream + A distinct flow of data (image data or metadata) over a media pipeline + from source to sink. A source may be e.g. an image sensor and a sink + e.g. a memory buffer. + V4L2 API **V4L2 userspace API** diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst index 0ffeece1e0c8..28941da27790 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: .. flat-table:: Media pad flags :header-rows: 0 @@ -382,6 +383,11 @@ 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`` + - The internal flag indicates an internal pad that has no external + connections. Such a pad shall not be connected with a link. The + internal flag indicates that the :ref:``stream + <media-glossary-stream>`` either starts or ends in the entity. 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 4991281dcccc..03a9e0b8ebab 100644 --- a/drivers/media/mc/mc-entity.c +++ b/drivers/media/mc/mc-entity.c @@ -1071,7 +1071,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)) != pad_type) continue; if (entity->pads[i].sig_type == sig_type) @@ -1094,6 +1095,9 @@ media_create_pad_link(struct media_entity *source, u16 source_pad, return -EINVAL; if (WARN_ON(!(source->pads[source_pad].flags & MEDIA_PAD_FL_SOURCE))) return -EINVAL; + if (WARN_ON(source->pads[source_pad].flags & MEDIA_PAD_FL_SOURCE && + source->pads[source_pad].flags & MEDIA_PAD_FL_INTERNAL)) + return -EINVAL; if (WARN_ON(!(sink->pads[sink_pad].flags & MEDIA_PAD_FL_SINK))) return -EINVAL; diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h index 1c80b1d6bbaf..80cfd12a43fc 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 (1U << 3) struct media_pad_desc { __u32 entity; /* entity ID */
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@linux.intel.com> --- Documentation/userspace-api/media/glossary.rst | 6 ++++++ Documentation/userspace-api/media/mediactl/media-types.rst | 6 ++++++ drivers/media/mc/mc-entity.c | 6 +++++- include/uapi/linux/media.h | 1 + 4 files changed, 18 insertions(+), 1 deletion(-)