On 11/20/19 1:24 PM, Christian König wrote: > Am 20.11.19 um 13:19 schrieb Daniel Vetter: >> On Wed, Nov 20, 2019 at 1:09 PM Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: >>> On Wed, Nov 20, 2019 at 1:02 PM Christian König >>> <christian.koenig@xxxxxxx> wrote: >>>>> What am I missing? >>>> The assumption is that when you want to create a vmap of a DMA-buf >>>> buffer the buffer needs to be pinned somehow. >>>> >>>> E.g. without dynamic dma-buf handling you would need to have an active >>>> attachment. With dynamic handling the requirements could be lowered to >>>> using the pin()/unpin() callbacks. >>> Yeah right now everyone seems to have an attachment, and that's how we >>> get away with all this. But the interface isn't supposed to work like >>> that, dma_buf_vmap/unmap is supposed to be a stand-alone thing (you >>> can call it directly on the struct dma_buf, no need for an >>> attachment). Also I don't think non-dynamic drivers should ever call >>> pin/unpin, not their job, holding onto a mapping should be enough to >>> get things pinned. >>> >>>> You also can't lock/unlock inside your vmap callback because you don't >>>> have any guarantee that the pointer stays valid as soon as your drop >>>> your lock. >>> Well that's why I asked where the pin/unpin pair is. If you lock&pin, >>> then you do know that the pointer will stay around. But looks like the >>> original patch from Dave for ttm based drivers played fast&loose here >>> with what should be done. >>> >>>> BTW: What is vmap() still used for? >>> udl, bunch of other things (e.g. bunch of tiny drivers). Not much, but >>> not stuff we can drop. >> If we're unlucky we'll actually have a problem with these now. For >> some of these the attached device is not dma-capable, so dma_map_sg >> will go boom. With the cached mapping logic we now have this might go >> sideways for dynamic exporters. Did you test your dynamic dma-buf >> support for amdgpu with udl? > Short answer no, not at all. Long one: What the heck is udl? And how is > it not dma-capable? > >> Worst case we need to get rid of the fake >> attachment, fix the vmap locking/pinning, and maybe some more >> headaches to sort this out. > Well of hand we could require that vmap will also pin a DMA-buf and > start fixing amgpu/nouveau/radeon/qxl. Perhaps with dynamic dma-bufs it might be possible to do something similar to vmwgfx (and recently other?) fbdev: The map is cached, but may be invalidated as soon as we release dma_resv / unpin. (move_notify() unmaps if needed). So each time it's needed we make sure we're locked / pinned and then call a map_validate() function. Typically the map is still around. If it isn't, the map_validate() function sets it up again. Saves a bunch of vmap() calls or the need for persistent pinning for performance reasons. /Thomas > > Christian. > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
