From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "Benjamin Herrenschmidt" <benh@kernel.crashing.org>,
"Paul Mackerras" <paulus@samba.org>,
"Michael Ellerman" <mpe@ellerman.id.au>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
"amd-gfx list" <amd-gfx@lists.freedesktop.org>,
"Christian König" <christian.koenig@amd.com>,
"Alex Deucher" <alexdeucher@gmail.com>
Subject: Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()
Date: Wed, 10 Mar 2021 14:33:25 +0100 [thread overview]
Message-ID: <5b579a54-e596-bcf2-b003-5c28345447b7@csgroup.eu> (raw)
In-Reply-To: <CAMuHMdUQcE7+O9NWH4Xxxv+r7ZFnTGqtHuteOMiSPY_gK5xkZw@mail.gmail.com>
Hi Geert,
Le 09/03/2021 à 11:55, Geert Uytterhoeven a écrit :
> Hi Christophe,
>
> On Tue, Mar 9, 2021 at 10:58 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>> Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :
>>> On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
>>> <christophe.leroy@csgroup.eu> wrote:
>>>> Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
>>>>> On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
>>>>> <christophe.leroy@csgroup.eu> wrote:
>>>>>> Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
>>>>>> when CONFIG_VSX is not set, to avoid following build failure.
>>>>>>
>>>>>> CC [M] drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
>>>>>> In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
>>>>>> from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
>>>>>> from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
>>>>>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
>>>>>> ./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
>>>>>> 64 | enable_kernel_vsx(); \
>>>>>> | ^~~~~~~~~~~~~~~~~
>>>>>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
>>>>>> 640 | DC_FP_START();
>>>>>> | ^~~~~~~~~~~
>>>>>> ./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
>>>>>> 75 | disable_kernel_vsx(); \
>>>>>> | ^~~~~~~~~~~~~~~~~~
>>>>>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
>>>>>> 676 | DC_FP_END();
>>>>>> | ^~~~~~~~~
>>>>>> cc1: some warnings being treated as errors
>>>>>> make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1
>>>>>>
>>>>>> Fixes: 16a9dea110a6 ("amdgpu: Enable initial DCN support on POWER")
>>>>>> Cc: stable@vger.kernel.org
>>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>>>>
>>>>> Thanks for your patch!
>>>>>
>>>>>> --- a/arch/powerpc/include/asm/switch_to.h
>>>>>> +++ b/arch/powerpc/include/asm/switch_to.h
>>>>>> @@ -71,6 +71,16 @@ static inline void disable_kernel_vsx(void)
>>>>>> {
>>>>>> msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
>>>>>> }
>>>>>> +#else
>>>>>> +static inline void enable_kernel_vsx(void)
>>>>>> +{
>>>>>> + BUILD_BUG();
>>>>>> +}
>>>>>> +
>>>>>> +static inline void disable_kernel_vsx(void)
>>>>>> +{
>>>>>> + BUILD_BUG();
>>>>>> +}
>>>>>> #endif
>>>>>
>>>>> I'm wondering how this is any better than the current situation: using
>>>>> BUILD_BUG() will still cause a build failure?
>>>>
>>>> No it won't cause a failure. In drivers/gpu/drm/amd/display/dc/os_types.h you have:
>>>>
>>>> #define DC_FP_START() { \
>>>> if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
>>>> preempt_disable(); \
>>>> enable_kernel_vsx(); \
>>>> } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
>>>> preempt_disable(); \
>>>> enable_kernel_altivec(); \
>>>> } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
>>>> preempt_disable(); \
>>>> enable_kernel_fp(); \
>>>> } \
>>>>
>>>> When CONFIG_VSX is not selected, cpu_has_feature(CPU_FTR_VSX_COMP) constant folds to 'false' so the
>>>> call to enable_kernel_vsx() is discarded and the build succeeds.
>>>
>>> IC. So you might as well have an empty (dummy) function instead?
>>>
>>
>> But with an empty function, you take the risk that one day, someone calls it without checking that
>> CONFIG_VSX is selected. Here if someone does that, build will fail.
>
> OK, convinced.
>
Note that following build test performed on kisskb, with gcc 4.9 the following change is required in
addition:
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/b231dfa040ce4cc37f702f5c3a595fdeabfe0462.1615378209.git.christophe.leroy@csgroup.eu/
Christophe
WARNING: multiple messages have this Message-ID (diff)
From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
"amd-gfx list" <amd-gfx@lists.freedesktop.org>,
"Paul Mackerras" <paulus@samba.org>,
"Alex Deucher" <alexdeucher@gmail.com>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
"Christian König" <christian.koenig@amd.com>
Subject: Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()
Date: Wed, 10 Mar 2021 14:33:25 +0100 [thread overview]
Message-ID: <5b579a54-e596-bcf2-b003-5c28345447b7@csgroup.eu> (raw)
In-Reply-To: <CAMuHMdUQcE7+O9NWH4Xxxv+r7ZFnTGqtHuteOMiSPY_gK5xkZw@mail.gmail.com>
Hi Geert,
Le 09/03/2021 à 11:55, Geert Uytterhoeven a écrit :
> Hi Christophe,
>
> On Tue, Mar 9, 2021 at 10:58 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>> Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :
>>> On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
>>> <christophe.leroy@csgroup.eu> wrote:
>>>> Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
>>>>> On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
>>>>> <christophe.leroy@csgroup.eu> wrote:
>>>>>> Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
>>>>>> when CONFIG_VSX is not set, to avoid following build failure.
>>>>>>
>>>>>> CC [M] drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
>>>>>> In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
>>>>>> from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
>>>>>> from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
>>>>>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
>>>>>> ./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
>>>>>> 64 | enable_kernel_vsx(); \
>>>>>> | ^~~~~~~~~~~~~~~~~
>>>>>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
>>>>>> 640 | DC_FP_START();
>>>>>> | ^~~~~~~~~~~
>>>>>> ./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
>>>>>> 75 | disable_kernel_vsx(); \
>>>>>> | ^~~~~~~~~~~~~~~~~~
>>>>>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
>>>>>> 676 | DC_FP_END();
>>>>>> | ^~~~~~~~~
>>>>>> cc1: some warnings being treated as errors
>>>>>> make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1
>>>>>>
>>>>>> Fixes: 16a9dea110a6 ("amdgpu: Enable initial DCN support on POWER")
>>>>>> Cc: stable@vger.kernel.org
>>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>>>>
>>>>> Thanks for your patch!
>>>>>
>>>>>> --- a/arch/powerpc/include/asm/switch_to.h
>>>>>> +++ b/arch/powerpc/include/asm/switch_to.h
>>>>>> @@ -71,6 +71,16 @@ static inline void disable_kernel_vsx(void)
>>>>>> {
>>>>>> msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
>>>>>> }
>>>>>> +#else
>>>>>> +static inline void enable_kernel_vsx(void)
>>>>>> +{
>>>>>> + BUILD_BUG();
>>>>>> +}
>>>>>> +
>>>>>> +static inline void disable_kernel_vsx(void)
>>>>>> +{
>>>>>> + BUILD_BUG();
>>>>>> +}
>>>>>> #endif
>>>>>
>>>>> I'm wondering how this is any better than the current situation: using
>>>>> BUILD_BUG() will still cause a build failure?
>>>>
>>>> No it won't cause a failure. In drivers/gpu/drm/amd/display/dc/os_types.h you have:
>>>>
>>>> #define DC_FP_START() { \
>>>> if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
>>>> preempt_disable(); \
>>>> enable_kernel_vsx(); \
>>>> } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
>>>> preempt_disable(); \
>>>> enable_kernel_altivec(); \
>>>> } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
>>>> preempt_disable(); \
>>>> enable_kernel_fp(); \
>>>> } \
>>>>
>>>> When CONFIG_VSX is not selected, cpu_has_feature(CPU_FTR_VSX_COMP) constant folds to 'false' so the
>>>> call to enable_kernel_vsx() is discarded and the build succeeds.
>>>
>>> IC. So you might as well have an empty (dummy) function instead?
>>>
>>
>> But with an empty function, you take the risk that one day, someone calls it without checking that
>> CONFIG_VSX is selected. Here if someone does that, build will fail.
>
> OK, convinced.
>
Note that following build test performed on kisskb, with gcc 4.9 the following change is required in
addition:
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/b231dfa040ce4cc37f702f5c3a595fdeabfe0462.1615378209.git.christophe.leroy@csgroup.eu/
Christophe
WARNING: multiple messages have this Message-ID (diff)
From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "Benjamin Herrenschmidt" <benh@kernel.crashing.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
"amd-gfx list" <amd-gfx@lists.freedesktop.org>,
"Paul Mackerras" <paulus@samba.org>,
"Michael Ellerman" <mpe@ellerman.id.au>,
"Alex Deucher" <alexdeucher@gmail.com>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
"Christian König" <christian.koenig@amd.com>
Subject: Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()
Date: Wed, 10 Mar 2021 14:33:25 +0100 [thread overview]
Message-ID: <5b579a54-e596-bcf2-b003-5c28345447b7@csgroup.eu> (raw)
In-Reply-To: <CAMuHMdUQcE7+O9NWH4Xxxv+r7ZFnTGqtHuteOMiSPY_gK5xkZw@mail.gmail.com>
Hi Geert,
Le 09/03/2021 à 11:55, Geert Uytterhoeven a écrit :
> Hi Christophe,
>
> On Tue, Mar 9, 2021 at 10:58 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>> Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :
>>> On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
>>> <christophe.leroy@csgroup.eu> wrote:
>>>> Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
>>>>> On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
>>>>> <christophe.leroy@csgroup.eu> wrote:
>>>>>> Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
>>>>>> when CONFIG_VSX is not set, to avoid following build failure.
>>>>>>
>>>>>> CC [M] drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
>>>>>> In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
>>>>>> from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
>>>>>> from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
>>>>>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
>>>>>> ./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
>>>>>> 64 | enable_kernel_vsx(); \
>>>>>> | ^~~~~~~~~~~~~~~~~
>>>>>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
>>>>>> 640 | DC_FP_START();
>>>>>> | ^~~~~~~~~~~
>>>>>> ./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
>>>>>> 75 | disable_kernel_vsx(); \
>>>>>> | ^~~~~~~~~~~~~~~~~~
>>>>>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
>>>>>> 676 | DC_FP_END();
>>>>>> | ^~~~~~~~~
>>>>>> cc1: some warnings being treated as errors
>>>>>> make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1
>>>>>>
>>>>>> Fixes: 16a9dea110a6 ("amdgpu: Enable initial DCN support on POWER")
>>>>>> Cc: stable@vger.kernel.org
>>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>>>>
>>>>> Thanks for your patch!
>>>>>
>>>>>> --- a/arch/powerpc/include/asm/switch_to.h
>>>>>> +++ b/arch/powerpc/include/asm/switch_to.h
>>>>>> @@ -71,6 +71,16 @@ static inline void disable_kernel_vsx(void)
>>>>>> {
>>>>>> msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
>>>>>> }
>>>>>> +#else
>>>>>> +static inline void enable_kernel_vsx(void)
>>>>>> +{
>>>>>> + BUILD_BUG();
>>>>>> +}
>>>>>> +
>>>>>> +static inline void disable_kernel_vsx(void)
>>>>>> +{
>>>>>> + BUILD_BUG();
>>>>>> +}
>>>>>> #endif
>>>>>
>>>>> I'm wondering how this is any better than the current situation: using
>>>>> BUILD_BUG() will still cause a build failure?
>>>>
>>>> No it won't cause a failure. In drivers/gpu/drm/amd/display/dc/os_types.h you have:
>>>>
>>>> #define DC_FP_START() { \
>>>> if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
>>>> preempt_disable(); \
>>>> enable_kernel_vsx(); \
>>>> } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
>>>> preempt_disable(); \
>>>> enable_kernel_altivec(); \
>>>> } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
>>>> preempt_disable(); \
>>>> enable_kernel_fp(); \
>>>> } \
>>>>
>>>> When CONFIG_VSX is not selected, cpu_has_feature(CPU_FTR_VSX_COMP) constant folds to 'false' so the
>>>> call to enable_kernel_vsx() is discarded and the build succeeds.
>>>
>>> IC. So you might as well have an empty (dummy) function instead?
>>>
>>
>> But with an empty function, you take the risk that one day, someone calls it without checking that
>> CONFIG_VSX is selected. Here if someone does that, build will fail.
>
> OK, convinced.
>
Note that following build test performed on kisskb, with gcc 4.9 the following change is required in
addition:
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/b231dfa040ce4cc37f702f5c3a595fdeabfe0462.1615378209.git.christophe.leroy@csgroup.eu/
Christophe
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2021-03-10 13:34 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-09 8:39 [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx() Christophe Leroy
2021-03-09 8:39 ` Christophe Leroy
2021-03-09 8:39 ` Christophe Leroy
2021-03-09 8:45 ` Geert Uytterhoeven
2021-03-09 8:45 ` Geert Uytterhoeven
2021-03-09 8:45 ` Geert Uytterhoeven
2021-03-09 8:52 ` Christophe Leroy
2021-03-09 8:52 ` Christophe Leroy
2021-03-09 8:52 ` Christophe Leroy
2021-03-09 9:16 ` Geert Uytterhoeven
2021-03-09 9:16 ` Geert Uytterhoeven
2021-03-09 9:16 ` Geert Uytterhoeven
2021-03-09 9:57 ` Christophe Leroy
2021-03-09 9:57 ` Christophe Leroy
2021-03-09 9:57 ` Christophe Leroy
2021-03-09 10:55 ` Geert Uytterhoeven
2021-03-09 10:55 ` Geert Uytterhoeven
2021-03-09 10:55 ` Geert Uytterhoeven
2021-03-10 13:33 ` Christophe Leroy [this message]
2021-03-10 13:33 ` Christophe Leroy
2021-03-10 13:33 ` Christophe Leroy
2021-03-14 10:01 ` Michael Ellerman
2021-03-14 10:01 ` Michael Ellerman
2021-03-14 10:01 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5b579a54-e596-bcf2-b003-5c28345447b7@csgroup.eu \
--to=christophe.leroy@csgroup.eu \
--cc=alexdeucher@gmail.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=benh@kernel.crashing.org \
--cc=christian.koenig@amd.com \
--cc=geert@linux-m68k.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.