RE: [PATCH 1/3] media: imx8mq-mipi-csi2: Add data type field in pixel format

[G.N. Zhou (OSS)]

Hi Frank,

Thanks for your feedback.

> -----Original Message-----
> From: Frank Li <frank.li@xxxxxxx>
> Sent: Thursday, July 17, 2025 11:44 PM
> To: G.N. Zhou (OSS) <guoniu.zhou@xxxxxxxxxxx>
> Cc: rmfrfs@xxxxxxxxx; Laurent Pinchart
> <laurent.pinchart@xxxxxxxxxxxxxxxx>; martink@xxxxxxxxx; kernel@xxxxxxx;
> mchehab@xxxxxxxxxx; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx;
> kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; linux-media@xxxxxxxxxxxxxxx;
> imx@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/3] media: imx8mq-mipi-csi2: Add data type field in pixel
> format
> 
> On Thu, Jul 17, 2025 at 03:13:57PM +0800, guoniu.zhou@xxxxxxxxxxx wrote:
> > From: "Guoniu.zhou" <guoniu.zhou@xxxxxxx>
> >
> > Add data type field in CSI pixel format info since the downstream
> > subdev in the pipeline need to know.
> >
> > Signed-off-by: Guoniu.zhou <guoniu.zhou@xxxxxxx>
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@xxxxxxx>
> 
> double signed off line.

[G.N] Will remove it in next version.

> 
> > ---
> >  drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 23
> > +++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > index 3a4645f59a44..7495400adc22 100644
> > --- a/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > +++ b/drivers/media/platform/nxp/imx8mq-mipi-csi2.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/reset.h>
> >  #include <linux/spinlock.h>
> >
> > +#include <media/mipi-csi2.h>
> >  #include <media/v4l2-common.h>
> >  #include <media/v4l2-device.h>
> >  #include <media/v4l2-fwnode.h>
> > @@ -138,6 +139,7 @@ struct csi_state {
> >
> >  struct csi2_pix_format {
> >  	u32 code;
> > +	u32 data_type;
> >  	u8 width;
> >  };
> >
> > @@ -262,68 +264,89 @@ static const struct csi2_pix_format
> imx8mq_mipi_csi_formats[] = {
> >  	/* RAW (Bayer and greyscale) formats. */
> >  	{
> >  		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
> > +		.data_type = MIPI_CSI2_DT_RAW8,
> 
> I found many place have similar convert. Is it fixed map for csi2?  If yes,

[G.N] Most of them are fixed, but some of them may depend on device specific, so I suggest to let driver to handle it.

> 
> Can you add helper function, like
> 
> in include mipi-csi2.h
> 
> media_bus_fmt_to_csi(int bus_fmt)
> {
> 	switch (buf_fmt)
> 	{
> 	case MEDIA_BUS_FMT_SBGGR8_1X8:
> 		return MIPI_CSI2_DT_RAW8;
> 
> 	...
> 	}
> }
> 
> Frank
> 
> >  		.width = 8,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
> > +		.data_type = MIPI_CSI2_DT_RAW8,
> >  		.width = 8,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
> > +		.data_type = MIPI_CSI2_DT_RAW8,
> >  		.width = 8,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
> > +		.data_type = MIPI_CSI2_DT_RAW8,
> >  		.width = 8,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_Y8_1X8,
> > +		.data_type = MIPI_CSI2_DT_RAW8,
> >  		.width = 8,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SBGGR10_1X10,
> > +		.data_type = MIPI_CSI2_DT_RAW10,
> >  		.width = 10,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SGBRG10_1X10,
> > +		.data_type = MIPI_CSI2_DT_RAW10,
> >  		.width = 10,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SGRBG10_1X10,
> > +		.data_type = MIPI_CSI2_DT_RAW10,
> >  		.width = 10,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SRGGB10_1X10,
> > +		.data_type = MIPI_CSI2_DT_RAW10,
> >  		.width = 10,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_Y10_1X10,
> > +		.data_type = MIPI_CSI2_DT_RAW10,
> >  		.width = 10,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SBGGR12_1X12,
> > +		.data_type = MIPI_CSI2_DT_RAW12,
> >  		.width = 12,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SGBRG12_1X12,
> > +		.data_type = MIPI_CSI2_DT_RAW12,
> >  		.width = 12,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SGRBG12_1X12,
> > +		.data_type = MIPI_CSI2_DT_RAW12,
> >  		.width = 12,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SRGGB12_1X12,
> > +		.data_type = MIPI_CSI2_DT_RAW12,
> >  		.width = 12,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_Y12_1X12,
> > +		.data_type = MIPI_CSI2_DT_RAW12,
> >  		.width = 12,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SBGGR14_1X14,
> > +		.data_type = MIPI_CSI2_DT_RAW14,
> >  		.width = 14,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SGBRG14_1X14,
> > +		.data_type = MIPI_CSI2_DT_RAW14,
> >  		.width = 14,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SGRBG14_1X14,
> > +		.data_type = MIPI_CSI2_DT_RAW14,
> >  		.width = 14,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_SRGGB14_1X14,
> > +		.data_type = MIPI_CSI2_DT_RAW14,
> >  		.width = 14,
> >  	},
> >  	/* YUV formats */
> >  	{
> >  		.code = MEDIA_BUS_FMT_YUYV8_1X16,
> > +		.data_type = MIPI_CSI2_DT_YUV422_8B,
> >  		.width = 16,
> >  	}, {
> >  		.code = MEDIA_BUS_FMT_UYVY8_1X16,
> > +		.data_type = MIPI_CSI2_DT_YUV422_8B,
> >  		.width = 16,
> >  	}
> >  };
> > --
> > 2.34.1
> >