[PATCH] drm/amd/display: Add clk_mgr NULL checks in dcn32_initialize_min_clocks()

[PATCH] drm/amd/display: Add clk_mgr NULL checks in dcn32_initialize_min_clocks()

[Srinivasan Shanmugam]

dcn32_init_hw() checks dc->clk_mgr before calling init_clocks(), so the
clock manager is not treated as unconditionally present on this path.
However, dcn32_initialize_min_clocks() later dereferences dc->clk_mgr,
bw_params, and clk_mgr callbacks without validating them.

Add the required guards in dcn32_initialize_min_clocks() before
accessing clk_mgr-dependent state, and check callback presence before
calling get_dispclk_from_dentist() and update_clocks().

Also guard the later update_bw_bounding_box() call in the FAMS2-disabled
path since it also dereferences dc->clk_mgr->bw_params.

This keeps clk_mgr handling consistent in the DCN32 HW init flow and
avoids possible NULL pointer dereferences reported by Smatch.

Fixes the below:
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn32/dcn32_hwseq.c:1012 dcn32_init_hw() error: we previously assumed 'dc->clk_mgr' could be null (see line 978)

Cc: Roman Li <roman.li@amd.com>
Cc: Alex Hung <alex.hung@amd.com>
Cc: Jerry Zuo <jerry.zuo@amd.com>
Cc: Sun peng Li <sunpeng.li@amd.com>
Cc: Tom Chung <chiahsuan.chung@amd.com>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 .../drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
index a0aaa727e9fa..e5d93dd348dd 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
@@ -757,6 +757,9 @@ static void dcn32_initialize_min_clocks(struct dc *dc)
 {
 	struct dc_clocks *clocks = &dc->current_state->bw_ctx.bw.dcn.clk;
 
+	if (!dc->clk_mgr || !dc->clk_mgr->bw_params || !dc->clk_mgr->funcs)
+		return;
+
 	clocks->dcfclk_deep_sleep_khz = DCN3_2_DCFCLK_DS_INIT_KHZ;
 	clocks->dcfclk_khz = dc->clk_mgr->bw_params->clk_table.entries[0].dcfclk_mhz * 1000;
 	clocks->socclk_khz = dc->clk_mgr->bw_params->clk_table.entries[0].socclk_mhz * 1000;
@@ -765,9 +768,10 @@ static void dcn32_initialize_min_clocks(struct dc *dc)
 	clocks->ref_dtbclk_khz = dc->clk_mgr->bw_params->clk_table.entries[0].dtbclk_mhz * 1000;
 	clocks->fclk_p_state_change_support = true;
 	clocks->p_state_change_support = true;
+
 	if (dc->debug.disable_boot_optimizations) {
 		clocks->dispclk_khz = dc->clk_mgr->bw_params->clk_table.entries[0].dispclk_mhz * 1000;
-	} else {
+	} else if (dc->clk_mgr->funcs->get_dispclk_from_dentist) {
 		/* Even though DPG_EN = 1 for the connected display, it still requires the
 		 * correct timing so we cannot set DISPCLK to min freq or it could cause
 		 * audio corruption. Read current DISPCLK from DENTIST and request the same
@@ -776,10 +780,10 @@ static void dcn32_initialize_min_clocks(struct dc *dc)
 		clocks->dispclk_khz = dc->clk_mgr->funcs->get_dispclk_from_dentist(dc->clk_mgr);
 	}
 
-	dc->clk_mgr->funcs->update_clocks(
-			dc->clk_mgr,
-			dc->current_state,
-			true);
+	if (dc->clk_mgr->funcs->update_clocks)
+		dc->clk_mgr->funcs->update_clocks(dc->clk_mgr,
+						  dc->current_state,
+						  true);
 }
 
 void dcn32_init_hw(struct dc *dc)
@@ -1007,7 +1011,8 @@ void dcn32_init_hw(struct dc *dc)
 				DMUB_FW_VERSION(7, 0, 35)) {
 			/* FAMS2 is disabled */
 			dc->debug.fams2_config.bits.enable = false;
-			if (dc->debug.using_dml2 && dc->res_pool->funcs->update_bw_bounding_box) {
+			if (dc->debug.using_dml2 && dc->res_pool->funcs->update_bw_bounding_box &&
+			    dc->clk_mgr && dc->clk_mgr->bw_params) {
 				/* update bounding box if FAMS2 disabled */
 				dc->res_pool->funcs->update_bw_bounding_box(dc, dc->clk_mgr->bw_params);
 			}
-- 
2.34.1

Re: [PATCH] drm/amd/display: Add clk_mgr NULL checks in dcn32_initialize_min_clocks()

[Alex Hung]

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 3/15/26 08:41, Srinivasan Shanmugam wrote:
> dcn32_init_hw() checks dc->clk_mgr before calling init_clocks(), so the
> clock manager is not treated as unconditionally present on this path.
> However, dcn32_initialize_min_clocks() later dereferences dc->clk_mgr,
> bw_params, and clk_mgr callbacks without validating them.
> 
> Add the required guards in dcn32_initialize_min_clocks() before
> accessing clk_mgr-dependent state, and check callback presence before
> calling get_dispclk_from_dentist() and update_clocks().
> 
> Also guard the later update_bw_bounding_box() call in the FAMS2-disabled
> path since it also dereferences dc->clk_mgr->bw_params.
> 
> This keeps clk_mgr handling consistent in the DCN32 HW init flow and
> avoids possible NULL pointer dereferences reported by Smatch.
> 
> Fixes the below:
> drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn32/dcn32_hwseq.c:1012 dcn32_init_hw() error: we previously assumed 'dc->clk_mgr' could be null (see line 978)
> 
> Cc: Roman Li <roman.li@amd.com>
> Cc: Alex Hung <alex.hung@amd.com>
> Cc: Jerry Zuo <jerry.zuo@amd.com>
> Cc: Sun peng Li <sunpeng.li@amd.com>
> Cc: Tom Chung <chiahsuan.chung@amd.com>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
>   .../drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
> index a0aaa727e9fa..e5d93dd348dd 100644
> --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
> +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
> @@ -757,6 +757,9 @@ static void dcn32_initialize_min_clocks(struct dc *dc)
>   {
>   	struct dc_clocks *clocks = &dc->current_state->bw_ctx.bw.dcn.clk;
>   
> +	if (!dc->clk_mgr || !dc->clk_mgr->bw_params || !dc->clk_mgr->funcs)
> +		return;
> +
>   	clocks->dcfclk_deep_sleep_khz = DCN3_2_DCFCLK_DS_INIT_KHZ;
>   	clocks->dcfclk_khz = dc->clk_mgr->bw_params->clk_table.entries[0].dcfclk_mhz * 1000;
>   	clocks->socclk_khz = dc->clk_mgr->bw_params->clk_table.entries[0].socclk_mhz * 1000;
> @@ -765,9 +768,10 @@ static void dcn32_initialize_min_clocks(struct dc *dc)
>   	clocks->ref_dtbclk_khz = dc->clk_mgr->bw_params->clk_table.entries[0].dtbclk_mhz * 1000;
>   	clocks->fclk_p_state_change_support = true;
>   	clocks->p_state_change_support = true;
> +
>   	if (dc->debug.disable_boot_optimizations) {
>   		clocks->dispclk_khz = dc->clk_mgr->bw_params->clk_table.entries[0].dispclk_mhz * 1000;
> -	} else {
> +	} else if (dc->clk_mgr->funcs->get_dispclk_from_dentist) {
>   		/* Even though DPG_EN = 1 for the connected display, it still requires the
>   		 * correct timing so we cannot set DISPCLK to min freq or it could cause
>   		 * audio corruption. Read current DISPCLK from DENTIST and request the same
> @@ -776,10 +780,10 @@ static void dcn32_initialize_min_clocks(struct dc *dc)
>   		clocks->dispclk_khz = dc->clk_mgr->funcs->get_dispclk_from_dentist(dc->clk_mgr);
>   	}
>   
> -	dc->clk_mgr->funcs->update_clocks(
> -			dc->clk_mgr,
> -			dc->current_state,
> -			true);
> +	if (dc->clk_mgr->funcs->update_clocks)
> +		dc->clk_mgr->funcs->update_clocks(dc->clk_mgr,
> +						  dc->current_state,
> +						  true);
>   }
>   
>   void dcn32_init_hw(struct dc *dc)
> @@ -1007,7 +1011,8 @@ void dcn32_init_hw(struct dc *dc)
>   				DMUB_FW_VERSION(7, 0, 35)) {
>   			/* FAMS2 is disabled */
>   			dc->debug.fams2_config.bits.enable = false;
> -			if (dc->debug.using_dml2 && dc->res_pool->funcs->update_bw_bounding_box) {
> +			if (dc->debug.using_dml2 && dc->res_pool->funcs->update_bw_bounding_box &&
> +			    dc->clk_mgr && dc->clk_mgr->bw_params) {
>   				/* update bounding box if FAMS2 disabled */
>   				dc->res_pool->funcs->update_bw_bounding_box(dc, dc->clk_mgr->bw_params);
>   			}