Re: [Intel-gfx] [PATCH 4/4] drm/i915: Implement SINGLE_TIMELINE with a syncobj

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux