[PATCH v2 0/7] media: imx: Destage imx7-mipi-csis

[PATCH v2 0/7] media: imx: Destage imx7-mipi-csis

[Jacopo Mondi]

Hello
  this series includes patches from two series previously sent:
https://lore.kernel.org/linux-media/20220119112024.11339-1-jacopo@jmondi.org/
https://lore.kernel.org/linux-media/20220211180216.290133-1-jacopo@jmondi.org/
v1:
https://lore.kernel.org/linux-media/20220214184318.409208-1-jacopo@jmondi.org/T/#t

Which can now be marked as superseded.

The first 2 patches performs the de-staging of the imx7-mipi-csis driver and
are now reviewed.

The rest of the series builds on top of the comment received on:
https://lore.kernel.org/linux-media/20220119112024.11339-3-jacopo@jmondi.org/

If DUAL pixel mode is used in the CSIS driver, then the CSI block of the IMX8MM
SoC needs to be operated in dual mode as well. To do so, use the image format
sample size to determine in the CSI bridge if dual or single mode should be
used.

Laurent could you test on MM to see if it works now ?

On top two small patches I was carrying in my tree to add more formats to the
CSIS driver, the last one with the caveat that RGB24 is transmitted on the wire
with one format and stored in memory with a different one.

Series based on top of the most recent media master branch.

Thanks
  j

v1->v2:
- Remove per-SoC handling in CSI bridge and only use image formats
- Add TODO note to the staging driver
- Fix PIXEL_DUAL mode comments for imx-mipi-csis
- Add output format translation to imx-mipi-csis to handle RGB24

Jacopo Mondi (7):
  media: imx: De-stage imx7-mipi-csis
  media: imx: Rename imx7-mipi-csis.c to imx-mipi-csis.c
  media: imx: imx7-media-csi: Use dual sampling for YUV 1X16
  media: imx: imx-mipi-csis: Set PIXEL_MODE for YUV422
  media: imx: imx-mipi-csis: Add RGB565_1X16
  media: imx: imx-mipi-csis: Add BGR888
  media: imx: imx-mipi-csis: Add output format

 Documentation/admin-guide/media/imx7.rst      |  2 +-
 ...-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} |  2 +-
 MAINTAINERS                                   |  4 +-
 drivers/media/platform/Kconfig                |  1 +
 drivers/media/platform/Makefile               |  1 +
 drivers/media/platform/imx/Kconfig            | 24 ++++++++
 drivers/media/platform/imx/Makefile           |  1 +
 .../platform/imx/imx-mipi-csis.c}             | 59 +++++++++++++++++--
 drivers/staging/media/imx/Makefile            |  1 -
 drivers/staging/media/imx/TODO                | 26 ++++++++
 drivers/staging/media/imx/imx7-media-csi.c    |  8 ++-
 11 files changed, 117 insertions(+), 12 deletions(-)
 rename Documentation/devicetree/bindings/media/{nxp,imx7-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} (98%)
 create mode 100644 drivers/media/platform/imx/Kconfig
 create mode 100644 drivers/media/platform/imx/Makefile
 rename drivers/{staging/media/imx/imx7-mipi-csis.c => media/platform/imx/imx-mipi-csis.c} (95%)

--
2.35.0

[PATCH v2 1/7] media: imx: De-stage imx7-mipi-csis

[Jacopo Mondi]

The imx7-mipi-csis driver is in a good state and can be destaged.

Move the imx7-mipi-csis.c driver to the newly created
drivers/media/platform/imx directory and plumb the related
options in Kconfig and in Makefile.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 MAINTAINERS                                   |  2 +-
 drivers/media/platform/Kconfig                |  1 +
 drivers/media/platform/Makefile               |  1 +
 drivers/media/platform/imx/Kconfig            | 24 +++++++++++++++++++
 drivers/media/platform/imx/Makefile           |  1 +
 .../platform}/imx/imx7-mipi-csis.c            |  0
 drivers/staging/media/imx/Makefile            |  1 -
 7 files changed, 28 insertions(+), 2 deletions(-)
 create mode 100644 drivers/media/platform/imx/Kconfig
 create mode 100644 drivers/media/platform/imx/Makefile
 rename drivers/{staging/media => media/platform}/imx/imx7-mipi-csis.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 83d27b57016f..5bdb8c881b0b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11891,8 +11891,8 @@ T:	git git://linuxtv.org/media_tree.git
 F:	Documentation/admin-guide/media/imx7.rst
 F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
 F:	Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
+F:	drivers/media/platform/imx/imx7-mipi-csis.c
 F:	drivers/staging/media/imx/imx7-media-csi.c
-F:	drivers/staging/media/imx/imx7-mipi-csis.c
 
 MEDIA DRIVERS FOR HELENE
 M:	Abylay Ospan <aospan@netup.ru>
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 9fbdba0fd1e7..d9eeccffea69 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -171,6 +171,7 @@ source "drivers/media/platform/xilinx/Kconfig"
 source "drivers/media/platform/rcar-vin/Kconfig"
 source "drivers/media/platform/atmel/Kconfig"
 source "drivers/media/platform/sunxi/Kconfig"
+source "drivers/media/platform/imx/Kconfig"
 
 config VIDEO_TI_CAL
 	tristate "TI CAL (Camera Adaptation Layer) driver"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 28eb4aadbf45..a9466c854610 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -20,6 +20,7 @@ obj-y	+= ti-vpe/
 obj-$(CONFIG_VIDEO_MX2_EMMAPRP)		+= mx2_emmaprp.o
 obj-$(CONFIG_VIDEO_CODA)		+= coda/
 
+obj-$(CONFIG_VIDEO_IMX)			+= imx/
 obj-$(CONFIG_VIDEO_IMX_PXP)		+= imx-pxp.o
 obj-$(CONFIG_VIDEO_IMX8_JPEG)		+= imx-jpeg/
 
diff --git a/drivers/media/platform/imx/Kconfig b/drivers/media/platform/imx/Kconfig
new file mode 100644
index 000000000000..683863572c20
--- /dev/null
+++ b/drivers/media/platform/imx/Kconfig
@@ -0,0 +1,24 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+menuconfig VIDEO_IMX
+	bool "V4L2 capture drivers for NXP i.MX devices"
+	depends on ARCH_MXC || COMPILE_TEST
+	depends on VIDEO_DEV && VIDEO_V4L2
+	help
+	  Say yes here to enable support for capture drivers on i.MX SoCs.
+	  Support for the single SoC features are selectable in the sub-menu
+	  options.
+
+if VIDEO_IMX
+
+config VIDEO_IMX_MIPI_CSIS
+	tristate "MIPI CSI-2 CSIS receiver found on i.MX7 and i.MX8 models"
+	select MEDIA_CONTROLLER
+	select V4L2_FWNODE
+	select VIDEO_V4L2_SUBDEV_API
+	default n
+	help
+	  Video4Linux2 sub-device driver for the MIPI CSI-2 CSIS receiver
+	  v3.3/v3.6.3 found on some i.MX7 and i.MX8 SoCs.
+
+endif # VIDEO_IMX
diff --git a/drivers/media/platform/imx/Makefile b/drivers/media/platform/imx/Makefile
new file mode 100644
index 000000000000..ee272234c8d7
--- /dev/null
+++ b/drivers/media/platform/imx/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx7-mipi-csis.o
diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/media/platform/imx/imx7-mipi-csis.c
similarity index 100%
rename from drivers/staging/media/imx/imx7-mipi-csis.c
rename to drivers/media/platform/imx/imx7-mipi-csis.c
diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
index 19c2fc54d424..d82be898145b 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -15,5 +15,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-media-csi.o
 obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
 
 obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
-obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o
 obj-$(CONFIG_VIDEO_IMX7_CSI) += imx8mq-mipi-csi2.o
-- 
2.35.0

[PATCH v2 2/7] media: imx: Rename imx7-mipi-csis.c to imx-mipi-csis.c

[Jacopo Mondi]

Rename the imx7-mipi-csis.c driver to remove the reference to i.MX7.

The driver is for an IP core found on i.MX7 and i.MX8 SoC, so do not
specify a SoC version number in the driver name.

Remove the references to the i.MX7 SoC in the driver symbols and expand
the driver's header with more information about the IP core the driver
controls.

Also rename the associated bindings documentation.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/admin-guide/media/imx7.rst               |  2 +-
 ...{nxp,imx7-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} |  2 +-
 MAINTAINERS                                            |  4 ++--
 drivers/media/platform/imx/Makefile                    |  2 +-
 .../platform/imx/{imx7-mipi-csis.c => imx-mipi-csis.c} | 10 +++++++---
 5 files changed, 12 insertions(+), 8 deletions(-)
 rename Documentation/devicetree/bindings/media/{nxp,imx7-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} (98%)
 rename drivers/media/platform/imx/{imx7-mipi-csis.c => imx-mipi-csis.c} (99%)

diff --git a/Documentation/admin-guide/media/imx7.rst b/Documentation/admin-guide/media/imx7.rst
index 4785ae8ac978..2fa27718f52a 100644
--- a/Documentation/admin-guide/media/imx7.rst
+++ b/Documentation/admin-guide/media/imx7.rst
@@ -33,7 +33,7 @@ reference manual [#f1]_.
 Entities
 --------
 
-imx7-mipi-csi2
+imx-mipi-csi2
 --------------
 
 This is the MIPI CSI-2 receiver entity. It has one sink pad to receive the pixel
diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
similarity index 98%
rename from Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
rename to Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
index e2e6e9aa0fe6..36b135bf9f2a 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/media/nxp,imx7-mipi-csi2.yaml#
+$id: http://devicetree.org/schemas/media/nxp,imx-mipi-csi2.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: NXP i.MX7 and i.MX8 MIPI CSI-2 receiver
diff --git a/MAINTAINERS b/MAINTAINERS
index 5bdb8c881b0b..d919ea3ed250 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11889,9 +11889,9 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
 F:	Documentation/admin-guide/media/imx7.rst
+F:	Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
 F:	Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
-F:	Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
-F:	drivers/media/platform/imx/imx7-mipi-csis.c
+F:	drivers/media/platform/imx/imx-mipi-csis.c
 F:	drivers/staging/media/imx/imx7-media-csi.c
 
 MEDIA DRIVERS FOR HELENE
diff --git a/drivers/media/platform/imx/Makefile b/drivers/media/platform/imx/Makefile
index ee272234c8d7..f72bdbe8e6ef 100644
--- a/drivers/media/platform/imx/Makefile
+++ b/drivers/media/platform/imx/Makefile
@@ -1 +1 @@
-obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx7-mipi-csis.o
+obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx-mipi-csis.o
diff --git a/drivers/media/platform/imx/imx7-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c
similarity index 99%
rename from drivers/media/platform/imx/imx7-mipi-csis.c
rename to drivers/media/platform/imx/imx-mipi-csis.c
index a22d0e6b3d44..3e7efcf43a5f 100644
--- a/drivers/media/platform/imx/imx7-mipi-csis.c
+++ b/drivers/media/platform/imx/imx-mipi-csis.c
@@ -1,6 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Freescale i.MX7 SoC series MIPI-CSI V3.3 receiver driver
+ * Samsung CSIS MIPI CSI-2 receiver driver.
+ *
+ * The Samsung CSIS IP is a MIPI CSI-2 receiver found in various NXP i.MX7 and
+ * i.MX8 SoCs. The i.MX7 features version 3.3 of the IP, while i.MX8 features
+ * version 3.6.3.
  *
  * Copyright (C) 2019 Linaro Ltd
  * Copyright (C) 2015-2016 Freescale Semiconductor, Inc. All Rights Reserved.
@@ -31,7 +35,7 @@
 #include <media/v4l2-mc.h>
 #include <media/v4l2-subdev.h>
 
-#define CSIS_DRIVER_NAME			"imx7-mipi-csis"
+#define CSIS_DRIVER_NAME			"imx-mipi-csis"
 
 #define CSIS_PAD_SINK				0
 #define CSIS_PAD_SOURCE				1
@@ -1515,4 +1519,4 @@ module_platform_driver(mipi_csis_driver);
 
 MODULE_DESCRIPTION("i.MX7 & i.MX8 MIPI CSI-2 receiver driver");
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:imx7-mipi-csi2");
+MODULE_ALIAS("platform:imx-mipi-csi2");
-- 
2.35.0

[PATCH v2 3/7] media: imx: imx7-media-csi: Use dual sampling for YUV 1X16

[Jacopo Mondi]

The CSI bridge should operate in dual pixel sampling mode when it is
connected to a pixel transmitter that transfers two pixel samples (16
bits) at the time in YUYV formats.

Use the image format variants to determine if single or dual pixel mode
should be used.

Add a note to the TODO file to record that the list of supported formats
should be restricted to the SoC model the CSI bridge is integrated on
to avoid potential pipeline mis-configurations.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/staging/media/imx/TODO             | 26 ++++++++++++++++++++++
 drivers/staging/media/imx/imx7-media-csi.c |  8 +++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/imx/TODO b/drivers/staging/media/imx/TODO
index 06c94f20ecf8..e15eba32cc94 100644
--- a/drivers/staging/media/imx/TODO
+++ b/drivers/staging/media/imx/TODO
@@ -27,3 +27,29 @@
 - i.MX7: all of the above, since it uses the imx media core

 - i.MX7: use Frame Interval Monitor
+
+- imx7-media-csi: Restrict the supported formats list to the SoC version.
+
+  The imx7 CSI bridge can be configured to sample pixel components from the Rx
+  queue in single  (8bpp) or double (16bpp) modes. Image format variations with
+  different sample sizes (ie YUYV_2X8 vs YUYV_1X16) determine the sampling size
+  (see imx7_csi_configure()).
+
+  As the imx7 CSI bridge can be interfaced with different IP blocks depending on
+  the SoC model it is integrated on, the Rx queue sampling size should match
+  the size of samples transferred by the transmitting IP block.
+
+  To avoid mis-configurations of the capture pipeline, the enumeration of the
+  supported formats should be restricted to match the pixel source
+  transmitting mode.
+
+  Examples: i.MX8MM SoC integrates the CSI bridge with the Samsung CSIS CSI-2
+  receiver which operates in dual pixel sampling mode. The CSI bridge should
+  only expose the 1X16 formats variant which instructs it to operate in dual
+  pixel sampling mode. When the CSI bridge is instead integrated on an i.MX8MQ
+  SoC, which features a CSI-2 receiver that operates in single sampling mode, it
+  should only expose the 2X8 formats variant which instruct it to operate in
+  single sampling mode.
+
+  This currently only applies to YUYV formats, but other formats might need
+  to be treated in the same way.
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 32311fc0e2a4..108360ae3710 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -503,11 +503,15 @@ static void imx7_csi_configure(struct imx7_csi *csi)
 		 * all of them comply. Support both variants.
 		 */
 		case MEDIA_BUS_FMT_UYVY8_2X8:
-		case MEDIA_BUS_FMT_UYVY8_1X16:
 		case MEDIA_BUS_FMT_YUYV8_2X8:
-		case MEDIA_BUS_FMT_YUYV8_1X16:
 			cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B;
 			break;
+		case MEDIA_BUS_FMT_UYVY8_1X16:
+		case MEDIA_BUS_FMT_YUYV8_1X16:
+			cr3 |= BIT_TWO_8BIT_SENSOR;
+			cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B |
+				BIT_MIPI_DOUBLE_CMPNT;
+			break;
 		}
 	}

