[PATCH 0/6] media: Centralize MIPI CSI-2 data types in shared header

[PATCH 0/6] media: Centralize MIPI CSI-2 data types in shared header

[Laurent Pinchart]

Hello,

This short patch series is the result of me being fed up with seeing
MIPI CSI-2 data types duplicated through drivers.

Patch 1/6 adds a new shared header to define the data types, and patches
2/6 to 6/6 replace custom macros or hardcoded values in drivers. There
are a few other drivers left that should still be updated, additional
patches are welcome on top of this series :-)

Laurent Pinchart (6):
  media: Define MIPI CSI-2 data types in a shared header file
  media: cadence: cdns-csi2tx: Use mipi-csi2.h
  media: rcar-isp: Use mipi-csi2.h
  media: rcar-csi2: Use mipi-csi2.h
  media: rockchip: rkisp1: Use mipi-csi2.h
  media: xilinx: csi2rxss: Use mipi-csi2.h

 drivers/media/platform/cadence/cdns-csi2tx.c  |   5 +-
 drivers/media/platform/rcar-isp.c             |  32 +++++-
 drivers/media/platform/rcar-vin/rcar-csi2.c   |  57 ++++++++--
 .../platform/rockchip/rkisp1/rkisp1-isp.c     |  34 +++---
 .../platform/rockchip/rkisp1/rkisp1-regs.h    |  11 --
 .../media/platform/xilinx/xilinx-csi2rxss.c   | 106 ++++++++----------
 include/media/mipi-csi2.h                     |  45 ++++++++
 7 files changed, 183 insertions(+), 107 deletions(-)
 create mode 100644 include/media/mipi-csi2.h


base-commit: 68b9bcc8a534cd11fe55f8bc82f948aae7d81b3c
-- 
Regards,

Laurent Pinchart

[PATCH 1/6] media: Define MIPI CSI-2 data types in a shared header file

[Laurent Pinchart]

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@ideasonboard.com>
---
 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@ideasonboard.com>
+ */
+
+#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 */
+
+/* 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
+#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
+#define MIPI_CSI2_DT_USER_DEFINED(n)	(0x30 + (n))	/* 0..7 */
+
+#endif /* _MEDIA_MIPI_CSI2_H */
-- 
Regards,

Laurent Pinchart

[PATCH 2/6] media: cadence: cdns-csi2tx: Use mipi-csi2.h

[Laurent Pinchart]

Replace the hardcoded MIPI CSI-2 data types with macros from
mipi-csi2.h.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/cadence/cdns-csi2tx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/cadence/cdns-csi2tx.c b/drivers/media/platform/cadence/cdns-csi2tx.c
index 8f8c36056354..58e405b69f67 100644
--- a/drivers/media/platform/cadence/cdns-csi2tx.c
+++ b/drivers/media/platform/cadence/cdns-csi2tx.c
@@ -15,6 +15,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
+#include <media/mipi-csi2.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-fwnode.h>
@@ -121,12 +122,12 @@ static const struct csi2tx_fmt csi2tx_formats[] = {
 	{
 		.mbus	= MEDIA_BUS_FMT_UYVY8_1X16,
 		.bpp	= 2,
-		.dt	= 0x1e,
+		.dt	= MIPI_CSI2_DT_YUV422_8B,
 	},
 	{
 		.mbus	= MEDIA_BUS_FMT_RGB888_1X24,
 		.bpp	= 3,
-		.dt	= 0x24,
+		.dt	= MIPI_CSI2_DT_RGB888,
 	},
 };
 
-- 
Regards,

Laurent Pinchart

[PATCH 3/6] media: rcar-isp: Use mipi-csi2.h

[Laurent Pinchart]

Replace the hardcoded MIPI CSI-2 data types with macros from
mipi-csi2.h.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/rcar-isp.c | 32 +++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/rcar-isp.c b/drivers/media/platform/rcar-isp.c
index 2ffab30bc011..10b3474f93a4 100644
--- a/drivers/media/platform/rcar-isp.c
+++ b/drivers/media/platform/rcar-isp.c
@@ -17,6 +17,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
 
+#include <media/mipi-csi2.h>
 #include <media/v4l2-subdev.h>
 
 #define ISPINPUTSEL0_REG				0x0008
@@ -51,12 +52,31 @@ struct rcar_isp_format {
 };
 
 static const struct rcar_isp_format rcar_isp_formats[] = {
-	{ .code = MEDIA_BUS_FMT_RGB888_1X24,	.datatype = 0x24, .procmode = 0x15 },
-	{ .code = MEDIA_BUS_FMT_Y10_1X10,	.datatype = 0x2b, .procmode = 0x10 },
-	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .procmode = 0x0c },
-	{ .code = MEDIA_BUS_FMT_YUYV8_1X16,	.datatype = 0x1e, .procmode = 0x0c },
-	{ .code = MEDIA_BUS_FMT_UYVY8_2X8,	.datatype = 0x1e, .procmode = 0x0c },
-	{ .code = MEDIA_BUS_FMT_YUYV10_2X10,	.datatype = 0x1e, .procmode = 0x0c },
+	{
+		.code = MEDIA_BUS_FMT_RGB888_1X24,
+		.datatype = MIPI_CSI2_DT_RGB888,
+		.procmode = 0x15
+	}, {
+		.code = MEDIA_BUS_FMT_Y10_1X10,
+		.datatype = MIPI_CSI2_DT_RAW10,
+		.procmode = 0x10,
+	}, {
+		.code = MEDIA_BUS_FMT_UYVY8_1X16,
+		.datatype = MIPI_CSI2_DT_YUV422_8B,
+		.procmode = 0x0c,
+	}, {
+		.code = MEDIA_BUS_FMT_YUYV8_1X16,
+		.datatype = MIPI_CSI2_DT_YUV422_8B,
+		.procmode = 0x0c,
+	}, {
+		.code = MEDIA_BUS_FMT_UYVY8_2X8,
+		.datatype = MIPI_CSI2_DT_YUV422_8B,
+		.procmode = 0x0c,
+	}, {
+		.code = MEDIA_BUS_FMT_YUYV10_2X10,
+		.datatype = MIPI_CSI2_DT_YUV422_8B,
+		.procmode = 0x0c,
+	},
 };
 
 static const struct rcar_isp_format *risp_code_to_fmt(unsigned int code)
-- 
Regards,

Laurent Pinchart

[PATCH 4/6] media: rcar-csi2: Use mipi-csi2.h

[Laurent Pinchart]

Replace the hardcoded MIPI CSI-2 data types with macros from
mipi-csi2.h.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 57 +++++++++++++++++----
 1 file changed, 46 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index 8c939cb3073d..93df9a5e3d80 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -17,6 +17,7 @@
 #include <linux/reset.h>
 #include <linux/sys_soc.h>
 
+#include <media/mipi-csi2.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-fwnode.h>
@@ -412,17 +413,51 @@ struct rcar_csi2_format {
 };
 
 static const struct rcar_csi2_format rcar_csi2_formats[] = {
-	{ .code = MEDIA_BUS_FMT_RGB888_1X24,	.datatype = 0x24, .bpp = 24 },
-	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .bpp = 16 },
-	{ .code = MEDIA_BUS_FMT_YUYV8_1X16,	.datatype = 0x1e, .bpp = 16 },
-	{ .code = MEDIA_BUS_FMT_UYVY8_2X8,	.datatype = 0x1e, .bpp = 16 },
-	{ .code = MEDIA_BUS_FMT_YUYV10_2X10,	.datatype = 0x1e, .bpp = 20 },
-	{ .code = MEDIA_BUS_FMT_Y10_1X10,	.datatype = 0x2b, .bpp = 10 },
-	{ .code = MEDIA_BUS_FMT_SBGGR8_1X8,     .datatype = 0x2a, .bpp = 8 },
-	{ .code = MEDIA_BUS_FMT_SGBRG8_1X8,     .datatype = 0x2a, .bpp = 8 },
-	{ .code = MEDIA_BUS_FMT_SGRBG8_1X8,     .datatype = 0x2a, .bpp = 8 },
-	{ .code = MEDIA_BUS_FMT_SRGGB8_1X8,     .datatype = 0x2a, .bpp = 8 },
-	{ .code = MEDIA_BUS_FMT_Y8_1X8,		.datatype = 0x2a, .bpp = 8 },
+	{
+		.code = MEDIA_BUS_FMT_RGB888_1X24,
+		.datatype = MIPI_CSI2_DT_RGB888,
+		.bpp = 24,
+	}, {
+		.code = MEDIA_BUS_FMT_UYVY8_1X16,
+		.datatype = MIPI_CSI2_DT_YUV422_8B,
+		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_YUYV8_1X16,
+		.datatype = MIPI_CSI2_DT_YUV422_8B,
+		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_UYVY8_2X8,
+		.datatype = MIPI_CSI2_DT_YUV422_8B,
+		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_YUYV10_2X10,
+		.datatype = MIPI_CSI2_DT_YUV422_8B,
+		.bpp = 20,
+	}, {
+		.code = MEDIA_BUS_FMT_Y10_1X10,
+		.datatype = MIPI_CSI2_DT_RAW10,
+		.bpp = 10,
+	}, {
+		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
+		.datatype = MIPI_CSI2_DT_RAW8,
+		.bpp = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
+		.datatype = MIPI_CSI2_DT_RAW8,
+		.bpp = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
+		.datatype = MIPI_CSI2_DT_RAW8,
+		.bpp = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
+		.datatype = MIPI_CSI2_DT_RAW8,
+		.bpp = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_Y8_1X8,
+		.datatype = MIPI_CSI2_DT_RAW8,
+		.bpp = 8,
+	},
 };
 
 static const struct rcar_csi2_format *rcsi2_code_to_fmt(unsigned int code)
-- 
Regards,

Laurent Pinchart

[PATCH 5/6] media: rockchip: rkisp1: Use mipi-csi2.h

[Laurent Pinchart]

Replace the driver-specific definitions of MIPI CSI-2 data types with
macros from mipi-csi2.h.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../platform/rockchip/rkisp1/rkisp1-isp.c     | 34 ++++++++++---------
 .../platform/rockchip/rkisp1/rkisp1-regs.h    | 11 ------
 2 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
index 2a35bf24e54e..4415c7248c2f 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
@@ -14,6 +14,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/videodev2.h>
 #include <linux/vmalloc.h>
+
+#include <media/mipi-csi2.h>
 #include <media/v4l2-event.h>
 
 #include "rkisp1-common.h"
@@ -62,112 +64,112 @@ static const struct rkisp1_isp_mbus_info rkisp1_isp_formats[] = {
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SRGGB10_1X10,
 		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW10,
+		.mipi_dt	= MIPI_CSI2_DT_RAW10,
 		.bayer_pat	= RKISP1_RAW_RGGB,
 		.bus_width	= 10,
 		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SBGGR10_1X10,
 		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW10,
+		.mipi_dt	= MIPI_CSI2_DT_RAW10,
 		.bayer_pat	= RKISP1_RAW_BGGR,
 		.bus_width	= 10,
 		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SGBRG10_1X10,
 		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW10,
+		.mipi_dt	= MIPI_CSI2_DT_RAW10,
 		.bayer_pat	= RKISP1_RAW_GBRG,
 		.bus_width	= 10,
 		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SGRBG10_1X10,
 		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW10,
+		.mipi_dt	= MIPI_CSI2_DT_RAW10,
 		.bayer_pat	= RKISP1_RAW_GRBG,
 		.bus_width	= 10,
 		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SRGGB12_1X12,
 		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW12,
+		.mipi_dt	= MIPI_CSI2_DT_RAW12,
 		.bayer_pat	= RKISP1_RAW_RGGB,
 		.bus_width	= 12,
 		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SBGGR12_1X12,
 		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW12,
+		.mipi_dt	= MIPI_CSI2_DT_RAW12,
 		.bayer_pat	= RKISP1_RAW_BGGR,
 		.bus_width	= 12,
 		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SGBRG12_1X12,
 		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW12,
+		.mipi_dt	= MIPI_CSI2_DT_RAW12,
 		.bayer_pat	= RKISP1_RAW_GBRG,
 		.bus_width	= 12,
 		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SGRBG12_1X12,
 		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW12,
+		.mipi_dt	= MIPI_CSI2_DT_RAW12,
 		.bayer_pat	= RKISP1_RAW_GRBG,
 		.bus_width	= 12,
 		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SRGGB8_1X8,
 		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW8,
+		.mipi_dt	= MIPI_CSI2_DT_RAW8,
 		.bayer_pat	= RKISP1_RAW_RGGB,
 		.bus_width	= 8,
 		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SBGGR8_1X8,
 		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW8,
+		.mipi_dt	= MIPI_CSI2_DT_RAW8,
 		.bayer_pat	= RKISP1_RAW_BGGR,
 		.bus_width	= 8,
 		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SGBRG8_1X8,
 		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW8,
+		.mipi_dt	= MIPI_CSI2_DT_RAW8,
 		.bayer_pat	= RKISP1_RAW_GBRG,
 		.bus_width	= 8,
 		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SGRBG8_1X8,
 		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW8,
+		.mipi_dt	= MIPI_CSI2_DT_RAW8,
 		.bayer_pat	= RKISP1_RAW_GRBG,
 		.bus_width	= 8,
 		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_YUYV8_1X16,
 		.pixel_enc	= V4L2_PIXEL_ENC_YUV,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_YUV422_8b,
+		.mipi_dt	= MIPI_CSI2_DT_YUV422_8B,
 		.yuv_seq	= RKISP1_CIF_ISP_ACQ_PROP_YCBYCR,
 		.bus_width	= 16,
 		.direction	= RKISP1_ISP_SD_SINK,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_YVYU8_1X16,
 		.pixel_enc	= V4L2_PIXEL_ENC_YUV,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_YUV422_8b,
+		.mipi_dt	= MIPI_CSI2_DT_YUV422_8B,
 		.yuv_seq	= RKISP1_CIF_ISP_ACQ_PROP_YCRYCB,
 		.bus_width	= 16,
 		.direction	= RKISP1_ISP_SD_SINK,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_UYVY8_1X16,
 		.pixel_enc	= V4L2_PIXEL_ENC_YUV,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_YUV422_8b,
+		.mipi_dt	= MIPI_CSI2_DT_YUV422_8B,
 		.yuv_seq	= RKISP1_CIF_ISP_ACQ_PROP_CBYCRY,
 		.bus_width	= 16,
 		.direction	= RKISP1_ISP_SD_SINK,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_VYUY8_1X16,
 		.pixel_enc	= V4L2_PIXEL_ENC_YUV,
-		.mipi_dt	= RKISP1_CIF_CSI2_DT_YUV422_8b,
+		.mipi_dt	= MIPI_CSI2_DT_YUV422_8B,
 		.yuv_seq	= RKISP1_CIF_ISP_ACQ_PROP_CRYCBY,
 		.bus_width	= 16,
 		.direction	= RKISP1_ISP_SD_SINK,
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
index d326214c7e07..82f8d33d98b3 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
@@ -333,17 +333,6 @@
 /* MIPI_DATA_SEL */
 #define RKISP1_CIF_MIPI_DATA_SEL_VC(a)			(((a) & 0x3) << 6)
 #define RKISP1_CIF_MIPI_DATA_SEL_DT(a)			(((a) & 0x3F) << 0)
-/* MIPI DATA_TYPE */
-#define RKISP1_CIF_CSI2_DT_YUV420_8b			0x18
-#define RKISP1_CIF_CSI2_DT_YUV420_10b			0x19
-#define RKISP1_CIF_CSI2_DT_YUV422_8b			0x1E
-#define RKISP1_CIF_CSI2_DT_YUV422_10b			0x1F
-#define RKISP1_CIF_CSI2_DT_RGB565			0x22
-#define RKISP1_CIF_CSI2_DT_RGB666			0x23
-#define RKISP1_CIF_CSI2_DT_RGB888			0x24
-#define RKISP1_CIF_CSI2_DT_RAW8				0x2A
-#define RKISP1_CIF_CSI2_DT_RAW10			0x2B
-#define RKISP1_CIF_CSI2_DT_RAW12			0x2C
 
 /* MIPI_IMSC, MIPI_RIS, MIPI_MIS, MIPI_ICR, MIPI_ISR */
 #define RKISP1_CIF_MIPI_SYNC_FIFO_OVFLW(a)		(((a) & 0xF) << 0)
-- 
Regards,

Laurent Pinchart

[PATCH 6/6] media: xilinx: csi2rxss: Use mipi-csi2.h

[Laurent Pinchart]

Replace the driver-specific definitions of MIPI CSI-2 data types with
macros from mipi-csi2.h.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../media/platform/xilinx/xilinx-csi2rxss.c   | 106 ++++++++----------
 1 file changed, 45 insertions(+), 61 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-csi2rxss.c b/drivers/media/platform/xilinx/xilinx-csi2rxss.c
index b1baf9d7b6ec..051c60cba1e0 100644
--- a/drivers/media/platform/xilinx/xilinx-csi2rxss.c
+++ b/drivers/media/platform/xilinx/xilinx-csi2rxss.c
@@ -18,6 +18,7 @@
 #include <linux/platform_device.h>
 #include <linux/v4l2-subdev.h>
 #include <media/media-entity.h>
+#include <media/mipi-csi2.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-fwnode.h>
@@ -115,23 +116,6 @@
 #define XCSI_DEFAULT_WIDTH	1920
 #define XCSI_DEFAULT_HEIGHT	1080
 
-/* MIPI CSI-2 Data Types from spec */
-#define XCSI_DT_YUV4228B	0x1e
-#define XCSI_DT_YUV42210B	0x1f
-#define XCSI_DT_RGB444		0x20
-#define XCSI_DT_RGB555		0x21
-#define XCSI_DT_RGB565		0x22
-#define XCSI_DT_RGB666		0x23
-#define XCSI_DT_RGB888		0x24
-#define XCSI_DT_RAW6		0x28
-#define XCSI_DT_RAW7		0x29
-#define XCSI_DT_RAW8		0x2a
-#define XCSI_DT_RAW10		0x2b
-#define XCSI_DT_RAW12		0x2c
-#define XCSI_DT_RAW14		0x2d
-#define XCSI_DT_RAW16		0x2e
-#define XCSI_DT_RAW20		0x2f
-
 #define XCSI_VCX_START		4
 #define XCSI_MAX_VC		4
 #define XCSI_MAX_VCX		16
@@ -183,32 +167,32 @@ static const struct xcsi2rxss_event xcsi2rxss_events[] = {
  * and media bus formats
  */
 static const u32 xcsi2dt_mbus_lut[][2] = {
-	{ XCSI_DT_YUV4228B, MEDIA_BUS_FMT_UYVY8_1X16 },
-	{ XCSI_DT_YUV42210B, MEDIA_BUS_FMT_UYVY10_1X20 },
-	{ XCSI_DT_RGB444, 0 },
-	{ XCSI_DT_RGB555, 0 },
-	{ XCSI_DT_RGB565, 0 },
-	{ XCSI_DT_RGB666, 0 },
-	{ XCSI_DT_RGB888, MEDIA_BUS_FMT_RBG888_1X24 },
-	{ XCSI_DT_RAW6, 0 },
-	{ XCSI_DT_RAW7, 0 },
-	{ XCSI_DT_RAW8, MEDIA_BUS_FMT_SRGGB8_1X8 },
-	{ XCSI_DT_RAW8, MEDIA_BUS_FMT_SBGGR8_1X8 },
-	{ XCSI_DT_RAW8, MEDIA_BUS_FMT_SGBRG8_1X8 },
-	{ XCSI_DT_RAW8, MEDIA_BUS_FMT_SGRBG8_1X8 },
-	{ XCSI_DT_RAW10, MEDIA_BUS_FMT_SRGGB10_1X10 },
-	{ XCSI_DT_RAW10, MEDIA_BUS_FMT_SBGGR10_1X10 },
-	{ XCSI_DT_RAW10, MEDIA_BUS_FMT_SGBRG10_1X10 },
-	{ XCSI_DT_RAW10, MEDIA_BUS_FMT_SGRBG10_1X10 },
-	{ XCSI_DT_RAW12, MEDIA_BUS_FMT_SRGGB12_1X12 },
-	{ XCSI_DT_RAW12, MEDIA_BUS_FMT_SBGGR12_1X12 },
-	{ XCSI_DT_RAW12, MEDIA_BUS_FMT_SGBRG12_1X12 },
-	{ XCSI_DT_RAW12, MEDIA_BUS_FMT_SGRBG12_1X12 },
-	{ XCSI_DT_RAW16, MEDIA_BUS_FMT_SRGGB16_1X16 },
-	{ XCSI_DT_RAW16, MEDIA_BUS_FMT_SBGGR16_1X16 },
-	{ XCSI_DT_RAW16, MEDIA_BUS_FMT_SGBRG16_1X16 },
-	{ XCSI_DT_RAW16, MEDIA_BUS_FMT_SGRBG16_1X16 },
-	{ XCSI_DT_RAW20, 0 },
+	{ MIPI_CSI2_DT_YUV422_8B, MEDIA_BUS_FMT_UYVY8_1X16 },
+	{ MIPI_CSI2_DT_YUV422_10B, MEDIA_BUS_FMT_UYVY10_1X20 },
+	{ MIPI_CSI2_DT_RGB444, 0 },
+	{ MIPI_CSI2_DT_RGB555, 0 },
+	{ MIPI_CSI2_DT_RGB565, 0 },
+	{ MIPI_CSI2_DT_RGB666, 0 },
+	{ MIPI_CSI2_DT_RGB888, MEDIA_BUS_FMT_RBG888_1X24 },
+	{ MIPI_CSI2_DT_RAW6, 0 },
+	{ MIPI_CSI2_DT_RAW7, 0 },
+	{ MIPI_CSI2_DT_RAW8, MEDIA_BUS_FMT_SRGGB8_1X8 },
+	{ MIPI_CSI2_DT_RAW8, MEDIA_BUS_FMT_SBGGR8_1X8 },
+	{ MIPI_CSI2_DT_RAW8, MEDIA_BUS_FMT_SGBRG8_1X8 },
+	{ MIPI_CSI2_DT_RAW8, MEDIA_BUS_FMT_SGRBG8_1X8 },
+	{ MIPI_CSI2_DT_RAW10, MEDIA_BUS_FMT_SRGGB10_1X10 },
+	{ MIPI_CSI2_DT_RAW10, MEDIA_BUS_FMT_SBGGR10_1X10 },
+	{ MIPI_CSI2_DT_RAW10, MEDIA_BUS_FMT_SGBRG10_1X10 },
+	{ MIPI_CSI2_DT_RAW10, MEDIA_BUS_FMT_SGRBG10_1X10 },
+	{ MIPI_CSI2_DT_RAW12, MEDIA_BUS_FMT_SRGGB12_1X12 },
+	{ MIPI_CSI2_DT_RAW12, MEDIA_BUS_FMT_SBGGR12_1X12 },
+	{ MIPI_CSI2_DT_RAW12, MEDIA_BUS_FMT_SGBRG12_1X12 },
+	{ MIPI_CSI2_DT_RAW12, MEDIA_BUS_FMT_SGRBG12_1X12 },
+	{ MIPI_CSI2_DT_RAW16, MEDIA_BUS_FMT_SRGGB16_1X16 },
+	{ MIPI_CSI2_DT_RAW16, MEDIA_BUS_FMT_SBGGR16_1X16 },
+	{ MIPI_CSI2_DT_RAW16, MEDIA_BUS_FMT_SGBRG16_1X16 },
+	{ MIPI_CSI2_DT_RAW16, MEDIA_BUS_FMT_SGRBG16_1X16 },
+	{ MIPI_CSI2_DT_RAW20, 0 },
 };
 
 /**
@@ -791,7 +775,7 @@ static int xcsi2rxss_set_format(struct v4l2_subdev *sd,
 	 * other RAW, YUV422 8/10 or RGB888, set appropriate media bus format.
 	 */
 	dt = xcsi2rxss_get_dt(fmt->format.code);
-	if (dt != xcsi2rxss->datatype && dt != XCSI_DT_RAW8) {
+	if (dt != xcsi2rxss->datatype && dt != MIPI_CSI2_DT_RAW8) {
 		dev_dbg(xcsi2rxss->dev, "Unsupported media bus format");
 		/* set the default format for the data type */
 		fmt->format.code = xcsi2rxss_get_nth_mbus(xcsi2rxss->datatype,
@@ -823,8 +807,8 @@ static int xcsi2rxss_enum_mbus_code(struct v4l2_subdev *sd,
 	/* RAW8 dt packets are available in all DT configurations */
 	if (code->index < 4) {
 		n = code->index;
-		dt = XCSI_DT_RAW8;
-	} else if (state->datatype != XCSI_DT_RAW8) {
+		dt = MIPI_CSI2_DT_RAW8;
+	} else if (state->datatype != MIPI_CSI2_DT_RAW8) {
 		n = code->index - 4;
 		dt = state->datatype;
 	} else {
@@ -895,22 +879,22 @@ static int xcsi2rxss_parse_of(struct xcsi2rxss_state *xcsi2rxss)
 	}
 
 	switch (xcsi2rxss->datatype) {
-	case XCSI_DT_YUV4228B:
-	case XCSI_DT_RGB444:
-	case XCSI_DT_RGB555:
-	case XCSI_DT_RGB565:
-	case XCSI_DT_RGB666:
-	case XCSI_DT_RGB888:
-	case XCSI_DT_RAW6:
-	case XCSI_DT_RAW7:
-	case XCSI_DT_RAW8:
-	case XCSI_DT_RAW10:
-	case XCSI_DT_RAW12:
-	case XCSI_DT_RAW14:
+	case MIPI_CSI2_DT_YUV422_8B:
+	case MIPI_CSI2_DT_RGB444:
+	case MIPI_CSI2_DT_RGB555:
+	case MIPI_CSI2_DT_RGB565:
+	case MIPI_CSI2_DT_RGB666:
+	case MIPI_CSI2_DT_RGB888:
+	case MIPI_CSI2_DT_RAW6:
+	case MIPI_CSI2_DT_RAW7:
+	case MIPI_CSI2_DT_RAW8:
+	case MIPI_CSI2_DT_RAW10:
+	case MIPI_CSI2_DT_RAW12:
+	case MIPI_CSI2_DT_RAW14:
 		break;
-	case XCSI_DT_YUV42210B:
-	case XCSI_DT_RAW16:
-	case XCSI_DT_RAW20:
+	case MIPI_CSI2_DT_YUV422_10B:
+	case MIPI_CSI2_DT_RAW16:
+	case MIPI_CSI2_DT_RAW20:
 		if (!en_csi_v20) {
 			ret = -EINVAL;
 			dev_dbg(dev, "enable csi v2 for this pixel format");
-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/6] media: Define MIPI CSI-2 data types in a shared header file

[Niklas Söderlund]

Hi Laurent,

Thanks for your work.

On 2022-01-23 18:08:52 +0200, 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@ideasonboard.com>

I like this effort. I have not double checked each DT code with the spec 
but choose to trust you on those.

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  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@ideasonboard.com>
> + */
> +
> +#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 */
> +
> +/* 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
> +#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
> +#define MIPI_CSI2_DT_USER_DEFINED(n)	(0x30 + (n))	/* 0..7 */
> +
> +#endif /* _MEDIA_MIPI_CSI2_H */
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH 3/6] media: rcar-isp: Use mipi-csi2.h

[Niklas Söderlund]

Hi Laurent,

Thanks for your patch.

On 2022-01-23 18:08:54 +0200, Laurent Pinchart wrote:
> Replace the hardcoded MIPI CSI-2 data types with macros from
> mipi-csi2.h.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/media/platform/rcar-isp.c | 32 +++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-isp.c b/drivers/media/platform/rcar-isp.c
> index 2ffab30bc011..10b3474f93a4 100644
> --- a/drivers/media/platform/rcar-isp.c
> +++ b/drivers/media/platform/rcar-isp.c
> @@ -17,6 +17,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/reset.h>
>  
> +#include <media/mipi-csi2.h>
>  #include <media/v4l2-subdev.h>
>  
>  #define ISPINPUTSEL0_REG				0x0008
> @@ -51,12 +52,31 @@ struct rcar_isp_format {
>  };
>  
>  static const struct rcar_isp_format rcar_isp_formats[] = {
> -	{ .code = MEDIA_BUS_FMT_RGB888_1X24,	.datatype = 0x24, .procmode = 0x15 },
> -	{ .code = MEDIA_BUS_FMT_Y10_1X10,	.datatype = 0x2b, .procmode = 0x10 },
> -	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .procmode = 0x0c },
> -	{ .code = MEDIA_BUS_FMT_YUYV8_1X16,	.datatype = 0x1e, .procmode = 0x0c },
> -	{ .code = MEDIA_BUS_FMT_UYVY8_2X8,	.datatype = 0x1e, .procmode = 0x0c },
> -	{ .code = MEDIA_BUS_FMT_YUYV10_2X10,	.datatype = 0x1e, .procmode = 0x0c },
> +	{
> +		.code = MEDIA_BUS_FMT_RGB888_1X24,
> +		.datatype = MIPI_CSI2_DT_RGB888,
> +		.procmode = 0x15
> +	}, {
> +		.code = MEDIA_BUS_FMT_Y10_1X10,
> +		.datatype = MIPI_CSI2_DT_RAW10,
> +		.procmode = 0x10,
> +	}, {
> +		.code = MEDIA_BUS_FMT_UYVY8_1X16,
> +		.datatype = MIPI_CSI2_DT_YUV422_8B,
> +		.procmode = 0x0c,
> +	}, {
> +		.code = MEDIA_BUS_FMT_YUYV8_1X16,
> +		.datatype = MIPI_CSI2_DT_YUV422_8B,
> +		.procmode = 0x0c,
> +	}, {
> +		.code = MEDIA_BUS_FMT_UYVY8_2X8,
> +		.datatype = MIPI_CSI2_DT_YUV422_8B,
> +		.procmode = 0x0c,
> +	}, {
> +		.code = MEDIA_BUS_FMT_YUYV10_2X10,
> +		.datatype = MIPI_CSI2_DT_YUV422_8B,
> +		.procmode = 0x0c,
> +	},
>  };
>  
>  static const struct rcar_isp_format *risp_code_to_fmt(unsigned int code)
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH 4/6] media: rcar-csi2: Use mipi-csi2.h

[Niklas Söderlund]

Hi Laurent,

Thanks for your patch.

On 2022-01-23 18:08:55 +0200, Laurent Pinchart wrote:
> Replace the hardcoded MIPI CSI-2 data types with macros from
> mipi-csi2.h.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 57 +++++++++++++++++----
>  1 file changed, 46 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index 8c939cb3073d..93df9a5e3d80 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -17,6 +17,7 @@
>  #include <linux/reset.h>
>  #include <linux/sys_soc.h>
>  
> +#include <media/mipi-csi2.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-fwnode.h>
> @@ -412,17 +413,51 @@ struct rcar_csi2_format {
>  };
>  
>  static const struct rcar_csi2_format rcar_csi2_formats[] = {
> -	{ .code = MEDIA_BUS_FMT_RGB888_1X24,	.datatype = 0x24, .bpp = 24 },
> -	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .bpp = 16 },
> -	{ .code = MEDIA_BUS_FMT_YUYV8_1X16,	.datatype = 0x1e, .bpp = 16 },
> -	{ .code = MEDIA_BUS_FMT_UYVY8_2X8,	.datatype = 0x1e, .bpp = 16 },
> -	{ .code = MEDIA_BUS_FMT_YUYV10_2X10,	.datatype = 0x1e, .bpp = 20 },
> -	{ .code = MEDIA_BUS_FMT_Y10_1X10,	.datatype = 0x2b, .bpp = 10 },
> -	{ .code = MEDIA_BUS_FMT_SBGGR8_1X8,     .datatype = 0x2a, .bpp = 8 },
> -	{ .code = MEDIA_BUS_FMT_SGBRG8_1X8,     .datatype = 0x2a, .bpp = 8 },
> -	{ .code = MEDIA_BUS_FMT_SGRBG8_1X8,     .datatype = 0x2a, .bpp = 8 },
> -	{ .code = MEDIA_BUS_FMT_SRGGB8_1X8,     .datatype = 0x2a, .bpp = 8 },
> -	{ .code = MEDIA_BUS_FMT_Y8_1X8,		.datatype = 0x2a, .bpp = 8 },
> +	{
> +		.code = MEDIA_BUS_FMT_RGB888_1X24,
> +		.datatype = MIPI_CSI2_DT_RGB888,
> +		.bpp = 24,
> +	}, {
> +		.code = MEDIA_BUS_FMT_UYVY8_1X16,
> +		.datatype = MIPI_CSI2_DT_YUV422_8B,
> +		.bpp = 16,
> +	}, {
> +		.code = MEDIA_BUS_FMT_YUYV8_1X16,
> +		.datatype = MIPI_CSI2_DT_YUV422_8B,
> +		.bpp = 16,
> +	}, {
> +		.code = MEDIA_BUS_FMT_UYVY8_2X8,
> +		.datatype = MIPI_CSI2_DT_YUV422_8B,
> +		.bpp = 16,
> +	}, {
> +		.code = MEDIA_BUS_FMT_YUYV10_2X10,
> +		.datatype = MIPI_CSI2_DT_YUV422_8B,
> +		.bpp = 20,
> +	}, {
> +		.code = MEDIA_BUS_FMT_Y10_1X10,
> +		.datatype = MIPI_CSI2_DT_RAW10,
> +		.bpp = 10,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
> +		.datatype = MIPI_CSI2_DT_RAW8,
> +		.bpp = 8,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
> +		.datatype = MIPI_CSI2_DT_RAW8,
> +		.bpp = 8,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
> +		.datatype = MIPI_CSI2_DT_RAW8,
> +		.bpp = 8,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
> +		.datatype = MIPI_CSI2_DT_RAW8,
> +		.bpp = 8,
> +	}, {
> +		.code = MEDIA_BUS_FMT_Y8_1X8,
> +		.datatype = MIPI_CSI2_DT_RAW8,
> +		.bpp = 8,
> +	},
>  };
>  
>  static const struct rcar_csi2_format *rcsi2_code_to_fmt(unsigned int code)
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH 1/6] media: Define MIPI CSI-2 data types in a shared header file

[Pratyush Yadav]

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@ideasonboard.com>
> ---
>  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@ideasonboard.com>
> + */
> +
> +#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.

Re: [PATCH 1/6] media: Define MIPI CSI-2 data types in a shared header file

[Kieran Bingham]

Hi Laurent

Quoting Laurent Pinchart (2022-01-23 16:08:52)
> 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@ideasonboard.com>
> ---
>  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@ideasonboard.com>
> + */
> +
> +#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 */
> +
> +/* 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
> +#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
> +#define MIPI_CSI2_DT_USER_DEFINED(n)   (0x30 + (n))    /* 0..7 */

I don't have an easy way to validate those values right now so as with
Niklas I'll leave those to your judgement, and Pratyush's review.

Also along side Pratyush's comment, I concur that the mapping tables too
could be common, but I suspect that's an even bigger topic as maybe that
falls into the trap of also being common to DRM formats...

And finally, are these defines in a location that can be accessible from
device tree? Or would it have to be further duplicated there still?

For instance, the bindings for the Xilinx CSI2 RX explicitly list DT
values to specify as the xlnx,csi-pxl-format which I think should also
come from this common header definition.

For the patches here so far, I can't see anything stark that is wrong
so for the series:


Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

as further extending this to the device tree bindings can be done on
top.


> +
> +#endif /* _MEDIA_MIPI_CSI2_H */
> -- 
> Regards,
> 
> Laurent Pinchart
>

Re: [PATCH 1/6] media: Define MIPI CSI-2 data types in a shared header file

[Dave Stevenson]

Hi Laurent and Pratyush

On Mon, 24 Jan 2022 at 15:22, Pratyush Yadav <p.yadav@ti.com> wrote:
>
> 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@ideasonboard.com>
> > ---
> >  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@ideasonboard.com>
> > + */
> > +
> > +#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.

  Dave

[1] https://www.mipi.org/specifications/csi-2

> > +#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.

Re: [PATCH 1/6] media: Define MIPI CSI-2 data types in a shared header file

[Laurent Pinchart]

Hi Kieran,

On Wed, Jan 26, 2022 at 09:47:12AM +0000, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2022-01-23 16:08:52)
> > 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@ideasonboard.com>
> > ---
> >  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@ideasonboard.com>
> > + */
> > +
> > +#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 */
> > +
> > +/* 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
> > +#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
> > +#define MIPI_CSI2_DT_USER_DEFINED(n)   (0x30 + (n))    /* 0..7 */
> 
> I don't have an easy way to validate those values right now so as with
> Niklas I'll leave those to your judgement, and Pratyush's review.
> 
> Also along side Pratyush's comment, I concur that the mapping tables too
> could be common, but I suspect that's an even bigger topic as maybe that
> falls into the trap of also being common to DRM formats...

Same information could be shared. I usually push back against
centralizing the mapping between media bus codes and pixel formats, as
that's device-specific, but the CSI-2 specification has an informative
section with recommended memory formats, so that at least could be
shared among drivers.

> And finally, are these defines in a location that can be accessible from
> device tree? Or would it have to be further duplicated there still?

They're not, but we can move them if needed.

> For instance, the bindings for the Xilinx CSI2 RX explicitly list DT
> values to specify as the xlnx,csi-pxl-format which I think should also
> come from this common header definition.

That's a good point. I don't see how it could work with the DT schema
though. At the moment, we have

  xlnx,csi-pxl-format:
    description: [...]
    $ref: /schemas/types.yaml#/definitions/uint32
    oneOf:
      - minimum: 0x1e
        maximum: 0x24
      - minimum: 0x28
        maximum: 0x2f

and as far as I know, you can't #include a C header in the schema
itself. It could be done in the examples and the device trees themselves
though.

> For the patches here so far, I can't see anything stark that is wrong
> so for the series:
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> as further extending this to the device tree bindings can be done on
> top.
> 
> > +
> > +#endif /* _MEDIA_MIPI_CSI2_H */

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/6] media: Define MIPI CSI-2 data types in a shared header file

[Laurent Pinchart]

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@ti.com> 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@ideasonboard.com>
> > > ---
> > >  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@ideasonboard.com>
> > > + */
> > > +
> > > +#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

Re: [PATCH 5/6] media: rockchip: rkisp1: Use mipi-csi2.h

[Dafna Hirschfeld]

Tested on rock-pi4 with few libcamera's 'cam' commands on several pixelformats

Tested-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Reviewed-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>

On 23.01.2022 18:08, Laurent Pinchart wrote:
> Replace the driver-specific definitions of MIPI CSI-2 data types with
> macros from mipi-csi2.h.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 34 ++++++++++---------
>  .../platform/rockchip/rkisp1/rkisp1-regs.h    | 11 ------
>  2 files changed, 18 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index 2a35bf24e54e..4415c7248c2f 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -14,6 +14,8 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/videodev2.h>
>  #include <linux/vmalloc.h>
> +
> +#include <media/mipi-csi2.h>
>  #include <media/v4l2-event.h>
>  
>  #include "rkisp1-common.h"
> @@ -62,112 +64,112 @@ static const struct rkisp1_isp_mbus_info rkisp1_isp_formats[] = {
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SRGGB10_1X10,
>  		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW10,
> +		.mipi_dt	= MIPI_CSI2_DT_RAW10,
>  		.bayer_pat	= RKISP1_RAW_RGGB,
>  		.bus_width	= 10,
>  		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SBGGR10_1X10,
>  		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW10,
> +		.mipi_dt	= MIPI_CSI2_DT_RAW10,
>  		.bayer_pat	= RKISP1_RAW_BGGR,
>  		.bus_width	= 10,
>  		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SGBRG10_1X10,
>  		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW10,
> +		.mipi_dt	= MIPI_CSI2_DT_RAW10,
>  		.bayer_pat	= RKISP1_RAW_GBRG,
>  		.bus_width	= 10,
>  		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SGRBG10_1X10,
>  		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW10,
> +		.mipi_dt	= MIPI_CSI2_DT_RAW10,
>  		.bayer_pat	= RKISP1_RAW_GRBG,
>  		.bus_width	= 10,
>  		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SRGGB12_1X12,
>  		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW12,
> +		.mipi_dt	= MIPI_CSI2_DT_RAW12,
>  		.bayer_pat	= RKISP1_RAW_RGGB,
>  		.bus_width	= 12,
>  		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SBGGR12_1X12,
>  		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW12,
> +		.mipi_dt	= MIPI_CSI2_DT_RAW12,
>  		.bayer_pat	= RKISP1_RAW_BGGR,
>  		.bus_width	= 12,
>  		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SGBRG12_1X12,
>  		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW12,
> +		.mipi_dt	= MIPI_CSI2_DT_RAW12,
>  		.bayer_pat	= RKISP1_RAW_GBRG,
>  		.bus_width	= 12,
>  		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SGRBG12_1X12,
>  		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW12,
> +		.mipi_dt	= MIPI_CSI2_DT_RAW12,
>  		.bayer_pat	= RKISP1_RAW_GRBG,
>  		.bus_width	= 12,
>  		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SRGGB8_1X8,
>  		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW8,
> +		.mipi_dt	= MIPI_CSI2_DT_RAW8,
>  		.bayer_pat	= RKISP1_RAW_RGGB,
>  		.bus_width	= 8,
>  		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SBGGR8_1X8,
>  		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW8,
> +		.mipi_dt	= MIPI_CSI2_DT_RAW8,
>  		.bayer_pat	= RKISP1_RAW_BGGR,
>  		.bus_width	= 8,
>  		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SGBRG8_1X8,
>  		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW8,
> +		.mipi_dt	= MIPI_CSI2_DT_RAW8,
>  		.bayer_pat	= RKISP1_RAW_GBRG,
>  		.bus_width	= 8,
>  		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_SGRBG8_1X8,
>  		.pixel_enc	= V4L2_PIXEL_ENC_BAYER,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_RAW8,
> +		.mipi_dt	= MIPI_CSI2_DT_RAW8,
>  		.bayer_pat	= RKISP1_RAW_GRBG,
>  		.bus_width	= 8,
>  		.direction	= RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_YUYV8_1X16,
>  		.pixel_enc	= V4L2_PIXEL_ENC_YUV,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_YUV422_8b,
> +		.mipi_dt	= MIPI_CSI2_DT_YUV422_8B,
>  		.yuv_seq	= RKISP1_CIF_ISP_ACQ_PROP_YCBYCR,
>  		.bus_width	= 16,
>  		.direction	= RKISP1_ISP_SD_SINK,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_YVYU8_1X16,
>  		.pixel_enc	= V4L2_PIXEL_ENC_YUV,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_YUV422_8b,
> +		.mipi_dt	= MIPI_CSI2_DT_YUV422_8B,
>  		.yuv_seq	= RKISP1_CIF_ISP_ACQ_PROP_YCRYCB,
>  		.bus_width	= 16,
>  		.direction	= RKISP1_ISP_SD_SINK,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_UYVY8_1X16,
>  		.pixel_enc	= V4L2_PIXEL_ENC_YUV,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_YUV422_8b,
> +		.mipi_dt	= MIPI_CSI2_DT_YUV422_8B,
>  		.yuv_seq	= RKISP1_CIF_ISP_ACQ_PROP_CBYCRY,
>  		.bus_width	= 16,
>  		.direction	= RKISP1_ISP_SD_SINK,
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_VYUY8_1X16,
>  		.pixel_enc	= V4L2_PIXEL_ENC_YUV,
> -		.mipi_dt	= RKISP1_CIF_CSI2_DT_YUV422_8b,
> +		.mipi_dt	= MIPI_CSI2_DT_YUV422_8B,
>  		.yuv_seq	= RKISP1_CIF_ISP_ACQ_PROP_CRYCBY,
>  		.bus_width	= 16,
>  		.direction	= RKISP1_ISP_SD_SINK,
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> index d326214c7e07..82f8d33d98b3 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> @@ -333,17 +333,6 @@
>  /* MIPI_DATA_SEL */
>  #define RKISP1_CIF_MIPI_DATA_SEL_VC(a)			(((a) & 0x3) << 6)
>  #define RKISP1_CIF_MIPI_DATA_SEL_DT(a)			(((a) & 0x3F) << 0)
> -/* MIPI DATA_TYPE */
> -#define RKISP1_CIF_CSI2_DT_YUV420_8b			0x18
> -#define RKISP1_CIF_CSI2_DT_YUV420_10b			0x19
> -#define RKISP1_CIF_CSI2_DT_YUV422_8b			0x1E
> -#define RKISP1_CIF_CSI2_DT_YUV422_10b			0x1F
> -#define RKISP1_CIF_CSI2_DT_RGB565			0x22
> -#define RKISP1_CIF_CSI2_DT_RGB666			0x23
> -#define RKISP1_CIF_CSI2_DT_RGB888			0x24
> -#define RKISP1_CIF_CSI2_DT_RAW8				0x2A
> -#define RKISP1_CIF_CSI2_DT_RAW10			0x2B
> -#define RKISP1_CIF_CSI2_DT_RAW12			0x2C
>  
>  /* MIPI_IMSC, MIPI_RIS, MIPI_MIS, MIPI_ICR, MIPI_ISR */
>  #define RKISP1_CIF_MIPI_SYNC_FIFO_OVFLW(a)		(((a) & 0xF) << 0)
> -- 
> Regards,
> 
> Laurent Pinchart
> 

Re: [PATCH 2/6] media: cadence: cdns-csi2tx: Use mipi-csi2.h

[Laurent Pinchart]

Hello,

Ping on this patch in case someone wants to review it. I'll send a pull
request shortly for the part of this series that hasn't been merged yet,
and this is the last one missing a reviewed-by tag.

On Sun, Jan 23, 2022 at 06:08:53PM +0200, Laurent Pinchart wrote:
> Replace the hardcoded MIPI CSI-2 data types with macros from
> mipi-csi2.h.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/cadence/cdns-csi2tx.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/cadence/cdns-csi2tx.c b/drivers/media/platform/cadence/cdns-csi2tx.c
> index 8f8c36056354..58e405b69f67 100644
> --- a/drivers/media/platform/cadence/cdns-csi2tx.c
> +++ b/drivers/media/platform/cadence/cdns-csi2tx.c
> @@ -15,6 +15,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> +#include <media/mipi-csi2.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-fwnode.h>
> @@ -121,12 +122,12 @@ static const struct csi2tx_fmt csi2tx_formats[] = {
>  	{
>  		.mbus	= MEDIA_BUS_FMT_UYVY8_1X16,
>  		.bpp	= 2,
> -		.dt	= 0x1e,
> +		.dt	= MIPI_CSI2_DT_YUV422_8B,
>  	},
>  	{
>  		.mbus	= MEDIA_BUS_FMT_RGB888_1X24,
>  		.bpp	= 3,
> -		.dt	= 0x24,
> +		.dt	= MIPI_CSI2_DT_RGB888,
>  	},
>  };
>  

-- 
Regards,

Laurent Pinchart