From 6c5403173a13a08ff61dbdafa4c0ed4a9dedbfe0 Mon Sep 17 00:00:00 2001 From: Christian König Date: Fri, 12 Mar 2021 09:34:39 +0100 Subject: drm/ttm: make ttm_bo_unpin more defensive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We seem to have some more driver bugs than thought. Signed-off-by: Christian König Fixes: deb0814b43f3 ("drm/ttm: add ttm_bo_pin()/ttm_bo_unpin() v2") Acked-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20210312093810.2202-1-christian.koenig@amd.com --- include/drm/ttm/ttm_bo_api.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'include/drm') diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index e17be324d95f..b8ca13664fa2 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -612,9 +612,11 @@ static inline void ttm_bo_pin(struct ttm_buffer_object *bo) static inline void ttm_bo_unpin(struct ttm_buffer_object *bo) { dma_resv_assert_held(bo->base.resv); - WARN_ON_ONCE(!bo->pin_count); WARN_ON_ONCE(!kref_read(&bo->kref)); - --bo->pin_count; + if (bo->pin_count) + --bo->pin_count; + else + WARN_ON_ONCE(true); } int ttm_mem_evict_first(struct ttm_bo_device *bdev, -- cgit v1.2.3 From be318fd85bf2c73c10850a6ce50a87e6f0068926 Mon Sep 17 00:00:00 2001 From: Christian König Date: Thu, 1 Apr 2021 14:50:15 +0200 Subject: drm/sched: add missing member documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just fix a warning. Signed-off-by: Christian König Reported-by: Stephen Rothwell Fixes: f2f12eb9c32b ("drm/scheduler: provide scheduler score externally") Reviewed-by: Alex Deucher Link: https://patchwork.freedesktop.org/patch/msgid/20210401125213.138855-1-christian.koenig@amd.com --- include/drm/gpu_scheduler.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include/drm') diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 1c815e0a14ed..f888b5e9583a 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -277,6 +277,7 @@ struct drm_sched_backend_ops { * @hang_limit: once the hangs by a job crosses this limit then it is marked * guilty and it will be considered for scheduling further. * @score: score to help loadbalancer pick a idle sched + * @_score: score used when the driver doesn't provide one * @ready: marks if the underlying HW is ready to work * @free_guilty: A hit to time out handler to free the guilty job. * -- cgit v1.2.3 From 45d969992c1893df42ccae064aba6f05dded67ee Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Fri, 26 Mar 2021 16:37:48 -0400 Subject: drm/dp: Fixup kernel docs for struct drm_dp_aux * Make sure that struct members are referred to using @, otherwise they won't be formatted as such * Make sure to refer to other struct types using & so they link back to each struct's definition * Make sure to precede constant values with % so they're formatted correctly Signed-off-by: Lyude Paul Acked-by: Randy Dunlap Link: https://patchwork.freedesktop.org/patch/msgid/20210326203807.105754-2-lyude@redhat.com --- include/drm/drm_dp_helper.h | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) (limited to 'include/drm') diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index edffd1dcca3e..ac63035b0226 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1839,34 +1839,34 @@ struct drm_dp_aux_cec { * @crc_count: counter of captured frame CRCs * @transfer: transfers a message representing a single AUX transaction * - * The .dev field should be set to a pointer to the device that implements - * the AUX channel. + * The @dev field should be set to a pointer to the device that implements the + * AUX channel. * - * The .name field may be used to specify the name of the I2C adapter. If set to - * NULL, dev_name() of .dev will be used. + * The @name field may be used to specify the name of the I2C adapter. If set to + * %NULL, dev_name() of @dev will be used. * - * Drivers provide a hardware-specific implementation of how transactions - * are executed via the .transfer() function. A pointer to a drm_dp_aux_msg + * Drivers provide a hardware-specific implementation of how transactions are + * executed via the @transfer() function. A pointer to a &drm_dp_aux_msg * structure describing the transaction is passed into this function. Upon - * success, the implementation should return the number of payload bytes - * that were transferred, or a negative error-code on failure. Helpers - * propagate errors from the .transfer() function, with the exception of - * the -EBUSY error, which causes a transaction to be retried. On a short, - * helpers will return -EPROTO to make it simpler to check for failure. + * success, the implementation should return the number of payload bytes that + * were transferred, or a negative error-code on failure. Helpers propagate + * errors from the @transfer() function, with the exception of the %-EBUSY + * error, which causes a transaction to be retried. On a short, helpers will + * return %-EPROTO to make it simpler to check for failure. * * An AUX channel can also be used to transport I2C messages to a sink. A - * typical application of that is to access an EDID that's present in the - * sink device. The .transfer() function can also be used to execute such - * transactions. The drm_dp_aux_register() function registers an I2C - * adapter that can be passed to drm_probe_ddc(). Upon removal, drivers - * should call drm_dp_aux_unregister() to remove the I2C adapter. - * The I2C adapter uses long transfers by default; if a partial response is - * received, the adapter will drop down to the size given by the partial - * response for this transaction only. + * typical application of that is to access an EDID that's present in the sink + * device. The @transfer() function can also be used to execute such + * transactions. The drm_dp_aux_register() function registers an I2C adapter + * that can be passed to drm_probe_ddc(). Upon removal, drivers should call + * drm_dp_aux_unregister() to remove the I2C adapter. The I2C adapter uses long + * transfers by default; if a partial response is received, the adapter will + * drop down to the size given by the partial response for this transaction + * only. * - * Note that the aux helper code assumes that the .transfer() function - * only modifies the reply field of the drm_dp_aux_msg structure. The - * retry logic and i2c helpers assume this is the case. + * Note that the aux helper code assumes that the @transfer() function only + * modifies the reply field of the &drm_dp_aux_msg structure. The retry logic + * and i2c helpers assume this is the case. */ struct drm_dp_aux { const char *name; -- cgit v1.2.3 From c5261e93758a6b36f4292403027af383ec9da129 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Fri, 26 Mar 2021 16:37:54 -0400 Subject: drm/print: Fixup DRM_DEBUG_KMS_RATELIMITED() Since we're about to move drm_dp_helper.c over to drm_dbg_*(), we'll want to make sure that we can also add ratelimited versions of these macros in order to retain some of the previous debugging output behavior we had. However, as I was preparing to do this I noticed that the current rate limited macros we have are kind of bogus. It looks like when I wrote these, I didn't notice that we'd always be calling __ratelimit() even if the debugging message we'd be printing would normally be filtered out due to the relevant DRM debugging category being disabled. So, let's fix this by making sure to check drm_debug_enabled() in our ratelimited macros before calling __ratelimit(), and start using drm_dev_printk() in order to print debugging messages since that will save us from doing a redundant drm_debug_enabled() check. And while we're at it, let's move the code for this into another macro that we can reuse for defining new ratelimited DRM debug macros more easily. v2: * Make sure to use tabs where possible in __DRM_DEFINE_DBG_RATELIMITED() Signed-off-by: Lyude Paul Cc: Robert Foss Reviewed-by: Robert Foss Link: https://patchwork.freedesktop.org/patch/msgid/20210326203807.105754-8-lyude@redhat.com --- include/drm/drm_print.h | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'include/drm') diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index f32d179e139d..a3c58c941bdc 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -524,16 +524,20 @@ void __drm_err(const char *format, ...); #define DRM_DEBUG_DP(fmt, ...) \ __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__) - -#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) \ -({ \ - static DEFINE_RATELIMIT_STATE(_rs, \ - DEFAULT_RATELIMIT_INTERVAL, \ - DEFAULT_RATELIMIT_BURST); \ - if (__ratelimit(&_rs)) \ - drm_dev_dbg(NULL, DRM_UT_KMS, fmt, ##__VA_ARGS__); \ +#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...) \ +({ \ + static DEFINE_RATELIMIT_STATE(rs_, DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);\ + const struct drm_device *drm_ = (drm); \ + \ + if (drm_debug_enabled(DRM_UT_ ## category) && __ratelimit(&rs_)) \ + drm_dev_printk(drm_ ? drm_->dev : NULL, KERN_DEBUG, fmt, ## __VA_ARGS__); \ }) +#define drm_dbg_kms_ratelimited(drm, fmt, ...) \ + __DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__) + +#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) drm_dbg_kms_ratelimited(NULL, fmt, ## __VA_ARGS__) + /* * struct drm_device based WARNs * -- cgit v1.2.3 From e6c6338f393b74ac0b303d567bb918b44ae7ad75 Mon Sep 17 00:00:00 2001 From: Jack Zhang Date: Mon, 8 Mar 2021 12:41:27 +0800 Subject: drm/amd/amdgpu implement tdr advanced mode [Why] Previous tdr design treats the first job in job_timeout as the bad job. But sometimes a later bad compute job can block a good gfx job and cause an unexpected gfx job timeout because gfx and compute ring share internal GC HW mutually. [How] This patch implements an advanced tdr mode.It involves an additinal synchronous pre-resubmit step(Step0 Resubmit) before normal resubmit step in order to find the real bad job. 1. At Step0 Resubmit stage, it synchronously submits and pends for the first job being signaled. If it gets timeout, we identify it as guilty and do hw reset. After that, we would do the normal resubmit step to resubmit left jobs. 2. For whole gpu reset(vram lost), do resubmit as the old way. v2: squash in build fix (Alex) Signed-off-by: Jack Zhang Reviewed-by: Andrey Grodzovsky Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 81 +++++++++++++++++++++++ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +- drivers/gpu/drm/scheduler/sched_main.c | 103 ++++++++++++++++++++--------- include/drm/gpu_scheduler.h | 3 + 4 files changed, 156 insertions(+), 33 deletions(-) (limited to 'include/drm') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 165192b2c336..3cbe8137a6af 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -4575,6 +4575,73 @@ static int amdgpu_device_suspend_display_audio(struct amdgpu_device *adev) return 0; } +void amdgpu_device_recheck_guilty_jobs(struct amdgpu_device *adev, + struct amdgpu_hive_info *hive, + struct list_head *device_list_handle, + bool *need_full_reset) +{ + int i, r = 0; + + for (i = 0; i < AMDGPU_MAX_RINGS; ++i) { + struct amdgpu_ring *ring = adev->rings[i]; + int ret = 0; + struct drm_sched_job *s_job; + + if (!ring || !ring->sched.thread) + continue; + + s_job = list_first_entry_or_null(&ring->sched.pending_list, + struct drm_sched_job, list); + if (s_job == NULL) + continue; + + /* clear job's guilty and depend the folowing step to decide the real one */ + drm_sched_reset_karma(s_job); + drm_sched_resubmit_jobs_ext(&ring->sched, 1); + + ret = dma_fence_wait_timeout(s_job->s_fence->parent, false, ring->sched.timeout); + if (ret == 0) { /* timeout */ + DRM_ERROR("Found the real bad job! ring:%s, job_id:%llx\n", + ring->sched.name, s_job->id); + + /* set guilty */ + drm_sched_increase_karma(s_job); +retry: + /* do hw reset */ + if (amdgpu_sriov_vf(adev)) { + amdgpu_virt_fini_data_exchange(adev); + r = amdgpu_device_reset_sriov(adev, false); + if (r) + adev->asic_reset_res = r; + } else { + r = amdgpu_do_asic_reset(hive, device_list_handle, + need_full_reset, false); + if (r && r == -EAGAIN) + goto retry; + } + + /* + * add reset counter so that the following + * resubmitted job could flush vmid + */ + atomic_inc(&adev->gpu_reset_counter); + continue; + } + + /* got the hw fence, signal finished fence */ + atomic_dec(ring->sched.score); + dma_fence_get(&s_job->s_fence->finished); + dma_fence_signal(&s_job->s_fence->finished); + dma_fence_put(&s_job->s_fence->finished); + + /* remove node from list and free the job */ + spin_lock(&ring->sched.job_list_lock); + list_del_init(&s_job->list); + spin_unlock(&ring->sched.job_list_lock); + ring->sched.ops->free_job(s_job); + } +} + /** * amdgpu_device_gpu_recover - reset the asic and recover scheduler * @@ -4597,6 +4664,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev, int i, r = 0; bool need_emergency_restart = false; bool audio_suspended = false; + int tmp_vram_lost_counter; /* * Special case: RAS triggered and full reset isn't supported @@ -4748,6 +4816,7 @@ retry: /* Rest of adevs pre asic reset from XGMI hive. */ } } + tmp_vram_lost_counter = atomic_read(&((adev)->vram_lost_counter)); /* Actual ASIC resets if needed.*/ /* TODO Implement XGMI hive reset logic for SRIOV */ if (amdgpu_sriov_vf(adev)) { @@ -4765,6 +4834,18 @@ skip_hw_reset: /* Post ASIC reset for all devs .*/ list_for_each_entry(tmp_adev, device_list_handle, reset_list) { + /* + * Sometimes a later bad compute job can block a good gfx job as gfx + * and compute ring share internal GC HW mutually. We add an additional + * guilty jobs recheck step to find the real guilty job, it synchronously + * submits and pends for the first job being signaled. If it gets timeout, + * we identify it as a real guilty job. + */ + if (amdgpu_gpu_recovery == 2 && + !(tmp_vram_lost_counter < atomic_read(&adev->vram_lost_counter))) + amdgpu_device_recheck_guilty_jobs(tmp_adev, hive, + device_list_handle, &need_full_reset); + for (i = 0; i < AMDGPU_MAX_RINGS; ++i) { struct amdgpu_ring *ring = tmp_adev->rings[i]; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 02b75fa64d7d..0e10c3958f94 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -516,7 +516,7 @@ module_param_named(compute_multipipe, amdgpu_compute_multipipe, int, 0444); * DOC: gpu_recovery (int) * Set to enable GPU recovery mechanism (1 = enable, 0 = disable). The default is -1 (auto, disabled except SRIOV). */ -MODULE_PARM_DESC(gpu_recovery, "Enable GPU recovery mechanism, (1 = enable, 0 = disable, -1 = auto)"); +MODULE_PARM_DESC(gpu_recovery, "Enable GPU recovery mechanism, (2 = advanced tdr mode, 1 = enable, 0 = disable, -1 = auto)"); module_param_named(gpu_recovery, amdgpu_gpu_recovery, int, 0444); /** diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index d82a7ebf6099..92d8de24d0a1 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -361,40 +361,16 @@ static void drm_sched_job_timedout(struct work_struct *work) */ void drm_sched_increase_karma(struct drm_sched_job *bad) { - int i; - struct drm_sched_entity *tmp; - struct drm_sched_entity *entity; - struct drm_gpu_scheduler *sched = bad->sched; - - /* don't increase @bad's karma if it's from KERNEL RQ, - * because sometimes GPU hang would cause kernel jobs (like VM updating jobs) - * corrupt but keep in mind that kernel jobs always considered good. - */ - if (bad->s_priority != DRM_SCHED_PRIORITY_KERNEL) { - atomic_inc(&bad->karma); - for (i = DRM_SCHED_PRIORITY_MIN; i < DRM_SCHED_PRIORITY_KERNEL; - i++) { - struct drm_sched_rq *rq = &sched->sched_rq[i]; - - spin_lock(&rq->lock); - list_for_each_entry_safe(entity, tmp, &rq->entities, list) { - if (bad->s_fence->scheduled.context == - entity->fence_context) { - if (atomic_read(&bad->karma) > - bad->sched->hang_limit) - if (entity->guilty) - atomic_set(entity->guilty, 1); - break; - } - } - spin_unlock(&rq->lock); - if (&entity->list != &rq->entities) - break; - } - } + drm_sched_increase_karma_ext(bad, 1); } EXPORT_SYMBOL(drm_sched_increase_karma); +void drm_sched_reset_karma(struct drm_sched_job *bad) +{ + drm_sched_increase_karma_ext(bad, 0); +} +EXPORT_SYMBOL(drm_sched_reset_karma); + /** * drm_sched_stop - stop the scheduler * @@ -533,15 +509,32 @@ EXPORT_SYMBOL(drm_sched_start); * */ void drm_sched_resubmit_jobs(struct drm_gpu_scheduler *sched) +{ + drm_sched_resubmit_jobs_ext(sched, INT_MAX); +} +EXPORT_SYMBOL(drm_sched_resubmit_jobs); + +/** + * drm_sched_resubmit_jobs_ext - helper to relunch certain number of jobs from mirror ring list + * + * @sched: scheduler instance + * @max: job numbers to relaunch + * + */ +void drm_sched_resubmit_jobs_ext(struct drm_gpu_scheduler *sched, int max) { struct drm_sched_job *s_job, *tmp; uint64_t guilty_context; bool found_guilty = false; struct dma_fence *fence; + int i = 0; list_for_each_entry_safe(s_job, tmp, &sched->pending_list, list) { struct drm_sched_fence *s_fence = s_job->s_fence; + if (i >= max) + break; + if (!found_guilty && atomic_read(&s_job->karma) > sched->hang_limit) { found_guilty = true; guilty_context = s_job->s_fence->scheduled.context; @@ -552,6 +545,7 @@ void drm_sched_resubmit_jobs(struct drm_gpu_scheduler *sched) dma_fence_put(s_job->s_fence->parent); fence = sched->ops->run_job(s_job); + i++; if (IS_ERR_OR_NULL(fence)) { if (IS_ERR(fence)) @@ -563,7 +557,7 @@ void drm_sched_resubmit_jobs(struct drm_gpu_scheduler *sched) } } } -EXPORT_SYMBOL(drm_sched_resubmit_jobs); +EXPORT_SYMBOL(drm_sched_resubmit_jobs_ext); /** * drm_sched_job_init - init a scheduler job @@ -903,3 +897,48 @@ void drm_sched_fini(struct drm_gpu_scheduler *sched) sched->ready = false; } EXPORT_SYMBOL(drm_sched_fini); + +/** + * drm_sched_increase_karma_ext - Update sched_entity guilty flag + * + * @bad: The job guilty of time out + * @type: type for increase/reset karma + * + */ +void drm_sched_increase_karma_ext(struct drm_sched_job *bad, int type) +{ + int i; + struct drm_sched_entity *tmp; + struct drm_sched_entity *entity; + struct drm_gpu_scheduler *sched = bad->sched; + + /* don't change @bad's karma if it's from KERNEL RQ, + * because sometimes GPU hang would cause kernel jobs (like VM updating jobs) + * corrupt but keep in mind that kernel jobs always considered good. + */ + if (bad->s_priority != DRM_SCHED_PRIORITY_KERNEL) { + if (type == 0) + atomic_set(&bad->karma, 0); + else if (type == 1) + atomic_inc(&bad->karma); + + for (i = DRM_SCHED_PRIORITY_MIN; i < DRM_SCHED_PRIORITY_KERNEL; + i++) { + struct drm_sched_rq *rq = &sched->sched_rq[i]; + + spin_lock(&rq->lock); + list_for_each_entry_safe(entity, tmp, &rq->entities, list) { + if (bad->s_fence->scheduled.context == + entity->fence_context) { + if (entity->guilty) + atomic_set(entity->guilty, type); + break; + } + } + spin_unlock(&rq->lock); + if (&entity->list != &rq->entities) + break; + } + } +} +EXPORT_SYMBOL(drm_sched_increase_karma_ext); diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index f888b5e9583a..10225a0a35d0 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -322,7 +322,10 @@ void drm_sched_wakeup(struct drm_gpu_scheduler *sched); void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job *bad); void drm_sched_start(struct drm_gpu_scheduler *sched, bool full_recovery); void drm_sched_resubmit_jobs(struct drm_gpu_scheduler *sched); +void drm_sched_resubmit_jobs_ext(struct drm_gpu_scheduler *sched, int max); void drm_sched_increase_karma(struct drm_sched_job *bad); +void drm_sched_reset_karma(struct drm_sched_job *bad); +void drm_sched_increase_karma_ext(struct drm_sched_job *bad, int type); bool drm_sched_dependency_optimized(struct dma_fence* fence, struct drm_sched_entity *entity); void drm_sched_fault(struct drm_gpu_scheduler *sched); -- cgit v1.2.3