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
