AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd/pm: Fix incomplete null pointer issue for smu v13.0.6
@ 2025-10-27  7:27 Yang Wang
  2025-10-27  7:43 ` Lazar, Lijo
  0 siblings, 1 reply; 5+ messages in thread
From: Yang Wang @ 2025-10-27  7:27 UTC (permalink / raw)
  To: amd-gfx; +Cc: hawking.zhang, alexander.deucher, lijo.lazar, asad.kamal

the smu v13.0.6 driver should handle return value of smu_v13_0_6_print_clks()
to avoid null pointer issue.

Fixes: 0354cd650daa ("drm/amd/pm: Avoid writing nulls into `pp_od_clk_voltage`")

Signed-off-by: Yang Wang <kevinyang.wang@amd.com>
---
 .../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c  | 41 ++++++++++++++-----
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
index 39ae6701147c..22fe4d3508fd 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
@@ -1514,9 +1514,14 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu,
 
 		single_dpm_table = &(dpm_context->dpm_tables.uclk_table);
 
-		return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
-					      now, "mclk");
+		ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
+					     now, "mclk");
+		if (ret < 0)
+			return ret;
+
+		size += ret;
 
+		break;
 	case SMU_SOCCLK:
 		ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_SOCCLK,
 								&now);
@@ -1528,9 +1533,13 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu,
 
 		single_dpm_table = &(dpm_context->dpm_tables.soc_table);
 
-		return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
-					      now, "socclk");
+		ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
+					     now, "socclk");
+		if (ret < 0)
+			return ret;
 
+		size += ret;
+		break;
 	case SMU_FCLK:
 		ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_FCLK,
 								&now);
@@ -1542,9 +1551,13 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu,
 
 		single_dpm_table = &(dpm_context->dpm_tables.fclk_table);
 
-		return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
-					      now, "fclk");
+		ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
+					     now, "fclk");
+		if (ret < 0)
+			return ret;
 
+		size += ret;
+		break;
 	case SMU_VCLK:
 		ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_VCLK,
 								&now);
@@ -1556,9 +1569,13 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu,
 
 		single_dpm_table = &(dpm_context->dpm_tables.vclk_table);
 
-		return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
-					      now, "vclk");
+		ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
+					     now, "vclk");
+		if (ret < 0)
+			return ret;
 
+		size += ret;
+		break;
 	case SMU_DCLK:
 		ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_DCLK,
 							       &now);
@@ -1570,9 +1587,13 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu,
 
 		single_dpm_table = &(dpm_context->dpm_tables.dclk_table);
 
-		return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
-					      now, "dclk");
+		ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
+					     now, "dclk");
+		if (ret < 0)
+			return ret;
 
+		size += ret;
+		break;
 	default:
 		break;
 	}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/amd/pm: Fix incomplete null pointer issue for smu v13.0.6
  2025-10-27  7:27 [PATCH] drm/amd/pm: Fix incomplete null pointer issue for smu v13.0.6 Yang Wang
@ 2025-10-27  7:43 ` Lazar, Lijo
  2025-10-27  7:46   ` Lazar, Lijo
  0 siblings, 1 reply; 5+ messages in thread
From: Lazar, Lijo @ 2025-10-27  7:43 UTC (permalink / raw)
  To: Yang Wang, amd-gfx; +Cc: hawking.zhang, alexander.deucher, asad.kamal



On 10/27/2025 12:57 PM, Yang Wang wrote:
> the smu v13.0.6 driver should handle return value of smu_v13_0_6_print_clks()
> to avoid null pointer issue.
> 
> Fixes: 0354cd650daa ("drm/amd/pm: Avoid writing nulls into `pp_od_clk_voltage`")
> 
> Signed-off-by: Yang Wang <kevinyang.wang@amd.com>
> ---
>   .../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c  | 41 ++++++++++++++-----
>   1 file changed, 31 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> index 39ae6701147c..22fe4d3508fd 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> @@ -1514,9 +1514,14 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu,
>   
>   		single_dpm_table = &(dpm_context->dpm_tables.uclk_table);
>   
> -		return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
> -					      now, "mclk");
> +		ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
> +					     now, "mclk");

Probably the fix needs to be in smu_v13_0_6_print_clks?


                         size += sysfs_emit_at(buf, size, "%d: %uMhz 
%s\n", i,
                                               clk1, (level == i) ? "*" 
: "");

'size += to size =' so that it returns only the total size emitted by 
the function.

That is the case for this condition now -

if (curr_clk < SMU_13_0_6_DSCLK_THRESHOLD)

Thanks,
Lijo

> +		if (ret < 0)
> +			return ret;
> +
> +		size += ret;
>   
> +		break;
>   	case SMU_SOCCLK:
>   		ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_SOCCLK,
>   								&now);
> @@ -1528,9 +1533,13 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu,
>   
>   		single_dpm_table = &(dpm_context->dpm_tables.soc_table);
>   
> -		return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
> -					      now, "socclk");
> +		ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
> +					     now, "socclk");
> +		if (ret < 0)
> +			return ret;
>   
> +		size += ret;
> +		break;
>   	case SMU_FCLK:
>   		ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_FCLK,
>   								&now);
> @@ -1542,9 +1551,13 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu,
>   
>   		single_dpm_table = &(dpm_context->dpm_tables.fclk_table);
>   
> -		return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
> -					      now, "fclk");
> +		ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
> +					     now, "fclk");
> +		if (ret < 0)
> +			return ret;
>   
> +		size += ret;
> +		break;
>   	case SMU_VCLK:
>   		ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_VCLK,
>   								&now);
> @@ -1556,9 +1569,13 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu,
>   
>   		single_dpm_table = &(dpm_context->dpm_tables.vclk_table);
>   
> -		return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
> -					      now, "vclk");
> +		ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
> +					     now, "vclk");
> +		if (ret < 0)
> +			return ret;
>   
> +		size += ret;
> +		break;
>   	case SMU_DCLK:
>   		ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_DCLK,
>   							       &now);
> @@ -1570,9 +1587,13 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu,
>   
>   		single_dpm_table = &(dpm_context->dpm_tables.dclk_table);
>   
> -		return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
> -					      now, "dclk");
> +		ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
> +					     now, "dclk");
> +		if (ret < 0)
> +			return ret;
>   
> +		size += ret;
> +		break;
>   	default:
>   		break;
>   	}


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/amd/pm: Fix incomplete null pointer issue for smu v13.0.6
  2025-10-27  7:43 ` Lazar, Lijo
@ 2025-10-27  7:46   ` Lazar, Lijo
  2025-10-27  7:56     ` Wang, Yang(Kevin)
  0 siblings, 1 reply; 5+ messages in thread
From: Lazar, Lijo @ 2025-10-27  7:46 UTC (permalink / raw)
  To: Yang Wang, amd-gfx; +Cc: hawking.zhang, alexander.deucher, asad.kamal



On 10/27/2025 1:13 PM, Lazar, Lijo wrote:
> 
> 
> On 10/27/2025 12:57 PM, Yang Wang wrote:
>> the smu v13.0.6 driver should handle return value of 
>> smu_v13_0_6_print_clks()
>> to avoid null pointer issue.
>>
>> Fixes: 0354cd650daa ("drm/amd/pm: Avoid writing nulls into 
>> `pp_od_clk_voltage`")
>>
>> Signed-off-by: Yang Wang <kevinyang.wang@amd.com>
>> ---
>>   .../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c  | 41 ++++++++++++++-----
>>   1 file changed, 31 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c b/ 
>> drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
>> index 39ae6701147c..22fe4d3508fd 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
>> @@ -1514,9 +1514,14 @@ static int smu_v13_0_6_print_clk_levels(struct 
>> smu_context *smu,
>>           single_dpm_table = &(dpm_context->dpm_tables.uclk_table);
>> -        return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
>> -                          now, "mclk");
>> +        ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
>> +                         now, "mclk");
> 
> Probably the fix needs to be in smu_v13_0_6_print_clks?
> 
> 
>                          size += sysfs_emit_at(buf, size, "%d: %uMhz 
> %s\n", i,
>                                                clk1, (level == i) ? 
> "*" : "");
> 
> 'size += to size =' so that it returns only the total size emitted by 
> the function.
> 

Never mind, this is not going to work. The function may return the total 
size it emitted, or it also needs to adjust the below condition.

Thanks,
Lijo

> That is the case for this condition now -
> 
> if (curr_clk < SMU_13_0_6_DSCLK_THRESHOLD)
> 
> Thanks,
> Lijo
> 
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        size += ret;
>> +        break;
>>       case SMU_SOCCLK:
>>           ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, 
>> SMU_SOCCLK,
>>                                   &now);
>> @@ -1528,9 +1533,13 @@ static int smu_v13_0_6_print_clk_levels(struct 
>> smu_context *smu,
>>           single_dpm_table = &(dpm_context->dpm_tables.soc_table);
>> -        return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
>> -                          now, "socclk");
>> +        ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
>> +                         now, "socclk");
>> +        if (ret < 0)
>> +            return ret;
>> +        size += ret;
>> +        break;
>>       case SMU_FCLK:
>>           ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_FCLK,
>>                                   &now);
>> @@ -1542,9 +1551,13 @@ static int smu_v13_0_6_print_clk_levels(struct 
>> smu_context *smu,
>>           single_dpm_table = &(dpm_context->dpm_tables.fclk_table);
>> -        return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
>> -                          now, "fclk");
>> +        ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
>> +                         now, "fclk");
>> +        if (ret < 0)
>> +            return ret;
>> +        size += ret;
>> +        break;
>>       case SMU_VCLK:
>>           ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_VCLK,
>>                                   &now);
>> @@ -1556,9 +1569,13 @@ static int smu_v13_0_6_print_clk_levels(struct 
>> smu_context *smu,
>>           single_dpm_table = &(dpm_context->dpm_tables.vclk_table);
>> -        return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
>> -                          now, "vclk");
>> +        ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
>> +                         now, "vclk");
>> +        if (ret < 0)
>> +            return ret;
>> +        size += ret;
>> +        break;
>>       case SMU_DCLK:
>>           ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_DCLK,
>>                                      &now);
>> @@ -1570,9 +1587,13 @@ static int smu_v13_0_6_print_clk_levels(struct 
>> smu_context *smu,
>>           single_dpm_table = &(dpm_context->dpm_tables.dclk_table);
>> -        return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
>> -                          now, "dclk");
>> +        ret = smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table,
>> +                         now, "dclk");
>> +        if (ret < 0)
>> +            return ret;
>> +        size += ret;
>> +        break;
>>       default:
>>           break;
>>       }
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] drm/amd/pm: Fix incomplete null pointer issue for smu v13.0.6
  2025-10-27  7:46   ` Lazar, Lijo
@ 2025-10-27  7:56     ` Wang, Yang(Kevin)
  2025-10-27  8:06       ` Lazar, Lijo
  0 siblings, 1 reply; 5+ messages in thread
From: Wang, Yang(Kevin) @ 2025-10-27  7:56 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx@lists.freedesktop.org
  Cc: Zhang, Hawking, Deucher, Alexander, Kamal, Asad

[AMD Official Use Only - AMD Internal Distribution Only]

>> Probably the fix needs to be in smu_v13_0_6_print_clks?

I would like to keep the current code design where there are centralized exit points for called functions to match the kernel coding style requirement..

https://www.kernel.org/doc/html/v6.16/process/coding-style.html
7) Centralized exiting of functions

Best Regards,
Kevin

-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Monday, October 27, 2025 15:46
To: Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>
Subject: Re: [PATCH] drm/amd/pm: Fix incomplete null pointer issue for smu v13.0.6



On 10/27/2025 1:13 PM, Lazar, Lijo wrote:
>
>
> On 10/27/2025 12:57 PM, Yang Wang wrote:
>> the smu v13.0.6 driver should handle return value of
>> smu_v13_0_6_print_clks()
>> to avoid null pointer issue.
>>
>> Fixes: 0354cd650daa ("drm/amd/pm: Avoid writing nulls into
>> `pp_od_clk_voltage`")
>>
>> Signed-off-by: Yang Wang <kevinyang.wang@amd.com>
>> ---
>>   .../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c  | 41
>> ++++++++++++++-----
>>   1 file changed, 31 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c b/
>> drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
>> index 39ae6701147c..22fe4d3508fd 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
>> @@ -1514,9 +1514,14 @@ static int smu_v13_0_6_print_clk_levels(struct
>> smu_context *smu,
>>           single_dpm_table = &(dpm_context->dpm_tables.uclk_table);
>> -        return smu_v13_0_6_print_clks(smu, buf, size,
>> single_dpm_table,
>> -                          now, "mclk");
>> +        ret = smu_v13_0_6_print_clks(smu, buf, size,
>> +single_dpm_table,
>> +                         now, "mclk");
>
> Probably the fix needs to be in smu_v13_0_6_print_clks?
>
>
>                          size += sysfs_emit_at(buf, size, "%d: %uMhz
> %s\n", i,
>                                                clk1, (level == i) ?
> "*" : "");
>
> 'size += to size =' so that it returns only the total size emitted by
> the function.
>

Never mind, this is not going to work. The function may return the total size it emitted, or it also needs to adjust the below condition.

Thanks,
Lijo

> That is the case for this condition now -
>
> if (curr_clk < SMU_13_0_6_DSCLK_THRESHOLD)
>
> Thanks,
> Lijo
>
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        size += ret;
>> +        break;
>>       case SMU_SOCCLK:
>>           ret = smu_v13_0_6_get_current_clk_freq_by_table(smu,
>> SMU_SOCCLK,
>>                                   &now); @@ -1528,9 +1533,13 @@
>> static int smu_v13_0_6_print_clk_levels(struct
>> smu_context *smu,
>>           single_dpm_table = &(dpm_context->dpm_tables.soc_table);
>> -        return smu_v13_0_6_print_clks(smu, buf, size,
>> single_dpm_table,
>> -                          now, "socclk");
>> +        ret = smu_v13_0_6_print_clks(smu, buf, size,
>> +single_dpm_table,
>> +                         now, "socclk");
>> +        if (ret < 0)
>> +            return ret;
>> +        size += ret;
>> +        break;
>>       case SMU_FCLK:
>>           ret = smu_v13_0_6_get_current_clk_freq_by_table(smu,
>> SMU_FCLK,
>>                                   &now); @@ -1542,9 +1551,13 @@
>> static int smu_v13_0_6_print_clk_levels(struct
>> smu_context *smu,
>>           single_dpm_table = &(dpm_context->dpm_tables.fclk_table);
>> -        return smu_v13_0_6_print_clks(smu, buf, size,
>> single_dpm_table,
>> -                          now, "fclk");
>> +        ret = smu_v13_0_6_print_clks(smu, buf, size,
>> +single_dpm_table,
>> +                         now, "fclk");
>> +        if (ret < 0)
>> +            return ret;
>> +        size += ret;
>> +        break;
>>       case SMU_VCLK:
>>           ret = smu_v13_0_6_get_current_clk_freq_by_table(smu,
>> SMU_VCLK,
>>                                   &now); @@ -1556,9 +1569,13 @@
>> static int smu_v13_0_6_print_clk_levels(struct
>> smu_context *smu,
>>           single_dpm_table = &(dpm_context->dpm_tables.vclk_table);
>> -        return smu_v13_0_6_print_clks(smu, buf, size,
>> single_dpm_table,
>> -                          now, "vclk");
>> +        ret = smu_v13_0_6_print_clks(smu, buf, size,
>> +single_dpm_table,
>> +                         now, "vclk");
>> +        if (ret < 0)
>> +            return ret;
>> +        size += ret;
>> +        break;
>>       case SMU_DCLK:
>>           ret = smu_v13_0_6_get_current_clk_freq_by_table(smu,
>> SMU_DCLK,
>>                                      &now); @@ -1570,9 +1587,13 @@
>> static int smu_v13_0_6_print_clk_levels(struct
>> smu_context *smu,
>>           single_dpm_table = &(dpm_context->dpm_tables.dclk_table);
>> -        return smu_v13_0_6_print_clks(smu, buf, size,
>> single_dpm_table,
>> -                          now, "dclk");
>> +        ret = smu_v13_0_6_print_clks(smu, buf, size,
>> +single_dpm_table,
>> +                         now, "dclk");
>> +        if (ret < 0)
>> +            return ret;
>> +        size += ret;
>> +        break;
>>       default:
>>           break;
>>       }
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/amd/pm: Fix incomplete null pointer issue for smu v13.0.6
  2025-10-27  7:56     ` Wang, Yang(Kevin)
