[Intel-gfx] [PATCH v5 17/19] drm/i915: Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset

[Michel Thierry]

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 at chris-wilson.co.uk>
>>> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk> (v4)
>>> Signed-off-by: Michel Thierry <michel.thierry at intel.com>
>>> ---
>>>   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.