| Message ID | 20240902-imx214-v1-0-c96cba989315@apitzsch.eu |
|---|---|
| Headers | show |
| Series | media: i2c: imx214: Miscellaneous cleanups and improvements | expand |
On Mon, Sep 2, 2024 at 11:53 PM André Apitzsch via B4 Relay <devnull+git.apitzsch.eu@kernel.org> wrote: > > From: André Apitzsch <git@apitzsch.eu> > > The imx214 sensor supports analogue gain up to 8x and digital gain up to > 16x. Implement the corresponding controls in the driver. Default gain > values are not modified by this patch. > Acked-by: Ricardo Ribalda <ribalda@chromium.org> > Signed-off-by: André Apitzsch <git@apitzsch.eu> > > --- > > With the analogue gain control added by this patch, the kernel log shows > the following message when closing megapixels and a similar one when > closing qcam (from libcamera): > > [ 100.042856] ------------[ cut here ]------------ > [ 100.042886] WARNING: CPU: 4 PID: 3444 at drivers/media/common/videobuf2/videobuf2-core.c:2182 __vb2_queue_cancel+0x228/0x2c0 [videobuf2_common] > [ 100.042948] Modules linked in: rfcomm zstd zstd_compress zram zsmalloc rpmsg_wwan_ctrl q6voice_dai q6asm_dai q6voice q6afe_dai q6routing q6cvs q6adm q6asm q6cvp q6mvm snd_q6dsp_common q6voice_common q6afe q6core apr pdr_interface nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables nfnetlink algif_hash algif_skcipher af_alg bnep qcom_pd_mapper qcom_pdr_msg wcn36xx btqcomsmd btqca ipv6 wcnss_ctrl qcom_bam_dmux imx214 v4l2_cci ledtrig_pattern bmi160_i2c ak8975 leds_ktd202x bmi160_core ltr501 leds_sy7802 qcom_wcnss_pil qcom_camss qcom_q6v5_mss snd_soc_msm8916_digital snd_soc_msm8916_analog pm8xxx_vibrator v4l2_fwnode qcom_pil_info videobuf2_dma_sg qcom_common qcom_q6v5 videobuf2_memops videobuf2_v4l2 videobuf2_common i2c_qcom_cci leds_sgm3140 v4l2_flash_led_class led_class_flash v4l2_async videodev qcom_memshare mc usb_f_ncm u_ether panel_longcheer_truly_nt35695 atmel_mxt_ts smb1360 msm mdt_loader drm_exec drm_display_he > lper gpu_sched libcomposite > [ 100.043688] CPU: 4 UID: 10000 PID: 3444 Comm: megapixels Not tainted 6.11.0-rc3-msm8916 #312 > [ 100.043716] Hardware name: BQ Aquaris M5 (Longcheer L9100) (DT) > [ 100.043730] pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 100.043756] pc : __vb2_queue_cancel+0x228/0x2c0 [videobuf2_common] > [ 100.043787] lr : __vb2_queue_cancel+0x28/0x2c0 [videobuf2_common] > [ 100.043815] sp : ffff80008450ba50 > [ 100.043826] x29: ffff80008450ba50 x28: 0000000000000001 x27: ffff00000f946180 > [ 100.043867] x26: 0000000000000000 x25: ffff00000f946780 x24: ffff00000f946910 > [ 100.043906] x23: ffff00000175a620 x22: ffff0000036ddc90 x21: ffff0000036e4410 > [ 100.043946] x20: ffff0000036e44b8 x19: ffff0000036e4410 x18: ffff80008450ba98 > [ 100.043985] x17: ffffffffffffffff x16: 0000000000000000 x15: 0000000000000040 > [ 100.044023] x14: 0256932925642338 x13: ffff00000f946200 x12: 0000000000000001 > [ 100.044062] x11: ffff00000f946200 x10: 0000000000000a20 x9 : 0000000000000000 > [ 100.044100] x8 : ffff0000bf964880 x7 : 0000000000000020 x6 : 0000000000000100 > [ 100.044138] x5 : ffff0000036e4b68 x4 : 0000000000000000 x3 : ffff00000f946180 > [ 100.044176] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 000000000000000f > [ 100.044214] Call trace: > [ 100.044226] __vb2_queue_cancel+0x228/0x2c0 [videobuf2_common] > [ 100.044257] vb2_core_queue_release+0x20/0x74 [videobuf2_common] > [ 100.044285] _vb2_fop_release+0x68/0xb0 [videobuf2_v4l2] > [ 100.044314] vb2_fop_release+0x28/0x50 [videobuf2_v4l2] > [ 100.044341] video_release+0x20/0x40 [qcom_camss] > [ 100.044406] v4l2_release+0xb4/0xf4 [videodev] > [ 100.044481] __fput+0xbc/0x274 > [ 100.044510] ____fput+0xc/0x14 > [ 100.044533] task_work_run+0x78/0xc0 > [ 100.044563] do_exit+0x2dc/0x8b0 > [ 100.044590] do_group_exit+0x30/0x8c > [ 100.044615] get_signal+0x7b4/0x8a0 > [ 100.044643] do_signal+0x94/0xd14 > [ 100.044672] do_notify_resume+0xd0/0x120 > [ 100.044697] el0_svc+0x44/0x60 > [ 100.044730] el0t_64_sync_handler+0x118/0x124 > [ 100.044753] el0t_64_sync+0x14c/0x150 > [ 100.044775] ---[ end trace 0000000000000000 ]--- > [ 100.044793] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 0 in active state > [ 100.045722] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 1 in active state > [ 100.046587] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 2 in active state > [ 100.047439] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 3 in active state > [ 100.048288] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 4 in active state > [ 100.049242] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 5 in active state > [ 100.050098] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 6 in active state > [ 100.050945] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 7 in active state > [ 100.051793] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 8 in active state > [ 100.052692] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 14 in active state > [ 100.053548] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 15 in active state > [ 100.054396] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 16 in active state > [ 100.055244] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 17 in active state > [ 100.056137] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 18 in active state > [ 100.056988] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 19 in active state > > From the log it looks like the cause is in some other module and not in > the imx214 driver, that's why the patch wasn't dropped from this series. > --- > drivers/media/i2c/imx214.c | 53 +++++++++++++++++++++++++++++++++------------- > 1 file changed, 38 insertions(+), 15 deletions(-) > > diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c > index 4a1433728cd5..ce6d8a90f4a1 100644 > --- a/drivers/media/i2c/imx214.c > +++ b/drivers/media/i2c/imx214.c > @@ -53,12 +53,20 @@ > /* Analog gain control */ > #define IMX214_REG_ANALOG_GAIN CCI_REG16(0x0204) > #define IMX214_REG_SHORT_ANALOG_GAIN CCI_REG16(0x0216) > +#define IMX214_ANA_GAIN_MIN 0 > +#define IMX214_ANA_GAIN_MAX 448 > +#define IMX214_ANA_GAIN_STEP 1 > +#define IMX214_ANA_GAIN_DEFAULT 0x0 > > /* Digital gain control */ > #define IMX214_REG_DIG_GAIN_GREENR CCI_REG16(0x020e) > #define IMX214_REG_DIG_GAIN_RED CCI_REG16(0x0210) > #define IMX214_REG_DIG_GAIN_BLUE CCI_REG16(0x0212) > #define IMX214_REG_DIG_GAIN_GREENB CCI_REG16(0x0214) > +#define IMX214_DGTL_GAIN_MIN 0x0100 > +#define IMX214_DGTL_GAIN_MAX 0x0fff > +#define IMX214_DGTL_GAIN_DEFAULT 0x0100 > +#define IMX214_DGTL_GAIN_STEP 1 > > #define IMX214_REG_ORIENTATION CCI_REG8(0x0101) > > @@ -271,13 +279,6 @@ static const struct cci_reg_sequence mode_4096x2304[] = { > > { IMX214_REG_SHORT_EXPOSURE, 500 }, > > - { IMX214_REG_ANALOG_GAIN, 0 }, > - { IMX214_REG_DIG_GAIN_GREENR, 256 }, > - { IMX214_REG_DIG_GAIN_RED, 256 }, > - { IMX214_REG_DIG_GAIN_BLUE, 256 }, > - { IMX214_REG_DIG_GAIN_GREENB, 256 }, > - { IMX214_REG_SHORT_ANALOG_GAIN, 0 }, > - > { CCI_REG8(0x4170), 0x00 }, > { CCI_REG8(0x4171), 0x10 }, > { CCI_REG8(0x4176), 0x00 }, > @@ -327,13 +328,6 @@ static const struct cci_reg_sequence mode_1920x1080[] = { > > { IMX214_REG_SHORT_EXPOSURE, 500 }, > > - { IMX214_REG_ANALOG_GAIN, 0 }, > - { IMX214_REG_DIG_GAIN_GREENR, 256 }, > - { IMX214_REG_DIG_GAIN_RED, 256 }, > - { IMX214_REG_DIG_GAIN_BLUE, 256 }, > - { IMX214_REG_DIG_GAIN_GREENB, 256 }, > - { IMX214_REG_SHORT_ANALOG_GAIN, 0 }, > - > { CCI_REG8(0x4170), 0x00 }, > { CCI_REG8(0x4171), 0x10 }, > { CCI_REG8(0x4176), 0x00 }, > @@ -757,6 +751,18 @@ static int imx214_entity_init_state(struct v4l2_subdev *subdev, > return 0; > } > > +static int imx214_update_digital_gain(struct imx214 *imx214, u32 val) > +{ > + int ret = 0; > + > + cci_write(imx214->regmap, IMX214_REG_DIG_GAIN_GREENR, val, &ret); > + cci_write(imx214->regmap, IMX214_REG_DIG_GAIN_RED, val, &ret); > + cci_write(imx214->regmap, IMX214_REG_DIG_GAIN_BLUE, val, &ret); > + cci_write(imx214->regmap, IMX214_REG_DIG_GAIN_GREENB, val, &ret); > + > + return ret; > +} > + > static int imx214_set_ctrl(struct v4l2_ctrl *ctrl) > { > struct imx214 *imx214 = container_of(ctrl->handler, > @@ -788,6 +794,15 @@ static int imx214_set_ctrl(struct v4l2_ctrl *ctrl) > return 0; > > switch (ctrl->id) { > + case V4L2_CID_ANALOGUE_GAIN: > + cci_write(imx214->regmap, IMX214_REG_ANALOG_GAIN, > + ctrl->val, &ret); > + cci_write(imx214->regmap, IMX214_REG_SHORT_ANALOG_GAIN, > + ctrl->val, &ret); > + break; > + case V4L2_CID_DIGITAL_GAIN: > + ret = imx214_update_digital_gain(imx214, ctrl->val); > + break; > case V4L2_CID_EXPOSURE: > cci_write(imx214->regmap, IMX214_REG_EXPOSURE, ctrl->val, &ret); > break; > @@ -834,7 +849,7 @@ static int imx214_ctrls_init(struct imx214 *imx214) > return ret; > > ctrl_hdlr = &imx214->ctrls; > - ret = v4l2_ctrl_handler_init(&imx214->ctrls, 10); > + ret = v4l2_ctrl_handler_init(&imx214->ctrls, 12); > if (ret) > return ret; > > @@ -871,6 +886,14 @@ static int imx214_ctrls_init(struct imx214 *imx214) > IMX214_EXPOSURE_STEP, > exposure_def); > > + v4l2_ctrl_new_std(ctrl_hdlr, &imx214_ctrl_ops, V4L2_CID_ANALOGUE_GAIN, > + IMX214_ANA_GAIN_MIN, IMX214_ANA_GAIN_MAX, > + IMX214_ANA_GAIN_STEP, IMX214_ANA_GAIN_DEFAULT); > + > + v4l2_ctrl_new_std(ctrl_hdlr, &imx214_ctrl_ops, V4L2_CID_DIGITAL_GAIN, > + IMX214_DGTL_GAIN_MIN, IMX214_DGTL_GAIN_MAX, > + IMX214_DGTL_GAIN_STEP, IMX214_DGTL_GAIN_DEFAULT); > + > imx214->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx214_ctrl_ops, > V4L2_CID_HFLIP, 0, 1, 1, 0); > if (imx214->hflip) > > -- > 2.46.0 > >
This patch series is a collection of miscellaneous cleanups and improvements to the imx214 driver. The series converts the driver to the CCI helpers and adds controls needed to make the driver work with libcamera. The changes are inspired by the imx219 driver. Signed-off-by: André Apitzsch <git@apitzsch.eu> --- André Apitzsch (13): media: i2c: imx214: Use subdev active state media: i2c: imx214: Remove unneeded goto media: i2c: imx214: Simplify with dev_err_probe() media: i2c: imx214: Convert to CCI register access helpers media: i2c: imx214: Replace register addresses with macros media: i2c: imx214: Drop IMX214_REG_EXPOSURE from mode reg arrays media: i2c: imx214: Use number of lanes from device tree media: i2c: imx214: Add vblank and hblank controls media: i2c: imx214: Extract format and crop settings media: i2c: imx214: Implement vflip/hflip controls media: i2c: imx214: Add analogue/digital gain control media: i2c: imx214: Verify chip ID media: i2c: imx214: Add test pattern control drivers/media/i2c/Kconfig | 1 + drivers/media/i2c/imx214.c | 1313 +++++++++++++++++++++++++++----------------- 2 files changed, 803 insertions(+), 511 deletions(-) --- base-commit: b891f84dd1f93ef4a716f10a70da80db443db431 change-id: 20240818-imx214-8324784c7bee Best regards,