Re: [PATCH v3 5/5] drm/i915/dp: Disable the AUX DPCD probe quirk if it's not required

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 05 Jun 2025, Imre Deak <imre.deak@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.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

I'll get back to this tomorrow, don't have time now to check all the
paths now. At a glance looks okay.

BR,
Jani.

> ---
>  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