| Message ID | 20211022114349.102552-1-vladimir.lypak@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm/a5xx: Add support for Adreno 506 GPU | expand |
On Fri 22 Oct 04:43 PDT 2021, Vladimir Lypak wrote: > This GPU is found on SoCs such as MSM8953(650MHz), SDM450(600MHz), > SDM632(725MHz). > > Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com> > --- > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 34 ++++++++++++++-------- > drivers/gpu/drm/msm/adreno/adreno_device.c | 18 ++++++++++++ > drivers/gpu/drm/msm/adreno/adreno_gpu.h | 5 ++++ > 3 files changed, 45 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > index 5e2750eb3810..249a0d8bc673 100644 > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > @@ -441,7 +441,7 @@ void a5xx_set_hwcg(struct msm_gpu *gpu, bool state) > const struct adreno_five_hwcg_regs *regs; > unsigned int i, sz; > > - if (adreno_is_a508(adreno_gpu)) { > + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu)) { > regs = a50x_hwcg; > sz = ARRAY_SIZE(a50x_hwcg); > } else if (adreno_is_a509(adreno_gpu) || adreno_is_a512(adreno_gpu)) { > @@ -485,7 +485,7 @@ static int a5xx_me_init(struct msm_gpu *gpu) > OUT_RING(ring, 0x00000000); > > /* Specify workarounds for various microcode issues */ > - if (adreno_is_a530(adreno_gpu)) { > + if (adreno_is_a506(adreno_gpu) || adreno_is_a530(adreno_gpu)) { > /* Workaround for token end syncs > * Force a WFI after every direct-render 3D mode draw and every > * 2D mode 3 draw > @@ -620,8 +620,17 @@ static int a5xx_ucode_init(struct msm_gpu *gpu) > > static int a5xx_zap_shader_resume(struct msm_gpu *gpu) > { > + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); > int ret; > > + /* > + * Adreno 506,508,512 have CPZ Retention feature and > + * don't need to resume zap shader > + */ > + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu) || > + adreno_is_a512(adreno_gpu)) > + return 0; Afaict all other changes in the patch adds a506 support, but this hunk changes a508 and a512 behavior. I'm not saying that the change is wrong, but this hunk deserves to be in it's own patch - so that if there's any impact on those other versions it can be tracked down to that specific patch. Thanks, Bjorn
On 22/10/2021 20:33, Bjorn Andersson wrote: > On Fri 22 Oct 04:43 PDT 2021, Vladimir Lypak wrote: > >> This GPU is found on SoCs such as MSM8953(650MHz), SDM450(600MHz), >> SDM632(725MHz). >> >> Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com> >> --- >> drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 34 ++++++++++++++-------- >> drivers/gpu/drm/msm/adreno/adreno_device.c | 18 ++++++++++++ >> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 5 ++++ >> 3 files changed, 45 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c >> index 5e2750eb3810..249a0d8bc673 100644 >> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c >> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c >> @@ -441,7 +441,7 @@ void a5xx_set_hwcg(struct msm_gpu *gpu, bool state) >> const struct adreno_five_hwcg_regs *regs; >> unsigned int i, sz; >> >> - if (adreno_is_a508(adreno_gpu)) { >> + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu)) { >> regs = a50x_hwcg; >> sz = ARRAY_SIZE(a50x_hwcg); >> } else if (adreno_is_a509(adreno_gpu) || adreno_is_a512(adreno_gpu)) { >> @@ -485,7 +485,7 @@ static int a5xx_me_init(struct msm_gpu *gpu) >> OUT_RING(ring, 0x00000000); >> >> /* Specify workarounds for various microcode issues */ >> - if (adreno_is_a530(adreno_gpu)) { >> + if (adreno_is_a506(adreno_gpu) || adreno_is_a530(adreno_gpu)) { >> /* Workaround for token end syncs >> * Force a WFI after every direct-render 3D mode draw and every >> * 2D mode 3 draw >> @@ -620,8 +620,17 @@ static int a5xx_ucode_init(struct msm_gpu *gpu) >> >> static int a5xx_zap_shader_resume(struct msm_gpu *gpu) >> { >> + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); >> int ret; >> >> + /* >> + * Adreno 506,508,512 have CPZ Retention feature and >> + * don't need to resume zap shader >> + */ >> + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu) || >> + adreno_is_a512(adreno_gpu)) >> + return 0; > > Afaict all other changes in the patch adds a506 support, but this hunk > changes a508 and a512 behavior. > > I'm not saying that the change is wrong, but this hunk deserves to be in > it's own patch - so that if there's any impact on those other versions > it can be tracked down to that specific patch. Vladimir, any plans to submit v2? This comment requests splitting the patch in two.
On Fri, Oct 22, 2021 at 10:33:29AM -0700, Bjorn Andersson wrote: > On Fri 22 Oct 04:43 PDT 2021, Vladimir Lypak wrote: > > > This GPU is found on SoCs such as MSM8953(650MHz), SDM450(600MHz), > > SDM632(725MHz). > > > > Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com> > > --- > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 34 ++++++++++++++-------- > > drivers/gpu/drm/msm/adreno/adreno_device.c | 18 ++++++++++++ > > drivers/gpu/drm/msm/adreno/adreno_gpu.h | 5 ++++ > > 3 files changed, 45 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > > index 5e2750eb3810..249a0d8bc673 100644 > > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > > @@ -441,7 +441,7 @@ void a5xx_set_hwcg(struct msm_gpu *gpu, bool state) > > const struct adreno_five_hwcg_regs *regs; > > unsigned int i, sz; > > > > - if (adreno_is_a508(adreno_gpu)) { > > + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu)) { > > regs = a50x_hwcg; > > sz = ARRAY_SIZE(a50x_hwcg); > > } else if (adreno_is_a509(adreno_gpu) || adreno_is_a512(adreno_gpu)) { > > @@ -485,7 +485,7 @@ static int a5xx_me_init(struct msm_gpu *gpu) > > OUT_RING(ring, 0x00000000); > > > > /* Specify workarounds for various microcode issues */ > > - if (adreno_is_a530(adreno_gpu)) { > > + if (adreno_is_a506(adreno_gpu) || adreno_is_a530(adreno_gpu)) { > > /* Workaround for token end syncs > > * Force a WFI after every direct-render 3D mode draw and every > > * 2D mode 3 draw > > @@ -620,8 +620,17 @@ static int a5xx_ucode_init(struct msm_gpu *gpu) > > > > static int a5xx_zap_shader_resume(struct msm_gpu *gpu) > > { > > + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); > > int ret; > > > > + /* > > + * Adreno 506,508,512 have CPZ Retention feature and > > + * don't need to resume zap shader > > + */ > > + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu) || > > + adreno_is_a512(adreno_gpu)) > > + return 0; > > Afaict all other changes in the patch adds a506 support, but this hunk > changes a508 and a512 behavior. > > I'm not saying that the change is wrong, but this hunk deserves to be in > it's own patch - so that if there's any impact on those other versions > it can be tracked down to that specific patch. > > Thanks, > Bjorn Hello, Bjorn. You're right on that. I'm going to remove those changes in V2, since that SCM call only causes problems on A506 and i can't test it on other GPUs. Best regards, Vladimir
On Fri, Dec 03, 2021 at 09:51:10PM +0300, Dmitry Baryshkov wrote: > On 22/10/2021 20:33, Bjorn Andersson wrote: > > On Fri 22 Oct 04:43 PDT 2021, Vladimir Lypak wrote: > > > > > This GPU is found on SoCs such as MSM8953(650MHz), SDM450(600MHz), > > > SDM632(725MHz). > > > > > > Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com> > > > --- > > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 34 ++++++++++++++-------- > > > drivers/gpu/drm/msm/adreno/adreno_device.c | 18 ++++++++++++ > > > drivers/gpu/drm/msm/adreno/adreno_gpu.h | 5 ++++ > > > 3 files changed, 45 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > > > index 5e2750eb3810..249a0d8bc673 100644 > > > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > > > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > > > @@ -441,7 +441,7 @@ void a5xx_set_hwcg(struct msm_gpu *gpu, bool state) > > > const struct adreno_five_hwcg_regs *regs; > > > unsigned int i, sz; > > > - if (adreno_is_a508(adreno_gpu)) { > > > + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu)) { > > > regs = a50x_hwcg; > > > sz = ARRAY_SIZE(a50x_hwcg); > > > } else if (adreno_is_a509(adreno_gpu) || adreno_is_a512(adreno_gpu)) { > > > @@ -485,7 +485,7 @@ static int a5xx_me_init(struct msm_gpu *gpu) > > > OUT_RING(ring, 0x00000000); > > > /* Specify workarounds for various microcode issues */ > > > - if (adreno_is_a530(adreno_gpu)) { > > > + if (adreno_is_a506(adreno_gpu) || adreno_is_a530(adreno_gpu)) { > > > /* Workaround for token end syncs > > > * Force a WFI after every direct-render 3D mode draw and every > > > * 2D mode 3 draw > > > @@ -620,8 +620,17 @@ static int a5xx_ucode_init(struct msm_gpu *gpu) > > > static int a5xx_zap_shader_resume(struct msm_gpu *gpu) > > > { > > > + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); > > > int ret; > > > + /* > > > + * Adreno 506,508,512 have CPZ Retention feature and > > > + * don't need to resume zap shader > > > + */ > > > + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu) || > > > + adreno_is_a512(adreno_gpu)) > > > + return 0; > > > > Afaict all other changes in the patch adds a506 support, but this hunk > > changes a508 and a512 behavior. > > > > I'm not saying that the change is wrong, but this hunk deserves to be in > > it's own patch - so that if there's any impact on those other versions > > it can be tracked down to that specific patch. > > Vladimir, any plans to submit v2? This comment requests splitting the patch > in two. > > > -- > With best wishes > Dmitry Hello, Dmitry! Sorry for delay. Yes, i'm going to submit v2 soon. Thanks Vladimir
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c index 5e2750eb3810..249a0d8bc673 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c @@ -441,7 +441,7 @@ void a5xx_set_hwcg(struct msm_gpu *gpu, bool state) const struct adreno_five_hwcg_regs *regs; unsigned int i, sz; - if (adreno_is_a508(adreno_gpu)) { + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu)) { regs = a50x_hwcg; sz = ARRAY_SIZE(a50x_hwcg); } else if (adreno_is_a509(adreno_gpu) || adreno_is_a512(adreno_gpu)) { @@ -485,7 +485,7 @@ static int a5xx_me_init(struct msm_gpu *gpu) OUT_RING(ring, 0x00000000); /* Specify workarounds for various microcode issues */ - if (adreno_is_a530(adreno_gpu)) { + if (adreno_is_a506(adreno_gpu) || adreno_is_a530(adreno_gpu)) { /* Workaround for token end syncs * Force a WFI after every direct-render 3D mode draw and every * 2D mode 3 draw @@ -620,8 +620,17 @@ static int a5xx_ucode_init(struct msm_gpu *gpu) static int a5xx_zap_shader_resume(struct msm_gpu *gpu) { + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); int ret; + /* + * Adreno 506,508,512 have CPZ Retention feature and + * don't need to resume zap shader + */ + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu) || + adreno_is_a512(adreno_gpu)) + return 0; + ret = qcom_scm_set_remote_state(SCM_GPU_ZAP_SHADER_RESUME, GPU_PAS_ID); if (ret) DRM_ERROR("%s: zap-shader resume failed: %d\n", @@ -733,9 +742,10 @@ static int a5xx_hw_init(struct msm_gpu *gpu) 0x00100000 + adreno_gpu->gmem - 1); gpu_write(gpu, REG_A5XX_UCHE_GMEM_RANGE_MAX_HI, 0x00000000); - if (adreno_is_a508(adreno_gpu) || adreno_is_a510(adreno_gpu)) { + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu) || + adreno_is_a510(adreno_gpu)) { gpu_write(gpu, REG_A5XX_CP_MEQ_THRESHOLDS, 0x20); - if (adreno_is_a508(adreno_gpu)) + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu)) gpu_write(gpu, REG_A5XX_CP_MERCIU_SIZE, 0x400); else gpu_write(gpu, REG_A5XX_CP_MERCIU_SIZE, 0x20); @@ -751,7 +761,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu) gpu_write(gpu, REG_A5XX_CP_ROQ_THRESHOLDS_1, 0x40201B16); } - if (adreno_is_a508(adreno_gpu)) + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu)) gpu_write(gpu, REG_A5XX_PC_DBG_ECO_CNTL, (0x100 << 11 | 0x100 << 22)); else if (adreno_is_a509(adreno_gpu) || adreno_is_a510(adreno_gpu) || @@ -769,8 +779,8 @@ static int a5xx_hw_init(struct msm_gpu *gpu) * Disable the RB sampler datapath DP2 clock gating optimization * for 1-SP GPUs, as it is enabled by default. */ - if (adreno_is_a508(adreno_gpu) || adreno_is_a509(adreno_gpu) || - adreno_is_a512(adreno_gpu)) + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu) || + adreno_is_a509(adreno_gpu) || adreno_is_a512(adreno_gpu)) gpu_rmw(gpu, REG_A5XX_RB_DBG_ECO_CNTL, 0, (1 << 9)); /* Disable UCHE global filter as SP can invalidate/flush independently */ @@ -895,8 +905,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu) if (ret) return ret; - if (!(adreno_is_a508(adreno_gpu) || adreno_is_a509(adreno_gpu) || - adreno_is_a510(adreno_gpu) || adreno_is_a512(adreno_gpu))) + if (adreno_is_a530(adreno_gpu) || adreno_is_a540(adreno_gpu)) a5xx_gpmu_ucode_init(gpu); ret = a5xx_ucode_init(gpu); @@ -1338,7 +1347,7 @@ static int a5xx_pm_resume(struct msm_gpu *gpu) if (ret) return ret; - /* Adreno 508, 509, 510, 512 needs manual RBBM sus/res control */ + /* Adreno 506, 508, 509, 510, 512 needs manual RBBM sus/res control */ if (!(adreno_is_a530(adreno_gpu) || adreno_is_a540(adreno_gpu))) { /* Halt the sp_input_clk at HM level */ gpu_write(gpu, REG_A5XX_RBBM_CLOCK_CNTL, 0x00000055); @@ -1381,8 +1390,9 @@ static int a5xx_pm_suspend(struct msm_gpu *gpu) u32 mask = 0xf; int i, ret; - /* A508, A510 have 3 XIN ports in VBIF */ - if (adreno_is_a508(adreno_gpu) || adreno_is_a510(adreno_gpu)) + /* A506, A508, A510 have 3 XIN ports in VBIF */ + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu) || + adreno_is_a510(adreno_gpu)) mask = 0x7; /* Clear the VBIF pipe before shutting down */ diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c index 2a6ce76656aa..eb4b8527153b 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_device.c +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c @@ -131,6 +131,24 @@ static const struct adreno_info gpulist[] = { .gmem = (SZ_1M + SZ_512K), .inactive_period = DRM_MSM_INACTIVE_PERIOD, .init = a4xx_gpu_init, + }, { + .rev = ADRENO_REV(5, 0, 6, ANY_ID), + .revn = 506, + .name = "A506", + .fw = { + [ADRENO_FW_PM4] = "a530_pm4.fw", + [ADRENO_FW_PFP] = "a530_pfp.fw", + }, + .gmem = (SZ_128K + SZ_8K), + /* + * Increase inactive period to 250 to avoid bouncing + * the GDSC which appears to make it grumpy + */ + .inactive_period = 250, + .quirks = ADRENO_QUIRK_TWO_PASS_USE_WFI | + ADRENO_QUIRK_LMLOADKILL_DISABLE, + .init = a5xx_gpu_init, + .zapfw = "a506_zap.mdt", }, { .rev = ADRENO_REV(5, 0, 8, ANY_ID), .revn = 508, diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h index 225c277a6223..427a96d81280 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h @@ -201,6 +201,11 @@ static inline int adreno_is_a430(struct adreno_gpu *gpu) return gpu->revn == 430; } +static inline int adreno_is_a506(struct adreno_gpu *gpu) +{ + return gpu->revn == 506; +} + static inline int adreno_is_a508(struct adreno_gpu *gpu) { return gpu->revn == 508;
This GPU is found on SoCs such as MSM8953(650MHz), SDM450(600MHz), SDM632(725MHz). Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com> --- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 34 ++++++++++++++-------- drivers/gpu/drm/msm/adreno/adreno_device.c | 18 ++++++++++++ drivers/gpu/drm/msm/adreno/adreno_gpu.h | 5 ++++ 3 files changed, 45 insertions(+), 12 deletions(-)