[PATCH v2 0/9] imx7/imx8mm media / csi patches

[PATCH v2 0/9] imx7/imx8mm media / csi patches

[Alexander Stein]

Hey everyone,

thanks for the feedback in v1.

Changes in v2:
* Switch back from bool to enum for distinguishing imx5/6 and imx7/8
* Added missing colorspace settings in PATCH 5
* Added PATCH 8 for hardening media-imx code

Below is theoriginal message slightly modified to match the v2 changes.

This is a set of patch for imx7 media drivers based on next-20220210. With
this set I was able to capture video frames from a MIPI CSI-2 camera in my
TQMa8MQML + MBA8MX hardware. The actual camera used is a Vision Components
'VC MIPI IMX327 C' camera. IMX327 is compatible to IMX290. Patch 9 shows the
DT overlay I'm using, not suitable for merging.
Please ignore the FPGA part, this is mainly for power supply and GPIO reset
line. This is currently a custom driver I'm working on, but I do not want to
focus on in this series.

Please note I tested this only on this imx8 platform.

First thanks to Dorota for the patchset at [1] (patches 1-4) which is necessary
to capture correct images.

Starting from patch 5 there are small fixes which allows me to configure my
media device.

Device configuration:
```
media-ctl -l "'imx290 2-001a':0->'csis-32e30000.mipi-csi':0 [1]"
media-ctl -V "'imx290 2-001a':0 [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw]"
media-ctl -V "'csi':0 [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw]"
v4l2-ctl -d0 --set-fmt-video width=1920,height=1080,pixelformat='RG10',field=none
media-ctl -p
```

The media-ctl topology is:
```
# media-ctl -p
Media controller API version 5.17.0

Media device information
------------------------
driver          imx7-csi
model           imx-media
serial          
bus info        platform:32e20000.csi
hw revision     0x0
driver version  5.17.0

Device topology
- entity 1: csi (2 pads, 2 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev0
        pad0: Sink
                [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range]
                <- "csis-32e30000.mipi-csi":1 [ENABLED,IMMUTABLE]
        pad1: Source
                [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range]
                -> "csi capture":0 [ENABLED,IMMUTABLE]

- entity 4: csi capture (1 pad, 1 link)
            type Node subtype V4L flags 0
            device node name /dev/video0
        pad0: Sink
                <- "csi":1 [ENABLED,IMMUTABLE]

- entity 10: csis-32e30000.mipi-csi (2 pads, 2 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev1
        pad0: Sink
                [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw xfer:709 ycbcr:601 quantization:lim-range]
                <- "imx290 2-001a":0 [ENABLED]
        pad1: Source
                [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw xfer:709 ycbcr:601 quantization:lim-range]
                -> "csi":0 [ENABLED,IMMUTABLE]

- entity 15: imx290 2-001a (1 pad, 1 link)
             type V4L2 subdev subtype Sensor flags 0
             device node name /dev/v4l-subdev2
        pad0: Source
                [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw]
                -> "csis-32e30000.mipi-csi":0 [ENABLED]
```

Note: MIPI CSI settle times are not calculated correctly right now and need a
manual overwrite:
echo 13 > /sys/kernel/debug/32e30000.mipi-csi/ths_settle 
echo 2 > /sys/kernel/debug/32e30000.mipi-csi/tclk_settle

I ignored the settings for xfer, ycbcr and quantization for now. I do neither
know how they will affect me nor what it should be.
Also I did not focus on v4l2-compliance tool, this is a further task as well.
IMHO imx7-mipi-csis.c should also create an immutable link to the camera
sensor.

Regards,
Alexander

[1] https://patchwork.linuxtv.org/project/linux-media/patch/20211104113631.206899-2-dorota.czaplejewicz@puri.sm/

Alexander Stein (5):
  media: imx: imx7_mipi_csis: store colorspace in set_fmt as well
  media: imx: imx7_media-csi: Add support for additional Bayer patterns
  media: imx: utils: Add more Bayer formats
  media: imx: utils: initialize local variable
  [DNI] arm64: dts: tqma8mqml: add IMX327 MIPI-CSI overlay

Dorota Czaplejewicz (4):
  media: imx: Store the type of hardware implementation
  media: imx: Forward type of hardware implementation
  media: imx: Use dedicated format handler for i.MX7/8
  media: imx: Fail conversion if pixel format not supported

 arch/arm64/boot/dts/freescale/Makefile        |   4 +
 .../imx8mm-tqma8mqml-mba8mx-imx327.dts        |  95 ++++++++++++
 drivers/staging/media/imx/imx-ic-prpencvf.c   |   3 +-
 drivers/staging/media/imx/imx-media-capture.c |  20 ++-
 drivers/staging/media/imx/imx-media-csi.c     |   3 +-
 drivers/staging/media/imx/imx-media-utils.c   | 136 +++++++++++++++++-
 drivers/staging/media/imx/imx-media.h         |  11 +-
 drivers/staging/media/imx/imx7-media-csi.c    |  15 +-
 drivers/staging/media/imx/imx7-mipi-csis.c    |   4 +
 9 files changed, 275 insertions(+), 16 deletions(-)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-tqma8mqml-mba8mx-imx327.dts

-- 
2.25.1

[PATCH v2 1/9] media: imx: Store the type of hardware implementation

[Alexander Stein]

From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>

The driver covers i.MX5/6, as well as i.MX7/8 hardware.
Those implementations differ, e.g. in the sizes of buffers they accept.

Some functionality should be abstracted, and storing type achieves that.

Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* Switch back to using enum

 drivers/staging/media/imx/imx-ic-prpencvf.c   | 3 ++-
 drivers/staging/media/imx/imx-media-capture.c | 5 ++++-
 drivers/staging/media/imx/imx-media-csi.c     | 3 ++-
 drivers/staging/media/imx/imx-media.h         | 8 +++++++-
 drivers/staging/media/imx/imx7-media-csi.c    | 3 ++-
 5 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 9b81cfbcd777..671bb9a681aa 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -1266,7 +1266,8 @@ static int prp_registered(struct v4l2_subdev *sd)
 
 	priv->vdev = imx_media_capture_device_init(ic_priv->ipu_dev,
 						   &ic_priv->sd,
-						   PRPENCVF_SRC_PAD, true);
+						   PRPENCVF_SRC_PAD, true,
+						   DEVICE_TYPE_IMX56);
 	if (IS_ERR(priv->vdev))
 		return PTR_ERR(priv->vdev);
 
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 93ba09236010..65dc95a48ecc 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -34,6 +34,7 @@ struct capture_priv {
 
 	struct imx_media_video_dev vdev;	/* Video device */
 	struct media_pad vdev_pad;		/* Video device pad */
+	enum imx_media_device_type type;	/* Type of hardware implementation */
 
 	struct v4l2_subdev *src_sd;		/* Source subdev */
 	int src_sd_pad;				/* Source subdev pad */
@@ -957,7 +958,8 @@ EXPORT_SYMBOL_GPL(imx_media_capture_device_unregister);
 
 struct imx_media_video_dev *
 imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
-			      int pad, bool legacy_api)
+			      int pad, bool legacy_api,
+			      enum imx_media_device_type type)
 {
 	struct capture_priv *priv;
 	struct video_device *vfd;
@@ -972,6 +974,7 @@ imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
 	priv->src_sd_pad = pad;
 	priv->dev = dev;
 	priv->legacy_api = legacy_api;
+	priv->type = type;
 
 	mutex_init(&priv->mutex);
 	INIT_LIST_HEAD(&priv->ready_q);
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index bd7f156f2d52..d5557bb4913d 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1803,7 +1803,8 @@ static int csi_registered(struct v4l2_subdev *sd)
 	}
 
 	priv->vdev = imx_media_capture_device_init(priv->sd.dev, &priv->sd,
-						   CSI_SRC_PAD_IDMAC, true);
+						   CSI_SRC_PAD_IDMAC, true,
+						   DEVICE_TYPE_IMX56);
 	if (IS_ERR(priv->vdev)) {
 		ret = PTR_ERR(priv->vdev);
 		goto free_fim;
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index f263fc3adbb9..e4c22b3ccd57 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -96,6 +96,11 @@ enum imx_pixfmt_sel {
 	PIXFMT_SEL_ANY = PIXFMT_SEL_YUV | PIXFMT_SEL_RGB | PIXFMT_SEL_BAYER,
 };
 
+enum imx_media_device_type {
+	DEVICE_TYPE_IMX56,
+	DEVICE_TYPE_IMX78,
+};
+
 struct imx_media_buffer {
 	struct vb2_v4l2_buffer vbuf; /* v4l buffer must be first */
 	struct list_head  list;
@@ -282,7 +287,8 @@ int imx_media_ic_unregister(struct v4l2_subdev *sd);
 /* imx-media-capture.c */
 struct imx_media_video_dev *
 imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
-			      int pad, bool legacy_api);
+			      int pad, bool legacy_api,
+			      enum imx_media_device_type type);
 void imx_media_capture_device_remove(struct imx_media_video_dev *vdev);
 int imx_media_capture_device_register(struct imx_media_video_dev *vdev,
 				      u32 link_flags);
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 32311fc0e2a4..173dd014c2d6 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -1039,7 +1039,8 @@ static int imx7_csi_registered(struct v4l2_subdev *sd)
 	}
 
 	csi->vdev = imx_media_capture_device_init(csi->sd.dev, &csi->sd,
-						  IMX7_CSI_PAD_SRC, false);
+						  IMX7_CSI_PAD_SRC, false,
+						  DEVICE_TYPE_IMX78);
 	if (IS_ERR(csi->vdev))
 		return PTR_ERR(csi->vdev);
 
-- 
2.25.1

[PATCH v2 2/9] media: imx: Forward type of hardware implementation

[Alexander Stein]

From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>

Pass down the hardware type so imx_media_mbus_fmt_to_pix_fmt can do
the actual switch.

Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* Switch back to using enum
* Added Reviewed-by: Laurent Pinchart

 drivers/staging/media/imx/imx-media-capture.c | 15 +++++++++------
 drivers/staging/media/imx/imx-media-utils.c   |  3 ++-
 drivers/staging/media/imx/imx-media.h         |  3 ++-
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 65dc95a48ecc..7a6384b3e5e6 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -139,7 +139,8 @@ static int capture_g_fmt_vid_cap(struct file *file, void *fh,
 }
 
 static const struct imx_media_pixfmt *
-__capture_try_fmt(struct v4l2_pix_format *pixfmt, struct v4l2_rect *compose)
+__capture_try_fmt(struct v4l2_pix_format *pixfmt, struct v4l2_rect *compose,
+		  enum imx_media_device_type type)
 {
 	struct v4l2_mbus_framefmt fmt_src;
 	const struct imx_media_pixfmt *cc;
@@ -171,7 +172,7 @@ __capture_try_fmt(struct v4l2_pix_format *pixfmt, struct v4l2_rect *compose)
 	}
 
 	v4l2_fill_mbus_format(&fmt_src, pixfmt, 0);
-	imx_media_mbus_fmt_to_pix_fmt(pixfmt, &fmt_src, cc);
+	imx_media_mbus_fmt_to_pix_fmt(pixfmt, &fmt_src, cc, type);
 
 	if (compose) {
 		compose->width = fmt_src.width;
@@ -184,7 +185,9 @@ __capture_try_fmt(struct v4l2_pix_format *pixfmt, struct v4l2_rect *compose)
 static int capture_try_fmt_vid_cap(struct file *file, void *fh,
 				   struct v4l2_format *f)
 {
-	__capture_try_fmt(&f->fmt.pix, NULL);
+	struct capture_priv *priv = video_drvdata(file);
+
+	__capture_try_fmt(&f->fmt.pix, NULL, priv->type);
 	return 0;
 }
 
@@ -199,7 +202,7 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
 		return -EBUSY;
 	}
 
-	cc = __capture_try_fmt(&f->fmt.pix, &priv->vdev.compose);
+	cc = __capture_try_fmt(&f->fmt.pix, &priv->vdev.compose, priv->type);
 
 	priv->vdev.cc = cc;
 	priv->vdev.fmt = f->fmt.pix;
@@ -418,7 +421,7 @@ __capture_legacy_try_fmt(struct capture_priv *priv,
 		}
 	}
 
-	imx_media_mbus_fmt_to_pix_fmt(pixfmt, &fmt_src->format, cc);
+	imx_media_mbus_fmt_to_pix_fmt(pixfmt, &fmt_src->format, cc, priv->type);
 
 	return cc;
 }
@@ -889,7 +892,7 @@ static int capture_init_format(struct capture_priv *priv)
 		fmt_src.format.height = IMX_MEDIA_DEF_PIX_HEIGHT;
 	}
 
-	imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt, &fmt_src.format, NULL);
+	imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt, &fmt_src.format, NULL, priv->type);
 	vdev->compose.width = fmt_src.format.width;
 	vdev->compose.height = fmt_src.format.height;
 
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 94bc866ca28c..c42f3da8e3a8 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -518,7 +518,8 @@ EXPORT_SYMBOL_GPL(imx_media_try_colorimetry);
 
 int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
 				  const struct v4l2_mbus_framefmt *mbus,
-				  const struct imx_media_pixfmt *cc)
+				  const struct imx_media_pixfmt *cc,
+				  enum imx_media_device_type type)
 {
 	u32 width;
 	u32 stride;
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index e4c22b3ccd57..f59feccb26bf 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -203,7 +203,8 @@ void imx_media_try_colorimetry(struct v4l2_mbus_framefmt *tryfmt,
 			       bool ic_route);
 int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
 				  const struct v4l2_mbus_framefmt *mbus,
-				  const struct imx_media_pixfmt *cc);
+				  const struct imx_media_pixfmt *cc,
+				  enum imx_media_device_type type);
 void imx_media_grp_id_to_sd_name(char *sd_name, int sz,
 				 u32 grp_id, int ipu_id);
 struct v4l2_subdev *
-- 
2.25.1

[PATCH v2 3/9] media: imx: Use dedicated format handler for i.MX7/8

[Alexander Stein]

From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>

This splits out a format handler which takes into account
the capabilities of the i.MX7/8 video device,
as opposed to the default handler compatible with both i.MX5/6 and i.MX7/8.

Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* Switch back to using enum
* Get rid of an additional call to v4l2_fill_pixfmt()

 drivers/staging/media/imx/imx-media-utils.c | 60 +++++++++++++++++++--
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index c42f3da8e3a8..02a4cb124d37 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -516,10 +516,9 @@ void imx_media_try_colorimetry(struct v4l2_mbus_framefmt *tryfmt,
 }
 EXPORT_SYMBOL_GPL(imx_media_try_colorimetry);
 
-int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
-				  const struct v4l2_mbus_framefmt *mbus,
-				  const struct imx_media_pixfmt *cc,
-				  enum imx_media_device_type type)
+static int imx56_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
+					   const struct v4l2_mbus_framefmt *mbus,
+					   const struct imx_media_pixfmt *cc)
 {
 	u32 width;
 	u32 stride;
@@ -568,6 +567,59 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
 
 	return 0;
 }
+
+static int imx78_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
+					   const struct v4l2_mbus_framefmt *mbus,
+					   const struct imx_media_pixfmt *cc)
+{
+	u32 aligned_width;
+	int ret;
+
+	if (!cc)
+		cc = imx_media_find_mbus_format(mbus->code, PIXFMT_SEL_ANY);
+
+	if (!cc)
+		return -EINVAL;
+	/*
+	 * The hardware can handle line lengths divisible by 4 pixels
+	 * as long as the whole buffer size ends up divisible by 8 bytes.
+	 * If not, use the value of 8 pixels recommended in the datasheet.
+	 * Only 8bits-per-pixel formats may need to get aligned to 8 pixels,
+	 * because both 10-bit and 16-bit pixels occupy 2 bytes.
+	 * In those, 4-pixel aligmnent is equal to 8-byte alignment.
+	 */
+	if (cc->bpp == 1)
+		aligned_width = round_up(mbus->width, 8);
+	else
+		aligned_width = round_up(mbus->width, 4);
+
+	ret = v4l2_fill_pixfmt(pix, cc->fourcc,
+			       aligned_width, mbus->height);
+	if (ret)
+		return ret;
+
+	pix->colorspace = mbus->colorspace;
+	pix->xfer_func = mbus->xfer_func;
+	pix->ycbcr_enc = mbus->ycbcr_enc;
+	pix->quantization = mbus->quantization;
+	pix->field = mbus->field;
+
+	return ret;
+}
+
+int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
+				  const struct v4l2_mbus_framefmt *mbus,
+				  const struct imx_media_pixfmt *cc,
+				  enum imx_media_device_type type)
+{
+	switch (type) {
+	case DEVICE_TYPE_IMX56:
+		return imx56_media_mbus_fmt_to_pix_fmt(pix, mbus, cc);
+	case DEVICE_TYPE_IMX78:
+		return imx78_media_mbus_fmt_to_pix_fmt(pix, mbus, cc);
+	}
+	return -EINVAL;
+}
 EXPORT_SYMBOL_GPL(imx_media_mbus_fmt_to_pix_fmt);
 
 void imx_media_free_dma_buf(struct device *dev,
-- 
2.25.1

[PATCH v2 4/9] media: imx: Fail conversion if pixel format not supported

[Alexander Stein]

From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>

imx_media_find_mbus_format has NULL as a valid return value,
therefore the caller should take it into account.

Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/staging/media/imx/imx-media-utils.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 02a4cb124d37..e59aaa77172a 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -544,6 +544,9 @@ static int imx56_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
 		cc = imx_media_find_mbus_format(code, PIXFMT_SEL_YUV);
 	}
 
+	if (!cc)
+		return -EINVAL;
+
 	/* Round up width for minimum burst size */
 	width = round_up(mbus->width, 8);
 
-- 
2.25.1

[PATCH v2 5/9] media: imx: imx7_mipi_csis: store colorspace in set_fmt as well

[Alexander Stein]

Without this the default (SMPTE 170M) from init_cfg stays unchanged.
Even after configuring 'srgb' colorspace (or 'raw')
$ media-ctl -V "'csis-32e30000.mipi-csi':0 [colorspace:srgb]"
the colorspace does not change at all:
$ media-ctl --get-v4l2 "'csis-32e30000.mipi-csi':0"
  [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:smpte170m xfer:709
   ycbcr:601 quantization:lim-range]

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* Store other colorspace-related fields as well

 drivers/staging/media/imx/imx7-mipi-csis.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index a22d0e6b3d44..388cfd012212 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -1062,6 +1062,10 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
 	fmt->code = csis_fmt->code;
 	fmt->width = sdformat->format.width;
 	fmt->height = sdformat->format.height;
+	fmt->colorspace = sdformat->format.colorspace;
+	fmt->quantization = sdformat->format.quantization;
+	fmt->xfer_func = sdformat->format.xfer_func;
+	fmt->ycbcr_enc = sdformat->format.ycbcr_enc;
 
 	sdformat->format = *fmt;
 
-- 
2.25.1

[PATCH v2 6/9] media: imx: imx7_media-csi: Add support for additional Bayer patterns

[Alexander Stein]

imx7_csi_configure() allows configuring these Bayer patterns when
starting a stream. So allow these in link_validate() as well.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* Added Reviewed-by: Laurent Pinchart

 drivers/staging/media/imx/imx7-media-csi.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 173dd014c2d6..9c4b72c99be9 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -1004,6 +1004,18 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd,
 	case V4L2_PIX_FMT_SGBRG8:
 	case V4L2_PIX_FMT_SGRBG8:
 	case V4L2_PIX_FMT_SRGGB8:
+	case V4L2_PIX_FMT_SBGGR10:
+	case V4L2_PIX_FMT_SGBRG10:
+	case V4L2_PIX_FMT_SGRBG10:
+	case V4L2_PIX_FMT_SRGGB10:
+	case V4L2_PIX_FMT_SBGGR12:
+	case V4L2_PIX_FMT_SGBRG12:
+	case V4L2_PIX_FMT_SGRBG12:
+	case V4L2_PIX_FMT_SRGGB12:
+	case V4L2_PIX_FMT_SBGGR14:
+	case V4L2_PIX_FMT_SGBRG14:
+	case V4L2_PIX_FMT_SGRBG14:
+	case V4L2_PIX_FMT_SRGGB14:
 	case V4L2_PIX_FMT_SBGGR16:
 	case V4L2_PIX_FMT_SGBRG16:
 	case V4L2_PIX_FMT_SGRBG16:
-- 
2.25.1

[PATCH v2 7/9] media: imx: utils: Add more Bayer formats

[Alexander Stein]

Without this the ioctl VIDIOC_ENUM_FMT will not list the 10/12/14-Bit
Bayer formats. This in return results in
"v4l2-ctl --set-fmt-video pixelformat='RG10'" failing to set the
pixelformat.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* Added Reviewed-by: Laurent Pinchart

 drivers/staging/media/imx/imx-media-utils.c | 72 +++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index e59aaa77172a..57eb1c5897c0 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -130,6 +130,78 @@ static const struct imx_media_pixfmt pixel_formats[] = {
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 8,
 		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SBGGR10,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SBGGR10_1X10),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 10,
+		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGBRG10,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SGBRG10_1X10),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 10,
+		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGRBG10,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SGRBG10_1X10),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 10,
+		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SRGGB10,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SRGGB10_1X10),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 10,
+		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SBGGR12,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SBGGR12_1X12),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 12,
+		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGBRG12,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SGBRG12_1X12),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 12,
+		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGRBG12,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SGRBG12_1X12),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 12,
+		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SRGGB12,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SRGGB12_1X12),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 12,
+		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SBGGR14,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SBGGR14_1X14),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 14,
+		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGBRG14,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SGBRG14_1X14),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 14,
+		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SGRBG14,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SGRBG14_1X14),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 14,
+		.bayer  = true,
+	}, {
+		.fourcc = V4L2_PIX_FMT_SRGGB14,
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SRGGB14_1X14),
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 14,
+		.bayer  = true,
 	}, {
 		.fourcc = V4L2_PIX_FMT_SBGGR16,
 		.codes  = IMX_BUS_FMTS(
-- 
2.25.1

[PATCH v2 8/9] media: imx: utils: initialize local variable

[Alexander Stein]

imx_media_enum_mbus_formats might not set code at all, this would result
in imx_media_find_mbus_format using an uninitialized variable.
Set code to 0 to avoid returning a wrong pixel format.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* New in v2

 drivers/staging/media/imx/imx-media-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 57eb1c5897c0..025c080c8c9f 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -610,7 +610,7 @@ static int imx56_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
 	 * so until it does convert to a supported YUV format.
 	 */
 	if (cc->ipufmt && cc->cs == IPUV3_COLORSPACE_YUV) {
-		u32 code;
+		u32 code = 0;
 
 		imx_media_enum_mbus_formats(&code, 0, PIXFMT_SEL_YUV);
 		cc = imx_media_find_mbus_format(code, PIXFMT_SEL_YUV);
-- 
2.25.1

[PATCH v2 9/9] [DNI] arm64: dts: tqma8mqml: add IMX327 MIPI-CSI overlay

[Alexander Stein]

---
 arch/arm64/boot/dts/freescale/Makefile        |  4 +
 .../imx8mm-tqma8mqml-mba8mx-imx327.dts        | 95 +++++++++++++++++++
 2 files changed, 99 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-tqma8mqml-mba8mx-imx327.dts

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 852615febf9a..5ef8ff05452e 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -57,6 +57,10 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-edimm2.2.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mm-kontron-n801x-s.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mm-nitrogen-r2.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mm-tqma8mqml-mba8mx.dtb
+
+tqma8mqml-mba8mx-imx327-dtbs += imx8mm-tqma8mqml-mba8mx.dtb imx8mm-tqma8mqml-mba8mx-imx327.dtbo
+dtb-$(CONFIG_ARCH_MXC) += tqma8mqml-mba8mx-imx327.dtb
+
 dtb-$(CONFIG_ARCH_MXC) += imx8mm-var-som-symphony.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw71xx-0x.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw72xx-0x.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-tqma8mqml-mba8mx-imx327.dts b/arch/arm64/boot/dts/freescale/imx8mm-tqma8mqml-mba8mx-imx327.dts
new file mode 100644
index 000000000000..3f1223d4d73b
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mm-tqma8mqml-mba8mx-imx327.dts
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR MIT)
+/*
+ * Copyright 2021 TQ-Systems GmbH
+ */
+
+#include <dt-bindings/gpio/gpio.h>
+
+/dts-v1/;
+/plugin/;
+
+&{/} {
+	compatible = "tq,imx8mm-tqma8mqml-mba8mx", "tq,imx8mm-tqma8mqml", "fsl,imx8mm";
+
+	sensor_clk: sensor-clk {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <37125000>;
+	};
+};
+
+&csi {
+       status = "okay";
+};
+
+&i2c3 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	vc_fpga: fpga@10 {
+		reg = <0x10>;
+		compatible = "vc,fpga";
+
+		vc_fpga_reg: regulator {
+			regulator-name = "CAM_VCC";
+		};
+
+		vc_fpga_reset: reset {
+			#reset-cells = <0>;
+		};
+
+		vc_fpga_gpio: gpio-chip {
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+	};
+
+	sony_imx327: camera@1a {
+		#address-cells = <0x1>;
+		#size-cells = <0x0>;
+		reg = <0x1a>;
+
+		compatible = "sony,imx327", "sony,imx290";
+
+		clock-names = "xclk";
+		clocks = <&sensor_clk>;
+		clock-frequency = <37125000>;
+
+		vdd-supply = <&vc_fpga_reg>;
+
+		vdda-supply = <&vc_fpga_reg>;
+		vddd-supply = <&vc_fpga_reg>;
+		vdddo-supply = <&vc_fpga_reg>;
+
+		reset-gpios = <&vc_fpga_gpio 0 GPIO_ACTIVE_HIGH>;
+
+		port@0 {
+			reg = <0>;
+
+			sony_imx327_ep0: endpoint {
+				remote-endpoint = <&imx8mm_mipi_csi_in>;
+				data-lanes = <1 2>;
+				clock-lanes = <0>;
+				clock-noncontinuous = <1>;
+				link-frequencies = /bits/ 64 <445500000 297000000>;
+			};
+		};
+	};
+};
+
+&mipi_csi {
+       status = "okay";
+
+       ports {
+               #address-cells = <1>;
+               #size-cells = <0>;
+
+               port@0 {
+                       reg = <0>;
+                       imx8mm_mipi_csi_in: endpoint {
+                               remote-endpoint = <&sony_imx327_ep0>;
+                               data-lanes = <1 2>;
+                       };
+               };
+       };
+};
\ No newline at end of file
-- 
2.25.1

Re: [PATCH v2 1/9] media: imx: Store the type of hardware implementation

[Jacopo Mondi]

Hi Alexander, Dorota,

On Fri, Feb 11, 2022 at 03:27:44PM +0100, Alexander Stein wrote:
> From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
>
> The driver covers i.MX5/6, as well as i.MX7/8 hardware.
> Those implementations differ, e.g. in the sizes of buffers they accept.
>
> Some functionality should be abstracted, and storing type achieves that.
>
> Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Changes in v2:
> * Switch back to using enum
>
>  drivers/staging/media/imx/imx-ic-prpencvf.c   | 3 ++-
>  drivers/staging/media/imx/imx-media-capture.c | 5 ++++-
>  drivers/staging/media/imx/imx-media-csi.c     | 3 ++-
>  drivers/staging/media/imx/imx-media.h         | 8 +++++++-
>  drivers/staging/media/imx/imx7-media-csi.c    | 3 ++-
>  5 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index 9b81cfbcd777..671bb9a681aa 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -1266,7 +1266,8 @@ static int prp_registered(struct v4l2_subdev *sd)
>
>  	priv->vdev = imx_media_capture_device_init(ic_priv->ipu_dev,
>  						   &ic_priv->sd,
> -						   PRPENCVF_SRC_PAD, true);
> +						   PRPENCVF_SRC_PAD, true,
> +						   DEVICE_TYPE_IMX56);
>  	if (IS_ERR(priv->vdev))
>  		return PTR_ERR(priv->vdev);
>
> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
> index 93ba09236010..65dc95a48ecc 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -34,6 +34,7 @@ struct capture_priv {
>
>  	struct imx_media_video_dev vdev;	/* Video device */
>  	struct media_pad vdev_pad;		/* Video device pad */
> +	enum imx_media_device_type type;	/* Type of hardware implementation */
>
>  	struct v4l2_subdev *src_sd;		/* Source subdev */
>  	int src_sd_pad;				/* Source subdev pad */
> @@ -957,7 +958,8 @@ EXPORT_SYMBOL_GPL(imx_media_capture_device_unregister);
>
>  struct imx_media_video_dev *
>  imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
> -			      int pad, bool legacy_api)
> +			      int pad, bool legacy_api,
> +			      enum imx_media_device_type type)
>  {
>  	struct capture_priv *priv;
>  	struct video_device *vfd;
> @@ -972,6 +974,7 @@ imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
>  	priv->src_sd_pad = pad;
>  	priv->dev = dev;
>  	priv->legacy_api = legacy_api;
> +	priv->type = type;
>
>  	mutex_init(&priv->mutex);
>  	INIT_LIST_HEAD(&priv->ready_q);
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index bd7f156f2d52..d5557bb4913d 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -1803,7 +1803,8 @@ static int csi_registered(struct v4l2_subdev *sd)
>  	}
>
>  	priv->vdev = imx_media_capture_device_init(priv->sd.dev, &priv->sd,
> -						   CSI_SRC_PAD_IDMAC, true);
> +						   CSI_SRC_PAD_IDMAC, true,
> +						   DEVICE_TYPE_IMX56);
>  	if (IS_ERR(priv->vdev)) {
>  		ret = PTR_ERR(priv->vdev);
>  		goto free_fim;
> diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
> index f263fc3adbb9..e4c22b3ccd57 100644
> --- a/drivers/staging/media/imx/imx-media.h
> +++ b/drivers/staging/media/imx/imx-media.h
> @@ -96,6 +96,11 @@ enum imx_pixfmt_sel {
>  	PIXFMT_SEL_ANY = PIXFMT_SEL_YUV | PIXFMT_SEL_RGB | PIXFMT_SEL_BAYER,
>  };
>
> +enum imx_media_device_type {
> +	DEVICE_TYPE_IMX56,
> +	DEVICE_TYPE_IMX78,
> +};
> +

Isn't this too coarse as a distinction ?

I tried adding per-soc identifiers here:
https://lore.kernel.org/linux-media/20220214184318.409208-5-jacopo@jmondi.org/T/#u

Maybe they can help ?

>  struct imx_media_buffer {
>  	struct vb2_v4l2_buffer vbuf; /* v4l buffer must be first */
>  	struct list_head  list;
> @@ -282,7 +287,8 @@ int imx_media_ic_unregister(struct v4l2_subdev *sd);
>  /* imx-media-capture.c */
>  struct imx_media_video_dev *
>  imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
> -			      int pad, bool legacy_api);
> +			      int pad, bool legacy_api,
> +			      enum imx_media_device_type type);
>  void imx_media_capture_device_remove(struct imx_media_video_dev *vdev);
>  int imx_media_capture_device_register(struct imx_media_video_dev *vdev,
>  				      u32 link_flags);
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index 32311fc0e2a4..173dd014c2d6 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -1039,7 +1039,8 @@ static int imx7_csi_registered(struct v4l2_subdev *sd)
>  	}
>
>  	csi->vdev = imx_media_capture_device_init(csi->sd.dev, &csi->sd,
> -						  IMX7_CSI_PAD_SRC, false);
> +						  IMX7_CSI_PAD_SRC, false,
> +						  DEVICE_TYPE_IMX78);
>  	if (IS_ERR(csi->vdev))
>  		return PTR_ERR(csi->vdev);
>
> --
> 2.25.1
>

