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

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Apr 30, 2014 at 09:19:32PM +0530, Deepak S wrote:
> 
> On Monday 28 April 2014 08:42 PM, Daniel Vetter wrote:
> >On Mon, Apr 28, 2014 at 4:47 PM,  <deepak.s@linux.intel.com> wrote:
> >>From: Deepak S <deepak.s@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@linux.intel.com>
> >>Reviewed-by: Ville Syrjälä <ville.syrjala@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.

Cool. Can you please get in contact with the otc libva guys for the
demonstration user? The libva patch should be approved by libva owners for
merging.

Wrt testcase I think we only need to update the execbuf flag tests in
gem_exec_params. Having a functional test seems less useful, we want to
regression test that this works through overall power consumption metrics
I guess.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch