Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] drm: missing weight index 0 in a810_pwrup_reglist_regs - [Low] drm: a640_gbif declared as extern in C file instead of shared header --
commit 417ccf9a602a6ae58f1d68b6bfbd7ec7f4880b14 Author: Rob Clark <[email protected]> drm/msm/a6xx: Split out a7xx/a8xx device tables This commit splits the Adreno GPU device tables into separate a7xx and a8xx catalog files. It moves hardware configuration arrays and structs to these new files to organize generation without conflicts across GPU generations. > --- /dev/null > +++ b/drivers/gpu/drm/msm/adreno/a7xx_catalog.c > @@ -0,0 +1,496 @@ [ ... ] > +#include "a6xx_gpu.h" > +#include "a6xx.xml.h" > +#include "a6xx_gmu.xml.h" > + > +extern const struct adreno_reglist a640_gbif[]; > + [Severity: Low] Is it intentional to declare a640_gbif as an extern array directly inside a7xx_catalog.c? This bypasses type checking across translation units and causes -Wmissing-prototypes and -Wmissing-declarations warnings on W=1 builds. It is generally preferred to put shared declarations in a common header like a6xx_gpu.h instead of placing ad-hoc externs inside C files. > --- /dev/null > +++ b/drivers/gpu/drm/msm/adreno/a8xx_catalog.c > @@ -0,0 +1,1847 @@ [ ... ] > +static const uint32_t a810_pwrup_reglist_regs[] = { > + REG_A6XX_UCHE_MODE_CNTL, [ ... ] > + REG_A7XX_SP_READ_SEL, > + REG_A6XX_TPL1_NC_MODE_CNTL, > + REG_A6XX_TPL1_DBG_ECO_CNTL, > + REG_A6XX_TPL1_DBG_ECO_CNTL1, > + REG_A8XX_TPL1_BICUBIC_WEIGHTS_TABLE(1), > + REG_A8XX_TPL1_BICUBIC_WEIGHTS_TABLE(2), > + REG_A8XX_TPL1_BICUBIC_WEIGHTS_TABLE(3), [Severity: Medium] Does this sequence intentionally omit index 0 for the bicubic weights table? The a810_pwrup_reglist_regs array is used to restore critical hardware state on power-up for the A810 GPU. The bicubic weights table initialization in this array skips REG_A8XX_TPL1_BICUBIC_WEIGHTS_TABLE(0), directly jumping to index 1. Other GPUs (like a830, a840, a850, and a750) correctly include index 0 in their power-up lists. Missing the 0th index here means the weight will not be restored during power-up or preemption, potentially leading to unpredictable bicubic filtering behavior or data corruption on the GPU. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
