From patchwork Thu Sep 25 22:17:04 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 24464 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 55C2FC32A9 for ; Thu, 25 Sep 2025 22:17:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EFCCB6B5FE; Fri, 26 Sep 2025 00:17:22 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="b5TJOizj"; dkim-atps=neutral Received: from sea.source.kernel.org (sea.source.kernel.org [IPv6:2600:3c0a:e001:78e:0:1991:8:25]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F6BF6B5AA for ; Fri, 26 Sep 2025 00:17:15 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id 5AEEC455BE; Thu, 25 Sep 2025 22:17:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2C88DC4CEF0; Thu, 25 Sep 2025 22:17:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758838634; bh=GsN3qabD8oF/CaS4DCismqZlNWjuYJsUJrbdjorqqsM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=b5TJOizjxR+t+odZfyENHJxvzsa+cvVXzIM45WnzZjJRIJPHLElzwlwiZZ2wV61kN ufUEEGpz5c6ppaRoYMAnnk//U2F3d0BlDMxXaFhtGQ2Z/bzMVpNtbhe33v0WQ++84W FH1Pu+NQKyK72+N/BnLjVHqvXrtUJIIz8WYmPinw8c97Gu6uvaVlpsl084gc9nxZ2Z ONiwZXvbV/VU/40zXqZyCVUchdt9xsLz9Qy1WaEXakMmt2x0G34cDE4mJ10bGEvY/P Yy7xQjimCTi7E+qUXf2pxU3BQKLlWalTMHWQMX3aKVDpcvdtm6R6lRPLqEoC5no+OZ S89yTSjRIN6ew== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede , Milan Zamazal , Kieran Bingham Subject: [PATCH v2 2/6] ipa: software_isp: AGC: Do not lower gain below 1.0 Date: Fri, 26 Sep 2025 00:17:04 +0200 Message-ID: <20250925221708.7471-3-hansg@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20250925221708.7471-1-hansg@kernel.org> References: <20250925221708.7471-1-hansg@kernel.org> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" At the moment when the overall image brightness is considered too high the AGC code will lower the gain all the way down to againMin before considering lowering the exposure. What should happen instead is lower the gain no lower than 1.0 and after that lower the exposure instead of lowering the gain. Otherwise there might be a heavily overexposed image (e.g. all white) which then is made less white by a gain < 1.0 which is no good. When there is no sensor-helper, assume the driver reported default-gain value is close to a gain of 1.0 . While at it also remove the weird limitation to only lower the gain when exposure is set to the maximum. As long as the gain is higher than the default gain, the gain should be lowered first. Reviewed-by: Milan Zamazal Tested-by: Milan Zamazal Tested-by: Kieran Bingham Signed-off-by: Hans de Goede Reviewed-by: Isaac Scott --- Changes in v2: - Use a gain of 1.0 as low-limit instead of always using the default-gain, falling back to the default if there is no sensor-helper for the sensor. --- src/ipa/simple/algorithms/agc.cpp | 3 +-- src/ipa/simple/ipa_context.h | 2 +- src/ipa/simple/soft_simple.cpp | 3 +++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp index c46bb0eb..1fc8d7f4 100644 --- a/src/ipa/simple/algorithms/agc.cpp +++ b/src/ipa/simple/algorithms/agc.cpp @@ -71,8 +71,7 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou } if (exposureMSV > kExposureOptimal + kExposureSatisfactory) { - if (exposure == context.configuration.agc.exposureMax && - again > context.configuration.agc.againMin) { + if (again > context.configuration.agc.again10) { next = again * kExpNumeratorDown / kExpDenominator; if (again - next < context.configuration.agc.againMinStep) again -= context.configuration.agc.againMinStep; diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h index a471b80a..468fccab 100644 --- a/src/ipa/simple/ipa_context.h +++ b/src/ipa/simple/ipa_context.h @@ -28,7 +28,7 @@ struct IPASessionConfiguration { float gamma; struct { int32_t exposureMin, exposureMax; - double againMin, againMax, againMinStep; + double againMin, againMax, again10, againMinStep; utils::Duration lineDuration; } agc; struct { diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index e70439ee..b147aca2 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -216,10 +216,12 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo) int32_t againMin = gainInfo.min().get(); int32_t againMax = gainInfo.max().get(); + int32_t againDef = gainInfo.def().get(); if (camHelper_) { context_.configuration.agc.againMin = camHelper_->gain(againMin); context_.configuration.agc.againMax = camHelper_->gain(againMax); + context_.configuration.agc.again10 = camHelper_->gain(1.0); context_.configuration.agc.againMinStep = (context_.configuration.agc.againMax - context_.configuration.agc.againMin) / @@ -246,6 +248,7 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo) * other) we limit the range of the gain values used. */ context_.configuration.agc.againMax = againMax; + context_.configuration.agc.again10 = againDef; if (againMin) { context_.configuration.agc.againMin = againMin; } else {