| Message ID | 20210809225845.916430-6-djrscally@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Extensions to ov8865 driver | expand |
Hi Daniel, On Mon, Aug 09, 2021 at 11:58:38PM +0100, Daniel Scally wrote: > The ov8865 driver's v4l2_subdev_pad_ops currently does not include > .get_selection() - add support for that callback. Could you use the same for .set_selection()? Even if it doesn't change anything.
Hi Sakari - sorry delayed reply On 10/08/2021 14:38, Sakari Ailus wrote: > Hi Daniel, > > On Mon, Aug 09, 2021 at 11:58:38PM +0100, Daniel Scally wrote: >> The ov8865 driver's v4l2_subdev_pad_ops currently does not include >> .get_selection() - add support for that callback. > Could you use the same for .set_selection()? Even if it doesn't change > anything. You mean do the same? Or use the same function?
On Wed, Aug 25, 2021 at 12:17:15AM +0100, Daniel Scally wrote: > Hi Sakari - sorry delayed reply > > On 10/08/2021 14:38, Sakari Ailus wrote: > > Hi Daniel, > > > > On Mon, Aug 09, 2021 at 11:58:38PM +0100, Daniel Scally wrote: > >> The ov8865 driver's v4l2_subdev_pad_ops currently does not include > >> .get_selection() - add support for that callback. > > Could you use the same for .set_selection()? Even if it doesn't change > > anything. > > > You mean do the same? Or use the same function? The same function. If the selection isn't changeable anyway, the functionality is the same for both.
On Wed, Aug 25, 2021 at 10:16:02AM +0300, Sakari Ailus wrote: > On Wed, Aug 25, 2021 at 12:17:15AM +0100, Daniel Scally wrote: > > Hi Sakari - sorry delayed reply > > > > On 10/08/2021 14:38, Sakari Ailus wrote: > > > Hi Daniel, > > > > > > On Mon, Aug 09, 2021 at 11:58:38PM +0100, Daniel Scally wrote: > > >> The ov8865 driver's v4l2_subdev_pad_ops currently does not include > > >> .get_selection() - add support for that callback. > > > Could you use the same for .set_selection()? Even if it doesn't change > > > anything. > > > > You mean do the same? Or use the same function? > > The same function. If the selection isn't changeable anyway, the > functionality is the same for both. Except that .s_selection() should return an error if you try to set the bounds or default rectangles.
On Wed, Aug 25, 2021 at 11:04:02AM +0300, Laurent Pinchart wrote: > On Wed, Aug 25, 2021 at 10:16:02AM +0300, Sakari Ailus wrote: > > On Wed, Aug 25, 2021 at 12:17:15AM +0100, Daniel Scally wrote: > > > Hi Sakari - sorry delayed reply > > > > > > On 10/08/2021 14:38, Sakari Ailus wrote: > > > > Hi Daniel, > > > > > > > > On Mon, Aug 09, 2021 at 11:58:38PM +0100, Daniel Scally wrote: > > > >> The ov8865 driver's v4l2_subdev_pad_ops currently does not include > > > >> .get_selection() - add support for that callback. > > > > Could you use the same for .set_selection()? Even if it doesn't change > > > > anything. > > > > > > You mean do the same? Or use the same function? > > > > The same function. If the selection isn't changeable anyway, the > > functionality is the same for both. > > Except that .s_selection() should return an error if you try to set the > bounds or default rectangles. That would make sense. But it's not documented. And in any case it should be implemented in the framework. :-)
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c index 1382b16d1a09..8c2b7d3cbc8c 100644 --- a/drivers/media/i2c/ov8865.c +++ b/drivers/media/i2c/ov8865.c @@ -450,6 +450,15 @@ #define OV8865_PRE_CTRL0_PATTERN_COLOR_SQUARES 2 #define OV8865_PRE_CTRL0_PATTERN_BLACK 3 +/* Pixel Array */ + +#define OV8865_NATIVE_WIDTH 3296 +#define OV8865_NATIVE_HEIGHT 2528 +#define OV8865_ACTIVE_START_TOP 32 +#define OV8865_ACTIVE_START_LEFT 80 +#define OV8865_ACTIVE_WIDTH 3264 +#define OV8865_ACTIVE_HEIGHT 2448 + /* Macros */ #define ov8865_subdev_sensor(s) \ @@ -2749,12 +2758,64 @@ static int ov8865_enum_frame_interval(struct v4l2_subdev *subdev, return 0; } +static void +__ov8865_get_pad_crop(struct ov8865_sensor *sensor, + struct v4l2_subdev_state *state, unsigned int pad, + enum v4l2_subdev_format_whence which, struct v4l2_rect *r) +{ + switch (which) { + case V4L2_SUBDEV_FORMAT_TRY: + *r = *v4l2_subdev_get_try_crop(&sensor->subdev, state, pad); + break; + case V4L2_SUBDEV_FORMAT_ACTIVE: + r->height = sensor->state.mode->output_size_y; + r->width = sensor->state.mode->output_size_x; + r->top = (OV8865_NATIVE_HEIGHT - sensor->state.mode->output_size_y) / 2; + r->left = (OV8865_NATIVE_WIDTH - sensor->state.mode->output_size_x) / 2; + break; + } +} + +static int ov8865_get_selection(struct v4l2_subdev *subdev, + struct v4l2_subdev_state *state, + struct v4l2_subdev_selection *sel) +{ + struct ov8865_sensor *sensor = ov8865_subdev_sensor(subdev); + + switch (sel->target) { + case V4L2_SEL_TGT_CROP: + mutex_lock(&sensor->mutex); + __ov8865_get_pad_crop(sensor, state, sel->pad, + sel->which, &sel->r); + mutex_unlock(&sensor->mutex); + break; + case V4L2_SEL_TGT_NATIVE_SIZE: + sel->r.top = 0; + sel->r.left = 0; + sel->r.width = OV8865_NATIVE_WIDTH; + sel->r.height = OV8865_NATIVE_HEIGHT; + break; + case V4L2_SEL_TGT_CROP_BOUNDS: + case V4L2_SEL_TGT_CROP_DEFAULT: + sel->r.top = OV8865_ACTIVE_START_TOP; + sel->r.left = OV8865_ACTIVE_START_LEFT; + sel->r.width = OV8865_ACTIVE_WIDTH; + sel->r.height = OV8865_ACTIVE_HEIGHT; + break; + default: + return -EINVAL; + } + + return 0; +} + static const struct v4l2_subdev_pad_ops ov8865_subdev_pad_ops = { .enum_mbus_code = ov8865_enum_mbus_code, .get_fmt = ov8865_get_fmt, .set_fmt = ov8865_set_fmt, .enum_frame_size = ov8865_enum_frame_size, .enum_frame_interval = ov8865_enum_frame_interval, + .get_selection = ov8865_get_selection, }; static const struct v4l2_subdev_ops ov8865_subdev_ops = {
The ov8865 driver's v4l2_subdev_pad_ops currently does not include .get_selection() - add support for that callback. Signed-off-by: Daniel Scally <djrscally@gmail.com> --- Changes in v2: - None drivers/media/i2c/ov8865.c | 61 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)