Re: [PATCH v9 8/8] media: imx214: Add new control with V4L2_CID_UNIT_CELL_SIZE
Hi Hans
On Fri, Oct 4, 2019 at 12:31 PM Hans Verkuil wrote:
>
> 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
> > ---
> > 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(>ctrls,
> > + NULL,
> > + V4L2_CID_UNIT_CELL_SIZE,
> > + v4l2_ctrl_ptr_create((void *)_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?
To be honest I do not know if it is cropping or binning. I can try to
turn on the sensor and figure out if the field of view changes.
>
> 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.
I think there are good arguments on both directions ;).
The issue with considering the binning is that if there a later step
on the pipeline that does the bining then it has to change also the
unit_cell_size control. I would say, for now lets keep them
independent.
>
> Regards,
>
> Hans
>
> > ret = imx214->ctrls.error;
> > if (ret) {
> > dev_err(>dev, "%s control init failed (%d)\n",
> >
>
Re: [PATCH v9 8/8] media: imx214: Add new control with V4L2_CID_UNIT_CELL_SIZE
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
> ---
> 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(>ctrls,
> + NULL,
> + V4L2_CID_UNIT_CELL_SIZE,
> + v4l2_ctrl_ptr_create((void *)_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(>dev, "%s control init failed (%d)\n",
>
[PATCH v9 8/8] media: imx214: Add new control with V4L2_CID_UNIT_CELL_SIZE
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
---
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(>ctrls,
+ NULL,
+ V4L2_CID_UNIT_CELL_SIZE,
+ v4l2_ctrl_ptr_create((void *)_size));
ret = imx214->ctrls.error;
if (ret) {
dev_err(>dev, "%s control init failed (%d)\n",
--
2.23.0
