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@xxxxxxxxxxxxxxx> Reviewed-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx> 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) { >