[PATCH 2/8] drm/msm/dpu: drop performance tuning modes

[Abhinav Kumar]

On 7/3/2023 3:20 PM, Dmitry Baryshkov wrote:
> On Tue, 4 Jul 2023 at 00:40, Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>>
>>
>>
>> On 6/19/2023 5:08 PM, Dmitry Baryshkov wrote:
>>> DPU performance module contains code to change performance state
>>> calculations. In addition to normal (sum plane and CRTC requirements),
>>> it can work in 'minimal' or 'fixed' modes. Both modes are impractical,
>>> since they can easily end up with the display underruns. Userspace also
>>> should not depend on these modes availability, since they are tuned
>>> through debugfs, which might not be available.
>>>
>>> Drop relevant code to simplify performance state calculations.
>>>
>>> Suggested-by: Konrad Dybcio <konrad.dybcio at linaro.org>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>> ---
>>
>> Sorry but NAK on this change for two reasons:
>>
>> This mode is not for usermode to depend on but for debugging. I have
>> personally used both the perf max and perf min modes for debug.
>>
>> 1) The purpose is that, if you do see an underrun, you can force the
>> perf mode as it will select max clk and bw rate
>>
>> So something like below:
>>
>> localhost /sys/kernel/debug/dri/1/debug/core_perf # echo 2 > perf_mode
>> localhost /sys/kernel/debug/dri/1/debug/core_perf # echo 300000000 >
>> fix_core_clk_rate
>>
>> This will allow us to force the clk to a particular value to see at what
>> point it starts underruning
>>
>> Also you can even do
>>
>> localhost /sys/kernel/debug/dri/1/debug/core_perf # echo 1 > perf_mode
>>
>> This will automatically max out the clk and BW
>>
>> With this, you can figure out if underrun is a performance related
>> underrun or a misconfiguration. We used it even recently to debug the
>> performance issue with pclk reduction
> 
> Hmm, 1 is minimum, not maxumum.
> 

The name is kind of confusing.

Yes 1 is min perf mode but it maxes out the clocks and BW.

I guess its named that way because a min perf mode gives you the minimum 
savings in terms of power.

>>
>> 2) Sometimes, you even want to force an underrun to debug devcoredump OR
>> the recovery code. Forcing the min clk mode by doing
>>
>> localhost /sys/kernel/debug/dri/1/debug/core_perf # echo 19200000 >
>> fix_core_clk_rate
>> localhost /sys/kernel/debug/dri/1/debug/core_perf # echo 2 > perf_mode
>>
>> Is the easiest way to trigger the recovery handler.
>>
>> Hence I am not at all convinced of dropping this.
> 
> I see, thanks for sharing the usecases. However I still think that it
> is overcomplicated for the debugging feature. What about dropping all
> perf modes and providing just 'override_core_clk_rate' and
> 'override_avg_bw', 'override_peak_bw'?
> 

No, we need both. Let me explain why:

1) Having just the min perf mode, directly uses the max clk and bw. This 
is useful when you just want to run at the max and see the behavior

2) If you want to figure out what is the sweet spot where the issue does 
not happen you need the "fixed" mode to find the range where the issue 
doesnt happen

This is one of the oldest and most effective debugging mechanisms.

I dont want to touch this and I personally use this quite often.