Re: [PATCH 14/18] drm/i915: vebox interrupt get/put

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Nov 06, 2012 at 04:25:38PM +0000, Ben Widawsky wrote:
> v2: Use the correct lock to protect PM interrupt regs, this was
> accidentally lost from earlier (Haihao)
> Fix return types (Ben)
> 
> CC: Xiang, Haihao <haihao.xiang@intel.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

[snip]

> ---
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index b2fe5b4..2a2bd20 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -58,8 +58,9 @@ struct  intel_ring_buffer {
>  	u32		last_retired_head;
>  
>  	struct {
> -		u32	gt;
> -	} irq_refcount;	/* protected by dev_priv->irq_lock */
> +		u32	gt; /*  protected by dev_priv->irq_lock */
> +		u32	pm; /*  protected by dev_priv->rps.lock (sucks) */
> +	} irq_refcount;

So Ben asked me on internal irc to at least ack the irq stuff in his VECS
patches here as a cheap excuse for why he never updated them. I'm ok with
it on a quick read, the #define unification seems to make tons of sense,
and the locking trick here is imo ok, too. Only bikeshed I could have come
up with is to put gt/pm irq_refcount into a union, since for a given ring
we'll never use both. Maybe also add a bigger comment above the
irq_refcount union/struct to explain clearly wtf is going on here.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch