From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 2/6] drm/i915: Use a private interface for register access within GT Date: Fri, 12 Jul 2013 22:57:25 +0200 Message-ID: <20130712205725.GK6143@phenom.ffwll.local> References: <1373637756-11852-1-git-send-email-chris@chris-wilson.co.uk> <1373637756-11852-2-git-send-email-chris@chris-wilson.co.uk> <20130712202527.GG15384@bwidawsk.net> <20130712205546.GH23323@cantiga.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f172.google.com (mail-ea0-f172.google.com [209.85.215.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 21FA6E60A6 for ; Fri, 12 Jul 2013 13:57:30 -0700 (PDT) Received: by mail-ea0-f172.google.com with SMTP id q10so6687225eaj.31 for ; Fri, 12 Jul 2013 13:57:29 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130712205546.GH23323@cantiga.alporthouse.com> 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: Chris Wilson , Ben Widawsky , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Jul 12, 2013 at 09:55:46PM +0100, Chris Wilson wrote: > On Fri, Jul 12, 2013 at 01:25:27PM -0700, Ben Widawsky wrote: > > On Fri, Jul 12, 2013 at 03:02:32PM +0100, Chris Wilson wrote: > > > The GT functions for enabling register access also need to occasionally > > > write to and read from registers. To avoid the potential recursion as we > > > modify the public interface to be stricter, introduce a private register > > > access API for the GT functions. > > > > > > Signed-off-by: Chris Wilson > > > --- > > > drivers/gpu/drm/i915/intel_gt.c | 92 +++++++++++++++++++++++++---------------- > > > 1 file changed, 56 insertions(+), 36 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_gt.c b/drivers/gpu/drm/i915/intel_gt.c > > > index 060e256..cb3116c 100644 > > > --- a/drivers/gpu/drm/i915/intel_gt.c > > > +++ b/drivers/gpu/drm/i915/intel_gt.c > > > @@ -26,6 +26,19 @@ > > > > > > #define FORCEWAKE_ACK_TIMEOUT_MS 2 > > > > > > +#define __raw_i915_read8(dev_priv__, reg__) readb((dev_priv__)->regs + (reg__)) > > > +#define __raw_i915_write8(dev_priv__, reg__, val__) writeb(val__, (dev_priv__)->regs + (reg__)) > > > + > > > +#define __raw_i915_read16(dev_priv__, reg__) readw((dev_priv__)->regs + (reg__)) > > > +#define __raw_i915_write16(dev_priv__, reg__, val__) writew(val__, (dev_priv__)->regs + (reg__)) > > > + > > > +#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + (reg__)) > > > +#define __raw_i915_write32(dev_priv__, reg__, val__) writel(val__, (dev_priv__)->regs + (reg__)) > > > > Instead of what you did, I would have preferred > > #define __raw_posting_read > > In hindsight, I agree with using __raw_posting_read. Patch on top to avoid another messy rebase? I've read through the patches and also thought that the (void)__raw_read sprinkled all over is unsightly, a __raw_posting_read would look better. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch