On 1/27/20 11:42 AM, Sakari Ailus wrote:
> Hi Daniel,
>
> On Fri, Jan 24, 2020 at 09:36:25PM +0100, Daniel Glöckner wrote:
>> Hello,
>>
>> the i.MX8QM and i.MX8QXP contain MIPI CSI-2 controllers that forward the
>> received data on a parallel bus to the Image Sensing Interface (ISI) of
>> the chip. If the data on the MIPI bus is in any of the six RAW formats
>> defined for CSI-2, the CSI controller will shift the values so that the
>> msb is always in bit 13. This was most likely done to allow following
>> hardware to process the data as RAW14 regardless of the actual RAW format.
>> Unfortunately the ISI is not able to shift the bits back before writing it
>> to memory. RAW8 data therefore has to be saved in two bytes per sample with
>> two unused bits at the top and six unused bits at the bottom.
>
> That's a rather peculiar implementation, indeed.
>
>>
>> The drivers for the hardware are still being developed in NXP's repository
>> at CodeAurora. We have extended them to support greyscale and Bayer
>> cameras. Now all we need are stable defines for the pixel and media bus
>> formats for use in libraries and applications and documentation for people
>> to know their meaning.
>>
>> To keep the number of needed formats low, we would like to ignore that
>> there might be unused bits at the bottom. Then we can use the existing
>> MEDIA_BUS_FMT_S{BGGR,GBRG,GRBG,RGGB}14_1X14 formats between the CSI
>> controller and the ISI and just have to add a MEDIA_BUS_FMT_Y14_1X14
>> format. For the pixel formats we would add V4L2_PIX_FMT_Y14 and rebase
>> e38d5f254ad8 from Sakari's packed12-postponed branch for Bayer.
>>
>> Now the questions:
>>
>> - Is it acceptable to use MEDIA_BUS_FMT_Y14_1X14 in this case instead of
>> using MEDIA_BUS_FMT_Y12_1X14_PADLO, MEDIA_BUS_FMT_Y10_1X14_PADLO,
>> MEDIA_BUS_FMT_Y8_1X14_PADLO, MEDIA_BUS_FMT_Y7_1X14_PADLO,
>> MEDIA_BUS_FMT_Y6_1X14_PADLO? Another 20 _PADLO formats would have to
>> be added for Bayer.
>
> I think I'd say yes, you could do this, *if* you're fully certain you'll
> *never* see this CSI-2 receiver paired with any other hardware than the
> ISI, which is the case for instance if it's part of the same device. As if
> there is hardware that can make use of the information on how many bits are
> actually used there, you'd need to expose that information on the uAPI as
> well. Changing that would be an uAPI change, something that should be
> avoided if at all possible.
>
>>
>> - Given the history of Sakari's packed12-postponed branch, is there a
>> chance to get these definitions merged into mainline although the
>> driver is not? I fear that NXP's drivers will not hit mainline for a
>> long time.
>
> Cc'ing Hans.
>
> We have a practice not merging the format definitions (or other API
> additions) before there are users. That's partly because often API
> additions merged before the user have ended never being used because it
> turned out to be something else that the driver actually needed, or the
> driver was never merged.
>
> That said, I don't foresee that problem here: these (14-bit raw Bayer
> pixel) formats are pretty standard stuff albeit still rare for 14 bits is
> more than you usually need, but we started off with 8 bits per pixel and 12
> isn't uncommon nowadays.
>
> I'd be leaning towards merging them --- it's just a question of time when
> they'll be needed somewhere else.
I see no problem with adding MEDIA_BUS_FMT_Y14_1X14.
Regards,
Hans
>
>>
>> - Sakari, do you mind me adding the same license header to your
>> pixfmt-y14.rst that is used by all other pixfmt-y*.rst files?
>
> Feel free to do so.
>
