Re: [Intel-gfx] [PATCH 1/6] dma-buf: add dynamic DMA-buf handling v12

["Christian König" <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[SNIP]
>>>>> I'm confused here: Atm ->moving isn't in resv_obj, there's only one 
>>> exclusive fence. And yes you need to set that every time you do a move
>>> (because a move needs to be pretty exclusive access). But I'm not seeing a
>>> separate not_quite_exclusive fence slot for moves.
>> Yeah, but shouldn't that be sufficient? I mean why does somebody else
>> than the exporter needs to know when a BO is moving?
> I think for buffer sharing there's not much use for this, but it
> sounded like you want to use ->move_notify also more internally. And
> in that case, for vk, you want to be able to ignore the implicit
> fences as much as possible. But you can't ignore the buffer moves ofc.
> Hence tracking those separate could be useful.

Yeah, but for this case I can still rely on using ttm_bo->moving. So no 
need to actually change that.

> amdgpu seems to be solving this internally by never attaching an
> exclusive fence for implicit stuff, or something like that, except
> when it's shared. But in general you need to assume a funky mix of
> implicit and explicit sync'ed workloads, and for those tracking the
> moves separately would be good.

Actually we have an "owner" for each fence which is basically a "void*" 
pointer.

If we see that a command submission is coming from the same "owner" we 
just avoid synchronization at all.

For buffer moves the owner is simply NULL (or some other special value), 
and so we always sync to those.

[SNIP]
>>>>> - You sound like you want to use this a lot more, even internally in
>>>>>      amdgpu. For that I do think the sepearate dma_fence just to make sure
>>>>>      the buffer is accessible will be needed in resv_obj.
>>>>>
>>>>> - Once we have ->moving I think there's some good chances to extract a bit
>>>>>      of the eviction/pipeline bo move boilerplate from ttm, and maybe use it
>>>>>      in other drivers. i915 could already make use of this in upstream, since
>>>>>      we already pipeline get_pages and clflush of buffers. Ofc once we have
>>>>>      vram support, even more useful.
>>>> I actually indeed wanted to add more stuff to the reservation object
>>>> implementation, like finally cleaning up the distinction of readers/writers.
>>> Hm, more details? Not ringing a bell ...
>> I'm not yet sure about the details either, so please just wait until I
>> solved that all up for me first.
> Ah is this about amdgpu doing something else for implicit sync than
> what's supposed to be done, and a bit a mismatch when you deal with
> shared buffers?

Yes, exactly.

>>>> And cleaning up the fence removal hack we have in the KFD for freed up BOs.
>>>> That would also allow for getting rid of this in the long term.
>>> Hm, what's that for?
>> When the KFD frees up memory it removes their eviction fence from the
>> reservation object instead of setting it as signaled and adding a new
>> one to all other used reservation objects.
> Oh so just a fast-path for destryoing memory that's in-flight in some move?

Yes exactly that again.

Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx