Re: [Linaro-mm-sig] [PATCHv6 00/13] Integration of videobuf2 with dmabuf
It probably is a fixed offset, but all the information describing it is in userspace... In my experience, it's always almost one of the pre-defined formats, but then there's some extra padding, weird alignment etc. I'm trying to convert code that used userptr, but where the planes were offsets into the same buffer, to use dma_buf. Looks like I need to dig a bit deeper. Thanks, Rebecca On Mon, Jun 4, 2012 at 2:58 PM, Hans Verkuil wrote: > Hi Rebecca, > > On Mon June 4 2012 21:34:23 Rebecca Schultz Zavin wrote: >> I have a system where the data is planar, but the kernel drivers >> expect to get one allocation with offsets for the planes. I can't >> figure out how to do that with the current dma_buf implementation. I >> thought I could pass the same dma_buf several times and use the >> data_offset field of the v4l2_plane struct but it looks like that's >> only for output. Am I missing something? Is this supported? > > v4l2_plane is typically used if the planes are allocated separately. > If you allocate it in one go, aren't the planes then at well-defined > offsets from the start? If so, then it is either one of the already > pre-defined planar formats found here: > > http://hverkuil.home.xs4all.nl/spec/media.html#yuv-formats > > or you define a pixelformat specific to your own hardware that identifies > that particular format. > > If it is one allocation, but there is no clear calculation based on width > and height that gives you the start of each plane, then we do not support > that at the moment. I believe I had a discussion about something similar > with people from Qualcomm, but that never came to anything. > > That would be something to discuss on the linux-media mailinglist. > > Regards, > > Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [PATCHv6 00/13] Integration of videobuf2 with dmabuf
Also the data_offset field (and bytes_used field) only get copied
from the v4l2_buffer into the vb2_buffer struct if the buffer is an
output buffer.
Rebecca
On Mon, Jun 4, 2012 at 1:46 PM, Rebecca Schultz Zavin
wrote:
> I'm trying to do it in my drivier, but I'm not sure how to make it
> safe since there is no way to tell the kernel the total size of the
> buffer. From what I can tell, I can't sanity check that the offset
> and lengths are within the buffer without adding a field.
>
> Rebecca
>
> On Mon, Jun 4, 2012 at 1:28 PM, Rob Clark wrote:
>> this is at least how we do it w/ drm/kms.. I would expect that if you
>> could do that w/ output for v4l you also could for input, but perhaps
>> the individual driver needs to do something to support mplane? I
>> guess the v4l folks would know better
>>
>> BR,
>> -R
>>
>> On Tue, Jun 5, 2012 at 3:34 AM, Rebecca Schultz Zavin
>> wrote:
>>> I have a system where the data is planar, but the kernel drivers
>>> expect to get one allocation with offsets for the planes. I can't
>>> figure out how to do that with the current dma_buf implementation. I
>>> thought I could pass the same dma_buf several times and use the
>>> data_offset field of the v4l2_plane struct but it looks like that's
>>> only for output. Am I missing something? Is this supported?
>>>
>>> Thanks,
>>> Rebecca
>>>
>>> On Wed, May 30, 2012 at 8:26 AM, Semwal, Sumit wrote:
>>>> On Tue, May 29, 2012 at 6:25 AM, Laurent Pinchart
>>>> wrote:
>>>>> Hi Tomasz,
>>>> Hi Tomasz, Laurent, Mauro,
>>>>>
>>>>> On Wednesday 23 May 2012 14:10:14 Tomasz Stanislawski wrote:
>>>>>> Hello everyone,
>>>>>> This patchset adds support for DMABUF [2] importing to V4L2 stack.
>>>>>> The support for DMABUF exporting was moved to separate patchset
>>>>>> due to dependency on patches for DMA mapping redesign by
>>>>>> Marek Szyprowski [4].
>>>>>
>>>>> Except for the small issue with patches 01/13 and 02/13, the set is ready
>>>>> for
>>>>> upstream as far as I'm concerned.
>>>> +1; Mauro: how do you think about this series? Getting it landed into
>>>> 3.5 would make life lot easier :)
>>>>>
>>>>>> v6:
>>>>>> - fixed missing entry in v4l2_memory_names
>>>>>> - fixed a bug occuring after get_user_pages failure
>>>>>
>>>>> I've missed that one, what was it ?
>>>>>
>>>>>> - fixed a bug caused by using invalid vma for get_user_pages
>>>>>> - prepare/finish no longer call dma_sync for dmabuf buffers
>>>>>>
>>>>>> v5:
>>>>>> - removed change of importer/exporter behaviour
>>>>>> - fixes vb2_dc_pages_to_sgt basing on Laurent's hints
>>>>>> - changed pin/unpin words to lock/unlock in Doc
>>>>>>
>>>>>> v4:
>>>>>> - rebased on mainline 3.4-rc2
>>>>>> - included missing importing support for s5p-fimc and s5p-tv
>>>>>> - added patch for changing map/unmap for importers
>>>>>> - fixes to Documentation part
>>>>>> - coding style fixes
>>>>>> - pairing {map/unmap}_dmabuf in vb2-core
>>>>>> - fixing variable types and semantic of arguments in
>>>>>> videobufb2-dma-contig.c
>>>>>>
>>>>>> v3:
>>>>>> - rebased on mainline 3.4-rc1
>>>>>> - split 'code refactor' patch to multiple smaller patches
>>>>>> - squashed fixes to Sumit's patches
>>>>>> - patchset is no longer dependant on 'DMA mapping redesign'
>>>>>> - separated path for handling IO and non-IO mappings
>>>>>> - add documentation for DMABUF importing to V4L
>>>>>> - removed all DMABUF exporter related code
>>>>>> - removed usage of dma_get_pages extension
>>>>>>
>>>>>> v2:
>>>>>> - extended VIDIOC_EXPBUF argument from integer memoffset to struct
>>>>>> v4l2_exportbuffer
>>>>>> - added patch that breaks DMABUF spec on (un)map_atachment callcacks but
>>>>>> allows to work with existing implementation of DMABUF prime in DRM
>>>>>> - all dma-contig code refactoring p
Re: [Linaro-mm-sig] [PATCHv6 00/13] Integration of videobuf2 with dmabuf
I'm trying to do it in my drivier, but I'm not sure how to make it
safe since there is no way to tell the kernel the total size of the
buffer. From what I can tell, I can't sanity check that the offset
and lengths are within the buffer without adding a field.
Rebecca
On Mon, Jun 4, 2012 at 1:28 PM, Rob Clark wrote:
> this is at least how we do it w/ drm/kms.. I would expect that if you
> could do that w/ output for v4l you also could for input, but perhaps
> the individual driver needs to do something to support mplane? I
> guess the v4l folks would know better
>
> BR,
> -R
>
> On Tue, Jun 5, 2012 at 3:34 AM, Rebecca Schultz Zavin
> wrote:
>> I have a system where the data is planar, but the kernel drivers
>> expect to get one allocation with offsets for the planes. I can't
>> figure out how to do that with the current dma_buf implementation. I
>> thought I could pass the same dma_buf several times and use the
>> data_offset field of the v4l2_plane struct but it looks like that's
>> only for output. Am I missing something? Is this supported?
>>
>> Thanks,
>> Rebecca
>>
>> On Wed, May 30, 2012 at 8:26 AM, Semwal, Sumit wrote:
>>> On Tue, May 29, 2012 at 6:25 AM, Laurent Pinchart
>>> wrote:
>>>> Hi Tomasz,
>>> Hi Tomasz, Laurent, Mauro,
>>>>
>>>> On Wednesday 23 May 2012 14:10:14 Tomasz Stanislawski wrote:
>>>>> Hello everyone,
>>>>> This patchset adds support for DMABUF [2] importing to V4L2 stack.
>>>>> The support for DMABUF exporting was moved to separate patchset
>>>>> due to dependency on patches for DMA mapping redesign by
>>>>> Marek Szyprowski [4].
>>>>
>>>> Except for the small issue with patches 01/13 and 02/13, the set is ready
>>>> for
>>>> upstream as far as I'm concerned.
>>> +1; Mauro: how do you think about this series? Getting it landed into
>>> 3.5 would make life lot easier :)
>>>>
>>>>> v6:
>>>>> - fixed missing entry in v4l2_memory_names
>>>>> - fixed a bug occuring after get_user_pages failure
>>>>
>>>> I've missed that one, what was it ?
>>>>
>>>>> - fixed a bug caused by using invalid vma for get_user_pages
>>>>> - prepare/finish no longer call dma_sync for dmabuf buffers
>>>>>
>>>>> v5:
>>>>> - removed change of importer/exporter behaviour
>>>>> - fixes vb2_dc_pages_to_sgt basing on Laurent's hints
>>>>> - changed pin/unpin words to lock/unlock in Doc
>>>>>
>>>>> v4:
>>>>> - rebased on mainline 3.4-rc2
>>>>> - included missing importing support for s5p-fimc and s5p-tv
>>>>> - added patch for changing map/unmap for importers
>>>>> - fixes to Documentation part
>>>>> - coding style fixes
>>>>> - pairing {map/unmap}_dmabuf in vb2-core
>>>>> - fixing variable types and semantic of arguments in
>>>>> videobufb2-dma-contig.c
>>>>>
>>>>> v3:
>>>>> - rebased on mainline 3.4-rc1
>>>>> - split 'code refactor' patch to multiple smaller patches
>>>>> - squashed fixes to Sumit's patches
>>>>> - patchset is no longer dependant on 'DMA mapping redesign'
>>>>> - separated path for handling IO and non-IO mappings
>>>>> - add documentation for DMABUF importing to V4L
>>>>> - removed all DMABUF exporter related code
>>>>> - removed usage of dma_get_pages extension
>>>>>
>>>>> v2:
>>>>> - extended VIDIOC_EXPBUF argument from integer memoffset to struct
>>>>> v4l2_exportbuffer
>>>>> - added patch that breaks DMABUF spec on (un)map_atachment callcacks but
>>>>> allows to work with existing implementation of DMABUF prime in DRM
>>>>> - all dma-contig code refactoring patches were squashed
>>>>> - bugfixes
>>>>>
>>>>> v1: List of changes since [1].
>>>>> - support for DMA api extension dma_get_pages, the function is used to
>>>>> retrieve pages used to create DMA mapping.
>>>>> - small fixes/code cleanup to videobuf2
>>>>> - added prepare and finish callbacks to vb2 allocators, it is used keep
>>>>> consistency between dma-cpu acess to the memory (by Marek Szyprowski)
>>>>> - support for exporting
Re: [Linaro-mm-sig] [PATCHv6 00/13] Integration of videobuf2 with dmabuf
I have a system where the data is planar, but the kernel drivers
expect to get one allocation with offsets for the planes. I can't
figure out how to do that with the current dma_buf implementation. I
thought I could pass the same dma_buf several times and use the
data_offset field of the v4l2_plane struct but it looks like that's
only for output. Am I missing something? Is this supported?
Thanks,
Rebecca
On Wed, May 30, 2012 at 8:26 AM, Semwal, Sumit wrote:
> On Tue, May 29, 2012 at 6:25 AM, Laurent Pinchart
> wrote:
>> Hi Tomasz,
> Hi Tomasz, Laurent, Mauro,
>>
>> On Wednesday 23 May 2012 14:10:14 Tomasz Stanislawski wrote:
>>> Hello everyone,
>>> This patchset adds support for DMABUF [2] importing to V4L2 stack.
>>> The support for DMABUF exporting was moved to separate patchset
>>> due to dependency on patches for DMA mapping redesign by
>>> Marek Szyprowski [4].
>>
>> Except for the small issue with patches 01/13 and 02/13, the set is ready for
>> upstream as far as I'm concerned.
> +1; Mauro: how do you think about this series? Getting it landed into
> 3.5 would make life lot easier :)
>>
>>> v6:
>>> - fixed missing entry in v4l2_memory_names
>>> - fixed a bug occuring after get_user_pages failure
>>
>> I've missed that one, what was it ?
>>
>>> - fixed a bug caused by using invalid vma for get_user_pages
>>> - prepare/finish no longer call dma_sync for dmabuf buffers
>>>
>>> v5:
>>> - removed change of importer/exporter behaviour
>>> - fixes vb2_dc_pages_to_sgt basing on Laurent's hints
>>> - changed pin/unpin words to lock/unlock in Doc
>>>
>>> v4:
>>> - rebased on mainline 3.4-rc2
>>> - included missing importing support for s5p-fimc and s5p-tv
>>> - added patch for changing map/unmap for importers
>>> - fixes to Documentation part
>>> - coding style fixes
>>> - pairing {map/unmap}_dmabuf in vb2-core
>>> - fixing variable types and semantic of arguments in videobufb2-dma-contig.c
>>>
>>> v3:
>>> - rebased on mainline 3.4-rc1
>>> - split 'code refactor' patch to multiple smaller patches
>>> - squashed fixes to Sumit's patches
>>> - patchset is no longer dependant on 'DMA mapping redesign'
>>> - separated path for handling IO and non-IO mappings
>>> - add documentation for DMABUF importing to V4L
>>> - removed all DMABUF exporter related code
>>> - removed usage of dma_get_pages extension
>>>
>>> v2:
>>> - extended VIDIOC_EXPBUF argument from integer memoffset to struct
>>> v4l2_exportbuffer
>>> - added patch that breaks DMABUF spec on (un)map_atachment callcacks but
>>> allows to work with existing implementation of DMABUF prime in DRM
>>> - all dma-contig code refactoring patches were squashed
>>> - bugfixes
>>>
>>> v1: List of changes since [1].
>>> - support for DMA api extension dma_get_pages, the function is used to
>>> retrieve pages used to create DMA mapping.
>>> - small fixes/code cleanup to videobuf2
>>> - added prepare and finish callbacks to vb2 allocators, it is used keep
>>> consistency between dma-cpu acess to the memory (by Marek Szyprowski)
>>> - support for exporting of DMABUF buffer in V4L2 and Videobuf2, originated
>>> from [3].
>>> - support for dma-buf exporting in vb2-dma-contig allocator
>>> - support for DMABUF for s5p-tv and s5p-fimc (capture interface) drivers,
>>> originated from [3]
>>> - changed handling for userptr buffers (by Marek Szyprowski, Andrzej
>>> Pietrasiewicz)
>>> - let mmap method to use dma_mmap_writecombine call (by Marek Szyprowski)
>>>
>>> [1]
>>> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/4296
>>> 6/focus=42968 [2] https://lkml.org/lkml/2011/12/26/29
>>> [3]
>>> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/3635
>>> 4/focus=36355 [4]
>>> http://thread.gmane.org/gmane.linux.kernel.cross-arch/12819
>>>
>>> Laurent Pinchart (2):
>>> v4l: vb2-dma-contig: Shorten vb2_dma_contig prefix to vb2_dc
>>> v4l: vb2-dma-contig: Reorder functions
>>>
>>> Marek Szyprowski (2):
>>> v4l: vb2: add prepare/finish callbacks to allocators
>>> v4l: vb2-dma-contig: add prepare/finish to dma-contig allocator
>>>
>>> Sumit Semwal (4):
>>> v4l: Add DMABUF as a memory type
>>> v4l: vb2: add support for shared buffer (dma_buf)
>>> v4l: vb: remove warnings about MEMORY_DMABUF
>>> v4l: vb2-dma-contig: add support for dma_buf importing
>>>
>>> Tomasz Stanislawski (5):
>>> Documentation: media: description of DMABUF importing in V4L2
>>> v4l: vb2-dma-contig: Remove unneeded allocation context structure
>>> v4l: vb2-dma-contig: add support for scatterlist in userptr mode
>>> v4l: s5p-tv: mixer: support for dmabuf importing
>>> v4l: s5p-fimc: support for dmabuf importing
>>>
>>> Documentation/DocBook/media/v4l/compat.xml | 4 +
>>> Documentation/DocBook/media/v4l/io.xml | 179 +++
>>> .../DocBook/media/v4l/vidioc-create-bufs.xml | 1 +
>>> Documentation/DocBook/media/v4l/vidioc-qbuf.xml | 15 +
>>> Documentation/DocBook/media/v4l/vidioc-reqbufs.xml |
Re: [PATCH] dma-buf: mmap support
I'd still rather see some form of explicit cache flusing api, but I can do that through my exporter (and probably will to get started). Otherwise this looks good to me except for comment inline. On Wed, Apr 18, 2012 at 6:52 AM, Daniel Vetter wrote: > Compared to Rob Clark's RFC I've ditched the prepare/finish hooks > and corresponding ioctls on the dma_buf file. The major reason for > that is that many people seem to be under the impression that this is > also for synchronization with outstanding asynchronous processsing. > I'm pretty massively opposed to this because: > > - It boils down reinventing a new rather general-purpose userspace > synchronization interface. If we look at things like futexes, this > is hard to get right. > - Furthermore a lot of kernel code has to interact with this > synchronization primitive. This smells a look like the dri1 hw_lock, > a horror show I prefer not to reinvent. > - Even more fun is that multiple different subsystems would interact > here, so we have plenty of opportunities to create funny deadlock > scenarios. > > I think synchronization is a wholesale different problem from data > sharing and should be tackled as an orthogonal problem. > > Now we could demand that prepare/finish may only ensure cache > coherency (as Rob intended), but that runs up into the next problem: > We not only need mmap support to facilitate sw-only processing nodes > in a pipeline (without jumping through hoops by importing the dma_buf > into some sw-access only importer), which allows for a nicer > ION->dma-buf upgrade path for existing Android userspace. We also need > mmap support for existing importing subsystems to support existing > userspace libraries. And a loot of these subsystems are expected to > export coherent userspace mappings. > > So prepare/finish can only ever be optional and the exporter /needs/ > to support coherent mappings. Given that mmap access is always > somewhat fallback-y in nature I've decided to drop this optimization, > instead of just making it optional. If we demonstrate a clear need for > this, supported by benchmark results, we can always add it in again > later as an optional extension. > > Other differences compared to Rob's RFC is the above mentioned support > for mapping a dma-buf through facilities provided by the importer. > Which results in mmap support no longer being optional. > > Note that this dma-buf mmap patch does _not_ support every possible > insanity an existing subsystem could pull of with mmap: Because it > does not allow to intercept pagefaults and shoot down ptes importing > subsystems can't add some magic of their own at these points (e.g. to > automatically synchronize with outstanding rendering or set up some > special resources). I've done a cursory read through a few mmap > implementions of various subsytems and I'm hopeful that we can avoid > this (and the complexity it'd bring with it). > > Additonally I've extended the documentation a bit to explain the hows > and whys of this mmap extension. > > In case we ever want to add support for explicitly cache maneged > userspace mmap with a prepare/finish ioctl pair, we could specify that > userspace needs to mmap a different part of the dma_buf, e.g. the > range starting at dma_buf->size up to dma_buf->size*2. This works > because the size of a dma_buf is invariant over it's lifetime. The > exporter would obviously need to fall back to coherent mappings for > both ranges if a legacy clients maps the coherent range and the > architecture cannot suppor conflicting caching policies. Also, this > would obviously be optional and userspace needs to be able to fall > back to coherent mappings. > > v2: > - Spelling fixes from Rob Clark. > - Compile fix for !DMA_BUF from Rob Clark. > - Extend commit message to explain how explicitly cache managed mmap > support could be added later. > - Extend the documentation with implementations notes for exporters > that need to manually fake coherency. > > Cc: Rob Clark > Cc: Rebecca Schultz Zavin > Signed-Off-by: Daniel Vetter > --- > Documentation/dma-buf-sharing.txt | 98 > ++--- > drivers/base/dma-buf.c | 64 +++- > include/linux/dma-buf.h | 16 ++ > 3 files changed, 170 insertions(+), 8 deletions(-) > > diff --git a/Documentation/dma-buf-sharing.txt > b/Documentation/dma-buf-sharing.txt > index 3bbd5c5..5ff4d2b 100644 > --- a/Documentation/dma-buf-sharing.txt > +++ b/Documentation/dma-buf-sharing.txt > @@ -29,13 +29,6 @@ The buffer-user > in memory, mapped into its own address space, so it can access the same > ar
Re: [Linaro-mm-sig] [PATCH] [RFC] dma-buf: mmap support
On Wed, Mar 21, 2012 at 3:25 PM, Daniel Vetter wrote: > On Wed, Mar 21, 2012 at 10:46:14AM -0700, Rebecca Schultz Zavin wrote: >> I want to make sure I understand how this would work. I've been planning >> on making cache maintenance implicit, and most of the corresponding >> userspace components I've seen for android expect to do implicit cache >> maintenance on these buffers if they need cached mappings. The android >> framework has a logical place for this maintenance to take place. I assume >> that you'd detect a buffer leaving the cpu domain by using the >> dma_data_direction passed to dma_buf_map_attachment? We're definitely >> pushing a bunch of complexity into the exporter, that at least on android >> could easily be covered by an explicit api. I'm not dead set against it, I >> just want to make sure I get it right if we go down this road. > > Hm, you're talking about implicit cache management, which I read as: The > kernel does some magic so that userspace doesn't have to care. But I guess > you mean what I'd call explicit cache management, where userspace tells > the kernel which ranges to invalidate/flush? Sorry, yeah, I got that backwards :) I definitely mean that I intended to do all the cache maintenance explicitly. > > The idea is that the exporter would invalidate cpu caches at fault time. > And when a dma-device wants to read from it (using the direction argument > of dma_buf_map_attachement) it would shoot down the mappings, flush caches > and then allow the dma op to happen. Note that this is obvously only > required if the mapping is not coherent (uc/wc on arm). This makes sense to me though I have to sort thought exactly how to implement it. > > I agree that for cached mappings this will suck, but to get dma-buf of the > ground I prefer a simpler interface at the beginning, which could get > extended later on. The issue is that I expect that a few importers simply > can't hanlde cache management with explicit flush/invalidate ioctl calls > from userspace and we hence need coherently-looking mappings anyway. Couldn't this just as easily be handled by not having those mappings be mapped cached or write combine to userspace? They'd be coherent, just slow. I'm not sure we can actually say that all these cpu access are necessary slow path operations anyway. On android we do sometimes decide to software render things to eliminate the overhead of maintaining a hardware context for context switching the gpu. If you want cached or writecombine mappings you'd have to manage them explicitly. If you can't manage them explicitly you have to settle for slow. That seems reasonable to me. As far as I can tell with explicit operations I have to invalidate before touching from mmap and clean after. With these implicit ones, I stil have to invalidate and clean, but now I also have to remap them before and after. I don't know what the performance hit of this remapping step is, but I'd like to if you have any insight. Rebecca > > Imo the best way to enable cached mappings is to later on extend dma-buf > (as soon as we have some actual exporters/importers in the mainline > kernel) with an optional cached_mmap interface which requires explict > prepare_mmap_access/finish_mmap_acces calls. Then if both exporter and > importer support this, it could get used - otherwise the dma-buf layer > could transparently fall back to coherent mappings. > > Cheers, Daniel > -- > Daniel Vetter > Mail: dan...@ffwll.ch > Mobile: +41 (0)79 365 57 48 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [PATCH] [RFC] dma-buf: mmap support
On Wed, Mar 21, 2012 at 8:45 AM, Rob Clark wrote: > On Tue, Mar 20, 2012 at 3:53 PM, Daniel Vetter wrote: >> Let's have some competition here for dma_buf mmap support ;-) >> >> Compared to Rob Clarke's RFC I've ditched the prepare/finish hooks >> and corresponding ioctls on the dma_buf file. The major reason for >> that is that many people seem to be under the impression that this is >> also for synchronization with outstanding asynchronous processsing. >> I'm pretty massively opposed to this because: >> >> - It boils down reinventing a new rather general-purpose userspace >> synchronization interface. If we look at things like futexes, this >> is hard to get right. >> - Furthermore a lot of kernel code has to interact with this >> synchronization primitive. This smells a look like the dri1 hw_lock, >> a horror show I prefer not to reinvent. >> - Even more fun is that multiple different subsystems would interact >> here, so we have plenty of opportunities to create funny deadlock >> scenarios. >> >> I think synchronization is a wholesale different problem from data >> sharing and should be tackled as an orthogonal problem. > > fwiw, I was debating about two approaches before I sent initial RFC.. > with or without the ioctl's. > > I do agree that trying to hide synchronization behind those is likely > to be asking for trouble. Although I think it is partially a symptom > of missing a synchronization primitive that we could use. (Ie. when > all you have is a hammer, everything looks like a nail.) > I'm in 100% agreement with you both on synchronization being a separate problem. The android team is working on a generic solution for that as well, expect to see some of it coming across this list in the next few weeks. I do hate the idea that an implementer might abuse this api for that purpose. > On the other hand, it does significantly ease dealing with cached > buffers, and seems useful for other debugging purposes (verifying > userspace isn't accessing buffers when they shouldn't. And adding > required ioctls later is an API change. Which was why I was leaning > towards the approach of including ioctls but just reviewing closely > the patches that add driver support. > > But, I have one idea. What about including the ioctls, but just > leaving them stubbed out (ie. not including the dmabuf ops). It at > least prevents drivers from abusing it, while leaving the API in place > for userspace to avoid changing API to kernel later in a way that > might break userspace. > > Other than that, the patch looks good.. including the extra > error/sanity checking and documentation, which I really skimped on for > the first version. (I mainly just wanted to get the flame-war over > ioctls going with the first version :-P) > > BR, > -R > >> Now we could demand that prepare/finish may only ensure cache >> coherency (as Rob intended), but that runs up into the next problem: >> We not only need mmap support to facilitate sw-only processing nodes >> in a pipeline (without jumping through hoops by importing the dma_buf >> into some sw-access only importer), which allows for a nicer >> ION->dma-buf upgrade path for existing Android userspace. We also need >> mmap support for existing importing subsystems to support existing >> userspace libraries. And a loot of these subsystems are expected to >> export coherent userspace mappings. >> >> So prepare/finish can only ever be optional and the exporter /needs/ >> to support coherent mappings. Given that mmap access is always >> somewhat fallback-y in nature I've decided to drop this optimization, >> instead of just making it optional. If we demonstrate a clear need for >> this, supported by benchmark results, we can always add it in again >> later as an optional extension. >> >> Other differences compared to Rob's RFC is the above mentioned support >> for mapping a dma-buf through facilities provided by the importer. >> Which results in mmap support no longer being optional. >> >> Note taht this dma-buf mmap patch does _not_ support every possible >> insanity an existing subsystem could pull of with mmap: Because it >> does not allow to intercept pagefaults and shoot down ptes importing >> subsystems can't add some magic of their own at these points (e.g. to >> automatically synchronize with outstanding rendering or set up some >> special resources). I've done a cursory read through a few mmap >> implementions of various subsytems and I'm hopeful that we can avoid >> this (and the complexity it'd bring with it). >> >> Additonally I've extended the documentation a bit to explain the hows >> and whys of this mmap extension. >> >> Comments, reviews and flames highly welcome. >> >> Cheers, Daniel >> --- >> Documentation/dma-buf-sharing.txt | 84 >> +--- >> drivers/base/dma-buf.c | 64 +++- >> include/linux/dma-buf.h | 16 +++ >> 3 files changed, 156 insertions(+), 8 deletions(-) >>
