From patchwork Sun May 3 22:06:16 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Janusz Krzysztofik X-Patchwork-Id: 63522 X-Patchwork-Delegate: sakari.ailus@iki.fi Received: from vger.kernel.org ([23.128.96.18]) by www.linuxtv.org with esmtp (Exim 4.92) (envelope-from ) id 1jVMhi-00A8vl-5h; Sun, 03 May 2020 22:03:19 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729156AbgECWG3 (ORCPT + 1 other); Sun, 3 May 2020 18:06:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40912 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1729114AbgECWG3 (ORCPT ); Sun, 3 May 2020 18:06:29 -0400 Received: from mail-lj1-x241.google.com (mail-lj1-x241.google.com [IPv6:2a00:1450:4864:20::241]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D9569C061A0E for ; Sun, 3 May 2020 15:06:28 -0700 (PDT) Received: by mail-lj1-x241.google.com with SMTP id u15so7843926ljd.3 for ; Sun, 03 May 2020 15:06:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=RD2lmTz2BQWeLN0p8b/I76lKaQDT+HW+7lWuS6rBiEU=; b=hAttu7RyLafBOkTOxGCYom7MSalCYbMzkTwignNKkmMt2MOfMItLyK3k5zQLeivlNz BCTSCiN/SK/HArhzKMXI3V7hoBPOxU177BpPtQ+CqBMXj+Tg0iEojkaQm20KmHj9ktaY RgDDv8E7bXK4o4BT4OXj5K32sEViuuvIJl3keZWh7k3HGjDOXaNA9ccNJP4J9O/YgroU dp6LTNLs8K2eaF5e6C9HmfYIGsb4FY3deOebOsL+QhE3keOg2TsEExg139bUj4jRDgB8 o9mUqA2wWbwiYRcS4+fyDcygRQjpHsVBPH4fVUcA/0q0Rt94uZgE56TZetI6Ys0uALRH ToTw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=RD2lmTz2BQWeLN0p8b/I76lKaQDT+HW+7lWuS6rBiEU=; b=kFmYnhc9NewjHo6Vn+O5QCceKY65/3zqn3bnqE4LDn/ydjYsLDwxCDD8zUUxH39wtl I4ZWaPTAbqsbX8nUhnuRWFz18ZStoj+rjU1+0wLNyffzgg7t7DuEQvSFT3jtX/aSDyIe VxrrZwoxLKISMvguKHOcaEM4+CCU116RX11Yaa8+e3R5QgH9C8O9OQ7c+/m3zFXZyxgX RAoLZeIBLvVaDVvSoB3vbPK9QRuXhUFNNwCSO0sEK7WGmIdMwHKArhAxy2JquL8dp/dV EPigg/JnUrBGmG8p90venlKH8yuM/H0DlwJkbaZj7C5OtZO62DXzNgkGpMJfYe3BiXXf fQGA== X-Gm-Message-State: AGi0PuYQA7AOAGMy+ryWCzHMWK+sTRGmZPqfHcqUBmYTmgxLCCF+WFTA CQTiux3YNk7u8bBbB4VyZFw= X-Google-Smtp-Source: APiQypIZ5bczAs5b3BcM33Bu0Vpgv9DZNtpC/AQksyT6FaiXp/A23KnVRftfTg7NFtDDLmzDC9g4fg== X-Received: by 2002:a2e:b4a5:: with SMTP id q5mr9069888ljm.58.1588543587180; Sun, 03 May 2020 15:06:27 -0700 (PDT) Received: from z50.gdansk-morena.vectranet.pl (109241122244.gdansk.vectranet.pl. [109.241.122.244]) by smtp.gmail.com with ESMTPSA id 16sm6637433ljr.55.2020.05.03.15.06.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 03 May 2020 15:06:26 -0700 (PDT) From: Janusz Krzysztofik To: Sakari Ailus , Hans Verkuil Cc: linux-media@vger.kernel.org, Janusz Krzysztofik Subject: [PATCH 1/3] media: ov6650: Fix set format try processing path Date: Mon, 4 May 2020 00:06:16 +0200 Message-Id: <20200503220618.27743-2-jmkrzyszt@gmail.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200503220618.27743-1-jmkrzyszt@gmail.com> References: <20200503220618.27743-1-jmkrzyszt@gmail.com> MIME-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-LSpam-Score: -2.5 (--) X-LSpam-Report: No, score=-2.5 required=5.0 tests=BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,FREEMAIL_FORGED_FROMDOMAIN=0.001,FREEMAIL_FROM=0.001,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1 autolearn=ham autolearn_force=no According to subdevice interface specification found in V4L2 API documentation, set format pad operations should not affect image geometry set in preceding image processing steps. Unfortunately, that requirement is not respected by the driver implementation of set format as it was not the case when that code was still implementing a pair of now obsolete .s_mbus_fmt() / .try_mbus_fmt() video operations before they have been merged and reused as an implementation of .set_fmt() pad operation by commit 717fd5b4907a ("[media] v4l2: replace try_mbus_fmt by set_fmt"). In case of set format active processing path the issue can be fixed easily by excluding a call to set active selection from that path. That will effectively limit frame size processing to optimal frame scaling against active crop rectangle without touching it. Users can just call set active selection themselves to obtain desired frame size. However, set format try processing path needs more work. First of all, the driver should be extended with set try selection support. Lack of it constraints video device drivers to not use subdevice cropping at all while processing user requested active frame size, otherwise their set try format results might differ from active. Next, set format try processing path should use pad config crop rectangle as a reference, not the active one as it does now. That issue can be resolved easily as soon as set try selection support is added to the driver so pad config crop rectangle can be maintained by users via selection API. Last, set format try processing path should give the same results as active in respect to active vs. pad config crop rectangle geometry. Both rectangles should be either not touched by set format (that's what we are going to achieve) or modified the same way, otherwise users won't be able to obtain equal results from both paths while iterating through set format and set selection operations in order to obtain desired frame size. We can't begin with modifying set format pad operation as not to touch crop rectangle since that depends on availability of set try selection for symmetry. Neither can we begin with adding set try selection since that in turn depends on equal handling of active and pad config crop rectangles by set format. We can either implement all required modifications in a single patch, or begin with fixing current set format try processing path to appropriately handle pad config crop rectangle. This patch implements the latter approach as believed to be more readable. Move crop rectangle adjustments code from a helper (the former implementation of .s_fmt(), now called from set format active processing path) to the body of set format pad operation function where it can be also used for processing try requests for symmetry with active ones. As the helper no longer processes frame geometry, only frame format and half scaling, simplify its API accordingly and update its users. Moreover, extract code that applies crop rectangle hardware limits (now a part of .set_selection() operation which is called from set format active processing path) to a new helper and call that helper from set format try processing path as well for symmetry with active. Fixes: 717fd5b4907a ("[media] v4l2: replace try_mbus_fmt by set_fmt") Signed-off-by: Janusz Krzysztofik --- drivers/media/i2c/ov6650.c | 83 ++++++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 35 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index 91906b94f978..c0ac3d0ae167 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -491,6 +491,17 @@ static int ov6650_get_selection(struct v4l2_subdev *sd, } } +static void ov6650_bind_align_crop_rectangle(struct v4l2_rect *rect) +{ + v4l_bound_align_image(&rect->width, 2, W_CIF, 1, + &rect->height, 2, H_CIF, 1, 0); + v4l_bound_align_image(&rect->left, DEF_HSTRT << 1, + (DEF_HSTRT << 1) + W_CIF - (__s32)rect->width, 1, + &rect->top, DEF_VSTRT << 1, + (DEF_VSTRT << 1) + H_CIF - (__s32)rect->height, + 1, 0); +} + static int ov6650_set_selection(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config *cfg, struct v4l2_subdev_selection *sel) @@ -503,13 +514,7 @@ static int ov6650_set_selection(struct v4l2_subdev *sd, sel->target != V4L2_SEL_TGT_CROP) return -EINVAL; - v4l_bound_align_image(&sel->r.width, 2, W_CIF, 1, - &sel->r.height, 2, H_CIF, 1, 0); - v4l_bound_align_image(&sel->r.left, DEF_HSTRT << 1, - (DEF_HSTRT << 1) + W_CIF - (__s32)sel->r.width, 1, - &sel->r.top, DEF_VSTRT << 1, - (DEF_VSTRT << 1) + H_CIF - (__s32)sel->r.height, - 1, 0); + ov6650_bind_align_crop_rectangle(&sel->r); ret = ov6650_reg_write(client, REG_HSTRT, sel->r.left >> 1); if (!ret) { @@ -570,22 +575,10 @@ static bool is_unscaled_ok(int width, int height, struct v4l2_rect *rect) #define to_clkrc(div) ((div) - 1) /* 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, 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; u8 coma_set = 0, coma_mask = 0, coml_set, coml_mask; int ret; @@ -653,9 +646,7 @@ static int ov6650_s_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf) coma_mask |= COMA_QCIF; } - 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) { priv->half_scale = half_scale; @@ -674,14 +665,16 @@ 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); + struct v4l2_subdev_selection sel = { + .which = V4L2_SUBDEV_FORMAT_ACTIVE, + .target = V4L2_SEL_TGT_CROP, + }; + struct v4l2_rect *crop = &sel.r; + bool half_scale; if (format->pad) return -EINVAL; - if (is_unscaled_ok(mf->width, mf->height, &priv->rect)) - 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; @@ -699,10 +692,24 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd, break; } + *crop = priv->rect; + half_scale = !is_unscaled_ok(mf->width, mf->height, crop); + + /* adjust new crop rectangle position against its current center */ + crop->left += (crop->width - (mf->width << half_scale)) / 2; + crop->top += (crop->height - (mf->height << half_scale)) / 2; + /* adjust new crop rectangle size */ + crop->width = mf->width << half_scale; + crop->height = mf->height << half_scale; + 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; + /* store new crop rectangle, hadware bound, in pad config */ + ov6650_bind_align_crop_rectangle(crop); + cfg->try_crop = *crop; + + /* store new mbus frame format code and size in pad config */ + cfg->try_fmt.width = crop->width >> half_scale; + cfg->try_fmt.height = crop->height >> half_scale; cfg->try_fmt.code = mf->code; /* return default mbus frame format updated with pad config */ @@ -712,9 +719,16 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd, mf->code = cfg->try_fmt.code; } else { - /* apply new media bus format code and frame size */ - int ret = ov6650_s_fmt(sd, mf); + int ret; + /* apply new crop rectangle */ + ret = ov6650_set_selection(sd, NULL, &sel); + if (ret) + return ret; + + /* apply new media bus frame format and scaling if changed */ + if (mf->code != priv->code || half_scale != priv->half_scale) + ret = ov6650_s_fmt(sd, mf->code, half_scale); if (ret) return ret; @@ -890,9 +904,8 @@ static int ov6650_video_probe(struct v4l2_subdev *sd) if (!ret) ret = ov6650_prog_dflt(client, xclk->clkrc); if (!ret) { - struct v4l2_mbus_framefmt mf = ov6650_def_fmt; - - ret = ov6650_s_fmt(sd, &mf); + /* driver default frame format, no scaling */ + ret = ov6650_s_fmt(sd, ov6650_def_fmt.code, false); } if (!ret) ret = v4l2_ctrl_handler_setup(&priv->hdl); From patchwork Sun May 3 22:06:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Janusz Krzysztofik X-Patchwork-Id: 63523 X-Patchwork-Delegate: sakari.ailus@iki.fi Received: from vger.kernel.org ([23.128.96.18]) by www.linuxtv.org with esmtp (Exim 4.92) (envelope-from ) id 1jVMhk-00A8vl-6t; Sun, 03 May 2020 22:03:21 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729164AbgECWGa (ORCPT + 1 other); Sun, 3 May 2020 18:06:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40918 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1729114AbgECWGa (ORCPT ); Sun, 3 May 2020 18:06:30 -0400 Received: from mail-lf1-x144.google.com (mail-lf1-x144.google.com [IPv6:2a00:1450:4864:20::144]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 21C87C061A0E for ; Sun, 3 May 2020 15:06:30 -0700 (PDT) Received: by mail-lf1-x144.google.com with SMTP id l11so8084190lfc.5 for ; Sun, 03 May 2020 15:06:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Zoml5Tpw0YFJPt4sw6pGgXfydvrJQLxIDNIKcMtfsDE=; b=FbO0G90K7lbHuqOTHgSd5KSBhbeuQphdMHFC01oMKIAQnmSQAnGvai8kLTwmZgZm8L hex56op1X73HpI3BqEMjOaFNeDdP4tZOhKMeTSqxlYtytlTtli1ygO3mWsKZ7D/jFRBw zc4mA8guHSjOmdsypAe2YgeHeOWVVYnhHgafm2GGNJxI6C32n+n13VGBuOS+yy8xc77j vj1TwOFv4MafFrhUzC1mELA+61kL9BpSQ94pihXPAPAYO6wRdYAPCBNMRmt+31nQThxk uoeDB2LLcmlXOpUMsYIJnBhxihdAHPNgy1LC7YD4r7WtkAeIxP9cFYQvLc4p6FvXwfsd TEnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Zoml5Tpw0YFJPt4sw6pGgXfydvrJQLxIDNIKcMtfsDE=; b=hSOAOcedCcMWZ6O9rXfIiy1D6IHS6onrGcJnpEXAzM49cEtb4A5DFfvn94JRuolezY eB+hDDK39j5ADVQZKuQF98gr8o/t0j2QzLWhZXGZ1u6Uc7zc3FvvIktbUgmmaf3kGKt/ g5lB9SNkMefY2oUguyPYybEo233wZtdYJ+i79xayBdaiCzzqsIA50PPtiMyA6aTEKhQn ju7F4CX1TMjSvszsg6O5eDEB9FxAwzFKpQn0shjUq32ZXHMSuvPjxH+VN+jCdeHo6Vmd 4+04pkWoq1MatLov6yf/Hoz6IkPrtuCu4g4G50xYn/Fo5wZX13T3fltHzhrkbe0Y0prf 7zWw== X-Gm-Message-State: AGi0PubyHE3Xm5SRvKEPN15cD52e82uQ8VgnQl/J053Kbkg5Zovzi2P1 9OTNO+MrSilLuvBijxbJwTHy4cMucwRkPg== X-Google-Smtp-Source: APiQypL5/0PBL788xGmMYVnZFul9tAg4JWHY2FXBZLggPFgqwL10hZnb1q9zpDnnvUKZUqfPmXcdqQ== X-Received: by 2002:a19:2389:: with SMTP id j131mr9887872lfj.116.1588543588605; Sun, 03 May 2020 15:06:28 -0700 (PDT) Received: from z50.gdansk-morena.vectranet.pl (109241122244.gdansk.vectranet.pl. [109.241.122.244]) by smtp.gmail.com with ESMTPSA id 16sm6637433ljr.55.2020.05.03.15.06.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 03 May 2020 15:06:27 -0700 (PDT) From: Janusz Krzysztofik To: Sakari Ailus , Hans Verkuil Cc: linux-media@vger.kernel.org, Janusz Krzysztofik Subject: [PATCH 2/3] media: ov6650: Add try support to selection API operations Date: Mon, 4 May 2020 00:06:17 +0200 Message-Id: <20200503220618.27743-3-jmkrzyszt@gmail.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200503220618.27743-1-jmkrzyszt@gmail.com> References: <20200503220618.27743-1-jmkrzyszt@gmail.com> MIME-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-LSpam-Score: -2.5 (--) X-LSpam-Report: No, score=-2.5 required=5.0 tests=BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,FREEMAIL_FORGED_FROMDOMAIN=0.001,FREEMAIL_FROM=0.001,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1 autolearn=ham autolearn_force=no Try requests are now only supported by format processing pad operations implemented by the driver. The driver selection API operations currently respond to them with -EINVAL. While that is correct, it constraints video device drivers to not use subdevice cropping at all while processing user requested active frame size, otherwise their set try format results might differ from active. As a consequence, we can't fix set format pad operation as not to touch crop rectangle since that would affect users not being able to set arbitrary frame sizes. Moreover, without a working set try selection support we are not able to use pad config crop rectangle as a reference while processing set try format requests. Implement missing try selection support. Moreover, as it will be now possible to maintain the pad config crop rectangle via selection API, start using it instead of the active one as a reference while processing set try format requests. is_unscaled_ok() helper, now also called from set selection operation, has been just moved up in the source file to avoid a prototype, with no functional changes. Fixes: 717fd5b4907a ("[media] v4l2: replace try_mbus_fmt by set_fmt") Signed-off-by: Janusz Krzysztofik --- drivers/media/i2c/ov6650.c | 54 ++++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index c0ac3d0ae167..65701f2c7c7c 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -472,9 +472,16 @@ static int ov6650_get_selection(struct v4l2_subdev *sd, { struct i2c_client *client = v4l2_get_subdevdata(sd); struct ov6650 *priv = to_ov6650(client); + struct v4l2_rect *rect; - if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE) - return -EINVAL; + if (sel->which == V4L2_SUBDEV_FORMAT_TRY) { + /* pre-select try crop rectangle */ + rect = &cfg->try_crop; + + } else { + /* pre-select active crop rectangle */ + rect = &priv->rect; + } switch (sel->target) { case V4L2_SEL_TGT_CROP_BOUNDS: @@ -483,14 +490,22 @@ static int ov6650_get_selection(struct v4l2_subdev *sd, sel->r.width = W_CIF; sel->r.height = H_CIF; return 0; + case V4L2_SEL_TGT_CROP: - sel->r = priv->rect; + /* use selected crop rectangle */ + sel->r = *rect; return 0; + default: return -EINVAL; } } +static bool is_unscaled_ok(int width, int height, struct v4l2_rect *rect) +{ + return width > rect->width >> 1 || height > rect->height >> 1; +} + static void ov6650_bind_align_crop_rectangle(struct v4l2_rect *rect) { v4l_bound_align_image(&rect->width, 2, W_CIF, 1, @@ -510,12 +525,30 @@ static int ov6650_set_selection(struct v4l2_subdev *sd, struct ov6650 *priv = to_ov6650(client); int ret; - if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE || - sel->target != V4L2_SEL_TGT_CROP) + if (sel->target != V4L2_SEL_TGT_CROP) return -EINVAL; ov6650_bind_align_crop_rectangle(&sel->r); + if (sel->which == V4L2_SUBDEV_FORMAT_TRY) { + struct v4l2_rect *crop = &cfg->try_crop; + struct v4l2_mbus_framefmt *mf = &cfg->try_fmt; + /* detect current pad config scaling factor */ + bool half_scale = !is_unscaled_ok(mf->width, mf->height, crop); + + /* store new crop rectangle */ + *crop = sel->r; + + /* adjust frame size */ + mf->width = crop->width >> half_scale; + mf->height = crop->height >> half_scale; + + return 0; + } + + /* V4L2_SUBDEV_FORMAT_ACTIVE */ + + /* apply new crop rectangle */ ret = ov6650_reg_write(client, REG_HSTRT, sel->r.left >> 1); if (!ret) { priv->rect.width += priv->rect.left - sel->r.left; @@ -567,11 +600,6 @@ static int ov6650_get_fmt(struct v4l2_subdev *sd, return 0; } -static bool is_unscaled_ok(int width, int height, struct v4l2_rect *rect) -{ - return width > rect->width >> 1 || height > rect->height >> 1; -} - #define to_clkrc(div) ((div) - 1) /* set the format we will capture in */ @@ -692,7 +720,11 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd, break; } - *crop = priv->rect; + if (format->which == V4L2_SUBDEV_FORMAT_TRY) + *crop = cfg->try_crop; + else + *crop = priv->rect; + half_scale = !is_unscaled_ok(mf->width, mf->height, crop); /* adjust new crop rectangle position against its current center */ From patchwork Sun May 3 22:06:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Janusz Krzysztofik X-Patchwork-Id: 63524 X-Patchwork-Delegate: sakari.ailus@iki.fi Received: from vger.kernel.org ([23.128.96.18]) by www.linuxtv.org with esmtp (Exim 4.92) (envelope-from ) id 1jVMhm-00A8vl-5F; Sun, 03 May 2020 22:03:22 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729165AbgECWGd (ORCPT + 1 other); Sun, 3 May 2020 18:06:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40926 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1729114AbgECWGd (ORCPT ); Sun, 3 May 2020 18:06:33 -0400 Received: from mail-lj1-x244.google.com (mail-lj1-x244.google.com [IPv6:2a00:1450:4864:20::244]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9650AC061A0E for ; Sun, 3 May 2020 15:06:31 -0700 (PDT) Received: by mail-lj1-x244.google.com with SMTP id e25so7824625ljg.5 for ; Sun, 03 May 2020 15:06:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=EfKamFfTVpvru80eT02A3+UKqspihWi9aVYBpbm4WN0=; b=RViQcO5taL2805iPYOaknLxXGwJ3NV9DADsyLaMOdtmSnukxX/fE88ntTpPZ1M/MnD vhEG7ShUEqfka3XEAXjbLGot3wn5eRqd7041aLg19Ijm9fvBCsco2zAgYd+AK1USiMEf 9nuSVXANGyfsBUq2W9MUZGn8sMmnAceBjTx5bi582uoNiCPCkm1gkww5LQfAiQEs2s8A 1d/VkeqWvgC6jft6ocQythtifWd4DFw0oHxWok13olhhzPcuq+RFIYQcNKqCYMQgzvad F0clUdxs3TlEBarLImwkMM0JrJeBO3zRa0AUFMvrnoLVGrtExwClgMfoSPF4vy5nduak bC/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=EfKamFfTVpvru80eT02A3+UKqspihWi9aVYBpbm4WN0=; b=E1b0k9CyLLCiZ1y/Lobb9+Nd2RVQmqiHzclDpn8kKhfuIl+fAApGy2m0UVDOtfSSOj iWKVFzDrlQtY2dlKGpZqoXvN09mO5lRprW1qw93SoBp0jCGwzD1z6VMYUpSHy2YHGcH+ pK5vEmuwJ2S5nh/pj7CTAgB5FxBKFcTDmqYVFJDsuJXyaOmS4GXFL4zQC2KocWYctHSw 3OtWyXufANN/DJCAyJygI/zuNRqMEtugz3mBYeMuOcDos9dMSfpzv7D1UB7vTALNxvW5 CBaPD56/oJoASw0G2lYetkxTlR7zADjY9aqHckJ+3nlsgd0niQaUGa+DPIgLsKA5v+ZT YzLQ== X-Gm-Message-State: AGi0Pubg73pdL1Zl1R6V/zopEFnAMPYKDx+fVUq5u5OmQrS/42C10bwc pu9jUuHyw2UCbpHJHfoxTdM= X-Google-Smtp-Source: APiQypIADUUI9UDgXt6N+zIq1Y35S14CHqgNt+la/no8cnUAqu1v+J5o6faFpRrvGeYXRb15CWt1dg== X-Received: by 2002:a2e:8686:: with SMTP id l6mr8641751lji.152.1588543590014; Sun, 03 May 2020 15:06:30 -0700 (PDT) Received: from z50.gdansk-morena.vectranet.pl (109241122244.gdansk.vectranet.pl. [109.241.122.244]) by smtp.gmail.com with ESMTPSA id 16sm6637433ljr.55.2020.05.03.15.06.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 03 May 2020 15:06:29 -0700 (PDT) From: Janusz Krzysztofik To: Sakari Ailus , Hans Verkuil Cc: linux-media@vger.kernel.org, Janusz Krzysztofik Subject: [PATCH 3/3] media: ov6650: Fix crop rectangle affected by set format Date: Mon, 4 May 2020 00:06:18 +0200 Message-Id: <20200503220618.27743-4-jmkrzyszt@gmail.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200503220618.27743-1-jmkrzyszt@gmail.com> References: <20200503220618.27743-1-jmkrzyszt@gmail.com> MIME-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-LSpam-Score: -2.5 (--) X-LSpam-Report: No, score=-2.5 required=5.0 tests=BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,FREEMAIL_FORGED_FROMDOMAIN=0.001,FREEMAIL_FROM=0.001,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1 autolearn=ham autolearn_force=no According to subdevice interface specification found in V4L2 API documentation, set format pad operations should not affect image geometry set in preceding image processing steps. Unfortunately, that requirement is not respected by the driver implementation of set format as it was not the case when that code was still implementing a pair of now obsolete .s_mbus_fmt() / .try_mbus_fmt() video operations before they have been merged and reused as an implementation of .set_fmt() pad operation by commit 717fd5b4907a ("[media] v4l2: replace try_mbus_fmt by set_fmt"). Exclude non-compliant crop rectangle adjustments from set format try, as well as a call to .set_selection() from set format active processing path, so only frame scaling is applied as needed and crop rectangle is no longer modified. Fixes: 717fd5b4907a ("[media] v4l2: replace try_mbus_fmt by set_fmt") Signed-off-by: Janusz Krzysztofik --- drivers/media/i2c/ov6650.c | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index 65701f2c7c7c..4439e2aca076 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -693,11 +693,7 @@ 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); - struct v4l2_subdev_selection sel = { - .which = V4L2_SUBDEV_FORMAT_ACTIVE, - .target = V4L2_SEL_TGT_CROP, - }; - struct v4l2_rect *crop = &sel.r; + struct v4l2_rect *crop; bool half_scale; if (format->pad) @@ -721,24 +717,13 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd, } if (format->which == V4L2_SUBDEV_FORMAT_TRY) - *crop = cfg->try_crop; + crop = &cfg->try_crop; else - *crop = priv->rect; + crop = &priv->rect; half_scale = !is_unscaled_ok(mf->width, mf->height, crop); - /* adjust new crop rectangle position against its current center */ - crop->left += (crop->width - (mf->width << half_scale)) / 2; - crop->top += (crop->height - (mf->height << half_scale)) / 2; - /* adjust new crop rectangle size */ - crop->width = mf->width << half_scale; - crop->height = mf->height << half_scale; - if (format->which == V4L2_SUBDEV_FORMAT_TRY) { - /* store new crop rectangle, hadware bound, in pad config */ - ov6650_bind_align_crop_rectangle(crop); - cfg->try_crop = *crop; - /* store new mbus frame format code and size in pad config */ cfg->try_fmt.width = crop->width >> half_scale; cfg->try_fmt.height = crop->height >> half_scale; @@ -751,12 +736,7 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd, mf->code = cfg->try_fmt.code; } else { - int ret; - - /* apply new crop rectangle */ - ret = ov6650_set_selection(sd, NULL, &sel); - if (ret) - return ret; + int ret = 0; /* apply new media bus frame format and scaling if changed */ if (mf->code != priv->code || half_scale != priv->half_scale)