[[PATCH,v1] ] ipa: rkisp1: filter: Fix sharpness enable/disable logic
diff mbox series

Message ID 20251123202816.3460261-1-rui.wang@ideasonboard.com
State New
Headers show
Series
  • [[PATCH,v1] ] ipa: rkisp1: filter: Fix sharpness enable/disable logic
Related show

Commit Message

Rui Wang Nov. 23, 2025, 8:28 p.m. UTC
The sharpness enable bit in the filter mode register was not being
controlled correctly based on the sharpness level. Add explicit code
to enable sharpness when sharpness > 1, and disable it otherwise.
This ensures the hardware register is set properly to activate
sharpness as intended.

Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
---
 src/ipa/rkisp1/algorithms/filter.cpp | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Kieran Bingham Nov. 24, 2025, 11 a.m. UTC | #1
Quoting Rui Wang (2025-11-23 20:28:16)
> The sharpness enable bit in the filter mode register was not being
> controlled correctly based on the sharpness level. Add explicit code
> to enable sharpness when sharpness > 1, and disable it otherwise.
> This ensures the hardware register is set properly to activate
> sharpness as intended.
> 
> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/filter.cpp | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp
> index 8ad79801..017618e4 100644
> --- a/src/ipa/rkisp1/algorithms/filter.cpp
> +++ b/src/ipa/rkisp1/algorithms/filter.cpp
> @@ -216,6 +216,11 @@ void Filter::prepare([[maybe_unused]] IPAContext &context,
>                         config->fac_bl1 /= 2;
>                 }
>         }
> +       /* Set bit 0 of the config->mode to toggle on/off sharpness

	/*
	 * block comments start with an open line
	 * like this.
	 */
	

> +        * if sharpness > 1, enable sharpness,  viseverse disable it

What' was viseverse meant to mean? I assume there's a typo - what it
'otherwise'?

> +        */
> +       config->mode &= ~(1u << 0);
> +       config->mode |= static_cast<uint32_t>(sharpness > 1) << 0;

Oh, I see we're passing this mode bit through to the kernel patch. That
likely needs some more clarifiaction, and the bit will need to be
documented.

Have you 'created' this bit ? or is it already exposed / expected in the
kernel and it's just that it wasn't being checked?

--
Regards

Kieran

>  }
>  
>  REGISTER_IPA_ALGORITHM(Filter, "Filter")
> -- 
> 2.43.0
>

Patch
diff mbox series

diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp
index 8ad79801..017618e4 100644
--- a/src/ipa/rkisp1/algorithms/filter.cpp
+++ b/src/ipa/rkisp1/algorithms/filter.cpp
@@ -216,6 +216,11 @@  void Filter::prepare([[maybe_unused]] IPAContext &context,
 			config->fac_bl1 /= 2;
 		}
 	}
+	/* Set bit 0 of the config->mode to toggle on/off sharpness
+	 * if sharpness > 1, enable sharpness,  viseverse disable it
+	 */
+	config->mode &= ~(1u << 0);
+	config->mode |= static_cast<uint32_t>(sharpness > 1) << 0;
 }
 
 REGISTER_IPA_ALGORITHM(Filter, "Filter")