Quoting Tvrtko Ursulin (2019-03-05 15:54:25)
>
> On 01/03/2019 14:03, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> > index 9d111eedad5a..e0807a61dcf4 100644
> > --- a/drivers/gpu/drm/i915/i915_request.c
> > +++ b/drivers/gpu/drm/i915/i915_request.c
> > @@ -1045,8 +1045,14 @@ void i915_request_add(struct i915_request *request)
> > prev = i915_active_request_raw(&timeline->last_request,
> > &request->i915->drm.struct_mutex);
> > if (prev && !i915_request_completed(prev)) {
> > - i915_sw_fence_await_sw_fence(&request->submit, &prev->submit,
> > - &request->submitq);
> > + if (is_power_of_2(prev->engine->mask | engine->mask))
> > + i915_sw_fence_await_sw_fence(&request->submit,
> > + &prev->submit,
> > + &request->submitq);
> > + else
> > + __i915_sw_fence_await_dma_fence(&request->submit,
> > + &prev->fence,
> > + &request->dmaq);
>
> Drop a comment here explaining what's happening in this if block.
>
> The subtlety of why we need a special flavours of await helper, new
> which use the builtin call back storage, vs using the existing ones
> which allocate that, totally escapes me at the moment.
>
> It's probably a good idea to put a paragraph in the commit message
> explaining what new sw fence facility needs to be added to implement
> this and why.
Alternatively, we could do add this fence during request_alloc and use
an actual malloc rather than rely on the embedded struct. We still have
to bypass the usual await call as that filters out awaits on the same
timeline (because we know we have this dependency). But since we always
have to to this allocation, why not keep on embedding it in the request
itself.
I'm leaning towards keeping the embedded fence for tracking the
dependency along the timeline (rather than kmalloc) as surely there will
be others later. Surely.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
