[Intel-gfx] [PATCH v3] drm/i915: Add boot paramter to control rps boost at boot time.

[Deepak S]

On Monday 28 April 2014 08:42 PM, Daniel Vetter wrote:
> On Mon, Apr 28, 2014 at 4:47 PM,  <deepak.s at linux.intel.com> wrote:
>> From: Deepak S <deepak.s at linux.intel.com>
>>
>> We are adding a module paramter to control rps boost. By default, we
>> enable the boost for better performace. Based on the need (perf/power)
>> we can either enable/disable.
>>
>> v2: Addressed rps default comment (Jani)
>>
>> v3: Use bool to represent the boot parameter (Ville).
>>
>> Signed-off-by: Deepak S <deepak.s at linux.intel.com>
>> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> I'm still unhappy about this since it feels like cheating in
> benchmarks and it gives me the impression that you guys frob this at
> runtime on Android ;-)
>
> A few more ideas:
> 1. "light-boost": We add some hysteris (either time or whether we're
> still above rpe or something like that) and don't boost if this is the
> case. I expect that we won't be able to have the full boost benefits
> without the downside.
>
> 2. "eco-boost". We try to boost just enough to not miss the next
> frame. For that the app needs to tell us (with two new execbuf flag)
> whether it hit or missed the last deadline. Once an app used those
> flags for the first time we decrease the boost target freq once per
> HIT_DEADLINE and until we get the first MISS_DEADLINE. The we only try
> to sporadically test the limit again. TCP flow control theory might be
> interesting for copying ideas.
>
> 3. "runtime-boost-control". The workloads with very predictable
> regular loads seem to be known. We can just add a new execbuf NO_BOOST
> flag which libva uses on all execbufs but the first one (since we
> don't want to drop the first frame really).
>
> Approach 3 should be the simplest to implement and also the simplest
> to demonstrate in the open-source libva (since that's always a merge
> criteria).
>
> Aside: If you really use this at runtime then you essentially create a
> new ABI with this patch. Which means we need open-source userspace for
> it anyway.
> -Daniel

Thanks for the review.
Agreed. option 3 is better. I will work on this.