Hi Jacopo,
On 7/24/19 4:49 PM, Jacopo Mondi wrote:
Hi Laura,
thanks for addressing this issue.
On Mon, Jul 22, 2019 at 05:50:35PM +0200, Laura Nao wrote:
Thanks Fabio!
I tried tweaking the PLL configuration in the driver and did some further
tests on 5.2 kernel.
I was finally able to capture RAW frames that match the test pattern for
1280x720 and 1920x1080 resolutions. The 2592x1944 frame is still not
perfectly aligned, but it looks much closer to the test pattern.
I uploaded the images here:
https://imgur.com/a/ywHokMf
The changes I made in the driver are below. Not sure these changes make much
sense, but they seem to fix 1280x720 and 1920x1080 frames.
So, this has finally happened :)
When ~1 year ago we addressed dynamic clock computation for this
driver the only cases that were tested where the DVP interface by
Maxime and the CSI-2 one on my side but only for formats with 16bpp
and 2 data lanes, so I'm not surprised by the fact 8-bit raw fails
with the current implementation.[1]
[1] https://patchwork.kernel.org/patch/10680571/
The driver has clearly commented that once we were to support more
formats some changes would be required. In example, just before the
ov5640_set_mipi_pclk() function a FIXME comment reports:
* FIXME: this have been tested with 16bpp and 2 lanes setup only.
* MIPI_DIV is fixed to value 2, but it -might- be changed according to the
* above formula for setups with 1 lane or image formats with different bpp.
*
* FIXME: this deviates from the sensor manual documentation which is quite
* thin on the MIPI clock tree generation part.
I tried to extensively comment there PLL calculation procedure I
deduced from the not-so-clear documentation for the sensor, and I
must admit everytime I look at it again, it is still a pain :)
While your below fixes are not consumable by mainline, I would be glad
to help integrating calculation of the PLL in case of raw8 (and
possibly other formats).
I suggest to go through the iteration of the above linked series from
Maxime, as they contain somewhat interesting discussions on both the
PLL architecture and links to some useful documentation. I will find
some time to study all of this again in the next days and try to help
if necessary.
In the meantime let me cc Maxime (even if he mostly worked on the
parallel interface support) and Sam, who gave some very interesting
insights on the PLL architecture.
Thanks for your work, and please keep me and Maxime CC-ed, as the PLL
calculation part and CSI-2 support has been quite a big effort, and I
would like to help having it succesfully working with most use cases!
Thanks
j
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 759d60c6..cfa678e 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -795,13 +795,13 @@ static int ov5640_mod_reg(struct ov5640_dev *sensor,
u16 reg,
* FIXME: to be re-calcualted for 1 data lanes setups
*/
#define OV5640_MIPI_DIV_PCLK 2
-#define OV5640_MIPI_DIV_SCLK 1
+#define OV5640_MIPI_DIV_SCLK 2
/*
* This is supposed to be ranging from 1 to 2, but the value is always
* set to 2 in the vendor kernels.
*/
-#define OV5640_PLL_ROOT_DIV 2
+#define OV5640_PLL_ROOT_DIV 1
#define OV5640_PLL_CTRL3_PLL_ROOT_DIV_2 BIT(4)
/*
@@ -836,8 +836,8 @@ static unsigned long ov5640_compute_sys_clk(struct
ov5640_dev *sensor,
unsigned long sysclk = sensor->xclk_freq / pll_prediv * pll_mult;
/* PLL1 output cannot exceed 1GHz. */
- if (sysclk / 1000000 > 1000)
- return 0;
+ // if (sysclk / 1000000 > 1000)
+ // return 0;
return sysclk / sysdiv;
}
@@ -1818,7 +1824,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor)
* All the formats we support have 16 bits per pixel, seems to require
* the same rate than YUV, so we can just use 16 bpp all the time.
*/
- rate = mode->vtot * mode->htot * 16;
+ rate = mode->vtot * mode->htot * 8;
rate *= ov5640_framerates[sensor->current_fr];
if (sensor->ep.bus_type == V4L2_MBUS_CSI2_DPHY) {
rate = rate / sensor->ep.bus.mipi_csi2.num_data_lanes;
Thanks,
Best,
Laura
On 7/22/19 2:45 PM, Fabio Estevam wrote:
[Adding Steve and Philipp]
On Thu, Jul 18, 2019 at 10:06 AM Laura Nao <laura.nao@xxxxxxxxxxxx> wrote:
Hello Loic,
I'm having some issues with RAW8 mode on the OV5640 camera and I'd like
to kindly ask for your advice, as I saw that you added support for RAW
mode in the mainline kernel driver.
I'm trying to capture some raw images on a i.MX6Q based board. I
configured the pipeline as follows:
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]"
media-ctl -V "'ov5640 1-003c':0 [fmt:SBGGR8_1X8/2592x1944 field:none]"
media-ctl -V "'imx6-mipi-csi2':2 [fmt:SBGGR8_1X8/2592x1944 field:none]"
I'm capturing the frame using v4l-utils:
v4l2-ctl -d /dev/video5 --verbose --set-fmt
video=width=2592,height=1944,pixelformat=BA81 --stream-mmap
--stream-count=1 --stream-to=bggr_2592x1944.raw
The images I'm obtaining are completely garbled. I tried both 5.2
mainline and 5.1.18 kernels.
I'm able to correctly capture YUV frames in all resolutions with the
same driver (with the pipeline configured to go through the
ipu1_ic_prpenc node first).
Do you have any insight on what might be causing these issues? Is the
PLL configuration supposed to be changed when RAW8 format is selected?
Thanks for your help,
Best regards,
Laura
Thanks for your reply!
I will go through the discussion you linked a study a bit more your
(impressive!) previous work for the PLL calculation.
I managed to find a register configuration that works for raw8 2592x1944
format on the vendor driver
(https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/mxc/capture/ov5640_mipi_v2.c?h=imx_4.14.98_2.0.0_ga),
hopefully this information will be useful to extract the correct PLL
calculation for mainline driver as well. I changed the SCLK DIV register
(0x3035) from 0x11 (default value in vendor driver) to 0x12. Resulting
2592x1944 raw8 image perfectly matches the test pattern. I didn't try
other resolutions yet. The configuration that works on vendor driver is
the following:
- PLL_CTRL1 (0x3035) = 0x12
- PLL_CTRL2 (0x3036) = 0x54
- PLL_CTRL3 (0x3037) = 0x13
This matches the dividers/multipliers obtained in mainline driver from
the ov5640_calc_sys_clk function when OV5640_MIPI_DIV_SCLK is set to
0x2. However, something is still missing in mainline driver and
resulting image is the same as my previous post.
I will try to compare the other register values, to see if I can find
what makes it work in the vendor driver.
I'll be happy to do some further tests and study a bit more. I'll keep
you posted if I find out anything useful.
Thanks a lot for the support,
Best,
Laura
