On Thu, Jun 30, 2016 at 04:12:49PM +0100, Dave Gordon wrote:
> Precursor for fix to secure batch execution. We will need to be able to
> retrieve the batch VMA (as well as the batch itself) from the eb list,
> so this patch extracts that part of eb_get_batch() into a separate
> function, and moves both parts to a more logical place in the file, near
> where the eb list is created.
>
> Also, it may not be obvious, but the current execbuffer2 ioctl interface
> requires that the buffer object containing the batch-to-be-executed be
> the LAST entry in the exec2_list[] array (I expected it to be the first!).
>
> To clarify this, we can replace the rather obscure construct
> "list_entry(eb->vmas.prev, ...)"
> in the old version of eb_get_batch() with the equivalent but more explicit
> "list_last_entry(&eb->vmas,...)"
> in the new eb_get_batch_vma() and of course add an explanatory comment.
>
> Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
I have no context on the secure batch fix you're talking about, but this
here makes sense as an independent cleanup.
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 49 ++++++++++++++++++------------
> 1 file changed, 29 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 608fdc4..eea8b1f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -186,6 +186,35 @@ struct eb_vmas {
> return ret;
> }
>
> +static inline struct i915_vma *
> +eb_get_batch_vma(struct eb_vmas *eb)
> +{
> + /* The batch is always the LAST item in the VMA list */
> + struct i915_vma *vma = list_last_entry(&eb->vmas, typeof(*vma), exec_list);
> +
> + return vma;
> +}
> +
> +static struct drm_i915_gem_object *
> +eb_get_batch(struct eb_vmas *eb)
> +{
> + struct i915_vma *vma = eb_get_batch_vma(eb);
> +
> + /*
> + * SNA is doing fancy tricks with compressing batch buffers, which leads
> + * to negative relocation deltas. Usually that works out ok since the
> + * relocate address is still positive, except when the batch is placed
> + * very low in the GTT. Ensure this doesn't happen.
> + *
> + * Note that actual hangs have only been observed on gen7, but for
> + * paranoia do it everywhere.
> + */
> + if ((vma->exec_entry->flags & EXEC_OBJECT_PINNED) == 0)
> + vma->exec_entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
> +
> + return vma->obj;
> +}
> +
> static struct i915_vma *eb_get_vma(struct eb_vmas *eb, unsigned long handle)
> {
> if (eb->and < 0) {
> @@ -1341,26 +1370,6 @@ static bool only_mappable_for_reloc(unsigned int flags)
> return file_priv->bsd_ring;
> }
>
> -static struct drm_i915_gem_object *
> -eb_get_batch(struct eb_vmas *eb)
> -{
> - struct i915_vma *vma = list_entry(eb->vmas.prev, typeof(*vma), exec_list);
> -
> - /*
> - * SNA is doing fancy tricks with compressing batch buffers, which leads
> - * to negative relocation deltas. Usually that works out ok since the
> - * relocate address is still positive, except when the batch is placed
> - * very low in the GTT. Ensure this doesn't happen.
> - *
> - * Note that actual hangs have only been observed on gen7, but for
> - * paranoia do it everywhere.
> - */
> - if ((vma->exec_entry->flags & EXEC_OBJECT_PINNED) == 0)
> - vma->exec_entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
> -
> - return vma->obj;
> -}
> -
> #define I915_USER_RINGS (4)
>
> static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = {
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
