AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Michel Dänzer" <michel@daenzer.net>
To: "Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>
Cc: Leo Liu <leo.liu@amd.com>, James Zhu <James.Zhu@amd.com>,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: [PATCH] drm/amdgpu: Cancel delayed work when GFXOFF is disabled
Date: Fri, 13 Aug 2021 12:29:20 +0200	[thread overview]
Message-ID: <20210813102920.3458-1-michel@daenzer.net> (raw)
In-Reply-To: <20210811165211.6811-1-michel@daenzer.net>

From: Michel Dänzer <mdaenzer@redhat.com>

schedule_delayed_work does not push back the work if it was already
scheduled before, so amdgpu_device_delay_enable_gfx_off ran ~100 ms
after the first time GFXOFF was disabled and re-enabled, even if GFXOFF
was disabled and re-enabled again during those 100 ms.

This resulted in frame drops / stutter with the upcoming mutter 41
release on Navi 14, due to constantly enabling GFXOFF in the HW and
disabling it again (for getting the GPU clock counter).

To fix this, call cancel_delayed_work_sync when GFXOFF transitions from
enabled to disabled. This makes sure the delayed work will be scheduled
as intended in the reverse case.

In order to avoid a deadlock, amdgpu_device_delay_enable_gfx_off needs
to use mutex_trylock instead of mutex_lock.

v2:
* Use cancel_delayed_work_sync & mutex_trylock instead of
  mod_delayed_work.

Signed-off-by: Michel Dänzer <mdaenzer@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 ++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 13 +++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    |  3 +++
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f3fd5ec710b6..8b025f70706c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2777,7 +2777,16 @@ static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
 	struct amdgpu_device *adev =
 		container_of(work, struct amdgpu_device, gfx.gfx_off_delay_work.work);
 
-	mutex_lock(&adev->gfx.gfx_off_mutex);
+	/* mutex_lock could deadlock with cancel_delayed_work_sync in amdgpu_gfx_off_ctrl. */
+	if (!mutex_trylock(&adev->gfx.gfx_off_mutex)) {
+		/* If there's a bug which causes amdgpu_gfx_off_ctrl to be called with enable=true
+		 * when adev->gfx.gfx_off_req_count is already 0, we might race with that.
+		 * Re-schedule to make sure gfx off will be re-enabled in the HW eventually.
+		 */
+		schedule_delayed_work(&adev->gfx.gfx_off_delay_work, AMDGPU_GFX_OFF_DELAY_ENABLE);
+		return;
+	}
+
 	if (!adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
 		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true))
 			adev->gfx.gfx_off_state = true;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index a0be0772c8b3..da4c46db3093 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -28,9 +28,6 @@
 #include "amdgpu_rlc.h"
 #include "amdgpu_ras.h"
 
-/* delay 0.1 second to enable gfx off feature */
-#define GFX_OFF_DELAY_ENABLE         msecs_to_jiffies(100)
-
 /*
  * GPU GFX IP block helpers function.
  */
@@ -569,9 +566,13 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
 		adev->gfx.gfx_off_req_count--;
 
 	if (enable && !adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
-		schedule_delayed_work(&adev->gfx.gfx_off_delay_work, GFX_OFF_DELAY_ENABLE);
-	} else if (!enable && adev->gfx.gfx_off_state) {
-		if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false)) {
+		schedule_delayed_work(&adev->gfx.gfx_off_delay_work, AMDGPU_GFX_OFF_DELAY_ENABLE);
+	} else if (!enable) {
+		if (adev->gfx.gfx_off_req_count == 1 && !adev->gfx.gfx_off_state)
+			cancel_delayed_work_sync(&adev->gfx.gfx_off_delay_work);
+
+		if (adev->gfx.gfx_off_state &&
+		    !amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false)) {
 			adev->gfx.gfx_off_state = false;
 
 			if (adev->gfx.funcs->init_spm_golden) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index d43fe2ed8116..dcdb505bb7f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -32,6 +32,9 @@
 #include "amdgpu_rlc.h"
 #include "soc15.h"
 
+/* delay 0.1 second to enable gfx off feature */
+#define AMDGPU_GFX_OFF_DELAY_ENABLE msecs_to_jiffies(100)
+
 /* GFX current status */
 #define AMDGPU_GFX_NORMAL_MODE			0x00000000L
 #define AMDGPU_GFX_SAFE_MODE			0x00000001L
-- 
2.32.0


  parent reply	other threads:[~2021-08-13 10:29 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-11 16:52 [PATCH 1/2] drm/amdgpu: Use mod_delayed_work in amdgpu_gfx_off_ctrl Michel Dänzer
2021-08-11 16:52 ` [PATCH 2/2] drm/amdgpu: Use mod_delayed_work in JPEG/UVD/VCE/VCN ring_end_use hooks Michel Dänzer
2021-08-11 20:34   ` Alex Deucher
2021-08-11 21:00     ` Zhu, James
2021-08-11 21:34   ` AW: " Koenig, Christian
2021-08-11 22:12     ` Zhu, James
2021-08-11 22:22       ` Zhu, James
2021-08-12  2:42     ` Quan, Evan
2021-08-12  5:55       ` AW: " Koenig, Christian
2021-08-12  8:11         ` Michel Dänzer
2021-08-12 11:33           ` Lazar, Lijo
2021-08-12 16:54             ` Michel Dänzer
2021-08-13  4:23               ` Lazar, Lijo
2021-08-13 10:31                 ` Michel Dänzer
2021-08-13 11:18                   ` Lazar, Lijo
2021-08-16  7:33           ` Christian König
2021-08-12  2:43 ` [PATCH 1/2] drm/amdgpu: Use mod_delayed_work in amdgpu_gfx_off_ctrl Quan, Evan
2021-08-13 10:29 ` Michel Dänzer [this message]
2021-08-13 11:50   ` [PATCH] drm/amdgpu: Cancel delayed work when GFXOFF is disabled Lazar, Lijo
2021-08-13 13:34     ` Michel Dänzer
2021-08-13 14:14       ` Lazar, Lijo
2021-08-13 14:40         ` Michel Dänzer
2021-08-13 15:07           ` Lazar, Lijo
2021-08-13 16:00             ` Michel Dänzer
2021-08-16  4:13               ` Lazar, Lijo
2021-08-16 10:45                 ` Michel Dänzer
2021-08-16  7:38   ` Christian König
2021-08-16 10:38     ` Michel Dänzer
2021-08-16 10:20   ` Quan, Evan
2021-08-16 10:43     ` Michel Dänzer
2021-08-16 10:35   ` [PATCH v3] " Michel Dänzer
2021-08-16 11:33     ` Lazar, Lijo
2021-08-16 12:06       ` Christian König
2021-08-16 15:06         ` Michel Dänzer
2021-08-16 19:02           ` Alex Deucher
2021-08-17  7:51     ` Quan, Evan
2021-08-17  8:17       ` Lazar, Lijo
2021-08-17  8:35         ` Michel Dänzer
2021-08-17  8:23     ` [PATCH] " Michel Dänzer
2021-08-17  9:12       ` Lazar, Lijo
2021-08-17  9:26         ` Michel Dänzer
2021-08-17  9:37           ` Lazar, Lijo
2021-08-17  9:59             ` Michel Dänzer
2021-08-17 10:37               ` Lazar, Lijo
2021-08-17 11:06                 ` Michel Dänzer
2021-08-17 11:49                   ` Lazar, Lijo
2021-08-17 12:55                     ` Lazar, Lijo
2021-08-17  9:33       ` Quan, Evan
2021-08-18 21:56       ` Alex Deucher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210813102920.3458-1-michel@daenzer.net \
    --to=michel@daenzer.net \
    --cc=James.Zhu@amd.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=leo.liu@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox