linux-kernel - [RFC PATCH 2/5] media: ov6650: Refactor ov6650_s

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190526204758.1904-3-jmkrzyszt@gmail.com>
Date:   Sun, 26 May 2019 22:47:55 +0200
From:   Janusz Krzysztofik <jmkrzyszt@...il.com>
To:     Mauro Carvalho Chehab <mchehab@...nel.org>
Cc:     Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        Janusz Krzysztofik <jmkrzyszt@...il.com>
Subject: [RFC PATCH 2/5] media: ov6650: Refactor ov6650_s_fmt() helper

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@...il.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