Re: [PATCH v2] drm/i915/edp: Read link status after exit link training

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 28/04/2017 10:58, Lee, Shawn C wrote:
From: "Lee, Shawn C" <shawn.c.lee@xxxxxxxxx>

Display driver read DPCD register 0x202, 0x203 and 0x204 to identify
eDP sink status. If PSR exit is ongoing at eDP sink, and eDP source
read these registers at the same time. Panel will report EQ & symbol
lock not done. It will cause panel display flicking.
So driver have to make sure PSR already exit before read link status.

Change log:
v2:
- Use intel_wait_for_register() to replace I915_READ().

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99639
TEST=Reboot DUT and no flicking on local display at login screen

Cc: Cooper Chiou <cooper.chiou@xxxxxxxxx>
Cc: Gary C Wang <gary.c.wang@xxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Cc: Jim Bride <jim.bride@xxxxxxxxx>
Cc: Ryan Lin <ryan.lin@xxxxxxxxx>

Signed-off-by: Shawn Lee <shawn.c.lee@xxxxxxxxx>
---
 drivers/gpu/drm/i915/intel_dp.c |   33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 08834f74d396..099b6c0605a7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4252,19 +4252,34 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 }

 static void
+intel_edp_wait_PSR_exit(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;

... dev_priv = to_i915(dev);

+	u16 count = 100;
+
+	while (count--)
+		if (!intel_wait_for_register(dev_priv,
+			    EDP_PSR_STATUS_CTL,
+			    (EDP_PSR_STATUS_SENDING_TP1 |
+			     EDP_PSR_STATUS_SENDING_TP2_TP3 |
+			     EDP_PSR_STATUS_SENDING_IDLE |
+			     EDP_PSR_STATUS_AUX_SENDING),
+			     0,
+			     1))
+			return;
+}

You don't need your own retry loop - intel_wait_for_register will do that for you. But it will only check every millisecond, if the inital busy wait for 2us wasn't successful.

I've noticed that your initial patch had a 10ms timeout with a 100us period. If you had a good reason for that, then with this conversion you only have a 1ms timeout with a 1ms period, so if you need a shorter period, this solution is not ideal for you.

I've complicated the explanation a bit - but a bottom line is, v1 and v2 are a lot different. Depending on the typical response time one of the two will have a much worse latency.

+
+static void
 intel_dp_check_link_status(struct intel_dp *intel_dp)
 {
 	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;

to_i915 also.

 	u8 link_status[DP_LINK_STATUS_SIZE];

 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));

-	if (!intel_dp_get_link_status(intel_dp, link_status)) {
-		DRM_ERROR("Failed to get link status\n");
-		return;
-	}
-
 	if (!intel_encoder->base.crtc)
 		return;

@@ -4278,6 +4293,14 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 	if (!intel_dp_link_params_valid(intel_dp))
 		return;

+	if (is_edp(intel_dp) && dev_priv->psr.enabled)
+		intel_edp_wait_PSR_exit(intel_dp);
+
+	if (!intel_dp_get_link_status(intel_dp, link_status)) {
+		DRM_ERROR("Failed to get link status\n");
+		return;
+	}
+
 	/* Retrain if Channel EQ or CR not ok */
 	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
 		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",


Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux