On Wed, Jun 26, 2019 at 1:53 PM Christian König <ckoenig.leichtzumerken@xxxxxxxxx> wrote: > > [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. Hm maybe wasn't clear what I had in mind. Roughly this: diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 435d02f719a8..33bb4eabe2eb 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -214,8 +214,6 @@ struct ttm_buffer_object { * Members protected by a bo reservation. */ - struct dma_fence *moving; - struct drm_vma_offset_node vma_node; unsigned priority; diff --git a/include/linux/reservation.h b/include/linux/reservation.h index 644a22dbe53b..1c8a8bd077a2 100644 --- a/include/linux/reservation.h +++ b/include/linux/reservation.h @@ -74,6 +74,8 @@ struct reservation_object { seqcount_t seq; struct dma_fence __rcu *fence_excl; + + struct dma_fence _rcu *moving; struct reservation_object_list __rcu *fence; }; Plus all the sorted fiddling. With the idea that we'd extract a bunch of these over. Essentially instead of ttm, amdgpu and i915 all having different ways to track the bo move fences, standardize on one. > > 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. Hm I guess I'll wait for you to untangel that then. -Daniel > [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. -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel