Re: enabling forcewake from userspace

[Chris Wilson <chris@chris-wilson.co.uk>]

On Sat, 26 Mar 2011 12:27:17 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> On Sat, Mar 26, 2011 at 05:18:57PM +0000, Chris Wilson wrote:
> > 
> > The GPU is definitely allowed to be powered down whilst the CPU is
> > servicing an interrupt. ;-) I take your point though that the current
> > method is not interrupt safe. However, we should not be doing such work
> > from an interrupt handler - if need be all the actual work is kicked off
> > from a workqueue.
> > 
> > Something we need to keep an eye on, but not a problem today. *touch wood*
> 
> Efficiency aside, workqueue suffers from the same hazard. Unless the
> assumption is struct_mutex (or something similar) is protecting all
> register access which are in the power wells that can get disabled.

Yes, currently we use struct mutex to serialise pretty much everything,
__gen6_gt_force_wake_* included.

> My
> quick inspection of the code shows this is probably true, but in this
> case I'd suggest a WARN_ON or something in force_wake_get(), because
> this would be a really hard bug to track down.

Similarly, a WARN_ON in i915_reg_write() if reg < 0x40000 && !force-wake.
 
> By the way, I think we currently have potential problem with
> kick_ring().  Although given when this occurs it is probably a DON'T
> CARE.

True. A task for a truly rainy day. And whilst you're there move the error
capture to a workqueue as well.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre