Re: [PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

From: Daniel Vetter <daniel@ffwll.ch>
To: "Michel Dänzer" <michel@daenzer.net>
Cc: dri-devel <dri-devel@lists.freedesktop.org>,
	amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.
Date: Mon, 26 Sep 2016 10:04:19 +0200	[thread overview]
Message-ID: <20160926080419.GV20761@phenom.ffwll.local> (raw)
In-Reply-To: <8c21f70a-7f96-480e-9784-93f2d48f52bb@daenzer.net>

On Mon, Sep 26, 2016 at 09:48:37AM +0900, Michel Dänzer wrote:
> On 23/09/16 09:09 PM, Daniel Vetter wrote:
> > On Fri, Sep 23, 2016 at 07:00:25PM +0900, Michel Dänzer wrote:
> >> On 22/09/16 10:22 PM, Christian König wrote:
> >>> Am 22.09.2016 um 15:05 schrieb Daniel Vetter:
> >>>> 
> >>>> But the current approach in amdgpu_sync.c of declaring a fence as
> >>>> exclusive after the fact (if owners don't match) just isn't how
> >>>> reservation_object works. You can of course change that, but that
> >>>> means you must change all drivers implementing support for implicit
> >>>> fencing of dma-buf. Fixing amdgpu will be easier ;-)
> >>>
> >>> Well as far as I can see there is no way I can fix amdgpu in this case.
> >>>
> >>> The handling clearly needs to be changed on the receiving side of the
> >>> reservation objects if I don't completely want to disable concurrent
> >>> access to BOs in amdgpu.
> >>
> >> Anyway, we need a solution for this between radeon and amdgpu, and I
> >> don't think a solution which involves those drivers using reservation
> >> object semantics between them which are different from all other drivers
> >> is a good idea.
> > 
> > Afaik there's also amd+intel machines out there,
> 
> Sure, what I meant was that even if we didn't care about those (which we
> do), we'd still need a solution between our own drivers.
> 
> 
> > so really the only option is to either fix amdgpu to correctly set
> > exclusive fences on shared buffers (with the help of userspace hints).
> > Or change all the existing drivers.
> 
> I got some fresh perspective on the problem while taking a walk, and I'm
> now fairly convinced that neither amdgpu userspace nor other drivers
> need to be modified:
> 
> It occurred to me that all the information we need is already there in
> the exclusive and shared fences set by amdgpu. We just need to use it
> differently to match the expectations of other drivers.
> 
> We should be able to set some sort of "pseudo" fence as the exclusive
> fence of the reservation object. When we are asked by another driver to
> wait for this fence to signal, we take the current "real" exclusive
> fence (which we can keep track of e.g. in our BO struct) and shared
> fences, and wait for all of those to signal; then we mark the "pseudo"
> exclusive fence as signalled.
> 
> Am I missing anything which would prevent this from working?

One thing to make sure is that you don't change the (real, private stored)
fences you're waiting on over the lifetime of the public exclusive fence.
That might lead to some hilarity wrt potentially creating fence depency
loops. But I think as long as you guarantee that the private internal
fences are always amdgpu ones, and never anything imported from a
different driver even that should be fine. Not because this would break
the loops, but since amgpud has a hangcheck it can still gurantee that the
fence eventually fires even if there is a loop.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel