Re: [PATCH RFC v2 08/14] temp: add LPAC regs

[sashiko-bot@kernel.org]

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 <anna.maniscalco2000@gmail.com>

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-a21a5c850961@gmail.com?part=8