Re: [PATCH 18/53] drm/i915: Moved the for_each_ring loop outside of i915_gem_context_enable()

Tomas Elf
2015-03-05T17:05:00Z

On 19/02/2015 17:17, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> The start of day context initialisation code in i915_gem_context_enable() loops
> over each ring and calls the legacy switch context or the execlist init context
> code as appropriate.
>
> This patch moves the ring looping out of that function in to the top level
> caller i915_gem_init_hw(). This means the a single pass can be made over all
> rings doing the PPGTT, L3 remap and context initialisation of each ring
> altogether.
>
> For: VIZ-5115
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 18 ++++++++++-------
> drivers/gpu/drm/i915/i915_gem_context.c | 32 +++++++++++--------------------
> 3 files changed, 23 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bfd7b47..653c82d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2989,7 +2989,7 @@ int __must_check i915_gem_context_init(struct drm_device *dev);
> void i915_gem_context_fini(struct drm_device *dev);
> void i915_gem_context_reset(struct drm_device *dev);
> int i915_gem_context_open(struct drm_device *dev, struct drm_file *file);
> -int i915_gem_context_enable(struct drm_i915_private *dev_priv);
> +int i915_gem_context_enable(struct intel_engine_cs *ring);
> void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
> int i915_switch_context(struct intel_engine_cs *ring,
> struct intel_context *to);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9bc60d7..5850991 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4844,6 +4844,8 @@ i915_gem_init_hw(struct drm_device *dev)
> */
> init_unused_rings(dev);
>
> + BUG_ON(!dev_priv->ring[RCS].default_context);
> +
> ret = i915_ppgtt_init_hw(dev);
> if (ret) {
> DRM_ERROR("PPGTT enable HW failed %d\n", ret);
> @@ -4859,6 +4861,8 @@ i915_gem_init_hw(struct drm_device *dev)
>
> /* Now it is safe to go back round and do everything else: */
> for_each_ring(ring, dev_priv, i) {
> + WARN_ON(!ring->default_context);
> +
> if (ring->id == RCS) {
> for (i = 0; i < NUM_L3_SLICES(dev); i++)
> i915_gem_l3_remap(ring, i);
> @@ -4870,17 +4874,17 @@ i915_gem_init_hw(struct drm_device *dev)
> i915_gem_cleanup_ringbuffer(dev);
> return ret;
> }
> - }
>
> - ret = i915_gem_context_enable(dev_priv);
> - if (ret && ret != -EIO) {
> - DRM_ERROR("Context enable failed %d\n", ret);
> - i915_gem_cleanup_ringbuffer(dev);
> + ret = i915_gem_context_enable(ring);
> + if (ret && ret != -EIO) {
> + DRM_ERROR("Context enable ring #%d failed %d\n", i, ret);
> + i915_gem_cleanup_ringbuffer(dev);
>
> - return ret;
> + return ret;
> + }
> }
>
> - return ret;
> + return 0;
> }
>
> int i915_gem_init(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 8603bf4..dd83d61 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -403,32 +403,22 @@ void i915_gem_context_fini(struct drm_device *dev)
> i915_gem_context_unreference(dctx);
> }
>
> -int i915_gem_context_enable(struct drm_i915_private *dev_priv)
> +int i915_gem_context_enable(struct intel_engine_cs *ring)
> {
> - struct intel_engine_cs *ring;
> - int ret, i;
> -
> - BUG_ON(!dev_priv->ring[RCS].default_context);
> + int ret;
>
> if (i915.enable_execlists) {
> - for_each_ring(ring, dev_priv, i) {
> - if (ring->init_context) {
> - ret = ring->init_context(ring,
> - ring->default_context);
> - if (ret) {
> - DRM_ERROR("ring init context: %d\n",
> - ret);
> - return ret;
> - }
> - }
> - }
> + if (ring->init_context == NULL)
> + return 0;
>
> + ret = ring->init_context(ring, ring->default_context);
> } else
> - for_each_ring(ring, dev_priv, i) {
> - ret = i915_switch_context(ring, ring->default_context);
> - if (ret)
> - return ret;
> - }
> + ret = i915_switch_context(ring, ring->default_context);
> +
> + if (ret) {
> + DRM_ERROR("ring init context: %d\n", ret);
> + return ret;
> + }
>
> return 0;
> }
>

Reviewed-by: Tomas Elf <tomas.elf@intel.com>

Thanks,
Tomas