[Intel-gfx] [PATCH 2/2] igt/pm_rps: Add checks for freq = idle (RPn) in specific cases.

[Imre Deak]

On Fri, 2015-12-04 at 10:37 -0800, Bob Paauwe wrote:
> On Fri, 4 Dec 2015 17:22:11 +0200
> Imre Deak <imre.deak at intel.com> wrote:
> [...]
> > > With the BIOS setting corrected, the driver started disabling the
> > > down
> > > interrupts once the freq dropped to min or just below because of
> > > the min
> > > vs. idle difference.  I have a patch for this that I'll send
> > > shortly.
> > 
> > Hm, that's not necessarily a problem. To reach the idle frequency
> > we
> > don't depend on the up/down interrupts. The idle frequency gets set
> > explicitly from intel_mark_idle(), so if you don't reach the idle
> > freq
> > then this function doesn't get called for some reason. Or as I said
> > earlier we just don't wait enough in do_load_gpu() or idle_check()
> > in
> > the test, so we read out cur-freq before intel_mark_idle() would
> > get
> > called.
> 
> Maybe because I'm just testing from a console environment, but if I
> change the min freq. via sysfs I don't get any type of call to drop
> back to idle.  Possibly because I'm not generating any GPU load so it
> never re-triggers an idle gpu?  I've waited quite a while and have
> bumped up the time in idle_check().  Waiting for minutes doesn't seem
> reasonable.
> 
> Should intel_mark_idle() get called periodically, even if the rings
> are
> empty?
>   
> > 
> > > The remaining issue seems to be some type of timing issue. I've
> > > had
> > > to
> > > add a 35000us sleep between updating the interrupt enable
> > > register
> > > (0xA168) and the posting read of freq. register (0xA008),
> > > otherwise
> > > it
> > > acts like the change to the interrupt enable register never
> > > happened.
> > > None of the documentation I have indicates that this is needed.
> > 
> > What happens exactly? The frequency gets stuck above min-freq and
> > you
> > never get a down interrupt? Not sure how this could happen, but
> > there
> > is an interesting comment in intel_rps_limits() about this
> > scenario.
> 
> Basically if I set the min freq. through sysfs, it flows through all
> the set threshold, create the interrupt mask code but the system
> behaves as if the new threshold limits or interrupts were not
> changed.
> 
> So say RPn = 100, min = 100, current = 100
>  - echo 383 > /sys/class/drm/card0/gt_min_freq_mhz  (midpoint freq)
> 
> gt_cur_freq_mhz will report 383 now and I don't see any interrupts.
> 
> If I add the delay, then right after I set the min to 383, I'll start
> seeing the down threshold interrupts and it will lower the freq back
> down to RPn (100) after two or three interrupts occur.
> 
> So I don't think this has anything to with RC6 and missing
> interrupts that the comment refers to.
> 
> It took me a while to track this down because I was using a bunch of
> printk's to trace the flow and values.  With the printk's it was
> working but when I removed them, it stopped working. So it was about
> 3
> or 4 printk's worth of delay that was needed.  But I really wonder if
> there's something else that should be checked vs. just delaying. 

Ok, so for both of the above observations: If you write the min-freq
value via sysfs, it will result in cur-freq raised and "getting stuck"
at min-freq if cur-freq was below the new min value. This is a bit
strange since as we already established normally cur-freq should always
drop to idle-freq if the GPU is idle regardless of min-freq. AFAIR the
sysfs entry works in this way, since setting the frequency also changes
the interrupt mask and what we really need is this latter thing. To get
back to the normal drop-to-idle-freq policy again you need to submit
some GPU work after setting the sysfs entry. It's done already in most
of the places in pm_rps, except for few. The following should fix up
those:

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index 74f08f4..ad06ef0 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -388,10 +388,14 @@ static void min_max_config(void (*check)(void),
bool load_gpu)
 
 	igt_debug("\nIncrease min to midpoint...\n");
 	writeval(stuff[MIN].filp, fmid);
+	if (load_gpu)
+		do_load_gpu();
 	check();
 
 	igt_debug("\nIncrease min to RP0...\n");
 	writeval(stuff[MIN].filp, origfreqs[RP0]);
+	if (load_gpu)
+		do_load_gpu();
 	check();
 
 	igt_debug("\nIncrease min above RP0 (invalid)...\n");
@@ -416,6 +420,8 @@ static void min_max_config(void (*check)(void),
bool load_gpu)
 
 	igt_debug("\nDecrease min below RPn (invalid)...\n");
 	writeval_inval(stuff[MIN].filp, 0);
+	if (load_gpu)
+		do_load_gpu();
 	check();
 
 	igt_debug("\nDecrease max to midpoint...\n");
-- 
2.5.0