On Wed, 20 Nov 2013 06:00:24 +0000 "S, Deepak" <deepak.s@xxxxxxxxx> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