--
2.35.0

[PATCH v2 4/7] media: imx: imx-mipi-csis: Set PIXEL_MODE for YUV422

[Jacopo Mondi]

Bits 13 and 12 of the ISP_CONFIGn register configure the PIXEL_MODE
which specifies the sampling size, in pixel component units, on the
CSI-2 output data interface when data are transferred to memory.

The register description in the chip manual specifies that DUAL mode
should be used for YUV422 data.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
---
 drivers/media/platform/imx/imx-mipi-csis.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c
index 3e7efcf43a5f..86e7ea3713d9 100644
--- a/drivers/media/platform/imx/imx-mipi-csis.c
+++ b/drivers/media/platform/imx/imx-mipi-csis.c
@@ -173,6 +173,7 @@
 #define MIPI_CSIS_ISPCFG_PIXEL_MODE_SINGLE	(0 << 12)
 #define MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL	(1 << 12)
 #define MIPI_CSIS_ISPCFG_PIXEL_MODE_QUAD	(2 << 12)	/* i.MX8M[MNP] only */
+#define MIPI_CSIS_ISPCFG_PIXEL_MASK		(3 << 12)
 #define MIPI_CSIS_ISPCFG_ALIGN_32BIT		BIT(11)
 #define MIPI_CSIS_ISPCFG_FMT(fmt)		((fmt) << 2)
 #define MIPI_CSIS_ISPCFG_FMT_MASK		(0x3f << 2)
@@ -506,7 +507,13 @@ static void __mipi_csis_set_format(struct csi_state *state)
 
 	/* Color format */
 	val = mipi_csis_read(state, MIPI_CSIS_ISP_CONFIG_CH(0));
-	val &= ~(MIPI_CSIS_ISPCFG_ALIGN_32BIT | MIPI_CSIS_ISPCFG_FMT_MASK);
+	val &= ~(MIPI_CSIS_ISPCFG_ALIGN_32BIT | MIPI_CSIS_ISPCFG_FMT_MASK
+		| MIPI_CSIS_ISPCFG_PIXEL_MASK);
+
+	/* TODO: Verify which other formats require DUAL (or QUAD) modes. */
+	if (state->csis_fmt->data_type == MIPI_CSI2_DATA_TYPE_YUV422_8)
+		val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
+
 	val |= MIPI_CSIS_ISPCFG_FMT(state->csis_fmt->data_type);
 	mipi_csis_write(state, MIPI_CSIS_ISP_CONFIG_CH(0), val);
 
-- 
2.35.0

[PATCH v2 5/7] media: imx: imx-mipi-csis: Add RGB565_1X16

[Jacopo Mondi]

Add RGB565_1X16 to the enumeration of supported image formats.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/imx/imx-mipi-csis.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c
index 86e7ea3713d9..c7a771e3fdf3 100644
--- a/drivers/media/platform/imx/imx-mipi-csis.c
+++ b/drivers/media/platform/imx/imx-mipi-csis.c
@@ -360,6 +360,12 @@ static const struct csis_pix_format mipi_csis_formats[] = {
 		.data_type = MIPI_CSI2_DATA_TYPE_YUV422_8,
 		.width = 16,
 	},
+	/* RGB formats. */
+	{
+		.code = MEDIA_BUS_FMT_RGB565_1X16,
+		.data_type = MIPI_CSI2_DATA_TYPE_RGB565,
+		.width = 16,
+	},
 	/* RAW (Bayer and greyscale) formats. */
 	{
 		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
-- 
2.35.0

[PATCH v2 6/7] media: imx: imx-mipi-csis: Add BGR888

[Jacopo Mondi]

Add support for the BGR888_1X24 image formats.

The BGR888 format definition matches how pixel components are sent on
the wire according to the CSI-2 specification.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/platform/imx/imx-mipi-csis.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c
index c7a771e3fdf3..fdf133f81c5b 100644
--- a/drivers/media/platform/imx/imx-mipi-csis.c
+++ b/drivers/media/platform/imx/imx-mipi-csis.c
@@ -366,6 +366,11 @@ static const struct csis_pix_format mipi_csis_formats[] = {
 		.data_type = MIPI_CSI2_DATA_TYPE_RGB565,
 		.width = 16,
 	},
+	{
+		.code = MEDIA_BUS_FMT_BGR888_1X24,
+		.data_type = MIPI_CSI2_DATA_TYPE_RGB888,
+		.width = 24,
+	},
 	/* RAW (Bayer and greyscale) formats. */
 	{
 		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
-- 
2.35.0

[PATCH v2 7/7] media: imx: imx-mipi-csis: Add output format

[Jacopo Mondi]

Due to how pixel components are transmitted on the CSI-2 serial bus
and how they are stored in memory by the CSI-2 receiver, the component
ordering might change and the image formats on the sink and source pads
of the receiver should reflect it.

For RGB24, in example, the component ordering on the wire as described by
the CSI-2 specification matches the BGR888 format, while once stored in
in memory by the CSIS receiver they match the RGB888 format.

Add an additional .output field to struct csis_pix_format to allow
propagating the correct format to the source pad after a format
configuration on the sink.

The change is only relevant for RGB24 but paves the way for further
format translations in future.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/platform/imx/imx-mipi-csis.c | 29 +++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c
index fdf133f81c5b..128f4180d1e9 100644
--- a/drivers/media/platform/imx/imx-mipi-csis.c
+++ b/drivers/media/platform/imx/imx-mipi-csis.c
@@ -351,6 +351,7 @@ struct csis_pix_format {
 	u32 code;
 	u32 data_type;
 	u8 width;
+	u32 output;
 };
 
 static const struct csis_pix_format mipi_csis_formats[] = {
@@ -359,95 +360,117 @@ static const struct csis_pix_format mipi_csis_formats[] = {
 		.code = MEDIA_BUS_FMT_UYVY8_1X16,
 		.data_type = MIPI_CSI2_DATA_TYPE_YUV422_8,
 		.width = 16,
+		.output = MEDIA_BUS_FMT_UYVY8_1X16,
 	},
 	/* RGB formats. */
 	{
 		.code = MEDIA_BUS_FMT_RGB565_1X16,
 		.data_type = MIPI_CSI2_DATA_TYPE_RGB565,
 		.width = 16,
+		.output = MEDIA_BUS_FMT_RGB565_1X16,
 	},
 	{
 		.code = MEDIA_BUS_FMT_BGR888_1X24,
 		.data_type = MIPI_CSI2_DATA_TYPE_RGB888,
 		.width = 24,
+		.output = MEDIA_BUS_FMT_RGB888_1X24,
 	},
 	/* RAW (Bayer and greyscale) formats. */
 	{
 		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
 		.width = 8,
+		.output = MEDIA_BUS_FMT_SBGGR8_1X8,
 	}, {
 		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
 		.width = 8,
+		.output = MEDIA_BUS_FMT_SGBRG8_1X8,
 	}, {
 		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
 		.width = 8,
+		.output = MEDIA_BUS_FMT_SGRBG8_1X8,
 	}, {
 		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
 		.width = 8,
+		.output = MEDIA_BUS_FMT_SRGGB8_1X8,
 	}, {
 		.code = MEDIA_BUS_FMT_Y8_1X8,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
 		.width = 8,
+		.output = MEDIA_BUS_FMT_Y8_1X8,
 	}, {
 		.code = MEDIA_BUS_FMT_SBGGR10_1X10,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
 		.width = 10,
+		.output = MEDIA_BUS_FMT_SBGGR10_1X10,
 	}, {
 		.code = MEDIA_BUS_FMT_SGBRG10_1X10,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
 		.width = 10,
+		.output = MEDIA_BUS_FMT_SGBRG10_1X10,
 	}, {
 		.code = MEDIA_BUS_FMT_SGRBG10_1X10,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
 		.width = 10,
+		.output = MEDIA_BUS_FMT_SGRBG10_1X10,
 	}, {
 		.code = MEDIA_BUS_FMT_SRGGB10_1X10,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
 		.width = 10,
+		.output = MEDIA_BUS_FMT_SRGGB10_1X10,
 	}, {
 		.code = MEDIA_BUS_FMT_Y10_1X10,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
 		.width = 10,
+		.output = MEDIA_BUS_FMT_Y10_1X10,
 	}, {
 		.code = MEDIA_BUS_FMT_SBGGR12_1X12,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
 		.width = 12,
+		.output = MEDIA_BUS_FMT_SBGGR12_1X12,
 	}, {
 		.code = MEDIA_BUS_FMT_SGBRG12_1X12,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
 		.width = 12,
+		.output = MEDIA_BUS_FMT_SGBRG12_1X12,
 	}, {
 		.code = MEDIA_BUS_FMT_SGRBG12_1X12,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
 		.width = 12,
+		.output = MEDIA_BUS_FMT_SGRBG12_1X12,
 	}, {
 		.code = MEDIA_BUS_FMT_SRGGB12_1X12,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
 		.width = 12,
+		.output = MEDIA_BUS_FMT_SRGGB12_1X12,
 	}, {
 		.code = MEDIA_BUS_FMT_Y12_1X12,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
 		.width = 12,
+		.output = MEDIA_BUS_FMT_Y12_1X12,
 	}, {
 		.code = MEDIA_BUS_FMT_SBGGR14_1X14,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW14,
 		.width = 14,
+		.output = MEDIA_BUS_FMT_SBGGR14_1X14,
 	}, {
 		.code = MEDIA_BUS_FMT_SGBRG14_1X14,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW14,
 		.width = 14,
+		.output = MEDIA_BUS_FMT_SGBRG14_1X14,
 	}, {
 		.code = MEDIA_BUS_FMT_SGRBG14_1X14,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW14,
 		.width = 14,
+		.output = MEDIA_BUS_FMT_SGRBG14_1X14,
 	}, {
 		.code = MEDIA_BUS_FMT_SRGGB14_1X14,
 		.data_type = MIPI_CSI2_DATA_TYPE_RAW14,
 		.width = 14,
+		.output = MEDIA_BUS_FMT_SRGGB14_1X14,
 	}
 };
 
@@ -1090,7 +1113,11 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
 	/* Propagate the format from sink to source. */
 	fmt = mipi_csis_get_format(state, sd_state, sdformat->which,
 				   CSIS_PAD_SOURCE);
-	*fmt = sdformat->format;
+
+	/* The format on the source pad might change due to unpacking. */
+	fmt->code = csis_fmt->output;
+	fmt->width = sdformat->format.width;
+	fmt->height = sdformat->format.height;
 
 	/* Store the CSIS format descriptor for active formats. */
 	if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
-- 
2.35.0

Re: [PATCH v2 3/7] media: imx: imx7-media-csi: Use dual sampling for YUV 1X16

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Feb 18, 2022 at 07:34:17PM +0100, Jacopo Mondi wrote:
> The CSI bridge should operate in dual pixel sampling mode when it is

s/dual pixel sampling mode/dual component mode/

> connected to a pixel transmitter that transfers two pixel samples (16
> bits) at the time in YUYV formats.

It's actually one pixel per clock. Without BIT_TWO_8BIT_SENSOR and
BIT_MIPI_DOUBLE_CMPNT, the CSI bridge expects 8-bit data, which requires
two clock cycles to transfer one pixel. When setting those bits, it
expects 16-bit data, with one clock cycle per pixel. The
MIPI_DOUBLE_CMPNT documentation states this clearly:

20 MIPI_DOUBLE_CMPNT
Double component per clock cycle in YUV422 formats.
0 Single component per clock cycle
(half pixel per clock cycle)
1 Double component per clock cycle
(a pixel per clock cycle)

> Use the image format variants to determine if single or dual pixel mode

"dual pixel" is a concept of the CSIS, not the CSI bridge. This should
mention single or double component mode.

> should be used.
> 
> Add a note to the TODO file to record that the list of supported formats
> should be restricted to the SoC model the CSI bridge is integrated on
> to avoid potential pipeline mis-configurations.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  drivers/staging/media/imx/TODO             | 26 ++++++++++++++++++++++
>  drivers/staging/media/imx/imx7-media-csi.c |  8 +++++--
>  2 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/TODO b/drivers/staging/media/imx/TODO
> index 06c94f20ecf8..e15eba32cc94 100644
> --- a/drivers/staging/media/imx/TODO
> +++ b/drivers/staging/media/imx/TODO
> @@ -27,3 +27,29 @@
>  - i.MX7: all of the above, since it uses the imx media core
> 
>  - i.MX7: use Frame Interval Monitor
> +
> +- imx7-media-csi: Restrict the supported formats list to the SoC version.
> +
> +  The imx7 CSI bridge can be configured to sample pixel components from the Rx
> +  queue in single  (8bpp) or double (16bpp) modes. Image format variations with
> +  different sample sizes (ie YUYV_2X8 vs YUYV_1X16) determine the sampling size
> +  (see imx7_csi_configure()).
> +
> +  As the imx7 CSI bridge can be interfaced with different IP blocks depending on
> +  the SoC model it is integrated on, the Rx queue sampling size should match
> +  the size of samples transferred by the transmitting IP block.
> +
> +  To avoid mis-configurations of the capture pipeline, the enumeration of the
> +  supported formats should be restricted to match the pixel source
> +  transmitting mode.
> +
> +  Examples: i.MX8MM SoC integrates the CSI bridge with the Samsung CSIS CSI-2
> +  receiver which operates in dual pixel sampling mode. The CSI bridge should
> +  only expose the 1X16 formats variant which instructs it to operate in dual
> +  pixel sampling mode. When the CSI bridge is instead integrated on an i.MX8MQ
> +  SoC, which features a CSI-2 receiver that operates in single sampling mode, it
> +  should only expose the 2X8 formats variant which instruct it to operate in
> +  single sampling mode.
> +
> +  This currently only applies to YUYV formats, but other formats might need
> +  to be treated in the same way.
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index 32311fc0e2a4..108360ae3710 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -503,11 +503,15 @@ static void imx7_csi_configure(struct imx7_csi *csi)
>  		 * all of them comply. Support both variants.
>  		 */

You can drop this comment, as the two variants are not related to the
CSI-2 bus format (which should always be UYVY8_1X16) but to the format
output by the CSI-2 RX.

I would add another comment to explain this clearly:

		/*
		 * The CSI bridge has a 16-bit input bus. Depending on
		 * the connected source, data may be transmitted with 8
		 * or 10 bits per clock sample (in bits [9:2] or [9:0]
		 * respectively) or with 16 bits per clock sample (in
		 * bits [15:0]). The data is then packed into a 32-bit
		 * FIFO (as shown in figure 13-11 of the i.MX8MM
		 * reference manual rev. 3).
		 *
		 * The data packing in a 32-bit FIFO input workd is
		 * controlled by the CR3 TWO_8BIT_SENSOR field (also
		 * known as SENSOR_16BITS in the i.MX8MM reference
		 * manual). When set to 0, data packing groups four
		 * 8-bit input samples (bits [9:2]). When set to 1, data
		 * packing groups two 16-bit input samples (bits
		 * [15:0]).
		 *
		 * The register field CR18 MIPI_DOUBLE_CMPNT also needs
		 * to be configured according to the input format for
		 * YUV 4:2:2 data. The field controls the gasket between
		 * the CSI-2 receiver and the CSI bridge. On i.MX7 and
		 * i.MX8MM, the field must be set to 1 when the CSIS
		 * outputs 16-bit samples. On i.MX8MQ, the gasket
		 * ignores the MIPI_DOUBLE_CMPNT bit and YUV 4:2:2
		 * always uses 16-bit samples. Setting MIPI_DOUBLE_CMPNT
		 * in that case has no effect, but doesn't cause any
		 * issue.
		 */

With this, someone trying to figure out what those bits do will
hopefully be able to get it right :-)

With these changes,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  		case MEDIA_BUS_FMT_UYVY8_2X8:
> -		case MEDIA_BUS_FMT_UYVY8_1X16:
>  		case MEDIA_BUS_FMT_YUYV8_2X8:
> -		case MEDIA_BUS_FMT_YUYV8_1X16:
>  			cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B;
>  			break;
> +		case MEDIA_BUS_FMT_UYVY8_1X16:
> +		case MEDIA_BUS_FMT_YUYV8_1X16:
> +			cr3 |= BIT_TWO_8BIT_SENSOR;
> +			cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B |
> +				BIT_MIPI_DOUBLE_CMPNT;
> +			break;
>  		}
>  	}
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 4/7] media: imx: imx-mipi-csis: Set PIXEL_MODE for YUV422

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Feb 18, 2022 at 07:34:18PM +0100, Jacopo Mondi wrote:
> Bits 13 and 12 of the ISP_CONFIGn register configure the PIXEL_MODE
> which specifies the sampling size, in pixel component units, on the
> CSI-2 output data interface when data are transferred to memory.
> 
> The register description in the chip manual specifies that DUAL mode
> should be used for YUV422 data.
>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
> ---
>  drivers/media/platform/imx/imx-mipi-csis.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c
> index 3e7efcf43a5f..86e7ea3713d9 100644
> --- a/drivers/media/platform/imx/imx-mipi-csis.c
> +++ b/drivers/media/platform/imx/imx-mipi-csis.c
> @@ -173,6 +173,7 @@
>  #define MIPI_CSIS_ISPCFG_PIXEL_MODE_SINGLE	(0 << 12)
>  #define MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL	(1 << 12)
>  #define MIPI_CSIS_ISPCFG_PIXEL_MODE_QUAD	(2 << 12)	/* i.MX8M[MNP] only */
> +#define MIPI_CSIS_ISPCFG_PIXEL_MASK		(3 << 12)
>  #define MIPI_CSIS_ISPCFG_ALIGN_32BIT		BIT(11)
>  #define MIPI_CSIS_ISPCFG_FMT(fmt)		((fmt) << 2)
>  #define MIPI_CSIS_ISPCFG_FMT_MASK		(0x3f << 2)
> @@ -506,7 +507,13 @@ static void __mipi_csis_set_format(struct csi_state *state)
>  
>  	/* Color format */
>  	val = mipi_csis_read(state, MIPI_CSIS_ISP_CONFIG_CH(0));
> -	val &= ~(MIPI_CSIS_ISPCFG_ALIGN_32BIT | MIPI_CSIS_ISPCFG_FMT_MASK);
> +	val &= ~(MIPI_CSIS_ISPCFG_ALIGN_32BIT | MIPI_CSIS_ISPCFG_FMT_MASK
> +		| MIPI_CSIS_ISPCFG_PIXEL_MASK);
> +

Let's add some more information here too:

	/*
	 * YUV 4:2:2 can be transferred with 8 or 16 bits per clock sample
	 * (referred to in the documentation as single and dual pixel modes
	 * respectively, although the 8-bit mode transfers half a pixel per
	 * clock sample and the 16-bit mode one pixel). While both mode work
	 * when the CSIS is connected to a receiver that supports either option,
	 * single pixel mode requires clock rates twice as high. As all SoCs
	 * that integrate the CSIS can operate in 16-bit bit mode, and some do
	 * not support 8-bit mode (this is the case of the i.MX8MP), use dual
	 * pixel mode unconditionally.
	 */

With this,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	/* TODO: Verify which other formats require DUAL (or QUAD) modes. */

I expect that at least RAW8 could benefit from dual mode too. While the
i.MX8MM reference manual states that dual mode can also be used for
RAW10 and RAW12, I think the CSI bridge has a 16-bit input bus, so this
may not be possible (I could be wrong).

> +	if (state->csis_fmt->data_type == MIPI_CSI2_DATA_TYPE_YUV422_8)
> +		val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
> +
>  	val |= MIPI_CSIS_ISPCFG_FMT(state->csis_fmt->data_type);
>  	mipi_csis_write(state, MIPI_CSIS_ISP_CONFIG_CH(0), val);
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/7] media: imx: imx-mipi-csis: Add BGR888

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Feb 18, 2022 at 07:34:20PM +0100, Jacopo Mondi wrote:
> Add support for the BGR888_1X24 image formats.
> 
> The BGR888 format definition matches how pixel components are sent on
> the wire according to the CSI-2 specification.

A bit more information that you may want to capture, for readers who are
not familiar with this:

No existing media bus codes describe exactly the way data is transferred
on the CSI-2 bus. This is not a new issue, the CSI-2 YUV422 8-bit format
is described by MEDIA_BUS_FMT_UYVY8_1X16 which is an arbitrary
convention and not an exact match. Use the MEDIA_BUS_FMT_BGR888_1X24 to
follow the same convention, based on the order in which bits are
transmitted over the CSI-2 bus.

> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  drivers/media/platform/imx/imx-mipi-csis.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c
> index c7a771e3fdf3..fdf133f81c5b 100644
> --- a/drivers/media/platform/imx/imx-mipi-csis.c
> +++ b/drivers/media/platform/imx/imx-mipi-csis.c
> @@ -366,6 +366,11 @@ static const struct csis_pix_format mipi_csis_formats[] = {
>  		.data_type = MIPI_CSI2_DATA_TYPE_RGB565,
>  		.width = 16,
>  	},
> +	{

	}, {

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +		.code = MEDIA_BUS_FMT_BGR888_1X24,
> +		.data_type = MIPI_CSI2_DATA_TYPE_RGB888,
> +		.width = 24,
> +	},
>  	/* RAW (Bayer and greyscale) formats. */
>  	{
>  		.code = MEDIA_BUS_FMT_SBGGR8_1X8,

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 7/7] media: imx: imx-mipi-csis: Add output format

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Feb 18, 2022 at 07:34:21PM +0100, Jacopo Mondi wrote:
> Due to how pixel components are transmitted on the CSI-2 serial bus
> and how they are stored in memory by the CSI-2 receiver, the component
> ordering might change and the image formats on the sink and source pads
> of the receiver should reflect it.
> 
> For RGB24, in example, the component ordering on the wire as described by
> the CSI-2 specification matches the BGR888 format, while once stored in
> in memory by the CSIS receiver they match the RGB888 format.

The CSI-2 receiver doesn't store data in memory. The issue this patch
fixes is that the CSI-2 receiver deserializes data and outputs it on a
parallel bus, with a bit order that is specific to the receiver and may
not match the media bus code used to described the format on the CSI-2
bus.

> Add an additional .output field to struct csis_pix_format to allow
> propagating the correct format to the source pad after a format
> configuration on the sink.
> 
> The change is only relevant for RGB24 but paves the way for further
> format translations in future.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  drivers/media/platform/imx/imx-mipi-csis.c | 29 +++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c
> index fdf133f81c5b..128f4180d1e9 100644
> --- a/drivers/media/platform/imx/imx-mipi-csis.c
> +++ b/drivers/media/platform/imx/imx-mipi-csis.c
> @@ -351,6 +351,7 @@ struct csis_pix_format {
>  	u32 code;
>  	u32 data_type;
>  	u8 width;
> +	u32 output;

I'd move this right after code, as they're related.

>  };
>  
>  static const struct csis_pix_format mipi_csis_formats[] = {
> @@ -359,95 +360,117 @@ static const struct csis_pix_format mipi_csis_formats[] = {
>  		.code = MEDIA_BUS_FMT_UYVY8_1X16,
>  		.data_type = MIPI_CSI2_DATA_TYPE_YUV422_8,
>  		.width = 16,
> +		.output = MEDIA_BUS_FMT_UYVY8_1X16,
>  	},
>  	/* RGB formats. */
>  	{
>  		.code = MEDIA_BUS_FMT_RGB565_1X16,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RGB565,
>  		.width = 16,
> +		.output = MEDIA_BUS_FMT_RGB565_1X16,
>  	},
>  	{
>  		.code = MEDIA_BUS_FMT_BGR888_1X24,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RGB888,
>  		.width = 24,
> +		.output = MEDIA_BUS_FMT_RGB888_1X24,
>  	},
>  	/* RAW (Bayer and greyscale) formats. */
>  	{
>  		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
>  		.width = 8,
> +		.output = MEDIA_BUS_FMT_SBGGR8_1X8,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
>  		.width = 8,
> +		.output = MEDIA_BUS_FMT_SGBRG8_1X8,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
>  		.width = 8,
> +		.output = MEDIA_BUS_FMT_SGRBG8_1X8,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
>  		.width = 8,
> +		.output = MEDIA_BUS_FMT_SRGGB8_1X8,
>  	}, {
>  		.code = MEDIA_BUS_FMT_Y8_1X8,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW8,
>  		.width = 8,
> +		.output = MEDIA_BUS_FMT_Y8_1X8,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SBGGR10_1X10,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
>  		.width = 10,
> +		.output = MEDIA_BUS_FMT_SBGGR10_1X10,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SGBRG10_1X10,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
>  		.width = 10,
> +		.output = MEDIA_BUS_FMT_SGBRG10_1X10,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SGRBG10_1X10,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
>  		.width = 10,
> +		.output = MEDIA_BUS_FMT_SGRBG10_1X10,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SRGGB10_1X10,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
>  		.width = 10,
> +		.output = MEDIA_BUS_FMT_SRGGB10_1X10,
>  	}, {
>  		.code = MEDIA_BUS_FMT_Y10_1X10,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW10,
>  		.width = 10,
> +		.output = MEDIA_BUS_FMT_Y10_1X10,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SBGGR12_1X12,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
>  		.width = 12,
> +		.output = MEDIA_BUS_FMT_SBGGR12_1X12,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SGBRG12_1X12,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
>  		.width = 12,
> +		.output = MEDIA_BUS_FMT_SGBRG12_1X12,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SGRBG12_1X12,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
>  		.width = 12,
> +		.output = MEDIA_BUS_FMT_SGRBG12_1X12,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SRGGB12_1X12,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
>  		.width = 12,
> +		.output = MEDIA_BUS_FMT_SRGGB12_1X12,
>  	}, {
>  		.code = MEDIA_BUS_FMT_Y12_1X12,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW12,
>  		.width = 12,
> +		.output = MEDIA_BUS_FMT_Y12_1X12,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SBGGR14_1X14,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW14,
>  		.width = 14,
> +		.output = MEDIA_BUS_FMT_SBGGR14_1X14,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SGBRG14_1X14,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW14,
>  		.width = 14,
> +		.output = MEDIA_BUS_FMT_SGBRG14_1X14,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SGRBG14_1X14,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW14,
>  		.width = 14,
> +		.output = MEDIA_BUS_FMT_SGRBG14_1X14,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SRGGB14_1X14,
>  		.data_type = MIPI_CSI2_DATA_TYPE_RAW14,
>  		.width = 14,
> +		.output = MEDIA_BUS_FMT_SRGGB14_1X14,
>  	}
>  };
>  
> @@ -1090,7 +1113,11 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
>  	/* Propagate the format from sink to source. */
>  	fmt = mipi_csis_get_format(state, sd_state, sdformat->which,
>  				   CSIS_PAD_SOURCE);
> -	*fmt = sdformat->format;
> +
> +	/* The format on the source pad might change due to unpacking. */
> +	fmt->code = csis_fmt->output;
> +	fmt->width = sdformat->format.width;
> +	fmt->height = sdformat->format.height;

