diff mbox series

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

Message ID 20230126151618.225127-10-konrad.dybcio@linaro.org (mailing list archive)
State Superseded
Headers show
Series GMU-less A6xx support (A610, A619_holi) | expand

Commit Message

Konrad Dybcio Jan. 26, 2023, 3:16 p.m. UTC
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@linaro.org>
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Jordan Crouse Feb. 8, 2023, 6:21 p.m. UTC | #1
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@linaro.org>
> ---
>  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
Konrad Dybcio Feb. 14, 2023, 11:25 a.m. UTC | #2
On 8.02.2023 19:21, Jordan Crouse wrote:
> 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@linaro.org>
>> ---
>>  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).
Makes sense!

Downstream the
> number would creep up over time as unexplained hangs were discovered and
> diagnosed or covered up as "just wait longer".
lol..

> 
> 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[].
I'm not sure it's a good idea to let things loose, as that may let some
bugs slip through.. Perhaps let's leave that as-is until we have a seriously
otherwise-unresolvable situation..

Konrad
> 
> Jordan
diff mbox series

Patch

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);