Re: [PATCH 68/75] media: imx: imx7_mipi_csis: Calculate Ths_settle from source pixel rate

Hi Sakari,

On Thu, Jan 07, 2021 at 12:59:24AM +0200, Sakari Ailus wrote:

On Tue, Jan 05, 2021 at 05:28:45PM +0200, Laurent Pinchart wrote:

quoted

The Ths_settle timing parameter depends solely on the pixel rate of the
source. Calculate it at runtime instead of requiring it to be specified
in the device tree.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 65 ++++++++++++++++++----
 1 file changed, 53 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 3c68ee8b2a59..c83450ac37fa 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c

@@ -24,6 +24,7 @@
 #include <linux/reset.h>
 #include <linux/spinlock.h>
 
+#include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-fwnode.h>
 #include <media/v4l2-mc.h>

@@ -233,7 +234,11 @@ struct csi_state {
 	struct media_pad pads[CSIS_PADS_NUM];
 	struct v4l2_subdev mipi_sd;
 	struct v4l2_async_notifier notifier;
-	struct v4l2_subdev *src_sd;
+
+	struct {
+		struct v4l2_subdev *sd;
+		struct v4l2_ctrl *pixel_rate;
+	} src;
 
 	u8 index;
 	struct platform_device *pdev;

@@ -482,6 +487,31 @@ static void __mipi_csis_set_format(struct csi_state *state)
 	mipi_csis_write(state, MIPI_CSIS_ISPRESOL_CH0, val);
 }
 
+static int mipi_csis_calculate_params(struct csi_state *state)
+{
+	u64 pixel_rate;
+	u32 lane_rate;
+
+	/* Calculate the line rate from the pixel rate. */
+	pixel_rate = v4l2_ctrl_g_ctrl_int64(state->src.pixel_rate);

Could you instead use v4l2_get_link_freq()?

I guess we're also moving to the LINK_FREQ control to tell this.

I've developed this patch before we discussed moving to LINQ_FREQ :-)
I'll fix this in v2.

quoted

+	lane_rate = div_u64(pixel_rate, state->bus.num_data_lanes)
+		  * state->csis_fmt->width;
+	if (lane_rate < 80000000 || lane_rate > 1500000000) {
+		dev_dbg(state->dev, "Out-of-bound lane rate %u\n", lane_rate);
+		return -EINVAL;
+	}
+
+	/*
+	 * The HSSETTLE counter value is document in a table, but can also
+	 * easily be calculated.
+	 */
+	state->hs_settle = (lane_rate - 5000000) / 45000000;

Much better, thank you!

-- 
Regards,

Laurent Pinchart