All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Semwal, Sumit" <sumit.semwal@ti.com>,
	linux@arm.linux.org.uk, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org,
	linux-mm@kvack.org, linux-media@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [Linaro-mm-sig] [RFC v2 1/2] dma-buf: Introduce dma buffer sharing mechanism
Date: Thu, 8 Dec 2011 22:44:08 +0100	[thread overview]
Message-ID: <CAKMK7uFQiiUbkU-7c3Os0d0FJNyLbqS2HLPRLy3LGnOoCXV5Pw@mail.gmail.com> (raw)
In-Reply-To: <201112071340.35267.arnd@arndb.de>

On Wed, Dec 7, 2011 at 14:40, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 07 December 2011, Semwal, Sumit wrote:
>> Thanks for the excellent discussion - it indeed is very good learning
>> for the relatively-inexperienced me :)
>>
>> So, for the purpose of dma-buf framework, could I summarize the
>> following and rework accordingly?:
>> 1. remove mmap() dma_buf_op [and mmap fop], and introduce cpu_start(),
>> cpu_finish() ops to bracket cpu accesses to the buffer. Also add
>> DMABUF_CPU_START / DMABUF_CPU_FINI IOCTLs?
>
> I think we'd be better off for now without the extra ioctls and
> just document that a shared buffer must not be exported to user
> space using mmap at all, to avoid those problems. Serialization
> between GPU and CPU is on a higher level than the dma_buf framework
> IMHO.

Agreed.

>> 2. remove sg_sync* ops for now (and we'll see if we need to add them
>> later if needed)
>
> Just removing the sg_sync_* operations is not enough. We have to make
> the decision whether we want to allow
> a) only coherent mappings of the buffer into kernel memory (requiring
> an extension to the dma_map_ops on ARM to not flush caches at map/unmap
> time)
> b) not allowing any in-kernel mappings (same requirement on ARM, also
> limits the usefulness of the dma_buf if we cannot access it from the
> kernel or from user space)
> c) only allowing streaming mappings, even if those are non-coherent
> (requiring strict serialization between CPU (in-kernel) and dma users of
> the buffer)

I think only allowing streaming access makes the most sense:
- I don't see much (if any need) for the kernel to access a dma_buf -
in all current usecases it just contains pixel data and no hw-specific
things (like sg tables, command buffers, ..). At most I see the need
for the kernel to access the buffer for dma bounce buffers, but that
is internal to the dma subsystem (and hence does not need to be
exposed).
- Userspace can still access the contents through the exporting
subsystem (e.g. use some gem mmap support). For efficiency reason gpu
drivers are already messing around with cache coherency in a platform
specific way (and hence violated the dma api a bit), so we could stuff
the mmap coherency in there, too. When we later on extend dma_buf
support so that other drivers than the gpu can export dma_bufs, we can
then extend the official dma api with already a few drivers with
use-patterns around.

But I still think that the kernel must not be required to enforce
correct access ordering for the reasons outlined in my other mail.
-Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Semwal, Sumit" <sumit.semwal@ti.com>,
	linux@arm.linux.org.uk, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org,
	linux-mm@kvack.org, linux-media@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [Linaro-mm-sig] [RFC v2 1/2] dma-buf: Introduce dma buffer sharing mechanism
Date: Thu, 8 Dec 2011 22:44:08 +0100	[thread overview]
Message-ID: <CAKMK7uFQiiUbkU-7c3Os0d0FJNyLbqS2HLPRLy3LGnOoCXV5Pw@mail.gmail.com> (raw)
In-Reply-To: <201112071340.35267.arnd@arndb.de>

On Wed, Dec 7, 2011 at 14:40, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 07 December 2011, Semwal, Sumit wrote:
>> Thanks for the excellent discussion - it indeed is very good learning
>> for the relatively-inexperienced me :)
>>
>> So, for the purpose of dma-buf framework, could I summarize the
>> following and rework accordingly?:
>> 1. remove mmap() dma_buf_op [and mmap fop], and introduce cpu_start(),
>> cpu_finish() ops to bracket cpu accesses to the buffer. Also add
>> DMABUF_CPU_START / DMABUF_CPU_FINI IOCTLs?
>
> I think we'd be better off for now without the extra ioctls and
> just document that a shared buffer must not be exported to user
> space using mmap at all, to avoid those problems. Serialization
> between GPU and CPU is on a higher level than the dma_buf framework
> IMHO.

Agreed.

>> 2. remove sg_sync* ops for now (and we'll see if we need to add them
>> later if needed)
>
> Just removing the sg_sync_* operations is not enough. We have to make
> the decision whether we want to allow
> a) only coherent mappings of the buffer into kernel memory (requiring
> an extension to the dma_map_ops on ARM to not flush caches at map/unmap
> time)
> b) not allowing any in-kernel mappings (same requirement on ARM, also
> limits the usefulness of the dma_buf if we cannot access it from the
> kernel or from user space)
> c) only allowing streaming mappings, even if those are non-coherent
> (requiring strict serialization between CPU (in-kernel) and dma users of
> the buffer)

I think only allowing streaming access makes the most sense:
- I don't see much (if any need) for the kernel to access a dma_buf -
in all current usecases it just contains pixel data and no hw-specific
things (like sg tables, command buffers, ..). At most I see the need
for the kernel to access the buffer for dma bounce buffers, but that
is internal to the dma subsystem (and hence does not need to be
exposed).
- Userspace can still access the contents through the exporting
subsystem (e.g. use some gem mmap support). For efficiency reason gpu
drivers are already messing around with cache coherency in a platform
specific way (and hence violated the dma api a bit), so we could stuff
the mmap coherency in there, too. When we later on extend dma_buf
support so that other drivers than the gpu can export dma_bufs, we can
then extend the official dma api with already a few drivers with
use-patterns around.

But I still think that the kernel must not be required to enforce
correct access ordering for the reasons outlined in my other mail.
-Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: daniel@ffwll.ch (Daniel Vetter)
To: linux-arm-kernel@lists.infradead.org
Subject: [Linaro-mm-sig] [RFC v2 1/2] dma-buf: Introduce dma buffer sharing mechanism
Date: Thu, 8 Dec 2011 22:44:08 +0100	[thread overview]
Message-ID: <CAKMK7uFQiiUbkU-7c3Os0d0FJNyLbqS2HLPRLy3LGnOoCXV5Pw@mail.gmail.com> (raw)
In-Reply-To: <201112071340.35267.arnd@arndb.de>

On Wed, Dec 7, 2011 at 14:40, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 07 December 2011, Semwal, Sumit wrote:
>> Thanks for the excellent discussion - it indeed is very good learning
>> for the relatively-inexperienced me :)
>>
>> So, for the purpose of dma-buf framework, could I summarize the
>> following and rework accordingly?:
>> 1. remove mmap() dma_buf_op [and mmap fop], and introduce cpu_start(),
>> cpu_finish() ops to bracket cpu accesses to the buffer. Also add
>> DMABUF_CPU_START / DMABUF_CPU_FINI IOCTLs?
>
> I think we'd be better off for now without the extra ioctls and
> just document that a shared buffer must not be exported to user
> space using mmap at all, to avoid those problems. Serialization
> between GPU and CPU is on a higher level than the dma_buf framework
> IMHO.

Agreed.

>> 2. remove sg_sync* ops for now (and we'll see if we need to add them
>> later if needed)
>
> Just removing the sg_sync_* operations is not enough. We have to make
> the decision whether we want to allow
> a) only coherent mappings of the buffer into kernel memory (requiring
> an extension to the dma_map_ops on ARM to not flush caches at map/unmap
> time)
> b) not allowing any in-kernel mappings (same requirement on ARM, also
> limits the usefulness of the dma_buf if we cannot access it from the
> kernel or from user space)
> c) only allowing streaming mappings, even if those are non-coherent
> (requiring strict serialization between CPU (in-kernel) and dma users of
> the buffer)

