| Message ID | 20230126213437.20796-2-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | a42b43f7b6708428ce25239d5c245a18758f68c5 |
| Headers | show |
| Series | media: imx-mipi-csis: Move to subdev active state | expand |
On Thu, Jan 26, 2023 at 3:34 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > It is customary to prefix error labels with 'err_' to make their purpose > clearer. Do so in the probe function. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- I tested this series on the imx8mm, but I get an error stating the capture format is invalid. My media info looks like: 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]
On Thu, Jan 26, 2023 at 9:34 PM Adam Ford <aford173@gmail.com> wrote: > > On Thu, Jan 26, 2023 at 3:34 PM Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > > > It is customary to prefix error labels with 'err_' to make their purpose > > clearer. Do so in the probe function. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > I tested this series on the imx8mm, but I get an error stating the > capture format is invalid. > > My media info looks like: > > 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] > > From what I can see, each node is configured for UYVY8_1X16/640x480 > > Yet, the following occurs: > > gst-launch-1.0 v4l2src ! video/x-raw,format=UYVY,width=640,height=480 ! fakesink > Setting pipeline to PAUSED ... > Pipeline is live and does not[ 335.986728] imx7-csi 32e20000.csi: > capture format not valid > need PREROLL ... > Pipeline is PREROLLED ... > Setting pipeline to PLAYING ... > New clock: GstSystemClock > ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Failed > to allocate required memory. > Additional debug info: > ../git/sys/v4l2/gstv4l2src.c(659): gst_v4l2src_decide_allocation (): > /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: > Buffer pool activation failed > Execution ended after 0:00:00.009848500 > Setting pipeline to NULL ... > ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: > Internal data stream error. > Additional debug info: > ../git/libs/gst/base/gstbasesrc.c(3127): gst_base_src_loop (): > /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: > streaming stopped, reason not-negotiated (-4) > Freeing pipeline ... > root@beacon-imx8mm-kit:~# > > > I'm going to unroll this series to see if the mini can capture, and > I'll report back my findings. I unrolled both the imx7 capture stuff and the CSIS stuff, but I still get the same error about the capture format not valid. I rolled back to 6.2-rc5 and it captures, but the colors seem off. I need to get some sleep, and I'll be traveling Friday-Sunday, but I'll try to git bisect to see where stuff stopped working on Mini when I get back. adam > > adam > > > > drivers/media/platform/nxp/imx-mipi-csis.c | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c > > index 905072871ed2..d949b2de8e74 100644 > > --- a/drivers/media/platform/nxp/imx-mipi-csis.c > > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c > > @@ -1496,20 +1496,20 @@ static int mipi_csis_probe(struct platform_device *pdev) > > dev_name(dev), csis); > > if (ret) { > > dev_err(dev, "Interrupt request failed\n"); > > - goto disable_clock; > > + goto err_disable_clock; > > } > > > > /* Initialize and register the subdev. */ > > ret = mipi_csis_subdev_init(csis); > > if (ret < 0) > > - goto disable_clock; > > + goto err_disable_clock; > > > > platform_set_drvdata(pdev, &csis->sd); > > > > ret = mipi_csis_async_register(csis); > > if (ret < 0) { > > dev_err(dev, "async register failed: %d\n", ret); > > - goto cleanup; > > + goto err_cleanup; > > } > > > > /* Initialize debugfs. */ > > @@ -1520,7 +1520,7 @@ static int mipi_csis_probe(struct platform_device *pdev) > > if (!pm_runtime_enabled(dev)) { > > ret = mipi_csis_runtime_resume(dev); > > if (ret < 0) > > - goto unregister_all; > > + goto err_unregister_all; > > } > > > > dev_info(dev, "lanes: %d, freq: %u\n", > > @@ -1528,14 +1528,14 @@ static int mipi_csis_probe(struct platform_device *pdev) > > > > return 0; > > > > -unregister_all: > > +err_unregister_all: > > mipi_csis_debugfs_exit(csis); > > -cleanup: > > +err_cleanup: > > media_entity_cleanup(&csis->sd.entity); > > v4l2_async_nf_unregister(&csis->notifier); > > v4l2_async_nf_cleanup(&csis->notifier); > > v4l2_async_unregister_subdev(&csis->sd); > > -disable_clock: > > +err_disable_clock: > > mipi_csis_clk_disable(csis); > > fwnode_handle_put(csis->sd.fwnode); > > mutex_destroy(&csis->lock); > > -- > > Regards, > > > > Laurent Pinchart > >
diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c index 905072871ed2..d949b2de8e74 100644 --- a/drivers/media/platform/nxp/imx-mipi-csis.c +++ b/drivers/media/platform/nxp/imx-mipi-csis.c @@ -1496,20 +1496,20 @@ static int mipi_csis_probe(struct platform_device *pdev) dev_name(dev), csis); if (ret) { dev_err(dev, "Interrupt request failed\n"); - goto disable_clock; + goto err_disable_clock; } /* Initialize and register the subdev. */ ret = mipi_csis_subdev_init(csis); if (ret < 0) - goto disable_clock; + goto err_disable_clock; platform_set_drvdata(pdev, &csis->sd); ret = mipi_csis_async_register(csis); if (ret < 0) { dev_err(dev, "async register failed: %d\n", ret); - goto cleanup; + goto err_cleanup; } /* Initialize debugfs. */ @@ -1520,7 +1520,7 @@ static int mipi_csis_probe(struct platform_device *pdev) if (!pm_runtime_enabled(dev)) { ret = mipi_csis_runtime_resume(dev); if (ret < 0) - goto unregister_all; + goto err_unregister_all; } dev_info(dev, "lanes: %d, freq: %u\n", @@ -1528,14 +1528,14 @@ static int mipi_csis_probe(struct platform_device *pdev) return 0; -unregister_all: +err_unregister_all: mipi_csis_debugfs_exit(csis); -cleanup: +err_cleanup: media_entity_cleanup(&csis->sd.entity); v4l2_async_nf_unregister(&csis->notifier); v4l2_async_nf_cleanup(&csis->notifier); v4l2_async_unregister_subdev(&csis->sd); -disable_clock: +err_disable_clock: mipi_csis_clk_disable(csis); fwnode_handle_put(csis->sd.fwnode); mutex_destroy(&csis->lock);
It is customary to prefix error labels with 'err_' to make their purpose clearer. Do so in the probe function. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/platform/nxp/imx-mipi-csis.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)