Re: [PATCH v5 06/27] media: ov5640: Update pixel_rate and link_freq

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jacopo,

Issue with link frequency value reported, see below.

On 2/24/22 10:42 AM, 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>
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
---
  drivers/media/i2c/ov5640.c | 66 ++++++++++++++++++++++++++++++++++++--
  1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index f9763edf2422..791694bfed41 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -31,6 +31,8 @@
#define OV5640_DEFAULT_SLAVE_ID 0x3c +#define OV5640_LINK_RATE_MAX 490000000U
+
  #define OV5640_REG_SYS_RESET02		0x3002
  #define OV5640_REG_SYS_CLOCK_ENABLE02	0x3006
  #define OV5640_REG_SYS_CTRL0		0x3008
@@ -2412,6 +2414,66 @@ 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;
+	unsigned int i = 0;
+	u32 pixel_rate;
+	s64 link_freq;
+	u32 num_lanes;
+	u32 bpp;
+
+	/*
+	 * Update the pixel rate control value.
+	 *
+	 * For DVP mode, maintain the pixel rate calculation using fixed FPS.
+	 */
+	if (!ov5640_is_csi2(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
+	 * specification and be lower than 1GHz.
+	 *
+	 * Start from the suggested pixel_rate for the current mode and
+	 * progressively slow it down if it exceeds 1GHz.
+	 */
+	num_lanes = sensor->ep.bus.mipi_csi2.num_data_lanes;
+	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;

Need to divide also the link_frequency (st-mipid02 bridge interfacing is broken here). Patch proposal below:

+	sensor->current_link_freq = link_freq;
+
+	 * Adjust the pixel rate & link frequency control value to report it
+	 * correctly to userspace.
 	 */
-	if (link_freq > OV5640_LINK_RATE_MAX)
+	if (link_freq > OV5640_LINK_RATE_MAX) {
 		pixel_rate /= 2;
+		link_freq /= 2;
+	}


Doing so we cannot relay anymore on link_frequency control reading in ov5640_set_mipi_pclk(), use current_link_freq variable instead

@@ -1440,7 +1453,7 @@ static int ov5640_set_mipi_pclk(struct ov5640_dev *sensor)
 	int ret;

 	/* Use the link freq computed at ov5640_update_pixel_rate() time. */
-	link_freq = ov5640_csi2_link_freqs[sensor->ctrls.link_freq->cur.val];
+	link_freq = sensor->current_link_freq;

@@ -3782,6 +3811,7 @@ static int ov5640_probe(struct i2c_client *client)
 	sensor->current_mode =
 		&ov5640_mode_data[OV5640_MODE_VGA_640_480];
 	sensor->last_mode = sensor->current_mode;
+	sensor->current_link_freq = OV5640_DEFAULT_LINK_FREQ;




+
+	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)
@@ -2451,8 +2513,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;


Hugues.



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux