kitakar5525 / surface-ipu3-cameras

19 stars 8 forks source link

ov8865: "multiple packet header errors detected" when trying to capture #9

Closed kitakar5525 closed 3 years ago

kitakar5525 commented 3 years ago

Current state of ov8865 is that, can't capture images yet with the following dmesg output:

kern  :err   : [193434.710867] ipu3-cio2 0000:00:14.3: CSI-2 receiver port 0: multiple packet header errors detected

I guess link frequency control is not enough yet.

kbingham commented 3 years ago

Yes, that's likely an incorrect bus frequency.

kbingham commented 3 years ago

We have this patch in our tree for the ov5693 currently:

-------------------------- drivers/media/i2c/ov5693.c --------------------------
index a57c290264c5..70a4d282ffa5 100644
@@ -705,13 +705,14 @@ static int ov5693_remove(struct i2c_client *client)
    return 0;
 }

 // TODO: Put these in header?
 #define OV5693_LINK_FREQ_19MHZ     19200000
-#define OV5693_PIXEL_RATE          (OV5693_LINK_FREQ_19MHZ * 2 * 2) / 10
+#define OV5693_LINK_FREQ_640MHZ                640000000
+#define OV5693_PIXEL_RATE          (OV5693_LINK_FREQ_640MHZ / 10) * 2 * 2
 static const s64 link_freq_menu_items[] = {
-   OV5693_LINK_FREQ_19MHZ
+   OV5693_LINK_FREQ_640MHZ
 };

 /**
  * Initialize controls:
  * - link frequency

But of course that's just a clue where to look. The values you'll need will be specific to the ov8865.

kitakar5525 commented 3 years ago

Thanks! the patch applied to ov5693 driver as https://github.com/kitakar5525/surface-ipu3-cameras/commit/13511347b343ba1a93093408988c370ed16f852e https://github.com/kitakar5525/surface-ipu3-cameras/commit/d50d9aa414686d8940f8a33932d42da9db222cf0

As I wrote in the commit message, it looks like the * 2 * 2 part should be number of lanes? So, I changed it to * 2 because ov5693 on Surface devices use 2 lanes. (That said, it seems that the link_freq can be any values? ov5693 was working with link_freq 19MHz anyway.)

Also, I finally fixed capture not working on this ov8865 by commit https://github.com/kitakar5525/surface-ipu3-cameras/commit/26d851d267682f221e0203bf64793284c7bf7f61 Before the commit, I directly called sensor power up function (set_power_on()) in s_stream() because I know s_power() won't be called on our systems (ipu3 designed for Windows).

But I missed the fact that v4l2_ctrl_handler_setup() is called from s_power() in this driver. So, I switched to call s_power() instead of set_power_on().

Closing this issue as resolved.

kitakar5525 commented 3 years ago

Ah, the * 2 * 2 part is indeed correct. I missed this comment

+/**
+ * v4l2_get_link_rate - Get link rate from transmitter
+ *
+ * @handler: The transmitter's control handler
+ * @mul: The multiplier between pixel rate and link frequency. Bits per pixel on
+ *  D-PHY, samples per clock on parallel. 0 otherwise.
+ * @div: The divisor between pixel rate and link frequency. Number of data lanes
+ *  times two on D-PHY, 1 on parallel. 0 otherwise.

and this document (https://www.kernel.org/doc/html/latest/driver-api/media/csi2.html)

The value of the V4L2_CID_PIXEL_RATE is calculated as follows:

pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample

and other drivers also do like this

Rebased the commit (https://github.com/kitakar5525/surface-ipu3-cameras/commit/12dc910d048866fb6bfa8eafd8e1a7a1bb1610a7)