Quoting Tvrtko Ursulin (2020-06-09 08:47:00)
>
> On 07/06/2020 23:20, Chris Wilson wrote:
> > Over the next couple of patches, we will want to lock all the modified
> > vma for relocation processing under a single ww_mutex. We neither want
> > to have to include the vma that are skipped (due to no modifications
> > required) nor do we want those to be marked as written too. So separate
> > out the reloc validation into an early step, which we can use both to
> > reject the execbuf before committing to making our changes, and to
> > filter out the unmodified vma.
> >
> > This does introduce a second pass through the reloc[], but only if we
> > need to emit relocations.
> >
> > v2: reuse the outer loop, not cut'n'paste.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> > .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 145 +++++++++++-------
> > 1 file changed, 86 insertions(+), 59 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > index 23db79b806db..01ab1e15a142 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > @@ -911,9 +911,9 @@ static void eb_destroy(const struct i915_execbuffer *eb)
> >
> > static inline u64
> > relocation_target(const struct drm_i915_gem_relocation_entry *reloc,
> > - const struct i915_vma *target)
> > + u64 target)
> > {
> > - return gen8_canonical_addr((int)reloc->delta + target->node.start);
> > + return gen8_canonical_addr((int)reloc->delta + target);
> > }
> >
> > static void reloc_cache_init(struct reloc_cache *cache,
> > @@ -1292,26 +1292,11 @@ static int __reloc_entry_gpu(struct i915_execbuffer *eb,
> > return 0;
> > }
> >
> > -static u64
> > -relocate_entry(struct i915_execbuffer *eb,
> > - struct i915_vma *vma,
> > - const struct drm_i915_gem_relocation_entry *reloc,
> > - const struct i915_vma *target)
> > -{
> > - u64 target_addr = relocation_target(reloc, target);
> > - int err;
> > -
> > - err = __reloc_entry_gpu(eb, vma, reloc->offset, target_addr);
> > - if (err)
> > - return err;
> > -
> > - return target->node.start | UPDATE;
> > -}
> > -
> > -static u64
> > -eb_relocate_entry(struct i915_execbuffer *eb,
> > - struct eb_vma *ev,
> > - const struct drm_i915_gem_relocation_entry *reloc)
> > +static int
> > +eb_reloc_prepare(struct i915_execbuffer *eb,
> > + struct eb_vma *ev,
> > + const struct drm_i915_gem_relocation_entry *reloc,
> > + struct drm_i915_gem_relocation_entry __user *user)
> > {
> > struct drm_i915_private *i915 = eb->i915;
> > struct eb_vma *target;
> > @@ -1389,6 +1374,32 @@ eb_relocate_entry(struct i915_execbuffer *eb,
> > return -EINVAL;
> > }
> >
> > + return 1;
> > +}
> > +
> > +static int
> > +eb_reloc_entry(struct i915_execbuffer *eb,
> > + struct eb_vma *ev,
> > + const struct drm_i915_gem_relocation_entry *reloc,
> > + struct drm_i915_gem_relocation_entry __user *user)
> > +{
> > + struct eb_vma *target;
> > + u64 offset;
> > + int err;
> > +
> > + /* we've already hold a reference to all valid objects */
> > + target = eb_get_vma(eb, reloc->target_handle);
> > + if (unlikely(!target))
> > + return -ENOENT;
> > +
> > + /*
> > + * If the relocation already has the right value in it, no
> > + * more work needs to be done.
> > + */
> > + offset = gen8_canonical_addr(target->vma->node.start);
> > + if (offset == reloc->presumed_offset) > + return 0;
> > +
>
> Haven't these reloc entries been removed from the list in the prepare phase?
No, we don't adjust the user reloc arrays, we only skip entire objects
that do not require relocs.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
