Re: [PATCH v2] drm/amdgpu: Replace 'amdgpu_job_submit_direct' with 'drm_sched_entity' in cleaner shader

[Christian König <christian.koenig@xxxxxxx>]

Am 04.09.24 um 10:38 schrieb Srinivasan Shanmugam:
> This commit replaces the use of amdgpu_job_submit_direct which submits
> the job to the ring directly, with drm_sched_entity in the cleaner
> shader job submission process. The change allows the GPU scheduler to
> manage the cleaner shader job.
>
> - The job is then submitted to the GPU using the
>    drm_sched_entity_push_job function, which allows the GPU scheduler to
>    manage the job.
>
> This change improves the reliability of the cleaner shader job
> submission process by leveraging the capabilities of the GPU scheduler.
>
> Fixes: f70111466165 ("drm/amdgpu: Add sysfs interface for running cleaner shader")
> Cc: Christian König <christian.koenig@xxxxxxx>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
> Suggested-by: Christian König <christian.koenig@xxxxxxx>

Reviewed-by: Christian König <christian.koenig@xxxxxxx>

> ---
> v2:
>    - Dropped default assignment of f. (Christian)
>    - Moved drm_sched entity; to stack from amdgpu_gfx structure.
>      (Christian)
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 35 ++++++++++++++-----------
>   1 file changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index b779d47a546a..83e54697f0ee 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -1397,14 +1397,23 @@ static ssize_t amdgpu_gfx_get_available_compute_partition(struct device *dev,
>   static int amdgpu_gfx_run_cleaner_shader_job(struct amdgpu_ring *ring)
>   {
>   	struct amdgpu_device *adev = ring->adev;
> -	long timeout = msecs_to_jiffies(1000);
> -	struct dma_fence *f = NULL;
> +	struct drm_gpu_scheduler *sched = &ring->sched;
> +	struct drm_sched_entity entity;
> +	struct dma_fence *f;
>   	struct amdgpu_job *job;
>   	struct amdgpu_ib *ib;
>   	int i, r;
>   
> -	r = amdgpu_job_alloc_with_ib(adev, NULL, NULL,
> -				     64, AMDGPU_IB_POOL_DIRECT,
> +	/* Initialize the scheduler entity */
> +	r = drm_sched_entity_init(&entity, DRM_SCHED_PRIORITY_NORMAL,
> +				  &sched, 1, NULL);
> +	if (r) {
> +		dev_err(adev->dev, "Failed setting up GFX kernel entity.\n");
> +		goto err;
> +	}
> +
> +	r = amdgpu_job_alloc_with_ib(ring->adev, &entity, NULL,
> +				     64, 0,
>   				     &job);
>   	if (r)
>   		goto err;
> @@ -1416,24 +1425,18 @@ static int amdgpu_gfx_run_cleaner_shader_job(struct amdgpu_ring *ring)
>   		ib->ptr[i] = ring->funcs->nop;
>   	ib->length_dw = ring->funcs->align_mask + 1;
>   
> -	r = amdgpu_job_submit_direct(job, ring, &f);
> -	if (r)
> -		goto err_free;
> +	f = amdgpu_job_submit(job);
>   
> -	r = dma_fence_wait_timeout(f, false, timeout);
> -	if (r == 0)
> -		r = -ETIMEDOUT;
> -	else if (r > 0)
> -		r = 0;
> +	r = dma_fence_wait(f, false);
> +	if (r)
> +		goto err;
>   
> -	amdgpu_ib_free(adev, ib, f);
>   	dma_fence_put(f);
>   
> +	/* Clean up the scheduler entity */
> +	drm_sched_entity_destroy(&entity);
>   	return 0;
>   
> -err_free:
> -	amdgpu_job_free(job);
> -	amdgpu_ib_free(adev, ib, f);
>   err:
>   	return r;
>   }