| Message ID | 20240218134434.2531636-1-dmitry.baryshkov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm/a6xx: skip programming of UBWC registers for a618 | expand |
On Sun, Feb 18, 2024 at 1:44 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > Historically the Adreno driver has not been updating memory > configuration registers on a618 (SC7180 platform) implying that the > default configuration is fine. After the rework performed in the commit > 8814455a0e54 ("drm/msm: Refactor UBWC config setting") the function > a6xx_calc_ubwc_config() still contained this shortcut and did not > calculate UBWC configuration. However the function which now actually > updates hardware registers, a6xx_set_ubwc_config(), doesn't contain such > check. Thus it ends up rewriting hardware registers with the default > (incorrect) values. Add the !a618 check to this function. > > Reported-by: Leonard Lausen <leonard@lausen.nl> > Link: https://gitlab.freedesktop.org/drm/msm/-/issues/49 > Fixes: 8814455a0e54 ("drm/msm: Refactor UBWC config setting") > Cc: Connor Abbott <cwabbott0@gmail.com> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > > Note, a proper fix would be to incorporate actual values for sc7180 > and drop the a618 shortcuts. However it might take some time to > materialize and to be properly tested. As such, I propose to merge this > for 6.8, keeping the existing behaviour. If we do this then there's a chance that 6.8 will report a broken value for MSM_PARAM_HIGHEST_BANK_BIT, which is going to require otherwise unnecessary workarounds in [1] for quite a while once I fix up a618 support. Can you at least dump what the default is to make sure that the value we report matches what's being programmed? [1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26578 Connor > > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index c9c55e2ea584..07d60dfacd23 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -1325,6 +1325,11 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu) > static void a6xx_set_ubwc_config(struct msm_gpu *gpu) > { > struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); > + > + /* a618 is using the hw default values */ > + if (adreno_is_a618(gpu)) > + return; > + > /* > * We subtract 13 from the highest bank bit (13 is the minimum value > * allowed by hw) and write the lowest two bits of the remaining value > -- > 2.39.2 >
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index c9c55e2ea584..07d60dfacd23 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -1325,6 +1325,11 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu) static void a6xx_set_ubwc_config(struct msm_gpu *gpu) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); + + /* a618 is using the hw default values */ + if (adreno_is_a618(gpu)) + return; + /* * We subtract 13 from the highest bank bit (13 is the minimum value * allowed by hw) and write the lowest two bits of the remaining value
Historically the Adreno driver has not been updating memory configuration registers on a618 (SC7180 platform) implying that the default configuration is fine. After the rework performed in the commit 8814455a0e54 ("drm/msm: Refactor UBWC config setting") the function a6xx_calc_ubwc_config() still contained this shortcut and did not calculate UBWC configuration. However the function which now actually updates hardware registers, a6xx_set_ubwc_config(), doesn't contain such check. Thus it ends up rewriting hardware registers with the default (incorrect) values. Add the !a618 check to this function. Reported-by: Leonard Lausen <leonard@lausen.nl> Link: https://gitlab.freedesktop.org/drm/msm/-/issues/49 Fixes: 8814455a0e54 ("drm/msm: Refactor UBWC config setting") Cc: Connor Abbott <cwabbott0@gmail.com> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- Note, a proper fix would be to incorporate actual values for sc7180 and drop the a618 shortcuts. However it might take some time to materialize and to be properly tested. As such, I propose to merge this for 6.8, keeping the existing behaviour. --- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 5 +++++ 1 file changed, 5 insertions(+)