From: Rob Clark <robin.cl...@oss.qualcomm.com> [ Upstream commit 13ed0a1af263b56a5ebbf38ab7163cbc9dcb009e ]
The bitfield positions changed in a7xx. v2: Don't open-code the bitfield building v3: Also fix cx_debugbus Signed-off-by: Rob Clark <robin.cl...@oss.qualcomm.com> Patchwork: https://patchwork.freedesktop.org/patch/666659/ Signed-off-by: Sasha Levin <sas...@kernel.org> --- LLM Generated explanations, may be completely bogus: **Backport Status: YES** This commit should be backported to stable kernel trees for the following reasons: ## Bug Fix Analysis The commit fixes a functional bug in the MSM DRM driver's debugbus reading functionality for Adreno 7xx GPUs. Specifically: 1. **Incorrect Bitfield Positions**: The code was using A6XX bitfield definitions for A7XX hardware, but the hardware changed the bitfield positions. Looking at the changes in `a6xx.xml`: - A6XX uses bits 15:8 for `PING_BLK_SEL` - A7XX uses bits 24:16 for `PING_BLK_SEL` This mismatch would cause incorrect values to be written to the debug bus selection registers, resulting in wrong or garbage data being read. 2. **Affects Core Debugging Functionality**: The debugbus is critical for GPU state capture during crashes and debugging. Without this fix, debugging A7XX GPUs would be severely impaired as the debugbus reads would return incorrect data. ## Stable Criteria Evaluation 1. **Fixes a real bug**: Yes - incorrect register programming leading to wrong debug data 2. **Small and contained**: Yes - only 46 lines changed across 2 files 3. **No major side effects**: The changes are conditional based on GPU family and only affect debug bus reading 4. **No architectural changes**: Simple conditional logic to use correct bitfield definitions 5. **Critical subsystem impact**: Moderate - affects GPU debugging capabilities which are important for diagnosing GPU hangs/crashes 6. **Already upstream**: Yes - commit 13ed0a1af263b56a5ebbf38ab7163cbc9dcb009e is already merged upstream ## Code Safety Analysis The fix is safe because: - It adds proper family checks: `if (to_adreno_gpu(gpu)->info->family >= ADRENO_7XX_GEN1)` - Maintains backward compatibility for A6XX hardware - Only changes register programming for debug bus reads, not normal GPU operation - The fix pattern is applied consistently to both `debugbus_read()` and `cx_debugbus_read()` functions ## Impact Assessment Without this fix, users with A7XX GPUs would experience: - Incorrect GPU state dumps when GPU crashes occur - Inability to properly debug GPU issues - Misleading diagnostic information that could hamper bug resolution This is a clear bugfix that restores proper functionality for A7XX hardware without introducing new features or risky changes, making it an ideal candidate for stable backporting. drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 32 ++++++++++++++----- drivers/gpu/drm/msm/registers/adreno/a6xx.xml | 14 +++++++- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c index f46bc906ca2a3..61850e2802914 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c @@ -174,8 +174,15 @@ static int a6xx_crashdumper_run(struct msm_gpu *gpu, static int debugbus_read(struct msm_gpu *gpu, u32 block, u32 offset, u32 *data) { - u32 reg = A6XX_DBGC_CFG_DBGBUS_SEL_D_PING_INDEX(offset) | - A6XX_DBGC_CFG_DBGBUS_SEL_D_PING_BLK_SEL(block); + u32 reg; + + if (to_adreno_gpu(gpu)->info->family >= ADRENO_7XX_GEN1) { + reg = A7XX_DBGC_CFG_DBGBUS_SEL_D_PING_INDEX(offset) | + A7XX_DBGC_CFG_DBGBUS_SEL_D_PING_BLK_SEL(block); + } else { + reg = A6XX_DBGC_CFG_DBGBUS_SEL_D_PING_INDEX(offset) | + A6XX_DBGC_CFG_DBGBUS_SEL_D_PING_BLK_SEL(block); + } gpu_write(gpu, REG_A6XX_DBGC_CFG_DBGBUS_SEL_A, reg); gpu_write(gpu, REG_A6XX_DBGC_CFG_DBGBUS_SEL_B, reg); @@ -198,11 +205,18 @@ static int debugbus_read(struct msm_gpu *gpu, u32 block, u32 offset, readl((ptr) + ((offset) << 2)) /* read a value from the CX debug bus */ -static int cx_debugbus_read(void __iomem *cxdbg, u32 block, u32 offset, +static int cx_debugbus_read(struct msm_gpu *gpu, void __iomem *cxdbg, u32 block, u32 offset, u32 *data) { - u32 reg = A6XX_CX_DBGC_CFG_DBGBUS_SEL_A_PING_INDEX(offset) | - A6XX_CX_DBGC_CFG_DBGBUS_SEL_A_PING_BLK_SEL(block); + u32 reg; + + if (to_adreno_gpu(gpu)->info->family >= ADRENO_7XX_GEN1) { + reg = A7XX_CX_DBGC_CFG_DBGBUS_SEL_A_PING_INDEX(offset) | + A7XX_CX_DBGC_CFG_DBGBUS_SEL_A_PING_BLK_SEL(block); + } else { + reg = A6XX_CX_DBGC_CFG_DBGBUS_SEL_A_PING_INDEX(offset) | + A6XX_CX_DBGC_CFG_DBGBUS_SEL_A_PING_BLK_SEL(block); + } cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_SEL_A, reg); cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_SEL_B, reg); @@ -315,7 +329,8 @@ static void a6xx_get_debugbus_block(struct msm_gpu *gpu, ptr += debugbus_read(gpu, block->id, i, ptr); } -static void a6xx_get_cx_debugbus_block(void __iomem *cxdbg, +static void a6xx_get_cx_debugbus_block(struct msm_gpu *gpu, + void __iomem *cxdbg, struct a6xx_gpu_state *a6xx_state, const struct a6xx_debugbus_block *block, struct a6xx_gpu_state_obj *obj) @@ -330,7 +345,7 @@ static void a6xx_get_cx_debugbus_block(void __iomem *cxdbg, obj->handle = block; for (ptr = obj->data, i = 0; i < block->count; i++) - ptr += cx_debugbus_read(cxdbg, block->id, i, ptr); + ptr += cx_debugbus_read(gpu, cxdbg, block->id, i, ptr); } static void a6xx_get_debugbus_blocks(struct msm_gpu *gpu, @@ -527,7 +542,8 @@ static void a6xx_get_debugbus(struct msm_gpu *gpu, int i; for (i = 0; i < nr_cx_debugbus_blocks; i++) - a6xx_get_cx_debugbus_block(cxdbg, + a6xx_get_cx_debugbus_block(gpu, + cxdbg, a6xx_state, &cx_debugbus_blocks[i], &a6xx_state->cx_debugbus[i]); diff --git a/drivers/gpu/drm/msm/registers/adreno/a6xx.xml b/drivers/gpu/drm/msm/registers/adreno/a6xx.xml index d860fd94feae8..86fab2750ba7b 100644 --- a/drivers/gpu/drm/msm/registers/adreno/a6xx.xml +++ b/drivers/gpu/drm/msm/registers/adreno/a6xx.xml @@ -594,10 +594,14 @@ by a particular renderpass/blit. <reg32 offset="0x0600" name="DBGC_CFG_DBGBUS_SEL_A"/> <reg32 offset="0x0601" name="DBGC_CFG_DBGBUS_SEL_B"/> <reg32 offset="0x0602" name="DBGC_CFG_DBGBUS_SEL_C"/> - <reg32 offset="0x0603" name="DBGC_CFG_DBGBUS_SEL_D"> + <reg32 offset="0x0603" name="DBGC_CFG_DBGBUS_SEL_D" variants="A6XX"> <bitfield high="7" low="0" name="PING_INDEX"/> <bitfield high="15" low="8" name="PING_BLK_SEL"/> </reg32> + <reg32 offset="0x0603" name="DBGC_CFG_DBGBUS_SEL_D" variants="A7XX-"> + <bitfield high="7" low="0" name="PING_INDEX"/> + <bitfield high="24" low="16" name="PING_BLK_SEL"/> + </reg32> <reg32 offset="0x0604" name="DBGC_CFG_DBGBUS_CNTLT"> <bitfield high="5" low="0" name="TRACEEN"/> <bitfield high="14" low="12" name="GRANU"/> @@ -3796,6 +3800,14 @@ by a particular renderpass/blit. <reg32 offset="0x0030" name="CFG_DBGBUS_TRACE_BUF2"/> </domain> +<domain name="A7XX_CX_DBGC" width="32"> + <!-- Bitfields shifted, but otherwise the same: --> + <reg32 offset="0x0000" name="CFG_DBGBUS_SEL_A" variants="A7XX-"> + <bitfield high="7" low="0" name="PING_INDEX"/> + <bitfield high="24" low="16" name="PING_BLK_SEL"/> + </reg32> +</domain> + <domain name="A6XX_CX_MISC" width="32" prefix="variant" varset="chip"> <reg32 offset="0x0001" name="SYSTEM_CACHE_CNTL_0"/> <reg32 offset="0x0002" name="SYSTEM_CACHE_CNTL_1"/> -- 2.50.1
