RE: [PATCH 05/12] drm/amd/pm: remove dead code in navi10_emit_clk_levels and navi10_print_clk_levels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[AMD Official Use Only - AMD Internal Distribution Only]

Thank you for your clarification.

Reviewed-by: Yang Wang <kevinyang.wang@xxxxxxx>

Best Regards,
Kevin

-----Original Message-----
From: Zhang, Jesse(Jie) <Jesse.Zhang@xxxxxxx>
Sent: Tuesday, June 4, 2024 11:52 AM
To: Wang, Yang(Kevin) <KevinYang.Wang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian <Christian.Koenig@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>; Huang, Tim <Tim.Huang@xxxxxxx>
Subject: RE: [PATCH 05/12] drm/amd/pm: remove dead code in navi10_emit_clk_levels and navi10_print_clk_levels

[AMD Official Use Only - AMD Internal Distribution Only]

Hi  Kevin

I guess you didn't apply the patch correctly.
This patch doesn't remove default branch  for clk_type.
In the case of SMU_OD_VDDC_CURVE in function navi10_emit_clk_levels , The variable i ranges from 0 -3, so the default branch in case SMU_OD_VDDC_CURVE  cannot be executed.

static int navi10_emit_clk_levels(struct smu_context *smu,
                                  enum smu_clk_type clk_type,
                                  char *buf,
                                  int *offset) {

        switch (clk_type) {
         ...
        case SMU_OD_VDDC_CURVE:
                if (!smu->od_enabled || !od_table || !od_settings)
                        return -EOPNOTSUPP;
                if (!navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_GFXCLK_CURVE))
                        break;
                *offset += sysfs_emit_at(buf, *offset, "OD_VDDC_CURVE:\n");
                for (i = 0; i < 3; i++) {
                        switch (i) {
                        case 0:
                                curve_settings = &od_table->GfxclkFreq1;
                                break;
                        case 1:
                                curve_settings = &od_table->GfxclkFreq2;
                                break;
                        case 2:
                                curve_settings = &od_table->GfxclkFreq3;
                                break;
                        default:  //don't need the default case.
                                break;
                        }
                    ....
        }

Thanks
Jesse

-----Original Message-----
From: Wang, Yang(Kevin) <KevinYang.Wang@xxxxxxx>
Sent: Tuesday, June 4, 2024 10:39 AM
To: Zhang, Jesse(Jie) <Jesse.Zhang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian <Christian.Koenig@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>; Huang, Tim <Tim.Huang@xxxxxxx>
Subject: RE: [PATCH 05/12] drm/amd/pm: remove dead code in navi10_emit_clk_levels and navi10_print_clk_levels

[AMD Official Use Only - AMD Internal Distribution Only]

  CC [M]  drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.o
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c: In function 'navi10_emit_clk_levels':
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_VCLK1' not handled in switch [-Wswitch]
 1275 |         switch (clk_type) {
      |         ^~~~~~
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_DCLK1' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_ECLK' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_DISPCLK' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_PIXCLK' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_PHYCLK' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_LCLK' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_OD_CCLK' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_OD_VDDGFX_OFFSET' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_OD_FAN_CURVE' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_OD_ACOUSTIC_LIMIT' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_OD_ACOUSTIC_TARGET' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_OD_FAN_TARGET_TEMPERATURE' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_OD_FAN_MINIMUM_PWM' not handled in switch [-Wswitch]
/home/kevin/Work/disk/linux/linux-next/drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:1275:9: warning: enumeration value 'SMU_CLK_COUNT' not handled in switch [-Wswitch]
  LD [M]  drivers/gpu/drm/amd/amdgpu/amdgpu.o

After applied your patch , the kernel will show above compile warnings with 'W=1' flag.
# grep -nR -A 1 "default:" drivers | grep -i 'break' -B 1 And the above command will show all similar cases in kernel driver folder.

So, your patch is not necessary as my understanding.

Best Regards,
Kevin

-----Original Message-----
From: Zhang, Jesse(Jie) <Jesse.Zhang@xxxxxxx>
Sent: Tuesday, June 4, 2024 9:23 AM
To: Wang, Yang(Kevin) <KevinYang.Wang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian <Christian.Koenig@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>; Huang, Tim <Tim.Huang@xxxxxxx>
Subject: RE: [PATCH 05/12] drm/amd/pm: remove dead code in navi10_emit_clk_levels and navi10_print_clk_levels

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Kevin,


-----Original Message-----
From: Wang, Yang(Kevin) <KevinYang.Wang@xxxxxxx>
Sent: Monday, June 3, 2024 5:09 PM
To: Zhang, Jesse(Jie) <Jesse.Zhang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian <Christian.Koenig@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>; Huang, Tim <Tim.Huang@xxxxxxx>; Zhang, Jesse(Jie) <Jesse.Zhang@xxxxxxx>; Zhang, Jesse(Jie) <Jesse.Zhang@xxxxxxx>
Subject: RE: [PATCH 05/12] drm/amd/pm: remove dead code in navi10_emit_clk_levels and navi10_print_clk_levels

[AMD Official Use Only - AMD Internal Distribution Only]

Could you share the problems you encountered?
Some compilers may prompt you to forget to handle default cases.
[Zhang, Jesse(Jie)] These warning scan by Coverity. These are useless redundant codes.

Thanks
Jesse

Best Regards,
Kevin

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Jesse Zhang
Sent: Monday, June 3, 2024 4:48 PM
To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian <Christian.Koenig@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>; Huang, Tim <Tim.Huang@xxxxxxx>; Zhang, Jesse(Jie) <Jesse.Zhang@xxxxxxx>; Zhang, Jesse(Jie) <Jesse.Zhang@xxxxxxx>
Subject: [PATCH 05/12] drm/amd/pm: remove dead code in navi10_emit_clk_levels and navi10_print_clk_levels

Since the range of the varibable i is 0 - 3.
So execution cannot reach this statement: default.

Signed-off-by: Jesse Zhang <Jesse.Zhang@xxxxxxx>
---
 drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index cf556f1b5ed1..076620fa3ef5 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1389,8 +1389,6 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
                        case 2:
                                curve_settings = &od_table->GfxclkFreq3;
                                break;
-                       default:
-                               break;
                        }
                        *offset += sysfs_emit_at(buf, *offset, "%d: %uMHz %umV\n",
                                                  i, curve_settings[0], @@ -1594,8 +1592,6 @@ static int navi10_print_clk_levels(struct smu_context *smu,
                        case 2:
                                curve_settings = &od_table->GfxclkFreq3;
                                break;
-                       default:
-                               break;
                        }
                        size += sysfs_emit_at(buf, size, "%d: %uMHz %umV\n",
                                              i, curve_settings[0],
--
2.25.1









[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux