[Intel-gfx] [PATCH 17/29] drm/i915: Make the high dword offset more explicit in i915_reg_read_ioctl

[Chris Wilson]

On Wed, Nov 04, 2015 at 11:20:05PM +0200, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Store the upper dword of the register offset in the whitelist as well.
> This would allow it to read register where the two halves aren't sitting
> right next to each other, and it'll make it easier to make register
> access type safe.
> 
> While at it change the register offsets to u32 from u64. Our register
> space isn't quite that big, yet :)
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h     |  1 +
>  drivers/gpu/drm/i915/intel_uncore.c | 10 ++++++----
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0510ca1..7cea51d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1567,6 +1567,7 @@ enum skl_disp_power_wells {
>  #define RING_IMR(base)		((base)+0xa8)
>  #define RING_HWSTAM(base)	((base)+0x98)
>  #define RING_TIMESTAMP(base)	((base)+0x358)
> +#define RING_TIMESTAMP_HI(base)	((base)+0x358 + 4)
>  #define   TAIL_ADDR		0x001FFFF8
>  #define   HEAD_WRAP_COUNT	0xFFE00000
>  #define   HEAD_WRAP_ONE		0x00200000
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index f0f97b2..ced494a 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1261,12 +1261,13 @@ void intel_uncore_fini(struct drm_device *dev)
>  #define GEN_RANGE(l, h) GENMASK(h, l)
>  
>  static const struct register_whitelist {
> -	uint64_t offset;
> +	uint32_t offset, offset_hi;

Hmm, fwiw I was confused here thinking that you were storing a 64bit
value split across the two u32. (I know that's silly but it has been
common enough in the past.) Maybe offset_ldw and offset_udw?

I'm not sure though if this is something that we are going to be reading
enough that taking an extra few seconds to trace usage of
offset/offset_hi is going to matter much.

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

We should probably also document that when passing a reg_read for a u64
register (that may or may not be split depending on gen) we always
specify the offset of the lower 32bits.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre