AMD General Hey, I didn't do it yet, was having some cloning issues with the branch, I will try to push it today
Thanks, Bhawan ________________________________ From: Alex Deucher <[email protected]> Sent: July 27, 2026 12:19 PM To: Lakha, Bhawanpreet <[email protected]> Cc: Jiri Slaby (SUSE) <[email protected]>; Deucher, Alexander <[email protected]>; [email protected] <[email protected]>; Wentland, Harry <[email protected]>; Li, Sun peng (Leo) <[email protected]>; Rodrigo Siqueira <[email protected]>; Koenig, Christian <[email protected]>; David Airlie <[email protected]>; Simona Vetter <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]> Subject: Re: [PATCH v2] drm/amd/display: use proper context for logging Bhawan, I assume you already committed this? Thanks, Alex On Thu, Jul 23, 2026 at 1:10 PM Lakha, Bhawanpreet <[email protected]> wrote: > > looks good, thank you > > Reviewed-by: Bhawanpreet Lakha <[email protected]> > > > Bhawan > > ________________________________________ > From: Jiri Slaby (SUSE) <[email protected]> > Sent: July 23, 2026 12:25 AM > To: Deucher, Alexander <[email protected]> > Cc: [email protected] <[email protected]>; Jiri Slaby > (SUSE) <[email protected]>; Lakha, Bhawanpreet > <[email protected]>; Wentland, Harry <[email protected]>; Li, > Sun peng (Leo) <[email protected]>; Rodrigo Siqueira <[email protected]>; > Koenig, Christian <[email protected]>; David Airlie > <[email protected]>; Simona Vetter <[email protected]>; > [email protected] <[email protected]>; > [email protected] <[email protected]> > Subject: [PATCH v2] drm/amd/display: use proper context for logging > > The same as the rest of the code, get_ss_info_from_atombios() uses > calc_pll_cs->ctx->logger for logging. But calc_pll_cs->ctx is > initialized only later in calc_pll_max_vco_construct(). Therefore, any > output using DC_LOG_SYNC() leads to a NULL pointer deference in > get_ss_info_from_atombios(). > > According to Sashiko, the very same problem exists in > dce112_get_pix_clk_dividers() and dcn3_get_pix_clk_dividers() too. > > To avoid accessing the NULL context, use clk_src->base.ctx->logger > everywhere. That context in base is initialized earlier in > dce110_clk_src_construct() and dce112_clk_src_construct(). Before > get_ss_info_from_atombios() or Sashiko's get_pix_clk_dividers functions > above are actually called. This is done by redefining DC_LOGGER to > CTX->logger. > > Before: > dce110_clk_src_construct() did: > -> sets clk_src->base.ctx = ctx; > -> ss_info_from_atombios_create() > -> get_ss_info_from_atombios() <- uses calc_pll_cs->ctx # BOOM > -> calc_pll_max_vco_construct() <- sets calc_pll_cs->ctx > > After: > dce110_clk_src_construct() does: > -> sets clk_src->base.ctx = ctx; > -> ss_info_from_atombios_create() > -> get_ss_info_from_atombios() <- uses clk_src->base.ctx > > Closes: https://bugzilla.suse.com/show_bug.cgi?id=1271175 > Closes: > https://lore.kernel.org/all/[email protected]/ > Fixes: 1296423bf23c ("drm/amd/display: define DC_LOGGER for logger") > Signed-off-by: Jiri Slaby (SUSE) <[email protected]> > Cc: Lakha, Bhawanpreet <[email protected]> > Cc: Harry Wentland <[email protected]> > Cc: Leo Li <[email protected]> > Cc: Rodrigo Siqueira <[email protected]> > Cc: Alex Deucher <[email protected]> > Cc: "Christian König" <[email protected]> > Cc: David Airlie <[email protected]> > Cc: Simona Vetter <[email protected]> > Cc: [email protected] > --- > Cc: [email protected] > > [v2] Set DC_LOGGER to CTX->logger unconditionally (Bhawanpreet). > [v1] > https://lore.kernel.org/all/[email protected]/ > --- > .../drm/amd/display/dc/dce/dce_clock_source.c | 20 +++++++++---------- > 1 file changed, 9 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c > b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c > index ecb8493ec523..8e3862f6dac2 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c > +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c > @@ -45,9 +45,7 @@ > clk_src->base.ctx > > #define DC_LOGGER \ > - calc_pll_cs->ctx->logger > -#define DC_LOGGER_INIT() \ > - struct calc_pll_clock_source *calc_pll_cs = &clk_src->calc_pll > + CTX->logger > > #undef FN > #define FN(reg_name, field_name) \ > @@ -287,6 +285,7 @@ static bool calc_pll_dividers_in_range( > } > > static uint32_t calculate_pixel_clock_pll_dividers( > + struct dce110_clk_src *clk_src, > struct calc_pll_clock_source *calc_pll_cs, > struct pll_settings *pll_settings) > { > @@ -474,7 +473,7 @@ static uint32_t dce110_get_pix_clk_dividers_helper ( > { > uint32_t field = 0; > uint32_t pll_calc_error = MAX_PLL_CALC_ERROR; > - DC_LOGGER_INIT(); > + > /* Check if reference clock is external (not pcie/xtalin) > * HW Dce80 spec: > * 00 - PCIE_REFCLK, 01 - XTALIN, 02 - GENERICA, 03 - GENERICB > @@ -517,12 +516,14 @@ static uint32_t dce110_get_pix_clk_dividers_helper ( > /*Calculate Dividers by HDMI object, no SS case or SS case */ > pll_calc_error = > calculate_pixel_clock_pll_dividers( > + clk_src, > &clk_src->calc_pll_hdmi, > pll_settings); > else > /*Calculate Dividers by default object, no SS case or SS > case */ > pll_calc_error = > calculate_pixel_clock_pll_dividers( > + clk_src, > &clk_src->calc_pll, > pll_settings); > > @@ -568,7 +569,6 @@ static uint32_t dce110_get_pix_clk_dividers( > { > struct dce110_clk_src *clk_src = TO_DCE110_CLK_SRC(cs); > uint32_t pll_calc_error = MAX_PLL_CALC_ERROR; > - DC_LOGGER_INIT(); > > if (pix_clk_params == NULL || pll_settings == NULL > || pix_clk_params->requested_pix_clk_100hz == 0) { > @@ -600,7 +600,6 @@ static uint32_t dce112_get_pix_clk_dividers( > struct pll_settings *pll_settings) > { > struct dce110_clk_src *clk_src = TO_DCE110_CLK_SRC(cs); > - DC_LOGGER_INIT(); > > if (pix_clk_params == NULL || pll_settings == NULL > || pix_clk_params->requested_pix_clk_100hz == 0) { > @@ -1442,8 +1441,6 @@ static uint32_t dcn3_get_pix_clk_dividers( > unsigned long long actual_pix_clk_100Hz = pix_clk_params ? > pix_clk_params->requested_pix_clk_100hz : 0; > struct dce110_clk_src *clk_src = TO_DCE110_CLK_SRC(cs); > > - DC_LOGGER_INIT(); > - > if (pix_clk_params == NULL || pll_settings == NULL > || pix_clk_params->requested_pix_clk_100hz == 0) { > DC_LOG_ERROR( > @@ -1513,7 +1510,6 @@ static const struct clock_source_funcs > dce110_clk_src_funcs = { > .get_dp_dto_frequency_100hz = get_dp_dto_frequency_100hz > }; > > - > static void get_ss_info_from_atombios( > struct dce110_clk_src *clk_src, > enum as_signal_type as_signal, > @@ -1526,7 +1522,7 @@ static void get_ss_info_from_atombios( > struct spread_spectrum_info *ss_info_cur; > struct spread_spectrum_data *ss_data_cur; > uint32_t i; > - DC_LOGGER_INIT(); > + > if (ss_entries_num == NULL) { > DC_LOG_SYNC( > "Invalid entry !!!\n"); > @@ -1657,6 +1653,7 @@ static void ss_info_from_atombios_create( > } > > static bool calc_pll_max_vco_construct( > + struct dce110_clk_src *clk_src, > struct calc_pll_clock_source *calc_pll_cs, > struct calc_pll_clock_source_init_data *init_data) > { > @@ -1808,6 +1805,7 @@ bool dce110_clk_src_construct( > ss_info_from_atombios_create(clk_src); > > if (!calc_pll_max_vco_construct( > + clk_src, > &clk_src->calc_pll, > &calc_pll_cs_init_data)) { > ASSERT_CRITICAL(false); > @@ -1822,7 +1820,7 @@ bool dce110_clk_src_construct( > > > if (!calc_pll_max_vco_construct( > - &clk_src->calc_pll_hdmi, > &calc_pll_cs_init_data_hdmi)) { > + clk_src, &clk_src->calc_pll_hdmi, > &calc_pll_cs_init_data_hdmi)) { > ASSERT_CRITICAL(false); > goto unexpected_failure; > } > -- > 2.55.0
