[PATCH v3 0/3] media: imx415: Improvements for framerate and link frequency control

[PATCH v3 0/3] media: imx415: Improvements for framerate and link frequency control

[Dave Stevenson]

The driver was using reverse engineered pixel rates, and had different
values for different link frequencies.
In common with the other Starvis sensors (eg imx290), this isn't
actually the situation, and the pixel array runs off a different
PLL to the MIPI block, and there is a FIFO between the two.

It's not quite so simple as HMAX is said to be in units of INCK,
not pixel clocks. Working through the various modes that appears that
even that is slightly wrong as it is units of a scaled INCK of
either 72MHz (for INCK = 24 or 72MHz) or 74.25MHz (for INCK = 27,
37.125, or 74.25MHz).

Rework the configuration so that the pixel rate is fixed, with the
different link frequencies adjusting the minimum hmax value so that
the FIFO doesn't overflow.

Switching between 2 and 4 data lanes also only changes the MIPI data
rate, so also can be handled by altering the minimum HMAX.

Once you've got HMAX / V4L2_CID_HBLANK represented sensibly, then
frame rate control through V4L2_CID_HBLANK and V4L2_CID_VBLANK is
trivial, so add in frame rate control too.
Exposure is set in lines (as usual), so amending VBLANK changes the
range for the exposure control.

This has been tested on a Pi5 with Waveshare "IMX415-98 IR-CUT Camera"
module (SKU 28524). That uses a 24MHz clock, so only the 360MHz and
720MHz link frequencies have been tested (other frequencies require
a 27, 37.125, or 74.25MHz clock input.

It'd be nice to add support for 12bit readout and windowed mode readout,
but those are a job for another day.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
Changes in v3:
- Fix s_ctrl for HBLANK to not return and miss a pm_runtime_put()
- Added Gerald's and Michael's R-b tags (not to patch 2 as that has been
  updated)
- Link to v2: https://lore.kernel.org/r/20250116-media-imx415-v2-0-735263f04652@raspberrypi.com

Changes in v2:
- Fixed VBLANK fall-through for setting the exposure correctly
- Sorted the checkpatch complaint over using fallthrough; vs commenting
  why we're falling through.
- Corrected HMAX for 891Mbit/s mode. I now have a module with 37.125MHz
  clock, and have validated that.
- Link to v1: https://lore.kernel.org/r/20250109-media-imx415-v1-0-366ba6a234ab@raspberrypi.com

---
Dave Stevenson (3):
      media: i2c: imx415: Add read/write control of VBLANK
      media: i2c: imx415: Make HBLANK controllable and in consistent units
      media: i2c: imx415: Link frequencies are not exclusive to num lanes

 drivers/media/i2c/imx415.c | 183 +++++++++++++++++++++++----------------------
 1 file changed, 94 insertions(+), 89 deletions(-)
---
base-commit: c4b7779abc6633677e6edb79e2809f4f61fde157
change-id: 20250107-media-imx415-2df67ae21107

Best regards,
-- 
Dave Stevenson <dave.stevenson@raspberrypi.com>

[PATCH v3 1/3] media: i2c: imx415: Add read/write control of VBLANK

[Dave Stevenson]

This also requires that the ranges for the exposure control
are updated.

Reviewed-by: Michael Riesch <michael.riesch@wolfvision.net>
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 drivers/media/i2c/imx415.c | 52 ++++++++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/drivers/media/i2c/imx415.c b/drivers/media/i2c/imx415.c
index 3f7924aa1bd3..fa7ffb9220e5 100644
--- a/drivers/media/i2c/imx415.c
+++ b/drivers/media/i2c/imx415.c
@@ -26,6 +26,7 @@
 #define IMX415_PIXEL_ARRAY_WIDTH  3864
 #define IMX415_PIXEL_ARRAY_HEIGHT 2192
 #define IMX415_PIXEL_ARRAY_VBLANK 58
+#define IMX415_EXPOSURE_OFFSET	  8
 
 #define IMX415_NUM_CLK_PARAM_REGS 11
 
@@ -51,6 +52,7 @@
 #define IMX415_OUTSEL		  CCI_REG8(0x30c0)
 #define IMX415_DRV		  CCI_REG8(0x30c1)
 #define IMX415_VMAX		  CCI_REG24_LE(0x3024)
+#define IMX415_VMAX_MAX		  0xfffff
 #define IMX415_HMAX		  CCI_REG16_LE(0x3028)
 #define IMX415_SHR0		  CCI_REG24_LE(0x3050)
 #define IMX415_GAIN_PCG_0	  CCI_REG16_LE(0x3090)
@@ -447,7 +449,6 @@ static const struct imx415_clk_params imx415_clk_params[] = {
 
 /* all-pixel 2-lane 720 Mbps 15.74 Hz mode */
 static const struct cci_reg_sequence imx415_mode_2_720[] = {
-	{ IMX415_VMAX, 0x08CA },
 	{ IMX415_HMAX, 0x07F0 },
 	{ IMX415_LANEMODE, IMX415_LANEMODE_2 },
 	{ IMX415_TCLKPOST, 0x006F },
@@ -463,7 +464,6 @@ static const struct cci_reg_sequence imx415_mode_2_720[] = {
 
 /* all-pixel 2-lane 1440 Mbps 30.01 Hz mode */
 static const struct cci_reg_sequence imx415_mode_2_1440[] = {
-	{ IMX415_VMAX, 0x08CA },
 	{ IMX415_HMAX, 0x042A },
 	{ IMX415_LANEMODE, IMX415_LANEMODE_2 },
 	{ IMX415_TCLKPOST, 0x009F },
@@ -479,7 +479,6 @@ static const struct cci_reg_sequence imx415_mode_2_1440[] = {
 
 /* all-pixel 4-lane 891 Mbps 30 Hz mode */
 static const struct cci_reg_sequence imx415_mode_4_891[] = {
-	{ IMX415_VMAX, 0x08CA },
 	{ IMX415_HMAX, 0x044C },
 	{ IMX415_LANEMODE, IMX415_LANEMODE_4 },
 	{ IMX415_TCLKPOST, 0x007F },
@@ -600,6 +599,7 @@ struct imx415 {
 	struct v4l2_ctrl *vblank;
 	struct v4l2_ctrl *hflip;
 	struct v4l2_ctrl *vflip;
+	struct v4l2_ctrl *exposure;
 
 	unsigned int cur_mode;
 	unsigned int num_data_lanes;
@@ -730,17 +730,38 @@ static int imx415_s_ctrl(struct v4l2_ctrl *ctrl)
 					     ctrls);
 	const struct v4l2_mbus_framefmt *format;
 	struct v4l2_subdev_state *state;
+	u32 exposure_max;
 	unsigned int vmax;
 	unsigned int flip;
 	int ret;
 
-	if (!pm_runtime_get_if_in_use(sensor->dev))
-		return 0;
-
 	state = v4l2_subdev_get_locked_active_state(&sensor->subdev);
 	format = v4l2_subdev_state_get_format(state, 0);
 
+	if (ctrl->id == V4L2_CID_VBLANK) {
+		exposure_max = format->height + ctrl->val -
+			       IMX415_EXPOSURE_OFFSET;
+		__v4l2_ctrl_modify_range(sensor->exposure,
+					 sensor->exposure->minimum,
+					 exposure_max, sensor->exposure->step,
+					 sensor->exposure->default_value);
+	}
+
+	if (!pm_runtime_get_if_in_use(sensor->dev))
+		return 0;
+
 	switch (ctrl->id) {
+	case V4L2_CID_VBLANK:
+		ret = cci_write(sensor->regmap, IMX415_VMAX,
+				format->height + ctrl->val, NULL);
+		if (ret)
+			return ret;
+		/*
+		 * Exposure is set based on VMAX which has just changed, so
+		 * program exposure register as well
+		 */
+		ctrl = sensor->exposure;
+		fallthrough;
 	case V4L2_CID_EXPOSURE:
 		/* clamp the exposure value to VMAX. */
 		vmax = format->height + sensor->vblank->cur.val;
@@ -787,7 +808,8 @@ static int imx415_ctrls_init(struct imx415 *sensor)
 	u64 pixel_rate = supported_modes[sensor->cur_mode].pixel_rate;
 	u64 lane_rate = supported_modes[sensor->cur_mode].lane_rate;
 	u32 exposure_max = IMX415_PIXEL_ARRAY_HEIGHT +
-			   IMX415_PIXEL_ARRAY_VBLANK - 8;
+			   IMX415_PIXEL_ARRAY_VBLANK -
+			   IMX415_EXPOSURE_OFFSET;
 	u32 hblank;
 	unsigned int i;
 	int ret;
@@ -816,8 +838,9 @@ static int imx415_ctrls_init(struct imx415 *sensor)
 	if (ctrl)
 		ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 
-	v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops, V4L2_CID_EXPOSURE,
-			  4, exposure_max, 1, exposure_max);
+	sensor->exposure = v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
+					     V4L2_CID_EXPOSURE, 4,
+					     exposure_max, 1, exposure_max);
 
 	v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
 			  V4L2_CID_ANALOGUE_GAIN, IMX415_AGAIN_MIN,
@@ -834,16 +857,9 @@ static int imx415_ctrls_init(struct imx415 *sensor)
 	sensor->vblank = v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
 					   V4L2_CID_VBLANK,
 					   IMX415_PIXEL_ARRAY_VBLANK,
-					   IMX415_PIXEL_ARRAY_VBLANK, 1,
-					   IMX415_PIXEL_ARRAY_VBLANK);
-	if (sensor->vblank)
-		sensor->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+					   IMX415_VMAX_MAX - IMX415_PIXEL_ARRAY_HEIGHT,
+					   1, IMX415_PIXEL_ARRAY_VBLANK);
 
-	/*
-	 * The pixel rate used here is a virtual value and can be used for
-	 * calculating the frame rate together with hblank. It may not
-	 * necessarily be the physically correct pixel clock.
-	 */
 	v4l2_ctrl_new_std(&sensor->ctrls, NULL, V4L2_CID_PIXEL_RATE, pixel_rate,
 			  pixel_rate, 1, pixel_rate);
 

-- 
2.34.1

[PATCH v3 2/3] media: i2c: imx415: Make HBLANK controllable and in consistent units

[Dave Stevenson]

The control of HMAX documented in the datasheet is consistent
with being in terms of a scaled INCK, being always 72MHz or
74.25MHz. It is NOT link frequency dependent, but the minimum
value for HMAX is dictated by the link frequency.

If PIXEL_RATE is defined as being 12 times the 72 or 74.25MHz,
and all values are scaled down again when writing HMAX, then
the numbers all work out regardless of INCK or link frequency.
Retain an hmax_min (set to the same value as the previous fixed
hmax register value) to set as the default value to avoid changing
the behaviour for existing users.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 drivers/media/i2c/imx415.c | 88 +++++++++++++++++++++-------------------------
 1 file changed, 40 insertions(+), 48 deletions(-)

diff --git a/drivers/media/i2c/imx415.c b/drivers/media/i2c/imx415.c
index fa7ffb9220e5..86dbcfcd820d 100644
--- a/drivers/media/i2c/imx415.c
+++ b/drivers/media/i2c/imx415.c
@@ -28,6 +28,9 @@
 #define IMX415_PIXEL_ARRAY_VBLANK 58
 #define IMX415_EXPOSURE_OFFSET	  8
 
+#define IMX415_PIXEL_RATE_74_25MHZ	891000000
+#define IMX415_PIXEL_RATE_72MHZ		864000000
+
 #define IMX415_NUM_CLK_PARAM_REGS 11
 
 #define IMX415_MODE		  CCI_REG8(0x3000)
@@ -54,6 +57,8 @@
 #define IMX415_VMAX		  CCI_REG24_LE(0x3024)
 #define IMX415_VMAX_MAX		  0xfffff
 #define IMX415_HMAX		  CCI_REG16_LE(0x3028)
+#define IMX415_HMAX_MAX		  0xffff
+#define IMX415_HMAX_MULTIPLIER	  12
 #define IMX415_SHR0		  CCI_REG24_LE(0x3050)
 #define IMX415_GAIN_PCG_0	  CCI_REG16_LE(0x3090)
 #define IMX415_AGAIN_MIN	  0
@@ -449,7 +454,6 @@ static const struct imx415_clk_params imx415_clk_params[] = {
 
 /* all-pixel 2-lane 720 Mbps 15.74 Hz mode */
 static const struct cci_reg_sequence imx415_mode_2_720[] = {
-	{ IMX415_HMAX, 0x07F0 },
 	{ IMX415_LANEMODE, IMX415_LANEMODE_2 },
 	{ IMX415_TCLKPOST, 0x006F },
 	{ IMX415_TCLKPREPARE, 0x002F },
@@ -464,7 +468,6 @@ static const struct cci_reg_sequence imx415_mode_2_720[] = {
 
 /* all-pixel 2-lane 1440 Mbps 30.01 Hz mode */
 static const struct cci_reg_sequence imx415_mode_2_1440[] = {
-	{ IMX415_HMAX, 0x042A },
 	{ IMX415_LANEMODE, IMX415_LANEMODE_2 },
 	{ IMX415_TCLKPOST, 0x009F },
 	{ IMX415_TCLKPREPARE, 0x0057 },
@@ -479,7 +482,6 @@ static const struct cci_reg_sequence imx415_mode_2_1440[] = {
 
 /* all-pixel 4-lane 891 Mbps 30 Hz mode */
 static const struct cci_reg_sequence imx415_mode_4_891[] = {
-	{ IMX415_HMAX, 0x044C },
 	{ IMX415_LANEMODE, IMX415_LANEMODE_4 },
 	{ IMX415_TCLKPOST, 0x007F },
 	{ IMX415_TCLKPREPARE, 0x0037 },
@@ -497,39 +499,10 @@ struct imx415_mode_reg_list {
 	const struct cci_reg_sequence *regs;
 };
 
-/*
- * Mode : number of lanes, lane rate and frame rate dependent settings
- *
- * pixel_rate and hmax_pix are needed to calculate hblank for the v4l2 ctrl
- * interface. These values can not be found in the data sheet and should be
- * treated as virtual values. Use following table when adding new modes.
- *
- * lane_rate  lanes    fps     hmax_pix   pixel_rate
- *
- *     594      2     10.000     4400       99000000
- *     891      2     15.000     4400      148500000
- *     720      2     15.748     4064      144000000
- *    1782      2     30.000     4400      297000000
- *    2079      2     30.000     4400      297000000
- *    1440      2     30.019     4510      304615385
- *
- *     594      4     20.000     5500      247500000
- *     594      4     25.000     4400      247500000
- *     720      4     25.000     4400      247500000
- *     720      4     30.019     4510      304615385
- *     891      4     30.000     4400      297000000
- *    1440      4     30.019     4510      304615385
- *    1440      4     60.038     4510      609230769
- *    1485      4     60.000     4400      594000000
- *    1782      4     60.000     4400      594000000
- *    2079      4     60.000     4400      594000000
- *    2376      4     90.164     4392      891000000
- */
 struct imx415_mode {
 	u64 lane_rate;
 	u32 lanes;
-	u32 hmax_pix;
-	u64 pixel_rate;
+	u32 hmax_min;
 	struct imx415_mode_reg_list reg_list;
 };
 
@@ -538,8 +511,7 @@ static const struct imx415_mode supported_modes[] = {
 	{
 		.lane_rate = 720000000,
 		.lanes = 2,
-		.hmax_pix = 4064,
-		.pixel_rate = 144000000,
+		.hmax_min = 2032,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(imx415_mode_2_720),
 			.regs = imx415_mode_2_720,
@@ -548,8 +520,7 @@ static const struct imx415_mode supported_modes[] = {
 	{
 		.lane_rate = 1440000000,
 		.lanes = 2,
-		.hmax_pix = 4510,
-		.pixel_rate = 304615385,
+		.hmax_min = 1066,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(imx415_mode_2_1440),
 			.regs = imx415_mode_2_1440,
@@ -558,8 +529,7 @@ static const struct imx415_mode supported_modes[] = {
 	{
 		.lane_rate = 891000000,
 		.lanes = 4,
-		.hmax_pix = 4400,
-		.pixel_rate = 297000000,
+		.hmax_min = 1100,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(imx415_mode_4_891),
 			.regs = imx415_mode_4_891,
@@ -586,6 +556,7 @@ static const char *const imx415_test_pattern_menu[] = {
 struct imx415 {
 	struct device *dev;
 	struct clk *clk;
+	unsigned long pixel_rate;
 	struct regulator_bulk_data supplies[ARRAY_SIZE(imx415_supply_names)];
 	struct gpio_desc *reset;
 	struct regmap *regmap;
@@ -597,6 +568,7 @@ struct imx415 {
 
 	struct v4l2_ctrl_handler ctrls;
 	struct v4l2_ctrl *vblank;
+	struct v4l2_ctrl *hblank;
 	struct v4l2_ctrl *hflip;
 	struct v4l2_ctrl *vflip;
 	struct v4l2_ctrl *exposure;
@@ -787,6 +759,13 @@ static int imx415_s_ctrl(struct v4l2_ctrl *ctrl)
 		ret = imx415_set_testpattern(sensor, ctrl->val);
 		break;
 
+	case V4L2_CID_HBLANK:
+		ret = cci_write(sensor->regmap, IMX415_HMAX,
+				(format->width + ctrl->val) /
+						IMX415_HMAX_MULTIPLIER,
+				NULL);
+		break;
+
 	default:
 		ret = -EINVAL;
 		break;
@@ -805,12 +784,11 @@ static int imx415_ctrls_init(struct imx415 *sensor)
 {
 	struct v4l2_fwnode_device_properties props;
 	struct v4l2_ctrl *ctrl;
-	u64 pixel_rate = supported_modes[sensor->cur_mode].pixel_rate;
 	u64 lane_rate = supported_modes[sensor->cur_mode].lane_rate;
 	u32 exposure_max = IMX415_PIXEL_ARRAY_HEIGHT +
 			   IMX415_PIXEL_ARRAY_VBLANK -
 			   IMX415_EXPOSURE_OFFSET;
-	u32 hblank;
+	u32 hblank_min, hblank_max;
 	unsigned int i;
 	int ret;
 
@@ -847,12 +825,14 @@ static int imx415_ctrls_init(struct imx415 *sensor)
 			  IMX415_AGAIN_MAX, IMX415_AGAIN_STEP,
 			  IMX415_AGAIN_MIN);
 
-	hblank = supported_modes[sensor->cur_mode].hmax_pix -
-		 IMX415_PIXEL_ARRAY_WIDTH;
+	hblank_min = (supported_modes[sensor->cur_mode].hmax_min *
+		      IMX415_HMAX_MULTIPLIER) - IMX415_PIXEL_ARRAY_WIDTH;
+	hblank_max = (IMX415_HMAX_MAX * IMX415_HMAX_MULTIPLIER) -
+		     IMX415_PIXEL_ARRAY_WIDTH;
 	ctrl = v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
-				 V4L2_CID_HBLANK, hblank, hblank, 1, hblank);
-	if (ctrl)
-		ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+				 V4L2_CID_HBLANK, hblank_min,
+				 hblank_max, IMX415_HMAX_MULTIPLIER,
+				 hblank_min);
 
 	sensor->vblank = v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
 					   V4L2_CID_VBLANK,
@@ -860,8 +840,9 @@ static int imx415_ctrls_init(struct imx415 *sensor)
 					   IMX415_VMAX_MAX - IMX415_PIXEL_ARRAY_HEIGHT,
 					   1, IMX415_PIXEL_ARRAY_VBLANK);
 
-	v4l2_ctrl_new_std(&sensor->ctrls, NULL, V4L2_CID_PIXEL_RATE, pixel_rate,
-			  pixel_rate, 1, pixel_rate);
+	v4l2_ctrl_new_std(&sensor->ctrls, NULL, V4L2_CID_PIXEL_RATE,
+			  sensor->pixel_rate, sensor->pixel_rate, 1,
+			  sensor->pixel_rate);
 
 	sensor->hflip = v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
 					  V4L2_CID_HFLIP, 0, 1, 1, 0);
@@ -1333,6 +1314,17 @@ static int imx415_parse_hw_config(struct imx415 *sensor)
 				    "no valid sensor mode defined\n");
 		goto done_endpoint_free;
 	}
+	switch (inck) {
+	case 27000000:
+	case 37125000:
+	case 74250000:
+		sensor->pixel_rate = IMX415_PIXEL_RATE_74_25MHZ;
+		break;
+	case 24000000:
+	case 72000000:
+		sensor->pixel_rate = IMX415_PIXEL_RATE_72MHZ;
+		break;
+	}
 
 	lane_rate = supported_modes[sensor->cur_mode].lane_rate;
 	for (i = 0; i < ARRAY_SIZE(imx415_clk_params); ++i) {

-- 
2.34.1

[PATCH v3 3/3] media: i2c: imx415: Link frequencies are not exclusive to num lanes

[Dave Stevenson]

The link frequencies are equally valid in 2 or 4 lane modes, but
they change the hmax_min value for the mode as the MIPI block
has to have sufficient time to send the pixel data for each line.

Remove the association with number of lanes, and add hmax_min
configuration for both lane options.

Reviewed-by: Michael Riesch <michael.riesch@wolfvision.net>
Reviewed-by: Gerald Loacker <gerald.loacker@wolfvision.net>
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 drivers/media/i2c/imx415.c | 53 ++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 28 deletions(-)

diff --git a/drivers/media/i2c/imx415.c b/drivers/media/i2c/imx415.c
index 86dbcfcd820d..9f37779bd611 100644
--- a/drivers/media/i2c/imx415.c
+++ b/drivers/media/i2c/imx415.c
@@ -452,9 +452,8 @@ static const struct imx415_clk_params imx415_clk_params[] = {
 	},
 };
 
-/* all-pixel 2-lane 720 Mbps 15.74 Hz mode */
-static const struct cci_reg_sequence imx415_mode_2_720[] = {
-	{ IMX415_LANEMODE, IMX415_LANEMODE_2 },
+/* 720 Mbps CSI configuration */
+static const struct cci_reg_sequence imx415_linkrate_720mbps[] = {
 	{ IMX415_TCLKPOST, 0x006F },
 	{ IMX415_TCLKPREPARE, 0x002F },
 	{ IMX415_TCLKTRAIL, 0x002F },
@@ -466,9 +465,8 @@ static const struct cci_reg_sequence imx415_mode_2_720[] = {
 	{ IMX415_TLPX, 0x0027 },
 };
 
-/* all-pixel 2-lane 1440 Mbps 30.01 Hz mode */
-static const struct cci_reg_sequence imx415_mode_2_1440[] = {
-	{ IMX415_LANEMODE, IMX415_LANEMODE_2 },
+/* 1440 Mbps CSI configuration */
+static const struct cci_reg_sequence imx415_linkrate_1440mbps[] = {
 	{ IMX415_TCLKPOST, 0x009F },
 	{ IMX415_TCLKPREPARE, 0x0057 },
 	{ IMX415_TCLKTRAIL, 0x0057 },
@@ -480,9 +478,8 @@ static const struct cci_reg_sequence imx415_mode_2_1440[] = {
 	{ IMX415_TLPX, 0x004F },
 };
 
-/* all-pixel 4-lane 891 Mbps 30 Hz mode */
-static const struct cci_reg_sequence imx415_mode_4_891[] = {
-	{ IMX415_LANEMODE, IMX415_LANEMODE_4 },
+/* 891 Mbps CSI configuration */
+static const struct cci_reg_sequence imx415_linkrate_891mbps[] = {
 	{ IMX415_TCLKPOST, 0x007F },
 	{ IMX415_TCLKPREPARE, 0x0037 },
 	{ IMX415_TCLKTRAIL, 0x0037 },
@@ -501,8 +498,7 @@ struct imx415_mode_reg_list {
 
 struct imx415_mode {
 	u64 lane_rate;
-	u32 lanes;
-	u32 hmax_min;
+	u32 hmax_min[2];
 	struct imx415_mode_reg_list reg_list;
 };
 
@@ -510,29 +506,26 @@ struct imx415_mode {
 static const struct imx415_mode supported_modes[] = {
 	{
 		.lane_rate = 720000000,
-		.lanes = 2,
-		.hmax_min = 2032,
+		.hmax_min = { 2032, 1066 },
 		.reg_list = {
-			.num_of_regs = ARRAY_SIZE(imx415_mode_2_720),
-			.regs = imx415_mode_2_720,
+			.num_of_regs = ARRAY_SIZE(imx415_linkrate_720mbps),
+			.regs = imx415_linkrate_720mbps,
 		},
 	},
 	{
 		.lane_rate = 1440000000,
-		.lanes = 2,
-		.hmax_min = 1066,
+		.hmax_min = { 1066, 533 },
 		.reg_list = {
-			.num_of_regs = ARRAY_SIZE(imx415_mode_2_1440),
-			.regs = imx415_mode_2_1440,
+			.num_of_regs = ARRAY_SIZE(imx415_linkrate_1440mbps),
+			.regs = imx415_linkrate_1440mbps,
 		},
 	},
 	{
 		.lane_rate = 891000000,
-		.lanes = 4,
-		.hmax_min = 1100,
+		.hmax_min = { 2200, 1100 },
 		.reg_list = {
-			.num_of_regs = ARRAY_SIZE(imx415_mode_4_891),
-			.regs = imx415_mode_4_891,
+			.num_of_regs = ARRAY_SIZE(imx415_linkrate_891mbps),
+			.regs = imx415_linkrate_891mbps,
 		},
 	},
 };
@@ -784,7 +777,8 @@ static int imx415_ctrls_init(struct imx415 *sensor)
 {
 	struct v4l2_fwnode_device_properties props;
 	struct v4l2_ctrl *ctrl;
-	u64 lane_rate = supported_modes[sensor->cur_mode].lane_rate;
+	const struct imx415_mode *cur_mode = &supported_modes[sensor->cur_mode];
+	u64 lane_rate = cur_mode->lane_rate;
 	u32 exposure_max = IMX415_PIXEL_ARRAY_HEIGHT +
 			   IMX415_PIXEL_ARRAY_VBLANK -
 			   IMX415_EXPOSURE_OFFSET;
@@ -825,7 +819,7 @@ static int imx415_ctrls_init(struct imx415 *sensor)
 			  IMX415_AGAIN_MAX, IMX415_AGAIN_STEP,
 			  IMX415_AGAIN_MIN);
 
-	hblank_min = (supported_modes[sensor->cur_mode].hmax_min *
+	hblank_min = (cur_mode->hmax_min[sensor->num_data_lanes == 2 ? 0 : 1] *
 		      IMX415_HMAX_MULTIPLIER) - IMX415_PIXEL_ARRAY_WIDTH;
 	hblank_max = (IMX415_HMAX_MAX * IMX415_HMAX_MULTIPLIER) -
 		     IMX415_PIXEL_ARRAY_WIDTH;
@@ -887,7 +881,12 @@ static int imx415_set_mode(struct imx415 *sensor, int mode)
 			    IMX415_NUM_CLK_PARAM_REGS,
 			    &ret);
 
-	return 0;
+	ret = cci_write(sensor->regmap, IMX415_LANEMODE,
+			sensor->num_data_lanes == 2 ? IMX415_LANEMODE_2 :
+						      IMX415_LANEMODE_4,
+			NULL);
+
+	return ret;
 }
 
 static int imx415_setup(struct imx415 *sensor, struct v4l2_subdev_state *state)
@@ -1298,8 +1297,6 @@ static int imx415_parse_hw_config(struct imx415 *sensor)
 		}
 
 		for (j = 0; j < ARRAY_SIZE(supported_modes); ++j) {
-			if (sensor->num_data_lanes != supported_modes[j].lanes)
-				continue;
 			if (bus_cfg.link_frequencies[i] * 2 !=
 			    supported_modes[j].lane_rate)
 				continue;

-- 
2.34.1

Re: [PATCH v3 2/3] media: i2c: imx415: Make HBLANK controllable and in consistent units

[Michael Riesch]

Hi Dave,

Thanks for the update -- good catch.

On 1/29/25 16:03, Dave Stevenson wrote:
> The control of HMAX documented in the datasheet is consistent
> with being in terms of a scaled INCK, being always 72MHz or
> 74.25MHz. It is NOT link frequency dependent, but the minimum
> value for HMAX is dictated by the link frequency.
> 
> If PIXEL_RATE is defined as being 12 times the 72 or 74.25MHz,
> and all values are scaled down again when writing HMAX, then
> the numbers all work out regardless of INCK or link frequency.
> Retain an hmax_min (set to the same value as the previous fixed
> hmax register value) to set as the default value to avoid changing
> the behaviour for existing users.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

Reviewed-by: Michael Riesch <michael.riesch@wolfvision.net>

Thanks and regards,
Michael


> ---
>  drivers/media/i2c/imx415.c | 88 +++++++++++++++++++++-------------------------
>  1 file changed, 40 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx415.c b/drivers/media/i2c/imx415.c
> index fa7ffb9220e5..86dbcfcd820d 100644
> --- a/drivers/media/i2c/imx415.c
> +++ b/drivers/media/i2c/imx415.c
> @@ -28,6 +28,9 @@
>  #define IMX415_PIXEL_ARRAY_VBLANK 58
>  #define IMX415_EXPOSURE_OFFSET	  8
>  
> +#define IMX415_PIXEL_RATE_74_25MHZ	891000000
> +#define IMX415_PIXEL_RATE_72MHZ		864000000
> +
>  #define IMX415_NUM_CLK_PARAM_REGS 11
>  
>  #define IMX415_MODE		  CCI_REG8(0x3000)
> @@ -54,6 +57,8 @@
>  #define IMX415_VMAX		  CCI_REG24_LE(0x3024)
>  #define IMX415_VMAX_MAX		  0xfffff
>  #define IMX415_HMAX		  CCI_REG16_LE(0x3028)
> +#define IMX415_HMAX_MAX		  0xffff
> +#define IMX415_HMAX_MULTIPLIER	  12
>  #define IMX415_SHR0		  CCI_REG24_LE(0x3050)
>  #define IMX415_GAIN_PCG_0	  CCI_REG16_LE(0x3090)
>  #define IMX415_AGAIN_MIN	  0
> @@ -449,7 +454,6 @@ static const struct imx415_clk_params imx415_clk_params[] = {
>  
>  /* all-pixel 2-lane 720 Mbps 15.74 Hz mode */
>  static const struct cci_reg_sequence imx415_mode_2_720[] = {
> -	{ IMX415_HMAX, 0x07F0 },
>  	{ IMX415_LANEMODE, IMX415_LANEMODE_2 },
>  	{ IMX415_TCLKPOST, 0x006F },
>  	{ IMX415_TCLKPREPARE, 0x002F },
> @@ -464,7 +468,6 @@ static const struct cci_reg_sequence imx415_mode_2_720[] = {
>  
>  /* all-pixel 2-lane 1440 Mbps 30.01 Hz mode */
>  static const struct cci_reg_sequence imx415_mode_2_1440[] = {
> -	{ IMX415_HMAX, 0x042A },
>  	{ IMX415_LANEMODE, IMX415_LANEMODE_2 },
>  	{ IMX415_TCLKPOST, 0x009F },
>  	{ IMX415_TCLKPREPARE, 0x0057 },
> @@ -479,7 +482,6 @@ static const struct cci_reg_sequence imx415_mode_2_1440[] = {
>  
>  /* all-pixel 4-lane 891 Mbps 30 Hz mode */
>  static const struct cci_reg_sequence imx415_mode_4_891[] = {
> -	{ IMX415_HMAX, 0x044C },
>  	{ IMX415_LANEMODE, IMX415_LANEMODE_4 },
>  	{ IMX415_TCLKPOST, 0x007F },
>  	{ IMX415_TCLKPREPARE, 0x0037 },
> @@ -497,39 +499,10 @@ struct imx415_mode_reg_list {
>  	const struct cci_reg_sequence *regs;
>  };
>  
> -/*
> - * Mode : number of lanes, lane rate and frame rate dependent settings
> - *
> - * pixel_rate and hmax_pix are needed to calculate hblank for the v4l2 ctrl
> - * interface. These values can not be found in the data sheet and should be
> - * treated as virtual values. Use following table when adding new modes.
> - *
> - * lane_rate  lanes    fps     hmax_pix   pixel_rate
> - *
> - *     594      2     10.000     4400       99000000
> - *     891      2     15.000     4400      148500000
> - *     720      2     15.748     4064      144000000
> - *    1782      2     30.000     4400      297000000
> - *    2079      2     30.000     4400      297000000
> - *    1440      2     30.019     4510      304615385
> - *
> - *     594      4     20.000     5500      247500000
> - *     594      4     25.000     4400      247500000
> - *     720      4     25.000     4400      247500000
> - *     720      4     30.019     4510      304615385
> - *     891      4     30.000     4400      297000000
> - *    1440      4     30.019     4510      304615385
> - *    1440      4     60.038     4510      609230769
> - *    1485      4     60.000     4400      594000000
> - *    1782      4     60.000     4400      594000000
> - *    2079      4     60.000     4400      594000000
> - *    2376      4     90.164     4392      891000000
> - */
>  struct imx415_mode {
>  	u64 lane_rate;
>  	u32 lanes;
> -	u32 hmax_pix;
> -	u64 pixel_rate;
> +	u32 hmax_min;
>  	struct imx415_mode_reg_list reg_list;
>  };
>  
> @@ -538,8 +511,7 @@ static const struct imx415_mode supported_modes[] = {
>  	{
>  		.lane_rate = 720000000,
>  		.lanes = 2,
> -		.hmax_pix = 4064,
> -		.pixel_rate = 144000000,
> +		.hmax_min = 2032,
>  		.reg_list = {
>  			.num_of_regs = ARRAY_SIZE(imx415_mode_2_720),
>  			.regs = imx415_mode_2_720,
> @@ -548,8 +520,7 @@ static const struct imx415_mode supported_modes[] = {
>  	{
>  		.lane_rate = 1440000000,
>  		.lanes = 2,
> -		.hmax_pix = 4510,
> -		.pixel_rate = 304615385,
> +		.hmax_min = 1066,
>  		.reg_list = {
>  			.num_of_regs = ARRAY_SIZE(imx415_mode_2_1440),
>  			.regs = imx415_mode_2_1440,
> @@ -558,8 +529,7 @@ static const struct imx415_mode supported_modes[] = {
>  	{
>  		.lane_rate = 891000000,
>  		.lanes = 4,
> -		.hmax_pix = 4400,
> -		.pixel_rate = 297000000,
> +		.hmax_min = 1100,
>  		.reg_list = {
>  			.num_of_regs = ARRAY_SIZE(imx415_mode_4_891),
>  			.regs = imx415_mode_4_891,
> @@ -586,6 +556,7 @@ static const char *const imx415_test_pattern_menu[] = {
>  struct imx415 {
>  	struct device *dev;
>  	struct clk *clk;
> +	unsigned long pixel_rate;
>  	struct regulator_bulk_data supplies[ARRAY_SIZE(imx415_supply_names)];
>  	struct gpio_desc *reset;
>  	struct regmap *regmap;
> @@ -597,6 +568,7 @@ struct imx415 {
>  
>  	struct v4l2_ctrl_handler ctrls;
>  	struct v4l2_ctrl *vblank;
> +	struct v4l2_ctrl *hblank;
>  	struct v4l2_ctrl *hflip;
>  	struct v4l2_ctrl *vflip;
>  	struct v4l2_ctrl *exposure;
> @@ -787,6 +759,13 @@ static int imx415_s_ctrl(struct v4l2_ctrl *ctrl)
>  		ret = imx415_set_testpattern(sensor, ctrl->val);
>  		break;
>  
> +	case V4L2_CID_HBLANK:
> +		ret = cci_write(sensor->regmap, IMX415_HMAX,
> +				(format->width + ctrl->val) /
> +						IMX415_HMAX_MULTIPLIER,
> +				NULL);
> +		break;
> +
>  	default:
>  		ret = -EINVAL;
>  		break;
> @@ -805,12 +784,11 @@ static int imx415_ctrls_init(struct imx415 *sensor)
>  {
>  	struct v4l2_fwnode_device_properties props;
>  	struct v4l2_ctrl *ctrl;
> -	u64 pixel_rate = supported_modes[sensor->cur_mode].pixel_rate;
>  	u64 lane_rate = supported_modes[sensor->cur_mode].lane_rate;
>  	u32 exposure_max = IMX415_PIXEL_ARRAY_HEIGHT +
>  			   IMX415_PIXEL_ARRAY_VBLANK -
>  			   IMX415_EXPOSURE_OFFSET;
> -	u32 hblank;
> +	u32 hblank_min, hblank_max;
>  	unsigned int i;
>  	int ret;
>  
> @@ -847,12 +825,14 @@ static int imx415_ctrls_init(struct imx415 *sensor)
>  			  IMX415_AGAIN_MAX, IMX415_AGAIN_STEP,
>  			  IMX415_AGAIN_MIN);
>  
> -	hblank = supported_modes[sensor->cur_mode].hmax_pix -
> -		 IMX415_PIXEL_ARRAY_WIDTH;
> +	hblank_min = (supported_modes[sensor->cur_mode].hmax_min *
> +		      IMX415_HMAX_MULTIPLIER) - IMX415_PIXEL_ARRAY_WIDTH;
> +	hblank_max = (IMX415_HMAX_MAX * IMX415_HMAX_MULTIPLIER) -
> +		     IMX415_PIXEL_ARRAY_WIDTH;
>  	ctrl = v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
> -				 V4L2_CID_HBLANK, hblank, hblank, 1, hblank);
> -	if (ctrl)
> -		ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +				 V4L2_CID_HBLANK, hblank_min,
> +				 hblank_max, IMX415_HMAX_MULTIPLIER,
> +				 hblank_min);
>  
>  	sensor->vblank = v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
>  					   V4L2_CID_VBLANK,
> @@ -860,8 +840,9 @@ static int imx415_ctrls_init(struct imx415 *sensor)
>  					   IMX415_VMAX_MAX - IMX415_PIXEL_ARRAY_HEIGHT,
>  					   1, IMX415_PIXEL_ARRAY_VBLANK);
>  
> -	v4l2_ctrl_new_std(&sensor->ctrls, NULL, V4L2_CID_PIXEL_RATE, pixel_rate,
> -			  pixel_rate, 1, pixel_rate);
> +	v4l2_ctrl_new_std(&sensor->ctrls, NULL, V4L2_CID_PIXEL_RATE,
> +			  sensor->pixel_rate, sensor->pixel_rate, 1,
> +			  sensor->pixel_rate);
>  
>  	sensor->hflip = v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
>  					  V4L2_CID_HFLIP, 0, 1, 1, 0);
> @@ -1333,6 +1314,17 @@ static int imx415_parse_hw_config(struct imx415 *sensor)
>  				    "no valid sensor mode defined\n");
>  		goto done_endpoint_free;
>  	}
> +	switch (inck) {
> +	case 27000000:
> +	case 37125000:
> +	case 74250000:
> +		sensor->pixel_rate = IMX415_PIXEL_RATE_74_25MHZ;
> +		break;
> +	case 24000000:
> +	case 72000000:
> +		sensor->pixel_rate = IMX415_PIXEL_RATE_72MHZ;
> +		break;
> +	}
>  
>  	lane_rate = supported_modes[sensor->cur_mode].lane_rate;
>  	for (i = 0; i < ARRAY_SIZE(imx415_clk_params); ++i) {
>