| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8e83588f-7e4e-4805-a681-473b9233d04f@wolfvision.net>
Date: Thu, 30 Jan 2025 08:42:37 +0100
From: Michael Riesch <michael.riesch@...fvision.net>
To: Dave Stevenson <dave.stevenson@...pberrypi.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: Gerald Loacker <Gerald.Loacker@...fvision.net>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/3] media: i2c: imx415: Make HBLANK controllable and
in consistent units
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@...pberrypi.com>
Reviewed-by: Michael Riesch <michael.riesch@...fvision.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) {
>
Powered by blists - more mailing lists