<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body>
<p style="font-family:Arial;font-size:11pt;color:#0078D7;margin:5pt;" align="Left">
[AMD Official Use Only - Internal Distribution Only]<br>
</p>
<br>
<div>
<div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; ">
hi,Christian,<br>
</div>
<div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; ">
<br>
</div>
<div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; ">
Good idea,thank you for these efforts.<br>
</div>
<div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; ">
I will update in next version. <br>
<br>
</div>
<div dir="auto" style="direction: ltr; margin: 0px; padding: 0px; font-family: sans-serif; font-size: 11pt; color: black; text-align: left;">
Jack<br>
</div>
<div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; ">
<br>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Koenig, Christian <Christian.Koenig@amd.com><br>
<b>Sent:</b> Thursday, March 11, 2021 6:41:05 PM<br>
<b>To:</b> Zhang, Jack (Jian) <Jack.Zhang1@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com>; Liu, Monk <Monk.Liu@amd.com>; Deng, Emily <Emily.Deng@amd.com><br>
<b>Subject:</b> Re: [PATCH v7] drm/amd/amdgpu implement tdr advanced mode</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText"><br>
<br>
Am 11.03.21 um 06:58 schrieb Jack Zhang:<br>
> [Why]<br>
> Previous tdr design treats the first job in job_timeout as the bad job.<br>
> But sometimes a later bad compute job can block a good gfx job and<br>
> cause an unexpected gfx job timeout because gfx and compute ring share<br>
> internal GC HW mutually.<br>
><br>
> [How]<br>
> This patch implements an advanced tdr mode.It involves an additinal<br>
> synchronous pre-resubmit step(Step0 Resubmit) before normal resubmit<br>
> step in order to find the real bad job.<br>
><br>
> 1. At Step0 Resubmit stage, it synchronously submits and pends for the<br>
> first job being signaled. If it gets timeout, we identify it as guilty<br>
> and do hw reset. After that, we would do the normal resubmit step to<br>
> resubmit left jobs.<br>
><br>
> 2. For whole gpu reset(vram lost), do resubmit as the old way.<br>
><br>
> Signed-off-by: Jack Zhang <Jack.Zhang1@amd.com><br>
> ---<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  63 ++++++++++++<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |   2 +-<br>
>   drivers/gpu/drm/scheduler/sched_main.c     | 107 +++++++++++++++------<br>
>   include/drm/gpu_scheduler.h                |   3 +<br>
>   4 files changed, 142 insertions(+), 33 deletions(-)<br>
><br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
> index e247c3a2ec08..5b182ade26ad 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
> @@ -4639,6 +4639,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,<br>
>        int i, r = 0;<br>
>        bool need_emergency_restart = false;<br>
>        bool audio_suspended = false;<br>
> +     int tmp_vram_lost_counter;<br>
>   <br>
>        /*<br>
>         * Special case: RAS triggered and full reset isn't supported<br>
> @@ -4788,6 +4789,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,<br>
>                }<br>
>        }<br>
>   <br>
> +     tmp_vram_lost_counter = atomic_read(&((adev)->vram_lost_counter));<br>
>        /* Actual ASIC resets if needed.*/<br>
>        /* TODO Implement XGMI hive reset logic for SRIOV */<br>
>        if (amdgpu_sriov_vf(adev)) {<br>
> @@ -4805,6 +4807,67 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,<br>
>        /* Post ASIC reset for all devs .*/<br>
>        list_for_each_entry(tmp_adev, device_list_handle, gmc.xgmi.head) {<br>
>   <br>
> +             if (amdgpu_gpu_recovery == 2 &&<br>
> +                     !(tmp_vram_lost_counter < atomic_read(&adev->vram_lost_counter))) {<br>
> +<br>
> +                     for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {<br>
<br>
Starting from here I would put this into a separate helper function in <br>
amdgpu_ring.c.<br>
<br>
You should also probably use checkpatch.pl once more since a couple of <br>
lines below might result in warnings.<br>
<br>
Christian.<br>
<br>
> +                             struct amdgpu_ring *ring = tmp_adev->rings[i];<br>
> +                             int ret = 0;<br>
> +                             struct drm_sched_job *s_job;<br>
> +<br>
> +                             if (!ring || !ring->sched.thread)<br>
> +                                     continue;<br>
> +<br>
> +                             /* No point to resubmit jobs if we didn't HW reset*/<br>
> +                             if (!tmp_adev->asic_reset_res && !job_signaled) {<br>
> +<br>
> +                                     s_job = list_first_entry_or_null(&ring->sched.pending_list, struct drm_sched_job, list);<br>
> +                                     if (s_job == NULL)<br>
> +                                             continue;<br>
> +<br>
> +                                     /* clear job's guilty and depend the folowing step to decide the real one */<br>
> +                                     drm_sched_reset_karma(s_job);<br>
> +                                     drm_sched_resubmit_jobs_ext(&ring->sched, 1);<br>
> +                                     ret = dma_fence_wait_timeout(s_job->s_fence->parent, false, ring->sched.timeout);<br>
> +<br>
> +                                     if (ret == 0) { /* timeout */<br>
> +                                             DRM_ERROR("Found the real bad job! ring:%s, job_id:%llx\n", ring->sched.name, s_job->id);<br>
> +                                             /* set guilty */<br>
> +                                             drm_sched_increase_karma(s_job);<br>
> +<br>
> +                                             /* do hw reset */<br>
> +                                             if (amdgpu_sriov_vf(adev)) {<br>
> +                                                     amdgpu_virt_fini_data_exchange(adev);<br>
> +                                                     r = amdgpu_device_reset_sriov(adev, false);<br>
> +                                                     if (r)<br>
> +                                                             adev->asic_reset_res = r;<br>
> +                                             } else {<br>
> +                                                     r  = amdgpu_do_asic_reset(hive, device_list_handle, &need_full_reset, false);<br>
> +                                                     if (r && r == -EAGAIN)<br>
> +                                                             goto retry;<br>
> +                                             }<br>
> +<br>
> +                                             /* add reset counter so that the following resubmitted job could flush vmid */<br>
> +                                             atomic_inc(&tmp_adev->gpu_reset_counter);<br>
> +                                             continue;<br>
> +                                     }<br>
> +<br>
> +                                     /* got the hw fence, signal finished fence */<br>
> +                                     atomic_dec(&ring->sched.num_jobs);<br>
> +                                     dma_fence_get(&s_job->s_fence->finished);<br>
> +                                     dma_fence_signal(&s_job->s_fence->finished);<br>
> +                                     dma_fence_put(&s_job->s_fence->finished);<br>
> +<br>
> +<br>
> +                                     /* remove node from list and free the job */<br>
> +                                     spin_lock(&ring->sched.job_list_lock);<br>
> +                                     list_del_init(&s_job->node);<br>
> +                                     spin_unlock(&ring->sched.job_list_lock);<br>
> +                                     ring->sched.ops->free_job(s_job);<br>
> +                             }<br>
> +                     }<br>
> +             }<br>
> +<br>
>                for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {<br>
>                        struct amdgpu_ring *ring = tmp_adev->rings[i];<br>
>   <br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c<br>
> index 865f924772b0..9c3f4edb7532 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c<br>
> @@ -509,7 +509,7 @@ module_param_named(compute_multipipe, amdgpu_compute_multipipe, int, 0444);<br>
>    * DOC: gpu_recovery (int)<br>
>    * Set to enable GPU recovery mechanism (1 = enable, 0 = disable). The default is -1 (auto, disabled except SRIOV).<br>
>    */<br>
> -MODULE_PARM_DESC(gpu_recovery, "Enable GPU recovery mechanism, (1 = enable, 0 = disable, -1 = auto)");<br>
> +MODULE_PARM_DESC(gpu_recovery, "Enable GPU recovery mechanism, (2 = advanced tdr mode, 1 = enable, 0 = disable, -1 = auto)");<br>
>   module_param_named(gpu_recovery, amdgpu_gpu_recovery, int, 0444);<br>
>   <br>
>   /**<br>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c<br>
> index d82a7ebf6099..bb0a129d1f40 100644<br>
> --- a/drivers/gpu/drm/scheduler/sched_main.c<br>
> +++ b/drivers/gpu/drm/scheduler/sched_main.c<br>
> @@ -361,40 +361,16 @@ static void drm_sched_job_timedout(struct work_struct *work)<br>
>     */<br>
>   void drm_sched_increase_karma(struct drm_sched_job *bad)<br>
>   {<br>
> -     int i;<br>
> -     struct drm_sched_entity *tmp;<br>
> -     struct drm_sched_entity *entity;<br>
> -     struct drm_gpu_scheduler *sched = bad->sched;<br>
> -<br>
> -     /* don't increase @bad's karma if it's from KERNEL RQ,<br>
> -      * because sometimes GPU hang would cause kernel jobs (like VM updating jobs)<br>
> -      * corrupt but keep in mind that kernel jobs always considered good.<br>
> -      */<br>
> -     if (bad->s_priority != DRM_SCHED_PRIORITY_KERNEL) {<br>
> -             atomic_inc(&bad->karma);<br>
> -             for (i = DRM_SCHED_PRIORITY_MIN; i < DRM_SCHED_PRIORITY_KERNEL;<br>
> -                  i++) {<br>
> -                     struct drm_sched_rq *rq = &sched->sched_rq[i];<br>
> -<br>
> -                     spin_lock(&rq->lock);<br>
> -                     list_for_each_entry_safe(entity, tmp, &rq->entities, list) {<br>
> -                             if (bad->s_fence->scheduled.context ==<br>
> -                                 entity->fence_context) {<br>
> -                                     if (atomic_read(&bad->karma) ><br>
> -                                         bad->sched->hang_limit)<br>
> -                                             if (entity->guilty)<br>
> -                                                     atomic_set(entity->guilty, 1);<br>
> -                                     break;<br>
> -                             }<br>
> -                     }<br>
> -                     spin_unlock(&rq->lock);<br>
> -                     if (&entity->list != &rq->entities)<br>
> -                             break;<br>
> -             }<br>
> -     }<br>
> +     drm_sched_increase_karma_ext(bad, 1);<br>
>   }<br>
>   EXPORT_SYMBOL(drm_sched_increase_karma);<br>
>   <br>
> +void drm_sched_reset_karma(struct drm_sched_job *bad)<br>
> +{<br>
> +     drm_sched_increase_karma_ext(bad, 0);<br>
> +}<br>
> +EXPORT_SYMBOL(drm_sched_reset_karma);<br>
> +<br>
>   /**<br>
>    * drm_sched_stop - stop the scheduler<br>
>    *<br>
> @@ -533,15 +509,32 @@ EXPORT_SYMBOL(drm_sched_start);<br>
>    *<br>
>    */<br>
>   void drm_sched_resubmit_jobs(struct drm_gpu_scheduler *sched)<br>
> +{<br>
> +     drm_sched_resubmit_jobs_ext(sched, INT_MAX);<br>
> +}<br>
> +EXPORT_SYMBOL(drm_sched_resubmit_jobs);<br>
> +<br>
> +/**<br>
> + * drm_sched_resubmit_jobs_ext - helper to relunch certain number of jobs from mirror ring list<br>
> + *<br>
> + * @sched: scheduler instance<br>
> + * @max: job numbers to relaunch<br>
> + *<br>
> + */<br>
> +void drm_sched_resubmit_jobs_ext(struct drm_gpu_scheduler *sched, int max)<br>
>   {<br>
>        struct drm_sched_job *s_job, *tmp;<br>
>        uint64_t guilty_context;<br>
>        bool found_guilty = false;<br>
>        struct dma_fence *fence;<br>
> +     int i = 0;<br>
>   <br>
>        list_for_each_entry_safe(s_job, tmp, &sched->pending_list, list) {<br>
>                struct drm_sched_fence *s_fence = s_job->s_fence;<br>
>   <br>
> +             if (i >= max)<br>
> +                     break;<br>
> +<br>
>                if (!found_guilty && atomic_read(&s_job->karma) > sched->hang_limit) {<br>
>                        found_guilty = true;<br>
>                        guilty_context = s_job->s_fence->scheduled.context;<br>
> @@ -552,6 +545,7 @@ void drm_sched_resubmit_jobs(struct drm_gpu_scheduler *sched)<br>
>   <br>
>                dma_fence_put(s_job->s_fence->parent);<br>
>                fence = sched->ops->run_job(s_job);<br>
> +             i++;<br>
>   <br>
>                if (IS_ERR_OR_NULL(fence)) {<br>
>                        if (IS_ERR(fence))<br>
> @@ -563,7 +557,7 @@ void drm_sched_resubmit_jobs(struct drm_gpu_scheduler *sched)<br>
>                }<br>
>        }<br>
>   }<br>
> -EXPORT_SYMBOL(drm_sched_resubmit_jobs);<br>
> +EXPORT_SYMBOL(drm_sched_resubmit_jobs_ext);<br>
>   <br>
>   /**<br>
>    * drm_sched_job_init - init a scheduler job<br>
> @@ -903,3 +897,52 @@ void drm_sched_fini(struct drm_gpu_scheduler *sched)<br>
>        sched->ready = false;<br>
>   }<br>
>   EXPORT_SYMBOL(drm_sched_fini);<br>
> +<br>
> +/**<br>
> + * drm_sched_increase_karma_ext - Update sched_entity guilty flag<br>
> + *<br>
> + * @bad: The job guilty of time out<br>
> + * @type: type for increase/reset karma<br>
> + *<br>
> + */<br>
> +void drm_sched_increase_karma_ext(struct drm_sched_job *bad, int type)<br>
> +{<br>
> +     int i;<br>
> +     struct drm_sched_entity *tmp;<br>
> +     struct drm_sched_entity *entity;<br>
> +     struct drm_gpu_scheduler *sched = bad->sched;<br>
> +<br>
> +     /* don't change @bad's karma if it's from KERNEL RQ,<br>
> +      * because sometimes GPU hang would cause kernel jobs (like VM updating jobs)<br>
> +      * corrupt but keep in mind that kernel jobs always considered good.<br>
> +      */<br>
> +     if (bad->s_priority != DRM_SCHED_PRIORITY_KERNEL) {<br>
> +             if (type == 0)<br>
> +                     atomic_set(&bad->karma, 0);<br>
> +             else if (type == 1)<br>
> +                     atomic_inc(&bad->karma);<br>
> +<br>
> +             for (i = DRM_SCHED_PRIORITY_MIN; i < DRM_SCHED_PRIORITY_KERNEL;<br>
> +                  i++) {<br>
> +                     struct drm_sched_rq *rq = &sched->sched_rq[i];<br>
> +<br>
> +                     spin_lock(&rq->lock);<br>
> +                     list_for_each_entry_safe(entity, tmp, &rq->entities, list) {<br>
> +                             if (bad->s_fence->scheduled.context ==<br>
> +                                 entity->fence_context) {<br>
> +                                     if (entity->guilty) {<br>
> +                                             if (type == 0)<br>
> +                                                     atomic_set(entity->guilty, 0);<br>
> +                                             else if (type == 1)<br>
> +                                                     atomic_set(entity->guilty, 1);<br>
> +                                             }<br>
> +                                     break;<br>
> +                             }<br>
> +                     }<br>
> +                     spin_unlock(&rq->lock);<br>
> +                     if (&entity->list != &rq->entities)<br>
> +                             break;<br>
> +             }<br>
> +     }<br>
> +}<br>
> +EXPORT_SYMBOL(drm_sched_increase_karma_ext);<br>
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h<br>
> index 1c815e0a14ed..4ea8606d91fe 100644<br>
> --- a/include/drm/gpu_scheduler.h<br>
> +++ b/include/drm/gpu_scheduler.h<br>
> @@ -321,7 +321,10 @@ void drm_sched_wakeup(struct drm_gpu_scheduler *sched);<br>
>   void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job *bad);<br>
>   void drm_sched_start(struct drm_gpu_scheduler *sched, bool full_recovery);<br>
>   void drm_sched_resubmit_jobs(struct drm_gpu_scheduler *sched);<br>
> +void drm_sched_resubmit_jobs_ext(struct drm_gpu_scheduler *sched, int max);<br>
>   void drm_sched_increase_karma(struct drm_sched_job *bad);<br>
> +void drm_sched_reset_karma(struct drm_sched_job *bad);<br>
> +void drm_sched_increase_karma_ext(struct drm_sched_job *bad, int type);<br>
>   bool drm_sched_dependency_optimized(struct dma_fence* fence,<br>
>                                    struct drm_sched_entity *entity);<br>
>   void drm_sched_fault(struct drm_gpu_scheduler *sched);<br>
<br>
</div>
</span></font></div>
</div>
</body>
</html>