[Intel-gfx] [PATCH 2/6] drm/i915: Use a private interface for register access within GT

[Ben Widawsky]

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