Re: [Intel-gfx] [PATCH] Revert "drm/i915: use localized __diag_ignore_all() instead of per file"
On Tue, 11 Jul 2023, Gustavo Sousa wrote:
> Quoting Jani Nikula (2023-07-11 08:02:14-03:00)
>>This reverts commit 88e9664434c994e97a9f6f8cdd1535495c660cea.
>>
>>__diag_ignore_all() only works for GCC 8 or later.
>>
>>-Woverride-init (from -Wextra, enabled in i915 Makefile) combined with
>>CONFIG_WERROR=y or W=e breaks the build for older GCC.
>>
>>With i386_defconfig and x86_64_defconfig enabling CONFIG_WERROR=y by
>>default, we really need to roll back the change.
>>
>>An alternative would be to disable -Woverride-init in the Makefile for
>>GCC <8, but the revert seems like the safest bet now.
>
> Sounds good. We would keep it compatible with older compilers and still have
> the
> warning enabled on newer environments.
>
> Another alternative I thought about was to have separate files only for
> initializations of that type (i.e. that would need the warning disabled), but
> I
> guess we would need to expose a lot of symbols for that to work...
>
> Reviewed-by: Gustavo Sousa
Thanks, pushed to drm-intel-next.
BR,
Jani.
>
>>
>>Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8768
>>Reported-by: John Garry
>>References:
>>https://lore.kernel.org/r/ad2601c0-84bb-c574-3702-a83ff8faf...@oracle.com
>>References: https://lore.kernel.org/r/87wmzezns4@intel.com
>>Fixes: 88e9664434c9 ("drm/i915: use localized __diag_ignore_all() instead of
>>per file")
>>Cc: Gustavo Sousa
>>Cc: Joonas Lahtinen
>>Cc: Rodrigo Vivi
>>Cc: Tvrtko Ursulin
>>Signed-off-by: Jani Nikula
>>---
>> 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 -
>> 4 files changed, 5 insertions(+), 15 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>>index 2be9dd960540..c5fc91cd58e7 100644
>>--- a/drivers/gpu/drm/i915/Makefile
>>+++ b/drivers/gpu/drm/i915/Makefile
>>@@ -23,6 +23,11 @@ subdir-ccflags-y += $(call cc-option,
>>-Wunused-but-set-variable)
>> subdir-ccflags-y += $(call cc-disable-warning, frame-address)
>> 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)
>>+
>> subdir-ccflags-y += -I$(srctree)/$(src)
>>
>> # Please keep these build lists sorted!
>>diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c
>>b/drivers/gpu/drm/i915/display/intel_display_device.c
>>index e4afa7334c82..8286e79522d1 100644
>>--- a/drivers/gpu/drm/i915/display/intel_display_device.c
>>+++ b/drivers/gpu/drm/i915/display/intel_display_device.c
>>@@ -16,9 +16,6 @@
>> #include "intel_display_reg_defs.h"
>> #include "intel_fbc.h"
>>
>>-__diag_push();
>>-__diag_ignore_all("-Woverride-init", "Allow overriding inherited members");
>>-
>> static const struct intel_display_device_info no_display = {};
>>
>> #define PIPE_A_OFFSET0x7
>>@@ -727,8 +724,6 @@ static const struct intel_display_device_info
>>xe_lpdp_display = {
>> BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) |
>> BIT(PORT_TC4),
>> };
>>
>>-__diag_pop();
>>-
>> #undef INTEL_VGA_DEVICE
>> #undef INTEL_QUANTA_VGA_DEVICE
>> #define INTEL_VGA_DEVICE(id, info) { id, info }
>>diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
>>b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>index 1cc0ddc6a310..80c3f88310db 100644
>>--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>>+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>@@ -135,9 +135,6 @@ 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 overriding the default ops");
>>-
>> static const struct fb_ops intelfb_ops = {
>> .owner = THIS_MODULE,
>> __FB_DEFAULT_DEFERRED_OPS_RDWR(intel_fbdev),
>>@@ -149,8 +146,6 @@ static const struct fb_ops intelfb_ops = {
>> .fb_mmap = intel_fbdev_mmap,
>> };
>>
>>-__diag_pop();
>>-
>> static int intelfb_alloc(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 dea3609fc496..fcacdc21643c 100644
>>--- a/drivers/gpu/drm/i915/i915_pci.c
>>+++ b/drivers/gpu/drm/i915/i915_pci.c
>>@@ -38,9 +38,6 @@
>> #include "i915_reg.h"
>> #include "intel_pci_config.h"
>>
>>-__diag_push();
>>-__diag_ignore_all("-Woverride-init", "Allow overriding inherited members");
>>-
>> #define PLATFORM(x) .platform = (x)
>> #define GEN(x) \
>> .__runtime.graphics.ip.ver = (x), \
>>@@ -846,8 +843,6 @@ static const struct intel_device_info mtl_info = {
>>
>> #undef PLAT
Re: [Intel-gfx] [PATCH] Revert "drm/i915: use localized __diag_ignore_all() instead of per file"
On 11/07/2023 12:02, Jani Nikula wrote:
This reverts commit 88e9664434c994e97a9f6f8cdd1535495c660cea.
__diag_ignore_all() only works for GCC 8 or later.
-Woverride-init (from -Wextra, enabled in i915 Makefile) combined with
CONFIG_WERROR=y or W=e breaks the build for older GCC.
With i386_defconfig and x86_64_defconfig enabling CONFIG_WERROR=y by
default, we really need to roll back the change.
An alternative would be to disable -Woverride-init in the Makefile for
GCC <8, but the revert seems like the safest bet now.
Closes:https://urldefense.com/v3/__https://gitlab.freedesktop.org/drm/intel/-/issues/8768__;!!ACWV5N9M2RV99hQ!IHKvxwtPYBXBj_pfrXmPg5H_6OVsPZaDRDgUUuYsj2IY_JpxM3APirbZR_0yg5UO61Gf2d79Ud7fgySHuVwhOPo$
Reported-by: John Garry
References:https://urldefense.com/v3/__https://lore.kernel.org/r/ad2601c0-84bb-c574-3702-a83ff8faf98c@oracle.com__;!!ACWV5N9M2RV99hQ!IHKvxwtPYBXBj_pfrXmPg5H_6OVsPZaDRDgUUuYsj2IY_JpxM3APirbZR_0yg5UO61Gf2d79Ud7fgySH0A7kxf4$
References:https://urldefense.com/v3/__https://lore.kernel.org/r/87wmzezns4.fsf@intel.com__;!!ACWV5N9M2RV99hQ!IHKvxwtPYBXBj_pfrXmPg5H_6OVsPZaDRDgUUuYsj2IY_JpxM3APirbZR_0yg5UO61Gf2d79Ud7fgySHAwJkTDk$
Fixes: 88e9664434c9 ("drm/i915: use localized __diag_ignore_all() instead of per file")
Cc: Gustavo Sousa
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Signed-off-by: Jani Nikula
I tested on next-20230711, as this does not apply to v6.5-rc1 (where I
originally experienced the issue):
Tested-by: John Garry
cheers
Re: [Intel-gfx] [PATCH] Revert "drm/i915: use localized __diag_ignore_all() instead of per file"
Quoting Jani Nikula (2023-07-11 08:02:14-03:00)
>This reverts commit 88e9664434c994e97a9f6f8cdd1535495c660cea.
>
>__diag_ignore_all() only works for GCC 8 or later.
>
>-Woverride-init (from -Wextra, enabled in i915 Makefile) combined with
>CONFIG_WERROR=y or W=e breaks the build for older GCC.
>
>With i386_defconfig and x86_64_defconfig enabling CONFIG_WERROR=y by
>default, we really need to roll back the change.
>
>An alternative would be to disable -Woverride-init in the Makefile for
>GCC <8, but the revert seems like the safest bet now.
Sounds good. We would keep it compatible with older compilers and still have the
warning enabled on newer environments.
Another alternative I thought about was to have separate files only for
initializations of that type (i.e. that would need the warning disabled), but I
guess we would need to expose a lot of symbols for that to work...
Reviewed-by: Gustavo Sousa
>
>Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8768
>Reported-by: John Garry
>References:
>https://lore.kernel.org/r/ad2601c0-84bb-c574-3702-a83ff8faf...@oracle.com
>References: https://lore.kernel.org/r/87wmzezns4@intel.com
>Fixes: 88e9664434c9 ("drm/i915: use localized __diag_ignore_all() instead of
>per file")
>Cc: Gustavo Sousa
>Cc: Joonas Lahtinen
>Cc: Rodrigo Vivi
>Cc: Tvrtko Ursulin
>Signed-off-by: Jani Nikula
>---
> 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 -
> 4 files changed, 5 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>index 2be9dd960540..c5fc91cd58e7 100644
>--- a/drivers/gpu/drm/i915/Makefile
>+++ b/drivers/gpu/drm/i915/Makefile
>@@ -23,6 +23,11 @@ subdir-ccflags-y += $(call cc-option,
>-Wunused-but-set-variable)
> subdir-ccflags-y += $(call cc-disable-warning, frame-address)
> 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)
>+
> subdir-ccflags-y += -I$(srctree)/$(src)
>
> # Please keep these build lists sorted!
>diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c
>b/drivers/gpu/drm/i915/display/intel_display_device.c
>index e4afa7334c82..8286e79522d1 100644
>--- a/drivers/gpu/drm/i915/display/intel_display_device.c
>+++ b/drivers/gpu/drm/i915/display/intel_display_device.c
>@@ -16,9 +16,6 @@
> #include "intel_display_reg_defs.h"
> #include "intel_fbc.h"
>
>-__diag_push();
>-__diag_ignore_all("-Woverride-init", "Allow overriding inherited members");
>-
> static const struct intel_display_device_info no_display = {};
>
> #define PIPE_A_OFFSET0x7
>@@ -727,8 +724,6 @@ static const struct intel_display_device_info
>xe_lpdp_display = {
> BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4),
> };
>
>-__diag_pop();
>-
> #undef INTEL_VGA_DEVICE
> #undef INTEL_QUANTA_VGA_DEVICE
> #define INTEL_VGA_DEVICE(id, info) { id, info }
>diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
>b/drivers/gpu/drm/i915/display/intel_fbdev.c
>index 1cc0ddc6a310..80c3f88310db 100644
>--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>@@ -135,9 +135,6 @@ 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 overriding the default ops");
>-
> static const struct fb_ops intelfb_ops = {
> .owner = THIS_MODULE,
> __FB_DEFAULT_DEFERRED_OPS_RDWR(intel_fbdev),
>@@ -149,8 +146,6 @@ static const struct fb_ops intelfb_ops = {
> .fb_mmap = intel_fbdev_mmap,
> };
>
>-__diag_pop();
>-
> static int intelfb_alloc(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 dea3609fc496..fcacdc21643c 100644
>--- a/drivers/gpu/drm/i915/i915_pci.c
>+++ b/drivers/gpu/drm/i915/i915_pci.c
>@@ -38,9 +38,6 @@
> #include "i915_reg.h"
> #include "intel_pci_config.h"
>
>-__diag_push();
>-__diag_ignore_all("-Woverride-init", "Allow overriding inherited members");
>-
> #define PLATFORM(x) .platform = (x)
> #define GEN(x) \
> .__runtime.graphics.ip.ver = (x), \
>@@ -846,8 +843,6 @@ 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
>--
>2.39.2
>
[Intel-gfx] [PATCH] Revert "drm/i915: use localized __diag_ignore_all() instead of per file"
This reverts commit 88e9664434c994e97a9f6f8cdd1535495c660cea.
__diag_ignore_all() only works for GCC 8 or later.
-Woverride-init (from -Wextra, enabled in i915 Makefile) combined with
CONFIG_WERROR=y or W=e breaks the build for older GCC.
With i386_defconfig and x86_64_defconfig enabling CONFIG_WERROR=y by
default, we really need to roll back the change.
An alternative would be to disable -Woverride-init in the Makefile for
GCC <8, but the revert seems like the safest bet now.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8768
Reported-by: John Garry
References:
https://lore.kernel.org/r/ad2601c0-84bb-c574-3702-a83ff8faf...@oracle.com
References: https://lore.kernel.org/r/87wmzezns4@intel.com
Fixes: 88e9664434c9 ("drm/i915: use localized __diag_ignore_all() instead of
per file")
Cc: Gustavo Sousa
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Signed-off-by: Jani Nikula
---
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 -
4 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 2be9dd960540..c5fc91cd58e7 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -23,6 +23,11 @@ subdir-ccflags-y += $(call cc-option,
-Wunused-but-set-variable)
subdir-ccflags-y += $(call cc-disable-warning, frame-address)
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)
+
subdir-ccflags-y += -I$(srctree)/$(src)
# Please keep these build lists sorted!
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c
b/drivers/gpu/drm/i915/display/intel_display_device.c
index e4afa7334c82..8286e79522d1 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.c
+++ b/drivers/gpu/drm/i915/display/intel_display_device.c
@@ -16,9 +16,6 @@
#include "intel_display_reg_defs.h"
#include "intel_fbc.h"
-__diag_push();
-__diag_ignore_all("-Woverride-init", "Allow overriding inherited members");
-
static const struct intel_display_device_info no_display = {};
#define PIPE_A_OFFSET 0x7
@@ -727,8 +724,6 @@ static const struct intel_display_device_info
xe_lpdp_display = {
BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4),
};
-__diag_pop();
-
#undef INTEL_VGA_DEVICE
#undef INTEL_QUANTA_VGA_DEVICE
#define INTEL_VGA_DEVICE(id, info) { id, info }
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 1cc0ddc6a310..80c3f88310db 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -135,9 +135,6 @@ 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 overriding the default ops");
-
static const struct fb_ops intelfb_ops = {
.owner = THIS_MODULE,
__FB_DEFAULT_DEFERRED_OPS_RDWR(intel_fbdev),
@@ -149,8 +146,6 @@ static const struct fb_ops intelfb_ops = {
.fb_mmap = intel_fbdev_mmap,
};
-__diag_pop();
-
static int intelfb_alloc(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 dea3609fc496..fcacdc21643c 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -38,9 +38,6 @@
#include "i915_reg.h"
#include "intel_pci_config.h"
-__diag_push();
-__diag_ignore_all("-Woverride-init", "Allow overriding inherited members");
-
#define PLATFORM(x) .platform = (x)
#define GEN(x) \
.__runtime.graphics.ip.ver = (x), \
@@ -846,8 +843,6 @@ 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
--
2.39.2