I think only allowing streaming access makes the most sense:
- I don't see much (if any need) for the kernel to access a dma_buf -
in all current usecases it just contains pixel data and no hw-specific
things (like sg tables, command buffers, ..). At most I see the need
for the kernel to access the buffer for dma bounce buffers, but that
is internal to the dma subsystem (and hence does not need to be
exposed).
- Userspace can still access the contents through the exporting
subsystem (e.g. use some gem mmap support). For efficiency reason gpu
drivers are already messing around with cache coherency in a platform
specific way (and hence violated the dma api a bit), so we could stuff
the mmap coherency in there, too. When we later on extend dma_buf
support so that other drivers than the gpu can export dma_bufs, we can
then extend the official dma api with already a few drivers with
use-patterns around.

But I still think that the kernel must not be required to enforce
correct access ordering for the reasons outlined in my other mail.
-Daniel
-- 
Daniel Vetter
daniel.vetter at ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2011-12-08 21:44 UTC|newest]

Thread overview: 198+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-02  8:57 [RFC v2 0/2] Introduce DMA buffer sharing mechanism Sumit Semwal
2011-12-02  8:57 ` Sumit Semwal
2011-12-02  8:57 ` Sumit Semwal
2011-12-02  8:57 ` [RFC v2 1/2] dma-buf: Introduce dma " Sumit Semwal
2011-12-02  8:57   ` Sumit Semwal
2011-12-02 17:11   ` Konrad Rzeszutek Wilk
2011-12-02 17:11     ` Konrad Rzeszutek Wilk
2011-12-02 17:11     ` Konrad Rzeszutek Wilk
2011-12-05  9:48     ` Semwal, Sumit
2011-12-05  9:48       ` Semwal, Sumit
2011-12-05  9:48       ` Semwal, Sumit
2011-12-05 17:18   ` Arnd Bergmann
2011-12-05 17:18     ` Arnd Bergmann
2011-12-05 17:18     ` Arnd Bergmann
2011-12-05 18:55     ` Daniel Vetter
2011-12-05 18:55       ` Daniel Vetter
2011-12-05 18:55       ` Daniel Vetter
2011-12-05 19:29       ` Arnd Bergmann
2011-12-05 19:29         ` Arnd Bergmann
2011-12-05 19:29         ` Arnd Bergmann
2011-12-05 20:58         ` Daniel Vetter
2011-12-05 20:58           ` Daniel Vetter
2011-12-05 20:58           ` Daniel Vetter
2011-12-05 22:04           ` Arnd Bergmann
2011-12-05 22:04             ` Arnd Bergmann
2011-12-05 22:04             ` Arnd Bergmann
2011-12-05 22:33             ` Daniel Vetter
2011-12-05 22:33               ` Daniel Vetter
2011-12-05 22:33               ` Daniel Vetter
2011-12-05 20:46     ` Rob Clark
2011-12-05 20:46       ` Rob Clark
2011-12-05 20:46       ` Rob Clark
2011-12-05 21:23       ` Daniel Vetter
2011-12-05 21:23         ` Daniel Vetter
2011-12-05 21:23         ` Daniel Vetter
2011-12-05 22:11         ` Rob Clark
2011-12-05 22:11           ` Rob Clark
2011-12-05 22:11           ` Rob Clark
2011-12-05 22:33           ` Daniel Vetter
2011-12-05 22:33             ` Daniel Vetter
2011-12-05 22:33             ` Daniel Vetter
2011-12-06 13:16           ` Arnd Bergmann
2011-12-06 13:16             ` Arnd Bergmann
2011-12-06 13:16             ` Arnd Bergmann
2011-12-06 15:28             ` Daniel Vetter
2011-12-06 15:28               ` Daniel Vetter
2011-12-06 15:28               ` Daniel Vetter
2011-12-07 13:27           ` Semwal, Sumit
2011-12-07 13:27             ` Semwal, Sumit
2011-12-07 13:27             ` Semwal, Sumit
2011-12-07 13:40             ` Arnd Bergmann
2011-12-07 13:40               ` Arnd Bergmann
2011-12-07 13:40               ` Arnd Bergmann
2011-12-08 21:44               ` Daniel Vetter [this message]
2011-12-08 21:44                 ` [Linaro-mm-sig] " Daniel Vetter
2011-12-08 21:44                 ` Daniel Vetter
2011-12-09 14:13                 ` Arnd Bergmann
2011-12-09 14:13                   ` Arnd Bergmann
2011-12-09 14:13                   ` Arnd Bergmann
2011-12-09 14:24                   ` Alan Cox
2011-12-09 14:24                     ` Alan Cox
2011-12-09 14:24                     ` Alan Cox
2011-12-10  4:01                     ` Daniel Vetter
2011-12-10  4:01                       ` Daniel Vetter
2011-12-10  4:01                       ` Daniel Vetter
2011-12-12 16:48                       ` Arnd Bergmann
2011-12-12 16:48                         ` Arnd Bergmann
2011-12-12 16:48                         ` Arnd Bergmann
2011-12-19  6:16                         ` Semwal, Sumit
2011-12-19  6:16                           ` Semwal, Sumit
2011-12-19  6:16                           ` Semwal, Sumit
2011-12-20 15:41                           ` Arnd Bergmann
2011-12-20 15:41                             ` Arnd Bergmann
2011-12-20 15:41                             ` Arnd Bergmann
2011-12-20 16:41                             ` Rob Clark
2011-12-20 16:41                               ` Rob Clark
2011-12-20 16:41                               ` Rob Clark
2011-12-20 17:14                               ` Daniel Vetter
2011-12-20 17:14                                 ` Daniel Vetter
2011-12-20 17:14                                 ` Daniel Vetter
2011-12-20 17:14                                 ` Daniel Vetter
2011-12-21 17:27                                 ` Arnd Bergmann
2011-12-21 17:27                                   ` Arnd Bergmann
2011-12-21 17:27                                   ` Arnd Bergmann
2011-12-21 19:04                                   ` Daniel Vetter
2011-12-21 19:04                                     ` Daniel Vetter
2011-12-21 19:04                                     ` Daniel Vetter
2011-12-23 10:00                                   ` Semwal, Sumit
2011-12-23 10:00                                     ` Semwal, Sumit
2011-12-23 10:00                                     ` Semwal, Sumit
2011-12-23 17:10                                     ` Rob Clark
2011-12-23 17:10                                       ` Rob Clark
2011-12-23 17:10                                       ` Rob Clark
2011-12-20  9:03                   ` Sakari Ailus
2011-12-20  9:03                     ` Sakari Ailus
2011-12-20  9:03                     ` Sakari Ailus
2011-12-20 15:36                     ` Arnd Bergmann
2011-12-20 15:36                       ` Arnd Bergmann
2011-12-20 15:36                       ` Arnd Bergmann
2012-01-01 20:53                       ` Sakari Ailus
2012-01-01 20:53                         ` Sakari Ailus
2012-01-01 20:53                         ` Sakari Ailus
2012-01-01 23:12                         ` Rob Clark
2012-01-01 23:12                           ` Rob Clark
2012-01-01 23:12                           ` Rob Clark
2011-12-13 13:33                 ` Hans Verkuil
2011-12-13 13:33                   ` Hans Verkuil
2011-12-13 13:33                   ` Hans Verkuil
2011-12-05 22:09       ` Arnd Bergmann
2011-12-05 22:09         ` Arnd Bergmann
2011-12-05 22:09         ` Arnd Bergmann
2011-12-05 22:09         ` Arnd Bergmann
2011-12-05 22:15         ` Rob Clark
2011-12-05 22:15           ` Rob Clark
2011-12-05 22:15           ` Rob Clark
2011-12-05 22:35         ` Rob Clark
2011-12-05 22:35           ` Rob Clark
2011-12-05 22:35           ` Rob Clark
2011-12-07  6:35     ` Semwal, Sumit
2011-12-07  6:35       ` Semwal, Sumit
2011-12-07  6:35       ` Semwal, Sumit
2011-12-07 10:11       ` Arnd Bergmann
2011-12-07 10:11         ` Arnd Bergmann
2011-12-07 10:11         ` Arnd Bergmann
2011-12-07 11:02         ` Semwal, Sumit
2011-12-07 11:02           ` Semwal, Sumit
2011-12-07 11:02           ` Semwal, Sumit
2011-12-07 11:34           ` Arnd Bergmann
2011-12-07 11:34             ` Arnd Bergmann
2011-12-07 11:34             ` Arnd Bergmann
2011-12-09 22:50     ` [Linaro-mm-sig] " Robert Morell
2011-12-09 22:50       ` Robert Morell
2011-12-09 22:50       ` Robert Morell
2011-12-09 22:50       ` Robert Morell
2011-12-10 11:13       ` Mauro Carvalho Chehab
2011-12-10 11:13         ` Mauro Carvalho Chehab
2011-12-10 11:13         ` Mauro Carvalho Chehab
2011-12-10 11:13         ` Mauro Carvalho Chehab
2011-12-12 22:44         ` Robert Morell
2011-12-12 22:44           ` Robert Morell
2011-12-12 22:44           ` Robert Morell
2011-12-12 22:44           ` Robert Morell
2011-12-13 15:10           ` Arnd Bergmann
2011-12-13 15:10             ` Arnd Bergmann
2011-12-13 15:10             ` Arnd Bergmann
2011-12-13 15:10             ` Arnd Bergmann
2011-12-20  2:05             ` Robert Morell
2011-12-20  2:05               ` Robert Morell
2011-12-20  2:05               ` Robert Morell
2011-12-20  2:05               ` Robert Morell
2011-12-20 14:29               ` Anca Emanuel
2011-12-20 14:29                 ` Anca Emanuel
2011-12-20 14:29                 ` Anca Emanuel
2011-12-20 14:29                 ` Anca Emanuel
2012-01-09  6:20   ` InKi Dae
2012-01-09  6:20     ` InKi Dae
2012-01-09  6:20     ` InKi Dae
2012-01-09  8:10     ` Daniel Vetter
2012-01-09  8:10       ` Daniel Vetter
2012-01-09  8:10       ` Daniel Vetter
2012-01-09  8:11       ` [Linaro-mm-sig] " Dave Airlie
2012-01-09  8:11         ` Dave Airlie
2012-01-09  8:11         ` Dave Airlie
2012-01-09 10:10       ` InKi Dae
2012-01-09 10:10         ` InKi Dae
2012-01-09 10:10         ` InKi Dae
2012-01-09 10:27         ` Daniel Vetter
2012-01-09 10:27           ` Daniel Vetter
2012-01-09 10:27           ` Daniel Vetter
2012-01-09 12:06           ` InKi Dae
2012-01-09 12:06             ` InKi Dae
2012-01-09 12:06             ` InKi Dae
2012-01-09 16:02             ` Daniel Vetter
2012-01-09 16:02               ` Daniel Vetter
2012-01-09 16:02               ` Daniel Vetter
2012-01-09 15:17         ` Rob Clark
2012-01-09 15:17           ` Rob Clark
2012-01-09 15:17           ` Rob Clark
2012-01-10  1:34           ` InKi Dae
2012-01-10  1:34             ` InKi Dae
2012-01-10  1:34             ` InKi Dae
2012-01-10  2:14             ` Rob Clark
2012-01-10  2:14               ` Rob Clark
2012-01-10  2:14               ` Rob Clark
2012-01-10  6:09               ` Semwal, Sumit
2012-01-10  6:09                 ` Semwal, Sumit
2012-01-10  6:09                 ` Semwal, Sumit
2012-01-10  7:28                 ` InKi Dae
2012-01-10  7:28                   ` InKi Dae
2012-01-10  7:28                   ` InKi Dae
2012-01-10  9:19                   ` InKi Dae
2012-01-10  9:19                     ` InKi Dae
2012-01-10  9:19                     ` InKi Dae
2012-01-11  1:08               ` InKi Dae
2012-01-11  1:08                 ` InKi Dae
2012-01-11  1:08                 ` InKi Dae
2011-12-02  8:57 ` [RFC v2 2/2] dma-buf: Documentation for buffer sharing framework Sumit Semwal
2011-12-02  8:57   ` Sumit Semwal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKMK7uFQiiUbkU-7c3Os0d0FJNyLbqS2HLPRLy3LGnOoCXV5Pw@mail.gmail.com \
    --to=daniel@ffwll.ch \
    --cc=arnd@arndb.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux@arm.linux.org.uk \
    --cc=sumit.semwal@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.