diff mbox series

[01/14] iosys-map: Introduce renamed dma-buf-map

Message ID 20220128083626.3012259-2-lucas.demarchi@intel.com (mailing list archive)
State New, archived
Headers show
Series Rename dma-buf-map | expand

Commit Message

Lucas De Marchi Jan. 28, 2022, 8:36 a.m. UTC 
Add a new type, struct iosys_map, to eventually replace
struct dma_buf_map and its helpers defiend in
include/linux/dma-buf-map.h.

This is mostly a copy of dma-buf-map with the renames in place and
slightly different wording to avoid tying iosys_map to dma-buf: in fact
it's just a shim layer to abstract system memory, that can be accessed
via regular load and store, from IO memory that needs to be acessed via
arch helpers. Over time the dma-buf-map proved useful outside of buffer
sharing among drivers and started to be used in helper functions for
allocation and generic use. See e.g.

	drivers/gpu/drm/drm_gem_shmem_helper.c
	drivers/gpu/drm/drm_gem_framebuffer_helper.c
	drivers/gpu/drm/drm_fb_helper.c

In the i915 driver it's also desired to share the implementation for
integrated graphics, which uses mostly system memory, with discrete
graphics, which may need to access IO memory.

Once all the drivers using dma_buf_map are converted, the dma_buf_map
can be retired and iosys_map extended to cover new use cases.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 MAINTAINERS                 |   1 +
 include/linux/dma-buf-map.h |   3 +
 include/linux/iosys-map.h   | 254 ++++++++++++++++++++++++++++++++++++
 3 files changed, 258 insertions(+)
 create mode 100644 include/linux/iosys-map.h

Comments

Thomas Zimmermann Jan. 28, 2022, 8:53 a.m. UTC | #1 
Hi

Am 28.01.22 um 09:36 schrieb Lucas De Marchi:
> Add a new type, struct iosys_map, to eventually replace
> struct dma_buf_map and its helpers defiend in
> include/linux/dma-buf-map.h.
> 
> This is mostly a copy of dma-buf-map with the renames in place and
> slightly different wording to avoid tying iosys_map to dma-buf: in fact
> it's just a shim layer to abstract system memory, that can be accessed
> via regular load and store, from IO memory that needs to be acessed via
> arch helpers. Over time the dma-buf-map proved useful outside of buffer
> sharing among drivers and started to be used in helper functions for
> allocation and generic use. See e.g.
> 
> 	drivers/gpu/drm/drm_gem_shmem_helper.c
> 	drivers/gpu/drm/drm_gem_framebuffer_helper.c
> 	drivers/gpu/drm/drm_fb_helper.c

Well, that was the original motivation: framebuffer memory can be 
located in system or I/O memory, and even change their location. For 
some architectures this makes difference. IIRC the framebuffer console 
crashed on sparc64 because we didn't access I/O memory in the correct 
way. Hence, we added dma_buf_map to return the type of memory from 
dma_buf_vmap/dma_buf_vunmap.  And because everything is tied together, 
we had quite a bit of churn throughout the DRM/media code. There are 
still places in DRM where we access the raw pointers within dma_buf_map. 
We need to clean this up at some point.

> 
> In the i915 driver it's also desired to share the implementation for
> integrated graphics, which uses mostly system memory, with discrete
> graphics, which may need to access IO memory.
> 
> Once all the drivers using dma_buf_map are converted, the dma_buf_map
> can be retired and iosys_map extended to cover new use cases.

Wrt the renaming: the old name isn't good and the new one isn't good 
either. But I don't have strong feelings for either of them.

> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   MAINTAINERS                 |   1 +
>   include/linux/dma-buf-map.h |   3 +
>   include/linux/iosys-map.h   | 254 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 258 insertions(+)
>   create mode 100644 include/linux/iosys-map.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ea3e6c914384..27ebaded85f8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5734,6 +5734,7 @@ F:	Documentation/driver-api/dma-buf.rst
>   F:	drivers/dma-buf/
>   F:	include/linux/*fence.h
>   F:	include/linux/dma-buf*
> +F:	include/linux/iosys-map.h

If anything, I'd complain tha twe now have something in dma-buf that 
isn't obviously connected to dma-buf.

Best regards
Thomas

>   F:	include/linux/dma-resv.h
>   K:	\bdma_(?:buf|fence|resv)\b
>   
> diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
> index 19fa0b5ae5ec..4b4b2930660b 100644
> --- a/include/linux/dma-buf-map.h
> +++ b/include/linux/dma-buf-map.h
> @@ -263,4 +263,7 @@ static inline void dma_buf_map_incr(struct dma_buf_map *map, size_t incr)
>   		map->vaddr += incr;
>   }
>   
> +/* Temporary include for API migration */
> +#include <linux/iosys-map.h>
> +
>   #endif /* __DMA_BUF_MAP_H__ */
> diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h
> new file mode 100644
> index 000000000000..ad1f08f8f97f
> --- /dev/null
> +++ b/include/linux/iosys-map.h
> @@ -0,0 +1,254 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Pointer abstraction for IO/system memory
> + */
> +
> +#ifndef __IOSYS_MAP_H__
> +#define __IOSYS_MAP_H__
> +
> +#include <linux/io.h>
> +#include <linux/string.h>
> +
> +/* Temporary include while user of dma-buf-map are converted to iosys-map */
> +#include <linux/dma-buf-map.h>
> +
> +/**
> + * DOC: overview
> + *
> + * When accessing a memory region, depending on the its location, users may have
> + * to access it with I/O operations or memory load/store operations. For
> + * example, copying to system memory could be done with memcpy(), copying to I/O
> + * memory would be done with memcpy_toio().
> + *
> + * .. code-block:: c
> + *
> + *	void *vaddr = ...; // pointer to system memory
> + *	memcpy(vaddr, src, len);
> + *
> + *	void *vaddr_iomem = ...; // pointer to I/O memory
> + *	memcpy_toio(vaddr, _iomem, src, len);
> + *
> + * The user of such pointer may not have information about the mapping of that
> + * region or may want to have a single code path to handle operations on that
> + * buffer, regardless if it's located in system or IO memory. The type
> + * :c:type:`struct iosys_map <iosys_map>` and its helpers abstract that so the
> + * buffer can be passed around to other drivers or have separate duties inside
> + * the same driver for allocation, read and write operations.
> + *
> + * Open-coding access to :c:type:`struct iosys_map <iosys_map>` is considered
> + * bad style. Rather then accessing its fields directly, use one of the provided
> + * helper functions, or implement your own. For example, instances of
> + * :c:type:`struct iosys_map <iosys_map>` can be initialized statically with
> + * IOSYS_MAP_INIT_VADDR(), or at runtime with iosys_map_set_vaddr(). These
> + * helpers will set an address in system memory.
> + *
> + * .. code-block:: c
> + *
> + *	struct iosys_map map = IOSYS_MAP_INIT_VADDR(0xdeadbeaf);
> + *
> + *	iosys_map_set_vaddr(&map, 0xdeadbeaf);
> + *
> + * To set an address in I/O memory, use iosys_map_set_vaddr_iomem().
> + *
> + * .. code-block:: c
> + *
> + *	iosys_map_set_vaddr_iomem(&map, 0xdeadbeaf);
> + *
> + * Instances of struct iosys_map do not have to be cleaned up, but
> + * can be cleared to NULL with iosys_map_clear(). Cleared mappings
> + * always refer to system memory.
> + *
> + * .. code-block:: c
> + *
> + *	iosys_map_clear(&map);
> + *
> + * Test if a mapping is valid with either iosys_map_is_set() or
> + * iosys_map_is_null().
> + *
> + * .. code-block:: c
> + *
> + *	if (iosys_map_is_set(&map) != iosys_map_is_null(&map))
> + *		// always true
> + *
> + * Instances of :c:type:`struct iosys_map <iosys_map>` can be compared for
> + * equality with iosys_map_is_equal(). Mappings that point to different memory
> + * spaces, system or I/O, are never equal. That's even true if both spaces are
> + * located in the same address space, both mappings contain the same address
> + * value, or both mappings refer to NULL.
> + *
> + * .. code-block:: c
> + *
> + *	struct iosys_map sys_map; // refers to system memory
> + *	struct iosys_map io_map; // refers to I/O memory
> + *
> + *	if (iosys_map_is_equal(&sys_map, &io_map))
> + *		// always false
> + *
> + * A set up instance of struct iosys_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
> + * iosys_map_memcpy_to(). The address can be manipulated with iosys_map_incr().
> + *
> + * .. code-block:: c
> + *
> + *	const void *src = ...; // source buffer
> + *	size_t len = ...; // length of src
> + *
> + *	iosys_map_memcpy_to(&map, src, len);
> + *	iosys_map_incr(&map, len); // go to first byte after the memcpy
> + */
> +
> +/**
> + * struct iosys_map - Pointer to IO/system memory
> + * @vaddr_iomem:	The buffer's address if in I/O memory
> + * @vaddr:		The buffer's address if in system memory
> + * @is_iomem:		True if the buffer is located in I/O memory, or false
> + *			otherwise.
> + */
> +#define iosys_map dma_buf_map
> +
> +/**
> + * IOSYS_MAP_INIT_VADDR - Initializes struct iosys_map to an address in system memory
> + * @vaddr_:	A system-memory address
> + */
> +#define IOSYS_MAP_INIT_VADDR(vaddr_)	\
> +	{				\
> +		.vaddr = (vaddr_),	\
> +		.is_iomem = false,	\
> +	}
> +
> +/**
> + * iosys_map_set_vaddr - Sets a iosys mapping structure to an address in system memory
> + * @map:	The iosys_map structure
> + * @vaddr:	A system-memory address
> + *
> + * Sets the address and clears the I/O-memory flag.
> + */
> +static inline void iosys_map_set_vaddr(struct iosys_map *map, void *vaddr)
> +{
> +	map->vaddr = vaddr;
> +	map->is_iomem = false;
> +}
> +
> +/**
> + * iosys_map_set_vaddr_iomem - Sets a iosys mapping structure to an address in I/O memory
> + * @map:		The iosys_map structure
> + * @vaddr_iomem:	An I/O-memory address
> + *
> + * Sets the address and the I/O-memory flag.
> + */
> +static inline void iosys_map_set_vaddr_iomem(struct iosys_map *map,
> +					     void __iomem *vaddr_iomem)
> +{
> +	map->vaddr_iomem = vaddr_iomem;
> +	map->is_iomem = true;
> +}
> +
> +/**
> + * iosys_map_is_equal - Compares two iosys mapping structures for equality
> + * @lhs:	The iosys_map structure
> + * @rhs:	A iosys_map structure to compare with
> + *
> + * Two iosys mapping structures are equal if they both refer to the same type of memory
> + * and to the same address within that memory.
> + *
> + * Returns:
> + * True is both structures are equal, or false otherwise.
> + */
> +static inline bool iosys_map_is_equal(const struct iosys_map *lhs,
> +				      const struct iosys_map *rhs)
> +{
> +	if (lhs->is_iomem != rhs->is_iomem)
> +		return false;
> +	else if (lhs->is_iomem)
> +		return lhs->vaddr_iomem == rhs->vaddr_iomem;
> +	else
> +		return lhs->vaddr == rhs->vaddr;
> +}
> +
> +/**
> + * iosys_map_is_null - Tests for a iosys mapping to be NULL
> + * @map:	The iosys_map structure
> + *
> + * Depending on the state of struct iosys_map.is_iomem, tests if the
> + * mapping is NULL.
> + *
> + * Returns:
> + * True if the mapping is NULL, or false otherwise.
> + */
> +static inline bool iosys_map_is_null(const struct iosys_map *map)
> +{
> +	if (map->is_iomem)
> +		return !map->vaddr_iomem;
> +	return !map->vaddr;
> +}
> +
> +/**
> + * iosys_map_is_set - Tests if the iosys mapping has been set
> + * @map:	The iosys_map structure
> + *
> + * Depending on the state of struct iosys_map.is_iomem, tests if the
> + * mapping has been set.
> + *
> + * Returns:
> + * True if the mapping is been set, or false otherwise.
> + */
> +static inline bool iosys_map_is_set(const struct iosys_map *map)
> +{
> +	return !iosys_map_is_null(map);
> +}
> +
> +/**
> + * iosys_map_clear - Clears a iosys mapping structure
> + * @map:	The iosys_map structure
> + *
> + * Clears all fields to zero, including struct iosys_map.is_iomem, so
> + * mapping structures that were set to point to I/O memory are reset for
> + * system memory. Pointers are cleared to NULL. This is the default.
> + */
> +static inline void iosys_map_clear(struct iosys_map *map)
> +{
> +	if (map->is_iomem) {
> +		map->vaddr_iomem = NULL;
> +		map->is_iomem = false;
> +	} else {
> +		map->vaddr = NULL;
> +	}
> +}
> +
> +/**
> + * iosys_map_memcpy_to - Memcpy into iosys mapping
> + * @dst:	The iosys_map structure
> + * @src:	The source buffer
> + * @len:	The number of byte in src
> + *
> + * Copies data into a iosys 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 iosys_map_memcpy_to(struct iosys_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);
> +}
> +
> +/**
> + * iosys_map_incr - Increments the address stored in a iosys mapping
> + * @map:	The iosys_map structure
> + * @incr:	The number of bytes to increment
> + *
> + * Increments the address stored in a iosys mapping. Depending on the
> + * buffer's location, the correct value will be updated.
> + */
> +static inline void iosys_map_incr(struct iosys_map *map, size_t incr)
> +{
> +	if (map->is_iomem)
> +		map->vaddr_iomem += incr;
> +	else
> +		map->vaddr += incr;
> +}
> +
> +#endif /* __IOSYS_MAP_H__ */
Lucas De Marchi Jan. 28, 2022, 9:24 a.m. UTC | #2 
On Fri, Jan 28, 2022 at 09:53:59AM +0100, Thomas Zimmermann wrote:
>Hi
>
>Am 28.01.22 um 09:36 schrieb Lucas De Marchi:
>>Add a new type, struct iosys_map, to eventually replace
>>struct dma_buf_map and its helpers defiend in
>>include/linux/dma-buf-map.h.
>>
>>This is mostly a copy of dma-buf-map with the renames in place and
>>slightly different wording to avoid tying iosys_map to dma-buf: in fact
>>it's just a shim layer to abstract system memory, that can be accessed
>>via regular load and store, from IO memory that needs to be acessed via
>>arch helpers. Over time the dma-buf-map proved useful outside of buffer
>>sharing among drivers and started to be used in helper functions for
>>allocation and generic use. See e.g.
>>
>>	drivers/gpu/drm/drm_gem_shmem_helper.c
>>	drivers/gpu/drm/drm_gem_framebuffer_helper.c
>>	drivers/gpu/drm/drm_fb_helper.c
>
>Well, that was the original motivation: framebuffer memory can be 
>located in system or I/O memory, and even change their location. For 
>some architectures this makes difference. IIRC the framebuffer console 
>crashed on sparc64 because we didn't access I/O memory in the correct 
>way. Hence, we added dma_buf_map to return the type of memory from 
>dma_buf_vmap/dma_buf_vunmap.  And because everything is tied together, 
>we had quite a bit of churn throughout the DRM/media code. There are 

Thanks for the history on this. It's helpful. My motivation for starting
using dma_buf_map was very similar:  i915 is full of direct load/store.
It works on x86 for IO memory. While testing it on arm64 the story
was not kind the same. Since we want to support archs other than x86 we
need to start using some abstractions and dma_buf_map seemed a nice fit
:)

>still places in DRM where we access the raw pointers within 
>dma_buf_map. We need to clean this up at some point.

at least those are easier to find than just the raw pointers without
the abstraction

>
>>
>>In the i915 driver it's also desired to share the implementation for
>>integrated graphics, which uses mostly system memory, with discrete
>>graphics, which may need to access IO memory.
>>
>>Once all the drivers using dma_buf_map are converted, the dma_buf_map
>>can be retired and iosys_map extended to cover new use cases.
>
>Wrt the renaming: the old name isn't good and the new one isn't good 
>either. But I don't have strong feelings for either of them.

:(. As the quote: "There are only two hard things in Computer Science:
cache invalidation and naming things."

[ oh, and I'm also having to deal with cache invalidation to properly
   support other archs. ]

At least I had 2 people saying the name was ok.

>
>>
>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>---
>>  MAINTAINERS                 |   1 +
>>  include/linux/dma-buf-map.h |   3 +
>>  include/linux/iosys-map.h   | 254 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 258 insertions(+)
>>  create mode 100644 include/linux/iosys-map.h
>>
>>diff --git a/MAINTAINERS b/MAINTAINERS
>>index ea3e6c914384..27ebaded85f8 100644
>>--- a/MAINTAINERS
>>+++ b/MAINTAINERS
>>@@ -5734,6 +5734,7 @@ F:	Documentation/driver-api/dma-buf.rst
>>  F:	drivers/dma-buf/
>>  F:	include/linux/*fence.h
>>  F:	include/linux/dma-buf*
>>+F:	include/linux/iosys-map.h
>
>If anything, I'd complain tha twe now have something in dma-buf that 
>isn't obviously connected to dma-buf.

Let's see if others come with a better name

thanks
Lucas De Marchi

>
>Best regards
>Thomas
>
>>  F:	include/linux/dma-resv.h
>>  K:	\bdma_(?:buf|fence|resv)\b
>>diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
>>index 19fa0b5ae5ec..4b4b2930660b 100644
>>--- a/include/linux/dma-buf-map.h
>>+++ b/include/linux/dma-buf-map.h
>>@@ -263,4 +263,7 @@ static inline void dma_buf_map_incr(struct dma_buf_map *map, size_t incr)
>>  		map->vaddr += incr;
>>  }
>>+/* Temporary include for API migration */
>>+#include <linux/iosys-map.h>
>>+
>>  #endif /* __DMA_BUF_MAP_H__ */
>>diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h
>>new file mode 100644
>>index 000000000000..ad1f08f8f97f
>>--- /dev/null
>>+++ b/include/linux/iosys-map.h
>>@@ -0,0 +1,254 @@
>>+/* SPDX-License-Identifier: GPL-2.0-only */
>>+/*
>>+ * Pointer abstraction for IO/system memory
>>+ */
>>+
>>+#ifndef __IOSYS_MAP_H__
>>+#define __IOSYS_MAP_H__
>>+
>>+#include <linux/io.h>
>>+#include <linux/string.h>
>>+
>>+/* Temporary include while user of dma-buf-map are converted to iosys-map */
>>+#include <linux/dma-buf-map.h>
>>+
>>+/**
>>+ * DOC: overview
>>+ *
>>+ * When accessing a memory region, depending on the its location, users may have
>>+ * to access it with I/O operations or memory load/store operations. For
>>+ * example, copying to system memory could be done with memcpy(), copying to I/O
>>+ * memory would be done with memcpy_toio().
>>+ *
>>+ * .. code-block:: c
>>+ *
>>+ *	void *vaddr = ...; // pointer to system memory
>>+ *	memcpy(vaddr, src, len);
>>+ *
>>+ *	void *vaddr_iomem = ...; // pointer to I/O memory
>>+ *	memcpy_toio(vaddr, _iomem, src, len);
>>+ *
>>+ * The user of such pointer may not have information about the mapping of that
>>+ * region or may want to have a single code path to handle operations on that
>>+ * buffer, regardless if it's located in system or IO memory. The type
>>+ * :c:type:`struct iosys_map <iosys_map>` and its helpers abstract that so the
>>+ * buffer can be passed around to other drivers or have separate duties inside
>>+ * the same driver for allocation, read and write operations.
>>+ *
>>+ * Open-coding access to :c:type:`struct iosys_map <iosys_map>` is considered
>>+ * bad style. Rather then accessing its fields directly, use one of the provided
>>+ * helper functions, or implement your own. For example, instances of
>>+ * :c:type:`struct iosys_map <iosys_map>` can be initialized statically with
>>+ * IOSYS_MAP_INIT_VADDR(), or at runtime with iosys_map_set_vaddr(). These
>>+ * helpers will set an address in system memory.
>>+ *
>>+ * .. code-block:: c
>>+ *
>>+ *	struct iosys_map map = IOSYS_MAP_INIT_VADDR(0xdeadbeaf);
>>+ *
>>+ *	iosys_map_set_vaddr(&map, 0xdeadbeaf);
>>+ *
>>+ * To set an address in I/O memory, use iosys_map_set_vaddr_iomem().
>>+ *
>>+ * .. code-block:: c
>>+ *
>>+ *	iosys_map_set_vaddr_iomem(&map, 0xdeadbeaf);
>>+ *
>>+ * Instances of struct iosys_map do not have to be cleaned up, but
>>+ * can be cleared to NULL with iosys_map_clear(). Cleared mappings
>>+ * always refer to system memory.
>>+ *
>>+ * .. code-block:: c
>>+ *
>>+ *	iosys_map_clear(&map);
>>+ *
>>+ * Test if a mapping is valid with either iosys_map_is_set() or
>>+ * iosys_map_is_null().
>>+ *
>>+ * .. code-block:: c
>>+ *
>>+ *	if (iosys_map_is_set(&map) != iosys_map_is_null(&map))
>>+ *		// always true
>>+ *
>>+ * Instances of :c:type:`struct iosys_map <iosys_map>` can be compared for
>>+ * equality with iosys_map_is_equal(). Mappings that point to different memory
>>+ * spaces, system or I/O, are never equal. That's even true if both spaces are
>>+ * located in the same address space, both mappings contain the same address
>>+ * value, or both mappings refer to NULL.
>>+ *
>>+ * .. code-block:: c
>>+ *
>>+ *	struct iosys_map sys_map; // refers to system memory
>>+ *	struct iosys_map io_map; // refers to I/O memory
>>+ *
>>+ *	if (iosys_map_is_equal(&sys_map, &io_map))
>>+ *		// always false
>>+ *
>>+ * A set up instance of struct iosys_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
>>+ * iosys_map_memcpy_to(). The address can be manipulated with iosys_map_incr().
>>+ *
>>+ * .. code-block:: c
>>+ *
>>+ *	const void *src = ...; // source buffer
>>+ *	size_t len = ...; // length of src
>>+ *
>>+ *	iosys_map_memcpy_to(&map, src, len);
>>+ *	iosys_map_incr(&map, len); // go to first byte after the memcpy
>>+ */
>>+
>>+/**
>>+ * struct iosys_map - Pointer to IO/system memory
>>+ * @vaddr_iomem:	The buffer's address if in I/O memory
>>+ * @vaddr:		The buffer's address if in system memory
>>+ * @is_iomem:		True if the buffer is located in I/O memory, or false
>>+ *			otherwise.
>>+ */
>>+#define iosys_map dma_buf_map
>>+
>>+/**
>>+ * IOSYS_MAP_INIT_VADDR - Initializes struct iosys_map to an address in system memory
>>+ * @vaddr_:	A system-memory address
>>+ */
>>+#define IOSYS_MAP_INIT_VADDR(vaddr_)	\
>>+	{				\
>>+		.vaddr = (vaddr_),	\
>>+		.is_iomem = false,	\
>>+	}
>>+
>>+/**
>>+ * iosys_map_set_vaddr - Sets a iosys mapping structure to an address in system memory
>>+ * @map:	The iosys_map structure
>>+ * @vaddr:	A system-memory address
>>+ *
>>+ * Sets the address and clears the I/O-memory flag.
>>+ */
>>+static inline void iosys_map_set_vaddr(struct iosys_map *map, void *vaddr)
>>+{
>>+	map->vaddr = vaddr;
>>+	map->is_iomem = false;
>>+}
>>+
>>+/**
>>+ * iosys_map_set_vaddr_iomem - Sets a iosys mapping structure to an address in I/O memory
>>+ * @map:		The iosys_map structure
>>+ * @vaddr_iomem:	An I/O-memory address
>>+ *
>>+ * Sets the address and the I/O-memory flag.
>>+ */
>>+static inline void iosys_map_set_vaddr_iomem(struct iosys_map *map,
>>+					     void __iomem *vaddr_iomem)
>>+{
>>+	map->vaddr_iomem = vaddr_iomem;
>>+	map->is_iomem = true;
>>+}
>>+
>>+/**
>>+ * iosys_map_is_equal - Compares two iosys mapping structures for equality
>>+ * @lhs:	The iosys_map structure
>>+ * @rhs:	A iosys_map structure to compare with
>>+ *
>>+ * Two iosys mapping structures are equal if they both refer to the same type of memory
>>+ * and to the same address within that memory.
>>+ *
>>+ * Returns:
>>+ * True is both structures are equal, or false otherwise.
>>+ */
>>+static inline bool iosys_map_is_equal(const struct iosys_map *lhs,
>>+				      const struct iosys_map *rhs)
>>+{
>>+	if (lhs->is_iomem != rhs->is_iomem)
>>+		return false;
>>+	else if (lhs->is_iomem)
>>+		return lhs->vaddr_iomem == rhs->vaddr_iomem;
>>+	else
>>+		return lhs->vaddr == rhs->vaddr;
>>+}
>>+
>>+/**
>>+ * iosys_map_is_null - Tests for a iosys mapping to be NULL
>>+ * @map:	The iosys_map structure
>>+ *
>>+ * Depending on the state of struct iosys_map.is_iomem, tests if the
>>+ * mapping is NULL.
>>+ *
>>+ * Returns:
>>+ * True if the mapping is NULL, or false otherwise.
>>+ */
>>+static inline bool iosys_map_is_null(const struct iosys_map *map)
>>+{
>>+	if (map->is_iomem)
>>+		return !map->vaddr_iomem;
>>+	return !map->vaddr;
>>+}
>>+
>>+/**
>>+ * iosys_map_is_set - Tests if the iosys mapping has been set
>>+ * @map:	The iosys_map structure
>>+ *
>>+ * Depending on the state of struct iosys_map.is_iomem, tests if the
>>+ * mapping has been set.
>>+ *
>>+ * Returns:
>>+ * True if the mapping is been set, or false otherwise.
>>+ */
>>+static inline bool iosys_map_is_set(const struct iosys_map *map)
>>+{
>>+	return !iosys_map_is_null(map);
>>+}
>>+
>>+/**
>>+ * iosys_map_clear - Clears a iosys mapping structure
>>+ * @map:	The iosys_map structure
>>+ *
>>+ * Clears all fields to zero, including struct iosys_map.is_iomem, so
>>+ * mapping structures that were set to point to I/O memory are reset for
>>+ * system memory. Pointers are cleared to NULL. This is the default.
>>+ */
>>+static inline void iosys_map_clear(struct iosys_map *map)
>>+{
>>+	if (map->is_iomem) {
>>+		map->vaddr_iomem = NULL;
>>+		map->is_iomem = false;
>>+	} else {
>>+		map->vaddr = NULL;
>>+	}
>>+}
>>+
>>+/**
>>+ * iosys_map_memcpy_to - Memcpy into iosys mapping
>>+ * @dst:	The iosys_map structure
>>+ * @src:	The source buffer
>>+ * @len:	The number of byte in src
>>+ *
>>+ * Copies data into a iosys 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 iosys_map_memcpy_to(struct iosys_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);
>>+}
>>+
>>+/**
>>+ * iosys_map_incr - Increments the address stored in a iosys mapping
>>+ * @map:	The iosys_map structure
>>+ * @incr:	The number of bytes to increment
>>+ *
>>+ * Increments the address stored in a iosys mapping. Depending on the
>>+ * buffer's location, the correct value will be updated.
>>+ */
>>+static inline void iosys_map_incr(struct iosys_map *map, size_t incr)
>>+{
>>+	if (map->is_iomem)
>>+		map->vaddr_iomem += incr;
>>+	else
>>+		map->vaddr += incr;
>>+}
>>+
>>+#endif /* __IOSYS_MAP_H__ */
>
>-- 
>Thomas Zimmermann
>Graphics Driver Developer
>SUSE Software Solutions Germany GmbH
>Maxfeldstr. 5, 90409 Nürnberg, Germany
>(HRB 36809, AG Nürnberg)
>Geschäftsführer: Ivo Totev
Thomas Zimmermann Jan. 28, 2022, 9:39 a.m. UTC | #3 
Hi

Am 28.01.22 um 10:24 schrieb Lucas De Marchi:
> On Fri, Jan 28, 2022 at 09:53:59AM +0100, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 28.01.22 um 09:36 schrieb Lucas De Marchi:
>>> Add a new type, struct iosys_map, to eventually replace
>>> struct dma_buf_map and its helpers defiend in
>>> include/linux/dma-buf-map.h.
>>>
>>> This is mostly a copy of dma-buf-map with the renames in place and
>>> slightly different wording to avoid tying iosys_map to dma-buf: in fact
>>> it's just a shim layer to abstract system memory, that can be accessed
>>> via regular load and store, from IO memory that needs to be acessed via
>>> arch helpers. Over time the dma-buf-map proved useful outside of buffer
>>> sharing among drivers and started to be used in helper functions for
>>> allocation and generic use. See e.g.
>>>
>>>     drivers/gpu/drm/drm_gem_shmem_helper.c
>>>     drivers/gpu/drm/drm_gem_framebuffer_helper.c
>>>     drivers/gpu/drm/drm_fb_helper.c
>>
>> Well, that was the original motivation: framebuffer memory can be 
>> located in system or I/O memory, and even change their location. For 
>> some architectures this makes difference. IIRC the framebuffer console 
>> crashed on sparc64 because we didn't access I/O memory in the correct 
>> way. Hence, we added dma_buf_map to return the type of memory from 
>> dma_buf_vmap/dma_buf_vunmap.  And because everything is tied together, 
>> we had quite a bit of churn throughout the DRM/media code. There are 
> 
> Thanks for the history on this. It's helpful. My motivation for starting
> using dma_buf_map was very similar:  i915 is full of direct load/store.
> It works on x86 for IO memory. While testing it on arm64 the story
> was not kind the same. Since we want to support archs other than x86 we
> need to start using some abstractions and dma_buf_map seemed a nice fit
> :)
> 
>> still places in DRM where we access the raw pointers within 
>> dma_buf_map. We need to clean this up at some point.
> 
> at least those are easier to find than just the raw pointers without
> the abstraction
> 
>>
>>>
>>> In the i915 driver it's also desired to share the implementation for
>>> integrated graphics, which uses mostly system memory, with discrete
>>> graphics, which may need to access IO memory.
>>>
>>> Once all the drivers using dma_buf_map are converted, the dma_buf_map
>>> can be retired and iosys_map extended to cover new use cases.
>>
>> Wrt the renaming: the old name isn't good and the new one isn't good 
>> either. But I don't have strong feelings for either of them.
> 
> :(. As the quote: "There are only two hard things in Computer Science:
> cache invalidation and naming things."

:D

> 
> [ oh, and I'm also having to deal with cache invalidation to properly
>    support other archs. ]
> 
> At least I had 2 people saying the name was ok.
> 
>>
>>>
>>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>> ---
>>>  MAINTAINERS                 |   1 +
>>>  include/linux/dma-buf-map.h |   3 +
>>>  include/linux/iosys-map.h   | 254 ++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 258 insertions(+)
>>>  create mode 100644 include/linux/iosys-map.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index ea3e6c914384..27ebaded85f8 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -5734,6 +5734,7 @@ F:    Documentation/driver-api/dma-buf.rst
>>>  F:    drivers/dma-buf/
>>>  F:    include/linux/*fence.h
>>>  F:    include/linux/dma-buf*
>>> +F:    include/linux/iosys-map.h
>>
>> If anything, I'd complain tha twe now have something in dma-buf that 
>> isn't obviously connected to dma-buf.
> 
> Let's see if others come with a better name

Maybe we can move that file into some other module? Driver core or 
memory management?

Best regards
Thomas

> 
> thanks
> Lucas De Marchi
> 
>>
>> Best regards
>> Thomas
>>
>>>  F:    include/linux/dma-resv.h
>>>  K:    \bdma_(?:buf|fence|resv)\b
>>> diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
>>> index 19fa0b5ae5ec..4b4b2930660b 100644
>>> --- a/include/linux/dma-buf-map.h
>>> +++ b/include/linux/dma-buf-map.h
>>> @@ -263,4 +263,7 @@ static inline void dma_buf_map_incr(struct 
>>> dma_buf_map *map, size_t incr)
>>>          map->vaddr += incr;
>>>  }
>>> +/* Temporary include for API migration */
>>> +#include <linux/iosys-map.h>
>>> +
>>>  #endif /* __DMA_BUF_MAP_H__ */
>>> diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h
>>> new file mode 100644
>>> index 000000000000..ad1f08f8f97f
>>> --- /dev/null
>>> +++ b/include/linux/iosys-map.h
>>> @@ -0,0 +1,254 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * Pointer abstraction for IO/system memory
>>> + */
>>> +
>>> +#ifndef __IOSYS_MAP_H__
>>> +#define __IOSYS_MAP_H__
>>> +
>>> +#include <linux/io.h>
>>> +#include <linux/string.h>
>>> +
>>> +/* Temporary include while user of dma-buf-map are converted to 
>>> iosys-map */
>>> +#include <linux/dma-buf-map.h>
>>> +
>>> +/**
>>> + * DOC: overview
>>> + *
>>> + * When accessing a memory region, depending on the its location, 
>>> users may have
>>> + * to access it with I/O operations or memory load/store operations. 
>>> For
>>> + * example, copying to system memory could be done with memcpy(), 
>>> copying to I/O
>>> + * memory would be done with memcpy_toio().
>>> + *
>>> + * .. code-block:: c
>>> + *
>>> + *    void *vaddr = ...; // pointer to system memory
>>> + *    memcpy(vaddr, src, len);
>>> + *
>>> + *    void *vaddr_iomem = ...; // pointer to I/O memory
>>> + *    memcpy_toio(vaddr, _iomem, src, len);
>>> + *
>>> + * The user of such pointer may not have information about the 
>>> mapping of that
>>> + * region or may want to have a single code path to handle 
>>> operations on that
>>> + * buffer, regardless if it's located in system or IO memory. The type
>>> + * :c:type:`struct iosys_map <iosys_map>` and its helpers abstract 
>>> that so the
>>> + * buffer can be passed around to other drivers or have separate 
>>> duties inside
>>> + * the same driver for allocation, read and write operations.
>>> + *
>>> + * Open-coding access to :c:type:`struct iosys_map <iosys_map>` is 
>>> considered
>>> + * bad style. Rather then accessing its fields directly, use one of 
>>> the provided
>>> + * helper functions, or implement your own. For example, instances of
>>> + * :c:type:`struct iosys_map <iosys_map>` can be initialized 
>>> statically with
>>> + * IOSYS_MAP_INIT_VADDR(), or at runtime with iosys_map_set_vaddr(). 
>>> These
>>> + * helpers will set an address in system memory.
>>> + *
>>> + * .. code-block:: c
>>> + *
>>> + *    struct iosys_map map = IOSYS_MAP_INIT_VADDR(0xdeadbeaf);
>>> + *
>>> + *    iosys_map_set_vaddr(&map, 0xdeadbeaf);
>>> + *
>>> + * To set an address in I/O memory, use iosys_map_set_vaddr_iomem().
>>> + *
>>> + * .. code-block:: c
>>> + *
>>> + *    iosys_map_set_vaddr_iomem(&map, 0xdeadbeaf);
>>> + *
>>> + * Instances of struct iosys_map do not have to be cleaned up, but
>>> + * can be cleared to NULL with iosys_map_clear(). Cleared mappings
>>> + * always refer to system memory.
>>> + *
>>> + * .. code-block:: c
>>> + *
>>> + *    iosys_map_clear(&map);
>>> + *
>>> + * Test if a mapping is valid with either iosys_map_is_set() or
>>> + * iosys_map_is_null().
>>> + *
>>> + * .. code-block:: c
>>> + *
>>> + *    if (iosys_map_is_set(&map) != iosys_map_is_null(&map))
>>> + *        // always true
>>> + *
>>> + * Instances of :c:type:`struct iosys_map <iosys_map>` can be 
>>> compared for
>>> + * equality with iosys_map_is_equal(). Mappings that point to 
>>> different memory
>>> + * spaces, system or I/O, are never equal. That's even true if both 
>>> spaces are
>>> + * located in the same address space, both mappings contain the same 
>>> address
>>> + * value, or both mappings refer to NULL.
>>> + *
>>> + * .. code-block:: c
>>> + *
>>> + *    struct iosys_map sys_map; // refers to system memory
>>> + *    struct iosys_map io_map; // refers to I/O memory
>>> + *
>>> + *    if (iosys_map_is_equal(&sys_map, &io_map))
>>> + *        // always false
>>> + *
>>> + * A set up instance of struct iosys_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
>>> + * iosys_map_memcpy_to(). The address can be manipulated with 
>>> iosys_map_incr().
>>> + *
>>> + * .. code-block:: c
>>> + *
>>> + *    const void *src = ...; // source buffer
>>> + *    size_t len = ...; // length of src
>>> + *
>>> + *    iosys_map_memcpy_to(&map, src, len);
>>> + *    iosys_map_incr(&map, len); // go to first byte after the memcpy
>>> + */
>>> +
>>> +/**
>>> + * struct iosys_map - Pointer to IO/system memory
>>> + * @vaddr_iomem:    The buffer's address if in I/O memory
>>> + * @vaddr:        The buffer's address if in system memory
>>> + * @is_iomem:        True if the buffer is located in I/O memory, or 
>>> false
>>> + *            otherwise.
>>> + */
>>> +#define iosys_map dma_buf_map
>>> +
>>> +/**
>>> + * IOSYS_MAP_INIT_VADDR - Initializes struct iosys_map to an address 
>>> in system memory
>>> + * @vaddr_:    A system-memory address
>>> + */
>>> +#define IOSYS_MAP_INIT_VADDR(vaddr_)    \
>>> +    {                \
>>> +        .vaddr = (vaddr_),    \
>>> +        .is_iomem = false,    \
>>> +    }
>>> +
>>> +/**
>>> + * iosys_map_set_vaddr - Sets a iosys mapping structure to an 
>>> address in system memory
>>> + * @map:    The iosys_map structure
>>> + * @vaddr:    A system-memory address
>>> + *
>>> + * Sets the address and clears the I/O-memory flag.
>>> + */
>>> +static inline void iosys_map_set_vaddr(struct iosys_map *map, void 
>>> *vaddr)
>>> +{
>>> +    map->vaddr = vaddr;
>>> +    map->is_iomem = false;
>>> +}
>>> +
>>> +/**
>>> + * iosys_map_set_vaddr_iomem - Sets a iosys mapping structure to an 
>>> address in I/O memory
>>> + * @map:        The iosys_map structure
>>> + * @vaddr_iomem:    An I/O-memory address
>>> + *
>>> + * Sets the address and the I/O-memory flag.
>>> + */
>>> +static inline void iosys_map_set_vaddr_iomem(struct iosys_map *map,
>>> +                         void __iomem *vaddr_iomem)
>>> +{
>>> +    map->vaddr_iomem = vaddr_iomem;
>>> +    map->is_iomem = true;
>>> +}
>>> +
>>> +/**
>>> + * iosys_map_is_equal - Compares two iosys mapping structures for 
>>> equality
>>> + * @lhs:    The iosys_map structure
>>> + * @rhs:    A iosys_map structure to compare with
>>> + *
>>> + * Two iosys mapping structures are equal if they both refer to the 
>>> same type of memory
>>> + * and to the same address within that memory.
>>> + *
>>> + * Returns:
>>> + * True is both structures are equal, or false otherwise.
>>> + */
>>> +static inline bool iosys_map_is_equal(const struct iosys_map *lhs,
>>> +                      const struct iosys_map *rhs)
>>> +{
>>> +    if (lhs->is_iomem != rhs->is_iomem)
>>> +        return false;
>>> +    else if (lhs->is_iomem)
>>> +        return lhs->vaddr_iomem == rhs->vaddr_iomem;
>>> +    else
>>> +        return lhs->vaddr == rhs->vaddr;
>>> +}
>>> +
>>> +/**
>>> + * iosys_map_is_null - Tests for a iosys mapping to be NULL
>>> + * @map:    The iosys_map structure
>>> + *
>>> + * Depending on the state of struct iosys_map.is_iomem, tests if the
>>> + * mapping is NULL.
>>> + *
>>> + * Returns:
>>> + * True if the mapping is NULL, or false otherwise.
>>> + */
>>> +static inline bool iosys_map_is_null(const struct iosys_map *map)
>>> +{
>>> +    if (map->is_iomem)
>>> +        return !map->vaddr_iomem;
>>> +    return !map->vaddr;
>>> +}
>>> +
>>> +/**
>>> + * iosys_map_is_set - Tests if the iosys mapping has been set
>>> + * @map:    The iosys_map structure
>>> + *
>>> + * Depending on the state of struct iosys_map.is_iomem, tests if the
>>> + * mapping has been set.
>>> + *
>>> + * Returns:
>>> + * True if the mapping is been set, or false otherwise.
>>> + */
>>> +static inline bool iosys_map_is_set(const struct iosys_map *map)
>>> +{
>>> +    return !iosys_map_is_null(map);
>>> +}
>>> +
>>> +/**
>>> + * iosys_map_clear - Clears a iosys mapping structure
>>> + * @map:    The iosys_map structure
>>> + *
>>> + * Clears all fields to zero, including struct iosys_map.is_iomem, so
>>> + * mapping structures that were set to point to I/O memory are reset 
>>> for
>>> + * system memory. Pointers are cleared to NULL. This is the default.
>>> + */
>>> +static inline void iosys_map_clear(struct iosys_map *map)
>>> +{
>>> +    if (map->is_iomem) {
>>> +        map->vaddr_iomem = NULL;
>>> +        map->is_iomem = false;
>>> +    } else {
>>> +        map->vaddr = NULL;
>>> +    }
>>> +}
>>> +
>>> +/**
>>> + * iosys_map_memcpy_to - Memcpy into iosys mapping
>>> + * @dst:    The iosys_map structure
>>> + * @src:    The source buffer
>>> + * @len:    The number of byte in src
>>> + *
>>> + * Copies data into a iosys 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 iosys_map_memcpy_to(struct iosys_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);
>>> +}
>>> +
>>> +/**
>>> + * iosys_map_incr - Increments the address stored in a iosys mapping
>>> + * @map:    The iosys_map structure
>>> + * @incr:    The number of bytes to increment
>>> + *
>>> + * Increments the address stored in a iosys mapping. Depending on the
>>> + * buffer's location, the correct value will be updated.
>>> + */
>>> +static inline void iosys_map_incr(struct iosys_map *map, size_t incr)
>>> +{
>>> +    if (map->is_iomem)
>>> +        map->vaddr_iomem += incr;
>>> +    else
>>> +        map->vaddr += incr;
>>> +}
>>> +
>>> +#endif /* __IOSYS_MAP_H__ */
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev
> 
> 
>
Daniel Vetter Jan. 28, 2022, 10:26 p.m. UTC | #4 
On Fri, Jan 28, 2022 at 10:39:49AM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 28.01.22 um 10:24 schrieb Lucas De Marchi:
> > On Fri, Jan 28, 2022 at 09:53:59AM +0100, Thomas Zimmermann wrote:
> > > Hi
> > > 
> > > Am 28.01.22 um 09:36 schrieb Lucas De Marchi:
> > > > Add a new type, struct iosys_map, to eventually replace
> > > > struct dma_buf_map and its helpers defiend in
> > > > include/linux/dma-buf-map.h.
> > > > 
> > > > This is mostly a copy of dma-buf-map with the renames in place and
> > > > slightly different wording to avoid tying iosys_map to dma-buf: in fact
> > > > it's just a shim layer to abstract system memory, that can be accessed
> > > > via regular load and store, from IO memory that needs to be acessed via
> > > > arch helpers. Over time the dma-buf-map proved useful outside of buffer
> > > > sharing among drivers and started to be used in helper functions for
> > > > allocation and generic use. See e.g.
> > > > 
> > > >     drivers/gpu/drm/drm_gem_shmem_helper.c
> > > >     drivers/gpu/drm/drm_gem_framebuffer_helper.c
> > > >     drivers/gpu/drm/drm_fb_helper.c
> > > 
> > > Well, that was the original motivation: framebuffer memory can be
> > > located in system or I/O memory, and even change their location. For
> > > some architectures this makes difference. IIRC the framebuffer
> > > console crashed on sparc64 because we didn't access I/O memory in
> > > the correct way. Hence, we added dma_buf_map to return the type of
> > > memory from dma_buf_vmap/dma_buf_vunmap.  And because everything is
> > > tied together, we had quite a bit of churn throughout the DRM/media
> > > code. There are
> > 
> > Thanks for the history on this. It's helpful. My motivation for starting
> > using dma_buf_map was very similar:  i915 is full of direct load/store.
> > It works on x86 for IO memory. While testing it on arm64 the story
> > was not kind the same. Since we want to support archs other than x86 we
> > need to start using some abstractions and dma_buf_map seemed a nice fit
> > :)
> > 
> > > still places in DRM where we access the raw pointers within
> > > dma_buf_map. We need to clean this up at some point.
> > 
> > at least those are easier to find than just the raw pointers without
> > the abstraction
> > 
> > > 
> > > > 
> > > > In the i915 driver it's also desired to share the implementation for
> > > > integrated graphics, which uses mostly system memory, with discrete
> > > > graphics, which may need to access IO memory.
> > > > 
> > > > Once all the drivers using dma_buf_map are converted, the dma_buf_map
> > > > can be retired and iosys_map extended to cover new use cases.
> > > 
> > > Wrt the renaming: the old name isn't good and the new one isn't good
> > > either. But I don't have strong feelings for either of them.
> > 
> > :(. As the quote: "There are only two hard things in Computer Science:
> > cache invalidation and naming things."
> 
> :D
> 
> > 
> > [ oh, and I'm also having to deal with cache invalidation to properly
> >    support other archs. ]
> > 
> > At least I had 2 people saying the name was ok.
> > 
> > > 
> > > > 
> > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > ---
> > > >  MAINTAINERS                 |   1 +
> > > >  include/linux/dma-buf-map.h |   3 +
> > > >  include/linux/iosys-map.h   | 254 ++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 258 insertions(+)
> > > >  create mode 100644 include/linux/iosys-map.h
> > > > 
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index ea3e6c914384..27ebaded85f8 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -5734,6 +5734,7 @@ F:    Documentation/driver-api/dma-buf.rst
> > > >  F:    drivers/dma-buf/
> > > >  F:    include/linux/*fence.h
> > > >  F:    include/linux/dma-buf*
> > > > +F:    include/linux/iosys-map.h
> > > 
> > > If anything, I'd complain tha twe now have something in dma-buf that
> > > isn't obviously connected to dma-buf.
> > 
> > Let's see if others come with a better name
> 
> Maybe we can move that file into some other module? Driver core or memory
> management?

Greg tends to be the fallback for random drivers stuff. Maybe also a good
idea to move the kerneldoc include to the driver writer's handbook. Or
well it's there already, but into a new separate section.

Adding Greg for input, also maybe he has a good idea on the bikeshed :-)
-Daniel

> 
> Best regards
> Thomas
> 
> > 
> > thanks
> > Lucas De Marchi
> > 
> > > 
> > > Best regards
> > > Thomas
> > > 
> > > >  F:    include/linux/dma-resv.h
> > > >  K:    \bdma_(?:buf|fence|resv)\b
> > > > diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
> > > > index 19fa0b5ae5ec..4b4b2930660b 100644
> > > > --- a/include/linux/dma-buf-map.h
> > > > +++ b/include/linux/dma-buf-map.h
> > > > @@ -263,4 +263,7 @@ static inline void dma_buf_map_incr(struct
> > > > dma_buf_map *map, size_t incr)
> > > >          map->vaddr += incr;
> > > >  }
> > > > +/* Temporary include for API migration */
> > > > +#include <linux/iosys-map.h>
> > > > +
> > > >  #endif /* __DMA_BUF_MAP_H__ */
> > > > diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h
> > > > new file mode 100644
> > > > index 000000000000..ad1f08f8f97f
> > > > --- /dev/null
> > > > +++ b/include/linux/iosys-map.h
> > > > @@ -0,0 +1,254 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > > +/*
> > > > + * Pointer abstraction for IO/system memory
> > > > + */
> > > > +
> > > > +#ifndef __IOSYS_MAP_H__
> > > > +#define __IOSYS_MAP_H__
> > > > +
> > > > +#include <linux/io.h>
> > > > +#include <linux/string.h>
> > > > +
> > > > +/* Temporary include while user of dma-buf-map are converted to
> > > > iosys-map */
> > > > +#include <linux/dma-buf-map.h>
> > > > +
> > > > +/**
> > > > + * DOC: overview
> > > > + *
> > > > + * When accessing a memory region, depending on the its
> > > > location, users may have
> > > > + * to access it with I/O operations or memory load/store
> > > > operations. For
> > > > + * example, copying to system memory could be done with
> > > > memcpy(), copying to I/O
> > > > + * memory would be done with memcpy_toio().
> > > > + *
> > > > + * .. code-block:: c
> > > > + *
> > > > + *    void *vaddr = ...; // pointer to system memory
> > > > + *    memcpy(vaddr, src, len);
> > > > + *
> > > > + *    void *vaddr_iomem = ...; // pointer to I/O memory
> > > > + *    memcpy_toio(vaddr, _iomem, src, len);
> > > > + *
> > > > + * The user of such pointer may not have information about the
> > > > mapping of that
> > > > + * region or may want to have a single code path to handle
> > > > operations on that
> > > > + * buffer, regardless if it's located in system or IO memory. The type
> > > > + * :c:type:`struct iosys_map <iosys_map>` and its helpers
> > > > abstract that so the
> > > > + * buffer can be passed around to other drivers or have
> > > > separate duties inside
> > > > + * the same driver for allocation, read and write operations.
> > > > + *
> > > > + * Open-coding access to :c:type:`struct iosys_map <iosys_map>`
> > > > is considered
> > > > + * bad style. Rather then accessing its fields directly, use
> > > > one of the provided
> > > > + * helper functions, or implement your own. For example, instances of
> > > > + * :c:type:`struct iosys_map <iosys_map>` can be initialized
> > > > statically with
> > > > + * IOSYS_MAP_INIT_VADDR(), or at runtime with
> > > > iosys_map_set_vaddr(). These
> > > > + * helpers will set an address in system memory.
> > > > + *
> > > > + * .. code-block:: c
> > > > + *
> > > > + *    struct iosys_map map = IOSYS_MAP_INIT_VADDR(0xdeadbeaf);
> > > > + *
> > > > + *    iosys_map_set_vaddr(&map, 0xdeadbeaf);
> > > > + *
> > > > + * To set an address in I/O memory, use iosys_map_set_vaddr_iomem().
> > > > + *
> > > > + * .. code-block:: c
> > > > + *
> > > > + *    iosys_map_set_vaddr_iomem(&map, 0xdeadbeaf);
> > > > + *
> > > > + * Instances of struct iosys_map do not have to be cleaned up, but
> > > > + * can be cleared to NULL with iosys_map_clear(). Cleared mappings
> > > > + * always refer to system memory.
> > > > + *
> > > > + * .. code-block:: c
> > > > + *
> > > > + *    iosys_map_clear(&map);
> > > > + *
> > > > + * Test if a mapping is valid with either iosys_map_is_set() or
> > > > + * iosys_map_is_null().
> > > > + *
> > > > + * .. code-block:: c
> > > > + *
> > > > + *    if (iosys_map_is_set(&map) != iosys_map_is_null(&map))
> > > > + *        // always true
> > > > + *
> > > > + * Instances of :c:type:`struct iosys_map <iosys_map>` can be
> > > > compared for
> > > > + * equality with iosys_map_is_equal(). Mappings that point to
> > > > different memory
> > > > + * spaces, system or I/O, are never equal. That's even true if
> > > > both spaces are
> > > > + * located in the same address space, both mappings contain the
> > > > same address
> > > > + * value, or both mappings refer to NULL.
> > > > + *
> > > > + * .. code-block:: c
> > > > + *
> > > > + *    struct iosys_map sys_map; // refers to system memory
> > > > + *    struct iosys_map io_map; // refers to I/O memory
> > > > + *
> > > > + *    if (iosys_map_is_equal(&sys_map, &io_map))
> > > > + *        // always false
> > > > + *
> > > > + * A set up instance of struct iosys_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
> > > > + * iosys_map_memcpy_to(). The address can be manipulated with
> > > > iosys_map_incr().
> > > > + *
> > > > + * .. code-block:: c
> > > > + *
> > > > + *    const void *src = ...; // source buffer
> > > > + *    size_t len = ...; // length of src
> > > > + *
> > > > + *    iosys_map_memcpy_to(&map, src, len);
> > > > + *    iosys_map_incr(&map, len); // go to first byte after the memcpy
> > > > + */
> > > > +
> > > > +/**
> > > > + * struct iosys_map - Pointer to IO/system memory
> > > > + * @vaddr_iomem:    The buffer's address if in I/O memory
> > > > + * @vaddr:        The buffer's address if in system memory
> > > > + * @is_iomem:        True if the buffer is located in I/O
> > > > memory, or false
> > > > + *            otherwise.
> > > > + */
> > > > +#define iosys_map dma_buf_map
> > > > +
> > > > +/**
> > > > + * IOSYS_MAP_INIT_VADDR - Initializes struct iosys_map to an
> > > > address in system memory
> > > > + * @vaddr_:    A system-memory address
> > > > + */
> > > > +#define IOSYS_MAP_INIT_VADDR(vaddr_)    \
> > > > +    {                \
> > > > +        .vaddr = (vaddr_),    \
> > > > +        .is_iomem = false,    \
> > > > +    }
> > > > +
> > > > +/**
> > > > + * iosys_map_set_vaddr - Sets a iosys mapping structure to an
> > > > address in system memory
> > > > + * @map:    The iosys_map structure
> > > > + * @vaddr:    A system-memory address
> > > > + *
> > > > + * Sets the address and clears the I/O-memory flag.
> > > > + */
> > > > +static inline void iosys_map_set_vaddr(struct iosys_map *map,
> > > > void *vaddr)
> > > > +{
> > > > +    map->vaddr = vaddr;
> > > > +    map->is_iomem = false;
> > > > +}
> > > > +
> > > > +/**
> > > > + * iosys_map_set_vaddr_iomem - Sets a iosys mapping structure
> > > > to an address in I/O memory
> > > > + * @map:        The iosys_map structure
> > > > + * @vaddr_iomem:    An I/O-memory address
> > > > + *
> > > > + * Sets the address and the I/O-memory flag.
> > > > + */
> > > > +static inline void iosys_map_set_vaddr_iomem(struct iosys_map *map,
> > > > +                         void __iomem *vaddr_iomem)
> > > > +{
> > > > +    map->vaddr_iomem = vaddr_iomem;
> > > > +    map->is_iomem = true;
> > > > +}
> > > > +
> > > > +/**
> > > > + * iosys_map_is_equal - Compares two iosys mapping structures
> > > > for equality
> > > > + * @lhs:    The iosys_map structure
> > > > + * @rhs:    A iosys_map structure to compare with
> > > > + *
> > > > + * Two iosys mapping structures are equal if they both refer to
> > > > the same type of memory
> > > > + * and to the same address within that memory.
> > > > + *
> > > > + * Returns:
> > > > + * True is both structures are equal, or false otherwise.
> > > > + */
> > > > +static inline bool iosys_map_is_equal(const struct iosys_map *lhs,
> > > > +                      const struct iosys_map *rhs)
> > > > +{
> > > > +    if (lhs->is_iomem != rhs->is_iomem)
> > > > +        return false;
> > > > +    else if (lhs->is_iomem)
> > > > +        return lhs->vaddr_iomem == rhs->vaddr_iomem;
> > > > +    else
> > > > +        return lhs->vaddr == rhs->vaddr;
> > > > +}
> > > > +
> > > > +/**
> > > > + * iosys_map_is_null - Tests for a iosys mapping to be NULL
> > > > + * @map:    The iosys_map structure
> > > > + *
> > > > + * Depending on the state of struct iosys_map.is_iomem, tests if the
> > > > + * mapping is NULL.
> > > > + *
> > > > + * Returns:
> > > > + * True if the mapping is NULL, or false otherwise.
> > > > + */
> > > > +static inline bool iosys_map_is_null(const struct iosys_map *map)
> > > > +{
> > > > +    if (map->is_iomem)
> > > > +        return !map->vaddr_iomem;
> > > > +    return !map->vaddr;
> > > > +}
> > > > +
> > > > +/**
> > > > + * iosys_map_is_set - Tests if the iosys mapping has been set
> > > > + * @map:    The iosys_map structure
> > > > + *
> > > > + * Depending on the state of struct iosys_map.is_iomem, tests if the
> > > > + * mapping has been set.
> > > > + *
> > > > + * Returns:
> > > > + * True if the mapping is been set, or false otherwise.
> > > > + */
> > > > +static inline bool iosys_map_is_set(const struct iosys_map *map)
> > > > +{
> > > > +    return !iosys_map_is_null(map);
> > > > +}
> > > > +
> > > > +/**
> > > > + * iosys_map_clear - Clears a iosys mapping structure
> > > > + * @map:    The iosys_map structure
> > > > + *
> > > > + * Clears all fields to zero, including struct iosys_map.is_iomem, so
> > > > + * mapping structures that were set to point to I/O memory are
> > > > reset for
> > > > + * system memory. Pointers are cleared to NULL. This is the default.
> > > > + */
> > > > +static inline void iosys_map_clear(struct iosys_map *map)
> > > > +{
> > > > +    if (map->is_iomem) {
> > > > +        map->vaddr_iomem = NULL;
> > > > +        map->is_iomem = false;
> > > > +    } else {
> > > > +        map->vaddr = NULL;
> > > > +    }
> > > > +}
> > > > +
> > > > +/**
> > > > + * iosys_map_memcpy_to - Memcpy into iosys mapping
> > > > + * @dst:    The iosys_map structure
> > > > + * @src:    The source buffer
> > > > + * @len:    The number of byte in src
> > > > + *
> > > > + * Copies data into a iosys 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 iosys_map_memcpy_to(struct iosys_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);
> > > > +}
> > > > +
> > > > +/**
> > > > + * iosys_map_incr - Increments the address stored in a iosys mapping
> > > > + * @map:    The iosys_map structure
> > > > + * @incr:    The number of bytes to increment
> > > > + *
> > > > + * Increments the address stored in a iosys mapping. Depending on the
> > > > + * buffer's location, the correct value will be updated.
> > > > + */
> > > > +static inline void iosys_map_incr(struct iosys_map *map, size_t incr)
> > > > +{
> > > > +    if (map->is_iomem)
> > > > +        map->vaddr_iomem += incr;
> > > > +    else
> > > > +        map->vaddr += incr;
> > > > +}
> > > > +
> > > > +#endif /* __IOSYS_MAP_H__ */
> > > 
> > > -- 
> > > Thomas Zimmermann
> > > Graphics Driver Developer
> > > SUSE Software Solutions Germany GmbH
> > > Maxfeldstr. 5, 90409 Nürnberg, Germany
> > > (HRB 36809, AG Nürnberg)
> > > Geschäftsführer: Ivo Totev
> > 
> > 
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index ea3e6c914384..27ebaded85f8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5734,6 +5734,7 @@  F:	Documentation/driver-api/dma-buf.rst
 F:	drivers/dma-buf/
 F:	include/linux/*fence.h
 F:	include/linux/dma-buf*
+F:	include/linux/iosys-map.h
 F:	include/linux/dma-resv.h
 K:	\bdma_(?:buf|fence|resv)\b
 
diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
index 19fa0b5ae5ec..4b4b2930660b 100644
--- a/include/linux/dma-buf-map.h
+++ b/include/linux/dma-buf-map.h
@@ -263,4 +263,7 @@  static inline void dma_buf_map_incr(struct dma_buf_map *map, size_t incr)
 		map->vaddr += incr;
 }
 
+/* Temporary include for API migration */
+#include <linux/iosys-map.h>
+
 #endif /* __DMA_BUF_MAP_H__ */
diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h
new file mode 100644
index 000000000000..ad1f08f8f97f
--- /dev/null
+++ b/include/linux/iosys-map.h
@@ -0,0 +1,254 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Pointer abstraction for IO/system memory
+ */
+
+#ifndef __IOSYS_MAP_H__
+#define __IOSYS_MAP_H__
+
+#include <linux/io.h>
+#include <linux/string.h>
+
+/* Temporary include while user of dma-buf-map are converted to iosys-map */
+#include <linux/dma-buf-map.h>
+
+/**
+ * DOC: overview
+ *
+ * When accessing a memory region, depending on the its location, users may have
+ * to access it with I/O operations or memory load/store operations. For
+ * example, copying to system memory could be done with memcpy(), copying to I/O
+ * memory would be done with memcpy_toio().
+ *
+ * .. code-block:: c
+ *
+ *	void *vaddr = ...; // pointer to system memory
+ *	memcpy(vaddr, src, len);
+ *
+ *	void *vaddr_iomem = ...; // pointer to I/O memory
+ *	memcpy_toio(vaddr, _iomem, src, len);
+ *
+ * The user of such pointer may not have information about the mapping of that
+ * region or may want to have a single code path to handle operations on that
+ * buffer, regardless if it's located in system or IO memory. The type
+ * :c:type:`struct iosys_map <iosys_map>` and its helpers abstract that so the
+ * buffer can be passed around to other drivers or have separate duties inside
+ * the same driver for allocation, read and write operations.
+ *
+ * Open-coding access to :c:type:`struct iosys_map <iosys_map>` is considered
+ * bad style. Rather then accessing its fields directly, use one of the provided
+ * helper functions, or implement your own. For example, instances of
+ * :c:type:`struct iosys_map <iosys_map>` can be initialized statically with
+ * IOSYS_MAP_INIT_VADDR(), or at runtime with iosys_map_set_vaddr(). These
+ * helpers will set an address in system memory.
+ *
+ * .. code-block:: c
+ *
+ *	struct iosys_map map = IOSYS_MAP_INIT_VADDR(0xdeadbeaf);
+ *
+ *	iosys_map_set_vaddr(&map, 0xdeadbeaf);
+ *
+ * To set an address in I/O memory, use iosys_map_set_vaddr_iomem().
+ *
+ * .. code-block:: c
+ *
+ *	iosys_map_set_vaddr_iomem(&map, 0xdeadbeaf);
+ *
+ * Instances of struct iosys_map do not have to be cleaned up, but
+ * can be cleared to NULL with iosys_map_clear(). Cleared mappings
+ * always refer to system memory.
+ *
+ * .. code-block:: c
+ *
+ *	iosys_map_clear(&map);
+ *
+ * Test if a mapping is valid with either iosys_map_is_set() or
+ * iosys_map_is_null().
+ *
+ * .. code-block:: c
+ *
+ *	if (iosys_map_is_set(&map) != iosys_map_is_null(&map))
+ *		// always true
+ *
+ * Instances of :c:type:`struct iosys_map <iosys_map>` can be compared for
+ * equality with iosys_map_is_equal(). Mappings that point to different memory
+ * spaces, system or I/O, are never equal. That's even true if both spaces are
+ * located in the same address space, both mappings contain the same address
+ * value, or both mappings refer to NULL.
+ *
+ * .. code-block:: c
+ *
+ *	struct iosys_map sys_map; // refers to system memory
+ *	struct iosys_map io_map; // refers to I/O memory
+ *
+ *	if (iosys_map_is_equal(&sys_map, &io_map))
+ *		// always false
+ *
+ * A set up instance of struct iosys_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
+ * iosys_map_memcpy_to(). The address can be manipulated with iosys_map_incr().
+ *
+ * .. code-block:: c
+ *
+ *	const void *src = ...; // source buffer
+ *	size_t len = ...; // length of src
+ *
+ *	iosys_map_memcpy_to(&map, src, len);
+ *	iosys_map_incr(&map, len); // go to first byte after the memcpy
+ */
+
+/**
+ * struct iosys_map - Pointer to IO/system memory
+ * @vaddr_iomem:	The buffer's address if in I/O memory
+ * @vaddr:		The buffer's address if in system memory
+ * @is_iomem:		True if the buffer is located in I/O memory, or false
+ *			otherwise.
+ */
+#define iosys_map dma_buf_map
+
+/**
+ * IOSYS_MAP_INIT_VADDR - Initializes struct iosys_map to an address in system memory
+ * @vaddr_:	A system-memory address
+ */
+#define IOSYS_MAP_INIT_VADDR(vaddr_)	\
+	{				\
+		.vaddr = (vaddr_),	\
+		.is_iomem = false,	\
+	}
+
+/**
+ * iosys_map_set_vaddr - Sets a iosys mapping structure to an address in system memory
+ * @map:	The iosys_map structure
+ * @vaddr:	A system-memory address
+ *
+ * Sets the address and clears the I/O-memory flag.
+ */
+static inline void iosys_map_set_vaddr(struct iosys_map *map, void *vaddr)
+{
+	map->vaddr = vaddr;
+	map->is_iomem = false;
+}
+
+/**
+ * iosys_map_set_vaddr_iomem - Sets a iosys mapping structure to an address in I/O memory
+ * @map:		The iosys_map structure
+ * @vaddr_iomem:	An I/O-memory address
+ *
+ * Sets the address and the I/O-memory flag.
+ */
+static inline void iosys_map_set_vaddr_iomem(struct iosys_map *map,
+					     void __iomem *vaddr_iomem)
+{
+	map->vaddr_iomem = vaddr_iomem;
+	map->is_iomem = true;
+}
+
+/**
+ * iosys_map_is_equal - Compares two iosys mapping structures for equality
+ * @lhs:	The iosys_map structure
+ * @rhs:	A iosys_map structure to compare with
+ *
+ * Two iosys mapping structures are equal if they both refer to the same type of memory
+ * and to the same address within that memory.
+ *
+ * Returns:
+ * True is both structures are equal, or false otherwise.
+ */
+static inline bool iosys_map_is_equal(const struct iosys_map *lhs,
+				      const struct iosys_map *rhs)
+{
+	if (lhs->is_iomem != rhs->is_iomem)
+		return false;
+	else if (lhs->is_iomem)
+		return lhs->vaddr_iomem == rhs->vaddr_iomem;
+	else
+		return lhs->vaddr == rhs->vaddr;
+}
+
+/**
+ * iosys_map_is_null - Tests for a iosys mapping to be NULL
+ * @map:	The iosys_map structure
+ *
+ * Depending on the state of struct iosys_map.is_iomem, tests if the
+ * mapping is NULL.
+ *
+ * Returns:
+ * True if the mapping is NULL, or false otherwise.
+ */
+static inline bool iosys_map_is_null(const struct iosys_map *map)
+{
+	if (map->is_iomem)
+		return !map->vaddr_iomem;
+	return !map->vaddr;
+}
+
+/**
+ * iosys_map_is_set - Tests if the iosys mapping has been set
+ * @map:	The iosys_map structure
+ *
+ * Depending on the state of struct iosys_map.is_iomem, tests if the
+ * mapping has been set.
+ *
+ * Returns:
+ * True if the mapping is been set, or false otherwise.
+ */
+static inline bool iosys_map_is_set(const struct iosys_map *map)
+{
+	return !iosys_map_is_null(map);
+}
+
+/**
+ * iosys_map_clear - Clears a iosys mapping structure
+ * @map:	The iosys_map structure
+ *
+ * Clears all fields to zero, including struct iosys_map.is_iomem, so
+ * mapping structures that were set to point to I/O memory are reset for
+ * system memory. Pointers are cleared to NULL. This is the default.
+ */
+static inline void iosys_map_clear(struct iosys_map *map)
+{
+	if (map->is_iomem) {
+		map->vaddr_iomem = NULL;
+		map->is_iomem = false;
+	} else {
+		map->vaddr = NULL;
+	}
+}
+
+/**
+ * iosys_map_memcpy_to - Memcpy into iosys mapping
+ * @dst:	The iosys_map structure
+ * @src:	The source buffer
+ * @len:	The number of byte in src
+ *
+ * Copies data into a iosys 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 iosys_map_memcpy_to(struct iosys_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);
+}
+
+/**
+ * iosys_map_incr - Increments the address stored in a iosys mapping
+ * @map:	The iosys_map structure
+ * @incr:	The number of bytes to increment
+ *
+ * Increments the address stored in a iosys mapping. Depending on the
+ * buffer's location, the correct value will be updated.
+ */
+static inline void iosys_map_incr(struct iosys_map *map, size_t incr)
+{
+	if (map->is_iomem)
+		map->vaddr_iomem += incr;
+	else
+		map->vaddr += incr;
+}
+
+#endif /* __IOSYS_MAP_H__ */