From d4fdd714b76de74698e232711e8f4e316e33fb8e Mon Sep 17 00:00:00 2001 From: Joshua Ashton Date: Tue, 16 Jan 2024 21:06:45 +0000 Subject: [PATCH] winsys/amdgpu: Limit usage of query_reset_state2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following discussion on kernel mailing list[1], we are not gaining anything from using this to figure out if we reset, and it does not handle soft recovery. We will hear about the context loss and rationale when we submit. Instead, only use this for figuring out if the reset we already knew about was completed. [1]: https://lists.freedesktop.org/archives/amd-gfx/2024-January/103337.html Signed-off-by: Joshua Ashton Reviewed-by: André Almeida Reviewed-by: Pierre-Eric Pelloux-Prayer Reviewed-by: Marek Olšák Part-of: --- src/gallium/winsys/amdgpu/drm/amdgpu_cs.c | 82 +++++++++++------------ 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c b/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c index 3d9a1ed86114..4e36aaf5918d 100644 --- a/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c @@ -472,7 +472,6 @@ amdgpu_ctx_query_reset_status(struct radeon_winsys_ctx *rwctx, bool full_reset_o bool *needs_reset, bool *reset_completed) { struct amdgpu_ctx *ctx = (struct amdgpu_ctx*)rwctx; - int r; if (needs_reset) *needs_reset = false; @@ -489,46 +488,45 @@ amdgpu_ctx_query_reset_status(struct radeon_winsys_ctx *rwctx, bool full_reset_o return PIPE_NO_RESET; } - r = amdgpu_cs_query_reset_state2(ctx->ctx, &flags); - if (r) { - fprintf(stderr, "amdgpu: amdgpu_cs_query_reset_state2 failed. (%i)\n", r); - return PIPE_NO_RESET; - } - - if (flags & AMDGPU_CTX_QUERY2_FLAGS_RESET) { - if (reset_completed) { - /* The ARB_robustness spec says: - * - * If a reset status other than NO_ERROR is returned and subsequent - * calls return NO_ERROR, the context reset was encountered and - * completed. If a reset status is repeatedly returned, the context may - * be in the process of resetting. - * - * Starting with drm_minor >= 54 amdgpu reports if the reset is complete, - * so don't do anything special. On older kernels, submit a no-op cs. If it - * succeeds then assume the reset is complete. - */ - if (!(flags & AMDGPU_CTX_QUERY2_FLAGS_RESET_IN_PROGRESS)) - *reset_completed = true; - - if (ctx->ws->info.drm_minor < 54 && ctx->ws->info.has_graphics) - *reset_completed = amdgpu_submit_gfx_nop(ctx) == 0; + /* + * ctx->sw_status is updated on alloc/ioctl failures. + * + * We only rely on amdgpu_cs_query_reset_state2 to tell us + * that the context reset is complete. + */ + if (ctx->sw_status != PIPE_NO_RESET) { + int r = amdgpu_cs_query_reset_state2(ctx->ctx, &flags); + if (!r) { + if (flags & AMDGPU_CTX_QUERY2_FLAGS_RESET) { + if (reset_completed) { + /* The ARB_robustness spec says: + * + * If a reset status other than NO_ERROR is returned and subsequent + * calls return NO_ERROR, the context reset was encountered and + * completed. If a reset status is repeatedly returned, the context may + * be in the process of resetting. + * + * Starting with drm_minor >= 54 amdgpu reports if the reset is complete, + * so don't do anything special. On older kernels, submit a no-op cs. If it + * succeeds then assume the reset is complete. + */ + if (!(flags & AMDGPU_CTX_QUERY2_FLAGS_RESET_IN_PROGRESS)) + *reset_completed = true; + + if (ctx->ws->info.drm_minor < 54 && ctx->ws->info.has_graphics) + *reset_completed = amdgpu_submit_gfx_nop(ctx) == 0; + } + } + } else { + fprintf(stderr, "amdgpu: amdgpu_cs_query_reset_state2 failed. (%i)\n", r); } - if (needs_reset) - *needs_reset = flags & AMDGPU_CTX_QUERY2_FLAGS_VRAMLOST; - if (flags & AMDGPU_CTX_QUERY2_FLAGS_GUILTY) - return PIPE_GUILTY_CONTEXT_RESET; - else - return PIPE_INNOCENT_CONTEXT_RESET; - } - - /* Return a failure due to SW issues. */ - if (ctx->sw_status != PIPE_NO_RESET) { + /* Return a failure due to SW issues. */ if (needs_reset) *needs_reset = true; return ctx->sw_status; } + if (needs_reset) *needs_reset = false; return PIPE_NO_RESET; @@ -1608,19 +1606,19 @@ static void amdgpu_cs_submit_ib(void *job, void *gdata, int thread_index) if (unlikely(r)) { if (r == -ECANCELED) { amdgpu_ctx_set_sw_reset_status((struct radeon_winsys_ctx*)acs->ctx, PIPE_INNOCENT_CONTEXT_RESET, - "amdgpu: The CS has cancelled because the context is lost. This context is innocent.\n"); + "amdgpu: The CS has cancelled because the context is lost. This context is innocent.\n"); } else if (r == -ENODATA) { amdgpu_ctx_set_sw_reset_status((struct radeon_winsys_ctx*)acs->ctx, PIPE_GUILTY_CONTEXT_RESET, - "amdgpu: The CS has cancelled because the context is lost. This context is guilty of a soft recovery.\n"); + "amdgpu: The CS has cancelled because the context is lost. This context is guilty of a soft recovery.\n"); } else if (r == -ETIME) { amdgpu_ctx_set_sw_reset_status((struct radeon_winsys_ctx*)acs->ctx, PIPE_GUILTY_CONTEXT_RESET, - "amdgpu: The CS has cancelled because the context is lost. This context is guilty of a hard recovery.\n"); + "amdgpu: The CS has cancelled because the context is lost. This context is guilty of a hard recovery.\n"); } else { amdgpu_ctx_set_sw_reset_status((struct radeon_winsys_ctx*)acs->ctx, - PIPE_UNKNOWN_CONTEXT_RESET, - "amdgpu: The CS has been rejected, " - "see dmesg for more information (%i).\n", - r); + PIPE_UNKNOWN_CONTEXT_RESET, + "amdgpu: The CS has been rejected, " + "see dmesg for more information (%i).\n", + r); } }