2023-05-25 16:46:22

by Hamza Mahfooz

[permalink] [raw]
Subject: [PATCH] drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR

We want to do -Werror builds on our CI. However, non-amdgpu breakages
have prevented us from doing so thus far. Also, there are a number of
additional checks that we should enable, that the community cares about
and are hidden behind -Wextra. So, define DRM_AMDGPU_WERROR to only
enable -Werror for the amdgpu kernel module and enable -Wextra while
disabling all of the checks that are too noisy.

Cc: Alex Deucher <[email protected]>
Cc: Kenny Ho <[email protected]>
Suggested-by: Jani Nikula <[email protected]>
Signed-off-by: Hamza Mahfooz <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/Kconfig | 10 ++++++++++
drivers/gpu/drm/amd/amdgpu/Makefile | 9 +++++++++
2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig
index 07135ffa6d24..334511f331e3 100644
--- a/drivers/gpu/drm/amd/amdgpu/Kconfig
+++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
@@ -66,6 +66,16 @@ config DRM_AMDGPU_USERPTR
This option selects CONFIG_HMM and CONFIG_HMM_MIRROR if it
isn't already selected to enabled full userptr support.

+config DRM_AMDGPU_WERROR
+ bool "Force the compiler to throw an error instead of a warning when compiling"
+ depends on DRM_AMDGPU
+ depends on EXPERT
+ depends on !COMPILE_TEST
+ default n
+ help
+ Add -Werror to the build flags for amdgpu.ko.
+ Only enable this if you are warning code for amdgpu.ko.
+
source "drivers/gpu/drm/amd/acp/Kconfig"
source "drivers/gpu/drm/amd/display/Kconfig"
source "drivers/gpu/drm/amd/amdkfd/Kconfig"
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index 74a9aa6fe18c..7ee68b1bbfed 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -39,6 +39,15 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
-I$(FULL_AMD_DISPLAY_PATH)/amdgpu_dm \
-I$(FULL_AMD_PATH)/amdkfd

+subdir-ccflags-y := -Wextra
+subdir-ccflags-y += -Wunused-but-set-variable
+subdir-ccflags-y += -Wno-unused-parameter
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-sign-compare
+subdir-ccflags-y += -Wno-missing-field-initializers
+subdir-ccflags-y += -Wno-override-init
+subdir-ccflags-$(CONFIG_DRM_AMDGPU_WERROR) += -Werror
+
amdgpu-y := amdgpu_drv.o

# add KMS driver
--
2.40.1



2023-05-30 15:37:25

by Hamza Mahfooz

[permalink] [raw]
Subject: Re: [PATCH] drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR

On 5/25/23 12:38, Hamza Mahfooz wrote:
> We want to do -Werror builds on our CI. However, non-amdgpu breakages
> have prevented us from doing so thus far. Also, there are a number of
> additional checks that we should enable, that the community cares about
> and are hidden behind -Wextra. So, define DRM_AMDGPU_WERROR to only
> enable -Werror for the amdgpu kernel module and enable -Wextra while
> disabling all of the checks that are too noisy.
>
> Cc: Alex Deucher <[email protected]>
> Cc: Kenny Ho <[email protected]>
> Suggested-by: Jani Nikula <[email protected]>
> Signed-off-by: Hamza Mahfooz <[email protected]>

I am able to get clean builds with this enabled on GCC 11-13 and Clang
15, at least as of commit e786aef0869c ("drm/amd/display: remove unused
definition") on amd-staging-drm-next.

> ---
> drivers/gpu/drm/amd/amdgpu/Kconfig | 10 ++++++++++
> drivers/gpu/drm/amd/amdgpu/Makefile | 9 +++++++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig
> index 07135ffa6d24..334511f331e3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
> @@ -66,6 +66,16 @@ config DRM_AMDGPU_USERPTR
> This option selects CONFIG_HMM and CONFIG_HMM_MIRROR if it
> isn't already selected to enabled full userptr support.
>
> +config DRM_AMDGPU_WERROR
> + bool "Force the compiler to throw an error instead of a warning when compiling"
> + depends on DRM_AMDGPU
> + depends on EXPERT
> + depends on !COMPILE_TEST
> + default n
> + help
> + Add -Werror to the build flags for amdgpu.ko.
> + Only enable this if you are warning code for amdgpu.ko.
> +
> source "drivers/gpu/drm/amd/acp/Kconfig"
> source "drivers/gpu/drm/amd/display/Kconfig"
> source "drivers/gpu/drm/amd/amdkfd/Kconfig"
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 74a9aa6fe18c..7ee68b1bbfed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -39,6 +39,15 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
> -I$(FULL_AMD_DISPLAY_PATH)/amdgpu_dm \
> -I$(FULL_AMD_PATH)/amdkfd
>
> +subdir-ccflags-y := -Wextra
> +subdir-ccflags-y += -Wunused-but-set-variable
> +subdir-ccflags-y += -Wno-unused-parameter
> +subdir-ccflags-y += -Wno-type-limits
> +subdir-ccflags-y += -Wno-sign-compare
> +subdir-ccflags-y += -Wno-missing-field-initializers
> +subdir-ccflags-y += -Wno-override-init
> +subdir-ccflags-$(CONFIG_DRM_AMDGPU_WERROR) += -Werror
> +
> amdgpu-y := amdgpu_drv.o
>
> # add KMS driver
--
Hamza


2023-05-30 16:00:51

by Ho, Kenny

[permalink] [raw]

2023-05-30 16:00:55

by Ho, Kenny

[permalink] [raw]
Subject: Re: [PATCH] drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR

[Public]

On 5/30/23 11:24, Hamza Mahfooz wrote:
> I am able to get clean builds with this enabled on GCC 11-13 and Clang
> 15, at least as of commit e786aef0869c ("drm/amd/display: remove unused
> definition") on amd-staging-drm-next.

Did you try intentionally introducing a warning to see if the build indeed fail?

2023-05-30 16:05:57

by Hamza Mahfooz

[permalink] [raw]
Subject: Re: [PATCH] drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR

On 5/30/23 11:50, Ho, Kenny wrote:
> [Public]
>
> On 5/30/23 11:24, Hamza Mahfooz wrote:
>> I am able to get clean builds with this enabled on GCC 11-13 and Clang
>> 15, at least as of commit e786aef0869c ("drm/amd/display: remove unused
>> definition") on amd-staging-drm-next.
>
> Did you try intentionally introducing a warning to see if the build indeed fail?

Yes, I tried a couple of different ones.

--
Hamza