[Intel-gfx] [PATCH v4] drm/i915: Add soft-pinning API for execbuffer

[Chris Wilson]

On Wed, Jul 15, 2015 at 03:58:33PM +0000, Daniel, Thomas wrote:
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> > Sent: Wednesday, July 15, 2015 4:47 PM
> > To: Daniel, Thomas
> > Cc: Goel, Akash; intel-gfx at lists.freedesktop.org; Belgaumkar, Vinay; Winiarski,
> > Michal; Zou, Nanhai
> > Subject: Re: [PATCH v4] drm/i915: Add soft-pinning API for execbuffer
> > 
> > On Wed, Jul 15, 2015 at 03:41:49PM +0000, Daniel, Thomas wrote:
> > > > -----Original Message-----
> > > > From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> > > > Sent: Wednesday, July 15, 2015 4:06 PM
> > > > To: Goel, Akash
> > > > Cc: Daniel, Thomas; intel-gfx at lists.freedesktop.org; Belgaumkar, Vinay;
> > > > Winiarski, Michal; Zou, Nanhai
> > > > Subject: Re: [PATCH v4] drm/i915: Add soft-pinning API for execbuffer
> > > >
> > > > On Wed, Jul 15, 2015 at 08:25:23PM +0530, Goel, Akash wrote:
> > > > > >>+int
> > > > > >>+i915_gem_evict_for_vma(struct i915_vma *target)
> > > > > >>+{
> > > > > >>+	struct drm_mm_node *node, *next;
> > > > > >>+
> > > > > >>+	list_for_each_entry_safe(node, next,
> > > > > >>+			&target->vm->mm.head_node.node_list,
> > > > > >>+			node_list) {
> > > > > >>+		struct i915_vma *vma;
> > > > > >>+		int ret;
> > > > > >>+
> > > > > >>+		if (node->start + node->size <= target->node.start)
> > > > > >>+			continue;
> > > > > >>+		if (node->start >= target->node.start + target-
> > >node.size)
> > > > > >>+			break;
> > > > > >>+
> > > > > >>+		vma = container_of(node, typeof(*vma), node);
> > > > > >>+
> > > > > >>+		if (vma->pin_count) {
> > > > > >>+			/* We may need to evict a buffer in the same
> > batch */
> > > > > >>+			if (!vma->exec_entry)
> > > > > >>+				return -EBUSY;
> > > > > >>+
> > > > > >>+			if (vma->exec_entry->flags &
> > EXEC_OBJECT_PINNED)
> > > > > >>+				/* Overlapping fixed objects in the
> > same batch
> > > > > >>*/
> > > > > >>+				return -EINVAL;
> > > > > >>+
> > > > > >>+			return -ENOSPC;
> > > > >
> > > > > Can we actually hit this condition, considering the soft pinned
> > > > > objects are now on the front side of 'eb->vmas' list ?
> > > > > If we do encounter such a case, it probably means that the
> > > > > overlapping object is already pinned from some other path.
> > > >
> > > > Note that softpinned objects are only first on the second pass through
> > > > the reservation.
> > > Eh?  I modified i915_gem_execbuffer_reserve() to always put the softpinned
> > vmas first so they should never collide with objects in the same execbuff.
> > 
> > That would be a severe performance penalty for the usual case where we
> > only have to run once through the list.
> How so?  execbuffer_reserve() already traverses the entire vma list and does a list_move or list_move_tail onto ordered_vmas for each entry.  I've simply created a new list for the softpinned vmas and spliced the two together at the end.

I posted the fix for that a few months ago.

> > > This contains special stuff which I don't have visibility of (mm.interval_tree,
> > vma.active).
> > 
> > Yes. I mentioned that using softpin introduced some perf regressions.
> > -Chris
> Can we get your improvements as follow-up patches?

No. Softpin has requirements that are not met by the current kernel.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre