| Message ID | 20240306081038.212412-2-umang.jain@ideasonboard.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: imx335: 2/4 lane ops and improvements | expand |
Hi Umang and Kieran On Wed, 6 Mar 2024 at 08:11, Umang Jain <umang.jain@ideasonboard.com> wrote: > > From: Kieran Bingham <kieran.bingham@ideasonboard.com> > > The IMX335 can support both 2 and 4 lane configurations. > Extend the driver to configure the lane mode accordingly. > Update the pixel rate depending on the number of lanes in use. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > drivers/media/i2c/imx335.c | 46 +++++++++++++++++++++++++++++++------- > 1 file changed, 38 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c > index dab6d080bc4c..a42f48823515 100644 > --- a/drivers/media/i2c/imx335.c > +++ b/drivers/media/i2c/imx335.c > @@ -21,6 +21,11 @@ > #define IMX335_MODE_STANDBY 0x01 > #define IMX335_MODE_STREAMING 0x00 > > +/* Data Lanes */ > +#define IMX335_LANEMODE 0x3a01 > +#define IMX335_2LANE 1 > +#define IMX335_4LANE 3 > + > /* Lines per frame */ > #define IMX335_REG_LPFR 0x3030 > > @@ -67,8 +72,6 @@ > #define IMX335_LINK_FREQ_594MHz 594000000LL > #define IMX335_LINK_FREQ_445MHz 445500000LL > > -#define IMX335_NUM_DATA_LANES 4 > - > #define IMX335_REG_MIN 0x00 > #define IMX335_REG_MAX 0xfffff > > @@ -115,7 +118,6 @@ static const char * const imx335_supply_name[] = { > * @vblank: Vertical blanking in lines > * @vblank_min: Minimum vertical blanking in lines > * @vblank_max: Maximum vertical blanking in lines > - * @pclk: Sensor pixel clock > * @reg_list: Register list for sensor mode > */ > struct imx335_mode { > @@ -126,7 +128,6 @@ struct imx335_mode { > u32 vblank; > u32 vblank_min; > u32 vblank_max; > - u64 pclk; > struct imx335_reg_list reg_list; > }; > > @@ -147,6 +148,7 @@ struct imx335_mode { > * @exp_ctrl: Pointer to exposure control > * @again_ctrl: Pointer to analog gain control > * @vblank: Vertical blanking in lines > + * @lane_mode Mode for number of connected data lanes > * @cur_mode: Pointer to current selected sensor mode > * @mutex: Mutex for serializing sensor controls > * @link_freq_bitmap: Menu bitmap for link_freq_ctrl > @@ -171,6 +173,7 @@ struct imx335 { > struct v4l2_ctrl *again_ctrl; > }; > u32 vblank; > + u32 lane_mode; > const struct imx335_mode *cur_mode; > struct mutex mutex; > unsigned long link_freq_bitmap; > @@ -377,7 +380,6 @@ static const struct imx335_mode supported_mode = { > .vblank = 2560, > .vblank_min = 2560, > .vblank_max = 133060, > - .pclk = 396000000, > .reg_list = { > .num_of_regs = ARRAY_SIZE(mode_2592x1940_regs), > .regs = mode_2592x1940_regs, > @@ -936,6 +938,11 @@ static int imx335_start_streaming(struct imx335 *imx335) > return ret; > } > > + /* Configure lanes */ > + ret = imx335_write_reg(imx335, IMX335_LANEMODE, 1, imx335->lane_mode); > + if (ret) > + return ret; > + > /* Setup handler will write actual exposure and gain */ > ret = __v4l2_ctrl_handler_setup(imx335->sd.ctrl_handler); > if (ret) { > @@ -1096,7 +1103,14 @@ static int imx335_parse_hw_config(struct imx335 *imx335) > if (ret) > return ret; > > - if (bus_cfg.bus.mipi_csi2.num_data_lanes != IMX335_NUM_DATA_LANES) { > + switch (bus_cfg.bus.mipi_csi2.num_data_lanes) { > + case 2: > + imx335->lane_mode = IMX335_2LANE; > + break; > + case 4: > + imx335->lane_mode = IMX335_4LANE; > + break; > + default: > dev_err(imx335->dev, > "number of CSI2 data lanes %d is not supported\n", > bus_cfg.bus.mipi_csi2.num_data_lanes); > @@ -1209,6 +1223,9 @@ static int imx335_init_controls(struct imx335 *imx335) > struct v4l2_ctrl_handler *ctrl_hdlr = &imx335->ctrl_handler; > const struct imx335_mode *mode = imx335->cur_mode; > u32 lpfr; > + u64 pclk; > + s64 link_freq_in_use; > + u8 bpp; > int ret; > > ret = v4l2_ctrl_handler_init(ctrl_hdlr, 7); > @@ -1252,11 +1269,24 @@ static int imx335_init_controls(struct imx335 *imx335) > 0, 0, imx335_tpg_menu); > > /* Read only controls */ > + > + /* pixel rate = link frequency * lanes * 2 / bits_per_pixel */ > + switch (imx335->cur_mbus_code) { > + case MEDIA_BUS_FMT_SRGGB10_1X10: > + bpp = 10; > + break; > + case MEDIA_BUS_FMT_SRGGB12_1X12: > + bpp = 12; > + break; > + } > + > + link_freq_in_use = link_freq[__ffs(imx335->link_freq_bitmap)]; > + pclk = link_freq_in_use * (imx335->lane_mode + 1) * 2 / bpp; > imx335->pclk_ctrl = v4l2_ctrl_new_std(ctrl_hdlr, > &imx335_ctrl_ops, > V4L2_CID_PIXEL_RATE, > - mode->pclk, mode->pclk, > - 1, mode->pclk); > + pclk, pclk, > + 1, pclk); Is this actually correct? A fair number of the sensors I've encountered have 2 PLL paths - one for the pixel array, and one for the CSI block. The bpp will generally be fed into the CSI block PLL path, but not into the pixel array one. The link frequency will therefore vary with bit depth, but V4L2_CID_PIXEL_RATE doesn't change. imx290 certainly has a disjoin between pixel rate and link freq (cropping reduces link freq, but not pixel rate), and we run imx477 in 2 lane mode with the pixel array at full tilt (840MPix/s) but large horizontal blanking to allow CSI2 enough time to send the data. If you've validated that for a range of frame rates you get the correct output from the sensor in both 10 and 12 bit modes, then I don't object. I just have an instinctive tick whenever I see drivers computing PIXEL_RATE from LINK_FREQ or vice versa :) If you get the right frame rate it may also imply that the link frequency isn't as configured, but that rarely has any negative effects. You need a reasonably good oscilloscope to be able to measure the link frequency. Dave > > imx335->link_freq_ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, > &imx335_ctrl_ops, > -- > 2.43.0 > >
Hi Umang, On Wed, Mar 06, 2024 at 01:40:34PM +0530, Umang Jain wrote: > From: Kieran Bingham <kieran.bingham@ideasonboard.com> > > The IMX335 can support both 2 and 4 lane configurations. > Extend the driver to configure the lane mode accordingly. > Update the pixel rate depending on the number of lanes in use. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > drivers/media/i2c/imx335.c | 46 +++++++++++++++++++++++++++++++------- > 1 file changed, 38 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c > index dab6d080bc4c..a42f48823515 100644 > --- a/drivers/media/i2c/imx335.c > +++ b/drivers/media/i2c/imx335.c > @@ -21,6 +21,11 @@ > #define IMX335_MODE_STANDBY 0x01 > #define IMX335_MODE_STREAMING 0x00 > > +/* Data Lanes */ > +#define IMX335_LANEMODE 0x3a01 > +#define IMX335_2LANE 1 > +#define IMX335_4LANE 3 > + > /* Lines per frame */ > #define IMX335_REG_LPFR 0x3030 > > @@ -67,8 +72,6 @@ > #define IMX335_LINK_FREQ_594MHz 594000000LL > #define IMX335_LINK_FREQ_445MHz 445500000LL > > -#define IMX335_NUM_DATA_LANES 4 > - > #define IMX335_REG_MIN 0x00 > #define IMX335_REG_MAX 0xfffff > > @@ -115,7 +118,6 @@ static const char * const imx335_supply_name[] = { > * @vblank: Vertical blanking in lines > * @vblank_min: Minimum vertical blanking in lines > * @vblank_max: Maximum vertical blanking in lines > - * @pclk: Sensor pixel clock > * @reg_list: Register list for sensor mode > */ > struct imx335_mode { > @@ -126,7 +128,6 @@ struct imx335_mode { > u32 vblank; > u32 vblank_min; > u32 vblank_max; > - u64 pclk; > struct imx335_reg_list reg_list; > }; > > @@ -147,6 +148,7 @@ struct imx335_mode { > * @exp_ctrl: Pointer to exposure control > * @again_ctrl: Pointer to analog gain control > * @vblank: Vertical blanking in lines > + * @lane_mode Mode for number of connected data lanes > * @cur_mode: Pointer to current selected sensor mode > * @mutex: Mutex for serializing sensor controls > * @link_freq_bitmap: Menu bitmap for link_freq_ctrl > @@ -171,6 +173,7 @@ struct imx335 { > struct v4l2_ctrl *again_ctrl; > }; > u32 vblank; > + u32 lane_mode; > const struct imx335_mode *cur_mode; > struct mutex mutex; > unsigned long link_freq_bitmap; > @@ -377,7 +380,6 @@ static const struct imx335_mode supported_mode = { > .vblank = 2560, > .vblank_min = 2560, > .vblank_max = 133060, > - .pclk = 396000000, > .reg_list = { > .num_of_regs = ARRAY_SIZE(mode_2592x1940_regs), > .regs = mode_2592x1940_regs, > @@ -936,6 +938,11 @@ static int imx335_start_streaming(struct imx335 *imx335) > return ret; > } > > + /* Configure lanes */ > + ret = imx335_write_reg(imx335, IMX335_LANEMODE, 1, imx335->lane_mode); > + if (ret) > + return ret; > + > /* Setup handler will write actual exposure and gain */ > ret = __v4l2_ctrl_handler_setup(imx335->sd.ctrl_handler); > if (ret) { > @@ -1096,7 +1103,14 @@ static int imx335_parse_hw_config(struct imx335 *imx335) > if (ret) > return ret; > > - if (bus_cfg.bus.mipi_csi2.num_data_lanes != IMX335_NUM_DATA_LANES) { > + switch (bus_cfg.bus.mipi_csi2.num_data_lanes) { > + case 2: > + imx335->lane_mode = IMX335_2LANE; > + break; > + case 4: > + imx335->lane_mode = IMX335_4LANE; > + break; > + default: > dev_err(imx335->dev, > "number of CSI2 data lanes %d is not supported\n", > bus_cfg.bus.mipi_csi2.num_data_lanes); > @@ -1209,6 +1223,9 @@ static int imx335_init_controls(struct imx335 *imx335) > struct v4l2_ctrl_handler *ctrl_hdlr = &imx335->ctrl_handler; > const struct imx335_mode *mode = imx335->cur_mode; > u32 lpfr; > + u64 pclk; > + s64 link_freq_in_use; > + u8 bpp; > int ret; > > ret = v4l2_ctrl_handler_init(ctrl_hdlr, 7); > @@ -1252,11 +1269,24 @@ static int imx335_init_controls(struct imx335 *imx335) > 0, 0, imx335_tpg_menu); > > /* Read only controls */ > + > + /* pixel rate = link frequency * lanes * 2 / bits_per_pixel */ > + switch (imx335->cur_mbus_code) { > + case MEDIA_BUS_FMT_SRGGB10_1X10: > + bpp = 10; > + break; > + case MEDIA_BUS_FMT_SRGGB12_1X12: > + bpp = 12; > + break; > + } > + > + link_freq_in_use = link_freq[__ffs(imx335->link_freq_bitmap)]; > + pclk = link_freq_in_use * (imx335->lane_mode + 1) * 2 / bpp; Please use div_s64() for this. > imx335->pclk_ctrl = v4l2_ctrl_new_std(ctrl_hdlr, > &imx335_ctrl_ops, > V4L2_CID_PIXEL_RATE, > - mode->pclk, mode->pclk, > - 1, mode->pclk); > + pclk, pclk, > + 1, pclk); > > imx335->link_freq_ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, > &imx335_ctrl_ops,
Hi Dave On 06/03/24 10:12 pm, Dave Stevenson wrote: > Hi Umang and Kieran > > On Wed, 6 Mar 2024 at 08:11, Umang Jain <umang.jain@ideasonboard.com> wrote: >> From: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >> The IMX335 can support both 2 and 4 lane configurations. >> Extend the driver to configure the lane mode accordingly. >> Update the pixel rate depending on the number of lanes in use. >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> drivers/media/i2c/imx335.c | 46 +++++++++++++++++++++++++++++++------- >> 1 file changed, 38 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c >> index dab6d080bc4c..a42f48823515 100644 >> --- a/drivers/media/i2c/imx335.c >> +++ b/drivers/media/i2c/imx335.c >> @@ -21,6 +21,11 @@ >> #define IMX335_MODE_STANDBY 0x01 >> #define IMX335_MODE_STREAMING 0x00 >> >> +/* Data Lanes */ >> +#define IMX335_LANEMODE 0x3a01 >> +#define IMX335_2LANE 1 >> +#define IMX335_4LANE 3 >> + >> /* Lines per frame */ >> #define IMX335_REG_LPFR 0x3030 >> >> @@ -67,8 +72,6 @@ >> #define IMX335_LINK_FREQ_594MHz 594000000LL >> #define IMX335_LINK_FREQ_445MHz 445500000LL >> >> -#define IMX335_NUM_DATA_LANES 4 >> - >> #define IMX335_REG_MIN 0x00 >> #define IMX335_REG_MAX 0xfffff >> >> @@ -115,7 +118,6 @@ static const char * const imx335_supply_name[] = { >> * @vblank: Vertical blanking in lines >> * @vblank_min: Minimum vertical blanking in lines >> * @vblank_max: Maximum vertical blanking in lines >> - * @pclk: Sensor pixel clock >> * @reg_list: Register list for sensor mode >> */ >> struct imx335_mode { >> @@ -126,7 +128,6 @@ struct imx335_mode { >> u32 vblank; >> u32 vblank_min; >> u32 vblank_max; >> - u64 pclk; >> struct imx335_reg_list reg_list; >> }; >> >> @@ -147,6 +148,7 @@ struct imx335_mode { >> * @exp_ctrl: Pointer to exposure control >> * @again_ctrl: Pointer to analog gain control >> * @vblank: Vertical blanking in lines >> + * @lane_mode Mode for number of connected data lanes >> * @cur_mode: Pointer to current selected sensor mode >> * @mutex: Mutex for serializing sensor controls >> * @link_freq_bitmap: Menu bitmap for link_freq_ctrl >> @@ -171,6 +173,7 @@ struct imx335 { >> struct v4l2_ctrl *again_ctrl; >> }; >> u32 vblank; >> + u32 lane_mode; >> const struct imx335_mode *cur_mode; >> struct mutex mutex; >> unsigned long link_freq_bitmap; >> @@ -377,7 +380,6 @@ static const struct imx335_mode supported_mode = { >> .vblank = 2560, >> .vblank_min = 2560, >> .vblank_max = 133060, >> - .pclk = 396000000, >> .reg_list = { >> .num_of_regs = ARRAY_SIZE(mode_2592x1940_regs), >> .regs = mode_2592x1940_regs, >> @@ -936,6 +938,11 @@ static int imx335_start_streaming(struct imx335 *imx335) >> return ret; >> } >> >> + /* Configure lanes */ >> + ret = imx335_write_reg(imx335, IMX335_LANEMODE, 1, imx335->lane_mode); >> + if (ret) >> + return ret; >> + >> /* Setup handler will write actual exposure and gain */ >> ret = __v4l2_ctrl_handler_setup(imx335->sd.ctrl_handler); >> if (ret) { >> @@ -1096,7 +1103,14 @@ static int imx335_parse_hw_config(struct imx335 *imx335) >> if (ret) >> return ret; >> >> - if (bus_cfg.bus.mipi_csi2.num_data_lanes != IMX335_NUM_DATA_LANES) { >> + switch (bus_cfg.bus.mipi_csi2.num_data_lanes) { >> + case 2: >> + imx335->lane_mode = IMX335_2LANE; >> + break; >> + case 4: >> + imx335->lane_mode = IMX335_4LANE; >> + break; >> + default: >> dev_err(imx335->dev, >> "number of CSI2 data lanes %d is not supported\n", >> bus_cfg.bus.mipi_csi2.num_data_lanes); >> @@ -1209,6 +1223,9 @@ static int imx335_init_controls(struct imx335 *imx335) >> struct v4l2_ctrl_handler *ctrl_hdlr = &imx335->ctrl_handler; >> const struct imx335_mode *mode = imx335->cur_mode; >> u32 lpfr; >> + u64 pclk; >> + s64 link_freq_in_use; >> + u8 bpp; >> int ret; >> >> ret = v4l2_ctrl_handler_init(ctrl_hdlr, 7); >> @@ -1252,11 +1269,24 @@ static int imx335_init_controls(struct imx335 *imx335) >> 0, 0, imx335_tpg_menu); >> >> /* Read only controls */ >> + >> + /* pixel rate = link frequency * lanes * 2 / bits_per_pixel */ >> + switch (imx335->cur_mbus_code) { >> + case MEDIA_BUS_FMT_SRGGB10_1X10: >> + bpp = 10; >> + break; >> + case MEDIA_BUS_FMT_SRGGB12_1X12: >> + bpp = 12; >> + break; >> + } >> + >> + link_freq_in_use = link_freq[__ffs(imx335->link_freq_bitmap)]; >> + pclk = link_freq_in_use * (imx335->lane_mode + 1) * 2 / bpp; >> imx335->pclk_ctrl = v4l2_ctrl_new_std(ctrl_hdlr, >> &imx335_ctrl_ops, >> V4L2_CID_PIXEL_RATE, >> - mode->pclk, mode->pclk, >> - 1, mode->pclk); >> + pclk, pclk, >> + 1, pclk); > Is this actually correct? > A fair number of the sensors I've encountered have 2 PLL paths - one > for the pixel array, and one for the CSI block. The bpp will generally > be fed into the CSI block PLL path, but not into the pixel array one. > The link frequency will therefore vary with bit depth, but > V4L2_CID_PIXEL_RATE doesn't change. > > imx290 certainly has a disjoin between pixel rate and link freq > (cropping reduces link freq, but not pixel rate), and we run imx477 in > 2 lane mode with the pixel array at full tilt (840MPix/s) but large > horizontal blanking to allow CSI2 enough time to send the data. > > If you've validated that for a range of frame rates you get the > correct output from the sensor in both 10 and 12 bit modes, then I I've not validated in such cases. Computing from pixel rate from the link_freq and lane mode came out to be the same as the value currently hardcoded in the mode structure hence I introduced this change. However, I am inclined to dropping it after reading your review ;-) > don't object. I just have an instinctive tick whenever I see drivers > computing PIXEL_RATE from LINK_FREQ or vice versa :) Having said that, I do know, the reporting is not perfect since the bpp can be changed and it would change the link-frequency. > If you get the right frame rate it may also imply that the link > frequency isn't as configured, but that rarely has any negative Indeed ;-) > effects. You need a reasonably good oscilloscope to be able to measure > the link frequency. AH, I see. > > Dave > >> imx335->link_freq_ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, >> &imx335_ctrl_ops, >> -- >> 2.43.0 >> >>
Hi Umang,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linuxtv-media-stage/master]
[cannot apply to media-tree/master sailus-media-tree/streams linus/master v6.8-rc7 next-20240308]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Umang-Jain/media-imx335-Support-2-or-4-lane-operation-modes/20240306-161903
base: https://git.linuxtv.org/media_stage.git master
patch link: https://lore.kernel.org/r/20240306081038.212412-2-umang.jain%40ideasonboard.com
patch subject: [PATCH 1/5] media: imx335: Support 2 or 4 lane operation modes
config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20240309/202403090232.FqnIE4cy-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240309/202403090232.FqnIE4cy-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403090232.FqnIE4cy-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/media/i2c/imx335.c:181: warning: Function parameter or struct member 'lane_mode' not described in 'imx335'
vim +181 drivers/media/i2c/imx335.c
45d19b5fb9aeab Martina Krasteva 2021-05-27 133
45d19b5fb9aeab Martina Krasteva 2021-05-27 134 /**
45d19b5fb9aeab Martina Krasteva 2021-05-27 135 * struct imx335 - imx335 sensor device structure
45d19b5fb9aeab Martina Krasteva 2021-05-27 136 * @dev: Pointer to generic device
45d19b5fb9aeab Martina Krasteva 2021-05-27 137 * @client: Pointer to i2c client
45d19b5fb9aeab Martina Krasteva 2021-05-27 138 * @sd: V4L2 sub-device
45d19b5fb9aeab Martina Krasteva 2021-05-27 139 * @pad: Media pad. Only one pad supported
45d19b5fb9aeab Martina Krasteva 2021-05-27 140 * @reset_gpio: Sensor reset gpio
fea91ee73b7cd1 Kieran Bingham 2023-12-11 141 * @supplies: Regulator supplies to handle power control
45d19b5fb9aeab Martina Krasteva 2021-05-27 142 * @inclk: Sensor input clock
45d19b5fb9aeab Martina Krasteva 2021-05-27 143 * @ctrl_handler: V4L2 control handler
45d19b5fb9aeab Martina Krasteva 2021-05-27 144 * @link_freq_ctrl: Pointer to link frequency control
45d19b5fb9aeab Martina Krasteva 2021-05-27 145 * @pclk_ctrl: Pointer to pixel clock control
45d19b5fb9aeab Martina Krasteva 2021-05-27 146 * @hblank_ctrl: Pointer to horizontal blanking control
45d19b5fb9aeab Martina Krasteva 2021-05-27 147 * @vblank_ctrl: Pointer to vertical blanking control
45d19b5fb9aeab Martina Krasteva 2021-05-27 148 * @exp_ctrl: Pointer to exposure control
45d19b5fb9aeab Martina Krasteva 2021-05-27 149 * @again_ctrl: Pointer to analog gain control
45d19b5fb9aeab Martina Krasteva 2021-05-27 150 * @vblank: Vertical blanking in lines
8c48b2175e7d73 Kieran Bingham 2024-03-06 151 * @lane_mode Mode for number of connected data lanes
45d19b5fb9aeab Martina Krasteva 2021-05-27 152 * @cur_mode: Pointer to current selected sensor mode
45d19b5fb9aeab Martina Krasteva 2021-05-27 153 * @mutex: Mutex for serializing sensor controls
0862582b5239a6 Umang Jain 2024-02-20 154 * @link_freq_bitmap: Menu bitmap for link_freq_ctrl
cfa49ff0558a32 Umang Jain 2023-12-11 155 * @cur_mbus_code: Currently selected media bus format code
45d19b5fb9aeab Martina Krasteva 2021-05-27 156 */
45d19b5fb9aeab Martina Krasteva 2021-05-27 157 struct imx335 {
45d19b5fb9aeab Martina Krasteva 2021-05-27 158 struct device *dev;
45d19b5fb9aeab Martina Krasteva 2021-05-27 159 struct i2c_client *client;
45d19b5fb9aeab Martina Krasteva 2021-05-27 160 struct v4l2_subdev sd;
45d19b5fb9aeab Martina Krasteva 2021-05-27 161 struct media_pad pad;
45d19b5fb9aeab Martina Krasteva 2021-05-27 162 struct gpio_desc *reset_gpio;
fea91ee73b7cd1 Kieran Bingham 2023-12-11 163 struct regulator_bulk_data supplies[ARRAY_SIZE(imx335_supply_name)];
fea91ee73b7cd1 Kieran Bingham 2023-12-11 164
45d19b5fb9aeab Martina Krasteva 2021-05-27 165 struct clk *inclk;
45d19b5fb9aeab Martina Krasteva 2021-05-27 166 struct v4l2_ctrl_handler ctrl_handler;
45d19b5fb9aeab Martina Krasteva 2021-05-27 167 struct v4l2_ctrl *link_freq_ctrl;
45d19b5fb9aeab Martina Krasteva 2021-05-27 168 struct v4l2_ctrl *pclk_ctrl;
45d19b5fb9aeab Martina Krasteva 2021-05-27 169 struct v4l2_ctrl *hblank_ctrl;
45d19b5fb9aeab Martina Krasteva 2021-05-27 170 struct v4l2_ctrl *vblank_ctrl;
45d19b5fb9aeab Martina Krasteva 2021-05-27 171 struct {
45d19b5fb9aeab Martina Krasteva 2021-05-27 172 struct v4l2_ctrl *exp_ctrl;
45d19b5fb9aeab Martina Krasteva 2021-05-27 173 struct v4l2_ctrl *again_ctrl;
45d19b5fb9aeab Martina Krasteva 2021-05-27 174 };
45d19b5fb9aeab Martina Krasteva 2021-05-27 175 u32 vblank;
8c48b2175e7d73 Kieran Bingham 2024-03-06 176 u32 lane_mode;
45d19b5fb9aeab Martina Krasteva 2021-05-27 177 const struct imx335_mode *cur_mode;
45d19b5fb9aeab Martina Krasteva 2021-05-27 178 struct mutex mutex;
0862582b5239a6 Umang Jain 2024-02-20 179 unsigned long link_freq_bitmap;
cfa49ff0558a32 Umang Jain 2023-12-11 180 u32 cur_mbus_code;
45d19b5fb9aeab Martina Krasteva 2021-05-27 @181 };
45d19b5fb9aeab Martina Krasteva 2021-05-27 182
Hi Umang,
kernel test robot noticed the following build errors:
[auto build test ERROR on linuxtv-media-stage/master]
[cannot apply to media-tree/master sailus-media-tree/streams linus/master v6.8-rc7 next-20240308]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Umang-Jain/media-imx335-Support-2-or-4-lane-operation-modes/20240306-161903
base: https://git.linuxtv.org/media_stage.git master
patch link: https://lore.kernel.org/r/20240306081038.212412-2-umang.jain%40ideasonboard.com
patch subject: [PATCH 1/5] media: imx335: Support 2 or 4 lane operation modes
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20240309/202403090439.t4lCQSzv-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240309/202403090439.t4lCQSzv-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403090439.t4lCQSzv-lkp@intel.com/
All errors (new ones prefixed by >>):
m68k-linux-ld: drivers/media/i2c/imx335.o: in function `imx335_init_controls.constprop.0':
>> imx335.c:(.text+0x33c): undefined reference to `__divdi3'
Hi Umang,
kernel test robot noticed the following build errors:
[auto build test ERROR on linuxtv-media-stage/master]
[cannot apply to media-tree/master sailus-media-tree/streams linus/master v6.8-rc7 next-20240308]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Umang-Jain/media-imx335-Support-2-or-4-lane-operation-modes/20240306-161903
base: https://git.linuxtv.org/media_stage.git master
patch link: https://lore.kernel.org/r/20240306081038.212412-2-umang.jain%40ideasonboard.com
patch subject: [PATCH 1/5] media: imx335: Support 2 or 4 lane operation modes
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240309/202403090429.oocT1Laz-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240309/202403090429.oocT1Laz-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403090429.oocT1Laz-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/parsers/tplink_safeloader.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/chips/cfi_util.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/chips/cfi_cmdset_0020.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/maps/map_funcs.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/spmi/hisi-spmi-controller.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/spmi/spmi-pmic-arb.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio_pruss.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/pcmcia/pcmcia_rsrc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/pcmcia/i82365.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hwmon/corsair-cpro.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hwmon/mr75203.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/vhost/vringh.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/greybus/greybus.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/greybus/gb-es2.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/rpmsg/rpmsg_char.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/iio/adc/ingenic-adc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/iio/adc/xilinx-ams.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/iio/buffer/kfifo_buf.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-core.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-hub.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-aspeed.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-gpio.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-ast-cf.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-scom.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/siox/siox-bus-gpio.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/counter/ftm-quaddec.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/oss/dmasound/dmasound_core.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/oss/dmasound/dmasound_atari.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/oss/dmasound/dmasound_paula.o
WARNING: modpost: sound/oss/dmasound/dmasound_paula: section mismatch in reference: amiga_audio_driver+0x8 (section: .data) -> amiga_audio_remove (section: .exit.text)
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/core/snd-pcm-dmaengine.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/drivers/snd-pcmtest.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/pci/hda/snd-hda-cirrus-scodec-test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/soc-topology-test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/codecs/snd-soc-ab8500-codec.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/codecs/snd-soc-sigmadsp.o
WARNING: modpost: sound/soc/codecs/snd-soc-tlv320adc3xxx: section mismatch in reference: adc3xxx_i2c_driver+0x8 (section: .data) -> adc3xxx_i2c_remove (section: .exit.text)
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/codecs/snd-soc-wm-adsp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/fsl/imx-pcm-dma.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/mxs/snd-soc-mxs-pcm.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/qcom/snd-soc-qcom-common.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/qcom/snd-soc-qcom-sdw.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/qcom/qdsp6/snd-q6dsp-common.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-intel-atom.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-acpi-intel-byt.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-acpi-intel-bdw.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/imx/snd-sof-imx8.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/imx/snd-sof-imx8m.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/imx/snd-sof-imx8ulp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/imx/imx-common.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/mediatek/mtk-adsp-common.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/mediatek/mt8195/snd-sof-mt8195.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/mediatek/mt8186/snd-sof-mt8186.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/snd-sof-utils.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/snd-sof-acpi.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/snd-sof-of.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/xilinx/snd-soc-xlnx-i2s.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/xilinx/snd-soc-xlnx-formatter-pcm.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/ac97_bus.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mtty.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mdpy.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mdpy-fb.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mbochs.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/configfs/configfs_sample.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kfifo/bytestream-example.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kfifo/dma-example.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kfifo/inttype-example.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kfifo/record-example.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kobject/kobject-example.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kobject/kset-example.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/sched/em_cmp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/sched/em_nbyte.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/sched/em_u32.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/sched/em_meta.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/sched/em_text.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/sched/em_canid.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/ip_tunnel.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/ipip.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/ip_gre.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/udp_tunnel.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/ip_vti.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/ah4.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/esp4.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/xfrm4_tunnel.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/tunnel4.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/xfrm/xfrm_algo.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/xfrm/xfrm_user.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/ah6.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/esp6.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/xfrm6_tunnel.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/tunnel6.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/mip6.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/sit.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/ip6_udp_tunnel.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/key/af_key.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/atm/mpoa.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/6lowpan/6lowpan.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ieee802154/6lowpan/ieee802154_6lowpan.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ieee802154/ieee802154_socket.o
>> ERROR: modpost: "__divdi3" [drivers/media/i2c/imx335.ko] undefined!
diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c index dab6d080bc4c..a42f48823515 100644 --- a/drivers/media/i2c/imx335.c +++ b/drivers/media/i2c/imx335.c @@ -21,6 +21,11 @@ #define IMX335_MODE_STANDBY 0x01 #define IMX335_MODE_STREAMING 0x00 +/* Data Lanes */ +#define IMX335_LANEMODE 0x3a01 +#define IMX335_2LANE 1 +#define IMX335_4LANE 3 + /* Lines per frame */ #define IMX335_REG_LPFR 0x3030 @@ -67,8 +72,6 @@ #define IMX335_LINK_FREQ_594MHz 594000000LL #define IMX335_LINK_FREQ_445MHz 445500000LL -#define IMX335_NUM_DATA_LANES 4 - #define IMX335_REG_MIN 0x00 #define IMX335_REG_MAX 0xfffff @@ -115,7 +118,6 @@ static const char * const imx335_supply_name[] = { * @vblank: Vertical blanking in lines * @vblank_min: Minimum vertical blanking in lines * @vblank_max: Maximum vertical blanking in lines - * @pclk: Sensor pixel clock * @reg_list: Register list for sensor mode */ struct imx335_mode { @@ -126,7 +128,6 @@ struct imx335_mode { u32 vblank; u32 vblank_min; u32 vblank_max; - u64 pclk; struct imx335_reg_list reg_list; }; @@ -147,6 +148,7 @@ struct imx335_mode { * @exp_ctrl: Pointer to exposure control * @again_ctrl: Pointer to analog gain control * @vblank: Vertical blanking in lines + * @lane_mode Mode for number of connected data lanes * @cur_mode: Pointer to current selected sensor mode * @mutex: Mutex for serializing sensor controls * @link_freq_bitmap: Menu bitmap for link_freq_ctrl @@ -171,6 +173,7 @@ struct imx335 { struct v4l2_ctrl *again_ctrl; }; u32 vblank; + u32 lane_mode; const struct imx335_mode *cur_mode; struct mutex mutex; unsigned long link_freq_bitmap; @@ -377,7 +380,6 @@ static const struct imx335_mode supported_mode = { .vblank = 2560, .vblank_min = 2560, .vblank_max = 133060, - .pclk = 396000000, .reg_list = { .num_of_regs = ARRAY_SIZE(mode_2592x1940_regs), .regs = mode_2592x1940_regs, @@ -936,6 +938,11 @@ static int imx335_start_streaming(struct imx335 *imx335) return ret; } + /* Configure lanes */ + ret = imx335_write_reg(imx335, IMX335_LANEMODE, 1, imx335->lane_mode); + if (ret) + return ret; + /* Setup handler will write actual exposure and gain */ ret = __v4l2_ctrl_handler_setup(imx335->sd.ctrl_handler); if (ret) { @@ -1096,7 +1103,14 @@ static int imx335_parse_hw_config(struct imx335 *imx335) if (ret) return ret; - if (bus_cfg.bus.mipi_csi2.num_data_lanes != IMX335_NUM_DATA_LANES) { + switch (bus_cfg.bus.mipi_csi2.num_data_lanes) { + case 2: + imx335->lane_mode = IMX335_2LANE; + break; + case 4: + imx335->lane_mode = IMX335_4LANE; + break; + default: dev_err(imx335->dev, "number of CSI2 data lanes %d is not supported\n", bus_cfg.bus.mipi_csi2.num_data_lanes); @@ -1209,6 +1223,9 @@ static int imx335_init_controls(struct imx335 *imx335) struct v4l2_ctrl_handler *ctrl_hdlr = &imx335->ctrl_handler; const struct imx335_mode *mode = imx335->cur_mode; u32 lpfr; + u64 pclk; + s64 link_freq_in_use; + u8 bpp; int ret; ret = v4l2_ctrl_handler_init(ctrl_hdlr, 7); @@ -1252,11 +1269,24 @@ static int imx335_init_controls(struct imx335 *imx335) 0, 0, imx335_tpg_menu); /* Read only controls */ + + /* pixel rate = link frequency * lanes * 2 / bits_per_pixel */ + switch (imx335->cur_mbus_code) { + case MEDIA_BUS_FMT_SRGGB10_1X10: + bpp = 10; + break; + case MEDIA_BUS_FMT_SRGGB12_1X12: + bpp = 12; + break; + } + + link_freq_in_use = link_freq[__ffs(imx335->link_freq_bitmap)]; + pclk = link_freq_in_use * (imx335->lane_mode + 1) * 2 / bpp; imx335->pclk_ctrl = v4l2_ctrl_new_std(ctrl_hdlr, &imx335_ctrl_ops, V4L2_CID_PIXEL_RATE, - mode->pclk, mode->pclk, - 1, mode->pclk); + pclk, pclk, + 1, pclk); imx335->link_freq_ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, &imx335_ctrl_ops,