Messages in this thread | | | Date | Thu, 27 Jan 2022 08:59:36 +0100 | Subject | Re: [PATCH 01/19] dma-buf-map: Add read/write helpers | From | Christian König <> |
| |
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.
Regards, Christian.
> Patches > later in this series do this for the GuC ads. > > Matt > >> Regards, >> Christian. >> >>> #endif /* __DMA_BUF_MAP_H__ */
| |