* [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state
@ 2023-01-27 2:27 Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 1/6] media: imx: imx7-media-csi: Drop imx7_csi.cc field Laurent Pinchart
` (7 more replies)
0 siblings, 8 replies; 15+ messages in thread
From: Laurent Pinchart @ 2023-01-27 2:27 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Paul Elder, Martin Kepplinger, Adam Ford,
kernel, linux-imx
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v1 1/6] media: imx: imx7-media-csi: Drop imx7_csi.cc field
2023-01-27 2:27 [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state Laurent Pinchart
@ 2023-01-27 2:27 ` Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format() Laurent Pinchart
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2023-01-27 2:27 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Paul Elder, Martin Kepplinger, Adam Ford,
kernel, linux-imx
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()
2023-01-27 2:27 [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 1/6] media: imx: imx7-media-csi: Drop imx7_csi.cc field Laurent Pinchart
@ 2023-01-27 2:27 ` Laurent Pinchart
2023-01-27 3:19 ` Adam Ford
2023-01-27 2:27 ` [PATCH v1 3/6] media: imx: imx7-media-csi: Drop unneeded check when starting streaming Laurent Pinchart
` (5 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Laurent Pinchart @ 2023-01-27 2:27 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Paul Elder, Martin Kepplinger, Adam Ford,
kernel, linux-imx
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 3/6] media: imx: imx7-media-csi: Drop unneeded check when starting streaming
2023-01-27 2:27 [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 1/6] media: imx: imx7-media-csi: Drop imx7_csi.cc field Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format() Laurent Pinchart
@ 2023-01-27 2:27 ` Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 4/6] media: imx: imx7-media-csi: Drop unneeded src_sd check Laurent Pinchart
` (4 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2023-01-27 2:27 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Paul Elder, Martin Kepplinger, Adam Ford,
kernel, linux-imx
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 4/6] media: imx: imx7-media-csi: Drop unneeded src_sd check
2023-01-27 2:27 [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state Laurent Pinchart
` (2 preceding siblings ...)
2023-01-27 2:27 ` [PATCH v1 3/6] media: imx: imx7-media-csi: Drop unneeded check when starting streaming Laurent Pinchart
@ 2023-01-27 2:27 ` Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 5/6] media: imx: imx7-media-csi: Drop unneeded pad checks Laurent Pinchart
` (3 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2023-01-27 2:27 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Paul Elder, Martin Kepplinger, Adam Ford,
kernel, linux-imx
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 5/6] media: imx: imx7-media-csi: Drop unneeded pad checks
2023-01-27 2:27 [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state Laurent Pinchart
` (3 preceding siblings ...)
2023-01-27 2:27 ` [PATCH v1 4/6] media: imx: imx7-media-csi: Drop unneeded src_sd check Laurent Pinchart
@ 2023-01-27 2:27 ` Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 6/6] media: imx: imx7-media-csi: Use V4L2 subdev active state Laurent Pinchart
` (2 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2023-01-27 2:27 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Paul Elder, Martin Kepplinger, Adam Ford,
kernel, linux-imx
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 6/6] media: imx: imx7-media-csi: Use V4L2 subdev active state
2023-01-27 2:27 [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state Laurent Pinchart
` (4 preceding siblings ...)
2023-01-27 2:27 ` [PATCH v1 5/6] media: imx: imx7-media-csi: Drop unneeded pad checks Laurent Pinchart
@ 2023-01-27 2:27 ` Laurent Pinchart
2023-01-27 2:41 ` [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to " Adam Ford
2023-01-27 11:41 ` Martin Kepplinger
7 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2023-01-27 2:27 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Paul Elder, Martin Kepplinger, Adam Ford,
kernel, linux-imx
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state
2023-01-27 2:27 [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state Laurent Pinchart
` (5 preceding siblings ...)
2023-01-27 2:27 ` [PATCH v1 6/6] media: imx: imx7-media-csi: Use V4L2 subdev active state Laurent Pinchart
@ 2023-01-27 2:41 ` Adam Ford
2023-01-27 11:41 ` Martin Kepplinger
7 siblings, 0 replies; 15+ messages in thread
From: Adam Ford @ 2023-01-27 2:41 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Rui Miguel Silva, Paul Elder, Martin Kepplinger,
kernel, linux-imx
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()
2023-01-27 2:27 ` [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format() Laurent Pinchart
@ 2023-01-27 3:19 ` Adam Ford
2023-01-27 6:57 ` Laurent Pinchart
0 siblings, 1 reply; 15+ messages in thread
From: Adam Ford @ 2023-01-27 3:19 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Rui Miguel Silva, Paul Elder, Martin Kepplinger,
kernel, linux-imx
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()
2023-01-27 3:19 ` Adam Ford
@ 2023-01-27 6:57 ` Laurent Pinchart
2023-01-27 11:07 ` Adam Ford
0 siblings, 1 reply; 15+ messages in thread
From: Laurent Pinchart @ 2023-01-27 6:57 UTC (permalink / raw)
To: Adam Ford
Cc: linux-media, Rui Miguel Silva, Paul Elder, Martin Kepplinger,
kernel, linux-imx
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()
2023-01-27 6:57 ` Laurent Pinchart
@ 2023-01-27 11:07 ` Adam Ford
2023-01-27 11:20 ` Adam Ford
0 siblings, 1 reply; 15+ messages in thread
From: Adam Ford @ 2023-01-27 11:07 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Rui Miguel Silva, Paul Elder, Martin Kepplinger,
kernel, linux-imx
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()
2023-01-27 11:07 ` Adam Ford
@ 2023-01-27 11:20 ` Adam Ford
2023-01-29 2:36 ` Laurent Pinchart
0 siblings, 1 reply; 15+ messages in thread
From: Adam Ford @ 2023-01-27 11:20 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Rui Miguel Silva, Paul Elder, Martin Kepplinger,
kernel, linux-imx
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state
2023-01-27 2:27 [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state Laurent Pinchart
` (6 preceding siblings ...)
2023-01-27 2:41 ` [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to " Adam Ford
@ 2023-01-27 11:41 ` Martin Kepplinger
2023-01-29 2:35 ` Laurent Pinchart
7 siblings, 1 reply; 15+ messages in thread
From: Martin Kepplinger @ 2023-01-27 11:41 UTC (permalink / raw)
To: Laurent Pinchart, linux-media
Cc: Rui Miguel Silva, Paul Elder, Adam Ford, kernel, linux-imx
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state
2023-01-27 11:41 ` Martin Kepplinger
@ 2023-01-29 2:35 ` Laurent Pinchart
0 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2023-01-29 2:35 UTC (permalink / raw)
To: Martin Kepplinger
Cc: linux-media, Rui Miguel Silva, Paul Elder, Adam Ford, kernel, linux-imx
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format()
2023-01-27 11:20 ` Adam Ford
@ 2023-01-29 2:36 ` Laurent Pinchart
0 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2023-01-29 2:36 UTC (permalink / raw)
To: Adam Ford
Cc: linux-media, Rui Miguel Silva, Paul Elder, Martin Kepplinger,
kernel, linux-imx
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
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-01-29 2:37 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-27 2:27 [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to subdev active state Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 1/6] media: imx: imx7-media-csi: Drop imx7_csi.cc field Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 2/6] media: imx: imx7-media-csi: Simplify imx7_csi_video_init_format() Laurent Pinchart
2023-01-27 3:19 ` Adam Ford
2023-01-27 6:57 ` Laurent Pinchart
2023-01-27 11:07 ` Adam Ford
2023-01-27 11:20 ` Adam Ford
2023-01-29 2:36 ` Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 3/6] media: imx: imx7-media-csi: Drop unneeded check when starting streaming Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 4/6] media: imx: imx7-media-csi: Drop unneeded src_sd check Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 5/6] media: imx: imx7-media-csi: Drop unneeded pad checks Laurent Pinchart
2023-01-27 2:27 ` [PATCH v1 6/6] media: imx: imx7-media-csi: Use V4L2 subdev active state Laurent Pinchart
2023-01-27 2:41 ` [PATCH v1 0/6] media: nxp: imx7-media-csi: Move to " Adam Ford
2023-01-27 11:41 ` Martin Kepplinger
2023-01-29 2:35 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.