This patch is wrong as noted by MIchel a while ago - quote from his
review of the patch.
"Actually, pflip_status should really only be set to
AMDGPU_FLIP_SUBMITTED after the flip has been programmed to the
hardware, at least as far as the lock holder is concerned. That's why
the code was previously holding the lock until after the
dc_commit_updates_for_stream call. Otherwise, it's at least
theoretically possible that either:
* dm_pflip_high_irq is called before dc_commit_updates_for_stream, but
sees flip_status == AMDGPU_FLIP_SUBMITTED and sends the event to
userspace prematurely
* dm_pflip_high_irq is called after dc_commit_updates_for_stream, but
sees flip_status != AMDGPU_FLIP_SUBMITTED, so it incorrectly doesn't
send the event to userspace "
It shouldn't go in.
Andrey
On 07/05/2018 11:59 AM, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
This is a note to let you know that I've just added the patch titled
drm/amd/display: release spinlock before committing updates to stream
to the 4.17-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-amd-display-release-spinlock-before-committing-updates-to-stream.patch
and it can be found in the queue-4.17 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
From 4de9f38bb2cce3a4821ffb8a83d6b08f6e37d905 Mon Sep 17 00:00:00 2001
From: Shirish S <shirish.s@xxxxxxx>
Date: Tue, 26 Jun 2018 09:32:39 +0530
Subject: drm/amd/display: release spinlock before committing updates to stream
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: Shirish S <shirish.s@xxxxxxx>
commit 4de9f38bb2cce3a4821ffb8a83d6b08f6e37d905 upstream.
Currently, amdgpu_do_flip() spinlocks crtc->dev->event_lock and
releases it only after committing updates to the stream.
dc_commit_updates_for_stream() should be moved out of
spinlock for the below reasons:
1. event_lock is supposed to protect access to acrct->pflip_status _only_
2. dc_commit_updates_for_stream() has potential sleep's
and also its not appropriate to be in an atomic state
for such long sequences of code.
Signed-off-by: Shirish S <shirish.s@xxxxxxx>
Suggested-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>
Reviewed-by: Michel Dänzer <michel.daenzer@xxxxxxx>
Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx>
Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3967,10 +3967,11 @@ static void amdgpu_dm_do_flip(struct drm
if (acrtc->base.state->event)
prepare_flip_isr(acrtc);
+ spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
+
surface_updates->surface = dc_stream_get_status(acrtc_state->stream)->plane_states[0];
surface_updates->flip_addr = &addr;
-
dc_commit_updates_for_stream(adev->dm.dc,
surface_updates,
1,
@@ -3983,9 +3984,6 @@ static void amdgpu_dm_do_flip(struct drm
__func__,
addr.address.grph.addr.high_part,
addr.address.grph.addr.low_part);
-
-
- spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
}
static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
Patches currently in stable-queue which might be from shirish.s@xxxxxxx are
queue-4.17/drm-amdgpu-add-apu-support-in-vi_set_vce_clocks.patch
queue-4.17/drm-amdgpu-add-apu-support-in-vi_set_uvd_clocks.patch
queue-4.17/drm-amd-display-release-spinlock-before-committing-updates-to-stream.patch
