* [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
@ 2017-04-06 8:44 Mika Kuoppala
2017-04-06 8:44 ` [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo Mika Kuoppala
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Mika Kuoppala @ 2017-04-06 8:44 UTC (permalink / raw)
To: intel-gfx
From: Chris Wilson <chris@chris-wilson.co.uk>
Remove the per-mmio checking of the FIFO debug register into the common
conditional mmio debug handling. Based on patch from Chris Wilson.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_uncore.c | 74 ++++++++++++++-----------------------
1 file changed, 28 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 6d1ea26..a129a73 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -172,22 +172,6 @@ static void fw_domains_get_with_thread_status(struct drm_i915_private *dev_priv,
__gen6_gt_wait_for_thread_c0(dev_priv);
}
-static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
-{
- u32 gtfifodbg;
-
- gtfifodbg = __raw_i915_read32(dev_priv, GTFIFODBG);
- if (WARN(gtfifodbg, "GT wake FIFO error 0x%x\n", gtfifodbg))
- __raw_i915_write32(dev_priv, GTFIFODBG, gtfifodbg);
-}
-
-static void fw_domains_put_with_fifo(struct drm_i915_private *dev_priv,
- enum forcewake_domains fw_domains)
-{
- fw_domains_put(dev_priv, fw_domains);
- gen6_gt_check_fifodbg(dev_priv);
-}
-
static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
{
u32 count = __raw_i915_read32(dev_priv, GTFIFOCTL);
@@ -384,15 +368,33 @@ vlv_check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
}
static bool
+gen6_check_for_fifo_debug(struct drm_i915_private *dev_priv)
+{
+ u32 fifodbg;
+
+ fifodbg = __raw_i915_read32(dev_priv, GTFIFODBG);
+
+ if (WARN(fifodbg, "GTFIFODBG = 0x%08x\n", fifodbg))
+ __raw_i915_write32(dev_priv, GTFIFODBG, fifodbg);
+
+ return fifodbg;
+}
+
+static bool
check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
{
+ bool ret = false;
+
if (HAS_FPGA_DBG_UNCLAIMED(dev_priv))
- return fpga_check_for_unclaimed_mmio(dev_priv);
+ ret |= fpga_check_for_unclaimed_mmio(dev_priv);
if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
- return vlv_check_for_unclaimed_mmio(dev_priv);
+ ret |= vlv_check_for_unclaimed_mmio(dev_priv);
- return false;
+ if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv))
+ ret |= gen6_check_for_fifo_debug(dev_priv);
+
+ return ret;
}
static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
@@ -404,11 +406,6 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
if (check_for_unclaimed_mmio(dev_priv))
DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
- /* clear out old GT FIFO errors */
- if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv))
- __raw_i915_write32(dev_priv, GTFIFODBG,
- __raw_i915_read32(dev_priv, GTFIFODBG));
-
/* WaDisableShadowRegForCpd:chv */
if (IS_CHERRYVIEW(dev_priv)) {
__raw_i915_write32(dev_priv, GTFIFOCTL,
@@ -1047,15 +1044,10 @@ __gen2_write(32)
#define __gen6_write(x) \
static void \
gen6_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
- u32 __fifo_ret = 0; \
GEN6_WRITE_HEADER; \
- if (NEEDS_FORCE_WAKE(offset)) { \
- __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
- } \
+ if (NEEDS_FORCE_WAKE(offset)) \
+ __gen6_gt_wait_for_fifo(dev_priv); \
__raw_i915_write##x(dev_priv, reg, val); \
- if (unlikely(__fifo_ret)) { \
- gen6_gt_check_fifodbg(dev_priv); \
- } \
GEN6_WRITE_FOOTER; \
}
@@ -1190,11 +1182,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
FORCEWAKE_MEDIA_GEN9, FORCEWAKE_ACK_MEDIA_GEN9);
} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
- if (!IS_CHERRYVIEW(dev_priv))
- dev_priv->uncore.funcs.force_wake_put =
- fw_domains_put_with_fifo;
- else
- dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
+ dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
FORCEWAKE_VLV, FORCEWAKE_ACK_VLV);
fw_domain_init(dev_priv, FW_DOMAIN_ID_MEDIA,
@@ -1202,11 +1190,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
dev_priv->uncore.funcs.force_wake_get =
fw_domains_get_with_thread_status;
- if (IS_HASWELL(dev_priv))
- dev_priv->uncore.funcs.force_wake_put =
- fw_domains_put_with_fifo;
- else
- dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
+ dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
FORCEWAKE_MT, FORCEWAKE_ACK_HSW);
} else if (IS_IVYBRIDGE(dev_priv)) {
@@ -1223,8 +1207,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
*/
dev_priv->uncore.funcs.force_wake_get =
fw_domains_get_with_thread_status;
- dev_priv->uncore.funcs.force_wake_put =
- fw_domains_put_with_fifo;
+ dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
/* We need to init first for ECOBUS access and then
* determine later if we want to reinit, in case of MT access is
@@ -1242,7 +1225,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
spin_lock_irq(&dev_priv->uncore.lock);
fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_RENDER);
ecobus = __raw_i915_read32(dev_priv, ECOBUS);
- fw_domains_put_with_fifo(dev_priv, FORCEWAKE_RENDER);
+ fw_domains_put(dev_priv, FORCEWAKE_RENDER);
spin_unlock_irq(&dev_priv->uncore.lock);
if (!(ecobus & FORCEWAKE_MT_ENABLE)) {
@@ -1254,8 +1237,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
} else if (IS_GEN6(dev_priv)) {
dev_priv->uncore.funcs.force_wake_get =
fw_domains_get_with_thread_status;
- dev_priv->uncore.funcs.force_wake_put =
- fw_domains_put_with_fifo;
+ dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
FORCEWAKE, FORCEWAKE_ACK);
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo
2017-04-06 8:44 [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Mika Kuoppala
@ 2017-04-06 8:44 ` Mika Kuoppala
2017-04-06 9:12 ` Chris Wilson
2017-04-06 9:03 ` [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Chris Wilson
` (4 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Mika Kuoppala @ 2017-04-06 8:44 UTC (permalink / raw)
To: intel-gfx
Replace the handcrafter loop when checking for fifo slots
with atomic wait for. This brings this wait in line with
the other waits on register access. We also get a readable
timeout constraint, so make it to fail after 10ms.
Chris suggested that we should fail silently as the fifo debug
handler, now attached to unclaimed mmio handling, will take care of the
possible errors at later stage.
Note that the decision to wait was changed so that we avoid
allocating the first reserved entry. Nor do we reduce the count
if we fail the wait, removing the possiblity to wrap the
count if the hw fifo returned zero.
References: https://bugs.freedesktop.org/show_bug.cgi?id=100247
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_uncore.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index a129a73..df744a8 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -29,6 +29,7 @@
#include <linux/pm_runtime.h>
#define FORCEWAKE_ACK_TIMEOUT_MS 50
+#define GT_FIFO_TIMEOUT_MS 10
#define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32((dev_priv__), (reg__))
@@ -181,28 +182,27 @@ static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
{
- int ret = 0;
+ u32 n;
/* On VLV, FIFO will be shared by both SW and HW.
* So, we need to read the FREE_ENTRIES everytime */
if (IS_VALLEYVIEW(dev_priv))
- dev_priv->uncore.fifo_count = fifo_free_entries(dev_priv);
-
- if (dev_priv->uncore.fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
- int loop = 500;
- u32 fifo = fifo_free_entries(dev_priv);
-
- while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) {
- udelay(10);
- fifo = fifo_free_entries(dev_priv);
+ n = fifo_free_entries(dev_priv);
+ else
+ n = dev_priv->uncore.fifo_count;
+
+ if (n <= GT_FIFO_NUM_RESERVED_ENTRIES) {
+ if (wait_for_atomic((n = fifo_free_entries(dev_priv)) >
+ GT_FIFO_NUM_RESERVED_ENTRIES,
+ GT_FIFO_TIMEOUT_MS)) {
+ DRM_DEBUG("GT_FIFO timeout, entries: %u, unclaimed: %d\n", n,
+ intel_uncore_unclaimed_mmio(dev_priv));
+ return -EBUSY;
}
- if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
- ++ret;
- dev_priv->uncore.fifo_count = fifo;
}
- dev_priv->uncore.fifo_count--;
- return ret;
+ dev_priv->uncore.fifo_count = n - 1;
+ return 0;
}
static enum hrtimer_restart
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
2017-04-06 8:44 [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Mika Kuoppala
2017-04-06 8:44 ` [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo Mika Kuoppala
@ 2017-04-06 9:03 ` Chris Wilson
2017-04-06 9:14 ` Chris Wilson
2017-04-06 9:35 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-04-06 9:03 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
On Thu, Apr 06, 2017 at 11:44:18AM +0300, Mika Kuoppala wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
From: Mika Kuoppala <mika.kuoppala@intel.com>
> Remove the per-mmio checking of the FIFO debug register into the common
> conditional mmio debug handling. Based on patch from Chris Wilson.
>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Or, keep me as the author, add your
r-b
[mkuoppal: keep a distinct error message for fifodbg]
s-o-b.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo
2017-04-06 8:44 ` [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo Mika Kuoppala
@ 2017-04-06 9:12 ` Chris Wilson
2017-04-06 9:32 ` Mika Kuoppala
2017-04-06 15:40 ` Mika Kuoppala
0 siblings, 2 replies; 20+ messages in thread
From: Chris Wilson @ 2017-04-06 9:12 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
On Thu, Apr 06, 2017 at 11:44:19AM +0300, Mika Kuoppala wrote:
> Replace the handcrafter loop when checking for fifo slots
> with atomic wait for. This brings this wait in line with
> the other waits on register access. We also get a readable
> timeout constraint, so make it to fail after 10ms.
>
> Chris suggested that we should fail silently as the fifo debug
> handler, now attached to unclaimed mmio handling, will take care of the
> possible errors at later stage.
>
> Note that the decision to wait was changed so that we avoid
> allocating the first reserved entry. Nor do we reduce the count
> if we fail the wait, removing the possiblity to wrap the
> count if the hw fifo returned zero.
Otoh, we don't abort the write so the slot is still taken. Nor does it
update the last known fifo_count along that path.
However, we leave it set to a value that will cause us to reread the
counter on the next call, so it comes out in the wash.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=100247
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index a129a73..df744a8 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -29,6 +29,7 @@
> #include <linux/pm_runtime.h>
>
> #define FORCEWAKE_ACK_TIMEOUT_MS 50
> +#define GT_FIFO_TIMEOUT_MS 10
>
> #define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32((dev_priv__), (reg__))
>
> @@ -181,28 +182,27 @@ static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
>
> static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
> {
> - int ret = 0;
> + u32 n;
>
> /* On VLV, FIFO will be shared by both SW and HW.
> * So, we need to read the FREE_ENTRIES everytime */
> if (IS_VALLEYVIEW(dev_priv))
> - dev_priv->uncore.fifo_count = fifo_free_entries(dev_priv);
> -
> - if (dev_priv->uncore.fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
> - int loop = 500;
> - u32 fifo = fifo_free_entries(dev_priv);
> -
> - while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) {
> - udelay(10);
> - fifo = fifo_free_entries(dev_priv);
> + n = fifo_free_entries(dev_priv);
> + else
> + n = dev_priv->uncore.fifo_count;
> +
> + if (n <= GT_FIFO_NUM_RESERVED_ENTRIES) {
> + if (wait_for_atomic((n = fifo_free_entries(dev_priv)) >
> + GT_FIFO_NUM_RESERVED_ENTRIES,
> + GT_FIFO_TIMEOUT_MS)) {
> + DRM_DEBUG("GT_FIFO timeout, entries: %u, unclaimed: %d\n", n,
> + intel_uncore_unclaimed_mmio(dev_priv));
What's the connection here between unclaimed mmio and the fifo count?
i.e. what information do we glean? Espcially as this information is part
of the generic mmio framework already.
> + return -EBUSY;
> }
> - if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
> - ++ret;
> - dev_priv->uncore.fifo_count = fifo;
> }
> - dev_priv->uncore.fifo_count--;
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
2017-04-06 9:03 ` [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Chris Wilson
@ 2017-04-06 9:14 ` Chris Wilson
2017-04-06 15:39 ` Mika Kuoppala
0 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-04-06 9:14 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
On Thu, Apr 06, 2017 at 10:03:23AM +0100, Chris Wilson wrote:
> On Thu, Apr 06, 2017 at 11:44:18AM +0300, Mika Kuoppala wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> From: Mika Kuoppala <mika.kuoppala@intel.com>
>
> > Remove the per-mmio checking of the FIFO debug register into the common
> > conditional mmio debug handling. Based on patch from Chris Wilson.
> >
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> Or, keep me as the author, add your
> r-b
> [mkuoppal: keep a distinct error message for fifodbg]
> s-o-b.
Actually you fixed check_for_unclaimed_mmio() for me, which is more or
less the heart of the patch, so please do assign yourself authorship
with my r-b.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo
2017-04-06 9:12 ` Chris Wilson
@ 2017-04-06 9:32 ` Mika Kuoppala
2017-04-06 15:40 ` Mika Kuoppala
1 sibling, 0 replies; 20+ messages in thread
From: Mika Kuoppala @ 2017-04-06 9:32 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> On Thu, Apr 06, 2017 at 11:44:19AM +0300, Mika Kuoppala wrote:
>> Replace the handcrafter loop when checking for fifo slots
>> with atomic wait for. This brings this wait in line with
>> the other waits on register access. We also get a readable
>> timeout constraint, so make it to fail after 10ms.
>>
>> Chris suggested that we should fail silently as the fifo debug
>> handler, now attached to unclaimed mmio handling, will take care of the
>> possible errors at later stage.
>>
>> Note that the decision to wait was changed so that we avoid
>> allocating the first reserved entry. Nor do we reduce the count
>> if we fail the wait, removing the possiblity to wrap the
>> count if the hw fifo returned zero.
>
> Otoh, we don't abort the write so the slot is still taken. Nor does it
> update the last known fifo_count along that path.
>
> However, we leave it set to a value that will cause us to reread the
> counter on the next call, so it comes out in the wash.
>
>>
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=100247
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_uncore.c | 30 +++++++++++++++---------------
>> 1 file changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index a129a73..df744a8 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -29,6 +29,7 @@
>> #include <linux/pm_runtime.h>
>>
>> #define FORCEWAKE_ACK_TIMEOUT_MS 50
>> +#define GT_FIFO_TIMEOUT_MS 10
>>
>> #define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32((dev_priv__), (reg__))
>>
>> @@ -181,28 +182,27 @@ static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
>>
>> static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
>> {
>> - int ret = 0;
>> + u32 n;
>>
>> /* On VLV, FIFO will be shared by both SW and HW.
>> * So, we need to read the FREE_ENTRIES everytime */
>> if (IS_VALLEYVIEW(dev_priv))
>> - dev_priv->uncore.fifo_count = fifo_free_entries(dev_priv);
>> -
>> - if (dev_priv->uncore.fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
>> - int loop = 500;
>> - u32 fifo = fifo_free_entries(dev_priv);
>> -
>> - while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) {
>> - udelay(10);
>> - fifo = fifo_free_entries(dev_priv);
>> + n = fifo_free_entries(dev_priv);
>> + else
>> + n = dev_priv->uncore.fifo_count;
>> +
>> + if (n <= GT_FIFO_NUM_RESERVED_ENTRIES) {
>> + if (wait_for_atomic((n = fifo_free_entries(dev_priv)) >
>> + GT_FIFO_NUM_RESERVED_ENTRIES,
>> + GT_FIFO_TIMEOUT_MS)) {
>> + DRM_DEBUG("GT_FIFO timeout, entries: %u, unclaimed: %d\n", n,
>> + intel_uncore_unclaimed_mmio(dev_priv));
>
> What's the connection here between unclaimed mmio and the fifo count?
> i.e. what information do we glean? Espcially as this information is part
> of the generic mmio framework already.
>
To trigger fifodbg check and clean. And to get a trace from this exact spot.
So this battles against the commit message to leave the warn into the
unclaimed handling. I will remove it.
-Mika
>> + return -EBUSY;
>> }
>> - if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
>> - ++ret;
>> - dev_priv->uncore.fifo_count = fifo;
>> }
>> - dev_priv->uncore.fifo_count--;
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
2017-04-06 8:44 [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Mika Kuoppala
2017-04-06 8:44 ` [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo Mika Kuoppala
2017-04-06 9:03 ` [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Chris Wilson
@ 2017-04-06 9:35 ` Patchwork
2017-04-06 11:13 ` Mika Kuoppala
2017-04-06 16:04 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev2) Patchwork
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Patchwork @ 2017-04-06 9:35 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
URL : https://patchwork.freedesktop.org/series/22571/
State : warning
== Summary ==
Series 22571v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/22571/revisions/1/mbox/
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> DMESG-WARN (fi-byt-j1900)
Subgroup basic-s4-devices:
dmesg-warn -> PASS (fi-kbl-7560u) fdo#100125
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
pass -> DMESG-WARN (fi-byt-j1900) fdo#100126
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-WARN (fi-byt-j1900)
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100126 https://bugs.freedesktop.org/show_bug.cgi?id=100126
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 428s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 425s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time: 581s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 508s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 565s
fi-byt-j1900 total:278 pass:251 dwarn:3 dfail:0 fail:0 skip:24 time: 484s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 413s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 402s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 418s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 478s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 463s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 458s
fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 568s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 451s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 575s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 465s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 500s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 433s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 530s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 401s
7303b362817aa4b4c62cdc2dfa586c272982917d drm-tip: 2017y-04m-06d-08h-26m-30s UTC integration manifest
b8cf5ad drm/i915: Use wait_for_atomic_us when waiting for gt fifo
49b367b drm/i915: Move the GTFIFODBG to the common mmio dbg framework
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4417/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
2017-04-06 9:35 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
@ 2017-04-06 11:13 ` Mika Kuoppala
2017-04-06 11:32 ` Chris Wilson
0 siblings, 1 reply; 20+ messages in thread
From: Mika Kuoppala @ 2017-04-06 11:13 UTC (permalink / raw)
To: Patchwork; +Cc: intel-gfx
Patchwork <patchwork@emeril.freedesktop.org> writes:
> == Series Details ==
>
> Series: series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
> URL : https://patchwork.freedesktop.org/series/22571/
> State : warning
>
> == Summary ==
>
> Series 22571v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/22571/revisions/1/mbox/
>
> Test gem_exec_suspend:
> Subgroup basic-s3:
> pass -> DMESG-WARN (fi-byt-j1900)
GT_FIFO_SB_READ_ABORTERR set on resume. We have squashed this
previously.
-Mika
> Subgroup basic-s4-devices:
> dmesg-warn -> PASS (fi-kbl-7560u) fdo#100125
> Test kms_pipe_crc_basic:
> Subgroup suspend-read-crc-pipe-a:
> pass -> DMESG-WARN (fi-byt-j1900) fdo#100126
> Subgroup suspend-read-crc-pipe-b:
> pass -> DMESG-WARN (fi-byt-j1900)
>
> fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
> fdo#100126 https://bugs.freedesktop.org/show_bug.cgi?id=100126
>
> fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 428s
> fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 425s
> fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time: 581s
> fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 508s
> fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 565s
> fi-byt-j1900 total:278 pass:251 dwarn:3 dfail:0 fail:0 skip:24 time: 484s
> fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 413s
> fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 402s
> fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 418s
> fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 478s
> fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 463s
> fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 458s
> fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 568s
> fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 451s
> fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 575s
> fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 465s
> fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 500s
> fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 433s
> fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 530s
> fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 401s
>
> 7303b362817aa4b4c62cdc2dfa586c272982917d drm-tip: 2017y-04m-06d-08h-26m-30s UTC integration manifest
> b8cf5ad drm/i915: Use wait_for_atomic_us when waiting for gt fifo
> 49b367b drm/i915: Move the GTFIFODBG to the common mmio dbg framework
>
> == Logs ==
>
> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4417/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
2017-04-06 11:13 ` Mika Kuoppala
@ 2017-04-06 11:32 ` Chris Wilson
0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-04-06 11:32 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
On Thu, Apr 06, 2017 at 02:13:44PM +0300, Mika Kuoppala wrote:
> Patchwork <patchwork@emeril.freedesktop.org> writes:
>
> > == Series Details ==
> >
> > Series: series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
> > URL : https://patchwork.freedesktop.org/series/22571/
> > State : warning
> >
> > == Summary ==
> >
> > Series 22571v1 Series without cover letter
> > https://patchwork.freedesktop.org/api/1.0/series/22571/revisions/1/mbox/
> >
> > Test gem_exec_suspend:
> > Subgroup basic-s3:
> > pass -> DMESG-WARN (fi-byt-j1900)
>
> GT_FIFO_SB_READ_ABORTERR set on resume. We have squashed this
> previously.
Ah, but the warn is now coming from the code to clear BIOS errors before
we begin.
It's reminding us that we can't use a WARN inside check_fifodbg. We have
the WARN already in __unclaimed_reg_debug, just make it a DRM_DEBUG
telltale.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
2017-04-06 9:14 ` Chris Wilson
@ 2017-04-06 15:39 ` Mika Kuoppala
2017-04-06 15:46 ` Ville Syrjälä
0 siblings, 1 reply; 20+ messages in thread
From: Mika Kuoppala @ 2017-04-06 15:39 UTC (permalink / raw)
To: intel-gfx
Remove the per-mmio checking of the FIFO debug register into the common
conditional mmio debug handling. Based on patch from Chris Wilson.
v2: postpone warn on fifodbg for unclaimed reg debugs
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_uncore.c | 76 +++++++++++++++----------------------
1 file changed, 30 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 6d1ea26..7a8eb2e 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -172,22 +172,6 @@ static void fw_domains_get_with_thread_status(struct drm_i915_private *dev_priv,
__gen6_gt_wait_for_thread_c0(dev_priv);
}
-static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
-{
- u32 gtfifodbg;
-
- gtfifodbg = __raw_i915_read32(dev_priv, GTFIFODBG);
- if (WARN(gtfifodbg, "GT wake FIFO error 0x%x\n", gtfifodbg))
- __raw_i915_write32(dev_priv, GTFIFODBG, gtfifodbg);
-}
-
-static void fw_domains_put_with_fifo(struct drm_i915_private *dev_priv,
- enum forcewake_domains fw_domains)
-{
- fw_domains_put(dev_priv, fw_domains);
- gen6_gt_check_fifodbg(dev_priv);
-}
-
static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
{
u32 count = __raw_i915_read32(dev_priv, GTFIFOCTL);
@@ -384,15 +368,35 @@ vlv_check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
}
static bool
+gen6_check_for_fifo_debug(struct drm_i915_private *dev_priv)
+{
+ u32 fifodbg;
+
+ fifodbg = __raw_i915_read32(dev_priv, GTFIFODBG);
+
+ if (unlikely(fifodbg)) {
+ DRM_DEBUG_DRIVER("GTFIFODBG = 0x08%x\n", fifodbg);
+ __raw_i915_write32(dev_priv, GTFIFODBG, fifodbg);
+ }
+
+ return fifodbg;
+}
+
+static bool
check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
{
+ bool ret = false;
+
if (HAS_FPGA_DBG_UNCLAIMED(dev_priv))
- return fpga_check_for_unclaimed_mmio(dev_priv);
+ ret |= fpga_check_for_unclaimed_mmio(dev_priv);
if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
- return vlv_check_for_unclaimed_mmio(dev_priv);
+ ret |= vlv_check_for_unclaimed_mmio(dev_priv);
- return false;
+ if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv))
+ ret |= gen6_check_for_fifo_debug(dev_priv);
+
+ return ret;
}
static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
@@ -404,11 +408,6 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
if (check_for_unclaimed_mmio(dev_priv))
DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
- /* clear out old GT FIFO errors */
- if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv))
- __raw_i915_write32(dev_priv, GTFIFODBG,
- __raw_i915_read32(dev_priv, GTFIFODBG));
-
/* WaDisableShadowRegForCpd:chv */
if (IS_CHERRYVIEW(dev_priv)) {
__raw_i915_write32(dev_priv, GTFIFOCTL,
@@ -1047,15 +1046,10 @@ __gen2_write(32)
#define __gen6_write(x) \
static void \
gen6_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
- u32 __fifo_ret = 0; \
GEN6_WRITE_HEADER; \
- if (NEEDS_FORCE_WAKE(offset)) { \
- __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
- } \
+ if (NEEDS_FORCE_WAKE(offset)) \
+ __gen6_gt_wait_for_fifo(dev_priv); \
__raw_i915_write##x(dev_priv, reg, val); \
- if (unlikely(__fifo_ret)) { \
- gen6_gt_check_fifodbg(dev_priv); \
- } \
GEN6_WRITE_FOOTER; \
}
@@ -1190,11 +1184,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
FORCEWAKE_MEDIA_GEN9, FORCEWAKE_ACK_MEDIA_GEN9);
} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
- if (!IS_CHERRYVIEW(dev_priv))
- dev_priv->uncore.funcs.force_wake_put =
- fw_domains_put_with_fifo;
- else
- dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
+ dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
FORCEWAKE_VLV, FORCEWAKE_ACK_VLV);
fw_domain_init(dev_priv, FW_DOMAIN_ID_MEDIA,
@@ -1202,11 +1192,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
dev_priv->uncore.funcs.force_wake_get =
fw_domains_get_with_thread_status;
- if (IS_HASWELL(dev_priv))
- dev_priv->uncore.funcs.force_wake_put =
- fw_domains_put_with_fifo;
- else
- dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
+ dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
FORCEWAKE_MT, FORCEWAKE_ACK_HSW);
} else if (IS_IVYBRIDGE(dev_priv)) {
@@ -1223,8 +1209,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
*/
dev_priv->uncore.funcs.force_wake_get =
fw_domains_get_with_thread_status;
- dev_priv->uncore.funcs.force_wake_put =
- fw_domains_put_with_fifo;
+ dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
/* We need to init first for ECOBUS access and then
* determine later if we want to reinit, in case of MT access is
@@ -1242,7 +1227,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
spin_lock_irq(&dev_priv->uncore.lock);
fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_RENDER);
ecobus = __raw_i915_read32(dev_priv, ECOBUS);
- fw_domains_put_with_fifo(dev_priv, FORCEWAKE_RENDER);
+ fw_domains_put(dev_priv, FORCEWAKE_RENDER);
spin_unlock_irq(&dev_priv->uncore.lock);
if (!(ecobus & FORCEWAKE_MT_ENABLE)) {
@@ -1254,8 +1239,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
} else if (IS_GEN6(dev_priv)) {
dev_priv->uncore.funcs.force_wake_get =
fw_domains_get_with_thread_status;
- dev_priv->uncore.funcs.force_wake_put =
- fw_domains_put_with_fifo;
+ dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
FORCEWAKE, FORCEWAKE_ACK);
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo
2017-04-06 9:12 ` Chris Wilson
2017-04-06 9:32 ` Mika Kuoppala
@ 2017-04-06 15:40 ` Mika Kuoppala
2017-04-06 15:46 ` Chris Wilson
2017-05-02 14:03 ` Mika Kuoppala
1 sibling, 2 replies; 20+ messages in thread
From: Mika Kuoppala @ 2017-04-06 15:40 UTC (permalink / raw)
To: intel-gfx
Replace the handcrafter loop when checking for fifo slots
with atomic wait for. This brings this wait in line with
the other waits on register access. We also get a readable
timeout constraint, so make it to fail after 10ms.
Chris suggested that we should fail silently as the fifo debug
handler, now attached to unclaimed mmio handling, will take care of the
possible errors at later stage.
Note that the decision to wait was changed so that we avoid
allocating the first reserved entry. Nor do we reduce the count
if we fail the wait, removing the possiblity to wrap the
count if the hw fifo returned zero.
v2: remove unclaimed check on timeout (Chris)
References: https://bugs.freedesktop.org/show_bug.cgi?id=100247
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_uncore.c | 29 ++++++++++++++---------------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 7a8eb2e..38ba97f 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -29,6 +29,7 @@
#include <linux/pm_runtime.h>
#define FORCEWAKE_ACK_TIMEOUT_MS 50
+#define GT_FIFO_TIMEOUT_MS 10
#define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32((dev_priv__), (reg__))
@@ -181,28 +182,26 @@ static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
{
- int ret = 0;
+ u32 n;
/* On VLV, FIFO will be shared by both SW and HW.
* So, we need to read the FREE_ENTRIES everytime */
if (IS_VALLEYVIEW(dev_priv))
- dev_priv->uncore.fifo_count = fifo_free_entries(dev_priv);
-
- if (dev_priv->uncore.fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
- int loop = 500;
- u32 fifo = fifo_free_entries(dev_priv);
-
- while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) {
- udelay(10);
- fifo = fifo_free_entries(dev_priv);
+ n = fifo_free_entries(dev_priv);
+ else
+ n = dev_priv->uncore.fifo_count;
+
+ if (n <= GT_FIFO_NUM_RESERVED_ENTRIES) {
+ if (wait_for_atomic((n = fifo_free_entries(dev_priv)) >
+ GT_FIFO_NUM_RESERVED_ENTRIES,
+ GT_FIFO_TIMEOUT_MS)) {
+ DRM_DEBUG("GT_FIFO timeout, entries: %u\n", n);
+ return -EBUSY;
}
- if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
- ++ret;
- dev_priv->uncore.fifo_count = fifo;
}
- dev_priv->uncore.fifo_count--;
- return ret;
+ dev_priv->uncore.fifo_count = n - 1;
+ return 0;
}
static enum hrtimer_restart
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo
2017-04-06 15:40 ` Mika Kuoppala
@ 2017-04-06 15:46 ` Chris Wilson
2017-05-03 9:54 ` Mika Kuoppala
2017-05-02 14:03 ` Mika Kuoppala
1 sibling, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-04-06 15:46 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
On Thu, Apr 06, 2017 at 06:40:16PM +0300, Mika Kuoppala wrote:
> Replace the handcrafter loop when checking for fifo slots
> with atomic wait for. This brings this wait in line with
> the other waits on register access. We also get a readable
> timeout constraint, so make it to fail after 10ms.
>
> Chris suggested that we should fail silently as the fifo debug
> handler, now attached to unclaimed mmio handling, will take care of the
> possible errors at later stage.
>
> Note that the decision to wait was changed so that we avoid
> allocating the first reserved entry. Nor do we reduce the count
> if we fail the wait, removing the possiblity to wrap the
> count if the hw fifo returned zero.
>
> v2: remove unclaimed check on timeout (Chris)
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=100247
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 29 ++++++++++++++---------------
> 1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 7a8eb2e..38ba97f 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -29,6 +29,7 @@
> #include <linux/pm_runtime.h>
>
> #define FORCEWAKE_ACK_TIMEOUT_MS 50
> +#define GT_FIFO_TIMEOUT_MS 10
>
> #define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32((dev_priv__), (reg__))
>
> @@ -181,28 +182,26 @@ static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
>
> static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
As this is patch 2/2, we could drop the int return here, and just make
this a void function now.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
2017-04-06 15:39 ` Mika Kuoppala
@ 2017-04-06 15:46 ` Ville Syrjälä
2017-04-06 16:05 ` Chris Wilson
0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2017-04-06 15:46 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
On Thu, Apr 06, 2017 at 06:39:42PM +0300, Mika Kuoppala wrote:
> Remove the per-mmio checking of the FIFO debug register into the common
> conditional mmio debug handling. Based on patch from Chris Wilson.
>
> v2: postpone warn on fifodbg for unclaimed reg debugs
>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 76 +++++++++++++++----------------------
> 1 file changed, 30 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 6d1ea26..7a8eb2e 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -172,22 +172,6 @@ static void fw_domains_get_with_thread_status(struct drm_i915_private *dev_priv,
> __gen6_gt_wait_for_thread_c0(dev_priv);
> }
>
> -static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
> -{
> - u32 gtfifodbg;
> -
> - gtfifodbg = __raw_i915_read32(dev_priv, GTFIFODBG);
> - if (WARN(gtfifodbg, "GT wake FIFO error 0x%x\n", gtfifodbg))
> - __raw_i915_write32(dev_priv, GTFIFODBG, gtfifodbg);
> -}
> -
> -static void fw_domains_put_with_fifo(struct drm_i915_private *dev_priv,
> - enum forcewake_domains fw_domains)
> -{
> - fw_domains_put(dev_priv, fw_domains);
> - gen6_gt_check_fifodbg(dev_priv);
> -}
> -
> static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
> {
> u32 count = __raw_i915_read32(dev_priv, GTFIFOCTL);
> @@ -384,15 +368,35 @@ vlv_check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> }
>
> static bool
> +gen6_check_for_fifo_debug(struct drm_i915_private *dev_priv)
> +{
> + u32 fifodbg;
> +
> + fifodbg = __raw_i915_read32(dev_priv, GTFIFODBG);
> +
> + if (unlikely(fifodbg)) {
> + DRM_DEBUG_DRIVER("GTFIFODBG = 0x08%x\n", fifodbg);
> + __raw_i915_write32(dev_priv, GTFIFODBG, fifodbg);
> + }
> +
> + return fifodbg;
> +}
> +
> +static bool
> check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> {
> + bool ret = false;
> +
> if (HAS_FPGA_DBG_UNCLAIMED(dev_priv))
> - return fpga_check_for_unclaimed_mmio(dev_priv);
> + ret |= fpga_check_for_unclaimed_mmio(dev_priv);
>
> if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> - return vlv_check_for_unclaimed_mmio(dev_priv);
> + ret |= vlv_check_for_unclaimed_mmio(dev_priv);
>
> - return false;
> + if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv))
> + ret |= gen6_check_for_fifo_debug(dev_priv);
I'd prefer to keep unclaim vs. wake FIFO separate because the
unclaimned stuff is only for display registers. In my plan to
split the uncore lock into gt and display locks the unclaimed
reg stuff would end up being protected by the display lock rather
than the gt lock.
> +
> + return ret;
> }
>
> static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
> @@ -404,11 +408,6 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
> if (check_for_unclaimed_mmio(dev_priv))
> DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
>
> - /* clear out old GT FIFO errors */
> - if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv))
> - __raw_i915_write32(dev_priv, GTFIFODBG,
> - __raw_i915_read32(dev_priv, GTFIFODBG));
> -
> /* WaDisableShadowRegForCpd:chv */
> if (IS_CHERRYVIEW(dev_priv)) {
> __raw_i915_write32(dev_priv, GTFIFOCTL,
> @@ -1047,15 +1046,10 @@ __gen2_write(32)
> #define __gen6_write(x) \
> static void \
> gen6_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
> - u32 __fifo_ret = 0; \
> GEN6_WRITE_HEADER; \
> - if (NEEDS_FORCE_WAKE(offset)) { \
> - __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
> - } \
> + if (NEEDS_FORCE_WAKE(offset)) \
> + __gen6_gt_wait_for_fifo(dev_priv); \
> __raw_i915_write##x(dev_priv, reg, val); \
> - if (unlikely(__fifo_ret)) { \
> - gen6_gt_check_fifodbg(dev_priv); \
> - } \
> GEN6_WRITE_FOOTER; \
> }
>
> @@ -1190,11 +1184,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
> FORCEWAKE_MEDIA_GEN9, FORCEWAKE_ACK_MEDIA_GEN9);
> } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
> - if (!IS_CHERRYVIEW(dev_priv))
> - dev_priv->uncore.funcs.force_wake_put =
> - fw_domains_put_with_fifo;
> - else
> - dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
> + dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
> fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
> FORCEWAKE_VLV, FORCEWAKE_ACK_VLV);
> fw_domain_init(dev_priv, FW_DOMAIN_ID_MEDIA,
> @@ -1202,11 +1192,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> dev_priv->uncore.funcs.force_wake_get =
> fw_domains_get_with_thread_status;
> - if (IS_HASWELL(dev_priv))
> - dev_priv->uncore.funcs.force_wake_put =
> - fw_domains_put_with_fifo;
> - else
> - dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
> + dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
> fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
> FORCEWAKE_MT, FORCEWAKE_ACK_HSW);
> } else if (IS_IVYBRIDGE(dev_priv)) {
> @@ -1223,8 +1209,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
> */
> dev_priv->uncore.funcs.force_wake_get =
> fw_domains_get_with_thread_status;
> - dev_priv->uncore.funcs.force_wake_put =
> - fw_domains_put_with_fifo;
> + dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
>
> /* We need to init first for ECOBUS access and then
> * determine later if we want to reinit, in case of MT access is
> @@ -1242,7 +1227,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
> spin_lock_irq(&dev_priv->uncore.lock);
> fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_RENDER);
> ecobus = __raw_i915_read32(dev_priv, ECOBUS);
> - fw_domains_put_with_fifo(dev_priv, FORCEWAKE_RENDER);
> + fw_domains_put(dev_priv, FORCEWAKE_RENDER);
> spin_unlock_irq(&dev_priv->uncore.lock);
>
> if (!(ecobus & FORCEWAKE_MT_ENABLE)) {
> @@ -1254,8 +1239,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
> } else if (IS_GEN6(dev_priv)) {
> dev_priv->uncore.funcs.force_wake_get =
> fw_domains_get_with_thread_status;
> - dev_priv->uncore.funcs.force_wake_put =
> - fw_domains_put_with_fifo;
> + dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
> fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
> FORCEWAKE, FORCEWAKE_ACK);
> }
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev2)
2017-04-06 8:44 [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Mika Kuoppala
` (2 preceding siblings ...)
2017-04-06 9:35 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
@ 2017-04-06 16:04 ` Patchwork
2017-04-06 16:24 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev3) Patchwork
2017-05-02 14:27 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev4) Patchwork
5 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-04-06 16:04 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev2)
URL : https://patchwork.freedesktop.org/series/22571/
State : success
== Summary ==
Series 22571v2 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/22571/revisions/2/mbox/
Test gem_exec_suspend:
Subgroup basic-s4-devices:
dmesg-warn -> PASS (fi-kbl-7560u) fdo#100125
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass -> INCOMPLETE (fi-byt-n2820) fdo#99740
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#99740 https://bugs.freedesktop.org/show_bug.cgi?id=99740
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 430s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 430s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time: 574s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 513s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 567s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time: 491s
fi-byt-n2820 total:241 pass:213 dwarn:0 dfail:0 fail:0 skip:27 time: 0s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 403s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 406s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 420s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 494s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 465s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 454s
fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 569s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 453s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 581s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 464s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 489s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 432s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 537s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 408s
e087f8395ca39c6988de8680bd6f80a20b08c0f4 drm-tip: 2017y-04m-06d-13h-28m-42s UTC integration manifest
ba22ef6 drm/i915: Use wait_for_atomic_us when waiting for gt fifo
e1554b7 drm/i915: Move the GTFIFODBG to the common mmio dbg framework
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4421/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
2017-04-06 15:46 ` Ville Syrjälä
@ 2017-04-06 16:05 ` Chris Wilson
2017-04-06 16:25 ` Ville Syrjälä
0 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-04-06 16:05 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Thu, Apr 06, 2017 at 06:46:29PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 06, 2017 at 06:39:42PM +0300, Mika Kuoppala wrote:
> > +static bool
> > check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> > {
> > + bool ret = false;
> > +
> > if (HAS_FPGA_DBG_UNCLAIMED(dev_priv))
> > - return fpga_check_for_unclaimed_mmio(dev_priv);
> > + ret |= fpga_check_for_unclaimed_mmio(dev_priv);
> >
> > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > - return vlv_check_for_unclaimed_mmio(dev_priv);
> > + ret |= vlv_check_for_unclaimed_mmio(dev_priv);
> >
> > - return false;
> > + if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv))
> > + ret |= gen6_check_for_fifo_debug(dev_priv);
>
> I'd prefer to keep unclaim vs. wake FIFO separate because the
> unclaimned stuff is only for display registers. In my plan to
> split the uncore lock into gt and display locks the unclaimed
> reg stuff would end up being protected by the display lock rather
> than the gt lock.
I don't think it is much of a hindrance, right? We just split it out when
splitting dpy vs gt. Mika was digging through GTFIFODBG and thought it
had some bits for a sideband underflow...
Random thought, would i915->mmio.writeX[reg < 0x40000](i915, reg, val)
or just push all the decisions to the backend? I hope gcc would be able
to automatically store the function pointer i915->mmio.writeX[reg < 0x40000]
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev3)
2017-04-06 8:44 [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Mika Kuoppala
` (3 preceding siblings ...)
2017-04-06 16:04 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev2) Patchwork
@ 2017-04-06 16:24 ` Patchwork
2017-05-02 14:27 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev4) Patchwork
5 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-04-06 16:24 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev3)
URL : https://patchwork.freedesktop.org/series/22571/
State : failure
== Summary ==
Series 22571v3 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/22571/revisions/3/mbox/
Test gem_exec_fence:
Subgroup nb-await-default:
pass -> INCOMPLETE (fi-hsw-4770)
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-bxt-t5700) fdo#100125
dmesg-warn -> PASS (fi-kbl-7560u) fdo#100125
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 437s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 424s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time: 570s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 508s
fi-bxt-t5700 total:278 pass:257 dwarn:1 dfail:0 fail:0 skip:20 time: 547s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time: 481s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 485s
fi-hsw-4770 total:49 pass:42 dwarn:0 dfail:0 fail:0 skip:6 time: 0s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 444s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 423s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 492s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 474s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 457s
fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 565s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 450s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 576s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 465s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 492s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 433s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 533s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 404s
e087f8395ca39c6988de8680bd6f80a20b08c0f4 drm-tip: 2017y-04m-06d-13h-28m-42s UTC integration manifest
5e88068 drm/i915: Use wait_for_atomic_us when waiting for gt fifo
f026393 drm/i915: Move the GTFIFODBG to the common mmio dbg framework
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4422/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
2017-04-06 16:05 ` Chris Wilson
@ 2017-04-06 16:25 ` Ville Syrjälä
0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-04-06 16:25 UTC (permalink / raw)
To: Chris Wilson, Mika Kuoppala, intel-gfx
On Thu, Apr 06, 2017 at 05:05:10PM +0100, Chris Wilson wrote:
> On Thu, Apr 06, 2017 at 06:46:29PM +0300, Ville Syrjälä wrote:
> > On Thu, Apr 06, 2017 at 06:39:42PM +0300, Mika Kuoppala wrote:
> > > +static bool
> > > check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
> > > {
> > > + bool ret = false;
> > > +
> > > if (HAS_FPGA_DBG_UNCLAIMED(dev_priv))
> > > - return fpga_check_for_unclaimed_mmio(dev_priv);
> > > + ret |= fpga_check_for_unclaimed_mmio(dev_priv);
> > >
> > > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > > - return vlv_check_for_unclaimed_mmio(dev_priv);
> > > + ret |= vlv_check_for_unclaimed_mmio(dev_priv);
> > >
> > > - return false;
> > > + if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv))
> > > + ret |= gen6_check_for_fifo_debug(dev_priv);
> >
> > I'd prefer to keep unclaim vs. wake FIFO separate because the
> > unclaimned stuff is only for display registers. In my plan to
> > split the uncore lock into gt and display locks the unclaimed
> > reg stuff would end up being protected by the display lock rather
> > than the gt lock.
>
> I don't think it is much of a hindrance, right? We just split it out when
> splitting dpy vs gt.
I suppose. Although if we want to do the FIFO checks for non-GT stuff as
well, then I guess we'd have to risk hitting the FIFO register from
both the display and gt code paths. Not sure if that's safe or not.
> Mika was digging through GTFIFODBG and thought it
> had some bits for a sideband underflow...
>
> Random thought, would i915->mmio.writeX[reg < 0x40000](i915, reg, val)
> or just push all the decisions to the backend? I hope gcc would be able
> to automatically store the function pointer i915->mmio.writeX[reg < 0x40000]
I haven't done any real performance analysis on this stuff TBH, apart
from looking at the duration of the atomic commits. I guess separate
function pointers might be nice for code readability at least.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo
2017-04-06 15:40 ` Mika Kuoppala
2017-04-06 15:46 ` Chris Wilson
@ 2017-05-02 14:03 ` Mika Kuoppala
1 sibling, 0 replies; 20+ messages in thread
From: Mika Kuoppala @ 2017-05-02 14:03 UTC (permalink / raw)
To: intel-gfx
From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Replace the handcrafter loop when checking for fifo slots
with atomic wait for. This brings this wait in line with
the other waits on register access. We also get a readable
timeout constraint, so make it to fail after 10ms.
Chris suggested that we should fail silently as the fifo debug
handler, now attached to unclaimed mmio handling, will take care of the
possible errors at later stage.
Note that the decision to wait was changed so that we avoid
allocating the first reserved entry. Nor do we reduce the count
if we fail the wait, removing the possiblity to wrap the
count if the hw fifo returned zero.
v2: remove unclaimed check on timeout (Chris)
v3: use void return (Chris)
References: https://bugs.freedesktop.org/show_bug.cgi?id=100247
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_uncore.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index ba7e9e8..aa9d306 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -29,6 +29,7 @@
#include <linux/pm_runtime.h>
#define FORCEWAKE_ACK_TIMEOUT_MS 50
+#define GT_FIFO_TIMEOUT_MS 10
#define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32((dev_priv__), (reg__))
@@ -179,30 +180,27 @@ static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
return count & GT_FIFO_FREE_ENTRIES_MASK;
}
-static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
+static void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
{
- int ret = 0;
+ u32 n;
/* On VLV, FIFO will be shared by both SW and HW.
* So, we need to read the FREE_ENTRIES everytime */
if (IS_VALLEYVIEW(dev_priv))
- dev_priv->uncore.fifo_count = fifo_free_entries(dev_priv);
-
- if (dev_priv->uncore.fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
- int loop = 500;
- u32 fifo = fifo_free_entries(dev_priv);
-
- while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) {
- udelay(10);
- fifo = fifo_free_entries(dev_priv);
+ n = fifo_free_entries(dev_priv);
+ else
+ n = dev_priv->uncore.fifo_count;
+
+ if (n <= GT_FIFO_NUM_RESERVED_ENTRIES) {
+ if (wait_for_atomic((n = fifo_free_entries(dev_priv)) >
+ GT_FIFO_NUM_RESERVED_ENTRIES,
+ GT_FIFO_TIMEOUT_MS)) {
+ DRM_DEBUG("GT_FIFO timeout, entries: %u\n", n);
+ return;
}
- if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
- ++ret;
- dev_priv->uncore.fifo_count = fifo;
}
- dev_priv->uncore.fifo_count--;
- return ret;
+ dev_priv->uncore.fifo_count = n - 1;
}
static enum hrtimer_restart
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev4)
2017-04-06 8:44 [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Mika Kuoppala
` (4 preceding siblings ...)
2017-04-06 16:24 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev3) Patchwork
@ 2017-05-02 14:27 ` Patchwork
5 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-05-02 14:27 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev4)
URL : https://patchwork.freedesktop.org/series/22571/
State : success
== Summary ==
Series 22571v4 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/22571/revisions/4/mbox/
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-kbl-7560u) fdo#100125
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:429s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:434s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:575s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:514s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:547s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:486s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:479s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:407s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:411s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:414s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:494s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:473s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:455s
fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:566s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:453s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:571s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:467s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:495s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:434s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:529s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:396s
310077224306c08a82476bb616de679715e83485 drm-tip: 2017y-05m-02d-12h-04m-57s UTC integration manifest
12c80e1 drm/i915: Use wait_for_atomic_us when waiting for gt fifo
d578fcc drm/i915: Move the GTFIFODBG to the common mmio dbg framework
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4597/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo
2017-04-06 15:46 ` Chris Wilson
@ 2017-05-03 9:54 ` Mika Kuoppala
0 siblings, 0 replies; 20+ messages in thread
From: Mika Kuoppala @ 2017-05-03 9:54 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
>> #define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32((dev_priv__), (reg__))
>>
>> @@ -181,28 +182,26 @@ static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
>>
>> static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
>
> As this is patch 2/2, we could drop the int return here, and just make
> this a void function now.
> -Chris
Made it void and pushed. Thanks for the original 1/2 and
review!
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-05-03 9:54 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-06 8:44 [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Mika Kuoppala
2017-04-06 8:44 ` [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo Mika Kuoppala
2017-04-06 9:12 ` Chris Wilson
2017-04-06 9:32 ` Mika Kuoppala
2017-04-06 15:40 ` Mika Kuoppala
2017-04-06 15:46 ` Chris Wilson
2017-05-03 9:54 ` Mika Kuoppala
2017-05-02 14:03 ` Mika Kuoppala
2017-04-06 9:03 ` [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework Chris Wilson
2017-04-06 9:14 ` Chris Wilson
2017-04-06 15:39 ` Mika Kuoppala
2017-04-06 15:46 ` Ville Syrjälä
2017-04-06 16:05 ` Chris Wilson
2017-04-06 16:25 ` Ville Syrjälä
2017-04-06 9:35 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
2017-04-06 11:13 ` Mika Kuoppala
2017-04-06 11:32 ` Chris Wilson
2017-04-06 16:04 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev2) Patchwork
2017-04-06 16:24 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev3) Patchwork
2017-05-02 14:27 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev4) Patchwork
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.