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