[PATCH V2] soc: qcom: icc-bwmon: Update zone1_thres_count to 3

Pushpendra Singh posted 1 patch 5 months ago
drivers/soc/qcom/icc-bwmon.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH V2] soc: qcom: icc-bwmon: Update zone1_thres_count to 3
Posted by Pushpendra Singh 5 months ago
From: Shivnandan Kumar <quic_kshivnan@quicinc.com>

Update zone1_thres_count to 3 from 16 so that
driver can reduce bus vote in 3 sample windows instead
of waiting for 16 windows.

The 16-window (64 ms) waiting time is too long to reduce the
bus vote. At higher FPS, there will be multiple frames in 64ms
e.g. 4 frames at 60FPS in 64ms. Hence, delay of 64ms in decision
making will lead to higher power regression. We tested across
multiple usecases, and observed significant power savings.

USECASE				zone1_thres_count=16     zone1_thres_count=3
4K video playback       236.15 mA                  203.15 mA
Sleep					   7mA			   			6.9mA
Display (dle display)    71.95mA			       67.11mA

Signed-off-by: Shivnandan Kumar <quic_kshivnan@quicinc.com>
Signed-off-by: Pushpendra Singh <quic_pussin@quicinc.com>
---
Changes in v2:
-Update commit message
-Link to v1:https://lore.kernel.org/lkml/463eb7c8-00fc-4441-91d1-6e48f6b052c8@quicinc.com

 drivers/soc/qcom/icc-bwmon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/icc-bwmon.c b/drivers/soc/qcom/icc-bwmon.c
