[PATCH] dma-buf: add dma_data_direction to unmap dma_buf_op

[PATCH] dma-buf: add dma_data_direction to unmap dma_buf_op

[Sumit Semwal]

Some exporters may use DMA map/unmap APIs in dma-buf ops, which require
enum dma_data_direction while unmapping.

Thus, the unmap dma_buf_op also needs to have enum dma_data_direction as
a parameter.

Reported-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
---
 drivers/base/Kconfig    |    2 +-
 drivers/base/dma-buf.c  |    7 +++++--
 include/linux/dma-buf.h |    8 +++++---
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 7be9f79..5edc5db 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -179,7 +179,7 @@ config GENERIC_CPU_DEVICES
 source "drivers/base/regmap/Kconfig"
 
 config DMA_SHARED_BUFFER
-	bool
+	bool "Temporary mechanism to enable build of dma-buf"
 	default n
 	select ANON_INODES
 	depends on EXPERIMENTAL
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 8afe2dd..c9a945f 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -271,16 +271,19 @@ EXPORT_SYMBOL_GPL(dma_buf_map_attachment);
  * dma_buf_ops.
  * @attach:	[in]	attachment to unmap buffer from
  * @sg_table:	[in]	scatterlist info of the buffer to unmap
+ * @direction:  [in]    direction of DMA transfer
  *
  */
 void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
-				struct sg_table *sg_table)
+				struct sg_table *sg_table,
+				enum dma_data_direction direction)
 {
 	if (WARN_ON(!attach || !attach->dmabuf || !sg_table))
 		return;
 
 	mutex_lock(&attach->dmabuf->lock);
-	attach->dmabuf->ops->unmap_dma_buf(attach, sg_table);
+	attach->dmabuf->ops->unmap_dma_buf(attach, sg_table,
+						direction);
 	mutex_unlock(&attach->dmabuf->lock);
 
 }
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 86f6241..847b026 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -63,7 +63,8 @@ struct dma_buf_ops {
 	struct sg_table * (*map_dma_buf)(struct dma_buf_attachment *,
 						enum dma_data_direction);
 	void (*unmap_dma_buf)(struct dma_buf_attachment *,
-						struct sg_table *);
+						struct sg_table *,
+						enum dma_data_direction);
 	/* TODO: Add try_map_dma_buf version, to return immed with -EBUSY
 	 * if the call would block.
 	 */
@@ -122,7 +123,8 @@ void dma_buf_put(struct dma_buf *dmabuf);
 
 struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *,
 					enum dma_data_direction);
-void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *);
+void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *,
+				enum dma_data_direction);
 #else
 
 static inline struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
@@ -166,7 +168,7 @@ static inline struct sg_table *dma_buf_map_attachment(
 }
 
 static inline void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
-						struct sg_table *sg)
+			struct sg_table *sg, enum dma_data_direction write)
 {
 	return;
 }
-- 
1.7.5.4

Re: [PATCH] dma-buf: add dma_data_direction to unmap dma_buf_op

[Semwal, Sumit]

Please ignore! I will send out a new version in a minute.

Thanks and best regards,
~Sumit.



On Fri, Jan 27, 2012 at 3:04 PM, Sumit Semwal <sumit.semwal@ti.com> wrote:
> Some exporters may use DMA map/unmap APIs in dma-buf ops, which require
> enum dma_data_direction while unmapping.
>
> Thus, the unmap dma_buf_op also needs to have enum dma_data_direction as
> a parameter.
>
> Reported-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
> ---
>  drivers/base/Kconfig    |    2 +-
>  drivers/base/dma-buf.c  |    7 +++++--
>  include/linux/dma-buf.h |    8 +++++---
>  3 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 7be9f79..5edc5db 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -179,7 +179,7 @@ config GENERIC_CPU_DEVICES
>  source "drivers/base/regmap/Kconfig"
>
>  config DMA_SHARED_BUFFER
> -       bool
> +       bool "Temporary mechanism to enable build of dma-buf"
>        default n
>        select ANON_INODES
>        depends on EXPERIMENTAL
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index 8afe2dd..c9a945f 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -271,16 +271,19 @@ EXPORT_SYMBOL_GPL(dma_buf_map_attachment);
>  * dma_buf_ops.
>  * @attach:    [in]    attachment to unmap buffer from
>  * @sg_table:  [in]    scatterlist info of the buffer to unmap
> + * @direction:  [in]    direction of DMA transfer
>  *
>  */
>  void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
> -                               struct sg_table *sg_table)
> +                               struct sg_table *sg_table,
> +                               enum dma_data_direction direction)
>  {
>        if (WARN_ON(!attach || !attach->dmabuf || !sg_table))
>                return;
>
>        mutex_lock(&attach->dmabuf->lock);
> -       attach->dmabuf->ops->unmap_dma_buf(attach, sg_table);
> +       attach->dmabuf->ops->unmap_dma_buf(attach, sg_table,
> +                                               direction);
>        mutex_unlock(&attach->dmabuf->lock);
>
>  }
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 86f6241..847b026 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -63,7 +63,8 @@ struct dma_buf_ops {
>        struct sg_table * (*map_dma_buf)(struct dma_buf_attachment *,
>                                                enum dma_data_direction);
>        void (*unmap_dma_buf)(struct dma_buf_attachment *,
> -                                               struct sg_table *);
> +                                               struct sg_table *,
> +                                               enum dma_data_direction);
>        /* TODO: Add try_map_dma_buf version, to return immed with -EBUSY
>         * if the call would block.
>         */
> @@ -122,7 +123,8 @@ void dma_buf_put(struct dma_buf *dmabuf);
>
>  struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *,
>                                        enum dma_data_direction);
> -void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *);
> +void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *,
> +                               enum dma_data_direction);
>  #else
>
>  static inline struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
> @@ -166,7 +168,7 @@ static inline struct sg_table *dma_buf_map_attachment(
>  }
>
>  static inline void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
> -                                               struct sg_table *sg)
> +                       struct sg_table *sg, enum dma_data_direction write)
>  {
>        return;
>  }
> --
> 1.7.5.4
>

[PATCH] dma-buf: add dma_data_direction to unmap dma_buf_op

[Sumit Semwal]

Some exporters may use DMA map/unmap APIs in dma-buf ops, which require
enum dma_data_direction for both map and unmap operations.

Thus, the unmap dma_buf_op also needs to have enum dma_data_direction as
a parameter.

Reported-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
---
 drivers/base/dma-buf.c  |    7 +++++--
 include/linux/dma-buf.h |    8 +++++---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 8afe2dd..c9a945f 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -271,16 +271,19 @@ EXPORT_SYMBOL_GPL(dma_buf_map_attachment);
  * dma_buf_ops.
  * @attach:	[in]	attachment to unmap buffer from
  * @sg_table:	[in]	scatterlist info of the buffer to unmap
+ * @direction:  [in]    direction of DMA transfer
  *
  */
 void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
-				struct sg_table *sg_table)
+				struct sg_table *sg_table,
+				enum dma_data_direction direction)
 {
 	if (WARN_ON(!attach || !attach->dmabuf || !sg_table))
 		return;
 
 	mutex_lock(&attach->dmabuf->lock);
-	attach->dmabuf->ops->unmap_dma_buf(attach, sg_table);
+	attach->dmabuf->ops->unmap_dma_buf(attach, sg_table,
+						direction);
 	mutex_unlock(&attach->dmabuf->lock);
 
 }
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 86f6241..847b026 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -63,7 +63,8 @@ struct dma_buf_ops {
 	struct sg_table * (*map_dma_buf)(struct dma_buf_attachment *,
 						enum dma_data_direction);
 	void (*unmap_dma_buf)(struct dma_buf_attachment *,
-						struct sg_table *);
+						struct sg_table *,
+						enum dma_data_direction);
 	/* TODO: Add try_map_dma_buf version, to return immed with -EBUSY
 	 * if the call would block.
 	 */
@@ -122,7 +123,8 @@ void dma_buf_put(struct dma_buf *dmabuf);
 
 struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *,
 					enum dma_data_direction);
-void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *);
+void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *,
+				enum dma_data_direction);
 #else
 
 static inline struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
@@ -166,7 +168,7 @@ static inline struct sg_table *dma_buf_map_attachment(
 }
 
 static inline void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
-						struct sg_table *sg)
+			struct sg_table *sg, enum dma_data_direction write)
 {
 	return;
 }
-- 
1.7.5.4

Re: [PATCH] dma-buf: add dma_data_direction to unmap dma_buf_op

[Daniel Vetter]

On Fri, Jan 27, 2012 at 03:13:28PM +0530, Sumit Semwal wrote:
> Some exporters may use DMA map/unmap APIs in dma-buf ops, which require
> enum dma_data_direction for both map and unmap operations.
> 
> Thus, the unmap dma_buf_op also needs to have enum dma_data_direction as
> a parameter.
> 
> Reported-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] dma-buf: add dma_data_direction to unmap dma_buf_op

[Laurent Pinchart]

Hi Sumit,

On Friday 27 January 2012 10:43:28 Sumit Semwal wrote:
> Some exporters may use DMA map/unmap APIs in dma-buf ops, which require
> enum dma_data_direction for both map and unmap operations.
> 
> Thus, the unmap dma_buf_op also needs to have enum dma_data_direction as
> a parameter.
> 
> Reported-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
> ---
>  drivers/base/dma-buf.c  |    7 +++++--
>  include/linux/dma-buf.h |    8 +++++---
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index 8afe2dd..c9a945f 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -271,16 +271,19 @@ EXPORT_SYMBOL_GPL(dma_buf_map_attachment);
>   * dma_buf_ops.
>   * @attach:	[in]	attachment to unmap buffer from
>   * @sg_table:	[in]	scatterlist info of the buffer to unmap
> + * @direction:  [in]    direction of DMA transfer
>   *
>   */
>  void dma_buf_unmap_attachment(struct dma_buf_attachment *attach,
> -				struct sg_table *sg_table)
> +				struct sg_table *sg_table,
> +				enum dma_data_direction direction)
>  {
>  	if (WARN_ON(!attach || !attach->dmabuf || !sg_table))
>  		return;
> 
>  	mutex_lock(&attach->dmabuf->lock);
> -	attach->dmabuf->ops->unmap_dma_buf(attach, sg_table);
> +	attach->dmabuf->ops->unmap_dma_buf(attach, sg_table,
> +						direction);
>  	mutex_unlock(&attach->dmabuf->lock);
> 
>  }
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 86f6241..847b026 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -63,7 +63,8 @@ struct dma_buf_ops {
>  	struct sg_table * (*map_dma_buf)(struct dma_buf_attachment *,
>  						enum dma_data_direction);
>  	void (*unmap_dma_buf)(struct dma_buf_attachment *,
> -						struct sg_table *);
> +						struct sg_table *,
> +						enum dma_data_direction);
>  	/* TODO: Add try_map_dma_buf version, to return immed with -EBUSY
>  	 * if the call would block.
>  	 */
> @@ -122,7 +123,8 @@ void dma_buf_put(struct dma_buf *dmabuf);
> 
>  struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *,
>  					enum dma_data_direction);
> -void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table
> *); +void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct
> sg_table *, +				enum dma_data_direction);
>  #else
> 
>  static inline struct dma_buf_attachment *dma_buf_attach(struct dma_buf
> *dmabuf, @@ -166,7 +168,7 @@ static inline struct sg_table
> *dma_buf_map_attachment( }
> 
>  static inline void dma_buf_unmap_attachment(struct dma_buf_attachment
> *attach, -						struct sg_table *sg)
> +			struct sg_table *sg, enum dma_data_direction write)

s/write/dir/ (or direction) ?

>  {
>  	return;
>  }

-- 
Regards,

Laurent Pinchart

Re: [PATCH] dma-buf: add dma_data_direction to unmap dma_buf_op

[Laurent Pinchart]

Hi Sumit,

> On Friday 27 January 2012 10:43:28 Sumit Semwal wrote:

[snip]

>  static inline void dma_buf_unmap_attachment(struct dma_buf_attachment
> *attach,
> -                                            struct sg_table *sg)
> +                     struct sg_table *sg, enum dma_data_direction write)

On a second thought, would it make sense to store the direction in struct 
dma_buf_attachment in dma_buf_map_attachment(), and pass the value directly to 
the .unmap_dma_buf() instead of requiring the dma_buf_unmap_attachment() 
caller to remember it ? Or is an attachment allowed to map the buffer several 
times with different directions ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] dma-buf: add dma_data_direction to unmap dma_buf_op

[Daniel Vetter]

On Tue, Jan 31, 2012 at 10:42:59AM +0100, Laurent Pinchart wrote:
> Hi Sumit,
> 
> > On Friday 27 January 2012 10:43:28 Sumit Semwal wrote:
> 
> [snip]
> 
> >  static inline void dma_buf_unmap_attachment(struct dma_buf_attachment
> > *attach,
> > -                                            struct sg_table *sg)
> > +                     struct sg_table *sg, enum dma_data_direction write)
> 
> On a second thought, would it make sense to store the direction in struct 
> dma_buf_attachment in dma_buf_map_attachment(), and pass the value directly to 
> the .unmap_dma_buf() instead of requiring the dma_buf_unmap_attachment() 
> caller to remember it ? Or is an attachment allowed to map the buffer several 
> times with different directions ?

Current dma api functions already require you to supply the direction
argument on unmap and I think for cpu access I'm also leaning towards an
interface where the importer has to supply the direction argument for both
begin_access and end_access. So for consistency reasons I'm leaning
towards adding it to unmap.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] dma-buf: add dma_data_direction to unmap dma_buf_op

[Laurent Pinchart]

Hi Daniel,

On Tuesday 31 January 2012 11:36:02 Daniel Vetter wrote:
> On Tue, Jan 31, 2012 at 10:42:59AM +0100, Laurent Pinchart wrote:
> > Hi Sumit,
> > 
> > > On Friday 27 January 2012 10:43:28 Sumit Semwal wrote:
> > [snip]
> > 
> > >  static inline void dma_buf_unmap_attachment(struct dma_buf_attachment
> > > 
> > > *attach,
> > > -                                            struct sg_table *sg)
> > > +                     struct sg_table *sg, enum dma_data_direction
> > > write)
> > 
> > On a second thought, would it make sense to store the direction in struct
> > dma_buf_attachment in dma_buf_map_attachment(), and pass the value
> > directly to the .unmap_dma_buf() instead of requiring the
> > dma_buf_unmap_attachment() caller to remember it ? Or is an attachment
> > allowed to map the buffer several times with different directions ?
> 
> Current dma api functions already require you to supply the direction
> argument on unmap

If I understand it correctly, that's mostly because the DMA API doesn't keep 
track of DMA mappings in a way that it can store the direction on map(), and 
use it on unmap(). In this case we have an attachment object that we can use 
to cache the information.

> and I think for cpu access I'm also leaning towards an interface where the
> importer has to supply the direction argument for both begin_access and
> end_access. So for consistency reasons I'm leaning towards adding it to
> unmap.

I'm OK with keeping the direction as an argument to unmap() if you think 
that's better.

-- 
Regards,

Laurent Pinchart