> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of > John.C.Harrison@xxxxxxxxx > Sent: Thursday, March 19, 2015 12:31 PM > To: Intel-GFX@xxxxxxxxxxxxxxxxxxxxx > Subject: [PATCH 55/59] drm/i915: Remove fallback poll for ring buffer > space > > From: John Harrison <John.C.Harrison@xxxxxxxxx> > > When the ring buffer is full, the driver finds an outstanding request that will > free up sufficient space for the current operation and waits for it to complete. > If no such request can be found, there is a fall back path of just polling until > sufficient space is available. > > This path should not be required any more. It is a hangover from the bad days of > OLR such that it was possible for the ring to be completely filled without ever > having submitted a request. This can no longer happen as requests are now > submitted in a timely manner. Hence the entire polling path is obsolete. As it > also causes headaches in LRC land due to nesting faked requests, it is being > removed entirely. > > For: VIZ-5115 > Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_lrc.c | 65 ++++--------------------------- > drivers/gpu/drm/i915/intel_ringbuffer.c | 62 +++-------------------------- > 2 files changed, 13 insertions(+), 114 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 60bcf9a..f21f449 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -622,8 +622,9 @@ int intel_logical_ring_alloc_request_extras(struct > drm_i915_gem_request *request > return 0; > } > > -static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf, > - int bytes) > +static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf, > + struct intel_context *ctx, > + int bytes) > { > struct intel_engine_cs *ring = ringbuf->ring; > struct drm_i915_gem_request *request; > @@ -652,8 +653,11 @@ static int logical_ring_wait_request(struct > intel_ringbuffer *ringbuf, > break; > } > > - if (&request->list == &ring->request_list) > + /* It should always be possible to find a suitable request! */ > + if (&request->list == &ring->request_list) { > + WARN_ON(true); > return -ENOSPC; > + } Don’t we normally say if (WARN_ON(&request->list == &ring->request_list)) return -ENOSPC; > > ret = i915_wait_request(request); > if (ret) > @@ -663,7 +667,7 @@ static int logical_ring_wait_request(struct > intel_ringbuffer *ringbuf, > > WARN_ON(intel_ring_space(ringbuf) < new_space); > > - return intel_ring_space(ringbuf) >= bytes ? 0 : -ENOSPC; > + return 0; > } > > /* > @@ -690,59 +694,6 @@ intel_logical_ring_advance_and_submit(struct > intel_ringbuffer *ringbuf, > execlists_context_queue(ring, ctx, ringbuf->tail, request); > } > > -static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf, > - struct intel_context *ctx, > - int bytes) > -{ > - struct intel_engine_cs *ring = ringbuf->ring; > - struct drm_device *dev = ring->dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > - unsigned long end; > - int ret; > - > - /* The whole point of reserving space is to not wait! */ > - WARN_ON(ringbuf->reserved_in_use); > - > - ret = logical_ring_wait_request(ringbuf, bytes); > - if (ret != -ENOSPC) > - return ret; > - > - /* Force the context submission in case we have been skipping it */ > - intel_logical_ring_advance_and_submit(ringbuf, ctx, NULL); > - > - /* With GEM the hangcheck timer should kick us out of the loop, > - * leaving it early runs the risk of corrupting GEM state (due > - * to running on almost untested codepaths). But on resume > - * timers don't work yet, so prevent a complete hang in that > - * case by choosing an insanely large timeout. */ > - end = jiffies + 60 * HZ; > - > - ret = 0; > - do { > - if (intel_ring_space(ringbuf) >= bytes) > - break; > - > - msleep(1); > - > - if (dev_priv->mm.interruptible && signal_pending(current)) { > - ret = -ERESTARTSYS; > - break; > - } > - > - ret = i915_gem_check_wedge(&dev_priv->gpu_error, > - dev_priv->mm.interruptible); > - if (ret) > - break; > - > - if (time_after(jiffies, end)) { > - ret = -EBUSY; > - break; > - } > - } while (1); > - > - return ret; > -} > - > static int logical_ring_wrap_buffer(struct intel_ringbuffer *ringbuf, > struct intel_context *ctx) > { > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index c5752c4..6099fce 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -2063,7 +2063,7 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs > *ring) > ring->buffer = NULL; > } > > -static int intel_ring_wait_request(struct intel_engine_cs *ring, int n) > +static int ring_wait_for_space(struct intel_engine_cs *ring, int n) > { > struct intel_ringbuffer *ringbuf = ring->buffer; > struct drm_i915_gem_request *request; > @@ -2082,8 +2082,11 @@ static int intel_ring_wait_request(struct > intel_engine_cs *ring, int n) > break; > } > > - if (&request->list == &ring->request_list) > + /* It should always be possible to find a suitable request! */ > + if (&request->list == &ring->request_list) { > + WARN_ON(true); > return -ENOSPC; > + } Same thing. Thomas. > > ret = i915_wait_request(request); > if (ret) > @@ -2096,61 +2099,6 @@ static int intel_ring_wait_request(struct > intel_engine_cs *ring, int n) > return 0; > } > > -static int ring_wait_for_space(struct intel_engine_cs *ring, int n) > -{ > - struct drm_device *dev = ring->dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_ringbuffer *ringbuf = ring->buffer; > - unsigned long end; > - int ret; > - > - /* The whole point of reserving space is to not wait! */ > - WARN_ON(ringbuf->reserved_in_use); > - > - ret = intel_ring_wait_request(ring, n); > - if (ret != -ENOSPC) > - return ret; > - > - /* force the tail write in case we have been skipping them */ > - __intel_ring_advance(ring); > - > - /* With GEM the hangcheck timer should kick us out of the loop, > - * leaving it early runs the risk of corrupting GEM state (due > - * to running on almost untested codepaths). But on resume > - * timers don't work yet, so prevent a complete hang in that > - * case by choosing an insanely large timeout. */ > - end = jiffies + 60 * HZ; > - > - ret = 0; > - trace_i915_ring_wait_begin(ring); > - do { > - if (intel_ring_space(ringbuf) >= n) > - break; > - ringbuf->head = I915_READ_HEAD(ring); > - if (intel_ring_space(ringbuf) >= n) > - break; > - > - msleep(1); > - > - if (dev_priv->mm.interruptible && signal_pending(current)) { > - ret = -ERESTARTSYS; > - break; > - } > - > - ret = i915_gem_check_wedge(&dev_priv->gpu_error, > - dev_priv->mm.interruptible); > - if (ret) > - break; > - > - if (time_after(jiffies, end)) { > - ret = -EBUSY; > - break; > - } > - } while (1); > - trace_i915_ring_wait_end(ring); > - return ret; > -} > - > static int intel_wrap_ring_buffer(struct intel_engine_cs *ring) > { > uint32_t __iomem *virt; > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
