On 12/17/2015 04:19 PM, Alex Deucher wrote:
> On Wed, Dec 16, 2015 at 5:25 PM, Tiago Vignatti
> <tiago.vignatti at intel.com> wrote:
>> From: Daniel Vetter <daniel.vetter at 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 at linaro.org>
>> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
>> Signed-off-by: Tiago Vignatti <tiago.vignatti at 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.
>>
>> 2. Supporting existing mmap interfaces in importers
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index b2ac13b..9a298bd 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -34,6 +34,8 @@
>> #include <linux/poll.h>
>> #include <linux/reservation.h>
>>
>> +#include <uapi/linux/dma-buf.h>
>> +
>> static inline int is_dma_buf_file(struct file *);
>>
>> struct dma_buf_list {
>> @@ -251,11 +253,52 @@ out:
>> return events;
>> }
>>
>> +static long dma_buf_ioctl(struct file *file,
>> + unsigned int cmd, unsigned long arg)
>> +{
>> + struct dma_buf *dmabuf;
>> + struct dma_buf_sync sync;
>> + enum dma_data_direction direction;
>> +
>> + dmabuf = file->private_data;
>> +
>> + if (!is_dma_buf_file(file))
>> + return -EINVAL;
>> +
>> + switch (cmd) {
>> + case DMA_BUF_IOCTL_SYNC:
>> + if (copy_from_user(&sync, (void __user *) arg, sizeof(sync)))
>> + return -EFAULT;
>> +
>> + if (sync.flags & DMA_BUF_SYNC_RW)
>> + direction = DMA_BIDIRECTIONAL;
>> + else if (sync.flags & DMA_BUF_SYNC_READ)
>> + direction = DMA_FROM_DEVICE;
>> + else if (sync.flags & DMA_BUF_SYNC_WRITE)
>> + direction = DMA_TO_DEVICE;
>> + else
>> + return -EINVAL;
>> +
>> + if (sync.flags & ~DMA_BUF_SYNC_VALID_FLAGS_MASK)
>> + return -EINVAL;
>> +
>> + if (sync.flags & DMA_BUF_SYNC_END)
>> + dma_buf_end_cpu_access(dmabuf, direction);
>> + else
>> + dma_buf_begin_cpu_access(dmabuf, direction);
>> +
>> + return 0;
>> + default:
>> + return -ENOTTY;
>> + }
>> +}
>> +
>> static const struct file_operations dma_buf_fops = {
>> .release = dma_buf_release,
>> .mmap = dma_buf_mmap_internal,
>> .llseek = dma_buf_llseek,
>> .poll = dma_buf_poll,
>> + .unlocked_ioctl = dma_buf_ioctl,
>> };
>>
>> /*
>> diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h
>> new file mode 100644
>> index 0000000..bd195f2
>> --- /dev/null
>> +++ b/include/uapi/linux/dma-buf.h
>> @@ -0,0 +1,38 @@
>> +/*
>> + * Framework for buffer objects that can be shared across
>> devices/subsystems.
>> + *
>> + * Copyright(C) 2015 Intel Ltd
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published
>> by
>> + * the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> with
>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef _DMA_BUF_UAPI_H_
>> +#define _DMA_BUF_UAPI_H_
>> +
>> +/* begin/end dma-buf functions used for userspace mmap. */
>> +struct dma_buf_sync {
>> + __u64 flags;
>> +};
>> +
>> +#define DMA_BUF_SYNC_READ (1 << 0)
>> +#define DMA_BUF_SYNC_WRITE (2 << 0)
>> +#define DMA_BUF_SYNC_RW (3 << 0)
>
> Maybe make this:
>
> #define DMA_BUF_SYNC_RW (DMA_BUF_SYNC_READ | DMA_BUF_SYNC_WRITE)
>
> or maybe:
>
> #define DMA_BUF_SYNC_RW_MASK (3 << 0)
Thanks Alex. I think I like your first option for being a bit more
suggestive than the other -- I'm changing for it now.
Tiago
