[PATCH 1/2] media: i2c: imx258: add HDR control

[PATCH 1/2] media: i2c: imx258: add HDR control

[Krzysztof Kozlowski]

The IMX258 supports High Dynamic Range with 5 levels of low/high
exposure ratio.  Add a V4L2 control for HDR (WIDE_DYNAMIC_RANGE).

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/media/i2c/imx258.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index 7bedbfe5c4d6..aedf8e7c6165 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -61,6 +61,15 @@
 #define IMX258_DGTL_GAIN_DEFAULT	1024
 #define IMX258_DGTL_GAIN_STEP		1
 
+/* HDR control */
+#define IMX258_REG_HDR			0x0220
+#define IMX258_HDR_ON			BIT(0)
+#define IMX258_REG_HDR_RATIO		0x0222
+#define IMX258_HDR_RATIO_MIN		0
+#define IMX258_HDR_RATIO_MAX		5
+#define IMX258_HDR_RATIO_STEP		1
+#define IMX258_HDR_RATIO_DEFAULT	0x0
+
 /* Test Pattern Control */
 #define IMX258_REG_TEST_PATTERN		0x0600
 
@@ -777,6 +786,22 @@ static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
 				!ctrl->val ? REG_CONFIG_MIRROR_FLIP :
 				REG_CONFIG_FLIP_TEST_PATTERN);
 		break;
+	case V4L2_CID_WIDE_DYNAMIC_RANGE:
+		if (!ctrl->val) {
+			ret = imx258_write_reg(imx258, IMX258_REG_HDR,
+					       IMX258_REG_VALUE_08BIT,
+					       IMX258_HDR_RATIO_MIN);
+		} else {
+			ret = imx258_write_reg(imx258, IMX258_REG_HDR,
+					       IMX258_REG_VALUE_08BIT,
+					       IMX258_HDR_ON);
+			if (ret)
+				break;
+			ret = imx258_write_reg(imx258, IMX258_REG_HDR_RATIO,
+					       IMX258_REG_VALUE_08BIT,
+					       BIT(IMX258_HDR_RATIO_MAX));
+		}
+		break;
 	default:
 		dev_info(&client->dev,
 			 "ctrl(id:0x%x,val:0x%x) is not handled\n",
@@ -1193,6 +1218,9 @@ static int imx258_init_controls(struct imx258 *imx258)
 				IMX258_DGTL_GAIN_STEP,
 				IMX258_DGTL_GAIN_DEFAULT);
 
+	v4l2_ctrl_new_std(ctrl_hdlr, &imx258_ctrl_ops, V4L2_CID_WIDE_DYNAMIC_RANGE,
+				0, 1, 1, IMX258_HDR_RATIO_DEFAULT);
+
 	v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &imx258_ctrl_ops,
 				V4L2_CID_TEST_PATTERN,
 				ARRAY_SIZE(imx258_test_pattern_menu) - 1,
-- 
2.17.1

[PATCH 2/2] media: i2c: imx258: validate rotation only if it is provided

[Krzysztof Kozlowski]

The sensor supports rotation by 180 degrees however the value of
"rotation" property should be validated only if it exists.  If
"rotation" is missing, do not fail the probe:

    imx258: probe of 3-001a failed with error -22

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/media/i2c/imx258.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index aedf8e7c6165..c52932e5b881 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -1284,7 +1284,7 @@ static int imx258_probe(struct i2c_client *client)
 	 * supports a single pixel order right now.
 	 */
 	ret = device_property_read_u32(&client->dev, "rotation", &val);
-	if (ret || val != 180)
+	if (!ret && val != 180)
 		return -EINVAL;
 
 	/* Initialize subdev */
-- 
2.17.1

Re: [PATCH 1/2] media: i2c: imx258: add HDR control

[Krzysztof Kozlowski]

On Mon, Oct 05, 2020 at 05:15:58PM +0200, Krzysztof Kozlowski wrote:
> The IMX258 supports High Dynamic Range with 5 levels of low/high
> exposure ratio.  Add a V4L2 control for HDR (WIDE_DYNAMIC_RANGE).
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  drivers/media/i2c/imx258.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 

Any comments here?

Best regards,
Krzysztof

Re: [PATCH 2/2] media: i2c: imx258: validate rotation only if it is provided

[Krzysztof Kozlowski]

On Mon, Oct 05, 2020 at 05:15:59PM +0200, Krzysztof Kozlowski wrote:
> The sensor supports rotation by 180 degrees however the value of
> "rotation" property should be validated only if it exists.  If
> "rotation" is missing, do not fail the probe:
> 
>     imx258: probe of 3-001a failed with error -22
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

This is actually a fix, so these should be added:
Fixes: 17121d12a5c1 ("media: imx258: Check the rotation property has a value of 180")
Cc: <stable@vger.kernel.org>

Best regards,
Krzysztof

> ---
>  drivers/media/i2c/imx258.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index aedf8e7c6165..c52932e5b881 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -1284,7 +1284,7 @@ static int imx258_probe(struct i2c_client *client)
>  	 * supports a single pixel order right now.
>  	 */
>  	ret = device_property_read_u32(&client->dev, "rotation", &val);
> -	if (ret || val != 180)
> +	if (!ret && val != 180)
>  		return -EINVAL;
>  
>  	/* Initialize subdev */
> -- 
> 2.17.1
> 

Re: [PATCH 2/2] media: i2c: imx258: validate rotation only if it is provided

[Krzysztof Kozlowski]

On Wed, Oct 28, 2020 at 10:23:43AM +0100, Krzysztof Kozlowski wrote:
> On Mon, Oct 05, 2020 at 05:15:59PM +0200, Krzysztof Kozlowski wrote:
> > The sensor supports rotation by 180 degrees however the value of
> > "rotation" property should be validated only if it exists.  If
> > "rotation" is missing, do not fail the probe:
> > 
> >     imx258: probe of 3-001a failed with error -22
> > 
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> This is actually a fix, so these should be added:
> Fixes: 17121d12a5c1 ("media: imx258: Check the rotation property has a value of 180")
> Cc: <stable@vger.kernel.org>
> 
> Best regards,
> Krzysztof

Please drop this patch. I misunderstood the comment.

Best regards,
Krzysztof