ipa: rkisp1: dpf: Enable strength after enable Dpf
diff mbox series

Message ID 20251024120107.681968-1-rui.wang@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: rkisp1: dpf: Enable strength after enable Dpf
Related show

Commit Message

Rui Wang Oct. 24, 2025, 12:01 p.m. UTC
The filter strength configuration block was previously only enabled on the
first frame (frame == 0). Apply the strength values when denoise is active.
This prevents the strength config from being disabled on subsequent frames.

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

Comments

Kieran Bingham Oct. 27, 2025, 1:59 p.m. UTC | #1
Quoting Rui Wang (2025-10-24 13:01:07)
> The filter strength configuration block was previously only enabled on the
> first frame (frame == 0). Apply the strength values when denoise is active.
> This prevents the strength config from being disabled on subsequent frames.
> 
> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/ipa/rkisp1/algorithms/dpf.cpp | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> index cb6095da..e95fb95d 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> @@ -251,9 +251,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
>                         mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;
>         }
>  
> -       if (frame == 0) {
> -               auto strengthConfig = params->block<BlockType::DpfStrength>();
> -               strengthConfig.setEnabled(true);
> +       auto strengthConfig = params->block<BlockType::DpfStrength>();
> +       strengthConfig.setEnabled(frameContext.dpf.denoise);
> +
> +       if (frameContext.dpf.denoise) {
>                 *strengthConfig = strengthConfig_;
>         }
>  }
> -- 
> 2.43.0
>
Barnabás Pőcze Oct. 27, 2025, 2:06 p.m. UTC | #2
Hi

2025. 10. 27. 14:59 keltezéssel, Kieran Bingham írta:
> Quoting Rui Wang (2025-10-24 13:01:07)
>> The filter strength configuration block was previously only enabled on the
>> first frame (frame == 0). Apply the strength values when denoise is active.
>> This prevents the strength config from being disabled on subsequent frames.
>>
>> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
>> ---
>>   src/ipa/rkisp1/algorithms/dpf.cpp | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
>> index cb6095da..e95fb95d 100644
>> --- a/src/ipa/rkisp1/algorithms/dpf.cpp
>> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
>> @@ -251,9 +251,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
>>                          mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;
>>          }
>>   
>> -       if (frame == 0) {
>> -               auto strengthConfig = params->block<BlockType::DpfStrength>();
>> -               strengthConfig.setEnabled(true);
>> +       auto strengthConfig = params->block<BlockType::DpfStrength>();
>> +       strengthConfig.setEnabled(frameContext.dpf.denoise);
>> +
>> +       if (frameContext.dpf.denoise) {
>>                  *strengthConfig = strengthConfig_;

This can be moved into the `if` block above.


But I must be missing something. As far as I understand the idea here is
to apply this on the first frame because it comes from the tuning file
and does not change (and the hardware keeps the state once set at least
until the end of the capture session). But then why is it not enough to
set it on the first frame? It will be reset if when `BlockType::Dpf` is
disabled or something similar? Or is my assumption that the hardware
remembers the last set value incorrect?


Regards,
Barnabás Pőcze


>>          }
>>   }
>> -- 
>> 2.43.0
>>
Rui Wang Nov. 10, 2025, 7:38 p.m. UTC | #3
On 2025-10-27 10:06, Barnabás Pőcze wrote:
> Hi
>
> 2025. 10. 27. 14:59 keltezéssel, Kieran Bingham írta:
>> Quoting Rui Wang (2025-10-24 13:01:07)
>>> The filter strength configuration block was previously only enabled 
>>> on the
>>> first frame (frame == 0). Apply the strength values when denoise is 
>>> active.
>>> This prevents the strength config from being disabled on subsequent 
>>> frames.
>>>
>>> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
>>
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>
>>> ---
>>>   src/ipa/rkisp1/algorithms/dpf.cpp | 7 ++++---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp 
>>> b/src/ipa/rkisp1/algorithms/dpf.cpp
>>> index cb6095da..e95fb95d 100644
>>> --- a/src/ipa/rkisp1/algorithms/dpf.cpp
>>> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
>>> @@ -251,9 +251,10 @@ void Dpf::prepare(IPAContext &context, const 
>>> uint32_t frame,
>>>                          mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;
>>>          }
>>>   -       if (frame == 0) {
>>> -               auto strengthConfig = 
>>> params->block<BlockType::DpfStrength>();
>>> -               strengthConfig.setEnabled(true);
>>> +       auto strengthConfig = params->block<BlockType::DpfStrength>();
>>> +       strengthConfig.setEnabled(frameContext.dpf.denoise);
>>> +
>>> +       if (frameContext.dpf.denoise) {
>>>                  *strengthConfig = strengthConfig_;
>
> This can be moved into the `if` block above.
>
>
> But I must be missing something. As far as I understand the idea here is
> to apply this on the first frame because it comes from the tuning file
> and does not change (and the hardware keeps the state once set at least
> until the end of the capture session). But then why is it not enough to
> set it on the first frame? It will be reset if when `BlockType::Dpf` is
> disabled or something similar? Or is my assumption that the hardware
> remembers the last set value incorrect?
>
yes , move this  *strengthConfig = strengthConfig_; into above 'if' 
block is better. will push a new patch

  this update triggered by "NoiseReduction*"  control from camshark .  
  which handle by enqeueRuest and then

update both frameContext.dpf.update and frameContext.dpf.denoise

  then prepare() will handle the isp hardware configuration applicant.

either camshark's NoiseReduction control modification would trigger the 
isp denoise block update .

and the spcific configuration of DPF , which read out from yaml file.
  To verify the code working : I have to add dpf configration into 
tuning file,

- Dpf:
DomainFilter:
g: [ 16, 16, 16, 16, 16, 16]
rb: [ 16, 16, 16, 16, 16, 16]
NoiseLevelFunction:
coeff: [
1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023,
1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023,
1023
]
scale-mode: "linear"
FilterStrength:
r: 1
g: 1
b: 1




>
> Regards,
> Barnabás Pőcze
>
>
>>>          }
>>>   }
>>> -- 
>>> 2.43.0
>>>
>

Patch
diff mbox series

diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
index cb6095da..e95fb95d 100644
--- a/src/ipa/rkisp1/algorithms/dpf.cpp
+++ b/src/ipa/rkisp1/algorithms/dpf.cpp
@@ -251,9 +251,10 @@  void Dpf::prepare(IPAContext &context, const uint32_t frame,
 			mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;
 	}
 
-	if (frame == 0) {
-		auto strengthConfig = params->block<BlockType::DpfStrength>();
-		strengthConfig.setEnabled(true);
+	auto strengthConfig = params->block<BlockType::DpfStrength>();
+	strengthConfig.setEnabled(frameContext.dpf.denoise);
+
+	if (frameContext.dpf.denoise) {
 		*strengthConfig = strengthConfig_;
 	}
 }