Re: [PATCH] drm/i915: Enabling RC6 immediately during init/resume

Daniel Vetter
2015-08-25T14:32:09Z

On Fri, Aug 21, 2015 at 01:41:26PM +0100, Chris Wilson wrote:
> On Sat, Aug 22, 2015 at 02:19:48AM +0530, Namrta Salonie wrote:
> > Since RC6 enabling does not involve PCU communication overhead,
> > it can be enabled immediately during the resume time.
> > This will help save additional power & meet power requirements
> > for active Idle KPI where power is evaluated over
> > number of transitions of suspend/resume.
> >
> > Signed-off-by: Namrta Salonie <namrta.salonie@intel.com>
> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>
> You can pull out gen9 rc6 as well, and apply a similar transformation to
> gen6-8. So instead of putting the if-chain in
> intel_enable_gt_powersave(), add intel_enable_rc6() and start placing
> the ready functions there.
>
> Reviewing the comments we only need the rpm lock until after rc6
> enabling and as you keep that wakelock, you are not getting the full
> improvement you seek. If you keep refactoring the remaining two rc6
> functions, you can then drop the wakelock.

Since this seems to not have much of a benefit due to the missing removal
of the wakelock I wonder how this was tested ... Next patch should have
(relative, we're not allowed to publish absolute) performance data
attached, e.g. "Over 100 suspend/resume cycles with 5s of idle time in
between each suspend/resume time this reduce in a reduction of $number
$unit."

Without this this patch is just unjustified tuning and I won't take it.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation