[PATCH] drm/i915/psr: WA for panels stating bad link status after PSR is enabled

[Imre Deak]

On Mon, Oct 28, 2024 at 09:46:42AM +0200, Jouni Högander wrote:
> We are currently seeing unexpected link trainings with several different
> eDP panels. These are caused by these panels stating bad link status in
> their dpcd registers. This can be observed by doing following test:
> 
> 1. Boot up without Xe module loaded
> 
> 2. Load Xe module with PSR disabled:
>     $ modprobe xe  enable_psr=0
> 
> 3. Read panel link status register
>     $ dpcd_reg read --offset 0x200e --count=1
>     0x200e:  00
> 
> 4. Enable PSR, sleep for 2 seconds and disable PSR again:
> 
>     $ echo 0x1 > /sys/kernel/debug/dri/0/i915_edp_psr_debug
>     $ echo "-1" > /sys/kernel/debug/dri/0000:00:02.0/xe_params/enable_psr
>     $ echo 0x0 > /sys/kernel/debug/dri/0/i915_edp_psr_debug
>     $ sleep 2
>     $ cat /sys/kernel/debug/dri/0/i915_edp_psr_status | grep status
>     $ echo 0x1 > /sys/kernel/debug/dri/0/i915_edp_psr_debug
>     Source PSR/PanelReplay status: DEEP_SLEEP [0x80310030]
> 
> 5. Now read panel link status registers again:
>     $ dpcd_reg read --offset 0x200e --count=1
>     0x200e:  80
> 
> Workaround this by not trusting link status registers after PSR is enabled
> until first short pulse interrupt is received.
> 
> Signed-off-by: Jouni Högander <jouni.hogander at intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |  2 +
>  drivers/gpu/drm/i915/display/intel_dp.c       |  3 +-
>  drivers/gpu/drm/i915/display/intel_psr.c      | 39 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_psr.h      |  1 +
>  4 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 2bb1fa64da2f..f0b7d7262961 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1618,6 +1618,8 @@ struct intel_psr {
>  	u32 dc3co_exit_delay;
>  	struct delayed_work dc3co_work;
>  	u8 entry_setup_frames;
> +
> +	bool link_ok;
>  };
>  
>  struct intel_dp {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index b036c6521659..efaaadfb12ea 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5007,7 +5007,8 @@ intel_dp_needs_link_retrain(struct intel_dp *intel_dp)
>  		return true;
>  
>  	/* Retrain if link not ok */
> -	return !intel_dp_link_ok(intel_dp, link_status);
> +	return !(intel_dp_link_ok(intel_dp, link_status) ||
> +		 intel_psr_link_ok(intel_dp));
>  }
>  
>  bool intel_dp_has_connector(struct intel_dp *intel_dp,
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 992543f508c1..0cd7388389e0 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2009,6 +2009,15 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
>  	intel_dp->psr.enabled = true;
>  	intel_dp->psr.paused = false;
>  
> +	/*
> +	 * Link_ok is sticky and set here on PSR enable. We can assume link
> +	 * training is complete as we never continue to PSR enable with
> +	 * untrained link. Link_ok is kept as set until first short pulse
> +	 * interrupt. This is targeted to workaround panels stating bad link
> +	 * after PSR is enabled.
> +	 */
> +	intel_dp->psr.link_ok = true;
> +
>  	intel_psr_activate(intel_dp);
>  }
>  
> @@ -3458,6 +3467,9 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
>  
>  	mutex_lock(&psr->lock);
>  
> +	/* Let's clear possibly set link_ok flag here */
> +	psr->link_ok = false;

Should psr->link_ok also get reset when the output is disabled, so it's
effect is consistent (i.e. doesn't effect the link state from output
enabling/link training until PSR is enabled)?

> +
>  	if (!psr->enabled)
>  		goto exit;
>  
> @@ -3517,6 +3529,33 @@ bool intel_psr_enabled(struct intel_dp *intel_dp)
>  	return ret;
>  }
>  
> +/**
> + * intel_psr_link_ok - return psr->link_ok
> + * @intel_dp: struct intel_dp
> + *
> + * We are seeing unexpected link re-trainings with some panels. This is caused
> + * by panel stating bad link status after PSR is enabled. Code checking link
> + * status can call this to ensure it can ignore bad link status stated by the
> + * panel I.e. if panel is stating bad link and intel_psr_link_ok is stating link
> + * is ok caller should rely on latter.
> + *
> + * Return value of link_ok
> + */
> +bool intel_psr_link_ok(struct intel_dp *intel_dp)
> +{
> +	bool ret;
> +
> +	if ((!CAN_PSR(intel_dp) && !CAN_PANEL_REPLAY(intel_dp)) ||
> +	    !intel_dp_is_edp(intel_dp))
> +		return false;
> +
> +	mutex_lock(&intel_dp->psr.lock);
> +	ret = intel_dp->psr.link_ok;
> +	mutex_unlock(&intel_dp->psr.lock);
> +
> +	return ret;
> +}
> +
>  /**
>   * intel_psr_lock - grab PSR lock
>   * @crtc_state: the crtc state
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
> index 5f26f61f82aa..956be263c09e 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -59,6 +59,7 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
>  void intel_psr_pause(struct intel_dp *intel_dp);
>  void intel_psr_resume(struct intel_dp *intel_dp);
>  bool intel_psr_needs_block_dc_vblank(const struct intel_crtc_state *crtc_state);
> +bool intel_psr_link_ok(struct intel_dp *intel_dp);
>  
>  void intel_psr_lock(const struct intel_crtc_state *crtc_state);
>  void intel_psr_unlock(const struct intel_crtc_state *crtc_state);
> -- 
> 2.34.1
>