[PATCH 0/3] drm/amdgpu: enable DKMS build

[PATCH 0/3] drm/amdgpu: enable DKMS build

[Jammy Zhou]

This series enable the DKMS build of amdgpu driver.

Jammy Zhou (3):
  drm/amdgpu: use $(src) in Makefile
  drm/amd/acp: use $(src) in Makefile
  drm/amd/powerplay: enable dkms build

 drivers/gpu/drm/amd/acp/Makefile       |  2 +-
 drivers/gpu/drm/amd/amdgpu/Makefile    | 14 ++++++++------
 drivers/gpu/drm/amd/powerplay/Makefile | 14 +++++++-------
 3 files changed, 16 insertions(+), 14 deletions(-)

-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] drm/amdgpu: use $(src) in Makefile

[Jammy Zhou]

This can solve the path problem when compile amdgpu with DKMS.

Signed-off-by: Jammy Zhou <Jammy.Zhou@amd.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/Makefile | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index 156b726..0540a20 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -2,12 +2,14 @@
 # Makefile for the drm device driver.  This driver provides support for the
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
-ccflags-y := -Iinclude/drm -Idrivers/gpu/drm/amd/include/asic_reg \
-	-Idrivers/gpu/drm/amd/include \
-	-Idrivers/gpu/drm/amd/amdgpu \
-	-Idrivers/gpu/drm/amd/scheduler \
-	-Idrivers/gpu/drm/amd/powerplay/inc \
-	-Idrivers/gpu/drm/amd/acp/include
+FULL_AMD_PATH=$(src)/..
+
+ccflags-y := -Iinclude/drm -I$(FULL_AMD_PATH)/include/asic_reg \
+		-I$(FULL_AMD_PATH)/include \
+		-I$(FULL_AMD_PATH)/amdgpu \
+		-I$(FULL_AMD_PATH)/scheduler \
+		-I$(FULL_AMD_PATH)/powerplay/inc \
+		-I$(FULL_AMD_PATH)/acp/include
 
 amdgpu-y := amdgpu_drv.o
 
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/amd/acp: use $(src) in Makefile

[Jammy Zhou]

Signed-off-by: Jammy Zhou <Jammy.Zhou@amd.com>
---
 drivers/gpu/drm/amd/acp/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/acp/Makefile b/drivers/gpu/drm/amd/acp/Makefile
index c8c3303..b33bbd6 100644
--- a/drivers/gpu/drm/amd/acp/Makefile
+++ b/drivers/gpu/drm/amd/acp/Makefile
@@ -3,7 +3,7 @@
 # of AMDSOC/AMDGPU drm driver.
 # It provides the HW control for ACP related functionalities.
 
-ccflags-y += -Idrivers/gpu/drm/amd/include/asic_reg/acp
+ccflags-y += -I$(src)/../include/asic_reg/acp
 subdir-ccflags-y += -I$(AMDACPPATH)/ -I$(AMDACPPATH)/include
 
 AMD_ACP_FILES := $(AMDACPPATH)/acp_hw.o
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/3] drm/amd/powerplay: enable dkms build

[Jammy Zhou]

Signed-off-by: Jammy Zhou <Jammy.Zhou@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/powerplay/Makefile | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/Makefile b/drivers/gpu/drm/amd/powerplay/Makefile
index e195bf5..043e6eb 100644
--- a/drivers/gpu/drm/amd/powerplay/Makefile
+++ b/drivers/gpu/drm/amd/powerplay/Makefile
@@ -1,17 +1,17 @@
 
 subdir-ccflags-y += -Iinclude/drm  \
-		-Idrivers/gpu/drm/amd/powerplay/inc/  \
-		-Idrivers/gpu/drm/amd/include/asic_reg  \
-		-Idrivers/gpu/drm/amd/include  \
-		-Idrivers/gpu/drm/amd/powerplay/smumgr\
-		-Idrivers/gpu/drm/amd/powerplay/hwmgr \
-		-Idrivers/gpu/drm/amd/powerplay/eventmgr
+		-I$(FULL_AMD_PATH)/powerplay/inc/  \
+		-I$(FULL_AMD_PATH)/include/asic_reg  \
+		-I$(FULL_AMD_PATH)/include  \
+		-I$(FULL_AMD_PATH)/powerplay/smumgr\
+		-I$(FULL_AMD_PATH)/powerplay/hwmgr \
+		-I$(FULL_AMD_PATH)/powerplay/eventmgr
 
 AMD_PP_PATH = ../powerplay
 
 PP_LIBS = smumgr hwmgr eventmgr
 
-AMD_POWERPLAY = $(addsuffix /Makefile,$(addprefix drivers/gpu/drm/amd/powerplay/,$(PP_LIBS)))
+AMD_POWERPLAY = $(addsuffix /Makefile,$(addprefix $(FULL_AMD_PATH)/powerplay/,$(PP_LIBS)))
 
 include $(AMD_POWERPLAY)
 
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/amdgpu: enable DKMS build

[Daniel Vetter]

On Tue, Nov 24, 2015 at 04:55:19PM +0800, Jammy Zhou wrote:
> This series enable the DKMS build of amdgpu driver.

For the curious: What's DKMS?
-Daniel

> 
> Jammy Zhou (3):
>   drm/amdgpu: use $(src) in Makefile
>   drm/amd/acp: use $(src) in Makefile
>   drm/amd/powerplay: enable dkms build
> 
>  drivers/gpu/drm/amd/acp/Makefile       |  2 +-
>  drivers/gpu/drm/amd/amdgpu/Makefile    | 14 ++++++++------
>  drivers/gpu/drm/amd/powerplay/Makefile | 14 +++++++-------
>  3 files changed, 16 insertions(+), 14 deletions(-)
> 
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/amdgpu: enable DKMS build

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 505 bytes --]

On Tue, Nov 24, 2015 at 09:59:09AM +0100, Daniel Vetter wrote:
> On Tue, Nov 24, 2015 at 04:55:19PM +0800, Jammy Zhou wrote:
> > This series enable the DKMS build of amdgpu driver.
> 
> For the curious: What's DKMS?

I believe in this context it's "Dynamic Kernel Module Support":

	https://wiki.debian.org/KernelDKMS

I'm somewhat surprised that one would have to do something special to
the kernel build system to "enable" such a build. But perhaps this is
completely unrelated.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/amdgpu: enable DKMS build

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 1192 bytes --]

On 24.11.2015 10:36, Thierry Reding wrote:
> On Tue, Nov 24, 2015 at 09:59:09AM +0100, Daniel Vetter wrote:
>> On Tue, Nov 24, 2015 at 04:55:19PM +0800, Jammy Zhou wrote:
>>> This series enable the DKMS build of amdgpu driver.
>> For the curious: What's DKMS?
> I believe in this context it's "Dynamic Kernel Module Support":
>
> 	https://wiki.debian.org/KernelDKMS

Yeah, correct.

>
> I'm somewhat surprised that one would have to do something special to
> the kernel build system to "enable" such a build. But perhaps this is
> completely unrelated.

I'm a bit torn apart on this. On the one hand I'm not sure if we have 
drivers upstream explicitly supporting this as well?

On the other hand it's just the Makefiles which need to be written in a 
way which makes them relocatable. E.g. no absolute path like 
drivers/gpu/drm/amd/... in them and that's a good idea anyway.

Maybe just changing the commit message to "don't use absolute paths in 
the makefiles" would be sufficient?

Regards,
Christian.

>
> Thierry
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel


[-- Attachment #1.2: Type: text/html, Size: 2412 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/amdgpu: enable DKMS build

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2019 bytes --]

On Tue, Nov 24, 2015 at 11:08:07AM +0100, Christian König wrote:
> On 24.11.2015 10:36, Thierry Reding wrote:
> >On Tue, Nov 24, 2015 at 09:59:09AM +0100, Daniel Vetter wrote:
> >>On Tue, Nov 24, 2015 at 04:55:19PM +0800, Jammy Zhou wrote:
> >>>This series enable the DKMS build of amdgpu driver.
> >>For the curious: What's DKMS?
> >I believe in this context it's "Dynamic Kernel Module Support":
> >
> >	https://wiki.debian.org/KernelDKMS
> 
> Yeah, correct.
> 
> >
> >I'm somewhat surprised that one would have to do something special to
> >the kernel build system to "enable" such a build. But perhaps this is
> >completely unrelated.
> 
> I'm a bit torn apart on this. On the one hand I'm not sure if we have
> drivers upstream explicitly supporting this as well?

It's been a long time since I last used Debian and worked with DKMS, but
wasn't the original idea for DKMS to be used on out-of-tree modules? Why
the need to build in-tree modules with DKMS? Shouldn't that be done as
part of a regular linux-image/linux-modules build?

> On the other hand it's just the Makefiles which need to be written in a way
> which makes them relocatable. E.g. no absolute path like
> drivers/gpu/drm/amd/... in them and that's a good idea anyway.

To clarify, I'm not opposing the change, I just thought it weird that
the kernel build had to be modified to support an external build system.
But in general it seems to be rare that drivers even need to specific
additional ccflags, which would explain my surprise.

There's precedent for using $(src), see drivers/gpu/drm/Makefile, so I
don't see any reason why this would be objectionable.

> Maybe just changing the commit message to "don't use absolute paths in the
> makefiles" would be sufficient?

I don't mind the mention of DKMS either, but perhaps extending the
commit message with a few words about what DKMS is and why the change
needs to happen (i.e. $(src) used for relocation) would be a good idea
anyway.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/amdgpu: enable DKMS build

[Daniel Vetter]

On Tue, Nov 24, 2015 at 11:08:07AM +0100, Christian König wrote:
> On 24.11.2015 10:36, Thierry Reding wrote:
> >On Tue, Nov 24, 2015 at 09:59:09AM +0100, Daniel Vetter wrote:
> >>On Tue, Nov 24, 2015 at 04:55:19PM +0800, Jammy Zhou wrote:
> >>>This series enable the DKMS build of amdgpu driver.
> >>For the curious: What's DKMS?
> >I believe in this context it's "Dynamic Kernel Module Support":
> >
> >	https://wiki.debian.org/KernelDKMS
> 
> Yeah, correct.
> 
> >
> >I'm somewhat surprised that one would have to do something special to
> >the kernel build system to "enable" such a build. But perhaps this is
> >completely unrelated.
> 
> I'm a bit torn apart on this. On the one hand I'm not sure if we have
> drivers upstream explicitly supporting this as well?
> 
> On the other hand it's just the Makefiles which need to be written in a way
> which makes them relocatable. E.g. no absolute path like
> drivers/gpu/drm/amd/... in them and that's a good idea anyway.
> 
> Maybe just changing the commit message to "don't use absolute paths in the
> makefiles" would be sufficient?

Oh I don't mind DKMS support, it makes sense to allow amd to use upstream
as the baseline for their enhanced blob driver stack. Just wanted to know
what it is. Imo what we shouldn't merge upstream would be compat code to
allow amdgpu to be built on old kernels, since that's a lot more invasive
than a few Makefile changes. Otoh that problem is largely address with the
linux backporting project, which solves the "new driver directoy in old
sources problem". So as long as it's just about building free-standing I
think it's perfectly fine.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH 0/3] drm/amdgpu: enable DKMS build

[Zhou, Jammy]

Thanks all for the comments. The simple changes in this series are just to use relative paths as Christian mentioned. It can also be helpful if some users want to try amdgpu driver from latest upstream on some systems with relatively older kernel installed, in which case the Dynamic Kernel Module Support mechanism is used.

Regards,
Jammy

-----Original Message-----
From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of Daniel Vetter
Sent: Tuesday, November 24, 2015 6:34 PM
To: Christian König
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 0/3] drm/amdgpu: enable DKMS build

On Tue, Nov 24, 2015 at 11:08:07AM +0100, Christian König wrote:
> On 24.11.2015 10:36, Thierry Reding wrote:
> >On Tue, Nov 24, 2015 at 09:59:09AM +0100, Daniel Vetter wrote:
> >>On Tue, Nov 24, 2015 at 04:55:19PM +0800, Jammy Zhou wrote:
> >>>This series enable the DKMS build of amdgpu driver.
> >>For the curious: What's DKMS?
> >I believe in this context it's "Dynamic Kernel Module Support":
> >
> >	https://wiki.debian.org/KernelDKMS
> 
> Yeah, correct.
> 
> >
> >I'm somewhat surprised that one would have to do something special to 
> >the kernel build system to "enable" such a build. But perhaps this is 
> >completely unrelated.
> 
> I'm a bit torn apart on this. On the one hand I'm not sure if we have 
> drivers upstream explicitly supporting this as well?
> 
> On the other hand it's just the Makefiles which need to be written in 
> a way which makes them relocatable. E.g. no absolute path like 
> drivers/gpu/drm/amd/... in them and that's a good idea anyway.
> 
> Maybe just changing the commit message to "don't use absolute paths in 
> the makefiles" would be sufficient?

Oh I don't mind DKMS support, it makes sense to allow amd to use upstream as the baseline for their enhanced blob driver stack. Just wanted to know what it is. Imo what we shouldn't merge upstream would be compat code to allow amdgpu to be built on old kernels, since that's a lot more invasive than a few Makefile changes. Otoh that problem is largely address with the linux backporting project, which solves the "new driver directoy in old sources problem". So as long as it's just about building free-standing I think it's perfectly fine.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/amdgpu: use $(src) in Makefile

[Alex Deucher]

On Tue, Nov 24, 2015 at 3:55 AM, Jammy Zhou <Jammy.Zhou@amd.com> wrote:
> This can solve the path problem when compile amdgpu with DKMS.
>
> Signed-off-by: Jammy Zhou <Jammy.Zhou@amd.com>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>

Applied and rebased for drm-next.  I'll adapt the acp, powerplay, etc.
changes as those components make it upstream.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/Makefile | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 156b726..0540a20 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -2,12 +2,14 @@
>  # Makefile for the drm device driver.  This driver provides support for the
>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>
> -ccflags-y := -Iinclude/drm -Idrivers/gpu/drm/amd/include/asic_reg \
> -       -Idrivers/gpu/drm/amd/include \
> -       -Idrivers/gpu/drm/amd/amdgpu \
> -       -Idrivers/gpu/drm/amd/scheduler \
> -       -Idrivers/gpu/drm/amd/powerplay/inc \
> -       -Idrivers/gpu/drm/amd/acp/include
> +FULL_AMD_PATH=$(src)/..
> +
> +ccflags-y := -Iinclude/drm -I$(FULL_AMD_PATH)/include/asic_reg \
> +               -I$(FULL_AMD_PATH)/include \
> +               -I$(FULL_AMD_PATH)/amdgpu \
> +               -I$(FULL_AMD_PATH)/scheduler \
> +               -I$(FULL_AMD_PATH)/powerplay/inc \
> +               -I$(FULL_AMD_PATH)/acp/include
>
>  amdgpu-y := amdgpu_drv.o
>
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel