On Sat, 10 Jun 2023, Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote: > On Sat, Jun 10, 2023 at 5:17 AM Nathan Chancellor <nathan@xxxxxxxxxx> wrote: >> >> + Masahiro and linux-kbuild >> >> On Fri, Jun 09, 2023 at 12:42:06PM -0400, Hamza Mahfooz wrote: >> > We have a clean build with W=1 as of >> > commit 12a15dd589ac ("drm/amd/display/amdgpu_dm/amdgpu_dm_helpers: Move >> > SYNAPTICS_DEVICE_ID into CONFIG_DRM_AMD_DC_DCN ifdef"). So, let's enable >> > these checks unconditionally for the entire module to catch these errors >> > during development. >> > >> > Cc: Alex Deucher <alexander.deucher@xxxxxxx> >> > Cc: Nathan Chancellor <nathan@xxxxxxxxxx> >> > Signed-off-by: Hamza Mahfooz <hamza.mahfooz@xxxxxxx> >> >> I think this is fine, especially since it will help catch issues in >> amdgpu quickly and hopefully encourage developers to fix their problems >> before they make it to a tree with wider impact lika -next. >> >> However, this is now the third place that W=1 has been effectively >> enabled (i915 and btrfs are the other two I know of) and it would be >> nice if this was a little more unified, especially since it is not >> uncommon for the warnings under W=1 to shift around and keeping them >> unified will make maintainence over the longer term a little easier. I >> am not sure if this has been brought up in the past and I don't want to >> hold up this change but I suspect this sentiment of wanting to enable >> W=1 on a per-subsystem basis is going to continue to grow. > > > > I believe this patch is the right way because > we will be able to add a new warning option to > scripts/Makefile.extrawarn without fixing any code. > > I remember somebody argued that drivers should be > able to do > subdir-ccflags-y += $(W1_FLAGS) Personally, I think this would be the viable way to make the kernel free of W=1 warnings. Make it clean driver and subsystem at a time, with constant progress. Currently, there's haphazard fixing of issues, with new ones creeping back in, because kernel-wide W=1 is too verbose for most developers. It's whac-a-mole. > However, if a new flag, -Wfoo, emits warnings > for drivers/gpu/drm/{i915,amd}, > you cannot add it to W=1 until fixing the code. Or adding -Wno-foo where it breaks, until fixed. > If many drivers start to do likewise, > W=1 warning will not be W=1 any more. I don't know, is the goal to fix the warnings, or keep adding stuff to W=1 so that it'll always emit warnings? :p BR, Jani. > Another good thing for hard-coding warning options > is you can lift up a warning flag one by one. > > > Let's say you fixed the entire DRM subsystem so > it is -Wunused free now. > > Then, you can move -Wunused to drivers/gpu/drm/Makefile, > while other warning options stay in drivers Makefiles. > > > > > > > >> >> Regardless, for clang 11.1.0 to 16.0.5, I see no warnings when building >> drivers/gpu/drm/amd/amdgpu/ with Arch Linux's configuration or >> allmodconfig. >> >> Reviewed-by: Nathan Chancellor <nathan@xxxxxxxxxx> >> Tested-by: Nathan Chancellor <nathan@xxxxxxxxxx> >> >> > --- >> > drivers/gpu/drm/amd/amdgpu/Makefile | 13 ++++++++++++- >> > 1 file changed, 12 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile >> > index 86b833085f19..8d16f280b695 100644 >> > --- a/drivers/gpu/drm/amd/amdgpu/Makefile >> > +++ b/drivers/gpu/drm/amd/amdgpu/Makefile >> > @@ -40,7 +40,18 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \ >> > -I$(FULL_AMD_PATH)/amdkfd >> > >> > subdir-ccflags-y := -Wextra >> > -subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable) >> > +subdir-ccflags-y += -Wunused >> > +subdir-ccflags-y += -Wmissing-prototypes >> > +subdir-ccflags-y += -Wmissing-declarations >> > +subdir-ccflags-y += -Wmissing-include-dirs >> > +subdir-ccflags-y += -Wold-style-definition >> > +subdir-ccflags-y += -Wmissing-format-attribute >> > +# Need this to avoid recursive variable evaluation issues >> > +cond-flags := $(call cc-option, -Wunused-but-set-variable) \ >> > + $(call cc-option, -Wunused-const-variable) \ >> > + $(call cc-option, -Wstringop-truncation) \ >> > + $(call cc-option, -Wpacked-not-aligned) >> > +subdir-ccflags-y += $(cond-flags) >> > subdir-ccflags-y += -Wno-unused-parameter >> > subdir-ccflags-y += -Wno-type-limits >> > subdir-ccflags-y += -Wno-sign-compare >> > -- >> > 2.40.1 >> > -- Jani Nikula, Intel Open Source Graphics Center