Hi Laurent, Tomi, On Wed, Sep 06, 2023 at 02:31:17PM +0300, Laurent Pinchart wrote: > Hi Tomi, > > On Wed, Sep 06, 2023 at 11:28:37AM +0300, Tomi Valkeinen wrote: > > On 05/09/2023 19:38, Laurent Pinchart wrote: > > > On Thu, Aug 24, 2023 at 11:26:56AM +0300, Tomi Valkeinen wrote: > > >> On 24/08/2023 10:24, Sakari Ailus wrote: > > >>> On Wed, Aug 23, 2023 at 04:16:13PM +0300, Tomi Valkeinen wrote: > > >>>> On 08/08/2023 10:55, Sakari Ailus wrote: > > >>>>> Add generic serial metadata mbus formats. These formats describe data > > >>>>> width and packing but not the content itself. The reason for specifying > > >>>>> such formats is that the formats as such are fairly device specific but > > >>>>> they are still handled by CSI-2 receiver drivers that should not be aware > > > > > > Do we want these formats to be CSI-2-specific ? > > > > > >>>>> of device specific formats. What makes generic metadata formats possible > > >>>>> is that these formats are parsed by software only, after capturing the > > >>>>> data to system memory. > > >>>> > > >>>> If I'm not mistaken, the CSI-2 spec doesn't say much about embedded data, > > >>>> except that it may exist. Afaics, in CSI-2, the embedded data is split into > > >>>> "lines", although the amount of data can be different than in the video > > >>>> lines. > > >>>> > > >>>> The CCS specs talks more about embedded data. Some of it is quite odd, like: > > >>>> "The length of the embedded data line shall not exceed the length of the > > >>>> image data line. The embedded data line should have the same length as the > > >>>> image data line.". I think it means the embedded line can be shorter than > > >>>> image line, but not longer. > > >>> > > >>> That's what it means, yes. The CCS also has means to obtain the actual > > >>> length --- frame format descriptors. > > >>> > > >>>> CCS also says that an embedded line should use 0x07 as padding at the end of > > >>>> the line, if there's less data than the image line. > > >>>> > > >>>> CCS also talks about how the embedded data would be packed, and in some > > >>>> cases that packing would be the same as for pixel data. > > >>> > > >>> In fact the packing is the same as for pixel data: the CSI-2 does not > > >>> really differentiate there. > > >> > > >> If I understand the CCS spec right, the packing is not the same as for > > >> the pixel data. You can have RAW12 pixel data but 8-byte embedded data. > > >> But if you meant that the different packing style options are the same > > >> for pixel and embedded data, yes. > > > > > > The idea is to allow CSI-2 receivers to unpack embedded data the same > > > way as pixel data at the hardware level, and guarantee that no embedded > > > > Yes, but my comment was to Sakari's "the packing is the same as for > > pixel data" comment. Afaics, there's nothing in CSI-2 or CCS that says > > the packing is the same for pixel data and embedded data. > > > > In fact, the CSS says "By default, top-embedded data shall use the same > > data packing as the Visible pixel data with one embedded data byte per > > pixel", implying that it would not always be the case. It continues that > > for RAW16/RAW20/RAW24 pixels the embedded data could be sent with > > RAW8/RAW10/RAW12. > > The spec if a bit ambiguous :-S "by default" implies that something else > could be done, but it's not clear what the other options are. The options for packing are detailed in the spec. For higher bit depths there are options (as detailed by Tomi above). > > > That was just for top-embedded data, and I couldn't right away see > > mention of other embedded data. > > The specification also mentions PDAF data for instance. The bottom PDAF data is vendor defined (as it's processed) so the vendor is free to do whatever there. Note that the better way to support PDAF is via embedded PDAF pixels, not via separate bottom embedded (PDAF) data. The bottom embedded data could also be different kinds of statistics. > > > > data significant byte gets split across multiple samples. When unpacking > > > RAW10 pixel data, the CSI-2 receiver will take 5 bytes and output 4 > > > samples on a 10-bit wide bus. For embedded data, it can conceptually do > > > that same, and output embedded data with one byte per sample, aligned on > > > the MSB of the 10-bit bus. However, in practice, I think the CSI-2 > > > receiver will simply receive the bytes and send them to the DMA engine, > > > without unpacking and then repacking. > > > > At least TI CAL CSI-2 RX can extract the data from the CSI-2 stream > > using different unpacking for pixel and embedded data, and then > > (optionally) pack it in different ways when writing to memory. > > Does this mean that the CAL could take RAW10 embedded data and drop the > padding bytes when writing to memory ? > > > > This can also be viewed as away to allow CSI-2 transmitters to serialize > > > pixel data and embedded data the same way. For a RAW10 sensor, for > > > instance, the embedded data would be generated on an internal 10-bit > > > wide bus, aligned to the MSB, and would go through CSI-2 packing the > > > same way that pixels do. That's nice, but it's internal to the sensor, > > > so we don't need to expose it to userspace on that side. > > > > > >>>> But I don't think these formats are generic. They're defined in CCS, so > > >>>> shouldn't the format be, e.g., MEDIA_BUS_FMT_META_CCS_RAW_12 or such? > > >>> > > >>> The reason for having generic definitions is that we do not need receiver > > >>> drivers to be aware of formats that are specific to another driver. > > >> > > >> Yes, I agree with that, and that's not my point here. But "generic" > > >> doesn't mean the definitions are not for some particular bus or > > >> standard, "generic" just means that it doesn't specify the content, only > > >> the packing. > > >> > > >> My point is that these packings seem to be specific to CCS. While > > >> non-CCS compliant sensors may use the same packing, the packing itself > > >> is still a "CCS packing". > > > > > > Agreed. > > > > > >> So why not call them that? The 8-bit format is > > >> fully generic, whereas the rest are CCS packings, as defined in the CCS > > >> spec (the CCS spec also specifies the content, but here were only using > > >> the CCS packing). > > > > > > I think an additional question is whether we actually need multiple > > > media bus codes here. Are there CSI-2 receivers, or other downstream > > > blocks, that will need to be configured differently to receive embedded > > > data from a CCS (in the sense of using CCS packing, not being fully > > > CCS-compliant) RAW10 sensor compared to a RAW12 sensor ? Or will the > > > CSI-2 receiver just send bytes to memory without caring ? In the latter > > > case, it will be the application that will need to know how the data is > > > packed to ignore one byte out of three for RAW12 and out of five for > > > RAW10. > > > > I would presume all receivers can operate in "just write what you > > receive" mode, in which case the packing doesn't matter as you say, but > > then again, I think it's safer to have mbus codes for the different > > packings. > > > > >> Maybe they shouldn't be called "generic", but... umm... Content unaware > > >> metadata formats... doesn't sound very good =). > > >> > > >> Also, I'm a bit confused about CSI-2 pixel and embedded data formats and > > >> how we handle them. > > >> > > >> For MEDIA_BUS_FMT_SBGGR10_1X10, we define that the data contains 10 bits > > >> per pixel, from bit 0 to 9. But CSI-2, for RAW10, actually sends it > > >> differently, with the higher bits first, and the lowest bits in the > > >> fourth byte. So that CSI-2 packing is implicit, kind of hidden here. > > > > > > Usage of MEDIA_BUS_FMT_SBGGR10_1X10 for CSI-2 is a historical decision > > > (or mistake, depending on how you look at it). It doesn't match what > > > actually happens on the bus. > > > > > >> Which is probably fine, as we're really only interested in the unpacked > > >> data, not the CSI-2 packed. And when writing this data to memory, the > > >> DMA engine can write it either as is, or unpack each pixel to a 16-bit > > >> container. > > >> > > >> For MEDIA_BUS_FMT_META_10, we define it similarly to > > >> MEDIA_BUS_FMT_SBGGR10_1X10, except the lowest 2 bits are marked as > > >> padding bits. And, I think, MEDIA_BUS_FMT_META_10 implies RAW10 CSI-2 > > >> format. > > > > > > That's how I understand it too > > > > > >> However, when writing the data to memory, we don't want either > > >> of the modes used in the above pixel data case, but rather we want to > > >> write the data as it is in the CSI-2 bus. So, the DMA engine can either > > >> reverse the RAW10 unpacking to get the wanted output format, or > > >> alternatively the CSI-2 receiver could instead use RAW8 mode to avoid > > >> any unpacking. > > > > > > I don't think the DMA engine would "reverse the RAW10 packing" here. It > > > will write bytes as they arrive (or more likely, a set of bytes at a > > > time to operate on bursts). The other option would be for the DMA engine > > > to drop the padding bytes, but I don't know of any CSI-2 receiver that > > > can do that. > > > > Well, for pixel data, TI CAL has the following options when writing to > > memory for, e.g. RAW12: > > - RAW12 MIPI (a separate byte for lsbs) > > - RAW12 Linear (12 consecutive bits per pixel) > > - RAW16 (12 consecutive bits per pixel in a 16bit container) > > > > Of these, only the RAW12 MIPI makes sense for embedded data (actually > > the whole processing block can be and should be skipped for embedded > > data, so it's more like CAL is receiving RAW8 and writing out RAW8). > > > > Or, if it would be possible, skipping the padding bytes. Which looks > > like it is almost possible with TI CAL, but not quite: it can write with > > RAW8, but afaics that always takes the lowest bits of the 12-bit pixel, > > but here we'd like it to take the highest bits. > > That's a weird one, even for image data, dropping the MSBs doesn't make > much sense, while writing the RAW8 MSB when receiving RAW10 or RAW12 > would be useful. I'd suppose it would drop the four most significant bits if it was a parallel receiver. Does CAL have 8-bit input support? That should just work in this case. -- Regards, Sakari Ailus
