[PATCH 1/1] media: uapi: Add an entity type for Image Signal Processors

[PATCH 1/1] media: uapi: Add an entity type for Image Signal Processors

[Sakari Ailus]

Add and document a media entity type for Image Signal Processor devices.
Surprisingly we didn't have one, so add one now. More or less all ISP
drivers should use this type instead of what they currently are using (or
not using anything).

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/userspace-api/media/mediactl/media-types.rst | 6 ++++++
 include/uapi/linux/media.h                                 | 1 +
 2 files changed, 7 insertions(+)

diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst
index 7b24a213cae7..f18b2004de6d 100644
--- a/Documentation/userspace-api/media/mediactl/media-types.rst
+++ b/Documentation/userspace-api/media/mediactl/media-types.rst
@@ -39,6 +39,7 @@ Types and flags used to represent the media graph elements
 .. _MEDIA-ENT-F-PROC-VIDEO-STATISTICS:
 .. _MEDIA-ENT-F-PROC-VIDEO-ENCODER:
 .. _MEDIA-ENT-F-PROC-VIDEO-DECODER:
+.. _MEDIA-ENT-F-PROC-VIDEO-ISP:
 .. _MEDIA-ENT-F-VID-MUX:
 .. _MEDIA-ENT-F-VID-IF-BRIDGE:
 .. _MEDIA-ENT-F-DV-DECODER:
@@ -201,6 +202,11 @@ Types and flags used to represent the media graph elements
           decompressing a compressed video stream into uncompressed video
 	  frames. Must have one sink pad and at least one source pad.
 
+    *  -  ``MEDIA_ENT_F_PROC_VIDEO_ISP``
+       -  An Image Signal Processor (ISP) device. ISPs generally are one of a
+	  kind devices that have their specific control interfaces. Most devices
+	  have either memory or data bus input.
+
     *  -  ``MEDIA_ENT_F_VID_MUX``
        - Video multiplexer. An entity capable of multiplexing must have at
          least two sink pads and one source pad, and must pass the video
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 383ac7b7d8f0..200fa8462b90 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -127,6 +127,7 @@ struct media_device_info {
 #define MEDIA_ENT_F_PROC_VIDEO_STATISTICS	(MEDIA_ENT_F_BASE + 0x4006)
 #define MEDIA_ENT_F_PROC_VIDEO_ENCODER		(MEDIA_ENT_F_BASE + 0x4007)
 #define MEDIA_ENT_F_PROC_VIDEO_DECODER		(MEDIA_ENT_F_BASE + 0x4008)
+#define MEDIA_ENT_F_PROC_VIDEO_ISP		(MEDIA_ENT_F_BASE + 0x4009)
 
 /*
  * Switch and bridge entity functions
-- 
2.29.2

Re: [PATCH 1/1] media: uapi: Add an entity type for Image Signal Processors

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Thu, Dec 17, 2020 at 08:29:03PM +0200, Sakari Ailus wrote:
> Add and document a media entity type for Image Signal Processor devices.
> Surprisingly we didn't have one, so add one now. More or less all ISP
> drivers should use this type instead of what they currently are using (or
> not using anything).
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  Documentation/userspace-api/media/mediactl/media-types.rst | 6 ++++++
>  include/uapi/linux/media.h                                 | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst
> index 7b24a213cae7..f18b2004de6d 100644
> --- a/Documentation/userspace-api/media/mediactl/media-types.rst
> +++ b/Documentation/userspace-api/media/mediactl/media-types.rst
> @@ -39,6 +39,7 @@ Types and flags used to represent the media graph elements
>  .. _MEDIA-ENT-F-PROC-VIDEO-STATISTICS:
>  .. _MEDIA-ENT-F-PROC-VIDEO-ENCODER:
>  .. _MEDIA-ENT-F-PROC-VIDEO-DECODER:
> +.. _MEDIA-ENT-F-PROC-VIDEO-ISP:
>  .. _MEDIA-ENT-F-VID-MUX:
>  .. _MEDIA-ENT-F-VID-IF-BRIDGE:
>  .. _MEDIA-ENT-F-DV-DECODER:
> @@ -201,6 +202,11 @@ Types and flags used to represent the media graph elements
>            decompressing a compressed video stream into uncompressed video
>  	  frames. Must have one sink pad and at least one source pad.
>  
> +    *  -  ``MEDIA_ENT_F_PROC_VIDEO_ISP``
> +       -  An Image Signal Processor (ISP) device. ISPs generally are one of a
> +	  kind devices that have their specific control interfaces. Most devices
> +	  have either memory or data bus input.
> +

This looks good to me. I don't think we need to formally define what an
ISP is here, it's a fairly well understood concept in the industry and
having some flexibility in the definition is useful.

We may want to expand a bit on the "specific control interface", to tell
it's about the V4L2 API, and not the hardware interface. Maybe "...,
using a combination of custom V4L2 controls and IOCTLs, and parameters
supplied in a metadata buffer" ?

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

>      *  -  ``MEDIA_ENT_F_VID_MUX``
>         - Video multiplexer. An entity capable of multiplexing must have at
>           least two sink pads and one source pad, and must pass the video
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 383ac7b7d8f0..200fa8462b90 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -127,6 +127,7 @@ struct media_device_info {
>  #define MEDIA_ENT_F_PROC_VIDEO_STATISTICS	(MEDIA_ENT_F_BASE + 0x4006)
>  #define MEDIA_ENT_F_PROC_VIDEO_ENCODER		(MEDIA_ENT_F_BASE + 0x4007)
>  #define MEDIA_ENT_F_PROC_VIDEO_DECODER		(MEDIA_ENT_F_BASE + 0x4008)
> +#define MEDIA_ENT_F_PROC_VIDEO_ISP		(MEDIA_ENT_F_BASE + 0x4009)
>  
>  /*
>   * Switch and bridge entity functions

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/1] media: uapi: Add an entity type for Image Signal Processors

[Sakari Ailus]

On Thu, Dec 17, 2020 at 08:41:12PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thu, Dec 17, 2020 at 08:29:03PM +0200, Sakari Ailus wrote:
> > Add and document a media entity type for Image Signal Processor devices.
> > Surprisingly we didn't have one, so add one now. More or less all ISP
> > drivers should use this type instead of what they currently are using (or
> > not using anything).
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  Documentation/userspace-api/media/mediactl/media-types.rst | 6 ++++++
> >  include/uapi/linux/media.h                                 | 1 +
> >  2 files changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst
> > index 7b24a213cae7..f18b2004de6d 100644
> > --- a/Documentation/userspace-api/media/mediactl/media-types.rst
> > +++ b/Documentation/userspace-api/media/mediactl/media-types.rst
> > @@ -39,6 +39,7 @@ Types and flags used to represent the media graph elements
> >  .. _MEDIA-ENT-F-PROC-VIDEO-STATISTICS:
> >  .. _MEDIA-ENT-F-PROC-VIDEO-ENCODER:
> >  .. _MEDIA-ENT-F-PROC-VIDEO-DECODER:
> > +.. _MEDIA-ENT-F-PROC-VIDEO-ISP:
> >  .. _MEDIA-ENT-F-VID-MUX:
> >  .. _MEDIA-ENT-F-VID-IF-BRIDGE:
> >  .. _MEDIA-ENT-F-DV-DECODER:
> > @@ -201,6 +202,11 @@ Types and flags used to represent the media graph elements
> >            decompressing a compressed video stream into uncompressed video
> >  	  frames. Must have one sink pad and at least one source pad.
> >  
> > +    *  -  ``MEDIA_ENT_F_PROC_VIDEO_ISP``
> > +       -  An Image Signal Processor (ISP) device. ISPs generally are one of a
> > +	  kind devices that have their specific control interfaces. Most devices
> > +	  have either memory or data bus input.
> > +
> 
> This looks good to me. I don't think we need to formally define what an
> ISP is here, it's a fairly well understood concept in the industry and
> having some flexibility in the definition is useful.
> 
> We may want to expand a bit on the "specific control interface", to tell
> it's about the V4L2 API, and not the hardware interface. Maybe "...,
> using a combination of custom V4L2 controls and IOCTLs, and parameters
> supplied in a metadata buffer" ?
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

v2 sent.

-- 
Sakari Ailus