Re: [PATCH] drm/i915/vlv: Add VLV specific force wake routines.

[Jesse Barnes <jbarnes@virtuousgeek.org>]

Also, you might re-run your power numbers with Chris's patch to drop
the force wake ref around the irq get/put.  That's the only one we
normally take at runtime, so getting rid of it should give you even
greater power savings in conjunction with the RC6 timeout update patch I
already pushed.

Thanks,
Jesse

On Wed, 20 Nov 2013 16:33:22 +0000
"S, Deepak" <deepak.s@intel.com> wrote:

> Thanks Jesse, I wil split the patches and send it for review.
> 
> Thanks
> Deepak
> 
> -----Original Message-----
> From: Jesse Barnes [mailto:jbarnes@virtuousgeek.org] 
> Sent: Wednesday, November 20, 2013 9:35 PM
> To: S, Deepak
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: Add VLV specific force wake routines.
> 
> On Wed, 20 Nov 2013 06:00:24 +0000
> "S, Deepak" <deepak.s@intel.com> wrote:
> 
> > Hi Jesse,
> > 
> > Thanks for the review. Below is my response. 
> > 
> > >  - why not use the callback to __vlv_force_wake_* from
> >     gen6_gt_force_wake_*?  i.e. why is VLV special here?
> > [Deepak]   Gen6 has a single power well whereas the  VLV is has spate wells. This was the reason  for the separate function
> > 
> >  > - having a new gen7_media_force_wake function may be better than
> >     passing an engine around, and would touch fewer pieces of code 
> > [Deepak]  Even Gen7  is also as single Power Well. Having common function between gen7 and vlv might be difficult to individually handle the wells.
> > 
> >   >- have you done measurements on this?  given how infrequently we
> >     ought to be waking the wells when they're idle, and how long we
> >     generally keep them awake, is this a real power win?
> > [Deepak] By Individually controlling the wells we observed around 100mW - 200mW  saving in different scenarios (GL Beanchmark & Media playback).
> 
> wow, that's a significant savings.
> 
> Can you split the patch into one that adds the power well arg, and another that adds the VLV support for the split?  That would make it easier to review.
> 
> Thanks,
> --
> Jesse Barnes, Intel Open Source Technology Center
> 


-- 
Jesse Barnes, Intel Open Source Technology Center