diff mbox series

[1/6] drm/i915/dpt: Treat the DPT BO as a framebuffer

Message ID 20230320090522.9909-2-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/dpt: Fix DPT+shmem combo and add i915.enable_dpt modparam | expand

Commit Message

Ville Syrjälä March 20, 2023, 9:05 a.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently i915_gem_object_is_framebuffer() doesn't treat the
BO containing the framebuffer's DPT as a framebuffer itself.
This means eg. that the shrinker can evict the DPT BO while
leaving the actual FB BO bound, when the DPT is allocated
from regular shmem.

That causes an immediate oops during hibernate as we
try to rewrite the PTEs inside the already evicted
DPT obj.

TODO: presumably this might also be the reason for the
DPT related display faults under heavy memory pressure,
but I'm still not sure how that would happen as the object
should be pinned by intel_dpt_pin() while in active use by
the display engine...

Cc: stable@vger.kernel.org
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Fixes: 0dc987b699ce ("drm/i915/display: Add smem fallback allocation for dpt")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpt.c         | 2 ++
 drivers/gpu/drm/i915/gem/i915_gem_object.h       | 2 +-
 drivers/gpu/drm/i915/gem/i915_gem_object_types.h | 3 +++
 3 files changed, 6 insertions(+), 1 deletion(-)

Comments

Matthew Auld March 20, 2023, 9:46 a.m. UTC | #1
On 20/03/2023 09:05, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently i915_gem_object_is_framebuffer() doesn't treat the
> BO containing the framebuffer's DPT as a framebuffer itself.
> This means eg. that the shrinker can evict the DPT BO while
> leaving the actual FB BO bound, when the DPT is allocated
> from regular shmem.
> 
> That causes an immediate oops during hibernate as we
> try to rewrite the PTEs inside the already evicted
> DPT obj.
> 
> TODO: presumably this might also be the reason for the
> DPT related display faults under heavy memory pressure,
> but I'm still not sure how that would happen as the object
> should be pinned by intel_dpt_pin() while in active use by
> the display engine...

Yeah that's strange, if it's pinned then it should not be evictable. 
Also with DG2, are there any similar issues, since lmem is used for dpt 
and that can be moved to smem and swapped-out? Keeping the object pinned 
should also prevent that.

> 
> Cc: stable@vger.kernel.org
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Fixes: 0dc987b699ce ("drm/i915/display: Add smem fallback allocation for dpt")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dpt.c         | 2 ++
>   drivers/gpu/drm/i915/gem/i915_gem_object.h       | 2 +-
>   drivers/gpu/drm/i915/gem/i915_gem_object_types.h | 3 +++
>   3 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
> index ad1a37b515fb..2a9f40a2b3ed 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpt.c
> @@ -301,6 +301,7 @@ intel_dpt_create(struct intel_framebuffer *fb)
>   	vm->pte_encode = gen8_ggtt_pte_encode;
>   
>   	dpt->obj = dpt_obj;
> +	dpt->obj->is_dpt = true;
>   
>   	return &dpt->vm;
>   }
> @@ -309,5 +310,6 @@ void intel_dpt_destroy(struct i915_address_space *vm)
>   {
>   	struct i915_dpt *dpt = i915_vm_to_dpt(vm);
>   
> +	dpt->obj->is_dpt = false;
>   	i915_vm_put(&dpt->vm);
>   }
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index f9a8acbba715..885ccde9dc3c 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -303,7 +303,7 @@ i915_gem_object_never_mmap(const struct drm_i915_gem_object *obj)
>   static inline bool
>   i915_gem_object_is_framebuffer(const struct drm_i915_gem_object *obj)
>   {
> -	return READ_ONCE(obj->frontbuffer);
> +	return READ_ONCE(obj->frontbuffer) || obj->is_dpt;
>   }
>   
>   static inline unsigned int
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> index 19c9bdd8f905..5dcbbef31d44 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> @@ -491,6 +491,9 @@ struct drm_i915_gem_object {
>   	 */
>   	unsigned int cache_dirty:1;
>   
> +	/* @is_dpt: Object houses a display page table (DPT) */
> +	unsigned int is_dpt:1;
> +
>   	/**
>   	 * @read_domains: Read memory domains.
>   	 *
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
index ad1a37b515fb..2a9f40a2b3ed 100644
--- a/drivers/gpu/drm/i915/display/intel_dpt.c
+++ b/drivers/gpu/drm/i915/display/intel_dpt.c
@@ -301,6 +301,7 @@  intel_dpt_create(struct intel_framebuffer *fb)
 	vm->pte_encode = gen8_ggtt_pte_encode;
 
 	dpt->obj = dpt_obj;
+	dpt->obj->is_dpt = true;
 
 	return &dpt->vm;
 }
@@ -309,5 +310,6 @@  void intel_dpt_destroy(struct i915_address_space *vm)
 {
 	struct i915_dpt *dpt = i915_vm_to_dpt(vm);
 
+	dpt->obj->is_dpt = false;
 	i915_vm_put(&dpt->vm);
 }
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index f9a8acbba715..885ccde9dc3c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -303,7 +303,7 @@  i915_gem_object_never_mmap(const struct drm_i915_gem_object *obj)
 static inline bool
 i915_gem_object_is_framebuffer(const struct drm_i915_gem_object *obj)
 {
-	return READ_ONCE(obj->frontbuffer);
+	return READ_ONCE(obj->frontbuffer) || obj->is_dpt;
 }
 
 static inline unsigned int
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 19c9bdd8f905..5dcbbef31d44 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -491,6 +491,9 @@  struct drm_i915_gem_object {
 	 */
 	unsigned int cache_dirty:1;
 
+	/* @is_dpt: Object houses a display page table (DPT) */
+	unsigned int is_dpt:1;
+
 	/**
 	 * @read_domains: Read memory domains.
 	 *