Received: by 2002:a05:7412:8d10:b0:f3:1519:9f41 with SMTP id bj16csp4629516rdb; Tue, 12 Dec 2023 05:16:24 -0800 (PST) X-Google-Smtp-Source: AGHT+IH4BhA5ONCYFrT49G8QRoXfulTkFZWBm4oIIAhEfxvjK34K47o1jNnfwGlB36nQADyGeALz X-Received: by 2002:a05:6e02:1d8b:b0:35d:59a2:a321 with SMTP id h11-20020a056e021d8b00b0035d59a2a321mr6183800ila.35.1702386984296; Tue, 12 Dec 2023 05:16:24 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702386984; cv=none; d=google.com; s=arc-20160816; b=B+u54jh+2esFlnw/MPP8+66w+OwUboVjJMLh47r6NilG2vpwGKatdeHMnajxL7Tx6e WDvnKS76sfk9NNvH5HHkL+0xFhArRVN6vtPeLmwzEWrZL3r9HtLuvYUJ+o/j6uI4q4y8 jCXXJnvW+XRDdLJ61s4BVnvLzPATdBaQNS763bW0PISJgA7A6ro8f5p4iTLZJk5S7zr1 PnoMS7+FoSqLvF8RWfANPqPjJ8PB4Jjivs+kknZy4Y+50EQicI152/ucOtPClEeWbs9N tW4w9L/sTdIgp+mA49lgqL7DjpNd9KIzSEsSap2zvFLMQs9WS4CyWBQWJfNl1hASPv23 Rn2Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:message-id:in-reply-to:date:subject :cc:to:from:user-agent:references:dkim-signature; bh=5SExUZ1QCBZQ3tm36o/3GyTyzDGvf+wP6hTem1Psbdw=; fh=bgcIV7PYCgHdpfvO9aaV74oOVEoi1tA2B9b8L/mRCu8=; b=euipJmptu3AP4h68fTHnj9fBuXtEtPKmQ4XcZemBfYZxQ9ykXE8BwOmViVL9jNU/2W suQzowUv7Gaeo38XEDqVvYMYL1r+Kuli2JlnhzVeubANe/b8r1UzAIFLiuERNLjA0Qek vKbdYF/k//u9bOK2TdvrTy+4ICQqx7TL9c9nqbsQLL7wlbscarFz9thXVbe4Ffk/cNgX Yu6A7skts9KEtaDiawsnCp3bR/ysXRXYpyU+mx4TFovRKdWrq/F1d3jhnkef/OF18zth YGle/OGRVbRL4qpDWLf5pEfk9k2Xz/PyT/nYLrhC9H+n9Mpi6Vufi7iRMMmhd41tT2Dv FKbA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=BUZ3zaUu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from morse.vger.email (morse.vger.email. [2620:137:e000::3:1]) by mx.google.com with ESMTPS id f20-20020a631014000000b005c680fbab22si7618951pgl.509.2023.12.12.05.16.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Dec 2023 05:16:24 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=BUZ3zaUu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id D8B898021754; Tue, 12 Dec 2023 05:16:21 -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 S1376602AbjLLNP7 (ORCPT + 99 others); Tue, 12 Dec 2023 08:15:59 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57620 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1376601AbjLLNPz (ORCPT ); Tue, 12 Dec 2023 08:15:55 -0500 Received: from mail-lf1-x136.google.com (mail-lf1-x136.google.com [IPv6:2a00:1450:4864:20::136]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E7EBD106; Tue, 12 Dec 2023 05:16:00 -0800 (PST) Received: by mail-lf1-x136.google.com with SMTP id 2adb3069b0e04-50c02628291so5951822e87.0; Tue, 12 Dec 2023 05:16:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1702386959; x=1702991759; darn=vger.kernel.org; h=mime-version:message-id:in-reply-to:date:subject:cc:to:from :user-agent:references:from:to:cc:subject:date:message-id:reply-to; bh=5SExUZ1QCBZQ3tm36o/3GyTyzDGvf+wP6hTem1Psbdw=; b=BUZ3zaUuVmM07v1oTE3ic1bmo9k62PPO4GygWUJ/HfM9oDhdL2uhldlR6soEpnzJHi uJWj5FWfBbENLUxuR4hvk7t3dsmwHSF2RL69iClVXgMCeS49M5sj0FZYc0rrNQsN3AQa SOMRtfIVs1KmeMsASsnjbkFGg/t2LAGmcTInZfgs+OYNY4AXyOQKUciz1X8xkR0P58S+ TI3mAOoDk8LzOmZjlQDh5CpBKazQlM07sNCkw29wFckjqsqs5A8eueq6tsM9680SKGaZ 8nJ7MClveVo7AUL95/jfH30r/QqX+xBb/jlgZf6qD1eIbDbr7iXPyB+1gmAUIp5y4oly kUbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702386959; x=1702991759; h=mime-version:message-id:in-reply-to:date:subject:cc:to:from :user-agent:references:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=5SExUZ1QCBZQ3tm36o/3GyTyzDGvf+wP6hTem1Psbdw=; b=Wxp+/8rejg8Jkbcwfv/Xq2JWCU2hSq2/+6ZYbErVRLqgdiGCUww+adOhUmZJ1cHOT1 XPX4ss65Y5GGLeQQIRSf7U3EIRtvtJJiTKOzqA10w/jpuomdM02evId5K/pcOdmFCdUa C4YSbxAbiBGYU2gOfOgqeU0Tm6B4zNQAO3TeyCCLwOANTKPxYRrH8SAGT88HVQdJtdMB yB24AY0uKZiv2Sw6sn7TZns5WpT4uyJiBSS1MMSXFZv0MSincRJ97MvCXLxwuOH223K9 Z9+dEJYYwSn4UITcfknWE4tN+UqldlIb74A3i/59q4JX6t8wDkfE2WknKkojjHfSGb6D 8SkA== X-Gm-Message-State: AOJu0YwUOKvDrZoiRipPCyW5mp6ZV/KRnWk8MJejo2bghA1xHjesSnOT GDQCOyk91qaUHNT3PGhjfM0= X-Received: by 2002:a05:6512:523:b0:50b:fd8e:28f7 with SMTP id o3-20020a056512052300b0050bfd8e28f7mr2777732lfc.94.1702386958775; Tue, 12 Dec 2023 05:15:58 -0800 (PST) Received: from razdolb (95-24-145-153.broadband.corbina.ru. [95.24.145.153]) by smtp.gmail.com with ESMTPSA id y9-20020ac255a9000000b0050be6252444sm1340577lfg.133.2023.12.12.05.15.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Dec 2023 05:15:58 -0800 (PST) References: <20231211175023.1680247-1-mike.rudenko@gmail.com> <20231211175023.1680247-13-mike.rudenko@gmail.com> <20231211221533.GK27535@pendragon.ideasonboard.com> User-agent: mu4e 1.10.7; emacs 29.1 From: Mikhail Rudenko To: Laurent Pinchart 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 12/19] media: i2c: ov4689: Implement digital gain control Date: Tue, 12 Dec 2023 15:52:48 +0300 In-reply-to: <20231211221533.GK27535@pendragon.ideasonboard.com> Message-ID: <875y13pnn6.fsf@gmail.com> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, 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]); Tue, 12 Dec 2023 05:16:22 -0800 (PST) On 2023-12-12 at 00:15 +02, Laurent Pinchart wrote: > Hi Mikhail, > > Thank you for the patch. > > On Mon, Dec 11, 2023 at 08:50:15PM +0300, Mikhail Rudenko wrote: >> The OV4689 sensor supports digital gain up to 16x. Implement >> corresponding control in the driver. Default digital gain value is not >> modified by this patch. >> >> Signed-off-by: Mikhail Rudenko >> --- >> drivers/media/i2c/ov4689.c | 16 ++++++++++++++-- >> 1 file changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c >> index 62aeae43d749..ed0ce1b9e55b 100644 >> --- a/drivers/media/i2c/ov4689.c >> +++ b/drivers/media/i2c/ov4689.c >> @@ -35,6 +35,12 @@ >> #define OV4689_GAIN_STEP 1 >> #define OV4689_GAIN_DEFAULT 0x80 >> >> +#define OV4689_REG_DIG_GAIN CCI_REG16(0x352A) > > Lowercase for hex constatns please. Ah, missed it somehow. Is this convention kernel-wide or media specific? I think checkpatch could have detetected this.. >> +#define OV4689_DIG_GAIN_MIN 1 >> +#define OV4689_DIG_GAIN_MAX 0x7fff >> +#define OV4689_DIG_GAIN_STEP 1 >> +#define OV4689_DIG_GAIN_DEFAULT 0x800 >> + >> #define OV4689_REG_TEST_PATTERN CCI_REG8(0x5040) >> #define OV4689_TEST_PATTERN_ENABLE 0x80 >> #define OV4689_TEST_PATTERN_DISABLE 0x0 >> @@ -131,7 +137,6 @@ static const struct cci_reg_sequence ov4689_2688x1520_regs[] = { >> >> /* AEC PK */ >> {CCI_REG8(0x3503), 0x04}, /* AEC_MANUAL gain_input_as_sensor_gain_format = 1 */ >> - {CCI_REG8(0x352a), 0x08}, /* DIG_GAIN_FRAC_LONG dig_gain_long[14:8] = 0x08 (2x) */ > > Is the default value really x2 ? That's not very nice :-S > > It would be much nicer if the default value of the control mapped to x1, > otherwise it's impossible for userspace to interpret the scale of the > digital gain value in a generic way. I suppose that could break existing > applications though, which isn't great. The datasheet does not explicitly say how register values are mapped to the actual gain. 0x8 comes from the original register tables, and can also be found in a few other drivers for this sensor, although they do not implement digital gain control. OTOH, the power-on value of this register, and default value as found in the datasheet, is 0x4. This was the motivation behind that "(2x)" annotation. So, I'm afraid that we cannot interpret the absolute scale of the digital gain in any case, unless we have more documentation. I tend to keep the default value of 0x8 for the reasons of not (possibly) breaking userspace. > Out of curiosity, can you tell what SoC(s) you're using this sensor with > ? It's Rockchip 3399. I run most of my tests with AGC and AWB off, to be sure they do not hide some important details. > >> >> /* ADC and analog control*/ >> {CCI_REG8(0x3603), 0x40}, >> @@ -622,6 +627,9 @@ static int ov4689_set_ctrl(struct v4l2_ctrl *ctrl) >> OV4689_TIMING_FLIP_MASK, >> val ? 0 : OV4689_TIMING_FLIP_BOTH, &ret); >> break; >> + case V4L2_CID_DIGITAL_GAIN: >> + cci_write(regmap, OV4689_REG_DIG_GAIN, val, &ret); >> + break; >> default: >> dev_warn(dev, "%s Unhandled id:0x%x, val:0x%x\n", >> __func__, ctrl->id, val); >> @@ -650,7 +658,7 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689) >> >> handler = &ov4689->ctrl_handler; >> mode = ov4689->cur_mode; >> - ret = v4l2_ctrl_handler_init(handler, 13); >> + ret = v4l2_ctrl_handler_init(handler, 14); >> if (ret) >> return ret; >> >> @@ -693,6 +701,10 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689) >> 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); >> >> + v4l2_ctrl_new_std(handler, &ov4689_ctrl_ops, V4L2_CID_DIGITAL_GAIN, >> + OV4689_DIG_GAIN_MIN, OV4689_DIG_GAIN_MAX, >> + OV4689_DIG_GAIN_STEP, OV4689_DIG_GAIN_DEFAULT); >> + >> if (handler->error) { >> ret = handler->error; >> dev_err(ov4689->dev, "Failed to init controls(%d)\n", ret); -- Best regards, Mikhail Rudenko