Am 29.09.20 um 11:14 schrieb Daniel Vetter:
On Mon, Sep 28, 2020 at 01:22:13PM +0200, Christian König wrote:
Am 28.09.20 um 09:37 schrieb Thomas Zimmermann:
Hi
Am 28.09.20 um 08:50 schrieb Christian König:
Am 27.09.20 um 21:16 schrieb Sam Ravnborg:
Hi Thomas.
struct simap {
union {
void __iomem *vaddr_iomem;
void *vaddr;
};
bool is_iomem;
};
Where simap is a shorthand for system_iomem_map
And it could al be stuffed into a include/linux/simap.h file.
Not totally sold on the simap name - but wanted to come up with
something.
Yes. Others, myself included, have suggested to use a name that does not
imply a connection to the dma-buf framework, but no one has come up with
a good name.
I strongly dislike simap, as it's entirely non-obvious what it does.
dma-buf-map is not actually wrong. The structures represents the mapping
of a dma-able buffer in most cases.
With this approach users do not have to pull in dma-buf to use it and
users will not confuse that this is only for dma-buf usage.
There's no need to enable dma-buf. It's all in the header file without
dependencies on dma-buf. It's really just the name.
But there's something else to take into account. The whole issue here is
that the buffer is disconnected from its originating driver, so we don't
know which kind of memory ops we have to use. Thinking about it, I
realized that no one else seemed to have this problem until now.
Otherwise there would be a solution already. So maybe the dma-buf
framework *is* the native use case for this data structure.
We have at least:
linux/fb.h:
union {
char __iomem *screen_base; /* Virtual address */
char *screen_buffer;
};
Which solve more or less the same problem.
I thought this was for convenience. The important is_iomem bit is missing.
I also already noted that in TTM we have exactly the same problem and a
whole bunch of helpers to allow operations on those pointers.
How do you call this within TTM?
ttm_bus_placement, but I really don't like that name.
The data structure represents a pointer to either system or I/O memory,
but not necessatrily device memory. It contains raw data. That would
give something like
struct databuf_map
struct databuf_ptr
struct dbuf_map
struct dbuf_ptr
My favorite would be dbuf_ptr. It's short and the API names would make
sense: dbuf_ptr_clear() for clearing, dbuf_ptr_set_vaddr() to set an
address, dbuf_ptr_incr() to increment, etc. Also, the _ptr indicates
that it's a single address; not an offset with length.
Puh, no idea. All of that doesn't sound like it 100% hits the underlying
meaning of the structure.
Imo first let's merge this and then second with more users we might come
up with a better name. And cocci is fairly good at large-scale rename, to
the point where we manged to rename struct fence to struct dma_fence.
Agreed, renaming things later on is easy if somebody comes up with
something better.
But blocking a necessary technical change just because of the naming is
usually not a good idea.
Christian.
Also this entire thread here imo shows that we haven't yet figured out the
perfect name anyway, and I don't think it's worth it to hold this up
because of this bikeshed :-)
Cheers, Daniel
Christian.
Best regards
Thomas
Christian.
Anyway, if a better name than dma-buf-map comes in, I'm willing to
rename the thing. Otherwise I intend to merge the patchset by the end of
the week.
Well, the main thing is that I think this shoud be moved away from
dma-buf. But if indeed dma-buf is the only relevant user in drm then
I am totally fine with the current naming.
One alternative named that popped up in my head: struct sys_io_map {}
But again, if this is kept in dma-buf then I am fine with the current
naming.
In other words, if you continue to think this is mostly a dma-buf
thing all three patches are:
Acked-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
Sam
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7C71c630a7ca1d48476eed08d864581e4f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637369676925032848&sdata=CsekzASvj2lY%2B74FIiLc9B5QG7AHma8B2M5y8Cassj4%3D&reserved=0
