Blob Blame History Raw
From: Lyude Paul <lyude@redhat.com>
Date: Mon, 4 Jun 2018 15:35:03 -0400
Subject: drm/amdgpu: Grab/put runtime PM references in atomic_commit_tail()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Git-commit: 97028037a38ae40c0e06789b71038d3a6045a413
Patch-mainline: v4.18-rc1
References: FATE#326289 FATE#326079 FATE#326049 FATE#322398 FATE#326166

So, unfortunately I recently made the discovery that in the upstream
kernel, the only reason that amdgpu is not currently suffering from
issues with runtime PM putting the GPU into suspend while it's driving
displays is due to the fact that on most prime systems, we have sound
devices associated with the GPU that hold their own runtime PM ref for
the GPU.

What this means however, is that in the event that there isn't any kind
of sound device active (which can easily be reproduced by building a
kernel with sound drivers disabled), the GPU will fall asleep even when
there's displays active. This appears to be in part due to the fact that
amdgpu has not actually ever relied on it's rpm_idle() function to be
the only thing keeping it running, and normally grabs it's own power
references whenever there are displays active (as can be seen with the
original pre-DC codepath in amdgpu_display_crtc_set_config() in
amdgpu_display.c). This means it's very likely that this bug was
introduced during the switch over the DC.

So to fix this, we start grabbing runtime PM references every time we
enable a previously disabled CRTC in atomic_commit_tail(). This appears
to be the correct solution, as it matches up with what i915 does in
i915/intel_runtime_pm.c.

The one sideaffect of this is that we ignore the variable that the
pre-DC code used to use for tracking when it needed runtime PM refs,
adev->have_disp_power_ref. This is mainly because there's no way for a
driver to tell whether or not all of it's CRTCs are enabled or disabled
when we've begun committing an atomic state, as there may be CRTC
commits happening in parallel that aren't contained within the atomic
state being committed. So, it's safer to just get/put a reference for
each CRTC being enabled or disabled in the new atomic state.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Acked-by: Christian König <christian.koenig@amd.com>.
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org

Acked-by: Petr Tesarik <ptesarik@suse.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -46,6 +46,7 @@
 #include <linux/moduleparam.h>
 #include <linux/version.h>
 #include <linux/types.h>
+#include <linux/pm_runtime.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic.h>
@@ -4278,6 +4279,8 @@ static void amdgpu_dm_atomic_commit_tail
 			if (dm_old_crtc_state->stream)
 				remove_stream(adev, acrtc, dm_old_crtc_state->stream);
 
+			pm_runtime_get_noresume(dev->dev);
+
 			acrtc->enabled = true;
 			acrtc->hw_mode = new_crtc_state->mode;
 			crtc->hwmode = new_crtc_state->mode;
@@ -4466,6 +4469,16 @@ static void amdgpu_dm_atomic_commit_tail
 		drm_atomic_helper_wait_for_flip_done(dev, state);
 
 	drm_atomic_helper_cleanup_planes(dev, state);
+
+	/* Finally, drop a runtime PM reference for each newly disabled CRTC,
+	 * so we can put the GPU into runtime suspend if we're not driving any
+	 * displays anymore
+	 */
+	pm_runtime_mark_last_busy(dev->dev);
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		if (old_crtc_state->active && !new_crtc_state->active)
+			pm_runtime_put_autosuspend(dev->dev);
+	}
 }