Re: [PATCH 0/3] dma-buf: Flag vmap'ed memory as system or I/O memory

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

 



Am 17.09.20 um 09:16 schrieb Thomas Zimmermann:
Hi Christian and Thomas

Am 16.09.20 um 15:37 schrieb Thomas Hellström (Intel):
On 9/16/20 2:59 PM, Christian König wrote:
Am 16.09.20 um 14:24 schrieb Daniel Vetter:
On Wed, Sep 16, 2020 at 12:48:20PM +0200, Thomas Zimmermann wrote:
Hi

Am 16.09.20 um 11:37 schrieb Daniel Vetter:
On Mon, Sep 14, 2020 at 01:25:18PM +0200, Thomas Zimmermann wrote:
Dma-buf provides vmap() and vunmap() for retrieving and releasing
mappings
of dma-buf memory in kernel address space. The functions operate
with plain
addresses and the assumption is that the memory can be accessed
with load
and store operations. This is not the case on some architectures
(e.g.,
sparc64) where I/O memory can only be accessed with dedicated
instructions.

This patchset introduces struct dma_buf_map, which contains the
address of
a buffer and a flag that tells whether system- or I/O-memory
instructions
are required.

Some background: updating the DRM framebuffer console on sparc64
makes the
kernel panic. This is because the framebuffer memory cannot be
accessed with
system-memory instructions. We currently employ a workaround in
DRM to
address this specific problem. [1]

To resolve the problem, we'd like to address it at the most common
point,
which is the dma-buf framework. The dma-buf mapping ideally knows
if I/O
instructions are required and exports this information to it's
users. The
new structure struct dma_buf_map stores the buffer address and a
flag that
signals I/O memory. Affected users of the buffer (e.g., drivers,
frameworks)
can then access the memory accordingly.

This patchset only introduces struct dma_buf_map, and updates
struct dma_buf
and it's interfaces. Further patches can update dma-buf users. For
example,
there's a prototype patchset for DRM that fixes the framebuffer
problem. [2]

Further work: TTM, one of DRM's memory managers, already exports an
is_iomem flag of its own. It could later be switched over to
exporting struct
dma_buf_map, thus simplifying some code. Several DRM drivers
expect their
fbdev console to operate on I/O memory. These could possibly be
switched over
to the generic fbdev emulation, as soon as the generic code uses
struct
dma_buf_map.

[1]
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20200725191012.GA434957%40ravnborg.org%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7C04e3cc3e03ae40f1fa0f08d85a3b6a68%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637358558524732385&sdata=wTmFuB95GhKUU%2F2Q91V0%2BtzAu4%2BEe3VBUcriBy3jx2g%3D&reserved=0

[2]
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20200806085239.4606-1-tzimmermann%40suse.de%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7C04e3cc3e03ae40f1fa0f08d85a3b6a68%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637358558524732385&sdata=L4rBHmegO63b%2FiTQdTyH158KNxAZwSuJCQOaFszo5L0%3D&reserved=0

lgtm, imo ready to convert the follow-up patches over to this. But
I think
would be good to get at least some ack from the ttm side for the
overall
plan.
Yup, it would be nice if TTM could had out these types automatically.
Then all TTM-based drivers would automatically support it.

Also, I think we should put all the various helpers (writel/readl,
memset,
memcpy, whatever else) into the dma-buf-map.h helper, so that most
code
using this can just treat it as an abstract pointer type and never
look
underneath it.
We have some framebuffer helpers that rely on pointer arithmetic, so
we'd need that too. No big deal wrt code, but I was worried about the
overhead. If a loop goes over framebuffer memory, there's an if/else
branch for each access to the memory buffer.
If we make all the helpers static inline, then the compiler should be
able
to see that dma_buf_map.is_iomem is always the same, and produced really
optimized code for it by pulling that check out from all the loops.

So should only result in somewhat verbose code of having to call
dma_buf_map pointer arthimetic helpers, but not in bad generated code.
Still worth double-checking I think, since e.g. on x86 the generated
code
should be the same for both cases (but maybe the compiler doesn't see
through the inline asm to realize that, so we might end up with 2
copies).
Can we have that even independent of DMA-buf? We have essentially the
same problem in TTM and the code around that is a complete mess if you
ask me.

Christian.

I think this patchset looks good. Changing ttm_bo_kmap() over to
returning a struct dma-buf-map would probably work just fine. If we then
can have a set of helpers to operate on it, that's great.

/Thomas
Can I count this as an A-b by one of you?

For the the general approach, certainly yes.

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux