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

Daniel Vetter
2013-02-13T19:26:17Z

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