linux-kernel - [RFC PATCH 3/5] media: ov6650: Fix active crop rectangle affected by .set

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-4-jmkrzyszt@gmail.com>
Date:   Sun, 26 May 2019 22:47:56 +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 3/5] media: ov6650: Fix active crop rectangle affected by .set_fmt()

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