On Thu, Feb 11, 2016 at 06:03:09PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursu...@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.ursu...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to