[PATCH 12/17] Revert "drm/amd/display: Fix green screen issue after suspend"
Mario Limonciello
mario.limonciello at amd.com
Mon Dec 16 20:20:03 UTC 2024
On 12/13/2024 09:52, Rodrigo Siqueira wrote:
> This reverts commit 87b7ebc2e16c14d32a912f18206a4d6cc9abc3e8.
Blast from the past, that's from 5.7!
Nice tech debt removal.
>
> A long time ago, we had an issue with the Raven system when it was
> connected to two displays: one with DP and another with HDMI. After the
> system woke up from suspension, we saw a solid green screen caused by an
> underflow generated by bad DCC metadata. To workaround this issue, the
> 'commit 87b7ebc2e16c ("drm/amd/display: Fix green screen issue after
> suspend")' was introduced to disable the DCC for a few frames after in
> the resume phase. However, in hindsight, this solution was probably a
> workaround at the kernel level for some issues from another part
> (probably other driver components or user space). After applying this
> patch and trying to reproduce the green issue in a similar hardware
> system but using the latest kernel and userspace, we cannot see the
> issue, which makes this workaround obsolete and creates extra
> unnecessary complexity to the code; for all of this reason, this commit
> reverts the original change.
>
> Cc: Mario Limonciello <mario.limonciello at amd.com>
> Cc: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer at amd.com>
> Reviewed-by: Aurabindo Pillai <aurabindo.pillai at amd.com>
> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 ++++------
> .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 22 +++++++------------
> .../amd/display/amdgpu_dm/amdgpu_dm_plane.h | 3 +--
> 3 files changed, 13 insertions(+), 24 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 3407741152f7..4f0267b2d5ad 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5524,8 +5524,7 @@ fill_dc_plane_info_and_addr(struct amdgpu_device *adev,
> const u64 tiling_flags,
> struct dc_plane_info *plane_info,
> struct dc_plane_address *address,
> - bool tmz_surface,
> - bool force_disable_dcc)
> + bool tmz_surface)
> {
> const struct drm_framebuffer *fb = plane_state->fb;
> const struct amdgpu_framebuffer *afb =
> @@ -5624,7 +5623,7 @@ fill_dc_plane_info_and_addr(struct amdgpu_device *adev,
> &plane_info->tiling_info,
> &plane_info->plane_size,
> &plane_info->dcc, address,
> - tmz_surface, force_disable_dcc);
> + tmz_surface);
> if (ret)
> return ret;
>
> @@ -5645,7 +5644,6 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
> struct dc_scaling_info scaling_info;
> struct dc_plane_info plane_info;
> int ret;
> - bool force_disable_dcc = false;
>
> ret = amdgpu_dm_plane_fill_dc_scaling_info(adev, plane_state, &scaling_info);
> if (ret)
> @@ -5656,13 +5654,11 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
> dc_plane_state->clip_rect = scaling_info.clip_rect;
> dc_plane_state->scaling_quality = scaling_info.scaling_quality;
>
> - force_disable_dcc = adev->asic_type == CHIP_RAVEN && adev->in_suspend;
> ret = fill_dc_plane_info_and_addr(adev, plane_state,
> afb->tiling_flags,
> &plane_info,
> &dc_plane_state->address,
> - afb->tmz_surface,
> - force_disable_dcc);
> + afb->tmz_surface);
> if (ret)
> return ret;
>
> @@ -9099,7 +9095,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> afb->tiling_flags,
> &bundle->plane_infos[planes_count],
> &bundle->flip_addrs[planes_count].address,
> - afb->tmz_surface, false);
> + afb->tmz_surface);
>
> drm_dbg_state(state->dev, "plane: id=%d dcc_en=%d\n",
> new_plane_state->plane->index,
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index c10cabb8b42b..1ec4e4b9ea94 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -310,8 +310,7 @@ static int amdgpu_dm_plane_fill_gfx9_plane_attributes_from_modifiers(struct amdg
> const struct plane_size *plane_size,
> union dc_tiling_info *tiling_info,
> struct dc_plane_dcc_param *dcc,
> - struct dc_plane_address *address,
> - const bool force_disable_dcc)
> + struct dc_plane_address *address)
> {
> const uint64_t modifier = afb->base.modifier;
> int ret = 0;
> @@ -319,7 +318,7 @@ static int amdgpu_dm_plane_fill_gfx9_plane_attributes_from_modifiers(struct amdg
> amdgpu_dm_plane_fill_gfx9_tiling_info_from_modifier(adev, tiling_info, modifier);
> tiling_info->gfx9.swizzle = amdgpu_dm_plane_modifier_gfx9_swizzle_mode(modifier);
>
> - if (amdgpu_dm_plane_modifier_has_dcc(modifier) && !force_disable_dcc) {
> + if (amdgpu_dm_plane_modifier_has_dcc(modifier)) {
> uint64_t dcc_address = afb->address + afb->base.offsets[1];
> bool independent_64b_blks = AMD_FMT_MOD_GET(DCC_INDEPENDENT_64B, modifier);
> bool independent_128b_blks = AMD_FMT_MOD_GET(DCC_INDEPENDENT_128B, modifier);
> @@ -361,8 +360,7 @@ static int amdgpu_dm_plane_fill_gfx12_plane_attributes_from_modifiers(struct amd
> const struct plane_size *plane_size,
> union dc_tiling_info *tiling_info,
> struct dc_plane_dcc_param *dcc,
> - struct dc_plane_address *address,
> - const bool force_disable_dcc)
> + struct dc_plane_address *address)
> {
> const uint64_t modifier = afb->base.modifier;
> int ret = 0;
> @@ -372,7 +370,7 @@ static int amdgpu_dm_plane_fill_gfx12_plane_attributes_from_modifiers(struct amd
>
> tiling_info->gfx9.swizzle = amdgpu_dm_plane_modifier_gfx9_swizzle_mode(modifier);
>
> - if (amdgpu_dm_plane_modifier_has_dcc(modifier) && !force_disable_dcc) {
> + if (amdgpu_dm_plane_modifier_has_dcc(modifier)) {
> int max_compressed_block = AMD_FMT_MOD_GET(DCC_MAX_COMPRESSED_BLOCK, modifier);
>
> dcc->enable = 1;
> @@ -840,8 +838,7 @@ int amdgpu_dm_plane_fill_plane_buffer_attributes(struct amdgpu_device *adev,
> struct plane_size *plane_size,
> struct dc_plane_dcc_param *dcc,
> struct dc_plane_address *address,
> - bool tmz_surface,
> - bool force_disable_dcc)
> + bool tmz_surface)
> {
> const struct drm_framebuffer *fb = &afb->base;
> int ret;
> @@ -901,16 +898,14 @@ int amdgpu_dm_plane_fill_plane_buffer_attributes(struct amdgpu_device *adev,
> ret = amdgpu_dm_plane_fill_gfx12_plane_attributes_from_modifiers(adev, afb, format,
> rotation, plane_size,
> tiling_info, dcc,
> - address,
> - force_disable_dcc);
> + address);
> if (ret)
> return ret;
> } else if (adev->family >= AMDGPU_FAMILY_AI) {
> ret = amdgpu_dm_plane_fill_gfx9_plane_attributes_from_modifiers(adev, afb, format,
> rotation, plane_size,
> tiling_info, dcc,
> - address,
> - force_disable_dcc);
> + address);
> if (ret)
> return ret;
> } else {
> @@ -1001,14 +996,13 @@ static int amdgpu_dm_plane_helper_prepare_fb(struct drm_plane *plane,
> dm_plane_state_old->dc_state != dm_plane_state_new->dc_state) {
> struct dc_plane_state *plane_state =
> dm_plane_state_new->dc_state;
> - bool force_disable_dcc = !plane_state->dcc.enable;
>
> amdgpu_dm_plane_fill_plane_buffer_attributes(
> adev, afb, plane_state->format, plane_state->rotation,
> afb->tiling_flags,
> &plane_state->tiling_info, &plane_state->plane_size,
> &plane_state->dcc, &plane_state->address,
> - afb->tmz_surface, force_disable_dcc);
> + afb->tmz_surface);
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h
> index 6498359bff6f..2eef13b1c05a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h
> @@ -51,8 +51,7 @@ int amdgpu_dm_plane_fill_plane_buffer_attributes(struct amdgpu_device *adev,
> struct plane_size *plane_size,
> struct dc_plane_dcc_param *dcc,
> struct dc_plane_address *address,
> - bool tmz_surface,
> - bool force_disable_dcc);
> + bool tmz_surface);
>
> int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
> struct drm_plane *plane,
More information about the amd-gfx
mailing list