[PATCH 1/2] drm/i915: Disable -Woverride-init

[PATCH 1/2] drm/i915: Disable -Woverride-init

[Chris Wilson]

We commonly use an inheritance style approach to device parameters,
where later generations inherit the defaults from earlier generations
and then override settings that change. For example, in i915_pci.c
BDW_FEATURES pulls in HSW_FEATURES, makes a few changes for 48bit
contexts and then individual Broadwell stanzas make further adjustments
for different GT configs.

This causes a lot of warnings with make W=1 from -Woverride-init. We
could use
	#pragma GCC diagnostic push
	#pragma GCC diagnostic ignored "-Woverride-init"
	...
	#pragma GCC diagnostic pop
around the offenders, but the pattern is used frequently enough in the
driver to prefer just disabling the warning entirely.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
---
 drivers/gpu/drm/i915/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 91136b425f77..24cc3c7f814d 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -2,7 +2,8 @@
 # Makefile for the drm device driver.  This driver provides support for the
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
-subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
+subdir-ccflags-y := -Wno-override-init # used frequently for "inheritance"
+subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
 subdir-ccflags-$(CONFIG_DRM_I915_SELFTEST) += -I$(src) -I$(src)/selftests
 subdir-ccflags-y += \
 	$(call as-instr,movntdqa (%eax)$(comma)%xmm0,-DCONFIG_AS_MOVNTDQA)
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Disable -Woverride-init

[Joonas Lahtinen]

On ke, 2017-01-18 at 12:18 +0000, Chris Wilson wrote:
> We commonly use an inheritance style approach to device parameters,
> where later generations inherit the defaults from earlier generations
> and then override settings that change. For example, in i915_pci.c
> BDW_FEATURES pulls in HSW_FEATURES, makes a few changes for 48bit
> contexts and then individual Broadwell stanzas make further adjustments
> for different GT configs.
> 
> This causes a lot of warnings with make W=1 from -Woverride-init. We
> could use
> 	#pragma GCC diagnostic push
> 	#pragma GCC diagnostic ignored "-Woverride-init"
> 	...
> 	#pragma GCC diagnostic pop
> around the offenders, but the pattern is used frequently enough in the
> driver to prefer just disabling the warning entirely.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>

<SNIP>
 
> -subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
> +subdir-ccflags-y := -Wno-override-init # used frequently for "inheritance"

Why always on, if somebody upper level decides to -Werror, this is
kinda unexpected for them?

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Disable -Woverride-init

[Chris Wilson]

On Wed, Jan 18, 2017 at 05:56:13PM +0200, Joonas Lahtinen wrote:
> On ke, 2017-01-18 at 12:18 +0000, Chris Wilson wrote:
> > We commonly use an inheritance style approach to device parameters,
> > where later generations inherit the defaults from earlier generations
> > and then override settings that change. For example, in i915_pci.c
> > BDW_FEATURES pulls in HSW_FEATURES, makes a few changes for 48bit
> > contexts and then individual Broadwell stanzas make further adjustments
> > for different GT configs.
> > 
> > This causes a lot of warnings with make W=1 from -Woverride-init. We
> > could use
> > 	#pragma GCC diagnostic push
> > 	#pragma GCC diagnostic ignored "-Woverride-init"
> > 	...
> > 	#pragma GCC diagnostic pop
> > around the offenders, but the pattern is used frequently enough in the
> > driver to prefer just disabling the warning entirely.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> 
> <SNIP>
>  
> > -subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
> > +subdir-ccflags-y := -Wno-override-init # used frequently for "inheritance"
> 
> Why always on, if somebody upper level decides to -Werror, this is
> kinda unexpected for them?

We intentionally use the { .a = 0, .a = 1 }. That is flagged by the set
of warnings enabled by W=1. If the user is using Werror, then they are
faced with an intentionally broken build.

Our choice, if we want to be W=1 clean, is to either markup using #pragma
or turn off the warning.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Disable -Woverride-init

[Joonas Lahtinen]

On ke, 2017-01-18 at 16:27 +0000, Chris Wilson wrote:
> On Wed, Jan 18, 2017 at 05:56:13PM +0200, Joonas Lahtinen wrote:
> > 
> > On ke, 2017-01-18 at 12:18 +0000, Chris Wilson wrote:
> > >
> > > -subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
> > > +subdir-ccflags-y := -Wno-override-init # used frequently for "inheritance"
> > 
> > Why always on, if somebody upper level decides to -Werror, this is
> > kinda unexpected for them?
> 
> We intentionally use the { .a = 0, .a = 1 }. That is flagged by the set
> of warnings enabled by W=1. If the user is using Werror, then they are
> faced with an intentionally broken build.
> 
> Our choice, if we want to be W=1 clean, is to either markup using #pragma
> or turn off the warning.

