* [PATCH 1/5] drm/i915: move dev_priv->mm out of line
2012-11-12 22:07 [PATCH 0/5] robustify reset state transitions Daniel Vetter
@ 2012-11-12 22:07 ` Daniel Vetter
2012-11-12 22:07 ` [PATCH 2/5] drm/i915: extract hangcheck/reset/error_state state into substruct Daniel Vetter
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2012-11-12 22:07 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
That one is really big, since it contains tons of comments explaining
how things work. Which is nice ;-)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_drv.h | 212 ++++++++++++++++++++--------------------
1 file changed, 107 insertions(+), 105 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index db6d71c..1b0a464 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -612,6 +612,112 @@ struct intel_l3_parity {
struct work_struct error_work;
};
+struct i915_gem_mm {
+ /** Bridge to intel-gtt-ko */
+ struct intel_gtt *gtt;
+ /** Memory allocator for GTT stolen memory */
+ struct drm_mm stolen;
+ /** Memory allocator for GTT */
+ struct drm_mm gtt_space;
+ /** List of all objects in gtt_space. Used to restore gtt
+ * mappings on resume */
+ struct list_head bound_list;
+ /**
+ * List of objects which are not bound to the GTT (thus
+ * are idle and not used by the GPU) but still have
+ * (presumably uncached) pages still attached.
+ */
+ struct list_head unbound_list;
+
+ /** Usable portion of the GTT for GEM */
+ unsigned long gtt_start;
+ unsigned long gtt_mappable_end;
+ unsigned long gtt_end;
+
+ struct io_mapping *gtt_mapping;
+ phys_addr_t gtt_base_addr;
+ int gtt_mtrr;
+
+ /** PPGTT used for aliasing the PPGTT with the GTT */
+ struct i915_hw_ppgtt *aliasing_ppgtt;
+
+ struct shrinker inactive_shrinker;
+
+ /**
+ * List of objects currently involved in rendering.
+ *
+ * Includes buffers having the contents of their GPU caches
+ * flushed, not necessarily primitives. last_rendering_seqno
+ * represents when the rendering involved will be completed.
+ *
+ * A reference is held on the buffer while on this list.
+ */
+ struct list_head active_list;
+
+ /**
+ * LRU list of objects which are not in the ringbuffer and
+ * are ready to unbind, but are still in the GTT.
+ *
+ * last_rendering_seqno is 0 while an object is in this list.
+ *
+ * A reference is not held on the buffer while on this list,
+ * as merely being GTT-bound shouldn't prevent its being
+ * freed, and we'll pull it off the list in the free path.
+ */
+ struct list_head inactive_list;
+
+ /** LRU list of objects with fence regs on them. */
+ struct list_head fence_list;
+
+ /**
+ * We leave the user IRQ off as much as possible,
+ * but this means that requests will finish and never
+ * be retired once the system goes idle. Set a timer to
+ * fire periodically while the ring is running. When it
+ * fires, go retire requests.
+ */
+ struct delayed_work retire_work;
+
+ /**
+ * Are we in a non-interruptible section of code like
+ * modesetting?
+ */
+ bool interruptible;
+
+ /**
+ * Flag if the X Server, and thus DRM, is not currently in
+ * control of the device.
+ *
+ * This is set between LeaveVT and EnterVT. It needs to be
+ * replaced with a semaphore. It also needs to be
+ * transitioned away from for kernel modesetting.
+ */
+ int suspended;
+
+ /**
+ * Flag if the hardware appears to be wedged.
+ *
+ * This is set when attempts to idle the device timeout.
+ * It prevents command submission from occurring and makes
+ * every pending request fail
+ */
+ atomic_t wedged;
+
+ /** Bit 6 swizzling required for X tiling */
+ uint32_t bit_6_swizzle_x;
+ /** Bit 6 swizzling required for Y tiling */
+ uint32_t bit_6_swizzle_y;
+
+ /* storage for physical objects */
+ struct drm_i915_gem_phys_object *phys_objs[I915_MAX_PHYS_OBJECT];
+
+ /* accounting, useful for userland debugging */
+ size_t gtt_total;
+ size_t mappable_gtt_total;
+ size_t object_memory;
+ u32 object_count;
+};
+
typedef struct drm_i915_private {
struct drm_device *dev;
@@ -744,111 +850,7 @@ typedef struct drm_i915_private {
/* Register state */
bool modeset_on_lid;
- struct {
- /** Bridge to intel-gtt-ko */
- struct intel_gtt *gtt;
- /** Memory allocator for GTT stolen memory */
- struct drm_mm stolen;
- /** Memory allocator for GTT */
- struct drm_mm gtt_space;
- /** List of all objects in gtt_space. Used to restore gtt
- * mappings on resume */
- struct list_head bound_list;
- /**
- * List of objects which are not bound to the GTT (thus
- * are idle and not used by the GPU) but still have
- * (presumably uncached) pages still attached.
- */
- struct list_head unbound_list;
-
- /** Usable portion of the GTT for GEM */
- unsigned long gtt_start;
- unsigned long gtt_mappable_end;
- unsigned long gtt_end;
-
- struct io_mapping *gtt_mapping;
- phys_addr_t gtt_base_addr;
- int gtt_mtrr;
-
- /** PPGTT used for aliasing the PPGTT with the GTT */
- struct i915_hw_ppgtt *aliasing_ppgtt;
-
- struct shrinker inactive_shrinker;
-
- /**
- * List of objects currently involved in rendering.
- *
- * Includes buffers having the contents of their GPU caches
- * flushed, not necessarily primitives. last_rendering_seqno
- * represents when the rendering involved will be completed.
- *
- * A reference is held on the buffer while on this list.
- */
- struct list_head active_list;
-
- /**
- * LRU list of objects which are not in the ringbuffer and
- * are ready to unbind, but are still in the GTT.
- *
- * last_rendering_seqno is 0 while an object is in this list.
- *
- * A reference is not held on the buffer while on this list,
- * as merely being GTT-bound shouldn't prevent its being
- * freed, and we'll pull it off the list in the free path.
- */
- struct list_head inactive_list;
-
- /** LRU list of objects with fence regs on them. */
- struct list_head fence_list;
-
- /**
- * We leave the user IRQ off as much as possible,
- * but this means that requests will finish and never
- * be retired once the system goes idle. Set a timer to
- * fire periodically while the ring is running. When it
- * fires, go retire requests.
- */
- struct delayed_work retire_work;
-
- /**
- * Are we in a non-interruptible section of code like
- * modesetting?
- */
- bool interruptible;
-
- /**
- * Flag if the X Server, and thus DRM, is not currently in
- * control of the device.
- *
- * This is set between LeaveVT and EnterVT. It needs to be
- * replaced with a semaphore. It also needs to be
- * transitioned away from for kernel modesetting.
- */
- int suspended;
-
- /**
- * Flag if the hardware appears to be wedged.
- *
- * This is set when attempts to idle the device timeout.
- * It prevents command submission from occurring and makes
- * every pending request fail
- */
- atomic_t wedged;
-
- /** Bit 6 swizzling required for X tiling */
- uint32_t bit_6_swizzle_x;
- /** Bit 6 swizzling required for Y tiling */
- uint32_t bit_6_swizzle_y;
-
- /* storage for physical objects */
- struct drm_i915_gem_phys_object *phys_objs[I915_MAX_PHYS_OBJECT];
-
- /* accounting, useful for userland debugging */
- size_t gtt_total;
- size_t mappable_gtt_total;
- size_t object_memory;
- u32 object_count;
- } mm;
+ struct i915_gem_mm mm;
/* Kernel Modesetting */
--
1.7.11.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/5] drm/i915: extract hangcheck/reset/error_state state into substruct
2012-11-12 22:07 [PATCH 0/5] robustify reset state transitions Daniel Vetter
2012-11-12 22:07 ` [PATCH 1/5] drm/i915: move dev_priv->mm out of line Daniel Vetter
@ 2012-11-12 22:07 ` Daniel Vetter
2012-11-12 22:07 ` [PATCH 3/5] drm/i915: move wedged to the other gpu error handling stuff Daniel Vetter
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2012-11-12 22:07 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
This has been sprinkled all over the place in dev_priv. I think
it'd be good to also move all the code into a separate file like
i915_gem_error.c, but that's for another patch.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_debugfs.c | 10 +++---
drivers/gpu/drm/i915/i915_dma.c | 9 +++---
drivers/gpu/drm/i915/i915_drv.c | 8 ++---
drivers/gpu/drm/i915/i915_drv.h | 39 +++++++++++++----------
drivers/gpu/drm/i915/i915_gem.c | 10 +++---
drivers/gpu/drm/i915/i915_irq.c | 56 ++++++++++++++++++---------------
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
7 files changed, 73 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 4568e7d..276997a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -809,11 +809,11 @@ static int i915_error_state_open(struct inode *inode, struct file *file)
error_priv->dev = dev;
- spin_lock_irqsave(&dev_priv->error_lock, flags);
- error_priv->error = dev_priv->first_error;
+ spin_lock_irqsave(&dev_priv->gpu_error.lock, flags);
+ error_priv->error = dev_priv->gpu_error.first_error;
if (error_priv->error)
kref_get(&error_priv->error->ref);
- spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+ spin_unlock_irqrestore(&dev_priv->gpu_error.lock, flags);
return single_open(file, i915_error_state, error_priv);
}
@@ -1651,7 +1651,7 @@ i915_ring_stop_read(struct file *filp,
int len;
len = snprintf(buf, sizeof(buf),
- "0x%08x\n", dev_priv->stop_rings);
+ "0x%08x\n", dev_priv->gpu_error.stop_rings);
if (len > sizeof(buf))
len = sizeof(buf);
@@ -1687,7 +1687,7 @@ i915_ring_stop_write(struct file *filp,
if (ret)
return ret;
- dev_priv->stop_rings = val;
+ dev_priv->gpu_error.stop_rings = val;
mutex_unlock(&dev->struct_mutex);
return cnt;
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 1eea5be..c715317 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1610,7 +1610,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
pci_enable_msi(dev->pdev);
spin_lock_init(&dev_priv->irq_lock);
- spin_lock_init(&dev_priv->error_lock);
+ spin_lock_init(&dev_priv->gpu_error.lock);
spin_lock_init(&dev_priv->rps.lock);
spin_lock_init(&dev_priv->dpio_lock);
@@ -1644,7 +1644,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
intel_opregion_init(dev);
acpi_video_register();
- setup_timer(&dev_priv->hangcheck_timer, i915_hangcheck_elapsed,
+ setup_timer(&dev_priv->gpu_error.hangcheck_timer,
+ i915_hangcheck_elapsed,
(unsigned long) dev);
if (IS_GEN5(dev))
@@ -1733,8 +1734,8 @@ int i915_driver_unload(struct drm_device *dev)
}
/* Free error state after interrupts are fully disabled. */
- del_timer_sync(&dev_priv->hangcheck_timer);
- cancel_work_sync(&dev_priv->error_work);
+ del_timer_sync(&dev_priv->gpu_error.hangcheck_timer);
+ cancel_work_sync(&dev_priv->gpu_error.work);
i915_destroy_error_state(dev);
if (dev->pdev->msi_enabled)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f8ba5fe..9128e21 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -772,9 +772,9 @@ int intel_gpu_reset(struct drm_device *dev)
}
/* Also reset the gpu hangman. */
- if (dev_priv->stop_rings) {
+ if (dev_priv->gpu_error.stop_rings) {
DRM_DEBUG("Simulated gpu hang, resetting stop_rings\n");
- dev_priv->stop_rings = 0;
+ dev_priv->gpu_error.stop_rings = 0;
if (ret == -ENODEV) {
DRM_ERROR("Reset not implemented, but ignoring "
"error for simulated gpu hangs\n");
@@ -813,12 +813,12 @@ int i915_reset(struct drm_device *dev)
i915_gem_reset(dev);
ret = -ENODEV;
- if (get_seconds() - dev_priv->last_gpu_reset < 5)
+ if (get_seconds() - dev_priv->gpu_error.last_reset < 5)
DRM_ERROR("GPU hanging too fast, declaring wedged!\n");
else
ret = intel_gpu_reset(dev);
- dev_priv->last_gpu_reset = get_seconds();
+ dev_priv->gpu_error.last_reset = get_seconds();
if (ret) {
DRM_ERROR("Failed to reset chip.\n");
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1b0a464..03218f9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -718,6 +718,28 @@ struct i915_gem_mm {
u32 object_count;
};
+struct i915_gpu_error {
+ /* For hangcheck timer */
+#define DRM_I915_HANGCHECK_PERIOD 1500 /* in ms */
+#define DRM_I915_HANGCHECK_JIFFIES msecs_to_jiffies(DRM_I915_HANGCHECK_PERIOD)
+ struct timer_list hangcheck_timer;
+ int hangcheck_count;
+ uint32_t last_acthd[I915_NUM_RINGS];
+ uint32_t prev_instdone[I915_NUM_INSTDONE_REG];
+
+ /* For reset and error_state handling. */
+ spinlock_t lock;
+ /* Protected by the above dev->gpu_error.lock. */
+ struct drm_i915_error_state *first_error;
+ struct work_struct work;
+ struct completion completion;
+
+ unsigned long last_reset;
+
+ /* For gpu hang simulation. */
+ unsigned int stop_rings;
+};
+
typedef struct drm_i915_private {
struct drm_device *dev;
@@ -774,16 +796,6 @@ typedef struct drm_i915_private {
int num_pipe;
int num_pch_pll;
- /* For hangcheck timer */
-#define DRM_I915_HANGCHECK_PERIOD 1500 /* in ms */
-#define DRM_I915_HANGCHECK_JIFFIES msecs_to_jiffies(DRM_I915_HANGCHECK_PERIOD)
- struct timer_list hangcheck_timer;
- int hangcheck_count;
- uint32_t last_acthd[I915_NUM_RINGS];
- uint32_t prev_instdone[I915_NUM_INSTDONE_REG];
-
- unsigned int stop_rings;
-
unsigned long cfb_size;
unsigned int cfb_fb;
enum plane cfb_plane;
@@ -832,11 +844,6 @@ typedef struct drm_i915_private {
unsigned int fsb_freq, mem_freq, is_ddr3;
- spinlock_t error_lock;
- /* Protected by dev->error_lock. */
- struct drm_i915_error_state *first_error;
- struct work_struct error_work;
- struct completion error_completion;
struct workqueue_struct *wq;
/* Display functions */
@@ -892,7 +899,7 @@ typedef struct drm_i915_private {
struct drm_mm_node *compressed_fb;
struct drm_mm_node *compressed_llb;
- unsigned long last_gpu_reset;
+ struct i915_gpu_error gpu_error;
/* list of fbdev register on this device */
struct intel_fbdev *fbdev;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cdcf19d..6e29bed 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -90,7 +90,7 @@ static int
i915_gem_wait_for_error(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct completion *x = &dev_priv->error_completion;
+ struct completion *x = &dev_priv->gpu_error.completion;
unsigned long flags;
int ret;
@@ -944,7 +944,7 @@ i915_gem_check_wedge(struct drm_i915_private *dev_priv,
bool interruptible)
{
if (atomic_read(&dev_priv->mm.wedged)) {
- struct completion *x = &dev_priv->error_completion;
+ struct completion *x = &dev_priv->gpu_error.completion;
bool recovery_complete;
unsigned long flags;
@@ -2018,7 +2018,7 @@ i915_add_request(struct intel_ring_buffer *ring,
if (!dev_priv->mm.suspended) {
if (i915_enable_hangcheck) {
- mod_timer(&dev_priv->hangcheck_timer,
+ mod_timer(&dev_priv->gpu_error.hangcheck_timer,
round_jiffies_up(jiffies + DRM_I915_HANGCHECK_JIFFIES));
}
if (was_empty) {
@@ -3809,7 +3809,7 @@ i915_gem_idle(struct drm_device *dev)
* And not confound mm.suspended!
*/
dev_priv->mm.suspended = 1;
- del_timer_sync(&dev_priv->hangcheck_timer);
+ del_timer_sync(&dev_priv->gpu_error.hangcheck_timer);
i915_kernel_lost_context(dev);
i915_gem_cleanup_ringbuffer(dev);
@@ -4107,7 +4107,7 @@ i915_gem_load(struct drm_device *dev)
INIT_LIST_HEAD(&dev_priv->fence_regs[i].lru_list);
INIT_DELAYED_WORK(&dev_priv->mm.retire_work,
i915_gem_retire_work_handler);
- init_completion(&dev_priv->error_completion);
+ init_completion(&dev_priv->gpu_error.completion);
/* On GEN3 we really need to make sure the ARB C3 LP bit is set */
if (IS_GEN3(dev)) {
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2604867..8b71e1d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -355,8 +355,8 @@ static void notify_ring(struct drm_device *dev,
wake_up_all(&ring->irq_queue);
if (i915_enable_hangcheck) {
- dev_priv->hangcheck_count = 0;
- mod_timer(&dev_priv->hangcheck_timer,
+ dev_priv->gpu_error.hangcheck_count = 0;
+ mod_timer(&dev_priv->gpu_error.hangcheck_timer,
round_jiffies_up(jiffies + DRM_I915_HANGCHECK_JIFFIES));
}
}
@@ -839,7 +839,7 @@ done:
static void i915_error_work_func(struct work_struct *work)
{
drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,
- error_work);
+ gpu_error.work);
struct drm_device *dev = dev_priv->dev;
char *error_event[] = { "ERROR=1", NULL };
char *reset_event[] = { "RESET=1", NULL };
@@ -854,7 +854,7 @@ static void i915_error_work_func(struct work_struct *work)
atomic_set(&dev_priv->mm.wedged, 0);
kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, reset_done_event);
}
- complete_all(&dev_priv->error_completion);
+ complete_all(&dev_priv->gpu_error.completion);
}
}
@@ -1207,9 +1207,9 @@ static void i915_capture_error_state(struct drm_device *dev)
unsigned long flags;
int i, pipe;
- spin_lock_irqsave(&dev_priv->error_lock, flags);
- error = dev_priv->first_error;
- spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+ spin_lock_irqsave(&dev_priv->gpu_error.lock, flags);
+ error = dev_priv->gpu_error.first_error;
+ spin_unlock_irqrestore(&dev_priv->gpu_error.lock, flags);
if (error)
return;
@@ -1293,12 +1293,12 @@ static void i915_capture_error_state(struct drm_device *dev)
error->overlay = intel_overlay_capture_error_state(dev);
error->display = intel_display_capture_error_state(dev);
- spin_lock_irqsave(&dev_priv->error_lock, flags);
- if (dev_priv->first_error == NULL) {
- dev_priv->first_error = error;
+ spin_lock_irqsave(&dev_priv->gpu_error.lock, flags);
+ if (dev_priv->gpu_error.first_error == NULL) {
+ dev_priv->gpu_error.first_error = error;
error = NULL;
}
- spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+ spin_unlock_irqrestore(&dev_priv->gpu_error.lock, flags);
if (error)
i915_error_state_free(&error->ref);
@@ -1310,10 +1310,10 @@ void i915_destroy_error_state(struct drm_device *dev)
struct drm_i915_error_state *error;
unsigned long flags;
- spin_lock_irqsave(&dev_priv->error_lock, flags);
- error = dev_priv->first_error;
- dev_priv->first_error = NULL;
- spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+ spin_lock_irqsave(&dev_priv->gpu_error.lock, flags);
+ error = dev_priv->gpu_error.first_error;
+ dev_priv->gpu_error.first_error = NULL;
+ spin_unlock_irqrestore(&dev_priv->gpu_error.lock, flags);
if (error)
kref_put(&error->ref, i915_error_state_free);
@@ -1434,7 +1434,7 @@ void i915_handle_error(struct drm_device *dev, bool wedged)
i915_report_and_clear_eir(dev);
if (wedged) {
- INIT_COMPLETION(dev_priv->error_completion);
+ INIT_COMPLETION(dev_priv->gpu_error.completion);
atomic_set(&dev_priv->mm.wedged, 1);
/*
@@ -1444,7 +1444,7 @@ void i915_handle_error(struct drm_device *dev, bool wedged)
wake_up_all(&ring->irq_queue);
}
- queue_work(dev_priv->wq, &dev_priv->error_work);
+ queue_work(dev_priv->wq, &dev_priv->gpu_error.work);
}
static void i915_pageflip_stall_check(struct drm_device *dev, int pipe)
@@ -1673,7 +1673,7 @@ static bool i915_hangcheck_hung(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = dev->dev_private;
- if (dev_priv->hangcheck_count++ > 1) {
+ if (dev_priv->gpu_error.hangcheck_count++ > 1) {
bool hung = true;
DRM_ERROR("Hangcheck timer elapsed... GPU hung\n");
@@ -1732,25 +1732,29 @@ void i915_hangcheck_elapsed(unsigned long data)
goto repeat;
}
- dev_priv->hangcheck_count = 0;
+ dev_priv->gpu_error.hangcheck_count = 0;
return;
}
i915_get_extra_instdone(dev, instdone);
- if (memcmp(dev_priv->last_acthd, acthd, sizeof(acthd)) == 0 &&
- memcmp(dev_priv->prev_instdone, instdone, sizeof(instdone)) == 0) {
+ if (memcmp(dev_priv->gpu_error.last_acthd, acthd,
+ sizeof(acthd)) == 0 &&
+ memcmp(dev_priv->gpu_error.prev_instdone, instdone,
+ sizeof(instdone)) == 0) {
if (i915_hangcheck_hung(dev))
return;
} else {
- dev_priv->hangcheck_count = 0;
+ dev_priv->gpu_error.hangcheck_count = 0;
- memcpy(dev_priv->last_acthd, acthd, sizeof(acthd));
- memcpy(dev_priv->prev_instdone, instdone, sizeof(instdone));
+ memcpy(dev_priv->gpu_error.last_acthd, acthd,
+ sizeof(acthd));
+ memcpy(dev_priv->gpu_error.prev_instdone, instdone,
+ sizeof(instdone));
}
repeat:
/* Reset timer case chip hangs without another request being added */
- mod_timer(&dev_priv->hangcheck_timer,
+ mod_timer(&dev_priv->gpu_error.hangcheck_timer,
round_jiffies_up(jiffies + DRM_I915_HANGCHECK_JIFFIES));
}
@@ -2679,7 +2683,7 @@ void intel_irq_init(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
INIT_WORK(&dev_priv->hotplug_work, i915_hotplug_work_func);
- INIT_WORK(&dev_priv->error_work, i915_error_work_func);
+ INIT_WORK(&dev_priv->gpu_error.work, i915_error_work_func);
INIT_WORK(&dev_priv->rps.work, gen6_pm_rps_work);
INIT_WORK(&dev_priv->l3_parity.error_work, ivybridge_parity_work);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a035ac2..a81cdb4 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1345,7 +1345,7 @@ void intel_ring_advance(struct intel_ring_buffer *ring)
struct drm_i915_private *dev_priv = ring->dev->dev_private;
ring->tail &= ring->size - 1;
- if (dev_priv->stop_rings & intel_ring_flag(ring))
+ if (dev_priv->gpu_error.stop_rings & intel_ring_flag(ring))
return;
ring->write_tail(ring, ring->tail);
}
--
1.7.11.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/5] drm/i915: move wedged to the other gpu error handling stuff
2012-11-12 22:07 [PATCH 0/5] robustify reset state transitions Daniel Vetter
2012-11-12 22:07 ` [PATCH 1/5] drm/i915: move dev_priv->mm out of line Daniel Vetter
2012-11-12 22:07 ` [PATCH 2/5] drm/i915: extract hangcheck/reset/error_state state into substruct Daniel Vetter
@ 2012-11-12 22:07 ` Daniel Vetter
2012-11-12 22:07 ` [PATCH 4/5] drm/i915: clear up wedged transitions Daniel Vetter
2012-11-12 22:07 ` [PATCH 5/5] drm/i915: create a race-free reset detection Daniel Vetter
4 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2012-11-12 22:07 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
And to make Ben Widawsky happier, use the gpu_error instead of
the entire device as the argument in some functions.
Drop the outdated comment on ->wedged for now, a follow-up patch will
change the semantics and add a proper comment again.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 13 +++----------
drivers/gpu/drm/i915/i915_gem.c | 34 ++++++++++++++++-----------------
drivers/gpu/drm/i915/i915_irq.c | 6 +++---
drivers/gpu/drm/i915/intel_display.c | 4 ++--
drivers/gpu/drm/i915/intel_ringbuffer.c | 6 ++++--
6 files changed, 30 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 276997a..ad4cdfe 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1596,7 +1596,7 @@ i915_wedged_read(struct file *filp,
len = snprintf(buf, sizeof(buf),
"wedged : %d\n",
- atomic_read(&dev_priv->mm.wedged));
+ atomic_read(&dev_priv->gpu_error.wedged));
if (len > sizeof(buf))
len = sizeof(buf);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 03218f9..6958bb0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -694,15 +694,6 @@ struct i915_gem_mm {
*/
int suspended;
- /**
- * Flag if the hardware appears to be wedged.
- *
- * This is set when attempts to idle the device timeout.
- * It prevents command submission from occurring and makes
- * every pending request fail
- */
- atomic_t wedged;
-
/** Bit 6 swizzling required for X tiling */
uint32_t bit_6_swizzle_x;
/** Bit 6 swizzling required for Y tiling */
@@ -736,6 +727,8 @@ struct i915_gpu_error {
unsigned long last_reset;
+ atomic_t wedged;
+
/* For gpu hang simulation. */
unsigned int stop_rings;
};
@@ -1462,7 +1455,7 @@ i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj)
void i915_gem_retire_requests(struct drm_device *dev);
void i915_gem_retire_requests_ring(struct intel_ring_buffer *ring);
-int __must_check i915_gem_check_wedge(struct drm_i915_private *dev_priv,
+int __must_check i915_gem_check_wedge(struct i915_gpu_error *error,
bool interruptible);
void i915_gem_reset(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6e29bed..b2620c7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -87,14 +87,13 @@ static void i915_gem_info_remove_obj(struct drm_i915_private *dev_priv,
}
static int
-i915_gem_wait_for_error(struct drm_device *dev)
+i915_gem_wait_for_error(struct i915_gpu_error *error)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct completion *x = &dev_priv->gpu_error.completion;
+ struct completion *x = &error->completion;
unsigned long flags;
int ret;
- if (!atomic_read(&dev_priv->mm.wedged))
+ if (!atomic_read(&error->wedged))
return 0;
/*
@@ -110,7 +109,7 @@ i915_gem_wait_for_error(struct drm_device *dev)
return ret;
}
- if (atomic_read(&dev_priv->mm.wedged)) {
+ if (atomic_read(&error->wedged)) {
/* GPU is hung, bump the completion count to account for
* the token we just consumed so that we never hit zero and
* end up waiting upon a subsequent completion event that
@@ -125,9 +124,10 @@ i915_gem_wait_for_error(struct drm_device *dev)
int i915_mutex_lock_interruptible(struct drm_device *dev)
{
+ struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
- ret = i915_gem_wait_for_error(dev);
+ ret = i915_gem_wait_for_error(&dev_priv->gpu_error);
if (ret)
return ret;
@@ -940,11 +940,11 @@ unlock:
}
int
-i915_gem_check_wedge(struct drm_i915_private *dev_priv,
+i915_gem_check_wedge(struct i915_gpu_error *error,
bool interruptible)
{
- if (atomic_read(&dev_priv->mm.wedged)) {
- struct completion *x = &dev_priv->gpu_error.completion;
+ if (atomic_read(&error->wedged)) {
+ struct completion *x = &error->completion;
bool recovery_complete;
unsigned long flags;
@@ -1026,7 +1026,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
#define EXIT_COND \
(i915_seqno_passed(ring->get_seqno(ring, false), seqno) || \
- atomic_read(&dev_priv->mm.wedged))
+ atomic_read(&dev_priv->gpu_error.wedged))
do {
if (interruptible)
end = wait_event_interruptible_timeout(ring->irq_queue,
@@ -1036,7 +1036,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
end = wait_event_timeout(ring->irq_queue, EXIT_COND,
timeout_jiffies);
- ret = i915_gem_check_wedge(dev_priv, interruptible);
+ ret = i915_gem_check_wedge(&dev_priv->gpu_error, interruptible);
if (ret)
end = ret;
} while (end == 0 && wait_forever);
@@ -1082,7 +1082,7 @@ i915_wait_seqno(struct intel_ring_buffer *ring, uint32_t seqno)
BUG_ON(!mutex_is_locked(&dev->struct_mutex));
BUG_ON(seqno == 0);
- ret = i915_gem_check_wedge(dev_priv, interruptible);
+ ret = i915_gem_check_wedge(&dev_priv->gpu_error, interruptible);
if (ret)
return ret;
@@ -1147,7 +1147,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
if (seqno == 0)
return 0;
- ret = i915_gem_check_wedge(dev_priv, true);
+ ret = i915_gem_check_wedge(&dev_priv->gpu_error, true);
if (ret)
return ret;
@@ -1385,7 +1385,7 @@ out:
/* If this -EIO is due to a gpu hang, give the reset code a
* chance to clean up the mess. Otherwise return the proper
* SIGBUS. */
- if (!atomic_read(&dev_priv->mm.wedged))
+ if (!atomic_read(&dev_priv->gpu_error.wedged))
return VM_FAULT_SIGBUS;
case -EAGAIN:
/* Give the error handler a chance to run and move the
@@ -3402,7 +3402,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
u32 seqno = 0;
int ret;
- if (atomic_read(&dev_priv->mm.wedged))
+ if (atomic_read(&dev_priv->gpu_error.wedged))
return -EIO;
spin_lock(&file_priv->mm.lock);
@@ -4027,9 +4027,9 @@ i915_gem_entervt_ioctl(struct drm_device *dev, void *data,
if (drm_core_check_feature(dev, DRIVER_MODESET))
return 0;
- if (atomic_read(&dev_priv->mm.wedged)) {
+ if (atomic_read(&dev_priv->gpu_error.wedged)) {
DRM_ERROR("Reenabling wedged hardware, good luck\n");
- atomic_set(&dev_priv->mm.wedged, 0);
+ atomic_set(&dev_priv->gpu_error.wedged, 0);
}
mutex_lock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8b71e1d..9d8921a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -847,11 +847,11 @@ static void i915_error_work_func(struct work_struct *work)
kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, error_event);
- if (atomic_read(&dev_priv->mm.wedged)) {
+ if (atomic_read(&dev_priv->gpu_error.wedged)) {
DRM_DEBUG_DRIVER("resetting chip\n");
kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, reset_event);
if (!i915_reset(dev)) {
- atomic_set(&dev_priv->mm.wedged, 0);
+ atomic_set(&dev_priv->gpu_error.wedged, 0);
kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, reset_done_event);
}
complete_all(&dev_priv->gpu_error.completion);
@@ -1435,7 +1435,7 @@ void i915_handle_error(struct drm_device *dev, bool wedged)
if (wedged) {
INIT_COMPLETION(dev_priv->gpu_error.completion);
- atomic_set(&dev_priv->mm.wedged, 1);
+ atomic_set(&dev_priv->gpu_error.wedged, 1);
/*
* Wakeup waiting processes so they don't hang
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 655f87c..e321c9e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2242,7 +2242,7 @@ intel_finish_fb(struct drm_framebuffer *old_fb)
int ret;
wait_event(dev_priv->pending_flip_queue,
- atomic_read(&dev_priv->mm.wedged) ||
+ atomic_read(&dev_priv->gpu_error.wedged) ||
atomic_read(&obj->pending_flip) == 0);
/* Big Hammer, we also need to ensure that any pending
@@ -2963,7 +2963,7 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
unsigned long flags;
bool pending;
- if (atomic_read(&dev_priv->mm.wedged))
+ if (atomic_read(&dev_priv->gpu_error.wedged))
return false;
spin_lock_irqsave(&dev->event_lock, flags);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a81cdb4..8279dd0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1305,7 +1305,8 @@ int intel_wait_ring_buffer(struct intel_ring_buffer *ring, int n)
msleep(1);
- ret = i915_gem_check_wedge(dev_priv, dev_priv->mm.interruptible);
+ ret = i915_gem_check_wedge(&dev_priv->gpu_error,
+ dev_priv->mm.interruptible);
if (ret)
return ret;
} while (!time_after(jiffies, end));
@@ -1320,7 +1321,8 @@ int intel_ring_begin(struct intel_ring_buffer *ring,
int n = 4*num_dwords;
int ret;
- ret = i915_gem_check_wedge(dev_priv, dev_priv->mm.interruptible);
+ ret = i915_gem_check_wedge(&dev_priv->gpu_error,
+ dev_priv->mm.interruptible);
if (ret)
return ret;
--
1.7.11.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/5] drm/i915: clear up wedged transitions
2012-11-12 22:07 [PATCH 0/5] robustify reset state transitions Daniel Vetter
` (2 preceding siblings ...)
2012-11-12 22:07 ` [PATCH 3/5] drm/i915: move wedged to the other gpu error handling stuff Daniel Vetter
@ 2012-11-12 22:07 ` Daniel Vetter
2012-11-13 8:56 ` Chris Wilson
2012-11-12 22:07 ` [PATCH 5/5] drm/i915: create a race-free reset detection Daniel Vetter
4 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2012-11-12 22:07 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
We have two important transitions of the wedged state in the current
code:
- 0 -> 1: This means a hang has been detected, and signals to everyone
that they please get of any locks, so that the reset work item can
do its job.
- 1 -> 0: The reset handler has completed.
Now the last transition mixes up two states: "Reset completed and
successful" and "Reset failed". To distinguish these two we do some
tricks with the reset completion, but I simply could not convince
myself that this doesn't race under odd circumstances.
Hence split this up, and add a new terminal state indicating that the
hw is gone for good.
Also add explicit #defines for both states, update comments.
While auditing the code I've noticed one place (the throttle ioctl)
which does not yet wait for the reset handler to complete and doesn't
properly decode the wedge state into -EAGAIN/-EIO. Fix this up by
calling the right helpers. This might explain the oddball "my
compositor just died in a successfull gpu reset" reports.
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_drv.h | 17 ++++++++++++++-
drivers/gpu/drm/i915/i915_gem.c | 46 ++++++++++++++++-------------------------
drivers/gpu/drm/i915/i915_irq.c | 23 ++++++++++++++-------
3 files changed, 49 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6958bb0..423541b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -723,11 +723,26 @@ struct i915_gpu_error {
/* Protected by the above dev->gpu_error.lock. */
struct drm_i915_error_state *first_error;
struct work_struct work;
- struct completion completion;
unsigned long last_reset;
+ /**
+ * State variable controlling the reset flow
+ *
+ * 1 means a reset is in progress. This state will (presuming we don't
+ * have any bugs) decay into either 0 (successful reset) or 2 (hw
+ * terminally sour). All waiters on the reset_queue will be woken when
+ * that happens.
+ */
atomic_t wedged;
+#define I915_RESET_IN_PROGRESS 1
+#define I915_WEDGED 2
+
+ /**
+ * Waitqueue to signal when the reset has completed. Used by clients
+ * that wait for dev_priv->mm.wedged to settle.
+ */
+ wait_queue_head_t reset_queue;
/* For gpu hang simulation. */
unsigned int stop_rings;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b2620c7..3a1f72c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -89,36 +89,28 @@ static void i915_gem_info_remove_obj(struct drm_i915_private *dev_priv,
static int
i915_gem_wait_for_error(struct i915_gpu_error *error)
{
- struct completion *x = &error->completion;
- unsigned long flags;
int ret;
if (!atomic_read(&error->wedged))
return 0;
+#define EXIT_COND (atomic_read(&error->reset_queue) != I915_RESET_IN_PROGRESS)
/*
* Only wait 10 seconds for the gpu reset to complete to avoid hanging
* userspace. If it takes that long something really bad is going on and
* we should simply try to bail out and fail as gracefully as possible.
*/
- ret = wait_for_completion_interruptible_timeout(x, 10*HZ);
+ ret = wait_event_interruptible_timeout(error->reset_queue,
+ EXIT_COND,
+ 10*HZ);
if (ret == 0) {
DRM_ERROR("Timed out waiting for the gpu reset to complete\n");
return -EIO;
} else if (ret < 0) {
return ret;
}
+#undef EXIT_COND
- if (atomic_read(&error->wedged)) {
- /* GPU is hung, bump the completion count to account for
- * the token we just consumed so that we never hit zero and
- * end up waiting upon a subsequent completion event that
- * will never happen.
- */
- spin_lock_irqsave(&x->wait.lock, flags);
- x->done++;
- spin_unlock_irqrestore(&x->wait.lock, flags);
- }
return 0;
}
@@ -943,23 +935,16 @@ int
i915_gem_check_wedge(struct i915_gpu_error *error,
bool interruptible)
{
- if (atomic_read(&error->wedged)) {
- struct completion *x = &error->completion;
- bool recovery_complete;
- unsigned long flags;
-
- /* Give the error handler a chance to run. */
- spin_lock_irqsave(&x->wait.lock, flags);
- recovery_complete = x->done > 0;
- spin_unlock_irqrestore(&x->wait.lock, flags);
+ unsigned tmp = atomic_read(&error->wedged);
+ if (tmp) {
/* Non-interruptible callers can't handle -EAGAIN, hence return
* -EIO unconditionally for these. */
if (!interruptible)
return -EIO;
- /* Recovery complete, but still wedged means reset failure. */
- if (recovery_complete)
+ /* Recovery complete, but the reset failed ... */
+ if (tmp == I915_WEDGED)
return -EIO;
return -EAGAIN;
@@ -1385,7 +1370,7 @@ out:
/* If this -EIO is due to a gpu hang, give the reset code a
* chance to clean up the mess. Otherwise return the proper
* SIGBUS. */
- if (!atomic_read(&dev_priv->gpu_error.wedged))
+ if (atomic_read(&dev_priv->gpu_error.wedged) == I915_WEDGED)
return VM_FAULT_SIGBUS;
case -EAGAIN:
/* Give the error handler a chance to run and move the
@@ -3402,8 +3387,13 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
u32 seqno = 0;
int ret;
- if (atomic_read(&dev_priv->gpu_error.wedged))
- return -EIO;
+ ret = i915_gem_wait_for_error(&dev_priv->gpu_error);
+ if (ret)
+ return ret;
+
+ ret = i915_gem_check_wedge(&dev_priv->gpu_error, false);
+ if (ret)
+ return ret;
spin_lock(&file_priv->mm.lock);
list_for_each_entry(request, &file_priv->mm.request_list, client_list) {
@@ -4107,7 +4097,7 @@ i915_gem_load(struct drm_device *dev)
INIT_LIST_HEAD(&dev_priv->fence_regs[i].lru_list);
INIT_DELAYED_WORK(&dev_priv->mm.retire_work,
i915_gem_retire_work_handler);
- init_completion(&dev_priv->gpu_error.completion);
+ init_waitqueue_head(&dev_priv->gpu_error.reset_queue);
/* On GEN3 we really need to make sure the ARB C3 LP bit is set */
if (IS_GEN3(dev)) {
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9d8921a..73d46f4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -838,8 +838,10 @@ done:
*/
static void i915_error_work_func(struct work_struct *work)
{
- drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,
- gpu_error.work);
+ struct i915_gpu_error *error = container_of(work, struct i915_gpu_error,
+ work);
+ drm_i915_private_t *dev_priv = container_of(error, drm_i915_private_t,
+ gpu_error);
struct drm_device *dev = dev_priv->dev;
char *error_event[] = { "ERROR=1", NULL };
char *reset_event[] = { "RESET=1", NULL };
@@ -847,14 +849,19 @@ static void i915_error_work_func(struct work_struct *work)
kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, error_event);
- if (atomic_read(&dev_priv->gpu_error.wedged)) {
+ if (atomic_read(&error->wedged) == I915_RESET_IN_PROGRESS) {
DRM_DEBUG_DRIVER("resetting chip\n");
kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, reset_event);
+
if (!i915_reset(dev)) {
- atomic_set(&dev_priv->gpu_error.wedged, 0);
+ atomic_set(&error->wedged, 0);
kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, reset_done_event);
+ } else {
+ atomic_set(&error->wedged,
+ I915_WEDGED);
}
- complete_all(&dev_priv->gpu_error.completion);
+
+ wake_up_all(&dev_priv->gpu_error.reset_queue);
}
}
@@ -1434,11 +1441,11 @@ void i915_handle_error(struct drm_device *dev, bool wedged)
i915_report_and_clear_eir(dev);
if (wedged) {
- INIT_COMPLETION(dev_priv->gpu_error.completion);
- atomic_set(&dev_priv->gpu_error.wedged, 1);
+ atomic_set(&dev_priv->gpu_error.wedged, I915_RESET_IN_PROGRESS);
/*
- * Wakeup waiting processes so they don't hang
+ * Wakeup waiting processes so that the reset work item
+ * doesn't deadlock trying to grab various locks.
*/
for_each_ring(ring, dev_priv, i)
wake_up_all(&ring->irq_queue);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 4/5] drm/i915: clear up wedged transitions
2012-11-12 22:07 ` [PATCH 4/5] drm/i915: clear up wedged transitions Daniel Vetter
@ 2012-11-13 8:56 ` Chris Wilson
2012-11-13 10:12 ` Daniel Vetter
0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2012-11-13 8:56 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
On Mon, 12 Nov 2012 23:07:52 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> While auditing the code I've noticed one place (the throttle ioctl)
> which does not yet wait for the reset handler to complete and doesn't
> properly decode the wedge state into -EAGAIN/-EIO. Fix this up by
> calling the right helpers. This might explain the oddball "my
> compositor just died in a successfull gpu reset" reports.
It is a standalone bugfix and should be split into its own patch. But I
don't think it quite has the impact you describe, since it only called
by the ddx and only sna bothers to check the error code. And one
side-effect is that I think for especilly this case
i915_gem_check_wedge() should be setting set_need_resched() before
returning -EAGAIN.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/5] drm/i915: clear up wedged transitions
2012-11-13 8:56 ` Chris Wilson
@ 2012-11-13 10:12 ` Daniel Vetter
2012-11-13 16:40 ` [PATCH 1/2] drm/i915: fix reset handling in the throttle ioctl Daniel Vetter
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2012-11-13 10:12 UTC (permalink / raw)
To: Chris Wilson; +Cc: Intel Graphics Development
On Tue, Nov 13, 2012 at 9:56 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, 12 Nov 2012 23:07:52 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> While auditing the code I've noticed one place (the throttle ioctl)
>> which does not yet wait for the reset handler to complete and doesn't
>> properly decode the wedge state into -EAGAIN/-EIO. Fix this up by
>> calling the right helpers. This might explain the oddball "my
>> compositor just died in a successfull gpu reset" reports.
>
> It is a standalone bugfix and should be split into its own patch. But I
> don't think it quite has the impact you describe, since it only called
> by the ddx and only sna bothers to check the error code. And one
> side-effect is that I think for especilly this case
> i915_gem_check_wedge() should be setting set_need_resched() before
> returning -EAGAIN.
Yeah, I've somehow ended up being lazy yesterday, will split out. For
the need-resched. Wrt the set_need_resched I think we're ok, to avoid
busy-looping I've added a call to i915_gem_wait_for_error. And imo
there's no need to micro-optimize the reset handling by getting of the
cpu a notch earlier. I'll extend the commit message of the split-out
patch to mention that little detail.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] drm/i915: fix reset handling in the throttle ioctl
2012-11-13 10:12 ` Daniel Vetter
@ 2012-11-13 16:40 ` Daniel Vetter
2012-11-13 16:40 ` [PATCH 2/2] drm/i915: clear up wedged transitions Daniel Vetter
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2012-11-13 16:40 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
While auditing the code I've noticed one place (the throttle ioctl)
which does not yet wait for the reset handler to complete and doesn't
properly decode the wedge state into -EAGAIN/-EIO. Fix this up by
calling the right helpers. This might explain the oddball "my
compositor just died in a successfull gpu reset" reports.
The throttle ioctl doesn't take the struct_mutex, so to avoid busy-looping
with -EAGAIN while a reset is in process, check for errors first and wait
for the handler to complete if a reset is pending by calling
i915_gem_wait_for_error.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_gem.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b2620c7..55cdad9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3402,8 +3402,13 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
u32 seqno = 0;
int ret;
- if (atomic_read(&dev_priv->gpu_error.wedged))
- return -EIO;
+ ret = i915_gem_wait_for_error(&dev_priv->gpu_error);
+ if (ret)
+ return ret;
+
+ ret = i915_gem_check_wedge(&dev_priv->gpu_error, false);
+ if (ret)
+ return ret;
spin_lock(&file_priv->mm.lock);
list_for_each_entry(request, &file_priv->mm.request_list, client_list) {
--
1.7.11.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/2] drm/i915: clear up wedged transitions
2012-11-13 16:40 ` [PATCH 1/2] drm/i915: fix reset handling in the throttle ioctl Daniel Vetter
@ 2012-11-13 16:40 ` Daniel Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2012-11-13 16:40 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
We have two important transitions of the wedged state in the current
code:
- 0 -> 1: This means a hang has been detected, and signals to everyone
that they please get of any locks, so that the reset work item can
do its job.
- 1 -> 0: The reset handler has completed.
Now the last transition mixes up two states: "Reset completed and
successful" and "Reset failed". To distinguish these two we do some
tricks with the reset completion, but I simply could not convince
myself that this doesn't race under odd circumstances.
Hence split this up, and add a new terminal state indicating that the
hw is gone for good.
Also add explicit #defines for both states, update comments.
v2: Split out the reset handling bugfix for the throttle ioctl.
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_drv.h | 17 ++++++++++++++++-
drivers/gpu/drm/i915/i915_gem.c | 37 +++++++++++--------------------------
drivers/gpu/drm/i915/i915_irq.c | 23 +++++++++++++++--------
3 files changed, 42 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6958bb0..423541b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -723,11 +723,26 @@ struct i915_gpu_error {
/* Protected by the above dev->gpu_error.lock. */
struct drm_i915_error_state *first_error;
struct work_struct work;
- struct completion completion;
unsigned long last_reset;
+ /**
+ * State variable controlling the reset flow
+ *
+ * 1 means a reset is in progress. This state will (presuming we don't
+ * have any bugs) decay into either 0 (successful reset) or 2 (hw
+ * terminally sour). All waiters on the reset_queue will be woken when
+ * that happens.
+ */
atomic_t wedged;
+#define I915_RESET_IN_PROGRESS 1
+#define I915_WEDGED 2
+
+ /**
+ * Waitqueue to signal when the reset has completed. Used by clients
+ * that wait for dev_priv->mm.wedged to settle.
+ */
+ wait_queue_head_t reset_queue;
/* For gpu hang simulation. */
unsigned int stop_rings;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 55cdad9..3a1f72c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -89,36 +89,28 @@ static void i915_gem_info_remove_obj(struct drm_i915_private *dev_priv,
static int
i915_gem_wait_for_error(struct i915_gpu_error *error)
{
- struct completion *x = &error->completion;
- unsigned long flags;
int ret;
if (!atomic_read(&error->wedged))
return 0;
+#define EXIT_COND (atomic_read(&error->reset_queue) != I915_RESET_IN_PROGRESS)
/*
* Only wait 10 seconds for the gpu reset to complete to avoid hanging
* userspace. If it takes that long something really bad is going on and
* we should simply try to bail out and fail as gracefully as possible.
*/
- ret = wait_for_completion_interruptible_timeout(x, 10*HZ);
+ ret = wait_event_interruptible_timeout(error->reset_queue,
+ EXIT_COND,
+ 10*HZ);
if (ret == 0) {
DRM_ERROR("Timed out waiting for the gpu reset to complete\n");
return -EIO;
} else if (ret < 0) {
return ret;
}
+#undef EXIT_COND
- if (atomic_read(&error->wedged)) {
- /* GPU is hung, bump the completion count to account for
- * the token we just consumed so that we never hit zero and
- * end up waiting upon a subsequent completion event that
- * will never happen.
- */
- spin_lock_irqsave(&x->wait.lock, flags);
- x->done++;
- spin_unlock_irqrestore(&x->wait.lock, flags);
- }
return 0;
}
@@ -943,23 +935,16 @@ int
i915_gem_check_wedge(struct i915_gpu_error *error,
bool interruptible)
{
- if (atomic_read(&error->wedged)) {
- struct completion *x = &error->completion;
- bool recovery_complete;
- unsigned long flags;
-
- /* Give the error handler a chance to run. */
- spin_lock_irqsave(&x->wait.lock, flags);
- recovery_complete = x->done > 0;
- spin_unlock_irqrestore(&x->wait.lock, flags);
+ unsigned tmp = atomic_read(&error->wedged);
+ if (tmp) {
/* Non-interruptible callers can't handle -EAGAIN, hence return
* -EIO unconditionally for these. */
if (!interruptible)
return -EIO;
- /* Recovery complete, but still wedged means reset failure. */
- if (recovery_complete)
+ /* Recovery complete, but the reset failed ... */
+ if (tmp == I915_WEDGED)
return -EIO;
return -EAGAIN;
@@ -1385,7 +1370,7 @@ out:
/* If this -EIO is due to a gpu hang, give the reset code a
* chance to clean up the mess. Otherwise return the proper
* SIGBUS. */
- if (!atomic_read(&dev_priv->gpu_error.wedged))
+ if (atomic_read(&dev_priv->gpu_error.wedged) == I915_WEDGED)
return VM_FAULT_SIGBUS;
case -EAGAIN:
/* Give the error handler a chance to run and move the
@@ -4112,7 +4097,7 @@ i915_gem_load(struct drm_device *dev)
INIT_LIST_HEAD(&dev_priv->fence_regs[i].lru_list);
INIT_DELAYED_WORK(&dev_priv->mm.retire_work,
i915_gem_retire_work_handler);
- init_completion(&dev_priv->gpu_error.completion);
+ init_waitqueue_head(&dev_priv->gpu_error.reset_queue);
/* On GEN3 we really need to make sure the ARB C3 LP bit is set */
if (IS_GEN3(dev)) {
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9d8921a..73d46f4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -838,8 +838,10 @@ done:
*/
static void i915_error_work_func(struct work_struct *work)
{
- drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,
- gpu_error.work);
+ struct i915_gpu_error *error = container_of(work, struct i915_gpu_error,
+ work);
+ drm_i915_private_t *dev_priv = container_of(error, drm_i915_private_t,
+ gpu_error);
struct drm_device *dev = dev_priv->dev;
char *error_event[] = { "ERROR=1", NULL };
char *reset_event[] = { "RESET=1", NULL };
@@ -847,14 +849,19 @@ static void i915_error_work_func(struct work_struct *work)
kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, error_event);
- if (atomic_read(&dev_priv->gpu_error.wedged)) {
+ if (atomic_read(&error->wedged) == I915_RESET_IN_PROGRESS) {
DRM_DEBUG_DRIVER("resetting chip\n");
kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, reset_event);
+
if (!i915_reset(dev)) {
- atomic_set(&dev_priv->gpu_error.wedged, 0);
+ atomic_set(&error->wedged, 0);
kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, reset_done_event);
+ } else {
+ atomic_set(&error->wedged,
+ I915_WEDGED);
}
- complete_all(&dev_priv->gpu_error.completion);
+
+ wake_up_all(&dev_priv->gpu_error.reset_queue);
}
}
@@ -1434,11 +1441,11 @@ void i915_handle_error(struct drm_device *dev, bool wedged)
i915_report_and_clear_eir(dev);
if (wedged) {
- INIT_COMPLETION(dev_priv->gpu_error.completion);
- atomic_set(&dev_priv->gpu_error.wedged, 1);
+ atomic_set(&dev_priv->gpu_error.wedged, I915_RESET_IN_PROGRESS);
/*
- * Wakeup waiting processes so they don't hang
+ * Wakeup waiting processes so that the reset work item
+ * doesn't deadlock trying to grab various locks.
*/
for_each_ring(ring, dev_priv, i)
wake_up_all(&ring->irq_queue);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] drm/i915: create a race-free reset detection
2012-11-12 22:07 [PATCH 0/5] robustify reset state transitions Daniel Vetter
` (3 preceding siblings ...)
2012-11-12 22:07 ` [PATCH 4/5] drm/i915: clear up wedged transitions Daniel Vetter
@ 2012-11-12 22:07 ` Daniel Vetter
4 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2012-11-12 22:07 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
With the previous patch the state transition handling of the reset
code itself is now (hopefully) race free and solid. But that still
leaves out everyone else - with the various lock-free wait paths
we have there's the possibility that the reset happens between the
point where we read the seqno we should wait on and the actual wait.
And if __wait_seqno then never sees the RESET_IN_PROGRESS state, we'll
happily wait for a seqno which will in all likelyhood never signal.
In practice this is not a big problem since the X server gets
constantly interrupted, and can then submit more work (hopefully) to
unblock everyone else: As soon as a new seqno write lands, all waiters
will unblock. But running the i-g-t reset testcase ZZ_hangman can
expose this race, especially on slower hw with fewer cpu cores.
Now looking forward to ARB_robustness and friends that's not the best
possible behaviour, hence this patch adds a reset_counter to be able
to detect any reset, even if a given thread never observed the
in-progress state.
The important part is to correctly order things:
- The write side needs to increment the counter after any seqno gets
reset. Hence we need to do that at the end of the reset work, and
again wake everyone up. We also need to place a barrier in between
any possible seqno changes and the counter increment.
- On the read side we need to ensure that no reset can sneak in and
invalidate the seqno. In many cases the dev->struct_mutex ensures
consistency and the unlocking provides the required memory barrier.
In some completely lockless cases we need to add a barrier and be
careful with the ordering.
The end-result of all this is that we need to wake up everyone twice
in a reset operation:
- First, before the reset starts, to get any lockholders of the locks,
so that the reset can proceed.
- Second, after the reset is completed, to allow waiters to properly
and reliably detect the reset condition and bail out.
I admit that this entire reset_counter thing smells a bit like
overkill, but I think it's justified since it makes it really explicit
what the bail-out condition is. And we need a reset counter anyway to
implement ARB_robustness, and imo with finer-grained locking on the
horizont this is the most resilient scheme I could think of.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_drv.h | 13 +++++++++++++
drivers/gpu/drm/i915/i915_gem.c | 42 +++++++++++++++++++++++++++++++++++------
drivers/gpu/drm/i915/i915_irq.c | 16 ++++++++++++++++
3 files changed, 65 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 423541b..b5ad2a8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -739,6 +739,19 @@ struct i915_gpu_error {
#define I915_WEDGED 2
/**
+ * Reset counter
+ *
+ * This is used by the wait_seqno code to race-free noticed that a reset
+ * event happened and that it needs to restart the entire ioctl (since
+ * most likely the seqno it waited for won't ever signal anytime soon).
+ *
+ * This is important for lock-free wait paths, where no contended lock
+ * naturally enforces the correct ordering between the bail-out of the
+ * waiter and the gpu reset work code.
+ */
+ atomic_t reset_counter;
+
+ /**
* Waitqueue to signal when the reset has completed. Used by clients
* that wait for dev_priv->mm.wedged to settle.
*/
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3a1f72c..73ab475 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -94,7 +94,7 @@ i915_gem_wait_for_error(struct i915_gpu_error *error)
if (!atomic_read(&error->wedged))
return 0;
-#define EXIT_COND (atomic_read(&error->reset_queue) != I915_RESET_IN_PROGRESS)
+#define EXIT_COND (atomic_read(&error->reset_counter) != I915_RESET_IN_PROGRESS)
/*
* Only wait 10 seconds for the gpu reset to complete to avoid hanging
* userspace. If it takes that long something really bad is going on and
@@ -975,13 +975,22 @@ i915_gem_check_olr(struct intel_ring_buffer *ring, u32 seqno)
* __wait_seqno - wait until execution of seqno has finished
* @ring: the ring expected to report seqno
* @seqno: duh!
+ * @reset_counter: reset sequence associated with the given seqno
* @interruptible: do an interruptible wait (normally yes)
* @timeout: in - how long to wait (NULL forever); out - how much time remaining
*
+ * Note: It is of utmost importance that the passed in seqno and reset_counter
+ * values have been read by the caller in an smb safe manner. Where read-side
+ * locks are involved, it is sufficient to read the reset_counter before
+ * unlocking the lock that protects the seqno. For lockless tricks, the
+ * reset_counter _must_ be read before, and an appropriate smb_rmb must be
+ * inserted.
+ *
* Returns 0 if the seqno was found within the alloted time. Else returns the
* errno with remaining time filled in timeout argument.
*/
static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
+ unsigned reset_counter,
bool interruptible, struct timespec *timeout)
{
drm_i915_private_t *dev_priv = ring->dev->dev_private;
@@ -1011,7 +1020,8 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
#define EXIT_COND \
(i915_seqno_passed(ring->get_seqno(ring, false), seqno) || \
- atomic_read(&dev_priv->gpu_error.wedged))
+ atomic_read(&dev_priv->gpu_error.wedged) || \
+ (reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter)))
do {
if (interruptible)
end = wait_event_interruptible_timeout(ring->irq_queue,
@@ -1021,6 +1031,11 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
end = wait_event_timeout(ring->irq_queue, EXIT_COND,
timeout_jiffies);
+ /* We need to check whether any gpu reset happened in between
+ * the caller grabbing the seqno and now. */
+ if (reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter))
+ end = -EAGAIN;
+
ret = i915_gem_check_wedge(&dev_priv->gpu_error, interruptible);
if (ret)
end = ret;
@@ -1075,7 +1090,9 @@ i915_wait_seqno(struct intel_ring_buffer *ring, uint32_t seqno)
if (ret)
return ret;
- return __wait_seqno(ring, seqno, interruptible, NULL);
+ return __wait_seqno(ring, seqno,
+ atomic_read(&dev_priv->gpu_error.reset_counter),
+ interruptible, NULL);
}
/**
@@ -1122,6 +1139,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_ring_buffer *ring = obj->ring;
+ unsigned reset_counter;
u32 seqno;
int ret;
@@ -1140,8 +1158,9 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
if (ret)
return ret;
+ reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
mutex_unlock(&dev->struct_mutex);
- ret = __wait_seqno(ring, seqno, true, NULL);
+ ret = __wait_seqno(ring, seqno, reset_counter, true, NULL);
mutex_lock(&dev->struct_mutex);
i915_gem_retire_requests_ring(ring);
@@ -2273,10 +2292,12 @@ i915_gem_object_flush_active(struct drm_i915_gem_object *obj)
int
i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
{
+ drm_i915_private_t *dev_priv = dev->dev_private;
struct drm_i915_gem_wait *args = data;
struct drm_i915_gem_object *obj;
struct intel_ring_buffer *ring = NULL;
struct timespec timeout_stack, *timeout = NULL;
+ unsigned reset_counter;
u32 seqno = 0;
int ret = 0;
@@ -2317,9 +2338,10 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
}
drm_gem_object_unreference(&obj->base);
+ reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
mutex_unlock(&dev->struct_mutex);
- ret = __wait_seqno(ring, seqno, true, timeout);
+ ret = __wait_seqno(ring, seqno, reset_counter, true, timeout);
if (timeout) {
WARN_ON(!timespec_valid(timeout));
args->timeout_ns = timespec_to_ns(timeout);
@@ -3384,6 +3406,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
unsigned long recent_enough = jiffies - msecs_to_jiffies(20);
struct drm_i915_gem_request *request;
struct intel_ring_buffer *ring = NULL;
+ unsigned reset_counter;
u32 seqno = 0;
int ret;
@@ -3395,6 +3418,13 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
if (ret)
return ret;
+ reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
+ /*
+ * We must read the reset_counter before reading the seqno - locks are
+ * only one-sided barriers!
+ */
+ smp_rmb();
+
spin_lock(&file_priv->mm.lock);
list_for_each_entry(request, &file_priv->mm.request_list, client_list) {
if (time_after_eq(request->emitted_jiffies, recent_enough))
@@ -3408,7 +3438,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
if (seqno == 0)
return 0;
- ret = __wait_seqno(ring, seqno, true, NULL);
+ ret = __wait_seqno(ring, seqno, reset_counter, true, NULL);
if (ret == 0)
queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 73d46f4..87f5031 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -843,9 +843,11 @@ static void i915_error_work_func(struct work_struct *work)
drm_i915_private_t *dev_priv = container_of(error, drm_i915_private_t,
gpu_error);
struct drm_device *dev = dev_priv->dev;
+ struct intel_ring_buffer *ring;
char *error_event[] = { "ERROR=1", NULL };
char *reset_event[] = { "RESET=1", NULL };
char *reset_done_event[] = { "ERROR=0", NULL };
+ int i;
kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, error_event);
@@ -861,6 +863,20 @@ static void i915_error_work_func(struct work_struct *work)
I915_WEDGED);
}
+ /*
+ * After all the gem state is reset, increment the reset counter
+ * and wake up everyone waiting for the reset to complete.
+ *
+ * Since unlock operations are a one-sided barrier only, we need
+ * to insert a barrier here to order any seqno updates before
+ * the counter increment.
+ */
+ smp_mb__before_atomic_inc();
+ atomic_inc(&dev_priv->gpu_error.reset_counter);
+
+ for_each_ring(ring, dev_priv, i)
+ wake_up_all(&ring->irq_queue);
+
wake_up_all(&dev_priv->gpu_error.reset_queue);
}
}
--
1.7.11.4
^ permalink raw reply related [flat|nested] 10+ messages in thread