diff mbox series

[4/6] drm/amd/display: fix dcn21 Makefile for clang

Message ID 20191002120136.1777161-5-arnd@arndb.de (mailing list archive)
State New, archived
Headers show
Series amdgpu build fixes | expand

Commit Message

Arnd Bergmann Oct. 2, 2019, 12:01 p.m. UTC
Just like all the other variants, this one passes invalid
compile-time options with clang after the new code got
merged:

clang: error: unknown argument: '-mpreferred-stack-boundary=4'
scripts/Makefile.build:265: recipe for target 'drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.o' failed

Use the same variant that we have for dcn20 to fix compilation.

Fixes: eced51f9babb ("drm/amd/display: Add hubp block for Renoir (v2)")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpu/drm/amd/display/dc/dcn21/Makefile | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Alex Deucher Oct. 2, 2019, 2:17 p.m. UTC | #1
On Wed, Oct 2, 2019 at 8:03 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Just like all the other variants, this one passes invalid
> compile-time options with clang after the new code got
> merged:
>
> clang: error: unknown argument: '-mpreferred-stack-boundary=4'
> scripts/Makefile.build:265: recipe for target 'drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.o' failed
>
> Use the same variant that we have for dcn20 to fix compilation.
>
> Fixes: eced51f9babb ("drm/amd/display: Add hubp block for Renoir (v2)")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I'm getting an error with gcc with this patch:
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.o
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.c: In
function ‘calculate_wm_set_for_vlevel’:
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.c:964:22:
error: SSE register return with SSE disabled
  wm_set->urgent_ns = get_wm_urgent(dml, pipes, pipe_cnt) * 1000;
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[4]: *** [scripts/Makefile.build:273:
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.o] Error
1
make[3]: *** [scripts/Makefile.build:490: drivers/gpu/drm/amd/amdgpu] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [scripts/Makefile.build:490: drivers/gpu/drm] Error 2
make[1]: *** [scripts/Makefile.build:490: drivers/gpu] Error 2
make: *** [Makefile:1080: drivers] Error 2

Alex

> ---
>  drivers/gpu/drm/amd/display/dc/dcn21/Makefile | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> index 8cd9de8b1a7a..ef673bffc241 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> +++ b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> @@ -3,7 +3,17 @@
>
>  DCN21 = dcn21_hubp.o dcn21_hubbub.o dcn21_resource.o
>
> -CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse -mpreferred-stack-boundary=4
> +ifneq ($(call cc-option, -mpreferred-stack-boundary=4),)
> +       cc_stack_align := -mpreferred-stack-boundary=4
> +else ifneq ($(call cc-option, -mstack-alignment=16),)
> +       cc_stack_align := -mstack-alignment=16
> +endif
> +
> +CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse $(cc_stack_align)
> +
> +ifdef CONFIG_CC_IS_CLANG
> +CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o += -msse2
> +endif
>
>  AMD_DAL_DCN21 = $(addprefix $(AMDDALPATH)/dc/dcn21/,$(DCN21))
>
> --
> 2.20.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Arnd Bergmann Oct. 2, 2019, 2:51 p.m. UTC | #2
On Wed, Oct 2, 2019 at 4:17 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> I'm getting an error with gcc with this patch:
>   CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.o
> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.c: In
> function ‘calculate_wm_set_for_vlevel’:
> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.c:964:22:
> error: SSE register return with SSE disabled

I checked again with gcc-8, but do not see that error message.

> > -CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse -mpreferred-stack-boundary=4
> > +CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse $(cc_stack_align)

Nothing should really change with regards to the -msse flag here, only
the stack alignment flag changed. Maybe there was some other change
in your Makefile that conflicts with my my patch?

       Arnd
Alex Deucher Oct. 2, 2019, 3:12 p.m. UTC | #3
On Wed, Oct 2, 2019 at 10:51 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Wed, Oct 2, 2019 at 4:17 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > I'm getting an error with gcc with this patch:
> >   CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.o
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.c: In
> > function ‘calculate_wm_set_for_vlevel’:
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.c:964:22:
> > error: SSE register return with SSE disabled
>
> I checked again with gcc-8, but do not see that error message.
>
> > > -CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse -mpreferred-stack-boundary=4
> > > +CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse $(cc_stack_align)
>
> Nothing should really change with regards to the -msse flag here, only
> the stack alignment flag changed. Maybe there was some other change
> in your Makefile that conflicts with my my patch?

This patch on top of yours seems to fix it and aligns better with the
other Makefiles:

diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
index ef673bffc241..e71f3ee76cd1 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
@@ -9,10 +9,10 @@ else ifneq ($(call cc-option, -mstack-alignment=16),)
        cc_stack_align := -mstack-alignment=16
 endif

-CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse
$(cc_stack_align)
+CFLAGS_dcn21_resource.o := -mhard-float -msse $(cc_stack_align)

 ifdef CONFIG_CC_IS_CLANG
-CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o += -msse2
+CFLAGS_dcn21_resource.o += -msse2
 endif

 AMD_DAL_DCN21 = $(addprefix $(AMDDALPATH)/dc/dcn21/,$(DCN21))
Arnd Bergmann Oct. 2, 2019, 3:39 p.m. UTC | #4
On Wed, Oct 2, 2019 at 5:12 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> On Wed, Oct 2, 2019 at 10:51 AM Arnd Bergmann <arnd@arndb.de> wrote:

> >
> > Nothing should really change with regards to the -msse flag here, only
> > the stack alignment flag changed. Maybe there was some other change
> > in your Makefile that conflicts with my my patch?
>
> This patch on top of yours seems to fix it and aligns better with the
> other Makefiles:
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> index ef673bffc241..e71f3ee76cd1 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> +++ b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> @@ -9,10 +9,10 @@ else ifneq ($(call cc-option, -mstack-alignment=16),)
>         cc_stack_align := -mstack-alignment=16
>  endif
>
> -CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse
> $(cc_stack_align)
> +CFLAGS_dcn21_resource.o := -mhard-float -msse $(cc_stack_align)
>
>  ifdef CONFIG_CC_IS_CLANG
> -CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o += -msse2
> +CFLAGS_dcn21_resource.o += -msse2
>  endif

Ok, so there is clearly a global change that went into your tree, or
is missing from it:

I see that as of linux-5.4-rc1, I have commit 54b8ae66ae1a ("kbuild: change
 *FLAGS_<basetarget>.o to take the path relative to $(obj)"), which changed
all these path names to include the AMDDALPATH.

It seems you are either on an older kernel that does not yet have this,
or you have applied another patch that reverts it.

       Arnd
Alex Deucher Oct. 2, 2019, 4:33 p.m. UTC | #5
On Wed, Oct 2, 2019 at 11:39 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Wed, Oct 2, 2019 at 5:12 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> > On Wed, Oct 2, 2019 at 10:51 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> > >
> > > Nothing should really change with regards to the -msse flag here, only
> > > the stack alignment flag changed. Maybe there was some other change
> > > in your Makefile that conflicts with my my patch?
> >
> > This patch on top of yours seems to fix it and aligns better with the
> > other Makefiles:
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> > b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> > index ef673bffc241..e71f3ee76cd1 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> > @@ -9,10 +9,10 @@ else ifneq ($(call cc-option, -mstack-alignment=16),)
> >         cc_stack_align := -mstack-alignment=16
> >  endif
> >
> > -CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse
> > $(cc_stack_align)
> > +CFLAGS_dcn21_resource.o := -mhard-float -msse $(cc_stack_align)
> >
> >  ifdef CONFIG_CC_IS_CLANG
> > -CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o += -msse2
> > +CFLAGS_dcn21_resource.o += -msse2
> >  endif
>
> Ok, so there is clearly a global change that went into your tree, or
> is missing from it:
>
> I see that as of linux-5.4-rc1, I have commit 54b8ae66ae1a ("kbuild: change
>  *FLAGS_<basetarget>.o to take the path relative to $(obj)"), which changed
> all these path names to include the AMDDALPATH.
>
> It seems you are either on an older kernel that does not yet have this,
> or you have applied another patch that reverts it.

Ah, I don't have that patch yet in my tree.  That explains it.

Alex
Nick Desaulniers Oct. 2, 2019, 9:14 p.m. UTC | #6
On Wed, Oct 2, 2019 at 5:03 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Just like all the other variants, this one passes invalid
> compile-time options with clang after the new code got
> merged:
>
> clang: error: unknown argument: '-mpreferred-stack-boundary=4'
> scripts/Makefile.build:265: recipe for target 'drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.o' failed
>
> Use the same variant that we have for dcn20 to fix compilation.
>
> Fixes: eced51f9babb ("drm/amd/display: Add hubp block for Renoir (v2)")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
(Though I think it's already been merged)

Alex, do you know why the AMDGPU driver uses a different stack
alignment (16B) than the rest of the x86 kernel?  (see
arch/x86/Makefile which uses 8B stack alignment).

> ---
>  drivers/gpu/drm/amd/display/dc/dcn21/Makefile | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> index 8cd9de8b1a7a..ef673bffc241 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> +++ b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> @@ -3,7 +3,17 @@
>
>  DCN21 = dcn21_hubp.o dcn21_hubbub.o dcn21_resource.o
>
> -CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse -mpreferred-stack-boundary=4
> +ifneq ($(call cc-option, -mpreferred-stack-boundary=4),)
> +       cc_stack_align := -mpreferred-stack-boundary=4
> +else ifneq ($(call cc-option, -mstack-alignment=16),)
> +       cc_stack_align := -mstack-alignment=16
> +endif
> +
> +CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse $(cc_stack_align)
> +
> +ifdef CONFIG_CC_IS_CLANG
> +CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o += -msse2
> +endif
>
>  AMD_DAL_DCN21 = $(addprefix $(AMDDALPATH)/dc/dcn21/,$(DCN21))
>
> --
> 2.20.0
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20191002120136.1777161-5-arnd%40arndb.de.
Alex Deucher Oct. 2, 2019, 9:24 p.m. UTC | #7
On Wed, Oct 2, 2019 at 5:19 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Wed, Oct 2, 2019 at 5:03 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > Just like all the other variants, this one passes invalid
> > compile-time options with clang after the new code got
> > merged:
> >
> > clang: error: unknown argument: '-mpreferred-stack-boundary=4'
> > scripts/Makefile.build:265: recipe for target 'drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.o' failed
> >
> > Use the same variant that we have for dcn20 to fix compilation.
> >
> > Fixes: eced51f9babb ("drm/amd/display: Add hubp block for Renoir (v2)")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Thanks for the patch!
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
> (Though I think it's already been merged)
>
> Alex, do you know why the AMDGPU driver uses a different stack
> alignment (16B) than the rest of the x86 kernel?  (see
> arch/x86/Makefile which uses 8B stack alignment).

Not sure.  Maybe Harry can comment.  I think it was added for the
floating point stuff.  Not sure if it's strictly required or not.

Alex

>
> > ---
> >  drivers/gpu/drm/amd/display/dc/dcn21/Makefile | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> > index 8cd9de8b1a7a..ef673bffc241 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> > @@ -3,7 +3,17 @@
> >
> >  DCN21 = dcn21_hubp.o dcn21_hubbub.o dcn21_resource.o
> >
> > -CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse -mpreferred-stack-boundary=4
> > +ifneq ($(call cc-option, -mpreferred-stack-boundary=4),)
> > +       cc_stack_align := -mpreferred-stack-boundary=4
> > +else ifneq ($(call cc-option, -mstack-alignment=16),)
> > +       cc_stack_align := -mstack-alignment=16
> > +endif
> > +
> > +CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse $(cc_stack_align)
> > +
> > +ifdef CONFIG_CC_IS_CLANG
> > +CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o += -msse2
> > +endif
> >
> >  AMD_DAL_DCN21 = $(addprefix $(AMDDALPATH)/dc/dcn21/,$(DCN21))
> >
> > --
> > 2.20.0
> >
> > --
> > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20191002120136.1777161-5-arnd%40arndb.de.
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Nick Desaulniers Oct. 2, 2019, 9:27 p.m. UTC | #8
On Wed, Oct 2, 2019 at 2:24 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Wed, Oct 2, 2019 at 5:19 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > Alex, do you know why the AMDGPU driver uses a different stack
> > alignment (16B) than the rest of the x86 kernel?  (see
> > arch/x86/Makefile which uses 8B stack alignment).
>
> Not sure.  Maybe Harry can comment.  I think it was added for the
> floating point stuff.  Not sure if it's strictly required or not.

Can you find out for me please who knows more about this and setup a
chat with all of us? (I don't want to deride this patch's review
thread, so let's start a new thread once we know more) We're facing
some interesting runtime issues when built with Clang.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
index 8cd9de8b1a7a..ef673bffc241 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
@@ -3,7 +3,17 @@ 
 
 DCN21 = dcn21_hubp.o dcn21_hubbub.o dcn21_resource.o
 
-CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse -mpreferred-stack-boundary=4
+ifneq ($(call cc-option, -mpreferred-stack-boundary=4),)
+	cc_stack_align := -mpreferred-stack-boundary=4
+else ifneq ($(call cc-option, -mstack-alignment=16),)
+	cc_stack_align := -mstack-alignment=16
+endif
+
+CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse $(cc_stack_align)
+
+ifdef CONFIG_CC_IS_CLANG
+CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o += -msse2
+endif
 
 AMD_DAL_DCN21 = $(addprefix $(AMDDALPATH)/dc/dcn21/,$(DCN21))