| Message ID | 20201209142527.26415-4-tzimmermann@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm: Support short-term vmap via vmap_local | expand |
On Wed, Dec 09, 2020 at 03:25:22PM +0100, Thomas Zimmermann wrote: > The existing dma-buf calls dma_buf_vmap() and dma_buf_vunmap() are > allowed to pin the buffer or acquire the buffer's reservation object > lock. > > This is a problem for callers that only require a short-term mapping > of the buffer without the pinning, or callers that have special locking > requirements. These may suffer from unnecessary overhead or interfere > with regular pin operations. > > The new interfaces dma_buf_vmap_local(), dma_buf_vunmapo_local(), and > their rsp callbacks in struct dma_buf_ops provide an alternative without > pinning or reservation locking. Callers are responsible for these > operations. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/dma-buf/dma-buf.c | 80 +++++++++++++++++++++++++++++++++++++++ > include/linux/dma-buf.h | 34 +++++++++++++++++ > 2 files changed, 114 insertions(+) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index e63684d4cd90..be9f80190a66 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -1265,6 +1265,86 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map) > } > EXPORT_SYMBOL_GPL(dma_buf_vunmap); > > +/** > + * dma_buf_vmap_local - Create virtual mapping for the buffer object into kernel > + * address space. > + * @dmabuf: [in] buffer to vmap > + * @map: [out] returns the vmap pointer > + * > + * 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. Kmap is gone, so the entire 2 sentences here are no longer needed. Maybe mention something like "Unlike dma_buf_vmap() this is a short term mapping and will not pin the buffer. The struct dma_resv for the @dmabuf must be locked until dma_buf_vunmap_local() is called." > + * > + * Returns: > + * 0 on success, or a negative errno code otherwise. > + */ > +int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map) > +{ > + struct dma_buf_map ptr; > + int ret = 0; > + > + dma_buf_map_clear(map); > + > + if (WARN_ON(!dmabuf)) > + return -EINVAL; > + > + dma_resv_assert_held(dmabuf->resv); > + > + if (!dmabuf->ops->vmap_local) > + return -EINVAL; > + > + mutex_lock(&dmabuf->lock); > + if (dmabuf->vmapping_counter) { > + dmabuf->vmapping_counter++; > + BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr)); > + *map = dmabuf->vmap_ptr; > + goto out_unlock; > + } > + > + BUG_ON(dma_buf_map_is_set(&dmabuf->vmap_ptr)); > + > + ret = dmabuf->ops->vmap_local(dmabuf, &ptr); > + if (WARN_ON_ONCE(ret)) > + goto out_unlock; > + > + dmabuf->vmap_ptr = ptr; > + dmabuf->vmapping_counter = 1; > + > + *map = dmabuf->vmap_ptr; > + > +out_unlock: > + mutex_unlock(&dmabuf->lock); > + return ret; > +} > +EXPORT_SYMBOL_GPL(dma_buf_vmap_local); > + > +/** > + * dma_buf_vunmap_local - Unmap a vmap obtained by dma_buf_vmap_local. > + * @dmabuf: [in] buffer to vunmap > + * @map: [in] vmap pointer to vunmap Maybe for hyperlinking add "Release a mapping established with dma_buf_vmap_local()." > + */ > +void dma_buf_vunmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map) > +{ > + if (WARN_ON(!dmabuf)) > + return; > + > + dma_resv_assert_held(dmabuf->resv); > + > + BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr)); > + BUG_ON(dmabuf->vmapping_counter == 0); > + BUG_ON(!dma_buf_map_is_equal(&dmabuf->vmap_ptr, map)); > + > + mutex_lock(&dmabuf->lock); > + if (--dmabuf->vmapping_counter == 0) { > + if (dmabuf->ops->vunmap_local) > + dmabuf->ops->vunmap_local(dmabuf, map); > + dma_buf_map_clear(&dmabuf->vmap_ptr); > + } > + mutex_unlock(&dmabuf->lock); > +} > +EXPORT_SYMBOL_GPL(dma_buf_vunmap_local); > + > #ifdef CONFIG_DEBUG_FS > static int dma_buf_debug_show(struct seq_file *s, void *unused) > { > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > index cf72699cb2bc..f66580d23a9b 100644 > --- a/include/linux/dma-buf.h > +++ b/include/linux/dma-buf.h > @@ -269,6 +269,38 @@ struct dma_buf_ops { > > int (*vmap)(struct dma_buf *dmabuf, struct dma_buf_map *map); > void (*vunmap)(struct dma_buf *dmabuf, struct dma_buf_map *map); > + > + /** > + * @vmap_local: > + * > + * Creates a virtual mapping for the buffer into kernel address space. > + * > + * This callback establishes short-term mappings for situations where > + * callers only use the buffer for a bounded amount of time; such as > + * updates to the framebuffer or reading back contained information. > + * In contrast to the regular @vmap callback, vmap_local does never pin > + * the buffer to a specific domain or acquire the buffer's reservation > + * lock. > + * > + * This is called with the dmabuf->resv object locked. Callers must hold ^^Not the right kerneldoc, I think it should be &dma_buf.resv to get the hyperlink. > + * the lock until after removing the mapping with @vunmap_local. > + * > + * This callback is optional. > + * > + * Returns: > + * > + * 0 on success or a negative error code on failure. > + */ > + int (*vmap_local)(struct dma_buf *dmabuf, struct dma_buf_map *map); > + > + /** > + * @vunmap_local: > + * > + * Removes a virtual mapping that wa sestablished by @vmap_local. ^^established > + * > + * This callback is optional. > + */ > + void (*vunmap_local)(struct dma_buf *dmabuf, struct dma_buf_map *map); > }; > > /** > @@ -506,4 +538,6 @@ int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *, > unsigned long); > int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map); > void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map); > +int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map); > +void dma_buf_vunmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map); > #endif /* __DMA_BUF_H__ */ With the doc nits addressed: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > -- > 2.29.2 >
On Wed, Dec 09, 2020 at 03:25:22PM +0100, Thomas Zimmermann wrote: > The existing dma-buf calls dma_buf_vmap() and dma_buf_vunmap() are > allowed to pin the buffer or acquire the buffer's reservation object > lock. > > This is a problem for callers that only require a short-term mapping > of the buffer without the pinning, or callers that have special locking > requirements. These may suffer from unnecessary overhead or interfere > with regular pin operations. > > The new interfaces dma_buf_vmap_local(), dma_buf_vunmapo_local(), and > their rsp callbacks in struct dma_buf_ops provide an alternative without > pinning or reservation locking. Callers are responsible for these > operations. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/dma-buf/dma-buf.c | 80 +++++++++++++++++++++++++++++++++++++++ > include/linux/dma-buf.h | 34 +++++++++++++++++ > 2 files changed, 114 insertions(+) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index e63684d4cd90..be9f80190a66 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -1265,6 +1265,86 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map) > } > EXPORT_SYMBOL_GPL(dma_buf_vunmap); > > +/** > + * dma_buf_vmap_local - Create virtual mapping for the buffer object into kernel > + * address space. > + * @dmabuf: [in] buffer to vmap > + * @map: [out] returns the vmap pointer > + * > + * 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. We also need to specify whether callers need to call dma_buf_begin/end_cpu access around these or not. For current implementations it doesn't matter, but if you want to convert udl/gm12u320, it will. I think requiring an explicit call would be good, for more consistency with how normal vmap works. -Daniel > + * > + * Returns: > + * 0 on success, or a negative errno code otherwise. > + */ > +int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map) > +{ > + struct dma_buf_map ptr; > + int ret = 0; > + > + dma_buf_map_clear(map); > + > + if (WARN_ON(!dmabuf)) > + return -EINVAL; > + > + dma_resv_assert_held(dmabuf->resv); > + > + if (!dmabuf->ops->vmap_local) > + return -EINVAL; > + > + mutex_lock(&dmabuf->lock); > + if (dmabuf->vmapping_counter) { > + dmabuf->vmapping_counter++; > + BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr)); > + *map = dmabuf->vmap_ptr; > + goto out_unlock; > + } > + > + BUG_ON(dma_buf_map_is_set(&dmabuf->vmap_ptr)); > + > + ret = dmabuf->ops->vmap_local(dmabuf, &ptr); > + if (WARN_ON_ONCE(ret)) > + goto out_unlock; > + > + dmabuf->vmap_ptr = ptr; > + dmabuf->vmapping_counter = 1; > + > + *map = dmabuf->vmap_ptr; > + > +out_unlock: > + mutex_unlock(&dmabuf->lock); > + return ret; > +} > +EXPORT_SYMBOL_GPL(dma_buf_vmap_local); > + > +/** > + * dma_buf_vunmap_local - Unmap a vmap obtained by dma_buf_vmap_local. > + * @dmabuf: [in] buffer to vunmap > + * @map: [in] vmap pointer to vunmap > + */ > +void dma_buf_vunmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map) > +{ > + if (WARN_ON(!dmabuf)) > + return; > + > + dma_resv_assert_held(dmabuf->resv); > + > + BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr)); > + BUG_ON(dmabuf->vmapping_counter == 0); > + BUG_ON(!dma_buf_map_is_equal(&dmabuf->vmap_ptr, map)); > + > + mutex_lock(&dmabuf->lock); > + if (--dmabuf->vmapping_counter == 0) { > + if (dmabuf->ops->vunmap_local) > + dmabuf->ops->vunmap_local(dmabuf, map); > + dma_buf_map_clear(&dmabuf->vmap_ptr); > + } > + mutex_unlock(&dmabuf->lock); > +} > +EXPORT_SYMBOL_GPL(dma_buf_vunmap_local); > + > #ifdef CONFIG_DEBUG_FS > static int dma_buf_debug_show(struct seq_file *s, void *unused) > { > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > index cf72699cb2bc..f66580d23a9b 100644 > --- a/include/linux/dma-buf.h > +++ b/include/linux/dma-buf.h > @@ -269,6 +269,38 @@ struct dma_buf_ops { > > int (*vmap)(struct dma_buf *dmabuf, struct dma_buf_map *map); > void (*vunmap)(struct dma_buf *dmabuf, struct dma_buf_map *map); > + > + /** > + * @vmap_local: > + * > + * Creates a virtual mapping for the buffer into kernel address space. > + * > + * This callback establishes short-term mappings for situations where > + * callers only use the buffer for a bounded amount of time; such as > + * updates to the framebuffer or reading back contained information. > + * In contrast to the regular @vmap callback, vmap_local does never pin > + * the buffer to a specific domain or acquire the buffer's reservation > + * lock. > + * > + * This is called with the dmabuf->resv object locked. Callers must hold > + * the lock until after removing the mapping with @vunmap_local. > + * > + * This callback is optional. > + * > + * Returns: > + * > + * 0 on success or a negative error code on failure. > + */ > + int (*vmap_local)(struct dma_buf *dmabuf, struct dma_buf_map *map); > + > + /** > + * @vunmap_local: > + * > + * Removes a virtual mapping that wa sestablished by @vmap_local. > + * > + * This callback is optional. > + */ > + void (*vunmap_local)(struct dma_buf *dmabuf, struct dma_buf_map *map); > }; > > /** > @@ -506,4 +538,6 @@ int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *, > unsigned long); > int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map); > void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map); > +int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map); > +void dma_buf_vunmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map); > #endif /* __DMA_BUF_H__ */ > -- > 2.29.2 >
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index e63684d4cd90..be9f80190a66 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1265,6 +1265,86 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map) } EXPORT_SYMBOL_GPL(dma_buf_vunmap); +/** + * dma_buf_vmap_local - Create virtual mapping for the buffer object into kernel + * address space. + * @dmabuf: [in] buffer to vmap + * @map: [out] returns the vmap pointer + * + * 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. + * + * Returns: + * 0 on success, or a negative errno code otherwise. + */ +int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map) +{ + struct dma_buf_map ptr; + int ret = 0; + + dma_buf_map_clear(map); + + if (WARN_ON(!dmabuf)) + return -EINVAL; + + dma_resv_assert_held(dmabuf->resv); + + if (!dmabuf->ops->vmap_local) + return -EINVAL; + + mutex_lock(&dmabuf->lock); + if (dmabuf->vmapping_counter) { + dmabuf->vmapping_counter++; + BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr)); + *map = dmabuf->vmap_ptr; + goto out_unlock; + } + + BUG_ON(dma_buf_map_is_set(&dmabuf->vmap_ptr)); + + ret = dmabuf->ops->vmap_local(dmabuf, &ptr); + if (WARN_ON_ONCE(ret)) + goto out_unlock; + + dmabuf->vmap_ptr = ptr; + dmabuf->vmapping_counter = 1; + + *map = dmabuf->vmap_ptr; + +out_unlock: + mutex_unlock(&dmabuf->lock); + return ret; +} +EXPORT_SYMBOL_GPL(dma_buf_vmap_local); + +/** + * dma_buf_vunmap_local - Unmap a vmap obtained by dma_buf_vmap_local. + * @dmabuf: [in] buffer to vunmap + * @map: [in] vmap pointer to vunmap + */ +void dma_buf_vunmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map) +{ + if (WARN_ON(!dmabuf)) + return; + + dma_resv_assert_held(dmabuf->resv); + + BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr)); + BUG_ON(dmabuf->vmapping_counter == 0); + BUG_ON(!dma_buf_map_is_equal(&dmabuf->vmap_ptr, map)); + + mutex_lock(&dmabuf->lock); + if (--dmabuf->vmapping_counter == 0) { + if (dmabuf->ops->vunmap_local) + dmabuf->ops->vunmap_local(dmabuf, map); + dma_buf_map_clear(&dmabuf->vmap_ptr); + } + mutex_unlock(&dmabuf->lock); +} +EXPORT_SYMBOL_GPL(dma_buf_vunmap_local); + #ifdef CONFIG_DEBUG_FS static int dma_buf_debug_show(struct seq_file *s, void *unused) { diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index cf72699cb2bc..f66580d23a9b 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -269,6 +269,38 @@ struct dma_buf_ops { int (*vmap)(struct dma_buf *dmabuf, struct dma_buf_map *map); void (*vunmap)(struct dma_buf *dmabuf, struct dma_buf_map *map); + + /** + * @vmap_local: + * + * Creates a virtual mapping for the buffer into kernel address space. + * + * This callback establishes short-term mappings for situations where + * callers only use the buffer for a bounded amount of time; such as + * updates to the framebuffer or reading back contained information. + * In contrast to the regular @vmap callback, vmap_local does never pin + * the buffer to a specific domain or acquire the buffer's reservation + * lock. + * + * This is called with the dmabuf->resv object locked. Callers must hold + * the lock until after removing the mapping with @vunmap_local. + * + * This callback is optional. + * + * Returns: + * + * 0 on success or a negative error code on failure. + */ + int (*vmap_local)(struct dma_buf *dmabuf, struct dma_buf_map *map); + + /** + * @vunmap_local: + * + * Removes a virtual mapping that wa sestablished by @vmap_local. + * + * This callback is optional. + */ + void (*vunmap_local)(struct dma_buf *dmabuf, struct dma_buf_map *map); }; /** @@ -506,4 +538,6 @@ int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *, unsigned long); int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map); void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map); +int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map); +void dma_buf_vunmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map); #endif /* __DMA_BUF_H__ */
The existing dma-buf calls dma_buf_vmap() and dma_buf_vunmap() are allowed to pin the buffer or acquire the buffer's reservation object lock. This is a problem for callers that only require a short-term mapping of the buffer without the pinning, or callers that have special locking requirements. These may suffer from unnecessary overhead or interfere with regular pin operations. The new interfaces dma_buf_vmap_local(), dma_buf_vunmapo_local(), and their rsp callbacks in struct dma_buf_ops provide an alternative without pinning or reservation locking. Callers are responsible for these operations. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/dma-buf/dma-buf.c | 80 +++++++++++++++++++++++++++++++++++++++ include/linux/dma-buf.h | 34 +++++++++++++++++ 2 files changed, 114 insertions(+)