Commit 828c7908 authored by Ben Widawsky's avatar Ben Widawsky Committed by Daniel Vetter

drm/i915: Disable GGTT PTEs on GEN6+ suspend

Once the machine gets to a certain point in the suspend process, we
expect the GPU to be idle. If it is not, we might corrupt memory.
Empirically (with an early version of this patch) we have seen this is
not the case. We cannot currently explain why the latent GPU writes
occur.

In the technical sense, this patch is a workaround in that we have an
issue we can't explain, and the patch indirectly solves the issue.
However, it's really better than a workaround because we understand why
it works, and it really should be a safe thing to do in all cases.

The noticeable effect other than the debug messages would be an increase
in the suspend time. I have not measure how expensive it actually is.

I think it would be good to spend further time to root cause why we're
seeing these latent writes, but it shouldn't preclude preventing the
fallout.

NOTE: It should be safe (and makes some sense IMO) to also keep the
VALID bit unset on resume when we clear_range(). I've opted not to do
this as properly clearing those bits at some later point would be extra
work.

v2: Fix bugzilla link

Bugzilla: http://bugs.freedesktop.org/show_bug.cgi?id=65496
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=59321Tested-by: default avatarTakashi Iwai <tiwai@suse.de>
Tested-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: default avatarBen Widawsky <ben@bwidawsk.net>
Tested-By: default avatarTodd Previte <tprevite@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent b35b380e
...@@ -505,6 +505,8 @@ static int i915_drm_freeze(struct drm_device *dev) ...@@ -505,6 +505,8 @@ static int i915_drm_freeze(struct drm_device *dev)
intel_modeset_suspend_hw(dev); intel_modeset_suspend_hw(dev);
} }
i915_gem_suspend_gtt_mappings(dev);
i915_save_state(dev); i915_save_state(dev);
intel_opregion_fini(dev); intel_opregion_fini(dev);
...@@ -648,7 +650,8 @@ static int i915_drm_thaw(struct drm_device *dev) ...@@ -648,7 +650,8 @@ static int i915_drm_thaw(struct drm_device *dev)
mutex_lock(&dev->struct_mutex); mutex_lock(&dev->struct_mutex);
i915_gem_restore_gtt_mappings(dev); i915_gem_restore_gtt_mappings(dev);
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
} } else if (drm_core_check_feature(dev, DRIVER_MODESET))
i915_check_and_clear_faults(dev);
__i915_drm_thaw(dev); __i915_drm_thaw(dev);
......
...@@ -501,7 +501,8 @@ struct i915_address_space { ...@@ -501,7 +501,8 @@ struct i915_address_space {
bool valid); /* Create a valid PTE */ bool valid); /* Create a valid PTE */
void (*clear_range)(struct i915_address_space *vm, void (*clear_range)(struct i915_address_space *vm,
unsigned int first_entry, unsigned int first_entry,
unsigned int num_entries); unsigned int num_entries,
bool use_scratch);
void (*insert_entries)(struct i915_address_space *vm, void (*insert_entries)(struct i915_address_space *vm,
struct sg_table *st, struct sg_table *st,
unsigned int first_entry, unsigned int first_entry,
...@@ -2066,6 +2067,8 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt, ...@@ -2066,6 +2067,8 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt, void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
struct drm_i915_gem_object *obj); struct drm_i915_gem_object *obj);
void i915_check_and_clear_faults(struct drm_device *dev);
void i915_gem_suspend_gtt_mappings(struct drm_device *dev);
void i915_gem_restore_gtt_mappings(struct drm_device *dev); void i915_gem_restore_gtt_mappings(struct drm_device *dev);
int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj); int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj);
void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj, void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
......
...@@ -241,7 +241,8 @@ static int gen6_ppgtt_enable(struct drm_device *dev) ...@@ -241,7 +241,8 @@ static int gen6_ppgtt_enable(struct drm_device *dev)
/* PPGTT support for Sandybdrige/Gen6 and later */ /* PPGTT support for Sandybdrige/Gen6 and later */
static void gen6_ppgtt_clear_range(struct i915_address_space *vm, static void gen6_ppgtt_clear_range(struct i915_address_space *vm,
unsigned first_entry, unsigned first_entry,
unsigned num_entries) unsigned num_entries,
bool use_scratch)
{ {
struct i915_hw_ppgtt *ppgtt = struct i915_hw_ppgtt *ppgtt =
container_of(vm, struct i915_hw_ppgtt, base); container_of(vm, struct i915_hw_ppgtt, base);
...@@ -372,7 +373,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt) ...@@ -372,7 +373,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
} }
ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.clear_range(&ppgtt->base, 0,
ppgtt->num_pd_entries * I915_PPGTT_PT_ENTRIES); ppgtt->num_pd_entries * I915_PPGTT_PT_ENTRIES, true);
ppgtt->pd_offset = first_pd_entry_in_global_pt * sizeof(gen6_gtt_pte_t); ppgtt->pd_offset = first_pd_entry_in_global_pt * sizeof(gen6_gtt_pte_t);
...@@ -449,7 +450,8 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt, ...@@ -449,7 +450,8 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
{ {
ppgtt->base.clear_range(&ppgtt->base, ppgtt->base.clear_range(&ppgtt->base,
i915_gem_obj_ggtt_offset(obj) >> PAGE_SHIFT, i915_gem_obj_ggtt_offset(obj) >> PAGE_SHIFT,
obj->base.size >> PAGE_SHIFT); obj->base.size >> PAGE_SHIFT,
true);
} }
extern int intel_iommu_gfx_mapped; extern int intel_iommu_gfx_mapped;
...@@ -490,15 +492,65 @@ static void undo_idling(struct drm_i915_private *dev_priv, bool interruptible) ...@@ -490,15 +492,65 @@ static void undo_idling(struct drm_i915_private *dev_priv, bool interruptible)
dev_priv->mm.interruptible = interruptible; dev_priv->mm.interruptible = interruptible;
} }
void i915_check_and_clear_faults(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_ring_buffer *ring;
int i;
if (INTEL_INFO(dev)->gen < 6)
return;
for_each_ring(ring, dev_priv, i) {
u32 fault_reg;
fault_reg = I915_READ(RING_FAULT_REG(ring));
if (fault_reg & RING_FAULT_VALID) {
DRM_DEBUG_DRIVER("Unexpected fault\n"
"\tAddr: 0x%08lx\\n"
"\tAddress space: %s\n"
"\tSource ID: %d\n"
"\tType: %d\n",
fault_reg & PAGE_MASK,
fault_reg & RING_FAULT_GTTSEL_MASK ? "GGTT" : "PPGTT",
RING_FAULT_SRCID(fault_reg),
RING_FAULT_FAULT_TYPE(fault_reg));
I915_WRITE(RING_FAULT_REG(ring),
fault_reg & ~RING_FAULT_VALID);
}
}
POSTING_READ(RING_FAULT_REG(&dev_priv->ring[RCS]));
}
void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
/* Don't bother messing with faults pre GEN6 as we have little
* documentation supporting that it's a good idea.
*/
if (INTEL_INFO(dev)->gen < 6)
return;
i915_check_and_clear_faults(dev);
dev_priv->gtt.base.clear_range(&dev_priv->gtt.base,
dev_priv->gtt.base.start / PAGE_SIZE,
dev_priv->gtt.base.total / PAGE_SIZE,
false);
}
void i915_gem_restore_gtt_mappings(struct drm_device *dev) void i915_gem_restore_gtt_mappings(struct drm_device *dev)
{ {
struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *obj; struct drm_i915_gem_object *obj;
i915_check_and_clear_faults(dev);
/* First fill our portion of the GTT with scratch pages */ /* First fill our portion of the GTT with scratch pages */
dev_priv->gtt.base.clear_range(&dev_priv->gtt.base, dev_priv->gtt.base.clear_range(&dev_priv->gtt.base,
dev_priv->gtt.base.start / PAGE_SIZE, dev_priv->gtt.base.start / PAGE_SIZE,
dev_priv->gtt.base.total / PAGE_SIZE); dev_priv->gtt.base.total / PAGE_SIZE,
true);
list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) { list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
i915_gem_clflush_object(obj, obj->pin_display); i915_gem_clflush_object(obj, obj->pin_display);
...@@ -565,7 +617,8 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm, ...@@ -565,7 +617,8 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
static void gen6_ggtt_clear_range(struct i915_address_space *vm, static void gen6_ggtt_clear_range(struct i915_address_space *vm,
unsigned int first_entry, unsigned int first_entry,
unsigned int num_entries) unsigned int num_entries,
bool use_scratch)
{ {
struct drm_i915_private *dev_priv = vm->dev->dev_private; struct drm_i915_private *dev_priv = vm->dev->dev_private;
gen6_gtt_pte_t scratch_pte, __iomem *gtt_base = gen6_gtt_pte_t scratch_pte, __iomem *gtt_base =
...@@ -578,7 +631,8 @@ static void gen6_ggtt_clear_range(struct i915_address_space *vm, ...@@ -578,7 +631,8 @@ static void gen6_ggtt_clear_range(struct i915_address_space *vm,
first_entry, num_entries, max_entries)) first_entry, num_entries, max_entries))
num_entries = max_entries; num_entries = max_entries;
scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, true); scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, use_scratch);
for (i = 0; i < num_entries; i++) for (i = 0; i < num_entries; i++)
iowrite32(scratch_pte, &gtt_base[i]); iowrite32(scratch_pte, &gtt_base[i]);
readl(gtt_base); readl(gtt_base);
...@@ -599,7 +653,8 @@ static void i915_ggtt_insert_entries(struct i915_address_space *vm, ...@@ -599,7 +653,8 @@ static void i915_ggtt_insert_entries(struct i915_address_space *vm,
static void i915_ggtt_clear_range(struct i915_address_space *vm, static void i915_ggtt_clear_range(struct i915_address_space *vm,
unsigned int first_entry, unsigned int first_entry,
unsigned int num_entries) unsigned int num_entries,
bool unused)
{ {
intel_gtt_clear_range(first_entry, num_entries); intel_gtt_clear_range(first_entry, num_entries);
} }
...@@ -627,7 +682,8 @@ void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj) ...@@ -627,7 +682,8 @@ void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj)
dev_priv->gtt.base.clear_range(&dev_priv->gtt.base, dev_priv->gtt.base.clear_range(&dev_priv->gtt.base,
entry, entry,
obj->base.size >> PAGE_SHIFT); obj->base.size >> PAGE_SHIFT,
true);
obj->has_global_gtt_mapping = 0; obj->has_global_gtt_mapping = 0;
} }
...@@ -714,11 +770,11 @@ void i915_gem_setup_global_gtt(struct drm_device *dev, ...@@ -714,11 +770,11 @@ void i915_gem_setup_global_gtt(struct drm_device *dev,
const unsigned long count = (hole_end - hole_start) / PAGE_SIZE; const unsigned long count = (hole_end - hole_start) / PAGE_SIZE;
DRM_DEBUG_KMS("clearing unused GTT space: [%lx, %lx]\n", DRM_DEBUG_KMS("clearing unused GTT space: [%lx, %lx]\n",
hole_start, hole_end); hole_start, hole_end);
ggtt_vm->clear_range(ggtt_vm, hole_start / PAGE_SIZE, count); ggtt_vm->clear_range(ggtt_vm, hole_start / PAGE_SIZE, count, true);
} }
/* And finally clear the reserved guard page */ /* And finally clear the reserved guard page */
ggtt_vm->clear_range(ggtt_vm, end / PAGE_SIZE - 1, 1); ggtt_vm->clear_range(ggtt_vm, end / PAGE_SIZE - 1, 1, true);
} }
static bool static bool
......
...@@ -604,6 +604,10 @@ ...@@ -604,6 +604,10 @@
#define ARB_MODE_SWIZZLE_IVB (1<<5) #define ARB_MODE_SWIZZLE_IVB (1<<5)
#define RENDER_HWS_PGA_GEN7 (0x04080) #define RENDER_HWS_PGA_GEN7 (0x04080)
#define RING_FAULT_REG(ring) (0x4094 + 0x100*(ring)->id) #define RING_FAULT_REG(ring) (0x4094 + 0x100*(ring)->id)
#define RING_FAULT_GTTSEL_MASK (1<<11)
#define RING_FAULT_SRCID(x) ((x >> 3) & 0xff)
#define RING_FAULT_FAULT_TYPE(x) ((x >> 1) & 0x3)
#define RING_FAULT_VALID (1<<0)
#define DONE_REG 0x40b0 #define DONE_REG 0x40b0
#define BSD_HWS_PGA_GEN7 (0x04180) #define BSD_HWS_PGA_GEN7 (0x04180)
#define BLT_HWS_PGA_GEN7 (0x04280) #define BLT_HWS_PGA_GEN7 (0x04280)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment