Re: [PATCH v5 17/19] drm/i915: Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/27/2015 3:46 PM, Chris Wilson wrote:
On Mon, Jul 27, 2015 at 08:04:50PM +0530, Goel, Akash wrote:


On 7/16/2015 3:03 PM, 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 Instruction 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)
v5: Don't touch PIN_OFFSET_MASK and update workaround comment (Chris)
v6: Apply pin-high for ggtt too (Chris)

Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> (v4)
Signed-off-by: Michel Thierry <michel.thierry@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_drv.h            |  2 ++
  drivers/gpu/drm/i915/i915_gem.c            | 14 ++++++++++++--
  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 13 +++++++++++++
  include/uapi/drm/i915_drm.h                |  3 ++-
  4 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1dbbbf0..f79cc7b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2771,6 +2771,8 @@ 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_ZONE_4G	(1<<6)
+#define PIN_HIGH	(1<<7)
  #define PIN_OFFSET_MASK (~4095)
  int __must_check
  i915_gem_object_pin(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 76b7612..cd7e4b6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3728,6 +3728,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 =
@@ -3771,6 +3773,14 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
  		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;
+	}
+
+	if (flags & PIN_ZONE_4G)
+		end = (1ULL << 32);

Would this be fine for platforms, where only 2 GB of GGTT space is
available ? For GEN7 & older platforms, only GGTT would be used.
Shouldn't this check for PIN_ZONE_4G flag, be done only for PPGTT vm ?
For GGTT we have to obey the PIN_MAPPABLE flag, if set then 'end'
will be 256 MB.
If both PIN_MAPPABLE & PIN_ZONE_4G flags are set, the 'end' should
still be 256 MB, for GGTT vm.
So we need to mindful in defining the 'end' for platforms where
only GGTT would be used.

Ah, indeed. I didn't notice since this is end = min(end, 1<<32) in my
tree.

Thanks, I'll resend with this fix.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux