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

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

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