[PATCH 0/5] Sink CRC stabilization

[PATCH 0/5] Sink CRC stabilization

[Rodrigo Vivi]

Let's start spliting that big series that enables PSR with this sink crc
stabilization.

Also I'm adding Wayne's mutex that stabilizes sink CRC on Skylake.

All patches already reviewed and ready to merge.

Thank you very much Paulo for the review and Thank you Wayne for the
SKL aux mutex.

Thanks,
Rodrigo.

Boyer, Wayne (1):
  drm/i915/skl: implement DP Aux Mutex framework

Rodrigo Vivi (4):
  drm/i915: Allow 1 vblank to let Sink CRC calculation to start or stop.
  drm/i915: Make Sink crc calculation waiting for counter to reset.
  drm/i915: Stop tracking last calculated Sink CRC.
  drm/i915: Rely on TEST_SINK_START instead of tracking Sink CRC state
    on dev_priv.

 drivers/gpu/drm/i915/i915_drv.h  |   1 +
 drivers/gpu/drm/i915/i915_reg.h  |   5 ++
 drivers/gpu/drm/i915/intel_dp.c  | 126 +++++++++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_drv.h |   7 ---
 4 files changed, 93 insertions(+), 46 deletions(-)

-- 
2.4.3

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

[PATCH 1/5] drm/i915: Allow 1 vblank to let Sink CRC calculation to start or stop.

[Rodrigo Vivi]

According to VESA DP Spec, setting TEST_SINK_START (bit 0)
of TEST_SINK (00270h) "Stop/Start calculating CRC on the next frame"

So let's wait at least 1 vblank to really say the calculation
stopped or started.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 99b7f1d..3121be5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3843,6 +3843,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp)
 static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = dig_port->base.base.dev;
 	struct intel_crtc *intel_crtc = to_intel_crtc(dig_port->base.base.crtc);
 	u8 buf;
 	int ret = 0;
@@ -3860,6 +3861,7 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp)
 		goto out;
 	}
 
+	intel_wait_for_vblank(dev, intel_crtc->pipe);
 	intel_dp->sink_crc.started = false;
  out:
 	hsw_enable_ips(intel_crtc);
@@ -3869,6 +3871,7 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp)
 static int intel_dp_sink_crc_start(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = dig_port->base.base.dev;
 	struct intel_crtc *intel_crtc = to_intel_crtc(dig_port->base.base.crtc);
 	u8 buf;
 	int ret;
@@ -3898,6 +3901,7 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp)
 		return -EIO;
 	}
 
+	intel_wait_for_vblank(dev, intel_crtc->pipe);
 	intel_dp->sink_crc.started = true;
 	return 0;
 }
-- 
2.4.3

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

[PATCH 2/5] drm/i915: Make Sink crc calculation waiting for counter to reset.

[Rodrigo Vivi]

According to VESA DP spec TEST_CRC_COUNT (Bits 3:0) at
TEST_SINK_MISC (00246h) is "Reset to 0 when TEST_SINK bit 0 = 0;

So let's give few vblanks so we are really sure that this counter
is really zeroed on the next sink_crc read.

v2: Use DRM_DEBUG_KMS instead of DRM_ERROR as Paulo suggested.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3121be5..5b72ce2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3847,6 +3847,8 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp)
 	struct intel_crtc *intel_crtc = to_intel_crtc(dig_port->base.base.crtc);
 	u8 buf;
 	int ret = 0;
+	int count = 0;
+	int attempts = 10;
 
 	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0) {
 		DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
@@ -3861,7 +3863,22 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp)
 		goto out;
 	}
 
-	intel_wait_for_vblank(dev, intel_crtc->pipe);
+	do {
+		intel_wait_for_vblank(dev, intel_crtc->pipe);
+
+		if (drm_dp_dpcd_readb(&intel_dp->aux,
+				      DP_TEST_SINK_MISC, &buf) < 0) {
+			ret = -EIO;
+			goto out;
+		}
+		count = buf & DP_TEST_COUNT_MASK;
+	} while (--attempts && count);
+
+	if (attempts == 0) {
+		DRM_DEBUG_KMS("TIMEOUT: Sink CRC counter is not zeroed\n");
+		ret = -ETIMEDOUT;
+	}
+
 	intel_dp->sink_crc.started = false;
  out:
 	hsw_enable_ips(intel_crtc);
-- 
2.4.3

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

[PATCH 3/5] drm/i915: Stop tracking last calculated Sink CRC.

[Rodrigo Vivi]

It was created at 'commit aabc95dcf20 (drm/i915: Dont -ETIMEDOUT
on identical new and previous (count, crc).")' becase the counter
wasn't reliable.

Now that we properly wait for the counter to be reset we can rely
a bit more in the counter.

Also that patch stopped to return -ETIMEDOUT so the test case is
unable to skip when it is unreliable and end up in many fails
that should be skip instead.

So, with the counter more reliable we can remove
this hack that just makes things more confusing when test cases
are really expecting the same CRC and let test case skip if that's
not the case.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 39 +++++++++------------------------------
 drivers/gpu/drm/i915/intel_drv.h |  2 --
 2 files changed, 9 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5b72ce2..150e680 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3905,8 +3905,6 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp)
 	if (!(buf & DP_TEST_CRC_SUPPORTED))
 		return -ENOTTY;
 
-	intel_dp->sink_crc.last_count = buf & DP_TEST_COUNT_MASK;
-
 	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
 		return -EIO;
 
@@ -3931,7 +3929,6 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 	u8 buf;
 	int count, ret;
 	int attempts = 6;
-	bool old_equal_new;
 
 	ret = intel_dp_sink_crc_start(intel_dp);
 	if (ret)
@@ -3947,35 +3944,17 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 		}
 		count = buf & DP_TEST_COUNT_MASK;
 
-		/*
-		 * Count might be reset during the loop. In this case
-		 * last known count needs to be reset as well.
-		 */
-		if (count == 0)
-			intel_dp->sink_crc.last_count = 0;
-
-		if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) {
-			ret = -EIO;
-			goto stop;
-		}
-
-		old_equal_new = (count == intel_dp->sink_crc.last_count &&
-				 !memcmp(intel_dp->sink_crc.last_crc, crc,
-					 6 * sizeof(u8)));
-
-	} while (--attempts && (count == 0 || old_equal_new));
-
-	intel_dp->sink_crc.last_count = buf & DP_TEST_COUNT_MASK;
-	memcpy(intel_dp->sink_crc.last_crc, crc, 6 * sizeof(u8));
+	} while (--attempts && count == 0);
 
 	if (attempts == 0) {
-		if (old_equal_new) {
-			DRM_DEBUG_KMS("Unreliable Sink CRC counter: Current returned CRC is identical to the previous one\n");
-		} else {
-			DRM_ERROR("Panel is unable to calculate any CRC after 6 vblanks\n");
-			ret = -ETIMEDOUT;
-			goto stop;
-		}
+		DRM_ERROR("Panel is unable to calculate any CRC after 6 vblanks\n");
+		ret = -ETIMEDOUT;
+		goto stop;
+	}
+
+	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) {
+		ret = -EIO;
+		goto stop;
 	}
 
 stop:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e3794d3..e922d6a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -740,8 +740,6 @@ enum link_m_n_set {
 
 struct sink_crc {
 	bool started;
-	u8 last_crc[6];
-	int last_count;
 };
 
 struct intel_dp {
-- 
2.4.3

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

[PATCH 4/5] drm/i915: Rely on TEST_SINK_START instead of tracking Sink CRC state on dev_priv.

[Rodrigo Vivi]

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 14 ++++++--------
 drivers/gpu/drm/i915/intel_drv.h |  5 -----
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 150e680..da02ed7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3879,7 +3879,6 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp)
 		ret = -ETIMEDOUT;
 	}
 
-	intel_dp->sink_crc.started = false;
  out:
 	hsw_enable_ips(intel_crtc);
 	return ret;
@@ -3893,12 +3892,6 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp)
 	u8 buf;
 	int ret;
 
-	if (intel_dp->sink_crc.started) {
-		ret = intel_dp_sink_crc_stop(intel_dp);
-		if (ret)
-			return ret;
-	}
-
 	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
 		return -EIO;
 
@@ -3908,6 +3901,12 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp)
 	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
 		return -EIO;
 
+	if (buf & DP_TEST_SINK_START) {
+		ret = intel_dp_sink_crc_stop(intel_dp);
+		if (ret)
+			return ret;
+	}
+
 	hsw_disable_ips(intel_crtc);
 
 	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
@@ -3917,7 +3916,6 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp)
 	}
 
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
-	intel_dp->sink_crc.started = true;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e922d6a..934d946 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -738,10 +738,6 @@ enum link_m_n_set {
 	M2_N2
 };
 
-struct sink_crc {
-	bool started;
-};
-
 struct intel_dp {
 	uint32_t output_reg;
 	uint32_t aux_ch_ctl_reg;
@@ -758,7 +754,6 @@ struct intel_dp {
 	/* sink rates as reported by DP_SUPPORTED_LINK_RATES */
 	uint8_t num_sink_rates;
 	int sink_rates[DP_MAX_SUPPORTED_RATES];
-	struct sink_crc sink_crc;
 	struct drm_dp_aux aux;
 	uint8_t train_set[4];
 	int panel_power_up_delay;
-- 
2.4.3

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

[PATCH 5/5] drm/i915/skl: implement DP Aux Mutex framework

[Rodrigo Vivi]

From: "Boyer, Wayne" <wayne.boyer@intel.com>

Beginning with SKL the DP Aux channel communication can be protected
using a built in HW mutex.

When PSR is enablabled the HW takes control on AUX and uses it to
control panel exit/entry states.

When validating PSR with automated tests, grabbing CRC from sink
revealed strange aux communication issues.  Aux reads were returning
a message read size equal to 0 and 0 is a forbidden message.

By using the HW mutex the HW is blocked from using aux when running
the automated PSR tests.

This patch provides an initial implementation for using that mutex.
The use is currently limited to protecting the sink crc request based
on feedback from the H/W designers indicating that using the mutex
for all aux channel communication is not recommended.

v2: Improved commit message to explain the case where the HW mutex is
helpful.  Also added bug reference.

Signed-off-by: Wayne Boyer <wayne.boyer@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91437
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Tested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_reg.h |  5 ++++
 drivers/gpu/drm/i915/intel_dp.c | 52 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d5cf30b..ac7ed0d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2585,6 +2585,7 @@ struct drm_i915_cmd_table {
 
 #define HAS_DDI(dev)		(INTEL_INFO(dev)->has_ddi)
 #define HAS_FPGA_DBG_UNCLAIMED(dev)	(INTEL_INFO(dev)->has_fpga_dbg)
+#define HAS_AUX_MUTEX(dev)	(INTEL_INFO(dev)->gen >= 9)
 #define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
 				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
 				 IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8bd2699..f9ee874 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4288,6 +4288,11 @@ enum skl_disp_power_wells {
 #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
 #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)
 
+#define DP_AUX_MUTEX_A			0x6402C
+#define DP_AUX_MUTEX_B			0x6412C
+#define   DP_AUX_MUTEX_ENABLE		    (1 << 31)
+#define   DP_AUX_MUTEX_STATUS		    (1 << 30)
+
 /*
  * Computing GMCH M and N values for the Display Port link
  *
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index da02ed7..b3c7d82 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -781,6 +781,47 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
 	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
 }
 
+static bool skl_aux_mutex(struct intel_dp *intel_dp, bool get)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t aux_ch_mutex, status;
+	int count = 0;
+
+	if (!HAS_AUX_MUTEX(dev))
+		return false;
+
+	/*
+	 * FIXME: determine actual aux channel
+	 * Hard coded to channel A for now to protect sink crc requests on eDP.
+	 */
+	aux_ch_mutex = DP_AUX_MUTEX_A;
+
+	if (!get) {
+		I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE | DP_AUX_MUTEX_STATUS);
+		return false;
+	}
+
+	/*
+	 * The Bspec specifies waiting 500us between attempts to acquire the
+	 * mutex.  Ten retries should be adequate to balance successfully
+	 * acquirng the mutex and spending too much time trying.
+	 */
+	while (count++ < 10) {
+		I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE);
+		status = I915_READ(aux_ch_mutex);
+		if (!(status & DP_AUX_MUTEX_STATUS))
+			return true;
+		udelay(500);
+	}
+
+	return false;
+}
+
+#define skl_aux_mutex_get(dev) skl_aux_mutex(dev, true)
+#define skl_aux_mutex_put(dev) skl_aux_mutex(dev, false)
+
 static int
 intel_dp_aux_ch(struct intel_dp *intel_dp,
 		const uint8_t *send, int send_bytes,
@@ -3927,10 +3968,14 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 	u8 buf;
 	int count, ret;
 	int attempts = 6;
+	bool aux_mutex_acquired = false;
+
+	aux_mutex_acquired = skl_aux_mutex_get(intel_dp);
 
 	ret = intel_dp_sink_crc_start(intel_dp);
+
 	if (ret)
-		return ret;
+		goto release;
 
 	do {
 		intel_wait_for_vblank(dev, intel_crtc->pipe);
@@ -3957,6 +4002,11 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 
 stop:
 	intel_dp_sink_crc_stop(intel_dp);
+
+release:
+	if (aux_mutex_acquired)
+		aux_mutex_acquired = skl_aux_mutex_put(intel_dp);
+
 	return ret;
 }
 
-- 
2.4.3

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

[PATCH] drm/i915/skl: Implement DP Aux Mutex framework

[Wayne Boyer]

From: "Boyer, Wayne" <wayne.boyer@intel.com>

Beginning with SKL the DP Aux channel communication can be protected
using a built in HW mutex.

When PSR is enabled the HW takes control on AUX and uses it to
control panel exit/entry states.

When validating PSR with automated tests, grabbing CRC from sink
revealed strange aux communication issues.  Aux reads were returning
a message read size equal to 0 and 0 is a forbidden message.

By using the HW mutex the HW is blocked from using aux when running
the automated PSR tests.

This patch provides an initial implementation for using that mutex.
The use is currently limited to protecting the sink crc request based
on feedback from the H/W designers indicating that using the mutex
for all aux channel communication is not recommended.

v2: Improved commit message to explain the case where the HW mutex is
helpful.  Also added bug reference.
v3: Fix typos in commit message.

Signed-off-by: Wayne Boyer <wayne.boyer@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91437
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Tested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_reg.h |  5 ++++
 drivers/gpu/drm/i915/intel_dp.c | 52 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d5cf30b..ac7ed0d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2585,6 +2585,7 @@ struct drm_i915_cmd_table {
 
 #define HAS_DDI(dev)		(INTEL_INFO(dev)->has_ddi)
 #define HAS_FPGA_DBG_UNCLAIMED(dev)	(INTEL_INFO(dev)->has_fpga_dbg)
+#define HAS_AUX_MUTEX(dev)	(INTEL_INFO(dev)->gen >= 9)
 #define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
 				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
 				 IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8bd2699..f9ee874 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4288,6 +4288,11 @@ enum skl_disp_power_wells {
 #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
 #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)
 
+#define DP_AUX_MUTEX_A			0x6402C
+#define DP_AUX_MUTEX_B			0x6412C
+#define   DP_AUX_MUTEX_ENABLE		    (1 << 31)
+#define   DP_AUX_MUTEX_STATUS		    (1 << 30)
+
 /*
  * Computing GMCH M and N values for the Display Port link
  *
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index da02ed7..b3c7d82 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -781,6 +781,47 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
 	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
 }
 
+static bool skl_aux_mutex(struct intel_dp *intel_dp, bool get)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t aux_ch_mutex, status;
+	int count = 0;
+
+	if (!HAS_AUX_MUTEX(dev))
+		return false;
+
+	/*
+	 * FIXME: determine actual aux channel
+	 * Hard coded to channel A for now to protect sink crc requests on eDP.
+	 */
+	aux_ch_mutex = DP_AUX_MUTEX_A;
+
+	if (!get) {
+		I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE | DP_AUX_MUTEX_STATUS);
+		return false;
+	}
+
+	/*
+	 * The Bspec specifies waiting 500us between attempts to acquire the
+	 * mutex.  Ten retries should be adequate to balance successfully
+	 * acquirng the mutex and spending too much time trying.
+	 */
+	while (count++ < 10) {
+		I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE);
+		status = I915_READ(aux_ch_mutex);
+		if (!(status & DP_AUX_MUTEX_STATUS))
+			return true;
+		udelay(500);
+	}
+
+	return false;
+}
+
+#define skl_aux_mutex_get(dev) skl_aux_mutex(dev, true)
+#define skl_aux_mutex_put(dev) skl_aux_mutex(dev, false)
+
 static int
 intel_dp_aux_ch(struct intel_dp *intel_dp,
 		const uint8_t *send, int send_bytes,
@@ -3927,10 +3968,14 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 	u8 buf;
 	int count, ret;
 	int attempts = 6;
+	bool aux_mutex_acquired = false;
+
+	aux_mutex_acquired = skl_aux_mutex_get(intel_dp);
 
 	ret = intel_dp_sink_crc_start(intel_dp);
+
 	if (ret)
-		return ret;
+		goto release;
 
 	do {
 		intel_wait_for_vblank(dev, intel_crtc->pipe);
@@ -3957,6 +4002,11 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 
 stop:
 	intel_dp_sink_crc_stop(intel_dp);
+
+release:
+	if (aux_mutex_acquired)
+		aux_mutex_acquired = skl_aux_mutex_put(intel_dp);
+
 	return ret;
 }
 
-- 
1.9.1

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

Re: [PATCH] drm/i915/skl: Implement DP Aux Mutex framework

[Jani Nikula]

"aux mutex framework" in the title sounds a bit grandiose, to be
honest. "add support for DP AUX hardware mutex" is what I would have
used.

Further comments inline.

On Thu, 12 Nov 2015, Wayne Boyer <wayne.boyer@intel.com> wrote:
> From: "Boyer, Wayne" <wayne.boyer@intel.com>
>
> Beginning with SKL the DP Aux channel communication can be protected
> using a built in HW mutex.
>
> When PSR is enabled the HW takes control on AUX and uses it to
> control panel exit/entry states.
>
> When validating PSR with automated tests, grabbing CRC from sink
> revealed strange aux communication issues.  Aux reads were returning
> a message read size equal to 0 and 0 is a forbidden message.
>
> By using the HW mutex the HW is blocked from using aux when running
> the automated PSR tests.
>
> This patch provides an initial implementation for using that mutex.
> The use is currently limited to protecting the sink crc request based
> on feedback from the H/W designers indicating that using the mutex
> for all aux channel communication is not recommended.
>
> v2: Improved commit message to explain the case where the HW mutex is
> helpful.  Also added bug reference.
> v3: Fix typos in commit message.
>
> Signed-off-by: Wayne Boyer <wayne.boyer@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91437
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Tested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  1 +
>  drivers/gpu/drm/i915/i915_reg.h |  5 ++++
>  drivers/gpu/drm/i915/intel_dp.c | 52 ++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d5cf30b..ac7ed0d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2585,6 +2585,7 @@ struct drm_i915_cmd_table {
>  
>  #define HAS_DDI(dev)		(INTEL_INFO(dev)->has_ddi)
>  #define HAS_FPGA_DBG_UNCLAIMED(dev)	(INTEL_INFO(dev)->has_fpga_dbg)
> +#define HAS_AUX_MUTEX(dev)	(INTEL_INFO(dev)->gen >= 9)
>  #define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
>  				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
>  				 IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8bd2699..f9ee874 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4288,6 +4288,11 @@ enum skl_disp_power_wells {
>  #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
>  #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)
>  
> +#define DP_AUX_MUTEX_A			0x6402C
> +#define DP_AUX_MUTEX_B			0x6412C

Please add all of these at once, using an underscore prefix, and add
DP_AUX_MUTEX(port) to pick the right one. Plenty of examples in the
file. And then Ville doesn't have to clean it up afterwards.

> +#define   DP_AUX_MUTEX_ENABLE		    (1 << 31)
> +#define   DP_AUX_MUTEX_STATUS		    (1 << 30)
> +
>  /*
>   * Computing GMCH M and N values for the Display Port link
>   *
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index da02ed7..b3c7d82 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -781,6 +781,47 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
>  	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
>  }
>  
> +static bool skl_aux_mutex(struct intel_dp *intel_dp, bool get)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_device *dev = intel_dig_port->base.base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	uint32_t aux_ch_mutex, status;
> +	int count = 0;
> +
> +	if (!HAS_AUX_MUTEX(dev))
> +		return false;
> +
> +	/*
> +	 * FIXME: determine actual aux channel
> +	 * Hard coded to channel A for now to protect sink crc requests on eDP.
> +	 */

If you want to leave it like this for now, the please add "|| !is_edp()"
alongside !HAS_AUX_MUTEX() for an early return.

> +	aux_ch_mutex = DP_AUX_MUTEX_A;
> +
> +	if (!get) {
> +		I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE | DP_AUX_MUTEX_STATUS);
> +		return false;
> +	}
> +
> +	/*
> +	 * The Bspec specifies waiting 500us between attempts to acquire the
> +	 * mutex.  Ten retries should be adequate to balance successfully
> +	 * acquirng the mutex and spending too much time trying.
> +	 */
> +	while (count++ < 10) {

Please use a for loop whenever you can use a for loop. The reader has to
pause for a second to ensure "while (count++ < 10)" does the right thing
and count is initialized properly; "for (i = 0; i < 10; i++)" comes from
the spine.

> +		I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE);
> +		status = I915_READ(aux_ch_mutex);
> +		if (!(status & DP_AUX_MUTEX_STATUS))
> +			return true;
> +		udelay(500);
> +	}

You should probably add an error or a debug message here.

I observe that the caller has no way of knowing whether the lock was not
acquired because the platform doesn't support it, or because we failed
to acquire it. I would imagine the callers might want to opt for not
proceeding to mess with the hardware if they can't ensure exclusive
access. That's kind of the point of locking.

> +
> +	return false;
> +}
> +
> +#define skl_aux_mutex_get(dev) skl_aux_mutex(dev, true)
> +#define skl_aux_mutex_put(dev) skl_aux_mutex(dev, false)

Please have two separate functions instead of this indirection. Name
them _lock and _unlock; _get and _put imply support for
refcounting/nesting, which isn't there.

> +
>  static int
>  intel_dp_aux_ch(struct intel_dp *intel_dp,
>  		const uint8_t *send, int send_bytes,
> @@ -3927,10 +3968,14 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  	u8 buf;
>  	int count, ret;
>  	int attempts = 6;
> +	bool aux_mutex_acquired = false;

Unnecessary initialization.

> +
> +	aux_mutex_acquired = skl_aux_mutex_get(intel_dp);
>  
>  	ret = intel_dp_sink_crc_start(intel_dp);
> +

Unwanted whitespace change.

>  	if (ret)
> -		return ret;
> +		goto release;
>  
>  	do {
>  		intel_wait_for_vblank(dev, intel_crtc->pipe);
> @@ -3957,6 +4002,11 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  
>  stop:
>  	intel_dp_sink_crc_stop(intel_dp);
> +
> +release:
> +	if (aux_mutex_acquired)
> +		aux_mutex_acquired = skl_aux_mutex_put(intel_dp);

No need to do an assignment here.

BR,
Jani.


> +
>  	return ret;
>  }

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/5] Sink CRC stabilization

[Jani Nikula]

On Wed, 11 Nov 2015, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Let's start spliting that big series that enables PSR with this sink crc
> stabilization.
>
> Also I'm adding Wayne's mutex that stabilizes sink CRC on Skylake.
>
> All patches already reviewed and ready to merge.

Disagreed on the hw mutex patch.

BR,
Jani.

>
> Thank you very much Paulo for the review and Thank you Wayne for the
> SKL aux mutex.
>
> Thanks,
> Rodrigo.
>
> Boyer, Wayne (1):
>   drm/i915/skl: implement DP Aux Mutex framework
>
> Rodrigo Vivi (4):
>   drm/i915: Allow 1 vblank to let Sink CRC calculation to start or stop.
>   drm/i915: Make Sink crc calculation waiting for counter to reset.
>   drm/i915: Stop tracking last calculated Sink CRC.
>   drm/i915: Rely on TEST_SINK_START instead of tracking Sink CRC state
>     on dev_priv.
>
>  drivers/gpu/drm/i915/i915_drv.h  |   1 +
>  drivers/gpu/drm/i915/i915_reg.h  |   5 ++
>  drivers/gpu/drm/i915/intel_dp.c  | 126 +++++++++++++++++++++++++++------------
>  drivers/gpu/drm/i915/intel_drv.h |   7 ---
>  4 files changed, 93 insertions(+), 46 deletions(-)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/5] Sink CRC stabilization

[Vivi, Rodrigo]

On Thu, 2015-11-12 at 12:17 +0200, Jani Nikula wrote:
> On Wed, 11 Nov 2015, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > Let's start spliting that big series that enables PSR with this 
> > sink crc
> > stabilization.
> > 
> > Also I'm adding Wayne's mutex that stabilizes sink CRC on Skylake.
> > 
> > All patches already reviewed and ready to merge.
> 
> Disagreed on the hw mutex patch.

I wasn't considered it as nacked. But it seems that we have more to
discuss around it. 

Although I don't see a reason of blocking a patch that follow spec, it
is reviewed, and it is used to fix a bug, unblock validation and only
used for the particular case that is just used on automated validation
and not broadly to all aux communications.

> 
> BR,
> Jani.
> 
> > 
> > Thank you very much Paulo for the review and Thank you Wayne for 
> > the
> > SKL aux mutex.
> > 
> > Thanks,
> > Rodrigo.
> > 
> > Boyer, Wayne (1):
> >   drm/i915/skl: implement DP Aux Mutex framework
> > 
> > Rodrigo Vivi (4):
> >   drm/i915: Allow 1 vblank to let Sink CRC calculation to start or 
> > stop.
> >   drm/i915: Make Sink crc calculation waiting for counter to reset.
> >   drm/i915: Stop tracking last calculated Sink CRC.
> >   drm/i915: Rely on TEST_SINK_START instead of tracking Sink CRC 
> > state
> >     on dev_priv.
> > 
> >  drivers/gpu/drm/i915/i915_drv.h  |   1 +
> >  drivers/gpu/drm/i915/i915_reg.h  |   5 ++
> >  drivers/gpu/drm/i915/intel_dp.c  | 126 +++++++++++++++++++++++++++
> > ------------
> >  drivers/gpu/drm/i915/intel_drv.h |   7 ---
> >  4 files changed, 93 insertions(+), 46 deletions(-)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/5] Sink CRC stabilization

[Rodrigo Vivi]

[-- Attachment #1.1: Type: text/plain, Size: 2049 bytes --]

On Thu, Nov 12, 2015 at 10:56 AM Vivi, Rodrigo <rodrigo.vivi@intel.com>
wrote:

> On Thu, 2015-11-12 at 12:17 +0200, Jani Nikula wrote:
> > On Wed, 11 Nov 2015, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > Let's start spliting that big series that enables PSR with this
> > > sink crc
> > > stabilization.
> > >
> > > Also I'm adding Wayne's mutex that stabilizes sink CRC on Skylake.
> > >
> > > All patches already reviewed and ready to merge.
> >
> > Disagreed on the hw mutex patch.
>
> I wasn't considered it as nacked. But it seems that we have more to
> discuss around it.
>
> Although I don't see a reason of blocking a patch that follow spec, it
> is reviewed, and it is used to fix a bug, unblock validation and only
> used for the particular case that is just used on automated validation
> and not broadly to all aux communications.
>

Oh, now I saw you had comments there on the patch. So please ignore this
complain and accept my apologies.


>
> >
> > BR,
> > Jani.
> >
> > >
> > > Thank you very much Paulo for the review and Thank you Wayne for
> > > the
> > > SKL aux mutex.
> > >
> > > Thanks,
> > > Rodrigo.
> > >
> > > Boyer, Wayne (1):
> > >   drm/i915/skl: implement DP Aux Mutex framework
> > >
> > > Rodrigo Vivi (4):
> > >   drm/i915: Allow 1 vblank to let Sink CRC calculation to start or
> > > stop.
> > >   drm/i915: Make Sink crc calculation waiting for counter to reset.
> > >   drm/i915: Stop tracking last calculated Sink CRC.
> > >   drm/i915: Rely on TEST_SINK_START instead of tracking Sink CRC
> > > state
> > >     on dev_priv.
> > >
> > >  drivers/gpu/drm/i915/i915_drv.h  |   1 +
> > >  drivers/gpu/drm/i915/i915_reg.h  |   5 ++
> > >  drivers/gpu/drm/i915/intel_dp.c  | 126 +++++++++++++++++++++++++++
> > > ------------
> > >  drivers/gpu/drm/i915/intel_drv.h |   7 ---
> > >  4 files changed, 93 insertions(+), 46 deletions(-)
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 3120 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 0/5] Sink CRC stabilization

[Rodrigo Vivi]

[-- Attachment #1.1: Type: text/plain, Size: 2404 bytes --]

Hi Daniel,

could you please ignore patch 5 in this series and merge the 4 first
patches.

The aux mutex is really unreliable and doesn't help on all SKLs so better
to just ignore that.

Thanks,
Rodrigo.

On Thu, Nov 12, 2015 at 11:03 AM Rodrigo Vivi <rodrigo.vivi@gmail.com>
wrote:

> On Thu, Nov 12, 2015 at 10:56 AM Vivi, Rodrigo <rodrigo.vivi@intel.com>
> wrote:
>
>> On Thu, 2015-11-12 at 12:17 +0200, Jani Nikula wrote:
>> > On Wed, 11 Nov 2015, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>> > > Let's start spliting that big series that enables PSR with this
>> > > sink crc
>> > > stabilization.
>> > >
>> > > Also I'm adding Wayne's mutex that stabilizes sink CRC on Skylake.
>> > >
>> > > All patches already reviewed and ready to merge.
>> >
>> > Disagreed on the hw mutex patch.
>>
>> I wasn't considered it as nacked. But it seems that we have more to
>> discuss around it.
>>
>> Although I don't see a reason of blocking a patch that follow spec, it
>> is reviewed, and it is used to fix a bug, unblock validation and only
>> used for the particular case that is just used on automated validation
>> and not broadly to all aux communications.
>>
>
> Oh, now I saw you had comments there on the patch. So please ignore this
> complain and accept my apologies.
>
>
>>
>> >
>> > BR,
>> > Jani.
>> >
>> > >
>> > > Thank you very much Paulo for the review and Thank you Wayne for
>> > > the
>> > > SKL aux mutex.
>> > >
>> > > Thanks,
>> > > Rodrigo.
>> > >
>> > > Boyer, Wayne (1):
>> > >   drm/i915/skl: implement DP Aux Mutex framework
>> > >
>> > > Rodrigo Vivi (4):
>> > >   drm/i915: Allow 1 vblank to let Sink CRC calculation to start or
>> > > stop.
>> > >   drm/i915: Make Sink crc calculation waiting for counter to reset.
>> > >   drm/i915: Stop tracking last calculated Sink CRC.
>> > >   drm/i915: Rely on TEST_SINK_START instead of tracking Sink CRC
>> > > state
>> > >     on dev_priv.
>> > >
>> > >  drivers/gpu/drm/i915/i915_drv.h  |   1 +
>> > >  drivers/gpu/drm/i915/i915_reg.h  |   5 ++
>> > >  drivers/gpu/drm/i915/intel_dp.c  | 126 +++++++++++++++++++++++++++
>> > > ------------
>> > >  drivers/gpu/drm/i915/intel_drv.h |   7 ---
>> > >  4 files changed, 93 insertions(+), 46 deletions(-)
>> >
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>

[-- Attachment #1.2: Type: text/html, Size: 3846 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 0/5] Sink CRC stabilization

[Daniel Vetter]

On Mon, Nov 16, 2015 at 03:59:15PM +0000, Rodrigo Vivi wrote:
> Hi Daniel,
> 
> could you please ignore patch 5 in this series and merge the 4 first
> patches.

I merged an earlier instance of the first 4 patches. Has anything changed?
Please double-check the right versions landed in dinq.

Thanks, Daniel

> 
> The aux mutex is really unreliable and doesn't help on all SKLs so better
> to just ignore that.
> 
> Thanks,
> Rodrigo.
> 
> On Thu, Nov 12, 2015 at 11:03 AM Rodrigo Vivi <rodrigo.vivi@gmail.com>
> wrote:
> 
> > On Thu, Nov 12, 2015 at 10:56 AM Vivi, Rodrigo <rodrigo.vivi@intel.com>
> > wrote:
> >
> >> On Thu, 2015-11-12 at 12:17 +0200, Jani Nikula wrote:
> >> > On Wed, 11 Nov 2015, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> >> > > Let's start spliting that big series that enables PSR with this
> >> > > sink crc
> >> > > stabilization.
> >> > >
> >> > > Also I'm adding Wayne's mutex that stabilizes sink CRC on Skylake.
> >> > >
> >> > > All patches already reviewed and ready to merge.
> >> >
> >> > Disagreed on the hw mutex patch.
> >>
> >> I wasn't considered it as nacked. But it seems that we have more to
> >> discuss around it.
> >>
> >> Although I don't see a reason of blocking a patch that follow spec, it
> >> is reviewed, and it is used to fix a bug, unblock validation and only
> >> used for the particular case that is just used on automated validation
> >> and not broadly to all aux communications.
> >>
> >
> > Oh, now I saw you had comments there on the patch. So please ignore this
> > complain and accept my apologies.
> >
> >
> >>
> >> >
> >> > BR,
> >> > Jani.
> >> >
> >> > >
> >> > > Thank you very much Paulo for the review and Thank you Wayne for
> >> > > the
> >> > > SKL aux mutex.
> >> > >
> >> > > Thanks,
> >> > > Rodrigo.
> >> > >
> >> > > Boyer, Wayne (1):
> >> > >   drm/i915/skl: implement DP Aux Mutex framework
> >> > >
> >> > > Rodrigo Vivi (4):
> >> > >   drm/i915: Allow 1 vblank to let Sink CRC calculation to start or
> >> > > stop.
> >> > >   drm/i915: Make Sink crc calculation waiting for counter to reset.
> >> > >   drm/i915: Stop tracking last calculated Sink CRC.
> >> > >   drm/i915: Rely on TEST_SINK_START instead of tracking Sink CRC
> >> > > state
> >> > >     on dev_priv.
> >> > >
> >> > >  drivers/gpu/drm/i915/i915_drv.h  |   1 +
> >> > >  drivers/gpu/drm/i915/i915_reg.h  |   5 ++
> >> > >  drivers/gpu/drm/i915/intel_dp.c  | 126 +++++++++++++++++++++++++++
> >> > > ------------
> >> > >  drivers/gpu/drm/i915/intel_drv.h |   7 ---
> >> > >  4 files changed, 93 insertions(+), 46 deletions(-)
> >> >
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>
> >

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/5] Sink CRC stabilization

[Vivi, Rodrigo]

On Wed, 2015-11-18 at 15:55 +0100, Daniel Vetter wrote:
> On Mon, Nov 16, 2015 at 03:59:15PM +0000, Rodrigo Vivi wrote:
> > Hi Daniel,
> > 
> > could you please ignore patch 5 in this series and merge the 4 
> > first
> > patches.
> 
> I merged an earlier instance of the first 4 patches. Has anything 
> changed?

no changes. thanks for merging and sorry for the doubled noise!

> Please double-check the right versions landed in dinq.

just did. thanks!

> 
> Thanks, Daniel
> 
> > 
> > The aux mutex is really unreliable and doesn't help on all SKLs so 
> > better
> > to just ignore that.
> > 
> > Thanks,
> > Rodrigo.
> > 
> > On Thu, Nov 12, 2015 at 11:03 AM Rodrigo Vivi <
> > rodrigo.vivi@gmail.com>
> > wrote:
> > 
> > > On Thu, Nov 12, 2015 at 10:56 AM Vivi, Rodrigo <
> > > rodrigo.vivi@intel.com>
> > > wrote:
> > > 
> > > > On Thu, 2015-11-12 at 12:17 +0200, Jani Nikula wrote:
> > > > > On Wed, 11 Nov 2015, Rodrigo Vivi <rodrigo.vivi@intel.com> 
> > > > > wrote:
> > > > > > Let's start spliting that big series that enables PSR with 
> > > > > > this
> > > > > > sink crc
> > > > > > stabilization.
> > > > > > 
> > > > > > Also I'm adding Wayne's mutex that stabilizes sink CRC on 
> > > > > > Skylake.
> > > > > > 
> > > > > > All patches already reviewed and ready to merge.
> > > > > 
> > > > > Disagreed on the hw mutex patch.
> > > > 
> > > > I wasn't considered it as nacked. But it seems that we have 
> > > > more to
> > > > discuss around it.
> > > > 
> > > > Although I don't see a reason of blocking a patch that follow 
> > > > spec, it
> > > > is reviewed, and it is used to fix a bug, unblock validation 
> > > > and only
> > > > used for the particular case that is just used on automated 
> > > > validation
> > > > and not broadly to all aux communications.
> > > > 
> > > 
> > > Oh, now I saw you had comments there on the patch. So please 
> > > ignore this
> > > complain and accept my apologies.
> > > 
> > > 
> > > > 
> > > > > 
> > > > > BR,
> > > > > Jani.
> > > > > 
> > > > > > 
> > > > > > Thank you very much Paulo for the review and Thank you 
> > > > > > Wayne for
> > > > > > the
> > > > > > SKL aux mutex.
> > > > > > 
> > > > > > Thanks,
> > > > > > Rodrigo.
> > > > > > 
> > > > > > Boyer, Wayne (1):
> > > > > >   drm/i915/skl: implement DP Aux Mutex framework
> > > > > > 
> > > > > > Rodrigo Vivi (4):
> > > > > >   drm/i915: Allow 1 vblank to let Sink CRC calculation to 
> > > > > > start or
> > > > > > stop.
> > > > > >   drm/i915: Make Sink crc calculation waiting for counter 
> > > > > > to reset.
> > > > > >   drm/i915: Stop tracking last calculated Sink CRC.
> > > > > >   drm/i915: Rely on TEST_SINK_START instead of tracking 
> > > > > > Sink CRC
> > > > > > state
> > > > > >     on dev_priv.
> > > > > > 
> > > > > >  drivers/gpu/drm/i915/i915_drv.h  |   1 +
> > > > > >  drivers/gpu/drm/i915/i915_reg.h  |   5 ++
> > > > > >  drivers/gpu/drm/i915/intel_dp.c  | 126 
> > > > > > +++++++++++++++++++++++++++
> > > > > > ------------
> > > > > >  drivers/gpu/drm/i915/intel_drv.h |   7 ---
> > > > > >  4 files changed, 93 insertions(+), 46 deletions(-)
> > > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > 
> > > 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/skl: implement DP Aux Mutex framework

[Wayne Boyer]

Beginning with SKL the DP Aux channel communication can be protected
using a built in H/W mutex.

This patch provides an initial implementation for using that mutex.
The use is currently limited to protecting the sink crc request based
on feedback from the H/W designers indicating that using the mutex
for all aux channel communication is not recommended.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Wayne Boyer <wayne.boyer@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_reg.h |  5 ++++
 drivers/gpu/drm/i915/intel_dp.c | 52 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b12594b..98e991d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2604,6 +2604,7 @@ struct drm_i915_cmd_table {
 
 #define HAS_DDI(dev)		(INTEL_INFO(dev)->has_ddi)
 #define HAS_FPGA_DBG_UNCLAIMED(dev)	(INTEL_INFO(dev)->has_fpga_dbg)
+#define HAS_AUX_MUTEX(dev)	(INTEL_INFO(dev)->gen >= 9)
 #define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
 				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
 				 IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8942532..a033e70 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4288,6 +4288,11 @@ enum skl_disp_power_wells {
 #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
 #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)
 
+#define DP_AUX_MUTEX_A			0x6402C
+#define DP_AUX_MUTEX_B			0x6412C
+#define   DP_AUX_MUTEX_ENABLE		    (1 << 31)
+#define   DP_AUX_MUTEX_STATUS		    (1 << 30)
+
 /*
  * Computing GMCH M and N values for the Display Port link
  *
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2fad873..e9e1239 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -781,6 +781,47 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
 	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
 }
 
+static bool skl_aux_mutex(struct intel_dp *intel_dp, bool get)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t aux_ch_mutex, status;
+	int count = 0;
+
+	if (!HAS_AUX_MUTEX(dev))
+		return false;
+
+	/*
+	 * FIXME: determine actual aux channel
+	 * Hard coded to channel A for now to protect sink crc requests on eDP.
+	 */
+	aux_ch_mutex = DP_AUX_MUTEX_A;
+
+	if (!get) {
+		I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE | DP_AUX_MUTEX_STATUS);
+		return false;
+	}
+
+	/*
+	 * The Bspec specifies waiting 500us between attempts to acquire the
+	 * mutex.  Ten retries should be adequate to balance successfully
+	 * acquirng the mutex and spending too much time trying.
+	 */
+	while (count++ < 10) {
+		I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE);
+		status = I915_READ(aux_ch_mutex);
+		if (!(status & DP_AUX_MUTEX_STATUS))
+			return true;
+		udelay(500);
+	}
+
+	return false;
+}
+
+#define skl_aux_mutex_get(dev) skl_aux_mutex(dev, true)
+#define skl_aux_mutex_put(dev) skl_aux_mutex(dev, false)
+
 static int
 intel_dp_aux_ch(struct intel_dp *intel_dp,
 		const uint8_t *send, int send_bytes,
@@ -4188,10 +4229,14 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 	u8 buf;
 	int count, ret;
 	int attempts = 6;
+	bool aux_mutex_acquired = false;
+
+	aux_mutex_acquired = skl_aux_mutex_get(intel_dp);
 
 	ret = intel_dp_sink_crc_start(intel_dp);
+
 	if (ret)
-		return ret;
+		goto release;
 
 	do {
 		intel_wait_for_vblank(dev, intel_crtc->pipe);
@@ -4218,6 +4263,11 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 
 stop:
 	intel_dp_sink_crc_stop(intel_dp);
+
+release:
+	if (aux_mutex_acquired)
+		aux_mutex_acquired = skl_aux_mutex_put(intel_dp);
+
 	return ret;
 }
 
-- 
1.9.1

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

Re: [PATCH] drm/i915/skl: implement DP Aux Mutex framework

[Jani Nikula]

On Tue, 10 Nov 2015, Wayne Boyer <wayne.boyer@intel.com> wrote:
> Beginning with SKL the DP Aux channel communication can be protected
> using a built in H/W mutex.
>
> This patch provides an initial implementation for using that mutex.
> The use is currently limited to protecting the sink crc request based
> on feedback from the H/W designers indicating that using the mutex
> for all aux channel communication is not recommended.

Please explain in the commit message what benefits this brings, and why
it's a good idea. Just that something *can* be done in hardware does not
mean that we should do it.

BR,
Jani.


>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Wayne Boyer <wayne.boyer@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  1 +
>  drivers/gpu/drm/i915/i915_reg.h |  5 ++++
>  drivers/gpu/drm/i915/intel_dp.c | 52 ++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b12594b..98e991d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2604,6 +2604,7 @@ struct drm_i915_cmd_table {
>  
>  #define HAS_DDI(dev)		(INTEL_INFO(dev)->has_ddi)
>  #define HAS_FPGA_DBG_UNCLAIMED(dev)	(INTEL_INFO(dev)->has_fpga_dbg)
> +#define HAS_AUX_MUTEX(dev)	(INTEL_INFO(dev)->gen >= 9)
>  #define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
>  				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
>  				 IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8942532..a033e70 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4288,6 +4288,11 @@ enum skl_disp_power_wells {
>  #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
>  #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)
>  
> +#define DP_AUX_MUTEX_A			0x6402C
> +#define DP_AUX_MUTEX_B			0x6412C
> +#define   DP_AUX_MUTEX_ENABLE		    (1 << 31)
> +#define   DP_AUX_MUTEX_STATUS		    (1 << 30)
> +
>  /*
>   * Computing GMCH M and N values for the Display Port link
>   *
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2fad873..e9e1239 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -781,6 +781,47 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
>  	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
>  }
>  
> +static bool skl_aux_mutex(struct intel_dp *intel_dp, bool get)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_device *dev = intel_dig_port->base.base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	uint32_t aux_ch_mutex, status;
> +	int count = 0;
> +
> +	if (!HAS_AUX_MUTEX(dev))
> +		return false;
> +
> +	/*
> +	 * FIXME: determine actual aux channel
> +	 * Hard coded to channel A for now to protect sink crc requests on eDP.
> +	 */
> +	aux_ch_mutex = DP_AUX_MUTEX_A;
> +
> +	if (!get) {
> +		I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE | DP_AUX_MUTEX_STATUS);
> +		return false;
> +	}
> +
> +	/*
> +	 * The Bspec specifies waiting 500us between attempts to acquire the
> +	 * mutex.  Ten retries should be adequate to balance successfully
> +	 * acquirng the mutex and spending too much time trying.
> +	 */
> +	while (count++ < 10) {
> +		I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE);
> +		status = I915_READ(aux_ch_mutex);
> +		if (!(status & DP_AUX_MUTEX_STATUS))
> +			return true;
> +		udelay(500);
> +	}
> +
> +	return false;
> +}
> +
> +#define skl_aux_mutex_get(dev) skl_aux_mutex(dev, true)
> +#define skl_aux_mutex_put(dev) skl_aux_mutex(dev, false)
> +
>  static int
>  intel_dp_aux_ch(struct intel_dp *intel_dp,
>  		const uint8_t *send, int send_bytes,
> @@ -4188,10 +4229,14 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  	u8 buf;
>  	int count, ret;
>  	int attempts = 6;
> +	bool aux_mutex_acquired = false;
> +
> +	aux_mutex_acquired = skl_aux_mutex_get(intel_dp);
>  
>  	ret = intel_dp_sink_crc_start(intel_dp);
> +
>  	if (ret)
> -		return ret;
> +		goto release;
>  
>  	do {
>  		intel_wait_for_vblank(dev, intel_crtc->pipe);
> @@ -4218,6 +4263,11 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  
>  stop:
>  	intel_dp_sink_crc_stop(intel_dp);
> +
> +release:
> +	if (aux_mutex_acquired)
> +		aux_mutex_acquired = skl_aux_mutex_put(intel_dp);
> +
>  	return ret;
>  }

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: implement DP Aux Mutex framework

[Dave Airlie]

On 10 November 2015 at 20:51, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Tue, 10 Nov 2015, Wayne Boyer <wayne.boyer@intel.com> wrote:
>> Beginning with SKL the DP Aux channel communication can be protected
>> using a built in H/W mutex.
>>
>> This patch provides an initial implementation for using that mutex.
>> The use is currently limited to protecting the sink crc request based
>> on feedback from the H/W designers indicating that using the mutex
>> for all aux channel communication is not recommended.
>
> Please explain in the commit message what benefits this brings, and why
> it's a good idea. Just that something *can* be done in hardware does not
> mean that we should do it.

I also have trouble understanding a mutex that only some things use. Seems
like a mutex needs to be used everywhere to provide locking.


Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: implement DP Aux Mutex framework

[Vivi, Rodrigo]

When PSR is enablabled the Hardware takes control on AUX and use it to
control panel exit/entry states.

When validating PSR with automated tests grabbing CRC from sink we were
facing strange aux communication issues where aux were returning
message size read equal 0. And 0 is a forbidden message.

So, by using the HW mutex you block HW from using aux when we are going
to use here on the automated PSR tests.

There was a bug opened at fd.o regarding sink_crc failing on SKL when
PSR was enabled but I couldn't find it. But we have 3 different
solutions for this case where we are trying to read sink crc and aux
being used by Hardware:

1. Use intel_dp_dpcd_read_wake for all dpcd reads at sink_crc. 

2. Return -EBUSY when message size is zeroed so drm takes care of
reties.

3. Use HW mutex so we let HW know we are going to use the aux.


Either one is fine for me as long as we have a reliable sink crc on SKL
and KBL.

Well, regarding this patch itself it seems that maintainers would need
a better description, but for the content I'm sure it is right and I
tested it here so feel free to use:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Tested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


On Tue, 2015-11-10 at 21:14 +1000, Dave Airlie wrote:
> On 10 November 2015 at 20:51, Jani Nikula <
> jani.nikula@linux.intel.com> wrote:
> > On Tue, 10 Nov 2015, Wayne Boyer <wayne.boyer@intel.com> wrote:
> > > Beginning with SKL the DP Aux channel communication can be 
> > > protected
> > > using a built in H/W mutex.
> > > 
> > > This patch provides an initial implementation for using that 
> > > mutex.
> > > The use is currently limited to protecting the sink crc request 
> > > based
> > > on feedback from the H/W designers indicating that using the 
> > > mutex
> > > for all aux channel communication is not recommended.
> > 
> > Please explain in the commit message what benefits this brings, and 
> > why
> > it's a good idea. Just that something *can* be done in hardware 
> > does not
> > mean that we should do it.
> 
> I also have trouble understanding a mutex that only some things use. 
> Seems
> like a mutex needs to be used everywhere to provide locking.
> 
> 
> Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/skl: implement DP Aux Mutex framework

[Wayne Boyer]

Beginning with SKL the DP Aux channel communication can be protected
using a built in HW mutex.

When PSR is enablabled the HW takes control on AUX and uses it to
control panel exit/entry states.

When validating PSR with automated tests, grabbing CRC from sink
revealed strange aux communication issues.  Aux reads were returning
a message read size equal to 0 and 0 is a forbidden message.

By using the HW mutex the HW is blocked from using aux when running
the automated PSR tests.

This patch provides an initial implementation for using that mutex.
The use is currently limited to protecting the sink crc request based
on feedback from the H/W designers indicating that using the mutex
for all aux channel communication is not recommended.

v2: Improved commit message to explain the case where the HW mutex is
helpful.  Also added bug reference.

Signed-off-by: Wayne Boyer <wayne.boyer@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91437
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Tested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_reg.h |  5 ++++
 drivers/gpu/drm/i915/intel_dp.c | 52 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b12594b..98e991d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2604,6 +2604,7 @@ struct drm_i915_cmd_table {
 
 #define HAS_DDI(dev)		(INTEL_INFO(dev)->has_ddi)
 #define HAS_FPGA_DBG_UNCLAIMED(dev)	(INTEL_INFO(dev)->has_fpga_dbg)
+#define HAS_AUX_MUTEX(dev)	(INTEL_INFO(dev)->gen >= 9)
 #define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
 				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
 				 IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8942532..a033e70 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4288,6 +4288,11 @@ enum skl_disp_power_wells {
 #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
 #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)
 
+#define DP_AUX_MUTEX_A			0x6402C
+#define DP_AUX_MUTEX_B			0x6412C
+#define   DP_AUX_MUTEX_ENABLE		    (1 << 31)
+#define   DP_AUX_MUTEX_STATUS		    (1 << 30)
+
 /*
  * Computing GMCH M and N values for the Display Port link
  *
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2fad873..e9e1239 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -781,6 +781,47 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
 	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
 }
 
+static bool skl_aux_mutex(struct intel_dp *intel_dp, bool get)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t aux_ch_mutex, status;
+	int count = 0;
+
+	if (!HAS_AUX_MUTEX(dev))
+		return false;
+
+	/*
+	 * FIXME: determine actual aux channel
+	 * Hard coded to channel A for now to protect sink crc requests on eDP.
+	 */
+	aux_ch_mutex = DP_AUX_MUTEX_A;
+
+	if (!get) {
+		I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE | DP_AUX_MUTEX_STATUS);
+		return false;
+	}
+
+	/*
+	 * The Bspec specifies waiting 500us between attempts to acquire the
+	 * mutex.  Ten retries should be adequate to balance successfully
+	 * acquirng the mutex and spending too much time trying.
+	 */
+	while (count++ < 10) {
+		I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE);
+		status = I915_READ(aux_ch_mutex);
+		if (!(status & DP_AUX_MUTEX_STATUS))
+			return true;
+		udelay(500);
+	}
+
+	return false;
+}
+
+#define skl_aux_mutex_get(dev) skl_aux_mutex(dev, true)
+#define skl_aux_mutex_put(dev) skl_aux_mutex(dev, false)
+
 static int
 intel_dp_aux_ch(struct intel_dp *intel_dp,
 		const uint8_t *send, int send_bytes,
@@ -4188,10 +4229,14 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 	u8 buf;
 	int count, ret;
 	int attempts = 6;
+	bool aux_mutex_acquired = false;
+
+	aux_mutex_acquired = skl_aux_mutex_get(intel_dp);
 
 	ret = intel_dp_sink_crc_start(intel_dp);
+
 	if (ret)
-		return ret;
+		goto release;
 
 	do {
 		intel_wait_for_vblank(dev, intel_crtc->pipe);
@@ -4218,6 +4263,11 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 
 stop:
 	intel_dp_sink_crc_stop(intel_dp);
+
+release:
+	if (aux_mutex_acquired)
+		aux_mutex_acquired = skl_aux_mutex_put(intel_dp);
+
 	return ret;
 }
 
-- 
2.5.0

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