Hi,
Thanks for the quick review!
No, I think patch 1 isn't going to be needed if the issue is already
being addressed by the other one.
Timur
On Tue, 2025-07-29 at 09:50 -0400, Alex Deucher wrote:
> Applied patches 2 and 3. Is 1 still needed with the other patch Alex
> mentioned?
>
> Thanks!
>
> Alex
>
> On Tue, Jul 22, 2025 at 12:23 PM Timur Kristóf
> <timur.kris...@gmail.com> wrote:
> >
> > Apparently, both DCE 6.0 and 6.4 have 3 PLLs, but PLL0 can only
> > be used for DP. Make sure to initialize the correct amount of PLLs
> > in DC for these DCE versions and use PLL0 only for DP.
> >
> > Also, on DCE 6.0 and 6.4, the PLL0 needs to be powered on at
> > initialization as opposed to DCE 6.1 and 7.x which use a different
> > clock source for DFS.
> >
> > The following functions were used as reference from the old
> > radeon driver implementation of DCE 6.x:
> > - radeon_atom_pick_pll
> > - atombios_crtc_set_disp_eng_pll
> >
> > Signed-off-by: Timur Kristóf <timur.kris...@gmail.com>
> > ---
> > .../display/dc/clk_mgr/dce100/dce_clk_mgr.c | 5 +++
> > .../dc/resource/dce60/dce60_resource.c | 34 +++++++++++----
> > ----
> > 2 files changed, 25 insertions(+), 14 deletions(-)
> >
> > diff --git
> > a/drivers/gpu/drm/amd/display/dc/clk_mgr/dce100/dce_clk_mgr.c
> > b/drivers/gpu/drm/amd/display/dc/clk_mgr/dce100/dce_clk_mgr.c
> > index 26feefbb8990..f5ad0a177038 100644
> > --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dce100/dce_clk_mgr.c
> > +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dce100/dce_clk_mgr.c
> > @@ -245,6 +245,11 @@ int dce_set_clock(
> > pxl_clk_params.target_pixel_clock_100hz = requested_clk_khz
> > * 10;
> > pxl_clk_params.pll_id = CLOCK_SOURCE_ID_DFS;
> >
> > + /* DCE 6.0, DCE 6.4: engine clock is the same as PLL0 */
> > + if (clk_mgr_base->ctx->dce_version == DCE_VERSION_6_0 ||
> > + clk_mgr_base->ctx->dce_version == DCE_VERSION_6_4)
> > + pxl_clk_params.pll_id = CLOCK_SOURCE_ID_PLL0;
> > +
> > if (clk_mgr_dce->dfs_bypass_active)
> > pxl_clk_params.flags.SET_DISPCLK_DFS_BYPASS = true;
> >
> > diff --git
> > a/drivers/gpu/drm/amd/display/dc/resource/dce60/dce60_resource.c
> > b/drivers/gpu/drm/amd/display/dc/resource/dce60/dce60_resource.c
> > index 58b59d52dc9d..53b60044653f 100644
> > ---
> > a/drivers/gpu/drm/amd/display/dc/resource/dce60/dce60_resource.c
> > +++
> > b/drivers/gpu/drm/amd/display/dc/resource/dce60/dce60_resource.c
> > @@ -373,7 +373,7 @@ static const struct resource_caps res_cap = {
> > .num_timing_generator = 6,
> > .num_audio = 6,
> > .num_stream_encoder = 6,
> > - .num_pll = 2,
> > + .num_pll = 3,
> > .num_ddc = 6,
> > };
> >
> > @@ -389,7 +389,7 @@ static const struct resource_caps res_cap_64 =
> > {
> > .num_timing_generator = 2,
> > .num_audio = 2,
> > .num_stream_encoder = 2,
> > - .num_pll = 2,
> > + .num_pll = 3,
> > .num_ddc = 2,
> > };
> >
> > @@ -973,21 +973,24 @@ static bool dce60_construct(
> >
> > if (bp->fw_info_valid && bp-
> > >fw_info.external_clock_source_frequency_for_dp != 0) {
> > pool->base.dp_clock_source =
> > - dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_EXTERNAL, NULL, true);
> > + dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_EXTERNAL, NULL, true);
> >
> > + /* DCE 6.0 and 6.4: PLL0 can only be used with DP.
> > Don't initialize it here. */
> > pool->base.clock_sources[0] =
> > - dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL0, &clk_src_regs[0], false);
> > + dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL1, &clk_src_regs[1], false);
> > pool->base.clock_sources[1] =
> > - dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL1, &clk_src_regs[1], false);
> > + dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL2, &clk_src_regs[2], false);
> > pool->base.clk_src_count = 2;
> >
> > } else {
> > pool->base.dp_clock_source =
> > - dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL0, &clk_src_regs[0], true);
> > + dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL0, &clk_src_regs[0], true);
> >
> > pool->base.clock_sources[0] =
> > - dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL1, &clk_src_regs[1], false);
> > - pool->base.clk_src_count = 1;
> > + dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL1, &clk_src_regs[1], false);
> > + pool->base.clock_sources[1] =
> > + dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL2, &clk_src_regs[2], false);
> > + pool->base.clk_src_count = 2;
> > }
> >
> > if (pool->base.dp_clock_source == NULL) {
> > @@ -1365,21 +1368,24 @@ static bool dce64_construct(
> >
> > if (bp->fw_info_valid && bp-
> > >fw_info.external_clock_source_frequency_for_dp != 0) {
> > pool->base.dp_clock_source =
> > - dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_EXTERNAL, NULL, true);
> > + dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_EXTERNAL, NULL, true);
> >
> > + /* DCE 6.0 and 6.4: PLL0 can only be used with DP.
> > Don't initialize it here. */
> > pool->base.clock_sources[0] =
> > - dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL1, &clk_src_regs[0], false);
> > + dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL1, &clk_src_regs[1], false);
> > pool->base.clock_sources[1] =
> > - dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL2, &clk_src_regs[1], false);
> > + dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL2, &clk_src_regs[2], false);
> > pool->base.clk_src_count = 2;
> >
> > } else {
> > pool->base.dp_clock_source =
> > - dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL1, &clk_src_regs[0], true);
> > + dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL0, &clk_src_regs[0], true);
> >
> > pool->base.clock_sources[0] =
> > - dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL2, &clk_src_regs[1], false);
> > - pool->base.clk_src_count = 1;
> > + dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL1, &clk_src_regs[1], false);
> > + pool->base.clock_sources[1] =
> > + dce60_clock_source_create(ctx, bp,
> > CLOCK_SOURCE_ID_PLL2, &clk_src_regs[2], false);
> > + pool->base.clk_src_count = 2;
> > }
> >
> > if (pool->base.dp_clock_source == NULL) {
> > --
> > 2.50.1
> >
