Re: (EXT) Re: [PATCH] media: Add 16-bit Bayer formats

[Alexander Stein]

Am Dienstag, dem 30.11.2021 um 04:33 +0200 schrieb Laurent Pinchart:
> Hi Dorota,
> 
> On Mon, Nov 29, 2021 at 05:05:23PM +0100, Dorota Czaplejewicz wrote:
> > On Mon, 29 Nov 2021 15:46:11 +0000 Kieran Bingham wrote:
> > > Quoting Dorota Czaplejewicz (2021-10-19 12:59:29)
> > > > 16-bit bayer formats are used by the i.MX driver.  
> > > 
> > > Can we expand upon this at all?
> > > 
> > > The Subject says "Add 16-bit Bayer formats" but this isn't adding
> > > the
> > > format, it's purely extending the v4l2_format_info table with the
> > > information for that format which is otherwise missing.
> > 
> > What do you suggest for a better commit message? My reasoning was
> > that
> > I'm adding entries to a table.
> 
> The format is defined by V4L2 but isn't present in that table. I'd
> state
> the this patch is fixing an oversight, and reference the commit that
> forgot to add these formats in a Fixes: tag. While at it, I'd also
> add
> at least the 14bpp Bayer formats, and possibly the packed formats
> too.
> 
> > > I wonder what other formats are missing from that table too?
> > > 
> > > > Signed-off-by: Dorota Czaplejewicz <
> > > > dorota.czaplejewicz@xxxxxxx
> > > > >
> > > > ---
> > > > Hello,
> > > > 
> > > > While working on the i.MX8 video driver, I discovered that
> > > > `v4l2_fill_pixfmt` will fail when using 10-bit sensor formats.
> > > > (For background, see the conversation at
> > > > https://lkml.org/lkml/2021/10/17/93
> > > >  .)
> > > > 
> > > > It appears that the video hardware will fill a 16-bit-per-pixel
> > > > buffer when fed 10-bit-per-pixel Bayer data, making
> > > > `v4l2_fill_pixfmt` effectively broken for this case.  
> > > 
> > > This statement is confusing to me. Are you saying you're
> > > programming the
> > > hardware with 10 bit, and it's using 16 bits per pixel to store
> > > that
> > > data? (Which is simply 'unpacked' I think ?)
> > 
> > I know the sensor I'm dealing with sends 10-bit data. I'm observing
> > that the data arriving at this stage of the pipeline is encoded
> > with
> > 16 bits per pixel. As far as I understand, that's what i.MX8 does
> > at
> > some stage of the MIPI/CSI2 pipeline by design, but I can't
> > elaborate
> > at the moment, and I don't think it affects the validity of the
> > addition.
> 
> Is the 10 bit data stored in the MSB or LSB of the 2 bytes ?

Oh, I get a dejavu here. I assume this is on an i.MX8QM or i.MX8QXP,
but not one of the other i.MX8 ones. They have a similar name, but are
very (!) diffeent in some aspects.

To answer your question, neither of those two alignments. The RM for
i.MX8QM and i.MX8QXP states:
> NOTE
> The CSI data is right LSB aligned and zero padded depending
> on data format. When interfacing ISI, CSI data is shifted 6-bits
> due to ISI bits [5:0] always being zero
> (0bxxCSIDATAxxxxxx). All RAW14, RAW12, RAW10,
> RAW8, and RAW6 video data is filled from BIT[13] to LSB,
> the remaining bits are zero padded. Only RAW16 input data
> will be saved to memory from BIT[15] to LSB.

See also [1]. 

This essentially means, unless you use RAW16, you will get RAW14 with a
different amount of LSB bits set to 0.
IIRC there was some patchset to introduce a RAW14 format ([2]) for
exactly this use cas.
There is also some kind of demo doing post-processing ([3]).

Best regards,
Alexander

[1] 
https://community.nxp.com/t5/i-MX-Processors/i-MX8QM-MIPI-Raw-formats-not-working-correctly/m-p/1040832/highlight/true#M153336
[2] 
https://yhbt.net/lore/all/20200226151431.GY5379@xxxxxxxxxxxxxxxxxxxxxx/T/
[3] 
https://community.nxp.com/t5/i-MX-Processors-Knowledge-Base/i-MX8QXP-capture-raw-bayer-data-and-debayer/ta-p/1098963