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

[Bob Paauwe]

On Fri, 4 Dec 2015 22:58:50 +0200
Imre Deak <imre.deak at intel.com> wrote:

> 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. 

So we want the policy to be that we'll only drop below min to idle when
the GPU transitions from not idle to idle.  Without that transition,
we'll stay at the user specified min.

> 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");

Yes, this does make the test pass.  However, what is the expected
behavior in the following:

echo 500 > gt_min_freq_mhz
echo 200 > gt_min_freq_mhz

With no GPU load?

Right now, the current frequency will stay stuck at 500 until there is
GPU load or until I double set the min freq to 200.  For example:

  bxt-test:drm root $ echo 500 > card0/gt_min_freq_mhz 
  bxt-test:drm root $ cat card0/gt_cur_freq_mhz 
  500
  bxt-test:drm root $ echo 200 > card0/gt_min_freq_mhz                         
  bxt-test:drm root $ cat card0/gt_cur_freq_mhz 
  500
  bxt-test:drm root $ echo 200 > card0/gt_min_freq_mhz                         
  bxt-test:drm root $ cat card0/gt_cur_freq_mhz 
  200

If I add a delay before the posting read in gen6_set_rps() then the 
frequency will drop after the first "echo 200 >". It's like the 
gen6_set_rps() is behind.  A slightly more interesting result
is:

echo 500 > min  --> cur = 500
echo 200 > min  --> cur = 500
echo 400 > min  --> cur = 200

Something doesn't seem right. 





-- 
--
Bob Paauwe                  
Bob.J.Paauwe at intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193