| Message ID | 1435764453-11954-17-git-send-email-michel.thierry@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <chris@chris-wilson.co.uk> > Cc: Ben Widawsky <ben@bwidawsk.net> > Signed-off-by: Michel Thierry <michel.thierry@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 <chris@chris-wilson.co.uk> -Chris
On 7/1/2015 4:43 PM, Chris Wilson wrote: > On Wed, Jul 01, 2015 at 04:27:32PM +0100, Michel Thierry wrote: >> >> + 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, Also the comment should be "limit address to the first 4GBs for _unflagged_ objects". I didn't update it after changing the logic. I'll resend with these changes. Thanks, > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > -Chris >
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) 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. + */ + if (flags & PIN_ZONE_4G) + end = (1ULL << 32); } if (alignment == 0) @@ -3801,8 +3814,8 @@ search_free: size, alignment, obj->cache_level, start, end, - DRM_MM_SEARCH_DEFAULT, - DRM_MM_CREATE_DEFAULT); + search_flag, + alloc_flag); if (ret) { ret = i915_gem_evict_something(dev, vm, size, alignment, obj->cache_level, 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; } ret = i915_gem_object_pin(obj, vma->vm, entry->alignment, flags); @@ -670,6 +676,10 @@ eb_vma_misplaced(struct i915_vma *vma) if (entry->flags & __EXEC_OBJECT_NEEDS_MAP && !obj->map_and_fenceable) return !only_mappable_for_reloc(entry->flags); + if (!(entry->flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS) && + (vma->node.start + vma->node.size) >= (1ULL << 32)) + return true; + return false; } diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index f88cc1c..b91cf45 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -685,7 +685,8 @@ struct drm_i915_gem_exec_object2 { #define EXEC_OBJECT_NEEDS_FENCE (1<<0) #define EXEC_OBJECT_NEEDS_GTT (1<<1) #define EXEC_OBJECT_WRITE (1<<2) -#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_WRITE<<1) +#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3) +#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_SUPPORTS_48B_ADDRESS<<1) __u64 flags; __u64 rsvd1;
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 <chris@chris-wilson.co.uk> Cc: Ben Widawsky <ben@bwidawsk.net> Signed-off-by: Michel Thierry <michel.thierry@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(-)