diff mbox series

[2/2] drm/msm/a6xx: Print GMU core firmware version at boot

Message ID 20241219-topic-gmu_fw_ver-v1-2-d403a70052d8@oss.qualcomm.com
State New
Headers show
Series Print GMU version at boot on >A630 | expand

Commit Message

Konrad Dybcio Dec. 19, 2024, 10:36 p.m. UTC 
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Log the version for informational purposes, such as for keeping track
of possible GMU fw-related failures in crash / CI logs.

Intentionally not implemented on the if (gmu->legacy) codepath, as
these registers seem not to be used on there.

Downstream additionally warns if the firmware version is too old for
a given GPU, but we already pair the binary to a given GPU, so let's
not go there at the moment.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Abel Vesa Feb. 13, 2025, 4:35 p.m. UTC | #1 
On 24-12-19 23:36:56, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> Log the version for informational purposes, such as for keeping track
> of possible GMU fw-related failures in crash / CI logs.
> 
> Intentionally not implemented on the if (gmu->legacy) codepath, as
> these registers seem not to be used on there.
> 
> Downstream additionally warns if the firmware version is too old for
> a given GPU, but we already pair the binary to a given GPU, so let's
> not go there at the moment.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 14db7376c712d19446b38152e480bd5a1e0a5198..a7ea2b2af1dc3816906236df929df36e37d8f606 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -729,6 +729,7 @@ static int a6xx_gmu_fw_load(struct a6xx_gmu *gmu)
>  	const struct firmware *fw_image = adreno_gpu->fw[ADRENO_FW_GMU];
>  	const struct block_header *blk;
>  	u32 reg_offset;
> +	u32 ver;
>  
>  	u32 itcm_base = 0x00000000;
>  	u32 dtcm_base = 0x00040000;
> @@ -775,6 +776,12 @@ static int a6xx_gmu_fw_load(struct a6xx_gmu *gmu)
>  		}
>  	}
>  
> +	ver = gmu_read(gmu, REG_A6XX_GMU_CORE_FW_VERSION);
> +	DRM_INFO("Loaded GMU firmware v%u.%u.%u\n",
> +		 FIELD_GET(A6XX_GMU_CORE_FW_VERSION_MAJOR__MASK, ver),
> +		 FIELD_GET(A6XX_GMU_CORE_FW_VERSION_MINOR__MASK, ver),
> +		 FIELD_GET(A6XX_GMU_CORE_FW_VERSION_STEP__MASK, ver));

I get the following spam:

[  109.928924] [drm] Loaded GMU firmware v3.1.10
[  110.301295] [drm] Loaded GMU firmware v3.1.10
[  110.472620] [drm] Loaded GMU firmware v3.1.10
[  111.164303] [drm] Loaded GMU firmware v3.1.10
[  111.864830] [drm] Loaded GMU firmware v3.1.10
...

Stacktrace shows this being called from runtime resume:

[   19.380390] Call trace:
[   19.380391]  show_stack+0x18/0x24 (C)
[   19.380399]  dump_stack_lvl+0x40/0x84
[   19.380403]  dump_stack+0x18/0x24
[   19.380405]  a6xx_gmu_resume+0x450/0xc44 [msm]
[   19.380426]  a6xx_gmu_pm_resume+0x34/0x220 [msm]
[   19.380437]  adreno_runtime_resume+0x28/0x34 [msm]
[   19.380446]  pm_generic_runtime_resume+0x28/0x3c
[   19.380451]  __rpm_callback+0x84/0x390
[   19.380453]  rpm_resume+0x3d0/0x5c0
[   19.380455]  __pm_runtime_resume+0x4c/0x94
[   19.380457]  adreno_get_param+0xdc/0x274 [msm]
[   19.380466]  msm_ioctl_get_param+0x5c/0x68 [msm]
[   19.380475]  drm_ioctl_kernel+0xd4/0x10c [drm]
[   19.380491]  drm_ioctl+0x26c/0x40c [drm]
[   19.380499]  __arm64_sys_ioctl+0x90/0xcc
[   19.380503]  invoke_syscall+0x40/0xf8
[   19.380505]  el0_svc_common+0xac/0xdc
[   19.380506]  do_el0_svc+0x1c/0x28
[   19.380508]  el0_svc+0x34/0x7c
[   19.380512]  el0t_64_sync_handler+0x84/0x108
[   19.380513]  el0t_64_sync+0x198/0x19c

So maybe DRM_INFO_ONCE instead ?

> +
>  	return 0;
>  }
>  
> 
> -- 
> 2.47.1
> 
>
Thomas Zimmermann Feb. 13, 2025, 4:41 p.m. UTC | #2 
Hi

Am 13.02.25 um 17:35 schrieb Abel Vesa:
> On 24-12-19 23:36:56, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> Log the version for informational purposes, such as for keeping track
>> of possible GMU fw-related failures in crash / CI logs.
>>
>> Intentionally not implemented on the if (gmu->legacy) codepath, as
>> these registers seem not to be used on there.
>>
>> Downstream additionally warns if the firmware version is too old for
>> a given GPU, but we already pair the binary to a given GPU, so let's
>> not go there at the moment.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> ---
>>   drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>> index 14db7376c712d19446b38152e480bd5a1e0a5198..a7ea2b2af1dc3816906236df929df36e37d8f606 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>> @@ -729,6 +729,7 @@ static int a6xx_gmu_fw_load(struct a6xx_gmu *gmu)
>>   	const struct firmware *fw_image = adreno_gpu->fw[ADRENO_FW_GMU];
>>   	const struct block_header *blk;
>>   	u32 reg_offset;
>> +	u32 ver;
>>   
>>   	u32 itcm_base = 0x00000000;
>>   	u32 dtcm_base = 0x00040000;
>> @@ -775,6 +776,12 @@ static int a6xx_gmu_fw_load(struct a6xx_gmu *gmu)
>>   		}
>>   	}
>>   
>> +	ver = gmu_read(gmu, REG_A6XX_GMU_CORE_FW_VERSION);
>> +	DRM_INFO("Loaded GMU firmware v%u.%u.%u\n",
>> +		 FIELD_GET(A6XX_GMU_CORE_FW_VERSION_MAJOR__MASK, ver),
>> +		 FIELD_GET(A6XX_GMU_CORE_FW_VERSION_MINOR__MASK, ver),
>> +		 FIELD_GET(A6XX_GMU_CORE_FW_VERSION_STEP__MASK, ver));
> I get the following spam:
>
> [  109.928924] [drm] Loaded GMU firmware v3.1.10
> [  110.301295] [drm] Loaded GMU firmware v3.1.10
> [  110.472620] [drm] Loaded GMU firmware v3.1.10
> [  111.164303] [drm] Loaded GMU firmware v3.1.10
> [  111.864830] [drm] Loaded GMU firmware v3.1.10
> ...
>
> Stacktrace shows this being called from runtime resume:
>
> [   19.380390] Call trace:
> [   19.380391]  show_stack+0x18/0x24 (C)
> [   19.380399]  dump_stack_lvl+0x40/0x84
> [   19.380403]  dump_stack+0x18/0x24
> [   19.380405]  a6xx_gmu_resume+0x450/0xc44 [msm]
> [   19.380426]  a6xx_gmu_pm_resume+0x34/0x220 [msm]
> [   19.380437]  adreno_runtime_resume+0x28/0x34 [msm]
> [   19.380446]  pm_generic_runtime_resume+0x28/0x3c
> [   19.380451]  __rpm_callback+0x84/0x390
> [   19.380453]  rpm_resume+0x3d0/0x5c0
> [   19.380455]  __pm_runtime_resume+0x4c/0x94
> [   19.380457]  adreno_get_param+0xdc/0x274 [msm]
> [   19.380466]  msm_ioctl_get_param+0x5c/0x68 [msm]
> [   19.380475]  drm_ioctl_kernel+0xd4/0x10c [drm]
> [   19.380491]  drm_ioctl+0x26c/0x40c [drm]
> [   19.380499]  __arm64_sys_ioctl+0x90/0xcc
> [   19.380503]  invoke_syscall+0x40/0xf8
> [   19.380505]  el0_svc_common+0xac/0xdc
> [   19.380506]  do_el0_svc+0x1c/0x28
> [   19.380508]  el0_svc+0x34/0x7c
> [   19.380512]  el0t_64_sync_handler+0x84/0x108
> [   19.380513]  el0t_64_sync+0x198/0x19c
>
> So maybe DRM_INFO_ONCE instead ?

drm_dbg() seems appropriate. Most of the time, firmware versions are not 
interesting.

Best regards
Thomas

>
>> +
>>   	return 0;
>>   }
>>   
>>
>> -- 
>> 2.47.1
>>
>>
Konrad Dybcio Feb. 13, 2025, 4:47 p.m. UTC | #3 
On 13.02.2025 5:41 PM, Thomas Zimmermann wrote:
> Hi
> 
> Am 13.02.25 um 17:35 schrieb Abel Vesa:
>> On 24-12-19 23:36:56, Konrad Dybcio wrote:
>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>
>>> Log the version for informational purposes, such as for keeping track
>>> of possible GMU fw-related failures in crash / CI logs.
>>>
>>> Intentionally not implemented on the if (gmu->legacy) codepath, as
>>> these registers seem not to be used on there.
>>>
>>> Downstream additionally warns if the firmware version is too old for
>>> a given GPU, but we already pair the binary to a given GPU, so let's
>>> not go there at the moment.
>>>
>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>> ---

[...]

>> So maybe DRM_INFO_ONCE instead ?
> 
> drm_dbg() seems appropriate. Most of the time, firmware versions are not interesting.

Unfortunately, in our case they are, given users source them from all
kinds of places..

Konrad
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 14db7376c712d19446b38152e480bd5a1e0a5198..a7ea2b2af1dc3816906236df929df36e37d8f606 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -729,6 +729,7 @@  static int a6xx_gmu_fw_load(struct a6xx_gmu *gmu)
 	const struct firmware *fw_image = adreno_gpu->fw[ADRENO_FW_GMU];
 	const struct block_header *blk;
 	u32 reg_offset;
+	u32 ver;
 
 	u32 itcm_base = 0x00000000;
 	u32 dtcm_base = 0x00040000;
@@ -775,6 +776,12 @@  static int a6xx_gmu_fw_load(struct a6xx_gmu *gmu)
 		}
 	}
 
+	ver = gmu_read(gmu, REG_A6XX_GMU_CORE_FW_VERSION);
+	DRM_INFO("Loaded GMU firmware v%u.%u.%u\n",
+		 FIELD_GET(A6XX_GMU_CORE_FW_VERSION_MAJOR__MASK, ver),
+		 FIELD_GET(A6XX_GMU_CORE_FW_VERSION_MINOR__MASK, ver),
+		 FIELD_GET(A6XX_GMU_CORE_FW_VERSION_STEP__MASK, ver));
+
 	return 0;
 }