| 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: <20240729190149.GE1552@pendragon.ideasonboard.com>
Date: Mon, 29 Jul 2024 22:01:49 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Umang Jain <umang.jain@...asonboard.com>
Cc: linux-media@...r.kernel.org,
Kieran Bingham <kieran.bingham@...asonboard.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: imx283: Provide optical black mode
Hi Umang,
Thank you for the patch.
On Tue, Jul 30, 2024 at 12:09:53AM +0530, Umang Jain wrote:
> The IMX283 is capable of delivering optical black regions as part of
> the image capture. These regions support capture of the black levels
> for calibration and are added as extra pixels on top of the full
> resolution capture.
>
> Supply an extra mode which accounts for this increased size that will
> produce black regions in the output image.
Sorry, but this shouldn't be an extra mode. We need a proper API, you
need to convert the driver to make it freely configurable. Furthermore,
the OB stream should probably be reported by .get_frame_desc().
> Signed-off-by: Umang Jain <umang.jain@...asonboard.com>
> ---
> - YUYV capture sample with Pi5 ISP for side-by-side comparison of OB regions:
> https://gcdnb.pbrd.co/images/XQV29MedwXxg.png
> ---
> drivers/media/i2c/imx283.c | 68 ++++++++++++++++++++++++++++++++------
> 1 file changed, 58 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c
> index 8490618c5071..9a0fe2c34a41 100644
> --- a/drivers/media/i2c/imx283.c
> +++ b/drivers/media/i2c/imx283.c
> @@ -66,6 +66,7 @@
> #define IMX283_REG_HTRIMMING CCI_REG8(0x300b)
> #define IMX283_MDVREV BIT(0) /* VFLIP */
> #define IMX283_HTRIMMING_EN BIT(4)
> +#define IMX283_HOB_EN BIT(5)
>
> #define IMX283_REG_VWINPOS CCI_REG16_LE(0x300f)
> #define IMX283_REG_VWIDCUT CCI_REG16_LE(0x3011)
> @@ -306,6 +307,7 @@ static const struct imx283_input_frequency imx283_frequencies[] = {
>
> enum imx283_modes {
> IMX283_MODE_0,
> + IMX283_MODE_0_OB,
> IMX283_MODE_1,
> IMX283_MODE_1A,
> IMX283_MODE_1S,
> @@ -327,6 +329,7 @@ struct imx283_readout_mode {
> static const struct imx283_readout_mode imx283_readout_modes[] = {
> /* All pixel scan modes */
> [IMX283_MODE_0] = { 0x04, 0x03, 0x10, 0x00 }, /* 12 bit */
> + [IMX283_MODE_0_OB] = { 0x04, 0x03, 0x10, 0x00 }, /* 12 bit */
> [IMX283_MODE_1] = { 0x04, 0x01, 0x00, 0x00 }, /* 10 bit */
> [IMX283_MODE_1A] = { 0x04, 0x01, 0x20, 0x50 }, /* 10 bit */
> [IMX283_MODE_1S] = { 0x04, 0x41, 0x20, 0x50 }, /* 10 bit */
> @@ -439,6 +442,36 @@ static const struct imx283_mode supported_modes_12bit[] = {
> .height = 3648,
> },
> },
> + {
> + /* 20MPix 21.40 fps readout mode 0 with optical blacks enabled */
> + .mode = IMX283_MODE_0_OB,
> + .bpp = 12,
> + .width = 5472 + 96, /* width + Horizontal optical black */
> + .height = 3648 + 16, /* height + Vertical optical black */
> + .min_hmax = 5914, /* 887 @ 480MHz/72MHz */
> + .min_vmax = 3793, /* Lines */
> +
> + .veff = 3694,
> + .vst = 0,
> + .vct = 0,
> +
> + .hbin_ratio = 1,
> + .vbin_ratio = 1,
> +
> + /* 20.00 FPS */
> + .default_hmax = 6000, /* 900 @ 480MHz/72MHz */
> + .default_vmax = 4000,
> +
> + .min_shr = 11,
> + .horizontal_ob = 96,
> + .vertical_ob = 16,
> + .crop = {
> + .top = 40,
> + .left = 108,
> + .width = 5472 + 96,
> + .height = 3648 + 16,
> + },
> + },
> {
> /*
> * Readout mode 2 : 2/2 binned mode (2736x1824)
> @@ -793,17 +826,20 @@ static int imx283_set_ctrl(struct v4l2_ctrl *ctrl)
> break;
>
> case V4L2_CID_VFLIP:
> + u32 htrim = IMX283_HTRIMMING_EN;
> +
> /*
> * VFLIP is managed by BIT(0) of IMX283_REG_HTRIMMING address, hence
> * both need to be set simultaneously.
> */
> - if (ctrl->val) {
> - cci_write(imx283->cci, IMX283_REG_HTRIMMING,
> - IMX283_HTRIMMING_EN | IMX283_MDVREV, &ret);
> - } else {
> - cci_write(imx283->cci, IMX283_REG_HTRIMMING,
> - IMX283_HTRIMMING_EN, &ret);
> - }
> + if (ctrl->val)
> + htrim = IMX283_HTRIMMING_EN | IMX283_MDVREV;
> +
> + if (mode->mode == IMX283_MODE_0_OB)
> + htrim |= IMX283_HOB_EN;
> +
> + cci_write(imx283->cci, IMX283_REG_HTRIMMING, htrim, &ret);
> +
> break;
>
> case V4L2_CID_TEST_PATTERN:
> @@ -1010,6 +1046,8 @@ static int imx283_start_streaming(struct imx283 *imx283,
> s32 v_pos;
> u32 write_v_size;
> u32 y_out_size;
> + u32 htrim_end;
> + u32 ob_size_v = 0;
> int ret = 0;
>
> fmt = v4l2_subdev_state_get_format(state, 0);
> @@ -1057,6 +1095,12 @@ static int imx283_start_streaming(struct imx283 *imx283,
> mode->crop.height);
>
> y_out_size = mode->crop.height / mode->vbin_ratio;
> +
> + if (mode->mode == IMX283_MODE_0_OB) {
> + y_out_size -= mode->vertical_ob;
> + ob_size_v = mode->vertical_ob;
> + }
> +
> write_v_size = y_out_size + mode->vertical_ob;
> /*
> * cropping start position = (VWINPOS – Vst) × 2
> @@ -1072,12 +1116,16 @@ static int imx283_start_streaming(struct imx283 *imx283,
> cci_write(imx283->cci, IMX283_REG_VWIDCUT, v_widcut, &ret);
> cci_write(imx283->cci, IMX283_REG_VWINPOS, v_pos, &ret);
>
> - cci_write(imx283->cci, IMX283_REG_OB_SIZE_V, mode->vertical_ob, &ret);
> + cci_write(imx283->cci, IMX283_REG_OB_SIZE_V, ob_size_v, &ret);
> +
> + htrim_end = mode->crop.left + mode->crop.width;
> +
> + if (mode->mode == IMX283_MODE_0_OB)
> + htrim_end -= mode->horizontal_ob * mode->hbin_ratio;
>
> /* TODO: Validate mode->crop is fully contained within imx283_native_area */
> cci_write(imx283->cci, IMX283_REG_HTRIMMING_START, mode->crop.left, &ret);
> - cci_write(imx283->cci, IMX283_REG_HTRIMMING_END,
> - mode->crop.left + mode->crop.width, &ret);
> + cci_write(imx283->cci, IMX283_REG_HTRIMMING_END, htrim_end, &ret);
>
> /* Disable embedded data */
> cci_write(imx283->cci, IMX283_REG_EBD_X_OUT_SIZE, 0, &ret);
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists