On Fri, Jul 12, 2013 at 06:08:23PM +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. > > v2: Rebase > v3: Rebase onto uncore > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_uncore.c | 107 +++++++++++++++++++++--------------- > 1 file changed, 64 insertions(+), 43 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index 8c2f460..9d4063f 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.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__)) > + > +#define __raw_i915_read64(dev_priv__, reg__) readq((dev_priv__)->regs + (reg__)) > +#define __raw_i915_write64(dev_priv__, reg__, val__) writeq(val__, (dev_priv__)->regs + (reg__)) > + > + Still waiting on the __raw_posting_read. And now I'm thinking a "posted_write" would be cool, so we don't need to explicitly insert posting reads. We have a special case in this file though, where we need a posting read, but I'd be fine with the ugly (void) in just the corner cases. If I am not mistaken, I915_*_NOTRACE would no longer be used after this patch. Maybe removing those here would also help clarify what you're doing, and make merge easier (in case someone adds a new one before this gets merged). And yes, I know you change it in an upcoming patch. Otherwise, looks good to me: Reviewed-by: Ben Widawsky <ben at bwidawsk.net> [snip] -- Ben Widawsky, Intel Open Source Technology Center