diff mbox series

[v3,12/15] drm/i915/create: apply ALLOC_GPU_ONLY by default

Message ID 20220211113437.874691-13-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show
Series Initial support for small BAR recovery | expand

Commit Message

Matthew Auld Feb. 11, 2022, 11:34 a.m. UTC
Starting from DG2+, when dealing with LMEM, we assume that by default
all userspace allocations should be placed in the non-mappable portion
of LMEM.  Note that dumb buffers are not included here, since these are
not "GPU accelerated" and likely need CPU access. We choose to just
always set GPU_ONLY, and let the backend figure out if that should be
ignored on discrete devices.

In a later patch userspace will be able to provide a hint if CPU access
to the buffer is needed.

v2(Thomas)
 - Apply GPU_ONLY on all discrete devices, but only if the BO can be
   placed in LMEM. Down in the depths this should be turned into a noop,
   where required, and as an annotation it still make some sense. If we
   apply it regardless of the placements then we end up needing to check
   the placements during exec capture. Also it's slightly inconsistent
   since the NEEDS_CPU_ACCESS can only be applied on objects that can be
   placed in LMEM. The other annoyance would be gem_create_ext vs plain
   gem_create, if we were to always apply GPU_ONLY.

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_create.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Thomas Hellström Feb. 11, 2022, 12:48 p.m. UTC | #1
On 2/11/22 12:34, Matthew Auld wrote:
> Starting from DG2+, when dealing with LMEM, we assume that by default
> all userspace allocations should be placed in the non-mappable portion
> of LMEM.  Note that dumb buffers are not included here, since these are
> not "GPU accelerated" and likely need CPU access. We choose to just
> always set GPU_ONLY, and let the backend figure out if that should be
> ignored on discrete devices.
>
> In a later patch userspace will be able to provide a hint if CPU access
> to the buffer is needed.
>
> v2(Thomas)
>   - Apply GPU_ONLY on all discrete devices, but only if the BO can be
>     placed in LMEM. Down in the depths this should be turned into a noop,
>     where required, and as an annotation it still make some sense. If we
>     apply it regardless of the placements then we end up needing to check
>     the placements during exec capture. Also it's slightly inconsistent
>     since the NEEDS_CPU_ACCESS can only be applied on objects that can be
>     placed in LMEM. The other annoyance would be gem_create_ext vs plain
>     gem_create, if we were to always apply GPU_ONLY.
>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>


> ---
>   drivers/gpu/drm/i915/gem/i915_gem_create.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c
> index 9402d4bf4ffc..ecb8c2feec46 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
> @@ -424,6 +424,14 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data,
>   		ext_data.n_placements = 1;
>   	}
>   
> +	/*
> +	 * TODO: add a userspace hint to force CPU_ACCESS for the object, which
> +	 * can override this.
> +	 */
> +	if (ext_data.n_placements > 1 ||
> +	    ext_data.placements[0]->type != INTEL_MEMORY_SYSTEM)
> +		ext_data.flags |= I915_BO_ALLOC_GPU_ONLY;
> +
>   	obj = __i915_gem_object_create_user_ext(i915, args->size,
>   						ext_data.placements,
>   						ext_data.n_placements,
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index 9402d4bf4ffc..ecb8c2feec46 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -424,6 +424,14 @@  i915_gem_create_ext_ioctl(struct drm_device *dev, void *data,
 		ext_data.n_placements = 1;
 	}
 
+	/*
+	 * TODO: add a userspace hint to force CPU_ACCESS for the object, which
+	 * can override this.
+	 */
+	if (ext_data.n_placements > 1 ||
+	    ext_data.placements[0]->type != INTEL_MEMORY_SYSTEM)
+		ext_data.flags |= I915_BO_ALLOC_GPU_ONLY;
+
 	obj = __i915_gem_object_create_user_ext(i915, args->size,
 						ext_data.placements,
 						ext_data.n_placements,