Re: [PATCH 4/7] drm/radeon: Pin buffers while they are vmap'ed

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

 



Am 26.11.20 um 13:14 schrieb Thomas Zimmermann:
Hi

Am 26.11.20 um 13:08 schrieb Christian König:
[...]
Yeah, I remember a bug report about frequent page-table modifications wrt to vram helpers. So we implemented the lazy unmapping / vmap caching, as suggested by Christian back them. My guess is that anything TTM-based can use a similar pattern. Christian probably knows the corner cases.

My memory is failing me, what corner case are you talking about?

Haha! :D What I meant was that if there were corner cases, you'd know about them. From your comment, I guess there are none.

Ah, ok :) I was wondering for a moment if I have missed something.

Cheers,
Christian.


Best regards
Thomas


Christian.


CMA seems obviously working correctly already.

For SHMEM, I'd have to figure out the reference counting of the pages involved. My guess is that the vunmap in drm_gem_shmem_vunmap() could be moved into the free callback, plus a few other modifications.

Best regards
Thomas


But if you're willing to do all that conversion of callers, then of
course I'm not stopping you. Not at all, it's great to see that kind
of maze untangled.
-Daniel


Best regards
Thomas


That should give us at least some way forward to gradually conver all the
drivers and helpers over to dma_resv locking.
-Daniel

The pin count is currently maintained by the vmap implementation in vram helpers. Calling vmap is an implicit pin; calling vunmap is an implicit unpin. This prevents eviction in the damage worker. But now I was told than pinning is only for BOs that are controlled by userspace and internal users should acquire the resv lock. So vram helpers have to be fixed, actually.

In vram helpers, unmapping does not mean eviction. The unmap operation only marks the BO as unmappable. The real unmap happens when the eviction takes place. This avoids many modifications to the page tables. IOW an unpinned, unmapped BO will remain in VRAM until the memory is actually needed.

Best regards
Thomas


So I'm still not seeing how this can go boom.

Now long term it'd be nice to cut everything over to dma_resv locking, but the issue there is that beyond ttm, none of the helpers (and few of the drivers) use dma_resv. So this is a fairly big uphill battle. Quick
interim fix seems like the right solution to me.
-Daniel


Regards,
Christian.


Best regards
Thomas


There's no recursion taking place, so I guess the reservation
lock could be
acquired/release in drm_client_buffer_vmap/vunmap(), or a
separate pair of
DRM client functions could do the locking.

Given how this "do the right locking" is a can of worms (and I think
it's
worse than what you dug out already) I think the fb_helper.lock hack is
perfectly good enough.

I'm also somewhat worried that starting to use dma_resv lock in generic code, while many helpers/drivers still have their hand-rolled locking, will make conversion over to dma_resv needlessly more complicated.
-Daniel


Best regards
Thomas

[1] https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/drm_fb_helper.c?id=ac60f3f3090115d21f028bffa2dcfb67f695c4f2#n394


Please note that the reservation lock you need to take here is part of
the GEM object.

Usually we design things in the way that the code needs to take a lock which protects an object, then do some operations with the object and
then release the lock again.

Having in the lock inside the operation can be done as well, but
returning with it is kind of unusual design.

Sorry for the noob questions. I'm still trying to understand the
implications of acquiring these locks.

Well this is the reservation lock of the GEM object we are
talking about
here. We need to take that for a couple of different operations,
vmap/vunmap doesn't sound like a special case to me.

Regards,
Christian.


Best regards
Thomas

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer










--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer







--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer





_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux