Re: [Freedreno] [PATCH 09/14] drm/msm/a6xx: Fix some A619 tunables

[Jordan Crouse]

On Thu, Jan 26, 2023 at 04:16:13PM +0100, Konrad Dybcio wrote:
> Adreno 619 expects some tunables to be set differently. Make up for it.
> 
> Fixes: b7616b5c69e6 ("drm/msm/adreno: Add A619 support")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 7a480705f407..f34ab3f39f09 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1171,6 +1171,8 @@ static int hw_init(struct msm_gpu *gpu)
>                 gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00200200);
>         else if (adreno_is_a650(adreno_gpu) || adreno_is_a660(adreno_gpu))
>                 gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00300200);
> +       else if (adreno_is_a619(adreno_gpu))
> +               gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00018000);
>         else if (adreno_is_a610(adreno_gpu))
>                 gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00080000);
>         else
> @@ -1188,7 +1190,9 @@ static int hw_init(struct msm_gpu *gpu)
>         a6xx_set_ubwc_config(gpu);
> 
>         /* Enable fault detection */
> -       if (adreno_is_a610(adreno_gpu))
> +       if (adreno_is_a619(adreno_gpu))
> +               gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 << 30) | 0x3fffff);
> +       else if (adreno_is_a610(adreno_gpu))
>                 gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 << 30) | 0x3ffff);
>         else
>                 gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 << 30) | 0x1fffff);

The number appended to the register is the number of clock ticks to wait
before declaring a hang. 0x3fffff happens to be the largest value that
can be set for the a6xx family (excepting the 610 which, IIRC, used older
hardware that had a smaller field for the counter). Downstream the
number would creep up over time as unexplained hangs were discovered and
diagnosed or covered up as "just wait longer".

So in theory you could leave this with the "default value" or even bump
up the default value to 0x3fffff for all targets if you wanted to. An
alternate solution (that downstream does) is to put this as a
pre-defined configuration in gpulist[].

Jordan