diff mbox series

drm/amd/powerplay: work around enum conversion warnings

Message ID 20190708135725.844960-1-arnd@arndb.de
State New
Headers show
Series drm/amd/powerplay: work around enum conversion warnings | expand

Commit Message

Arnd Bergmann July 8, 2019, 1:57 p.m. UTC 
A couple of calls to smu_get_current_clk_freq() and smu_force_clk_levels()
pass constants of the wrong type, leading to warnings with clang-8:

drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:995:39: error: implicit conversion from enumeration type 'PPCLK_e' to different enumeration type 'enum smu_clk_type' [-Werror,-Wenum-conversion]
                ret = smu_get_current_clk_freq(smu, PPCLK_SOCCLK, &now);
                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:775:82: note: expanded from macro 'smu_get_current_clk_freq'
        ((smu)->funcs->get_current_clk_freq? (smu)->funcs->get_current_clk_freq((smu), (clk_id), (value)) : 0)

I could not figure out what the purpose is of mixing the types
like this and if it is written like this intentionally.
Assuming this is all correct, adding an explict case is an
easy way to shut up the warnings.

Fixes: bc0fcffd36ba ("drm/amd/powerplay: Unify smu handle task function (v2)")
Fixes: 096761014227 ("drm/amd/powerplay: support sysfs to get socclk, fclk, dcefclk")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
Please check carefully if the warning is just a false positive
or we need a different patch.
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 4 ++--
 drivers/gpu/drm/amd/powerplay/vega20_ppt.c | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.20.0

Comments

Nathan Chancellor July 8, 2019, 2:54 p.m. UTC | #1 
Hi Arnd,

On Mon, Jul 08, 2019 at 03:57:06PM +0200, Arnd Bergmann wrote:
> A couple of calls to smu_get_current_clk_freq() and smu_force_clk_levels()

> pass constants of the wrong type, leading to warnings with clang-8:

> 

> drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:995:39: error: implicit conversion from enumeration type 'PPCLK_e' to different enumeration type 'enum smu_clk_type' [-Werror,-Wenum-conversion]

>                 ret = smu_get_current_clk_freq(smu, PPCLK_SOCCLK, &now);

>                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~

> drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:775:82: note: expanded from macro 'smu_get_current_clk_freq'

>         ((smu)->funcs->get_current_clk_freq? (smu)->funcs->get_current_clk_freq((smu), (clk_id), (value)) : 0)

> 

> I could not figure out what the purpose is of mixing the types

> like this and if it is written like this intentionally.

> Assuming this is all correct, adding an explict case is an

> easy way to shut up the warnings.

> 

> Fixes: bc0fcffd36ba ("drm/amd/powerplay: Unify smu handle task function (v2)")

> Fixes: 096761014227 ("drm/amd/powerplay: support sysfs to get socclk, fclk, dcefclk")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


I sent a series last week for all of the clang warnings that were added
in this driver recently.

https://lore.kernel.org/lkml/20190704055217.45860-1-natechancellor@gmail.com/

I think it is safe to use the CLK enums from the expected type (from
what I could see from going down the code flow rabbit hole).

https://lore.kernel.org/lkml/20190704055217.45860-4-natechancellor@gmail.com/

https://lore.kernel.org/lkml/20190704055217.45860-7-natechancellor@gmail.com/

Cheers,
Nathan
Arnd Bergmann July 15, 2019, 9:20 a.m. UTC | #2 
On Mon, Jul 8, 2019 at 6:05 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Jul 8, 2019 at 4:54 PM Nathan Chancellor

> <natechancellor@gmail.com> wrote:


> > On Mon, Jul 08, 2019 at 03:57:06PM +0200, Arnd Bergmann wrote:

> > > A couple of calls to smu_get_current_clk_freq() and smu_force_clk_levels()

> > > pass constants of the wrong type, leading to warnings with clang-8:

> > >

> > > drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:995:39: error: implicit conversion from enumeration type 'PPCLK_e' to different enumeration type 'enum smu_clk_type' [-Werror,-Wenum-conversion]

> > >                 ret = smu_get_current_clk_freq(smu, PPCLK_SOCCLK, &now);

> > >                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~

> > > drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:775:82: note: expanded from macro 'smu_get_current_clk_freq'

> > >         ((smu)->funcs->get_current_clk_freq? (smu)->funcs->get_current_clk_freq((smu), (clk_id), (value)) : 0)

> > >

> > > I could not figure out what the purpose is of mixing the types

> > > like this and if it is written like this intentionally.

> > > Assuming this is all correct, adding an explict case is an

> > > easy way to shut up the warnings.

> > >

> > > Fixes: bc0fcffd36ba ("drm/amd/powerplay: Unify smu handle task function (v2)")

> > > Fixes: 096761014227 ("drm/amd/powerplay: support sysfs to get socclk, fclk, dcefclk")

> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> >

> > I sent a series last week for all of the clang warnings that were added

> > in this driver recently.

> >

> > https://lore.kernel.org/lkml/20190704055217.45860-1-natechancellor@gmail.com/

> >

> > I think it is safe to use the CLK enums from the expected type (from

> > what I could see from going down the code flow rabbit hole).

> >

> > https://lore.kernel.org/lkml/20190704055217.45860-4-natechancellor@gmail.com/

> >

> > https://lore.kernel.org/lkml/20190704055217.45860-7-natechancellor@gmail.com/

>

> I tried that at first but concluded that it could not work because the constants

> are different. Either it's currently broken and you patches fix the runtime

> behavior, or it's currently correct and your patches break it.


d36893362d22 ("drm/amd/powerplay: fix smu clock type change miss error")
was now applied and contains the same change as your first patch.

I assume the other one is still needed though.

       Arnd
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 755c6c79f724..93732623b507 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1386,8 +1386,8 @@  int smu_adjust_power_state_dynamic(struct smu_context *smu,
 							 &soc_mask);
 			if (ret)
 				return ret;
-			smu_force_clk_levels(smu, PP_SCLK, 1 << sclk_mask);
-			smu_force_clk_levels(smu, PP_MCLK, 1 << mclk_mask);
+			smu_force_clk_levels(smu, (enum smu_clk_type)PP_SCLK, 1 << sclk_mask);
+			smu_force_clk_levels(smu, (enum smu_clk_type)PP_MCLK, 1 << mclk_mask);
 			break;
 
 		case AMD_DPM_FORCED_LEVEL_MANUAL:
diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
index dba02fa0de01..20d477f8dc84 100644
--- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
@@ -992,7 +992,7 @@  static int vega20_print_clk_levels(struct smu_context *smu,
 		break;
 
 	case SMU_SOCCLK:
-		ret = smu_get_current_clk_freq(smu, PPCLK_SOCCLK, &now);
+		ret = smu_get_current_clk_freq(smu, (enum smu_clk_type)PPCLK_SOCCLK, &now);
 		if (ret) {
 			pr_err("Attempt to get current socclk Failed!");
 			return ret;
@@ -1013,7 +1013,7 @@  static int vega20_print_clk_levels(struct smu_context *smu,
 		break;
 
 	case SMU_FCLK:
-		ret = smu_get_current_clk_freq(smu, PPCLK_FCLK, &now);
+		ret = smu_get_current_clk_freq(smu, (enum smu_clk_type)PPCLK_FCLK, &now);
 		if (ret) {
 			pr_err("Attempt to get current fclk Failed!");
 			return ret;
@@ -1028,7 +1028,7 @@  static int vega20_print_clk_levels(struct smu_context *smu,
 		break;
 
 	case SMU_DCEFCLK:
-		ret = smu_get_current_clk_freq(smu, PPCLK_DCEFCLK, &now);
+		ret = smu_get_current_clk_freq(smu, (enum smu_clk_type)PPCLK_DCEFCLK, &now);
 		if (ret) {
 			pr_err("Attempt to get current dcefclk Failed!");
 			return ret;