Re: [PATCH v2 3/9] media: imx: Use dedicated format handler for i.MX7/8

[Jacopo Mondi]

Hello

On Fri, Feb 11, 2022 at 03:27:46PM +0100, Alexander Stein wrote:
> From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
>
> This splits out a format handler which takes into account
> the capabilities of the i.MX7/8 video device,
> as opposed to the default handler compatible with both i.MX5/6 and i.MX7/8.
>
> Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Changes in v2:
> * Switch back to using enum
> * Get rid of an additional call to v4l2_fill_pixfmt()
>
>  drivers/staging/media/imx/imx-media-utils.c | 60 +++++++++++++++++++--
>  1 file changed, 56 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> index c42f3da8e3a8..02a4cb124d37 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -516,10 +516,9 @@ void imx_media_try_colorimetry(struct v4l2_mbus_framefmt *tryfmt,
>  }
>  EXPORT_SYMBOL_GPL(imx_media_try_colorimetry);
>
> -int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> -				  const struct v4l2_mbus_framefmt *mbus,
> -				  const struct imx_media_pixfmt *cc,
> -				  enum imx_media_device_type type)
> +static int imx56_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> +					   const struct v4l2_mbus_framefmt *mbus,
> +					   const struct imx_media_pixfmt *cc)
>  {
>  	u32 width;
>  	u32 stride;
> @@ -568,6 +567,59 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
>
>  	return 0;
>  }
> +
> +static int imx78_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> +					   const struct v4l2_mbus_framefmt *mbus,
> +					   const struct imx_media_pixfmt *cc)
> +{
> +	u32 aligned_width;
> +	int ret;
> +
> +	if (!cc)
> +		cc = imx_media_find_mbus_format(mbus->code, PIXFMT_SEL_ANY);
> +
> +	if (!cc)
> +		return -EINVAL;
> +	/*
> +	 * The hardware can handle line lengths divisible by 4 pixels
> +	 * as long as the whole buffer size ends up divisible by 8 bytes.
> +	 * If not, use the value of 8 pixels recommended in the datasheet.
> +	 * Only 8bits-per-pixel formats may need to get aligned to 8 pixels,
> +	 * because both 10-bit and 16-bit pixels occupy 2 bytes.
> +	 * In those, 4-pixel aligmnent is equal to 8-byte alignment.
> +	 */
> +	if (cc->bpp == 1)

Am I mistaken or in the format enumeration bpp is expressed in bits
and not bytes ?

Thanks
  j

> +		aligned_width = round_up(mbus->width, 8);
> +	else
> +		aligned_width = round_up(mbus->width, 4);
> +
> +	ret = v4l2_fill_pixfmt(pix, cc->fourcc,
> +			       aligned_width, mbus->height);
> +	if (ret)
> +		return ret;
> +
> +	pix->colorspace = mbus->colorspace;
> +	pix->xfer_func = mbus->xfer_func;
> +	pix->ycbcr_enc = mbus->ycbcr_enc;
> +	pix->quantization = mbus->quantization;
> +	pix->field = mbus->field;
> +
> +	return ret;
> +}
> +
> +int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> +				  const struct v4l2_mbus_framefmt *mbus,
> +				  const struct imx_media_pixfmt *cc,
> +				  enum imx_media_device_type type)
> +{
> +	switch (type) {
> +	case DEVICE_TYPE_IMX56:
> +		return imx56_media_mbus_fmt_to_pix_fmt(pix, mbus, cc);
> +	case DEVICE_TYPE_IMX78:
> +		return imx78_media_mbus_fmt_to_pix_fmt(pix, mbus, cc);
> +	}
> +	return -EINVAL;
> +}
>  EXPORT_SYMBOL_GPL(imx_media_mbus_fmt_to_pix_fmt);
>
>  void imx_media_free_dma_buf(struct device *dev,
> --
> 2.25.1
>

Re: [PATCH v2 1/9] media: imx: Store the type of hardware implementation

[Laurent Pinchart]

Hi Jacopo,

On Mon, Feb 14, 2022 at 07:50:35PM +0100, Jacopo Mondi wrote:
> On Fri, Feb 11, 2022 at 03:27:44PM +0100, Alexander Stein wrote:
> > From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> >
> > The driver covers i.MX5/6, as well as i.MX7/8 hardware.
> > Those implementations differ, e.g. in the sizes of buffers they accept.
> >
> > Some functionality should be abstracted, and storing type achieves that.
> >
> > Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > Changes in v2:
> > * Switch back to using enum
> >
> >  drivers/staging/media/imx/imx-ic-prpencvf.c   | 3 ++-
> >  drivers/staging/media/imx/imx-media-capture.c | 5 ++++-
> >  drivers/staging/media/imx/imx-media-csi.c     | 3 ++-
> >  drivers/staging/media/imx/imx-media.h         | 8 +++++++-
> >  drivers/staging/media/imx/imx7-media-csi.c    | 3 ++-
> >  5 files changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
> > index 9b81cfbcd777..671bb9a681aa 100644
> > --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> > +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> > @@ -1266,7 +1266,8 @@ static int prp_registered(struct v4l2_subdev *sd)
> >
> >  	priv->vdev = imx_media_capture_device_init(ic_priv->ipu_dev,
> >  						   &ic_priv->sd,
> > -						   PRPENCVF_SRC_PAD, true);
> > +						   PRPENCVF_SRC_PAD, true,
> > +						   DEVICE_TYPE_IMX56);
> >  	if (IS_ERR(priv->vdev))
> >  		return PTR_ERR(priv->vdev);
> >
> > diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
> > index 93ba09236010..65dc95a48ecc 100644
> > --- a/drivers/staging/media/imx/imx-media-capture.c
> > +++ b/drivers/staging/media/imx/imx-media-capture.c
> > @@ -34,6 +34,7 @@ struct capture_priv {
> >
> >  	struct imx_media_video_dev vdev;	/* Video device */
> >  	struct media_pad vdev_pad;		/* Video device pad */
> > +	enum imx_media_device_type type;	/* Type of hardware implementation */
> >
> >  	struct v4l2_subdev *src_sd;		/* Source subdev */
> >  	int src_sd_pad;				/* Source subdev pad */
> > @@ -957,7 +958,8 @@ EXPORT_SYMBOL_GPL(imx_media_capture_device_unregister);
> >
> >  struct imx_media_video_dev *
> >  imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
> > -			      int pad, bool legacy_api)
> > +			      int pad, bool legacy_api,
> > +			      enum imx_media_device_type type)
> >  {
> >  	struct capture_priv *priv;
> >  	struct video_device *vfd;
> > @@ -972,6 +974,7 @@ imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
> >  	priv->src_sd_pad = pad;
> >  	priv->dev = dev;
> >  	priv->legacy_api = legacy_api;
> > +	priv->type = type;
> >
> >  	mutex_init(&priv->mutex);
> >  	INIT_LIST_HEAD(&priv->ready_q);
> > diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> > index bd7f156f2d52..d5557bb4913d 100644
> > --- a/drivers/staging/media/imx/imx-media-csi.c
> > +++ b/drivers/staging/media/imx/imx-media-csi.c
> > @@ -1803,7 +1803,8 @@ static int csi_registered(struct v4l2_subdev *sd)
> >  	}
> >
> >  	priv->vdev = imx_media_capture_device_init(priv->sd.dev, &priv->sd,
> > -						   CSI_SRC_PAD_IDMAC, true);
> > +						   CSI_SRC_PAD_IDMAC, true,
> > +						   DEVICE_TYPE_IMX56);
> >  	if (IS_ERR(priv->vdev)) {
> >  		ret = PTR_ERR(priv->vdev);
> >  		goto free_fim;
> > diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
> > index f263fc3adbb9..e4c22b3ccd57 100644
> > --- a/drivers/staging/media/imx/imx-media.h
> > +++ b/drivers/staging/media/imx/imx-media.h
> > @@ -96,6 +96,11 @@ enum imx_pixfmt_sel {
> >  	PIXFMT_SEL_ANY = PIXFMT_SEL_YUV | PIXFMT_SEL_RGB | PIXFMT_SEL_BAYER,
> >  };
> >
> > +enum imx_media_device_type {
> > +	DEVICE_TYPE_IMX56,
> > +	DEVICE_TYPE_IMX78,
> > +};
> > +
> 
> Isn't this too coarse as a distinction ?
> 
> I tried adding per-soc identifiers here:
> https://lore.kernel.org/linux-media/20220214184318.409208-5-jacopo@jmondi.org/T/#u
> 
> Maybe they can help ?

I'd really prefer not mixing the two. This enumeration is meant to
select which backend to use in helpers that should not be shared in the
first place. I've started decoupling the i.MX6 and i.MX7+ code, but it
will still take some time (the work in progress is available at [1] if
anyone is interested). In the meantime I'm OK with this patch, but any
need for additional device identification should be limited to the
imx7-media-csi driver or the i.MX6-specific code, not added to shared
helpers.

[1] https://gitlab.com/ideasonboard/nxp/linux/-/tree/pinchartl/csi-bridge/destage

> >  struct imx_media_buffer {
> >  	struct vb2_v4l2_buffer vbuf; /* v4l buffer must be first */
> >  	struct list_head  list;
> > @@ -282,7 +287,8 @@ int imx_media_ic_unregister(struct v4l2_subdev *sd);
> >  /* imx-media-capture.c */
> >  struct imx_media_video_dev *
> >  imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
> > -			      int pad, bool legacy_api);
> > +			      int pad, bool legacy_api,
> > +			      enum imx_media_device_type type);
> >  void imx_media_capture_device_remove(struct imx_media_video_dev *vdev);
> >  int imx_media_capture_device_register(struct imx_media_video_dev *vdev,
> >  				      u32 link_flags);
> > diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> > index 32311fc0e2a4..173dd014c2d6 100644
> > --- a/drivers/staging/media/imx/imx7-media-csi.c
> > +++ b/drivers/staging/media/imx/imx7-media-csi.c
> > @@ -1039,7 +1039,8 @@ static int imx7_csi_registered(struct v4l2_subdev *sd)
> >  	}
> >
> >  	csi->vdev = imx_media_capture_device_init(csi->sd.dev, &csi->sd,
> > -						  IMX7_CSI_PAD_SRC, false);
> > +						  IMX7_CSI_PAD_SRC, false,
> > +						  DEVICE_TYPE_IMX78);
> >  	if (IS_ERR(csi->vdev))
> >  		return PTR_ERR(csi->vdev);
> >

-- 
Regards,

Laurent Pinchart

Re: (EXT) Re: [PATCH v2 3/9] media: imx: Use dedicated format handler for i.MX7/8

[Alexander Stein]

Hi Jacopo,

Am Montag, 14. Februar 2022, 19:51:15 CET schrieb Jacopo Mondi:
> Hello
> 
> On Fri, Feb 11, 2022 at 03:27:46PM +0100, Alexander Stein wrote:
> > From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> > 
> > This splits out a format handler which takes into account
> > the capabilities of the i.MX7/8 video device,
> > as opposed to the default handler compatible with both i.MX5/6 and
> > i.MX7/8.
> > 
> > Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > Changes in v2:
> > * Switch back to using enum
> > * Get rid of an additional call to v4l2_fill_pixfmt()
> > 
> >  drivers/staging/media/imx/imx-media-utils.c | 60 +++++++++++++++++++--
> >  1 file changed, 56 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/media/imx/imx-media-utils.c
> > b/drivers/staging/media/imx/imx-media-utils.c index
> > c42f3da8e3a8..02a4cb124d37 100644
> > --- a/drivers/staging/media/imx/imx-media-utils.c
> > +++ b/drivers/staging/media/imx/imx-media-utils.c
> > @@ -516,10 +516,9 @@ void imx_media_try_colorimetry(struct
> > v4l2_mbus_framefmt *tryfmt,> 
> >  }
> >  EXPORT_SYMBOL_GPL(imx_media_try_colorimetry);
> > 
> > -int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> > -				  const struct v4l2_mbus_framefmt 
*mbus,
> > -				  const struct imx_media_pixfmt 
*cc,
> > -				  enum imx_media_device_type type)
> > +static int imx56_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> > +					   const struct 
v4l2_mbus_framefmt *mbus,
> > +					   const struct 
imx_media_pixfmt *cc)
> > 
> >  {
> >  
> >  	u32 width;
> >  	u32 stride;
> > 
> > @@ -568,6 +567,59 @@ int imx_media_mbus_fmt_to_pix_fmt(struct
> > v4l2_pix_format *pix,> 
> >  	return 0;
> >  
> >  }
> > 
> > +
> > +static int imx78_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> > +					   const struct 
v4l2_mbus_framefmt *mbus,
> > +					   const struct 
imx_media_pixfmt *cc)
> > +{
> > +	u32 aligned_width;
> > +	int ret;
> > +
> > +	if (!cc)
> > +		cc = imx_media_find_mbus_format(mbus->code, 
PIXFMT_SEL_ANY);
> > +
> > +	if (!cc)
> > +		return -EINVAL;
> > +	/*
> > +	 * The hardware can handle line lengths divisible by 4 pixels
> > +	 * as long as the whole buffer size ends up divisible by 8 bytes.
> > +	 * If not, use the value of 8 pixels recommended in the datasheet.
> > +	 * Only 8bits-per-pixel formats may need to get aligned to 8 
pixels,
> > +	 * because both 10-bit and 16-bit pixels occupy 2 bytes.
> > +	 * In those, 4-pixel aligmnent is equal to 8-byte alignment.
> > +	 */
> > +	if (cc->bpp == 1)
> 
> Am I mistaken or in the format enumeration bpp is expressed in bits
> and not bytes ?

No, you are completely right. Thanks for find noticing that.

Regards,
Alexander

> > +		aligned_width = round_up(mbus->width, 8);
> > +	else
> > +		aligned_width = round_up(mbus->width, 4);
> > +
> > +	ret = v4l2_fill_pixfmt(pix, cc->fourcc,
> > +			       aligned_width, mbus->height);
> > +	if (ret)
> > +		return ret;
> > +
> > +	pix->colorspace = mbus->colorspace;
> > +	pix->xfer_func = mbus->xfer_func;
> > +	pix->ycbcr_enc = mbus->ycbcr_enc;
> > +	pix->quantization = mbus->quantization;
> > +	pix->field = mbus->field;
> > +
> > +	return ret;
> > +}
> > +
> > +int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> > +				  const struct v4l2_mbus_framefmt 
*mbus,
> > +				  const struct imx_media_pixfmt 
*cc,
> > +				  enum imx_media_device_type type)
> > +{
> > +	switch (type) {
> > +	case DEVICE_TYPE_IMX56:
> > +		return imx56_media_mbus_fmt_to_pix_fmt(pix, mbus, cc);
> > +	case DEVICE_TYPE_IMX78:
> > +		return imx78_media_mbus_fmt_to_pix_fmt(pix, mbus, cc);
> > +	}
> > +	return -EINVAL;
> > +}
> > 
> >  EXPORT_SYMBOL_GPL(imx_media_mbus_fmt_to_pix_fmt);
> >  
> >  void imx_media_free_dma_buf(struct device *dev,
> > 
> > --
> > 2.25.1

Re: [PATCH v2 1/9] media: imx: Store the type of hardware implementation

[Laurent Pinchart]

Hi Alexander and Dorota,

Thank you for the patch.

On Fri, Feb 11, 2022 at 03:27:44PM +0100, Alexander Stein wrote:
> From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> 
> The driver covers i.MX5/6, as well as i.MX7/8 hardware.
> Those implementations differ, e.g. in the sizes of buffers they accept.
> 
> Some functionality should be abstracted, and storing type achieves that.

As much as I'd love it one of you would pick
https://gitlab.com/ideasonboard/nxp/linux/-/tree/pinchartl/csi-bridge/destage
and bring it to completion, I won't ask for yak shaving.

Could you however mention in the commit message that this is a temporary
solution ? Maybe as follows:

This is a temporary solution until the imx7-media-csi driver gets
decoupled from the helpers shared with the i.MX6 implementation, and
should be reverted once this happens.

With that,

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

> Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Changes in v2:
> * Switch back to using enum
> 
>  drivers/staging/media/imx/imx-ic-prpencvf.c   | 3 ++-
>  drivers/staging/media/imx/imx-media-capture.c | 5 ++++-
>  drivers/staging/media/imx/imx-media-csi.c     | 3 ++-
>  drivers/staging/media/imx/imx-media.h         | 8 +++++++-
>  drivers/staging/media/imx/imx7-media-csi.c    | 3 ++-
>  5 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index 9b81cfbcd777..671bb9a681aa 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -1266,7 +1266,8 @@ static int prp_registered(struct v4l2_subdev *sd)
>  
>  	priv->vdev = imx_media_capture_device_init(ic_priv->ipu_dev,
>  						   &ic_priv->sd,
> -						   PRPENCVF_SRC_PAD, true);
> +						   PRPENCVF_SRC_PAD, true,
> +						   DEVICE_TYPE_IMX56);
>  	if (IS_ERR(priv->vdev))
>  		return PTR_ERR(priv->vdev);
>  
> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
> index 93ba09236010..65dc95a48ecc 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -34,6 +34,7 @@ struct capture_priv {
>  
>  	struct imx_media_video_dev vdev;	/* Video device */
>  	struct media_pad vdev_pad;		/* Video device pad */
> +	enum imx_media_device_type type;	/* Type of hardware implementation */
>  
>  	struct v4l2_subdev *src_sd;		/* Source subdev */
>  	int src_sd_pad;				/* Source subdev pad */
> @@ -957,7 +958,8 @@ EXPORT_SYMBOL_GPL(imx_media_capture_device_unregister);
>  
>  struct imx_media_video_dev *
>  imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
> -			      int pad, bool legacy_api)
> +			      int pad, bool legacy_api,
> +			      enum imx_media_device_type type)
>  {
>  	struct capture_priv *priv;
>  	struct video_device *vfd;
> @@ -972,6 +974,7 @@ imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
>  	priv->src_sd_pad = pad;
>  	priv->dev = dev;
>  	priv->legacy_api = legacy_api;
> +	priv->type = type;
>  
>  	mutex_init(&priv->mutex);
>  	INIT_LIST_HEAD(&priv->ready_q);
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index bd7f156f2d52..d5557bb4913d 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -1803,7 +1803,8 @@ static int csi_registered(struct v4l2_subdev *sd)
>  	}
>  
>  	priv->vdev = imx_media_capture_device_init(priv->sd.dev, &priv->sd,
> -						   CSI_SRC_PAD_IDMAC, true);
> +						   CSI_SRC_PAD_IDMAC, true,
> +						   DEVICE_TYPE_IMX56);
>  	if (IS_ERR(priv->vdev)) {
>  		ret = PTR_ERR(priv->vdev);
>  		goto free_fim;
> diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
> index f263fc3adbb9..e4c22b3ccd57 100644
> --- a/drivers/staging/media/imx/imx-media.h
> +++ b/drivers/staging/media/imx/imx-media.h
> @@ -96,6 +96,11 @@ enum imx_pixfmt_sel {
>  	PIXFMT_SEL_ANY = PIXFMT_SEL_YUV | PIXFMT_SEL_RGB | PIXFMT_SEL_BAYER,
>  };
>  
> +enum imx_media_device_type {
> +	DEVICE_TYPE_IMX56,
> +	DEVICE_TYPE_IMX78,
> +};
> +
>  struct imx_media_buffer {
>  	struct vb2_v4l2_buffer vbuf; /* v4l buffer must be first */
>  	struct list_head  list;
> @@ -282,7 +287,8 @@ int imx_media_ic_unregister(struct v4l2_subdev *sd);
>  /* imx-media-capture.c */
>  struct imx_media_video_dev *
>  imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
> -			      int pad, bool legacy_api);
> +			      int pad, bool legacy_api,
> +			      enum imx_media_device_type type);
>  void imx_media_capture_device_remove(struct imx_media_video_dev *vdev);
>  int imx_media_capture_device_register(struct imx_media_video_dev *vdev,
>  				      u32 link_flags);
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index 32311fc0e2a4..173dd014c2d6 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -1039,7 +1039,8 @@ static int imx7_csi_registered(struct v4l2_subdev *sd)
>  	}
>  
>  	csi->vdev = imx_media_capture_device_init(csi->sd.dev, &csi->sd,
> -						  IMX7_CSI_PAD_SRC, false);
> +						  IMX7_CSI_PAD_SRC, false,
> +						  DEVICE_TYPE_IMX78);
>  	if (IS_ERR(csi->vdev))
>  		return PTR_ERR(csi->vdev);
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 4/9] media: imx: Fail conversion if pixel format not supported

