[AMD Official Use Only - AMD Internal Distribution Only] Hi Srini,
Please remember to add Fixes tag, so this gets picked up by stable kernels if the original patch ends up in stable. -- Regards, Jay ________________________________ From: Hung, Alex <[email protected]> Sent: Friday, March 20, 2026 12:41 PM To: SHANMUGAM, SRINIVASAN <[email protected]>; Pillai, Aurabindo <[email protected]> Cc: [email protected] <[email protected]>; Li, Roman <[email protected]>; Zuo, Jerry <[email protected]>; Li, Sun peng (Leo) <[email protected]>; Chung, ChiaHsuan (Tom) <[email protected]>; Dan Carpenter <[email protected]> Subject: Re: [PATCH] drm/amd/display: Fix NULL pointer assumptions in dcn42_init_hw() 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) {
