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

[Dmitry Baryshkov]

On Tue, 4 Jul 2023 at 01:26, Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>
>
>
> 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.

Ack, I'll see if I can keep the interface / idea. I still think that
override + in-kernel limits should do the same trick as the current
'maximum' does.



-- 
With best wishes
Dmitry