Applied the series. Thanks!
Alex
On Tue, Sep 9, 2025 at 10:18 AM Timur Kristóf <timur.kris...@gmail.com> wrote:
>
> This commit is necessary for DC to function well with chips
> that use the legacy power management code, ie. SI and KV.
> Communicate display information from DC to the legacy PM code.
>
> Currently DC uses pm_display_cfg to communicate power management
> requirements from the display code to the DPM code.
> However, the legacy (non-DC) code path used different fields
> and therefore could not take into account anything from DC.
>
> Change the legacy display code to fill the same pm_display_cfg
> struct as DC and use the same in the legacy DPM code.
>
> To ease review and reduce churn, this commit does not yet
> delete the now unneeded code, that is done in the next commit.
>
> v2:
> Rebase.
> Fix single_display in amdgpu_dpm_pick_power_state.
>
> Signed-off-by: Timur Kristóf <timur.kris...@gmail.com>
> ---
> drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c | 67 +++++++++++++++++++
> .../gpu/drm/amd/pm/inc/amdgpu_dpm_internal.h | 2 +
> drivers/gpu/drm/amd/pm/legacy-dpm/kv_dpm.c | 4 +-
> .../gpu/drm/amd/pm/legacy-dpm/legacy_dpm.c | 5 +-
> drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c | 65 ++++++------------
> .../gpu/drm/amd/pm/powerplay/amd_powerplay.c | 11 +--
> 6 files changed, 97 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c
> b/drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c
> index 2d2d2d5e6763..9ef965e4a92e 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c
> @@ -100,3 +100,70 @@ u32 amdgpu_dpm_get_vrefresh(struct amdgpu_device *adev)
>
> return vrefresh;
> }
> +
> +void amdgpu_dpm_get_display_cfg(struct amdgpu_device *adev)
> +{
> + struct drm_device *ddev = adev_to_drm(adev);
> + struct amd_pp_display_configuration *cfg = &adev->pm.pm_display_cfg;
> + struct single_display_configuration *display_cfg;
> + struct drm_crtc *crtc;
> + struct amdgpu_crtc *amdgpu_crtc;
> + struct amdgpu_connector *conn;
> + int num_crtcs = 0;
> + int vrefresh;
> + u32 vblank_in_pixels, vblank_time_us;
> +
> + cfg->min_vblank_time = 0xffffffff; /* if the displays are off, vblank
> time is max */
> +
> + if (adev->mode_info.num_crtc &&
> adev->mode_info.mode_config_initialized) {
> + list_for_each_entry(crtc, &ddev->mode_config.crtc_list, head)
> {
> + amdgpu_crtc = to_amdgpu_crtc(crtc);
> +
> + /* The array should only contain active displays. */
> + if (!amdgpu_crtc->enabled)
> + continue;
> +
> + conn = to_amdgpu_connector(amdgpu_crtc->connector);
> + display_cfg =
> &adev->pm.pm_display_cfg.displays[num_crtcs++];
> +
> + if (amdgpu_crtc->hw_mode.clock) {
> + vrefresh =
> drm_mode_vrefresh(&amdgpu_crtc->hw_mode);
> +
> + vblank_in_pixels =
> + amdgpu_crtc->hw_mode.crtc_htotal *
> + (amdgpu_crtc->hw_mode.crtc_vblank_end
> -
> + amdgpu_crtc->hw_mode.crtc_vdisplay +
> + (amdgpu_crtc->v_border * 2));
> +
> + vblank_time_us =
> + vblank_in_pixels * 1000 /
> amdgpu_crtc->hw_mode.clock;
> +
> + /* The legacy (non-DC) code has issues with
> mclk switching
> + * with refresh rates over 120 Hz. Disable
> mclk switching.
> + */
> + if (vrefresh > 120)
> + vblank_time_us = 0;
> +
> + /* Find minimum vblank time. */
> + if (vblank_time_us < cfg->min_vblank_time)
> + cfg->min_vblank_time = vblank_time_us;
> +
> + /* Find vertical refresh rate of first active
> display. */
> + if (!cfg->vrefresh)
> + cfg->vrefresh = vrefresh;
> + }
> +
> + if (amdgpu_crtc->crtc_id < cfg->crtc_index) {
> + /* Find first active CRTC and its line time.
> */
> + cfg->crtc_index = amdgpu_crtc->crtc_id;
> + cfg->line_time_in_us = amdgpu_crtc->line_time;
> + }
> +
> + display_cfg->controller_id = amdgpu_crtc->crtc_id;
> + display_cfg->pixel_clock =
> conn->pixelclock_for_modeset;
> + }
> + }
> +
> + cfg->display_clk = adev->clock.default_dispclk;
> + cfg->num_display = num_crtcs;
> +}
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm_internal.h
> b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm_internal.h
> index 5c2a89f0d5d5..8be11510cd92 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm_internal.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm_internal.h
> @@ -29,4 +29,6 @@ u32 amdgpu_dpm_get_vblank_time(struct amdgpu_device *adev);
>
> u32 amdgpu_dpm_get_vrefresh(struct amdgpu_device *adev);
>
> +void amdgpu_dpm_get_display_cfg(struct amdgpu_device *adev);
> +
> #endif
> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/kv_dpm.c
> b/drivers/gpu/drm/amd/pm/legacy-dpm/kv_dpm.c
> index 307ebf7e3226..33eb85dd68e9 100644
> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/kv_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/kv_dpm.c
> @@ -2299,7 +2299,7 @@ static void kv_apply_state_adjust_rules(struct
> amdgpu_device *adev,
>
> if (pi->sys_info.nb_dpm_enable) {
> force_high = (mclk >=
> pi->sys_info.nbp_memory_clock[3]) ||
> - pi->video_start ||
> (adev->pm.dpm.new_active_crtc_count >= 3) ||
> + pi->video_start ||
> (adev->pm.pm_display_cfg.num_display >= 3) ||
> pi->disable_nb_ps3_in_battery;
> ps->dpm0_pg_nb_ps_lo = force_high ? 0x2 : 0x3;
> ps->dpm0_pg_nb_ps_hi = 0x2;
> @@ -2358,7 +2358,7 @@ static int kv_calculate_nbps_level_settings(struct
> amdgpu_device *adev)
> return 0;
>
> force_high = ((mclk >= pi->sys_info.nbp_memory_clock[3]) ||
> - (adev->pm.dpm.new_active_crtc_count >= 3) ||
> pi->video_start);
> + (adev->pm.pm_display_cfg.num_display >= 3) ||
> pi->video_start);
>
> if (force_high) {
> for (i = pi->lowest_valid; i <= pi->highest_valid;
> i++)
> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/legacy_dpm.c
> b/drivers/gpu/drm/amd/pm/legacy-dpm/legacy_dpm.c
> index 52dbf6d0469d..6ebe3d0f5b87 100644
> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/legacy_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/legacy_dpm.c
> @@ -771,7 +771,7 @@ static struct amdgpu_ps
> *amdgpu_dpm_pick_power_state(struct amdgpu_device *adev,
> int i;
> struct amdgpu_ps *ps;
> u32 ui_class;
> - bool single_display = adev->pm.dpm.new_active_crtc_count < 2;
> + bool single_display = adev->pm.pm_display_cfg.num_display < 2;
>
> /* check if the vblank period is too short to adjust the mclk */
> if (single_display && adev->powerplay.pp_funcs->vblank_too_short) {
> @@ -967,7 +967,8 @@ void amdgpu_legacy_dpm_compute_clocks(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> - amdgpu_dpm_get_active_displays(adev);
> + if (!adev->dc_enabled)
> + amdgpu_dpm_get_display_cfg(adev);
>
> amdgpu_dpm_change_power_state_locked(adev);
> }
> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> index 6595a611ce6e..cf9932e68055 100644
> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> @@ -3081,7 +3081,7 @@ static int si_get_vce_clock_voltage(struct
> amdgpu_device *adev,
> static bool si_dpm_vblank_too_short(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> - u32 vblank_time = amdgpu_dpm_get_vblank_time(adev);
> + u32 vblank_time = adev->pm.pm_display_cfg.min_vblank_time;
> /* we never hit the non-gddr5 limit so disable it */
> u32 switch_limit = adev->gmc.vram_type == AMDGPU_VRAM_TYPE_GDDR5 ?
> 450 : 0;
>
> @@ -3447,9 +3447,10 @@ static void rv770_get_engine_memory_ss(struct
> amdgpu_device *adev)
> static void si_apply_state_adjust_rules(struct amdgpu_device *adev,
> struct amdgpu_ps *rps)
> {
> + const struct amd_pp_display_configuration *display_cfg =
> + &adev->pm.pm_display_cfg;
> struct si_ps *ps = si_get_ps(rps);
> struct amdgpu_clock_and_voltage_limits *max_limits;
> - struct amdgpu_connector *conn;
> bool disable_mclk_switching = false;
> bool disable_sclk_switching = false;
> u32 mclk, sclk;
> @@ -3488,14 +3489,9 @@ static void si_apply_state_adjust_rules(struct
> amdgpu_device *adev,
> * For example, 4K 60Hz and 1080p 144Hz fall into this category.
> * Find number of such displays connected.
> */
> - for (i = 0; i < adev->mode_info.num_crtc; i++) {
> - if (!(adev->pm.dpm.new_active_crtcs & (1 << i)) ||
> - !adev->mode_info.crtcs[i]->enabled)
> - continue;
> -
> - conn =
> to_amdgpu_connector(adev->mode_info.crtcs[i]->connector);
> -
> - if (conn->pixelclock_for_modeset > 297000)
> + for (i = 0; i < display_cfg->num_display; i++) {
> + /* The array only contains active displays. */
> + if (display_cfg->displays[i].pixel_clock > 297000)
> high_pixelclock_count++;
> }
>
> @@ -3523,7 +3519,7 @@ static void si_apply_state_adjust_rules(struct
> amdgpu_device *adev,
> rps->ecclk = 0;
> }
>
> - if ((adev->pm.dpm.new_active_crtc_count > 1) ||
> + if ((adev->pm.pm_display_cfg.num_display > 1) ||
> si_dpm_vblank_too_short(adev))
> disable_mclk_switching = true;
>
> @@ -3671,7 +3667,7 @@ static void si_apply_state_adjust_rules(struct
> amdgpu_device *adev,
>
> ps->performance_levels[i].mclk,
> max_limits->vddc,
> &ps->performance_levels[i].vddc);
>
> btc_apply_voltage_dependency_rules(&adev->pm.dpm.dyn_state.vddc_dependency_on_dispclk,
> -
> adev->clock.current_dispclk,
> + display_cfg->display_clk,
> max_limits->vddc,
> &ps->performance_levels[i].vddc);
> }
>
> @@ -4196,16 +4192,16 @@ static void si_program_ds_registers(struct
> amdgpu_device *adev)
>
> static void si_program_display_gap(struct amdgpu_device *adev)
> {
> + const struct amd_pp_display_configuration *cfg =
> &adev->pm.pm_display_cfg;
> u32 tmp, pipe;
> - int i;
>
> tmp = RREG32(mmCG_DISPLAY_GAP_CNTL) &
> ~(CG_DISPLAY_GAP_CNTL__DISP1_GAP_MASK | CG_DISPLAY_GAP_CNTL__DISP2_GAP_MASK);
> - if (adev->pm.dpm.new_active_crtc_count > 0)
> + if (cfg->num_display > 0)
> tmp |= R600_PM_DISPLAY_GAP_VBLANK_OR_WM <<
> CG_DISPLAY_GAP_CNTL__DISP1_GAP__SHIFT;
> else
> tmp |= R600_PM_DISPLAY_GAP_IGNORE <<
> CG_DISPLAY_GAP_CNTL__DISP1_GAP__SHIFT;
>
> - if (adev->pm.dpm.new_active_crtc_count > 1)
> + if (cfg->num_display > 1)
> tmp |= R600_PM_DISPLAY_GAP_VBLANK_OR_WM <<
> CG_DISPLAY_GAP_CNTL__DISP2_GAP__SHIFT;
> else
> tmp |= R600_PM_DISPLAY_GAP_IGNORE <<
> CG_DISPLAY_GAP_CNTL__DISP2_GAP__SHIFT;
> @@ -4215,17 +4211,8 @@ static void si_program_display_gap(struct
> amdgpu_device *adev)
> tmp = RREG32(DCCG_DISP_SLOW_SELECT_REG);
> pipe = (tmp & DCCG_DISP1_SLOW_SELECT_MASK) >>
> DCCG_DISP1_SLOW_SELECT_SHIFT;
>
> - if ((adev->pm.dpm.new_active_crtc_count > 0) &&
> - (!(adev->pm.dpm.new_active_crtcs & (1 << pipe)))) {
> - /* find the first active crtc */
> - for (i = 0; i < adev->mode_info.num_crtc; i++) {
> - if (adev->pm.dpm.new_active_crtcs & (1 << i))
> - break;
> - }
> - if (i == adev->mode_info.num_crtc)
> - pipe = 0;
> - else
> - pipe = i;
> + if (cfg->num_display > 0 && pipe != cfg->crtc_index) {
> + pipe = cfg->crtc_index;
>
> tmp &= ~DCCG_DISP1_SLOW_SELECT_MASK;
> tmp |= DCCG_DISP1_SLOW_SELECT(pipe);
> @@ -4236,7 +4223,7 @@ static void si_program_display_gap(struct amdgpu_device
> *adev)
> * This can be a problem on PowerXpress systems or if you want to use
> the card
> * for offscreen rendering or compute if there are no crtcs enabled.
> */
> - si_notify_smc_display_change(adev, adev->pm.dpm.new_active_crtc_count
> > 0);
> + si_notify_smc_display_change(adev, cfg->num_display > 0);
> }
>
> static void si_enable_spread_spectrum(struct amdgpu_device *adev, bool
> enable)
> @@ -5545,7 +5532,7 @@ static int si_convert_power_level_to_smc(struct
> amdgpu_device *adev,
> (pl->mclk <= pi->mclk_stutter_mode_threshold) &&
> !eg_pi->uvd_enabled &&
> (RREG32(mmDPG_PIPE_STUTTER_CONTROL) &
> DPG_PIPE_STUTTER_CONTROL__STUTTER_ENABLE_MASK) &&
> - (adev->pm.dpm.new_active_crtc_count <= 2)) {
> + (adev->pm.pm_display_cfg.num_display <= 2)) {
> level->mcFlags |= SISLANDS_SMC_MC_STUTTER_EN;
> }
>
> @@ -5694,7 +5681,7 @@ static bool si_is_state_ulv_compatible(struct
> amdgpu_device *adev,
> /* XXX validate against display requirements! */
>
> for (i = 0; i <
> adev->pm.dpm.dyn_state.vddc_dependency_on_dispclk.count; i++) {
> - if (adev->clock.current_dispclk <=
> + if (adev->pm.pm_display_cfg.display_clk <=
>
> adev->pm.dpm.dyn_state.vddc_dependency_on_dispclk.entries[i].clk) {
> if (ulv->pl.vddc <
>
> adev->pm.dpm.dyn_state.vddc_dependency_on_dispclk.entries[i].v)
> @@ -5848,30 +5835,22 @@ static int si_upload_ulv_state(struct amdgpu_device
> *adev)
>
> static int si_upload_smc_data(struct amdgpu_device *adev)
> {
> - struct amdgpu_crtc *amdgpu_crtc = NULL;
> - int i;
> + const struct amd_pp_display_configuration *cfg =
> &adev->pm.pm_display_cfg;
> u32 crtc_index = 0;
> u32 mclk_change_block_cp_min = 0;
> u32 mclk_change_block_cp_max = 0;
>
> - for (i = 0; i < adev->mode_info.num_crtc; i++) {
> - if (adev->pm.dpm.new_active_crtcs & (1 << i)) {
> - amdgpu_crtc = adev->mode_info.crtcs[i];
> - break;
> - }
> - }
> -
> /* When a display is plugged in, program these so that the SMC
> * performs MCLK switching when it doesn't cause flickering.
> * When no display is plugged in, there is no need to restrict
> * MCLK switching, so program them to zero.
> */
> - if (adev->pm.dpm.new_active_crtc_count && amdgpu_crtc) {
> - crtc_index = amdgpu_crtc->crtc_id;
> + if (cfg->num_display) {
> + crtc_index = cfg->crtc_index;
>
> - if (amdgpu_crtc->line_time) {
> - mclk_change_block_cp_min = 200 /
> amdgpu_crtc->line_time;
> - mclk_change_block_cp_max = 100 /
> amdgpu_crtc->line_time;
> + if (cfg->line_time_in_us) {
> + mclk_change_block_cp_min = 200 / cfg->line_time_in_us;
> + mclk_change_block_cp_max = 100 / cfg->line_time_in_us;
> }
> }
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> index b48a031cbba0..554492dfa3c0 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> @@ -1554,16 +1554,7 @@ static void pp_pm_compute_clocks(void *handle)
> struct amdgpu_device *adev = hwmgr->adev;
>
> if (!adev->dc_enabled) {
> - amdgpu_dpm_get_active_displays(adev);
> - adev->pm.pm_display_cfg.num_display =
> adev->pm.dpm.new_active_crtc_count;
> - adev->pm.pm_display_cfg.vrefresh =
> amdgpu_dpm_get_vrefresh(adev);
> - adev->pm.pm_display_cfg.min_vblank_time =
> amdgpu_dpm_get_vblank_time(adev);
> - /* we have issues with mclk switching with
> - * refresh rates over 120 hz on the non-DC code.
> - */
> - if (adev->pm.pm_display_cfg.vrefresh > 120)
> - adev->pm.pm_display_cfg.min_vblank_time = 0;
> -
> + amdgpu_dpm_get_display_cfg(adev);
> pp_display_configuration_change(handle,
> &adev->pm.pm_display_cfg);
> }
> --
> 2.51.0
>
