| Message ID | 20220406075132.3263-6-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/16] dma-buf/drivers: make reserving a shared slot mandatory v4 | expand |
On Wed, Apr 06, 2022 at 09:51:21AM +0200, Christian König wrote: > Add an usage for kernel submissions. Waiting for those are mandatory for > dynamic DMA-bufs. > > As a precaution this patch also changes all occurrences where fences are > added as part of memory management in TTM, VMWGFX and i915 to use the > new value because it now becomes possible for drivers to ignore fences > with the WRITE usage. > > v2: use "must" in documentation, fix whitespaces > v3: separate out some driver changes and better document why some > changes should still be part of this patch. > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/dma-buf/dma-resv.c | 2 +- > drivers/dma-buf/st-dma-resv.c | 2 +- > drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 2 +- > drivers/gpu/drm/ttm/ttm_bo.c | 2 +- > drivers/gpu/drm/ttm/ttm_bo_util.c | 4 ++-- > drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 2 +- > include/linux/dma-resv.h | 24 ++++++++++++++++++--- > 7 files changed, 28 insertions(+), 10 deletions(-) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 378d47e1cfea..f4860e5f2d8b 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -726,7 +726,7 @@ EXPORT_SYMBOL_GPL(dma_resv_test_signaled); > */ > void dma_resv_describe(struct dma_resv *obj, struct seq_file *seq) > { > - static const char *usage[] = { "write", "read" }; > + static const char *usage[] = { "kernel", "write", "read" }; > struct dma_resv_iter cursor; > struct dma_fence *fence; > > diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c > index d0f7c2bfd4f0..062b57d63fa6 100644 > --- a/drivers/dma-buf/st-dma-resv.c > +++ b/drivers/dma-buf/st-dma-resv.c > @@ -296,7 +296,7 @@ int dma_resv(void) > int r; > > spin_lock_init(&fence_lock); > - for (usage = DMA_RESV_USAGE_WRITE; usage <= DMA_RESV_USAGE_READ; > + for (usage = DMA_RESV_USAGE_KERNEL; usage <= DMA_RESV_USAGE_READ; > ++usage) { > r = subtests(tests, (void *)(unsigned long)usage); > if (r) > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c > index f5f2b8b115ea..0512afdd20d8 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c > @@ -117,7 +117,7 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, > i915_fence_timeout(i915), > I915_FENCE_GFP); > dma_resv_add_fence(obj->base.resv, &clflush->base.dma, > - DMA_RESV_USAGE_WRITE); > + DMA_RESV_USAGE_KERNEL); > dma_fence_work_commit(&clflush->base); > /* > * We must have successfully populated the pages(since we are > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index d74f9eea855e..6bf3fb1c8045 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -739,7 +739,7 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo, > return ret; > } > > - dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_WRITE); > + dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_KERNEL); > > ret = dma_resv_reserve_fences(bo->base.resv, 1); > if (unlikely(ret)) { > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c > index 7a96a1db13a7..99deb45894f4 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c > @@ -508,7 +508,7 @@ static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo, > return ret; > > dma_resv_add_fence(&ghost_obj->base._resv, fence, > - DMA_RESV_USAGE_WRITE); > + DMA_RESV_USAGE_KERNEL); > > /** > * If we're not moving to fixed memory, the TTM object > @@ -562,7 +562,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo, > struct ttm_resource_manager *man = ttm_manager_type(bdev, new_mem->mem_type); > int ret = 0; > > - dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_WRITE); > + dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_KERNEL); > if (!evict) > ret = ttm_bo_move_to_ghost(bo, fence, man->use_tt); > else if (!from->use_tt && pipeline) > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > index bec50223efe5..408ede1f967f 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > @@ -759,7 +759,7 @@ void vmw_bo_fence_single(struct ttm_buffer_object *bo, > ret = dma_resv_reserve_fences(bo->base.resv, 1); > if (!ret) > dma_resv_add_fence(bo->base.resv, &fence->base, > - DMA_RESV_USAGE_WRITE); > + DMA_RESV_USAGE_KERNEL); > else > /* Last resort fallback when we are OOM */ > dma_fence_wait(&fence->base, false); > diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h > index 7bb7e7edbb6f..a749f229ae91 100644 > --- a/include/linux/dma-resv.h > +++ b/include/linux/dma-resv.h > @@ -55,11 +55,29 @@ struct dma_resv_list; > * This enum describes the different use cases for a dma_resv object and > * controls which fences are returned when queried. > * > - * An important fact is that there is the order WRITE<READ and when the > - * dma_resv object is asked for fences for one use case the fences for the > - * lower use case are returned as well. > + * An important fact is that there is the order KERNEL<WRITE<READ and > + * when the dma_resv object is asked for fences for one use case the fences > + * for the lower use case are returned as well. > + * > + * For example when asking for WRITE fences then the KERNEL fences are returned > + * as well. Similar when asked for READ fences then both WRITE and KERNEL > + * fences are returned as well. > */ > enum dma_resv_usage { > + /** > + * @DMA_RESV_USAGE_KERNEL: For in kernel memory management only. > + * > + * This should only be used for things like copying or clearing memory > + * with a DMA hardware engine for the purpose of kernel memory > + * management. > + * > + * Drivers *always* must wait for those fences before accessing the > + * resource protected by the dma_resv object. The only exception for > + * that is when the resource is known to be locked down in place by > + * pinning it previously. > + */ > + DMA_RESV_USAGE_KERNEL, > + > /** > * @DMA_RESV_USAGE_WRITE: Implicit write synchronization. > * > -- > 2.25.1 All the functional changes in drivers make sense to me now. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> >
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 378d47e1cfea..f4860e5f2d8b 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -726,7 +726,7 @@ EXPORT_SYMBOL_GPL(dma_resv_test_signaled); */ void dma_resv_describe(struct dma_resv *obj, struct seq_file *seq) { - static const char *usage[] = { "write", "read" }; + static const char *usage[] = { "kernel", "write", "read" }; struct dma_resv_iter cursor; struct dma_fence *fence; diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c index d0f7c2bfd4f0..062b57d63fa6 100644 --- a/drivers/dma-buf/st-dma-resv.c +++ b/drivers/dma-buf/st-dma-resv.c @@ -296,7 +296,7 @@ int dma_resv(void) int r; spin_lock_init(&fence_lock); - for (usage = DMA_RESV_USAGE_WRITE; usage <= DMA_RESV_USAGE_READ; + for (usage = DMA_RESV_USAGE_KERNEL; usage <= DMA_RESV_USAGE_READ; ++usage) { r = subtests(tests, (void *)(unsigned long)usage); if (r) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c index f5f2b8b115ea..0512afdd20d8 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c @@ -117,7 +117,7 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, i915_fence_timeout(i915), I915_FENCE_GFP); dma_resv_add_fence(obj->base.resv, &clflush->base.dma, - DMA_RESV_USAGE_WRITE); + DMA_RESV_USAGE_KERNEL); dma_fence_work_commit(&clflush->base); /* * We must have successfully populated the pages(since we are diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index d74f9eea855e..6bf3fb1c8045 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -739,7 +739,7 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo, return ret; } - dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_WRITE); + dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_KERNEL); ret = dma_resv_reserve_fences(bo->base.resv, 1); if (unlikely(ret)) { diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 7a96a1db13a7..99deb45894f4 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -508,7 +508,7 @@ static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo, return ret; dma_resv_add_fence(&ghost_obj->base._resv, fence, - DMA_RESV_USAGE_WRITE); + DMA_RESV_USAGE_KERNEL); /** * If we're not moving to fixed memory, the TTM object @@ -562,7 +562,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo, struct ttm_resource_manager *man = ttm_manager_type(bdev, new_mem->mem_type); int ret = 0; - dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_WRITE); + dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_KERNEL); if (!evict) ret = ttm_bo_move_to_ghost(bo, fence, man->use_tt); else if (!from->use_tt && pipeline) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index bec50223efe5..408ede1f967f 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -759,7 +759,7 @@ void vmw_bo_fence_single(struct ttm_buffer_object *bo, ret = dma_resv_reserve_fences(bo->base.resv, 1); if (!ret) dma_resv_add_fence(bo->base.resv, &fence->base, - DMA_RESV_USAGE_WRITE); + DMA_RESV_USAGE_KERNEL); else /* Last resort fallback when we are OOM */ dma_fence_wait(&fence->base, false); diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index 7bb7e7edbb6f..a749f229ae91 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -55,11 +55,29 @@ struct dma_resv_list; * This enum describes the different use cases for a dma_resv object and * controls which fences are returned when queried. * - * An important fact is that there is the order WRITE<READ and when the - * dma_resv object is asked for fences for one use case the fences for the - * lower use case are returned as well. + * An important fact is that there is the order KERNEL<WRITE<READ and + * when the dma_resv object is asked for fences for one use case the fences + * for the lower use case are returned as well. + * + * For example when asking for WRITE fences then the KERNEL fences are returned + * as well. Similar when asked for READ fences then both WRITE and KERNEL + * fences are returned as well. */ enum dma_resv_usage { + /** + * @DMA_RESV_USAGE_KERNEL: For in kernel memory management only. + * + * This should only be used for things like copying or clearing memory + * with a DMA hardware engine for the purpose of kernel memory + * management. + * + * Drivers *always* must wait for those fences before accessing the + * resource protected by the dma_resv object. The only exception for + * that is when the resource is known to be locked down in place by + * pinning it previously. + */ + DMA_RESV_USAGE_KERNEL, + /** * @DMA_RESV_USAGE_WRITE: Implicit write synchronization. *
Add an usage for kernel submissions. Waiting for those are mandatory for dynamic DMA-bufs. As a precaution this patch also changes all occurrences where fences are added as part of memory management in TTM, VMWGFX and i915 to use the new value because it now becomes possible for drivers to ignore fences with the WRITE usage. v2: use "must" in documentation, fix whitespaces v3: separate out some driver changes and better document why some changes should still be part of this patch. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/dma-buf/dma-resv.c | 2 +- drivers/dma-buf/st-dma-resv.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 2 +- drivers/gpu/drm/ttm/ttm_bo_util.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 2 +- include/linux/dma-resv.h | 24 ++++++++++++++++++--- 7 files changed, 28 insertions(+), 10 deletions(-)