| Message ID | 1455213790-4724-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 11, 2016 at 06:03:09PM +0000, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@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@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
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)); + + return I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer)); } /** @@ -547,9 +546,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring) spin_lock(&ring->execlist_lock); while (read_pointer < write_pointer) { - - get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES, - &status, &status_id); + status = get_context_status(ring, ++read_pointer, &status_id); if (status & GEN8_CTX_STATUS_IDLE_ACTIVE) continue;