dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Hellstrom <thellstrom@vmware.com>
To: Tiago Vignatti <tiago.vignatti@intel.com>,
	dri-devel@lists.freedesktop.org
Cc: jglisse@redhat.com, daniel.vetter@ffwll.ch,
	daniel.thompson@linaro.org,
	Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [PATCH v6 3/5] dma-buf: Add ioctls to allow userspace to flush
Date: Mon, 21 Dec 2015 10:38:32 +0100	[thread overview]
Message-ID: <5677C898.8090500@vmware.com> (raw)
In-Reply-To: <56746398.5080305@intel.com>

On 12/18/2015 08:50 PM, Tiago Vignatti wrote:
> On 12/17/2015 07:58 PM, Thomas Hellstrom wrote:
>> On 12/16/2015 11:25 PM, Tiago Vignatti wrote:
>>> From: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>
>>> The userspace might need some sort of cache coherency management
>>> e.g. when CPU
>>> and GPU domains are being accessed through dma-buf at the same time. To
>>> circumvent this problem there are begin/end coherency markers, that
>>> forward
>>> directly to existing dma-buf device drivers vfunc hooks. Userspace
>>> can make use
>>> of those markers through the DMA_BUF_IOCTL_SYNC ioctl. The sequence
>>> would be
>>> used like following:
>>>       - mmap dma-buf fd
>>>       - for each drawing/upload cycle in CPU 1. SYNC_START ioctl, 2.
>>> read/write
>>>         to mmap area 3. SYNC_END ioctl. This can be repeated as
>>> often as you
>>>         want (with the new data being consumed by the GPU or say
>>> scanout device)
>>>       - munmap once you don't need the buffer any more
>>>
>>> v2 (Tiago): Fix header file type names (u64 -> __u64)
>>> v3 (Tiago): Add documentation. Use enum dma_buf_sync_flags to the
>>> begin/end
>>> dma-buf functions. Check for overflows in start/length.
>>> v4 (Tiago): use 2d regions for sync.
>>> v5 (Tiago): forget about 2d regions (v4); use _IOW in
>>> DMA_BUF_IOCTL_SYNC and
>>> remove range information from struct dma_buf_sync.
>>> v6 (Tiago): use __u64 structured padded flags instead enum. Adjust
>>> documentation about the recommendation on using sync ioctls.
>>>
>>> Cc: Sumit Semwal <sumit.semwal@linaro.org>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
>>> ---
>>>   Documentation/dma-buf-sharing.txt | 22 +++++++++++++++++++-
>>>   drivers/dma-buf/dma-buf.c         | 43
>>> +++++++++++++++++++++++++++++++++++++++
>>>   include/uapi/linux/dma-buf.h      | 38
>>> ++++++++++++++++++++++++++++++++++
>>>   3 files changed, 102 insertions(+), 1 deletion(-)
>>>   create mode 100644 include/uapi/linux/dma-buf.h
>>>
>>> diff --git a/Documentation/dma-buf-sharing.txt
>>> b/Documentation/dma-buf-sharing.txt
>>> index 4f4a84b..2ddd4b2 100644
>>> --- a/Documentation/dma-buf-sharing.txt
>>> +++ b/Documentation/dma-buf-sharing.txt
>>> @@ -350,7 +350,27 @@ Being able to mmap an export dma-buf buffer
>>> object has 2 main use-cases:
>>>      handles, too). So it's beneficial to support this in a similar
>>> fashion on
>>>      dma-buf to have a good transition path for existing Android
>>> userspace.
>>>
>>> -   No special interfaces, userspace simply calls mmap on the
>>> dma-buf fd.
>>> +   No special interfaces, userspace simply calls mmap on the
>>> dma-buf fd. Very
>>> +   important to note though is that, even if it is not mandatory,
>>> the userspace
>>> +   is strongly recommended to always use the cache synchronization
>>> ioctl
>>> +   (DMA_BUF_IOCTL_SYNC) discussed next.
>>> +
>>> +   Some systems might need some sort of cache coherency management
>>> e.g. when
>>> +   CPU and GPU domains are being accessed through dma-buf at the
>>> same time. To
>>> +   circumvent this problem there are begin/end coherency markers,
>>> that forward
>>> +   directly to existing dma-buf device drivers vfunc hooks.
>>> Userspace can make
>>> +   use of those markers through the DMA_BUF_IOCTL_SYNC ioctl. The
>>> sequence
>>> +   would be used like following:
>>> +     - mmap dma-buf fd
>>> +     - for each drawing/upload cycle in CPU 1. SYNC_START ioctl, 2.
>>> read/write
>>> +       to mmap area 3. SYNC_END ioctl. This can be repeated as
>>> often as you
>>> +       want (with the new data being consumed by the GPU or say
>>> scanout device)
>>> +     - munmap once you don't need the buffer any more
>>> +
>>> +    In principle systems with the memory cache shared by the GPU
>>> and CPU may
>>> +    not need SYNC_START and SYNC_END but still, userspace is always
>>> encouraged
>>> +    to use these ioctls before and after, respectively, when
>>> accessing the
>>> +    mapped address.
>>>
>>
>> I think the wording here is far too weak. If this is a generic
>> user-space interface and syncing
>> is required for
>> a) Correctness: then syncing must be mandatory.
>> b) Optimal performance then an implementation must generate expected
>> results also in the absence of SYNC ioctls, but is allowed to rely on
>> correct pairing of SYNC_START and SYNC_END to render correctly.
>
> Thomas, do you think the following write-up captures this?
>
>
> -   No special interfaces, userspace simply calls mmap on the dma-buf fd.
> +   No special interfaces, userspace simply calls mmap on the dma-buf
> fd, making
> +   sure that the cache synchronization ioctl (DMA_BUF_IOCTL_SYNC) is
> *always*
> +   used when the access happens. This is discussed next paragraphs.
> +
> +   Some systems might need some sort of cache coherency management
> e.g. when
> +   CPU and GPU domains are being accessed through dma-buf at the same
> time. To
> +   circumvent this problem there are begin/end coherency markers,
> that forward
> +   directly to existing dma-buf device drivers vfunc hooks. Userspace
> can make
> +   use of those markers through the DMA_BUF_IOCTL_SYNC ioctl. The
> sequence
> +   would be used like following:
> +     - mmap dma-buf fd
> +     - for each drawing/upload cycle in CPU 1. SYNC_START ioctl, 2.
> read/write
> +       to mmap area 3. SYNC_END ioctl. This can be repeated as often
> as you
> +       want (with the new data being consumed by the GPU or say
> scanout device)
> +     - munmap once you don't need the buffer any more
> +
> +    Therefore, for correctness and optimal performance, systems with
> the memory
> +    cache shared by the GPU and CPU i.e. the "coherent" and also
> "incoherent"
> +    systems are always required to use SYNC_START and SYNC_END before
> and
> +    after, respectively, when accessing the mapped address.
>
>
> Thank you,
>
> Tiago
Yes, that sounds better,

