[Intel-gfx] [PATCH 02/15] dma-buf: specify usage while adding fences to dma_resv obj v7

2022-04-11 Thread Christian König 
Instead of distingting between shared and exclusive fences specify
the fence usage while adding fences.

Rework all drivers to use this interface instead and deprecate the old one.

v2: some kerneldoc comments suggested by Daniel
v3: fix a missing case in radeon
v4: rebase on nouveau changes, fix lockdep and temporary disable warning
v5: more documentation updates
v6: separate internal dma_resv changes from this patch, avoids to
disable warning temporary, rebase on upstream changes
v7: fix missed case in lima driver, minimize changes to i915_gem_busy_ioctl

Signed-off-by: Christian König 
---
 drivers/dma-buf/dma-resv.c|  48 +++--
 drivers/dma-buf/st-dma-resv.c | 101 +-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c|   4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c|   6 +-
 drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c  |  10 +-
 drivers/gpu/drm/i915/gem/i915_gem_busy.c  |   6 +-
 drivers/gpu/drm/i915/gem/i915_gem_clflush.c   |   3 +-
 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c  |   5 +-
 .../drm/i915/gem/selftests/i915_gem_migrate.c |   4 +-
 .../drm/i915/gem/selftests/i915_gem_mman.c|   3 +-
 drivers/gpu/drm/i915/i915_vma.c   |   8 +-
 .../drm/i915/selftests/intel_memory_region.c  |   3 +-
 drivers/gpu/drm/lima/lima_gem.c   |   7 +-
 drivers/gpu/drm/msm/msm_gem_submit.c  |   6 +-
 drivers/gpu/drm/nouveau/nouveau_bo.c  |   9 +-
 drivers/gpu/drm/nouveau/nouveau_fence.c   |   4 +-
 drivers/gpu/drm/panfrost/panfrost_job.c   |   2 +-
 drivers/gpu/drm/qxl/qxl_release.c |   3 +-
 drivers/gpu/drm/radeon/radeon_object.c|   6 +-
 drivers/gpu/drm/ttm/ttm_bo.c  |   2 +-
 drivers/gpu/drm/ttm/ttm_bo_util.c |   5 +-
 drivers/gpu/drm/ttm/ttm_execbuf_util.c|   6 +-
 drivers/gpu/drm/v3d/v3d_gem.c |   4 +-
 drivers/gpu/drm/vc4/vc4_gem.c |   2 +-
 drivers/gpu/drm/vgem/vgem_fence.c |   9 +-
 drivers/gpu/drm/virtio/virtgpu_gem.c  |   3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c|   3 +-
 include/linux/dma-buf.h   |  16 +--
 include/linux/dma-resv.h  |  25 +++--
 30 files changed, 149 insertions(+), 166 deletions(-)

diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 17237e6ee30c..543dae6566d2 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -234,14 +234,14 @@ EXPORT_SYMBOL(dma_resv_reserve_fences);
 
 #ifdef CONFIG_DEBUG_MUTEXES
 /**
- * dma_resv_reset_shared_max - reset shared fences for debugging
+ * dma_resv_reset_max_fences - reset shared fences for debugging
  * @obj: the dma_resv object to reset
  *
  * Reset the number of pre-reserved shared slots to test that drivers do
  * correct slot allocation using dma_resv_reserve_fences(). See also
  * _resv_list.shared_max.
  */
-void dma_resv_reset_shared_max(struct dma_resv *obj)
+void dma_resv_reset_max_fences(struct dma_resv *obj)
 {
struct dma_resv_list *fences = dma_resv_shared_list(obj);
 
@@ -251,7 +251,7 @@ void dma_resv_reset_shared_max(struct dma_resv *obj)
if (fences)
fences->shared_max = fences->shared_count;
 }
-EXPORT_SYMBOL(dma_resv_reset_shared_max);
+EXPORT_SYMBOL(dma_resv_reset_max_fences);
 #endif
 
 /**
@@ -264,7 +264,8 @@ EXPORT_SYMBOL(dma_resv_reset_shared_max);
  *
  * See also _resv.fence for a discussion of the semantics.
  */
-void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence)
+static void dma_resv_add_shared_fence(struct dma_resv *obj,
+ struct dma_fence *fence)
 {
struct dma_resv_list *fobj;
struct dma_fence *old;
@@ -305,13 +306,13 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, 
struct dma_fence *fence)
write_seqcount_end(>seq);
dma_fence_put(old);
 }
-EXPORT_SYMBOL(dma_resv_add_shared_fence);
 
 /**
  * dma_resv_replace_fences - replace fences in the dma_resv obj
  * @obj: the reservation object
  * @context: the context of the fences to replace
  * @replacement: the new fence to use instead
+ * @usage: how the new fence is used, see enum dma_resv_usage
  *
  * Replace fences with a specified context with a new fence. Only valid if the
  * operation represented by the original fence has no longer access to the
@@ -321,12 +322,16 @@ EXPORT_SYMBOL(dma_resv_add_shared_fence);
  * update fence which makes the resource inaccessible.
  */
 void dma_resv_replace_fences(struct dma_resv *obj, uint64_t context,
-struct dma_fence *replacement)
+struct dma_fence *replacement,
+enum dma_resv_usage usage)
 {
struct dma_resv_list *list;
struct dma_fence *old;
unsigned int i;
 
+   /* Only readers supported for now */
+   WARN_ON(usage

Re: [Intel-gfx] [PATCH 02/15] dma-buf: specify usage while adding fences to dma_resv obj v7

2022-04-07 Thread Javier Martinez Canillas 
On 4/7/22 15:13, Christian König wrote:
> Am 07.04.22 um 15:08 schrieb Javier Martinez Canillas:
>> Hello Christian,
>>
>> On 4/7/22 10:59, Christian König wrote:
>>> Instead of distingting between shared and exclusive fences specify
>>> the fence usage while adding fences.
>>>
>>> Rework all drivers to use this interface instead and deprecate the old one.
>>>
>> This patch broke compilation for the vc4 DRM driver.
> 
> My apologies for that. I've tried really hard to catch all cases, but 
> looks like I missed some.
> 

No worries, I know that's not easy to get all callers when doing these
subsystem wide changes.

>> I've this patch locally
>> which seems to work but I don't know enough about the fence API to know if
>> is correct.
>>
>> If you think is the proper fix then I can post it as a patch.
> 
> Yes, that patch looks absolutely correct to me.
>

Thanks for looking at it.
 
> Feel free to add an Reviewed-by: Christian König 
>  and CC me so that I can push it to 
> drm-misc-next ASAP.
> 

I can also do it after posting (just to get a proper Link: tag with dim)

Already have another set that wanted to push but found this issue after
doing a build test before pushing.

> Thanks,
> Christian.
> 
-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [Intel-gfx] [PATCH 02/15] dma-buf: specify usage while adding fences to dma_resv obj v7

2022-04-07 Thread Javier Martinez Canillas 
Hello Christian,

On 4/7/22 10:59, Christian König wrote:
> Instead of distingting between shared and exclusive fences specify
> the fence usage while adding fences.
> 
> Rework all drivers to use this interface instead and deprecate the old one.
> 

This patch broke compilation for the vc4 DRM driver. I've this patch locally
which seems to work but I don't know enough about the fence API to know if
is correct.

If you think is the proper fix then I can post it as a patch.

>From 3e96db4827ef69b38927476659cbb4469a0246e6 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas 
Date: Thu, 7 Apr 2022 14:54:07 +0200
Subject: [PATCH] drm/vc4: Use newer fence API to fix build error

The commit 73511edf8b19 ("dma-buf: specify usage while adding fences to
dma_resv obj v7") ported all the DRM drivers to use the newer fence API
that specifies the usage with the enum dma_resv_usage rather than doing
an explicit shared / exclusive distinction.

But the commit didn't do it properly in two callers of the vc4 driver,
leading to build errors.

Fixes: 73511edf8b19 ("dma-buf: specify usage while adding fences to dma_resv 
obj v7")
Signed-off-by: Javier Martinez Canillas 
---
 drivers/gpu/drm/vc4/vc4_gem.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 38550317e025..9eaf304fc20d 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -546,7 +546,8 @@ vc4_update_bo_seqnos(struct vc4_exec_info *exec, uint64_t 
seqno)
bo = to_vc4_bo(>bo[i]->base);
bo->seqno = seqno;
 
-   dma_resv_add_fence(bo->base.base.resv, exec->fence);
+   dma_resv_add_fence(bo->base.base.resv, exec->fence,
+  DMA_RESV_USAGE_READ);
}
 
list_for_each_entry(bo, >unref_list, unref_head) {
@@ -557,7 +558,8 @@ vc4_update_bo_seqnos(struct vc4_exec_info *exec, uint64_t 
seqno)
bo = to_vc4_bo(>rcl_write_bo[i]->base);
bo->write_seqno = seqno;
 
-   dma_resv_add_excl_fence(bo->base.base.resv, exec->fence);
+   dma_resv_add_fence(bo->base.base.resv, exec->fence,
+  DMA_RESV_USAGE_WRITE);
}
 }
 
-- 
2.35.1

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [Intel-gfx] [PATCH 02/15] dma-buf: specify usage while adding fences to dma_resv obj v7

2022-04-07 Thread Daniel Vetter 
On Thu, Apr 07, 2022 at 10:59:33AM +0200, Christian König wrote:
> Instead of distingting between shared and exclusive fences specify
> the fence usage while adding fences.
> 
> Rework all drivers to use this interface instead and deprecate the old one.
> 
> v2: some kerneldoc comments suggested by Daniel
> v3: fix a missing case in radeon
> v4: rebase on nouveau changes, fix lockdep and temporary disable warning
> v5: more documentation updates
> v6: separate internal dma_resv changes from this patch, avoids to
> disable warning temporary, rebase on upstream changes
> v7: fix missed case in lima driver, minimize changes to i915_gem_busy_ioctl

I've only done an incremental review of these two things, lgtm.

Reviewed-by: Daniel Vetter 

> 
> Signed-off-by: Christian König 
> ---
>  drivers/dma-buf/dma-resv.c|  48 +++--
>  drivers/dma-buf/st-dma-resv.c | 101 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c|   4 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c|   6 +-
>  drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c  |  10 +-
>  drivers/gpu/drm/i915/gem/i915_gem_busy.c  |   6 +-
>  drivers/gpu/drm/i915/gem/i915_gem_clflush.c   |   3 +-
>  drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c  |   5 +-
>  .../drm/i915/gem/selftests/i915_gem_migrate.c |   4 +-
>  .../drm/i915/gem/selftests/i915_gem_mman.c|   3 +-
>  drivers/gpu/drm/i915/i915_vma.c   |   8 +-
>  .../drm/i915/selftests/intel_memory_region.c  |   3 +-
>  drivers/gpu/drm/lima/lima_gem.c   |   7 +-
>  drivers/gpu/drm/msm/msm_gem_submit.c  |   6 +-
>  drivers/gpu/drm/nouveau/nouveau_bo.c  |   9 +-
>  drivers/gpu/drm/nouveau/nouveau_fence.c   |   4 +-
>  drivers/gpu/drm/panfrost/panfrost_job.c   |   2 +-
>  drivers/gpu/drm/qxl/qxl_release.c |   3 +-
>  drivers/gpu/drm/radeon/radeon_object.c|   6 +-
>  drivers/gpu/drm/ttm/ttm_bo.c  |   2 +-
>  drivers/gpu/drm/ttm/ttm_bo_util.c |   5 +-
>  drivers/gpu/drm/ttm/ttm_execbuf_util.c|   6 +-
>  drivers/gpu/drm/v3d/v3d_gem.c |   4 +-
>  drivers/gpu/drm/vc4/vc4_gem.c |   2 +-
>  drivers/gpu/drm/vgem/vgem_fence.c |   9 +-
>  drivers/gpu/drm/virtio/virtgpu_gem.c  |   3 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c|   3 +-
>  include/linux/dma-buf.h   |  16 +--
>  include/linux/dma-resv.h  |  25 +++--
>  30 files changed, 149 insertions(+), 166 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> index 17237e6ee30c..543dae6566d2 100644
> --- a/drivers/dma-buf/dma-resv.c
> +++ b/drivers/dma-buf/dma-resv.c
> @@ -234,14 +234,14 @@ EXPORT_SYMBOL(dma_resv_reserve_fences);
>  
>  #ifdef CONFIG_DEBUG_MUTEXES
>  /**
> - * dma_resv_reset_shared_max - reset shared fences for debugging
> + * dma_resv_reset_max_fences - reset shared fences for debugging
>   * @obj: the dma_resv object to reset
>   *
>   * Reset the number of pre-reserved shared slots to test that drivers do
>   * correct slot allocation using dma_resv_reserve_fences(). See also
>   * _resv_list.shared_max.
>   */
> -void dma_resv_reset_shared_max(struct dma_resv *obj)
> +void dma_resv_reset_max_fences(struct dma_resv *obj)
>  {
>   struct dma_resv_list *fences = dma_resv_shared_list(obj);
>  
> @@ -251,7 +251,7 @@ void dma_resv_reset_shared_max(struct dma_resv *obj)
>   if (fences)
>   fences->shared_max = fences->shared_count;
>  }
> -EXPORT_SYMBOL(dma_resv_reset_shared_max);
> +EXPORT_SYMBOL(dma_resv_reset_max_fences);
>  #endif
>  
>  /**
> @@ -264,7 +264,8 @@ EXPORT_SYMBOL(dma_resv_reset_shared_max);
>   *
>   * See also _resv.fence for a discussion of the semantics.
>   */
> -void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence)
> +static void dma_resv_add_shared_fence(struct dma_resv *obj,
> +   struct dma_fence *fence)
>  {
>   struct dma_resv_list *fobj;
>   struct dma_fence *old;
> @@ -305,13 +306,13 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, 
> struct dma_fence *fence)
>   write_seqcount_end(>seq);
>   dma_fence_put(old);
>  }
> -EXPORT_SYMBOL(dma_resv_add_shared_fence);
>  
>  /**
>   * dma_resv_replace_fences - replace fences in the dma_resv obj
>   * @obj: the reservation object
>   * @context: the context of the fences to replace
>   * @replacement: the new fence to use instead
> + * @usage: how the new fence is used, see enum dma_resv_usage
>   *
>   * Replace fences with a specified context with a new fence. Only valid if 
> the
>   * operation represented by the original fence has no longer access to the
> @@ -321,12 +322,16 @@ EXPORT_SYMBOL(dma_resv_add_shared_fence);
>   * update fence which makes the resource inaccessible.
>   */
>  void dma_resv_replace_fences(struct dma_resv