On Mon, Dec 21, 2015 at 04:04:42PM +0000, Dave Gordon wrote:
> There are a number of places where the driver needs a request, but isn't
> working on behalf of any specific user or in a specific context. For
> such requests, we associate them with the default context for the engine
> that the request will be submitted to.
> 
> This patch provides a shorthand for doing such request allocations and
> changes all such calls to use the new function.
> 
> Signed-off-by: Dave Gordon <david.s.gor...@intel.com>
> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 ++
>  drivers/gpu/drm/i915/i915_gem.c      | 36 
> ++++++++++++++++++++++++++++--------
>  drivers/gpu/drm/i915/intel_display.c |  6 ++++--
>  drivers/gpu/drm/i915/intel_overlay.c | 24 ++++++++++++------------
>  4 files changed, 46 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 666d07c..4955db9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2270,6 +2270,8 @@ struct drm_i915_gem_request {
>  int i915_gem_request_alloc(struct intel_engine_cs *ring,
>                          struct intel_context *ctx,
>                          struct drm_i915_gem_request **req_out);
> +struct drm_i915_gem_request * __must_check
> +     i915_gem_request_alloc_anon(struct intel_engine_cs *ring);
>  void i915_gem_request_cancel(struct drm_i915_gem_request *req);
>  void i915_gem_request_free(struct kref *req_ref);
>  int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index be1f984..9f9c0c0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2751,6 +2751,21 @@ err:
>       return ret;
>  }
>  
> +/*
> + * Allocate a request associated with the default context for the given
> + * ring. This can be used where the driver needs a request for internal
> + * purposes not directly related to a user batch submission.
> + */
> +struct drm_i915_gem_request *
> +i915_gem_request_alloc_anon(struct intel_engine_cs *ring)
> +{

As demonstrated, no. Contexts need to be considered properly first.

> +     struct drm_i915_gem_request *req;
> +     int err;
> +
> +     err = i915_gem_request_alloc(ring, ring->default_context, &req);
> +     return err ? ERR_PTR(err) : req;
> +}
> +
>  void i915_gem_request_cancel(struct drm_i915_gem_request *req)
>  {
>       intel_ring_reserved_space_cancel(req->ringbuf);
> @@ -3168,9 +3183,13 @@ __i915_gem_object_sync(struct drm_i915_gem_object *obj,
>                       return 0;
>  
>               if (*to_req == NULL) {
> -                     ret = i915_gem_request_alloc(to, to->default_context, 
> to_req);
> -                     if (ret)
> -                             return ret;
> +                     struct drm_i915_gem_request *req;
> +
> +                     req = i915_gem_request_alloc_anon(to);

Wrong context. Please see patches to fix this mess.

> +                     if (IS_ERR(req))
> +                             return PTR_ERR(req);
> +
> +                     *to_req = req;
>               }
>  
>               trace_i915_gem_ring_sync_to(*to_req, from, from_req);
> @@ -3370,9 +3389,9 @@ int i915_gpu_idle(struct drm_device *dev)
>               if (!i915.enable_execlists) {
>                       struct drm_i915_gem_request *req;
>  
> -                     ret = i915_gem_request_alloc(ring, 
> ring->default_context, &req);
> -                     if (ret)
> -                             return ret;
> +                     req = i915_gem_request_alloc_anon(ring);
> +                     if (IS_ERR(req))
> +                             return PTR_ERR(req);
>  
>                       ret = i915_switch_context(req);
>                       if (ret) {
> @@ -4867,8 +4886,9 @@ i915_gem_init_hw(struct drm_device *dev)
>       for_each_ring(ring, dev_priv, i) {
>               struct drm_i915_gem_request *req;
>  
> -             ret = i915_gem_request_alloc(ring, ring->default_context, &req);
> -             if (ret) {
> +             req = i915_gem_request_alloc_anon(ring);
> +             if (IS_ERR(req)) {
> +                     ret = PTR_ERR(req);
>                       i915_gem_cleanup_ringbuffer(dev);
>                       goto out;
>               }
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index abd2d29..5716f4a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11662,9 +11662,11 @@ static int intel_crtc_page_flip(struct drm_crtc 
> *crtc,
>                                       obj->last_write_req);
>       } else {
>               if (!request) {
> -                     ret = i915_gem_request_alloc(ring, 
> ring->default_context, &request);
> -                     if (ret)

Wrong context.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to