[Intel-gfx] [PATCH v6 5/5] drm/i915: Tidy up execbuffer command parsing code
Daniel Vetter
daniel at ffwll.ch
Tue Dec 9 05:22:58 PST 2014
On Mon, Dec 08, 2014 at 02:33:50PM -0800, michael.h.nguyen at intel.com wrote:
> From: Brad Volkin <bradley.d.volkin at 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.volkin at intel.com>
>
> Conflicts:
> drivers/gpu/drm/i915/i915_gem_execbuffer.c
Please remove those when rebasing. When actually something changed please
mention that in the patch revlog, e.g.
v3: Rebase (conflicts with patch "foo" in execbuf code).
But if it's a boring rebase without any functional conflicts I wouldn't
bother with a changelog entry.
-Daniel
> ---
> 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;
> + } 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list