From: Alex Deucher alexander.deucher@amd.com
[ Upstream commit c8b8d7a4f1c5cdfbd61d75302fb3e3cdefb1a7ab ]
We shouldn't return after the last section. We need to update the rest of the CSIB.
Reviewed-by: Rodrigo Siqueira siqueira@igalia.com Signed-off-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Sasha Levin sashal@kernel.org ---
**YES** This commit should be backported to stable kernel trees. **Extensive Analysis:** **1. Critical Bug Nature:** The code changes fix a serious logic error in the CSIB (Clear State Indirect Buffer) handling for AMD GFX8 hardware. The bug occurs in `gfx_v8_0_get_csb_buffer()` where there's an early `return` statement in the `else` branch at line 1252: ```c if (sect->id == SECT_CONTEXT) { // Handle SECT_CONTEXT sections... } else { return; // BUG: Early exit prevents rest of buffer setup! } ``` **2. Impact and Severity:** This bug has significant functional impact because: - The function is responsible for setting up the Clear State Buffer, which is critical for GPU state initialization - When encountering any section that isn't `SECT_CONTEXT` (such as `SECT_CLEAR`, `SECT_CTRLCONST`, or `SECT_NONE` as defined in `clearstate_defs.h`), the function exits early - This prevents the essential final buffer setup steps from executing: - PA_SC_RASTER_CONFIG register setup (lines 1257-1261) - PACKET3_PREAMBLE_END_CLEAR_STATE packet (lines 1263-1264) - PACKET3_CLEAR_STATE packet (lines 1266-1267) **3. Real-World Consequences:** An incomplete CSB buffer can lead to: - GPU initialization failures - Rendering corruption - System instability - Potential hangs during graphics operations **4. Code Quality Assessment:** - **Small and contained:** The fix removes only 2 lines of problematic code - **No architectural changes:** Pure bug fix with no design modifications - **Clear intent:** The commit message explicitly states the problem and solution - **Low regression risk:** Removing an early return cannot introduce new failures **5. Comparison with Historical Examples:** This commit aligns perfectly with the "Backport Status: YES" example (#4) which also fixed a critical handling issue in AMD GPU code. Like that commit, this: - Fixes a clear functional bug - Has immediate user impact - Is small and low-risk - Affects critical GPU subsystem functionality **6. Stable Tree Compliance:** ✓ Fixes important user-affecting bug ✓ Small, contained change ✓ No new features introduced ✓ Critical hardware subsystem affected ✓ Clear regression prevention The fix ensures that all necessary GPU state initialization packets are properly written to the buffer, which is essential for correct hardware operation on GFX8 hardware generations.
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c index 9d741695ca07d..15795ece13f6b 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c @@ -1232,8 +1232,6 @@ static void gfx_v8_0_get_csb_buffer(struct amdgpu_device *adev, PACKET3_SET_CONTEXT_REG_START); for (i = 0; i < ext->reg_count; i++) buffer[count++] = cpu_to_le32(ext->extent[i]); - } else { - return; } } }