Re: [PATCH] drm/i915: paper over missed irq issues with force wake vodoo

[Kenneth Graunke <kenneth@whitecape.org>]

On 01/10/2012 04:20 AM, Daniel Vetter wrote:
> On Wed, Jan 04, 2012 at 07:40:45PM +0100, Daniel Vetter wrote:
>> Two things seem to do the trick on my ivb machine here:
>> - prevent the gt from powering down while waiting for seqno
>>    notification interrupts by grabbing the force_wake in get_irq (and
>>    dropping it in put_irq again).
>> - ordering writes from the ring's CS by reading a CS register, ACTHD
>>    seems to work.
>>
>> Only the blt&bsd ring on ivb seem to be massively affected by this,
>> but for paranoia do this dance also on the render ring and on snb
>> (i.e. all gpus with forcewake).
>>
>> Tested with Eric's glCopyPixels loop which without this patch scores a
>> missed irq every few seconds.
>>
>> This patch needs my forcewake rework to use a spinlock instead of
>> dev->struct_mutex.
>>
>> v2: Improve the comment per Eugeni Dodonov's suggestion.
>>
>> Cc: stable@kernel.org
>> Cc: Eric Anholt<eric@anholt.net>
>> Cc: Kenneth Graunke<kenneth@whitecape.org>
>> Cc: Eugeni Dodonov<eugeni.dodonov@intel.com>
>> Tested-by: Eugeni Dodonov<eugeni.dodonov@intel.com>
>> Reviewed-by: Eugeni Dodonov<eugeni.dodonov@intel.com>
>> Signed-Off-by: Daniel Vetter<daniel.vetter@ffwll.ch>
>
>  From the internal doc "SNB GT PM Programming Guide", Section 4.3.1:
>
> "GT does not generate interrupts while in RC6 (by design)"
>
> It talks about the PM interrupt but I think this might also apply to
> interrupts in general. So I think we should apply the voodoo patch also to
> snb.
>
> My current working theory is hw engineers changed the way hwstam writes
> are generated wrt to the read/write/irq pipeline on ivb and we've only
> been lucky that it syncs out everything on snb before the gpu goes into
> deep sleep states.
> -Daniel

Daniel,

This is a good find!  Your patch looked good to me in the first place, 
but with this extra bit of information, I believe it all the more.  It's 
simple and it makes a lot of sense.

FWIW,
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

I'm in favor of applying this for Gen7, and I'd be fine with applying it 
on Gen6 too.  At the very least we should experiment with it on 
Gen6---as Keith said, it might help with some of the mysterious issues 
we've seen.  Worth a try at least.

Thanks for your great work here!

--Kenneth