Re: [PATCH] drm/amd/display: fix resuming from S3 sleep for Renoir iGPU

[Leo Li <sunpeng.li@amd.com>]

On 2026-03-07 19:04, Rafael Passos wrote:
> [You don't often get email from rafael@rcpassos.me. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> [WHAT]
> Set the register offset MICROSECOND_TIME_BASE_DIV in dccg_registers for DCN21.
> Introduce a new dccg21_init function, used in dccg_funcs.dccg_init for DCN21.
> The new dccg21_init sets 0x00120464 to set the MICROSECOND_TIME_BASE_DIV
> register instead of 0x00120264, set by dccg2_init.
> 
> [WHY]
> The previous commit introduced a change where the dcn21_s0i3_golden_init_wa
> function used to read the MICROSECOND_TIME_BASE_DIV reg from hwseq, and
> now started reading from dccg using dccg2_is_s0i3_golden_init_wa_done.
> However, this register is not properly initialized in dccg.
> Also, the value was initialized to 0x00120264 by dccg2_init, but
> compared to 0x00120464. For this reason, we created a new dccg21_init
> with the values specific to this card.
> 
> Fixes: 4c595e75110e ("drm/amd/display: Migrate DCCG registers access from hwseq to dccg component.")
> Signed-off-by: Rafael Passos <rafael@rcpassos.me>
> Co-developed-by: David Tadokoro <davidbtadokoro@ime.usp.br>
> Signed-off-by: David Tadokoro <davidbtadokoro@ime.usp.br>
> ---
> 
> It took a lot of debugging to get to this point.
> We are not sure this is the right fix, but it works.
> We found that when reading the MICROSECOND_TIME_BASE_DIV register,
> the offset was 13b in the old path and 0 in the new path.
> 
> The dcn21_s0i3_golden_init_wa is called when booting
> and when waking from sleep. It compares the value from
> MICROSECOND_TIME_BASE_DIV to 0x00120464.
> When booting, the value was different (and this function returns true).
> When waking from sleep, the value should be equal; thus,
> this function would return false.
> 
> After 4c595e75110e, the value was always different than 0x00120464, so
> this function always returned true, failing to wake the screen.
> This happened because the offset of MICROSECOND_TIME_BASE_DIV was 0,
> and READ_REG always returned 0x1186A0 (value from MILLISECOND_TIME_BASE_DIV?).
> 
> Things we are unsure of:
> - We used SR to set MICROSECOND_TIME_BASE_DIV direclty in the
>         dccg_registers struct. We did not find other examples of this.
>         Should we set MICROSECOND_TIME_BASE_DIV to the DCCG_COMMON_REG_LIST_DCN_BASE ?
>         I only added it to DCN21, because it is the hardware I have (and validated it works).
> - We changed 0x00120264 to 0x00120464 in the init, but dccg2 has the
>         same difference in setting and reading. We would like to know if this issue
>         also affects dccg2 (and other cards), or if we are missing something.
>         Maybe we should change this value in dccg2_is_s0i3_golden_init_wa_done.
> 
> It applies to the mainline master, amdgpu drm-next and amd-staging-drm-next.
> 
> Any feedback is appreciated. It was a fun-frustrating-veryfun journey. :)
> Code written only by humans.

Hi Rafael,

Thanks for bisecting and identifying the root cause. A fix has been submitted here:
https://lore.kernel.org/all/20260306031932.136179-14-alex.hung@amd.com/

Additionally, the offending change missed updating register definitions, which was
fixed here:
https://lore.kernel.org/all/20260306031932.136179-10-alex.hung@amd.com/

- Leo

> 
> 
>  .../drm/amd/display/dc/dccg/dcn21/dcn21_dccg.c  | 17 ++++++++++++++++-
>  .../display/dc/resource/dcn21/dcn21_resource.c  |  3 ++-
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dccg/dcn21/dcn21_dccg.c b/drivers/gpu/drm/amd/display/dc/dccg/dcn21/dcn21_dccg.c
> index 75c69348027e..6f96e9c189dc 100644
> --- a/drivers/gpu/drm/amd/display/dc/dccg/dcn21/dcn21_dccg.c
> +++ b/drivers/gpu/drm/amd/display/dc/dccg/dcn21/dcn21_dccg.c
> @@ -96,6 +96,21 @@ static void dccg21_update_dpp_dto(struct dccg *dccg, int dpp_inst, int req_dppcl
>         dccg->pipe_dppclk_khz[dpp_inst] = req_dppclk;
>  }
> 
> +void dccg21_init(struct dccg *dccg)
> +{
> +       struct dcn_dccg *dccg_dcn = TO_DCN_DCCG(dccg);
> +
> +       /* Hardcoded register values for DCN21
> +        * These are specific to 100Mhz refclk
> +        * Different ASICs with different refclk may override this in their own init
> +        */
> +       REG_WRITE(MICROSECOND_TIME_BASE_DIV, 0x00120464);
> +       REG_WRITE(MILLISECOND_TIME_BASE_DIV, 0x001186a0);
> +       REG_WRITE(DISPCLK_FREQ_CHANGE_CNTL, 0x0e01003c);
> +
> +       if (REG(REFCLK_CNTL))
> +               REG_WRITE(REFCLK_CNTL, 0);
> +}
> 
>  static const struct dccg_funcs dccg21_funcs = {
>         .update_dpp_dto = dccg21_update_dpp_dto,
> @@ -103,7 +118,7 @@ static const struct dccg_funcs dccg21_funcs = {
>         .set_fifo_errdet_ovr_en = dccg2_set_fifo_errdet_ovr_en,
>         .otg_add_pixel = dccg2_otg_add_pixel,
>         .otg_drop_pixel = dccg2_otg_drop_pixel,
> -       .dccg_init = dccg2_init,
> +       .dccg_init = dccg21_init,
>         .refclk_setup = dccg2_refclk_setup, /* Deprecated - for backward compatibility only */
>         .allow_clock_gating = dccg2_allow_clock_gating,
>         .enable_memory_low_power = dccg2_enable_memory_low_power,
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn21/dcn21_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn21/dcn21_resource.c
> index 0f4307f8f3dd..7f8f657eb0f2 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn21/dcn21_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn21/dcn21_resource.c
> @@ -222,7 +222,8 @@ static const struct dce_audio_mask audio_mask = {
>  };
> 
>  static const struct dccg_registers dccg_regs = {
> -               DCCG_COMMON_REG_LIST_DCN_BASE()
> +               DCCG_COMMON_REG_LIST_DCN_BASE(),
> +               SR(MICROSECOND_TIME_BASE_DIV)
>  };
> 
>  static const struct dccg_shift dccg_shift = {
> --
> 2.53.0
>