[Public] Hi Andrey and Christian, Thanks for your comment, I will send out a new patch set later on after I verify it. Regards, Guchun -----Original Message----- From: Grodzovsky, Andrey <Andrey.Grodzovsky@xxxxxxx> Sent: Saturday, August 28, 2021 2:28 AM To: Koenig, Christian <Christian.Koenig@xxxxxxx>; Chen, Guchun <Guchun.Chen@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Zhang, Hawking <Hawking.Zhang@xxxxxxx>; mike@xxxxxxxxxxxxxx; Deucher, Alexander <Alexander.Deucher@xxxxxxx> Subject: Re: [PATCH] drm/amdgpu: stop scheduler when calling hw_fini I don't think it will start/stop twice because amdgpu_fence_driver_hw_fini/inint is not called during reset. I am worried about calling drm_sched_start without calling drm_sched_resubmit_job first since that the place where the jobs are actually restarted. Also calling drm_sched_start with false flag wrong here since it skips all the pending list handling. Andrey On 2021-08-27 7:34 a.m., Christian König wrote: > In general that looks good to me, but what could be is that we now try > to stop/start the scheduler during reset twice. > > Andrey what do you think? > > Christian. > > Am 27.08.21 um 12:40 schrieb Guchun Chen: >> This gurantees no more work on the ring can be submitted to hardware >> in suspend/resume case, otherwise the ring will not be empty before >> suspend. >> >> Suggested-by: Christian König <christian.koenig@xxxxxxx> >> Signed-off-by: Guchun Chen <guchun.chen@xxxxxxx> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c >> index b439eb7d4177..d6e429e63604 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c >> @@ -552,6 +552,9 @@ void amdgpu_fence_driver_hw_fini(struct >> amdgpu_device *adev) >> if (!ring || !ring->fence_drv.initialized) >> continue; >> + if (!ring->no_scheduler) >> + drm_sched_stop(&ring->sched, NULL); >> + >> /* You can't wait for HW to signal if it's gone */ >> if (!drm_dev_is_unplugged(&adev->ddev)) >> r = amdgpu_fence_wait_empty(ring); @@ -611,6 +614,9 @@ >> void amdgpu_fence_driver_hw_init(struct >> amdgpu_device *adev) >> if (!ring || !ring->fence_drv.initialized) >> continue; >> + if (!ring->no_scheduler) >> + drm_sched_start(&ring->sched, false); >> + >> /* enable the interrupt */ >> if (ring->fence_drv.irq_src) >> amdgpu_irq_get(adev, ring->fence_drv.irq_src, >
