Re: [PATCH 47/51] drm/i915: Update ironlake_enable_rc6() to do explicit request management

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 25/02/2015 21:31, Daniel Vetter wrote:
On Wed, Feb 18, 2015 at 02:28:16PM +0000, John Harrison wrote:
On 13/02/2015 17:03, Chris Wilson wrote:
On Fri, Feb 13, 2015 at 04:58:24PM +0000, John Harrison wrote:
On 13/02/2015 12:19, Chris Wilson wrote:
On Fri, Feb 13, 2015 at 11:48:56AM +0000, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>

Updated ironlake_enable_rc6() to do explicit request creation and submission.
If you merged the context here with the common context switching code,
we don't even need to touch the ring here.
-Chris

It would certainly be preferable to not have any ring commands
written from deep within the power management code. However, I
didn't want to change anything I didn't really need to, especially
in code that I'm not at all sure about. Plus I don't have an
ironlake to test any significant change on.
I did and tested extensively.
-Chris
Do you have a patch that just does the move of this from PM code to context
switch code? Something that I can drop into this series would be great. If
not, exactly where about in the context switch code should it go? Should it
be in the start of day initialisation, in the per context intialisation,
every context switch, only the first switch after a resume, ...?

Tracing back to where/when this code is currently executed seems to be quite
complicated. The _enable_rc6() function is called during ring reset but only
for Gen6+ because Ironlake is broken according to the comment. It is also
called by a system power management callback but it is unclear when that
would occur. Finally, it is also called from the display code in
intel_modeset_init_hw().
ilk rc6 is disabled by default because it crashes machines hard and
doesn't seem to be all that useful really. Compared to gen6+ rc6 which has
a massive impact on idle power consumption, more with each generation.

I've also never heard of anyone managing to make this work reliably. We
could also just rip the entire code out I think, at least I wouldn't be
surprised if it has bitrot completely by now.
-Daniel

So just remove the ironlake_enable_rc6() function completely? Or keep it as a stub that just says '/* this has never worked so has been removed */'.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux