From patchwork Mon Dec 17 16:04:32 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jerome Glisse X-Patchwork-Id: 1887881 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 289B43FCA5 for ; Mon, 17 Dec 2012 16:08:50 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C3558E61BC for ; Mon, 17 Dec 2012 08:08:49 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-gg0-f177.google.com (mail-gg0-f177.google.com [209.85.161.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 9B337E61DF for ; Mon, 17 Dec 2012 08:07:44 -0800 (PST) Received: by mail-gg0-f177.google.com with SMTP id y3so1162369ggc.36 for ; Mon, 17 Dec 2012 08:07:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer; bh=cCCj0i8gmn3TpeE8Kj4Y430FsBrVx5YhdHQGJKsg424=; b=YmlDsPWbr8fskHz0YIDTTnLZibLmbrR1TgQxBNeVxGDqOq2+FEVgKr6qAW94ku2zMY B5YiEEh0U5eAZxywVuXZ9ck6IVZ/sKx+oL9CO6wo9l5Q149GYePxT9o4zSqpTzwBOycM 9x7k4WcfScQveXUiuc7EML3qZDCMNJx/+OZK7GKO1GmJ8akDzIxFTOxR3+qQfgVl6vdO g1HsgwZC3e3kfLpVjaB50/IgX3ZTMQHf9V+GP6A7tnntyfOUlTjmEuwbXgD28HJwJb55 8mbgPy1ZFYIqQNiaC+byn+Enxtr1F/TJU2jzLddtOyDFB30xKLxNNLZ/Cki03Z2iBWwG gwPw== Received: by 10.58.252.72 with SMTP id zq8mr24974967vec.20.1355760464136; Mon, 17 Dec 2012 08:07:44 -0800 (PST) Received: from localhost.boston.devel.redhat.com ([66.187.233.206]) by mx.google.com with ESMTPS id a10sm11562117vez.10.2012.12.17.08.07.41 (version=SSLv3 cipher=OTHER); Mon, 17 Dec 2012 08:07:43 -0800 (PST) From: j.glisse@gmail.com To: dri-devel@lists.freedesktop.org Subject: [PATCH] drm/radeon: avoid deadlock in pm path when waiting for fence Date: Mon, 17 Dec 2012 11:04:32 -0500 Message-Id: <1355760272-18402-1-git-send-email-j.glisse@gmail.com> X-Mailer: git-send-email 1.7.11.7 Cc: Jerome Glisse X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org From: Jerome Glisse radeon_fence_wait_empty_locked should not trigger GPU reset as no place where it's call from would benefit from such thing and it actually lead to a kernel deadlock in case the reset is triggered from pm codepath. Instead force ring completion in place where it makes sense or return early in others. Signed-off-by: Jerome Glisse Reviewed-by: Christian König --- drivers/gpu/drm/radeon/radeon.h | 2 +- drivers/gpu/drm/radeon/radeon_device.c | 13 +++++++++++-- drivers/gpu/drm/radeon/radeon_fence.c | 30 ++++++++++++++---------------- drivers/gpu/drm/radeon/radeon_pm.c | 15 ++++++++++++--- 4 files changed, 38 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 9c7625c..071b2d7 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -231,7 +231,7 @@ void radeon_fence_process(struct radeon_device *rdev, int ring); bool radeon_fence_signaled(struct radeon_fence *fence); int radeon_fence_wait(struct radeon_fence *fence, bool interruptible); int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring); -void radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring); +int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring); int radeon_fence_wait_any(struct radeon_device *rdev, struct radeon_fence **fences, bool intr); diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 774fae7..53a9223 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1163,6 +1163,7 @@ int radeon_suspend_kms(struct drm_device *dev, pm_message_t state) struct drm_crtc *crtc; struct drm_connector *connector; int i, r; + bool force_completion = false; if (dev == NULL || dev->dev_private == NULL) { return -ENODEV; @@ -1205,8 +1206,16 @@ int radeon_suspend_kms(struct drm_device *dev, pm_message_t state) mutex_lock(&rdev->ring_lock); /* wait for gpu to finish processing current batch */ - for (i = 0; i < RADEON_NUM_RINGS; i++) - radeon_fence_wait_empty_locked(rdev, i); + for (i = 0; i < RADEON_NUM_RINGS; i++) { + r = radeon_fence_wait_empty_locked(rdev, i); + if (r) { + /* delay GPU reset to resume */ + force_completion = true; + } + } + if (force_completion) { + radeon_fence_driver_force_completion(rdev); + } mutex_unlock(&rdev->ring_lock); radeon_save_bios_scratch_regs(rdev); diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c index bf7b20e..28c09b6 100644 --- a/drivers/gpu/drm/radeon/radeon_fence.c +++ b/drivers/gpu/drm/radeon/radeon_fence.c @@ -609,26 +609,20 @@ int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring) * Returns 0 if the fences have passed, error for all other cases. * Caller must hold ring lock. */ -void radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring) +int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring) { uint64_t seq = rdev->fence_drv[ring].sync_seq[ring]; + int r; - while(1) { - int r; - r = radeon_fence_wait_seq(rdev, seq, ring, false, false); + r = radeon_fence_wait_seq(rdev, seq, ring, false, false); + if (r) { if (r == -EDEADLK) { - mutex_unlock(&rdev->ring_lock); - r = radeon_gpu_reset(rdev); - mutex_lock(&rdev->ring_lock); - if (!r) - continue; - } - if (r) { - dev_err(rdev->dev, "error waiting for ring to become" - " idle (%d)\n", r); + return -EDEADLK; } - return; + dev_err(rdev->dev, "error waiting for ring[%d] to become idle (%d)\n", + ring, r); } + return 0; } /** @@ -854,13 +848,17 @@ int radeon_fence_driver_init(struct radeon_device *rdev) */ void radeon_fence_driver_fini(struct radeon_device *rdev) { - int ring; + int ring, r; mutex_lock(&rdev->ring_lock); for (ring = 0; ring < RADEON_NUM_RINGS; ring++) { if (!rdev->fence_drv[ring].initialized) continue; - radeon_fence_wait_empty_locked(rdev, ring); + r = radeon_fence_wait_empty_locked(rdev, ring); + if (r) { + /* no need to trigger GPU reset as we are unloading */ + radeon_fence_driver_force_completion(rdev); + } wake_up_all(&rdev->fence_queue); radeon_scratch_free(rdev, rdev->fence_drv[ring].scratch_reg); rdev->fence_drv[ring].initialized = false; diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index aa14dbb..0bfa656 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -234,7 +234,7 @@ static void radeon_set_power_state(struct radeon_device *rdev) static void radeon_pm_set_clocks(struct radeon_device *rdev) { - int i; + int i, r; /* no need to take locks, etc. if nothing's going to change */ if ((rdev->pm.requested_clock_mode_index == rdev->pm.current_clock_mode_index) && @@ -248,8 +248,17 @@ static void radeon_pm_set_clocks(struct radeon_device *rdev) /* wait for the rings to drain */ for (i = 0; i < RADEON_NUM_RINGS; i++) { struct radeon_ring *ring = &rdev->ring[i]; - if (ring->ready) - radeon_fence_wait_empty_locked(rdev, i); + if (!ring->ready) { + continue; + } + r = radeon_fence_wait_empty_locked(rdev, i); + if (r) { + /* needs a GPU reset dont reset here */ + mutex_unlock(&rdev->ring_lock); + up_write(&rdev->pm.mclk_lock); + mutex_unlock(&rdev->ddev->struct_mutex); + return; + } } radeon_unmap_vram_bos(rdev);