[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

[PATCH 2/2] drm/i915: Fix W=1 warning for csr_load_work_fn()

[Chris Wilson]

drivers/gpu/drm/i915/intel_csr.c: In function ‘csr_load_work_fn’:
drivers/gpu/drm/i915/intel_csr.c:399:6: error: variable ‘ret’ set but not used [-Werror=unused-but-set-variable]

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

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 0085bc745f6a..9dcc434d3b74 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -396,13 +396,11 @@ static void csr_load_work_fn(struct work_struct *work)
 	struct drm_i915_private *dev_priv;
 	struct intel_csr *csr;
 	const struct firmware *fw = NULL;
-	int ret;
 
 	dev_priv = container_of(work, typeof(*dev_priv), csr.work);
 	csr = &dev_priv->csr;
 
-	ret = request_firmware(&fw, dev_priv->csr.fw_path,
-			       &dev_priv->drm.pdev->dev);
+	request_firmware(&fw, dev_priv->csr.fw_path, &dev_priv->drm.pdev->dev);
 	if (fw)
 		dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv, fw);
 
-- 
2.11.0

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

Re: [PATCH 2/2] drm/i915: Fix W=1 warning for csr_load_work_fn()

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> drivers/gpu/drm/i915/intel_csr.c: In function ‘csr_load_work_fn’:
> drivers/gpu/drm/i915/intel_csr.c:399:6: error: variable ‘ret’ set but not used [-Werror=unused-but-set-variable]
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_csr.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 0085bc745f6a..9dcc434d3b74 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -396,13 +396,11 @@ static void csr_load_work_fn(struct work_struct *work)
>  	struct drm_i915_private *dev_priv;
>  	struct intel_csr *csr;
>  	const struct firmware *fw = NULL;
> -	int ret;
>  
>  	dev_priv = container_of(work, typeof(*dev_priv), csr.work);
>  	csr = &dev_priv->csr;
>  
> -	ret = request_firmware(&fw, dev_priv->csr.fw_path,
> -			       &dev_priv->drm.pdev->dev);
> +	request_firmware(&fw, dev_priv->csr.fw_path, &dev_priv->drm.pdev->dev);
>  	if (fw)
>  		dev_priv->csr.dmc_payload = parse_csr_fw(dev_priv, fw);
>  
> -- 
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
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

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 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