Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/msm: conflicting offsets and variants for LPAC regs in a6xx.xml --
commit cffea449b0ea5b2e3282237ed9e0160eef7997c8 Author: Anna Maniscalco <[email protected]> temp: add LPAC regs This commit adds LPAC and UCHE register definitions to the Adreno a6xx XML register database. > diff --git a/drivers/gpu/drm/msm/registers/adreno/a6xx.xml > b/drivers/gpu/drm/msm/registers/adreno/a6xx.xml > index 3349c01646e12..bb49f6a61a9af 100644 > --- a/drivers/gpu/drm/msm/registers/adreno/a6xx.xml > +++ b/drivers/gpu/drm/msm/registers/adreno/a6xx.xml > @@ -276,6 +276,11 @@ by a particular renderpass/blit. > <bitfield pos="1" name="ACCESS_FAULT_ON_VIOL_EN" > type="boolean"/> > <bitfield pos="0" name="ACCESS_PROT_EN" type="boolean"/> > </bitset> > + <reg32 offset="0x0B09" name="CP_LPAC_PROTECT_CNTL"> > + <bitfield pos="3" name="LAST_SPAN_INF_RANGE" type="boolean"/> > + <bitfield pos="1" name="ACCESS_FAULT_ON_VIOL_EN" > type="boolean"/> > + <bitfield pos="0" name="ACCESS_PROT_EN" type="boolean"/> > + </reg32> > > <reg32 offset="0x084f" name="CP_PROTECT_CNTL" > type="a6xx_cp_protect_cntl" variants="A6XX-A7XX"/> > <bitset name="a8xx_cp_protect_cntl" inline="yes"> > @@ -333,6 +338,11 @@ by a particular renderpass/blit. > <reg32 offset="0x0845" name="CP_CRASH_DUMP_STATUS" variants="A8XX-"/> > <reg32 offset="0x0908" name="CP_SQE_STAT_ADDR" variants="A6XX-A7XX"/> > <reg32 offset="0x0909" name="CP_SQE_STAT_DATA" variants="A6XX-A7XX"/> > + <reg64 offset="0x0b00" name="CP_LPAC_RB_BASE" variants="A7XX-"/> > + <reg32 offset="0x0b02" name="CP_LPAC_RB_CNTL" variants="A7XX-"/> > + <reg64 offset="0x0b04" name="CP_LPAC_RB_RPTR_ADDR" variants="A7XX-"/> > + <reg32 offset="0x0b06" name="CP_LPAC_RB_RPTR" variants="A7XX-"/> > + <reg32 offset="0x0b07" name="CP_LPAC_RB_WPTR" variants="A7XX-"/> > <reg32 offset="0x090a" name="CP_DRAW_STATE_ADDR" variants="A6XX-A7XX"/> > <reg32 offset="0x090b" name="CP_DRAW_STATE_DATA" variants="A6XX-A7XX"/> > <reg32 offset="0x090c" name="CP_ROQ_DBG_ADDR" variants="A6XX-A7XX"/> > @@ -521,6 +531,7 @@ by a particular renderpass/blit. > <reg32 offset="0x0b29" name="CP_SQE_AC_STAT_ADDR" variants="A7XX"/> > <reg32 offset="0x0b2a" name="CP_SQE_AC_STAT_DATA" variants="A7XX"/> > > + <reg32 offset="0x0b30" name="CP_LPAC_CHICKEN_DBG" variants="A7XX-"/> > <reg32 offset="0x0b31" name="CP_LPAC_APRIV_CNTL" variants="A7XX"/> > <reg32 offset="0x0b34" name="CP_LPAC_PROG_FIFO_SIZE" variants="A7XX"/> > <reg32 offset="0x0b35" name="CP_LPAC_ROQ_DBG_DATA" variants="A7XX"/> > @@ -1207,6 +1218,7 @@ by a particular renderpass/blit. > <reg64 offset="0x0E08" name="UCHE_TRAP_BASE" variants="A8XX-"/> > <reg64 offset="0x0E0B" name="UCHE_GMEM_RANGE_MIN" variants="A6XX-A7XX"/> > <reg64 offset="0x0E0D" name="UCHE_GMEM_RANGE_MAX" variants="A6XX-A7XX"/> > + <reg32 offset="0x0E12" name="UCHE_DEBUG_CNTL_1" usage="cmd"/> [Severity: Medium] Should these new register definitions specify narrower hardware variants to avoid colliding with existing A6XX, A7XX, and A8XX registers during header generation in drivers/gpu/drm/msm/registers/gen_header.py? First, UCHE_DEBUG_CNTL_1 is added at offset 0x0E12 with usage="cmd" and no variants attribute, while leaving the existing definition for 0x0e12 on A6XX-A7XX in place and overlapping with UCHE_GBIF_GX_CONFIG on A8XX-: <reg32 offset="0x0e3a" name="UCHE_GBIF_GX_CONFIG" variants="A6XX-A7XX"/> <reg32 offset="0x0e12" name="UCHE_GBIF_GX_CONFIG" variants="A8XX-"/> ... <!-- always 0x03200000 ? --> <reg32 offset="0x0e12" name="UCHE_UNKNOWN_0E12" variants="A6XX-A7XX" usage="init"/> Because UCHE_DEBUG_CNTL_1 has no variants restriction, dump_reg_usages() in gen_header.py emits 0x0e12 into CMD_REGS<A8XX> where that offset is UCHE_GBIF_GX_CONFIG, and leaves duplicate definitions for 0x0e12 on A6XX-A7XX. Second, CP_LPAC_RB_BASE (0x0b00), CP_LPAC_RB_CNTL (0x0b02), CP_LPAC_RB_RPTR_ADDR (0x0b04), CP_LPAC_RB_RPTR (0x0b06), CP_LPAC_RB_WPTR (0x0b07), and CP_LPAC_CHICKEN_DBG (0x0b30) use open-ended variants="A7XX-", which includes A8XX. However, on A8XX- the LPAC ringbuffer registers are at 0x080b-0x0812 and offsets 0x0b00-0x0b01 are reassigned: <reg64 offset="0x080b" name="CP_RB_BASE_LPAC" variants="A8XX-"/> <reg32 offset="0x080d" name="CP_RB_CNTL_LPAC" variants="A8XX-"/> <reg32 offset="0x080e" name="CP_RB_RPTR_WR_LPAC" variants="A8XX-"/> <reg64 offset="0x080f" name="CP_RB_RPTR_ADDR_LPAC" variants="A8XX-"/> <reg32 offset="0x0811" name="CP_RB_RPTR_LPAC" variants="A8XX-"/> <reg32 offset="0x0812" name="CP_RB_WPTR_LPAC" variants="A8XX-"/> ... <reg32 offset="0x0b00" name="CP_SLICE_MEM_POOL_DBG_ADDR_PIPE" variants="A8XX-"/> <reg32 offset="0x0b01" name="CP_SLICE_MEM_POOL_DBG_DATA_PIPE" variants="A8XX-"/> Other A7XX LPAC registers in the 0x0b0a-0x0b40 range use variants="A7XX" or variants="A6XX-A7XX". Finally, CP_LPAC_PROTECT_CNTL at offset 0x0B09 omits a variants attribute (exposing it on A8XX) and duplicates the bitfields inline instead of using type="a6xx_cp_protect_cntl" defined immediately above it: <bitset name="a6xx_cp_protect_cntl" inline="yes"> <bitfield pos="3" name="LAST_SPAN_INF_RANGE" type="boolean"/> <bitfield pos="1" name="ACCESS_FAULT_ON_VIOL_EN" type="boolean"/> <bitfield pos="0" name="ACCESS_PROT_EN" type="boolean"/> </bitset> <reg32 offset="0x084f" name="CP_PROTECT_CNTL" type="a6xx_cp_protect_cntl" variants="A6XX-A7XX"/> -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850...@gmail.com?part=8
