Re: [PATCH 11/13] media: i2c: imx214: Add analogue/digital gain control

From: Ricardo Ribalda Delgado
Date: Thu Sep 12 2024 - 09:41:10 EST

Next message: Ricardo Ribalda Delgado: "Re: [PATCH 08/13] media: i2c: imx214: Add vblank and hblank controls"
Previous message: Ricardo Ribalda Delgado: "Re: [PATCH 12/13] media: i2c: imx214: Verify chip ID"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, Sep 2, 2024 at 11:53 PM André Apitzsch via B4 Relay
<devnull+git.apitzsch.eu@xxxxxxxxxx> wrote:
>
> From: André Apitzsch <git@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>

> Signed-off-by: André Apitzsch <git@xxxxxxxxxxx>
>
> ---
>
> 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
>
>

Next message: Ricardo Ribalda Delgado: "Re: [PATCH 08/13] media: i2c: imx214: Add vblank and hblank controls"
Previous message: Ricardo Ribalda Delgado: "Re: [PATCH 12/13] media: i2c: imx214: Verify chip ID"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]