diff mbox series

[08/13] drm/amdgpu: use scheduler depenencies for VM updates

Message ID 20221014084641.128280-9-christian.koenig@amd.com (mailing list archive)
State New, archived
Headers show
Series [01/13] drm/scheduler: fix fence ref counting | expand

Commit Message

Christian König Oct. 14, 2022, 8:46 a.m. UTC 
Instead of putting that into the job sync object.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c    | 56 +++++++++++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h    |  2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 10 +++-
 3 files changed, 52 insertions(+), 16 deletions(-)

Comments

Luben Tuikov Oct. 24, 2022, 5:50 a.m. UTC | #1 
"dependencies" in the title.

The rest looks good. I like pulling that sync free code into its own function.

Regards,
Luben

On 2022-10-14 04:46, Christian König wrote:
> Instead of putting that into the job sync object.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c    | 56 +++++++++++++++------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h    |  2 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 10 +++-
>  3 files changed, 52 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> index 090e66a1b284..bac7976975bd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> @@ -259,6 +259,14 @@ int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync *sync,
>  	return 0;
>  }
>  
> +/* Free the entry back to the slab */
> +static void amdgpu_sync_entry_free(struct amdgpu_sync_entry *e)
> +{
> +	hash_del(&e->node);
> +	dma_fence_put(e->fence);
> +	kmem_cache_free(amdgpu_sync_slab, e);
> +}
> +
>  /**
>   * amdgpu_sync_peek_fence - get the next fence not signaled yet
>   *
> @@ -280,9 +288,7 @@ struct dma_fence *amdgpu_sync_peek_fence(struct amdgpu_sync *sync,
>  		struct drm_sched_fence *s_fence = to_drm_sched_fence(f);
>  
>  		if (dma_fence_is_signaled(f)) {
> -			hash_del(&e->node);
> -			dma_fence_put(f);
> -			kmem_cache_free(amdgpu_sync_slab, e);
> +			amdgpu_sync_entry_free(e);
>  			continue;
>  		}
>  		if (ring && s_fence) {
> @@ -355,15 +361,42 @@ int amdgpu_sync_clone(struct amdgpu_sync *source, struct amdgpu_sync *clone)
>  			if (r)
>  				return r;
>  		} else {
> -			hash_del(&e->node);
> -			dma_fence_put(f);
> -			kmem_cache_free(amdgpu_sync_slab, e);
> +			amdgpu_sync_entry_free(e);
>  		}
>  	}
>  
>  	return 0;
>  }
>  
> +/**
> + * amdgpu_sync_push_to_job - push fences into job
> + * @sync: sync object to get the fences from
> + * @job: job to push the fences into
> + *
> + * Add all unsignaled fences from sync to job.
> + */
> +int amdgpu_sync_push_to_job(struct amdgpu_sync *sync, struct amdgpu_job *job)
> +{
> +	struct amdgpu_sync_entry *e;
> +	struct hlist_node *tmp;
> +	struct dma_fence *f;
> +	int i, r;
> +
> +	hash_for_each_safe(sync->fences, i, tmp, e, node) {
> +		f = e->fence;
> +		if (dma_fence_is_signaled(f)) {
> +			amdgpu_sync_entry_free(e);
> +			continue;
> +		}
> +
> +		dma_fence_get(f);
> +		r = drm_sched_job_add_dependency(&job->base, f);
> +		if (r)
> +			return r;
> +	}
> +	return 0;
> +}
> +
>  int amdgpu_sync_wait(struct amdgpu_sync *sync, bool intr)
>  {
>  	struct amdgpu_sync_entry *e;
> @@ -375,9 +408,7 @@ int amdgpu_sync_wait(struct amdgpu_sync *sync, bool intr)
>  		if (r)
>  			return r;
>  
> -		hash_del(&e->node);
> -		dma_fence_put(e->fence);
> -		kmem_cache_free(amdgpu_sync_slab, e);
> +		amdgpu_sync_entry_free(e);
>  	}
>  
>  	return 0;
> @@ -396,11 +427,8 @@ void amdgpu_sync_free(struct amdgpu_sync *sync)
>  	struct hlist_node *tmp;
>  	unsigned int i;
>  
> -	hash_for_each_safe(sync->fences, i, tmp, e, node) {
> -		hash_del(&e->node);
> -		dma_fence_put(e->fence);
> -		kmem_cache_free(amdgpu_sync_slab, e);
> -	}
> +	hash_for_each_safe(sync->fences, i, tmp, e, node)
> +		amdgpu_sync_entry_free(e);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> index 2d5c613cda10..cf1e9e858efd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> @@ -30,6 +30,7 @@ struct dma_fence;
>  struct dma_resv;
>  struct amdgpu_device;
>  struct amdgpu_ring;
> +struct amdgpu_job;
>  
>  enum amdgpu_sync_mode {
>  	AMDGPU_SYNC_ALWAYS,
> @@ -54,6 +55,7 @@ struct dma_fence *amdgpu_sync_peek_fence(struct amdgpu_sync *sync,
>  				     struct amdgpu_ring *ring);
>  struct dma_fence *amdgpu_sync_get_fence(struct amdgpu_sync *sync);
>  int amdgpu_sync_clone(struct amdgpu_sync *source, struct amdgpu_sync *clone);
> +int amdgpu_sync_push_to_job(struct amdgpu_sync *sync, struct amdgpu_job *job);
>  int amdgpu_sync_wait(struct amdgpu_sync *sync, bool intr);
>  void amdgpu_sync_free(struct amdgpu_sync *sync);
>  int amdgpu_sync_init(void);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> index 126364882d09..59cf64216fbb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> @@ -87,6 +87,7 @@ static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p,
>  				  struct dma_resv *resv,
>  				  enum amdgpu_sync_mode sync_mode)
>  {
> +	struct amdgpu_sync sync;
>  	int r;
>  
>  	r = amdgpu_vm_sdma_alloc_job(p, 0);
> @@ -96,7 +97,12 @@ static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p,
>  	if (!resv)
>  		return 0;
>  
> -	return amdgpu_sync_resv(p->adev, &p->job->sync, resv, sync_mode, p->vm);
> +	amdgpu_sync_create(&sync);
> +	r = amdgpu_sync_resv(p->adev, &sync, resv, sync_mode, p->vm);
> +	if (!r)
> +		r = amdgpu_sync_push_to_job(&sync, p->job);
> +	amdgpu_sync_free(&sync);
> +	return r;
>  }
>  
>  /**
> @@ -232,7 +238,7 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,
>  	/* Wait for PD/PT moves to be completed */
>  	dma_resv_iter_begin(&cursor, bo->tbo.base.resv, DMA_RESV_USAGE_KERNEL);
>  	dma_resv_for_each_fence_unlocked(&cursor, fence) {
> -		r = amdgpu_sync_fence(&p->job->sync, fence);
> +		r = drm_sched_job_add_dependency(&p->job->base, fence);
>  		if (r) {
>  			dma_resv_iter_end(&cursor);
>  			return r;
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
index 090e66a1b284..bac7976975bd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
@@ -259,6 +259,14 @@  int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync *sync,
 	return 0;
 }
 
+/* Free the entry back to the slab */
+static void amdgpu_sync_entry_free(struct amdgpu_sync_entry *e)
+{
+	hash_del(&e->node);
+	dma_fence_put(e->fence);
+	kmem_cache_free(amdgpu_sync_slab, e);
+}
+
 /**
  * amdgpu_sync_peek_fence - get the next fence not signaled yet
  *
@@ -280,9 +288,7 @@  struct dma_fence *amdgpu_sync_peek_fence(struct amdgpu_sync *sync,
 		struct drm_sched_fence *s_fence = to_drm_sched_fence(f);
 
 		if (dma_fence_is_signaled(f)) {
-			hash_del(&e->node);
-			dma_fence_put(f);
-			kmem_cache_free(amdgpu_sync_slab, e);
+			amdgpu_sync_entry_free(e);
 			continue;
 		}
 		if (ring && s_fence) {
@@ -355,15 +361,42 @@  int amdgpu_sync_clone(struct amdgpu_sync *source, struct amdgpu_sync *clone)
 			if (r)
 				return r;
 		} else {
-			hash_del(&e->node);
-			dma_fence_put(f);
-			kmem_cache_free(amdgpu_sync_slab, e);
+			amdgpu_sync_entry_free(e);
 		}
 	}
 
 	return 0;
 }
 
+/**
+ * amdgpu_sync_push_to_job - push fences into job
+ * @sync: sync object to get the fences from
+ * @job: job to push the fences into
+ *
+ * Add all unsignaled fences from sync to job.
+ */
+int amdgpu_sync_push_to_job(struct amdgpu_sync *sync, struct amdgpu_job *job)
+{
+	struct amdgpu_sync_entry *e;
+	struct hlist_node *tmp;
+	struct dma_fence *f;
+	int i, r;
+
+	hash_for_each_safe(sync->fences, i, tmp, e, node) {
+		f = e->fence;
+		if (dma_fence_is_signaled(f)) {
+			amdgpu_sync_entry_free(e);
+			continue;
+		}
+
+		dma_fence_get(f);
+		r = drm_sched_job_add_dependency(&job->base, f);
+		if (r)
+			return r;
+	}
+	return 0;
+}
+
 int amdgpu_sync_wait(struct amdgpu_sync *sync, bool intr)
 {
 	struct amdgpu_sync_entry *e;
@@ -375,9 +408,7 @@  int amdgpu_sync_wait(struct amdgpu_sync *sync, bool intr)
 		if (r)
 			return r;
 
-		hash_del(&e->node);
-		dma_fence_put(e->fence);
-		kmem_cache_free(amdgpu_sync_slab, e);
+		amdgpu_sync_entry_free(e);
 	}
 
 	return 0;
@@ -396,11 +427,8 @@  void amdgpu_sync_free(struct amdgpu_sync *sync)
 	struct hlist_node *tmp;
 	unsigned int i;
 
-	hash_for_each_safe(sync->fences, i, tmp, e, node) {
-		hash_del(&e->node);
-		dma_fence_put(e->fence);
-		kmem_cache_free(amdgpu_sync_slab, e);
-	}
+	hash_for_each_safe(sync->fences, i, tmp, e, node)
+		amdgpu_sync_entry_free(e);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
index 2d5c613cda10..cf1e9e858efd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
@@ -30,6 +30,7 @@  struct dma_fence;
 struct dma_resv;
 struct amdgpu_device;
 struct amdgpu_ring;
+struct amdgpu_job;
 
 enum amdgpu_sync_mode {
 	AMDGPU_SYNC_ALWAYS,
@@ -54,6 +55,7 @@  struct dma_fence *amdgpu_sync_peek_fence(struct amdgpu_sync *sync,
 				     struct amdgpu_ring *ring);
 struct dma_fence *amdgpu_sync_get_fence(struct amdgpu_sync *sync);
 int amdgpu_sync_clone(struct amdgpu_sync *source, struct amdgpu_sync *clone);
+int amdgpu_sync_push_to_job(struct amdgpu_sync *sync, struct amdgpu_job *job);
 int amdgpu_sync_wait(struct amdgpu_sync *sync, bool intr);
 void amdgpu_sync_free(struct amdgpu_sync *sync);
 int amdgpu_sync_init(void);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
index 126364882d09..59cf64216fbb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
@@ -87,6 +87,7 @@  static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p,
 				  struct dma_resv *resv,
 				  enum amdgpu_sync_mode sync_mode)
 {
+	struct amdgpu_sync sync;
 	int r;
 
 	r = amdgpu_vm_sdma_alloc_job(p, 0);
@@ -96,7 +97,12 @@  static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p,
 	if (!resv)
 		return 0;
 
-	return amdgpu_sync_resv(p->adev, &p->job->sync, resv, sync_mode, p->vm);
+	amdgpu_sync_create(&sync);
+	r = amdgpu_sync_resv(p->adev, &sync, resv, sync_mode, p->vm);
+	if (!r)
+		r = amdgpu_sync_push_to_job(&sync, p->job);
+	amdgpu_sync_free(&sync);
+	return r;
 }
 
 /**
@@ -232,7 +238,7 @@  static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,
 	/* Wait for PD/PT moves to be completed */
 	dma_resv_iter_begin(&cursor, bo->tbo.base.resv, DMA_RESV_USAGE_KERNEL);
 	dma_resv_for_each_fence_unlocked(&cursor, fence) {
-		r = amdgpu_sync_fence(&p->job->sync, fence);
+		r = drm_sched_job_add_dependency(&p->job->base, fence);
 		if (r) {
 			dma_resv_iter_end(&cursor);
 			return r;