| Message ID | 20201015123806.32416-10-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Support GEM object mappings from I/O memory | expand |
Hi Thomas. On Thu, Oct 15, 2020 at 02:38:05PM +0200, Thomas Zimmermann wrote: > To do framebuffer updates, one needs memcpy from system memory and a > pointer-increment function. Add both interfaces with documentation. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Looks good. Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > --- > include/linux/dma-buf-map.h | 72 +++++++++++++++++++++++++++++++------ > 1 file changed, 62 insertions(+), 10 deletions(-) > > diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h > index 2e8bbecb5091..6ca0f304dda2 100644 > --- a/include/linux/dma-buf-map.h > +++ b/include/linux/dma-buf-map.h > @@ -32,6 +32,14 @@ > * accessing the buffer. Use the returned instance and the helper functions > * to access the buffer's memory in the correct way. > * > + * The type :c:type:`struct dma_buf_map <dma_buf_map>` and its helpers are > + * actually independent from the dma-buf infrastructure. When sharing buffers > + * among devices, drivers have to know the location of the memory to access > + * the buffers in a safe way. :c:type:`struct dma_buf_map <dma_buf_map>` > + * solves this problem for dma-buf and its users. If other drivers or > + * sub-systems require similar functionality, the type could be generalized > + * and moved to a more prominent header file. > + * > * Open-coding access to :c:type:`struct dma_buf_map <dma_buf_map>` is > * considered bad style. Rather then accessing its fields directly, use one > * of the provided helper functions, or implement your own. For example, > @@ -51,6 +59,14 @@ > * > * dma_buf_map_set_vaddr_iomem(&map. 0xdeadbeaf); > * > + * Instances of struct dma_buf_map do not have to be cleaned up, but > + * can be cleared to NULL with dma_buf_map_clear(). Cleared mappings > + * always refer to system memory. > + * > + * .. code-block:: c > + * > + * dma_buf_map_clear(&map); > + * > * Test if a mapping is valid with either dma_buf_map_is_set() or > * dma_buf_map_is_null(). > * > @@ -73,17 +89,19 @@ > * if (dma_buf_map_is_equal(&sys_map, &io_map)) > * // always false > * > - * Instances of struct dma_buf_map do not have to be cleaned up, but > - * can be cleared to NULL with dma_buf_map_clear(). Cleared mappings > - * always refer to system memory. > + * A set up instance of struct dma_buf_map can be used to access or manipulate > + * the buffer memory. Depending on the location of the memory, the provided > + * helpers will pick the correct operations. Data can be copied into the memory > + * with dma_buf_map_memcpy_to(). The address can be manipulated with > + * dma_buf_map_incr(). > * > - * The type :c:type:`struct dma_buf_map <dma_buf_map>` and its helpers are > - * actually independent from the dma-buf infrastructure. When sharing buffers > - * among devices, drivers have to know the location of the memory to access > - * the buffers in a safe way. :c:type:`struct dma_buf_map <dma_buf_map>` > - * solves this problem for dma-buf and its users. If other drivers or > - * sub-systems require similar functionality, the type could be generalized > - * and moved to a more prominent header file. > + * .. code-block:: c > + * > + * const void *src = ...; // source buffer > + * size_t len = ...; // length of src > + * > + * dma_buf_map_memcpy_to(&map, src, len); > + * dma_buf_map_incr(&map, len); // go to first byte after the memcpy > */ > > /** > @@ -210,4 +228,38 @@ static inline void dma_buf_map_clear(struct dma_buf_map *map) > } > } > > +/** > + * dma_buf_map_memcpy_to - Memcpy into dma-buf mapping > + * @dst: The dma-buf mapping structure > + * @src: The source buffer > + * @len: The number of byte in src > + * > + * Copies data into a dma-buf mapping. The source buffer is in system > + * memory. Depending on the buffer's location, the helper picks the correct > + * method of accessing the memory. > + */ > +static inline void dma_buf_map_memcpy_to(struct dma_buf_map *dst, const void *src, size_t len) > +{ > + if (dst->is_iomem) > + memcpy_toio(dst->vaddr_iomem, src, len); > + else > + memcpy(dst->vaddr, src, len); > +} > + > +/** > + * dma_buf_map_incr - Increments the address stored in a dma-buf mapping > + * @map: The dma-buf mapping structure > + * @incr: The number of bytes to increment > + * > + * Increments the address stored in a dma-buf mapping. Depending on the > + * buffer's location, the correct value will be updated. > + */ > +static inline void dma_buf_map_incr(struct dma_buf_map *map, size_t incr) > +{ > + if (map->is_iomem) > + map->vaddr_iomem += incr; > + else > + map->vaddr += incr; > +} > + > #endif /* __DMA_BUF_MAP_H__ */ > -- > 2.28.0
Hi Sam On Fri, 16 Oct 2020 12:08:54 +0200 Sam Ravnborg <sam@ravnborg.org> wrote: > Hi Thomas. > > On Thu, Oct 15, 2020 at 02:38:05PM +0200, Thomas Zimmermann wrote: > > To do framebuffer updates, one needs memcpy from system memory and a > > pointer-increment function. Add both interfaces with documentation. > > > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > Looks good. > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> Thanks. If you have the time, may I ask you to test this patchset on the bochs/sparc64 system that failed with the original code? Best regards Thomas > > > --- > > include/linux/dma-buf-map.h | 72 +++++++++++++++++++++++++++++++------ > > 1 file changed, 62 insertions(+), 10 deletions(-) > > > > diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h > > index 2e8bbecb5091..6ca0f304dda2 100644 > > --- a/include/linux/dma-buf-map.h > > +++ b/include/linux/dma-buf-map.h > > @@ -32,6 +32,14 @@ > > * accessing the buffer. Use the returned instance and the helper > > functions > > * to access the buffer's memory in the correct way. > > * > > + * The type :c:type:`struct dma_buf_map <dma_buf_map>` and its helpers > > are > > + * actually independent from the dma-buf infrastructure. When sharing > > buffers > > + * among devices, drivers have to know the location of the memory to > > access > > + * the buffers in a safe way. :c:type:`struct dma_buf_map <dma_buf_map>` > > + * solves this problem for dma-buf and its users. If other drivers or > > + * sub-systems require similar functionality, the type could be > > generalized > > + * and moved to a more prominent header file. > > + * > > * Open-coding access to :c:type:`struct dma_buf_map <dma_buf_map>` is > > * considered bad style. Rather then accessing its fields directly, use > > one > > * of the provided helper functions, or implement your own. For example, > > @@ -51,6 +59,14 @@ > > * > > * dma_buf_map_set_vaddr_iomem(&map. 0xdeadbeaf); > > * > > + * Instances of struct dma_buf_map do not have to be cleaned up, but > > + * can be cleared to NULL with dma_buf_map_clear(). Cleared mappings > > + * always refer to system memory. > > + * > > + * .. code-block:: c > > + * > > + * dma_buf_map_clear(&map); > > + * > > * Test if a mapping is valid with either dma_buf_map_is_set() or > > * dma_buf_map_is_null(). > > * > > @@ -73,17 +89,19 @@ > > * if (dma_buf_map_is_equal(&sys_map, &io_map)) > > * // always false > > * > > - * Instances of struct dma_buf_map do not have to be cleaned up, but > > - * can be cleared to NULL with dma_buf_map_clear(). Cleared mappings > > - * always refer to system memory. > > + * A set up instance of struct dma_buf_map can be used to access or > > manipulate > > + * the buffer memory. Depending on the location of the memory, the > > provided > > + * helpers will pick the correct operations. Data can be copied into the > > memory > > + * with dma_buf_map_memcpy_to(). The address can be manipulated with > > + * dma_buf_map_incr(). > > * > > - * The type :c:type:`struct dma_buf_map <dma_buf_map>` and its helpers > > are > > - * actually independent from the dma-buf infrastructure. When sharing > > buffers > > - * among devices, drivers have to know the location of the memory to > > access > > - * the buffers in a safe way. :c:type:`struct dma_buf_map <dma_buf_map>` > > - * solves this problem for dma-buf and its users. If other drivers or > > - * sub-systems require similar functionality, the type could be > > generalized > > - * and moved to a more prominent header file. > > + * .. code-block:: c > > + * > > + * const void *src = ...; // source buffer > > + * size_t len = ...; // length of src > > + * > > + * dma_buf_map_memcpy_to(&map, src, len); > > + * dma_buf_map_incr(&map, len); // go to first byte after the > > memcpy */ > > > > /** > > @@ -210,4 +228,38 @@ static inline void dma_buf_map_clear(struct > > dma_buf_map *map) } > > } > > > > +/** > > + * dma_buf_map_memcpy_to - Memcpy into dma-buf mapping > > + * @dst: The dma-buf mapping structure > > + * @src: The source buffer > > + * @len: The number of byte in src > > + * > > + * Copies data into a dma-buf mapping. The source buffer is in system > > + * memory. Depending on the buffer's location, the helper picks the > > correct > > + * method of accessing the memory. > > + */ > > +static inline void dma_buf_map_memcpy_to(struct dma_buf_map *dst, const > > void *src, size_t len) +{ > > + if (dst->is_iomem) > > + memcpy_toio(dst->vaddr_iomem, src, len); > > + else > > + memcpy(dst->vaddr, src, len); > > +} > > + > > +/** > > + * dma_buf_map_incr - Increments the address stored in a dma-buf mapping > > + * @map: The dma-buf mapping structure > > + * @incr: The number of bytes to increment > > + * > > + * Increments the address stored in a dma-buf mapping. Depending on the > > + * buffer's location, the correct value will be updated. > > + */ > > +static inline void dma_buf_map_incr(struct dma_buf_map *map, size_t incr) > > +{ > > + if (map->is_iomem) > > + map->vaddr_iomem += incr; > > + else > > + map->vaddr += incr; > > +} > > + > > #endif /* __DMA_BUF_MAP_H__ */ > > -- > > 2.28.0 > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Thomas. On Thu, Oct 15, 2020 at 02:38:05PM +0200, Thomas Zimmermann wrote: > To do framebuffer updates, one needs memcpy from system memory and a > pointer-increment function. Add both interfaces with documentation. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > include/linux/dma-buf-map.h | 72 +++++++++++++++++++++++++++++++------ > 1 file changed, 62 insertions(+), 10 deletions(-) > > diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h > index 2e8bbecb5091..6ca0f304dda2 100644 > --- a/include/linux/dma-buf-map.h > +++ b/include/linux/dma-buf-map.h > @@ -32,6 +32,14 @@ > * accessing the buffer. Use the returned instance and the helper functions > * to access the buffer's memory in the correct way. > * > + * The type :c:type:`struct dma_buf_map <dma_buf_map>` and its helpers are > + * actually independent from the dma-buf infrastructure. When sharing buffers > + * among devices, drivers have to know the location of the memory to access > + * the buffers in a safe way. :c:type:`struct dma_buf_map <dma_buf_map>` > + * solves this problem for dma-buf and its users. If other drivers or > + * sub-systems require similar functionality, the type could be generalized > + * and moved to a more prominent header file. > + * > * Open-coding access to :c:type:`struct dma_buf_map <dma_buf_map>` is > * considered bad style. Rather then accessing its fields directly, use one > * of the provided helper functions, or implement your own. For example, > @@ -51,6 +59,14 @@ > * > * dma_buf_map_set_vaddr_iomem(&map. 0xdeadbeaf); > * > + * Instances of struct dma_buf_map do not have to be cleaned up, but > + * can be cleared to NULL with dma_buf_map_clear(). Cleared mappings > + * always refer to system memory. > + * > + * .. code-block:: c > + * > + * dma_buf_map_clear(&map); > + * > * Test if a mapping is valid with either dma_buf_map_is_set() or > * dma_buf_map_is_null(). > * > @@ -73,17 +89,19 @@ > * if (dma_buf_map_is_equal(&sys_map, &io_map)) > * // always false > * > - * Instances of struct dma_buf_map do not have to be cleaned up, but > - * can be cleared to NULL with dma_buf_map_clear(). Cleared mappings > - * always refer to system memory. > + * A set up instance of struct dma_buf_map can be used to access or manipulate > + * the buffer memory. Depending on the location of the memory, the provided > + * helpers will pick the correct operations. Data can be copied into the memory > + * with dma_buf_map_memcpy_to(). The address can be manipulated with > + * dma_buf_map_incr(). > * > - * The type :c:type:`struct dma_buf_map <dma_buf_map>` and its helpers are > - * actually independent from the dma-buf infrastructure. When sharing buffers > - * among devices, drivers have to know the location of the memory to access > - * the buffers in a safe way. :c:type:`struct dma_buf_map <dma_buf_map>` > - * solves this problem for dma-buf and its users. If other drivers or > - * sub-systems require similar functionality, the type could be generalized > - * and moved to a more prominent header file. > + * .. code-block:: c > + * > + * const void *src = ...; // source buffer > + * size_t len = ...; // length of src > + * > + * dma_buf_map_memcpy_to(&map, src, len); > + * dma_buf_map_incr(&map, len); // go to first byte after the memcpy > */ > > /** > @@ -210,4 +228,38 @@ static inline void dma_buf_map_clear(struct dma_buf_map *map) > } > } > > +/** > + * dma_buf_map_memcpy_to - Memcpy into dma-buf mapping > + * @dst: The dma-buf mapping structure > + * @src: The source buffer > + * @len: The number of byte in src > + * > + * Copies data into a dma-buf mapping. The source buffer is in system > + * memory. Depending on the buffer's location, the helper picks the correct > + * method of accessing the memory. > + */ > +static inline void dma_buf_map_memcpy_to(struct dma_buf_map *dst, const void *src, size_t len) > +{ > + if (dst->is_iomem) > + memcpy_toio(dst->vaddr_iomem, src, len); > + else > + memcpy(dst->vaddr, src, len); sparc64 needs "#include <linux/string.h>" to build as is does not get this via io.h Sam > +} > + > +/** > + * dma_buf_map_incr - Increments the address stored in a dma-buf mapping > + * @map: The dma-buf mapping structure > + * @incr: The number of bytes to increment > + * > + * Increments the address stored in a dma-buf mapping. Depending on the > + * buffer's location, the correct value will be updated. > + */ > +static inline void dma_buf_map_incr(struct dma_buf_map *map, size_t incr) > +{ > + if (map->is_iomem) > + map->vaddr_iomem += incr; > + else > + map->vaddr += incr; > +} > + > #endif /* __DMA_BUF_MAP_H__ */ > -- > 2.28.0
diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h index 2e8bbecb5091..6ca0f304dda2 100644 --- a/include/linux/dma-buf-map.h +++ b/include/linux/dma-buf-map.h @@ -32,6 +32,14 @@ * accessing the buffer. Use the returned instance and the helper functions * to access the buffer's memory in the correct way. * + * The type :c:type:`struct dma_buf_map <dma_buf_map>` and its helpers are + * actually independent from the dma-buf infrastructure. When sharing buffers + * among devices, drivers have to know the location of the memory to access + * the buffers in a safe way. :c:type:`struct dma_buf_map <dma_buf_map>` + * solves this problem for dma-buf and its users. If other drivers or + * sub-systems require similar functionality, the type could be generalized + * and moved to a more prominent header file. + * * Open-coding access to :c:type:`struct dma_buf_map <dma_buf_map>` is * considered bad style. Rather then accessing its fields directly, use one * of the provided helper functions, or implement your own. For example, @@ -51,6 +59,14 @@ * * dma_buf_map_set_vaddr_iomem(&map. 0xdeadbeaf); * + * Instances of struct dma_buf_map do not have to be cleaned up, but + * can be cleared to NULL with dma_buf_map_clear(). Cleared mappings + * always refer to system memory. + * + * .. code-block:: c + * + * dma_buf_map_clear(&map); + * * Test if a mapping is valid with either dma_buf_map_is_set() or * dma_buf_map_is_null(). * @@ -73,17 +89,19 @@ * if (dma_buf_map_is_equal(&sys_map, &io_map)) * // always false * - * Instances of struct dma_buf_map do not have to be cleaned up, but - * can be cleared to NULL with dma_buf_map_clear(). Cleared mappings - * always refer to system memory. + * A set up instance of struct dma_buf_map can be used to access or manipulate + * the buffer memory. Depending on the location of the memory, the provided + * helpers will pick the correct operations. Data can be copied into the memory + * with dma_buf_map_memcpy_to(). The address can be manipulated with + * dma_buf_map_incr(). * - * The type :c:type:`struct dma_buf_map <dma_buf_map>` and its helpers are - * actually independent from the dma-buf infrastructure. When sharing buffers - * among devices, drivers have to know the location of the memory to access - * the buffers in a safe way. :c:type:`struct dma_buf_map <dma_buf_map>` - * solves this problem for dma-buf and its users. If other drivers or - * sub-systems require similar functionality, the type could be generalized - * and moved to a more prominent header file. + * .. code-block:: c + * + * const void *src = ...; // source buffer + * size_t len = ...; // length of src + * + * dma_buf_map_memcpy_to(&map, src, len); + * dma_buf_map_incr(&map, len); // go to first byte after the memcpy */ /** @@ -210,4 +228,38 @@ static inline void dma_buf_map_clear(struct dma_buf_map *map) } } +/** + * dma_buf_map_memcpy_to - Memcpy into dma-buf mapping + * @dst: The dma-buf mapping structure + * @src: The source buffer + * @len: The number of byte in src + * + * Copies data into a dma-buf mapping. The source buffer is in system + * memory. Depending on the buffer's location, the helper picks the correct + * method of accessing the memory. + */ +static inline void dma_buf_map_memcpy_to(struct dma_buf_map *dst, const void *src, size_t len) +{ + if (dst->is_iomem) + memcpy_toio(dst->vaddr_iomem, src, len); + else + memcpy(dst->vaddr, src, len); +} + +/** + * dma_buf_map_incr - Increments the address stored in a dma-buf mapping + * @map: The dma-buf mapping structure + * @incr: The number of bytes to increment + * + * Increments the address stored in a dma-buf mapping. Depending on the + * buffer's location, the correct value will be updated. + */ +static inline void dma_buf_map_incr(struct dma_buf_map *map, size_t incr) +{ + if (map->is_iomem) + map->vaddr_iomem += incr; + else + map->vaddr += incr; +} + #endif /* __DMA_BUF_MAP_H__ */
To do framebuffer updates, one needs memcpy from system memory and a pointer-increment function. Add both interfaces with documentation. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- include/linux/dma-buf-map.h | 72 +++++++++++++++++++++++++++++++------ 1 file changed, 62 insertions(+), 10 deletions(-)