The driver suffers from a few compliance and implementation issues: - crop rectangle is affected by .set_fmt() pad operation callback, - frame scaling is not reset on modification of crop rectangle, - V4L2_SUBDEV_FORMAT_TRY support in .set_fmt() uses active crop rectangle, not the one from a pad config, as a reference.
For easy resolution of those issues, ov6650_s_fmt() will first be refactored. The ov6650_s_fmt() helper function, mostly called form .set_fmt() pad operation callback, now takes a decision if half scaling is applicable for current crop rectangle and requested frame size, then possibly adjusts hardware crop settings, frame scaling and media bus frame format. It accepts a struct v4l2_mbus_framefmt argument passed by a user to .set_fmt(). Move the decision on applicability of half scaling up to .set_fmt(), then modify the ov6650_s_fmt() API so it accepts a half scaling flag. In order to avoid passing full struct v4l2_mbus_framefmt argument to it when called from functions other than .set_fmt(), also accept a target pixel code as an argument and make the struct v4l2_mbus_framefmt used for crop rectangle modification optional. Signed-off-by: Janusz Krzysztofik <jmkrzy...@gmail.com> --- drivers/media/i2c/ov6650.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index 1b02479b616f..8cb30f3e4851 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -570,25 +570,18 @@ 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) +static int ov6650_s_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf, + u32 code, bool half_scale) { struct i2c_client *client = v4l2_get_subdevdata(sd); struct ov6650 *priv = to_ov6650(client); - bool half_scale = !is_unscaled_ok(mf->width, mf->height, &priv->rect); struct v4l2_subdev_selection sel = { .which = V4L2_SUBDEV_FORMAT_ACTIVE, .target = V4L2_SEL_TGT_CROP, - .r.left = priv->rect.left + (priv->rect.width >> 1) - - (mf->width >> (1 - half_scale)), - .r.top = priv->rect.top + (priv->rect.height >> 1) - - (mf->height >> (1 - half_scale)), - .r.width = mf->width << half_scale, - .r.height = mf->height << half_scale, }; - u32 code = mf->code; unsigned long mclk, pclk; u8 coma_set = 0, coma_mask = 0, coml_set, coml_mask, clkrc; - int ret; + int ret = 0; /* select color matrix configuration for given color encoding */ switch (code) { @@ -668,7 +661,16 @@ 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); - ret = ov6650_set_selection(sd, NULL, &sel); + 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); if (!ret) @@ -691,11 +693,14 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf = &format->format; struct i2c_client *client = v4l2_get_subdevdata(sd); struct ov6650 *priv = to_ov6650(client); + bool half_scale; if (format->pad) return -EINVAL; - if (is_unscaled_ok(mf->width, mf->height, &priv->rect)) + 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); @@ -730,7 +735,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); + int ret = ov6650_s_fmt(sd, mf, mf->code, half_scale); if (ret) return ret; @@ -885,11 +890,8 @@ static int ov6650_video_probe(struct v4l2_subdev *sd) ret = ov6650_reset(client); if (!ret) ret = ov6650_prog_dflt(client); - if (!ret) { - struct v4l2_mbus_framefmt mf = ov6650_def_fmt; - - ret = ov6650_s_fmt(sd, &mf); - } + if (!ret) + ret = ov6650_s_fmt(sd, NULL, ov6650_def_fmt.code, false); if (!ret) ret = v4l2_ctrl_handler_setup(&priv->hdl); -- 2.21.0