Hi Jacopo,
Thank you for the patch.
On Mon, Jan 31, 2022 at 03:32:29PM +0100, Jacopo Mondi wrote:
> After having set a new format re-calculate the pixel_rate and link_freq
> control values and update them when in MIPI mode.
>
> Take into account the limitation of the link frequency having to be
> strictly smaller than 1GHz when computing the desired link_freq, and
> adjust the resulting pixel_rate acounting for the clock tree
> configuration.
>
> Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> ---
> drivers/media/i2c/ov5640.c | 63 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 8322b99eb2b7..457f76030163 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2375,6 +2375,65 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
> return 0;
> }
>
> +static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
> +{
> + const struct ov5640_mode_info *mode = sensor->current_mode;
> + struct v4l2_mbus_framefmt *fmt = &sensor->fmt;
> + enum ov5640_pixel_rate_id pixel_rate_id = mode->pixel_rate;
> + u32 num_lanes = sensor->ep.bus.mipi_csi2.num_data_lanes;
As this is only valid for CSI-2, I'd move the initialization of the
variable after the !ov5640_is_mipi() check.
> + unsigned int i = 0;
> + u32 pixel_rate;
> + s64 link_freq;
> + u32 bpp;
> +
> + /*
> + * Update the pixel rate control value.
> + *
> + * For DVP mode, maintain the pixel rate calculation using fixed FPS.
> + */
> + if (!ov5640_is_mipi(sensor)) {
> + __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
> + ov5640_calc_pixel_rate(sensor));
> +
> + return 0;
> + }
> +
> + /*
> + * The MIPI CSI-2 link frequency should comply with the CSI-2
> + * specifications and be lower than 1GHz.
s/specifications/specification/
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> + *
> + * Start from the suggested pixel_rate for the current mode and
> + * progressively slow it down if it exceeds 1GHz.
> + */
> + bpp = ov5640_code_to_bpp(fmt->code);
> + do {
> + pixel_rate = ov5640_pixel_rates[pixel_rate_id];
> + link_freq = pixel_rate * bpp / (2 * num_lanes);
> + } while (link_freq >= 1000000000U &&
> + ++pixel_rate_id < OV5640_NUM_PIXEL_RATES);
> +
> + /*
> + * Higher link rates require the clock tree to be programmed with
> + * 'mipi_div' = 1; this has the effect of halving the actual output
> + * pixel rate in the MIPI domain.
> + *
> + * Adjust the pixel rate control value to report it correctly to
> + * userspace.
> + */
> + if (link_freq > OV5640_LINK_RATE_MAX)
> + pixel_rate /= 2;
> +
> + for (i = 0; i < ARRAY_SIZE(ov5640_csi2_link_freqs); ++i) {
> + if (ov5640_csi2_link_freqs[i] == link_freq)
> + break;
> + }
> +
> + __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate, pixel_rate);
> + __v4l2_ctrl_s_ctrl(sensor->ctrls.link_freq, i);
> +
> + return 0;
> +}
> +
> static int ov5640_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_format *format)
> @@ -2414,8 +2473,8 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> /* update format even if code is unchanged, resolution might change */
> sensor->fmt = *mbus_fmt;
>
> - __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
> - ov5640_calc_pixel_rate(sensor));
> + ov5640_update_pixel_rate(sensor);
> +
> out:
> mutex_unlock(&sensor->lock);
> return ret;
--
Regards,
Laurent Pinchart
