Received: by 2002:a05:6a10:2726:0:0:0:0 with SMTP id ib38csp704587pxb; Tue, 5 Apr 2022 19:32:57 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxRFzSW7BSDFGOOTk3qy+xolNm2238E01CIWOJkYTi91mTxArXv8il3fyQ82yU9gDKPGHkl X-Received: by 2002:a17:902:8684:b0:154:af35:82ce with SMTP id g4-20020a170902868400b00154af3582cemr6458249plo.137.1649212376845; Tue, 05 Apr 2022 19:32:56 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1649212376; cv=none; d=google.com; s=arc-20160816; b=G9es3pTe1wUIeUbi+BkQLZGNAKwbLfQQFG4IJlDuSzqxzY0WZgBLtiA0H+U/b1IAdY Soa5XjQAKTUgwe9WUqN1wllLXtFno7lyHbbpMquOPsvuKwaiwdsSVoUfq2qmoubIxnEj Sol9cBL3eXZ4B0YvL2q7EvxC63ProYihN9VNLQPxNTRA4/5wrc+LZvoGFdH/WdxxJq3k aTXK+aUoLLo7+fa1O1UX6FuE+lva7BpXTIYD0I7nIzU94IOgdWxj8G4AD+kZWjHxViEH wmFkxHqF+zvXYQ76IfkkrliupMSIKVxQuVC+mhJ2QlGnlAolgg4LRTJXxK0vnwkWlsg9 D/4g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=v0tnb8KKtKRKcnuXE4x3NpXX79SFRqDyjUQ9iApAwRY=; b=kZ9ZM/4qMAW8Pw1VW+1RBAl+Dh2V2P64QWA+cMLBE8OSv08K/d+0zAyuuqfUij8AtJ cFSGjt6ms8qPFHqTuf2WjIhLv6zaTCWViGPkeHVSY2JOeO24ZAX7V2Ns+PzhcCIcEUhe AIbJ4E9bwImF+A8v4UltMxho5KOCD1GtxzMS2oeqJXVNSkZop3Teia3H9k0X3qJbphih 1MfiDkr7V4gYVvDIB9+LLzZpYkYivKiMjTL+OWRKnBLikcmHhc6xQvlTYBKj5cp0cqCU LW5AAE2vbksBrN6skX0CXDvV2rBXPSzEKHKa0oNr03DpkbV8uIDG66GnHnmFMOt549f6 aQMg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=1hSuKGf8; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id u15-20020a63140f000000b003816043f15bsi6763630pgl.848.2022.04.05.19.32.42; Tue, 05 Apr 2022 19:32:56 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=1hSuKGf8; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244983AbiDELoy (ORCPT + 99 others); Tue, 5 Apr 2022 07:44:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46852 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244843AbiDEIwm (ORCPT ); Tue, 5 Apr 2022 04:52:42 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9066722B10; Tue, 5 Apr 2022 01:44:40 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 2EBF2609D0; Tue, 5 Apr 2022 08:44:40 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 37963C385A0; Tue, 5 Apr 2022 08:44:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1649148279; bh=leSf3GxWHfIMm9Uxk5RMI2sOS+9VSqtrVZCVADOtH/U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=1hSuKGf8N6LYkhCRP18bl2ZArJP7O50jJjDKMgzqXhwxtKsKnVvZRupqa2n4inGvE 6A4kzjtKFIbdxRt5u5s1cvLWuJ/K+R50/W9+9+OMlrYDGYBop5Rix5Lbd7bytxUi2Q whPmV63EAJzz0un8SjUFcEKcj16Z4lHd8tIpt/ec= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Janusz Krzysztofik , Sakari Ailus , Mauro Carvalho Chehab , Sasha Levin Subject: [PATCH 5.16 0298/1017] media: ov6650: Fix set format try processing path Date: Tue, 5 Apr 2022 09:20:11 +0200 Message-Id: <20220405070403.120895208@linuxfoundation.org> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220405070354.155796697@linuxfoundation.org> References: <20220405070354.155796697@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,PDS_OTHER_BAD_TLD, RCVD_IN_DNSWL_HI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Janusz Krzysztofik [ Upstream commit 1f6f1e959a85ee999fbc86f4b094827f63194c7f ] 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. [Sakari Ailus: Rebase on subdev state patches] Fixes: 717fd5b4907a ("[media] v4l2: replace try_mbus_fmt by set_fmt") Signed-off-by: Janusz Krzysztofik Signed-off-by: Sakari Ailus Signed-off-by: Mauro Carvalho Chehab Signed-off-by: Sasha Levin --- 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 f67412150b16..8b7540e80685 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_state *sd_state, 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 */ - sd_state->pads->try_fmt.width = mf->width; - sd_state->pads->try_fmt.height = mf->height; + /* store new crop rectangle, hadware bound, in pad config */ + ov6650_bind_align_crop_rectangle(crop); + sd_state->pads->try_crop = *crop; + + /* store new mbus frame format code and size in pad config */ + sd_state->pads->try_fmt.width = crop->width >> half_scale; + sd_state->pads->try_fmt.height = crop->height >> half_scale; sd_state->pads->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 = sd_state->pads->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); -- 2.34.1