Received: by 2002:a05:7412:8d10:b0:f3:1519:9f41 with SMTP id bj16csp4272485rdb; Mon, 11 Dec 2023 14:13:00 -0800 (PST) X-Google-Smtp-Source: AGHT+IEjj68x3bFg2WOKUb7TtOsZeWQBszIMOEIkowwwx45suyWrQMhjLISh6t2cWagD/zD2IxXh X-Received: by 2002:a05:6358:939b:b0:170:56ee:2892 with SMTP id h27-20020a056358939b00b0017056ee2892mr6265452rwb.7.1702332779768; Mon, 11 Dec 2023 14:12:59 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702332779; cv=none; d=google.com; s=arc-20160816; b=awV5ZszDU0IlcqO0UNVn663+THZd6l6Ok+HhmnUc7TUJx8qZ6sfpFQpImlzADHwbDP 5od7cv03KpjyjBjTed3wDLaqI4uNt5jOoK7xVAorQk0Y+iRX90SXJdaqwl0prj3RBlby tbsGFXM917Wqn+t9TCLaQ7A4fjDeYDOGr6snsPDgrdGYZF5B4oQeOileZb++0fU6kXId YzsaPO076+6tj2hOjoi1KUyG5CnO3PQwwILlbJfhjg/JuloLTx58SULNQDeCoo40ULOu BvB7MI8PCdqYQ6/NceHLohVgruTb5T5USIwhUx5DTOzf5fBRkJC7XuUVhVj+zCxJlEo8 qEoA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=PbCy0PNY+0M0JjddXZ3oeyvqjqVv9HHqkfKuRk744/Q=; fh=47XaZSaQjHB/8jIPgKLdqhzRb437ezGzdFwdhCFigTs=; b=Bwqfk0D50tv4TKNy5U3k0CLs41ZWf48fdNXnQow7GOiB2ZnttQMwdsfyQev4gZFtQk aNGGkfvTY5ezpzsAgFld826PoqoclcOdWn5T1sMA5IzztTivBXkDq6W2xvbBcXe4d2P5 9eMYP0VOU6bckPhzxJL4Dv7x3UI+fJg17wJMRhcTCcf73abU/jQTG4JKncbSH1hhNTZC RxMgQiRiSUss7cFKgNrPGZrZ0eqaWl4AXc6q5pCHMZGxfy2yjtS6cfam/lZqYqW8kIQn O3Tm/BsnviJGrNbZPuoz3H9b4+WY6v7wZazRxSWLY1AU+mzYigSlwG6rbfkbO3av6qwa q9yQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass (test mode) header.i=@ideasonboard.com header.s=mail header.b=cHIjNOp3; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id h12-20020a056a00230c00b006cb88a284dfsi6664410pfh.164.2023.12.11.14.12.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 Dec 2023 14:12:59 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass (test mode) header.i=@ideasonboard.com header.s=mail header.b=cHIjNOp3; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 9BBF780A9193; Mon, 11 Dec 2023 14:12:58 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344946AbjLKWMk (ORCPT + 99 others); Mon, 11 Dec 2023 17:12:40 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40614 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345736AbjLKWKr (ORCPT ); Mon, 11 Dec 2023 17:10:47 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 26A17E3; Mon, 11 Dec 2023 14:10:54 -0800 (PST) Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 53AAA922; Mon, 11 Dec 2023 23:10:08 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1702332608; bh=VrTyIgpUM/k+qU2dG83GSARqhJhB72+1d672e8YuOxE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=cHIjNOp3A6ptDnnGiSAxFtLA2NzdWvnTer1TEtKORcEau2KGFZ1gx2Kuq+fHA/i9G V0xHHbM9aL08OXw5b4HQPSRAVcQeR1UvWdQz0TaD0qt6wW3uo0ZjEJlQU3AvXj/jD+ OOo/8sYxjHMSRdto7bdC+WLfSavGhSIh/QLWdQdE= Date: Tue, 12 Dec 2023 00:10:59 +0200 From: Laurent Pinchart To: Mikhail Rudenko Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, Sakari Ailus , Jacopo Mondi , Christophe JAILLET , Dave Stevenson , Mauro Carvalho Chehab Subject: Re: [PATCH 11/19] media: i2c: ov4689: Implement vflip/hflip controls Message-ID: <20231211221059.GJ27535@pendragon.ideasonboard.com> References: <20231211175023.1680247-1-mike.rudenko@gmail.com> <20231211175023.1680247-12-mike.rudenko@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20231211175023.1680247-12-mike.rudenko@gmail.com> X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_PASS,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 X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Mon, 11 Dec 2023 14:12:58 -0800 (PST) Hi Mikhail, Thank you for the patch. On Mon, Dec 11, 2023 at 08:50:14PM +0300, Mikhail Rudenko wrote: > The OV4689 sensor supports horizontal and vertical flipping. Add > appropriate controls to the driver. Toggling both array flip and > digital flip bits allows to achieve flipping while maintaining output > Bayer order. Note that the default value of hflip control corresponds > to both bits set, as it was before this patch. What happens if only hlip or vflip is set, does the bayer pattern change ? Sakari, I think this patch could use your attention. > Signed-off-by: Mikhail Rudenko > --- > drivers/media/i2c/ov4689.c | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c > index 67d4004bdcfb..62aeae43d749 100644 > --- a/drivers/media/i2c/ov4689.c > +++ b/drivers/media/i2c/ov4689.c > @@ -46,6 +46,14 @@ > #define OV4689_HTS_DIVIDER 4 > #define OV4689_HTS_MAX 0x7fff > > +#define OV4689_REG_TIMING_FORMAT1 CCI_REG8(0x3820) > +#define OV4689_REG_TIMING_FORMAT2 CCI_REG8(0x3821) > +#define OV4689_TIMING_FLIP_MASK GENMASK(2, 1) > +#define OV4689_TIMING_FLIP_ARRAY BIT(1) > +#define OV4689_TIMING_FLIP_DIGITAL BIT(2) > +#define OV4689_TIMING_FLIP_BOTH (OV4689_TIMING_FLIP_ARRAY |\ > + OV4689_TIMING_FLIP_DIGITAL) > + > #define OV4689_LANES 4 > #define OV4689_XVCLK_FREQ 24000000 > > @@ -183,7 +191,6 @@ static const struct cci_reg_sequence ov4689_2688x1520_regs[] = { > {CCI_REG8(0x3811), 0x08}, /* H_WIN_OFF_L h_win_off[7:0] = 0x08*/ > {CCI_REG8(0x3813), 0x04}, /* V_WIN_OFF_L v_win_off[7:0] = 0x04 */ > {CCI_REG8(0x3819), 0x01}, /* VSYNC_END_L vsync_end_point[7:0] = 0x01 */ > - {CCI_REG8(0x3821), 0x06}, /* TIMING_FORMAT2 array_h_mirror = 1, digital_h_mirror = 1 */ > > /* OTP control */ > {CCI_REG8(0x3d85), 0x36}, /* OTP_REG85 OTP_power_up_load_setting_enable = 1, > @@ -605,6 +612,16 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl) > (val + ov4689->cur_mode->width) / > OV4689_HTS_DIVIDER, &ret); > break; > + case V4L2_CID_VFLIP: > + cci_update_bits(regmap, OV4689_REG_TIMING_FORMAT1, > + OV4689_TIMING_FLIP_MASK, > + val ? OV4689_TIMING_FLIP_BOTH : 0, &ret); > + break; > + case V4L2_CID_HFLIP: > + cci_update_bits(regmap, OV4689_REG_TIMING_FORMAT2, > + OV4689_TIMING_FLIP_MASK, > + val ? 0 : OV4689_TIMING_FLIP_BOTH, &ret); > + break; > default: > dev_warn(dev, "%s Unhandled id:0x%x, val:0x%x\n", > __func__, ctrl->id, val); > @@ -633,7 +650,7 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689) > > handler = &ov4689->ctrl_handler; > mode = ov4689->cur_mode; > - ret = v4l2_ctrl_handler_init(handler, 11); > + ret = v4l2_ctrl_handler_init(handler, 13); This should be 12 if my comment on 10/19 is correct. Further patches in the series may need to be adjusted too. > if (ret) > return ret; > > @@ -673,6 +690,9 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689) > ARRAY_SIZE(ov4689_test_pattern_menu) - 1, > 0, 0, ov4689_test_pattern_menu); > > + v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); > + v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_HFLIP, 0, 1, 1, 0); > + > if (handler->error) { > ret = handler->error; > dev_err(ov4689->dev, "Failed to init controls(%d)\n", ret); -- Regards, Laurent Pinchart