[PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state

[PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state

[Laurent Pinchart]

Hello,

This small series moves the imx7-mipi-csi driver to use the subdev
active state. Patches 1/6 to 5/6 are small preparatory cleanups, with
the main change in 6/6.

I haven't tested the series yet as I need to dig up the hardware first.
Adam, you offered to test the similar imx-mipi-csis series I've sent
recently on the i.MX8MM, would you be able to test this one at the same
time ?

Laurent Pinchart (6):
  media: imx: imx7-media-csi: Drop imx7_csi.cc field
  media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()
  media: imx: imx7-media-csi: Drop unneeded check when starting
    streaming
  media: imx: imx7-media-csi: Drop unneeded src_sd check
  media: imx: imx7-media-csi: Drop unneeded pad checks
  media: imx: imx7-media-csi: Use V4L2 subdev active state

 drivers/media/platform/nxp/imx7-media-csi.c | 208 ++++++--------------
 1 file changed, 58 insertions(+), 150 deletions(-)


base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
-- 
Regards,

Laurent Pinchart

[PATCH v1 1/6] media: imx: imx7-media-csi: Drop imx7_csi.cc field

[Laurent Pinchart]

The imx7_csi.cc field is set but never used. Drop it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/nxp/imx7-media-csi.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
index 886374d3a6ff..be3c1494cfb3 100644
--- a/drivers/media/platform/nxp/imx7-media-csi.c
+++ b/drivers/media/platform/nxp/imx7-media-csi.c
@@ -228,7 +228,6 @@ struct imx7_csi {
 	struct media_pad pad[IMX7_CSI_PADS_NUM];
 
 	struct v4l2_mbus_framefmt format_mbus[IMX7_CSI_PADS_NUM];
-	const struct imx7_csi_pixfmt *cc[IMX7_CSI_PADS_NUM];
 
 	/* Video device */
 	struct video_device *vdev;		/* Video device */
@@ -1805,8 +1804,6 @@ static int imx7_csi_init_cfg(struct v4l2_subdev *sd,
 		mf->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(mf->colorspace);
 		mf->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(!cc->yuv,
 					mf->colorspace, mf->ycbcr_enc);
-
-		csi->cc[i] = cc;
 	}
 
 	return 0;
@@ -2014,14 +2011,8 @@ static int imx7_csi_set_fmt(struct v4l2_subdev *sd,
 		outfmt = imx7_csi_get_format(csi, sd_state, IMX7_CSI_PAD_SRC,
 					     sdformat->which);
 		*outfmt = format.format;
-
-		if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
-			csi->cc[IMX7_CSI_PAD_SRC] = outcc;
 	}
 
-	if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
-		csi->cc[sdformat->pad] = cc;
-
 out_unlock:
 	mutex_unlock(&csi->lock);
 
-- 
Regards,

Laurent Pinchart

[PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()

[Laurent Pinchart]

The imx7_csi_video_init_format() function instantiates a
v4l2_subdev_format on the stack, to only use the .format field of that
structure. Replace it with a v4l2_mbus_framefmt instance.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/nxp/imx7-media-csi.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
index be3c1494cfb3..e96bee4e5921 100644
--- a/drivers/media/platform/nxp/imx7-media-csi.c
+++ b/drivers/media/platform/nxp/imx7-media-csi.c
@@ -1598,17 +1598,15 @@ static struct imx7_csi_vb2_buffer *imx7_csi_video_next_buf(struct imx7_csi *csi)
 
 static int imx7_csi_video_init_format(struct imx7_csi *csi)
 {
-	struct v4l2_subdev_format fmt_src = {
-		.pad = IMX7_CSI_PAD_SRC,
-		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
-	};
-	fmt_src.format.code = IMX7_CSI_DEF_MBUS_CODE;
-	fmt_src.format.width = IMX7_CSI_DEF_PIX_WIDTH;
-	fmt_src.format.height = IMX7_CSI_DEF_PIX_HEIGHT;
+	struct v4l2_mbus_framefmt format = { };
 
-	imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &fmt_src.format, NULL);
-	csi->vdev_compose.width = fmt_src.format.width;
-	csi->vdev_compose.height = fmt_src.format.height;
+	format.code = IMX7_CSI_DEF_MBUS_CODE;
+	format.width = IMX7_CSI_DEF_PIX_WIDTH;
+	format.height = IMX7_CSI_DEF_PIX_HEIGHT;
+
+	imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &format, NULL);
+	csi->vdev_compose.width = format.width;
+	csi->vdev_compose.height = format.height;
 
 	csi->vdev_cc = imx7_csi_find_pixel_format(csi->vdev_fmt.pixelformat);
 
-- 
Regards,

Laurent Pinchart

[PATCH v1 3/6] media: imx: imx7-media-csi: Drop unneeded check when starting streaming

[Laurent Pinchart]

The .s_stream() operation is guaranteed not to be called to start/stop
an already started/stopped subdev. Remove the unneeded check.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/nxp/imx7-media-csi.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
index e96bee4e5921..973a4d015279 100644
--- a/drivers/media/platform/nxp/imx7-media-csi.c
+++ b/drivers/media/platform/nxp/imx7-media-csi.c
@@ -1734,9 +1734,6 @@ static int imx7_csi_s_stream(struct v4l2_subdev *sd, int enable)
 		goto out_unlock;
 	}
 
-	if (csi->is_streaming == !!enable)
-		goto out_unlock;
-
 	if (enable) {
 		ret = imx7_csi_init(csi);
 		if (ret < 0)
-- 
Regards,

Laurent Pinchart

[PATCH v1 4/6] media: imx: imx7-media-csi: Drop unneeded src_sd check

[Laurent Pinchart]

The .s_stream() and .link_validate() operations can't be called with a
NULL src_sd, as subdev nodes are not registered before the async
notifier completes. Remove the unneeded checks.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/nxp/imx7-media-csi.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
index 973a4d015279..f02a88e1ca10 100644
--- a/drivers/media/platform/nxp/imx7-media-csi.c
+++ b/drivers/media/platform/nxp/imx7-media-csi.c
@@ -1729,11 +1729,6 @@ static int imx7_csi_s_stream(struct v4l2_subdev *sd, int enable)
 
 	mutex_lock(&csi->lock);
 
-	if (!csi->src_sd) {
-		ret = -EPIPE;
-		goto out_unlock;
-	}
-
 	if (enable) {
 		ret = imx7_csi_init(csi);
 		if (ret < 0)
@@ -2024,9 +2019,6 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd,
 	unsigned int i;
 	int ret;
 
-	if (!csi->src_sd)
-		return -EPIPE;
-
 	/*
 	 * Validate the source link, and record whether the source uses the
 	 * parallel input or the CSI-2 receiver.
-- 
Regards,

Laurent Pinchart

[PATCH v1 5/6] media: imx: imx7-media-csi: Drop unneeded pad checks

[Laurent Pinchart]

The subdev core guarantees that the .set_fmt() operation is always
called with a valid pad. Drop the unneeded pad checks.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/nxp/imx7-media-csi.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
index f02a88e1ca10..052acbd683a8 100644
--- a/drivers/media/platform/nxp/imx7-media-csi.c
+++ b/drivers/media/platform/nxp/imx7-media-csi.c
@@ -1934,6 +1934,7 @@ static int imx7_csi_try_fmt(struct imx7_csi *csi,
 		sdformat->format.quantization = in_fmt->quantization;
 		sdformat->format.ycbcr_enc = in_fmt->ycbcr_enc;
 		break;
+
 	case IMX7_CSI_PAD_SINK:
 		*cc = imx7_csi_find_mbus_format(sdformat->format.code);
 		if (!*cc) {
@@ -1945,8 +1946,6 @@ static int imx7_csi_try_fmt(struct imx7_csi *csi,
 		if (sdformat->format.field != V4L2_FIELD_INTERLACED)
 			sdformat->format.field = V4L2_FIELD_NONE;
 		break;
-	default:
-		return -EINVAL;
 	}
 
 	imx7_csi_try_colorimetry(&sdformat->format);
@@ -1966,9 +1965,6 @@ static int imx7_csi_set_fmt(struct v4l2_subdev *sd,
 	struct v4l2_subdev_format format;
 	int ret = 0;
 
-	if (sdformat->pad >= IMX7_CSI_PADS_NUM)
-		return -EINVAL;
-
 	mutex_lock(&csi->lock);
 
 	if (csi->is_streaming) {
-- 
Regards,

Laurent Pinchart

[PATCH v1 6/6] media: imx: imx7-media-csi: Use V4L2 subdev active state

[Laurent Pinchart]

Use the V4L2 subdev active state API to store the active format. This
simplifies the driver not only by dropping the state stored in the
imx7_csi structure, but also by replacing the manual lock with the state
lock.

The is_streaming field is now protected by the active state lock, either
explicitly in .s_stream(), where the active state is locked manually, or
implicitly in .set_fmt(), which is called with the state locked.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/nxp/imx7-media-csi.c | 164 ++++++--------------
 1 file changed, 49 insertions(+), 115 deletions(-)

diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
index 052acbd683a8..9275447987d1 100644
--- a/drivers/media/platform/nxp/imx7-media-csi.c
+++ b/drivers/media/platform/nxp/imx7-media-csi.c
@@ -211,7 +211,6 @@ struct imx7_csi {
 	int irq;
 	struct clk *mclk;
 
-	struct mutex lock; /* Protects is_streaming, format_mbus, cc */
 	spinlock_t irqlock; /* Protects last_eof */
 
 	/* Media and V4L2 device */
@@ -227,8 +226,6 @@ struct imx7_csi {
 	struct v4l2_subdev sd;
 	struct media_pad pad[IMX7_CSI_PADS_NUM];
 
-	struct v4l2_mbus_framefmt format_mbus[IMX7_CSI_PADS_NUM];
-
 	/* Video device */
 	struct video_device *vdev;		/* Video device */
 	struct media_pad vdev_pad;		/* Video device pad */
@@ -509,7 +506,8 @@ static void imx7_csi_dma_stop(struct imx7_csi *csi)
 	imx7_csi_hw_disable_irq(csi);
 }
 
-static void imx7_csi_configure(struct imx7_csi *csi)
+static void imx7_csi_configure(struct imx7_csi *csi,
+			       struct v4l2_subdev_state *sd_state)
 {
 	struct v4l2_pix_format *out_pix = &csi->vdev_fmt;
 	int width = out_pix->width;
@@ -540,12 +538,17 @@ static void imx7_csi_configure(struct imx7_csi *csi)
 		    out_pix->pixelformat == V4L2_PIX_FMT_YUYV)
 			width *= 2;
 	} else {
+		const struct v4l2_mbus_framefmt *sink_fmt;
+
+		sink_fmt = v4l2_subdev_get_pad_format(&csi->sd, sd_state,
+						      IMX7_CSI_PAD_SINK);
+
 		cr1 = BIT_SOF_POL | BIT_REDGE | BIT_HSYNC_POL | BIT_FCC
 		    | BIT_MCLKDIV(1) | BIT_MCLKEN;
 
 		cr18 |= BIT_DATA_FROM_MIPI;
 
-		switch (csi->format_mbus[IMX7_CSI_PAD_SINK].code) {
+		switch (sink_fmt->code) {
 		case MEDIA_BUS_FMT_Y8_1X8:
 		case MEDIA_BUS_FMT_SBGGR8_1X8:
 		case MEDIA_BUS_FMT_SGBRG8_1X8:
@@ -626,7 +629,8 @@ static void imx7_csi_configure(struct imx7_csi *csi)
 	imx7_csi_reg_write(csi, stride, CSI_CSIFBUF_PARA);
 }
 
-static int imx7_csi_init(struct imx7_csi *csi)
+static int imx7_csi_init(struct imx7_csi *csi,
+			 struct v4l2_subdev_state *sd_state)
 {
 	int ret;
 
@@ -634,7 +638,7 @@ static int imx7_csi_init(struct imx7_csi *csi)
 	if (ret < 0)
 		return ret;
 
-	imx7_csi_configure(csi);
+	imx7_csi_configure(csi, sd_state);
 
 	ret = imx7_csi_dma_setup(csi);
 	if (ret < 0)
@@ -1417,7 +1421,7 @@ static int imx7_csi_video_validate_fmt(struct imx7_csi *csi)
 	/* Retrieve the media bus format on the source subdev. */
 	fmt_src.pad = IMX7_CSI_PAD_SRC;
 	fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-	ret = v4l2_subdev_call(&csi->sd, pad, get_fmt, NULL, &fmt_src);
+	ret = v4l2_subdev_call_state_active(&csi->sd, pad, get_fmt, &fmt_src);
 	if (ret)
 		return ret;
 
@@ -1725,12 +1729,13 @@ static int imx7_csi_video_init(struct imx7_csi *csi)
 static int imx7_csi_s_stream(struct v4l2_subdev *sd, int enable)
 {
 	struct imx7_csi *csi = v4l2_get_subdevdata(sd);
+	struct v4l2_subdev_state *sd_state;
 	int ret = 0;
 
-	mutex_lock(&csi->lock);
+	sd_state = v4l2_subdev_lock_and_get_active_state(sd);
 
 	if (enable) {
-		ret = imx7_csi_init(csi);
+		ret = imx7_csi_init(csi, sd_state);
 		if (ret < 0)
 			goto out_unlock;
 
@@ -1752,29 +1757,14 @@ static int imx7_csi_s_stream(struct v4l2_subdev *sd, int enable)
 	csi->is_streaming = !!enable;
 
 out_unlock:
-	mutex_unlock(&csi->lock);
+	v4l2_subdev_unlock_state(sd_state);
 
 	return ret;
 }
 
-static struct v4l2_mbus_framefmt *
-imx7_csi_get_format(struct imx7_csi *csi,
-		    struct v4l2_subdev_state *sd_state,
-		    unsigned int pad,
-		    enum v4l2_subdev_format_whence which)
-{
-	if (which == V4L2_SUBDEV_FORMAT_TRY)
-		return v4l2_subdev_get_try_format(&csi->sd, sd_state, pad);
-
-	return &csi->format_mbus[pad];
-}
-
 static int imx7_csi_init_cfg(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_state *sd_state)
 {
-	const enum v4l2_subdev_format_whence which =
-		sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
-	struct imx7_csi *csi = v4l2_get_subdevdata(sd);
 	const struct imx7_csi_pixfmt *cc;
 	int i;
 
@@ -1782,7 +1772,7 @@ static int imx7_csi_init_cfg(struct v4l2_subdev *sd,
 
 	for (i = 0; i < IMX7_CSI_PADS_NUM; i++) {
 		struct v4l2_mbus_framefmt *mf =
-			imx7_csi_get_format(csi, sd_state, i, which);
+			v4l2_subdev_get_pad_format(sd, sd_state, i);
 
 		mf->code = IMX7_CSI_DEF_MBUS_CODE;
 		mf->width = IMX7_CSI_DEF_PIX_WIDTH;
@@ -1803,59 +1793,30 @@ static int imx7_csi_enum_mbus_code(struct v4l2_subdev *sd,
 				   struct v4l2_subdev_state *sd_state,
 				   struct v4l2_subdev_mbus_code_enum *code)
 {
-	struct imx7_csi *csi = v4l2_get_subdevdata(sd);
 	struct v4l2_mbus_framefmt *in_fmt;
 	int ret = 0;
 
-	mutex_lock(&csi->lock);
-
-	in_fmt = imx7_csi_get_format(csi, sd_state, IMX7_CSI_PAD_SINK,
-				     code->which);
+	in_fmt = v4l2_subdev_get_pad_format(sd, sd_state, IMX7_CSI_PAD_SINK);
 
 	switch (code->pad) {
 	case IMX7_CSI_PAD_SINK:
 		ret = imx7_csi_enum_mbus_formats(&code->code, code->index);
 		break;
+
 	case IMX7_CSI_PAD_SRC:
 		if (code->index != 0) {
 			ret = -EINVAL;
-			goto out_unlock;
+			break;
 		}
 
 		code->code = in_fmt->code;
 		break;
+
 	default:
 		ret = -EINVAL;
+		break;
 	}
 
-out_unlock:
-	mutex_unlock(&csi->lock);
-
-	return ret;
-}
-
-static int imx7_csi_get_fmt(struct v4l2_subdev *sd,
-			    struct v4l2_subdev_state *sd_state,
-			    struct v4l2_subdev_format *sdformat)
-{
-	struct imx7_csi *csi = v4l2_get_subdevdata(sd);
-	struct v4l2_mbus_framefmt *fmt;
-	int ret = 0;
-
-	mutex_lock(&csi->lock);
-
-	fmt = imx7_csi_get_format(csi, sd_state, sdformat->pad,
-				  sdformat->which);
-	if (!fmt) {
-		ret = -EINVAL;
-		goto out_unlock;
-	}
-
-	sdformat->format = *fmt;
-
-out_unlock:
-	mutex_unlock(&csi->lock);
-
 	return ret;
 }
 
@@ -1905,19 +1866,16 @@ static void imx7_csi_try_colorimetry(struct v4l2_mbus_framefmt *tryfmt)
 						      tryfmt->ycbcr_enc);
 }
 
-static int imx7_csi_try_fmt(struct imx7_csi *csi,
-			    struct v4l2_subdev_state *sd_state,
-			    struct v4l2_subdev_format *sdformat,
-			    const struct imx7_csi_pixfmt **cc)
+static void imx7_csi_try_fmt(struct v4l2_subdev *sd,
+			     struct v4l2_subdev_state *sd_state,
+			     struct v4l2_subdev_format *sdformat,
+			     const struct imx7_csi_pixfmt **cc)
 {
 	const struct imx7_csi_pixfmt *in_cc;
 	struct v4l2_mbus_framefmt *in_fmt;
 	u32 code;
 
-	in_fmt = imx7_csi_get_format(csi, sd_state, IMX7_CSI_PAD_SINK,
-				     sdformat->which);
-	if (!in_fmt)
-		return -EINVAL;
+	in_fmt = v4l2_subdev_get_pad_format(sd, sd_state, IMX7_CSI_PAD_SINK);
 
 	switch (sdformat->pad) {
 	case IMX7_CSI_PAD_SRC:
@@ -1949,8 +1907,6 @@ static int imx7_csi_try_fmt(struct imx7_csi *csi,
 	}
 
 	imx7_csi_try_colorimetry(&sdformat->format);
-
-	return 0;
 }
 
 static int imx7_csi_set_fmt(struct v4l2_subdev *sd,
@@ -1963,25 +1919,13 @@ static int imx7_csi_set_fmt(struct v4l2_subdev *sd,
 	const struct imx7_csi_pixfmt *cc;
 	struct v4l2_mbus_framefmt *fmt;
 	struct v4l2_subdev_format format;
-	int ret = 0;
 
-	mutex_lock(&csi->lock);
+	if (csi->is_streaming)
+		return -EBUSY;
 
-	if (csi->is_streaming) {
-		ret = -EBUSY;
-		goto out_unlock;
-	}
+	imx7_csi_try_fmt(sd, sd_state, sdformat, &cc);
 
-	ret = imx7_csi_try_fmt(csi, sd_state, sdformat, &cc);
-	if (ret < 0)
-		goto out_unlock;
-
-	fmt = imx7_csi_get_format(csi, sd_state, sdformat->pad,
-				  sdformat->which);
-	if (!fmt) {
-		ret = -EINVAL;
-		goto out_unlock;
-	}
+	fmt = v4l2_subdev_get_pad_format(sd, sd_state, sdformat->pad);
 
 	*fmt = sdformat->format;
 
@@ -1990,19 +1934,14 @@ static int imx7_csi_set_fmt(struct v4l2_subdev *sd,
 		format.pad = IMX7_CSI_PAD_SRC;
 		format.which = sdformat->which;
 		format.format = sdformat->format;
-		if (imx7_csi_try_fmt(csi, sd_state, &format, &outcc)) {
-			ret = -EINVAL;
-			goto out_unlock;
-		}
-		outfmt = imx7_csi_get_format(csi, sd_state, IMX7_CSI_PAD_SRC,
-					     sdformat->which);
+		imx7_csi_try_fmt(sd, sd_state, &format, &outcc);
+
+		outfmt = v4l2_subdev_get_pad_format(sd, sd_state,
+						    IMX7_CSI_PAD_SRC);
 		*outfmt = format.format;
 	}
 
-out_unlock:
-	mutex_unlock(&csi->lock);
-
-	return ret;
+	return 0;
 }
 
 static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd,
@@ -2102,7 +2041,7 @@ static const struct v4l2_subdev_video_ops imx7_csi_video_ops = {
 static const struct v4l2_subdev_pad_ops imx7_csi_pad_ops = {
 	.init_cfg	= imx7_csi_init_cfg,
 	.enum_mbus_code	= imx7_csi_enum_mbus_code,
-	.get_fmt	= imx7_csi_get_fmt,
+	.get_fmt	= v4l2_subdev_get_fmt,
 	.set_fmt	= imx7_csi_set_fmt,
 	.link_validate	= imx7_csi_pad_link_validate,
 };
@@ -2192,6 +2131,7 @@ static void imx7_csi_media_cleanup(struct imx7_csi *csi)
 {
 	v4l2_device_unregister(&csi->v4l2_dev);
 	media_device_unregister(&csi->mdev);
+	v4l2_subdev_cleanup(&csi->sd);
 	media_device_cleanup(&csi->mdev);
 }
 
@@ -2259,6 +2199,10 @@ static int imx7_csi_media_init(struct imx7_csi *csi)
 	if (ret)
 		goto error;
 
+	ret = v4l2_subdev_init_finalize(&csi->sd);
+	if (ret)
+		goto error;
+
 	ret = v4l2_device_register_subdev(&csi->v4l2_dev, &csi->sd);
 	if (ret)
 		goto error;
@@ -2284,27 +2228,22 @@ static int imx7_csi_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, csi);
 
 	spin_lock_init(&csi->irqlock);
-	mutex_init(&csi->lock);
 
 	/* Acquire resources and install interrupt handler. */
 	csi->mclk = devm_clk_get(&pdev->dev, "mclk");
 	if (IS_ERR(csi->mclk)) {
 		ret = PTR_ERR(csi->mclk);
 		dev_err(dev, "Failed to get mclk: %d", ret);
-		goto destroy_mutex;
+		return ret;
 	}
 
 	csi->irq = platform_get_irq(pdev, 0);
-	if (csi->irq < 0) {
-		ret = csi->irq;
-		goto destroy_mutex;
-	}
+	if (csi->irq < 0)
+		return csi->irq;
 
 	csi->regbase = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(csi->regbase)) {
-		ret = PTR_ERR(csi->regbase);
-		goto destroy_mutex;
-	}
+	if (IS_ERR(csi->regbase))
+		return PTR_ERR(csi->regbase);
 
 	csi->model = (enum imx_csi_model)(uintptr_t)of_device_get_match_data(&pdev->dev);
 
@@ -2312,13 +2251,13 @@ static int imx7_csi_probe(struct platform_device *pdev)
 			       (void *)csi);
 	if (ret < 0) {
 		dev_err(dev, "Request CSI IRQ failed.\n");
-		goto destroy_mutex;
+		return ret;
 	}
 
 	/* Initialize all the media device infrastructure. */
 	ret = imx7_csi_media_init(csi);
 	if (ret)
-		goto destroy_mutex;
+		return ret;
 
 	/* Set the default mbus formats. */
 	ret = imx7_csi_init_cfg(&csi->sd, NULL);
@@ -2337,9 +2276,6 @@ static int imx7_csi_probe(struct platform_device *pdev)
 media_cleanup:
 	imx7_csi_media_cleanup(csi);
 
-destroy_mutex:
-	mutex_destroy(&csi->lock);
-
 	return ret;
 }
 
@@ -2353,8 +2289,6 @@ static int imx7_csi_remove(struct platform_device *pdev)
 	v4l2_async_nf_cleanup(&csi->notifier);
 	v4l2_async_unregister_subdev(&csi->sd);
 
-	mutex_destroy(&csi->lock);
-
 	return 0;
 }
 
-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state

[Adam Ford]

On Thu, Jan 26, 2023 at 8:27 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hello,
>
> This small series moves the imx7-mipi-csi driver to use the subdev
> active state. Patches 1/6 to 5/6 are small preparatory cleanups, with
> the main change in 6/6.
>
> I haven't tested the series yet as I need to dig up the hardware first.
> Adam, you offered to test the similar imx-mipi-csis series I've sent
> recently on the i.MX8MM, would you be able to test this one at the same
> time ?

Yes.  I'll try to do it tonight and get you feedback.  I'm out of town
for the next few days, so if I don't respond to follow up stuff right
away, it's because I am away.

adam
>
> Laurent Pinchart (6):
>   media: imx: imx7-media-csi: Drop imx7_csi.cc field
>   media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()
>   media: imx: imx7-media-csi: Drop unneeded check when starting
>     streaming
>   media: imx: imx7-media-csi: Drop unneeded src_sd check
>   media: imx: imx7-media-csi: Drop unneeded pad checks
>   media: imx: imx7-media-csi: Use V4L2 subdev active state
>
>  drivers/media/platform/nxp/imx7-media-csi.c | 208 ++++++--------------
>  1 file changed, 58 insertions(+), 150 deletions(-)
>
>
> base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()

[Adam Ford]

On Thu, Jan 26, 2023 at 8:27 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> The imx7_csi_video_init_format() function instantiates a
> v4l2_subdev_format on the stack, to only use the .format field of that
> structure. Replace it with a v4l2_mbus_framefmt instance.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---

With this series and the CSIS series you posted earlier, I get a ton
of splat and the ov5640 camera doesn't appear in the media information
with media-ctrl -p

   12.386980] lr : imx7_csi_probe+0x26c/0x380 [imx7_media_csi]
[   12.387010] sp : ffff80000afd3900
[   12.387013] x29: ffff80000afd3900 x28: 0000000000000000 x27: 0000000000000000
[   12.387025] x26: ffff8000012ae180 x25: 0000000000000001 x24: ffff000005bb8340
[   12.387033] x23: ffff000005bb8450 x22: ffff000005bb80a8 x21: ffff8000012ac4f8
[   12.387040] x20: 0000000000000000 x19: ffff000005bb8080 x18: ffffffffffffffff
[   12.387048] x17: 0000000000000000
[   12.393690] Bluetooth: HCI UART protocol QCA registered
[   12.397321]  x16: 0000000000000000 x15: 64656d3d4d455453
[   12.397327] x14: ffff80000a56d220 x13: 0000000000000040 x12: 0000000000000228
[   12.397335] x11: 0000000000000000 x10: 0000000000000000 x9 : 000001e000000280
[   12.397342] x8 : 0000000100002006 x7 : 0002000100000008 x6 : 0000000000000002
[   12.397350] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
[   12.397357] x2 : ffff000005bb84c8 x1 : 0000000000000000 x0 : ffff000005bb8450
[   12.397365] Call trace:
[   12.397368]  imx7_csi_init_cfg+0x64/0x9c [imx7_media_csi]
[   12.397385]  platform_probe+0x68/0xe0
[   12.406436] Bluetooth: HCI UART protocol Marvell registered
[   12.413735]  really_probe+0xbc/0x2dc
[   12.413743]  __driver_probe_device+0x78/0xe0
[   12.413748]  driver_probe_device+0xd8/0x160
[   12.413754]  __driver_attach+0x94/0x19c
[   12.413759]  bus_for_each_dev+0x70/0xd0
[   12.413764]  driver_attach+0x24/0x30
[   12.413769]  bus_add_driver+0x154/0x20c
[   12.413774]  driver_register+0x78/0x130
[   12.413780]  __platform_driver_register+0x28/0x34
[   12.413786]  imx7_csi_driver_init+0x20/0x1000 [imx7_media_csi]
[   12.413803]  do_one_initcall+0x50/0x1d0
[   12.413810]  do_init_module+0x48/0x1d0
[   12.413817]  load_module+0x193c/0x1cb0
[   12.413822]  __do_sys_finit_module+0xa8/0x100
[   12.413828]  __arm64_sys_finit_module+0x20/0x30
[   12.413834]  invoke_syscall+0x48/0x114
[   12.413842]  el0_svc_common.constprop.0+0xd4/0xfc
[   12.413848]  do_el0_svc+0x3c/0xc0
[   12.413854]  el0_svc+0x2c/0x84
[   12.413863]  el0t_64_sync_handler+0xbc/0x140
[   12.624336]  el0t_64_sync+0x190/0x194
[   12.628002] ---[ end trace 0000000000000000 ]---
[   12.633012] Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000000
[   12.641948] Mem abort info:
[   12.644812]   ESR = 0x0000000096000044
[   12.648652]   EC = 0x25: DABT (current EL), IL = 32 bits
[   12.654047]   SET = 0, FnV = 0
[   12.654923] imx8m-ddrc-devfreq 3d400000.memory-controller: failed
to init firmware freq info: -19
[   12.657176]   EA = 0, S1PTW = 0
[   12.669382]   FSC = 0x04: level 0 translation fault
[   12.674349] Data abort info:
[   12.677284]   ISV = 0, ISS = 0x00000044
[   12.681169]   CM = 0, WnR = 1
[   12.684189] user pgtable: 4k pages, 48-bit VAs, pgdp=000000004597e000
[   12.690698] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
[   12.697570] Internal error: Oops: 0000000096000044 [#1] PREEMPT SMP
[   12.703848] Modules linked in: imx8m_ddrc v4l2_h264
fsl_imx8_ddr_perf hci_uart cfg80211 imx7_media_csi(+) v4l2_mem2mem
btqca videobuf2_dma_contig videobuf2_memops btbcm videobuf2_v4l2
imx_mipi_csis etnaviv videobuf2_common gpu_sched bluetooth
snd_soc_wm8962 clk_bd718x7 ecdh_generic ecc rfkill rtc_pcf85363 at24
caam error spi_imx snd_soc_fsl_sai rtc_snvs snvs_pwrkey
snd_soc_fsl_utils imx_pcm_dma imx8mm_thermal imx_cpufreq_dt imx_sdma
ov5640 v4l2_fwnode v4l2_async videodev mc fuse drm ipv6
[   12.747111] CPU: 0 PID: 161 Comm: systemd-udevd Tainted: G        W
         6.2.0-rc3-30330-gb58b9dd3fb9e-dirty #3
[   12.757549] Hardware name: Beacon EmbeddedWorks i.MX8M Mini
Development Kit (DT)
[   12.764945] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   12.771908] pc : imx7_csi_init_cfg+0x70/0x9c [imx7_media_csi]
[   12.777674] lr : imx7_csi_probe+0x26c/0x380 [imx7_media_csi]
[   12.783344] sp : ffff80000afd3900
[   12.786655] x29: ffff80000afd3900 x28: 0000000000000000 x27: 0000000000000000
[   12.793796] x26: ffff8000012ae180 x25: 0000000000000001 x24: ffff000005bb8340
[   12.800934] x23: ffff000005bb8450 x22: ffff000005bb80a8 x21: ffff8000012ac4f8
[   12.808072] x20: 0000000000000000 x19: ffff000005bb8080 x18: ffffffffffffffff
[   12.815215] x17: 0000000000000000 x16: 0000000000000000 x15: 64656d3d4d455453
[   12.822354] x14: ffff80000a56d220 x13: 0000000000000040 x12: 0000000000000228
[   12.829497] x11: 0000000000000000 x10: 0000000000000000 x9 : 000001e000000280
[   12.836636] x8 : 0000000100002006 x7 : 0002000100000008 x6 : 0000000000000002
[   12.843778] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000050
[   12.850918] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000005bb8450
[   12.858058] Call trace:
[   12.860503]  imx7_csi_init_cfg+0x70/0x9c [imx7_media_csi]
[   12.865915]  platform_probe+0x68/0xe0
[   12.869584]  really_probe+0xbc/0x2dc
[   12.873160]  __driver_probe_device+0x78/0xe0
[   12.877434]  driver_probe_device+0xd8/0x160
[   12.881618]  __driver_attach+0x94/0x19c
[   12.885456]  bus_for_each_dev+0x70/0xd0
[   12.889293]  driver_attach+0x24/0x30
[   12.892868]  bus_add_driver+0x154/0x20c
[   12.896707]  driver_register+0x78/0x130
[   12.900545]  __platform_driver_register+0x28/0x34
[   12.905255]  imx7_csi_driver_init+0x20/0x1000 [imx7_media_csi]
[   12.911099]  do_one_initcall+0x50/0x1d0
[   12.914937]  do_init_module+0x48/0x1d0
[   12.918691]  load_module+0x193c/0x1cb0
[   12.922442]  __do_sys_finit_module+0xa8/0x100
[   12.926802]  __arm64_sys_finit_module+0x20/0x30
[   12.931336]  invoke_syscall+0x48/0x114
[   12.935090]  el0_svc_common.constprop.0+0xd4/0xfc
[   12.939796]  do_el0_svc+0x3c/0xc0
[   12.943114]  el0_svc+0x2c/0x84
[   12.946174]  el0t_64_sync_handler+0xbc/0x140
[   12.950446]  el0t_64_sync+0x190/0x194
[   12.954114] Code: b5fffe81 d4210000 d2800002 91014063 (a9002049)
[   12.960209] ---[ end trace 0000000000000000 ]---


The media information:

root@beacon-imx8mm-kit:~# media-ctl -p
Media controller API version 6.2.0

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

Device topology
- entity 1: csi (2 pads, 1 link)
            type V4L2 subdev subtype Unknown flags 0
pad0: Sink
pad1: Source
-> "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]

I confirmed the ov5640 camera enumerated:

I'm going to roll back this latest series to verify whether or not
this series caused the splat.

adam

>  drivers/media/platform/nxp/imx7-media-csi.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
> index be3c1494cfb3..e96bee4e5921 100644
> --- a/drivers/media/platform/nxp/imx7-media-csi.c
> +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> @@ -1598,17 +1598,15 @@ static struct imx7_csi_vb2_buffer *imx7_csi_video_next_buf(struct imx7_csi *csi)
>
>  static int imx7_csi_video_init_format(struct imx7_csi *csi)
>  {
> -       struct v4l2_subdev_format fmt_src = {
> -               .pad = IMX7_CSI_PAD_SRC,
> -               .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> -       };
> -       fmt_src.format.code = IMX7_CSI_DEF_MBUS_CODE;
> -       fmt_src.format.width = IMX7_CSI_DEF_PIX_WIDTH;
> -       fmt_src.format.height = IMX7_CSI_DEF_PIX_HEIGHT;
> +       struct v4l2_mbus_framefmt format = { };
>
> -       imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &fmt_src.format, NULL);
> -       csi->vdev_compose.width = fmt_src.format.width;
> -       csi->vdev_compose.height = fmt_src.format.height;
> +       format.code = IMX7_CSI_DEF_MBUS_CODE;
> +       format.width = IMX7_CSI_DEF_PIX_WIDTH;
> +       format.height = IMX7_CSI_DEF_PIX_HEIGHT;
> +
> +       imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &format, NULL);
> +       csi->vdev_compose.width = format.width;
> +       csi->vdev_compose.height = format.height;
>
>         csi->vdev_cc = imx7_csi_find_pixel_format(csi->vdev_fmt.pixelformat);
>
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()

[Laurent Pinchart]

Hi Adam,

On Thu, Jan 26, 2023 at 09:19:28PM -0600, Adam Ford wrote:
> On Thu, Jan 26, 2023 at 8:27 PM Laurent Pinchart wrote:
> >
> > The imx7_csi_video_init_format() function instantiates a
> > v4l2_subdev_format on the stack, to only use the .format field of that
> > structure. Replace it with a v4l2_mbus_framefmt instance.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> 
> With this series and the CSIS series you posted earlier, I get a ton
> of splat and the ov5640 camera doesn't appear in the media information
> with media-ctrl -p

Oops :-S Thank a lot for testing.

>    12.386980] lr : imx7_csi_probe+0x26c/0x380 [imx7_media_csi]
> [   12.387010] sp : ffff80000afd3900
> [   12.387013] x29: ffff80000afd3900 x28: 0000000000000000 x27: 0000000000000000
> [   12.387025] x26: ffff8000012ae180 x25: 0000000000000001 x24: ffff000005bb8340
> [   12.387033] x23: ffff000005bb8450 x22: ffff000005bb80a8 x21: ffff8000012ac4f8
> [   12.387040] x20: 0000000000000000 x19: ffff000005bb8080 x18: ffffffffffffffff
> [   12.387048] x17: 0000000000000000
> [   12.393690] Bluetooth: HCI UART protocol QCA registered
> [   12.397321]  x16: 0000000000000000 x15: 64656d3d4d455453
> [   12.397327] x14: ffff80000a56d220 x13: 0000000000000040 x12: 0000000000000228
> [   12.397335] x11: 0000000000000000 x10: 0000000000000000 x9 : 000001e000000280
> [   12.397342] x8 : 0000000100002006 x7 : 0002000100000008 x6 : 0000000000000002
> [   12.397350] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> [   12.397357] x2 : ffff000005bb84c8 x1 : 0000000000000000 x0 : ffff000005bb8450
> [   12.397365] Call trace:
> [   12.397368]  imx7_csi_init_cfg+0x64/0x9c [imx7_media_csi]
> [   12.397385]  platform_probe+0x68/0xe0
> [   12.406436] Bluetooth: HCI UART protocol Marvell registered
> [   12.413735]  really_probe+0xbc/0x2dc
> [   12.413743]  __driver_probe_device+0x78/0xe0
> [   12.413748]  driver_probe_device+0xd8/0x160
> [   12.413754]  __driver_attach+0x94/0x19c
> [   12.413759]  bus_for_each_dev+0x70/0xd0
> [   12.413764]  driver_attach+0x24/0x30
> [   12.413769]  bus_add_driver+0x154/0x20c
> [   12.413774]  driver_register+0x78/0x130
> [   12.413780]  __platform_driver_register+0x28/0x34
> [   12.413786]  imx7_csi_driver_init+0x20/0x1000 [imx7_media_csi]
> [   12.413803]  do_one_initcall+0x50/0x1d0
> [   12.413810]  do_init_module+0x48/0x1d0
> [   12.413817]  load_module+0x193c/0x1cb0
> [   12.413822]  __do_sys_finit_module+0xa8/0x100
> [   12.413828]  __arm64_sys_finit_module+0x20/0x30
> [   12.413834]  invoke_syscall+0x48/0x114
> [   12.413842]  el0_svc_common.constprop.0+0xd4/0xfc
> [   12.413848]  do_el0_svc+0x3c/0xc0
> [   12.413854]  el0_svc+0x2c/0x84
> [   12.413863]  el0t_64_sync_handler+0xbc/0x140
> [   12.624336]  el0t_64_sync+0x190/0x194
> [   12.628002] ---[ end trace 0000000000000000 ]---
> [   12.633012] Unable to handle kernel NULL pointer dereference at
> virtual address 0000000000000000
> [   12.641948] Mem abort info:
> [   12.644812]   ESR = 0x0000000096000044
> [   12.648652]   EC = 0x25: DABT (current EL), IL = 32 bits
> [   12.654047]   SET = 0, FnV = 0
> [   12.654923] imx8m-ddrc-devfreq 3d400000.memory-controller: failed
> to init firmware freq info: -19
> [   12.657176]   EA = 0, S1PTW = 0
> [   12.669382]   FSC = 0x04: level 0 translation fault
> [   12.674349] Data abort info:
> [   12.677284]   ISV = 0, ISS = 0x00000044
> [   12.681169]   CM = 0, WnR = 1
> [   12.684189] user pgtable: 4k pages, 48-bit VAs, pgdp=000000004597e000
> [   12.690698] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
> [   12.697570] Internal error: Oops: 0000000096000044 [#1] PREEMPT SMP
> [   12.703848] Modules linked in: imx8m_ddrc v4l2_h264
> fsl_imx8_ddr_perf hci_uart cfg80211 imx7_media_csi(+) v4l2_mem2mem
> btqca videobuf2_dma_contig videobuf2_memops btbcm videobuf2_v4l2
> imx_mipi_csis etnaviv videobuf2_common gpu_sched bluetooth
> snd_soc_wm8962 clk_bd718x7 ecdh_generic ecc rfkill rtc_pcf85363 at24
> caam error spi_imx snd_soc_fsl_sai rtc_snvs snvs_pwrkey
> snd_soc_fsl_utils imx_pcm_dma imx8mm_thermal imx_cpufreq_dt imx_sdma
> ov5640 v4l2_fwnode v4l2_async videodev mc fuse drm ipv6
> [   12.747111] CPU: 0 PID: 161 Comm: systemd-udevd Tainted: G        W
>          6.2.0-rc3-30330-gb58b9dd3fb9e-dirty #3
> [   12.757549] Hardware name: Beacon EmbeddedWorks i.MX8M Mini
> Development Kit (DT)
> [   12.764945] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [   12.771908] pc : imx7_csi_init_cfg+0x70/0x9c [imx7_media_csi]
> [   12.777674] lr : imx7_csi_probe+0x26c/0x380 [imx7_media_csi]
> [   12.783344] sp : ffff80000afd3900
> [   12.786655] x29: ffff80000afd3900 x28: 0000000000000000 x27: 0000000000000000
> [   12.793796] x26: ffff8000012ae180 x25: 0000000000000001 x24: ffff000005bb8340
> [   12.800934] x23: ffff000005bb8450 x22: ffff000005bb80a8 x21: ffff8000012ac4f8
> [   12.808072] x20: 0000000000000000 x19: ffff000005bb8080 x18: ffffffffffffffff
> [   12.815215] x17: 0000000000000000 x16: 0000000000000000 x15: 64656d3d4d455453
> [   12.822354] x14: ffff80000a56d220 x13: 0000000000000040 x12: 0000000000000228
> [   12.829497] x11: 0000000000000000 x10: 0000000000000000 x9 : 000001e000000280
> [   12.836636] x8 : 0000000100002006 x7 : 0002000100000008 x6 : 0000000000000002
> [   12.843778] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000050
> [   12.850918] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000005bb8450
> [   12.858058] Call trace:
> [   12.860503]  imx7_csi_init_cfg+0x70/0x9c [imx7_media_csi]
> [   12.865915]  platform_probe+0x68/0xe0
> [   12.869584]  really_probe+0xbc/0x2dc
> [   12.873160]  __driver_probe_device+0x78/0xe0
> [   12.877434]  driver_probe_device+0xd8/0x160
> [   12.881618]  __driver_attach+0x94/0x19c
> [   12.885456]  bus_for_each_dev+0x70/0xd0
> [   12.889293]  driver_attach+0x24/0x30
> [   12.892868]  bus_add_driver+0x154/0x20c
> [   12.896707]  driver_register+0x78/0x130
> [   12.900545]  __platform_driver_register+0x28/0x34
> [   12.905255]  imx7_csi_driver_init+0x20/0x1000 [imx7_media_csi]
> [   12.911099]  do_one_initcall+0x50/0x1d0
> [   12.914937]  do_init_module+0x48/0x1d0
> [   12.918691]  load_module+0x193c/0x1cb0
> [   12.922442]  __do_sys_finit_module+0xa8/0x100
> [   12.926802]  __arm64_sys_finit_module+0x20/0x30
> [   12.931336]  invoke_syscall+0x48/0x114
> [   12.935090]  el0_svc_common.constprop.0+0xd4/0xfc
> [   12.939796]  do_el0_svc+0x3c/0xc0
> [   12.943114]  el0_svc+0x2c/0x84
> [   12.946174]  el0t_64_sync_handler+0xbc/0x140
> [   12.950446]  el0t_64_sync+0x190/0x194
> [   12.954114] Code: b5fffe81 d4210000 d2800002 91014063 (a9002049)
> [   12.960209] ---[ end trace 0000000000000000 ]---

It appears I forgot something. Could you try with the following change ?

diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
index 9275447987d1..81d5f08b02d1 100644
--- a/drivers/media/platform/nxp/imx7-media-csi.c
+++ b/drivers/media/platform/nxp/imx7-media-csi.c
@@ -2259,21 +2259,15 @@ static int imx7_csi_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;

-	/* Set the default mbus formats. */
-	ret = imx7_csi_init_cfg(&csi->sd, NULL);
-	if (ret)
-		goto media_cleanup;
-
 	ret = imx7_csi_async_register(csi);
 	if (ret)
-		goto subdev_notifier_cleanup;
+		goto err_async_unregister;

 	return 0;

-subdev_notifier_cleanup:
+err_async_unregister:
 	v4l2_async_nf_unregister(&csi->notifier);
 	v4l2_async_nf_cleanup(&csi->notifier);
-media_cleanup:
 	imx7_csi_media_cleanup(csi);

 	return ret;

> The media information:
> 
> root@beacon-imx8mm-kit:~# media-ctl -p
> Media controller API version 6.2.0
> 
> Media device information
> ------------------------
> driver          imx7-csi
> model           imx-media
> serial
> bus info        platform:32e20000.csi
> hw revision     0x0
> driver version  6.2.0
> 
> Device topology
> - entity 1: csi (2 pads, 1 link)
>             type V4L2 subdev subtype Unknown flags 0
> pad0: Sink
> pad1: Source
> -> "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]
> 
> I confirmed the ov5640 camera enumerated:
> 
> I'm going to roll back this latest series to verify whether or not
> this series caused the splat.
> 
> adam
> 
> >  drivers/media/platform/nxp/imx7-media-csi.c | 18 ++++++++----------
> >  1 file changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
> > index be3c1494cfb3..e96bee4e5921 100644
> > --- a/drivers/media/platform/nxp/imx7-media-csi.c
> > +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> > @@ -1598,17 +1598,15 @@ static struct imx7_csi_vb2_buffer *imx7_csi_video_next_buf(struct imx7_csi *csi)
> >
> >  static int imx7_csi_video_init_format(struct imx7_csi *csi)
> >  {
> > -       struct v4l2_subdev_format fmt_src = {
> > -               .pad = IMX7_CSI_PAD_SRC,
> > -               .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > -       };
> > -       fmt_src.format.code = IMX7_CSI_DEF_MBUS_CODE;
> > -       fmt_src.format.width = IMX7_CSI_DEF_PIX_WIDTH;
> > -       fmt_src.format.height = IMX7_CSI_DEF_PIX_HEIGHT;
> > +       struct v4l2_mbus_framefmt format = { };
> >
> > -       imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &fmt_src.format, NULL);
> > -       csi->vdev_compose.width = fmt_src.format.width;
> > -       csi->vdev_compose.height = fmt_src.format.height;
> > +       format.code = IMX7_CSI_DEF_MBUS_CODE;
> > +       format.width = IMX7_CSI_DEF_PIX_WIDTH;
> > +       format.height = IMX7_CSI_DEF_PIX_HEIGHT;
> > +
> > +       imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &format, NULL);
> > +       csi->vdev_compose.width = format.width;
> > +       csi->vdev_compose.height = format.height;
> >
> >         csi->vdev_cc = imx7_csi_find_pixel_format(csi->vdev_fmt.pixelformat);
> >

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()

[Adam Ford]

On Fri, Jan 27, 2023 at 12:57 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Adam,
>
> On Thu, Jan 26, 2023 at 09:19:28PM -0600, Adam Ford wrote:
> > On Thu, Jan 26, 2023 at 8:27 PM Laurent Pinchart wrote:
> > >
> > > The imx7_csi_video_init_format() function instantiates a
> > > v4l2_subdev_format on the stack, to only use the .format field of that
> > > structure. Replace it with a v4l2_mbus_framefmt instance.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> >
> > With this series and the CSIS series you posted earlier, I get a ton
> > of splat and the ov5640 camera doesn't appear in the media information
> > with media-ctrl -p
>
> Oops :-S Thank a lot for testing.
>
> >    12.386980] lr : imx7_csi_probe+0x26c/0x380 [imx7_media_csi]
> > [   12.387010] sp : ffff80000afd3900
> > [   12.387013] x29: ffff80000afd3900 x28: 0000000000000000 x27: 0000000000000000
> > [   12.387025] x26: ffff8000012ae180 x25: 0000000000000001 x24: ffff000005bb8340
> > [   12.387033] x23: ffff000005bb8450 x22: ffff000005bb80a8 x21: ffff8000012ac4f8
> > [   12.387040] x20: 0000000000000000 x19: ffff000005bb8080 x18: ffffffffffffffff
> > [   12.387048] x17: 0000000000000000
> > [   12.393690] Bluetooth: HCI UART protocol QCA registered
> > [   12.397321]  x16: 0000000000000000 x15: 64656d3d4d455453
> > [   12.397327] x14: ffff80000a56d220 x13: 0000000000000040 x12: 0000000000000228
> > [   12.397335] x11: 0000000000000000 x10: 0000000000000000 x9 : 000001e000000280
> > [   12.397342] x8 : 0000000100002006 x7 : 0002000100000008 x6 : 0000000000000002
> > [   12.397350] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> > [   12.397357] x2 : ffff000005bb84c8 x1 : 0000000000000000 x0 : ffff000005bb8450
> > [   12.397365] Call trace:
> > [   12.397368]  imx7_csi_init_cfg+0x64/0x9c [imx7_media_csi]
> > [   12.397385]  platform_probe+0x68/0xe0
> > [   12.406436] Bluetooth: HCI UART protocol Marvell registered
> > [   12.413735]  really_probe+0xbc/0x2dc
> > [   12.413743]  __driver_probe_device+0x78/0xe0
> > [   12.413748]  driver_probe_device+0xd8/0x160
> > [   12.413754]  __driver_attach+0x94/0x19c
> > [   12.413759]  bus_for_each_dev+0x70/0xd0
> > [   12.413764]  driver_attach+0x24/0x30
> > [   12.413769]  bus_add_driver+0x154/0x20c
> > [   12.413774]  driver_register+0x78/0x130
> > [   12.413780]  __platform_driver_register+0x28/0x34
> > [   12.413786]  imx7_csi_driver_init+0x20/0x1000 [imx7_media_csi]
> > [   12.413803]  do_one_initcall+0x50/0x1d0
> > [   12.413810]  do_init_module+0x48/0x1d0
> > [   12.413817]  load_module+0x193c/0x1cb0
> > [   12.413822]  __do_sys_finit_module+0xa8/0x100
> > [   12.413828]  __arm64_sys_finit_module+0x20/0x30
> > [   12.413834]  invoke_syscall+0x48/0x114
> > [   12.413842]  el0_svc_common.constprop.0+0xd4/0xfc
> > [   12.413848]  do_el0_svc+0x3c/0xc0
> > [   12.413854]  el0_svc+0x2c/0x84
> > [   12.413863]  el0t_64_sync_handler+0xbc/0x140
> > [   12.624336]  el0t_64_sync+0x190/0x194
> > [   12.628002] ---[ end trace 0000000000000000 ]---
> > [   12.633012] Unable to handle kernel NULL pointer dereference at
> > virtual address 0000000000000000
> > [   12.641948] Mem abort info:
> > [   12.644812]   ESR = 0x0000000096000044
> > [   12.648652]   EC = 0x25: DABT (current EL), IL = 32 bits
> > [   12.654047]   SET = 0, FnV = 0
> > [   12.654923] imx8m-ddrc-devfreq 3d400000.memory-controller: failed
> > to init firmware freq info: -19
> > [   12.657176]   EA = 0, S1PTW = 0
> > [   12.669382]   FSC = 0x04: level 0 translation fault
> > [   12.674349] Data abort info:
> > [   12.677284]   ISV = 0, ISS = 0x00000044
> > [   12.681169]   CM = 0, WnR = 1
> > [   12.684189] user pgtable: 4k pages, 48-bit VAs, pgdp=000000004597e000
> > [   12.690698] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
> > [   12.697570] Internal error: Oops: 0000000096000044 [#1] PREEMPT SMP
> > [   12.703848] Modules linked in: imx8m_ddrc v4l2_h264
> > fsl_imx8_ddr_perf hci_uart cfg80211 imx7_media_csi(+) v4l2_mem2mem
> > btqca videobuf2_dma_contig videobuf2_memops btbcm videobuf2_v4l2
> > imx_mipi_csis etnaviv videobuf2_common gpu_sched bluetooth
> > snd_soc_wm8962 clk_bd718x7 ecdh_generic ecc rfkill rtc_pcf85363 at24
> > caam error spi_imx snd_soc_fsl_sai rtc_snvs snvs_pwrkey
> > snd_soc_fsl_utils imx_pcm_dma imx8mm_thermal imx_cpufreq_dt imx_sdma
> > ov5640 v4l2_fwnode v4l2_async videodev mc fuse drm ipv6
> > [   12.747111] CPU: 0 PID: 161 Comm: systemd-udevd Tainted: G        W
> >          6.2.0-rc3-30330-gb58b9dd3fb9e-dirty #3
> > [   12.757549] Hardware name: Beacon EmbeddedWorks i.MX8M Mini
> > Development Kit (DT)
> > [   12.764945] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > [   12.771908] pc : imx7_csi_init_cfg+0x70/0x9c [imx7_media_csi]
> > [   12.777674] lr : imx7_csi_probe+0x26c/0x380 [imx7_media_csi]
> > [   12.783344] sp : ffff80000afd3900
> > [   12.786655] x29: ffff80000afd3900 x28: 0000000000000000 x27: 0000000000000000
> > [   12.793796] x26: ffff8000012ae180 x25: 0000000000000001 x24: ffff000005bb8340
> > [   12.800934] x23: ffff000005bb8450 x22: ffff000005bb80a8 x21: ffff8000012ac4f8
> > [   12.808072] x20: 0000000000000000 x19: ffff000005bb8080 x18: ffffffffffffffff
> > [   12.815215] x17: 0000000000000000 x16: 0000000000000000 x15: 64656d3d4d455453
> > [   12.822354] x14: ffff80000a56d220 x13: 0000000000000040 x12: 0000000000000228
> > [   12.829497] x11: 0000000000000000 x10: 0000000000000000 x9 : 000001e000000280
> > [   12.836636] x8 : 0000000100002006 x7 : 0002000100000008 x6 : 0000000000000002
> > [   12.843778] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000050
> > [   12.850918] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000005bb8450
> > [   12.858058] Call trace:
> > [   12.860503]  imx7_csi_init_cfg+0x70/0x9c [imx7_media_csi]
> > [   12.865915]  platform_probe+0x68/0xe0
> > [   12.869584]  really_probe+0xbc/0x2dc
> > [   12.873160]  __driver_probe_device+0x78/0xe0
> > [   12.877434]  driver_probe_device+0xd8/0x160
> > [   12.881618]  __driver_attach+0x94/0x19c
> > [   12.885456]  bus_for_each_dev+0x70/0xd0
> > [   12.889293]  driver_attach+0x24/0x30
> > [   12.892868]  bus_add_driver+0x154/0x20c
> > [   12.896707]  driver_register+0x78/0x130
> > [   12.900545]  __platform_driver_register+0x28/0x34
> > [   12.905255]  imx7_csi_driver_init+0x20/0x1000 [imx7_media_csi]
> > [   12.911099]  do_one_initcall+0x50/0x1d0
> > [   12.914937]  do_init_module+0x48/0x1d0
> > [   12.918691]  load_module+0x193c/0x1cb0
> > [   12.922442]  __do_sys_finit_module+0xa8/0x100
> > [   12.926802]  __arm64_sys_finit_module+0x20/0x30
> > [   12.931336]  invoke_syscall+0x48/0x114
> > [   12.935090]  el0_svc_common.constprop.0+0xd4/0xfc
> > [   12.939796]  do_el0_svc+0x3c/0xc0
> > [   12.943114]  el0_svc+0x2c/0x84
> > [   12.946174]  el0t_64_sync_handler+0xbc/0x140
> > [   12.950446]  el0t_64_sync+0x190/0x194
> > [   12.954114] Code: b5fffe81 d4210000 d2800002 91014063 (a9002049)
> > [   12.960209] ---[ end trace 0000000000000000 ]---
>
> It appears I forgot something. Could you try with the following change ?
>
> diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
> index 9275447987d1..81d5f08b02d1 100644
> --- a/drivers/media/platform/nxp/imx7-media-csi.c
> +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> @@ -2259,21 +2259,15 @@ static int imx7_csi_probe(struct platform_device *pdev)
>         if (ret)
>                 return ret;
>
> -       /* Set the default mbus formats. */
> -       ret = imx7_csi_init_cfg(&csi->sd, NULL);
> -       if (ret)
> -               goto media_cleanup;
> -
>         ret = imx7_csi_async_register(csi);
>         if (ret)
> -               goto subdev_notifier_cleanup;
> +               goto err_async_unregister;
>
>         return 0;
>
> -subdev_notifier_cleanup:
> +err_async_unregister:
>         v4l2_async_nf_unregister(&csi->notifier);
>         v4l2_async_nf_cleanup(&csi->notifier);
> -media_cleanup:
>         imx7_csi_media_cleanup(csi);
>
>         return ret;
>


The patch seems to have fixed the splat, and the camera appears in the
media device information again, but I still cannot capture:

root@beacon-imx8mm-kit:~# gst-launch-1.0 -v v4l2src num-buffers=1 !
video/x-raw,format=UYVY,width=640,height=480 ! videoconvert ! jpegenc
! filesink location=tst2.jpg
Setting pipeline to PAUSED ...
Pipeline is live and does not need PREROLL ...
Pipeline is PREROLLED ...
Setting pipeline t[  186.993715] imx7-csi 32e20000.csi: capture format not valid
o PLAYING ...
New clock: GstSystemClock


Yet every node shows the same format:

Media device information
------------------------
driver          imx7-csi
model           imx-media
serial
bus info        platform:32e20000.csi
hw revision     0x0
driver version  6.2.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:UYVY8_1X16/640x480 field:none colorspace:srgb xfer:srgb ycbcr:601
quantization:lim-range]
<- "csis-32e30000.mipi-csi":1 [ENABLED,IMMUTABLE]
pad1: Source
[fmt:UYVY8_1X16/640x480 field:none colorspace:srgb xfer:srgb ycbcr:601
quantization:lim-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:UYVY8_1X16/640x480 field:none]
<- "ov5640 1-003c":0 [ENABLED]
pad1: Source
[fmt:UYVY8_1X16/640x480 field:none]
-> "csi":0 [ENABLED,IMMUTABLE]

- entity 15: ov5640 1-003c (1 pad, 1 link)
             type V4L2 subdev subtype Sensor flags 0
             device node name /dev/v4l-subdev2
pad0: Source
[fmt:UYVY8_1X16/640x480@1/30 field:none colorspace:srgb xfer:srgb
ycbcr:601 quantization:full-range
crop.bounds:(0,0)/2624x1964
crop:(16,14)/2592x1944]
-> "csis-32e30000.mipi-csi":0 [ENABLED]






> > The media information:
> >
> > root@beacon-imx8mm-kit:~# media-ctl -p
> > Media controller API version 6.2.0
> >
> > Media device information
> > ------------------------
> > driver          imx7-csi
> > model           imx-media
> > serial
> > bus info        platform:32e20000.csi
> > hw revision     0x0
> > driver version  6.2.0
> >
> > Device topology
> > - entity 1: csi (2 pads, 1 link)
> >             type V4L2 subdev subtype Unknown flags 0
> > pad0: Sink
> > pad1: Source
> > -> "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]
> >
> > I confirmed the ov5640 camera enumerated:
> >
> > I'm going to roll back this latest series to verify whether or not
> > this series caused the splat.
> >
> > adam
> >
> > >  drivers/media/platform/nxp/imx7-media-csi.c | 18 ++++++++----------
> > >  1 file changed, 8 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
> > > index be3c1494cfb3..e96bee4e5921 100644
> > > --- a/drivers/media/platform/nxp/imx7-media-csi.c
> > > +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> > > @@ -1598,17 +1598,15 @@ static struct imx7_csi_vb2_buffer *imx7_csi_video_next_buf(struct imx7_csi *csi)
> > >
> > >  static int imx7_csi_video_init_format(struct imx7_csi *csi)
> > >  {
> > > -       struct v4l2_subdev_format fmt_src = {
> > > -               .pad = IMX7_CSI_PAD_SRC,
> > > -               .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > > -       };
> > > -       fmt_src.format.code = IMX7_CSI_DEF_MBUS_CODE;
> > > -       fmt_src.format.width = IMX7_CSI_DEF_PIX_WIDTH;
> > > -       fmt_src.format.height = IMX7_CSI_DEF_PIX_HEIGHT;
> > > +       struct v4l2_mbus_framefmt format = { };
> > >
> > > -       imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &fmt_src.format, NULL);
> > > -       csi->vdev_compose.width = fmt_src.format.width;
> > > -       csi->vdev_compose.height = fmt_src.format.height;
> > > +       format.code = IMX7_CSI_DEF_MBUS_CODE;
> > > +       format.width = IMX7_CSI_DEF_PIX_WIDTH;
> > > +       format.height = IMX7_CSI_DEF_PIX_HEIGHT;
> > > +
> > > +       imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &format, NULL);
> > > +       csi->vdev_compose.width = format.width;
> > > +       csi->vdev_compose.height = format.height;
> > >
> > >         csi->vdev_cc = imx7_csi_find_pixel_format(csi->vdev_fmt.pixelformat);
> > >
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()

[Adam Ford]

On Fri, Jan 27, 2023 at 5:07 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Fri, Jan 27, 2023 at 12:57 AM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi Adam,
> >
> > On Thu, Jan 26, 2023 at 09:19:28PM -0600, Adam Ford wrote:
> > > On Thu, Jan 26, 2023 at 8:27 PM Laurent Pinchart wrote:
> > > >
> > > > The imx7_csi_video_init_format() function instantiates a
> > > > v4l2_subdev_format on the stack, to only use the .format field of that
> > > > structure. Replace it with a v4l2_mbus_framefmt instance.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > ---
> > >
> > > With this series and the CSIS series you posted earlier, I get a ton
> > > of splat and the ov5640 camera doesn't appear in the media information
> > > with media-ctrl -p
> >
> > Oops :-S Thank a lot for testing.
> >
> > >    12.386980] lr : imx7_csi_probe+0x26c/0x380 [imx7_media_csi]
> > > [   12.387010] sp : ffff80000afd3900
> > > [   12.387013] x29: ffff80000afd3900 x28: 0000000000000000 x27: 0000000000000000
> > > [   12.387025] x26: ffff8000012ae180 x25: 0000000000000001 x24: ffff000005bb8340
> > > [   12.387033] x23: ffff000005bb8450 x22: ffff000005bb80a8 x21: ffff8000012ac4f8
> > > [   12.387040] x20: 0000000000000000 x19: ffff000005bb8080 x18: ffffffffffffffff
> > > [   12.387048] x17: 0000000000000000
> > > [   12.393690] Bluetooth: HCI UART protocol QCA registered
> > > [   12.397321]  x16: 0000000000000000 x15: 64656d3d4d455453
> > > [   12.397327] x14: ffff80000a56d220 x13: 0000000000000040 x12: 0000000000000228
> > > [   12.397335] x11: 0000000000000000 x10: 0000000000000000 x9 : 000001e000000280
> > > [   12.397342] x8 : 0000000100002006 x7 : 0002000100000008 x6 : 0000000000000002
> > > [   12.397350] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> > > [   12.397357] x2 : ffff000005bb84c8 x1 : 0000000000000000 x0 : ffff000005bb8450
> > > [   12.397365] Call trace:
> > > [   12.397368]  imx7_csi_init_cfg+0x64/0x9c [imx7_media_csi]
> > > [   12.397385]  platform_probe+0x68/0xe0
> > > [   12.406436] Bluetooth: HCI UART protocol Marvell registered
> > > [   12.413735]  really_probe+0xbc/0x2dc
> > > [   12.413743]  __driver_probe_device+0x78/0xe0
> > > [   12.413748]  driver_probe_device+0xd8/0x160
> > > [   12.413754]  __driver_attach+0x94/0x19c
> > > [   12.413759]  bus_for_each_dev+0x70/0xd0
> > > [   12.413764]  driver_attach+0x24/0x30
> > > [   12.413769]  bus_add_driver+0x154/0x20c
> > > [   12.413774]  driver_register+0x78/0x130
> > > [   12.413780]  __platform_driver_register+0x28/0x34
> > > [   12.413786]  imx7_csi_driver_init+0x20/0x1000 [imx7_media_csi]
> > > [   12.413803]  do_one_initcall+0x50/0x1d0
> > > [   12.413810]  do_init_module+0x48/0x1d0
> > > [   12.413817]  load_module+0x193c/0x1cb0
> > > [   12.413822]  __do_sys_finit_module+0xa8/0x100
> > > [   12.413828]  __arm64_sys_finit_module+0x20/0x30
> > > [   12.413834]  invoke_syscall+0x48/0x114
> > > [   12.413842]  el0_svc_common.constprop.0+0xd4/0xfc
> > > [   12.413848]  do_el0_svc+0x3c/0xc0
> > > [   12.413854]  el0_svc+0x2c/0x84
> > > [   12.413863]  el0t_64_sync_handler+0xbc/0x140
> > > [   12.624336]  el0t_64_sync+0x190/0x194
> > > [   12.628002] ---[ end trace 0000000000000000 ]---
> > > [   12.633012] Unable to handle kernel NULL pointer dereference at
> > > virtual address 0000000000000000
> > > [   12.641948] Mem abort info:
> > > [   12.644812]   ESR = 0x0000000096000044
> > > [   12.648652]   EC = 0x25: DABT (current EL), IL = 32 bits
> > > [   12.654047]   SET = 0, FnV = 0
> > > [   12.654923] imx8m-ddrc-devfreq 3d400000.memory-controller: failed
> > > to init firmware freq info: -19
> > > [   12.657176]   EA = 0, S1PTW = 0
> > > [   12.669382]   FSC = 0x04: level 0 translation fault
> > > [   12.674349] Data abort info:
> > > [   12.677284]   ISV = 0, ISS = 0x00000044
> > > [   12.681169]   CM = 0, WnR = 1
> > > [   12.684189] user pgtable: 4k pages, 48-bit VAs, pgdp=000000004597e000
> > > [   12.690698] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
> > > [   12.697570] Internal error: Oops: 0000000096000044 [#1] PREEMPT SMP
> > > [   12.703848] Modules linked in: imx8m_ddrc v4l2_h264
> > > fsl_imx8_ddr_perf hci_uart cfg80211 imx7_media_csi(+) v4l2_mem2mem
> > > btqca videobuf2_dma_contig videobuf2_memops btbcm videobuf2_v4l2
> > > imx_mipi_csis etnaviv videobuf2_common gpu_sched bluetooth
> > > snd_soc_wm8962 clk_bd718x7 ecdh_generic ecc rfkill rtc_pcf85363 at24
> > > caam error spi_imx snd_soc_fsl_sai rtc_snvs snvs_pwrkey
> > > snd_soc_fsl_utils imx_pcm_dma imx8mm_thermal imx_cpufreq_dt imx_sdma
> > > ov5640 v4l2_fwnode v4l2_async videodev mc fuse drm ipv6
> > > [   12.747111] CPU: 0 PID: 161 Comm: systemd-udevd Tainted: G        W
> > >          6.2.0-rc3-30330-gb58b9dd3fb9e-dirty #3
> > > [   12.757549] Hardware name: Beacon EmbeddedWorks i.MX8M Mini
> > > Development Kit (DT)
> > > [   12.764945] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > [   12.771908] pc : imx7_csi_init_cfg+0x70/0x9c [imx7_media_csi]
> > > [   12.777674] lr : imx7_csi_probe+0x26c/0x380 [imx7_media_csi]
> > > [   12.783344] sp : ffff80000afd3900
> > > [   12.786655] x29: ffff80000afd3900 x28: 0000000000000000 x27: 0000000000000000
> > > [   12.793796] x26: ffff8000012ae180 x25: 0000000000000001 x24: ffff000005bb8340
> > > [   12.800934] x23: ffff000005bb8450 x22: ffff000005bb80a8 x21: ffff8000012ac4f8
> > > [   12.808072] x20: 0000000000000000 x19: ffff000005bb8080 x18: ffffffffffffffff
> > > [   12.815215] x17: 0000000000000000 x16: 0000000000000000 x15: 64656d3d4d455453
> > > [   12.822354] x14: ffff80000a56d220 x13: 0000000000000040 x12: 0000000000000228
> > > [   12.829497] x11: 0000000000000000 x10: 0000000000000000 x9 : 000001e000000280
> > > [   12.836636] x8 : 0000000100002006 x7 : 0002000100000008 x6 : 0000000000000002
> > > [   12.843778] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000050
> > > [   12.850918] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000005bb8450
> > > [   12.858058] Call trace:
> > > [   12.860503]  imx7_csi_init_cfg+0x70/0x9c [imx7_media_csi]
> > > [   12.865915]  platform_probe+0x68/0xe0
> > > [   12.869584]  really_probe+0xbc/0x2dc
> > > [   12.873160]  __driver_probe_device+0x78/0xe0
> > > [   12.877434]  driver_probe_device+0xd8/0x160
> > > [   12.881618]  __driver_attach+0x94/0x19c
> > > [   12.885456]  bus_for_each_dev+0x70/0xd0
> > > [   12.889293]  driver_attach+0x24/0x30
> > > [   12.892868]  bus_add_driver+0x154/0x20c
> > > [   12.896707]  driver_register+0x78/0x130
> > > [   12.900545]  __platform_driver_register+0x28/0x34
> > > [   12.905255]  imx7_csi_driver_init+0x20/0x1000 [imx7_media_csi]
> > > [   12.911099]  do_one_initcall+0x50/0x1d0
> > > [   12.914937]  do_init_module+0x48/0x1d0
> > > [   12.918691]  load_module+0x193c/0x1cb0
> > > [   12.922442]  __do_sys_finit_module+0xa8/0x100
> > > [   12.926802]  __arm64_sys_finit_module+0x20/0x30
> > > [   12.931336]  invoke_syscall+0x48/0x114
> > > [   12.935090]  el0_svc_common.constprop.0+0xd4/0xfc
> > > [   12.939796]  do_el0_svc+0x3c/0xc0
> > > [   12.943114]  el0_svc+0x2c/0x84
> > > [   12.946174]  el0t_64_sync_handler+0xbc/0x140
> > > [   12.950446]  el0t_64_sync+0x190/0x194
> > > [   12.954114] Code: b5fffe81 d4210000 d2800002 91014063 (a9002049)
> > > [   12.960209] ---[ end trace 0000000000000000 ]---
> >
> > It appears I forgot something. Could you try with the following change ?
> >
> > diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
> > index 9275447987d1..81d5f08b02d1 100644
> > --- a/drivers/media/platform/nxp/imx7-media-csi.c
> > +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> > @@ -2259,21 +2259,15 @@ static int imx7_csi_probe(struct platform_device *pdev)
> >         if (ret)
> >                 return ret;
> >
> > -       /* Set the default mbus formats. */
> > -       ret = imx7_csi_init_cfg(&csi->sd, NULL);
> > -       if (ret)
> > -               goto media_cleanup;
> > -
> >         ret = imx7_csi_async_register(csi);
> >         if (ret)
> > -               goto subdev_notifier_cleanup;
> > +               goto err_async_unregister;
> >
> >         return 0;
> >
> > -subdev_notifier_cleanup:
> > +err_async_unregister:
> >         v4l2_async_nf_unregister(&csi->notifier);
> >         v4l2_async_nf_cleanup(&csi->notifier);
> > -media_cleanup:
> >         imx7_csi_media_cleanup(csi);
> >
> >         return ret;
> >
>
>
> The patch seems to have fixed the splat, and the camera appears in the
> media device information again, but I still cannot capture:
>
> root@beacon-imx8mm-kit:~# gst-launch-1.0 -v v4l2src num-buffers=1 !
> video/x-raw,format=UYVY,width=640,height=480 ! videoconvert ! jpegenc
> ! filesink location=tst2.jpg
> Setting pipeline to PAUSED ...
> Pipeline is live and does not need PREROLL ...
> Pipeline is PREROLLED ...
> Setting pipeline t[  186.993715] imx7-csi 32e20000.csi: capture format not valid
> o PLAYING ...
> New clock: GstSystemClock
>
>
> Yet every node shows the same format:
>
> Media device information
> ------------------------
> driver          imx7-csi
> model           imx-media
> serial
> bus info        platform:32e20000.csi
> hw revision     0x0
> driver version  6.2.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:UYVY8_1X16/640x480 field:none colorspace:srgb xfer:srgb ycbcr:601
> quantization:lim-range]
> <- "csis-32e30000.mipi-csi":1 [ENABLED,IMMUTABLE]
> pad1: Source
> [fmt:UYVY8_1X16/640x480 field:none colorspace:srgb xfer:srgb ycbcr:601
> quantization:lim-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:UYVY8_1X16/640x480 field:none]
> <- "ov5640 1-003c":0 [ENABLED]
> pad1: Source
> [fmt:UYVY8_1X16/640x480 field:none]
> -> "csi":0 [ENABLED,IMMUTABLE]
>
> - entity 15: ov5640 1-003c (1 pad, 1 link)
>              type V4L2 subdev subtype Sensor flags 0
>              device node name /dev/v4l-subdev2
> pad0: Source
> [fmt:UYVY8_1X16/640x480@1/30 field:none colorspace:srgb xfer:srgb
> ycbcr:601 quantization:full-range
> crop.bounds:(0,0)/2624x1964
> crop:(16,14)/2592x1944]
> -> "csis-32e30000.mipi-csi":0 [ENABLED]
>
>
>

After a little debugging, i found that imx7_csi_video_validate_fmt is
returning EINVAL.  Unfortunately, I have to be done for the weekend,
but I can try something again on Monday.

adam

>
>
>
> > > The media information:
> > >
> > > root@beacon-imx8mm-kit:~# media-ctl -p
> > > Media controller API version 6.2.0
> > >
> > > Media device information
> > > ------------------------
> > > driver          imx7-csi
> > > model           imx-media
> > > serial
> > > bus info        platform:32e20000.csi
> > > hw revision     0x0
> > > driver version  6.2.0
> > >
> > > Device topology
> > > - entity 1: csi (2 pads, 1 link)
> > >             type V4L2 subdev subtype Unknown flags 0
> > > pad0: Sink
> > > pad1: Source
> > > -> "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]
> > >
> > > I confirmed the ov5640 camera enumerated:
> > >
> > > I'm going to roll back this latest series to verify whether or not
> > > this series caused the splat.
> > >
> > > adam
> > >
> > > >  drivers/media/platform/nxp/imx7-media-csi.c | 18 ++++++++----------
> > > >  1 file changed, 8 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
> > > > index be3c1494cfb3..e96bee4e5921 100644
> > > > --- a/drivers/media/platform/nxp/imx7-media-csi.c
> > > > +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> > > > @@ -1598,17 +1598,15 @@ static struct imx7_csi_vb2_buffer *imx7_csi_video_next_buf(struct imx7_csi *csi)
> > > >
> > > >  static int imx7_csi_video_init_format(struct imx7_csi *csi)
> > > >  {
> > > > -       struct v4l2_subdev_format fmt_src = {
> > > > -               .pad = IMX7_CSI_PAD_SRC,
> > > > -               .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > > > -       };
> > > > -       fmt_src.format.code = IMX7_CSI_DEF_MBUS_CODE;
> > > > -       fmt_src.format.width = IMX7_CSI_DEF_PIX_WIDTH;
> > > > -       fmt_src.format.height = IMX7_CSI_DEF_PIX_HEIGHT;
> > > > +       struct v4l2_mbus_framefmt format = { };
> > > >
> > > > -       imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &fmt_src.format, NULL);
> > > > -       csi->vdev_compose.width = fmt_src.format.width;
> > > > -       csi->vdev_compose.height = fmt_src.format.height;
> > > > +       format.code = IMX7_CSI_DEF_MBUS_CODE;
> > > > +       format.width = IMX7_CSI_DEF_PIX_WIDTH;
> > > > +       format.height = IMX7_CSI_DEF_PIX_HEIGHT;
> > > > +
> > > > +       imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &format, NULL);
> > > > +       csi->vdev_compose.width = format.width;
> > > > +       csi->vdev_compose.height = format.height;
> > > >
> > > >         csi->vdev_cc = imx7_csi_find_pixel_format(csi->vdev_fmt.pixelformat);
> > > >
> >
> > --
> > Regards,
> >
> > Laurent Pinchart

