[Intel-gfx] [PATCH] drm/i915: Support 64b relocations

[Chris Wilson]

On Mon, Apr 28, 2014 at 05:18:28PM -0700, Ben Widawsky wrote:
> All the rest of the code to enable this is in my branch. Without my
> branch, hitting > 32b offsets is impossible. The code has always
> "supported" 64b, but it's never actually been run of tested. This change
> doesn't actually fix anything. [1] I am not sure why X won't work yet. I
> do not get hangs or obvious errors.
> 
> There are 3 fixes grouped together here. First is to remove the
> hardcoded 0 for the upper dword of the relocation. The next fix is to
> use a 64b value for target_offset. The final fix is to not directly
> apply target_offset to reloc->delta. reloc->delta is part of ABI, and so
> we cannot change it. As it stands, 32b is enough to represent everything
> we're interested in representing anyway. The main problem is, we cannot
> add greater than 32b values to it directly.
> 
> [1] Almost all of intel-gpu-tools is not yet ready to test 64b
> relocations. There are a few places that expect 32b values for offsets
> and these all won't work.
> 
> Cc: Rafael Barbalho <rafael.barbalho at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>

Seriously, we did this? I am ashamed. I was annoyed by the original
assertion that no userspace was ready in the first place, and to see
that the code was a complete farce anyway...

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

> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 0d806fc..6ffecd2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -262,10 +262,12 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
>  
>  static int
>  relocate_entry_cpu(struct drm_i915_gem_object *obj,
> -		   struct drm_i915_gem_relocation_entry *reloc)
> +		   struct drm_i915_gem_relocation_entry *reloc,
> +		   uint64_t target_offset)
>  {
>  	struct drm_device *dev = obj->base.dev;
>  	uint32_t page_offset = offset_in_page(reloc->offset);
> +	uint64_t delta = reloc->delta + target_offset;

I would not have called the final value delta, but target_offset.
I was going to quible over the use of a local variable instead of
reloc->delta, but you successfully argued in my head that your way was
less obtuse.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre