[Intel-gfx] [PATCH v2 17/18] drm/i915: Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset

[Chris Wilson]

On Wed, Jun 10, 2015 at 05:46:54PM +0100, Michel Thierry wrote:
> There are some allocations that must be only referenced by 32bit
> 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
> 
> User must pass I915_EXEC_SUPPORTS_48BADDRESS flag to indicate it can
> be allocated above the 32b address range.

This should be a per-object flag not per-execbuffer.
 
> The flag is ignored in 32b PPGTT.
> 
> v2: Changed flag logic from neeeds_32b, to supports_48b.
> 
> Signed-off-by: Michel Thierry <michel.thierry at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h            |  1 +
>  drivers/gpu/drm/i915/i915_gem.c            | 19 ++++++++++++++--
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 36 +++++++++++++++++++++---------
>  include/uapi/drm/i915_drm.h                |  4 +++-
>  4 files changed, 47 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 95f59d3..d73dddb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2714,6 +2714,7 @@ 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_FULL_RANGE	(1<<6)

Halfway through our flags. We should get around to putting a reminder
that 1<<11 is the last flag.

>  #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 35690ef..e6325a4a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3672,6 +3672,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 =
> @@ -3713,6 +3715,19 @@ 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;
> +
> +		/* Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset,
> +		 * limit address to 4GB-1 for objects requiring this wa; for
> +		 * others, set alloc flag to TOP.
> +		 */
> +		if (USES_FULL_48BIT_PPGTT(dev)) {
> +			if (flags & PIN_FULL_RANGE) {
> +				search_flag = DRM_MM_SEARCH_BELOW;
> +				alloc_flag = DRM_MM_CREATE_TOP;
> +			} else {
> +				end = ((4ULL << GEN8_PDPE_SHIFT) - 1);

Looking at this, I think this is better as two flags. I have used
SEARCH_BELOW in the past to try and keep objects out of the mappable
aperture. Having that as separate flag is quite useful in its own right.

Then the second flag is PIN_BELOW_4G which we can set by default (and
cleared when the user specifies EXEC_OBJECT_48BIT). Not so sure, but I
think with the right combination of flags you can avoid having device
and w/a specific logic here. (This should be mechanism, push the policy
out to the boundary, preferrably into userspace.)

All the i915_gem_execbuffer changes are wrong due to the flag not being
on the object.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre