On Fri, 2025-09-19 at 13:26 +0300, Jani Nikula wrote: > On Thu, 18 Sep 2025, Khaled Almahallawy > <[email protected]> wrote: > > According to DP Specs[1]: > > "SSC is enabled or disabled when the SPREAD_AMP bit in the > > DOWNSPREAD_CTRL register is set or cleared (DPCD 00107h[4] = 1 or > > 0, > > respectively)." > > > > Set the SPREAD_AMP bit before starting link training to ensure SSC > > is enabled as required. > > > > [1]: DisplayPort Standard v2.1 - Sec 2.2.3.1 De-spreading of the > > Regenerated Stream Clock > > > > Cc: Uma Shankar <[email protected]> > > Cc: Jani Nikula <[email protected]> > > Signed-off-by: Khaled Almahallawy <[email protected]> > > --- > > drivers/gpu/drm/i915/display/intel_dp_link_training.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > index 27f3716bdc1f..db2ea3c51a5f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > @@ -711,9 +711,13 @@ static bool > > intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp, > > > > void intel_dp_link_training_set_mode(struct intel_dp *intel_dp, > > int link_rate, bool is_vrr) > > { > > + struct intel_display *display = > > to_intel_display(intel_dp); > > u8 link_config[2]; > > + bool ssc_enabled = intel_panel_use_ssc(display) && > > + (intel_dp->dpcd[DP_MAX_DOWNSPREAD] & > > DP_MAX_DOWNSPREAD_0_5); > > This is actually pretty messy. > > intel_panel_use_ssc() looks at a module parameter, and then at an > *obsolete* field in the VBT, but may also be quirked away. > > And as the name implies, it's for panels, originally LVDS only. > > Someone needs to look at the VBT spec, there's a handful of places > mentioning SSC, and *all* usage of intel_panel_use_ssc() in the > driver, > and match those properly, using the right part from VBT depending on > BDB > version and panel/child device, etc.
Noted, I will give a try to implement this. > > (Side note, there's Cx0 PHY code looking at DPCD registers for this, > WTF.) > > This code here would probably benefit from having: > > intel_dp_source_supports_ssc() > > and > > intel_dp_sink_supports_ssc() > > to make all of this clear here. > > > - link_config[0] = is_vrr ? DP_MSA_TIMING_PAR_IGNORE_EN : 0; > > + link_config[0] = (is_vrr ? DP_MSA_TIMING_PAR_IGNORE_EN : > > 0) | > > + (ssc_enabled ? DP_SPREAD_AMP_0_5 : 0); > > Getting complex, maybe something like: > > if (is_vrr) > link_config[0] |= DP_MSA_TIMING_PAR_IGNORE_EN; > if (intel_dp_source_supports_ssc() && > intel_dp_sink_supports_ssc()) > link_config[0] |= DP_SPREAD_AMP_0_5; Sure, Will incorporate that. > > > link_config[1] = drm_dp_is_uhbr_rate(link_rate) ? > > DP_SET_ANSI_128B132B : DP_SET_ANSI_8B10B; > > drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, > > link_config, 2); > Thank You for your review Khaled
