> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
> john.c.harri...@intel.com
> Sent: Thursday, March 19, 2015 12:31 PM
> To: Intel-GFX@Lists.FreeDesktop.Org
> Subject: [Intel-gfx] [PATCH 55/59] drm/i915: Remove fallback poll for ring
> buffer
> space
>
> From: John Harrison <john.c.harri...@intel.com>
>
> 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.harri...@intel.com>
> ---
> 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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
