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

Daniel Vetter
2015-02-25T21:29:36Z

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@Intel.com wrote:
> >>>>From: John Harrison <John.C.Harrison@Intel.com>
> >>>>
> >>>>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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch