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

[Jesse Barnes <jbarnes@virtuousgeek.org>]

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