[PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

[PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

[Mika Kuoppala]

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

[PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo

[Mika Kuoppala]

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

Re: [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

[Chris Wilson]

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

Re: [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo

[Chris Wilson]

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

Re: [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

[Chris Wilson]

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

Re: [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo

[Mika Kuoppala]

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

✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

[Patchwork]

== 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

Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

[Mika Kuoppala]

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

Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

[Chris Wilson]

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

[PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

[Mika Kuoppala]

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

[PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo

[Mika Kuoppala]

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

Re: [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo

[Chris Wilson]

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

Re: [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

[Ville Syrjälä]

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev2)

[Patchwork]

== 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

Re: [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

[Chris Wilson]

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

✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev3)

[Patchwork]

== 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

Re: [PATCH 1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

[Ville Syrjälä]

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

[PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo

[Mika Kuoppala]

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework (rev4)

[Patchwork]

== 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

Re: [PATCH 2/2] drm/i915: Use wait_for_atomic_us when waiting for gt fifo

[Mika Kuoppala]

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