Am 01.12.20 um 13:53 schrieb Thomas Zimmermann:
Hi
Am 01.12.20 um 13:51 schrieb Thomas Zimmermann:
Hi
Am 01.12.20 um 13:38 schrieb Christian König:
Am 01.12.20 um 13:33 schrieb Thomas Zimmermann:
Hi
Am 01.12.20 um 13:14 schrieb Christian König:
Am 01.12.20 um 12:30 schrieb Thomas Zimmermann:
Hi
Am 01.12.20 um 11:34 schrieb Christian König:
[...]
In patch 6 of this series, there's ast cursor code that
acquires two BO's reservation locks and vmaps them afterwards.
That's probably how you intend to use dma_buf_vmap_local.
However, I think it's more logically to have a vmap callback
that only does the actual vmap. This is all that exporters
would have to implement.
And with that, build one helper that pins before vmap and one
helper that gets the resv lock.
I don't think that this is will work nor is it a good approach.
See the ast cursor handling for example. You need to acquire two
BOs here, not just one. And this can't be done cleanly with a
single vmap call.
That seems to be a misunderstanding.
I don't mentioned it explicitly, but there's of course another
helper that only vmaps and nothing else. This would be useful for
cases like the cursor code. So there would be:
dma_buf_vmap() - pin + vmap
dma_buf_vmap_local() - lock + vmap
dma_buf_vmap_locked() - only vmap; caller has set up the BOs
Well that zoo of helpers will certainly get a NAK from my side.
See interfaces like this should implement simple functions and not
hide what's actually needs to be done inside the drivers using
this interface.
If 9 of 10 invocations use the same pattern, why not put that
pattern in a helper? I see nothing wrong with that.
Because it hides the locking semantics inside the helper. See when
you have the lock/unlock inside the driver it is obvious that you
need to be careful not to take locks in different orders.
What we could do is to add a pin count to the DMA-buf and then do
WARN_ON(dma_buf->pin_count || dma_resv_lock_help(dma_buf->resv))
in the vmap/vunmap calls.
Most of the vmap code is either CMA or SHMEM GEM stuff. They don't
need to pin. It's just baggage to them. The TTM stuff that does
need pinning is the minority.
I did some conversion of drivers that use vram and shmem. They
occasionally update a buffer (ast cursors) or flush a BO from
system memory to HW (udl, cirrus, mgag200). In terms of these 3
interfaces: I never needed dma_buf_vmap() because pinning was
never really required here. Almost all of the cases were handled
by dma_buf_vmap_local(). And the ast cursor code uses the
equivalent of dma_buf_vmap_locked().
Yeah, that is kind of expected. I was already wondering as well
why we didn't used the reservation lock more extensively.
As a side note, I found only 6 trivial implementations of vmap
outside of drivers/gpu/drm. I cannot find a single implementation
of pin there. What am I missing?
Amdgpu is the only one currently implementing the new interface. So
far we didn't had the time nor the need to correctly move the
locking into the calling drivers.
That's what the whole dynamic DMA-buf patches where all about.
Thanks for the pointer.
That was not a snarky comment, although it might sound like one. I
found the series in my inbox. :)
It wasn't recognized as such. And just to be clear your work here is
extremely welcomed.
Regards,
Christian.
Best regards
Thomas
Best regards
Thomas
Regards,
Christian.
Best regards
Thomas
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
