[1/4] dma-buf: Remove kmap kerneldoc vestiges

  Also try to clarify a bit when dma_buf_begin/end_cpu_access should
be called.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
 include/linux/dma-buf.h   | 25 +++++++++----------------
 2 files changed, 23 insertions(+), 22 deletions(-)
  

Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> Also try to clarify a bit when dma_buf_begin/end_cpu_access should
> be called.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> ---
>   drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
>   include/linux/dma-buf.h   | 25 +++++++++----------------
>   2 files changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index e63684d4cd90..a12fdffa130f 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
>    *   vmalloc space might be limited and result in vmap calls failing.
>    *
>    *   Interfaces::
> + *
>    *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
>    *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
>    *
>    *   The vmap call can fail if there is no vmap support in the exporter, or if
> - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
> - *   that the dma-buf layer keeps a reference count for all vmap access and
> - *   calls down into the exporter's vmap function only when no vmapping exists,
> - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
> - *   provided by taking the dma_buf->lock mutex.
> + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
> + *   count for all vmap access and calls down into the exporter's vmap function
> + *   only when no vmapping exists, and only unmaps it once. Protection against
> + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.

Who is talking the lock? The caller of the dma_buf_vmap/vunmap() 
functions, the functions itself or the callback inside the exporter?

Christian.

>    *
>    * - For full compatibility on the importer side with existing userspace
>    *   interfaces, which might already support mmap'ing buffers. This is needed in
> @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>    * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
>    * it guaranteed to be coherent with other DMA access.
>    *
> + * This function will also wait for any DMA transactions tracked through
> + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
> + * synchronization this function will only ensure cache coherency, callers must
> + * ensure synchronization with such DMA transactions on their own.
> + *
>    * Can return negative error values, returns 0 on success.
>    */
>   int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
>    * This call may fail due to lack of virtual mapping address space.
>    * These calls are optional in drivers. The intended use for them
>    * is for mapping objects linear in kernel space for high use objects.
> - * Please attempt to use kmap/kunmap before thinking about these interfaces.
> + *
> + * To ensure coherency users must call dma_buf_begin_cpu_access() and
> + * dma_buf_end_cpu_access() around any cpu access performed through this
> + * mapping.
>    *
>    * Returns 0 on success, or a negative errno code otherwise.
>    */
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index cf72699cb2bc..7eca37c8b10c 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -183,24 +183,19 @@ struct dma_buf_ops {
>   	 * @begin_cpu_access:
>   	 *
>   	 * This is called from dma_buf_begin_cpu_access() and allows the
> -	 * exporter to ensure that the memory is actually available for cpu
> -	 * access - the exporter might need to allocate or swap-in and pin the
> -	 * backing storage. The exporter also needs to ensure that cpu access is
> -	 * coherent for the access direction. The direction can be used by the
> -	 * exporter to optimize the cache flushing, i.e. access with a different
> +	 * exporter to ensure that the memory is actually coherent for cpu
> +	 * access. The exporter also needs to ensure that cpu access is coherent
> +	 * for the access direction. The direction can be used by the exporter
> +	 * to optimize the cache flushing, i.e. access with a different
>   	 * direction (read instead of write) might return stale or even bogus
>   	 * data (e.g. when the exporter needs to copy the data to temporary
>   	 * storage).
>   	 *
> -	 * This callback is optional.
> +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
> +	 * command for userspace mappings established through @mmap, and also
> +	 * for kernel mappings established with @vmap.
>   	 *
> -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
> -	 * from userspace (where storage shouldn't be pinned to avoid handing
> -	 * de-factor mlock rights to userspace) and for the kernel-internal
> -	 * users of the various kmap interfaces, where the backing storage must
> -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
> -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
> -	 * real problem.
> +	 * This callback is optional.
>   	 *
>   	 * Returns:
>   	 *
> @@ -216,9 +211,7 @@ struct dma_buf_ops {
>   	 *
>   	 * This is called from dma_buf_end_cpu_access() when the importer is
>   	 * done accessing the CPU. The exporter can use this to flush caches and
> -	 * unpin any resources pinned in @begin_cpu_access.
> -	 * The result of any dma_buf kmap calls after end_cpu_access is
> -	 * undefined.
> +	 * undo anything else done in @begin_cpu_access.
>   	 *
>   	 * This callback is optional.
>   	 *
  

On Mon, Dec 14, 2020 at 11:33:10AM +0100, Christian König wrote:
> Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> > Also try to clarify a bit when dma_buf_begin/end_cpu_access should
> > be called.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: linux-media@vger.kernel.org
> > Cc: linaro-mm-sig@lists.linaro.org
> > ---
> >   drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
> >   include/linux/dma-buf.h   | 25 +++++++++----------------
> >   2 files changed, 23 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index e63684d4cd90..a12fdffa130f 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
> >    *   vmalloc space might be limited and result in vmap calls failing.
> >    *
> >    *   Interfaces::
> > + *
> >    *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
> >    *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
> >    *
> >    *   The vmap call can fail if there is no vmap support in the exporter, or if
> > - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
> > - *   that the dma-buf layer keeps a reference count for all vmap access and
> > - *   calls down into the exporter's vmap function only when no vmapping exists,
> > - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
> > - *   provided by taking the dma_buf->lock mutex.
> > + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
> > + *   count for all vmap access and calls down into the exporter's vmap function
> > + *   only when no vmapping exists, and only unmaps it once. Protection against
> > + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
> 
> Who is talking the lock? The caller of the dma_buf_vmap/vunmap() functions,
> the functions itself or the callback inside the exporter?

That's the part I didn't change at all here, just re-laid out the line
breaking. I only removed the outdated kmap section here.

Should I do another patch and remove this one sentence here (it's kinda
pointless and generally we don't muse about implementation details that
callers don't care about)?

I did try and do a cursory review of the dma-buf docs, but this is kinda
not meant as an all-out revamp. Just a few things I've noticed while
reviewing Thomas' vmap_local stuff.
-Daniel

> 
> Christian.
> 
> >    *
> >    * - For full compatibility on the importer side with existing userspace
> >    *   interfaces, which might already support mmap'ing buffers. This is needed in
> > @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> >    * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
> >    * it guaranteed to be coherent with other DMA access.
> >    *
> > + * This function will also wait for any DMA transactions tracked through
> > + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
> > + * synchronization this function will only ensure cache coherency, callers must
> > + * ensure synchronization with such DMA transactions on their own.
> > + *
> >    * Can return negative error values, returns 0 on success.
> >    */
> >   int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
> >    * This call may fail due to lack of virtual mapping address space.
> >    * These calls are optional in drivers. The intended use for them
> >    * is for mapping objects linear in kernel space for high use objects.
> > - * Please attempt to use kmap/kunmap before thinking about these interfaces.
> > + *
> > + * To ensure coherency users must call dma_buf_begin_cpu_access() and
> > + * dma_buf_end_cpu_access() around any cpu access performed through this
> > + * mapping.
> >    *
> >    * Returns 0 on success, or a negative errno code otherwise.
> >    */
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index cf72699cb2bc..7eca37c8b10c 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -183,24 +183,19 @@ struct dma_buf_ops {
> >   	 * @begin_cpu_access:
> >   	 *
> >   	 * This is called from dma_buf_begin_cpu_access() and allows the
> > -	 * exporter to ensure that the memory is actually available for cpu
> > -	 * access - the exporter might need to allocate or swap-in and pin the
> > -	 * backing storage. The exporter also needs to ensure that cpu access is
> > -	 * coherent for the access direction. The direction can be used by the
> > -	 * exporter to optimize the cache flushing, i.e. access with a different
> > +	 * exporter to ensure that the memory is actually coherent for cpu
> > +	 * access. The exporter also needs to ensure that cpu access is coherent
> > +	 * for the access direction. The direction can be used by the exporter
> > +	 * to optimize the cache flushing, i.e. access with a different
> >   	 * direction (read instead of write) might return stale or even bogus
> >   	 * data (e.g. when the exporter needs to copy the data to temporary
> >   	 * storage).
> >   	 *
> > -	 * This callback is optional.
> > +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
> > +	 * command for userspace mappings established through @mmap, and also
> > +	 * for kernel mappings established with @vmap.
> >   	 *
> > -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
> > -	 * from userspace (where storage shouldn't be pinned to avoid handing
> > -	 * de-factor mlock rights to userspace) and for the kernel-internal
> > -	 * users of the various kmap interfaces, where the backing storage must
> > -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
> > -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
> > -	 * real problem.
> > +	 * This callback is optional.
> >   	 *
> >   	 * Returns:
> >   	 *
> > @@ -216,9 +211,7 @@ struct dma_buf_ops {
> >   	 *
> >   	 * This is called from dma_buf_end_cpu_access() when the importer is
> >   	 * done accessing the CPU. The exporter can use this to flush caches and
> > -	 * unpin any resources pinned in @begin_cpu_access.
> > -	 * The result of any dma_buf kmap calls after end_cpu_access is
> > -	 * undefined.
> > +	 * undo anything else done in @begin_cpu_access.
> >   	 *
> >   	 * This callback is optional.
> >   	 *
>
  

Am 14.12.20 um 17:01 schrieb Daniel Vetter:
> On Mon, Dec 14, 2020 at 11:33:10AM +0100, Christian König wrote:
>> Am 11.12.20 um 16:58 schrieb Daniel Vetter:
>>> Also try to clarify a bit when dma_buf_begin/end_cpu_access should
>>> be called.
>>>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: Sumit Semwal <sumit.semwal@linaro.org>
>>> Cc: "Christian König" <christian.koenig@amd.com>
>>> Cc: linux-media@vger.kernel.org
>>> Cc: linaro-mm-sig@lists.linaro.org
>>> ---
>>>    drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
>>>    include/linux/dma-buf.h   | 25 +++++++++----------------
>>>    2 files changed, 23 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index e63684d4cd90..a12fdffa130f 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
>>>     *   vmalloc space might be limited and result in vmap calls failing.
>>>     *
>>>     *   Interfaces::
>>> + *
>>>     *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
>>>     *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
>>>     *
>>>     *   The vmap call can fail if there is no vmap support in the exporter, or if
>>> - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
>>> - *   that the dma-buf layer keeps a reference count for all vmap access and
>>> - *   calls down into the exporter's vmap function only when no vmapping exists,
>>> - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
>>> - *   provided by taking the dma_buf->lock mutex.
>>> + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
>>> + *   count for all vmap access and calls down into the exporter's vmap function
>>> + *   only when no vmapping exists, and only unmaps it once. Protection against
>>> + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
>> Who is talking the lock? The caller of the dma_buf_vmap/vunmap() functions,
>> the functions itself or the callback inside the exporter?
> That's the part I didn't change at all here, just re-laid out the line
> breaking. I only removed the outdated kmap section here.

I just wanted to point out that this still isn't described here very very.


> Should I do another patch and remove this one sentence here (it's kinda
> pointless and generally we don't muse about implementation details that
> callers don't care about)?

Na, works like this for me.

> I did try and do a cursory review of the dma-buf docs, but this is kinda
> not meant as an all-out revamp. Just a few things I've noticed while
> reviewing Thomas' vmap_local stuff.


Fell free to add an Acked-by: Christian König <christian.koenig@amd.com> 
to the series.

Christian.

> -Daniel
>
>> Christian.
>>
>>>     *
>>>     * - For full compatibility on the importer side with existing userspace
>>>     *   interfaces, which might already support mmap'ing buffers. This is needed in
>>> @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>>>     * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
>>>     * it guaranteed to be coherent with other DMA access.
>>>     *
>>> + * This function will also wait for any DMA transactions tracked through
>>> + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
>>> + * synchronization this function will only ensure cache coherency, callers must
>>> + * ensure synchronization with such DMA transactions on their own.
>>> + *
>>>     * Can return negative error values, returns 0 on success.
>>>     */
>>>    int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>>> @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
>>>     * This call may fail due to lack of virtual mapping address space.
>>>     * These calls are optional in drivers. The intended use for them
>>>     * is for mapping objects linear in kernel space for high use objects.
>>> - * Please attempt to use kmap/kunmap before thinking about these interfaces.
>>> + *
>>> + * To ensure coherency users must call dma_buf_begin_cpu_access() and
>>> + * dma_buf_end_cpu_access() around any cpu access performed through this
>>> + * mapping.
>>>     *
>>>     * Returns 0 on success, or a negative errno code otherwise.
>>>     */
>>> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
>>> index cf72699cb2bc..7eca37c8b10c 100644
>>> --- a/include/linux/dma-buf.h
>>> +++ b/include/linux/dma-buf.h
>>> @@ -183,24 +183,19 @@ struct dma_buf_ops {
>>>    	 * @begin_cpu_access:
>>>    	 *
>>>    	 * This is called from dma_buf_begin_cpu_access() and allows the
>>> -	 * exporter to ensure that the memory is actually available for cpu
>>> -	 * access - the exporter might need to allocate or swap-in and pin the
>>> -	 * backing storage. The exporter also needs to ensure that cpu access is
>>> -	 * coherent for the access direction. The direction can be used by the
>>> -	 * exporter to optimize the cache flushing, i.e. access with a different
>>> +	 * exporter to ensure that the memory is actually coherent for cpu
>>> +	 * access. The exporter also needs to ensure that cpu access is coherent
>>> +	 * for the access direction. The direction can be used by the exporter
>>> +	 * to optimize the cache flushing, i.e. access with a different
>>>    	 * direction (read instead of write) might return stale or even bogus
>>>    	 * data (e.g. when the exporter needs to copy the data to temporary
>>>    	 * storage).
>>>    	 *
>>> -	 * This callback is optional.
>>> +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
>>> +	 * command for userspace mappings established through @mmap, and also
>>> +	 * for kernel mappings established with @vmap.
>>>    	 *
>>> -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
>>> -	 * from userspace (where storage shouldn't be pinned to avoid handing
>>> -	 * de-factor mlock rights to userspace) and for the kernel-internal
>>> -	 * users of the various kmap interfaces, where the backing storage must
>>> -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
>>> -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
>>> -	 * real problem.
>>> +	 * This callback is optional.
>>>    	 *
>>>    	 * Returns:
>>>    	 *
>>> @@ -216,9 +211,7 @@ struct dma_buf_ops {
>>>    	 *
>>>    	 * This is called from dma_buf_end_cpu_access() when the importer is
>>>    	 * done accessing the CPU. The exporter can use this to flush caches and
>>> -	 * unpin any resources pinned in @begin_cpu_access.
>>> -	 * The result of any dma_buf kmap calls after end_cpu_access is
>>> -	 * undefined.
>>> +	 * undo anything else done in @begin_cpu_access.
>>>    	 *
>>>    	 * This callback is optional.
>>>    	 *
  

On Tue, Dec 15, 2020 at 03:18:49PM +0100, Christian König wrote:
> Am 14.12.20 um 17:01 schrieb Daniel Vetter:
> > On Mon, Dec 14, 2020 at 11:33:10AM +0100, Christian König wrote:
> > > Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> > > > Also try to clarify a bit when dma_buf_begin/end_cpu_access should
> > > > be called.
> > > > 
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > > > Cc: "Christian König" <christian.koenig@amd.com>
> > > > Cc: linux-media@vger.kernel.org
> > > > Cc: linaro-mm-sig@lists.linaro.org
> > > > ---
> > > >    drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
> > > >    include/linux/dma-buf.h   | 25 +++++++++----------------
> > > >    2 files changed, 23 insertions(+), 22 deletions(-)
> > > > 
> > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > > > index e63684d4cd90..a12fdffa130f 100644
> > > > --- a/drivers/dma-buf/dma-buf.c
> > > > +++ b/drivers/dma-buf/dma-buf.c
> > > > @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
> > > >     *   vmalloc space might be limited and result in vmap calls failing.
> > > >     *
> > > >     *   Interfaces::
> > > > + *
> > > >     *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
> > > >     *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
> > > >     *
> > > >     *   The vmap call can fail if there is no vmap support in the exporter, or if
> > > > - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
> > > > - *   that the dma-buf layer keeps a reference count for all vmap access and
> > > > - *   calls down into the exporter's vmap function only when no vmapping exists,
> > > > - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
> > > > - *   provided by taking the dma_buf->lock mutex.
> > > > + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
> > > > + *   count for all vmap access and calls down into the exporter's vmap function
> > > > + *   only when no vmapping exists, and only unmaps it once. Protection against
> > > > + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
> > > Who is talking the lock? The caller of the dma_buf_vmap/vunmap() functions,
> > > the functions itself or the callback inside the exporter?
> > That's the part I didn't change at all here, just re-laid out the line
> > breaking. I only removed the outdated kmap section here.
> 
> I just wanted to point out that this still isn't described here very very.
> 
> 
> > Should I do another patch and remove this one sentence here (it's kinda
> > pointless and generally we don't muse about implementation details that
> > callers don't care about)?
> 
> Na, works like this for me.
> 
> > I did try and do a cursory review of the dma-buf docs, but this is kinda
> > not meant as an all-out revamp. Just a few things I've noticed while
> > reviewing Thomas' vmap_local stuff.
> 
> 
> Fell free to add an Acked-by: Christian König <christian.koenig@amd.com> to
> the series.

Thanks for taking a look, and yeah I actually want to do a review of all
the dma-buf docs but just haven't found the quiet time for that yet.

Patches pushed to drm-misc-next.
-Daniel

> 
> Christian.
> 
> > -Daniel
> > 
> > > Christian.
> > > 
> > > >     *
> > > >     * - For full compatibility on the importer side with existing userspace
> > > >     *   interfaces, which might already support mmap'ing buffers. This is needed in
> > > > @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > > >     * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
> > > >     * it guaranteed to be coherent with other DMA access.
> > > >     *
> > > > + * This function will also wait for any DMA transactions tracked through
> > > > + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
> > > > + * synchronization this function will only ensure cache coherency, callers must
> > > > + * ensure synchronization with such DMA transactions on their own.
> > > > + *
> > > >     * Can return negative error values, returns 0 on success.
> > > >     */
> > > >    int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > > > @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
> > > >     * This call may fail due to lack of virtual mapping address space.
> > > >     * These calls are optional in drivers. The intended use for them
> > > >     * is for mapping objects linear in kernel space for high use objects.
> > > > - * Please attempt to use kmap/kunmap before thinking about these interfaces.
> > > > + *
> > > > + * To ensure coherency users must call dma_buf_begin_cpu_access() and
> > > > + * dma_buf_end_cpu_access() around any cpu access performed through this
> > > > + * mapping.
> > > >     *
> > > >     * Returns 0 on success, or a negative errno code otherwise.
> > > >     */
> > > > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > > > index cf72699cb2bc..7eca37c8b10c 100644
> > > > --- a/include/linux/dma-buf.h
> > > > +++ b/include/linux/dma-buf.h
> > > > @@ -183,24 +183,19 @@ struct dma_buf_ops {
> > > >    	 * @begin_cpu_access:
> > > >    	 *
> > > >    	 * This is called from dma_buf_begin_cpu_access() and allows the
> > > > -	 * exporter to ensure that the memory is actually available for cpu
> > > > -	 * access - the exporter might need to allocate or swap-in and pin the
> > > > -	 * backing storage. The exporter also needs to ensure that cpu access is
> > > > -	 * coherent for the access direction. The direction can be used by the
> > > > -	 * exporter to optimize the cache flushing, i.e. access with a different
> > > > +	 * exporter to ensure that the memory is actually coherent for cpu
> > > > +	 * access. The exporter also needs to ensure that cpu access is coherent
> > > > +	 * for the access direction. The direction can be used by the exporter
> > > > +	 * to optimize the cache flushing, i.e. access with a different
> > > >    	 * direction (read instead of write) might return stale or even bogus
> > > >    	 * data (e.g. when the exporter needs to copy the data to temporary
> > > >    	 * storage).
> > > >    	 *
> > > > -	 * This callback is optional.
> > > > +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
> > > > +	 * command for userspace mappings established through @mmap, and also
> > > > +	 * for kernel mappings established with @vmap.
> > > >    	 *
> > > > -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
> > > > -	 * from userspace (where storage shouldn't be pinned to avoid handing
> > > > -	 * de-factor mlock rights to userspace) and for the kernel-internal
> > > > -	 * users of the various kmap interfaces, where the backing storage must
> > > > -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
> > > > -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
> > > > -	 * real problem.
> > > > +	 * This callback is optional.
> > > >    	 *
> > > >    	 * Returns:
> > > >    	 *
> > > > @@ -216,9 +211,7 @@ struct dma_buf_ops {
> > > >    	 *
> > > >    	 * This is called from dma_buf_end_cpu_access() when the importer is
> > > >    	 * done accessing the CPU. The exporter can use this to flush caches and
> > > > -	 * unpin any resources pinned in @begin_cpu_access.
> > > > -	 * The result of any dma_buf kmap calls after end_cpu_access is
> > > > -	 * undefined.
> > > > +	 * undo anything else done in @begin_cpu_access.
> > > >    	 *
> > > >    	 * This callback is optional.
> > > >    	 *
>