Hi Laurent, Thanks for the patch. 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? > + > +/* 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. > +#define MIPI_CSI2_DT_USER_DEFINED(n) (0x30 + (n)) /* 0..7 */ > + > +#endif /* _MEDIA_MIPI_CSI2_H */ > -- > Regards, > > Laurent Pinchart > 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. -- Regards, Pratyush Yadav Texas Instruments Inc.
