HI Hans,
On Thu, Aug 29, 2019 at 12:20:18PM +0200, Hans Verkuil wrote:
> On 8/27/19 11:23 AM, Jacopo Mondi wrote:
> > Report the native pixel array size and the crop bounds for the ov5670
> > sensor driver.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> > ---
> > drivers/media/i2c/ov5670.c | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
> > index 2bc57e85f721..3e22fe9ccad1 100644
> > --- a/drivers/media/i2c/ov5670.c
> > +++ b/drivers/media/i2c/ov5670.c
> > @@ -2258,6 +2258,25 @@ static int ov5670_set_pad_format(struct v4l2_subdev *sd,
> > return 0;
> > }
> >
> > +static int ov5670_get_selection(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_selection *sel)
> > +{
> > + switch (sel->target) {
> > + case V4L2_SEL_TGT_CROP_BOUNDS:
> > + case V4L2_SEL_TGT_NATIVE_SIZE:
> > + sel->r.left = 0;
> > + sel->r.top = 0;
> > + sel->r.width = 2592;
> > + sel->r.height = 1944;
>
> Why do you need this?
>
I need to expose the pixel array size and the active pixel area size,
and I interpreted the two above targets as the right place where to do
so.
At least for NATIVE_SIZE the documentation seems to match my
understanding:
"The native size of the device, e.g. a sensor’s pixel array. left and top
fields are zero for this target."
> Since the format can change for this and the next driver I think CROP_BOUNDS
> at least should match the current format.
>
Ah, does it? It was not clear to me from the documentation, as it
suggested to me that target actually identifies the active pixel area
"Bounds of the crop rectangle. All valid crop rectangles fit inside the
crop bounds rectangle."
It does not mention format, should this be updated?
How would you report the two information I need?
> I don't think this patch and the next have anything to do with the location/rotate
> support. I would split it off from this series.
>
Agreed, they were split in v1, maybe it has not been a wise move to
make a single series out of them. I'll split again.
Thanks
j
> Regards,
>
> Hans
>
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int ov5670_get_skip_frames(struct v4l2_subdev *sd, u32 *frames)
> > {
> > *frames = OV5670_NUM_OF_SKIP_FRAMES;
> > @@ -2425,6 +2444,7 @@ static const struct v4l2_subdev_pad_ops ov5670_pad_ops = {
> > .enum_mbus_code = ov5670_enum_mbus_code,
> > .get_fmt = ov5670_get_pad_format,
> > .set_fmt = ov5670_set_pad_format,
> > + .get_selection = ov5670_get_selection,
> > .enum_frame_size = ov5670_enum_frame_size,
> > };
> >
> >
>
Attachment:
signature.asc
Description: PGP signature
