Re: [PATCH] media: v4l2-async: fix binding async subdevs with multiple source ports

[Aishwarya Kothari <aishwaryakothari75@gmail.com>]

Hi Philipp,

On 20.01.23 17:33, Philipp Zabel wrote:
> Hi,
> 
> On Wed, Jan 18, 2023 at 02:14:54PM +0100, Aishwarya Kothari wrote:
> [...]
>> I stumbled over the commit 1f391df44607 ("media: v4l2-async: Use endpoints
>> in __v4l2_async_nf_add_fwnode_remote()") and started this discussion :
>> https://lore.kernel.org/linux-media/Y8AIRPd4RFYmssal@valkosipuli.retiisi.eu/
>>
>> I applied this patch on top of the commit c1649ec55708.The hardware used is
>> Apalis iMX6 (i.MX 6Q) with ov5640 mipi-csi2 camera.
>>
>> The /dev/media0 is created and pipeline was configured using below script :
>> root@apalis-imx6-10774951:~# cat ov5640.sh
>> media-ctl -l "'ov5640 1-003c':0 -> 'imx6-mipi-csi2':0[1]"
>> media-ctl -l "'imx6-mipi-csi2':2 -> 'ipu1_csi1':0[1]"
>> media-ctl -l "'ipu1_csi1':2 -> 'ipu1_csi1 capture':0[1]"
>> # Configure pads
>> media-ctl -V "'ov5640 1-003c':0 [fmt:UYVY8_1X16/1920x1080 field:none]"
>> media-ctl -V "'imx6-mipi-csi2':2 [fmt:UYVY8_1X16/1920x1080 field:none]"
>> media-ctl -V "'ipu1_csi1':2 [fmt:UYVY8_1X16/1920x1080 field:none]"
>>
>> But it gives the following error when trying to set up the pipeline:
>>
>> [   37.211276] ipu1_csi1: entity ov5640 1-003c does not implement
>> get_mbus_config()
>> [   37.218872] ipu1_csi1: failed to get upstream media bus configuration
>>
>> When adding the missing functionality as follows:
>>
>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
>> index e0f908af581b..618c677ec89b 100644
>> --- a/drivers/media/i2c/ov5640.c
>> +++ b/drivers/media/i2c/ov5640.c
>> @@ -3733,6 +3733,17 @@ static int ov5640_init_cfg(struct v4l2_subdev *sd,
>>          return 0;
>>   }
>>
>> +static int ov5640_get_mbus_config(struct v4l2_subdev *sd,
>> +                                  unsigned int pad,
>> +                                  struct v4l2_mbus_config *cfg)
>> +{
>> +       cfg->type = V4L2_MBUS_CSI2_DPHY;
>> +       cfg->bus.mipi_csi2.num_data_lanes = 1;
> 
> Isn't OV5640 dual-lane by default?

Thank you for pointing this out, I updated the number of data lanes to 2 
in this function and applied your patch and everything works now.

I tested on top of v6.2-rc5 on an Apalis iMX6Q with an ov5640 camera.
Tested-by: Aishwarya Kothari <aishwarya.kothari@toradex.com>

> 
>> +       cfg->bus.mipi_csi2.flags = 0;
>> +
>> +       return 0;
>> +}
>> +
>>   static const struct v4l2_subdev_core_ops ov5640_core_ops = {
>>          .log_status = v4l2_ctrl_subdev_log_status,
>>          .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
>> @@ -3753,6 +3764,7 @@ static const struct v4l2_subdev_pad_ops ov5640_pad_ops
>> = {
>>          .get_selection = ov5640_get_selection,
>>          .enum_frame_size = ov5640_enum_frame_size,
>>          .enum_frame_interval = ov5640_enum_frame_interval,
>> +       .get_mbus_config = ov5640_get_mbus_config,
>>   };
>>
>>   static const struct v4l2_subdev_ops ov5640_subdev_ops = {
>>
>> The script was executed correctly without any errors and the links were
>> created. Now when running the Gstreamer it gives the below output :
>>
>> root@apalis-imx6-10774951:~# gst-launch-1.0 v4l2src device='/dev/video3' !
>> videoconvert ! waylandsink
>> Setting pipeline to PAUSED ...
>> Pipeline is live and does not need PREROLL ...
>> Pipeline is PREROLLED ...
>> Setting pipeline to PLAYING ...
>> New clock: GstSystemClock
>> [  192.526110] imx6-mipi-csi2: LP-11 wait timeout, likely a sensor driver
>> bug, expect capture failures.
>> [  192.535550] imx6-mipi-csi2: phy_state = 0x00000200
> 
> This is timing out in the imx6-mipi-csi2 driver, waiting for OV5640 to
> signal the LP-11 stop state on lane 0 (phy_state bit 4).
> 
>> [  192.833456] ov5640 1-003c: ov5640_write_reg: error: reg=3008, val=2
> 
> A write to the SYS_CTRL0 register failed, presumably trying to clear the
> software reset or power down bits. Could this be the reason that OV5640
> doesn't put the MIPI CSI-2 link into LP-11 as expected?
> 
> All further errors follow from the timeout above.
> 
> [...]
>> While going through the dmesg kernel logs I found this :
>>
>>      4.333202] imx6-mipi-csi2 21dc000.mipi: Consider updating driver
>> imx6-mipi-csi2 to match on endpoints
>> [    4.347001] imx6-mipi-csi2 21dc000.mipi: Consider updating driver
>> imx6-mipi-csi2 to match on endpoints
>> [    5.173588] video-mux 20e0000.iomuxc-gpr:ipu2_csi1_mux: Consider updating
>> driver video-mux to match on endpoints
> 
> These shouldn't cause any issue. These drivers should be updated
> to set the subdev->fwnode field to an endpoint fwnode before calling
> v4l2_async_register_subdev().
> 
> regards
> Philipp
> 

Kind regards,
Aishwarya