[PATCH] drm/amdgpu: drop the long-double-128 powerpc check/hack

[PATCH] drm/amdgpu: drop the long-double-128 powerpc check/hack

[Daniel Kolesa]

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 <daniel@octaforge.org>
---
 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

RE: [PATCH] drm/amdgpu: drop the long-double-128 powerpc check/hack

[Deucher, Alexander]

[Public]

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Daniel Kolesa
> Sent: Wednesday, February 1, 2023 1:46 PM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: dan@danny.cz; stable@vger.kernel.org;
> tpearson@raptorengineering.com; amd-gfx@lists.freedesktop.org;
> alexdeucher@gmail.com; torvalds@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 <daniel@octaforge.org>

Acked-by: Alex Deucher <alexander.deucher@amd.com>

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

Re: [PATCH] drm/amdgpu: drop the long-double-128 powerpc check/hack

[Michael Ellerman]

"Daniel Kolesa" <daniel@octaforge.org> 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

[Christophe Leroy]

Le 31/03/2023 à 12:53, Michael Ellerman a écrit :
> "Daniel Kolesa" <daniel@octaforge.org> 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

[Michael Ellerman]

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 31/03/2023 à 12:53, Michael Ellerman a écrit :
>> "Daniel Kolesa" <daniel@octaforge.org> 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