Hi Andrzej,
On Mon, Oct 24, 2022 at 04:05:57PM +0200, Andrzej Hajda wrote:
> On 21.10.2022 17:39, Andi Shyti wrote:
> > Hi Andrzej,
> >
> > [...]
> >
> > > +static inline int __must_check
> > > +igt_vma_move_to_active_unlocked(struct i915_vma *vma, struct i915_request *rq,
> > > + unsigned int flags)
> > > +{
> > > + int err;
> > > +
> > > + i915_vma_lock(vma);
> > > + err = _i915_vma_move_to_active(vma, rq, &rq->fence, flags);
> > > + i915_vma_unlock(vma);
> > > + return err;
> > > +}
> > > +
> >
> > there are calls to i915_vma_move_to_active also outside
> > selftests, why not having a i915_move_to_active_unlocked() in
> > i915_vma.h?
>
> As I said before, Chris suggested real users of this call should use locking
> explicitly.
Yeah, sure... I was just thinking about it... no big opinion,
besides I don't hink my proposal in Patch 1 makes things easier.
> > Besides here you break also the bisect, because between patch 1
> > and 2 the i915_move_to_avtive would also call
> > i915_request_await_object(). Right or am I getting confused?
>
> Hmm, looking at v2, I do not see breakage. Patch 1 moves all occurrences of
> i915_request_await_object inside i915_vma_move_to_active.
> Patch 2, just replaces sequence of calls with call to new helper.
Are you sure?
I might be getting confused, but in Patch 1
"i915_vma_move_to_active()" takes "i915_request_await_object()"
inside. This affects all the calls to "i915_vma_move_to_active()"
in the selftests that are not actually requesting
"i915_request_await_object()".
We need to wait for Patch 2 in order to have a local redefinition
of "i915_vma_move_to_active()" for those selftests.
Andi
