Re: [PATCH v2] drm/amd/display: use proper context for logging

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