Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752697AbcD0TFQ (ORCPT <rfc822;w@1wt.eu>);
	Wed, 27 Apr 2016 15:05:16 -0400
Received: from mail-wm0-f65.google.com ([74.125.82.65]:34611 "EHLO
	mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1750753AbcD0TFN (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 27 Apr 2016 15:05:13 -0400
Date: Wed, 27 Apr 2016 21:05:08 +0200
From: Daniel Vetter <daniel@ffwll.ch>
To: Gustavo Padovan <gustavo@padovan.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
        dri-devel@lists.freedesktop.org, Daniel Stone <daniels@collabora.com>,
        Arve =?iso-8859-1?B?SGr4bm5lduVn?= <arve@android.com>,
        Riley Andrews <riandrews@android.com>,
        Daniel Vetter <daniel.vetter@ffwll.ch>,
        Rob Clark <robdclark@gmail.com>, Greg Hackmann <ghackmann@google.com>,
        John Harrison <John.C.Harrison@Intel.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Subject: Re: [PATCH 12/12] Documentation: add Sync File doc
Message-ID: <20160427190508.GE2558@phenom.ffwll.local>
Mail-Followup-To: Gustavo Padovan <gustavo@padovan.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
	dri-devel@lists.freedesktop.org,
	Daniel Stone <daniels@collabora.com>,
	Arve =?iso-8859-1?B?SGr4bm5lduVn?= <arve@android.com>,
	Riley Andrews <riandrews@android.com>,
	Rob Clark <robdclark@gmail.com>,
	Greg Hackmann <ghackmann@google.com>,
	John Harrison <John.C.Harrison@Intel.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Sumit Semwal <sumit.semwal@linaro.org>,
	Gustavo Padovan <gustavo.padovan@collabora.co.uk>
References: <1461774439-11512-1-git-send-email-gustavo@padovan.org>
 <1461774439-11512-13-git-send-email-gustavo@padovan.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1461774439-11512-13-git-send-email-gustavo@padovan.org>
X-Operating-System: Linux phenom 4.6.0-rc5+ 
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 4373
Lines: 126

On Wed, Apr 27, 2016 at 01:27:19PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Add sync_file documentation on dma-buf-sync_file.txt
> ---
>  Documentation/dma-buf-sync_file.txt | 65 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/dma-buf-sync_file.txt
> 
> diff --git a/Documentation/dma-buf-sync_file.txt b/Documentation/dma-buf-sync_file.txt
> new file mode 100644
> index 0000000..aa7320f
> --- /dev/null
> +++ b/Documentation/dma-buf-sync_file.txt
> @@ -0,0 +1,65 @@
> +			 DMA Buffer Sync File API Guide
> +			 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Strictly speaking you can use sync_file fences without dma-bufs. E.g. when
your gpu uses something like shared virtual memory and directly access the
userspace address space there's not buffer handles involved, but you still
might want to pass around fences between drivers and/or processes. So
please drop dma-buf from the filename and everything else in here. It's ok
to mention that this is used together with buffers/processing, just as an
example.

> +
> +				Gustavo Padovan
> +			  <gustavo at padovan dot org>
> +
> +This document serves as a guide for device drivers writers on what is the
> +dma-buf sync_file API, and how drivers can support it. Sync file is the

"... on what the sync_file API _is_, ..."

> +carrier of the fences(struct fence) that needs to synchronized between drivers.

"... between drivers or across process boundaries."

Android wants/needs both.

> +
> +The sync_file API is meant to be used to send and receive fence information
> +to/from userspace. It enables userspace to do explicit fencing, where instead
> +of attaching a fence to the buffer a Producer driver (such as GPU or V4L

_p_roducer

"(such as _a_ GPU ...)"

> +driver) it sends the fence related to the buffer to userspace. The fence then

s/it//

> +can be sent to the Consumer (DRM driver for example), that will not use the

_c_onsumer

> +buffer for anything before the fence signals, i.e., the driver that issued the
> +fence is not using/processing the buffer anymore, so it signals that the buffer
> +is ready to use. And vice-versa for the Consumer -> Producer part of the cycle.

A bit a run-on sentence above. Might want to split it.

> +Sync files allows userspace awareness on the DMA buffer sharing synchronization
> +between drivers.
> +
> +Sync file was originally added in the Android kernel but current Linux Desktop
> +can benefit a lot from it.
> +
> +in-fences and out-fences
> +------------------------
> +
> +Sync files can go either to or from userspace. When a sync_file is sent from
> +the driver to userspace we call the fences it contains 'out-fences'. They are
> +related to a buffer that the driver is processing or is going to process, so
> +the driver create out-fences to be able to notify, through fence_signal(), when

"... create_s_ _an_ out fence ..."

> +it has finished using (or processing) that buffer. Out-fences are fences that
> +the driver creates.
> +
> +On the other hand if the driver receives fence(s) through a sync_file from
> +userspace we call these fence(s) 'in-fences'. Receiveing in-fences means that
> +we need to wait for the fence(s) to signal before using any buffer related to
> +the in-fences.
> +
> +Creating Sync Files
> +-------------------
> +
> +When a driver needs to send an out-fence userspace it creates a sync_file.
> +
> +Interface:
> +	struct sync_file *sync_file_create(const char *name, struct fence *fence);
> +
> +The caller pass the name and the out-fence and gets back the sync_file. That is
> +just the first step, next it needs to install an fd on sync_file->file. So it
> +gets an fd:
> +
> +	fd = get_unused_fd_flags(O_CLOEXEC);
> +
> +and installs it on sync_file->file:
> +
> +	fd_install(fd, sync_file->file);
> +
> +The sync_file fd now can be sent to userspace.
> +
> +If the creation process fail, or the sync_file needs to be released by any
> +other reason fput(sync_file->file) should be used.

With the above nitpicks fixed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> +
> +References:
> +[1] struct sync_file in include/linux/sync_file.h
> +[2] All interfaces mentioned above defined in include/linux/sync_file.h
> -- 
> 2.5.5
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch