[PATCH 0/6] Add new bayer ir formats

[PATCH 0/6] Add new bayer ir formats

[Marco Felsch]

Hi all,

this adds the inital support for a new sensor format called RGB-IR
found on sensors from OnSemi [1]. This is a new custom bayer format with
interleaved ir pixels. For more information see the documentation
commit.

[1] https://www.onsemi.com/products/sensors/image-sensors-processors/image-sensors/ar0237sr

Marco Felsch (6):
  media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats
  media: v4l: Add definition for bayered IR formats
  media: v4l2-ioctl.c: add V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG to
    v4l_fill_fmtdesc
  media: video-mux: add new SGRGB_IGIG_GBGR_IGIG format support
  gpu: ipu-v3: add custom SGRGB_IGIG_GBGR_IGIG format support
  media: imx: csi: add custom SGRGB_IGIG_GBGR_IGIG format support

 .../media/v4l/subdev-formats.rst              | 42 +++++++++++++++++++
 drivers/gpu/ipu-v3/ipu-cpmem.c                |  2 +
 drivers/gpu/ipu-v3/ipu-csi.c                  |  2 +
 drivers/media/platform/video-mux.c            |  2 +
 drivers/media/v4l2-core/v4l2-ioctl.c          |  2 +
 drivers/staging/media/imx/imx-media-csi.c     |  2 +
 drivers/staging/media/imx/imx-media-utils.c   | 12 ++++++
 include/uapi/linux/media-bus-format.h         |  4 +-
 include/uapi/linux/videodev2.h                |  4 ++
 9 files changed, 71 insertions(+), 1 deletion(-)

-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Marco Felsch]

Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
with the interleaved IR pixels looks like:

        |  G |  R |  G |  B | ...
        +----+----+----+----+---
        | IR |  G | IR |  G | ...
        +----+----+----+----+---
        |  G |  B |  G |  R | ...
        +----+----+----+----+---
        | IR |  G | IR |  G | ...
        +----+----+----+----+---
        | .. | .. | .. | .. | ..

[1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 .../media/v4l/subdev-formats.rst              | 42 +++++++++++++++++++
 include/uapi/linux/media-bus-format.h         |  4 +-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index bd68588b2683..d774ccd57c1b 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -2252,6 +2252,27 @@ organization is given as an example for the first pixel only.
       - g\ :sub:`2`
       - g\ :sub:`1`
       - g\ :sub:`0`
+    * .. _MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG8_1X8:
+
+      - MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG8_1X8
+      - 0x3021
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      - g\ :sub:`7`
+      - g\ :sub:`6`
+      - g\ :sub:`5`
+      - g\ :sub:`4`
+      - g\ :sub:`3`
+      - g\ :sub:`2`
+      - g\ :sub:`1`
+      - g\ :sub:`0`
     * .. _MEDIA-BUS-FMT-SRGGB8-1X8:
 
       - MEDIA_BUS_FMT_SRGGB8_1X8
@@ -2748,6 +2769,27 @@ organization is given as an example for the first pixel only.
       - g\ :sub:`2`
       - g\ :sub:`1`
       - g\ :sub:`0`
+    * .. _MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG12_1X12:
+
+      - MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG12_1X12
+      - 0x3022
+      -
+      -
+      -
+      -
+      -
+      - g\ :sub:`11`
+      - g\ :sub:`10`
+      - g\ :sub:`9`
+      - g\ :sub:`8`
+      - g\ :sub:`7`
+      - g\ :sub:`6`
+      - g\ :sub:`5`
+      - g\ :sub:`4`
+      - g\ :sub:`3`
+      - g\ :sub:`2`
+      - g\ :sub:`1`
+      - g\ :sub:`0`
     * .. _MEDIA-BUS-FMT-SRGGB12-1X12:
 
       - MEDIA_BUS_FMT_SRGGB12_1X12
diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h
index 0dfc11ee243a..cdd995e44926 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -112,10 +112,11 @@
 #define MEDIA_BUS_FMT_YUV16_1X48		0x202a
 #define MEDIA_BUS_FMT_UYYVYY16_0_5X48		0x202b
 
-/* Bayer - next is	0x3021 */
+/* Bayer - next is	0x3023 */
 #define MEDIA_BUS_FMT_SBGGR8_1X8		0x3001
 #define MEDIA_BUS_FMT_SGBRG8_1X8		0x3013
 #define MEDIA_BUS_FMT_SGRBG8_1X8		0x3002
+#define MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG8_1X8	0x3021
 #define MEDIA_BUS_FMT_SRGGB8_1X8		0x3014
 #define MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8		0x3015
 #define MEDIA_BUS_FMT_SGBRG10_ALAW8_1X8		0x3016
@@ -136,6 +137,7 @@
 #define MEDIA_BUS_FMT_SBGGR12_1X12		0x3008
 #define MEDIA_BUS_FMT_SGBRG12_1X12		0x3010
 #define MEDIA_BUS_FMT_SGRBG12_1X12		0x3011
+#define MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG12_1X12	0x3022
 #define MEDIA_BUS_FMT_SRGGB12_1X12		0x3012
 #define MEDIA_BUS_FMT_SBGGR14_1X14		0x3019
 #define MEDIA_BUS_FMT_SGBRG14_1X14		0x301a
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Laurent Pinchart]

Hi Marco,

Thank you for the patch.

On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:
> Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> with the interleaved IR pixels looks like:
> 
>         |  G |  R |  G |  B | ...
>         +----+----+----+----+---
>         | IR |  G | IR |  G | ...
>         +----+----+----+----+---
>         |  G |  B |  G |  R | ...
>         +----+----+----+----+---
>         | IR |  G | IR |  G | ...
>         +----+----+----+----+---
>         | .. | .. | .. | .. | ..
> 
> [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf

I think we're reaching a limit of the media bus codes model here, due to
a historical mistake. The four possible Bayer patterns, times the
different number of bits per pixel, creates a lot of media bus codes,
and drivers for CSI-2 receivers and IP cores further down the pipeline
have to support them all. This is already painful, and if we had a
non-Bayer pattern such as this one, we'll open the door to an explosion
of the number of media bus codes (imagine all the different possible
arrangements of this pattern, for instance if you enable horizontal
and/or vertical flipping on the sensor). All drivers would need to be
updated to support these new bus codes, and this really kills the
current model.

The historical mistake was to tie the Bayer pattern with the media bus
code. We should really have specified raw 8/10/12/14/16 media bus codes,
and conveyed the pattern separately. Most IP cores in the pipeline don't
need to care about the pattern at all, and those who do (that's mostly
ISPs) could be programmed explicitly by userspace as long as we have an
API to retrieve the pattern from the sensor. I believe it's time to bite
the bullet and go in that direction. I'm sorry for this case of yak
shaving, but it really wouldn't be manageable anymore :-(

> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  .../media/v4l/subdev-formats.rst              | 42 +++++++++++++++++++
>  include/uapi/linux/media-bus-format.h         |  4 +-
>  2 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> index bd68588b2683..d774ccd57c1b 100644
> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> @@ -2252,6 +2252,27 @@ organization is given as an example for the first pixel only.
>        - g\ :sub:`2`
>        - g\ :sub:`1`
>        - g\ :sub:`0`
> +    * .. _MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG8_1X8:
> +
> +      - MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG8_1X8
> +      - 0x3021
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      - g\ :sub:`7`
> +      - g\ :sub:`6`
> +      - g\ :sub:`5`
> +      - g\ :sub:`4`
> +      - g\ :sub:`3`
> +      - g\ :sub:`2`
> +      - g\ :sub:`1`
> +      - g\ :sub:`0`
>      * .. _MEDIA-BUS-FMT-SRGGB8-1X8:
>  
>        - MEDIA_BUS_FMT_SRGGB8_1X8
> @@ -2748,6 +2769,27 @@ organization is given as an example for the first pixel only.
>        - g\ :sub:`2`
>        - g\ :sub:`1`
>        - g\ :sub:`0`
> +    * .. _MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG12_1X12:
> +
> +      - MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG12_1X12
> +      - 0x3022
> +      -
> +      -
> +      -
> +      -
> +      -
> +      - g\ :sub:`11`
> +      - g\ :sub:`10`
> +      - g\ :sub:`9`
> +      - g\ :sub:`8`
> +      - g\ :sub:`7`
> +      - g\ :sub:`6`
> +      - g\ :sub:`5`
> +      - g\ :sub:`4`
> +      - g\ :sub:`3`
> +      - g\ :sub:`2`
> +      - g\ :sub:`1`
> +      - g\ :sub:`0`
>      * .. _MEDIA-BUS-FMT-SRGGB12-1X12:
>  
>        - MEDIA_BUS_FMT_SRGGB12_1X12
> diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h
> index 0dfc11ee243a..cdd995e44926 100644
> --- a/include/uapi/linux/media-bus-format.h
> +++ b/include/uapi/linux/media-bus-format.h
> @@ -112,10 +112,11 @@
>  #define MEDIA_BUS_FMT_YUV16_1X48		0x202a
>  #define MEDIA_BUS_FMT_UYYVYY16_0_5X48		0x202b
>  
> -/* Bayer - next is	0x3021 */
> +/* Bayer - next is	0x3023 */
>  #define MEDIA_BUS_FMT_SBGGR8_1X8		0x3001
>  #define MEDIA_BUS_FMT_SGBRG8_1X8		0x3013
>  #define MEDIA_BUS_FMT_SGRBG8_1X8		0x3002
> +#define MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG8_1X8	0x3021
>  #define MEDIA_BUS_FMT_SRGGB8_1X8		0x3014
>  #define MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8		0x3015
>  #define MEDIA_BUS_FMT_SGBRG10_ALAW8_1X8		0x3016
> @@ -136,6 +137,7 @@
>  #define MEDIA_BUS_FMT_SBGGR12_1X12		0x3008
>  #define MEDIA_BUS_FMT_SGBRG12_1X12		0x3010
>  #define MEDIA_BUS_FMT_SGRBG12_1X12		0x3011
> +#define MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG12_1X12	0x3022
>  #define MEDIA_BUS_FMT_SRGGB12_1X12		0x3012
>  #define MEDIA_BUS_FMT_SBGGR14_1X14		0x3019
>  #define MEDIA_BUS_FMT_SGBRG14_1X14		0x301a

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Marco Felsch]

Hi Laurent,

On 21-04-29 04:51, Laurent Pinchart wrote:
> Hi Marco,
> 
> Thank you for the patch.
> 
> On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:
> > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > with the interleaved IR pixels looks like:
> > 
> >         |  G |  R |  G |  B | ...
> >         +----+----+----+----+---
> >         | IR |  G | IR |  G | ...
> >         +----+----+----+----+---
> >         |  G |  B |  G |  R | ...
> >         +----+----+----+----+---
> >         | IR |  G | IR |  G | ...
> >         +----+----+----+----+---
> >         | .. | .. | .. | .. | ..
> > 
> > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf
> 
> I think we're reaching a limit of the media bus codes model here, due to
> a historical mistake. The four possible Bayer patterns, times the
> different number of bits per pixel, creates a lot of media bus codes,
> and drivers for CSI-2 receivers and IP cores further down the pipeline
> have to support them all.

That's correct but it is not bayer related. Currently it is what it is,
if a new code is added it must be propagated through all the involved
subdevs. On the other hand I wouldn't say that it is better to support
new codes per default for all drivers. Since this would add a lot of
untested code paths.

> This is already painful, and if we had a
> non-Bayer pattern such as this one,

That's not exactly true since it is a bayer pattern but instead of using
4x4 it uses 8x8 and it as some special pixels.

> we'll open the door to an explosion
> of the number of media bus codes (imagine all the different possible
> arrangements of this pattern, for instance if you enable horizontal
> and/or vertical flipping on the sensor). All drivers would need to be
> updated to support these new bus codes, and this really kills the
> current model.

Yep, I know what you mean but as I said above I think that adding it
explicite is the better abbroach since it involves somone who add _and_
test the new code on the particular platform.

> The historical mistake was to tie the Bayer pattern with the media bus
> code. We should really have specified raw 8/10/12/14/16 media bus codes,
> and conveyed the pattern separately. Most IP cores in the pipeline don't
> need to care about the pattern at all, and those who do (that's mostly
> ISPs) could be programmed explicitly by userspace as long as we have an
> API to retrieve the pattern from the sensor. I believe it's time to bite
> the bullet and go in that direction. I'm sorry for this case of yak
> shaving, but it really wouldn't be manageable anymore :-(

I got all your points and would agree but this is not a bayer only
related problem. You will have this problem with all new other formats
as well. I'm with you, most IP cores don't care but I wouldn't
guarantee that.

Regards,
  Marco

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Mauro Carvalho Chehab]

Em Thu, 29 Apr 2021 09:49:03 +0200
Marco Felsch <m.felsch@pengutronix.de> escreveu:

> Hi Laurent,
> 
> On 21-04-29 04:51, Laurent Pinchart wrote:
> > Hi Marco,
> > 
> > Thank you for the patch.
> > 
> > On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:  
> > > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > > with the interleaved IR pixels looks like:
> > > 
> > >         |  G |  R |  G |  B | ...
> > >         +----+----+----+----+---
> > >         | IR |  G | IR |  G | ...
> > >         +----+----+----+----+---
> > >         |  G |  B |  G |  R | ...
> > >         +----+----+----+----+---
> > >         | IR |  G | IR |  G | ...
> > >         +----+----+----+----+---
> > >         | .. | .. | .. | .. | ..
> > > 
> > > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf  
> > 
> > I think we're reaching a limit of the media bus codes model here, due to
> > a historical mistake. The four possible Bayer patterns, times the
> > different number of bits per pixel, creates a lot of media bus codes,
> > and drivers for CSI-2 receivers and IP cores further down the pipeline
> > have to support them all.  
> 
> That's correct but it is not bayer related.

Err... there are two separate things here:

1) for the uAPI part, we're not even close to the limit of a 4-bytes
   fourcc;

2) the kAPI is currently sharing the same fourcc from the uAPI,
   because it is a lot simpler than doing something different.

Yet, nothing prevents that the kAPI could be improved in order to
better describe each format, provided that:

1. there will be a 1:1 mapping with the uAPI.
   In other words, it would be possible to write a function that
   would convert a "struct foo" from/to a 32-bits fourcc;

2. such new kAPI should be optional, as usually only drivers
   for hardware with ISP (plus the UVC driver) would be flexible
   enough to allow random formats. For the rest, the bridges are
   typically limited to support only a few formats, so it doesn't
   make sense to modify existing drivers to use such new kAPI.

Ok, someone has to come up with a proposal about what a "struct foo"
would contain. On a real quick brainstorm, something like this could
be a start:

enum v4l2_pixformat_type {
	VIDEO_PIXFORMAT_RGB,
	VIDEO_PIXFORMAT_YUV,
	VIDEO_PIXFORMAT_COMPRESSED,
	VIDEO_PIXFORMAT_BAYER_RGB,
	VIDEO_PIXFORMAT_BAYER_RGB_IR,
};

struct v4l2_pixformat_desc {
	enum v4l2_pixformat_type	pixfmt_type;
	bool				is_packed;
	int				bits_per_component;

	union {
		enum v4l2_pixformat_rgb_order		rgb_order;
		enum v4l2_pixformat_yuv_order		yuv_order;
		enum v4l2_pixformat_bayer_rgb_order	bayer_rgb_order;
		enum v4l2_pixformat_bayer_rgb_ir_order	bayer_rgb_ir_order;
		enum v4l2_pixformat_compress_type	compress_type;
	};
	...
};

Btw, I remember someone suggested a model similar to that in the past,
shared with DRM. Well, I don't think it would be easy to share
an internal subsystem-specific kAPI like that with other subsystems,
as this is the kind of thing that people will change from time to time,
and coordinating something like that can be painful, but we can try
to fork the model applied to DRM on media. For instance, I doubt that
Bayer RGB + IR would make any sense for DRM drivers.

Thanks,
Mauro

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Laurent Pinchart]

Hi Mauro,

On Thu, Apr 29, 2021 at 10:44:41AM +0200, Mauro Carvalho Chehab wrote:
> Em Thu, 29 Apr 2021 09:49:03 +0200 Marco Felsch escreveu:
> > On 21-04-29 04:51, Laurent Pinchart wrote:
> > > On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:  
> > > > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > > > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > > > with the interleaved IR pixels looks like:
> > > > 
> > > >         |  G |  R |  G |  B | ...
> > > >         +----+----+----+----+---
> > > >         | IR |  G | IR |  G | ...
> > > >         +----+----+----+----+---
> > > >         |  G |  B |  G |  R | ...
> > > >         +----+----+----+----+---
> > > >         | IR |  G | IR |  G | ...
> > > >         +----+----+----+----+---
> > > >         | .. | .. | .. | .. | ..
> > > > 
> > > > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf  
> > > 
> > > I think we're reaching a limit of the media bus codes model here, due to
> > > a historical mistake. The four possible Bayer patterns, times the
> > > different number of bits per pixel, creates a lot of media bus codes,
> > > and drivers for CSI-2 receivers and IP cores further down the pipeline
> > > have to support them all.  
> > 
> > That's correct but it is not bayer related.
> 
> Err... there are two separate things here:
> 
> 1) for the uAPI part, we're not even close to the limit of a 4-bytes
>    fourcc;
> 
> 2) the kAPI is currently sharing the same fourcc from the uAPI,
>    because it is a lot simpler than doing something different.

Please note that we're talking about media bus codes here, not pixel
formats. Both are part of the UAPI though, and pixel formats suffer from
a similar issue, but I'd like to focus on the media bus codes first.

> Yet, nothing prevents that the kAPI could be improved in order to
> better describe each format, provided that:
> 
> 1. there will be a 1:1 mapping with the uAPI.
>    In other words, it would be possible to write a function that
>    would convert a "struct foo" from/to a 32-bits fourcc;
> 
> 2. such new kAPI should be optional, as usually only drivers
>    for hardware with ISP (plus the UVC driver) would be flexible
>    enough to allow random formats. For the rest, the bridges are
>    typically limited to support only a few formats, so it doesn't
>    make sense to modify existing drivers to use such new kAPI.
> 
> Ok, someone has to come up with a proposal about what a "struct foo"
> would contain. On a real quick brainstorm, something like this could
> be a start:
> 
> enum v4l2_pixformat_type {
> 	VIDEO_PIXFORMAT_RGB,
> 	VIDEO_PIXFORMAT_YUV,
> 	VIDEO_PIXFORMAT_COMPRESSED,
> 	VIDEO_PIXFORMAT_BAYER_RGB,
> 	VIDEO_PIXFORMAT_BAYER_RGB_IR,
> };
> 
> struct v4l2_pixformat_desc {
> 	enum v4l2_pixformat_type	pixfmt_type;
> 	bool				is_packed;
> 	int				bits_per_component;
> 
> 	union {
> 		enum v4l2_pixformat_rgb_order		rgb_order;
> 		enum v4l2_pixformat_yuv_order		yuv_order;
> 		enum v4l2_pixformat_bayer_rgb_order	bayer_rgb_order;
> 		enum v4l2_pixformat_bayer_rgb_ir_order	bayer_rgb_ir_order;
> 		enum v4l2_pixformat_compress_type	compress_type;
> 	};
> 	...
> };
> 
> Btw, I remember someone suggested a model similar to that in the past,
> shared with DRM. Well, I don't think it would be easy to share
> an internal subsystem-specific kAPI like that with other subsystems,
> as this is the kind of thing that people will change from time to time,
> and coordinating something like that can be painful, but we can try
> to fork the model applied to DRM on media. For instance, I doubt that
> Bayer RGB + IR would make any sense for DRM drivers.

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Mauro Carvalho Chehab]

Em Thu, 29 Apr 2021 19:53:33 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> Hi Mauro,
> 
> On Thu, Apr 29, 2021 at 10:44:41AM +0200, Mauro Carvalho Chehab wrote:
> > Em Thu, 29 Apr 2021 09:49:03 +0200 Marco Felsch escreveu:  
> > > On 21-04-29 04:51, Laurent Pinchart wrote:  
> > > > On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:    
> > > > > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > > > > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > > > > with the interleaved IR pixels looks like:
> > > > > 
> > > > >         |  G |  R |  G |  B | ...
> > > > >         +----+----+----+----+---
> > > > >         | IR |  G | IR |  G | ...
> > > > >         +----+----+----+----+---
> > > > >         |  G |  B |  G |  R | ...
> > > > >         +----+----+----+----+---
> > > > >         | IR |  G | IR |  G | ...
> > > > >         +----+----+----+----+---
> > > > >         | .. | .. | .. | .. | ..
> > > > > 
> > > > > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf    
> > > > 
> > > > I think we're reaching a limit of the media bus codes model here, due to
> > > > a historical mistake. The four possible Bayer patterns, times the
> > > > different number of bits per pixel, creates a lot of media bus codes,
> > > > and drivers for CSI-2 receivers and IP cores further down the pipeline
> > > > have to support them all.    
> > > 
> > > That's correct but it is not bayer related.  
> > 
> > Err... there are two separate things here:
> > 
> > 1) for the uAPI part, we're not even close to the limit of a 4-bytes
> >    fourcc;
> > 
> > 2) the kAPI is currently sharing the same fourcc from the uAPI,
> >    because it is a lot simpler than doing something different.  
> 
> Please note that we're talking about media bus codes here, not pixel
> formats. Both are part of the UAPI though, and pixel formats suffer from
> a similar issue, but I'd like to focus on the media bus codes first.

Yes, I'm aware of that, but the same principle used by fourcc pixel
formats can also be applied to media bus codes and vice versa[1].

[1] IMO, a kAPI change like that should consider the big picture, 
    and allow using the same process for both, even if we start
    implementing it for media bus (where it makes more sense).

On both cases, we're talking about a 32-bit code (either encoded
as fourcc or via MEDIA_BUS_FMT_* codespace).

Both can be 1:1 mapped to some structure similar to:

enum v4l2_pixformat_type {
	VIDEO_PIXFORMAT_RGB,
	VIDEO_PIXFORMAT_YUV,
	VIDEO_PIXFORMAT_COMPRESSED,
	VIDEO_PIXFORMAT_BAYER_RGB,
	VIDEO_PIXFORMAT_BAYER_RGB_IR,
};
 
struct v4l2_pixformat_desc {
	enum v4l2_pixformat_type	pixfmt_type;
	bool				is_packed;
 	int				bits_per_component;
 
	union {
		enum v4l2_pixformat_rgb_order		rgb_order;
		enum v4l2_pixformat_yuv_order		yuv_order;
		enum v4l2_pixformat_bayer_rgb_order	bayer_rgb_order;
		enum v4l2_pixformat_bayer_rgb_ir_order	bayer_rgb_ir_order;
		enum v4l2_pixformat_compress_type	compress_type;
	};
	...
};

And new drivers can use such struct, instead of handling the
fourcc/mbus code directly.

Also, this can be gradually implemented, in order to avoid the
need of touching at the existing drivers.

Thanks,
Mauro

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Laurent Pinchart]

Hi Marco,

On Thu, Apr 29, 2021 at 09:49:03AM +0200, Marco Felsch wrote:
> On 21-04-29 04:51, Laurent Pinchart wrote:
> > On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:
> > > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > > with the interleaved IR pixels looks like:
> > > 
> > >         |  G |  R |  G |  B | ...
> > >         +----+----+----+----+---
> > >         | IR |  G | IR |  G | ...
> > >         +----+----+----+----+---
> > >         |  G |  B |  G |  R | ...
> > >         +----+----+----+----+---
> > >         | IR |  G | IR |  G | ...
> > >         +----+----+----+----+---
> > >         | .. | .. | .. | .. | ..
> > > 
> > > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf
> > 
> > I think we're reaching a limit of the media bus codes model here, due to
> > a historical mistake. The four possible Bayer patterns, times the
> > different number of bits per pixel, creates a lot of media bus codes,
> > and drivers for CSI-2 receivers and IP cores further down the pipeline
> > have to support them all.
> 
> That's correct but it is not bayer related. Currently it is what it is,
> if a new code is added it must be propagated through all the involved
> subdevs. On the other hand I wouldn't say that it is better to support
> new codes per default for all drivers. Since this would add a lot of
> untested code paths.

It's not an issue limited to Bayer patterns, but they make the issue
worse given the number of possible combinations (think about adding DPCM
and ALAW compression on top of that...).

> > This is already painful, and if we had a
> > non-Bayer pattern such as this one,
> 
> That's not exactly true since it is a bayer pattern but instead of using
> 4x4 it uses 8x8 and it as some special pixels.
> 
> > we'll open the door to an explosion
> > of the number of media bus codes (imagine all the different possible
> > arrangements of this pattern, for instance if you enable horizontal
> > and/or vertical flipping on the sensor). All drivers would need to be
> > updated to support these new bus codes, and this really kills the
> > current model.
> 
> Yep, I know what you mean but as I said above I think that adding it
> explicite is the better abbroach since it involves somone who add _and_
> test the new code on the particular platform.
> 
> > The historical mistake was to tie the Bayer pattern with the media bus
> > code. We should really have specified raw 8/10/12/14/16 media bus codes,
> > and conveyed the pattern separately. Most IP cores in the pipeline don't
> > need to care about the pattern at all, and those who do (that's mostly
> > ISPs) could be programmed explicitly by userspace as long as we have an
> > API to retrieve the pattern from the sensor. I believe it's time to bite
> > the bullet and go in that direction. I'm sorry for this case of yak
> > shaving, but it really wouldn't be manageable anymore :-(
> 
> I got all your points and would agree but this is not a bayer only
> related problem. You will have this problem with all new other formats
> as well. I'm with you, most IP cores don't care but I wouldn't
> guarantee that.

Sorry, but adding new media bus formats like this one will just not
scale. We have two options, either fixing the issue, or considering that
V4L2 is a barely alive API with no future, and merging this without
caring anymore.

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Marco Felsch]

Hi Laurent,

On 21-04-30 01:14, Laurent Pinchart wrote:
> Hi Marco,
> 
> On Thu, Apr 29, 2021 at 09:49:03AM +0200, Marco Felsch wrote:
> > On 21-04-29 04:51, Laurent Pinchart wrote:
> > > On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:
> > > > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > > > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > > > with the interleaved IR pixels looks like:
> > > > 
> > > >         |  G |  R |  G |  B | ...
> > > >         +----+----+----+----+---
> > > >         | IR |  G | IR |  G | ...
> > > >         +----+----+----+----+---
> > > >         |  G |  B |  G |  R | ...
> > > >         +----+----+----+----+---
> > > >         | IR |  G | IR |  G | ...
> > > >         +----+----+----+----+---
> > > >         | .. | .. | .. | .. | ..
> > > > 
> > > > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf
> > > 
> > > I think we're reaching a limit of the media bus codes model here, due to
> > > a historical mistake. The four possible Bayer patterns, times the
> > > different number of bits per pixel, creates a lot of media bus codes,
> > > and drivers for CSI-2 receivers and IP cores further down the pipeline
> > > have to support them all.
> > 
> > That's correct but it is not bayer related. Currently it is what it is,
> > if a new code is added it must be propagated through all the involved
> > subdevs. On the other hand I wouldn't say that it is better to support
> > new codes per default for all drivers. Since this would add a lot of
> > untested code paths.
> 
> It's not an issue limited to Bayer patterns, but they make the issue
> worse given the number of possible combinations (think about adding DPCM
> and ALAW compression on top of that...).

You're right and again this will apply to all mbus formats...

> > > This is already painful, and if we had a
> > > non-Bayer pattern such as this one,
> > 
> > That's not exactly true since it is a bayer pattern but instead of using
> > 4x4 it uses 8x8 and it as some special pixels.
> > 
> > > we'll open the door to an explosion
> > > of the number of media bus codes (imagine all the different possible
> > > arrangements of this pattern, for instance if you enable horizontal
> > > and/or vertical flipping on the sensor). All drivers would need to be
> > > updated to support these new bus codes, and this really kills the
> > > current model.
> > 
> > Yep, I know what you mean but as I said above I think that adding it
> > explicite is the better abbroach since it involves somone who add _and_
> > test the new code on the particular platform.
> > 
> > > The historical mistake was to tie the Bayer pattern with the media bus
> > > code. We should really have specified raw 8/10/12/14/16 media bus codes,
> > > and conveyed the pattern separately. Most IP cores in the pipeline don't
> > > need to care about the pattern at all, and those who do (that's mostly
> > > ISPs) could be programmed explicitly by userspace as long as we have an
> > > API to retrieve the pattern from the sensor. I believe it's time to bite
> > > the bullet and go in that direction. I'm sorry for this case of yak
> > > shaving, but it really wouldn't be manageable anymore :-(
> > 
> > I got all your points and would agree but this is not a bayer only
> > related problem. You will have this problem with all new other formats
> > as well. I'm with you, most IP cores don't care but I wouldn't
> > guarantee that.
> 
> Sorry, but adding new media bus formats like this one will just not
> scale. We have two options, either fixing the issue, or considering that
> V4L2 is a barely alive API with no future, and merging this without
> caring anymore.

Hm.. you're right that it doesn't scale, as I said I'm absolute on your
side. So let us consider a new approach @Mauro, @Hans, @Sailus what do
you think about?

BTW: IMHO videobuf2 interface isn't that good as well, since you are
blaming ;)

Regards,
  Marco

> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Laurent Pinchart]

Hi Marco,

On Fri, Apr 30, 2021 at 08:51:34AM +0200, Marco Felsch wrote:
> On 21-04-30 01:14, Laurent Pinchart wrote:
> > On Thu, Apr 29, 2021 at 09:49:03AM +0200, Marco Felsch wrote:
> > > On 21-04-29 04:51, Laurent Pinchart wrote:
> > > > On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:
> > > > > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > > > > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > > > > with the interleaved IR pixels looks like:
> > > > > 
> > > > >         |  G |  R |  G |  B | ...
> > > > >         +----+----+----+----+---
> > > > >         | IR |  G | IR |  G | ...
> > > > >         +----+----+----+----+---
> > > > >         |  G |  B |  G |  R | ...
> > > > >         +----+----+----+----+---
> > > > >         | IR |  G | IR |  G | ...
> > > > >         +----+----+----+----+---
> > > > >         | .. | .. | .. | .. | ..
> > > > > 
> > > > > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf
> > > > 
> > > > I think we're reaching a limit of the media bus codes model here, due to
> > > > a historical mistake. The four possible Bayer patterns, times the
> > > > different number of bits per pixel, creates a lot of media bus codes,
> > > > and drivers for CSI-2 receivers and IP cores further down the pipeline
> > > > have to support them all.
> > > 
> > > That's correct but it is not bayer related. Currently it is what it is,
> > > if a new code is added it must be propagated through all the involved
> > > subdevs. On the other hand I wouldn't say that it is better to support
> > > new codes per default for all drivers. Since this would add a lot of
> > > untested code paths.
> > 
> > It's not an issue limited to Bayer patterns, but they make the issue
> > worse given the number of possible combinations (think about adding DPCM
> > and ALAW compression on top of that...).
> 
> You're right and again this will apply to all mbus formats...
> 
> > > > This is already painful, and if we had a
> > > > non-Bayer pattern such as this one,
> > > 
> > > That's not exactly true since it is a bayer pattern but instead of using
> > > 4x4 it uses 8x8 and it as some special pixels.
> > > 
> > > > we'll open the door to an explosion
> > > > of the number of media bus codes (imagine all the different possible
> > > > arrangements of this pattern, for instance if you enable horizontal
> > > > and/or vertical flipping on the sensor). All drivers would need to be
> > > > updated to support these new bus codes, and this really kills the
> > > > current model.
> > > 
> > > Yep, I know what you mean but as I said above I think that adding it
> > > explicite is the better abbroach since it involves somone who add _and_
> > > test the new code on the particular platform.
> > > 
> > > > The historical mistake was to tie the Bayer pattern with the media bus
> > > > code. We should really have specified raw 8/10/12/14/16 media bus codes,
> > > > and conveyed the pattern separately. Most IP cores in the pipeline don't
> > > > need to care about the pattern at all, and those who do (that's mostly
> > > > ISPs) could be programmed explicitly by userspace as long as we have an
> > > > API to retrieve the pattern from the sensor. I believe it's time to bite
> > > > the bullet and go in that direction. I'm sorry for this case of yak
> > > > shaving, but it really wouldn't be manageable anymore :-(
> > > 
> > > I got all your points and would agree but this is not a bayer only
> > > related problem. You will have this problem with all new other formats
> > > as well. I'm with you, most IP cores don't care but I wouldn't
> > > guarantee that.
> > 
> > Sorry, but adding new media bus formats like this one will just not
> > scale. We have two options, either fixing the issue, or considering that
> > V4L2 is a barely alive API with no future, and merging this without
> > caring anymore.
> 
> Hm.. you're right that it doesn't scale, as I said I'm absolute on your
> side. So let us consider a new approach @Mauro, @Hans, @Sailus what do
> you think about?

Starting brainstorming, how about new media bus codes for
raw{8,10,12,14,16}, and a read-only CFA pattern control to retrieve the
pattern from the sensor subdev ? We could use the same control to set
the pattern on subdevs that require it, which would mostly be ISPs. As
ISPs are configured using parameter buffers these days, it may be better
to pass the pattern in the parameter buffer instead though.

This shouldn't be too hard to implement, but the devil is of course in
the details, and we should consider how to handle the pattern control
when flipping and/or cropping is configured on the sensor.

> BTW: IMHO videobuf2 interface isn't that good as well, since you are
> blaming ;)

Have you looked at videobuf1 ? ;-) Jokes aside, there's certainly room
for improvement, but it hasn't struck me as a particularly bad part of
the framework. Is there anything in particular you think is painful ?

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Mauro Carvalho Chehab]

Em Fri, 30 Apr 2021 15:18:52 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> Hi Marco,
> 
> On Fri, Apr 30, 2021 at 08:51:34AM +0200, Marco Felsch wrote:
> > On 21-04-30 01:14, Laurent Pinchart wrote:  
> > > On Thu, Apr 29, 2021 at 09:49:03AM +0200, Marco Felsch wrote:  
> > > > On 21-04-29 04:51, Laurent Pinchart wrote:  
> > > > > On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:  
> > > > > > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > > > > > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > > > > > with the interleaved IR pixels looks like:
> > > > > > 
> > > > > >         |  G |  R |  G |  B | ...
> > > > > >         +----+----+----+----+---
> > > > > >         | IR |  G | IR |  G | ...
> > > > > >         +----+----+----+----+---
> > > > > >         |  G |  B |  G |  R | ...
> > > > > >         +----+----+----+----+---
> > > > > >         | IR |  G | IR |  G | ...
> > > > > >         +----+----+----+----+---
> > > > > >         | .. | .. | .. | .. | ..
> > > > > > 
> > > > > > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf  
> > > > > 
> > > > > I think we're reaching a limit of the media bus codes model here, due to
> > > > > a historical mistake. The four possible Bayer patterns, times the
> > > > > different number of bits per pixel, creates a lot of media bus codes,
> > > > > and drivers for CSI-2 receivers and IP cores further down the pipeline
> > > > > have to support them all.  
> > > > 
> > > > That's correct but it is not bayer related. Currently it is what it is,
> > > > if a new code is added it must be propagated through all the involved
> > > > subdevs. On the other hand I wouldn't say that it is better to support
> > > > new codes per default for all drivers. Since this would add a lot of
> > > > untested code paths.  
> > > 
> > > It's not an issue limited to Bayer patterns, but they make the issue
> > > worse given the number of possible combinations (think about adding DPCM
> > > and ALAW compression on top of that...).  
> > 
> > You're right and again this will apply to all mbus formats...
> >   
> > > > > This is already painful, and if we had a
> > > > > non-Bayer pattern such as this one,  
> > > > 
> > > > That's not exactly true since it is a bayer pattern but instead of using
> > > > 4x4 it uses 8x8 and it as some special pixels.
> > > >   
> > > > > we'll open the door to an explosion
> > > > > of the number of media bus codes (imagine all the different possible
> > > > > arrangements of this pattern, for instance if you enable horizontal
> > > > > and/or vertical flipping on the sensor). All drivers would need to be
> > > > > updated to support these new bus codes, and this really kills the
> > > > > current model.  
> > > > 
> > > > Yep, I know what you mean but as I said above I think that adding it
> > > > explicite is the better abbroach since it involves somone who add _and_
> > > > test the new code on the particular platform.
> > > >   
> > > > > The historical mistake was to tie the Bayer pattern with the media bus
> > > > > code. We should really have specified raw 8/10/12/14/16 media bus codes,
> > > > > and conveyed the pattern separately. Most IP cores in the pipeline don't
> > > > > need to care about the pattern at all, and those who do (that's mostly
> > > > > ISPs) could be programmed explicitly by userspace as long as we have an
> > > > > API to retrieve the pattern from the sensor. I believe it's time to bite
> > > > > the bullet and go in that direction. I'm sorry for this case of yak
> > > > > shaving, but it really wouldn't be manageable anymore :-(  
> > > > 
> > > > I got all your points and would agree but this is not a bayer only
> > > > related problem. You will have this problem with all new other formats
> > > > as well. I'm with you, most IP cores don't care but I wouldn't
> > > > guarantee that.  
> > > 
> > > Sorry, but adding new media bus formats like this one will just not
> > > scale. We have two options, either fixing the issue, or considering that
> > > V4L2 is a barely alive API with no future, and merging this without
> > > caring anymore.  
> > 
> > Hm.. you're right that it doesn't scale, as I said I'm absolute on your
> > side. So let us consider a new approach @Mauro, @Hans, @Sailus what do
> > you think about?  
> 
> Starting brainstorming, how about new media bus codes for
> raw{8,10,12,14,16}, 

By "raw", are you meaning vendor-specific formats? If so, that sounds 
a bad idea. Different vendor-specific formats should use different
media bus codes (and fourccs) as otherwise there won't be an easy way
to distinguish them and to describe the raw formats at the media specs.

Thanks,
Mauro

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Laurent Pinchart]

On Fri, Apr 30, 2021 at 02:51:46PM +0200, Mauro Carvalho Chehab wrote:
> Em Fri, 30 Apr 2021 15:18:52 +0300
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:
> 
> > Hi Marco,
> > 
> > On Fri, Apr 30, 2021 at 08:51:34AM +0200, Marco Felsch wrote:
> > > On 21-04-30 01:14, Laurent Pinchart wrote:  
> > > > On Thu, Apr 29, 2021 at 09:49:03AM +0200, Marco Felsch wrote:  
> > > > > On 21-04-29 04:51, Laurent Pinchart wrote:  
> > > > > > On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:  
> > > > > > > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > > > > > > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > > > > > > with the interleaved IR pixels looks like:
> > > > > > > 
> > > > > > >         |  G |  R |  G |  B | ...
> > > > > > >         +----+----+----+----+---
> > > > > > >         | IR |  G | IR |  G | ...
> > > > > > >         +----+----+----+----+---
> > > > > > >         |  G |  B |  G |  R | ...
> > > > > > >         +----+----+----+----+---
> > > > > > >         | IR |  G | IR |  G | ...
> > > > > > >         +----+----+----+----+---
> > > > > > >         | .. | .. | .. | .. | ..
> > > > > > > 
> > > > > > > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf  
> > > > > > 
> > > > > > I think we're reaching a limit of the media bus codes model here, due to
> > > > > > a historical mistake. The four possible Bayer patterns, times the
> > > > > > different number of bits per pixel, creates a lot of media bus codes,
> > > > > > and drivers for CSI-2 receivers and IP cores further down the pipeline
> > > > > > have to support them all.  
> > > > > 
> > > > > That's correct but it is not bayer related. Currently it is what it is,
> > > > > if a new code is added it must be propagated through all the involved
> > > > > subdevs. On the other hand I wouldn't say that it is better to support
> > > > > new codes per default for all drivers. Since this would add a lot of
> > > > > untested code paths.  
> > > > 
> > > > It's not an issue limited to Bayer patterns, but they make the issue
> > > > worse given the number of possible combinations (think about adding DPCM
> > > > and ALAW compression on top of that...).  
> > > 
> > > You're right and again this will apply to all mbus formats...
> > >   
> > > > > > This is already painful, and if we had a
> > > > > > non-Bayer pattern such as this one,  
> > > > > 
> > > > > That's not exactly true since it is a bayer pattern but instead of using
> > > > > 4x4 it uses 8x8 and it as some special pixels.
> > > > >   
> > > > > > we'll open the door to an explosion
> > > > > > of the number of media bus codes (imagine all the different possible
> > > > > > arrangements of this pattern, for instance if you enable horizontal
> > > > > > and/or vertical flipping on the sensor). All drivers would need to be
> > > > > > updated to support these new bus codes, and this really kills the
> > > > > > current model.  
> > > > > 
> > > > > Yep, I know what you mean but as I said above I think that adding it
> > > > > explicite is the better abbroach since it involves somone who add _and_
> > > > > test the new code on the particular platform.
> > > > >   
> > > > > > The historical mistake was to tie the Bayer pattern with the media bus
> > > > > > code. We should really have specified raw 8/10/12/14/16 media bus codes,
> > > > > > and conveyed the pattern separately. Most IP cores in the pipeline don't
> > > > > > need to care about the pattern at all, and those who do (that's mostly
> > > > > > ISPs) could be programmed explicitly by userspace as long as we have an
> > > > > > API to retrieve the pattern from the sensor. I believe it's time to bite
> > > > > > the bullet and go in that direction. I'm sorry for this case of yak
> > > > > > shaving, but it really wouldn't be manageable anymore :-(  
> > > > > 
> > > > > I got all your points and would agree but this is not a bayer only
> > > > > related problem. You will have this problem with all new other formats
> > > > > as well. I'm with you, most IP cores don't care but I wouldn't
> > > > > guarantee that.  
> > > > 
> > > > Sorry, but adding new media bus formats like this one will just not
> > > > scale. We have two options, either fixing the issue, or considering that
> > > > V4L2 is a barely alive API with no future, and merging this without
> > > > caring anymore.  
> > > 
> > > Hm.. you're right that it doesn't scale, as I said I'm absolute on your
> > > side. So let us consider a new approach @Mauro, @Hans, @Sailus what do
> > > you think about?  
> > 
> > Starting brainstorming, how about new media bus codes for
> > raw{8,10,12,14,16}, 
> 
> By "raw", are you meaning vendor-specific formats? If so, that sounds 
> a bad idea. Different vendor-specific formats should use different
> media bus codes (and fourccs) as otherwise there won't be an easy way
> to distinguish them and to describe the raw formats at the media specs.

I mean what the CSI-2 spec means. The exact interpretation of the format
will be a combination of the media bus code and the CFA pattern control.
The whole point of this discussion is to not have different media bus
codes for all possible combinations of formats, as that clearly doesn't
scale.

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Marco Felsch]

Hi all,

sorry for the long absence on this topic. Only a few years later I'm
back on this topic :)

On 21-04-30, Laurent Pinchart wrote:
> On Fri, Apr 30, 2021 at 02:51:46PM +0200, Mauro Carvalho Chehab wrote:
> > Em Fri, 30 Apr 2021 15:18:52 +0300
> > Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:
> > 
> > > Hi Marco,
> > > 
> > > On Fri, Apr 30, 2021 at 08:51:34AM +0200, Marco Felsch wrote:
> > > > On 21-04-30 01:14, Laurent Pinchart wrote:  
> > > > > On Thu, Apr 29, 2021 at 09:49:03AM +0200, Marco Felsch wrote:  
> > > > > > On 21-04-29 04:51, Laurent Pinchart wrote:  
> > > > > > > On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:  
> > > > > > > > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > > > > > > > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > > > > > > > with the interleaved IR pixels looks like:
> > > > > > > > 
> > > > > > > >         |  G |  R |  G |  B | ...
> > > > > > > >         +----+----+----+----+---
> > > > > > > >         | IR |  G | IR |  G | ...
> > > > > > > >         +----+----+----+----+---
> > > > > > > >         |  G |  B |  G |  R | ...
> > > > > > > >         +----+----+----+----+---
> > > > > > > >         | IR |  G | IR |  G | ...
> > > > > > > >         +----+----+----+----+---
> > > > > > > >         | .. | .. | .. | .. | ..
> > > > > > > > 
> > > > > > > > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf  
> > > > > > > 
> > > > > > > I think we're reaching a limit of the media bus codes model here, due to
> > > > > > > a historical mistake. The four possible Bayer patterns, times the
> > > > > > > different number of bits per pixel, creates a lot of media bus codes,
> > > > > > > and drivers for CSI-2 receivers and IP cores further down the pipeline
> > > > > > > have to support them all.  
> > > > > > 
> > > > > > That's correct but it is not bayer related. Currently it is what it is,
> > > > > > if a new code is added it must be propagated through all the involved
> > > > > > subdevs. On the other hand I wouldn't say that it is better to support
> > > > > > new codes per default for all drivers. Since this would add a lot of
> > > > > > untested code paths.  
> > > > > 
> > > > > It's not an issue limited to Bayer patterns, but they make the issue
> > > > > worse given the number of possible combinations (think about adding DPCM
> > > > > and ALAW compression on top of that...).  
> > > > 
> > > > You're right and again this will apply to all mbus formats...
> > > >   
> > > > > > > This is already painful, and if we had a
> > > > > > > non-Bayer pattern such as this one,  
> > > > > > 
> > > > > > That's not exactly true since it is a bayer pattern but instead of using
> > > > > > 4x4 it uses 8x8 and it as some special pixels.
> > > > > >   
> > > > > > > we'll open the door to an explosion
> > > > > > > of the number of media bus codes (imagine all the different possible
> > > > > > > arrangements of this pattern, for instance if you enable horizontal
> > > > > > > and/or vertical flipping on the sensor). All drivers would need to be
> > > > > > > updated to support these new bus codes, and this really kills the
> > > > > > > current model.  
> > > > > > 
> > > > > > Yep, I know what you mean but as I said above I think that adding it
> > > > > > explicite is the better abbroach since it involves somone who add _and_
> > > > > > test the new code on the particular platform.
> > > > > >   
> > > > > > > The historical mistake was to tie the Bayer pattern with the media bus
> > > > > > > code. We should really have specified raw 8/10/12/14/16 media bus codes,
> > > > > > > and conveyed the pattern separately. Most IP cores in the pipeline don't
> > > > > > > need to care about the pattern at all, and those who do (that's mostly
> > > > > > > ISPs) could be programmed explicitly by userspace as long as we have an
> > > > > > > API to retrieve the pattern from the sensor. I believe it's time to bite
> > > > > > > the bullet and go in that direction. I'm sorry for this case of yak
> > > > > > > shaving, but it really wouldn't be manageable anymore :-(  
> > > > > > 
> > > > > > I got all your points and would agree but this is not a bayer only
> > > > > > related problem. You will have this problem with all new other formats
> > > > > > as well. I'm with you, most IP cores don't care but I wouldn't
> > > > > > guarantee that.  
> > > > > 
> > > > > Sorry, but adding new media bus formats like this one will just not
> > > > > scale. We have two options, either fixing the issue, or considering that
> > > > > V4L2 is a barely alive API with no future, and merging this without
> > > > > caring anymore.  
> > > > 
> > > > Hm.. you're right that it doesn't scale, as I said I'm absolute on your
> > > > side. So let us consider a new approach @Mauro, @Hans, @Sailus what do
> > > > you think about?  
> > > 
> > > Starting brainstorming, how about new media bus codes for
> > > raw{8,10,12,14,16}, 
> > 
> > By "raw", are you meaning vendor-specific formats? If so, that sounds 
> > a bad idea. Different vendor-specific formats should use different
> > media bus codes (and fourccs) as otherwise there won't be an easy way
> > to distinguish them and to describe the raw formats at the media specs.
> 
> I mean what the CSI-2 spec means. The exact interpretation of the format
> will be a combination of the media bus code and the CFA pattern control.
> The whole point of this discussion is to not have different media bus
> codes for all possible combinations of formats, as that clearly doesn't
> scale.

You mean that we should propagate the value as raw{size} through the
entire pipeline, if I got this correctly. How the picture should be
interpreted is up to the user-space by calling a new read-only CFA
v4l2-control. This way we don't need to patch each subdev driver and
take the user-space into account of interpreting the data. For the CFA
control we could use a global-unique list so the control returns an
enum.

Regards,
  Marco

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats

[Laurent Pinchart]

Hi Marco,

On Wed, Feb 08, 2023 at 08:44:25PM +0100, Marco Felsch wrote:
> Hi all,
> 
> sorry for the long absence on this topic. Only a few years later I'm
> back on this topic :)

No worries, I rarely complain when I receive less e-mails :-)

> On 21-04-30, Laurent Pinchart wrote:
> > On Fri, Apr 30, 2021 at 02:51:46PM +0200, Mauro Carvalho Chehab wrote:
> > > Em Fri, 30 Apr 2021 15:18:52 +0300 Laurent Pinchart escreveu:
> > > > On Fri, Apr 30, 2021 at 08:51:34AM +0200, Marco Felsch wrote:
> > > > > On 21-04-30 01:14, Laurent Pinchart wrote:  
> > > > > > On Thu, Apr 29, 2021 at 09:49:03AM +0200, Marco Felsch wrote:  
> > > > > > > On 21-04-29 04:51, Laurent Pinchart wrote:  
> > > > > > > > On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:  
> > > > > > > > > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > > > > > > > > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > > > > > > > > with the interleaved IR pixels looks like:
> > > > > > > > > 
> > > > > > > > >         |  G |  R |  G |  B | ...
> > > > > > > > >         +----+----+----+----+---
> > > > > > > > >         | IR |  G | IR |  G | ...
> > > > > > > > >         +----+----+----+----+---
> > > > > > > > >         |  G |  B |  G |  R | ...
> > > > > > > > >         +----+----+----+----+---
> > > > > > > > >         | IR |  G | IR |  G | ...
> > > > > > > > >         +----+----+----+----+---
> > > > > > > > >         | .. | .. | .. | .. | ..
> > > > > > > > > 
> > > > > > > > > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf  
> > > > > > > > 
> > > > > > > > I think we're reaching a limit of the media bus codes model here, due to
> > > > > > > > a historical mistake. The four possible Bayer patterns, times the
> > > > > > > > different number of bits per pixel, creates a lot of media bus codes,
> > > > > > > > and drivers for CSI-2 receivers and IP cores further down the pipeline
> > > > > > > > have to support them all.  
> > > > > > > 
> > > > > > > That's correct but it is not bayer related. Currently it is what it is,
> > > > > > > if a new code is added it must be propagated through all the involved
> > > > > > > subdevs. On the other hand I wouldn't say that it is better to support
> > > > > > > new codes per default for all drivers. Since this would add a lot of
> > > > > > > untested code paths.  
> > > > > > 
> > > > > > It's not an issue limited to Bayer patterns, but they make the issue
> > > > > > worse given the number of possible combinations (think about adding DPCM
> > > > > > and ALAW compression on top of that...).  
> > > > > 
> > > > > You're right and again this will apply to all mbus formats...
> > > > >   
> > > > > > > > This is already painful, and if we had a
> > > > > > > > non-Bayer pattern such as this one,  
> > > > > > > 
> > > > > > > That's not exactly true since it is a bayer pattern but instead of using
> > > > > > > 4x4 it uses 8x8 and it as some special pixels.
> > > > > > >   
> > > > > > > > we'll open the door to an explosion
> > > > > > > > of the number of media bus codes (imagine all the different possible
> > > > > > > > arrangements of this pattern, for instance if you enable horizontal
> > > > > > > > and/or vertical flipping on the sensor). All drivers would need to be
> > > > > > > > updated to support these new bus codes, and this really kills the
> > > > > > > > current model.  
> > > > > > > 
> > > > > > > Yep, I know what you mean but as I said above I think that adding it
> > > > > > > explicite is the better abbroach since it involves somone who add _and_
> > > > > > > test the new code on the particular platform.
> > > > > > >   
> > > > > > > > The historical mistake was to tie the Bayer pattern with the media bus
> > > > > > > > code. We should really have specified raw 8/10/12/14/16 media bus codes,
> > > > > > > > and conveyed the pattern separately. Most IP cores in the pipeline don't
> > > > > > > > need to care about the pattern at all, and those who do (that's mostly
> > > > > > > > ISPs) could be programmed explicitly by userspace as long as we have an
> > > > > > > > API to retrieve the pattern from the sensor. I believe it's time to bite
> > > > > > > > the bullet and go in that direction. I'm sorry for this case of yak
> > > > > > > > shaving, but it really wouldn't be manageable anymore :-(  
> > > > > > > 
> > > > > > > I got all your points and would agree but this is not a bayer only
> > > > > > > related problem. You will have this problem with all new other formats
> > > > > > > as well. I'm with you, most IP cores don't care but I wouldn't
> > > > > > > guarantee that.  
> > > > > > 
> > > > > > Sorry, but adding new media bus formats like this one will just not
> > > > > > scale. We have two options, either fixing the issue, or considering that
> > > > > > V4L2 is a barely alive API with no future, and merging this without
> > > > > > caring anymore.  
> > > > > 
> > > > > Hm.. you're right that it doesn't scale, as I said I'm absolute on your
> > > > > side. So let us consider a new approach @Mauro, @Hans, @Sailus what do
> > > > > you think about?  
> > > > 
> > > > Starting brainstorming, how about new media bus codes for
> > > > raw{8,10,12,14,16}, 
> > > 
> > > By "raw", are you meaning vendor-specific formats? If so, that sounds 
> > > a bad idea. Different vendor-specific formats should use different
> > > media bus codes (and fourccs) as otherwise there won't be an easy way
> > > to distinguish them and to describe the raw formats at the media specs.
> > 
> > I mean what the CSI-2 spec means. The exact interpretation of the format
> > will be a combination of the media bus code and the CFA pattern control.
> > The whole point of this discussion is to not have different media bus
> > codes for all possible combinations of formats, as that clearly doesn't
> > scale.
> 
> You mean that we should propagate the value as raw{size} through the
> entire pipeline, if I got this correctly. How the picture should be
> interpreted is up to the user-space by calling a new read-only CFA
> v4l2-control. This way we don't need to patch each subdev driver and
> take the user-space into account of interpreting the data. For the CFA
> control we could use a global-unique list so the control returns an
> enum.

That's the idea, yes. The CFA pattern control would be read-only on the
sensor side (but its value may change if h/v flip is implemented,
depending on the sensor, or possibly if we allow moving the crop
rectangle by one pixel instead of two), and would be writable on any
subdev that needs the information (typically the subdev that implements
CFA interpolation).

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/6] media: v4l: Add definition for bayered IR formats

[Marco Felsch]

Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
with the interleaved IR pixels looks like:

        |  G |  R |  G |  B | ...
        +----+----+----+----+---
        | IR |  G | IR |  G | ...
        +----+----+----+----+---
        |  G |  B |  G |  R | ...
        +----+----+----+----+---
        | IR |  G | IR |  G | ...
        +----+----+----+----+---
        | .. | .. | .. | .. | ..

[1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 include/uapi/linux/videodev2.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 311a01cc5775..45ffd3867394 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -632,6 +632,8 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_SGBRG8  v4l2_fourcc('G', 'B', 'R', 'G') /*  8  GBGB.. RGRG.. */
 #define V4L2_PIX_FMT_SGRBG8  v4l2_fourcc('G', 'R', 'B', 'G') /*  8  GRGR.. BGBG.. */
 #define V4L2_PIX_FMT_SRGGB8  v4l2_fourcc('R', 'G', 'G', 'B') /*  8  RGRG.. GBGB.. */
+	/* 8bit infrared interleaved bayer format */
+#define V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG8 v4l2_fourcc('I', 'R', '0', '8') /* 8 GRGB.. IGIG.. GBGR.. IGIG.. */
 #define V4L2_PIX_FMT_SBGGR10 v4l2_fourcc('B', 'G', '1', '0') /* 10  BGBG.. GRGR.. */
 #define V4L2_PIX_FMT_SGBRG10 v4l2_fourcc('G', 'B', '1', '0') /* 10  GBGB.. RGRG.. */
 #define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B', 'A', '1', '0') /* 10  GRGR.. BGBG.. */
@@ -673,6 +675,8 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_SGBRG16 v4l2_fourcc('G', 'B', '1', '6') /* 16  GBGB.. RGRG.. */
 #define V4L2_PIX_FMT_SGRBG16 v4l2_fourcc('G', 'R', '1', '6') /* 16  GRGR.. BGBG.. */
 #define V4L2_PIX_FMT_SRGGB16 v4l2_fourcc('R', 'G', '1', '6') /* 16  RGRG.. GBGB.. */
+	/* 16bit infrared interleaved bayer format */
+#define V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG16 v4l2_fourcc('I', 'R', '1', '6') /* 16 GRGB.. IGIG.. GBGR.. IGIG.. */
 
 /* HSV formats */
 #define V4L2_PIX_FMT_HSV24 v4l2_fourcc('H', 'S', 'V', '3')
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] media: v4l: Add definition for bayered IR formats

[Laurent Pinchart]

Hi Marco,

Thank you for the patch.

On Tue, Apr 27, 2021 at 02:06:57PM +0200, Marco Felsch wrote:
> Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> with the interleaved IR pixels looks like:
> 
>         |  G |  R |  G |  B | ...
>         +----+----+----+----+---
>         | IR |  G | IR |  G | ...
>         +----+----+----+----+---
>         |  G |  B |  G |  R | ...
>         +----+----+----+----+---
>         | IR |  G | IR |  G | ...
>         +----+----+----+----+---
>         | .. | .. | .. | .. | ..
> 
> [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  include/uapi/linux/videodev2.h | 4 ++++

The documentation is missing.

>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 311a01cc5775..45ffd3867394 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -632,6 +632,8 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_SGBRG8  v4l2_fourcc('G', 'B', 'R', 'G') /*  8  GBGB.. RGRG.. */
>  #define V4L2_PIX_FMT_SGRBG8  v4l2_fourcc('G', 'R', 'B', 'G') /*  8  GRGR.. BGBG.. */
>  #define V4L2_PIX_FMT_SRGGB8  v4l2_fourcc('R', 'G', 'G', 'B') /*  8  RGRG.. GBGB.. */
> +	/* 8bit infrared interleaved bayer format */
> +#define V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG8 v4l2_fourcc('I', 'R', '0', '8') /* 8 GRGB.. IGIG.. GBGR.. IGIG.. */
>  #define V4L2_PIX_FMT_SBGGR10 v4l2_fourcc('B', 'G', '1', '0') /* 10  BGBG.. GRGR.. */
>  #define V4L2_PIX_FMT_SGBRG10 v4l2_fourcc('G', 'B', '1', '0') /* 10  GBGB.. RGRG.. */
>  #define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B', 'A', '1', '0') /* 10  GRGR.. BGBG.. */
> @@ -673,6 +675,8 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_SGBRG16 v4l2_fourcc('G', 'B', '1', '6') /* 16  GBGB.. RGRG.. */
>  #define V4L2_PIX_FMT_SGRBG16 v4l2_fourcc('G', 'R', '1', '6') /* 16  GRGR.. BGBG.. */
>  #define V4L2_PIX_FMT_SRGGB16 v4l2_fourcc('R', 'G', '1', '6') /* 16  RGRG.. GBGB.. */
> +	/* 16bit infrared interleaved bayer format */
> +#define V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG16 v4l2_fourcc('I', 'R', '1', '6') /* 16 GRGB.. IGIG.. GBGR.. IGIG.. */
>  
>  /* HSV formats */
>  #define V4L2_PIX_FMT_HSV24 v4l2_fourcc('H', 'S', 'V', '3')

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] media: v4l: Add definition for bayered IR formats

[Marco Felsch]

Hi Laurent,

On 21-04-29 04:45, Laurent Pinchart wrote:
> Hi Marco,
> 
> Thank you for the patch.
> 
> On Tue, Apr 27, 2021 at 02:06:57PM +0200, Marco Felsch wrote:
> > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > with the interleaved IR pixels looks like:
> > 
> >         |  G |  R |  G |  B | ...
> >         +----+----+----+----+---
> >         | IR |  G | IR |  G | ...
> >         +----+----+----+----+---
> >         |  G |  B |  G |  R | ...
> >         +----+----+----+----+---
> >         | IR |  G | IR |  G | ...
> >         +----+----+----+----+---
> >         | .. | .. | .. | .. | ..
> > 
> > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  include/uapi/linux/videodev2.h | 4 ++++
> 
> The documentation is missing.

I've send a seperate patch for this. Those two could be squashed.

Regards,
  Marco

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/6] media: v4l2-ioctl.c: add V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG to v4l_fill_fmtdesc

[Marco Felsch]

Add custom OnSemi RGB-IR pixel formats to the list of pixelformats
that v4l_fill_fmtdesc() understands. This format is used by the OnSemi
AR0237IR camera sensor [1].

[1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 6a5d1c6d11d6..e9d0c9417719 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1337,6 +1337,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 	case V4L2_PIX_FMT_SGBRG8:	descr = "8-bit Bayer GBGB/RGRG"; break;
 	case V4L2_PIX_FMT_SGRBG8:	descr = "8-bit Bayer GRGR/BGBG"; break;
 	case V4L2_PIX_FMT_SRGGB8:	descr = "8-bit Bayer RGRG/GBGB"; break;
+	case V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG8: descr = "8-bit GRGB/IGIG/GBGR/IGIG"; break;
 	case V4L2_PIX_FMT_SBGGR10:	descr = "10-bit Bayer BGBG/GRGR"; break;
 	case V4L2_PIX_FMT_SGBRG10:	descr = "10-bit Bayer GBGB/RGRG"; break;
 	case V4L2_PIX_FMT_SGRBG10:	descr = "10-bit Bayer GRGR/BGBG"; break;
@@ -1377,6 +1378,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 	case V4L2_PIX_FMT_SGBRG16:	descr = "16-bit Bayer GBGB/RGRG"; break;
 	case V4L2_PIX_FMT_SGRBG16:	descr = "16-bit Bayer GRGR/BGBG"; break;
 	case V4L2_PIX_FMT_SRGGB16:	descr = "16-bit Bayer RGRG/GBGB"; break;
+	case V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG16: descr = "16-bit GRGB/IGIG/GBGR/IGIG"; break;
 	case V4L2_PIX_FMT_SN9C20X_I420:	descr = "GSPCA SN9C20X I420"; break;
 	case V4L2_PIX_FMT_SPCA501:	descr = "GSPCA SPCA501"; break;
 	case V4L2_PIX_FMT_SPCA505:	descr = "GSPCA SPCA505"; break;
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/6] media: video-mux: add new SGRGB_IGIG_GBGR_IGIG format support

[Marco Felsch]

Add custom OnSemi RGB-IR pixel formats. This format is used by the OnSemi
AR0237IR camera sensor [1].

[1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/video-mux.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c
index 133122e38515..aaec24d60101 100644
--- a/drivers/media/platform/video-mux.c
+++ b/drivers/media/platform/video-mux.c
@@ -268,6 +268,7 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
 	case MEDIA_BUS_FMT_SGBRG8_1X8:
 	case MEDIA_BUS_FMT_SGRBG8_1X8:
 	case MEDIA_BUS_FMT_SRGGB8_1X8:
+	case MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG8_1X8:
 	case MEDIA_BUS_FMT_SBGGR10_1X10:
 	case MEDIA_BUS_FMT_SGBRG10_1X10:
 	case MEDIA_BUS_FMT_SGRBG10_1X10:
@@ -276,6 +277,7 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
 	case MEDIA_BUS_FMT_SGBRG12_1X12:
 	case MEDIA_BUS_FMT_SGRBG12_1X12:
 	case MEDIA_BUS_FMT_SRGGB12_1X12:
+	case MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG12_1X12:
 	case MEDIA_BUS_FMT_SBGGR14_1X14:
 	case MEDIA_BUS_FMT_SGBRG14_1X14:
 	case MEDIA_BUS_FMT_SGRBG14_1X14:
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/6] gpu: ipu-v3: add custom SGRGB_IGIG_GBGR_IGIG format support

[Marco Felsch]

Add custom OnSemi RGB-IR pixel formats to the ipu library functions. This
format is used by the OnSemi AR0237IR camera sensor [1].

[1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/ipu-v3/ipu-cpmem.c | 2 ++
 drivers/gpu/ipu-v3/ipu-csi.c   | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
index a1c85d1521f5..c579aafb60ba 100644
--- a/drivers/gpu/ipu-v3/ipu-cpmem.c
+++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
@@ -861,6 +861,7 @@ int ipu_cpmem_set_image(struct ipuv3_channel *ch, struct ipu_image *image)
 	case V4L2_PIX_FMT_SGBRG8:
 	case V4L2_PIX_FMT_SGRBG8:
 	case V4L2_PIX_FMT_SRGGB8:
+	case V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG8:
 	case V4L2_PIX_FMT_GREY:
 		offset = image->rect.left + image->rect.top * pix->bytesperline;
 		break;
@@ -868,6 +869,7 @@ int ipu_cpmem_set_image(struct ipuv3_channel *ch, struct ipu_image *image)
 	case V4L2_PIX_FMT_SGBRG16:
 	case V4L2_PIX_FMT_SGRBG16:
 	case V4L2_PIX_FMT_SRGGB16:
+	case V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG16:
 	case V4L2_PIX_FMT_Y16:
 		offset = image->rect.left * 2 +
 			 image->rect.top * pix->bytesperline;
diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index 8ae301eef643..cf7763f50fdf 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -268,6 +268,7 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
 	case MEDIA_BUS_FMT_SGBRG8_1X8:
 	case MEDIA_BUS_FMT_SGRBG8_1X8:
 	case MEDIA_BUS_FMT_SRGGB8_1X8:
+	case MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG8_1X8:
 	case MEDIA_BUS_FMT_Y8_1X8:
 		cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_BAYER;
 		cfg->mipi_dt = MIPI_DT_RAW8;
@@ -298,6 +299,7 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
 	case MEDIA_BUS_FMT_SGBRG12_1X12:
 	case MEDIA_BUS_FMT_SGRBG12_1X12:
 	case MEDIA_BUS_FMT_SRGGB12_1X12:
+	case MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG12_1X12:
 	case MEDIA_BUS_FMT_Y12_1X12:
 		cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_BAYER;
 		cfg->mipi_dt = MIPI_DT_RAW12;
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/6] media: imx: csi: add custom SGRGB_IGIG_GBGR_IGIG format support

[Marco Felsch]

Add custom OnSemi RGB-IR pixel formats which is used by the OnSemi
AR0237IR sensor [1]. The colorspace is irrelevant since the
user space has to debayer the images.

[1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/staging/media/imx/imx-media-csi.c   |  2 ++
 drivers/staging/media/imx/imx-media-utils.c | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index e3bfd635a89a..1085feacf27f 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -448,6 +448,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
 	case V4L2_PIX_FMT_SGBRG8:
 	case V4L2_PIX_FMT_SGRBG8:
 	case V4L2_PIX_FMT_SRGGB8:
+	case V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG8:
 	case V4L2_PIX_FMT_GREY:
 		burst_size = 16;
 		passthrough_bits = 8;
@@ -456,6 +457,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
 	case V4L2_PIX_FMT_SGBRG16:
 	case V4L2_PIX_FMT_SGRBG16:
 	case V4L2_PIX_FMT_SRGGB16:
+	case V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG16:
 	case V4L2_PIX_FMT_Y10:
 	case V4L2_PIX_FMT_Y12:
 		burst_size = 8;
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 5128915a5d6f..c4ed8bf89b31 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -174,6 +174,18 @@ static const struct imx_media_pixfmt pixel_formats[] = {
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 16,
 		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG8,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG8_1X8),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 8,
+		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG16,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG12_1X12),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 16,
+		.bayer  = true,
 	}, {
 		.fourcc = V4L2_PIX_FMT_GREY,
 		.codes = IMX_BUS_FMTS(
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/6] Add new bayer ir formats

[Marco Felsch]

On 21-04-27 14:06, Marco Felsch wrote:
> Hi all,
> 
> this adds the inital support for a new sensor format called RGB-IR
> found on sensors from OnSemi [1]. This is a new custom bayer format with
> interleaved ir pixels. For more information see the documentation
> commit.
> 
> [1] https://www.onsemi.com/products/sensors/image-sensors-processors/image-sensors/ar0237sr

Sorry, I forget to say that I know that checkpatch.pl complains but I
kept my changes to be more readable.

Regards,
  Marco

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/6] Add new bayer ir formats

[Hans Verkuil]

Hi Marco,

On 27/04/2021 14:06, Marco Felsch wrote:
> Hi all,
> 
> this adds the inital support for a new sensor format called RGB-IR
> found on sensors from OnSemi [1]. This is a new custom bayer format with
> interleaved ir pixels. For more information see the documentation
> commit.

After a lot of discussion this series went nowhere.

If you want to restart this, then please rebase and post a v2.

I'm marking this as 'Obsoleted' in patchwork.

(Doing a bit of patchwork cleanup of old series today)

Regards,

	Hans

> 
> [1] https://www.onsemi.com/products/sensors/image-sensors-processors/image-sensors/ar0237sr
> 
> Marco Felsch (6):
>   media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats
>   media: v4l: Add definition for bayered IR formats
>   media: v4l2-ioctl.c: add V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG to
>     v4l_fill_fmtdesc
>   media: video-mux: add new SGRGB_IGIG_GBGR_IGIG format support
>   gpu: ipu-v3: add custom SGRGB_IGIG_GBGR_IGIG format support
>   media: imx: csi: add custom SGRGB_IGIG_GBGR_IGIG format support
> 
>  .../media/v4l/subdev-formats.rst              | 42 +++++++++++++++++++
>  drivers/gpu/ipu-v3/ipu-cpmem.c                |  2 +
>  drivers/gpu/ipu-v3/ipu-csi.c                  |  2 +
>  drivers/media/platform/video-mux.c            |  2 +
>  drivers/media/v4l2-core/v4l2-ioctl.c          |  2 +
>  drivers/staging/media/imx/imx-media-csi.c     |  2 +
>  drivers/staging/media/imx/imx-media-utils.c   | 12 ++++++
>  include/uapi/linux/media-bus-format.h         |  4 +-
>  include/uapi/linux/videodev2.h                |  4 ++
>  9 files changed, 71 insertions(+), 1 deletion(-)
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel