[1/5] ipa: software_isp: Fix context_.configuration.agc.againMin init
diff mbox series

Message ID 20250923190657.21453-2-hansg@kernel.org
State Superseded
Headers show
Series
  • ipa: software_isp: AGC: Fox AGC oscillation bug
Related show

Commit Message

Hans de Goede Sept. 23, 2025, 7:06 p.m. UTC 
Currently context_.configuration.agc.againMin is not initialized
when the control reports a non 0 minumum gain value.

So far only the againMin == 0 case was handled and
context_.configuration.agc.againMin was left unset otherwise.

Signed-off-by: Hans de Goede <hansg@kernel.org>
---
 src/ipa/simple/soft_simple.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Milan Zamazal Sept. 24, 2025, 7:36 a.m. UTC | #1 
Hi Hans,

thank you for the patch.

Hans de Goede <hansg@kernel.org> writes:

> Currently context_.configuration.agc.againMin is not initialized
> when the control reports a non 0 minumum gain value.
>
> So far only the againMin == 0 case was handled and
> context_.configuration.agc.againMin was left unset otherwise.
>
> Signed-off-by: Hans de Goede <hansg@kernel.org>

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>

> ---
>  src/ipa/simple/soft_simple.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index c94c4cd55..e70439ee5 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -246,7 +246,9 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)
>  		 * other) we limit the range of the gain values used.
>  		 */
>  		context_.configuration.agc.againMax = againMax;
> -		if (!againMin) {
> +		if (againMin) {
> +			context_.configuration.agc.againMin = againMin;
> +		} else {
>  			LOG(IPASoft, Warning)
>  				<< "Minimum gain is zero, that can't be linear";
>  			context_.configuration.agc.againMin =
Isaac Scott Sept. 25, 2025, 9:48 a.m. UTC | #2 
Hi Hans,

Thank you for the patch!

Quoting Hans de Goede (2025-09-23 20:06:53)
> Currently context_.configuration.agc.againMin is not initialized
> when the control reports a non 0 minumum gain value.
> 
> So far only the againMin == 0 case was handled and
> context_.configuration.agc.againMin was left unset otherwise.
> 
> Signed-off-by: Hans de Goede <hansg@kernel.org>
> ---
>  src/ipa/simple/soft_simple.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index c94c4cd55..e70439ee5 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -246,7 +246,9 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)
>                  * other) we limit the range of the gain values used.
>                  */
>                 context_.configuration.agc.againMax = againMax;
> -               if (!againMin) {
> +               if (againMin) {
> +                       context_.configuration.agc.againMin = againMin;
> +               } else {

Nice!

Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>

>                         LOG(IPASoft, Warning)
>                                 << "Minimum gain is zero, that can't be linear";
>                         context_.configuration.agc.againMin =
> -- 
> 2.51.0
> 

Best wishes,
Isaac

Patch
diff mbox series 

diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
index c94c4cd55..e70439ee5 100644
--- a/src/ipa/simple/soft_simple.cpp
+++ b/src/ipa/simple/soft_simple.cpp
@@ -246,7 +246,9 @@  int IPASoftSimple::configure(const IPAConfigInfo &configInfo)
 		 * other) we limit the range of the gain values used.
 		 */
 		context_.configuration.agc.againMax = againMax;
-		if (!againMin) {
+		if (againMin) {
+			context_.configuration.agc.againMin = againMin;
+		} else {
 			LOG(IPASoft, Warning)
 				<< "Minimum gain is zero, that can't be linear";
 			context_.configuration.agc.againMin =