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@linuxfoundation.org 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 streamto the 4.17-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
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@vger.kernel.org know about it.
From 4de9f38bb2cce3a4821ffb8a83d6b08f6e37d905 Mon Sep 17 00:00:00 2001 From: Shirish S shirish.s@amd.com 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@amd.com
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:
- event_lock is supposed to protect access to acrct->pflip_status _only_
- 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@amd.com Suggested-by: Andrey Grodzovsky andrey.grodzovsky@amd.com Reviewed-by: Michel Dänzer michel.daenzer@amd.com Reviewed-by: Harry Wentland harry.wentland@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com Cc: stable@vger.kernel.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
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@amd.com 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