* [PATCH 2/4] drm/i915: use cpu_relax() in wait_for_atomic
2012-09-02 5:59 [PATCH 1/4] drm/i915: Extract forcewake ack timeout Ben Widawsky
@ 2012-09-02 5:59 ` Ben Widawsky
2012-09-02 5:59 ` [PATCH 3/4] drm/i915: Change forcewake timeout to 2ms Ben Widawsky
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Ben Widawsky @ 2012-09-02 5:59 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
As part of the advice given to us from the hardware designers regarding
the maximum wait time on the forcewake handshake we need to move from us
granularity to ms granularity. In earlier patches to do this, Jani
noticed that wait_for_us was properly converted to use cpu_relax(), but
wait_for was not.
The issue has existed since the introduction of the macro:
commit 913d8d110078788c14812dce8bb62c37946821d2
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Sat Aug 7 11:01:35 2010 +0100
drm/i915: Ensure that while(INREG()) are bounded (v2)
CC: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/intel_drv.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 95f635b..1707de5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -41,7 +41,11 @@
ret__ = -ETIMEDOUT; \
break; \
} \
- if (W && drm_can_sleep()) msleep(W); \
+ if (W && drm_can_sleep()) { \
+ msleep(W); \
+ } else { \
+ cpu_relax(); \
+ } \
} \
ret__; \
})
--
1.7.12
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/4] drm/i915: Change forcewake timeout to 2ms
2012-09-02 5:59 [PATCH 1/4] drm/i915: Extract forcewake ack timeout Ben Widawsky
2012-09-02 5:59 ` [PATCH 2/4] drm/i915: use cpu_relax() in wait_for_atomic Ben Widawsky
@ 2012-09-02 5:59 ` Ben Widawsky
2012-09-02 5:59 ` [PATCH 4/4 v2] drm/i915: Never read FORCEWAKE Ben Widawsky
2012-09-03 8:42 ` [PATCH 1/4] drm/i915: Extract forcewake ack timeout Jani Nikula
3 siblings, 0 replies; 7+ messages in thread
From: Ben Widawsky @ 2012-09-02 5:59 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
A designer familiar with the hardware has stated that the forcewake
timeout can theoretically be as high as a little over 1ms. Therefore we
modify our code to use 2ms (appropriate fudge and because we don't want
to round down).
Hopefully this can't prevent spurious timeouts.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/intel_pm.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f42c142..2a8468d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -31,7 +31,7 @@
#include "../../../platform/x86/intel_ips.h"
#include <linux/module.h>
-#define FORCEWAKE_ACK_TIMEOUT_US 500
+#define FORCEWAKE_ACK_TIMEOUT_MS 2
/* FBC, or Frame Buffer Compression, is a technique employed to compress the
* framebuffer contents in-memory, aiming at reducing the required bandwidth
@@ -3970,15 +3970,15 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
else
forcewake_ack = FORCEWAKE_ACK;
- if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0,
- FORCEWAKE_ACK_TIMEOUT_US))
+ if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1) == 0,
+ FORCEWAKE_ACK_TIMEOUT_MS))
DRM_ERROR("Force wake wait timed out\n");
I915_WRITE_NOTRACE(FORCEWAKE, 1);
POSTING_READ(FORCEWAKE);
- if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1),
- FORCEWAKE_ACK_TIMEOUT_US))
+ if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1),
+ FORCEWAKE_ACK_TIMEOUT_MS))
DRM_ERROR("Force wake wait timed out\n");
__gen6_gt_wait_for_thread_c0(dev_priv);
@@ -3993,15 +3993,15 @@ static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
else
forcewake_ack = FORCEWAKE_MT_ACK;
- if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0,
- FORCEWAKE_ACK_TIMEOUT_US))
+ if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1) == 0,
+ FORCEWAKE_ACK_TIMEOUT_MS))
DRM_ERROR("Force wake wait timed out\n");
I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
POSTING_READ(FORCEWAKE_MT);
- if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1),
- FORCEWAKE_ACK_TIMEOUT_US))
+ if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1),
+ FORCEWAKE_ACK_TIMEOUT_MS))
DRM_ERROR("Force wake wait timed out\n");
__gen6_gt_wait_for_thread_c0(dev_priv);
@@ -4088,8 +4088,8 @@ static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffffffff);
POSTING_READ(FORCEWAKE_VLV);
- if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1),
- FORCEWAKE_ACK_TIMEOUT_US))
+ if (wait_for_atomic((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1),
+ FORCEWAKE_ACK_TIMEOUT_MS))
DRM_ERROR("Force wake wait timed out\n");
__gen6_gt_wait_for_thread_c0(dev_priv);
--
1.7.12
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/4 v2] drm/i915: Never read FORCEWAKE
2012-09-02 5:59 [PATCH 1/4] drm/i915: Extract forcewake ack timeout Ben Widawsky
2012-09-02 5:59 ` [PATCH 2/4] drm/i915: use cpu_relax() in wait_for_atomic Ben Widawsky
2012-09-02 5:59 ` [PATCH 3/4] drm/i915: Change forcewake timeout to 2ms Ben Widawsky
@ 2012-09-02 5:59 ` Ben Widawsky
2012-09-02 8:41 ` Chris Wilson
2012-09-03 8:42 ` [PATCH 1/4] drm/i915: Extract forcewake ack timeout Jani Nikula
3 siblings, 1 reply; 7+ messages in thread
From: Ben Widawsky @ 2012-09-02 5:59 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
The same designer from the previous patch has told us to never read
FORCEWAKE. We only do this for the POSTING_READ(), so simply change that
to something within the same cacheline (for no reason in particular
other than it sounds nice). In the _mt case we can leverage
the gtfifodbg check for the POSTING_READ.
This partially reverts
commit 6af2d180f82151cf3d58952e35a4f96e45bc453a
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu Jul 26 16:24:50 2012 +0200
drm/i915: fix forcewake related hangs on snb
v2: commit message, comments about posting read from (Daniel)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/intel_pm.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2a8468d..0d71ba7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3975,7 +3975,7 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
DRM_ERROR("Force wake wait timed out\n");
I915_WRITE_NOTRACE(FORCEWAKE, 1);
- POSTING_READ(FORCEWAKE);
+ POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */
if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1),
FORCEWAKE_ACK_TIMEOUT_MS))
@@ -3998,7 +3998,7 @@ static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
DRM_ERROR("Force wake wait timed out\n");
I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
- POSTING_READ(FORCEWAKE_MT);
+ POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */
if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1),
FORCEWAKE_ACK_TIMEOUT_MS))
@@ -4035,14 +4035,14 @@ void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
{
I915_WRITE_NOTRACE(FORCEWAKE, 0);
- POSTING_READ(FORCEWAKE);
+ /* gen6_gt_check_fifodbg doubles as the POSTING_READ */
gen6_gt_check_fifodbg(dev_priv);
}
static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
{
I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(1));
- POSTING_READ(FORCEWAKE_MT);
+ /* gen6_gt_check_fifodbg doubles as the POSTING_READ */
gen6_gt_check_fifodbg(dev_priv);
}
--
1.7.12
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 4/4 v2] drm/i915: Never read FORCEWAKE
2012-09-02 5:59 ` [PATCH 4/4 v2] drm/i915: Never read FORCEWAKE Ben Widawsky
@ 2012-09-02 8:41 ` Chris Wilson
2012-09-03 8:30 ` Daniel Vetter
0 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2012-09-02 8:41 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
On Sat, 1 Sep 2012 22:59:50 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> The same designer from the previous patch has told us to never read
> FORCEWAKE. We only do this for the POSTING_READ(), so simply change that
> to something within the same cacheline (for no reason in particular
> other than it sounds nice). In the _mt case we can leverage
> the gtfifodbg check for the POSTING_READ.
The first two are Reviewed-by: Chris Wilson <chris@chris-wilson.oc.uk>
The real meat of the change in the second two look good, so
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4 v2] drm/i915: Never read FORCEWAKE
2012-09-02 8:41 ` Chris Wilson
@ 2012-09-03 8:30 ` Daniel Vetter
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2012-09-03 8:30 UTC (permalink / raw)
To: Chris Wilson; +Cc: Ben Widawsky, intel-gfx
On Sun, Sep 02, 2012 at 09:41:44AM +0100, Chris Wilson wrote:
> On Sat, 1 Sep 2012 22:59:50 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > The same designer from the previous patch has told us to never read
> > FORCEWAKE. We only do this for the POSTING_READ(), so simply change that
> > to something within the same cacheline (for no reason in particular
> > other than it sounds nice). In the _mt case we can leverage
> > the gtfifodbg check for the POSTING_READ.
>
> The first two are Reviewed-by: Chris Wilson <chris@chris-wilson.oc.uk>
> The real meat of the change in the second two look good, so
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
All slurped into dinq (with some frobbing due to the vlv patches), thanks.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4] drm/i915: Extract forcewake ack timeout
2012-09-02 5:59 [PATCH 1/4] drm/i915: Extract forcewake ack timeout Ben Widawsky
` (2 preceding siblings ...)
2012-09-02 5:59 ` [PATCH 4/4 v2] drm/i915: Never read FORCEWAKE Ben Widawsky
@ 2012-09-03 8:42 ` Jani Nikula
3 siblings, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2012-09-03 8:42 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
On Sun, 02 Sep 2012, Ben Widawsky <ben@bwidawsk.net> wrote:
> It's used all over the place, and we want to be able to play around with
> the value, apparently. Note that it doesn't touch other timeouts of the
> same value (like gtfifo, and thread C0 wait).
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
On the series,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c0721ff..f42c142 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -31,6 +31,8 @@
> #include "../../../platform/x86/intel_ips.h"
> #include <linux/module.h>
>
> +#define FORCEWAKE_ACK_TIMEOUT_US 500
> +
> /* FBC, or Frame Buffer Compression, is a technique employed to compress the
> * framebuffer contents in-memory, aiming at reducing the required bandwidth
> * during in-memory transfers and, therefore, reduce the power packet.
> @@ -3968,13 +3970,15 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
> else
> forcewake_ack = FORCEWAKE_ACK;
>
> - if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
> + if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0,
> + FORCEWAKE_ACK_TIMEOUT_US))
> DRM_ERROR("Force wake wait timed out\n");
>
> I915_WRITE_NOTRACE(FORCEWAKE, 1);
> POSTING_READ(FORCEWAKE);
>
> - if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
> + if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1),
> + FORCEWAKE_ACK_TIMEOUT_US))
> DRM_ERROR("Force wake wait timed out\n");
>
> __gen6_gt_wait_for_thread_c0(dev_priv);
> @@ -3989,13 +3993,15 @@ static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
> else
> forcewake_ack = FORCEWAKE_MT_ACK;
>
> - if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0, 500))
> + if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1) == 0,
> + FORCEWAKE_ACK_TIMEOUT_US))
> DRM_ERROR("Force wake wait timed out\n");
>
> I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
> POSTING_READ(FORCEWAKE_MT);
>
> - if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1), 500))
> + if (wait_for_atomic_us((I915_READ_NOTRACE(forcewake_ack) & 1),
> + FORCEWAKE_ACK_TIMEOUT_US))
> DRM_ERROR("Force wake wait timed out\n");
>
> __gen6_gt_wait_for_thread_c0(dev_priv);
> @@ -4082,7 +4088,8 @@ static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
> I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffffffff);
> POSTING_READ(FORCEWAKE_VLV);
>
> - if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500))
> + if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1),
> + FORCEWAKE_ACK_TIMEOUT_US))
> DRM_ERROR("Force wake wait timed out\n");
>
> __gen6_gt_wait_for_thread_c0(dev_priv);
> --
> 1.7.12
>
> _______________________________________________
> 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