Hello, On Wed, Jan 26, 2022 at 11:50:21AM +0000, Dave Stevenson wrote: > On Mon, 24 Jan 2022 at 15:22, Pratyush Yadav <p.yadav@xxxxxx> wrote: > > On 23/01/22 06:08PM, Laurent Pinchart wrote: > > > There are many CSI-2-related drivers in the media subsystem that come > > > with their own macros to handle the CSI-2 data types (or just hardcode > > > the numerical values). Provide a shared header with definitions for > > > those data types that driver can use. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > > > --- > > > include/media/mipi-csi2.h | 45 +++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 45 insertions(+) > > > create mode 100644 include/media/mipi-csi2.h > > > > > > diff --git a/include/media/mipi-csi2.h b/include/media/mipi-csi2.h > > > new file mode 100644 > > > index 000000000000..392794e5badd > > > --- /dev/null > > > +++ b/include/media/mipi-csi2.h > > > @@ -0,0 +1,45 @@ > > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > > +/* > > > + * MIPI CSI-2 Data Types > > > + * > > > + * Copyright (C) 2022 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > > + */ > > > + > > > +#ifndef _MEDIA_MIPI_CSI2_H > > > +#define _MEDIA_MIPI_CSI2_H > > > + > > > +/* Short packet data types */ > > > +#define MIPI_CSI2_DT_FS 0x00 > > > +#define MIPI_CSI2_DT_FE 0x01 > > > +#define MIPI_CSI2_DT_LS 0x02 > > > +#define MIPI_CSI2_DT_LE 0x03 > > > +#define MIPI_CSI2_DT_GENERIC_SHORT(n) (0x08 + (n)) /* 0..7 */ > > > > IIUC there is currently no way to actually capture packets with these > > data types, and these are added here for completeness's sake, right? > > They aren't generally captured, but are of use. > For Unicam we have a packet compare & capture trigger generally used > for debug. However it can also be used for capturing the 16bit frame > number attached to FS and FE events. > It's been of use for devices such as Analog Devices ADV728[0|2]M which > use the frame number to identify the field when sending interlaced > content. > > > > + > > > +/* Long packet data types */ > > > +#define MIPI_CSI2_DT_NULL 0x10 > > > +#define MIPI_CSI2_DT_BLANKING 0x11 > > > +#define MIPI_CSI2_DT_EMBEDDED_8B 0x12 > > > +#define MIPI_CSI2_DT_YUV420_8B 0x18 > > > +#define MIPI_CSI2_DT_YUV420_10B 0x19 > > > +#define MIPI_CSI2_DT_YUV420_8B_LEGACY 0x1a > > > +#define MIPI_CSI2_DT_YUV420_8B_CS 0x1c > > > +#define MIPI_CSI2_DT_YUV420_10B_CS 0x1d > > > +#define MIPI_CSI2_DT_YUV422_8B 0x1e > > > +#define MIPI_CSI2_DT_YUV422_10B 0x1f > > > +#define MIPI_CSI2_DT_RGB444 0x20 > > > +#define MIPI_CSI2_DT_RGB555 0x21 > > > +#define MIPI_CSI2_DT_RGB565 0x22 > > > +#define MIPI_CSI2_DT_RGB666 0x23 > > > +#define MIPI_CSI2_DT_RGB888 0x24 > > > +#define MIPI_CSI2_DT_RAW24 0x27 > > > > I have the CSI-2 spec v1.3, and it lists 0x27 as reserved under RGB > > Image data, and I don't see a data type value for RAW24. Where did you > > get this value from? > > > > > +#define MIPI_CSI2_DT_RAW6 0x28 > > > +#define MIPI_CSI2_DT_RAW7 0x29 > > > +#define MIPI_CSI2_DT_RAW8 0x2a > > > +#define MIPI_CSI2_DT_RAW10 0x2b > > > +#define MIPI_CSI2_DT_RAW12 0x2c > > > +#define MIPI_CSI2_DT_RAW14 0x2d > > > +#define MIPI_CSI2_DT_RAW16 0x2e > > > +#define MIPI_CSI2_DT_RAW20 0x2f > > > > These two are also listed as reserved in the spec I have. Rest of the > > values look good to me. > > I'm also only on v1.3, but otherwise agree that all the other values match. > I see that MIPI are now up to v4.0, and their performance > highlights[1] include RAW16 and RAW24 support, so I assume they have > been added in a later revision. I don't have access to more a more recent version of the CSI-2 specification, but I've gathered RAW16, RAW20 and RAW24 from out-of-tree code. Sakari, could you confirm those values ? > [1] https://www.mipi.org/specifications/csi-2 > > > > +#define MIPI_CSI2_DT_USER_DEFINED(n) (0x30 + (n)) /* 0..7 */ > > > + > > > +#endif /* _MEDIA_MIPI_CSI2_H */ > > > > I think this patch is a good idea in general, and it should remove a lot > > of repetition in the drivers. > > > > BTW, I also see lots of drivers adding tables having mapping between > > MBUS formats, FOURCC formats, bpp, data type, etc. It would be useful to > > have those in a central place IMO. I agree. Patches are welcome :-) -- Regards, Laurent Pinchart
