[Intel-gfx] [PATCH 06/43] drm/i915: protect force_wake_(get|put) with the gt_lock

[Daniel Vetter]

On Tue, Jan 3, 2012 at 19:51, Keith Packard <keithp at keithp.com> wrote:
> On Wed, 14 Dec 2011 13:57:03 +0100, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
>
>> The problem this patch solves is that the forcewake accounting
>> necessary for register reads is protected by dev->struct_mutex. But the
>> hangcheck and error_capture code need to access registers without
>> grabbing this mutex because we hold it while waiting for the gpu.
>> So a new lock is required. Because currently the error_state capture
>> is called from the error irq handler and the hangcheck code runs from
>> a timer, it needs to be an irqsafe spinlock (note that the registers
>> used by the irq handler (neglecting the error handling part) only uses
>> registers that don't need the forcewake dance).
>
> I think this description is wrong -- the only difference between using
> atomic objects and using a spinlock is that with the spinlock the call
> to ->force_wake_get is correctly serialized so that no register access
> can occur without the chip being awoken. Without a spinlock, a second
> thread can pass right through gen6_gt_force_wake_get and then go touch
> registers while the first thread is busy waking the chip up.

I'm a bit confused by this. With the current code forcewake is
protected by dev->struct_mutex. Which doesn't work out because we need
to access registers that require the forcewake dance from non-process
context.

Afaik the atomic ops stuff is just ducttape for paranoia reasons.
-Daniel
-- 
Daniel Vetter
daniel.vetter at ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch