Commit 45c5f202 authored by Chris Wilson's avatar Chris Wilson Committed by Daniel Vetter

drm/i915: Disable all GEM timers and work on unload

We have two once very similar functions, i915_gpu_idle() and
i915_gem_idle(). The former is used as the lower level operation to
flush work on the GPU, whereas the latter is the high level interface to
flush the GEM bookkeeping in addition to flushing the GPU. As such
i915_gem_idle() also clears out the request and activity lists and
cancels the delayed work. This is what we need for unloading the driver,
unfortunately we called i915_gpu_idle() instead.

In the process, make sure that when cancelling the delayed work and
timer, which is synchronous, that we do not hold any locks to prevent a
deadlock if the work item is already waiting upon the mutex. This
requires us to push the mutex down from the caller to i915_gem_idle().

v2: s/i915_gem_idle/i915_gem_suspend/

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70334Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Tested-by: xunx.fang@intel.com
[danvet: Only set ums.suspended for !kms as discussed earlier. Chris
noticed that this slipped through.]
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 9514ac6e
...@@ -1732,15 +1732,9 @@ int i915_driver_unload(struct drm_device *dev) ...@@ -1732,15 +1732,9 @@ int i915_driver_unload(struct drm_device *dev)
if (dev_priv->mm.inactive_shrinker.scan_objects) if (dev_priv->mm.inactive_shrinker.scan_objects)
unregister_shrinker(&dev_priv->mm.inactive_shrinker); unregister_shrinker(&dev_priv->mm.inactive_shrinker);
mutex_lock(&dev->struct_mutex); ret = i915_gem_suspend(dev);
ret = i915_gpu_idle(dev);
if (ret) if (ret)
DRM_ERROR("failed to idle hardware: %d\n", ret); DRM_ERROR("failed to idle hardware: %d\n", ret);
i915_gem_retire_requests(dev);
mutex_unlock(&dev->struct_mutex);
/* Cancel the retire work handler, which should be idle now. */
cancel_delayed_work_sync(&dev_priv->mm.retire_work);
io_mapping_free(dev_priv->gtt.mappable); io_mapping_free(dev_priv->gtt.mappable);
arch_phys_wc_del(dev_priv->gtt.mtrr); arch_phys_wc_del(dev_priv->gtt.mtrr);
......
...@@ -487,9 +487,7 @@ static int i915_drm_freeze(struct drm_device *dev) ...@@ -487,9 +487,7 @@ static int i915_drm_freeze(struct drm_device *dev)
if (drm_core_check_feature(dev, DRIVER_MODESET)) { if (drm_core_check_feature(dev, DRIVER_MODESET)) {
int error; int error;
mutex_lock(&dev->struct_mutex); error = i915_gem_suspend(dev);
error = i915_gem_idle(dev);
mutex_unlock(&dev->struct_mutex);
if (error) { if (error) {
dev_err(&dev->pdev->dev, dev_err(&dev->pdev->dev,
"GEM idle failed, resume might fail\n"); "GEM idle failed, resume might fail\n");
......
...@@ -2037,7 +2037,7 @@ int i915_gem_l3_remap(struct intel_ring_buffer *ring, int slice); ...@@ -2037,7 +2037,7 @@ int i915_gem_l3_remap(struct intel_ring_buffer *ring, int slice);
void i915_gem_init_swizzling(struct drm_device *dev); void i915_gem_init_swizzling(struct drm_device *dev);
void i915_gem_cleanup_ringbuffer(struct drm_device *dev); void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
int __must_check i915_gpu_idle(struct drm_device *dev); int __must_check i915_gpu_idle(struct drm_device *dev);
int __must_check i915_gem_idle(struct drm_device *dev); int __must_check i915_gem_suspend(struct drm_device *dev);
int __i915_add_request(struct intel_ring_buffer *ring, int __i915_add_request(struct intel_ring_buffer *ring,
struct drm_file *file, struct drm_file *file,
struct drm_i915_gem_object *batch_obj, struct drm_i915_gem_object *batch_obj,
......
...@@ -4265,17 +4265,18 @@ void i915_gem_vma_destroy(struct i915_vma *vma) ...@@ -4265,17 +4265,18 @@ void i915_gem_vma_destroy(struct i915_vma *vma)
} }
int int
i915_gem_idle(struct drm_device *dev) i915_gem_suspend(struct drm_device *dev)
{ {
drm_i915_private_t *dev_priv = dev->dev_private; drm_i915_private_t *dev_priv = dev->dev_private;
int ret; int ret = 0;
mutex_lock(&dev->struct_mutex);
if (dev_priv->ums.mm_suspended) if (dev_priv->ums.mm_suspended)
return 0; goto err;
ret = i915_gpu_idle(dev); ret = i915_gpu_idle(dev);
if (ret) if (ret)
return ret; goto err;
i915_gem_retire_requests(dev); i915_gem_retire_requests(dev);
...@@ -4283,16 +4284,26 @@ i915_gem_idle(struct drm_device *dev) ...@@ -4283,16 +4284,26 @@ i915_gem_idle(struct drm_device *dev)
if (!drm_core_check_feature(dev, DRIVER_MODESET)) if (!drm_core_check_feature(dev, DRIVER_MODESET))
i915_gem_evict_everything(dev); i915_gem_evict_everything(dev);
del_timer_sync(&dev_priv->gpu_error.hangcheck_timer);
i915_kernel_lost_context(dev); i915_kernel_lost_context(dev);
i915_gem_cleanup_ringbuffer(dev); i915_gem_cleanup_ringbuffer(dev);
/* Cancel the retire work handler, which should be idle now. */ /* Hack! Don't let anybody do execbuf while we don't control the chip.
* We need to replace this with a semaphore, or something.
* And not confound ums.mm_suspended!
*/
dev_priv->ums.mm_suspended = !drm_core_check_feature(dev,
DRIVER_MODESET);
mutex_unlock(&dev->struct_mutex);
del_timer_sync(&dev_priv->gpu_error.hangcheck_timer);
cancel_delayed_work_sync(&dev_priv->mm.retire_work); cancel_delayed_work_sync(&dev_priv->mm.retire_work);
cancel_delayed_work_sync(&dev_priv->mm.idle_work); cancel_delayed_work_sync(&dev_priv->mm.idle_work);
return 0; return 0;
err:
mutex_unlock(&dev->struct_mutex);
return ret;
} }
int i915_gem_l3_remap(struct intel_ring_buffer *ring, int slice) int i915_gem_l3_remap(struct intel_ring_buffer *ring, int slice)
...@@ -4545,26 +4556,12 @@ int ...@@ -4545,26 +4556,12 @@ int
i915_gem_leavevt_ioctl(struct drm_device *dev, void *data, i915_gem_leavevt_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv) struct drm_file *file_priv)
{ {
struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
if (drm_core_check_feature(dev, DRIVER_MODESET)) if (drm_core_check_feature(dev, DRIVER_MODESET))
return 0; return 0;
drm_irq_uninstall(dev); drm_irq_uninstall(dev);
mutex_lock(&dev->struct_mutex); return i915_gem_suspend(dev);
ret = i915_gem_idle(dev);
/* Hack! Don't let anybody do execbuf while we don't control the chip.
* We need to replace this with a semaphore, or something.
* And not confound ums.mm_suspended!
*/
if (ret != 0)
dev_priv->ums.mm_suspended = 1;
mutex_unlock(&dev->struct_mutex);
return ret;
} }
void void
...@@ -4575,11 +4572,9 @@ i915_gem_lastclose(struct drm_device *dev) ...@@ -4575,11 +4572,9 @@ i915_gem_lastclose(struct drm_device *dev)
if (drm_core_check_feature(dev, DRIVER_MODESET)) if (drm_core_check_feature(dev, DRIVER_MODESET))
return; return;
mutex_lock(&dev->struct_mutex); ret = i915_gem_suspend(dev);
ret = i915_gem_idle(dev);
if (ret) if (ret)
DRM_ERROR("failed to idle hardware: %d\n", ret); DRM_ERROR("failed to idle hardware: %d\n", ret);
mutex_unlock(&dev->struct_mutex);
} }
static void static void
......
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