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 <jmkrzyszt@xxxxxxxxx>
---
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
