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

[Pushpendra Singh <quic_pussin@quicinc.com>]

On 5/22/2024 10:27 PM, Bjorn Andersson wrote:
> On Wed, May 22, 2024 at 02:35:21PM +0530, Shivnandan Kumar wrote:
>>
>>
>> On 5/22/2024 1:58 PM, Krzysztof Kozlowski wrote:
>>> On 22/05/2024 10:15, Shivnandan Kumar wrote:
>>>> 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. This is in line with downstream
>>>> implementation.
>>>>
>>>
>>> This might make bwmon quite jittery. I don't think downstream is the
>>> source of truth here. Please provide some measurements *on mainline tree*.
>>>
>>
>> Hi Krzysztof,
>>
>> 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. I’ve tested this change for 4K video playback on mainline tree,
>> and there’s a significant power-saving.
>>
> 
> As requested by Krzysztof already, update your commit message to capture
> such motivation.
> 
> Please read and follow this:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
> 

Ack, will fix it in the next re-spin

>> I propose to make it a tunable,so that user space can tune it
>> based on runtime depending on fps.
>>
> 
> I presume that in e.g. Android there could be some sort of power HAL
> that tweaks this value dynamically? In a general purpose system, how do
> we make sure that this value stays relevant for multiple types of use
> cases?
> 

At this point, we’re keeping the value static and not exposing it as a tunable parameter.

>> USECASE                     zone1_thres_count=16     zone1_thres_count=3
>> 4K video playback           236.15 mA	             203.15 mA
> 
> 4k video playback is a fairly specific (and generally unusual) use case.
> Is there any impact (negative or positive) for other use
> cases/workloads?
> 

We ran additional use cases and observed significant power savings with the updated zone1_thres_count value. Below are the results, 
 
USECASE                     zone1_thres_count=16     zone1_thres_count=3
4K video playback           	236.15 mA                  203.15 mA
Sleep			    	7mA			   6.9mA
Display (idle display)      	71.95mA			   67.11mA

Regards,
Pushpendra Singh

> Regards,
> Bjorn
> 
>>
>> Thanks,
>> Shivnandan
>>
>>> Best regards,
>>> Krzysztof
>>>