diff mbox

dma-buf: add vmap interface (v2)

Message ID 1337250710-31113-1-git-send-email-airlied@gmail.com
State Superseded
Headers show

Commit Message

Dave Airlie May 17, 2012, 10:31 a.m. UTC
From: Dave Airlie <airlied@redhat.com>

The main requirement I have for this interface is for scanning out
using the USB gpu devices. Since these devices have to read the
framebuffer on updates and linearly compress it, using kmaps
is a major overhead for every update.

v2: fix warn issues pointed out by Sylwester Nawrocki.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/base/dma-buf.c  |   34 ++++++++++++++++++++++++++++++++++
 include/linux/dma-buf.h |   14 ++++++++++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

Comments

Rob Clark May 17, 2012, 12:32 p.m. UTC | #1
On Thu, May 17, 2012 at 4:31 AM, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> The main requirement I have for this interface is for scanning out
> using the USB gpu devices. Since these devices have to read the
> framebuffer on updates and linearly compress it, using kmaps
> is a major overhead for every update.
>
> v2: fix warn issues pointed out by Sylwester Nawrocki.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/base/dma-buf.c  |   34 ++++++++++++++++++++++++++++++++++
>  include/linux/dma-buf.h |   14 ++++++++++++++
>  2 files changed, 48 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index 07cbbc6..750f92c 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -406,3 +406,37 @@ void dma_buf_kunmap(struct dma_buf *dmabuf, unsigned long page_num,
>                dmabuf->ops->kunmap(dmabuf, page_num, vaddr);
>  }
>  EXPORT_SYMBOL_GPL(dma_buf_kunmap);
> +
> +/**
> + * dma_buf_vmap - Create virtual mapping for the buffer object into kernel address space. The same restrictions as for vmap and friends apply.
> + * @dma_buf:   [in]    buffer to vmap
> + *
> + * This call may fail due to lack of virtual mapping address space.
> + * These calls are optional in drivers. The intended use for them
> + * is for mapping objects linear in kernel space for high use objects.
> + * Please attempt to use kmap/kunmap before thinking about these interfaces.
> + */
> +void *dma_buf_vmap(struct dma_buf *dmabuf)
> +{
> +       if (WARN_ON(!dmabuf))
> +               return NULL;
> +
> +       if (dmabuf->ops->vmap)
> +               return dmabuf->ops->vmap(dmabuf);
> +       return NULL;
> +}
> +EXPORT_SYMBOL(dma_buf_vmap);
> +
> +/**
> + * dma_buf_vunmap - Unmap a vmap obtained by dma_buf_vmap.
> + * @dma_buf:   [in]    buffer to vmap
> + */
> +void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
> +{
> +       if (WARN_ON(!dmabuf))
> +               return;
> +
> +       if (dmabuf->ops->vunmap)
> +               dmabuf->ops->vunmap(dmabuf, vaddr);
> +}
> +EXPORT_SYMBOL(dma_buf_vunmap);
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 3efbfc2..b92b6de 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -92,6 +92,9 @@ struct dma_buf_ops {
>        void (*kunmap_atomic)(struct dma_buf *, unsigned long, void *);
>        void *(*kmap)(struct dma_buf *, unsigned long);
>        void (*kunmap)(struct dma_buf *, unsigned long, void *);
> +
> +       void *(*vmap)(struct dma_buf *);
> +       void (*vunmap)(struct dma_buf *, void *vaddr);
>  };
>
>  /**
> @@ -167,6 +170,9 @@ void *dma_buf_kmap_atomic(struct dma_buf *, unsigned long);
>  void dma_buf_kunmap_atomic(struct dma_buf *, unsigned long, void *);
>  void *dma_buf_kmap(struct dma_buf *, unsigned long);
>  void dma_buf_kunmap(struct dma_buf *, unsigned long, void *);
> +
> +void *dma_buf_vmap(struct dma_buf *);
> +void dma_buf_vunmap(struct dma_buf *, void *vaddr);
>  #else
>
>  static inline struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
> @@ -248,6 +254,14 @@ static inline void dma_buf_kunmap(struct dma_buf *dmabuf,
>                                  unsigned long pnum, void *vaddr)
>  {
>  }
> +
> +static inline void *dma_buf_vmap(struct dma_buf *)
> +{
> +}
> +
> +static inline void dma_buf_vunmap(struct dma_buf *, void *vaddr);
> +{
> +}

I think these two will cause compile issues for
!CONFIG_DMA_SHARED_BUFFER case due to no parameter name for first arg.

Other than that, it looks ok.. although is vmap really less overhead?
Using kmap can use existing lowmem address for lowmem pages.  Or is
the issue that you somehow need access to the entire buffer in one
shot?

BR,
-R

>  #endif /* CONFIG_DMA_SHARED_BUFFER */
>
>  #endif /* __DMA_BUF_H__ */
> --
> 1.7.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Dave Airlie May 17, 2012, 1:14 p.m. UTC | #2
On Thu, May 17, 2012 at 1:32 PM, Rob Clark <rob.clark@linaro.org> wrote:
> On Thu, May 17, 2012 at 4:31 AM, Dave Airlie <airlied@gmail.com> wrote:
>> From: Dave Airlie <airlied@redhat.com>
>>
>> The main requirement I have for this interface is for scanning out
>> using the USB gpu devices. Since these devices have to read the
>> framebuffer on updates and linearly compress it, using kmaps
>> is a major overhead for every update.
>>
>> v2: fix warn issues pointed out by Sylwester Nawrocki.
>>
>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>> ---
>>  drivers/base/dma-buf.c  |   34 ++++++++++++++++++++++++++++++++++
>>  include/linux/dma-buf.h |   14 ++++++++++++++
>>  2 files changed, 48 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
>> index 07cbbc6..750f92c 100644
>> --- a/drivers/base/dma-buf.c
>> +++ b/drivers/base/dma-buf.c
>> @@ -406,3 +406,37 @@ void dma_buf_kunmap(struct dma_buf *dmabuf, unsigned long page_num,
>>                dmabuf->ops->kunmap(dmabuf, page_num, vaddr);
>>  }
>>  EXPORT_SYMBOL_GPL(dma_buf_kunmap);
>> +
>> +/**
>> + * dma_buf_vmap - Create virtual mapping for the buffer object into kernel address space. The same restrictions as for vmap and friends apply.
>> + * @dma_buf:   [in]    buffer to vmap
>> + *
>> + * This call may fail due to lack of virtual mapping address space.
>> + * These calls are optional in drivers. The intended use for them
>> + * is for mapping objects linear in kernel space for high use objects.
>> + * Please attempt to use kmap/kunmap before thinking about these interfaces.
>> + */
>> +void *dma_buf_vmap(struct dma_buf *dmabuf)
>> +{
>> +       if (WARN_ON(!dmabuf))
>> +               return NULL;
>> +
>> +       if (dmabuf->ops->vmap)
>> +               return dmabuf->ops->vmap(dmabuf);
>> +       return NULL;
>> +}
>> +EXPORT_SYMBOL(dma_buf_vmap);
>> +
>> +/**
>> + * dma_buf_vunmap - Unmap a vmap obtained by dma_buf_vmap.
>> + * @dma_buf:   [in]    buffer to vmap
>> + */
>> +void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
>> +{
>> +       if (WARN_ON(!dmabuf))
>> +               return;
>> +
>> +       if (dmabuf->ops->vunmap)
>> +               dmabuf->ops->vunmap(dmabuf, vaddr);
>> +}
>> +EXPORT_SYMBOL(dma_buf_vunmap);
>> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
>> index 3efbfc2..b92b6de 100644
>> --- a/include/linux/dma-buf.h
>> +++ b/include/linux/dma-buf.h
>> @@ -92,6 +92,9 @@ struct dma_buf_ops {
>>        void (*kunmap_atomic)(struct dma_buf *, unsigned long, void *);
>>        void *(*kmap)(struct dma_buf *, unsigned long);
>>        void (*kunmap)(struct dma_buf *, unsigned long, void *);
>> +
>> +       void *(*vmap)(struct dma_buf *);
>> +       void (*vunmap)(struct dma_buf *, void *vaddr);
>>  };
>>
>>  /**
>> @@ -167,6 +170,9 @@ void *dma_buf_kmap_atomic(struct dma_buf *, unsigned long);
>>  void dma_buf_kunmap_atomic(struct dma_buf *, unsigned long, void *);
>>  void *dma_buf_kmap(struct dma_buf *, unsigned long);
>>  void dma_buf_kunmap(struct dma_buf *, unsigned long, void *);
>> +
>> +void *dma_buf_vmap(struct dma_buf *);
>> +void dma_buf_vunmap(struct dma_buf *, void *vaddr);
>>  #else
>>
>>  static inline struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
>> @@ -248,6 +254,14 @@ static inline void dma_buf_kunmap(struct dma_buf *dmabuf,
>>                                  unsigned long pnum, void *vaddr)
>>  {
>>  }
>> +
>> +static inline void *dma_buf_vmap(struct dma_buf *)
>> +{
>> +}
>> +
>> +static inline void dma_buf_vunmap(struct dma_buf *, void *vaddr);
>> +{
>> +}
>
> I think these two will cause compile issues for
> !CONFIG_DMA_SHARED_BUFFER case due to no parameter name for first arg.

Oops, will send a new one,

>
> Other than that, it looks ok.. although is vmap really less overhead?
> Using kmap can use existing lowmem address for lowmem pages.  Or is
> the issue that you somehow need access to the entire buffer in one
> shot?

Well the USB code has to linearly read chunks of the framebuffer and
RLE compress them,
having to cut things up into kmapped chunks and shove those out, now
if we are rendering
something that draws a lot, the CPU overheads start to mount right up.

Dave.
Marcin Ślusarz May 17, 2012, 5:32 p.m. UTC | #3
On Thu, May 17, 2012 at 06:32:19AM -0600, Rob Clark wrote:
> On Thu, May 17, 2012 at 4:31 AM, Dave Airlie <airlied@gmail.com> wrote:
> > From: Dave Airlie <airlied@redhat.com>
> >
> > The main requirement I have for this interface is for scanning out
> > using the USB gpu devices. Since these devices have to read the
> > framebuffer on updates and linearly compress it, using kmaps
> > is a major overhead for every update.
> >
> > v2: fix warn issues pointed out by Sylwester Nawrocki.
> >
> > Signed-off-by: Dave Airlie <airlied@redhat.com>
> > ---
> >  drivers/base/dma-buf.c  |   34 ++++++++++++++++++++++++++++++++++
> >  include/linux/dma-buf.h |   14 ++++++++++++++
> >  2 files changed, 48 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> > index 07cbbc6..750f92c 100644
> > --- a/drivers/base/dma-buf.c
> > +++ b/drivers/base/dma-buf.c
> > @@ -406,3 +406,37 @@ void dma_buf_kunmap(struct dma_buf *dmabuf, unsigned long page_num,
> >                dmabuf->ops->kunmap(dmabuf, page_num, vaddr);
> >  }
> >  EXPORT_SYMBOL_GPL(dma_buf_kunmap);
> > +
> > +/**
> > + * dma_buf_vmap - Create virtual mapping for the buffer object into kernel address space. The same restrictions as for vmap and friends apply.
> > + * @dma_buf:   [in]    buffer to vmap
> > + *
> > + * This call may fail due to lack of virtual mapping address space.
> > + * These calls are optional in drivers. The intended use for them
> > + * is for mapping objects linear in kernel space for high use objects.
> > + * Please attempt to use kmap/kunmap before thinking about these interfaces.
> > + */
> > +void *dma_buf_vmap(struct dma_buf *dmabuf)
> > +{
> > +       if (WARN_ON(!dmabuf))
> > +               return NULL;
> > +
> > +       if (dmabuf->ops->vmap)
> > +               return dmabuf->ops->vmap(dmabuf);
> > +       return NULL;
> > +}
> > +EXPORT_SYMBOL(dma_buf_vmap);
> > +
> > +/**
> > + * dma_buf_vunmap - Unmap a vmap obtained by dma_buf_vmap.
> > + * @dma_buf:   [in]    buffer to vmap
> > + */
> > +void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
> > +{
> > +       if (WARN_ON(!dmabuf))
> > +               return;
> > +
> > +       if (dmabuf->ops->vunmap)
> > +               dmabuf->ops->vunmap(dmabuf, vaddr);
> > +}
> > +EXPORT_SYMBOL(dma_buf_vunmap);
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index 3efbfc2..b92b6de 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -92,6 +92,9 @@ struct dma_buf_ops {
> >        void (*kunmap_atomic)(struct dma_buf *, unsigned long, void *);
> >        void *(*kmap)(struct dma_buf *, unsigned long);
> >        void (*kunmap)(struct dma_buf *, unsigned long, void *);
> > +
> > +       void *(*vmap)(struct dma_buf *);
> > +       void (*vunmap)(struct dma_buf *, void *vaddr);
> >  };
> >
> >  /**
> > @@ -167,6 +170,9 @@ void *dma_buf_kmap_atomic(struct dma_buf *, unsigned long);
> >  void dma_buf_kunmap_atomic(struct dma_buf *, unsigned long, void *);
> >  void *dma_buf_kmap(struct dma_buf *, unsigned long);
> >  void dma_buf_kunmap(struct dma_buf *, unsigned long, void *);
> > +
> > +void *dma_buf_vmap(struct dma_buf *);
> > +void dma_buf_vunmap(struct dma_buf *, void *vaddr);
> >  #else
> >
> >  static inline struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
> > @@ -248,6 +254,14 @@ static inline void dma_buf_kunmap(struct dma_buf *dmabuf,
> >                                  unsigned long pnum, void *vaddr)
> >  {
> >  }
> > +
> > +static inline void *dma_buf_vmap(struct dma_buf *)
> > +{
> > +}
> > +
> > +static inline void dma_buf_vunmap(struct dma_buf *, void *vaddr);
> > +{
> > +}
> 
> I think these two will cause compile issues for
> !CONFIG_DMA_SHARED_BUFFER case due to no parameter name for first arg.

And because of ";" between ) and { :)

Marcin
Sumit Semwal May 18, 2012, 4:07 a.m. UTC | #4
Hi Dave,

On 17 May 2012 16:01, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> The main requirement I have for this interface is for scanning out
> using the USB gpu devices. Since these devices have to read the
> framebuffer on updates and linearly compress it, using kmaps
> is a major overhead for every update.
>
> v2: fix warn issues pointed out by Sylwester Nawrocki.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/base/dma-buf.c  |   34 ++++++++++++++++++++++++++++++++++
>  include/linux/dma-buf.h |   14 ++++++++++++++
>  2 files changed, 48 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index 07cbbc6..750f92c 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -406,3 +406,37 @@ void dma_buf_kunmap(struct dma_buf *dmabuf, unsigned long page_num,
>                dmabuf->ops->kunmap(dmabuf, page_num, vaddr);
>  }
>  EXPORT_SYMBOL_GPL(dma_buf_kunmap);
> +
> +/**
> + * dma_buf_vmap - Create virtual mapping for the buffer object into kernel address space. The same restrictions as for vmap and friends apply.
> + * @dma_buf:   [in]    buffer to vmap
> + *
> + * This call may fail due to lack of virtual mapping address space.
> + * These calls are optional in drivers. The intended use for them
> + * is for mapping objects linear in kernel space for high use objects.
> + * Please attempt to use kmap/kunmap before thinking about these interfaces.
> + */
> +void *dma_buf_vmap(struct dma_buf *dmabuf)
> +{
> +       if (WARN_ON(!dmabuf))
> +               return NULL;
> +
> +       if (dmabuf->ops->vmap)
> +               return dmabuf->ops->vmap(dmabuf);
> +       return NULL;
> +}
> +EXPORT_SYMBOL(dma_buf_vmap);
I am afraid we don't yet have a clear consensus on the usage of
EXPORT_SYMBOL - till it is done, I would prefer that we use
EXPORT_SYMBOL_GPL for consistency. Once we reach agreement, we can
change them all in one go if required.
> +
<snip>
diff mbox

Patch

diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 07cbbc6..750f92c 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -406,3 +406,37 @@  void dma_buf_kunmap(struct dma_buf *dmabuf, unsigned long page_num,
 		dmabuf->ops->kunmap(dmabuf, page_num, vaddr);
 }
 EXPORT_SYMBOL_GPL(dma_buf_kunmap);
+
+/**
+ * dma_buf_vmap - Create virtual mapping for the buffer object into kernel address space. The same restrictions as for vmap and friends apply.
+ * @dma_buf:	[in]	buffer to vmap
+ *
+ * This call may fail due to lack of virtual mapping address space.
+ * These calls are optional in drivers. The intended use for them
+ * is for mapping objects linear in kernel space for high use objects.
+ * Please attempt to use kmap/kunmap before thinking about these interfaces.
+ */
+void *dma_buf_vmap(struct dma_buf *dmabuf)
+{
+	if (WARN_ON(!dmabuf))
+		return NULL;
+
+	if (dmabuf->ops->vmap)
+		return dmabuf->ops->vmap(dmabuf);
+	return NULL;
+}
+EXPORT_SYMBOL(dma_buf_vmap);
+
+/**
+ * dma_buf_vunmap - Unmap a vmap obtained by dma_buf_vmap.
+ * @dma_buf:	[in]	buffer to vmap
+ */
+void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
+{
+	if (WARN_ON(!dmabuf))
+		return;
+
+	if (dmabuf->ops->vunmap)
+		dmabuf->ops->vunmap(dmabuf, vaddr);
+}
+EXPORT_SYMBOL(dma_buf_vunmap);
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 3efbfc2..b92b6de 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -92,6 +92,9 @@  struct dma_buf_ops {
 	void (*kunmap_atomic)(struct dma_buf *, unsigned long, void *);
 	void *(*kmap)(struct dma_buf *, unsigned long);
 	void (*kunmap)(struct dma_buf *, unsigned long, void *);
+
+	void *(*vmap)(struct dma_buf *);
+	void (*vunmap)(struct dma_buf *, void *vaddr);
 };
 
 /**
@@ -167,6 +170,9 @@  void *dma_buf_kmap_atomic(struct dma_buf *, unsigned long);
 void dma_buf_kunmap_atomic(struct dma_buf *, unsigned long, void *);
 void *dma_buf_kmap(struct dma_buf *, unsigned long);
 void dma_buf_kunmap(struct dma_buf *, unsigned long, void *);
+
+void *dma_buf_vmap(struct dma_buf *);
+void dma_buf_vunmap(struct dma_buf *, void *vaddr);
 #else
 
 static inline struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
@@ -248,6 +254,14 @@  static inline void dma_buf_kunmap(struct dma_buf *dmabuf,
 				  unsigned long pnum, void *vaddr)
 {
 }
+
+static inline void *dma_buf_vmap(struct dma_buf *)
+{
+}
+
+static inline void dma_buf_vunmap(struct dma_buf *, void *vaddr);
+{
+}
 #endif /* CONFIG_DMA_SHARED_BUFFER */
 
 #endif /* __DMA_BUF_H__ */