Reviewed-by: Alex Hung <[email protected]>
On 3/16/26 21:08, Srinivasan Shanmugam wrote:
dcn42_init_hw() calls update_bw_bounding_box() when FAMS2 is disabled or
when the dchub reference clock changes. However the existing condition
mixes the callback pointer check with only one side of the || expression:
((!fams2_enable && update_bw_bounding_box) || freq_changed)
This allows the block to be entered through the freq_changed path even
when update_bw_bounding_box() is NULL. The function is then called
unconditionally inside the block, which can lead to a NULL pointer
dereference.
Additionally, the code dereferences dc->clk_mgr->bw_params without
verifying that dc->clk_mgr and bw_params are valid.
Restructure the condition so that the update trigger remains the same
(FAMS2 disabled or dchub ref clock changed), but guard the call with
explicit checks for:
- update_bw_bounding_box callback
- dc->clk_mgr
- dc->clk_mgr->bw_params
Also introduce a helper boolean (dchub_ref_freq_changed) to improve
readability of the clock-change condition.
This fixes Smatch warnings about inconsistent NULL assumptions in
dcn42_init_hw().
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn42/dcn42_hwseq.c:264
dcn42_init_hw() error: we previously assumed 'dc->clk_mgr' could be null (see
line 253)
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn42/dcn42_hwseq.c:278 dcn42_init_hw()
error: we previously assumed 'dc->res_pool->funcs->update_bw_bounding_box'
could be null (see line 274)
Cc: Roman Li <[email protected]>
Cc: Alex Hung <[email protected]>
Cc: Jerry Zuo <[email protected]>
Cc: Sun peng Li <[email protected]>
Cc: Tom Chung <[email protected]>
Cc: Dan Carpenter <[email protected]>
Cc: Aurabindo Pillai <[email protected]>
Signed-off-by: Srinivasan Shanmugam <[email protected]>
---
.../amd/display/dc/hwss/dcn42/dcn42_hwseq.c | 24 +++++++++++++------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn42/dcn42_hwseq.c
b/drivers/gpu/drm/amd/display/dc/hwss/dcn42/dcn42_hwseq.c
index 8e12dc1297c4..e307cc6363dd 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn42/dcn42_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn42/dcn42_hwseq.c
@@ -69,6 +69,7 @@ void dcn42_init_hw(struct dc *dc)
int edp_num;
uint32_t backlight = MAX_BACKLIGHT_LEVEL;
uint32_t user_level = MAX_BACKLIGHT_LEVEL;
+ bool dchub_ref_freq_changed;
int current_dchub_ref_freq = 0;
if (dc->clk_mgr && dc->clk_mgr->funcs && dc->clk_mgr->funcs->init_clocks) {
@@ -260,8 +261,12 @@ void dcn42_init_hw(struct dc *dc)
if (dc->res_pool->hubbub->funcs->init_crb)
dc->res_pool->hubbub->funcs->init_crb(dc->res_pool->hubbub);
- if (dc->res_pool->hubbub->funcs->set_request_limit && dc->config.sdpif_request_limit_words_per_umc > 0)
- dc->res_pool->hubbub->funcs->set_request_limit(dc->res_pool->hubbub,
dc->clk_mgr->bw_params->num_channels, dc->config.sdpif_request_limit_words_per_umc);
+ if (dc->res_pool->hubbub->funcs->set_request_limit &&
+ dc->clk_mgr && dc->clk_mgr->bw_params &&
+ dc->config.sdpif_request_limit_words_per_umc > 0)
+
dc->res_pool->hubbub->funcs->set_request_limit(dc->res_pool->hubbub,
+
dc->clk_mgr->bw_params->num_channels,
+
dc->config.sdpif_request_limit_words_per_umc);
// Get DMCUB capabilities
if (dc->ctx->dmub_srv) {
@@ -269,13 +274,18 @@ void dcn42_init_hw(struct dc *dc)
dc->caps.dmub_caps.psr =
dc->ctx->dmub_srv->dmub->feature_caps.psr;
dc->caps.dmub_caps.mclk_sw =
dc->ctx->dmub_srv->dmub->feature_caps.fw_assisted_mclk_switch_ver > 0;
dc->caps.dmub_caps.fams_ver =
dc->ctx->dmub_srv->dmub->feature_caps.fw_assisted_mclk_switch_ver;
+
+ /* sw and fw FAMS versions must match for support */
dc->debug.fams2_config.bits.enable &=
- dc->caps.dmub_caps.fams_ver ==
dc->debug.fams_version.ver; // sw & fw fams versions must match for support
- if ((!dc->debug.fams2_config.bits.enable &&
dc->res_pool->funcs->update_bw_bounding_box)
- || res_pool->ref_clocks.dchub_ref_clock_inKhz / 1000 !=
current_dchub_ref_freq) {
+ dc->caps.dmub_caps.fams_ver ==
dc->debug.fams_version.ver;
+ dchub_ref_freq_changed =
+ res_pool->ref_clocks.dchub_ref_clock_inKhz / 1000 !=
current_dchub_ref_freq;
+
+ if ((!dc->debug.fams2_config.bits.enable || dchub_ref_freq_changed)
&&
+ dc->res_pool->funcs->update_bw_bounding_box &&
+ dc->clk_mgr && dc->clk_mgr->bw_params) {
/* update bounding box if FAMS2 disabled, or if dchub
clk has changed */
- if (dc->clk_mgr)
- dc->res_pool->funcs->update_bw_bounding_box(dc,
dc->clk_mgr->bw_params);
+ dc->res_pool->funcs->update_bw_bounding_box(dc,
dc->clk_mgr->bw_params);
}
}
if (dc->res_pool->pg_cntl) {
