On Tue, 4 Jul 2023 at 01:26, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > > > > On 7/3/2023 3:20 PM, Dmitry Baryshkov wrote: > > On Tue, 4 Jul 2023 at 00:40, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> 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@xxxxxxxxxx> > >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > >>> --- > >> > >> 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