Thanks,
Thomas

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2015-12-21  9:38 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-16 22:25 Direct userspace dma-buf mmap (v6) Tiago Vignatti
2015-12-16 22:25 ` [PATCH v6 1/5] drm: prime: Honour O_RDWR during prime-handle-to-fd Tiago Vignatti
2015-12-16 22:25 ` [PATCH v6 2/5] dma-buf: Remove range-based flush Tiago Vignatti
2015-12-16 22:25 ` [PATCH v6 3/5] dma-buf: Add ioctls to allow userspace to flush Tiago Vignatti
2015-12-17 18:19   ` Alex Deucher
2015-12-18 18:46     ` Tiago Vignatti
2015-12-17 21:58   ` Thomas Hellstrom
2015-12-18 15:29     ` Daniel Vetter
2015-12-18 19:50     ` Tiago Vignatti
2015-12-21  9:38       ` Thomas Hellstrom [this message]
2015-12-16 22:25 ` [PATCH v6 4/5] drm/i915: Implement end_cpu_access Tiago Vignatti
2015-12-17  8:01   ` Chris Wilson
2015-12-18 19:02     ` Tiago Vignatti
2015-12-18 19:19       ` Tiago Vignatti
2015-12-22 20:51         ` Chris Wilson
2015-12-16 22:25 ` [PATCH v6 5/5] drm/i915: Use CPU mapping for userspace dma-buf mmap() Tiago Vignatti
2015-12-16 22:25 ` [PATCH igt v6 1/6] lib: Add gem_userptr and __gem_userptr helpers Tiago Vignatti
2015-12-16 22:25 ` [PATCH igt v6 2/6] prime_mmap: Add new test for calling mmap() on dma-buf fds Tiago Vignatti
2015-12-16 22:25 ` [PATCH igt v6 3/6] prime_mmap: Add basic tests to write in a bo using CPU Tiago Vignatti
2015-12-16 22:25 ` [PATCH igt v6 4/6] lib: Add prime_sync_start and prime_sync_end helpers Tiago Vignatti
2015-12-17 10:18   ` Daniel Vetter
2015-12-16 22:25 ` [PATCH igt v6 5/6] tests: Add kms_mmap_write_crc for cache coherency tests Tiago Vignatti
2015-12-17  7:53   ` Chris Wilson
2015-12-16 22:25 ` [PATCH igt v6 6/6] tests: Add prime_mmap_coherency " Tiago Vignatti
2015-12-17 10:15 ` Direct userspace dma-buf mmap (v6) Daniel Vetter

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=5677C898.8090500@vmware.com \
    --to=thellstrom@vmware.com \
    --cc=daniel.thompson@linaro.org \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jglisse@redhat.com \
    --cc=tiago.vignatti@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox