[PATCH 092/459] drm/amdgpu: add ib preemption status in amdgpu_job

[Xiao, Jack]

In original patch, there is no deletion on job->preamble_status. 

Regards,
Jack

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken at gmail.com> 
Sent: Tuesday, June 18, 2019 4:04 PM
To: Alex Deucher <alexdeucher at gmail.com>; amd-gfx at lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Xiao, Jack <Jack.Xiao at amd.com>; Zhang, Hawking <Hawking.Zhang at amd.com>
Subject: Re: [PATCH 092/459] drm/amdgpu: add ib preemption status in amdgpu_job

Am 17.06.19 um 21:10 schrieb Alex Deucher:
> From: Jack Xiao <Jack.Xiao at amd.com>
>
> Add ib preemption status in amdgpu_job, so that ring level function 
> can detect preemption and program for resuming it.
>
> Acked-by: Alex Deucher <alexander.deucher at amd.com>
> Reviewed-by: Hawking Zhang <Hawking.Zhang at amd.com>
> Signed-off-by: Jack Xiao <Jack.Xiao at amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 +++
>   2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index e15531bfcd47..90cc0841b564 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -208,7 +208,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, 
> unsigned num_ibs,
>   
>   	skip_preamble = ring->current_ctx == fence_ctx;
>   	if (job && ring->funcs->emit_cntxcntl) {
> -		status |= job->preamble_status;
> +		status |= job->preemption_status;

That actually looks completely incorrect to me since it breaks the old status handling.

Christian.

>   		amdgpu_ring_emit_cntxcntl(ring, status);
>   	}
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> index e1b46a6703de..51e62504c279 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> @@ -29,6 +29,8 @@
>   #define AMDGPU_PREAMBLE_IB_PRESENT_FIRST    (1 << 1)
>   /* bit set means context switch occured */
>   #define AMDGPU_HAVE_CTX_SWITCH              (1 << 2)
> +/* bit set means IB is preempted */
> +#define AMDGPU_IB_PREEMPTED                 (1 << 3)
>   
>   #define to_amdgpu_job(sched_job)		\
>   		container_of((sched_job), struct amdgpu_job, base) @@ -45,6 +47,7 
> @@ struct amdgpu_job {
>   	struct amdgpu_ib	*ibs;
>   	struct dma_fence	*fence; /* the hw fence */
>   	uint32_t		preamble_status;
> +	uint32_t                preemption_status;
>   	uint32_t		num_ibs;
>   	void			*owner;
>   	bool                    vm_needs_flush;