public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Jouni Högander" <jouni.hogander@intel.com>
To: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Cc: jani.nikula@linux.intel.com, imre.deak@intel.com,
	"Jouni Högander" <jouni.hogander@intel.com>
Subject: [PATCH v2] drm/i915/psr: WA for panels stating bad link status after PSR is enabled
Date: Tue, 29 Oct 2024 14:24:15 +0200	[thread overview]
Message-ID: <20241029122415.1789528-1-jouni.hogander@intel.com> (raw)

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.

v2:
  - clear link_ok flag on pipe disable
  - remove useless comment
  - modify intel_dp_needs_link_retrain return statement

Signed-off-by: Jouni Högander <jouni.hogander@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      | 40 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_psr.h      |  1 +
 4 files changed, 45 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 9dd4610c749a..2212a9d97121 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5011,7 +5011,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 880ea845207f..7695225b3745 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -2013,6 +2013,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);
 }
 
@@ -2172,6 +2181,8 @@ void intel_psr_disable(struct intel_dp *intel_dp,
 
 	intel_psr_disable_locked(intel_dp);
 
+	intel_dp->psr.link_ok = false;
+
 	mutex_unlock(&intel_dp->psr.lock);
 	cancel_work_sync(&intel_dp->psr.work);
 	cancel_delayed_work_sync(&intel_dp->psr.dc3co_work);
@@ -3462,6 +3473,8 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
 
 	mutex_lock(&psr->lock);
 
+	psr->link_ok = false;
+
 	if (!psr->enabled)
 		goto exit;
 
@@ -3521,6 +3534,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


             reply	other threads:[~2024-10-29 12:24 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-29 12:24 Jouni Högander [this message]
2024-10-29 21:06 ` ✓ Fi.CI.BAT: success for drm/i915/psr: WA for panels stating bad link status after PSR is enabled (rev2) Patchwork
2024-10-30 16:51 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-11-04  7:31   ` Hogander, Jouni
2024-11-01 15:12 ` [PATCH v2] drm/i915/psr: WA for panels stating bad link status after PSR is enabled Imre Deak
2024-11-04  8:43   ` Hogander, Jouni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241029122415.1789528-1-jouni.hogander@intel.com \
    --to=jouni.hogander@intel.com \
    --cc=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox