On Wed, Jul 01, 2015 at 04:27:32PM +0100, Michel Thierry wrote:
> There are some allocations that must be only referenced by 32-bit
> offsets. To limit the chances of having the first 4GB already full,
> objects not requiring this workaround use DRM_MM_SEARCH_BELOW/
> DRM_MM_CREATE_TOP flags
> 
> In specific, any resource used with flat/heapless (0x00000000-0xfffff000)
> General State Heap (GSH) or Intructions State Heap (ISH) must be in a
> 32-bit range, because the General State Offset and Instruction State
> Offset are limited to 32-bits.
> 
> Objects must have EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag to indicate if
> they can be allocated above the 32-bit address range. To limit the
> chances of having the first 4GB already full, objects will use
> DRM_MM_SEARCH_BELOW + DRM_MM_CREATE_TOP flags when possible.
> 
> v2: Changed flag logic from neeeds_32b, to supports_48b.
> v3: Moved 48-bit support flag back to exec_object. (Chris, Daniel)
> v4: Split pin flags into PIN_ZONE_4G and PIN_HIGH; update PIN_OFFSET_MASK
> to use last PIN_ defined instead of hard-coded value; use correct limit
> check in eb_vma_misplaced. (Chris)
> 
> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Ben Widawsky <b...@bwidawsk.net>
> Signed-off-by: Michel Thierry <michel.thie...@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h            |  4 +++-
>  drivers/gpu/drm/i915/i915_gem.c            | 17 +++++++++++++++--
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 ++++++++++
>  include/uapi/drm/i915_drm.h                |  3 ++-
>  4 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c720a18..aac51fb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2765,7 +2765,9 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
>  #define PIN_OFFSET_BIAS      (1<<3)
>  #define PIN_USER     (1<<4)
>  #define PIN_UPDATE   (1<<5)
> -#define PIN_OFFSET_MASK (~4095)
> +#define PIN_ZONE_4G  (1<<6)
> +#define PIN_HIGH     (1<<7)
> +#define PIN_OFFSET_MASK -(PIN_HIGH<<1)

The offset has to be 4096 aligned - it imposes an upper limit on how
many low bits we can use for flags. When we exceed it, it probably past
time for a params struct!

>  int __must_check
>  i915_gem_object_pin(struct drm_i915_gem_object *obj,
>                   struct i915_address_space *vm,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index eeea748..8aa0189 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3718,6 +3718,8 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object 
> *obj,
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       u32 fence_alignment, unfenced_alignment;
>       u64 size, fence_size;
> +     u32 search_flag = DRM_MM_SEARCH_DEFAULT;
> +     u32 alloc_flag = DRM_MM_CREATE_DEFAULT;
>       u64 start =
>               flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0;
>       u64 end =
> @@ -3759,6 +3761,17 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object 
> *obj,
>                                                  obj->tiling_mode,
>                                                  false);
>               size = flags & PIN_MAPPABLE ? fence_size : obj->base.size;
> +
> +             if (flags & PIN_HIGH) {
> +                     search_flag = DRM_MM_SEARCH_BELOW;
> +                     alloc_flag = DRM_MM_CREATE_TOP;
> +             }
> +
> +             /* Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset,
> +              * limit address to the first 4GBs for flagged objects.
> +              */

This note is best next to where we set PIN_ZONE_4G in execbuffer.

> +             if (flags & PIN_ZONE_4G)
> +                     end = (1ULL << 32);
>       }
>  
>       if (alignment == 0)

> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 600db74..f52b736 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -588,11 +588,17 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
>       if (entry->flags & EXEC_OBJECT_NEEDS_GTT)
>               flags |= PIN_GLOBAL;
>  
> +     flags |= PIN_ZONE_4G;
> +     if (entry->flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS)
> +             flags &= ~PIN_ZONE_4G;
> +
>       if (!drm_mm_node_allocated(&vma->node)) {
>               if (entry->flags & __EXEC_OBJECT_NEEDS_MAP)
>                       flags |= PIN_GLOBAL | PIN_MAPPABLE;
>               if (entry->flags & __EXEC_OBJECT_NEEDS_BIAS)
>                       flags |= BATCH_OFFSET_BIAS | PIN_OFFSET_BIAS;
> +             if ((flags & PIN_MAPPABLE) == 0)
> +                     flags |= PIN_HIGH;

I'm still debating the right semantics to use, but I'm happy with this
until I can find something better. (The biggest issue is that drm_mm is
not indexed for fast top-down searching. The current search code I put
into drm_mm is unfortunately broken, the idea I have in mind to fix it is
to add a hole_list into drm_mm/drm_mm_node, so that we can just walk
holes in up/down, recent/old order. And with that allocating top-down
will not be any more expensive than the current reuse recent hole -
though perhaps given a fragment drm_mm the hole stack probably requires
fewer steps to find a large hole.)

Other than don't touch PIN_OFFSET_MASK and move the w/a note next to
where we tweak the PIN_ZONE_4G, it lgtm, so with those changes,

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

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to