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

[Jani Nikula <jani.nikula@intel.com>]

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