Received: by 2002:a05:7412:8d10:b0:f3:1519:9f41 with SMTP id bj16csp4270427rdb; Mon, 11 Dec 2023 14:08:22 -0800 (PST) X-Google-Smtp-Source: AGHT+IGDHlOUoKS68GIHXsLbL1+6AqMGRzafJvjja+OpS/WA8jn9rEsIanVp2uqktDOfpm37agqE X-Received: by 2002:a05:6a00:b51:b0:6ce:3f02:108c with SMTP id p17-20020a056a000b5100b006ce3f02108cmr2545193pfo.22.1702332501796; Mon, 11 Dec 2023 14:08:21 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702332501; cv=none; d=google.com; s=arc-20160816; b=LmJxT85oEbngU4ko/wL4o6CA1KEgoBbwR4d6h6eWJC7ab89EPAUpg+e/NfYzRWADJQ t6Hob3HGSFm55GxSgmLsWm21RQNglFYV/iIwd/FM2UpVxPV4iTbHKOTluejoKvFxXHzx NyqeB80qTADMKmyOw7Fhc9O6LQ7CJ0yZLNY8endkDvG4bdhQIFH8VhLwbxOqvKkyYT2O 6xd9SulR+3HFsV6iDaH5rtHOIww5ioYfnZFUna5ebOtu++BTXuBVFZXX9ZeVOYFp/3rA SG6dnZfwqa28oNf1VoUs+L6pEyZEbQlYTV9wE/Ka7mVWw3LufO/suKP9KwpaLr7k81O9 e59A== 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=whg6HHyAfUDvwgHXbefv2Mv1pAP1o3urDG6E8ZsW5cE=; fh=jqTWrQ5bCH5SYcBidMUA87AoUf4V55L1bAZhntpQF1g=; b=WXBkH0VzFO+47zxC8U1gvCCIZn6rf4cRSVEMijKQmuLWWu/JwpnbMK04ySK3jHUQGC th07FQ4ASFRPFqxDhJedlm29sM1sl/eHwk1dPkso0XnDXkzE1wk5tt3w69JcLr2EkWot tCRi3gNPBt7p5SWNMPWYTeMqqdJ//KCgCuTLfrEAzkJ/sPDxAz2wB2mLKX8PqXBq68RB pBqHjw8bgySNRtE3Ow2gkgdT5QzUh1CBmWTr+ekUZ7mMWhK+/YLukiAZ4hPt9JxTchP9 fbc09CEQL2DgQ9zh5PSfHMSYdq+VQfhnSDzSU7X/3n9s1mdxFTASEQEes3MwbpEyMXfx vJ8w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass (test mode) header.i=@ideasonboard.com header.s=mail header.b=Ixj4kEoN; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from morse.vger.email (morse.vger.email. [23.128.96.31]) by mx.google.com with ESMTPS id o21-20020a634e55000000b005c693ea6618si6750498pgl.523.2023.12.11.14.08.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 Dec 2023 14:08:21 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31; Authentication-Results: mx.google.com; dkim=pass (test mode) header.i=@ideasonboard.com header.s=mail header.b=Ixj4kEoN; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 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 morse.vger.email (Postfix) with ESMTP id 54029802D7BC; Mon, 11 Dec 2023 14:08:19 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345531AbjLKWIE (ORCPT + 99 others); Mon, 11 Dec 2023 17:08:04 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60990 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345514AbjLKWID (ORCPT ); Mon, 11 Dec 2023 17:08:03 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5025CA6; Mon, 11 Dec 2023 14:08:09 -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 F09DC922; Mon, 11 Dec 2023 23:07:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1702332443; bh=OGGjmvt7t8Zrpu5flQgJGLONHAQUCWanMmv4maIIF5M=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Ixj4kEoN5o83f3UJxVYoNiHdbykFsYDkJQDS/pUcDGEXNoIZFvyDxmPl+69ah0TIh fncxNU3ji9A6OWbajw8AVous8AXF53V/QoKkhREulg7GS0pUidsHnyqSyYvka1l31f NiscOKlLK01jb8roY/G2lhF8+zC68Pa3zhMLByUg= Date: Tue, 12 Dec 2023 00:08:13 +0200 From: Laurent Pinchart To: Mikhail Rudenko Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, Sakari Ailus , Jacopo Mondi , Tommaso Merciai , Christophe JAILLET , Dave Stevenson , Mauro Carvalho Chehab Subject: Re: [PATCH 10/19] media: i2c: ov4689: Make horizontal blanking configurable Message-ID: <20231211220813.GI27535@pendragon.ideasonboard.com> References: <20231211175023.1680247-1-mike.rudenko@gmail.com> <20231211175023.1680247-11-mike.rudenko@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20231211175023.1680247-11-mike.rudenko@gmail.com> X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email 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 (morse.vger.email [0.0.0.0]); Mon, 11 Dec 2023 14:08:19 -0800 (PST) Hi Mikhail, Thank you for the patch. On Mon, Dec 11, 2023 at 08:50:13PM +0300, Mikhail Rudenko wrote: > Make horizontal blanking configurable. To do so, set HTS register > according to the requested horizontal blanking in ov4689_set_ctrl > instead of the register table. Default HTS value is not changed by > this patch. Minimal HTS value is found experimentally and corresponds > to 90 fps framerate at minimum vertical blanking. Real HTS value is > the register value multiplied by 4. > > Signed-off-by: Mikhail Rudenko > --- > drivers/media/i2c/ov4689.c | 33 +++++++++++++++++++++------------ > 1 file changed, 21 insertions(+), 12 deletions(-) > > diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c > index 9fa06941a0e5..67d4004bdcfb 100644 > --- a/drivers/media/i2c/ov4689.c > +++ b/drivers/media/i2c/ov4689.c > @@ -30,7 +30,6 @@ > #define OV4689_REG_EXPOSURE CCI_REG24(0x3500) > #define OV4689_EXPOSURE_MIN 4 > #define OV4689_EXPOSURE_STEP 1 > -#define OV4689_VTS_MAX 0x7fff > > #define OV4689_REG_GAIN CCI_REG16(0x3508) > #define OV4689_GAIN_STEP 1 > @@ -41,6 +40,11 @@ > #define OV4689_TEST_PATTERN_DISABLE 0x0 > > #define OV4689_REG_VTS CCI_REG16(0x380e) > +#define OV4689_VTS_MAX 0x7fff > + > +#define OV4689_REG_HTS CCI_REG16(0x380c) > +#define OV4689_HTS_DIVIDER 4 > +#define OV4689_HTS_MAX 0x7fff Could you move this just before REG_VTS to keep registers sorted by address ? > > #define OV4689_LANES 4 > #define OV4689_XVCLK_FREQ 24000000 > @@ -61,6 +65,7 @@ struct ov4689_mode { > u32 width; > u32 height; > u32 hts_def; > + u32 hts_min; > u32 vts_def; > u32 exp_def; > u32 pixel_rate; > @@ -104,7 +109,7 @@ struct ov4689_gain_range { > > /* > * Xclk 24Mhz > - * max_framerate 30fps > + * max_framerate 90fps > * mipi_datarate per lane 1008Mbps > */ > static const struct cci_reg_sequence ov4689_2688x1520_regs[] = { > @@ -175,8 +180,6 @@ static const struct cci_reg_sequence ov4689_2688x1520_regs[] = { > /* Timing control */ > {CCI_REG8(0x3801), 0x08}, /* H_CROP_START_L h_crop_start[7:0] = 0x08 */ > {CCI_REG8(0x3805), 0x97}, /* H_CROP_END_L h_crop_end[7:0] = 0x97 */ > - {CCI_REG8(0x380c), 0x0a}, /* TIMING_HTS_H hts[14:8] = 0x0a */ > - {CCI_REG8(0x380d), 0x0e}, /* TIMING_HTS_L hts[7:0] = 0x0e */ > {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 */ > @@ -237,7 +240,8 @@ static const struct ov4689_mode supported_modes[] = { > .crop_top = 8, > .crop_left = 16, > .exp_def = 1536, > - .hts_def = 4 * 2574, > + .hts_def = 10296, > + .hts_min = 3432, > .vts_def = 1554, > .pixel_rate = 480000000, > .reg_list = ov4689_2688x1520_regs, > @@ -596,6 +600,11 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl) > case V4L2_CID_TEST_PATTERN: > ret = ov4689_enable_test_pattern(ov4689, val); > break; > + case V4L2_CID_HBLANK: > + cci_write(regmap, OV4689_REG_HTS, > + (val + ov4689->cur_mode->width) / > + OV4689_HTS_DIVIDER, &ret); > + break; > default: > dev_warn(dev, "%s Unhandled id:0x%x, val:0x%x\n", > __func__, ctrl->id, val); > @@ -618,13 +627,13 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689) > struct v4l2_ctrl_handler *handler; > const struct ov4689_mode *mode; > s64 exposure_max, vblank_def; > + s64 hblank_def, hblank_min; > struct v4l2_ctrl *ctrl; > - s64 h_blank_def; > int ret; > > handler = &ov4689->ctrl_handler; > mode = ov4689->cur_mode; > - ret = v4l2_ctrl_handler_init(handler, 10); > + ret = v4l2_ctrl_handler_init(handler, 11); The HBLANK control already exists, you're only changing how it is initialized. I see 8 controls being created by the driver directly, plus 2 created by v4l2_ctrl_new_fwnode_properties(), so I think 10 is a correct value here. With these small issues addressed, Reviewed-by: Laurent Pinchart > if (ret) > return ret; > > @@ -636,11 +645,11 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689) > v4l2_ctrl_new_std(handler, NULL, V4L2_CID_PIXEL_RATE, 0, > mode->pixel_rate, 1, mode->pixel_rate); > > - h_blank_def = mode->hts_def - mode->width; > - ctrl = v4l2_ctrl_new_std(handler, NULL, V4L2_CID_HBLANK, h_blank_def, > - h_blank_def, 1, h_blank_def); > - if (ctrl) > - ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; > + hblank_def = mode->hts_def - mode->width; > + hblank_min = mode->hts_min - mode->width; > + v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_HBLANK, > + hblank_min, OV4689_HTS_MAX - mode->width, > + OV4689_HTS_DIVIDER, hblank_def); > > vblank_def = mode->vts_def - mode->height; > v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_VBLANK, -- Regards, Laurent Pinchart