| Message ID | 20240902-imx214-v1-11-c96cba989315@apitzsch.eu (mailing list archive) |
|---|---|
| State | New |
| 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 > >
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)