On 2026-03-07 19:04, Rafael Passos wrote:
> [You don't often get email from [email protected]. 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 <[email protected]>
> Co-developed-by: David Tadokoro <[email protected]>
> Signed-off-by: David Tadokoro <[email protected]>
> ---
>
> 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/[email protected]/
Additionally, the offending change missed updating register definitions, which
was
fixed here:
https://lore.kernel.org/all/[email protected]/
- 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
>