Re: [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state

[Martin Kepplinger]

Am Freitag, dem 27.01.2023 um 04:27 +0200 schrieb Laurent Pinchart:
> Hello,
> 
> This small series moves the imx7-mipi-csi driver to use the subdev
> active state. Patches 1/6 to 5/6 are small preparatory cleanups, with
> the main change in 6/6.
> 
> I haven't tested the series yet as I need to dig up the hardware
> first.
> Adam, you offered to test the similar imx-mipi-csis series I've sent
> recently on the i.MX8MM, would you be able to test this one at the
> same
> time ?
> 

a first test of streaming frames on imx8mq with these patches + your
inline fix works fine. just so you know. I can keep testing possible
future revisions. thanks a lot!

                          martin


ps. I know something similar needs to be done for the imx8mq mipi csi
driver.



> Laurent Pinchart (6):
>   media: imx: imx7-media-csi: Drop imx7_csi.cc field
>   media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()
>   media: imx: imx7-media-csi: Drop unneeded check when starting
>     streaming
>   media: imx: imx7-media-csi: Drop unneeded src_sd check
>   media: imx: imx7-media-csi: Drop unneeded pad checks
>   media: imx: imx7-media-csi: Use V4L2 subdev active state
> 
>  drivers/media/platform/nxp/imx7-media-csi.c | 208 ++++++------------
> --
>  1 file changed, 58 insertions(+), 150 deletions(-)
> 
> 
> base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2

Re: [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state

[Laurent Pinchart]

Hi Martin,

On Fri, Jan 27, 2023 at 12:41:26PM +0100, Martin Kepplinger wrote:
> Am Freitag, dem 27.01.2023 um 04:27 +0200 schrieb Laurent Pinchart:
> > Hello,
> > 
> > This small series moves the imx7-mipi-csi driver to use the subdev
> > active state. Patches 1/6 to 5/6 are small preparatory cleanups, with
> > the main change in 6/6.
> > 
> > I haven't tested the series yet as I need to dig up the hardware first.
> > Adam, you offered to test the similar imx-mipi-csis series I've sent
> > recently on the i.MX8MM, would you be able to test this one at the same
> > time ?
> 
> a first test of streaming frames on imx8mq with these patches + your
> inline fix works fine. just so you know. I can keep testing possible
> future revisions. thanks a lot!

Thank you for testing. I've just sent v2 and CC'ed you, could you please
test it too ? If it works, a Tested-by tag would be nice :-)

> ps. I know something similar needs to be done for the imx8mq mipi csi
> driver.

That would be nice. I won't have time to do so myself I'm afraid.

> > Laurent Pinchart (6):
> >   media: imx: imx7-media-csi: Drop imx7_csi.cc field
> >   media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()
> >   media: imx: imx7-media-csi: Drop unneeded check when starting
> >     streaming
> >   media: imx: imx7-media-csi: Drop unneeded src_sd check
> >   media: imx: imx7-media-csi: Drop unneeded pad checks
> >   media: imx: imx7-media-csi: Use V4L2 subdev active state
> > 
> >  drivers/media/platform/nxp/imx7-media-csi.c | 208 ++++++------------
> > --
> >  1 file changed, 58 insertions(+), 150 deletions(-)
> > 
> > 
> > base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()

[Laurent Pinchart]

Hi Adam,

On Fri, Jan 27, 2023 at 05:20:05AM -0600, Adam Ford wrote:
> On Fri, Jan 27, 2023 at 5:07 AM Adam Ford wrote:
> > On Fri, Jan 27, 2023 at 12:57 AM Laurent Pinchart wrote:
> > > On Thu, Jan 26, 2023 at 09:19:28PM -0600, Adam Ford wrote:
> > > > On Thu, Jan 26, 2023 at 8:27 PM Laurent Pinchart wrote:
> > > > >
> > > > > The imx7_csi_video_init_format() function instantiates a
> > > > > v4l2_subdev_format on the stack, to only use the .format field of that
> > > > > structure. Replace it with a v4l2_mbus_framefmt instance.
> > > > >
> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > ---
> > > >
> > > > With this series and the CSIS series you posted earlier, I get a ton
> > > > of splat and the ov5640 camera doesn't appear in the media information
> > > > with media-ctrl -p
> > >
> > > Oops :-S Thank a lot for testing.
> > >
> > > >    12.386980] lr : imx7_csi_probe+0x26c/0x380 [imx7_media_csi]
> > > > [   12.387010] sp : ffff80000afd3900
> > > > [   12.387013] x29: ffff80000afd3900 x28: 0000000000000000 x27: 0000000000000000
> > > > [   12.387025] x26: ffff8000012ae180 x25: 0000000000000001 x24: ffff000005bb8340
> > > > [   12.387033] x23: ffff000005bb8450 x22: ffff000005bb80a8 x21: ffff8000012ac4f8
> > > > [   12.387040] x20: 0000000000000000 x19: ffff000005bb8080 x18: ffffffffffffffff
> > > > [   12.387048] x17: 0000000000000000
> > > > [   12.393690] Bluetooth: HCI UART protocol QCA registered
> > > > [   12.397321]  x16: 0000000000000000 x15: 64656d3d4d455453
> > > > [   12.397327] x14: ffff80000a56d220 x13: 0000000000000040 x12: 0000000000000228
> > > > [   12.397335] x11: 0000000000000000 x10: 0000000000000000 x9 : 000001e000000280
> > > > [   12.397342] x8 : 0000000100002006 x7 : 0002000100000008 x6 : 0000000000000002
> > > > [   12.397350] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> > > > [   12.397357] x2 : ffff000005bb84c8 x1 : 0000000000000000 x0 : ffff000005bb8450
> > > > [   12.397365] Call trace:
> > > > [   12.397368]  imx7_csi_init_cfg+0x64/0x9c [imx7_media_csi]
> > > > [   12.397385]  platform_probe+0x68/0xe0
> > > > [   12.406436] Bluetooth: HCI UART protocol Marvell registered
> > > > [   12.413735]  really_probe+0xbc/0x2dc
> > > > [   12.413743]  __driver_probe_device+0x78/0xe0
> > > > [   12.413748]  driver_probe_device+0xd8/0x160
> > > > [   12.413754]  __driver_attach+0x94/0x19c
> > > > [   12.413759]  bus_for_each_dev+0x70/0xd0
> > > > [   12.413764]  driver_attach+0x24/0x30
> > > > [   12.413769]  bus_add_driver+0x154/0x20c
> > > > [   12.413774]  driver_register+0x78/0x130
> > > > [   12.413780]  __platform_driver_register+0x28/0x34
> > > > [   12.413786]  imx7_csi_driver_init+0x20/0x1000 [imx7_media_csi]
> > > > [   12.413803]  do_one_initcall+0x50/0x1d0
> > > > [   12.413810]  do_init_module+0x48/0x1d0
> > > > [   12.413817]  load_module+0x193c/0x1cb0
> > > > [   12.413822]  __do_sys_finit_module+0xa8/0x100
> > > > [   12.413828]  __arm64_sys_finit_module+0x20/0x30
> > > > [   12.413834]  invoke_syscall+0x48/0x114
> > > > [   12.413842]  el0_svc_common.constprop.0+0xd4/0xfc
> > > > [   12.413848]  do_el0_svc+0x3c/0xc0
> > > > [   12.413854]  el0_svc+0x2c/0x84
> > > > [   12.413863]  el0t_64_sync_handler+0xbc/0x140
> > > > [   12.624336]  el0t_64_sync+0x190/0x194
> > > > [   12.628002] ---[ end trace 0000000000000000 ]---
> > > > [   12.633012] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> > > > [   12.641948] Mem abort info:
> > > > [   12.644812]   ESR = 0x0000000096000044
> > > > [   12.648652]   EC = 0x25: DABT (current EL), IL = 32 bits
> > > > [   12.654047]   SET = 0, FnV = 0
> > > > [   12.654923] imx8m-ddrc-devfreq 3d400000.memory-controller: failed to init firmware freq info: -19
> > > > [   12.657176]   EA = 0, S1PTW = 0
> > > > [   12.669382]   FSC = 0x04: level 0 translation fault
> > > > [   12.674349] Data abort info:
> > > > [   12.677284]   ISV = 0, ISS = 0x00000044
> > > > [   12.681169]   CM = 0, WnR = 1
> > > > [   12.684189] user pgtable: 4k pages, 48-bit VAs, pgdp=000000004597e000
> > > > [   12.690698] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
> > > > [   12.697570] Internal error: Oops: 0000000096000044 [#1] PREEMPT SMP
> > > > [   12.703848] Modules linked in: imx8m_ddrc v4l2_h264
> > > > fsl_imx8_ddr_perf hci_uart cfg80211 imx7_media_csi(+) v4l2_mem2mem
> > > > btqca videobuf2_dma_contig videobuf2_memops btbcm videobuf2_v4l2
> > > > imx_mipi_csis etnaviv videobuf2_common gpu_sched bluetooth
> > > > snd_soc_wm8962 clk_bd718x7 ecdh_generic ecc rfkill rtc_pcf85363 at24
> > > > caam error spi_imx snd_soc_fsl_sai rtc_snvs snvs_pwrkey
> > > > snd_soc_fsl_utils imx_pcm_dma imx8mm_thermal imx_cpufreq_dt imx_sdma
> > > > ov5640 v4l2_fwnode v4l2_async videodev mc fuse drm ipv6
> > > > [   12.747111] CPU: 0 PID: 161 Comm: systemd-udevd Tainted: G        W
> > > >          6.2.0-rc3-30330-gb58b9dd3fb9e-dirty #3
> > > > [   12.757549] Hardware name: Beacon EmbeddedWorks i.MX8M Mini> Development Kit (DT)
> > > > [   12.764945] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > > [   12.771908] pc : imx7_csi_init_cfg+0x70/0x9c [imx7_media_csi]
> > > > [   12.777674] lr : imx7_csi_probe+0x26c/0x380 [imx7_media_csi]
> > > > [   12.783344] sp : ffff80000afd3900
> > > > [   12.786655] x29: ffff80000afd3900 x28: 0000000000000000 x27: 0000000000000000
> > > > [   12.793796] x26: ffff8000012ae180 x25: 0000000000000001 x24: ffff000005bb8340
> > > > [   12.800934] x23: ffff000005bb8450 x22: ffff000005bb80a8 x21: ffff8000012ac4f8
> > > > [   12.808072] x20: 0000000000000000 x19: ffff000005bb8080 x18: ffffffffffffffff
> > > > [   12.815215] x17: 0000000000000000 x16: 0000000000000000 x15: 64656d3d4d455453
> > > > [   12.822354] x14: ffff80000a56d220 x13: 0000000000000040 x12: 0000000000000228
> > > > [   12.829497] x11: 0000000000000000 x10: 0000000000000000 x9 : 000001e000000280
> > > > [   12.836636] x8 : 0000000100002006 x7 : 0002000100000008 x6 : 0000000000000002
> > > > [   12.843778] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000050
> > > > [   12.850918] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000005bb8450
> > > > [   12.858058] Call trace:
> > > > [   12.860503]  imx7_csi_init_cfg+0x70/0x9c [imx7_media_csi]
> > > > [   12.865915]  platform_probe+0x68/0xe0
> > > > [   12.869584]  really_probe+0xbc/0x2dc
> > > > [   12.873160]  __driver_probe_device+0x78/0xe0
> > > > [   12.877434]  driver_probe_device+0xd8/0x160
> > > > [   12.881618]  __driver_attach+0x94/0x19c
> > > > [   12.885456]  bus_for_each_dev+0x70/0xd0
> > > > [   12.889293]  driver_attach+0x24/0x30
> > > > [   12.892868]  bus_add_driver+0x154/0x20c
> > > > [   12.896707]  driver_register+0x78/0x130
> > > > [   12.900545]  __platform_driver_register+0x28/0x34
> > > > [   12.905255]  imx7_csi_driver_init+0x20/0x1000 [imx7_media_csi]
> > > > [   12.911099]  do_one_initcall+0x50/0x1d0
> > > > [   12.914937]  do_init_module+0x48/0x1d0
> > > > [   12.918691]  load_module+0x193c/0x1cb0
> > > > [   12.922442]  __do_sys_finit_module+0xa8/0x100
> > > > [   12.926802]  __arm64_sys_finit_module+0x20/0x30
> > > > [   12.931336]  invoke_syscall+0x48/0x114
> > > > [   12.935090]  el0_svc_common.constprop.0+0xd4/0xfc
> > > > [   12.939796]  do_el0_svc+0x3c/0xc0
> > > > [   12.943114]  el0_svc+0x2c/0x84
> > > > [   12.946174]  el0t_64_sync_handler+0xbc/0x140
> > > > [   12.950446]  el0t_64_sync+0x190/0x194
> > > > [   12.954114] Code: b5fffe81 d4210000 d2800002 91014063 (a9002049)
> > > > [   12.960209] ---[ end trace 0000000000000000 ]---
> > >
> > > It appears I forgot something. Could you try with the following change ?
> > >
> > > diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
> > > index 9275447987d1..81d5f08b02d1 100644
> > > --- a/drivers/media/platform/nxp/imx7-media-csi.c
> > > +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> > > @@ -2259,21 +2259,15 @@ static int imx7_csi_probe(struct platform_device *pdev)
> > >         if (ret)
> > >                 return ret;
> > >
> > > -       /* Set the default mbus formats. */
> > > -       ret = imx7_csi_init_cfg(&csi->sd, NULL);
> > > -       if (ret)
> > > -               goto media_cleanup;
> > > -
> > >         ret = imx7_csi_async_register(csi);
> > >         if (ret)
> > > -               goto subdev_notifier_cleanup;
> > > +               goto err_async_unregister;
> > >
> > >         return 0;
> > >
> > > -subdev_notifier_cleanup:
> > > +err_async_unregister:
> > >         v4l2_async_nf_unregister(&csi->notifier);
> > >         v4l2_async_nf_cleanup(&csi->notifier);
> > > -media_cleanup:
> > >         imx7_csi_media_cleanup(csi);
> > >
> > >         return ret;
> >
> > The patch seems to have fixed the splat, and the camera appears in the
> > media device information again, but I still cannot capture:

So a mix of good and bad news :-)

