According to subdevice interface specification found in V4L2 API
documentation, .set_fmt() pad operation callback should not affect
image geometry set in preceding image processing steps. Unfortunately,
ov6650_s_fmt() helper, when called from .set_fmt() with new user
requested frame size passed to it, does not respect that requirement
as that was not the case before, when the helper was still called from
.mbus_set_fmt() video operation callback.
Remmove a call to .set_selection() from ov6650_s_fmt() helper so it no
longer modifies the active crop rectangle and simplify its API by
removing no longer needed struct v4l2_mbus_framefmt argument. For
consistency of its results with active format processing, also update
try format path inside .set_fmt() (still using active crop rectangle as
a reference instead of the try one from pad config which is not yet
maintained by the driver).
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@xxxxxxxxx>
---
drivers/media/i2c/ov6650.c | 32 ++++++--------------------------
1 file changed, 6 insertions(+), 26 deletions(-)
diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
index 8cb30f3e4851..47590cd51190 100644
--- a/drivers/media/i2c/ov6650.c
+++ b/drivers/media/i2c/ov6650.c
@@ -570,15 +570,10 @@ static u8 to_clkrc(struct v4l2_fract *timeperframe,
}
/* set the format we will capture in */
-static int ov6650_s_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf,
- u32 code, bool half_scale)
+static int ov6650_s_fmt(struct v4l2_subdev *sd, u32 code, bool half_scale)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov6650 *priv = to_ov6650(client);
- struct v4l2_subdev_selection sel = {
- .which = V4L2_SUBDEV_FORMAT_ACTIVE,
- .target = V4L2_SEL_TGT_CROP,
- };
unsigned long mclk, pclk;
u8 coma_set = 0, coma_mask = 0, coml_set, coml_mask, clkrc;
int ret = 0;
@@ -661,18 +656,7 @@ static int ov6650_s_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf,
dev_dbg(&client->dev, "pixel clock divider: %ld.%ld\n",
mclk / pclk, 10 * mclk % pclk / pclk);
- if (mf) {
- sel.r.left = priv->rect.left + (priv->rect.width >> 1) -
- (mf->width >> (1 - half_scale)),
- sel.r.top = priv->rect.top + (priv->rect.height >> 1) -
- (mf->height >> (1 - half_scale)),
- sel.r.width = mf->width << half_scale,
- sel.r.height = mf->height << half_scale,
-
- ret = ov6650_set_selection(sd, NULL, &sel);
- }
- if (!ret)
- ret = ov6650_reg_rmw(client, REG_COMA, coma_set, coma_mask);
+ ret = ov6650_reg_rmw(client, REG_COMA, coma_set, coma_mask);
if (!ret)
ret = ov6650_reg_write(client, REG_CLKRC, clkrc);
if (!ret) {
@@ -700,10 +684,6 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd,
half_scale = !is_unscaled_ok(mf->width, mf->height, &priv->rect);
- if (!half_scale)
- v4l_bound_align_image(&mf->width, 2, W_CIF, 1,
- &mf->height, 2, H_CIF, 1, 0);
-
switch (mf->code) {
case MEDIA_BUS_FMT_Y10_1X10:
mf->code = MEDIA_BUS_FMT_Y8_1X8;
@@ -723,8 +703,8 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd,
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
/* store media bus format code and frame size in pad config */
- cfg->try_fmt.width = mf->width;
- cfg->try_fmt.height = mf->height;
+ cfg->try_fmt.width = priv->rect.width >> half_scale;
+ cfg->try_fmt.height = priv->rect.height >> half_scale;
cfg->try_fmt.code = mf->code;
/* return default mbus frame format updated with pad config */
@@ -735,7 +715,7 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd,
} else {
/* apply new media bus format code and frame size */
- int ret = ov6650_s_fmt(sd, mf, mf->code, half_scale);
+ int ret = ov6650_s_fmt(sd, mf->code, half_scale);
if (ret)
return ret;
@@ -891,7 +871,7 @@ static int ov6650_video_probe(struct v4l2_subdev *sd)
if (!ret)
ret = ov6650_prog_dflt(client);
if (!ret)
- ret = ov6650_s_fmt(sd, NULL, ov6650_def_fmt.code, false);
+ ret = ov6650_s_fmt(sd, ov6650_def_fmt.code, false);
if (!ret)
ret = v4l2_ctrl_handler_setup(&priv->hdl);
--
2.21.0
