From: Rob Clark robin.clark@oss.qualcomm.com
[ Upstream commit a506578d8909e7e6f0d545af9850ccd4318bf6cf ]
Program the selector _after_ selecting the aperture. This aligns with the downstream driver, and fixes a case where we were failing to capture ctx0 regs (and presumably what we thought were ctx1 regs were actually ctx0).
Suggested-by: Akhil P Oommen akhilpo@oss.qualcomm.com Signed-off-by: Rob Clark robin.clark@oss.qualcomm.com Patchwork: https://patchwork.freedesktop.org/patch/666655/ Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
**Backport Status: YES**
This commit should be backported to stable kernel trees. Here's my extensive analysis:
## Bug Fix Nature
The commit fixes a **clear functional bug** in the GPU crash dump/snapshot functionality for Qualcomm Adreno GPUs (specifically the a7xx series). The bug causes incorrect register capture during GPU crash dumps: - **Symptom**: Failing to capture ctx0 (context 0) registers correctly - **Impact**: ctx1 registers were actually ctx0 registers, meaning critical debugging information was lost or misrepresented
## Code Analysis
The fix is **extremely simple and surgical** - it only reorders two blocks of code in the `a7xx_get_cluster()` function:
**Before the fix:** ```c /* Some clusters need a selector register to be programmed too */ if (cluster->sel) in += CRASHDUMP_WRITE(in, cluster->sel->cd_reg, cluster->sel->val);
in += CRASHDUMP_WRITE(in, REG_A7XX_CP_APERTURE_CNTL_CD, ...); ```
**After the fix:** ```c in += CRASHDUMP_WRITE(in, REG_A7XX_CP_APERTURE_CNTL_CD, ...);
/* Some clusters need a selector register to be programmed too */ if (cluster->sel) in += CRASHDUMP_WRITE(in, cluster->sel->cd_reg, cluster->sel->val); ```
The aperture control register (`REG_A7XX_CP_APERTURE_CNTL_CD`) must be programmed **before** the selector register. This is a classic hardware programming sequence issue where register order matters.
## Stable Backport Criteria Met
1. **Fixes a real bug**: Yes - incorrect crash dump data affects debugging capability 2. **Small and contained**: Yes - only 4 lines moved, no logic changes 3. **No architectural changes**: Correct - purely a reordering fix 4. **Minimal regression risk**: Yes - only affects crash dump path, not normal operation 5. **Aligns with vendor driver**: The commit explicitly states it "aligns with the downstream driver" 6. **No new features**: Correct - purely a bug fix
## Additional Supporting Evidence
- The commit has already been marked with "Upstream commit a506578d8909..." suggesting it's been accepted upstream - Similar fixes in the same file (e.g., `f28c9fc2c82de drm/msm: Fix debugbus snapshot`) show a pattern of fixing crash dump issues - The fix is isolated to the crash dump code path (`a7xx_get_cluster()`), which is only executed during GPU error recovery - The author (Rob Clark) is a maintainer of the MSM DRM driver, lending credibility to the fix
This is an ideal stable backport candidate - it fixes a clear bug with minimal code change and virtually no risk of regression.
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c index 61850e2802914..6e8dbd27addbe 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c @@ -776,15 +776,15 @@ static void a7xx_get_cluster(struct msm_gpu *gpu, size_t datasize; int i, regcount = 0;
- /* Some clusters need a selector register to be programmed too */ - if (cluster->sel) - in += CRASHDUMP_WRITE(in, cluster->sel->cd_reg, cluster->sel->val); - in += CRASHDUMP_WRITE(in, REG_A7XX_CP_APERTURE_CNTL_CD, A7XX_CP_APERTURE_CNTL_CD_PIPE(cluster->pipe_id) | A7XX_CP_APERTURE_CNTL_CD_CLUSTER(cluster->cluster_id) | A7XX_CP_APERTURE_CNTL_CD_CONTEXT(cluster->context_id));
+ /* Some clusters need a selector register to be programmed too */ + if (cluster->sel) + in += CRASHDUMP_WRITE(in, cluster->sel->cd_reg, cluster->sel->val); + for (i = 0; cluster->regs[i] != UINT_MAX; i += 2) { int count = RANGE(cluster->regs, i);