[PATCH 1/2] drm/i915: Implement WaEdpLinkRateDataReload

[PATCH 1/2] drm/i915: Implement WaEdpLinkRateDataReload

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

A lot of modern laptops use the Parade PS8461E MUX for eDP
switching. The MUX can operate in jitter cleaning mode or
redriver mode, the first one resulting in higher link
quality. The jitter cleaning mode needs to know the link
rate used and the MUX achieves this by snooping the
LINK_BW_SET, LINK_RATE_SELECT and SUPPORTED_LINK_RATES
DPCD accesses.

When the MUX is powered down (seems this can happen whenever
the display is turned off) it loses track of the snooped
link rates so when we do the LINK_RATE_SELECT write it no
longer knowns which link rate we're selecting, and thus it
falls back to the lower quality redriver mode. This results
in unstable high link rates (eg. usually 8.1Gbps link rate
no longer works correctly).

In order to avoid all that let's re-snoop SUPPORTED_LINK_RATES
from the sink at the start of every link training.

Unfortunately we don't have a way to detect the presence of
the MUX. It looks like the set of laptops equipped with this
MUX is fairly large and contains devices from multiple
manufacturers. It may also still be growing with new models.
So a quirk doesn't seem like a very easily maintainable
option, thus we shall attempt to do this unconditionally on
all machines that use LINK_RATE_SELECT. Hopefully this extra
DPCD read doesn't cause issues for any unaffected machine.
If that turns out to be the case we'll need to convert this
into a quirk in the future.

Cc: stable@vger.kernel.org
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6205
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/display/intel_dp_link_training.c | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

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 9feaf1a589f3..d213d8ad1ea5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -671,6 +671,28 @@ intel_dp_prepare_link_train(struct intel_dp *intel_dp,
 	intel_dp_compute_rate(intel_dp, crtc_state->port_clock,
 			      &link_bw, &rate_select);
 
+	/*
+	 * WaEdpLinkRateDataReload
+	 *
+	 * Parade PS8461E MUX (used on varius TGL+ laptops) needs
+	 * to snoop the link rates reported by the sink when we
+	 * use LINK_RATE_SET in order to operate in jitter cleaning
+	 * mode (as opposed to redriver mode). Unfortunately it
+	 * loses track of the snooped link rates when powered down,
+	 * so we need to make it re-snoop often. Without this high
+	 * link rates are not stable.
+	 */
+	if (!link_bw) {
+		struct intel_connector *connector = intel_dp->attached_connector;
+		__le16 sink_rates[DP_MAX_SUPPORTED_RATES];
+
+		drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Reloading eDP link rates\n",
+			    connector->base.base.id, connector->base.name);
+
+		drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES,
+				 sink_rates, sizeof(sink_rates));
+	}
+
 	if (link_bw)
 		drm_dbg_kms(&i915->drm,
 			    "[ENCODER:%d:%s] Using LINK_BW_SET value %02x\n",
-- 
2.35.1

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Implement WaEdpLinkRateDataReload

[Aaron Ma]

Tested-by: Aaron Ma <aaron.ma@canonical.com>

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Implement WaEdpLinkRateDataReload

[Jani Nikula]

On Fri, 02 Sep 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> A lot of modern laptops use the Parade PS8461E MUX for eDP
> switching. The MUX can operate in jitter cleaning mode or
> redriver mode, the first one resulting in higher link
> quality. The jitter cleaning mode needs to know the link
> rate used and the MUX achieves this by snooping the
> LINK_BW_SET, LINK_RATE_SELECT and SUPPORTED_LINK_RATES
> DPCD accesses.
>
> When the MUX is powered down (seems this can happen whenever
> the display is turned off) it loses track of the snooped
> link rates so when we do the LINK_RATE_SELECT write it no
> longer knowns which link rate we're selecting, and thus it
> falls back to the lower quality redriver mode. This results
> in unstable high link rates (eg. usually 8.1Gbps link rate
> no longer works correctly).
>
> In order to avoid all that let's re-snoop SUPPORTED_LINK_RATES
> from the sink at the start of every link training.
>
> Unfortunately we don't have a way to detect the presence of
> the MUX. It looks like the set of laptops equipped with this
> MUX is fairly large and contains devices from multiple
> manufacturers. It may also still be growing with new models.
> So a quirk doesn't seem like a very easily maintainable
> option, thus we shall attempt to do this unconditionally on
> all machines that use LINK_RATE_SELECT. Hopefully this extra
> DPCD read doesn't cause issues for any unaffected machine.
> If that turns out to be the case we'll need to convert this
> into a quirk in the future.
>
> Cc: stable@vger.kernel.org
> Cc: Jason A. Donenfeld <Jason@zx2c4.com>
> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6205
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  .../drm/i915/display/intel_dp_link_training.c | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> 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 9feaf1a589f3..d213d8ad1ea5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -671,6 +671,28 @@ intel_dp_prepare_link_train(struct intel_dp *intel_dp,
>  	intel_dp_compute_rate(intel_dp, crtc_state->port_clock,
>  			      &link_bw, &rate_select);
>  
> +	/*
> +	 * WaEdpLinkRateDataReload
> +	 *
> +	 * Parade PS8461E MUX (used on varius TGL+ laptops) needs
> +	 * to snoop the link rates reported by the sink when we
> +	 * use LINK_RATE_SET in order to operate in jitter cleaning
> +	 * mode (as opposed to redriver mode). Unfortunately it
> +	 * loses track of the snooped link rates when powered down,
> +	 * so we need to make it re-snoop often. Without this high
> +	 * link rates are not stable.
> +	 */
> +	if (!link_bw) {
> +		struct intel_connector *connector = intel_dp->attached_connector;
> +		__le16 sink_rates[DP_MAX_SUPPORTED_RATES];
> +
> +		drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Reloading eDP link rates\n",
> +			    connector->base.base.id, connector->base.name);
> +
> +		drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES,
> +				 sink_rates, sizeof(sink_rates));
> +	}
> +
>  	if (link_bw)
>  		drm_dbg_kms(&i915->drm,
>  			    "[ENCODER:%d:%s] Using LINK_BW_SET value %02x\n",

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Implement WaEdpLinkRateDataReload

[Jason A. Donenfeld]

Tested-by: Jason A. Donenfeld <Jason@zx2c4.com>