On Tue, Dec 08, 2015 at 03:02:36PM +0000, Dave Gordon wrote:
> Based on Chris Wilson's patch from 6 months ago, rebased and adapted.
>
> The current implementation of intel_ring_initialized() is too heavyweight;
> it's a non-inlined function that chases several levels of pointers. This
> wouldn't matter too much if it were rarely called, but it's used inside
> the iterator test of for_each_ring() and is therefore called quite
> frequently. So let's make it simple and inline ...
>
> The idea here is to use ring->dev as an indicator showing which engines
> have been initialised and are therefore to be included in iterations that
> use for_each_ring(). This allows us to avoid multiple memory references
> and a (non-inlined) function call on each iteration of each such loop.
>
> Fixes regression from
> commit 48d823878d64f93163f5a949623346748bbce1b4
> Author: Oscar Mateo <oscar.ma...@intel.com>
> Date: Thu Jul 24 17:04:23 2014 +0100
>
> drm/i915/bdw: Generic logical ring init and cleanup
>
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Signed-off-by: Dave Gordon <david.s.gor...@intel.com>
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 17 +++++++++-----
> drivers/gpu/drm/i915/intel_ringbuffer.c | 39
> +++++++++++----------------------
> drivers/gpu/drm/i915/intel_ringbuffer.h | 6 ++++-
> 3 files changed, 30 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 4ebafab..7644c48 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1894,8 +1894,10 @@ void intel_logical_ring_cleanup(struct intel_engine_cs
> *ring)
>
> dev_priv = ring->dev->dev_private;
>
> - intel_logical_ring_stop(ring);
> - WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0);
> + if (ring->buffer) {
> + intel_logical_ring_stop(ring);
> + WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0);
> + }
>
> if (ring->cleanup)
> ring->cleanup(ring);
> @@ -1909,6 +1911,7 @@ void intel_logical_ring_cleanup(struct intel_engine_cs
> *ring)
> }
>
> lrc_destroy_wa_ctx_obj(ring);
> + ring->dev = NULL;
> }
>
> static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs
> *ring)
> @@ -1931,11 +1934,11 @@ static int logical_ring_init(struct drm_device *dev,
> struct intel_engine_cs *rin
>
> ret = i915_cmd_parser_init_ring(ring);
> if (ret)
> - return ret;
> + goto error;
>
> ret = intel_lr_context_deferred_alloc(ring->default_context, ring);
> if (ret)
> - return ret;
> + goto error;
>
> /* As this is the default context, always pin it */
> ret = intel_lr_context_do_pin(
> @@ -1946,9 +1949,13 @@ static int logical_ring_init(struct drm_device *dev,
> struct intel_engine_cs *rin
> DRM_ERROR(
> "Failed to pin and map ringbuffer %s: %d\n",
> ring->name, ret);
> - return ret;
> + goto error;
> }
>
> + return 0;
> +
> +error:
> + intel_logical_ring_cleanup(ring);
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 57d78f2..921c8a6 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -33,23 +33,6 @@
> #include "i915_trace.h"
> #include "intel_drv.h"
>
> -bool
> -intel_ring_initialized(struct intel_engine_cs *ring)
> -{
> - struct drm_device *dev = ring->dev;
> -
> - if (!dev)
> - return false;
> -
> - if (i915.enable_execlists) {
> - struct intel_context *dctx = ring->default_context;
> - struct intel_ringbuffer *ringbuf =
> dctx->engine[ring->id].ringbuf;
> -
> - return ringbuf->obj;
> - } else
> - return ring->buffer && ring->buffer->obj;
> -}
> -
> int __intel_ring_space(int head, int tail, int size)
> {
> int space = head - tail;
> @@ -2167,8 +2150,10 @@ static int intel_init_ring_buffer(struct drm_device
> *dev,
> init_waitqueue_head(&ring->irq_queue);
>
> ringbuf = intel_engine_create_ringbuffer(ring, 32 * PAGE_SIZE);
> - if (IS_ERR(ringbuf))
> - return PTR_ERR(ringbuf);
> + if (IS_ERR(ringbuf)) {
> + ret = PTR_ERR(ringbuf);
> + goto error;
> + }
> ring->buffer = ringbuf;
>
> if (I915_NEED_GFX_HWS(dev)) {
> @@ -2197,8 +2182,7 @@ static int intel_init_ring_buffer(struct drm_device
> *dev,
> return 0;
>
> error:
> - intel_ringbuffer_free(ringbuf);
> - ring->buffer = NULL;
> + intel_cleanup_ring_buffer(ring);
> return ret;
> }
>
> @@ -2211,12 +2195,14 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs
> *ring)
>
> dev_priv = to_i915(ring->dev);
>
> - intel_stop_ring_buffer(ring);
> - WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & MODE_IDLE) == 0);
> + if (ring->buffer) {
> + intel_stop_ring_buffer(ring);
> + WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) &
> MODE_IDLE) == 0);
>
> - intel_unpin_ringbuffer_obj(ring->buffer);
> - intel_ringbuffer_free(ring->buffer);
> - ring->buffer = NULL;
> + intel_unpin_ringbuffer_obj(ring->buffer);
> + intel_ringbuffer_free(ring->buffer);
> + ring->buffer = NULL;
> + }
>
> if (ring->cleanup)
> ring->cleanup(ring);
> @@ -2225,6 +2211,7 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs
> *ring)
>
> i915_cmd_parser_fini_ring(ring);
> i915_gem_batch_pool_fini(&ring->batch_pool);
> + ring->dev = NULL;
> }
>
> static int ring_wait_for_space(struct intel_engine_cs *ring, int n)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
> b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 5d1eb20..49574ff 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -350,7 +350,11 @@ struct intel_engine_cs {
> u32 (*get_cmd_length_mask)(u32 cmd_header);
> };
>
> -bool intel_ring_initialized(struct intel_engine_cs *ring);
> +static inline bool
> +intel_ring_initialized(struct intel_engine_cs *ring)
> +{
> + return ring->dev != NULL;
> +}
>
> static inline unsigned
> intel_ring_flag(struct intel_engine_cs *ring)
> --
> 1.9.1
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
