Re: [PATCH v9 8/8] media: imx214: Add new control with V4L2_CID_UNIT_CELL_SIZE

[Hans Verkuil]

On 10/1/19 1:24 PM, Ricardo Ribalda Delgado wrote:
> According to the product brief, the unit cell size is 1120 nanometers^2.
> 
> https://www.sony-semicon.co.jp/products_en/IS/sensor1/img/products/ProductBrief_IMX214_20150428.pdf
> 
> Signed-off-by: Ricardo Ribalda Delgado <ribalda@xxxxxxxxxx>
> ---
>  drivers/media/i2c/imx214.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
> index 159a3a604f0e..adcaaa8c86d1 100644
> --- a/drivers/media/i2c/imx214.c
> +++ b/drivers/media/i2c/imx214.c
> @@ -47,6 +47,7 @@ struct imx214 {
>  	struct v4l2_ctrl *pixel_rate;
>  	struct v4l2_ctrl *link_freq;
>  	struct v4l2_ctrl *exposure;
> +	struct v4l2_ctrl *unit_size;
>  
>  	struct regulator_bulk_data	supplies[IMX214_NUM_SUPPLIES];
>  
> @@ -948,6 +949,10 @@ static int imx214_probe(struct i2c_client *client)
>  	static const s64 link_freq[] = {
>  		IMX214_DEFAULT_LINK_FREQ,
>  	};
> +	static const struct v4l2_area unit_size = {
> +		.width = 1120,
> +		.height = 1120,
> +	};
>  	int ret;
>  
>  	ret = imx214_parse_fwnode(dev);
> @@ -1029,6 +1034,10 @@ static int imx214_probe(struct i2c_client *client)
>  					     V4L2_CID_EXPOSURE,
>  					     0, 3184, 1, 0x0c70);
>  
> +	imx214->unit_size = v4l2_ctrl_new_std_compound(&imx214->ctrls,
> +				NULL,
> +				V4L2_CID_UNIT_CELL_SIZE,
> +				v4l2_ctrl_ptr_create((void *)&unit_size));

The imx214 supports two modes: 4096x2304 and 1920x1080. I assume that the
latter is using binning? So shouldn't the unit cell size be different in that
case?

I'm not so sure the unit cell size should depend on binning. I'd rather have
the binning information exposed somehow and let userspace do the calculation.

Regards,

	Hans

>  	ret = imx214->ctrls.error;
>  	if (ret) {
>  		dev_err(&client->dev, "%s control init failed (%d)\n",
>