On Thu, Jan 27, 2022 at 08:59:36AM +0100, Christian König wrote:
Am 27.01.22 um 08:36 schrieb Matthew Brost:
[SNIP]
/** * dma_buf_map_memcpy_to - Memcpy into dma-buf mapping * @dst: The dma-buf mapping structure @@ -263,4 +304,44 @@ static inline void dma_buf_map_incr(struct dma_buf_map *map, size_t incr) map->vaddr += incr; } +/**
- dma_buf_map_read_field - Read struct member from dma-buf mapping with
- arbitrary size and handling un-aligned accesses
- @map__: The dma-buf mapping structure
- @type__: The struct to be used containing the field to read
- @field__: Member from struct we want to read
- Read a value from dma-buf mapping calculating the offset and size: this assumes
- the dma-buf mapping is aligned with a a struct type__. A single u8, u16, u32
- or u64 can be read, based on the offset and size of type__.field__.
- */
+#define dma_buf_map_read_field(map__, type__, field__) ({ \
- type__ *t__; \
- typeof(t__->field__) val__; \
- dma_buf_map_memcpy_from_offset(&val__, map__, offsetof(type__, field__), \
sizeof(t__->field__)); \- val__; \
+})
+/**
- dma_buf_map_write_field - Write struct member to the dma-buf mapping with
- arbitrary size and handling un-aligned accesses
- @map__: The dma-buf mapping structure
- @type__: The struct to be used containing the field to write
- @field__: Member from struct we want to write
- @val__: Value to be written
- Write a value to the dma-buf mapping calculating the offset and size.
- A single u8, u16, u32 or u64 can be written based on the offset and size of
- type__.field__.
- */
+#define dma_buf_map_write_field(map__, type__, field__, val__) ({ \
- type__ *t__; \
- typeof(t__->field__) val____ = val__; \
- dma_buf_map_memcpy_to_offset(map__, offsetof(type__, field__), \
&val____, sizeof(t__->field__)); \+})
Uff well that absolutely looks like overkill to me.
Hold on...
That's a rather special use case as far as I can see and I think we should only have this in the common framework if more than one driver is using it.
I disagree, this is rather elegant.
The i915 can't be the *only* driver that defines a struct which describes the layout of a dma_buf object.
That's not the problem, amdgpu as well as nouveau are doing that as well. The problem is DMA-buf is a buffer sharing framework between drivers.
In other words which importer is supposed to use this with a DMA-buf exported by another device?
IMO this base macro allows *all* other drivers to build on this write directly to fields in structures those drivers have defined.
Exactly that's the point. This is something drivers should absolutely *NOT* do.
That are driver internals and it is extremely questionable to move this into the common framework.
See my other reply.
This is about struct dma_buf_map, which is just a tagged pointer.
Which happens to be used by the dma_buf cross-driver interface, but it's also used plenty internally in buffer allocation helpers, fbdev, everything else. And it was _meant_ to be used like that - this thing is my idea, I know :-)
I guess we could move/rename it, but like I said I really don't have any good ideas. Got some? -Daniel