Currently, on a handful of ASICs. We allow the framebuffer for a given plane to exist in either VRAM or GTT. However, if the plane's new framebuffer is in a different memory domain than it's previous framebuffer, flipping between them can cause the screen to flicker. So, to fix this, don't perform an immediate flip in the aforementioned case.
Cc: stable@vger.kernel.org Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2354 Fixes: 81d0bcf99009 ("drm/amdgpu: make display pinning more flexible (v2)") Signed-off-by: Hamza Mahfooz hamza.mahfooz@amd.com --- v2: make a number of clarifications to the commit message and drop locking. v3: use a stronger check --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index dfcb9815b5a8..875111340203 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7900,6 +7900,13 @@ static void amdgpu_dm_commit_cursors(struct drm_atomic_state *state) amdgpu_dm_plane_handle_cursor_update(plane, old_plane_state); }
+static inline uint32_t get_mem_type(struct drm_framebuffer *fb) +{ + struct amdgpu_bo *abo = gem_to_amdgpu_bo(fb->obj[0]); + + return abo->tbo.resource ? abo->tbo.resource->mem_type : 0; +} + static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, struct dc_state *dc_state, struct drm_device *dev, @@ -7919,6 +7926,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc)); int planes_count = 0, vpos, hpos; unsigned long flags; + uint32_t mem_type; u32 target_vblank, last_flip_vblank; bool vrr_active = amdgpu_dm_crtc_vrr_active(acrtc_state); bool cursor_update = false; @@ -8040,13 +8048,17 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, } }
+ mem_type = get_mem_type(old_plane_state->fb); + /* * Only allow immediate flips for fast updates that don't - * change FB pitch, DCC state, rotation or mirroing. + * change memory domain, FB pitch, DCC state, rotation or + * mirroring. */ bundle->flip_addrs[planes_count].flip_immediate = crtc->state->async_flip && - acrtc_state->update_type == UPDATE_TYPE_FAST; + acrtc_state->update_type == UPDATE_TYPE_FAST && + mem_type && get_mem_type(fb) == mem_type;
timestamp_ns = ktime_get_ns(); bundle->flip_addrs[planes_count].flip_timestamp_in_us = div_u64(timestamp_ns, 1000);
[Public]
Reviewed-by: Roman Li Roman.Li@amd.com
-----Original Message----- From: amd-gfx amd-gfx-bounces@lists.freedesktop.org On Behalf Of Hamza Mahfooz Sent: Thursday, April 20, 2023 9:44 AM To: amd-gfx@lists.freedesktop.org Cc: Wang, Chao-kai (Stylon) Stylon.Wang@amd.com; Tuikov, Luben Luben.Tuikov@amd.com; dri-devel@lists.freedesktop.org; Li, Sun peng (Leo) Sunpeng.Li@amd.com; David Airlie airlied@gmail.com; Zhuo, Qingqing (Lillian) Qingqing.Zhuo@amd.com; Pan, Xinhui Xinhui.Pan@amd.com; Siqueira, Rodrigo Rodrigo.Siqueira@amd.com; linux-kernel@vger.kernel.org; stable@vger.kernel.org; Hans de Goede hdegoede@redhat.com; Pillai, Aurabindo Aurabindo.Pillai@amd.com; Wu, Hersen hersenxs.wu@amd.com; Mahfooz, Hamza Hamza.Mahfooz@amd.com; Daniel Vetter daniel@ffwll.ch; Deucher, Alexander Alexander.Deucher@amd.com; Wentland, Harry Harry.Wentland@amd.com; Koenig, Christian Christian.Koenig@amd.com Subject: [PATCH v3] drm/amd/display: fix flickering caused by S/G mode
Currently, on a handful of ASICs. We allow the framebuffer for a given plane to exist in either VRAM or GTT. However, if the plane's new framebuffer is in a different memory domain than it's previous framebuffer, flipping between them can cause the screen to flicker. So, to fix this, don't perform an immediate flip in the aforementioned case.
Cc: stable@vger.kernel.org Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2354 Fixes: 81d0bcf99009 ("drm/amdgpu: make display pinning more flexible (v2)") Signed-off-by: Hamza Mahfooz hamza.mahfooz@amd.com
v2: make a number of clarifications to the commit message and drop locking. v3: use a stronger check
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 16 ++++++++++++++-
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index dfcb9815b5a8..875111340203 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7900,6 +7900,13 @@ static void amdgpu_dm_commit_cursors(struct drm_atomic_state *state) amdgpu_dm_plane_handle_cursor_update(plane, old_plane_state); }
+static inline uint32_t get_mem_type(struct drm_framebuffer *fb) {
struct amdgpu_bo *abo = gem_to_amdgpu_bo(fb->obj[0]);return abo->tbo.resource ? abo->tbo.resource->mem_type : 0; }static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, struct dc_state *dc_state, struct drm_device *dev, @@ -7919,6 +7926,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc)); int planes_count = 0, vpos, hpos; unsigned long flags;
uint32_t mem_type; u32 target_vblank, last_flip_vblank; bool vrr_active = amdgpu_dm_crtc_vrr_active(acrtc_state); bool cursor_update = false;@@ -8040,13 +8048,17 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, } }
mem_type = get_mem_type(old_plane_state->fb);/* * Only allow immediate flips for fast updates that don't
* change FB pitch, DCC state, rotation or mirroing.
* change memory domain, FB pitch, DCC state, rotation or* mirroring. */ bundle->flip_addrs[planes_count].flip_immediate = crtc->state->async_flip &&
acrtc_state->update_type == UPDATE_TYPE_FAST;
acrtc_state->update_type == UPDATE_TYPE_FAST &&mem_type && get_mem_type(fb) == mem_type; timestamp_ns = ktime_get_ns(); bundle->flip_addrs[planes_count].flip_timestamp_in_us =div_u64(timestamp_ns, 1000);
2.40.0
Am 20.04.23 um 15:44 schrieb Hamza Mahfooz:
Currently, on a handful of ASICs. We allow the framebuffer for a given plane to exist in either VRAM or GTT. However, if the plane's new framebuffer is in a different memory domain than it's previous framebuffer, flipping between them can cause the screen to flicker. So, to fix this, don't perform an immediate flip in the aforementioned case.
Cc: stable@vger.kernel.org Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2354 Fixes: 81d0bcf99009 ("drm/amdgpu: make display pinning more flexible (v2)") Signed-off-by: Hamza Mahfooz hamza.mahfooz@amd.com
v2: make a number of clarifications to the commit message and drop locking. v3: use a stronger check
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index dfcb9815b5a8..875111340203 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7900,6 +7900,13 @@ static void amdgpu_dm_commit_cursors(struct drm_atomic_state *state) amdgpu_dm_plane_handle_cursor_update(plane, old_plane_state); } +static inline uint32_t get_mem_type(struct drm_framebuffer *fb) +{
- struct amdgpu_bo *abo = gem_to_amdgpu_bo(fb->obj[0]);
- return abo->tbo.resource ? abo->tbo.resource->mem_type : 0;
+}
- static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, struct dc_state *dc_state, struct drm_device *dev,
@@ -7919,6 +7926,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc)); int planes_count = 0, vpos, hpos; unsigned long flags;
- uint32_t mem_type; u32 target_vblank, last_flip_vblank; bool vrr_active = amdgpu_dm_crtc_vrr_active(acrtc_state); bool cursor_update = false;
@@ -8040,13 +8048,17 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, } }
mem_type = get_mem_type(old_plane_state->fb);- /*
- Only allow immediate flips for fast updates that don't
* change FB pitch, DCC state, rotation or mirroing.
* change memory domain, FB pitch, DCC state, rotation or* mirroring.
acrtc_state->update_type == UPDATE_TYPE_FAST;
acrtc_state->update_type == UPDATE_TYPE_FAST &&mem_type && get_mem_type(fb) == mem_type;
I think you can actually drop checking mem_type here.
When the FB has no backing store, we actually don't have anything to scan out. In this case flickering is the least of your problems :)
Christian
timestamp_ns = ktime_get_ns(); bundle->flip_addrs[planes_count].flip_timestamp_in_us = div_u64(timestamp_ns, 1000);
linux-stable-mirror@lists.linaro.org
-
Christian König -
Hamza Mahfooz -
Li, Roman