Message ID | 20191001112421.2778-9-ribalda@kernel.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Implement UNIT_CELL_SIZE control | expand |
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@kernel.org> > --- > 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", >
Hi Hans On Fri, Oct 4, 2019 at 12:31 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> 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 <ribalda@kernel.org> > > --- > > 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? 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(&client->dev, "%s control init failed (%d)\n", > > >
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)); ret = imx214->ctrls.error; if (ret) { dev_err(&client->dev, "%s control init failed (%d)\n",
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@kernel.org> --- drivers/media/i2c/imx214.c | 9 +++++++++ 1 file changed, 9 insertions(+)