| Message ID | 20240328102455.944131-1-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: use fine grained -Woverride-init disable | expand |
On Thu, Mar 28, 2024, at 11:24, Jani Nikula wrote: > Use localized __diag_push(), __diag_ignore_all() with rationale, and > __diag_pop() for specific initializations instead of blanket disabling > of -Woverride-init across several files. > > Note that we've tried this before with commit 88e9664434c9 ("drm/i915: > use localized __diag_ignore_all() instead of per file") and reverted in > commit 290d16104575 ("Revert "drm/i915: use localized > __diag_ignore_all() instead of per file""). The issue turned out to be > in __diag_ignore_all() and it was fixed by commit 689b097a06ba > ("compiler-gcc: Suppress -Wmissing-prototypes warning for all supported > GCC"). So we should be able to pull this off now. > > Cc: "Arnd Bergmann" <arnd@arndb.de> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> Looks good to me, Acked-by: Arnd Bergmann <arnd@arndb.de>
On Thu, 28 Mar 2024, "Arnd Bergmann" <arnd@arndb.de> wrote: > On Thu, Mar 28, 2024, at 11:24, Jani Nikula wrote: >> Use localized __diag_push(), __diag_ignore_all() with rationale, and >> __diag_pop() for specific initializations instead of blanket disabling >> of -Woverride-init across several files. >> >> Note that we've tried this before with commit 88e9664434c9 ("drm/i915: >> use localized __diag_ignore_all() instead of per file") and reverted in >> commit 290d16104575 ("Revert "drm/i915: use localized >> __diag_ignore_all() instead of per file""). The issue turned out to be >> in __diag_ignore_all() and it was fixed by commit 689b097a06ba >> ("compiler-gcc: Suppress -Wmissing-prototypes warning for all supported >> GCC"). So we should be able to pull this off now. >> >> Cc: "Arnd Bergmann" <arnd@arndb.de> >> Cc: Lucas De Marchi <lucas.demarchi@intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > Looks good to me, > > Acked-by: Arnd Bergmann <arnd@arndb.de> Thanks! I'll take this via drm-intel-next for v6.10. Up to you what to do with your patch [1], either drop the i915 and xe changes, or we can handle the trivial conflict too if keeping the changes helps you somehow. BR, Jani. [1] https://lore.kernel.org/all/20240326144741.3094687-2-arnd@kernel.org/
On Thu, Mar 28, 2024, at 11:46, Jani Nikula wrote: > On Thu, 28 Mar 2024, "Arnd Bergmann" <arnd@arndb.de> wrote: >> On Thu, Mar 28, 2024, at 11:24, Jani Nikula wrote: >>> Use localized __diag_push(), __diag_ignore_all() with rationale, and >>> __diag_pop() for specific initializations instead of blanket disabling >>> of -Woverride-init across several files. >>> >>> Note that we've tried this before with commit 88e9664434c9 ("drm/i915: >>> use localized __diag_ignore_all() instead of per file") and reverted in >>> commit 290d16104575 ("Revert "drm/i915: use localized >>> __diag_ignore_all() instead of per file""). The issue turned out to be >>> in __diag_ignore_all() and it was fixed by commit 689b097a06ba >>> ("compiler-gcc: Suppress -Wmissing-prototypes warning for all supported >>> GCC"). So we should be able to pull this off now. >>> >>> Cc: "Arnd Bergmann" <arnd@arndb.de> >>> Cc: Lucas De Marchi <lucas.demarchi@intel.com> >>> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> >> Looks good to me, >> >> Acked-by: Arnd Bergmann <arnd@arndb.de> > > Thanks! I'll take this via drm-intel-next for v6.10. Up to you what to > do with your patch [1], either drop the i915 and xe changes, or we can > handle the trivial conflict too if keeping the changes helps you > somehow. I'll just drop all of the parts for drivers/gpu and send another patch for the amdgpu driver to do the same as your patch. Arnd
On Thu, Mar 28, 2024 at 12:24:55PM +0200, Jani Nikula wrote: >Use localized __diag_push(), __diag_ignore_all() with rationale, and >__diag_pop() for specific initializations instead of blanket disabling >of -Woverride-init across several files. > >Note that we've tried this before with commit 88e9664434c9 ("drm/i915: >use localized __diag_ignore_all() instead of per file") and reverted in >commit 290d16104575 ("Revert "drm/i915: use localized >__diag_ignore_all() instead of per file""). The issue turned out to be >in __diag_ignore_all() and it was fixed by commit 689b097a06ba >("compiler-gcc: Suppress -Wmissing-prototypes warning for all supported >GCC"). So we should be able to pull this off now. > >Cc: "Arnd Bergmann" <arnd@arndb.de> >Cc: Lucas De Marchi <lucas.demarchi@intel.com> >Signed-off-by: Jani Nikula <jani.nikula@intel.com> great! I had a vague memory of us using these localized warning-suppress rather than doing it on the entire files. Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> Lucas De Marchi
On Thu, 28 Mar 2024, "Arnd Bergmann" <arnd@arndb.de> wrote: > On Thu, Mar 28, 2024, at 11:46, Jani Nikula wrote: >> On Thu, 28 Mar 2024, "Arnd Bergmann" <arnd@arndb.de> wrote: >>> On Thu, Mar 28, 2024, at 11:24, Jani Nikula wrote: >>>> Use localized __diag_push(), __diag_ignore_all() with rationale, and >>>> __diag_pop() for specific initializations instead of blanket disabling >>>> of -Woverride-init across several files. >>>> >>>> Note that we've tried this before with commit 88e9664434c9 ("drm/i915: >>>> use localized __diag_ignore_all() instead of per file") and reverted in >>>> commit 290d16104575 ("Revert "drm/i915: use localized >>>> __diag_ignore_all() instead of per file""). The issue turned out to be >>>> in __diag_ignore_all() and it was fixed by commit 689b097a06ba >>>> ("compiler-gcc: Suppress -Wmissing-prototypes warning for all supported >>>> GCC"). So we should be able to pull this off now. >>>> >>>> Cc: "Arnd Bergmann" <arnd@arndb.de> >>>> Cc: Lucas De Marchi <lucas.demarchi@intel.com> >>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >>> >>> Looks good to me, >>> >>> Acked-by: Arnd Bergmann <arnd@arndb.de> >> >> Thanks! I'll take this via drm-intel-next for v6.10. Up to you what to >> do with your patch [1], either drop the i915 and xe changes, or we can >> handle the trivial conflict too if keeping the changes helps you >> somehow. > > I'll just drop all of the parts for drivers/gpu and send another > patch for the amdgpu driver to do the same as your patch. Works for me, thanks! In the mean time, merged this one to drm-intel-next, thanks for the ack and review! BR, Jani.
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 3ef6ed41e62b..87d6ba8d2341 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -32,11 +32,6 @@ endif # Enable -Werror in CI and development subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror -# Fine grained warnings disable -CFLAGS_i915_pci.o = $(call cc-disable-warning, override-init) -CFLAGS_display/intel_display_device.o = $(call cc-disable-warning, override-init) -CFLAGS_display/intel_fbdev.o = $(call cc-disable-warning, override-init) - # Support compiling the display code separately for both i915 and xe # drivers. Define I915 when building i915. subdir-ccflags-y += -DI915 diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c index c02d79b50006..b8903bd0e82a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_device.c +++ b/drivers/gpu/drm/i915/display/intel_display_device.c @@ -17,6 +17,9 @@ #include "intel_display_reg_defs.h" #include "intel_fbc.h" +__diag_push(); +__diag_ignore_all("-Woverride-init", "Allow field initialization overrides for display info"); + static const struct intel_display_device_info no_display = {}; #define PIPE_A_OFFSET 0x70000 @@ -768,6 +771,8 @@ static const struct intel_display_device_info xe2_lpd_display = { BIT(INTEL_FBC_C) | BIT(INTEL_FBC_D), }; +__diag_pop(); + /* * Separate detection for no display cases to keep the display id array simple. * diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c index 99894a855ef0..43855c6c3509 100644 --- a/drivers/gpu/drm/i915/display/intel_fbdev.c +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c @@ -135,6 +135,9 @@ static int intel_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma) return i915_gem_fb_mmap(obj, vma); } +__diag_push(); +__diag_ignore_all("-Woverride-init", "Allow field initialization overrides for fb ops"); + static const struct fb_ops intelfb_ops = { .owner = THIS_MODULE, __FB_DEFAULT_DEFERRED_OPS_RDWR(intel_fbdev), @@ -146,6 +149,8 @@ static const struct fb_ops intelfb_ops = { .fb_mmap = intel_fbdev_mmap, }; +__diag_pop(); + static int intelfb_create(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes) { diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 1e69783ae4fd..405ca17a990b 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -38,6 +38,9 @@ #include "i915_reg.h" #include "intel_pci_config.h" +__diag_push(); +__diag_ignore_all("-Woverride-init", "Allow field initialization overrides for device info"); + #define PLATFORM(x) .platform = (x) #define GEN(x) \ .__runtime.graphics.ip.ver = (x), \ @@ -785,6 +788,8 @@ static const struct intel_device_info mtl_info = { #undef PLATFORM +__diag_pop(); + /* * Make sure any device matches here are from most specific to most * general. For example, since the Quanta match is based on the subsystem diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile index 705c0eaf6e71..aa06a5508ab1 100644 --- a/drivers/gpu/drm/xe/Makefile +++ b/drivers/gpu/drm/xe/Makefile @@ -173,9 +173,6 @@ subdir-ccflags-$(CONFIG_DRM_XE_DISPLAY) += \ -Ddrm_i915_gem_object=xe_bo \ -Ddrm_i915_private=xe_device -CFLAGS_i915-display/intel_fbdev.o = $(call cc-disable-warning, override-init) -CFLAGS_i915-display/intel_display_device.o = $(call cc-disable-warning, override-init) - # Rule to build SOC code shared with i915 $(obj)/i915-soc/%.o: $(srctree)/drivers/gpu/drm/i915/soc/%.c FORCE $(call cmd,force_checksrc)
Use localized __diag_push(), __diag_ignore_all() with rationale, and __diag_pop() for specific initializations instead of blanket disabling of -Woverride-init across several files. Note that we've tried this before with commit 88e9664434c9 ("drm/i915: use localized __diag_ignore_all() instead of per file") and reverted in commit 290d16104575 ("Revert "drm/i915: use localized __diag_ignore_all() instead of per file""). The issue turned out to be in __diag_ignore_all() and it was fixed by commit 689b097a06ba ("compiler-gcc: Suppress -Wmissing-prototypes warning for all supported GCC"). So we should be able to pull this off now. Cc: "Arnd Bergmann" <arnd@arndb.de> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/Makefile | 5 ----- drivers/gpu/drm/i915/display/intel_display_device.c | 5 +++++ drivers/gpu/drm/i915/display/intel_fbdev.c | 5 +++++ drivers/gpu/drm/i915/i915_pci.c | 5 +++++ drivers/gpu/drm/xe/Makefile | 3 --- 5 files changed, 15 insertions(+), 8 deletions(-)