index 3dfa448bf8cf..a245a8b2cfef 100644
--- a/drivers/soc/qcom/icc-bwmon.c
+++ b/drivers/soc/qcom/icc-bwmon.c
@@ -827,7 +827,7 @@ static const struct icc_bwmon_data msm8998_bwmon_data = {
 static const struct icc_bwmon_data sdm845_cpu_bwmon_data = {
 	.sample_ms = 4,
 	.count_unit_kb = 64,
-	.zone1_thres_count = 16,
+	.zone1_thres_count = 3,
 	.zone3_thres_count = 1,
 	.quirks = BWMON_HAS_GLOBAL_IRQ,
 	.regmap_fields = sdm845_cpu_bwmon_reg_fields,
@@ -846,7 +846,7 @@ static const struct icc_bwmon_data sdm845_llcc_bwmon_data = {
 static const struct icc_bwmon_data sc7280_llcc_bwmon_data = {
 	.sample_ms = 4,
 	.count_unit_kb = 64,
-	.zone1_thres_count = 16,
+	.zone1_thres_count = 3,
 	.zone3_thres_count = 1,
 	.quirks = BWMON_NEEDS_FORCE_CLEAR,
 	.regmap_fields = sdm845_llcc_bwmon_reg_fields,
-- 
2.34.1
Re: [PATCH V2] soc: qcom: icc-bwmon: Update zone1_thres_count to 3
Posted by Bjorn Andersson 5 months ago
On Fri, Sep 05, 2025 at 05:09:23PM +0530, Pushpendra Singh wrote:
> From: Shivnandan Kumar <quic_kshivnan@quicinc.com>
> 
> Update zone1_thres_count to 3 from 16 so that
> driver can reduce bus vote in 3 sample windows instead
> of waiting for 16 windows.
> 

Please start your commit message by describing the problem.

> The 16-window (64 ms) waiting time is too long to reduce the
> bus vote. At higher FPS, there will be multiple frames in 64ms
> e.g. 4 frames at 60FPS in 64ms. Hence, delay of 64ms in decision
> making will lead to higher power regression. We tested across
> multiple usecases, and observed significant power savings.
> 

I asked in v1 what the tradeoff is here. Is lower number better in all
cases? What are we loosing by making it 3?

And why 3, why not 2 or 4, or 7?


I'm not saying that 3 is wrong, just saying that the commit message
needs to sufficiently explain why 3 is the "best" number.

Regards,
Bjorn

> USECASE				zone1_thres_count=16     zone1_thres_count=3
> 4K video playback       236.15 mA                  203.15 mA
> Sleep					   7mA			   			6.9mA
> Display (dle display)    71.95mA			       67.11mA
> 
> Signed-off-by: Shivnandan Kumar <quic_kshivnan@quicinc.com>
> Signed-off-by: Pushpendra Singh <quic_pussin@quicinc.com>
> ---
> Changes in v2:
> -Update commit message
> -Link to v1:https://lore.kernel.org/lkml/463eb7c8-00fc-4441-91d1-6e48f6b052c8@quicinc.com
> 
>  drivers/soc/qcom/icc-bwmon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/qcom/icc-bwmon.c b/drivers/soc/qcom/icc-bwmon.c
> index 3dfa448bf8cf..a245a8b2cfef 100644
> --- a/drivers/soc/qcom/icc-bwmon.c
> +++ b/drivers/soc/qcom/icc-bwmon.c
> @@ -827,7 +827,7 @@ static const struct icc_bwmon_data msm8998_bwmon_data = {
>  static const struct icc_bwmon_data sdm845_cpu_bwmon_data = {
>  	.sample_ms = 4,
>  	.count_unit_kb = 64,
> -	.zone1_thres_count = 16,
> +	.zone1_thres_count = 3,
>  	.zone3_thres_count = 1,
>  	.quirks = BWMON_HAS_GLOBAL_IRQ,
>  	.regmap_fields = sdm845_cpu_bwmon_reg_fields,
> @@ -846,7 +846,7 @@ static const struct icc_bwmon_data sdm845_llcc_bwmon_data = {
>  static const struct icc_bwmon_data sc7280_llcc_bwmon_data = {
>  	.sample_ms = 4,
>  	.count_unit_kb = 64,
> -	.zone1_thres_count = 16,
> +	.zone1_thres_count = 3,
>  	.zone3_thres_count = 1,
>  	.quirks = BWMON_NEEDS_FORCE_CLEAR,
>  	.regmap_fields = sdm845_llcc_bwmon_reg_fields,
> -- 
> 2.34.1
>
Re: [PATCH V2] soc: qcom: icc-bwmon: Update zone1_thres_count to 3
Posted by Pushpendra Singh 4 months, 2 weeks ago

On 9/6/2025 1:17 AM, Bjorn Andersson wrote:
> On Fri, Sep 05, 2025 at 05:09:23PM +0530, Pushpendra Singh wrote:
>> From: Shivnandan Kumar <quic_kshivnan@quicinc.com>
>>
>> Update zone1_thres_count to 3 from 16 so that
>> driver can reduce bus vote in 3 sample windows instead
>> of waiting for 16 windows.
>>
> 
> Please start your commit message by describing the problem.
> 

Ack, will fix it in the next re-spin

>> The 16-window (64 ms) waiting time is too long to reduce the
>> bus vote. At higher FPS, there will be multiple frames in 64ms
>> e.g. 4 frames at 60FPS in 64ms. Hence, delay of 64ms in decision
>> making will lead to higher power regression. We tested across
>> multiple usecases, and observed significant power savings.
>>
> 
> I asked in v1 what the tradeoff is here. Is lower number better in all
> cases? What are we loosing by making it 3?
> 
> And why 3, why not 2 or 4, or 7?
> 
> 
> I'm not saying that 3 is wrong, just saying that the commit message
> needs to sufficiently explain why 3 is the "best" number.
> 

There is no strict algorithmic logic behind selecting the value 3. 
We experimented with multiple values including 2, 3, 4, 5, 7, etc.  
to evaluate the impact on both power savings and system stability.
-Lower values (like 2) made bwmon more jittery. 
-Higher values (like 4 ,5 or 6) were more stable but less responsive,
 leading to reduced power savings.
-Value 3 struck the best balance, it provided efficient power
 savings while maintaining stable bwmon behavior without jitter.

Sure will update the commit message too.

Regards,
Pushpendra Singh

> Regards,
> Bjorn
> 
>> USECASE				zone1_thres_count=16     zone1_thres_count=3
>> 4K video playback       236.15 mA                  203.15 mA
>> Sleep					   7mA			   			6.9mA
>> Display (dle display)    71.95mA			       67.11mA
>>
>> Signed-off-by: Shivnandan Kumar <quic_kshivnan@quicinc.com>
>> Signed-off-by: Pushpendra Singh <quic_pussin@quicinc.com>
>> ---
>> Changes in v2:
>> -Update commit message
>> -Link to v1:https://lore.kernel.org/lkml/463eb7c8-00fc-4441-91d1-6e48f6b052c8@quicinc.com
>>
>>  drivers/soc/qcom/icc-bwmon.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/icc-bwmon.c b/drivers/soc/qcom/icc-bwmon.c
>> index 3dfa448bf8cf..a245a8b2cfef 100644
>> --- a/drivers/soc/qcom/icc-bwmon.c
>> +++ b/drivers/soc/qcom/icc-bwmon.c
>> @@ -827,7 +827,7 @@ static const struct icc_bwmon_data msm8998_bwmon_data = {
>>  static const struct icc_bwmon_data sdm845_cpu_bwmon_data = {
>>  	.sample_ms = 4,
>>  	.count_unit_kb = 64,
>> -	.zone1_thres_count = 16,
>> +	.zone1_thres_count = 3,
>>  	.zone3_thres_count = 1,
>>  	.quirks = BWMON_HAS_GLOBAL_IRQ,
>>  	.regmap_fields = sdm845_cpu_bwmon_reg_fields,
>> @@ -846,7 +846,7 @@ static const struct icc_bwmon_data sdm845_llcc_bwmon_data = {
>>  static const struct icc_bwmon_data sc7280_llcc_bwmon_data = {
>>  	.sample_ms = 4,
>>  	.count_unit_kb = 64,
>> -	.zone1_thres_count = 16,
>> +	.zone1_thres_count = 3,
>>  	.zone3_thres_count = 1,
>>  	.quirks = BWMON_NEEDS_FORCE_CLEAR,
>>  	.regmap_fields = sdm845_llcc_bwmon_reg_fields,
>> -- 
>> 2.34.1
>>