On Tue, 03 Jun 2025, Imre Deak <imre.d...@intel.com> wrote: > Reading DPCD registers has side-effects and some of these can cause a > problem for instance during link training. Based on this it's better to > avoid the probing quirk done before each DPCD register read, limiting > this to the monitor which requires it. The only known problematic > monitor is an external SST sink, so keep the quirk disabled always for > eDP and MST sinks. Reenable the quirk after a hotplug event and after > resuming from a power state without hotplug support, until the > subsequent EDID based detection.
It's a bummer we have to do this in so many places. :( > > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com> > Cc: Jani Nikula <jani.nik...@linux.intel.com> > Signed-off-by: Imre Deak <imre.d...@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 11 +++++++++-- > drivers/gpu/drm/i915/display/intel_dp_aux.c | 2 ++ > drivers/gpu/drm/i915/display/intel_hotplug.c | 10 ++++++++++ > 3 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 208a953b04a2f..d65a18fc1aeb9 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -5747,6 +5747,11 @@ intel_dp_set_edid(struct intel_dp *intel_dp) > /* Below we depend on display info having been updated */ > drm_edid_connector_update(&connector->base, drm_edid); > > + if (!intel_dp_is_edp(intel_dp)) > + drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, > + drm_edid_has_quirk(&connector->base, > + > DRM_EDID_QUIRK_DP_DPCD_PROBE)); > + > vrr_capable = intel_vrr_is_capable(connector); > drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n", > connector->base.base.id, connector->base.name, > str_yes_no(vrr_capable)); > @@ -5881,6 +5886,7 @@ intel_dp_detect(struct drm_connector *_connector, > intel_dp_print_rates(intel_dp); > > if (intel_dp->is_mst) { > + drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, false); > /* > * If we are in MST mode then this connector > * won't appear connected or have anything > @@ -6321,10 +6327,11 @@ intel_dp_hpd_pulse(struct intel_digital_port > *dig_port, bool long_hpd) > * complete the DP tunnel BW request for the latter connector/encoder > * waiting for this encoder's DPRX read, perform a dummy read here. > */ > - if (long_hpd) > + if (long_hpd) { > + drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true); > + > intel_dp_read_dprx_caps(intel_dp, dpcd); > > - if (long_hpd) { > intel_dp->reset_link_params = true; > intel_dp_invalidate_source_oui(intel_dp); > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c > b/drivers/gpu/drm/i915/display/intel_dp_aux.c > index bf8e8e0cc19c9..410252ba6fd16 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c > @@ -835,6 +835,8 @@ void intel_dp_aux_init(struct intel_dp *intel_dp) > > intel_dp->aux.transfer = intel_dp_aux_transfer; > cpu_latency_qos_add_request(&intel_dp->pm_qos, PM_QOS_DEFAULT_VALUE); > + > + drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, !intel_dp_is_edp(intel_dp)); > } > > static enum aux_ch default_aux_ch(struct intel_encoder *encoder) > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c > b/drivers/gpu/drm/i915/display/intel_hotplug.c > index 74fe398663d63..1093c6c3714c0 100644 > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c > @@ -33,6 +33,7 @@ > #include "intel_display_core.h" > #include "intel_display_rpm.h" > #include "intel_display_types.h" > +#include "intel_dp.h" > #include "intel_hdcp.h" > #include "intel_hotplug.h" > #include "intel_hotplug_irq.h" > @@ -906,9 +907,18 @@ void intel_hpd_poll_enable(struct intel_display *display) > */ > void intel_hpd_poll_disable(struct intel_display *display) > { > + struct intel_encoder *encoder; > + > if (!HAS_DISPLAY(display)) > return; > > + for_each_intel_dp(display->drm, encoder) { > + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > + > + if (!intel_dp_is_edp(intel_dp)) > + drm_dp_dpcd_set_probe_quirk(&intel_dp->aux, true); > + } > + > WRITE_ONCE(display->hotplug.poll_enabled, false); > > spin_lock_irq(&display->irq.lock); -- Jani Nikula, Intel