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 <[email protected]>
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 <[email protected]> > --- > 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 > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