@ 2025-10-27  8:06       ` Lazar, Lijo
  0 siblings, 0 replies; 5+ messages in thread
From: Lazar, Lijo @ 2025-10-27  8:06 UTC (permalink / raw)
  To: Wang, Yang(Kevin), amd-gfx@lists.freedesktop.org
  Cc: Zhang, Hawking, Deucher, Alexander, Kamal, Asad



On 10/27/2025 1:26 PM, Wang, Yang(Kevin) wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
>>> Probably the fix needs to be in smu_v13_0_6_print_clks?
> 
> I would like to keep the current code design where there are centralized exit points for called functions to match the kernel coding style requirement..
> 
> https://www.kernel.org/doc/html/v6.16/process/coding-style.html
> 7) Centralized exiting of functions
> 

Not seeing a case for cleanup or multiple unwind steps in common exit 
point.

Anyway, if you prefer to keep this style, below one also needs a fix 
inside smu_v13_0_6_print_clks()

         if (curr_clk < SMU_13_0_6_DSCLK_THRESHOLD) {
                 size = sysfs_emit_at(buf, size, "S: %uMhz *\n", curr_clk);


Thanks,
Lijo

> Best Regards,
> Kevin
> 
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Monday, October 27, 2025 15:46
> To: Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>
> Subject: Re: [PATCH] drm/amd/pm: Fix incomplete null pointer issue for smu v13.0.6
> 
> 
> 
> On 10/27/2025 1:13 PM, Lazar, Lijo wrote:
>>
>>
>> On 10/27/2025 12:57 PM, Yang Wang wrote:
>>> the smu v13.0.6 driver should handle return value of
>>> smu_v13_0_6_print_clks()
>>> to avoid null pointer issue.
>>>
>>> Fixes: 0354cd650daa ("drm/amd/pm: Avoid writing nulls into
>>> `pp_od_clk_voltage`")
>>>
>>> Signed-off-by: Yang Wang <kevinyang.wang@amd.com>
>>> ---
>>>    .../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c  | 41
>>> ++++++++++++++-----
>>>    1 file changed, 31 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c b/
>>> drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
>>> index 39ae6701147c..22fe4d3508fd 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
>>> @@ -1514,9 +1514,14 @@ static int smu_v13_0_6_print_clk_levels(struct
>>> smu_context *smu,
>>>            single_dpm_table = &(dpm_context->dpm_tables.uclk_table);
>>> -        return smu_v13_0_6_print_clks(smu, buf, size,
>>> single_dpm_table,
>>> -                          now, "mclk");
>>> +        ret = smu_v13_0_6_print_clks(smu, buf, size,
>>> +single_dpm_table,
>>> +                         now, "mclk");
>>
>> Probably the fix needs to be in smu_v13_0_6_print_clks?
>>
>>
>>                           size += sysfs_emit_at(buf, size, "%d: %uMhz
>> %s\n", i,
>>                                                 clk1, (level == i) ?
>> "*" : "");
>>
>> 'size += to size =' so that it returns only the total size emitted by
>> the function.
>>
> 
> Never mind, this is not going to work. The function may return the total size it emitted, or it also needs to adjust the below condition.
> 
> Thanks,
> Lijo
> 
>> That is the case for this condition now -
>>
>> if (curr_clk < SMU_13_0_6_DSCLK_THRESHOLD)
>>
>> Thanks,
>> Lijo
>>
>>> +        if (ret < 0)
>>> +            return ret;
>>> +
>>> +        size += ret;
>>> +        break;
>>>        case SMU_SOCCLK:
>>>            ret = smu_v13_0_6_get_current_clk_freq_by_table(smu,
>>> SMU_SOCCLK,
>>>                                    &now); @@ -1528,9 +1533,13 @@
>>> static int smu_v13_0_6_print_clk_levels(struct
>>> smu_context *smu,
>>>            single_dpm_table = &(dpm_context->dpm_tables.soc_table);
>>> -        return smu_v13_0_6_print_clks(smu, buf, size,
>>> single_dpm_table,
>>> -                          now, "socclk");
>>> +        ret = smu_v13_0_6_print_clks(smu, buf, size,
>>> +single_dpm_table,
>>> +                         now, "socclk");
>>> +        if (ret < 0)
>>> +            return ret;
>>> +        size += ret;
>>> +        break;
>>>        case SMU_FCLK:
>>>            ret = smu_v13_0_6_get_current_clk_freq_by_table(smu,
>>> SMU_FCLK,
>>>                                    &now); @@ -1542,9 +1551,13 @@
>>> static int smu_v13_0_6_print_clk_levels(struct
>>> smu_context *smu,
>>>            single_dpm_table = &(dpm_context->dpm_tables.fclk_table);
>>> -        return smu_v13_0_6_print_clks(smu, buf, size,
>>> single_dpm_table,
>>> -                          now, "fclk");
>>> +        ret = smu_v13_0_6_print_clks(smu, buf, size,
>>> +single_dpm_table,
>>> +                         now, "fclk");
>>> +        if (ret < 0)
>>> +            return ret;
>>> +        size += ret;
>>> +        break;
>>>        case SMU_VCLK:
>>>            ret = smu_v13_0_6_get_current_clk_freq_by_table(smu,
>>> SMU_VCLK,
>>>                                    &now); @@ -1556,9 +1569,13 @@
>>> static int smu_v13_0_6_print_clk_levels(struct
>>> smu_context *smu,
>>>            single_dpm_table = &(dpm_context->dpm_tables.vclk_table);
>>> -        return smu_v13_0_6_print_clks(smu, buf, size,
>>> single_dpm_table,
>>> -                          now, "vclk");
>>> +        ret = smu_v13_0_6_print_clks(smu, buf, size,
>>> +single_dpm_table,
>>> +                         now, "vclk");
>>> +        if (ret < 0)
>>> +            return ret;
>>> +        size += ret;
>>> +        break;
>>>        case SMU_DCLK:
>>>            ret = smu_v13_0_6_get_current_clk_freq_by_table(smu,
>>> SMU_DCLK,
>>>                                       &now); @@ -1570,9 +1587,13 @@
>>> static int smu_v13_0_6_print_clk_levels(struct
>>> smu_context *smu,
>>>            single_dpm_table = &(dpm_context->dpm_tables.dclk_table);
>>> -        return smu_v13_0_6_print_clks(smu, buf, size,
>>> single_dpm_table,
>>> -                          now, "dclk");
>>> +        ret = smu_v13_0_6_print_clks(smu, buf, size,
>>> +single_dpm_table,
>>> +                         now, "dclk");
>>> +        if (ret < 0)
>>> +            return ret;
>>> +        size += ret;
>>> +        break;
>>>        default:
>>>            break;
>>>        }
>>
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-10-27  8:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-27  7:27 [PATCH] drm/amd/pm: Fix incomplete null pointer issue for smu v13.0.6 Yang Wang
2025-10-27  7:43 ` Lazar, Lijo
2025-10-27  7:46   ` Lazar, Lijo
2025-10-27  7:56     ` Wang, Yang(Kevin)
2025-10-27  8:06       ` Lazar, Lijo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox