Re: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus ppt driver
[AMD Official Use Only - Internal Distribution Only]
Hi Kevin,
Ah ok disregard the (v2) I just sent then :-)
Thanks,
Tom
From: Wang, Kevin(Yang)
Sent: Wednesday, August 12, 2020 22:47
To: Quan, Evan; StDenis, Tom; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus
ppt driver
[AMD Official Use Only - Internal Distribution Only]
Hi Tom,
drm/amdgpu: fix uninit-value in arcturus_log_thermal_throttling_event()
the fixed patch has been merged into drm-next branch.
Best Regards,
Kevin
From: amd-gfx on behalf of Quan, Evan
Sent: Thursday, August 13, 2020 10:07 AM
To: StDenis, Tom ; amd-gfx@lists.freedesktop.org
Cc: StDenis, Tom
Subject: RE: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus
ppt driver
[AMD Official Use Only - Internal Distribution Only]
[AMD Official Use Only - Internal Distribution Only]
Your change below should be able to suppress the compile warning.
-arcturus_get_smu_metrics_data(smu,
+ret = arcturus_get_smu_metrics_data(smu,
METRICS_THROTTLER_STATUS,
_status);
+if (ret) {
+dev_err(adev->dev, "Could not read from arcturus_get_smu_metrics_data()\n");
+return;
+}
+
Setting *value as 0 may be unnecessary. However anyway this patch is
reviewed-by: Evan Quan
BR
Evan
-Original Message-
From: amd-gfx On Behalf Of Tom St Denis
Sent: Wednesday, August 12, 2020 8:21 PM
To: amd-gfx@lists.freedesktop.org
Cc: StDenis, Tom
Subject: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus ppt
driver
Fixes:
CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.o
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c: In function
‘arcturus_log_thermal_throttling_event’:
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c:2223:24: warning:
‘throttler_status’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
2223 | if (throttler_status & logging_label[throttler_idx].feature_mask) {
by making arcturus_get_smu_metrics_data() assign a default value (of zero)
before any possible return point as well as simply error out of
arcturus_log_thermal_throttling_event() if it fails.
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 8b1025dc54fd..78f7ec95e4f5 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -551,6 +551,9 @@ static int arcturus_get_smu_metrics_data(struct smu_context
*smu,
mutex_lock(>metrics_lock);
+// assign default value
+*value = 0;
+
ret = smu_cmn_get_metrics_table_locked(smu,
NULL,
false);
@@ -2208,15 +2211,20 @@ static const struct throttling_logging_label { };
static void arcturus_log_thermal_throttling_event(struct smu_context *smu) {
-int throttler_idx, throtting_events = 0, buf_idx = 0;
+int throttler_idx, throtting_events = 0, buf_idx = 0, ret;
struct amdgpu_device *adev = smu->adev;
uint32_t throttler_status;
char log_buf[256];
-arcturus_get_smu_metrics_data(smu,
+ret = arcturus_get_smu_metrics_data(smu,
METRICS_THROTTLER_STATUS,
_status);
+if (ret) {
+dev_err(adev->dev, "Could not read from arcturus_get_smu_metrics_data()\n");
+return;
+}
+
memset(log_buf, 0, sizeof(log_buf));
for (throttler_idx = 0; throttler_idx < ARRAY_SIZE(logging_label);
throttler_idx++) {
--
2.26.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7CKevin1.Wang%40amd.com%7Cf47512097dfc40168e1a08d83f2db359%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637328812813110771sdata=xedbRrZeOi0PK3EM%2FKBYhfXxdfpOkocXPjQjcQ5ErI0%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7CKevin1.Wang%40amd.com%7Cf47512097dfc40168e1a08d83f2db359%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637328812813110771sdata=xedbRrZeOi0PK3EM%2FKBYhfXxdfpOkocXPjQjcQ5ErI0%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus ppt driver (v2)
Fixes:
CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.o
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c: In function
‘arcturus_log_thermal_throttling_event’:
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c:2223:24: warning:
‘throttler_status’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
2223 | if (throttler_status & logging_label[throttler_idx].feature_mask) {
By assigning an initial value and also checking for errors on the call to
arcturus_get_smu_metrics_data().
(v2): don't set default in arcturus_get_smu_metrics_data()
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 8b1025dc54fd..f6fe8e0ff977 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -2208,15 +2208,20 @@ static const struct throttling_logging_label {
};
static void arcturus_log_thermal_throttling_event(struct smu_context *smu)
{
- int throttler_idx, throtting_events = 0, buf_idx = 0;
+ int throttler_idx, throtting_events = 0, buf_idx = 0, ret;
struct amdgpu_device *adev = smu->adev;
- uint32_t throttler_status;
+ uint32_t throttler_status = 0;
char log_buf[256];
- arcturus_get_smu_metrics_data(smu,
+ ret = arcturus_get_smu_metrics_data(smu,
METRICS_THROTTLER_STATUS,
_status);
+ if (ret) {
+ dev_err(adev->dev, "Could not read from
arcturus_get_smu_metrics_data()\n");
+ return;
+ }
+
memset(log_buf, 0, sizeof(log_buf));
for (throttler_idx = 0; throttler_idx < ARRAY_SIZE(logging_label);
throttler_idx++) {
--
2.26.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus ppt driver
[AMD Official Use Only - Internal Distribution Only]
Hi Tom,
drm/amdgpu: fix uninit-value in arcturus_log_thermal_throttling_event()
the fixed patch has been merged into drm-next branch.
Best Regards,
Kevin
From: amd-gfx on behalf of Quan, Evan
Sent: Thursday, August 13, 2020 10:07 AM
To: StDenis, Tom ; amd-gfx@lists.freedesktop.org
Cc: StDenis, Tom
Subject: RE: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus
ppt driver
[AMD Official Use Only - Internal Distribution Only]
[AMD Official Use Only - Internal Distribution Only]
Your change below should be able to suppress the compile warning.
-arcturus_get_smu_metrics_data(smu,
+ret = arcturus_get_smu_metrics_data(smu,
METRICS_THROTTLER_STATUS,
_status);
+if (ret) {
+dev_err(adev->dev, "Could not read from arcturus_get_smu_metrics_data()\n");
+return;
+}
+
Setting *value as 0 may be unnecessary. However anyway this patch is
reviewed-by: Evan Quan
BR
Evan
-Original Message-
From: amd-gfx On Behalf Of Tom St Denis
Sent: Wednesday, August 12, 2020 8:21 PM
To: amd-gfx@lists.freedesktop.org
Cc: StDenis, Tom
Subject: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus ppt
driver
Fixes:
CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.o
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c: In function
‘arcturus_log_thermal_throttling_event’:
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c:2223:24: warning:
‘throttler_status’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
2223 | if (throttler_status & logging_label[throttler_idx].feature_mask) {
by making arcturus_get_smu_metrics_data() assign a default value (of zero)
before any possible return point as well as simply error out of
arcturus_log_thermal_throttling_event() if it fails.
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 8b1025dc54fd..78f7ec95e4f5 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -551,6 +551,9 @@ static int arcturus_get_smu_metrics_data(struct smu_context
*smu,
mutex_lock(>metrics_lock);
+// assign default value
+*value = 0;
+
ret = smu_cmn_get_metrics_table_locked(smu,
NULL,
false);
@@ -2208,15 +2211,20 @@ static const struct throttling_logging_label { };
static void arcturus_log_thermal_throttling_event(struct smu_context *smu) {
-int throttler_idx, throtting_events = 0, buf_idx = 0;
+int throttler_idx, throtting_events = 0, buf_idx = 0, ret;
struct amdgpu_device *adev = smu->adev;
uint32_t throttler_status;
char log_buf[256];
-arcturus_get_smu_metrics_data(smu,
+ret = arcturus_get_smu_metrics_data(smu,
METRICS_THROTTLER_STATUS,
_status);
+if (ret) {
+dev_err(adev->dev, "Could not read from arcturus_get_smu_metrics_data()\n");
+return;
+}
+
memset(log_buf, 0, sizeof(log_buf));
for (throttler_idx = 0; throttler_idx < ARRAY_SIZE(logging_label);
throttler_idx++) {
--
2.26.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7CKevin1.Wang%40amd.com%7Cf47512097dfc40168e1a08d83f2db359%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637328812813110771sdata=xedbRrZeOi0PK3EM%2FKBYhfXxdfpOkocXPjQjcQ5ErI0%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7CKevin1.Wang%40amd.com%7Cf47512097dfc40168e1a08d83f2db359%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637328812813110771sdata=xedbRrZeOi0PK3EM%2FKBYhfXxdfpOkocXPjQjcQ5ErI0%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus ppt driver
[AMD Official Use Only - Internal Distribution Only]
Your change below should be able to suppress the compile warning.
-arcturus_get_smu_metrics_data(smu,
+ret = arcturus_get_smu_metrics_data(smu,
METRICS_THROTTLER_STATUS,
_status);
+if (ret) {
+dev_err(adev->dev, "Could not read from arcturus_get_smu_metrics_data()\n");
+return;
+}
+
Setting *value as 0 may be unnecessary. However anyway this patch is
reviewed-by: Evan Quan
BR
Evan
-Original Message-
From: amd-gfx On Behalf Of Tom St Denis
Sent: Wednesday, August 12, 2020 8:21 PM
To: amd-gfx@lists.freedesktop.org
Cc: StDenis, Tom
Subject: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus ppt
driver
Fixes:
CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.o
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c: In function
‘arcturus_log_thermal_throttling_event’:
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c:2223:24: warning:
‘throttler_status’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
2223 | if (throttler_status & logging_label[throttler_idx].feature_mask) {
by making arcturus_get_smu_metrics_data() assign a default value (of zero)
before any possible return point as well as simply error out of
arcturus_log_thermal_throttling_event() if it fails.
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 8b1025dc54fd..78f7ec95e4f5 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -551,6 +551,9 @@ static int arcturus_get_smu_metrics_data(struct smu_context
*smu,
mutex_lock(>metrics_lock);
+// assign default value
+*value = 0;
+
ret = smu_cmn_get_metrics_table_locked(smu,
NULL,
false);
@@ -2208,15 +2211,20 @@ static const struct throttling_logging_label { };
static void arcturus_log_thermal_throttling_event(struct smu_context *smu) {
-int throttler_idx, throtting_events = 0, buf_idx = 0;
+int throttler_idx, throtting_events = 0, buf_idx = 0, ret;
struct amdgpu_device *adev = smu->adev;
uint32_t throttler_status;
char log_buf[256];
-arcturus_get_smu_metrics_data(smu,
+ret = arcturus_get_smu_metrics_data(smu,
METRICS_THROTTLER_STATUS,
_status);
+if (ret) {
+dev_err(adev->dev, "Could not read from arcturus_get_smu_metrics_data()\n");
+return;
+}
+
memset(log_buf, 0, sizeof(log_buf));
for (throttler_idx = 0; throttler_idx < ARRAY_SIZE(logging_label);
throttler_idx++) {
--
2.26.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cevan.quan%40amd.com%7C33cba706ddbb41b6f01508d83eba26bd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637328316549041092sdata=7BcUmmThbZU8quP0g4t3ajSYe3scCOahigO%2Bye2GHaw%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus ppt driver
On 8/12/20 2:43 PM, StDenis, Tom wrote:
[AMD Official Use Only - Internal Distribution Only]
Possibly, but since the arcturus_get_smu_metrics_data() can error out we should
check that return value no?
Yes, we need that return check.
(also setting *value to 0 avoids this bug in the future...).
I think caller should initialize the result value before passing it to
arcturus_get_smu_metrics_data
as the warning is generated from the calling function. My comment is
only concerning about setting "value"
to 0, which seems wrong. Rest of the patch is fine.
Nirmoy
Tom
From: Das, Nirmoy
Sent: Wednesday, August 12, 2020 08:40
To: StDenis, Tom; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus
ppt driver
On 8/12/20 2:20 PM, Tom St Denis wrote:
Fixes:
CC [M]
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.o
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c: In function
‘arcturus_log_thermal_throttling_event’:
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c:2223:24: warning:
‘throttler_status’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
2223 | if (throttler_status & logging_label[throttler_idx].feature_mask) {
by making arcturus_get_smu_metrics_data() assign a default value
(of zero) before any possible return point as well as simply error
out of arcturus_log_thermal_throttling_event() if it fails.
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 8b1025dc54fd..78f7ec95e4f5 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -551,6 +551,9 @@ static int arcturus_get_smu_metrics_data(struct smu_context
*smu,
mutex_lock(>metrics_lock);
+ // assign default value
+ *value = 0;
+
ret = smu_cmn_get_metrics_table_locked(smu,
NULL,
false);
@@ -2208,15 +2211,20 @@ static const struct throttling_logging_label {
};
static void arcturus_log_thermal_throttling_event(struct smu_context *smu)
{
- int throttler_idx, throtting_events = 0, buf_idx = 0;
+ int throttler_idx, throtting_events = 0, buf_idx = 0, ret;
struct amdgpu_device *adev = smu->adev;
uint32_t throttler_status;
I think initializing throttler_status here should resolve the warning.
char log_buf[256];
- arcturus_get_smu_metrics_data(smu,
+ ret = arcturus_get_smu_metrics_data(smu,
METRICS_THROTTLER_STATUS,
_status);
+ if (ret) {
+ dev_err(adev->dev, "Could not read from
arcturus_get_smu_metrics_data()\n");
+ return;
+ }
+
memset(log_buf, 0, sizeof(log_buf));
for (throttler_idx = 0; throttler_idx < ARRAY_SIZE(logging_label);
throttler_idx++) {
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus ppt driver
[AMD Official Use Only - Internal Distribution Only]
Possibly, but since the arcturus_get_smu_metrics_data() can error out we should
check that return value no?
(also setting *value to 0 avoids this bug in the future...).
Tom
From: Das, Nirmoy
Sent: Wednesday, August 12, 2020 08:40
To: StDenis, Tom; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus
ppt driver
On 8/12/20 2:20 PM, Tom St Denis wrote:
> Fixes:
>
>CC [M]
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.o
> drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c: In function
> ‘arcturus_log_thermal_throttling_event’:
> drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c:2223:24: warning:
> ‘throttler_status’ may be used uninitialized in this function
> [-Wmaybe-uninitialized]
> 2223 | if (throttler_status & logging_label[throttler_idx].feature_mask) {
>
> by making arcturus_get_smu_metrics_data() assign a default value
> (of zero) before any possible return point as well as simply error
> out of arcturus_log_thermal_throttling_event() if it fails.
>
> Signed-off-by: Tom St Denis
> ---
> drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index 8b1025dc54fd..78f7ec95e4f5 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -551,6 +551,9 @@ static int arcturus_get_smu_metrics_data(struct
> smu_context *smu,
>
> mutex_lock(>metrics_lock);
>
> + // assign default value
> + *value = 0;
> +
> ret = smu_cmn_get_metrics_table_locked(smu,
> NULL,
> false);
> @@ -2208,15 +2211,20 @@ static const struct throttling_logging_label {
> };
> static void arcturus_log_thermal_throttling_event(struct smu_context *smu)
> {
> - int throttler_idx, throtting_events = 0, buf_idx = 0;
> + int throttler_idx, throtting_events = 0, buf_idx = 0, ret;
> struct amdgpu_device *adev = smu->adev;
> uint32_t throttler_status;
I think initializing throttler_status here should resolve the warning.
> char log_buf[256];
>
> - arcturus_get_smu_metrics_data(smu,
> + ret = arcturus_get_smu_metrics_data(smu,
> METRICS_THROTTLER_STATUS,
> _status);
>
> + if (ret) {
> + dev_err(adev->dev, "Could not read from
> arcturus_get_smu_metrics_data()\n");
> + return;
> + }
> +
> memset(log_buf, 0, sizeof(log_buf));
> for (throttler_idx = 0; throttler_idx < ARRAY_SIZE(logging_label);
>throttler_idx++) {
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus ppt driver
On 8/12/20 2:20 PM, Tom St Denis wrote:
Fixes:
CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.o
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c: In function
‘arcturus_log_thermal_throttling_event’:
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c:2223:24: warning:
‘throttler_status’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
2223 | if (throttler_status & logging_label[throttler_idx].feature_mask) {
by making arcturus_get_smu_metrics_data() assign a default value
(of zero) before any possible return point as well as simply error
out of arcturus_log_thermal_throttling_event() if it fails.
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 8b1025dc54fd..78f7ec95e4f5 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -551,6 +551,9 @@ static int arcturus_get_smu_metrics_data(struct smu_context
*smu,
mutex_lock(>metrics_lock);
+ // assign default value
+ *value = 0;
+
ret = smu_cmn_get_metrics_table_locked(smu,
NULL,
false);
@@ -2208,15 +2211,20 @@ static const struct throttling_logging_label {
};
static void arcturus_log_thermal_throttling_event(struct smu_context *smu)
{
- int throttler_idx, throtting_events = 0, buf_idx = 0;
+ int throttler_idx, throtting_events = 0, buf_idx = 0, ret;
struct amdgpu_device *adev = smu->adev;
uint32_t throttler_status;
I think initializing throttler_status here should resolve the warning.
char log_buf[256];
- arcturus_get_smu_metrics_data(smu,
+ ret = arcturus_get_smu_metrics_data(smu,
METRICS_THROTTLER_STATUS,
_status);
+ if (ret) {
+ dev_err(adev->dev, "Could not read from
arcturus_get_smu_metrics_data()\n");
+ return;
+ }
+
memset(log_buf, 0, sizeof(log_buf));
for (throttler_idx = 0; throttler_idx < ARRAY_SIZE(logging_label);
throttler_idx++) {
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/powerplay: Fix uninitialized warning in arcturus ppt driver
Fixes:
CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.o
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c: In function
‘arcturus_log_thermal_throttling_event’:
drivers/gpu/drm/amd/amdgpu/../powerplay/arcturus_ppt.c:2223:24: warning:
‘throttler_status’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
2223 | if (throttler_status & logging_label[throttler_idx].feature_mask) {
by making arcturus_get_smu_metrics_data() assign a default value
(of zero) before any possible return point as well as simply error
out of arcturus_log_thermal_throttling_event() if it fails.
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 8b1025dc54fd..78f7ec95e4f5 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -551,6 +551,9 @@ static int arcturus_get_smu_metrics_data(struct smu_context
*smu,
mutex_lock(>metrics_lock);
+ // assign default value
+ *value = 0;
+
ret = smu_cmn_get_metrics_table_locked(smu,
NULL,
false);
@@ -2208,15 +2211,20 @@ static const struct throttling_logging_label {
};
static void arcturus_log_thermal_throttling_event(struct smu_context *smu)
{
- int throttler_idx, throtting_events = 0, buf_idx = 0;
+ int throttler_idx, throtting_events = 0, buf_idx = 0, ret;
struct amdgpu_device *adev = smu->adev;
uint32_t throttler_status;
char log_buf[256];
- arcturus_get_smu_metrics_data(smu,
+ ret = arcturus_get_smu_metrics_data(smu,
METRICS_THROTTLER_STATUS,
_status);
+ if (ret) {
+ dev_err(adev->dev, "Could not read from
arcturus_get_smu_metrics_data()\n");
+ return;
+ }
+
memset(log_buf, 0, sizeof(log_buf));
for (throttler_idx = 0; throttler_idx < ARRAY_SIZE(logging_label);
throttler_idx++) {
--
2.26.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
