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

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On Tue, 4 Jul 2023 at 00:40, Abhinav Kumar <quic_abhinavk@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@linaro.org>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@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.

>
> 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'?

-- 
With best wishes
Dmitry