Chris Wilson <[email protected]> writes:

> [ text/plain ]
> This is so that we have symmetry with intel_lrc.c and avoid a source of
> if (i915.enable_execlists) layering violation within i915_gem_context.c -
> that is we move the specific handling of the dev_priv->kernel_context
> for legacy submission into the legacy submission code.
>
> This depends upon the init/fini ordering between contexts and engines
> already defined by intel_lrc.c, and also exporting the context alignment
> required for pinning the legacy context.
>
> v2: Separate out pin/unpin context funcs for greater symmetry with
> intel_lrc. One more step towards unifying behaviour between the two
> classes of engines and towards fixing another bug in i915_switch_context
> vs requests.
>
> Signed-off-by: Chris Wilson <[email protected]>

Reviewed-by: Mika Kuoppala <[email protected]>


> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  2 ++
>  drivers/gpu/drm/i915/i915_gem_context.c | 27 ++--------------
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 55 
> +++++++++++++++++++++++++++++++++
>  3 files changed, 60 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e4c8e341655c..19d0194c728f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -867,6 +867,8 @@ struct i915_gem_context {
>       u32 user_handle;
>  #define CONTEXT_NO_ZEROMAP           (1<<0)
>  
> +     u32 ggtt_alignment;
> +
>       struct intel_context {
>               struct drm_i915_gem_object *state;
>               struct intel_ringbuffer *ringbuf;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index a3b11aac23a4..c620fe6c9d96 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -268,6 +268,8 @@ __create_hw_context(struct drm_device *dev,
>       list_add_tail(&ctx->link, &dev_priv->context_list);
>       ctx->i915 = dev_priv;
>  
> +     ctx->ggtt_alignment = get_context_alignment(dev_priv);
> +
>       if (dev_priv->hw_context_size) {
>               struct drm_i915_gem_object *obj =
>                               i915_gem_alloc_context_obj(dev, 
> dev_priv->hw_context_size);
> @@ -413,26 +415,6 @@ int i915_gem_context_init(struct drm_device *dev)
>               return PTR_ERR(ctx);
>       }
>  
> -     if (!i915.enable_execlists && ctx->engine[RCS].state) {
> -             int ret;
> -
> -             /* We may need to do things with the shrinker which
> -              * require us to immediately switch back to the default
> -              * context. This can cause a problem as pinning the
> -              * default context also requires GTT space which may not
> -              * be available. To avoid this we always pin the default
> -              * context.
> -              */
> -             ret = i915_gem_obj_ggtt_pin(ctx->engine[RCS].state,
> -                                         get_context_alignment(dev_priv), 0);
> -             if (ret) {
> -                     DRM_ERROR("Failed to pinned default global context 
> (error %d)\n",
> -                               ret);
> -                     i915_gem_context_unreference(ctx);
> -                     return ret;
> -             }
> -     }
> -
>       dev_priv->kernel_context = ctx;
>  
>       DRM_DEBUG_DRIVER("%s context support initialized\n",
> @@ -469,9 +451,6 @@ void i915_gem_context_fini(struct drm_device *dev)
>  
>       lockdep_assert_held(&dev->struct_mutex);
>  
> -     if (!i915.enable_execlists && dctx->engine[RCS].state)
> -             i915_gem_object_ggtt_unpin(dctx->engine[RCS].state);
> -
>       i915_gem_context_unreference(dctx);
>       dev_priv->kernel_context = NULL;
>  
> @@ -721,7 +700,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
>  
>       /* Trying to pin first makes error handling easier. */
>       ret = i915_gem_obj_ggtt_pin(to->engine[RCS].state,
> -                                 get_context_alignment(engine->i915),
> +                                 to->ggtt_alignment,
>                                   0);
>       if (ret)
>               return ret;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 8d35a3978f9b..92bb376e5039 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2240,6 +2240,47 @@ intel_ringbuffer_free(struct intel_ringbuffer *ring)
>       kfree(ring);
>  }
>  
> +static int intel_ring_context_pin(struct i915_gem_context *ctx,
> +                               struct intel_engine_cs *engine)
> +{
> +     struct intel_context *ce = &ctx->engine[engine->id];
> +     int ret;
> +
> +     lockdep_assert_held(&ctx->i915->dev->struct_mutex);
> +
> +     if (ce->pin_count++)
> +             return 0;
> +
> +     if (ce->state) {
> +             ret = i915_gem_obj_ggtt_pin(ce->state, ctx->ggtt_alignment, 0);
> +             if (ret)
> +                     goto error;
> +     }
> +
> +     i915_gem_context_reference(ctx);
> +     return 0;
> +
> +error:
> +     ce->pin_count = 0;
> +     return ret;
> +}
> +
> +static void intel_ring_context_unpin(struct i915_gem_context *ctx,
> +                                  struct intel_engine_cs *engine)
> +{
> +     struct intel_context *ce = &ctx->engine[engine->id];
> +
> +     lockdep_assert_held(&ctx->i915->dev->struct_mutex);
> +
> +     if (--ce->pin_count)
> +             return;
> +
> +     if (ce->state)
> +             i915_gem_object_ggtt_unpin(ce->state);
> +
> +     i915_gem_context_unreference(ctx);
> +}
> +
>  static int intel_init_ring_buffer(struct drm_device *dev,
>                                 struct intel_engine_cs *engine)
>  {
> @@ -2260,6 +2301,17 @@ static int intel_init_ring_buffer(struct drm_device 
> *dev,
>  
>       init_waitqueue_head(&engine->irq_queue);
>  
> +     /* We may need to do things with the shrinker which
> +      * require us to immediately switch back to the default
> +      * context. This can cause a problem as pinning the
> +      * default context also requires GTT space which may not
> +      * be available. To avoid this we always pin the default
> +      * context.
> +      */
> +     ret = intel_ring_context_pin(dev_priv->kernel_context, engine);
> +     if (ret)
> +             goto error;
> +
>       ringbuf = intel_engine_create_ringbuffer(engine, 32 * PAGE_SIZE);
>       if (IS_ERR(ringbuf)) {
>               ret = PTR_ERR(ringbuf);
> @@ -2327,6 +2379,9 @@ void intel_cleanup_engine(struct intel_engine_cs 
> *engine)
>  
>       i915_cmd_parser_fini_ring(engine);
>       i915_gem_batch_pool_fini(&engine->batch_pool);
> +
> +     intel_ring_context_unpin(dev_priv->kernel_context, engine);
> +
>       engine->i915 = NULL;
>  }
>  
> -- 
> 2.8.1
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to