> -----Original Message-----
> From: Nguyen, Michael H
> Sent: Monday, December 08, 2014 10:34 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Bloomfield, Jon; Brad Volkin
> Subject: [PATCH v6 5/5] drm/i915: Tidy up execbuffer command parsing code
> 
> From: Brad Volkin <bradley.d.vol...@intel.com>
> 
> Move it to a separate function since the main do_execbuffer function
> already has so much going on.
> 
> v2:
> - Move pin/unpin calls inside i915_parse_cmds() (Chris W, v4 7/7
>   feedback)
> 
> Issue: VIZ-4719
> Signed-off-by: Brad Volkin <bradley.d.vol...@intel.com>
> 
> Conflicts:
>       drivers/gpu/drm/i915/i915_gem_execbuffer.c
> ---
>  drivers/gpu/drm/i915/i915_cmd_parser.c     |   8 ++
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 126 ++++++++++++++++---
> ----------
>  2 files changed, 77 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
> b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index 118079d..80b1b28 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -1042,10 +1042,17 @@ int i915_parse_cmds(struct intel_engine_cs
> *ring,
>       struct drm_i915_cmd_descriptor default_desc = { 0 };
>       bool oacontrol_set = false; /* OACONTROL tracking. See
> check_cmd() */
> 
> +     ret = i915_gem_obj_ggtt_pin(shadow_batch_obj, 4096, 0);
> +     if (ret) {
> +             DRM_DEBUG_DRIVER("CMD: Failed to pin shadow batch\n");
> +             return -1;
> +     }
> +
>       batch_base = copy_batch(shadow_batch_obj, batch_obj,
>                               batch_start_offset, batch_len);
>       if (IS_ERR(batch_base)) {
>               DRM_DEBUG_DRIVER("CMD: Failed to copy batch\n");
> +             i915_gem_object_ggtt_unpin(shadow_batch_obj);
>               return PTR_ERR(batch_base);
>       }
> 
> @@ -1116,6 +1123,7 @@ int i915_parse_cmds(struct intel_engine_cs *ring,
>       }
> 
>       vunmap(batch_base);
> +     i915_gem_object_ggtt_unpin(shadow_batch_obj);
> 
>       return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 2071938..3d36465 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1069,6 +1069,65 @@ i915_emit_box(struct intel_engine_cs *ring,
>       return 0;
>  }
> 
> +static struct drm_i915_gem_object*
> +i915_gem_execbuffer_parse(struct intel_engine_cs *ring,
> +                       struct drm_i915_gem_exec_object2
> *shadow_exec_entry,
> +                       struct eb_vmas *eb,
> +                       struct drm_i915_gem_object *batch_obj,
> +                       u32 batch_start_offset,
> +                       u32 batch_len,
> +                       bool is_master,
> +                       u32 *flags)
> +{
> +     struct drm_i915_private *dev_priv = to_i915(batch_obj->base.dev);
> +     struct drm_i915_gem_object *shadow_batch_obj;
> +     int ret;
> +
> +     shadow_batch_obj = i915_gem_batch_pool_get(&dev_priv-
> >mm.batch_pool,
> +                                                batch_obj->base.size);
> +     if (IS_ERR(shadow_batch_obj))
> +             return shadow_batch_obj;
> +
> +     ret = i915_parse_cmds(ring,
> +                           batch_obj,
> +                           shadow_batch_obj,
> +                           batch_start_offset,
> +                           batch_len,
> +                           is_master);
> +     if (ret) {
> +             if (ret == -EACCES)
> +                     return batch_obj;

Shouldn't this be returning an error code ?



> +     } else {
> +             struct i915_vma *vma;
> +
> +             memset(shadow_exec_entry, 0,
> sizeof(*shadow_exec_entry));
> +
> +             vma = i915_gem_obj_to_ggtt(shadow_batch_obj);
> +             vma->exec_entry = shadow_exec_entry;
> +             vma->exec_entry->flags = __EXEC_OBJECT_PURGEABLE;
> +             drm_gem_object_reference(&shadow_batch_obj->base);
> +             list_add_tail(&vma->exec_list, &eb->vmas);
> +
> +             shadow_batch_obj->base.pending_read_domains =
> +                     batch_obj->base.pending_read_domains;
> +
> +             /*
> +              * Set the DISPATCH_SECURE bit to remove the
> NON_SECURE
> +              * bit from MI_BATCH_BUFFER_START commands issued in
> the
> +              * dispatch_execbuffer implementations. We specifically
> +              * don't want that set when the command parser is
> +              * enabled.
> +              *
> +              * FIXME: with aliasing ppgtt, buffers that should only
> +              * be in ggtt still end up in the aliasing ppgtt. remove
> +              * this check when that is fixed.
> +              */
> +             if (USES_FULL_PPGTT(dev))
> +                     *flags |= I915_DISPATCH_SECURE;
> +     }
> +
> +     return ret ? ERR_PTR(ret) : shadow_batch_obj; }
> 
>  int
>  i915_gem_ringbuffer_submission(struct drm_device *dev, struct drm_file
> *file, @@ -1286,7 +1345,6 @@ i915_gem_do_execbuffer(struct drm_device
> *dev, void *data,
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       struct eb_vmas *eb;
>       struct drm_i915_gem_object *batch_obj;
> -     struct drm_i915_gem_object *shadow_batch_obj = NULL;
>       struct drm_i915_gem_exec_object2 shadow_exec_entry;
>       struct intel_engine_cs *ring;
>       struct intel_context *ctx;
> @@ -1406,63 +1464,17 @@ i915_gem_do_execbuffer(struct drm_device
> *dev, void *data,
>       }
> 
>       if (i915_needs_cmd_parser(ring)) {
> -             shadow_batch_obj =
> -                     i915_gem_batch_pool_get(&dev_priv-
> >mm.batch_pool,
> -                                             batch_obj->base.size);
> -             if (IS_ERR(shadow_batch_obj)) {
> -                     ret = PTR_ERR(shadow_batch_obj);
> -                     /* Don't try to clean up the obj in the error path */
> -                     shadow_batch_obj = NULL;
> -                     goto err;
> -             }
> -
> -             shadow_batch_obj->madv = I915_MADV_WILLNEED;
> -
> -             ret = i915_gem_obj_ggtt_pin(shadow_batch_obj, 4096, 0);
> -             if (ret)
> +             batch_obj = i915_gem_execbuffer_parse(ring,
> +                                                   &shadow_exec_entry,
> +                                                   eb,
> +                                                   batch_obj,
> +                                                   args->batch_start_offset,
> +                                                   args->batch_len,
> +                                                   file->is_master,
> +                                                   &flags);
> +             if (IS_ERR(batch_obj)) {
> +                     ret = PTR_ERR(batch_obj);
>                       goto err;
> -
> -             ret = i915_parse_cmds(ring,
> -                                   batch_obj,
> -                                   shadow_batch_obj,
> -                                   args->batch_start_offset,
> -                                   args->batch_len,
> -                                   file->is_master);
> -             i915_gem_object_ggtt_unpin(shadow_batch_obj);
> -
> -             if (ret) {
> -                     if (ret != -EACCES)
> -                             goto err;
> -             } else {
> -                     struct i915_vma *vma;
> -
> -                     memset(&shadow_exec_entry, 0,
> -                            sizeof(shadow_exec_entry));
> -
> -                     vma = i915_gem_obj_to_ggtt(shadow_batch_obj);
> -                     vma->exec_entry = &shadow_exec_entry;
> -                     vma->exec_entry->flags =
> __EXEC_OBJECT_PURGEABLE;
> -                     drm_gem_object_reference(&shadow_batch_obj-
> >base);
> -                     list_add_tail(&vma->exec_list, &eb->vmas);
> -
> -                     shadow_batch_obj->base.pending_read_domains =
> -                             batch_obj->base.pending_read_domains;
> -
> -                     batch_obj = shadow_batch_obj;
> -
> -                     /*
> -                      * Set the DISPATCH_SECURE bit to remove the
> NON_SECURE
> -                      * bit from MI_BATCH_BUFFER_START commands
> issued in the
> -                      * dispatch_execbuffer implementations. We
> specifically
> -                      * don't want that set when the command parser is
> -                      * enabled.
> -                      *
> -                      * FIXME: with aliasing ppgtt, buffers that should only
> -                      * be in ggtt still end up in the aliasing ppgtt. remove
> -                      * this check when that is fixed.
> -                      */
> -                     if (USES_FULL_PPGTT(dev))
> -                             flags |= I915_DISPATCH_SECURE;
>               }
>       }
> 
> --
> 1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to