[Laurent Pinchart]

Hi Alexander and Dorota,

Thank you for the patch.

On Fri, Feb 11, 2022 at 03:27:47PM +0100, Alexander Stein wrote:
> From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> 
> imx_media_find_mbus_format has NULL as a valid return value,
> therefore the caller should take it into account.
> 
> Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>

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

> ---
>  drivers/staging/media/imx/imx-media-utils.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> index 02a4cb124d37..e59aaa77172a 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -544,6 +544,9 @@ static int imx56_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
>  		cc = imx_media_find_mbus_format(code, PIXFMT_SEL_YUV);
>  	}
>  
> +	if (!cc)
> +		return -EINVAL;
> +
>  	/* Round up width for minimum burst size */
>  	width = round_up(mbus->width, 8);
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 5/9] media: imx: imx7_mipi_csis: store colorspace in set_fmt as well

[Laurent Pinchart]

Hi Alexander,

Thank you for the patch.

On Fri, Feb 11, 2022 at 03:27:48PM +0100, Alexander Stein wrote:
> Without this the default (SMPTE 170M) from init_cfg stays unchanged.
> Even after configuring 'srgb' colorspace (or 'raw')
> $ media-ctl -V "'csis-32e30000.mipi-csi':0 [colorspace:srgb]"
> the colorspace does not change at all:
> $ media-ctl --get-v4l2 "'csis-32e30000.mipi-csi':0"
>   [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:smpte170m xfer:709
>    ycbcr:601 quantization:lim-range]
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>

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

As this change is independent from the previous patches in the series,
I'll take it in my tree and will send a pull request for v5.18.

> ---
> Changes in v2:
> * Store other colorspace-related fields as well
> 
>  drivers/staging/media/imx/imx7-mipi-csis.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
> index a22d0e6b3d44..388cfd012212 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -1062,6 +1062,10 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
>  	fmt->code = csis_fmt->code;
>  	fmt->width = sdformat->format.width;
>  	fmt->height = sdformat->format.height;
> +	fmt->colorspace = sdformat->format.colorspace;
> +	fmt->quantization = sdformat->format.quantization;
> +	fmt->xfer_func = sdformat->format.xfer_func;
> +	fmt->ycbcr_enc = sdformat->format.ycbcr_enc;
>  
>  	sdformat->format = *fmt;
>  

-- 
Regards,

Laurent Pinchart