Hi Dave, On 26.01.2021 16:01, Dave Stevenson wrote:
Hi Andrey On Tue, 26 Jan 2021 at 07:50, Andrey Konovalov <andrey.konovalov@xxxxxxxxxx> wrote:This control is needed for imx219 driver, as the link frequency is independent from the pixel rate in this case, and can't be calculated from the pixel rate. Signed-off-by: Andrey Konovalov <andrey.konovalov@xxxxxxxxxx> --- drivers/media/i2c/imx219.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c index 92a8d52776b8..6e3382b85a90 100644 --- a/drivers/media/i2c/imx219.c +++ b/drivers/media/i2c/imx219.c @@ -390,6 +390,10 @@ static const struct imx219_reg raw10_framefmt_regs[] = { {0x0309, 0x0a}, }; +static const s64 imx219_link_freq_menu[] = { + IMX219_DEFAULT_LINK_FREQ,Link frequency is one of the parameters that is largely irrelevant on the Pi, so I've partially ignored it.
I faced a problem with the imx219 8-bit modes not working with the camss driver (drivers/media/platform/qcom/camss), as based on the link frequency calculated from the pixel rate the driver sets the csiphy clock to 100MHz which is too low for the actual link frequency (4 * 100MHz < 456MHz), and the captured image becomes garbage.
Is the link frequency really the same for all modes? Even 8 bit vs 10 bit readout?
Yes, this is exactly the case.
The pixel rate is constant at 182.4Mpix/s for all modes.
Right.
Switching to 8 bit changes register 0x0309 (op_pix_clk_div) from 10 to 8. Figure 43 "Clock System Block Diagram" in the datasheet I have says this reduces the divider to the FIFO between the pipeline and MIPI. As we haven't changed PLL2 or Pre-div2 I'd expect the link frequency to stay the same,
That's true.
but that leaves me confused over that FIFO clock as it'll go UP in frequency. I can't quite see how that works, but it clearly does.
Yes, the FIFO makes it possible for the different write and read rates to work. There are few words regarding this in the datasheet, but this isn't enough to fully understand how it works: "If, Pix Rate of PLL1 domain < Data Rate of PLL2 domain, data is always correctly output from the sensor" (page 81) If I read the datasheet right, for 10-bit modes the both rates are the same (91.2 MHz). In the 8-bit modes the "Data Rate" increases to 114 MHz while the "Pix Rate" remains at 91.2 MHz.
Both 8 and 10 bit modes do read out at the same frame / pixel rate, therefore that bit is correct, but that leaves me puzzling over link frequency. I have no information on how big that FIFO is, or how it's clocked on input and output. Simplest option is that as I need to go into the office in the next day or so I'll pop into the lab and measure it in each mode.
That would be nice! In my home "office" I only have a small piece of hardware which claims to be able to deal with 2 signals up to 72MHz each, which is not enough for such kind of measurements.
Otherwise I have no issues with the implementation of the patch. Dave
Thanks, Andrey
+}; + static const char * const imx219_test_pattern_menu[] = { "Disabled", "Color Bars", @@ -547,6 +551,7 @@ struct imx219 { struct v4l2_ctrl_handler ctrl_handler; /* V4L2 Controls */ struct v4l2_ctrl *pixel_rate; + struct v4l2_ctrl *link_freq; struct v4l2_ctrl *exposure; struct v4l2_ctrl *vflip; struct v4l2_ctrl *hflip; @@ -1269,7 +1274,7 @@ static int imx219_init_controls(struct imx219 *imx219) int i, ret; ctrl_hdlr = &imx219->ctrl_handler; - ret = v4l2_ctrl_handler_init(ctrl_hdlr, 11); + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 12); if (ret) return ret; @@ -1283,6 +1288,14 @@ static int imx219_init_controls(struct imx219 *imx219) IMX219_PIXEL_RATE, 1, IMX219_PIXEL_RATE); + imx219->link_freq = + v4l2_ctrl_new_int_menu(ctrl_hdlr, &imx219_ctrl_ops, + V4L2_CID_LINK_FREQ, + ARRAY_SIZE(imx219_link_freq_menu) - 1, 0, + imx219_link_freq_menu); + if (imx219->link_freq) + imx219->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY; + /* Initial vblank/hblank/exposure parameters based on current mode */ imx219->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, V4L2_CID_VBLANK, IMX219_VBLANK_MIN, -- 2.17.1