Guess we can then merge this.

Regards, Joonas

-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915: Disable -Woverride-init

[Chris Wilson]

We commonly use an inheritance style approach to device parameters,
where later generations inherit the defaults from earlier generations
and then override settings that change. For example, in i915_pci.c
BDW_FEATURES pulls in HSW_FEATURES, makes a few changes for 48bit
contexts and then individual Broadwell stanzas make further adjustments
for different GT configs.

This causes a lot of warnings with make W=1 from -Woverride-init. We
could use
	#pragma GCC diagnostic push
	#pragma GCC diagnostic ignored "-Woverride-init"
	...
	#pragma GCC diagnostic pop
around the offenders, but the pattern is used frequently enough in the
driver to prefer just disabling the warning entirely.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
---
 drivers/gpu/drm/i915/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 66d23b619db1..0bb6e423ecd7 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -2,7 +2,8 @@
 # Makefile for the drm device driver.  This driver provides support for the
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
-subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
+subdir-ccflags-y := -Wno-override-init # used frequently for "inheritance"
+subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
 subdir-ccflags-y += \
 	$(call as-instr,movntdqa (%eax)$(comma)%xmm0,-DCONFIG_AS_MOVNTDQA)
 
-- 
2.15.0.rc0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: Add -Wunused-const-variable to our build

[Chris Wilson]

Recently this warning caught a regression that had been lurking for 6
months, so it seems to be justified!

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0bb6e423ecd7..4351b7b6f09b 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -3,6 +3,7 @@
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
 subdir-ccflags-y := -Wno-override-init # used frequently for "inheritance"
+subdir-ccflags-y += -Wunused-const-variable
 subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
 subdir-ccflags-y += \
 	$(call as-instr,movntdqa (%eax)$(comma)%xmm0,-DCONFIG_AS_MOVNTDQA)
-- 
2.15.0.rc0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Disable -Woverride-init

[Michal Wajdeczko]

On Fri, 13 Oct 2017 18:08:29 +0200, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> We commonly use an inheritance style approach to device parameters,
> where later generations inherit the defaults from earlier generations
> and then override settings that change. For example, in i915_pci.c
> BDW_FEATURES pulls in HSW_FEATURES, makes a few changes for 48bit
> contexts and then individual Broadwell stanzas make further adjustments
> for different GT configs.
>
> This causes a lot of warnings with make W=1 from -Woverride-init. We
> could use
> 	#pragma GCC diagnostic push
> 	#pragma GCC diagnostic ignored "-Woverride-init"
> 	...
> 	#pragma GCC diagnostic pop
> around the offenders, but the pattern is used frequently enough in the
> driver to prefer just disabling the warning entirely.

I'm not sure that disabling any warnings globally is a good idea.
What about disabling them just for this single file that uses poor
man's "inheritance"

CFLAGS_i915_pci.o := -Wno-override-init

Michal

>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/Makefile  
> b/drivers/gpu/drm/i915/Makefile
> index 66d23b619db1..0bb6e423ecd7 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -2,7 +2,8 @@
>  # Makefile for the drm device driver.  This driver provides support for  
> the
>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
> -subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
> +subdir-ccflags-y := -Wno-override-init # used frequently for  
> "inheritance"
> +subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
>  subdir-ccflags-y += \
>  	$(call as-instr,movntdqa (%eax)$(comma)%xmm0,-DCONFIG_AS_MOVNTDQA)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Disable -Woverride-init

[Ville Syrjälä]

On Fri, Oct 13, 2017 at 05:08:29PM +0100, Chris Wilson wrote:
> We commonly use an inheritance style approach to device parameters,
> where later generations inherit the defaults from earlier generations
> and then override settings that change. For example, in i915_pci.c
> BDW_FEATURES pulls in HSW_FEATURES, makes a few changes for 48bit
> contexts and then individual Broadwell stanzas make further adjustments
> for different GT configs.
> 
> This causes a lot of warnings with make W=1 from -Woverride-init. We
> could use
> 	#pragma GCC diagnostic push
> 	#pragma GCC diagnostic ignored "-Woverride-init"
> 	...
> 	#pragma GCC diagnostic pop
> around the offenders, but the pattern is used frequently enough in the
> driver to prefer just disabling the warning entirely.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 66d23b619db1..0bb6e423ecd7 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -2,7 +2,8 @@
>  # Makefile for the drm device driver.  This driver provides support for the
>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>  
> -subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
> +subdir-ccflags-y := -Wno-override-init # used frequently for "inheritance"

This doesn't seem to override what W=n does so not sure why we'd need it
here, unless we add more warning flags ourselves. Or does you gcc spew
these without W=1?

Looks like I can can actually do 
subdir-ccflags-y := -Wall -Wextra -Wno-unused-parameter -Wno-type-limits
		    -Wno-missing-field-initializers -Wno-override-init
without any warnings with gcc 5.4.

-Wunused-but-set-variable reveals some more (noticed those when I ran
with W=1). Most look like easy to silence. The atomic interator macros
seem to trigger this all the time however, which may not be so easy to
sort out. Not sure.

> +subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
>  subdir-ccflags-y += \
>  	$(call as-instr,movntdqa (%eax)$(comma)%xmm0,-DCONFIG_AS_MOVNTDQA)
>  
> -- 
> 2.15.0.rc0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Add -Wunused-const-variable to our build

[Ville Syrjälä]

On Fri, Oct 13, 2017 at 05:08:30PM +0100, Chris Wilson wrote:
> Recently this warning caught a regression that had been lurking for 6
> months, so it seems to be justified!
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 0bb6e423ecd7..4351b7b6f09b 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -3,6 +3,7 @@
>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>  
>  subdir-ccflags-y := -Wno-override-init # used frequently for "inheritance"
> +subdir-ccflags-y += -Wunused-const-variable

My gcc 5.4 doesn't even seem to have this one :(

>  subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
>  subdir-ccflags-y += \
>  	$(call as-instr,movntdqa (%eax)$(comma)%xmm0,-DCONFIG_AS_MOVNTDQA)
> -- 
> 2.15.0.rc0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Disable -Woverride-init

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Disable -Woverride-init
URL   : https://patchwork.freedesktop.org/series/31942/
State : failure

== Summary ==

  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CHK     include/generated/bounds.h
  CHK     include/generated/timeconst.h
  CHK     include/generated/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  CHK     scripts/mod/devicetable-offsets.h
  CHK     include/generated/compile.h
  CHK     kernel/config_data.h
  CC [M]  drivers/gpu/drm/i915/intel_ddi.o
drivers/gpu/drm/i915/intel_ddi.c:118:35: error: ‘bdw_ddi_translations_fdi’ defined but not used [-Werror=unused-const-variable=]
 static const struct ddi_buf_trans bdw_ddi_translations_fdi[] = {
                                   ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
scripts/Makefile.build:313: recipe for target 'drivers/gpu/drm/i915/intel_ddi.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_ddi.o] Error 1
scripts/Makefile.build:572: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:572: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:572: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1019: recipe for target 'drivers' failed
make: *** [drivers] Error 2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Disable -Woverride-init

[Chris Wilson]

Quoting Ville Syrjälä (2017-10-13 17:47:25)
> On Fri, Oct 13, 2017 at 05:08:29PM +0100, Chris Wilson wrote:
> > We commonly use an inheritance style approach to device parameters,
> > where later generations inherit the defaults from earlier generations
> > and then override settings that change. For example, in i915_pci.c
> > BDW_FEATURES pulls in HSW_FEATURES, makes a few changes for 48bit
> > contexts and then individual Broadwell stanzas make further adjustments
> > for different GT configs.
> > 
> > This causes a lot of warnings with make W=1 from -Woverride-init. We
> > could use
> >       #pragma GCC diagnostic push
> >       #pragma GCC diagnostic ignored "-Woverride-init"
> >       ...
> >       #pragma GCC diagnostic pop
> > around the offenders, but the pattern is used frequently enough in the
> > driver to prefer just disabling the warning entirely.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> > ---
> >  drivers/gpu/drm/i915/Makefile | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 66d23b619db1..0bb6e423ecd7 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -2,7 +2,8 @@
> >  # Makefile for the drm device driver.  This driver provides support for the
> >  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
> >  
> > -subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
> > +subdir-ccflags-y := -Wno-override-init # used frequently for "inheritance"
> 
> This doesn't seem to override what W=n does so not sure why we'd need it
> here, unless we add more warning flags ourselves. Or does you gcc spew
> these without W=1?

It's for the majority of the noise in W=1.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx