[PATCH] drm/i915: Distinguish between timeout and error in sideband transactions

[PATCH] drm/i915: Distinguish between timeout and error in sideband transactions

[Chris Wilson]

After initiating a sideband transaction, we only want to wait for the
transaction to become idle. If, as we are, we wait for both the busy
and error flag to clear, if an error is raised we just spin until the
timeout. Once the hw is idle, we can then check to see if the hw flagged
an error, and report it distinctly.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_sideband.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
index 9f782b5eb6e6..41712ea9f5f8 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -216,6 +216,7 @@ u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
 	}
 
 	I915_WRITE(SBI_ADDR, (reg << 16));
+	I915_WRITE(SBI_DATA, 0);
 
 	if (destination == SBI_ICLK)
 		value = SBI_CTL_DEST_ICLK | SBI_CTL_OP_CRRD;
@@ -225,10 +226,15 @@ u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
 
 	if (intel_wait_for_register(dev_priv,
 				    SBI_CTL_STAT,
-				    SBI_BUSY | SBI_RESPONSE_FAIL,
+				    SBI_BUSY,
 				    0,
 				    100)) {
-		DRM_ERROR("timeout waiting for SBI to complete read transaction\n");
+		DRM_ERROR("timeout waiting for SBI to complete read\n");
+		return 0;
+
+	}
+	if (I915_READ(SBI_CTL_STAT) & SBI_RESPONSE_FAIL) {
+		DRM_ERROR("error during SBI read of reg %x\n", reg);
 		return 0;
 	}
 
@@ -260,10 +266,16 @@ void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
 
 	if (intel_wait_for_register(dev_priv,
 				    SBI_CTL_STAT,
-				    SBI_BUSY | SBI_RESPONSE_FAIL,
+				    SBI_BUSY,
 				    0,
 				    100)) {
-		DRM_ERROR("timeout waiting for SBI to complete write transaction\n");
+		DRM_ERROR("timeout waiting for SBI to complete write\n");
+		return;
+	}
+
+	if (I915_READ(SBI_CTL_STAT) & SBI_RESPONSE_FAIL) {
+		DRM_ERROR("error during SBI write of %x to reg %x\n",
+			  value, reg);
 		return;
 	}
 }
-- 
2.11.0

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

✗ Fi.CI.BAT: warning for drm/i915: Distinguish between timeout and error in sideband transactions

[Patchwork]

== Series Details ==

Series: drm/i915: Distinguish between timeout and error in sideband transactions
URL   : https://patchwork.freedesktop.org/series/20147/
State : warning

== Summary ==

Series 20147v1 drm/i915: Distinguish between timeout and error in sideband transactions
https://patchwork.freedesktop.org/api/1.0/series/20147/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-a:
                pass       -> DMESG-WARN (fi-byt-n2820)

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:108  pass:95   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:278  pass:246  dwarn:1   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29 

e5c3890db41e8f63c193ce2d9ba5a3e52e23f82d drm-tip: 2017y-02m-23d-13h-18m-37s UTC integration manifest
adc282b drm/i915: Distinguish between timeout and error in sideband transactions

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3946/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Distinguish between timeout and error in sideband transactions

[Jani Nikula]

On Thu, 23 Feb 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> After initiating a sideband transaction, we only want to wait for the
> transaction to become idle. If, as we are, we wait for both the busy
> and error flag to clear, if an error is raised we just spin until the
> timeout. Once the hw is idle, we can then check to see if the hw flagged
> an error, and report it distinctly.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@intel.com>

Much better indeed.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

(The pedantic nitpick is that response status is really two bits, and
you could read them both and fail if they're not zero. But meh.)

> ---
>  drivers/gpu/drm/i915/intel_sideband.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index 9f782b5eb6e6..41712ea9f5f8 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -216,6 +216,7 @@ u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
>  	}
>  
>  	I915_WRITE(SBI_ADDR, (reg << 16));
> +	I915_WRITE(SBI_DATA, 0);
>  
>  	if (destination == SBI_ICLK)
>  		value = SBI_CTL_DEST_ICLK | SBI_CTL_OP_CRRD;
> @@ -225,10 +226,15 @@ u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
>  
>  	if (intel_wait_for_register(dev_priv,
>  				    SBI_CTL_STAT,
> -				    SBI_BUSY | SBI_RESPONSE_FAIL,
> +				    SBI_BUSY,
>  				    0,
>  				    100)) {
> -		DRM_ERROR("timeout waiting for SBI to complete read transaction\n");
> +		DRM_ERROR("timeout waiting for SBI to complete read\n");
> +		return 0;
> +
> +	}
> +	if (I915_READ(SBI_CTL_STAT) & SBI_RESPONSE_FAIL) {
> +		DRM_ERROR("error during SBI read of reg %x\n", reg);
>  		return 0;
>  	}
>  
> @@ -260,10 +266,16 @@ void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
>  
>  	if (intel_wait_for_register(dev_priv,
>  				    SBI_CTL_STAT,
> -				    SBI_BUSY | SBI_RESPONSE_FAIL,
> +				    SBI_BUSY,
>  				    0,
>  				    100)) {
> -		DRM_ERROR("timeout waiting for SBI to complete write transaction\n");
> +		DRM_ERROR("timeout waiting for SBI to complete write\n");
> +		return;
> +	}
> +
> +	if (I915_READ(SBI_CTL_STAT) & SBI_RESPONSE_FAIL) {
> +		DRM_ERROR("error during SBI write of %x to reg %x\n",
> +			  value, reg);
>  		return;
>  	}
>  }

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

Re: [PATCH] drm/i915: Distinguish between timeout and error in sideband transactions

[Chris Wilson]

On Mon, Feb 27, 2017 at 06:34:26PM +0200, Jani Nikula wrote:
> On Thu, 23 Feb 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > After initiating a sideband transaction, we only want to wait for the
> > transaction to become idle. If, as we are, we wait for both the busy
> > and error flag to clear, if an error is raised we just spin until the
> > timeout. Once the hw is idle, we can then check to see if the hw flagged
> > an error, and report it distinctly.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> 
> Much better indeed.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> (The pedantic nitpick is that response status is really two bits, and
> you could read them both and fail if they're not zero. But meh.)

First goal is just figuring out what the warnings mean on the gvtg
machines. :)
-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