On Sun, Feb 18, 2024 at 1:44 PM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> 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@xxxxxxxxx>
> Link: https://gitlab.freedesktop.org/drm/msm/-/issues/49
> Fixes: 8814455a0e54 ("drm/msm: Refactor UBWC config setting")
> Cc: Connor Abbott <cwabbott0@xxxxxxxxx>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
>
> 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
>