> > root@beacon-imx8mm-kit:~# gst-launch-1.0 -v v4l2src num-buffers=1 !
> > video/x-raw,format=UYVY,width=640,height=480 ! videoconvert ! jpegenc
> > ! filesink location=tst2.jpg
> > Setting pipeline to PAUSED ...
> > Pipeline is live and does not need PREROLL ...
> > Pipeline is PREROLLED ...
> > Setting pipeline t[  186.993715] imx7-csi 32e20000.csi: capture format not valid
> > o PLAYING ...
> > New clock: GstSystemClock
> >
> >
> > Yet every node shows the same format:
> >
> > Media device information
> > ------------------------
> > driver          imx7-csi
> > model           imx-media
> > serial
> > bus info        platform:32e20000.csi
> > hw revision     0x0
> > driver version  6.2.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:UYVY8_1X16/640x480 field:none colorspace:srgb xfer:srgb ycbcr:601
> > quantization:lim-range]
> > <- "csis-32e30000.mipi-csi":1 [ENABLED,IMMUTABLE]
> > pad1: Source
> > [fmt:UYVY8_1X16/640x480 field:none colorspace:srgb xfer:srgb ycbcr:601
> > quantization:lim-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:UYVY8_1X16/640x480 field:none]
> > <- "ov5640 1-003c":0 [ENABLED]
> > pad1: Source
> > [fmt:UYVY8_1X16/640x480 field:none]
> > -> "csi":0 [ENABLED,IMMUTABLE]
> >
> > - entity 15: ov5640 1-003c (1 pad, 1 link)
> >              type V4L2 subdev subtype Sensor flags 0
> >              device node name /dev/v4l-subdev2
> > pad0: Source
> > [fmt:UYVY8_1X16/640x480@1/30 field:none colorspace:srgb xfer:srgb
> > ycbcr:601 quantization:full-range
> > crop.bounds:(0,0)/2624x1964
> > crop:(16,14)/2592x1944]
> > -> "csis-32e30000.mipi-csi":0 [ENABLED]
> 
> After a little debugging, i found that imx7_csi_video_validate_fmt is
> returning EINVAL.  Unfortunately, I have to be done for the weekend,
> but I can try something again on Monday.

This should be fixed in v2.

> > > > The media information:
> > > >
> > > > root@beacon-imx8mm-kit:~# media-ctl -p
> > > > Media controller API version 6.2.0
> > > >
> > > > Media device information
> > > > ------------------------
> > > > driver          imx7-csi
> > > > model           imx-media
> > > > serial
> > > > bus info        platform:32e20000.csi
> > > > hw revision     0x0
> > > > driver version  6.2.0
> > > >
> > > > Device topology
> > > > - entity 1: csi (2 pads, 1 link)
> > > >             type V4L2 subdev subtype Unknown flags 0
> > > > pad0: Sink
> > > > pad1: Source
> > > > -> "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]
> > > >
> > > > I confirmed the ov5640 camera enumerated:
> > > >
> > > > I'm going to roll back this latest series to verify whether or not
> > > > this series caused the splat.
> > > >
> > > > adam
> > > >
> > > > >  drivers/media/platform/nxp/imx7-media-csi.c | 18 ++++++++----------
> > > > >  1 file changed, 8 insertions(+), 10 deletions(-)
> > > > >
> > > > > diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
> > > > > index be3c1494cfb3..e96bee4e5921 100644
> > > > > --- a/drivers/media/platform/nxp/imx7-media-csi.c
> > > > > +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> > > > > @@ -1598,17 +1598,15 @@ static struct imx7_csi_vb2_buffer *imx7_csi_video_next_buf(struct imx7_csi *csi)
> > > > >
> > > > >  static int imx7_csi_video_init_format(struct imx7_csi *csi)
> > > > >  {
> > > > > -       struct v4l2_subdev_format fmt_src = {
> > > > > -               .pad = IMX7_CSI_PAD_SRC,
> > > > > -               .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > > > > -       };
> > > > > -       fmt_src.format.code = IMX7_CSI_DEF_MBUS_CODE;
> > > > > -       fmt_src.format.width = IMX7_CSI_DEF_PIX_WIDTH;
> > > > > -       fmt_src.format.height = IMX7_CSI_DEF_PIX_HEIGHT;
> > > > > +       struct v4l2_mbus_framefmt format = { };
> > > > >
> > > > > -       imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &fmt_src.format, NULL);
> > > > > -       csi->vdev_compose.width = fmt_src.format.width;
> > > > > -       csi->vdev_compose.height = fmt_src.format.height;
> > > > > +       format.code = IMX7_CSI_DEF_MBUS_CODE;
> > > > > +       format.width = IMX7_CSI_DEF_PIX_WIDTH;
> > > > > +       format.height = IMX7_CSI_DEF_PIX_HEIGHT;
> > > > > +
> > > > > +       imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &format, NULL);
> > > > > +       csi->vdev_compose.width = format.width;
> > > > > +       csi->vdev_compose.height = format.height;
> > > > >
> > > > >         csi->vdev_cc = imx7_csi_find_pixel_format(csi->vdev_fmt.pixelformat);
> > > > >

-- 
Regards,

Laurent Pinchart