| 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: <CAPY8ntDxCZnoaVY3-TsbsFgsj=WAwLa74vGQrf7xU2M2YgrVzg@mail.gmail.com>
Date: Tue, 14 Jan 2025 12:15:58 +0000
From: Dave Stevenson <dave.stevenson@...pberrypi.com>
To: Gerald Loacker <Gerald.Loacker@...fvision.net>
Cc: Sakari Ailus <sakari.ailus@...ux.intel.com>,
Michael Riesch <Michael.Riesch@...fvision.net>, Mauro Carvalho Chehab <mchehab@...nel.org>,
"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] media: i2c: imx415: Link frequencies are not
exclusive to num lanes
Hi Gerald
On Tue, 14 Jan 2025 at 11:02, Gerald Loacker
<Gerald.Loacker@...fvision.net> wrote:
>
> Hi Dave,
>
> Thanks for the insight into the imx415 architecture and for implementing
> this correctly.
Please note that the insight is from external inspection rather than
having any specific knowledge of the insides of the sensor.
Having worked with imx290 before, all the Starvis sensors seem to
follow the same pattern. I don't know about the Starvis2 sensors
though.
> I've tested the 4-lane 891 Mbit/s mode (supported_modes[2]), and with the
> modification described below, it worked fine for me.
Thanks for testing!
> > -----Ursprüngliche Nachricht-----
> > Von: Dave Stevenson <dave.stevenson@...pberrypi.com>
> > Gesendet: Donnerstag, 9. Januar 2025 12:17
> > An: Sakari Ailus <sakari.ailus@...ux.intel.com>; Michael Riesch
> > <Michael.Riesch@...fvision.net>; Mauro Carvalho Chehab
> > <mchehab@...nel.org>
> > Cc: linux-media@...r.kernel.org; linux-kernel@...r.kernel.org; Dave Stevenson
> > <dave.stevenson@...pberrypi.com>
> > Betreff: [PATCH 3/3] media: i2c: imx415: Link frequencies are not exclusive to num
> > lanes
> >
> > 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.
> >
> > Signed-off-by: Dave Stevenson <dave.stevenson@...pberrypi.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 e23b41027987..1071900416d2 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 },
>
> 1016?
I'd taken the number from the datasheet. (My copy was found off a
websearch at https://www.alldatasheet.com/datasheet-pdf/download/1643695/SONY/IMX415.html)
All-pixel mode List of Setting Register.
Page 54, 2 lanes, 720Mbit/s for 15.74fps lists HMAX as 7F0h = 2032
Page 55, 4 lanes, 720Mbit/s for 30.01fps lists HMAX as 42Ah = 1066
1016 may work, but with LP<>HS transitions on D-PHY often being time
related rather than bitrate, it doesn't always follow.
My preference is to follow the datasheet here as the extra FPS is
unlikely to be useful - most users will want 30fps.
> > .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 = { 1100, 550 },
>
> These values result in a frame rate of 60 Hz, but the MIPI interface can only
> transfer 30 fps at 891Mbit/s. Shouldn't it be { 2200, 1100 }?
I've obviously grabbed the wrong column from the datasheet.
As you say:
2 lanes, 891Mbit/s is HMAX of 898h = 2200
4 lanes, 891Mbit/s is HMAX of 44Ch = 1100.
The module I had been testing with had a 24MHz clock. I've just been
told that one of the modules I have has a 37.125MHz clock source, so I
should be able to verify this mode.
Hopefully I can get a V2 out in the next couple of days.
Cheers,
Dave
> Regards,
> Gerald
>
> > .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,
> > },
> > },
> > };
> > @@ -782,7 +775,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;
> > @@ -823,7 +817,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;
> > @@ -885,7 +879,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)
> > @@ -1296,8 +1295,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
> >
>
Powered by blists - more mailing lists