All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/dp: wait for previous AUX channel activity to clear
@ 2011-08-01 22:02 Jesse Barnes
  0 siblings, 0 replies; 7+ messages in thread
From: Jesse Barnes @ 2011-08-01 22:02 UTC (permalink / raw)
  To: intel-gfx

Before initiating a new read or write on the DP AUX channel, wait for
any outstanding activity to complete.  This may happen during normal
retry behavior.  If the wait fails (i.e. after 1ms the AUX channel is
still busy) dump a backtrace to make the caller easier to spot.

v2: use msleep instead, and timeout after 3ms (only ever saw 1 retry
    with msleep in testing)
v3: fix backtrace check to trigger if the 3ms wait times out

Fixes https://bugs.freedesktop.org/show_bug.cgi?id=38136.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_dp.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4493641..ba72fbc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -315,9 +315,17 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	else
 		precharge = 5;
 
-	if (I915_READ(ch_ctl) & DP_AUX_CH_CTL_SEND_BUSY) {
-		DRM_ERROR("dp_aux_ch not started status 0x%08x\n",
-			  I915_READ(ch_ctl));
+	/* Try to wait for any previous AUX channel activity */
+	for (try = 0; try < 3; try++) {
+		status = I915_READ(ch_ctl);
+		if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
+			break;
+		msleep(1);
+	}
+
+	if (try == 3) {
+		WARN(1, "dp_aux_ch not started status 0x%08x\n",
+		     I915_READ(ch_ctl));
 		return -EBUSY;
 	}
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/dp: wait for previous AUX channel activity to clear
  2011-08-01 21:42 ` Keith Packard
@ 2011-08-01 22:00   ` Jesse Barnes
  0 siblings, 0 replies; 7+ messages in thread
From: Jesse Barnes @ 2011-08-01 22:00 UTC (permalink / raw)
  To: Keith Packard; +Cc: intel-gfx

On Mon, 01 Aug 2011 14:42:25 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Mon,  1 Aug 2011 14:14:35 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > +	for (try = 0; try < 3; try++) {
> > +		status = I915_READ(ch_ctl);
> > +		if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > +			break;
> > +		msleep(1);
> > +	}
> > +
> > +	if (try == 10) {
> 
> 10 != 3

Oh well that will make the backtrace silent, won't it...

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/dp: wait for previous AUX channel activity to clear
  2011-08-01 21:14 Jesse Barnes
@ 2011-08-01 21:42 ` Keith Packard
  2011-08-01 22:00   ` Jesse Barnes
  0 siblings, 1 reply; 7+ messages in thread
From: Keith Packard @ 2011-08-01 21:42 UTC (permalink / raw)
  To: Jesse Barnes, intel-gfx


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

On Mon,  1 Aug 2011 14:14:35 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> +	for (try = 0; try < 3; try++) {
> +		status = I915_READ(ch_ctl);
> +		if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> +			break;
> +		msleep(1);
> +	}
> +
> +	if (try == 10) {

10 != 3

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] drm/i915/dp: wait for previous AUX channel activity to clear
@ 2011-08-01 21:14 Jesse Barnes
  2011-08-01 21:42 ` Keith Packard
  0 siblings, 1 reply; 7+ messages in thread
From: Jesse Barnes @ 2011-08-01 21:14 UTC (permalink / raw)
  To: intel-gfx

Before initiating a new read or write on the DP AUX channel, wait for
any outstanding activity to complete.  This may happen during normal
retry behavior.  If the wait fails (i.e. after 1ms the AUX channel is
still busy) dump a backtrace to make the caller easier to spot.

v2: use msleep instead, and timeout after 3ms (only ever saw 1 retry
    with msleep in testing)

Fixes https://bugs.freedesktop.org/show_bug.cgi?id=38136.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_dp.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4493641..de570ce 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -315,9 +315,17 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	else
 		precharge = 5;
 
-	if (I915_READ(ch_ctl) & DP_AUX_CH_CTL_SEND_BUSY) {
-		DRM_ERROR("dp_aux_ch not started status 0x%08x\n",
-			  I915_READ(ch_ctl));
+	/* Try to wait for any previous AUX channel activity */
+	for (try = 0; try < 3; try++) {
+		status = I915_READ(ch_ctl);
+		if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
+			break;
+		msleep(1);
+	}
+
+	if (try == 10) {
+		WARN(1, "dp_aux_ch not started status 0x%08x\n",
+		     I915_READ(ch_ctl));
 		return -EBUSY;
 	}
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/dp: wait for previous AUX channel activity to clear
  2011-08-01 20:53 ` Keith Packard
@ 2011-08-01 21:10   ` Jesse Barnes
  0 siblings, 0 replies; 7+ messages in thread
From: Jesse Barnes @ 2011-08-01 21:10 UTC (permalink / raw)
  To: Keith Packard; +Cc: intel-gfx

On Mon, 01 Aug 2011 13:53:59 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Mon,  1 Aug 2011 13:03:33 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > +		udelay(100);
> 
> udelay isn't my favorite function to use while waiting for hardware to
> clean up. Do we expect to get into this path regularly? Is it just when
> hardware is broken? How many times around this loop do we go before
> things work?
> 
> If we only occasionally enter this loop, and if we end up going around
> several times, it seems like using msleep would be nicer to the system.

We enter it enough that a sleep would be preferable (3-5 times in my
testing).  I'll fix it up and resend.

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/dp: wait for previous AUX channel activity to clear
  2011-08-01 20:03 Jesse Barnes
@ 2011-08-01 20:53 ` Keith Packard
  2011-08-01 21:10   ` Jesse Barnes
  0 siblings, 1 reply; 7+ messages in thread
From: Keith Packard @ 2011-08-01 20:53 UTC (permalink / raw)
  To: Jesse Barnes, intel-gfx


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

On Mon,  1 Aug 2011 13:03:33 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> +		udelay(100);

udelay isn't my favorite function to use while waiting for hardware to
clean up. Do we expect to get into this path regularly? Is it just when
hardware is broken? How many times around this loop do we go before
things work?

If we only occasionally enter this loop, and if we end up going around
several times, it seems like using msleep would be nicer to the system.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] drm/i915/dp: wait for previous AUX channel activity to clear
@ 2011-08-01 20:03 Jesse Barnes
  2011-08-01 20:53 ` Keith Packard
  0 siblings, 1 reply; 7+ messages in thread
From: Jesse Barnes @ 2011-08-01 20:03 UTC (permalink / raw)
  To: intel-gfx

Before initiating a new read or write on the DP AUX channel, wait for
any outstanding activity to complete.  This may happen during normal
retry behavior.  If the wait fails (i.e. after 1ms the AUX channel is
still busy) dump a backtrace to make the caller easier to spot.

Fixes https://bugs.freedesktop.org/show_bug.cgi?id=38136.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_dp.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4493641..34fbe7b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -315,9 +315,17 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	else
 		precharge = 5;
 
-	if (I915_READ(ch_ctl) & DP_AUX_CH_CTL_SEND_BUSY) {
-		DRM_ERROR("dp_aux_ch not started status 0x%08x\n",
-			  I915_READ(ch_ctl));
+	/* Try to wait for any previous AUX channel activity */
+	for (try = 0; try < 10; try++) {
+		status = I915_READ(ch_ctl);
+		if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
+			break;
+		udelay(100);
+	}
+
+	if (try == 10) {
+		WARN(1, "dp_aux_ch not started status 0x%08x\n",
+		     I915_READ(ch_ctl));
 		return -EBUSY;
 	}
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2011-08-01 22:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-01 22:02 [PATCH] drm/i915/dp: wait for previous AUX channel activity to clear Jesse Barnes
  -- strict thread matches above, loose matches on Subject: below --
2011-08-01 21:14 Jesse Barnes
2011-08-01 21:42 ` Keith Packard
2011-08-01 22:00   ` Jesse Barnes
2011-08-01 20:03 Jesse Barnes
2011-08-01 20:53 ` Keith Packard
2011-08-01 21:10   ` Jesse Barnes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.