Re: [PATCH 01/14] drm/amd/display: Delete unimplemented dm_pp_apply_power_level_change_request()

["Timur Kristóf" <timur.kristof@gmail.com>]

On Friday, April 24, 2026 4:19:34 PM Central European Summer Time Melissa Wen 
wrote:
> On 23/04/2026 16:15, Timur Kristóf wrote:
> > dm_pp_apply_power_level_change_request() was called from old
> > DCE clock manager implementations on DCE6, 8, 10, 11.2
> > but has not been implemented ever since the beginning of DC.
> > 
> > Affected GPUs have been working fine without that implementation
> > for many years. Let's delete it now.
> > 
> > Signed-off-by: Timur Kristóf<timur.kristof@gmail.com>
> > ---
> > 
> >   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 8 --------
> >   .../gpu/drm/amd/display/dc/clk_mgr/dce100/dce_clk_mgr.c  | 9 ---------
> >   .../drm/amd/display/dc/clk_mgr/dce110/dce110_clk_mgr.c   | 9 ---------
> >   .../drm/amd/display/dc/clk_mgr/dce112/dce112_clk_mgr.c   | 9 ---------
> >   drivers/gpu/drm/amd/display/dc/dm_services.h             | 4 ----
> >   5 files changed, 39 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c index
> > 11b2ea6edf953..17f42201ab862 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> > @@ -417,14 +417,6 @@ bool dm_pp_notify_wm_clock_changes(
> > 
> >   	return false;
> >   
> >   }
> > 
> > -bool dm_pp_apply_power_level_change_request(
> > -	const struct dc_context *ctx,
> > -	struct dm_pp_power_level_change_request *level_change_req)
> 
> Hi Timur,
> 
> > -{
> > -	/* TODO: to be implemented */
> 
> I feel a little uneasy about removing all this infrastructure with this
> series, as AFAIU, it could be avoided by implementing this TODO (?)
> Any idea if AMD has this code somewhere that could be upstreamed, or did
> it end up in the firmware?

The display power requirements are already implemented in a different way.
Here how it works:

1. We have dce_pplib_apply_display_requirements() to communicate the display 
power requirements to the power management code.
2. In the power management code, we have display_configuration_change() and 
pm_compute_clocks() that take care of the display power requirements.

Note that on DCE 6, 8, 10, currently DC always just sets the maximum possible 
clock, and it works fine. On DCE 11 and 11.2, we rely on dce_calcs so we don't 
need a DAL power level there either.

As far as I understand, the DAL power levels were a concept from the old 
Windows driver for these GPUs and were never really implemented in Linux.
The newest affected GPU is about 10 years old by now and it has been working 
fine on Linux all these years without dm_pp_apply_power_level_change_request().

Side note: after this cleanup series lands, I plan to improve the situation 
and implement dm_pp_apply_clock_for_voltage_request() for these GPUs to match 
what we are doing on DCE 12.

> 
> BTW, Looks like `struct dm_pp_power_level_change_request` also becomes
> unused with this change, right?

Yes, that's right. It is removed in a subsequent commit.

> 
> Melissa
> 
> > -	return false;
> > -}
> > -
> > 
> >   bool dm_pp_apply_clock_for_voltage_request(
> >   
> >   	const struct dc_context *ctx,
> >   	struct dm_pp_clock_for_voltage_req *clock_for_voltage_req)
> > 
> > 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
> > 6d41df52d7c9b..ffb70120362e7 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
> > @@ -431,19 +431,10 @@ static void dce_update_clocks(struct clk_mgr
> > *clk_mgr_base,> 
> >   			bool safe_to_lower)
> >   
> >   {
> >   
> >   	struct clk_mgr_internal *clk_mgr_dce =
> >   	TO_CLK_MGR_INTERNAL(clk_mgr_base);
> > 
> > -	struct dm_pp_power_level_change_request level_change_req;
> > 
> >   	const int max_disp_clk =
> >   	
> >   		clk_mgr_dce-
>max_clks_by_state[DM_PP_CLOCKS_STATE_PERFORMANCE].display
> >   		_clk_khz;>   	
> >   	int patched_disp_clk = MIN(max_disp_clk,
> >   	context->bw_ctx.bw.dce.dispclk_khz);> 
> > -	level_change_req.power_level =
> > dce_get_required_clocks_state(clk_mgr_base, context); -	/* get max 
clock
> > state from PPLIB */
> > -	if ((level_change_req.power_level < clk_mgr_dce-
>cur_min_clks_state &&
> > safe_to_lower) -			|| 
level_change_req.power_level >
> > clk_mgr_dce->cur_min_clks_state) { -		if
> > (dm_pp_apply_power_level_change_request(clk_mgr_base->ctx,
> > &level_change_req)) -			clk_mgr_dce-
>cur_min_clks_state =
> > level_change_req.power_level; -	}
> > -
> > 
> >   	if (should_set_clock(safe_to_lower, patched_disp_clk,
> >   	clk_mgr_base->clks.dispclk_khz)) {>   	
> >   		patched_disp_clk = dce_set_clock(clk_mgr_base, 
patched_disp_clk);
> >   		clk_mgr_base->clks.dispclk_khz = patched_disp_clk;
> > 
> > diff --git
> > a/drivers/gpu/drm/amd/display/dc/clk_mgr/dce110/dce110_clk_mgr.c
> > b/drivers/gpu/drm/amd/display/dc/clk_mgr/dce110/dce110_clk_mgr.c index
> > 13296c6ec08f4..ae922f1a31ff8 100644
> > --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dce110/dce110_clk_mgr.c
> > +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dce110/dce110_clk_mgr.c
> > @@ -257,21 +257,12 @@ static void dce11_update_clocks(struct clk_mgr
> > *clk_mgr_base,> 
> >   			bool safe_to_lower)
> >   
> >   {
> >   
> >   	struct clk_mgr_internal *clk_mgr_dce =
> >   	TO_CLK_MGR_INTERNAL(clk_mgr_base);
> > 
> > -	struct dm_pp_power_level_change_request level_change_req;
> > 
> >   	int patched_disp_clk = context->bw_ctx.bw.dce.dispclk_khz;
> >   	
> >   	/*TODO: W/A for dal3 linux, investigate why this works */
> >   	if (!clk_mgr_dce->dfs_bypass_active)
> >   	
> >   		patched_disp_clk = patched_disp_clk * 115 / 100;
> > 
> > -	level_change_req.power_level =
> > dce_get_required_clocks_state(clk_mgr_base, context); -	/* get max 
clock
> > state from PPLIB */
> > -	if ((level_change_req.power_level < clk_mgr_dce-
>cur_min_clks_state &&
> > safe_to_lower) -			|| 
level_change_req.power_level >
> > clk_mgr_dce->cur_min_clks_state) { -		if
> > (dm_pp_apply_power_level_change_request(clk_mgr_base->ctx,
> > &level_change_req)) -			clk_mgr_dce-
>cur_min_clks_state =
> > level_change_req.power_level; -	}
> > -
> > 
> >   	if (should_set_clock(safe_to_lower, patched_disp_clk,
> >   	clk_mgr_base->clks.dispclk_khz)) {>   	
> >   		context->bw_ctx.bw.dce.dispclk_khz = 
dce_set_clock(clk_mgr_base,
> >   		patched_disp_clk); clk_mgr_base->clks.dispclk_khz = 
patched_disp_clk;
> > 
> > diff --git
> > a/drivers/gpu/drm/amd/display/dc/clk_mgr/dce112/dce112_clk_mgr.c
> > b/drivers/gpu/drm/amd/display/dc/clk_mgr/dce112/dce112_clk_mgr.c index
> > 1f36ad8a7de46..48393c69735b6 100644
> > --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dce112/dce112_clk_mgr.c
> > +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dce112/dce112_clk_mgr.c
> > @@ -193,21 +193,12 @@ static void dce112_update_clocks(struct clk_mgr
> > *clk_mgr_base,> 
> >   			bool safe_to_lower)
> >   
> >   {
> >   
> >   	struct clk_mgr_internal *clk_mgr_dce =
> >   	TO_CLK_MGR_INTERNAL(clk_mgr_base);
> > 
> > -	struct dm_pp_power_level_change_request level_change_req;
> > 
> >   	int patched_disp_clk = context->bw_ctx.bw.dce.dispclk_khz;
> >   	
> >   	/*TODO: W/A for dal3 linux, investigate why this works */
> >   	if (!clk_mgr_dce->dfs_bypass_active)
> >   	
> >   		patched_disp_clk = patched_disp_clk * 115 / 100;
> > 
> > -	level_change_req.power_level =
> > dce_get_required_clocks_state(clk_mgr_base, context); -	/* get max 
clock
> > state from PPLIB */
> > -	if ((level_change_req.power_level < clk_mgr_dce-
>cur_min_clks_state &&
> > safe_to_lower) -			|| 
level_change_req.power_level >
> > clk_mgr_dce->cur_min_clks_state) { -		if
> > (dm_pp_apply_power_level_change_request(clk_mgr_base->ctx,
> > &level_change_req)) -			clk_mgr_dce-
>cur_min_clks_state =
> > level_change_req.power_level; -	}
> > -
> > 
> >   	if (should_set_clock(safe_to_lower, patched_disp_clk,
> >   	clk_mgr_base->clks.dispclk_khz)) {>   	
> >   		patched_disp_clk = dce112_set_clock(clk_mgr_base, 
patched_disp_clk);
> >   		clk_mgr_base->clks.dispclk_khz = patched_disp_clk;
> > 
> > diff --git a/drivers/gpu/drm/amd/display/dc/dm_services.h
> > b/drivers/gpu/drm/amd/display/dc/dm_services.h index
> > fbbf9c757b3c3..1395d36bfabe9 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dm_services.h
> > +++ b/drivers/gpu/drm/amd/display/dc/dm_services.h
> > @@ -224,10 +224,6 @@ bool dm_pp_apply_display_requirements(
> > 
> >   	const struct dc_context *ctx,
> >   	const struct dm_pp_display_configuration *pp_display_cfg);
> > 
> > -bool dm_pp_apply_power_level_change_request(
> > -	const struct dc_context *ctx,
> > -	struct dm_pp_power_level_change_request *level_change_req);
> > -
> > 
> >   bool dm_pp_apply_clock_for_voltage_request(
> >   
> >   	const struct dc_context *ctx,
> >   	struct dm_pp_clock_for_voltage_req *clock_for_voltage_req);