[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1aac1f8c-00e7-87e5-2dc9-433367e284c2@infradead.org>
Date: Tue, 20 Apr 2021 11:25:26 -0700
From: Randy Dunlap <rdunlap@...radead.org>
To: Michael Ellerman <mpe@...erman.id.au>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Segher Boessenkool <segher@...nel.crashing.org>
Cc: PowerPC <linuxppc-dev@...ts.ozlabs.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr
On 4/20/21 6:15 AM, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@...roup.eu> writes:
>> Le 19/04/2021 à 23:39, Randy Dunlap a écrit :
>>> On 4/19/21 6:16 AM, Michael Ellerman wrote:
>>>> Randy Dunlap <rdunlap@...radead.org> writes:
>>>
>>>>> Sure. I'll post them later today.
>>>>> They keep FPU and ALTIVEC as independent (build) features.
>>>>
>>>> Those patches look OK.
>>>>
>>>> But I don't think it makes sense to support that configuration, FPU=n
>>>> ALTVEC=y. No one is ever going to make a CPU like that. We have enough
>>>> testing surface due to configuration options, without adding artificial
>>>> combinations that no one is ever going to use.
>>>>
>>>> IMHO :)
>>>>
>>>> So I'd rather we just make ALTIVEC depend on FPU.
>>>
>>> That's rather simple. See below.
>>> I'm doing a bunch of randconfig builds with it now.
>>>
>>> ---
>>> From: Randy Dunlap <rdunlap@...radead.org>
>>> Subject: [PATCH] powerpc: make ALTIVEC depend PPC_FPU
>>>
>>> On a kernel config with ALTIVEC=y and PPC_FPU not set/enabled,
>>> there are build errors:
>>>
>>> drivers/cpufreq/pmac32-cpufreq.c:262:2: error: implicit declaration of function 'enable_kernel_fp' [-Werror,-Wimplicit-function-declaration]
>>> enable_kernel_fp();
>>> ../arch/powerpc/lib/sstep.c: In function 'do_vec_load':
>>> ../arch/powerpc/lib/sstep.c:637:3: error: implicit declaration of function 'put_vr' [-Werror=implicit-function-declaration]
>>> 637 | put_vr(rn, &u.v);
>>> | ^~~~~~
>>> ../arch/powerpc/lib/sstep.c: In function 'do_vec_store':
>>> ../arch/powerpc/lib/sstep.c:660:3: error: implicit declaration of function 'get_vr'; did you mean 'get_oc'? [-Werror=implicit-function-declaration]
>>> 660 | get_vr(rn, &u.v);
>>> | ^~~~~~
>>>
>>> In theory ALTIVEC is independent of PPC_FPU but in practice nobody
>>> is going to build such a machine, so make ALTIVEC require PPC_FPU
>>> by depending on PPC_FPU.
>>>
>>> Signed-off-by: Randy Dunlap <rdunlap@...radead.org>
>>> Reported-by: kernel test robot <lkp@...el.com>
>>> Cc: Michael Ellerman <mpe@...erman.id.au>
>>> Cc: linuxppc-dev@...ts.ozlabs.org
>>> Cc: Christophe Leroy <christophe.leroy@...roup.eu>
>>> Cc: Segher Boessenkool <segher@...nel.crashing.org>
>>> Cc: lkp@...el.com
>>> ---
>>> arch/powerpc/platforms/86xx/Kconfig | 1 +
>>> arch/powerpc/platforms/Kconfig.cputype | 2 ++
>>> 2 files changed, 3 insertions(+)
>>>
>>> --- linux-next-20210416.orig/arch/powerpc/platforms/86xx/Kconfig
>>> +++ linux-next-20210416/arch/powerpc/platforms/86xx/Kconfig
>>> @@ -4,6 +4,7 @@ menuconfig PPC_86xx
>>> bool "86xx-based boards"
>>> depends on PPC_BOOK3S_32
>>> select FSL_SOC
>>> + select PPC_FPU
>>> select ALTIVEC
>>> help
>>> The Freescale E600 SoCs have 74xx cores.
>>> --- linux-next-20210416.orig/arch/powerpc/platforms/Kconfig.cputype
>>> +++ linux-next-20210416/arch/powerpc/platforms/Kconfig.cputype
>>> @@ -186,6 +186,7 @@ config E300C3_CPU
>>> config G4_CPU
>>> bool "G4 (74xx)"
>>> depends on PPC_BOOK3S_32
>>> + select PPC_FPU
>>> select ALTIVEC
>>>
>>> endchoice
>>> @@ -309,6 +310,7 @@ config PHYS_64BIT
>>>
>>> config ALTIVEC
>>> bool "AltiVec Support"
>>> + depends on PPC_FPU
>>
>> Shouldn't we do it the other way round ? In extenso make ALTIVEC select PPC_FPU and avoid the two
>> selects that are above ?
>
> Yes, ALTIVEC should select PPC_FPU.
>
> The latter is (generally) not user selectable, so there's no issue with
> selecting it, whereas the reverse is not true.
>
> For 64-bit Book3S I think we could just always enable ALTIVEC these
> days. It's only Power5 that doesn't have it, and essentially no one is
> running mainline on those AFAIK. But that can be done separately.
OK, I'll run that thru some tests today.
thanks.
--
~Randy
Powered by blists - more mailing lists