RE: [PATCH] drm/amdgpu: suppress compile warnings

2022-06-02 Thread Quan, Evan
[AMD Official Use Only - General]



> -Original Message-
> From: Christian König 
> Sent: Monday, May 30, 2022 3:12 PM
> To: Quan, Evan ; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander 
> Subject: Re: [PATCH] drm/amdgpu: suppress compile warnings
> 
> Am 30.05.22 um 08:50 schrieb Evan Quan:
> > Suppress the compile warnings below:
> >
> >
> drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0_7_ppt.c:1
> 407:12: warning:
> > stack frame size (1040) exceeds limit (1024) in
> > 'smu_v13_0_7_get_power_profile_mode' [-Wframe-larger-than]
> > drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c:1986:6: warning:
> > no previous prototype for function 'gfx_v11_0_rlc_stop'
> > [-Wmissing-prototypes]
> >
> > Signed-off-by: Evan Quan 
> > Change-Id: I679436c91cb98afb9fcbef8942fd90a17e5234b5
> > ---
> >   drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c   |  2 +-
> >   drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 12
> ++--
> >   2 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > index 8c0a3fc7aaa6..cb581cfc7464 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > @@ -1983,7 +1983,7 @@ static int gfx_v11_0_init_csb(struct
> amdgpu_device *adev)
> > return 0;
> >   }
> >
> > -void gfx_v11_0_rlc_stop(struct amdgpu_device *adev)
> > +static void gfx_v11_0_rlc_stop(struct amdgpu_device *adev)
> >   {
> > u32 tmp = RREG32_SOC15(GC, 0, regRLC_CNTL);
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> > index 4e1861fb2c6a..8fd7652ae883 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> > @@ -1406,7 +1406,7 @@ static int smu_v13_0_7_get_power_limit(struct
> > smu_context *smu,
> >
> >   static int smu_v13_0_7_get_power_profile_mode(struct smu_context
> *smu, char *buf)
> >   {
> > -   DpmActivityMonitorCoeffIntExternal_t
> activity_monitor_external[PP_SMC_POWER_PROFILE_COUNT];
> > +   DpmActivityMonitorCoeffIntExternal_t *activity_monitor_external;
> > uint32_t i, j, size = 0;
> > int16_t workload_type = 0;
> > int result = 0;
> > @@ -1414,6 +1414,11 @@ static int
> smu_v13_0_7_get_power_profile_mode(struct smu_context *smu, char
> *buf
> > if (!buf)
> > return -EINVAL;
> >
> > +   activity_monitor_external =
> kzalloc(sizeof(DpmActivityMonitorCoeffIntExternal_t) *
> PP_SMC_POWER_PROFILE_COUNT,
> > +   GFP_KERNEL);
> 
> The static checkers will warn about that as well. Please use 
> kcalloc(sizeof(...),
> count, GFP_KERNEL);
[Quan, Evan] I actually do not know that.. The warning is about the stack frame 
size (1040).
The space allocated by kzalloc/kmalloc is not on the stack. Per my knowledge, 
it should not have such limitation.
Did I miss something?

BR
Evan
> 
> Apart from that looks good to me.
> 
> Regards,
> Christian.
> 
> > +   if (!activity_monitor_external)
> > +   return -ENOMEM;
> > +
> > size += sysfs_emit_at(buf, size, "  ");
> > for (i = 0; i <= PP_SMC_POWER_PROFILE_WINDOW3D; i++)
> > size += sysfs_emit_at(buf, size, "%-14s%s",
> > amdgpu_pp_profile_name[i], @@ -1426,14 +1431,17 @@ static int
> smu_v13_0_7_get_power_profile_mode(struct smu_context *smu, char
> *buf
> > workload_type = smu_cmn_to_asic_specific_index(smu,
> >
> CMN2ASIC_MAPPING_WORKLOAD,
> >i);
> > -   if (workload_type < 0)
> > +   if (workload_type < 0) {
> > +   kfree(activity_monitor_external);
> > return -EINVAL;
> > +   }
> >
> > result = smu_cmn_update_table(smu,
> >
> SMU_TABLE_ACTIVITY_MONITOR_COEFF, workload_type,
> >   (void
> *)(_monitor_external[i]), false);
> > if (result) {
> > dev_err(smu->adev->dev, "[%s] Failed to get activity
> monitor!",
> > __func__);
> > +   kfree(activity_monitor_external);
> > return result;
> > }
> > }


Re: [PATCH] drm/amdgpu: suppress compile warnings

2022-05-30 Thread Christian König

Am 30.05.22 um 08:50 schrieb Evan Quan:

Suppress the compile warnings below:

drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0_7_ppt.c:1407:12: warning:
stack frame size (1040) exceeds limit (1024) in 
'smu_v13_0_7_get_power_profile_mode' [-Wframe-larger-than]
drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c:1986:6: warning:
no previous prototype for function 'gfx_v11_0_rlc_stop' [-Wmissing-prototypes]

Signed-off-by: Evan Quan 
Change-Id: I679436c91cb98afb9fcbef8942fd90a17e5234b5
---
  drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c   |  2 +-
  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 12 ++--
  2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index 8c0a3fc7aaa6..cb581cfc7464 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -1983,7 +1983,7 @@ static int gfx_v11_0_init_csb(struct amdgpu_device *adev)
return 0;
  }
  
-void gfx_v11_0_rlc_stop(struct amdgpu_device *adev)

+static void gfx_v11_0_rlc_stop(struct amdgpu_device *adev)
  {
u32 tmp = RREG32_SOC15(GC, 0, regRLC_CNTL);
  
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c

index 4e1861fb2c6a..8fd7652ae883 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
@@ -1406,7 +1406,7 @@ static int smu_v13_0_7_get_power_limit(struct smu_context 
*smu,
  
  static int smu_v13_0_7_get_power_profile_mode(struct smu_context *smu, char *buf)

  {
-   DpmActivityMonitorCoeffIntExternal_t 
activity_monitor_external[PP_SMC_POWER_PROFILE_COUNT];
+   DpmActivityMonitorCoeffIntExternal_t *activity_monitor_external;
uint32_t i, j, size = 0;
int16_t workload_type = 0;
int result = 0;
@@ -1414,6 +1414,11 @@ static int smu_v13_0_7_get_power_profile_mode(struct 
smu_context *smu, char *buf
if (!buf)
return -EINVAL;
  
+	activity_monitor_external = kzalloc(sizeof(DpmActivityMonitorCoeffIntExternal_t) * PP_SMC_POWER_PROFILE_COUNT,

+   GFP_KERNEL);


The static checkers will warn about that as well. Please use 
kcalloc(sizeof(...), count, GFP_KERNEL);


Apart from that looks good to me.

Regards,
Christian.


+   if (!activity_monitor_external)
+   return -ENOMEM;
+
size += sysfs_emit_at(buf, size, "  ");
for (i = 0; i <= PP_SMC_POWER_PROFILE_WINDOW3D; i++)
size += sysfs_emit_at(buf, size, "%-14s%s", 
amdgpu_pp_profile_name[i],
@@ -1426,14 +1431,17 @@ static int smu_v13_0_7_get_power_profile_mode(struct 
smu_context *smu, char *buf
workload_type = smu_cmn_to_asic_specific_index(smu,
   
CMN2ASIC_MAPPING_WORKLOAD,
   i);
-   if (workload_type < 0)
+   if (workload_type < 0) {
+   kfree(activity_monitor_external);
return -EINVAL;
+   }
  
  		result = smu_cmn_update_table(smu,

  SMU_TABLE_ACTIVITY_MONITOR_COEFF, 
workload_type,
  (void 
*)(_monitor_external[i]), false);
if (result) {
dev_err(smu->adev->dev, "[%s] Failed to get activity 
monitor!", __func__);
+   kfree(activity_monitor_external);
return result;
}
}