Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm/msm/a8xx: wrong protect table pakala_protect used for Kaanapali - [High] drm/msm/a8xx: missing CP_PROTECT_GLOBAL(56..62) in a850_ifpc_reglist_regs --
commit 552d3a60893e50e06127da749c3d585e911f2b2e Author: Rob Clark <[email protected]> drm/msm/a8xx: Switch to generated protect regs This commit replaces the hardcoded protect register tables in a8xx_catalog.c with the generated protect tables from the per-target catalog headers. > diff --git a/drivers/gpu/drm/msm/adreno/a8xx_catalog.c > b/drivers/gpu/drm/msm/adreno/a8xx_catalog.c > index bc79013165c13..5f5f71d86df93 100644 > --- a/drivers/gpu/drm/msm/adreno/a8xx_catalog.c > +++ b/drivers/gpu/drm/msm/adreno/a8xx_catalog.c [ ... ] > @@ -1305,7 +1026,7 @@ static const struct adreno_info a8xx_gpus[] = { > ADRENO_QUIRK_IFPC, > .funcs = &a8xx_gpu_funcs, > .a6xx = &(const struct a6xx_info) { > - .protect = &a840_protect, > + .protect = &pakala_protect, > .nonctxt_reglist = kaanapali_nonctxt_regs, > .pwrup_reglist = &a840_pwrup_reglist, > .dyn_pwrup_reglist = &a840_dyn_pwrup_reglist, [Severity: High] Should the Kaanapali (Adreno 840, 0x44050a01) entry in a8xx_gpus[] use &kaanapali_protect instead of &pakala_protect? __build_asserts() checks kaanapali_protect, which is defined in drivers/gpu/drm/msm/adreno/catalog/adreno_kaanapali.h, but kaanapali_protect is left unused in a8xx_gpus[]. When hw_init() -> a8xx_set_cp_protect() programs the protection registers from pakala_protect_regs on Kaanapali, several Kaanapali-specific spans from kaanapali_protect_regs (including 0x00775..0x0077f, 0x00c07..0x00c0f, 0x03d00..0x05cff, 0x0a82e, 0x0aed0..0x0b17f, and 0x1fd80..0x1ffff) are left unprotected against unprivileged userspace command streams, while 0x00000..0x00007 and 0x27c02..0x27c05 are protected instead. [ ... ] > @@ -1481,7 +1202,7 @@ static const struct adreno_info a8xx_gpus[] = { > ADRENO_QUIRK_IFPC, > .funcs = &a8xx_gpu_funcs, > .a6xx = &(const struct a6xx_info) { > - .protect = &a850_protect, > + .protect = &hawi_protect, > .nonctxt_reglist = hawi_nonctxt_regs, > .pwrup_reglist = &a850_pwrup_reglist, > .dyn_pwrup_reglist = &a850_dyn_pwrup_reglist, [Severity: High] Does switching Hawi (Adreno 850, 0x44051401) to hawi_protect without updating a850_ifpc_reglist_regs cause REG_A8XX_CP_PROTECT_GLOBAL(56) through REG_A8XX_CP_PROTECT_GLOBAL(62) to be lost across inter-frame power collapse? Previously, a850_protect_regs had 57 entries, and a850_ifpc_reglist_regs omitted REG_A8XX_CP_PROTECT_GLOBAL(56) through REG_A8XX_CP_PROTECT_GLOBAL(62): drivers/gpu/drm/msm/adreno/a8xx_catalog.c: static const u32 a850_ifpc_reglist_regs[] = { ... REG_A8XX_CP_PROTECT_GLOBAL(54), REG_A8XX_CP_PROTECT_GLOBAL(55), REG_A8XX_CP_PROTECT_GLOBAL(63), }; In drivers/gpu/drm/msm/adreno/catalog/adreno_hawi.h, hawi_protect_regs has 64 entries, so hw_init() -> a8xx_set_cp_protect() populates all 64 REG_A8XX_CP_PROTECT_GLOBAL(0..63) registers: drivers/gpu/drm/msm/adreno/catalog/adreno_hawi.h: static const u32 hawi_protect_regs[] __maybe_unused = { ... A6XX_PROTECT_NORDWR(0x1ac00, 0x1fff), A6XX_PROTECT_NORDWR(0x1f400, 0x0477), A6XX_PROTECT_NORDWR(0x1f930, 0x032f), A6XX_PROTECT_NORDWR(0x1fd80, 0x1fff), A6XX_PROTECT_NORDWR(0x27800, 0x007f), A6XX_PROTECT_RDONLY(0x27880, 0x0001), A6XX_PROTECT_NORDWR(0x27882, 0x037d), A6XX_PROTECT_NORDWR(0x27c06, 0x0000), }; DECLARE_ADRENO_PROTECT(hawi_protect, 64); Because Hawi sets ADRENO_QUIRK_IFPC and uses a850_ifpc_reglist in a8xx_patch_pwrup_reglist(), those 7 protection registers (covering ranges in 0x1ac00..0x27bff) will not be restored after inter-frame power collapse. > @@ -1516,6 +1237,10 @@ DECLARE_ADRENO_GPULIST(a8xx); > > static inline __always_unused void __build_asserts(void) > { > - BUILD_BUG_ON(a810_protect.count > a810_protect.count_max); > - BUILD_BUG_ON(a830_protect.count > a830_protect.count_max); > + BUILD_BUG_ON(glymur_protect.count > glymur_protect.count_max); > + BUILD_BUG_ON(hawi_protect.count > hawi_protect.count_max); > + BUILD_BUG_ON(kaanapali_protect.count > kaanapali_protect.count_max); > + BUILD_BUG_ON(maili_protect.count > maili_protect.count_max); > + BUILD_BUG_ON(milos_protect.count > milos_protect.count_max); > + BUILD_BUG_ON(pakala_protect.count > pakala_protect.count_max); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
