On Tue, 11 May 2021 at 14:26, Thomas Hellström
<thomas.hellstrom@xxxxxxxxxxxxxxx> wrote:
>
> Since objects can be migrated or evicted when not pinned or locked,
> update the checks for lmem residency or future residency so that
> the value returned is not immediately stale.
>
> Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 2 +-
> drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 42 +++++++++++++++++++-
> drivers/gpu/drm/i915/gem/i915_gem_object.c | 29 ++++++++++++++
> drivers/gpu/drm/i915/gem/i915_gem_object.h | 4 ++
> 4 files changed, 75 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index de1f13d203b5..b95def2d5af3 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -11615,7 +11615,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
>
> /* object is backed with LMEM for discrete */
> i915 = to_i915(obj->base.dev);
> - if (HAS_LMEM(i915) && !i915_gem_object_is_lmem(obj)) {
> + if (HAS_LMEM(i915) && !i915_gem_object_validates_to_lmem(obj)) {
> /* object is "remote", not in local memory */
> i915_gem_object_put(obj);
> return ERR_PTR(-EREMOTE);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> index 2b8cd15de1d9..d539dffa1554 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> @@ -23,10 +23,50 @@ i915_gem_object_lmem_io_map(struct drm_i915_gem_object *obj,
> return io_mapping_map_wc(&obj->mm.region->iomap, offset, size);
> }
>
> +/**
> + * i915_gem_object_validates_to_lmem - Whether the object is resident in
> + * lmem when pages are present.
> + * @obj: The object to check.
> + *
> + * Migratable objects residency may change from under us if the object is
> + * not pinned or locked. This function is intended to be used to check whether
> + * the object can only reside in lmem when pages are present.
> + *
> + * Return: Whether the object is always resident in lmem when pages are
> + * present.
> + */
> +bool i915_gem_object_validates_to_lmem(struct drm_i915_gem_object *obj)
> +{
> + struct intel_memory_region *mr = READ_ONCE(obj->mm.region);
> +
> + return !i915_gem_object_migratable(obj) &&
> + mr && (mr->type == INTEL_MEMORY_LOCAL ||
> + mr->type == INTEL_MEMORY_STOLEN_LOCAL);
> +}
> +
> +/**
> + * i915_gem_object_is_lmem - Whether the object is resident in
> + * lmem
> + * @obj: The object to check.
> + *
> + * Even if an object is allowed to migrate and change memory region,
> + * this function checks whether it will always be present in lmem when
> + * valid *or* if that's not the case, whether it's currently resident in lmem.
> + * For migratable and evictable objects, the latter only makes sense when
> + * the object is locked.
> + *
> + * Return: Whether the object migratable but resident in lmem, or not
> + * migratable and will be present in lmem when valid.
> + */
> bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj)
> {
> - struct intel_memory_region *mr = obj->mm.region;
> + struct intel_memory_region *mr = READ_ONCE(obj->mm.region);
>
> +#ifdef CONFIG_LOCKDEP
> + if (i915_gem_object_migratable(obj) &&
> + i915_gem_object_evictable(obj))
> + assert_object_held(obj);
> +#endif
> return mr && (mr->type == INTEL_MEMORY_LOCAL ||
> mr->type == INTEL_MEMORY_STOLEN_LOCAL);
> }
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index c53488f391dd..0475b1c94454 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -458,6 +458,35 @@ bool i915_gem_object_evictable(struct drm_i915_gem_object *obj)
> return pin_count == 0;
> }
>
> +/**
> + * i915_gem_object_migratable - Whether the object is migratable out of the
> + * current region.
> + * @obj: Pointer to the object.
> + *
> + * Return: Whether the object is allowed to be resident in other
> + * regions than the current while pages are present.
> + */
> +bool i915_gem_object_migratable(struct drm_i915_gem_object *obj)
> +{
> + struct intel_memory_region *mr = READ_ONCE(obj->mm.region);
> + struct intel_memory_region *placement;
> + int i;
> +
> + if (!mr)
> + return false;
> +
> + if (!obj->mm.n_placements)
> + return false;
> +
> + for (i = 0; i < obj->mm.n_placements; ++i) {
> + placement = obj->mm.placements[i];
> + if (placement != mr)
> + return true;
> + }
Maybe this can simply be:
return obj->mm.n_placements > 1;
?
The uAPI guarantees that mm.placements are each unique.
> +
> + return false;
> +}
> +
> void i915_gem_init__objects(struct drm_i915_private *i915)
> {
> INIT_WORK(&i915->mm.free_work, __i915_gem_free_work);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index ae5930e307d5..a3ad8cf4eefd 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -596,6 +596,10 @@ void __i915_gem_free_object(struct drm_i915_gem_object *obj);
>
> bool i915_gem_object_evictable(struct drm_i915_gem_object *obj);
>
> +bool i915_gem_object_migratable(struct drm_i915_gem_object *obj);
> +
> +bool i915_gem_object_validates_to_lmem(struct drm_i915_gem_object *obj);
> +
> #ifdef CONFIG_MMU_NOTIFIER
> static inline bool
> i915_gem_object_is_userptr(struct drm_i915_gem_object *obj)
> --
> 2.30.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
