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

[Mauro Carvalho Chehab <mchehab@kernel.org>]

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