On 22/03/2021 16:10, Jason Ekstrand wrote:
On Mon, Mar 22, 2021 at 7:28 AM Tvrtko Ursulin
[snip]
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 96403130a373d..2c56796f6a71b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -3295,6 +3295,15 @@ i915_gem_do_execbuffer(struct drm_device *dev,
goto err_vma;
}
+ if (eb.gem_context->syncobj) {
+ struct dma_fence *fence;
+
+ fence = drm_syncobj_fence_get(eb.gem_context->syncobj);
Who drops this reference?
i915_request_await_dma_fence() below consumes a reference.
Not sure, please check on difference wrt input fence handling.
+ err = i915_request_await_dma_fence(eb.request, fence);
+ if (err)
+ goto err_ext;
+ }
+
if (in_fence) {
if (args->flags & I915_EXEC_FENCE_SUBMIT)
err = i915_request_await_execution(eb.request,
@@ -3351,6 +3360,12 @@ i915_gem_do_execbuffer(struct drm_device *dev,
fput(out_fence->file);
}
}
+
+ if (eb.gem_context->syncobj) {
+ drm_syncobj_replace_fence(eb.gem_context->syncobj,
+ &eb.request->fence);
+ }
+
i915_request_put(eb.request);
err_vma:
So essentially moving the synchronisation to top level which is extra
work, but given limited and questionable usage of the uapi may be
acceptable. Need full picture on motivation to understand.
For one thing, the GuC scheduler doesn't natively have a concept of
"timelines" which can be shared like this. To work with the GuC
Confused - neither does execlists. It is handled in common layer in
i915. GuC scheduler has the same concept of one hw context is one
timeline because that is the hw concept and not backend specific.
scheduler as currently proposed in DII, they've asked the media driver
to stop using this flag in favor of passing a sync file from batch to
batch. If we want to slide GuC scheduling in smoothly, we've got to
keep it working. This means either making timelines a concept there
or doing an emulation like this.
Hm not aware and don't see that GuC backend can't or doesn't implement
this. Perhaps this would be best discussed once GuC patches are posted.
Semantics are also not 1:1 since dma fence context will be different.
Could you elaborate?
Exported dma fence context as an "timeline" id will be single with the
current patch and multiple contexts with this implementation.
Daniel also raised another difference caused by lack of serialisation
due multiple tl->mutex here.
I don't think this is important, it was never part of a contract what
happens with racing execbufs, but it is definitely required covering
both topics in the commit message.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
