Re: [Intel-gfx] [PATCH v4 5/5] Revert "drm/i915: Improve on suspend / resume time with VT-d enabled"
On 30/11/2022 23:58, Andi Shyti wrote:
This reverts commit 2ef6efa79fecd5e3457b324155d35524d95f2b6b.
Checking the presence if the IRST (Intel Rapid Start Technology)
through the ACPI to decide whether to rebuild or not the GGTT
puts us at the mercy of the boot firmware and we need to
unnecessarily rely on third parties.
Because now we avoid adding scratch pages to the entire GGTT we
don't need this hack anymore.
Signed-off-by: Andi Shyti
Cc: Thomas Hellström
Cc: Chris Wilson
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 69 ++--
drivers/gpu/drm/i915/gt/intel_gtt.h | 24 --
drivers/gpu/drm/i915/i915_driver.c | 16 ---
3 files changed, 13 insertions(+), 96 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index fa96d925c2596..5896ac44010b0 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -27,13 +27,6 @@
#include "intel_gtt.h"
#include "gen8_ppgtt.h"
-static inline bool suspend_retains_ptes(struct i915_address_space *vm)
-{
- return GRAPHICS_VER(vm->i915) >= 8 &&
- !HAS_LMEM(vm->i915) &&
- vm->is_ggtt;
-}
-
static void i915_ggtt_color_adjust(const struct drm_mm_node *node,
unsigned long color,
u64 *start,
@@ -105,23 +98,6 @@ int i915_ggtt_init_hw(struct drm_i915_private *i915)
return 0;
}
-/*
- * Return the value of the last GGTT pte cast to an u64, if
- * the system is supposed to retain ptes across resume. 0 otherwise.
- */
-static u64 read_last_pte(struct i915_address_space *vm)
-{
- struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
- gen8_pte_t __iomem *ptep;
-
- if (!suspend_retains_ptes(vm))
- return 0;
-
- GEM_BUG_ON(GRAPHICS_VER(vm->i915) < 8);
- ptep = (typeof(ptep))ggtt->gsm + (ggtt_total_entries(ggtt) - 1);
- return readq(ptep);
-}
-
/**
* i915_ggtt_suspend_vm - Suspend the memory mappings for a GGTT or DPT VM
* @vm: The VM to suspend the mappings for
@@ -185,10 +161,7 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
i915_gem_object_unlock(obj);
}
- if (!suspend_retains_ptes(vm))
- vm->clear_range(vm, 0, vm->total);
- else
- i915_vm_to_ggtt(vm)->probed_pte = read_last_pte(vm);
+ vm->clear_range(vm, 0, vm->total);
vm->skip_pte_rewrite = save_skip_rewrite;
@@ -545,8 +518,6 @@ static int init_ggtt(struct i915_ggtt *ggtt)
struct drm_mm_node *entry;
int ret;
- ggtt->pte_lost = true;
-
/*
* GuC requires all resources that we're sharing with it to be placed in
* non-WOPCM memory. If GuC is not present or not in use we still need a
@@ -1263,20 +1234,11 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
{
struct i915_vma *vma;
bool write_domain_objs = false;
- bool retained_ptes;
drm_WARN_ON(&vm->i915->drm, !vm->is_ggtt && !vm->is_dpt);
- /*
-* First fill our portion of the GTT with scratch pages if
-* they were not retained across suspend.
-*/
- retained_ptes = suspend_retains_ptes(vm) &&
- !i915_vm_to_ggtt(vm)->pte_lost &&
- !GEM_WARN_ON(i915_vm_to_ggtt(vm)->probed_pte !=
read_last_pte(vm));
-
- if (!retained_ptes)
- vm->clear_range(vm, 0, vm->total);
+ /* First fill our portion of the GTT with scratch pages */
+ vm->clear_range(vm, 0, vm->total);
/* clflush objects bound into the GGTT and rebind them. */
list_for_each_entry(vma, &vm->bound_list, vm_link) {
@@ -1285,16 +1247,16 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
atomic_read(&vma->flags) & I915_VMA_BIND_MASK;
GEM_BUG_ON(!was_bound);
- if (!retained_ptes) {
- /*
-* Clear the bound flags of the vma resource to allow
-* ptes to be repopulated.
-*/
- vma->resource->bound_flags = 0;
- vma->ops->bind_vma(vm, NULL, vma->resource,
- obj ? obj->cache_level : 0,
- was_bound);
- }
+
+ /*
+* Clear the bound flags of the vma resource to allow
+* ptes to be repopulated.
+*/
+ vma->resource->bound_flags = 0;
+ vma->ops->bind_vma(vm, NULL, vma->resource,
+ obj ? obj->cache_level : 0,
+ was_bound);
+
if (obj) { /* only used during resume => exclusive access */
write_domain_objs |= fetch_and_zero(&obj->write_domain);
obj->read_domains
[Intel-gfx] [PATCH v4 5/5] Revert "drm/i915: Improve on suspend / resume time with VT-d enabled"
This reverts commit 2ef6efa79fecd5e3457b324155d35524d95f2b6b.
Checking the presence if the IRST (Intel Rapid Start Technology)
through the ACPI to decide whether to rebuild or not the GGTT
puts us at the mercy of the boot firmware and we need to
unnecessarily rely on third parties.
Because now we avoid adding scratch pages to the entire GGTT we
don't need this hack anymore.
Signed-off-by: Andi Shyti
Cc: Thomas Hellström
Cc: Chris Wilson
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 69 ++--
drivers/gpu/drm/i915/gt/intel_gtt.h | 24 --
drivers/gpu/drm/i915/i915_driver.c | 16 ---
3 files changed, 13 insertions(+), 96 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index fa96d925c2596..5896ac44010b0 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -27,13 +27,6 @@
#include "intel_gtt.h"
#include "gen8_ppgtt.h"
-static inline bool suspend_retains_ptes(struct i915_address_space *vm)
-{
- return GRAPHICS_VER(vm->i915) >= 8 &&
- !HAS_LMEM(vm->i915) &&
- vm->is_ggtt;
-}
-
static void i915_ggtt_color_adjust(const struct drm_mm_node *node,
unsigned long color,
u64 *start,
@@ -105,23 +98,6 @@ int i915_ggtt_init_hw(struct drm_i915_private *i915)
return 0;
}
-/*
- * Return the value of the last GGTT pte cast to an u64, if
- * the system is supposed to retain ptes across resume. 0 otherwise.
- */
-static u64 read_last_pte(struct i915_address_space *vm)
-{
- struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
- gen8_pte_t __iomem *ptep;
-
- if (!suspend_retains_ptes(vm))
- return 0;
-
- GEM_BUG_ON(GRAPHICS_VER(vm->i915) < 8);
- ptep = (typeof(ptep))ggtt->gsm + (ggtt_total_entries(ggtt) - 1);
- return readq(ptep);
-}
-
/**
* i915_ggtt_suspend_vm - Suspend the memory mappings for a GGTT or DPT VM
* @vm: The VM to suspend the mappings for
@@ -185,10 +161,7 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
i915_gem_object_unlock(obj);
}
- if (!suspend_retains_ptes(vm))
- vm->clear_range(vm, 0, vm->total);
- else
- i915_vm_to_ggtt(vm)->probed_pte = read_last_pte(vm);
+ vm->clear_range(vm, 0, vm->total);
vm->skip_pte_rewrite = save_skip_rewrite;
@@ -545,8 +518,6 @@ static int init_ggtt(struct i915_ggtt *ggtt)
struct drm_mm_node *entry;
int ret;
- ggtt->pte_lost = true;
-
/*
* GuC requires all resources that we're sharing with it to be placed in
* non-WOPCM memory. If GuC is not present or not in use we still need a
@@ -1263,20 +1234,11 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
{
struct i915_vma *vma;
bool write_domain_objs = false;
- bool retained_ptes;
drm_WARN_ON(&vm->i915->drm, !vm->is_ggtt && !vm->is_dpt);
- /*
-* First fill our portion of the GTT with scratch pages if
-* they were not retained across suspend.
-*/
- retained_ptes = suspend_retains_ptes(vm) &&
- !i915_vm_to_ggtt(vm)->pte_lost &&
- !GEM_WARN_ON(i915_vm_to_ggtt(vm)->probed_pte !=
read_last_pte(vm));
-
- if (!retained_ptes)
- vm->clear_range(vm, 0, vm->total);
+ /* First fill our portion of the GTT with scratch pages */
+ vm->clear_range(vm, 0, vm->total);
/* clflush objects bound into the GGTT and rebind them. */
list_for_each_entry(vma, &vm->bound_list, vm_link) {
@@ -1285,16 +1247,16 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
atomic_read(&vma->flags) & I915_VMA_BIND_MASK;
GEM_BUG_ON(!was_bound);
- if (!retained_ptes) {
- /*
-* Clear the bound flags of the vma resource to allow
-* ptes to be repopulated.
-*/
- vma->resource->bound_flags = 0;
- vma->ops->bind_vma(vm, NULL, vma->resource,
- obj ? obj->cache_level : 0,
- was_bound);
- }
+
+ /*
+* Clear the bound flags of the vma resource to allow
+* ptes to be repopulated.
+*/
+ vma->resource->bound_flags = 0;
+ vma->ops->bind_vma(vm, NULL, vma->resource,
+ obj ? obj->cache_level : 0,
+ was_bound);
+
if (obj) { /* only used during resume => exclusive access */
write_domain_objs |= fetch_and_zero(&obj->write_domain);
obj->read_domains |= I915_GEM_DOMAIN_GTT;
@@ -
