From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 14/18] drm/i915: vebox interrupt get/put Date: Wed, 13 Feb 2013 20:28:32 +0100 Message-ID: <20130213192832.GQ5813@phenom.ffwll.local> References: <1352219142-1395-1-git-send-email-ben@bwidawsk.net> <1352219142-1395-15-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f177.google.com (mail-ea0-f177.google.com [209.85.215.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 2630CE5E4B for ; Wed, 13 Feb 2013 11:26:17 -0800 (PST) Received: by mail-ea0-f177.google.com with SMTP id n13so618683eaa.36 for ; Wed, 13 Feb 2013 11:26:17 -0800 (PST) Content-Disposition: inline In-Reply-To: <1352219142-1395-15-git-send-email-ben@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ben Widawsky Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org 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 > Signed-off-by: Ben Widawsky [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