On 07/22, Timur Kristóf 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
>
Reviewed-by: Rodrigo Siqueira <sique...@igalia.com>
Again, I'm adding other displays folks for further review.
Thanks
--
Rodrigo Siqueira
