| Message ID | 20240329072441.591471-14-samuel.holland@sifive.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Unified cross-architecture kernel-mode FPU API | expand |
Hello, Samuel Holland <samuel.holland@sifive.com> writes: > Now that all previously-supported architectures select > ARCH_HAS_KERNEL_FPU_SUPPORT, this code can depend on that symbol instead > of the existing list of architectures. It can also take advantage of the > common kernel-mode FPU API and method of adjusting CFLAGS. > > Acked-by: Alex Deucher <alexander.deucher@amd.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Samuel Holland <samuel.holland@sifive.com> Unfortunately this patch causes build failures on arm with allyesconfig and allmodconfig. Tested with next-20240410. Error with allyesconfig: $ make -j 8 \ O=$HOME/.cache/builds/linux-cross-arm \ ARCH=arm \ CROSS_COMPILE=arm-linux-gnueabihf- make[1]: Entering directory '/home/bauermann/.cache/builds/linux-cross-arm' ⋮ arm-linux-gnueabihf-ld: drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.o: in function `dcn20_populate_dml_pipes_from_context': dcn20_fpu.c:(.text+0x20f4): undefined reference to `__aeabi_l2d' arm-linux-gnueabihf-ld: dcn20_fpu.c:(.text+0x210c): undefined reference to `__aeabi_l2d' arm-linux-gnueabihf-ld: dcn20_fpu.c:(.text+0x2124): undefined reference to `__aeabi_l2d' arm-linux-gnueabihf-ld: dcn20_fpu.c:(.text+0x213c): undefined reference to `__aeabi_l2d' arm-linux-gnueabihf-ld: drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.o: in function `pipe_ctx_to_e2e_pipe_params': dcn_calcs.c:(.text+0x390): undefined reference to `__aeabi_l2d' arm-linux-gnueabihf-ld: drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.o:dcn_calcs.c:(.text+0x3a4): more undefined references to `__aeabi_l2d' follow arm-linux-gnueabihf-ld: drivers/gpu/drm/amd/display/dc/dml2/dml2_wrapper.o: in function `optimize_configuration': dml2_wrapper.c:(.text+0xcbc): undefined reference to `__aeabi_d2ulz' arm-linux-gnueabihf-ld: drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.o: in function `populate_dml_plane_cfg_from_plane_state': dml2_translation_helper.c:(.text+0x9e4): undefined reference to `__aeabi_l2d' arm-linux-gnueabihf-ld: dml2_translation_helper.c:(.text+0xa20): undefined reference to `__aeabi_l2d' arm-linux-gnueabihf-ld: dml2_translation_helper.c:(.text+0xa58): undefined reference to `__aeabi_l2d' arm-linux-gnueabihf-ld: dml2_translation_helper.c:(.text+0xa90): undefined reference to `__aeabi_l2d' make[3]: *** [/home/bauermann/src/linux/scripts/Makefile.vmlinux:37: vmlinux] Error 1 make[2]: *** [/home/bauermann/src/linux/Makefile:1165: vmlinux] Error 2 make[1]: *** [/home/bauermann/src/linux/Makefile:240: __sub-make] Error 2 make[1]: Leaving directory '/home/bauermann/.cache/builds/linux-cross-arm' make: *** [Makefile:240: __sub-make] Error 2 The error with allmodconfig is slightly different: $ make -j 8 \ O=$HOME/.cache/builds/linux-cross-arm \ ARCH=arm \ CROSS_COMPILE=arm-linux-gnueabihf- make[1]: Entering directory '/home/bauermann/.cache/builds/linux-cross-arm' ⋮ ERROR: modpost: "__aeabi_d2ulz" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined! ERROR: modpost: "__aeabi_l2d" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined! make[3]: *** [/home/bauermann/src/linux/scripts/Makefile.modpost:145: Module.symvers] Error 1 make[2]: *** [/home/bauermann/src/linux/Makefile:1876: modpost] Error 2 make[1]: *** [/home/bauermann/src/linux/Makefile:240: __sub-make] Error 2 make[1]: Leaving directory '/home/bauermann/.cache/builds/linux-cross-arm' make: *** [Makefile:240: __sub-make] Error 2 -- Thiago
Hi Thiago, On 2024-04-10 5:21 PM, Thiago Jung Bauermann wrote: > Samuel Holland <samuel.holland@sifive.com> writes: > >> Now that all previously-supported architectures select >> ARCH_HAS_KERNEL_FPU_SUPPORT, this code can depend on that symbol instead >> of the existing list of architectures. It can also take advantage of the >> common kernel-mode FPU API and method of adjusting CFLAGS. >> >> Acked-by: Alex Deucher <alexander.deucher@amd.com> >> Reviewed-by: Christoph Hellwig <hch@lst.de> >> Signed-off-by: Samuel Holland <samuel.holland@sifive.com> > > Unfortunately this patch causes build failures on arm with allyesconfig > and allmodconfig. Tested with next-20240410. > > Error with allyesconfig: > > $ make -j 8 \ > O=$HOME/.cache/builds/linux-cross-arm \ > ARCH=arm \ > CROSS_COMPILE=arm-linux-gnueabihf- > make[1]: Entering directory '/home/bauermann/.cache/builds/linux-cross-arm' > ⋮ > arm-linux-gnueabihf-ld: drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.o: in function `dcn20_populate_dml_pipes_from_context': > dcn20_fpu.c:(.text+0x20f4): undefined reference to `__aeabi_l2d' > arm-linux-gnueabihf-ld: dcn20_fpu.c:(.text+0x210c): undefined reference to `__aeabi_l2d' > arm-linux-gnueabihf-ld: dcn20_fpu.c:(.text+0x2124): undefined reference to `__aeabi_l2d' > arm-linux-gnueabihf-ld: dcn20_fpu.c:(.text+0x213c): undefined reference to `__aeabi_l2d' > arm-linux-gnueabihf-ld: drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.o: in function `pipe_ctx_to_e2e_pipe_params': > dcn_calcs.c:(.text+0x390): undefined reference to `__aeabi_l2d' > arm-linux-gnueabihf-ld: drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.o:dcn_calcs.c:(.text+0x3a4): more undefined references to `__aeabi_l2d' follow > arm-linux-gnueabihf-ld: drivers/gpu/drm/amd/display/dc/dml2/dml2_wrapper.o: in function `optimize_configuration': > dml2_wrapper.c:(.text+0xcbc): undefined reference to `__aeabi_d2ulz' > arm-linux-gnueabihf-ld: drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.o: in function `populate_dml_plane_cfg_from_plane_state': > dml2_translation_helper.c:(.text+0x9e4): undefined reference to `__aeabi_l2d' > arm-linux-gnueabihf-ld: dml2_translation_helper.c:(.text+0xa20): undefined reference to `__aeabi_l2d' > arm-linux-gnueabihf-ld: dml2_translation_helper.c:(.text+0xa58): undefined reference to `__aeabi_l2d' > arm-linux-gnueabihf-ld: dml2_translation_helper.c:(.text+0xa90): undefined reference to `__aeabi_l2d' > make[3]: *** [/home/bauermann/src/linux/scripts/Makefile.vmlinux:37: vmlinux] Error 1 > make[2]: *** [/home/bauermann/src/linux/Makefile:1165: vmlinux] Error 2 > make[1]: *** [/home/bauermann/src/linux/Makefile:240: __sub-make] Error 2 > make[1]: Leaving directory '/home/bauermann/.cache/builds/linux-cross-arm' > make: *** [Makefile:240: __sub-make] Error 2 > > The error with allmodconfig is slightly different: > > $ make -j 8 \ > O=$HOME/.cache/builds/linux-cross-arm \ > ARCH=arm \ > CROSS_COMPILE=arm-linux-gnueabihf- > make[1]: Entering directory '/home/bauermann/.cache/builds/linux-cross-arm' > ⋮ > ERROR: modpost: "__aeabi_d2ulz" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined! > ERROR: modpost: "__aeabi_l2d" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined! > make[3]: *** [/home/bauermann/src/linux/scripts/Makefile.modpost:145: Module.symvers] Error 1 > make[2]: *** [/home/bauermann/src/linux/Makefile:1876: modpost] Error 2 > make[1]: *** [/home/bauermann/src/linux/Makefile:240: __sub-make] Error 2 > make[1]: Leaving directory '/home/bauermann/.cache/builds/linux-cross-arm' > make: *** [Makefile:240: __sub-make] Error 2 In both cases, the issue is that the toolchain requires runtime support to convert between `unsigned long long` and `double`, even when hardware FP is enabled. There was some past discussion about GCC inlining some of these conversions[1], but that did not get implemented. The short-term fix would be to drop the `select ARCH_HAS_KERNEL_FPU_SUPPORT` for 32-bit arm until we can provide these runtime library functions. Regards, Samuel [1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91970
Hello Samuel, Thank you for the quick reply! Samuel Holland <samuel.holland@sifive.com> writes: > On 2024-04-10 5:21 PM, Thiago Jung Bauermann wrote: >> >> Unfortunately this patch causes build failures on arm with allyesconfig >> and allmodconfig. Tested with next-20240410. <snip> > In both cases, the issue is that the toolchain requires runtime support to > convert between `unsigned long long` and `double`, even when hardware FP is > enabled. There was some past discussion about GCC inlining some of these > conversions[1], but that did not get implemented. Thank you for the explanation and the bugzilla reference. I added a comment there mentioning that the problem came up again with this patch series. > The short-term fix would be to drop the `select ARCH_HAS_KERNEL_FPU_SUPPORT` for > 32-bit arm until we can provide these runtime library functions. Does this mean that patch 2 in this series: [PATCH v4 02/15] ARM: Implement ARCH_HAS_KERNEL_FPU_SUPPORT will be dropped? -- Thiago
Hi Thiago, On 2024-04-10 8:02 PM, Thiago Jung Bauermann wrote: > Samuel Holland <samuel.holland@sifive.com> writes: >> On 2024-04-10 5:21 PM, Thiago Jung Bauermann wrote: >>> >>> Unfortunately this patch causes build failures on arm with allyesconfig >>> and allmodconfig. Tested with next-20240410. > > <snip> > >> In both cases, the issue is that the toolchain requires runtime support to >> convert between `unsigned long long` and `double`, even when hardware FP is >> enabled. There was some past discussion about GCC inlining some of these >> conversions[1], but that did not get implemented. > > Thank you for the explanation and the bugzilla reference. I added a > comment there mentioning that the problem came up again with this patch > series. > >> The short-term fix would be to drop the `select ARCH_HAS_KERNEL_FPU_SUPPORT` for >> 32-bit arm until we can provide these runtime library functions. > > Does this mean that patch 2 in this series: > > [PATCH v4 02/15] ARM: Implement ARCH_HAS_KERNEL_FPU_SUPPORT > > will be dropped? No, because later patches in the series (3, 6) depend on the definition of CC_FLAGS_FPU from that patch. I will need to send a fixup patch unless I can find a GPL-2 compatible implementation of the runtime library functions. Regards, Samuel
Hello Samuel, Samuel Holland <samuel.holland@sifive.com> writes: > On 2024-04-10 8:02 PM, Thiago Jung Bauermann wrote: >> Samuel Holland <samuel.holland@sifive.com> writes: >>> On 2024-04-10 5:21 PM, Thiago Jung Bauermann wrote: >>>> >>>> Unfortunately this patch causes build failures on arm with allyesconfig >>>> and allmodconfig. Tested with next-20240410. >> >> <snip> >> >>> In both cases, the issue is that the toolchain requires runtime support to >>> convert between `unsigned long long` and `double`, even when hardware FP is >>> enabled. There was some past discussion about GCC inlining some of these >>> conversions[1], but that did not get implemented. >> >> Thank you for the explanation and the bugzilla reference. I added a >> comment there mentioning that the problem came up again with this patch >> series. >> >>> The short-term fix would be to drop the `select ARCH_HAS_KERNEL_FPU_SUPPORT` for >>> 32-bit arm until we can provide these runtime library functions. >> >> Does this mean that patch 2 in this series: >> >> [PATCH v4 02/15] ARM: Implement ARCH_HAS_KERNEL_FPU_SUPPORT >> >> will be dropped? > > No, because later patches in the series (3, 6) depend on the definition of > CC_FLAGS_FPU from that patch. I will need to send a fixup patch unless I can > find a GPL-2 compatible implementation of the runtime library functions. Ok, thank you for clarifying. Andrew Pinski just responded on the GCC bugzilla and if I understood his point correctly, it seems to be a matter of changing function names to what GCC (or actually the arm EABI) expects... -- Thiago
(cc Arnd) On Thu, 11 Apr 2024 at 03:11, Samuel Holland <samuel.holland@sifive.com> wrote: > > Hi Thiago, > > On 2024-04-10 8:02 PM, Thiago Jung Bauermann wrote: > > Samuel Holland <samuel.holland@sifive.com> writes: > >> On 2024-04-10 5:21 PM, Thiago Jung Bauermann wrote: > >>> > >>> Unfortunately this patch causes build failures on arm with allyesconfig > >>> and allmodconfig. Tested with next-20240410. > > > > <snip> > > > >> In both cases, the issue is that the toolchain requires runtime support to > >> convert between `unsigned long long` and `double`, even when hardware FP is > >> enabled. There was some past discussion about GCC inlining some of these > >> conversions[1], but that did not get implemented. > > > > Thank you for the explanation and the bugzilla reference. I added a > > comment there mentioning that the problem came up again with this patch > > series. > > > >> The short-term fix would be to drop the `select ARCH_HAS_KERNEL_FPU_SUPPORT` for > >> 32-bit arm until we can provide these runtime library functions. > > > > Does this mean that patch 2 in this series: > > > > [PATCH v4 02/15] ARM: Implement ARCH_HAS_KERNEL_FPU_SUPPORT > > > > will be dropped? > > No, because later patches in the series (3, 6) depend on the definition of > CC_FLAGS_FPU from that patch. I will need to send a fixup patch unless I can > find a GPL-2 compatible implementation of the runtime library functions. > Is there really a point to doing that? Do 32-bit ARM systems even have enough address space to the map the BARs of the AMD GPUs that need this support? Given that this was not enabled before, I don't think the upshot of this series should be that we enable support for something on 32-bit ARM that may cause headaches down the road without any benefit. So I'd prefer a fixup patch that opts ARM out of this over adding support code for 64-bit conversions.
On Thu, Apr 11, 2024, at 09:15, Ard Biesheuvel wrote: > On Thu, 11 Apr 2024 at 03:11, Samuel Holland <samuel.holland@sifive.com> wrote: >> On 2024-04-10 8:02 PM, Thiago Jung Bauermann wrote: >> > Samuel Holland <samuel.holland@sifive.com> writes: >> >> >> The short-term fix would be to drop the `select ARCH_HAS_KERNEL_FPU_SUPPORT` for >> >> 32-bit arm until we can provide these runtime library functions. >> > >> > Does this mean that patch 2 in this series: >> > >> > [PATCH v4 02/15] ARM: Implement ARCH_HAS_KERNEL_FPU_SUPPORT >> > >> > will be dropped? >> >> No, because later patches in the series (3, 6) depend on the definition of >> CC_FLAGS_FPU from that patch. I will need to send a fixup patch unless I can >> find a GPL-2 compatible implementation of the runtime library functions. >> > > Is there really a point to doing that? Do 32-bit ARM systems even have > enough address space to the map the BARs of the AMD GPUs that need > this support? > > Given that this was not enabled before, I don't think the upshot of > this series should be that we enable support for something on 32-bit > ARM that may cause headaches down the road without any benefit. > > So I'd prefer a fixup patch that opts ARM out of this over adding > support code for 64-bit conversions. I have not found any dts file for a 32-bit platform with support for a 64-bit prefetchable BAR, and there are very few that even have a pcie slot (as opposed on on-board devices) you could plug a card into. That said, I also don't think we should encourage the use of floating-point code in random device drivers. There is really no excuse for the amdgpu driver to use floating point math here, and we should get AMD to fix their driver instead. Arnd
On Thu, 11 Apr 2024 at 17:32, Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Apr 11, 2024, at 09:15, Ard Biesheuvel wrote: > > On Thu, 11 Apr 2024 at 03:11, Samuel Holland <samuel.holland@sifive.com> wrote: > >> On 2024-04-10 8:02 PM, Thiago Jung Bauermann wrote: > >> > Samuel Holland <samuel.holland@sifive.com> writes: > >> > >> >> The short-term fix would be to drop the `select ARCH_HAS_KERNEL_FPU_SUPPORT` for > >> >> 32-bit arm until we can provide these runtime library functions. > >> > > >> > Does this mean that patch 2 in this series: > >> > > >> > [PATCH v4 02/15] ARM: Implement ARCH_HAS_KERNEL_FPU_SUPPORT > >> > > >> > will be dropped? > >> > >> No, because later patches in the series (3, 6) depend on the definition of > >> CC_FLAGS_FPU from that patch. I will need to send a fixup patch unless I can > >> find a GPL-2 compatible implementation of the runtime library functions. > >> > > > > Is there really a point to doing that? Do 32-bit ARM systems even have > > enough address space to the map the BARs of the AMD GPUs that need > > this support? > > > > Given that this was not enabled before, I don't think the upshot of > > this series should be that we enable support for something on 32-bit > > ARM that may cause headaches down the road without any benefit. > > > > So I'd prefer a fixup patch that opts ARM out of this over adding > > support code for 64-bit conversions. > > I have not found any dts file for a 32-bit platform with support > for a 64-bit prefetchable BAR, and there are very few that even > have a pcie slot (as opposed on on-board devices) you could > plug a card into. > > That said, I also don't think we should encourage the use of > floating-point code in random device drivers. There is really > no excuse for the amdgpu driver to use floating point math > here, and we should get AMD to fix their driver instead. That would be nice, but it won't happen, there are many reasons for that code to exist like it does, unless someone can write an automated converter to fixed point and validate it produces the same results for a long series of input values, it isn't really something that will get "fixed". AMD's hardware team produces the calculations, and will only look into hardware problems in that area if the driver is using the calculations they produce and validate. If you've looked at the calculation complexity you'd understand this isn't a trivial use of float-point for no reason. Dave.
On 2024-03-29 03:18, Samuel Holland wrote: > Now that all previously-supported architectures select > ARCH_HAS_KERNEL_FPU_SUPPORT, this code can depend on that symbol instead > of the existing list of architectures. It can also take advantage of the > common kernel-mode FPU API and method of adjusting CFLAGS. > > Acked-by: Alex Deucher <alexander.deucher@amd.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> Really nice set of changes. Acked-by: Harry Wentland <harry.wentland@amd.com> Harry > Signed-off-by: Samuel Holland <samuel.holland@sifive.com> > --- > > (no changes since v2) > > Changes in v2: > - Split altivec removal to a separate patch > - Use linux/fpu.h instead of asm/fpu.h in consumers > > drivers/gpu/drm/amd/display/Kconfig | 2 +- > .../gpu/drm/amd/display/amdgpu_dm/dc_fpu.c | 27 ++------------ > drivers/gpu/drm/amd/display/dc/dml/Makefile | 36 ++----------------- > drivers/gpu/drm/amd/display/dc/dml2/Makefile | 36 ++----------------- > 4 files changed, 7 insertions(+), 94 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig > index 901d1961b739..5fcd4f778dc3 100644 > --- a/drivers/gpu/drm/amd/display/Kconfig > +++ b/drivers/gpu/drm/amd/display/Kconfig > @@ -8,7 +8,7 @@ config DRM_AMD_DC > depends on BROKEN || !CC_IS_CLANG || ARM64 || RISCV || SPARC64 || X86_64 > select SND_HDA_COMPONENT if SND_HDA_CORE > # !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752 > - select DRM_AMD_DC_FP if (X86 || LOONGARCH || (PPC64 && ALTIVEC) || (ARM64 && KERNEL_MODE_NEON && !CC_IS_CLANG)) > + select DRM_AMD_DC_FP if ARCH_HAS_KERNEL_FPU_SUPPORT && (!ARM64 || !CC_IS_CLANG) > help > Choose this option if you want to use the new display engine > support for AMDGPU. This adds required support for Vega and > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c > index 0de16796466b..e46f8ce41d87 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c > @@ -26,16 +26,7 @@ > > #include "dc_trace.h" > > -#if defined(CONFIG_X86) > -#include <asm/fpu/api.h> > -#elif defined(CONFIG_PPC64) > -#include <asm/switch_to.h> > -#include <asm/cputable.h> > -#elif defined(CONFIG_ARM64) > -#include <asm/neon.h> > -#elif defined(CONFIG_LOONGARCH) > -#include <asm/fpu.h> > -#endif > +#include <linux/fpu.h> > > /** > * DOC: DC FPU manipulation overview > @@ -87,16 +78,9 @@ void dc_fpu_begin(const char *function_name, const int line) > WARN_ON_ONCE(!in_task()); > preempt_disable(); > depth = __this_cpu_inc_return(fpu_recursion_depth); > - > if (depth == 1) { > -#if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH) > + BUG_ON(!kernel_fpu_available()); > kernel_fpu_begin(); > -#elif defined(CONFIG_PPC64) > - if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) > - enable_kernel_fp(); > -#elif defined(CONFIG_ARM64) > - kernel_neon_begin(); > -#endif > } > > TRACE_DCN_FPU(true, function_name, line, depth); > @@ -118,14 +102,7 @@ void dc_fpu_end(const char *function_name, const int line) > > depth = __this_cpu_dec_return(fpu_recursion_depth); > if (depth == 0) { > -#if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH) > kernel_fpu_end(); > -#elif defined(CONFIG_PPC64) > - if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) > - disable_kernel_fp(); > -#elif defined(CONFIG_ARM64) > - kernel_neon_end(); > -#endif > } else { > WARN_ON_ONCE(depth < 0); > } > diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile > index 59d3972341d2..a94b6d546cd1 100644 > --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile > +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile > @@ -25,40 +25,8 @@ > # It provides the general basic services required by other DAL > # subcomponents. > > -ifdef CONFIG_X86 > -dml_ccflags-$(CONFIG_CC_IS_GCC) := -mhard-float > -dml_ccflags := $(dml_ccflags-y) -msse > -endif > - > -ifdef CONFIG_PPC64 > -dml_ccflags := -mhard-float > -endif > - > -ifdef CONFIG_ARM64 > -dml_rcflags := -mgeneral-regs-only > -endif > - > -ifdef CONFIG_LOONGARCH > -dml_ccflags := -mfpu=64 > -dml_rcflags := -msoft-float > -endif > - > -ifdef CONFIG_CC_IS_GCC > -ifneq ($(call gcc-min-version, 70100),y) > -IS_OLD_GCC = 1 > -endif > -endif > - > -ifdef CONFIG_X86 > -ifdef IS_OLD_GCC > -# Stack alignment mismatch, proceed with caution. > -# GCC < 7.1 cannot compile code using `double` and -mpreferred-stack-boundary=3 > -# (8B stack alignment). > -dml_ccflags += -mpreferred-stack-boundary=4 > -else > -dml_ccflags += -msse2 > -endif > -endif > +dml_ccflags := $(CC_FLAGS_FPU) > +dml_rcflags := $(CC_FLAGS_NO_FPU) > > ifneq ($(CONFIG_FRAME_WARN),0) > ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y) > diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile b/drivers/gpu/drm/amd/display/dc/dml2/Makefile > index 7b51364084b5..4f6c804a26ad 100644 > --- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile > +++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile > @@ -24,40 +24,8 @@ > # > # Makefile for dml2. > > -ifdef CONFIG_X86 > -dml2_ccflags-$(CONFIG_CC_IS_GCC) := -mhard-float > -dml2_ccflags := $(dml2_ccflags-y) -msse > -endif > - > -ifdef CONFIG_PPC64 > -dml2_ccflags := -mhard-float > -endif > - > -ifdef CONFIG_ARM64 > -dml2_rcflags := -mgeneral-regs-only > -endif > - > -ifdef CONFIG_LOONGARCH > -dml2_ccflags := -mfpu=64 > -dml2_rcflags := -msoft-float > -endif > - > -ifdef CONFIG_CC_IS_GCC > -ifeq ($(call cc-ifversion, -lt, 0701, y), y) > -IS_OLD_GCC = 1 > -endif > -endif > - > -ifdef CONFIG_X86 > -ifdef IS_OLD_GCC > -# Stack alignment mismatch, proceed with caution. > -# GCC < 7.1 cannot compile code using `double` and -mpreferred-stack-boundary=3 > -# (8B stack alignment). > -dml2_ccflags += -mpreferred-stack-boundary=4 > -else > -dml2_ccflags += -msse2 > -endif > -endif > +dml2_ccflags := $(CC_FLAGS_FPU) > +dml2_rcflags := $(CC_FLAGS_NO_FPU) > > ifneq ($(CONFIG_FRAME_WARN),0) > ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y)
Hi, On Fri, Mar 29, 2024 at 12:18:28AM -0700, Samuel Holland wrote: > Now that all previously-supported architectures select > ARCH_HAS_KERNEL_FPU_SUPPORT, this code can depend on that symbol instead > of the existing list of architectures. It can also take advantage of the > common kernel-mode FPU API and method of adjusting CFLAGS. > > Acked-by: Alex Deucher <alexander.deucher@amd.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Samuel Holland <samuel.holland@sifive.com> With this patch in the mainline kernel, I get the following build error when trying to build powerpc:ppc32_allmodconfig. powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.o uses soft float powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o [ repeated many times ] The problem is no longer seen after reverting this patch. Guenter
On Fri, May 24, 2024 at 5:16 AM Guenter Roeck <linux@roeck-us.net> wrote: > > Hi, > > On Fri, Mar 29, 2024 at 12:18:28AM -0700, Samuel Holland wrote: > > Now that all previously-supported architectures select > > ARCH_HAS_KERNEL_FPU_SUPPORT, this code can depend on that symbol instead > > of the existing list of architectures. It can also take advantage of the > > common kernel-mode FPU API and method of adjusting CFLAGS. > > > > Acked-by: Alex Deucher <alexander.deucher@amd.com> > > Reviewed-by: Christoph Hellwig <hch@lst.de> > > Signed-off-by: Samuel Holland <samuel.holland@sifive.com> > > With this patch in the mainline kernel, I get the following build error > when trying to build powerpc:ppc32_allmodconfig. > > powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.o uses soft float > powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o > > [ repeated many times ] > > The problem is no longer seen after reverting this patch. Should be fixed with this patch: https://gitlab.freedesktop.org/agd5f/linux/-/commit/5f56be33f33dd1d50b9433f842c879a20dc00f5b Will pull it into my -fixes branch. Alex > > Guenter
On Fri, May 24, 2024 at 9:17 AM Alex Deucher <alexdeucher@gmail.com> wrote: > > On Fri, May 24, 2024 at 5:16 AM Guenter Roeck <linux@roeck-us.net> wrote: > > > > Hi, > > > > On Fri, Mar 29, 2024 at 12:18:28AM -0700, Samuel Holland wrote: > > > Now that all previously-supported architectures select > > > ARCH_HAS_KERNEL_FPU_SUPPORT, this code can depend on that symbol instead > > > of the existing list of architectures. It can also take advantage of the > > > common kernel-mode FPU API and method of adjusting CFLAGS. > > > > > > Acked-by: Alex Deucher <alexander.deucher@amd.com> > > > Reviewed-by: Christoph Hellwig <hch@lst.de> > > > Signed-off-by: Samuel Holland <samuel.holland@sifive.com> > > > > With this patch in the mainline kernel, I get the following build error > > when trying to build powerpc:ppc32_allmodconfig. > > > > powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.o uses soft float > > powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o > > > > [ repeated many times ] > > > > The problem is no longer seen after reverting this patch. > > Should be fixed with this patch: > https://gitlab.freedesktop.org/agd5f/linux/-/commit/5f56be33f33dd1d50b9433f842c879a20dc00f5b > Will pull it into my -fixes branch. > Nevermind, this is something else. Alex > Alex > > > > > Guenter
On 5/24/24 06:18, Alex Deucher wrote: > On Fri, May 24, 2024 at 9:17 AM Alex Deucher <alexdeucher@gmail.com> wrote: >> >> On Fri, May 24, 2024 at 5:16 AM Guenter Roeck <linux@roeck-us.net> wrote: >>> >>> Hi, >>> >>> On Fri, Mar 29, 2024 at 12:18:28AM -0700, Samuel Holland wrote: >>>> Now that all previously-supported architectures select >>>> ARCH_HAS_KERNEL_FPU_SUPPORT, this code can depend on that symbol instead >>>> of the existing list of architectures. It can also take advantage of the >>>> common kernel-mode FPU API and method of adjusting CFLAGS. >>>> >>>> Acked-by: Alex Deucher <alexander.deucher@amd.com> >>>> Reviewed-by: Christoph Hellwig <hch@lst.de> >>>> Signed-off-by: Samuel Holland <samuel.holland@sifive.com> >>> >>> With this patch in the mainline kernel, I get the following build error >>> when trying to build powerpc:ppc32_allmodconfig. >>> >>> powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.o uses soft float >>> powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o >>> >>> [ repeated many times ] >>> >>> The problem is no longer seen after reverting this patch. >> >> Should be fixed with this patch: >> https://gitlab.freedesktop.org/agd5f/linux/-/commit/5f56be33f33dd1d50b9433f842c879a20dc00f5b >> Will pull it into my -fixes branch. >> > > Nevermind, this is something else. > Yes, CONFIG_DRM_AMD_DC_FP is enabled in that configuration. Guenter
On 5/24/24 06:43, Guenter Roeck wrote: > On 5/24/24 06:18, Alex Deucher wrote: >> On Fri, May 24, 2024 at 9:17 AM Alex Deucher <alexdeucher@gmail.com> wrote: >>> >>> On Fri, May 24, 2024 at 5:16 AM Guenter Roeck <linux@roeck-us.net> wrote: >>>> >>>> Hi, >>>> >>>> On Fri, Mar 29, 2024 at 12:18:28AM -0700, Samuel Holland wrote: >>>>> Now that all previously-supported architectures select >>>>> ARCH_HAS_KERNEL_FPU_SUPPORT, this code can depend on that symbol instead >>>>> of the existing list of architectures. It can also take advantage of the >>>>> common kernel-mode FPU API and method of adjusting CFLAGS. >>>>> >>>>> Acked-by: Alex Deucher <alexander.deucher@amd.com> >>>>> Reviewed-by: Christoph Hellwig <hch@lst.de> >>>>> Signed-off-by: Samuel Holland <samuel.holland@sifive.com> >>>> >>>> With this patch in the mainline kernel, I get the following build error >>>> when trying to build powerpc:ppc32_allmodconfig. >>>> >>>> powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.o uses soft float >>>> powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o >>>> >>>> [ repeated many times ] >>>> >>>> The problem is no longer seen after reverting this patch. >>> >>> Should be fixed with this patch: >>> https://gitlab.freedesktop.org/agd5f/linux/-/commit/5f56be33f33dd1d50b9433f842c879a20dc00f5b >>> Will pull it into my -fixes branch. >>> >> >> Nevermind, this is something else. >> > > Yes, CONFIG_DRM_AMD_DC_FP is enabled in that configuration. > Also, the above patch does not apply upstream; the endif is already in the correct place in the upstream kernel (though the commit introducing it adds a blank line at the end of the file) Guenter
On 5/24/24 06:18, Alex Deucher wrote: > On Fri, May 24, 2024 at 9:17 AM Alex Deucher <alexdeucher@gmail.com> wrote: >> >> On Fri, May 24, 2024 at 5:16 AM Guenter Roeck <linux@roeck-us.net> wrote: >>> >>> Hi, >>> >>> On Fri, Mar 29, 2024 at 12:18:28AM -0700, Samuel Holland wrote: >>>> Now that all previously-supported architectures select >>>> ARCH_HAS_KERNEL_FPU_SUPPORT, this code can depend on that symbol instead >>>> of the existing list of architectures. It can also take advantage of the >>>> common kernel-mode FPU API and method of adjusting CFLAGS. >>>> >>>> Acked-by: Alex Deucher <alexander.deucher@amd.com> >>>> Reviewed-by: Christoph Hellwig <hch@lst.de> >>>> Signed-off-by: Samuel Holland <samuel.holland@sifive.com> >>> >>> With this patch in the mainline kernel, I get the following build error >>> when trying to build powerpc:ppc32_allmodconfig. >>> >>> powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.o uses soft float >>> powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o >>> >>> [ repeated many times ] >>> >>> The problem is no longer seen after reverting this patch. >> >> Should be fixed with this patch: >> https://gitlab.freedesktop.org/agd5f/linux/-/commit/5f56be33f33dd1d50b9433f842c879a20dc00f5b >> Will pull it into my -fixes branch. >> > > Nevermind, this is something else. > mdgpu_dm_helpers.o is built with -msoft-float, display_mode_lib.o is built with -mhard-float. -msoft-float is from arch/powerpc/Makefile:KBUILD_CFLAGS += $(CC_FLAGS_NO_FPU) -mhard-float is from drivers/gpu/drm/amd/display/dc/dml/Makefile:dml_ccflags := $(CC_FLAGS_FPU) drivers/gpu/drm/amd/display/dc/dml/Makefile:dml_rcflags := $(CC_FLAGS_NO_FPU) drivers/gpu/drm/amd/display/dc/dml2/Makefile:dml2_ccflags := $(CC_FLAGS_FPU) drivers/gpu/drm/amd/display/dc/dml2/Makefile:dml2_rcflags := $(CC_FLAGS_NO_FPU) Unfortunately I have no idea know how to resolve this. Guenter
diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig index 901d1961b739..5fcd4f778dc3 100644 --- a/drivers/gpu/drm/amd/display/Kconfig +++ b/drivers/gpu/drm/amd/display/Kconfig @@ -8,7 +8,7 @@ config DRM_AMD_DC depends on BROKEN || !CC_IS_CLANG || ARM64 || RISCV || SPARC64 || X86_64 select SND_HDA_COMPONENT if SND_HDA_CORE # !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752 - select DRM_AMD_DC_FP if (X86 || LOONGARCH || (PPC64 && ALTIVEC) || (ARM64 && KERNEL_MODE_NEON && !CC_IS_CLANG)) + select DRM_AMD_DC_FP if ARCH_HAS_KERNEL_FPU_SUPPORT && (!ARM64 || !CC_IS_CLANG) help Choose this option if you want to use the new display engine support for AMDGPU. This adds required support for Vega and diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c index 0de16796466b..e46f8ce41d87 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c @@ -26,16 +26,7 @@ #include "dc_trace.h" -#if defined(CONFIG_X86) -#include <asm/fpu/api.h> -#elif defined(CONFIG_PPC64) -#include <asm/switch_to.h> -#include <asm/cputable.h> -#elif defined(CONFIG_ARM64) -#include <asm/neon.h> -#elif defined(CONFIG_LOONGARCH) -#include <asm/fpu.h> -#endif +#include <linux/fpu.h> /** * DOC: DC FPU manipulation overview @@ -87,16 +78,9 @@ void dc_fpu_begin(const char *function_name, const int line) WARN_ON_ONCE(!in_task()); preempt_disable(); depth = __this_cpu_inc_return(fpu_recursion_depth); - if (depth == 1) { -#if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH) + BUG_ON(!kernel_fpu_available()); kernel_fpu_begin(); -#elif defined(CONFIG_PPC64) - if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) - enable_kernel_fp(); -#elif defined(CONFIG_ARM64) - kernel_neon_begin(); -#endif } TRACE_DCN_FPU(true, function_name, line, depth); @@ -118,14 +102,7 @@ void dc_fpu_end(const char *function_name, const int line) depth = __this_cpu_dec_return(fpu_recursion_depth); if (depth == 0) { -#if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH) kernel_fpu_end(); -#elif defined(CONFIG_PPC64) - if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) - disable_kernel_fp(); -#elif defined(CONFIG_ARM64) - kernel_neon_end(); -#endif } else { WARN_ON_ONCE(depth < 0); } diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile index 59d3972341d2..a94b6d546cd1 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile @@ -25,40 +25,8 @@ # It provides the general basic services required by other DAL # subcomponents. -ifdef CONFIG_X86 -dml_ccflags-$(CONFIG_CC_IS_GCC) := -mhard-float -dml_ccflags := $(dml_ccflags-y) -msse -endif - -ifdef CONFIG_PPC64 -dml_ccflags := -mhard-float -endif - -ifdef CONFIG_ARM64 -dml_rcflags := -mgeneral-regs-only -endif - -ifdef CONFIG_LOONGARCH -dml_ccflags := -mfpu=64 -dml_rcflags := -msoft-float -endif - -ifdef CONFIG_CC_IS_GCC -ifneq ($(call gcc-min-version, 70100),y) -IS_OLD_GCC = 1 -endif -endif - -ifdef CONFIG_X86 -ifdef IS_OLD_GCC -# Stack alignment mismatch, proceed with caution. -# GCC < 7.1 cannot compile code using `double` and -mpreferred-stack-boundary=3 -# (8B stack alignment). -dml_ccflags += -mpreferred-stack-boundary=4 -else -dml_ccflags += -msse2 -endif -endif +dml_ccflags := $(CC_FLAGS_FPU) +dml_rcflags := $(CC_FLAGS_NO_FPU) ifneq ($(CONFIG_FRAME_WARN),0) ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y) diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile b/drivers/gpu/drm/amd/display/dc/dml2/Makefile index 7b51364084b5..4f6c804a26ad 100644 --- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile @@ -24,40 +24,8 @@ # # Makefile for dml2. -ifdef CONFIG_X86 -dml2_ccflags-$(CONFIG_CC_IS_GCC) := -mhard-float -dml2_ccflags := $(dml2_ccflags-y) -msse -endif - -ifdef CONFIG_PPC64 -dml2_ccflags := -mhard-float -endif - -ifdef CONFIG_ARM64 -dml2_rcflags := -mgeneral-regs-only -endif - -ifdef CONFIG_LOONGARCH -dml2_ccflags := -mfpu=64 -dml2_rcflags := -msoft-float -endif - -ifdef CONFIG_CC_IS_GCC -ifeq ($(call cc-ifversion, -lt, 0701, y), y) -IS_OLD_GCC = 1 -endif -endif - -ifdef CONFIG_X86 -ifdef IS_OLD_GCC -# Stack alignment mismatch, proceed with caution. -# GCC < 7.1 cannot compile code using `double` and -mpreferred-stack-boundary=3 -# (8B stack alignment). -dml2_ccflags += -mpreferred-stack-boundary=4 -else -dml2_ccflags += -msse2 -endif -endif +dml2_ccflags := $(CC_FLAGS_FPU) +dml2_rcflags := $(CC_FLAGS_NO_FPU) ifneq ($(CONFIG_FRAME_WARN),0) ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y)