[Intel-gfx] [PATCH 1/2] drm/i915: Do not return stale status / remove impossible WARN

[Chris Wilson]

On Thu, Feb 11, 2016 at 06:03:09PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Only caller to get_context_status ensures read pointer stays in
> range so the WARN is impossible. Also, if the WARN would be
> triggered by a hypothetical new caller stale status would be
> returned to them.
> 
> Maybe it is better to wrap the pointer in the function itself
> then to avoid both and even results in smaller code.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 89eb892df4ae..951f1e6af947 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -507,17 +507,16 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
>  	return false;
>  }
>  
> -static void get_context_status(struct intel_engine_cs *ring,
> -			       u8 read_pointer,
> -			       u32 *status, u32 *context_id)
> +static u32 get_context_status(struct intel_engine_cs *ring, u8 read_pointer,
> +			      u32 *context_id)
>  {
>  	struct drm_i915_private *dev_priv = ring->dev->dev_private;
>  
> -	if (WARN_ON(read_pointer >= GEN8_CSB_ENTRIES))
> -		return;
> +	read_pointer %= GEN8_CSB_ENTRIES;
>  
> -	*status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer));
>  	*context_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer));

Micro-optimising hat says not to even do the uncached, spinlocked mmio
read when not required.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre