Re: [PATCH] drm/amdgpu: drop the long-double-128 powerpc check/hack
Christophe Leroy writes:
> Le 31/03/2023 à 12:53, Michael Ellerman a écrit :
>> "Daniel Kolesa" writes:
>>> Commit c653c591789b ("drm/amdgpu: Re-enable DCN for 64-bit powerpc")
>>> introduced this check as a workaround for the driver not building
>>> with toolchains that default to 64-bit long double.
>> ...
>>> In mainline, this work is now fully done, so this check is fully
>>> redundant and does not do anything except preventing AMDGPU DC
>>> from being built on systems such as those using musl libc. The
>>> last piece of work to enable this was commit c92b7fe0d92a
>>> ("drm/amd/display: move remaining FPU code to dml folder")
>>> and this has since been backported to 6.1 stable (in 6.1.7).
>>>
>>> Relevant issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2288
>>
>> I looked to pick this up for 6.3 but was still seeing build errors with
>> some compilers. I assumed that was due to some fixes coming in
>> linux-next that I didn't have.
>>
>> But applying the patch on v6.3-rc4 I still see build errors. This is
>> building allyesconfig with the kernel.org GCC 12.2.0 / binutils 2.39
>> toolchain:
>>
>>powerpc64le-linux-gnu-ld:
>> drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.o uses hard float,
>> arch/powerpc/lib/test_emulate_step.o uses soft float
>>powerpc64le-linux-gnu-ld: failed to merge target specific data of file
>> drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.o
>>
>> etc.
>>
>> All the conflicts are between test_emulate_step.o and some file in
>> drivers/gpu/drm/amd/display/dc/dml.
>>
>> So even with all the hard-float code isolated in the dml folder, we
>> still hit build errors, because allyesconfig wants to link those
>> hard-float using objects with soft-float objects from elsewhere in the
>> kernel.
>>
>> It seems like the only workable fix is to force the kernel build to use
>> 128-bit long double. I'll send a patch doing that.
>>
>
> Commit 78f0929884d4 ("powerpc/64: Always build with 128-bit long
> double") I guess ?
Yes.
> Let's drop this patch from patchwork then.
Thanks.
cheers
Re: [PATCH] drm/amdgpu: drop the long-double-128 powerpc check/hack
Le 31/03/2023 à 12:53, Michael Ellerman a écrit :
> "Daniel Kolesa" writes:
>> Commit c653c591789b ("drm/amdgpu: Re-enable DCN for 64-bit powerpc")
>> introduced this check as a workaround for the driver not building
>> with toolchains that default to 64-bit long double.
> ...
>> In mainline, this work is now fully done, so this check is fully
>> redundant and does not do anything except preventing AMDGPU DC
>> from being built on systems such as those using musl libc. The
>> last piece of work to enable this was commit c92b7fe0d92a
>> ("drm/amd/display: move remaining FPU code to dml folder")
>> and this has since been backported to 6.1 stable (in 6.1.7).
>>
>> Relevant issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2288
>
> I looked to pick this up for 6.3 but was still seeing build errors with
> some compilers. I assumed that was due to some fixes coming in
> linux-next that I didn't have.
>
> But applying the patch on v6.3-rc4 I still see build errors. This is
> building allyesconfig with the kernel.org GCC 12.2.0 / binutils 2.39
> toolchain:
>
>powerpc64le-linux-gnu-ld:
> drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.o uses hard float,
> arch/powerpc/lib/test_emulate_step.o uses soft float
>powerpc64le-linux-gnu-ld: failed to merge target specific data of file
> drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.o
>
> etc.
>
> All the conflicts are between test_emulate_step.o and some file in
> drivers/gpu/drm/amd/display/dc/dml.
>
> So even with all the hard-float code isolated in the dml folder, we
> still hit build errors, because allyesconfig wants to link those
> hard-float using objects with soft-float objects from elsewhere in the
> kernel.
>
> It seems like the only workable fix is to force the kernel build to use
> 128-bit long double. I'll send a patch doing that.
>
Commit 78f0929884d4 ("powerpc/64: Always build with 128-bit long
double") I guess ?
Let's drop this patch from patchwork then.
Re: [PATCH] drm/amdgpu: drop the long-double-128 powerpc check/hack
"Daniel Kolesa" writes:
> Commit c653c591789b ("drm/amdgpu: Re-enable DCN for 64-bit powerpc")
> introduced this check as a workaround for the driver not building
> with toolchains that default to 64-bit long double.
...
> In mainline, this work is now fully done, so this check is fully
> redundant and does not do anything except preventing AMDGPU DC
> from being built on systems such as those using musl libc. The
> last piece of work to enable this was commit c92b7fe0d92a
> ("drm/amd/display: move remaining FPU code to dml folder")
> and this has since been backported to 6.1 stable (in 6.1.7).
>
> Relevant issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2288
I looked to pick this up for 6.3 but was still seeing build errors with
some compilers. I assumed that was due to some fixes coming in
linux-next that I didn't have.
But applying the patch on v6.3-rc4 I still see build errors. This is
building allyesconfig with the kernel.org GCC 12.2.0 / binutils 2.39
toolchain:
powerpc64le-linux-gnu-ld:
drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.o uses hard float,
arch/powerpc/lib/test_emulate_step.o uses soft float
powerpc64le-linux-gnu-ld: failed to merge target specific data of file
drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.o
etc.
All the conflicts are between test_emulate_step.o and some file in
drivers/gpu/drm/amd/display/dc/dml.
So even with all the hard-float code isolated in the dml folder, we
still hit build errors, because allyesconfig wants to link those
hard-float using objects with soft-float objects from elsewhere in the
kernel.
It seems like the only workable fix is to force the kernel build to use
128-bit long double. I'll send a patch doing that.
cheers
RE: [PATCH] drm/amdgpu: drop the long-double-128 powerpc check/hack
[Public]
> -Original Message-
> From: amd-gfx On Behalf Of
> Daniel Kolesa
> Sent: Wednesday, February 1, 2023 1:46 PM
> To: linuxppc-...@lists.ozlabs.org
> Cc: d...@danny.cz; sta...@vger.kernel.org;
> tpear...@raptorengineering.com; amd-gfx@lists.freedesktop.org;
> alexdeuc...@gmail.com; torva...@linux-foundation.org
> Subject: [PATCH] drm/amdgpu: drop the long-double-128 powerpc
> check/hack
>
> Commit c653c591789b ("drm/amdgpu: Re-enable DCN for 64-bit powerpc")
> introduced this check as a workaround for the driver not building with
> toolchains that default to 64-bit long double.
>
> The reason things worked on 128-bit-long-double toolchains and not
> otherwise was however largely accidental. The real issue was that some files
> containing floating point code were compiled without -mhard-float, while
> others were compiled with -mhard-float.
>
> The PowerPC compilers tag object files that use long doubles with a special
> ABI tag in order to differentiate 64-bit long double, IBM long double and IEEE
> 128-bit long double. When no long double is used in a source file, the file
> does not receive the ABI tag.
> Since only regular doubles are used in the AMDGPU source, there is no ABI
> tag on the soft-float object files with 128-bit-ldbl compilers, and therefore
> no
> error. With 64-bit long double, the double and long double types are equal,
> and an ABI tag is introduced.
>
> Of course, this resulted in the real bug, which was mixing of hard and soft
> float object files, getting hidden, which makes this check technically
> incorrect.
>
> Since then, work has been done to ensure that all float code is separately
> compiled. This was also necessary in order to enable
> AArch64 support in the display stack, as AArch64 does not have any soft-float
> ABI, and all code that does not explicitly conatain floats is compiled with -
> mgeneral-regs-only, which prevents float-using code from being compiled at
> all. That means AArch64 support will from now on always safeguard against
> such cases happening ever again.
>
> In mainline, this work is now fully done, so this check is fully redundant and
> does not do anything except preventing AMDGPU DC from being built on
> systems such as those using musl libc. The last piece of work to enable this
> was commit c92b7fe0d92a
> ("drm/amd/display: move remaining FPU code to dml folder") and this has
> since been backported to 6.1 stable (in 6.1.7).
>
> Relevant issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2288
>
> Signed-off-by: Daniel Kolesa
Acked-by: Alex Deucher
> ---
> arch/powerpc/Kconfig| 4
> drivers/gpu/drm/amd/display/Kconfig | 2 +-
> 2 files changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index
> b8c4ac56b..267805072 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -289,10 +289,6 @@ config PPC
> # Please keep this list sorted alphabetically.
> #
>
> -config PPC_LONG_DOUBLE_128
> - depends on PPC64 && ALTIVEC
> - def_bool $(success,test "$(shell,echo __LONG_DOUBLE_128__ |
> $(CC) -E -P -)" = 1)
> -
> config PPC_BARRIER_NOSPEC
> bool
> default y
> diff --git a/drivers/gpu/drm/amd/display/Kconfig
> b/drivers/gpu/drm/amd/display/Kconfig
> index 2efe93f74..94645b6ef 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 || X86_64 || SPARC64 ||
> ARM64
> select SND_HDA_COMPONENT if SND_HDA_CORE
> # !CC_IS_CLANG:
> https://github.com/ClangBuiltLinux/linux/issues/1752
> - select DRM_AMD_DC_DCN if (X86 || PPC_LONG_DOUBLE_128 ||
> (ARM64 && KERNEL_MODE_NEON && !CC_IS_CLANG))
> + select DRM_AMD_DC_DCN if (X86 || PPC64 || (ARM64 &&
> KERNEL_MODE_NEON
> +&& !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
> --
> 2.34.1