You need to also propagate colorspace. The simplest solution is

	*fmt = sdformat->format;

	/* The format on the source pad might change due to unpacking. */
	fmt->code = csis_fmt->output;

>  	/* Store the CSIS format descriptor for active formats. */
>  	if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/7] media: imx: Destage imx7-mipi-csis

[Laurent Pinchart]

Hi Jacopo,

On Fri, Feb 18, 2022 at 07:34:14PM +0100, Jacopo Mondi wrote:
> Hello
>   this series includes patches from two series previously sent:
> https://lore.kernel.org/linux-media/20220119112024.11339-1-jacopo@jmondi.org/
> https://lore.kernel.org/linux-media/20220211180216.290133-1-jacopo@jmondi.org/
> v1:
> https://lore.kernel.org/linux-media/20220214184318.409208-1-jacopo@jmondi.org/T/#t
> 
> Which can now be marked as superseded.
> 
> The first 2 patches performs the de-staging of the imx7-mipi-csis driver and
> are now reviewed.
> 
> The rest of the series builds on top of the comment received on:
> https://lore.kernel.org/linux-media/20220119112024.11339-3-jacopo@jmondi.org/
> 
> If DUAL pixel mode is used in the CSIS driver, then the CSI block of the IMX8MM
> SoC needs to be operated in dual mode as well. To do so, use the image format
> sample size to determine in the CSI bridge if dual or single mode should be
> used.
> 
> Laurent could you test on MM to see if it works now ?

Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> # On i.MX8MM

with and without the changes I've proposed in the reviews.

Have you tested 3/7 on an i.MX8MQ ?

> On top two small patches I was carrying in my tree to add more formats to the
> CSIS driver, the last one with the caveat that RGB24 is transmitted on the wire
> with one format and stored in memory with a different one.
> 
> Series based on top of the most recent media master branch.
> 
> Thanks
>   j
> 
> v1->v2:
> - Remove per-SoC handling in CSI bridge and only use image formats
> - Add TODO note to the staging driver
> - Fix PIXEL_DUAL mode comments for imx-mipi-csis
> - Add output format translation to imx-mipi-csis to handle RGB24
> 
> Jacopo Mondi (7):
>   media: imx: De-stage imx7-mipi-csis
>   media: imx: Rename imx7-mipi-csis.c to imx-mipi-csis.c
>   media: imx: imx7-media-csi: Use dual sampling for YUV 1X16
>   media: imx: imx-mipi-csis: Set PIXEL_MODE for YUV422
>   media: imx: imx-mipi-csis: Add RGB565_1X16
>   media: imx: imx-mipi-csis: Add BGR888
>   media: imx: imx-mipi-csis: Add output format
> 
>  Documentation/admin-guide/media/imx7.rst      |  2 +-
>  ...-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} |  2 +-
>  MAINTAINERS                                   |  4 +-
>  drivers/media/platform/Kconfig                |  1 +
>  drivers/media/platform/Makefile               |  1 +
>  drivers/media/platform/imx/Kconfig            | 24 ++++++++
>  drivers/media/platform/imx/Makefile           |  1 +
>  .../platform/imx/imx-mipi-csis.c}             | 59 +++++++++++++++++--
>  drivers/staging/media/imx/Makefile            |  1 -
>  drivers/staging/media/imx/TODO                | 26 ++++++++
>  drivers/staging/media/imx/imx7-media-csi.c    |  8 ++-
>  11 files changed, 117 insertions(+), 12 deletions(-)
>  rename Documentation/devicetree/bindings/media/{nxp,imx7-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} (98%)
>  create mode 100644 drivers/media/platform/imx/Kconfig
>  create mode 100644 drivers/media/platform/imx/Makefile
>  rename drivers/{staging/media/imx/imx7-mipi-csis.c => media/platform/imx/imx-mipi-csis.c} (95%)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/7] media: imx: Destage imx7-mipi-csis

[Adam Ford]

On Sun, Feb 20, 2022 at 8:56 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hello
>   this series includes patches from two series previously sent:
> https://lore.kernel.org/linux-media/20220119112024.11339-1-jacopo@jmondi.org/
> https://lore.kernel.org/linux-media/20220211180216.290133-1-jacopo@jmondi.org/
> v1:
> https://lore.kernel.org/linux-media/20220214184318.409208-1-jacopo@jmondi.org/T/#t
>
> Which can now be marked as superseded.
>
> The first 2 patches performs the de-staging of the imx7-mipi-csis driver and
> are now reviewed.
>
> The rest of the series builds on top of the comment received on:
> https://lore.kernel.org/linux-media/20220119112024.11339-3-jacopo@jmondi.org/
>
> If DUAL pixel mode is used in the CSIS driver, then the CSI block of the IMX8MM
> SoC needs to be operated in dual mode as well. To do so, use the image format
> sample size to determine in the CSI bridge if dual or single mode should be
> used.
>
> Laurent could you test on MM to see if it works now ?

Jacopo,

Do you have a repo I can clone?  If not, I need to know which branch
to apply to the series. I have an 8MM with an OV5640, and I'm willing
to test if Laurent can't.

adam
>
> On top two small patches I was carrying in my tree to add more formats to the
> CSIS driver, the last one with the caveat that RGB24 is transmitted on the wire
> with one format and stored in memory with a different one.
>
> Series based on top of the most recent media master branch.
>
> Thanks
>   j
>
> v1->v2:
> - Remove per-SoC handling in CSI bridge and only use image formats
> - Add TODO note to the staging driver
> - Fix PIXEL_DUAL mode comments for imx-mipi-csis
> - Add output format translation to imx-mipi-csis to handle RGB24
>
> Jacopo Mondi (7):
>   media: imx: De-stage imx7-mipi-csis
>   media: imx: Rename imx7-mipi-csis.c to imx-mipi-csis.c
>   media: imx: imx7-media-csi: Use dual sampling for YUV 1X16
>   media: imx: imx-mipi-csis: Set PIXEL_MODE for YUV422
>   media: imx: imx-mipi-csis: Add RGB565_1X16
>   media: imx: imx-mipi-csis: Add BGR888
>   media: imx: imx-mipi-csis: Add output format
>
>  Documentation/admin-guide/media/imx7.rst      |  2 +-
>  ...-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} |  2 +-
>  MAINTAINERS                                   |  4 +-
>  drivers/media/platform/Kconfig                |  1 +
>  drivers/media/platform/Makefile               |  1 +
>  drivers/media/platform/imx/Kconfig            | 24 ++++++++
>  drivers/media/platform/imx/Makefile           |  1 +
>  .../platform/imx/imx-mipi-csis.c}             | 59 +++++++++++++++++--
>  drivers/staging/media/imx/Makefile            |  1 -
>  drivers/staging/media/imx/TODO                | 26 ++++++++
>  drivers/staging/media/imx/imx7-media-csi.c    |  8 ++-
>  11 files changed, 117 insertions(+), 12 deletions(-)
>  rename Documentation/devicetree/bindings/media/{nxp,imx7-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} (98%)
>  create mode 100644 drivers/media/platform/imx/Kconfig
>  create mode 100644 drivers/media/platform/imx/Makefile
>  rename drivers/{staging/media/imx/imx7-mipi-csis.c => media/platform/imx/imx-mipi-csis.c} (95%)
>
> --
> 2.35.0
>

Re: [PATCH v2 0/7] media: imx: Destage imx7-mipi-csis

[Laurent Pinchart]

Hi Adam,

On Sun, Feb 20, 2022 at 12:19:30PM -0600, Adam Ford wrote:
> On Sun, Feb 20, 2022 at 8:56 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
> >
> > Hello
> >   this series includes patches from two series previously sent:
> > https://lore.kernel.org/linux-media/20220119112024.11339-1-jacopo@jmondi.org/
> > https://lore.kernel.org/linux-media/20220211180216.290133-1-jacopo@jmondi.org/
> > v1:
> > https://lore.kernel.org/linux-media/20220214184318.409208-1-jacopo@jmondi.org/T/#t
> >
> > Which can now be marked as superseded.
> >
> > The first 2 patches performs the de-staging of the imx7-mipi-csis driver and
> > are now reviewed.
> >
> > The rest of the series builds on top of the comment received on:
> > https://lore.kernel.org/linux-media/20220119112024.11339-3-jacopo@jmondi.org/
> >
> > If DUAL pixel mode is used in the CSIS driver, then the CSI block of the IMX8MM
> > SoC needs to be operated in dual mode as well. To do so, use the image format
> > sample size to determine in the CSI bridge if dual or single mode should be
> > used.
> >
> > Laurent could you test on MM to see if it works now ?
> 
> Jacopo,
> 
> Do you have a repo I can clone?  If not, I need to know which branch
> to apply to the series. I have an 8MM with an OV5640, and I'm willing
> to test if Laurent can't.

I've applied the patches on top of v5.17-rc4 plus a few backports, and
pushed the result to
https://gitlab.com/ideasonboard/nxp/linux/-/tree/pinchartl/v5.17/csis.

> > On top two small patches I was carrying in my tree to add more formats to the
> > CSIS driver, the last one with the caveat that RGB24 is transmitted on the wire
> > with one format and stored in memory with a different one.
> >
> > Series based on top of the most recent media master branch.
> >
> > Thanks
> >   j
> >
> > v1->v2:
> > - Remove per-SoC handling in CSI bridge and only use image formats
> > - Add TODO note to the staging driver
> > - Fix PIXEL_DUAL mode comments for imx-mipi-csis
> > - Add output format translation to imx-mipi-csis to handle RGB24
> >
> > Jacopo Mondi (7):
> >   media: imx: De-stage imx7-mipi-csis
> >   media: imx: Rename imx7-mipi-csis.c to imx-mipi-csis.c
> >   media: imx: imx7-media-csi: Use dual sampling for YUV 1X16
> >   media: imx: imx-mipi-csis: Set PIXEL_MODE for YUV422
> >   media: imx: imx-mipi-csis: Add RGB565_1X16
> >   media: imx: imx-mipi-csis: Add BGR888
> >   media: imx: imx-mipi-csis: Add output format
> >
> >  Documentation/admin-guide/media/imx7.rst      |  2 +-
> >  ...-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} |  2 +-
> >  MAINTAINERS                                   |  4 +-
> >  drivers/media/platform/Kconfig                |  1 +
> >  drivers/media/platform/Makefile               |  1 +
> >  drivers/media/platform/imx/Kconfig            | 24 ++++++++
> >  drivers/media/platform/imx/Makefile           |  1 +
> >  .../platform/imx/imx-mipi-csis.c}             | 59 +++++++++++++++++--
> >  drivers/staging/media/imx/Makefile            |  1 -
> >  drivers/staging/media/imx/TODO                | 26 ++++++++
> >  drivers/staging/media/imx/imx7-media-csi.c    |  8 ++-
> >  11 files changed, 117 insertions(+), 12 deletions(-)
> >  rename Documentation/devicetree/bindings/media/{nxp,imx7-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} (98%)
> >  create mode 100644 drivers/media/platform/imx/Kconfig
> >  create mode 100644 drivers/media/platform/imx/Makefile
> >  rename drivers/{staging/media/imx/imx7-mipi-csis.c => media/platform/imx/imx-mipi-csis.c} (95%)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/7] media: imx: Destage imx7-mipi-csis

[Jacopo Mondi]

Hi Adam, Laurent,

On Mon, Feb 21, 2022 at 12:41:45AM +0200, Laurent Pinchart wrote:
> Hi Adam,
>
> On Sun, Feb 20, 2022 at 12:19:30PM -0600, Adam Ford wrote:
> > On Sun, Feb 20, 2022 at 8:56 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
> > >
> > > Hello
> > >   this series includes patches from two series previously sent:
> > > https://lore.kernel.org/linux-media/20220119112024.11339-1-jacopo@jmondi.org/
> > > https://lore.kernel.org/linux-media/20220211180216.290133-1-jacopo@jmondi.org/
> > > v1:
> > > https://lore.kernel.org/linux-media/20220214184318.409208-1-jacopo@jmondi.org/T/#t
> > >
> > > Which can now be marked as superseded.
> > >
> > > The first 2 patches performs the de-staging of the imx7-mipi-csis driver and
> > > are now reviewed.
> > >
> > > The rest of the series builds on top of the comment received on:
> > > https://lore.kernel.org/linux-media/20220119112024.11339-3-jacopo@jmondi.org/
> > >
> > > If DUAL pixel mode is used in the CSIS driver, then the CSI block of the IMX8MM
> > > SoC needs to be operated in dual mode as well. To do so, use the image format
> > > sample size to determine in the CSI bridge if dual or single mode should be
> > > used.
> > >
> > > Laurent could you test on MM to see if it works now ?
> >
> > Jacopo,
> >
> > Do you have a repo I can clone?  If not, I need to know which branch
> > to apply to the series. I have an 8MM with an OV5640, and I'm willing
> > to test if Laurent can't.
>
> I've applied the patches on top of v5.17-rc4 plus a few backports, and
> pushed the result to
> https://gitlab.com/ideasonboard/nxp/linux/-/tree/pinchartl/v5.17/csis.
>

Oh, you've been slightly quicker then me :p

I was about to ask Adam if he was interested in a branch which also
contains
https://patchwork.linuxtv.org/project/linux-media/list/?series=7311

As he has an ov5640 :)

Adam, if you get here faster than me, please try Laurent's branch and
let me know. Otherwise I will provide a branch with a v3 of this
series and the ov5640 changes as well, if you're interested in testing
them.

Thanks
  j

> > > On top two small patches I was carrying in my tree to add more formats to the
> > > CSIS driver, the last one with the caveat that RGB24 is transmitted on the wire
> > > with one format and stored in memory with a different one.
> > >
> > > Series based on top of the most recent media master branch.
> > >
> > > Thanks
> > >   j
> > >
> > > v1->v2:
> > > - Remove per-SoC handling in CSI bridge and only use image formats
> > > - Add TODO note to the staging driver
> > > - Fix PIXEL_DUAL mode comments for imx-mipi-csis
> > > - Add output format translation to imx-mipi-csis to handle RGB24
> > >
> > > Jacopo Mondi (7):
> > >   media: imx: De-stage imx7-mipi-csis
> > >   media: imx: Rename imx7-mipi-csis.c to imx-mipi-csis.c
> > >   media: imx: imx7-media-csi: Use dual sampling for YUV 1X16
> > >   media: imx: imx-mipi-csis: Set PIXEL_MODE for YUV422
> > >   media: imx: imx-mipi-csis: Add RGB565_1X16
> > >   media: imx: imx-mipi-csis: Add BGR888
> > >   media: imx: imx-mipi-csis: Add output format
> > >
> > >  Documentation/admin-guide/media/imx7.rst      |  2 +-
> > >  ...-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} |  2 +-
> > >  MAINTAINERS                                   |  4 +-
> > >  drivers/media/platform/Kconfig                |  1 +
> > >  drivers/media/platform/Makefile               |  1 +
> > >  drivers/media/platform/imx/Kconfig            | 24 ++++++++
> > >  drivers/media/platform/imx/Makefile           |  1 +
> > >  .../platform/imx/imx-mipi-csis.c}             | 59 +++++++++++++++++--
> > >  drivers/staging/media/imx/Makefile            |  1 -
> > >  drivers/staging/media/imx/TODO                | 26 ++++++++
> > >  drivers/staging/media/imx/imx7-media-csi.c    |  8 ++-
> > >  11 files changed, 117 insertions(+), 12 deletions(-)
> > >  rename Documentation/devicetree/bindings/media/{nxp,imx7-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} (98%)
> > >  create mode 100644 drivers/media/platform/imx/Kconfig
> > >  create mode 100644 drivers/media/platform/imx/Makefile
> > >  rename drivers/{staging/media/imx/imx7-mipi-csis.c => media/platform/imx/imx-mipi-csis.c} (95%)
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH v2 0/7] media: imx: Destage imx7-mipi-csis

[Laurent Pinchart]

Hi Jacopo,

On Mon, Feb 21, 2022 at 08:58:59AM +0100, Jacopo Mondi wrote:
> On Mon, Feb 21, 2022 at 12:41:45AM +0200, Laurent Pinchart wrote:
> > On Sun, Feb 20, 2022 at 12:19:30PM -0600, Adam Ford wrote:
> > > On Sun, Feb 20, 2022 at 8:56 AM Jacopo Mondi wrote:
> > > >
> > > > Hello
> > > >   this series includes patches from two series previously sent:
> > > > https://lore.kernel.org/linux-media/20220119112024.11339-1-jacopo@jmondi.org/
> > > > https://lore.kernel.org/linux-media/20220211180216.290133-1-jacopo@jmondi.org/
> > > > v1:
> > > > https://lore.kernel.org/linux-media/20220214184318.409208-1-jacopo@jmondi.org/T/#t
> > > >
> > > > Which can now be marked as superseded.
> > > >
> > > > The first 2 patches performs the de-staging of the imx7-mipi-csis driver and
> > > > are now reviewed.
> > > >
> > > > The rest of the series builds on top of the comment received on:
> > > > https://lore.kernel.org/linux-media/20220119112024.11339-3-jacopo@jmondi.org/
> > > >
> > > > If DUAL pixel mode is used in the CSIS driver, then the CSI block of the IMX8MM
> > > > SoC needs to be operated in dual mode as well. To do so, use the image format
> > > > sample size to determine in the CSI bridge if dual or single mode should be
> > > > used.
> > > >
> > > > Laurent could you test on MM to see if it works now ?
> > >
> > > Jacopo,
> > >
> > > Do you have a repo I can clone?  If not, I need to know which branch
> > > to apply to the series. I have an 8MM with an OV5640, and I'm willing
> > > to test if Laurent can't.
> >
> > I've applied the patches on top of v5.17-rc4 plus a few backports, and
> > pushed the result to
> > https://gitlab.com/ideasonboard/nxp/linux/-/tree/pinchartl/v5.17/csis.
> >
> 
> Oh, you've been slightly quicker then me :p
> 
> I was about to ask Adam if he was interested in a branch which also
> contains
> https://patchwork.linuxtv.org/project/linux-media/list/?series=7311
> 
> As he has an ov5640 :)

Do you mean
https://gitlab.com/ideasonboard/nxp/linux/-/tree/pinchartl/v5.17/sensors/ov5640/v2
? :-)

> Adam, if you get here faster than me, please try Laurent's branch and
> let me know. Otherwise I will provide a branch with a v3 of this
> series and the ov5640 changes as well, if you're interested in testing
> them.
> 
> Thanks
>   j
> 
> > > > On top two small patches I was carrying in my tree to add more formats to the
> > > > CSIS driver, the last one with the caveat that RGB24 is transmitted on the wire
> > > > with one format and stored in memory with a different one.
> > > >
> > > > Series based on top of the most recent media master branch.
> > > >
> > > > Thanks
> > > >   j
> > > >
> > > > v1->v2:
> > > > - Remove per-SoC handling in CSI bridge and only use image formats
> > > > - Add TODO note to the staging driver
> > > > - Fix PIXEL_DUAL mode comments for imx-mipi-csis
> > > > - Add output format translation to imx-mipi-csis to handle RGB24
> > > >
> > > > Jacopo Mondi (7):
> > > >   media: imx: De-stage imx7-mipi-csis
> > > >   media: imx: Rename imx7-mipi-csis.c to imx-mipi-csis.c
> > > >   media: imx: imx7-media-csi: Use dual sampling for YUV 1X16
> > > >   media: imx: imx-mipi-csis: Set PIXEL_MODE for YUV422
> > > >   media: imx: imx-mipi-csis: Add RGB565_1X16
> > > >   media: imx: imx-mipi-csis: Add BGR888
> > > >   media: imx: imx-mipi-csis: Add output format
> > > >
> > > >  Documentation/admin-guide/media/imx7.rst      |  2 +-
> > > >  ...-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} |  2 +-
> > > >  MAINTAINERS                                   |  4 +-
> > > >  drivers/media/platform/Kconfig                |  1 +
> > > >  drivers/media/platform/Makefile               |  1 +
> > > >  drivers/media/platform/imx/Kconfig            | 24 ++++++++
> > > >  drivers/media/platform/imx/Makefile           |  1 +
> > > >  .../platform/imx/imx-mipi-csis.c}             | 59 +++++++++++++++++--
> > > >  drivers/staging/media/imx/Makefile            |  1 -
> > > >  drivers/staging/media/imx/TODO                | 26 ++++++++
> > > >  drivers/staging/media/imx/imx7-media-csi.c    |  8 ++-
> > > >  11 files changed, 117 insertions(+), 12 deletions(-)
> > > >  rename Documentation/devicetree/bindings/media/{nxp,imx7-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} (98%)
> > > >  create mode 100644 drivers/media/platform/imx/Kconfig
> > > >  create mode 100644 drivers/media/platform/imx/Makefile
> > > >  rename drivers/{staging/media/imx/imx7-mipi-csis.c => media/platform/imx/imx-mipi-csis.c} (95%)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/7] media: imx: imx7-media-csi: Use dual sampling for YUV 1X16

[Jacopo Mondi]

Hi Laurent,

On Sun, Feb 20, 2022 at 10:16:15AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Fri, Feb 18, 2022 at 07:34:17PM +0100, Jacopo Mondi wrote:
> > The CSI bridge should operate in dual pixel sampling mode when it is
>
> s/dual pixel sampling mode/dual component mode/
>
> > connected to a pixel transmitter that transfers two pixel samples (16
> > bits) at the time in YUYV formats.
>
> It's actually one pixel per clock. Without BIT_TWO_8BIT_SENSOR and
> BIT_MIPI_DOUBLE_CMPNT, the CSI bridge expects 8-bit data, which requires
> two clock cycles to transfer one pixel. When setting those bits, it
> expects 16-bit data, with one clock cycle per pixel. The
> MIPI_DOUBLE_CMPNT documentation states this clearly:

Indeed, that's why I said two pixels -samples-.

Maybe the usage of 'sample' is not right ? Two pixel samples in YUYV
mode to me means two bytes which form 1 pixel.

>
> 20 MIPI_DOUBLE_CMPNT
> Double component per clock cycle in YUV422 formats.
> 0 Single component per clock cycle
> (half pixel per clock cycle)
> 1 Double component per clock cycle
> (a pixel per clock cycle)
>
> > Use the image format variants to determine if single or dual pixel mode
>
> "dual pixel" is a concept of the CSIS, not the CSI bridge. This should
> mention single or double component mode.

Ack

>
> > should be used.
> >
> > Add a note to the TODO file to record that the list of supported formats
> > should be restricted to the SoC model the CSI bridge is integrated on
> > to avoid potential pipeline mis-configurations.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  drivers/staging/media/imx/TODO             | 26 ++++++++++++++++++++++
> >  drivers/staging/media/imx/imx7-media-csi.c |  8 +++++--
> >  2 files changed, 32 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/media/imx/TODO b/drivers/staging/media/imx/TODO
> > index 06c94f20ecf8..e15eba32cc94 100644
> > --- a/drivers/staging/media/imx/TODO
> > +++ b/drivers/staging/media/imx/TODO
> > @@ -27,3 +27,29 @@
> >  - i.MX7: all of the above, since it uses the imx media core
> >
> >  - i.MX7: use Frame Interval Monitor
> > +
> > +- imx7-media-csi: Restrict the supported formats list to the SoC version.
> > +
> > +  The imx7 CSI bridge can be configured to sample pixel components from the Rx
> > +  queue in single  (8bpp) or double (16bpp) modes. Image format variations with
> > +  different sample sizes (ie YUYV_2X8 vs YUYV_1X16) determine the sampling size
> > +  (see imx7_csi_configure()).
> > +
> > +  As the imx7 CSI bridge can be interfaced with different IP blocks depending on
> > +  the SoC model it is integrated on, the Rx queue sampling size should match
> > +  the size of samples transferred by the transmitting IP block.
> > +
> > +  To avoid mis-configurations of the capture pipeline, the enumeration of the
> > +  supported formats should be restricted to match the pixel source
> > +  transmitting mode.
> > +
> > +  Examples: i.MX8MM SoC integrates the CSI bridge with the Samsung CSIS CSI-2
> > +  receiver which operates in dual pixel sampling mode. The CSI bridge should
> > +  only expose the 1X16 formats variant which instructs it to operate in dual
> > +  pixel sampling mode. When the CSI bridge is instead integrated on an i.MX8MQ
> > +  SoC, which features a CSI-2 receiver that operates in single sampling mode, it
> > +  should only expose the 2X8 formats variant which instruct it to operate in
> > +  single sampling mode.
> > +
> > +  This currently only applies to YUYV formats, but other formats might need
> > +  to be treated in the same way.
> > diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> > index 32311fc0e2a4..108360ae3710 100644
> > --- a/drivers/staging/media/imx/imx7-media-csi.c
> > +++ b/drivers/staging/media/imx/imx7-media-csi.c
> > @@ -503,11 +503,15 @@ static void imx7_csi_configure(struct imx7_csi *csi)
> >  		 * all of them comply. Support both variants.
> >  		 */
>
> You can drop this comment, as the two variants are not related to the
> CSI-2 bus format (which should always be UYVY8_1X16) but to the format
> output by the CSI-2 RX.
>
> I would add another comment to explain this clearly:
>
> 		/*
> 		 * The CSI bridge has a 16-bit input bus. Depending on
> 		 * the connected source, data may be transmitted with 8
> 		 * or 10 bits per clock sample (in bits [9:2] or [9:0]
> 		 * respectively) or with 16 bits per clock sample (in
> 		 * bits [15:0]). The data is then packed into a 32-bit
> 		 * FIFO (as shown in figure 13-11 of the i.MX8MM
> 		 * reference manual rev. 3).
> 		 *
> 		 * The data packing in a 32-bit FIFO input workd is

s/workd/word

> 		 * controlled by the CR3 TWO_8BIT_SENSOR field (also
> 		 * known as SENSOR_16BITS in the i.MX8MM reference
> 		 * manual). When set to 0, data packing groups four
> 		 * 8-bit input samples (bits [9:2]). When set to 1, data
> 		 * packing groups two 16-bit input samples (bits
> 		 * [15:0]).
> 		 *
> 		 * The register field CR18 MIPI_DOUBLE_CMPNT also needs
> 		 * to be configured according to the input format for
> 		 * YUV 4:2:2 data. The field controls the gasket between
> 		 * the CSI-2 receiver and the CSI bridge. On i.MX7 and
> 		 * i.MX8MM, the field must be set to 1 when the CSIS
> 		 * outputs 16-bit samples. On i.MX8MQ, the gasket
> 		 * ignores the MIPI_DOUBLE_CMPNT bit and YUV 4:2:2
> 		 * always uses 16-bit samples. Setting MIPI_DOUBLE_CMPNT
> 		 * in that case has no effect, but doesn't cause any
> 		 * issue.
> 		 */
>
> With this, someone trying to figure out what those bits do will
> hopefully be able to get it right :-)

Thanks, very clear. I didn't get the last part about the MQ. I thought
the NW csi-2 receiver worked with 8-bit samples that's why we need to
maintain the CSI bridge compatible with the 2X8 format variant (this
and also for imx7 + parallel).

Actually, looking at the MQ reference manual

"15.2.1.3.5 CSI-2 Controller Core Configurations"  reports:
The CSI-2 RX Controller Core User Interface supports either a single,
double, or quad pixel wide data path. The double and quad wide pixel
modes can help reduce the frequency the user interface must run at
while the single pixel wide mode can be easier to interface to for
some applications.

This chip supports the following:
• Single Pixel Configuration

It seems the csi-2 receiver on the MQ works in single pixel mode only.
It should then expose the 2X8 format variant only, which is
technically incorrect for a serial transmitter. Cul de sac ?


>
> With these changes,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks
   j

>
> >  		case MEDIA_BUS_FMT_UYVY8_2X8:
> > -		case MEDIA_BUS_FMT_UYVY8_1X16:
> >  		case MEDIA_BUS_FMT_YUYV8_2X8:
> > -		case MEDIA_BUS_FMT_YUYV8_1X16:
> >  			cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B;
> >  			break;
> > +		case MEDIA_BUS_FMT_UYVY8_1X16:
> > +		case MEDIA_BUS_FMT_YUYV8_1X16:
> > +			cr3 |= BIT_TWO_8BIT_SENSOR;
> > +			cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B |
> > +				BIT_MIPI_DOUBLE_CMPNT;
> > +			break;
> >  		}
> >  	}
> >
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH v2 0/7] media: imx: Destage imx7-mipi-csis

[Jacopo Mondi]

Hi Laurent,

On Mon, Feb 21, 2022 at 10:24:32AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Mon, Feb 21, 2022 at 08:58:59AM +0100, Jacopo Mondi wrote:
> > On Mon, Feb 21, 2022 at 12:41:45AM +0200, Laurent Pinchart wrote:
> > > On Sun, Feb 20, 2022 at 12:19:30PM -0600, Adam Ford wrote:
> > > > On Sun, Feb 20, 2022 at 8:56 AM Jacopo Mondi wrote:
> > > > >
> > > > > Hello
> > > > >   this series includes patches from two series previously sent:
> > > > > https://lore.kernel.org/linux-media/20220119112024.11339-1-jacopo@jmondi.org/
> > > > > https://lore.kernel.org/linux-media/20220211180216.290133-1-jacopo@jmondi.org/
> > > > > v1:
> > > > > https://lore.kernel.org/linux-media/20220214184318.409208-1-jacopo@jmondi.org/T/#t
> > > > >
> > > > > Which can now be marked as superseded.
> > > > >
> > > > > The first 2 patches performs the de-staging of the imx7-mipi-csis driver and
> > > > > are now reviewed.
> > > > >
> > > > > The rest of the series builds on top of the comment received on:
> > > > > https://lore.kernel.org/linux-media/20220119112024.11339-3-jacopo@jmondi.org/
> > > > >
> > > > > If DUAL pixel mode is used in the CSIS driver, then the CSI block of the IMX8MM
> > > > > SoC needs to be operated in dual mode as well. To do so, use the image format
> > > > > sample size to determine in the CSI bridge if dual or single mode should be
> > > > > used.
> > > > >
> > > > > Laurent could you test on MM to see if it works now ?
> > > >
> > > > Jacopo,
> > > >
> > > > Do you have a repo I can clone?  If not, I need to know which branch
> > > > to apply to the series. I have an 8MM with an OV5640, and I'm willing
> > > > to test if Laurent can't.
> > >
> > > I've applied the patches on top of v5.17-rc4 plus a few backports, and
> > > pushed the result to
> > > https://gitlab.com/ideasonboard/nxp/linux/-/tree/pinchartl/v5.17/csis.
> > >
> >
> > Oh, you've been slightly quicker then me :p
> >
> > I was about to ask Adam if he was interested in a branch which also
> > contains
> > https://patchwork.linuxtv.org/project/linux-media/list/?series=7311
> >
> > As he has an ov5640 :)
>
> Do you mean
> https://gitlab.com/ideasonboard/nxp/linux/-/tree/pinchartl/v5.17/sensors/ov5640/v2
> ? :-)

Almost! Your branch does not contain csis changes.

I'll provide Adam a branch with csis-v3 + ov5640 on top once the
few questions I have on csis v2 have been clarified.

>
> > Adam, if you get here faster than me, please try Laurent's branch and
> > let me know. Otherwise I will provide a branch with a v3 of this
> > series and the ov5640 changes as well, if you're interested in testing
> > them.
> >
> > Thanks
> >   j
> >
> > > > > On top two small patches I was carrying in my tree to add more formats to the
> > > > > CSIS driver, the last one with the caveat that RGB24 is transmitted on the wire
> > > > > with one format and stored in memory with a different one.
> > > > >
> > > > > Series based on top of the most recent media master branch.
> > > > >
> > > > > Thanks
> > > > >   j
> > > > >
> > > > > v1->v2:
> > > > > - Remove per-SoC handling in CSI bridge and only use image formats
> > > > > - Add TODO note to the staging driver
> > > > > - Fix PIXEL_DUAL mode comments for imx-mipi-csis
> > > > > - Add output format translation to imx-mipi-csis to handle RGB24
> > > > >
> > > > > Jacopo Mondi (7):
> > > > >   media: imx: De-stage imx7-mipi-csis
> > > > >   media: imx: Rename imx7-mipi-csis.c to imx-mipi-csis.c
> > > > >   media: imx: imx7-media-csi: Use dual sampling for YUV 1X16
> > > > >   media: imx: imx-mipi-csis: Set PIXEL_MODE for YUV422
> > > > >   media: imx: imx-mipi-csis: Add RGB565_1X16
> > > > >   media: imx: imx-mipi-csis: Add BGR888
> > > > >   media: imx: imx-mipi-csis: Add output format
> > > > >
> > > > >  Documentation/admin-guide/media/imx7.rst      |  2 +-
> > > > >  ...-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} |  2 +-
> > > > >  MAINTAINERS                                   |  4 +-
> > > > >  drivers/media/platform/Kconfig                |  1 +
> > > > >  drivers/media/platform/Makefile               |  1 +
> > > > >  drivers/media/platform/imx/Kconfig            | 24 ++++++++
> > > > >  drivers/media/platform/imx/Makefile           |  1 +
> > > > >  .../platform/imx/imx-mipi-csis.c}             | 59 +++++++++++++++++--
> > > > >  drivers/staging/media/imx/Makefile            |  1 -
> > > > >  drivers/staging/media/imx/TODO                | 26 ++++++++
> > > > >  drivers/staging/media/imx/imx7-media-csi.c    |  8 ++-
> > > > >  11 files changed, 117 insertions(+), 12 deletions(-)
> > > > >  rename Documentation/devicetree/bindings/media/{nxp,imx7-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} (98%)
> > > > >  create mode 100644 drivers/media/platform/imx/Kconfig
> > > > >  create mode 100644 drivers/media/platform/imx/Makefile
> > > > >  rename drivers/{staging/media/imx/imx7-mipi-csis.c => media/platform/imx/imx-mipi-csis.c} (95%)
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH v2 3/7] media: imx: imx7-media-csi: Use dual sampling for YUV 1X16

[Laurent Pinchart]

Hi Jacopo,

On Mon, Feb 21, 2022 at 09:43:07AM +0100, Jacopo Mondi wrote:
> On Sun, Feb 20, 2022 at 10:16:15AM +0200, Laurent Pinchart wrote:
> > On Fri, Feb 18, 2022 at 07:34:17PM +0100, Jacopo Mondi wrote:
> > > The CSI bridge should operate in dual pixel sampling mode when it is
> >
> > s/dual pixel sampling mode/dual component mode/
> >
> > > connected to a pixel transmitter that transfers two pixel samples (16
> > > bits) at the time in YUYV formats.
> >
> > It's actually one pixel per clock. Without BIT_TWO_8BIT_SENSOR and
> > BIT_MIPI_DOUBLE_CMPNT, the CSI bridge expects 8-bit data, which requires
> > two clock cycles to transfer one pixel. When setting those bits, it
> > expects 16-bit data, with one clock cycle per pixel. The
> > MIPI_DOUBLE_CMPNT documentation states this clearly:
> 
> Indeed, that's why I said two pixels -samples-.
> 
> Maybe the usage of 'sample' is not right ? Two pixel samples in YUYV
> mode to me means two bytes which form 1 pixel.

I think it's a bit unclear. Maybe you could write

The CSI bridge should operate in dual component mode when it is
connected to a pixel transmitter that transfers two components at a time
in YUV 422 formats (16 bits, Y + U/V).

> > 20 MIPI_DOUBLE_CMPNT
> > Double component per clock cycle in YUV422 formats.
> > 0 Single component per clock cycle
> > (half pixel per clock cycle)
> > 1 Double component per clock cycle
> > (a pixel per clock cycle)
> >
> > > Use the image format variants to determine if single or dual pixel mode
> >
> > "dual pixel" is a concept of the CSIS, not the CSI bridge. This should
> > mention single or double component mode.
> 
> Ack
> 
> > > should be used.
> > >
> > > Add a note to the TODO file to record that the list of supported formats
> > > should be restricted to the SoC model the CSI bridge is integrated on
> > > to avoid potential pipeline mis-configurations.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > ---
> > >  drivers/staging/media/imx/TODO             | 26 ++++++++++++++++++++++
> > >  drivers/staging/media/imx/imx7-media-csi.c |  8 +++++--
> > >  2 files changed, 32 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/media/imx/TODO b/drivers/staging/media/imx/TODO
> > > index 06c94f20ecf8..e15eba32cc94 100644
> > > --- a/drivers/staging/media/imx/TODO
> > > +++ b/drivers/staging/media/imx/TODO
> > > @@ -27,3 +27,29 @@
> > >  - i.MX7: all of the above, since it uses the imx media core
> > >
> > >  - i.MX7: use Frame Interval Monitor
> > > +
> > > +- imx7-media-csi: Restrict the supported formats list to the SoC version.
> > > +
> > > +  The imx7 CSI bridge can be configured to sample pixel components from the Rx
> > > +  queue in single  (8bpp) or double (16bpp) modes. Image format variations with
> > > +  different sample sizes (ie YUYV_2X8 vs YUYV_1X16) determine the sampling size
> > > +  (see imx7_csi_configure()).
> > > +
> > > +  As the imx7 CSI bridge can be interfaced with different IP blocks depending on
> > > +  the SoC model it is integrated on, the Rx queue sampling size should match
> > > +  the size of samples transferred by the transmitting IP block.
> > > +
> > > +  To avoid mis-configurations of the capture pipeline, the enumeration of the
> > > +  supported formats should be restricted to match the pixel source
> > > +  transmitting mode.
> > > +
> > > +  Examples: i.MX8MM SoC integrates the CSI bridge with the Samsung CSIS CSI-2
> > > +  receiver which operates in dual pixel sampling mode. The CSI bridge should
> > > +  only expose the 1X16 formats variant which instructs it to operate in dual
> > > +  pixel sampling mode. When the CSI bridge is instead integrated on an i.MX8MQ
> > > +  SoC, which features a CSI-2 receiver that operates in single sampling mode, it
> > > +  should only expose the 2X8 formats variant which instruct it to operate in
> > > +  single sampling mode.
> > > +
> > > +  This currently only applies to YUYV formats, but other formats might need
> > > +  to be treated in the same way.
> > > diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> > > index 32311fc0e2a4..108360ae3710 100644
> > > --- a/drivers/staging/media/imx/imx7-media-csi.c
> > > +++ b/drivers/staging/media/imx/imx7-media-csi.c
> > > @@ -503,11 +503,15 @@ static void imx7_csi_configure(struct imx7_csi *csi)
> > >  		 * all of them comply. Support both variants.
> > >  		 */
> >
> > You can drop this comment, as the two variants are not related to the
> > CSI-2 bus format (which should always be UYVY8_1X16) but to the format
> > output by the CSI-2 RX.
> >
> > I would add another comment to explain this clearly:
> >
> > 		/*
> > 		 * The CSI bridge has a 16-bit input bus. Depending on
> > 		 * the connected source, data may be transmitted with 8
> > 		 * or 10 bits per clock sample (in bits [9:2] or [9:0]
> > 		 * respectively) or with 16 bits per clock sample (in
> > 		 * bits [15:0]). The data is then packed into a 32-bit
> > 		 * FIFO (as shown in figure 13-11 of the i.MX8MM
> > 		 * reference manual rev. 3).
> > 		 *
> > 		 * The data packing in a 32-bit FIFO input workd is
> 
> s/workd/word
> 
> > 		 * controlled by the CR3 TWO_8BIT_SENSOR field (also
> > 		 * known as SENSOR_16BITS in the i.MX8MM reference
> > 		 * manual). When set to 0, data packing groups four
> > 		 * 8-bit input samples (bits [9:2]). When set to 1, data
> > 		 * packing groups two 16-bit input samples (bits
> > 		 * [15:0]).
> > 		 *
> > 		 * The register field CR18 MIPI_DOUBLE_CMPNT also needs
> > 		 * to be configured according to the input format for
> > 		 * YUV 4:2:2 data. The field controls the gasket between
> > 		 * the CSI-2 receiver and the CSI bridge. On i.MX7 and
> > 		 * i.MX8MM, the field must be set to 1 when the CSIS
> > 		 * outputs 16-bit samples. On i.MX8MQ, the gasket
> > 		 * ignores the MIPI_DOUBLE_CMPNT bit and YUV 4:2:2
> > 		 * always uses 16-bit samples. Setting MIPI_DOUBLE_CMPNT
> > 		 * in that case has no effect, but doesn't cause any
> > 		 * issue.
> > 		 */
> >
> > With this, someone trying to figure out what those bits do will
> > hopefully be able to get it right :-)
> 
> Thanks, very clear. I didn't get the last part about the MQ. I thought
> the NW csi-2 receiver worked with 8-bit samples that's why we need to
> maintain the CSI bridge compatible with the 2X8 format variant (this
> and also for imx7 + parallel).
> 
> Actually, looking at the MQ reference manual
> 
> "15.2.1.3.5 CSI-2 Controller Core Configurations"  reports:
> The CSI-2 RX Controller Core User Interface supports either a single,
> double, or quad pixel wide data path. The double and quad wide pixel
> modes can help reduce the frequency the user interface must run at
> while the single pixel wide mode can be easier to interface to for
> some applications.
> 
> This chip supports the following:
> • Single Pixel Configuration
> 
> It seems the csi-2 receiver on the MQ works in single pixel mode only.
> It should then expose the 2X8 format variant only, which is
> technically incorrect for a serial transmitter. Cul de sac ?

It's a different CSI-2 receiver, and uses a different terminology.
"Single pixel mode" there refers to one pixel per clock cycle, which
means, for YUV422 8-bit, 16 bits per cycle. The MIPI_DOUBLE_CMPNT bit is
ignored by the gasket on i.MX8MQ.

> > With these changes,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thanks
>    j
> 
> > >  		case MEDIA_BUS_FMT_UYVY8_2X8:
> > > -		case MEDIA_BUS_FMT_UYVY8_1X16:
> > >  		case MEDIA_BUS_FMT_YUYV8_2X8:
> > > -		case MEDIA_BUS_FMT_YUYV8_1X16:
> > >  			cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B;
> > >  			break;
> > > +		case MEDIA_BUS_FMT_UYVY8_1X16:
> > > +		case MEDIA_BUS_FMT_YUYV8_1X16:
> > > +			cr3 |= BIT_TWO_8BIT_SENSOR;
> > > +			cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B |
> > > +				BIT_MIPI_DOUBLE_CMPNT;
> > > +			break;
> > >  		}
> > >  	}
> > >

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/7] media: imx: imx7-media-csi: Use dual sampling for YUV 1X16

[Jacopo Mondi]

On Mon, Feb 21, 2022 at 10:49:15AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Mon, Feb 21, 2022 at 09:43:07AM +0100, Jacopo Mondi wrote:
> > On Sun, Feb 20, 2022 at 10:16:15AM +0200, Laurent Pinchart wrote:
> > > On Fri, Feb 18, 2022 at 07:34:17PM +0100, Jacopo Mondi wrote:
> > > > The CSI bridge should operate in dual pixel sampling mode when it is
> > >
> > > s/dual pixel sampling mode/dual component mode/
> > >
> > > > connected to a pixel transmitter that transfers two pixel samples (16
> > > > bits) at the time in YUYV formats.
> > >
> > > It's actually one pixel per clock. Without BIT_TWO_8BIT_SENSOR and
> > > BIT_MIPI_DOUBLE_CMPNT, the CSI bridge expects 8-bit data, which requires
> > > two clock cycles to transfer one pixel. When setting those bits, it
> > > expects 16-bit data, with one clock cycle per pixel. The
> > > MIPI_DOUBLE_CMPNT documentation states this clearly:
> >
> > Indeed, that's why I said two pixels -samples-.
> >
> > Maybe the usage of 'sample' is not right ? Two pixel samples in YUYV
> > mode to me means two bytes which form 1 pixel.
>
> I think it's a bit unclear. Maybe you could write
>
> The CSI bridge should operate in dual component mode when it is
> connected to a pixel transmitter that transfers two components at a time
> in YUV 422 formats (16 bits, Y + U/V).
>
> > > 20 MIPI_DOUBLE_CMPNT
> > > Double component per clock cycle in YUV422 formats.
> > > 0 Single component per clock cycle
> > > (half pixel per clock cycle)
> > > 1 Double component per clock cycle
> > > (a pixel per clock cycle)
> > >
> > > > Use the image format variants to determine if single or dual pixel mode
> > >
> > > "dual pixel" is a concept of the CSIS, not the CSI bridge. This should
> > > mention single or double component mode.
> >
> > Ack
> >
> > > > should be used.
> > > >
> > > > Add a note to the TODO file to record that the list of supported formats
> > > > should be restricted to the SoC model the CSI bridge is integrated on
> > > > to avoid potential pipeline mis-configurations.
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > > ---
> > > >  drivers/staging/media/imx/TODO             | 26 ++++++++++++++++++++++
> > > >  drivers/staging/media/imx/imx7-media-csi.c |  8 +++++--
> > > >  2 files changed, 32 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/media/imx/TODO b/drivers/staging/media/imx/TODO
> > > > index 06c94f20ecf8..e15eba32cc94 100644
> > > > --- a/drivers/staging/media/imx/TODO
> > > > +++ b/drivers/staging/media/imx/TODO
> > > > @@ -27,3 +27,29 @@
> > > >  - i.MX7: all of the above, since it uses the imx media core
> > > >
> > > >  - i.MX7: use Frame Interval Monitor
> > > > +
> > > > +- imx7-media-csi: Restrict the supported formats list to the SoC version.
> > > > +
> > > > +  The imx7 CSI bridge can be configured to sample pixel components from the Rx
> > > > +  queue in single  (8bpp) or double (16bpp) modes. Image format variations with
> > > > +  different sample sizes (ie YUYV_2X8 vs YUYV_1X16) determine the sampling size
> > > > +  (see imx7_csi_configure()).
> > > > +
> > > > +  As the imx7 CSI bridge can be interfaced with different IP blocks depending on
> > > > +  the SoC model it is integrated on, the Rx queue sampling size should match
> > > > +  the size of samples transferred by the transmitting IP block.
> > > > +
> > > > +  To avoid mis-configurations of the capture pipeline, the enumeration of the
> > > > +  supported formats should be restricted to match the pixel source
> > > > +  transmitting mode.
> > > > +
> > > > +  Examples: i.MX8MM SoC integrates the CSI bridge with the Samsung CSIS CSI-2
> > > > +  receiver which operates in dual pixel sampling mode. The CSI bridge should
> > > > +  only expose the 1X16 formats variant which instructs it to operate in dual
> > > > +  pixel sampling mode. When the CSI bridge is instead integrated on an i.MX8MQ
> > > > +  SoC, which features a CSI-2 receiver that operates in single sampling mode, it
> > > > +  should only expose the 2X8 formats variant which instruct it to operate in
> > > > +  single sampling mode.
> > > > +
> > > > +  This currently only applies to YUYV formats, but other formats might need
> > > > +  to be treated in the same way.
> > > > diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> > > > index 32311fc0e2a4..108360ae3710 100644
> > > > --- a/drivers/staging/media/imx/imx7-media-csi.c
> > > > +++ b/drivers/staging/media/imx/imx7-media-csi.c
> > > > @@ -503,11 +503,15 @@ static void imx7_csi_configure(struct imx7_csi *csi)
> > > >  		 * all of them comply. Support both variants.
> > > >  		 */
> > >
> > > You can drop this comment, as the two variants are not related to the
> > > CSI-2 bus format (which should always be UYVY8_1X16) but to the format
> > > output by the CSI-2 RX.
> > >
> > > I would add another comment to explain this clearly:
> > >
> > > 		/*
> > > 		 * The CSI bridge has a 16-bit input bus. Depending on
> > > 		 * the connected source, data may be transmitted with 8
> > > 		 * or 10 bits per clock sample (in bits [9:2] or [9:0]
> > > 		 * respectively) or with 16 bits per clock sample (in
> > > 		 * bits [15:0]). The data is then packed into a 32-bit
> > > 		 * FIFO (as shown in figure 13-11 of the i.MX8MM
> > > 		 * reference manual rev. 3).
> > > 		 *
> > > 		 * The data packing in a 32-bit FIFO input workd is
> >
> > s/workd/word
> >
> > > 		 * controlled by the CR3 TWO_8BIT_SENSOR field (also
> > > 		 * known as SENSOR_16BITS in the i.MX8MM reference
> > > 		 * manual). When set to 0, data packing groups four
> > > 		 * 8-bit input samples (bits [9:2]). When set to 1, data
> > > 		 * packing groups two 16-bit input samples (bits
> > > 		 * [15:0]).
> > > 		 *
> > > 		 * The register field CR18 MIPI_DOUBLE_CMPNT also needs
> > > 		 * to be configured according to the input format for
> > > 		 * YUV 4:2:2 data. The field controls the gasket between
> > > 		 * the CSI-2 receiver and the CSI bridge. On i.MX7 and
> > > 		 * i.MX8MM, the field must be set to 1 when the CSIS
> > > 		 * outputs 16-bit samples. On i.MX8MQ, the gasket
> > > 		 * ignores the MIPI_DOUBLE_CMPNT bit and YUV 4:2:2
> > > 		 * always uses 16-bit samples. Setting MIPI_DOUBLE_CMPNT
> > > 		 * in that case has no effect, but doesn't cause any
> > > 		 * issue.
> > > 		 */
> > >
> > > With this, someone trying to figure out what those bits do will
> > > hopefully be able to get it right :-)
> >
> > Thanks, very clear. I didn't get the last part about the MQ. I thought
> > the NW csi-2 receiver worked with 8-bit samples that's why we need to
> > maintain the CSI bridge compatible with the 2X8 format variant (this
> > and also for imx7 + parallel).
> >
> > Actually, looking at the MQ reference manual
> >
> > "15.2.1.3.5 CSI-2 Controller Core Configurations"  reports:
> > The CSI-2 RX Controller Core User Interface supports either a single,
> > double, or quad pixel wide data path. The double and quad wide pixel
> > modes can help reduce the frequency the user interface must run at
> > while the single pixel wide mode can be easier to interface to for
> > some applications.
> >
> > This chip supports the following:
> > • Single Pixel Configuration
> >
> > It seems the csi-2 receiver on the MQ works in single pixel mode only.
> > It should then expose the 2X8 format variant only, which is
> > technically incorrect for a serial transmitter. Cul de sac ?
>
> It's a different CSI-2 receiver, and uses a different terminology.
> "Single pixel mode" there refers to one pixel per clock cycle, which
> means, for YUV422 8-bit, 16 bits per cycle. The MIPI_DOUBLE_CMPNT bit is
> ignored by the gasket on i.MX8MQ.

   -Lovely-

I'll send patch to drop 2X8 variant from the mq csi-2 receiver.

Thanks
  j

>
> > > With these changes,
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Thanks
> >    j
> >
> > > >  		case MEDIA_BUS_FMT_UYVY8_2X8:
> > > > -		case MEDIA_BUS_FMT_UYVY8_1X16:
> > > >  		case MEDIA_BUS_FMT_YUYV8_2X8:
> > > > -		case MEDIA_BUS_FMT_YUYV8_1X16:
> > > >  			cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B;
> > > >  			break;
> > > > +		case MEDIA_BUS_FMT_UYVY8_1X16:
> > > > +		case MEDIA_BUS_FMT_YUYV8_1X16:
> > > > +			cr3 |= BIT_TWO_8BIT_SENSOR;
> > > > +			cr18 |= BIT_MIPI_DATA_FORMAT_YUV422_8B |
> > > > +				BIT_MIPI_DOUBLE_CMPNT;
> > > > +			break;
> > > >  		}
> > > >  	}
> > > >
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH v2 0/7] media: imx: Destage imx7-mipi-csis

[Adam Ford]

On Mon, Feb 21, 2022 at 2:24 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Jacopo,
>
> On Mon, Feb 21, 2022 at 08:58:59AM +0100, Jacopo Mondi wrote:
> > On Mon, Feb 21, 2022 at 12:41:45AM +0200, Laurent Pinchart wrote:
> > > On Sun, Feb 20, 2022 at 12:19:30PM -0600, Adam Ford wrote:
> > > > On Sun, Feb 20, 2022 at 8:56 AM Jacopo Mondi wrote:
> > > > >
> > > > > Hello
> > > > >   this series includes patches from two series previously sent:
> > > > > https://lore.kernel.org/linux-media/20220119112024.11339-1-jacopo@jmondi.org/
> > > > > https://lore.kernel.org/linux-media/20220211180216.290133-1-jacopo@jmondi.org/
> > > > > v1:
> > > > > https://lore.kernel.org/linux-media/20220214184318.409208-1-jacopo@jmondi.org/T/#t
> > > > >
> > > > > Which can now be marked as superseded.
> > > > >
> > > > > The first 2 patches performs the de-staging of the imx7-mipi-csis driver and
> > > > > are now reviewed.
> > > > >
> > > > > The rest of the series builds on top of the comment received on:
> > > > > https://lore.kernel.org/linux-media/20220119112024.11339-3-jacopo@jmondi.org/
> > > > >
> > > > > If DUAL pixel mode is used in the CSIS driver, then the CSI block of the IMX8MM
> > > > > SoC needs to be operated in dual mode as well. To do so, use the image format
> > > > > sample size to determine in the CSI bridge if dual or single mode should be
> > > > > used.
> > > > >
> > > > > Laurent could you test on MM to see if it works now ?
> > > >
> > > > Jacopo,
> > > >
> > > > Do you have a repo I can clone?  If not, I need to know which branch
> > > > to apply to the series. I have an 8MM with an OV5640, and I'm willing
> > > > to test if Laurent can't.
> > >
> > > I've applied the patches on top of v5.17-rc4 plus a few backports, and
> > > pushed the result to
> > > https://gitlab.com/ideasonboard/nxp/linux/-/tree/pinchartl/v5.17/csis.
> > >
> >
> > Oh, you've been slightly quicker then me :p
> >
> > I was about to ask Adam if he was interested in a branch which also
> > contains
> > https://patchwork.linuxtv.org/project/linux-media/list/?series=7311
> >
> > As he has an ov5640 :)
>
> Do you mean
> https://gitlab.com/ideasonboard/nxp/linux/-/tree/pinchartl/v5.17/sensors/ov5640/v2
> ? :-)
>
> > Adam, if you get here faster than me, please try Laurent's branch and
> > let me know. Otherwise I will provide a branch with a v3 of this
> > series and the ov5640 changes as well, if you're interested in testing
> > them.

I should be able to get to this today.  Are there specific resolutions
or formats you want me to test?  I assume I should test the new ones,
but I wasn't if there were some that were less confident.

adam
> >
> > Thanks
> >   j
> >
> > > > > On top two small patches I was carrying in my tree to add more formats to the
> > > > > CSIS driver, the last one with the caveat that RGB24 is transmitted on the wire
> > > > > with one format and stored in memory with a different one.
> > > > >
> > > > > Series based on top of the most recent media master branch.
> > > > >
> > > > > Thanks
> > > > >   j
> > > > >
> > > > > v1->v2:
> > > > > - Remove per-SoC handling in CSI bridge and only use image formats
> > > > > - Add TODO note to the staging driver
> > > > > - Fix PIXEL_DUAL mode comments for imx-mipi-csis
> > > > > - Add output format translation to imx-mipi-csis to handle RGB24
> > > > >
> > > > > Jacopo Mondi (7):
> > > > >   media: imx: De-stage imx7-mipi-csis
> > > > >   media: imx: Rename imx7-mipi-csis.c to imx-mipi-csis.c
> > > > >   media: imx: imx7-media-csi: Use dual sampling for YUV 1X16
> > > > >   media: imx: imx-mipi-csis: Set PIXEL_MODE for YUV422
> > > > >   media: imx: imx-mipi-csis: Add RGB565_1X16
> > > > >   media: imx: imx-mipi-csis: Add BGR888
> > > > >   media: imx: imx-mipi-csis: Add output format
> > > > >
> > > > >  Documentation/admin-guide/media/imx7.rst      |  2 +-
> > > > >  ...-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} |  2 +-
> > > > >  MAINTAINERS                                   |  4 +-
> > > > >  drivers/media/platform/Kconfig                |  1 +
> > > > >  drivers/media/platform/Makefile               |  1 +
> > > > >  drivers/media/platform/imx/Kconfig            | 24 ++++++++
> > > > >  drivers/media/platform/imx/Makefile           |  1 +
> > > > >  .../platform/imx/imx-mipi-csis.c}             | 59 +++++++++++++++++--
> > > > >  drivers/staging/media/imx/Makefile            |  1 -
> > > > >  drivers/staging/media/imx/TODO                | 26 ++++++++
> > > > >  drivers/staging/media/imx/imx7-media-csi.c    |  8 ++-
> > > > >  11 files changed, 117 insertions(+), 12 deletions(-)
> > > > >  rename Documentation/devicetree/bindings/media/{nxp,imx7-mipi-csi2.yaml => nxp,imx-mipi-csi2.yaml} (98%)
> > > > >  create mode 100644 drivers/media/platform/imx/Kconfig
> > > > >  create mode 100644 drivers/media/platform/imx/Makefile
> > > > >  rename drivers/{staging/media/imx/imx7-mipi-csis.c => media/platform/imx/imx-mipi-csis.c} (95%)
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH v2 1/7] media: imx: De-stage imx7-mipi-csis

[Adam Ford]

On Mon, Feb 21, 2022 at 3:16 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> The imx7-mipi-csis driver is in a good state and can be destaged.
>
> Move the imx7-mipi-csis.c driver to the newly created
> drivers/media/platform/imx directory and plumb the related
> options in Kconfig and in Makefile.
>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  MAINTAINERS                                   |  2 +-
>  drivers/media/platform/Kconfig                |  1 +
>  drivers/media/platform/Makefile               |  1 +
>  drivers/media/platform/imx/Kconfig            | 24 +++++++++++++++++++
>  drivers/media/platform/imx/Makefile           |  1 +
>  .../platform}/imx/imx7-mipi-csis.c            |  0
>  drivers/staging/media/imx/Makefile            |  1 -
>  7 files changed, 28 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/media/platform/imx/Kconfig
>  create mode 100644 drivers/media/platform/imx/Makefile
>  rename drivers/{staging/media => media/platform}/imx/imx7-mipi-csis.c (100%)
>

Jacopo,

Will there be a subsequent patch to modify the arch64 defconfig to
enable these modules by default?  With these changes, neither the CSI
nor CSIS appear to be enabled as modules any more, and I thought they
used to be enabled as modules by default.

adam
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 83d27b57016f..5bdb8c881b0b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11891,8 +11891,8 @@ T:      git git://linuxtv.org/media_tree.git
>  F:     Documentation/admin-guide/media/imx7.rst
>  F:     Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
>  F:     Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> +F:     drivers/media/platform/imx/imx7-mipi-csis.c
>  F:     drivers/staging/media/imx/imx7-media-csi.c
> -F:     drivers/staging/media/imx/imx7-mipi-csis.c
>
>  MEDIA DRIVERS FOR HELENE
>  M:     Abylay Ospan <aospan@netup.ru>
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 9fbdba0fd1e7..d9eeccffea69 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -171,6 +171,7 @@ source "drivers/media/platform/xilinx/Kconfig"
>  source "drivers/media/platform/rcar-vin/Kconfig"
>  source "drivers/media/platform/atmel/Kconfig"
>  source "drivers/media/platform/sunxi/Kconfig"
> +source "drivers/media/platform/imx/Kconfig"
>
>  config VIDEO_TI_CAL
>         tristate "TI CAL (Camera Adaptation Layer) driver"
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 28eb4aadbf45..a9466c854610 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -20,6 +20,7 @@ obj-y += ti-vpe/
>  obj-$(CONFIG_VIDEO_MX2_EMMAPRP)                += mx2_emmaprp.o
>  obj-$(CONFIG_VIDEO_CODA)               += coda/
>
> +obj-$(CONFIG_VIDEO_IMX)                        += imx/
>  obj-$(CONFIG_VIDEO_IMX_PXP)            += imx-pxp.o
>  obj-$(CONFIG_VIDEO_IMX8_JPEG)          += imx-jpeg/
>
> diff --git a/drivers/media/platform/imx/Kconfig b/drivers/media/platform/imx/Kconfig
> new file mode 100644
> index 000000000000..683863572c20
> --- /dev/null
> +++ b/drivers/media/platform/imx/Kconfig
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +menuconfig VIDEO_IMX
> +       bool "V4L2 capture drivers for NXP i.MX devices"
> +       depends on ARCH_MXC || COMPILE_TEST
> +       depends on VIDEO_DEV && VIDEO_V4L2
> +       help
> +         Say yes here to enable support for capture drivers on i.MX SoCs.
> +         Support for the single SoC features are selectable in the sub-menu
> +         options.
> +
> +if VIDEO_IMX
> +
> +config VIDEO_IMX_MIPI_CSIS
> +       tristate "MIPI CSI-2 CSIS receiver found on i.MX7 and i.MX8 models"
> +       select MEDIA_CONTROLLER
> +       select V4L2_FWNODE
> +       select VIDEO_V4L2_SUBDEV_API
> +       default n
> +       help
> +         Video4Linux2 sub-device driver for the MIPI CSI-2 CSIS receiver
> +         v3.3/v3.6.3 found on some i.MX7 and i.MX8 SoCs.
> +
> +endif # VIDEO_IMX
> diff --git a/drivers/media/platform/imx/Makefile b/drivers/media/platform/imx/Makefile
> new file mode 100644
> index 000000000000..ee272234c8d7
> --- /dev/null
> +++ b/drivers/media/platform/imx/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx7-mipi-csis.o
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/media/platform/imx/imx7-mipi-csis.c
> similarity index 100%
> rename from drivers/staging/media/imx/imx7-mipi-csis.c
> rename to drivers/media/platform/imx/imx7-mipi-csis.c
> diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
> index 19c2fc54d424..d82be898145b 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -15,5 +15,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-media-csi.o
>  obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
>
>  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
> -obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o
>  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx8mq-mipi-csi2.o
> --
> 2.35.0
>

Re: [PATCH v2 1/7] media: imx: De-stage imx7-mipi-csis

[Jacopo Mondi]

Hi Adam,

On Thu, Feb 24, 2022 at 10:49:13AM -0600, Adam Ford wrote:
> On Mon, Feb 21, 2022 at 3:16 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
> >
> > The imx7-mipi-csis driver is in a good state and can be destaged.
> >
> > Move the imx7-mipi-csis.c driver to the newly created
> > drivers/media/platform/imx directory and plumb the related
> > options in Kconfig and in Makefile.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  MAINTAINERS                                   |  2 +-
> >  drivers/media/platform/Kconfig                |  1 +
> >  drivers/media/platform/Makefile               |  1 +
> >  drivers/media/platform/imx/Kconfig            | 24 +++++++++++++++++++
> >  drivers/media/platform/imx/Makefile           |  1 +
> >  .../platform}/imx/imx7-mipi-csis.c            |  0
> >  drivers/staging/media/imx/Makefile            |  1 -
> >  7 files changed, 28 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/media/platform/imx/Kconfig
> >  create mode 100644 drivers/media/platform/imx/Makefile
> >  rename drivers/{staging/media => media/platform}/imx/imx7-mipi-csis.c (100%)
> >
>
> Jacopo,
>
> Will there be a subsequent patch to modify the arch64 defconfig to
> enable these modules by default?  With these changes, neither the CSI
> nor CSIS appear to be enabled as modules any more, and I thought they
> used to be enabled as modules by default.

I don't see the VIDEO_IMX7_CSI selected in v5.17-rc5 arch/arm64/configs/defconfig
What am I missing ? :)

I'm not aware of what are the policies to add a new symbol to the
arch64 defconfig, but if appropriate I would be happy to send a patch
to enable it.

Thanks
  j


>
> adam
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 83d27b57016f..5bdb8c881b0b 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11891,8 +11891,8 @@ T:      git git://linuxtv.org/media_tree.git
> >  F:     Documentation/admin-guide/media/imx7.rst
> >  F:     Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> >  F:     Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
> > +F:     drivers/media/platform/imx/imx7-mipi-csis.c
> >  F:     drivers/staging/media/imx/imx7-media-csi.c
> > -F:     drivers/staging/media/imx/imx7-mipi-csis.c
> >
> >  MEDIA DRIVERS FOR HELENE
> >  M:     Abylay Ospan <aospan@netup.ru>
> > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> > index 9fbdba0fd1e7..d9eeccffea69 100644
> > --- a/drivers/media/platform/Kconfig
> > +++ b/drivers/media/platform/Kconfig
> > @@ -171,6 +171,7 @@ source "drivers/media/platform/xilinx/Kconfig"
> >  source "drivers/media/platform/rcar-vin/Kconfig"
> >  source "drivers/media/platform/atmel/Kconfig"
> >  source "drivers/media/platform/sunxi/Kconfig"
> > +source "drivers/media/platform/imx/Kconfig"
> >
> >  config VIDEO_TI_CAL
> >         tristate "TI CAL (Camera Adaptation Layer) driver"
> > diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> > index 28eb4aadbf45..a9466c854610 100644
> > --- a/drivers/media/platform/Makefile
> > +++ b/drivers/media/platform/Makefile
> > @@ -20,6 +20,7 @@ obj-y += ti-vpe/
> >  obj-$(CONFIG_VIDEO_MX2_EMMAPRP)                += mx2_emmaprp.o
> >  obj-$(CONFIG_VIDEO_CODA)               += coda/
> >
> > +obj-$(CONFIG_VIDEO_IMX)                        += imx/
> >  obj-$(CONFIG_VIDEO_IMX_PXP)            += imx-pxp.o
> >  obj-$(CONFIG_VIDEO_IMX8_JPEG)          += imx-jpeg/
> >
> > diff --git a/drivers/media/platform/imx/Kconfig b/drivers/media/platform/imx/Kconfig
> > new file mode 100644
> > index 000000000000..683863572c20
> > --- /dev/null
> > +++ b/drivers/media/platform/imx/Kconfig
> > @@ -0,0 +1,24 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +
> > +menuconfig VIDEO_IMX
> > +       bool "V4L2 capture drivers for NXP i.MX devices"
> > +       depends on ARCH_MXC || COMPILE_TEST
> > +       depends on VIDEO_DEV && VIDEO_V4L2
> > +       help
> > +         Say yes here to enable support for capture drivers on i.MX SoCs.
> > +         Support for the single SoC features are selectable in the sub-menu
> > +         options.
> > +
> > +if VIDEO_IMX
> > +
> > +config VIDEO_IMX_MIPI_CSIS
> > +       tristate "MIPI CSI-2 CSIS receiver found on i.MX7 and i.MX8 models"
> > +       select MEDIA_CONTROLLER
> > +       select V4L2_FWNODE
> > +       select VIDEO_V4L2_SUBDEV_API
> > +       default n
> > +       help
> > +         Video4Linux2 sub-device driver for the MIPI CSI-2 CSIS receiver
> > +         v3.3/v3.6.3 found on some i.MX7 and i.MX8 SoCs.
> > +
> > +endif # VIDEO_IMX
> > diff --git a/drivers/media/platform/imx/Makefile b/drivers/media/platform/imx/Makefile
> > new file mode 100644
> > index 000000000000..ee272234c8d7
> > --- /dev/null
> > +++ b/drivers/media/platform/imx/Makefile
> > @@ -0,0 +1 @@
> > +obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx7-mipi-csis.o
> > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/media/platform/imx/imx7-mipi-csis.c
> > similarity index 100%
> > rename from drivers/staging/media/imx/imx7-mipi-csis.c
> > rename to drivers/media/platform/imx/imx7-mipi-csis.c
> > diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
> > index 19c2fc54d424..d82be898145b 100644
> > --- a/drivers/staging/media/imx/Makefile
> > +++ b/drivers/staging/media/imx/Makefile
> > @@ -15,5 +15,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-media-csi.o
> >  obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
> >
> >  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
> > -obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o
> >  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx8mq-mipi-csi2.o
> > --
> > 2.35.0
> >