On Fri, Mar 4, 2022 at 1:47 PM Dmitry Baryshkov dmitry.baryshkov@linaro.org wrote:
On Fri, 4 Mar 2022 at 23:23, Rob Clark robdclark@gmail.com wrote:
From: Rob Clark robdclark@chromium.org
Fixes: f6d62d091cfd ("drm/msm/a6xx: add support for Adreno 660 GPU") Signed-off-by: Rob Clark robdclark@chromium.org
Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org However see the comment below.
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index 02b47977b5c3..6406d8c3411a 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -687,6 +687,7 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu)
BUILD_BUG_ON(ARRAY_SIZE(a6xx_protect) > 32); BUILD_BUG_ON(ARRAY_SIZE(a650_protect) > 48);
BUILD_BUG_ON(ARRAY_SIZE(a660_protect) > 48);
The magic number 32 and 48 are repeated through this code. I'd suggest to define them and use defined names. It can come up as a separate commit.
Or perhaps instead: ---- diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index 6406d8c3411a..58c371930fb4 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -683,20 +683,23 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); const u32 *regs = a6xx_protect; - unsigned i, count = ARRAY_SIZE(a6xx_protect), count_max = 32; - - BUILD_BUG_ON(ARRAY_SIZE(a6xx_protect) > 32); - BUILD_BUG_ON(ARRAY_SIZE(a650_protect) > 48); - BUILD_BUG_ON(ARRAY_SIZE(a660_protect) > 48); + unsigned i, count, count_max;
if (adreno_is_a650(adreno_gpu)) { regs = a650_protect; count = ARRAY_SIZE(a650_protect); count_max = 48; + BUILD_BUG_ON(ARRAY_SIZE(a650_protect) > 48); } else if (adreno_is_a660_family(adreno_gpu)) { regs = a660_protect; count = ARRAY_SIZE(a660_protect); count_max = 48; + BUILD_BUG_ON(ARRAY_SIZE(a660_protect) > 48); + } else { + regs = a6xx_protect; + count = ARRAY_SIZE(a6xx_protect); + count_max = 32; + BUILD_BUG_ON(ARRAY_SIZE(a6xx_protect) > 32); }
/* ----
that moves each of the two uses of constant together.. adding three #defines each used only twice seems a bit silly, IMHO
BR, -R