On Thu, Nov 30, 2023 at 11:46:00AM +0200, Jani Nikula wrote:
> On Thu, 30 Nov 2023, Javier Martinez Canillas <javierm@redhat.com> wrote:
> > Maxime Ripard <mripard@kernel.org> writes:
> >
> >> Hi,
> >>
> >> On Thu, Nov 30, 2023 at 10:52:17AM +0200, Jani Nikula wrote:
> >>> On Wed, 29 Nov 2023, Hamza Mahfooz <hamza.mahfooz@amd.com> wrote:
> >>> > Cc: Nathan Chancellor <nathan@kernel.org>
> >>> >
> >>> > On 11/29/23 13:12, Jani Nikula wrote:
> >>> >> At least the i915 and amd drivers enable a bunch more compiler warnings
> >>> >> than the kernel defaults.
> >>> >> 
> >>> >> Extend the W=1 warnings to the entire drm subsystem by default. Use the
> >>> >> copy-pasted warnings from scripts/Makefile.extrawarn with
> >>> >> s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep
> >>> >> up with them in the future.
> >>> >> 
> >>> >> This is similar to the approach currently used in i915.
> >>> >> 
> >>> >> Some of the -Wextra warnings do need to be disabled, just like in
> >>> >> Makefile.extrawarn, but take care to not disable them for W=2 or W=3
> >>> >> builds, depending on the warning.
> >>> >
> >>> > I think this should go in after drm-misc-next has a clean build (for
> >>> > COMPILE_TEST builds) with this patch applied. Otherwise, it will break a
> >>> > lot of build configs.
> >>> 
> >>> Oh, I'm absolutely not suggesting this should be merged before known
> >>> warnings have been addressed one way or another. Either by fixing them
> >>> or by disabling said warning in driver local Makefiles, depending on the
> >>> case.
> >>
> >> I'm all for it, but yeah, we need some easy way to opt-in/opt-out. Some
> >> drivers are pretty much unmaintained now and are likely to never fix
> >> those warnings.
> 
> Then I'd go for enabling in drm level and disabling individual warnings
> in the driver Makefile level if they won't get fixed.
> 
> > Maybe add a Kconfig symbol for it instead of making unconditional?
> >
> > Something like:
> >
> > +# Unconditionally enable W=1 warnings locally
> > +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
> > +subdir-ccflags-$(CONFIG_DRM_EXTRA_CHECKS)  += -Wextra -Wunused -Wno-unused-parameter
> > ...
> 
> Then we'll have a ping pong of people not using W=1 or
> CONFIG_DRM_EXTRA_CHECKS introducing warnings, and people using them
> fixing the warnings...
> 
> I really do think making it unconditional is the only way.

Yeah, I agree.

Plus, if we need to have an extra Kconfig option, it's pretty equivalent
to using W=1

Maxime