[PATCH 0/3] Deferr load of radeon/amdgpu until amdkfd is loaded

[PATCH 0/3] Deferr load of radeon/amdgpu until amdkfd is loaded

[Oded Gabbay]

Following Daniel's request, I spent some time removing the hard requirement 
that radeon and amdgpu will always appear _after_ amdkfd in the drm Makefile.

This was done by modifing radeon/amdgpu to defer their loading if they detect 
that amdkfd is not loaded yet, in case the drivers are built inside the 
kernel image. 

See the patch's individiual commit messages for more explanation.

This patch-set was tested on a KAVERI machine, with multiple configurations:

1. radeon + amdgpu (CIK disabled) + amdkfd as kernel modules
2. radeon + amdgpu (CIK disabled) + amdkfd inside the kernel image
3. amdgpu (CIK enabled) + amdkfd inside the kernel image (radeon not compiled)
4. amdgpu (CIK enabled) inside the kernel image (radeon + amdkfd not compiled)
5. radeon + amdgpu (CIK disabled) as kernel modules (amdkfd not compiled)

Thanks,

    Oded

Oded Gabbay (3):
  drm/amdkfd: Track when module's init is complete
  drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded
  drm/amdgpu: Return -EPROBE_DEFER when amdkfd not loaded

 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c      | 57 +++++++++----------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h      |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c         | 10 ++++-
 drivers/gpu/drm/amd/amdkfd/kfd_module.c         | 15 +++++--
 drivers/gpu/drm/amd/include/kgd_kfd_interface.h |  2 +-
 drivers/gpu/drm/radeon/radeon_drv.c             | 10 ++++-
 drivers/gpu/drm/radeon/radeon_kfd.c             | 25 ++++++-----
 drivers/gpu/drm/radeon/radeon_kfd.h             |  2 +-
 8 files changed, 64 insertions(+), 59 deletions(-)

-- 
2.5.0

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

[PATCH 1/3] drm/amdkfd: Track when module's init is complete

[Oded Gabbay]

Current dependencies between amdkfd and radeon/amdgpu force the loading
of amdkfd _before_ radeon and/or amdgpu are loaded. When all these kernel
drivers are built as modules, this ordering is enforced by the kernel
built-in mechanism of loading dependent modules.

However, there is no such mechanism in case where all these drivers are
compiled inside the kernel image (not as modules). The current way to
enforce loading of amdkfd before radeon/amdgpu, is to put amdkfd before
radeon/amdgpu in the drm Makefile, but that method is way too fragile.

In addition, there is no kernel mechanism to check whether a kernel
driver that is built inside the kernel image, has already been loaded.

To solve this, this patch adds to kfd_module.c a new static variable,
amdkfd_init_completed, that is set to 1 only when amdkfd's
module initialization function has been completed (successfully).

kgd2kfd_init(), which is the initialization function of the
kgd-->kfd interface, and which is the first function in amdkfd called by
radeon/amdgpu, will return successfully only if amdkfd_init_completed is
equal 1.

If amdkfd_init_completed is not equal to 1, kgd2kfd_init() will
return -EPROBE_DEFER to signal radeon/amdgpu they need to defer
their loading until amdkfd is loaded.

Signed-off-by: Oded Gabbay <oded.gabbay@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c      |  4 ++--
 drivers/gpu/drm/amd/amdkfd/kfd_module.c         | 15 ++++++++++++---
 drivers/gpu/drm/amd/include/kgd_kfd_interface.h |  2 +-
 drivers/gpu/drm/radeon/radeon_kfd.c             |  4 ++--
 4 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 84d68d6..44ba8be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -70,7 +70,7 @@ bool amdgpu_amdkfd_load_interface(struct amdgpu_device *rdev)
 		return false;
 	}
 
-	if (!kgd2kfd_init_p(KFD_INTERFACE_VERSION, &kgd2kfd)) {
+	if (kgd2kfd_init_p(KFD_INTERFACE_VERSION, &kgd2kfd)) {
 		symbol_put(kgd2kfd_init);
 		kfd2kgd = NULL;
 		kgd2kfd = NULL;
@@ -80,7 +80,7 @@ bool amdgpu_amdkfd_load_interface(struct amdgpu_device *rdev)
 
 	return true;
 #elif defined(CONFIG_HSA_AMD)
-	if (!kgd2kfd_init(KFD_INTERFACE_VERSION, &kgd2kfd)) {
+	if (kgd2kfd_init(KFD_INTERFACE_VERSION, &kgd2kfd)) {
 		kfd2kgd = NULL;
 		kgd2kfd = NULL;
 		return false;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_module.c b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
index ca8410e..850a562 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_module.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
@@ -59,18 +59,23 @@ module_param(send_sigterm, int, 0444);
 MODULE_PARM_DESC(send_sigterm,
 	"Send sigterm to HSA process on unhandled exception (0 = disable, 1 = enable)");
 
-bool kgd2kfd_init(unsigned interface_version, const struct kgd2kfd_calls **g2f)
+static int amdkfd_init_completed;
+
+int kgd2kfd_init(unsigned interface_version, const struct kgd2kfd_calls **g2f)
 {
+	if (!amdkfd_init_completed)
+		return -EPROBE_DEFER;
+
 	/*
 	 * Only one interface version is supported,
 	 * no kfd/kgd version skew allowed.
 	 */
 	if (interface_version != KFD_INTERFACE_VERSION)
-		return false;
+		return -EINVAL;
 
 	*g2f = &kgd2kfd;
 
-	return true;
+	return 0;
 }
 EXPORT_SYMBOL(kgd2kfd_init);
 
@@ -111,6 +116,8 @@ static int __init kfd_module_init(void)
 
 	kfd_process_create_wq();
 
+	amdkfd_init_completed = 1;
+
 	dev_info(kfd_device, "Initialized module\n");
 
 	return 0;
@@ -125,6 +132,8 @@ err_pasid:
 
 static void __exit kfd_module_exit(void)
 {
+	amdkfd_init_completed = 0;
+
 	kfd_process_destroy_wq();
 	kfd_topology_shutdown();
 	kfd_chardev_exit();
diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
index 888250b..a09d9f3 100644
--- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
@@ -221,7 +221,7 @@ struct kgd2kfd_calls {
 	int (*resume)(struct kfd_dev *kfd);
 };
 
-bool kgd2kfd_init(unsigned interface_version,
+int kgd2kfd_init(unsigned interface_version,
 		const struct kgd2kfd_calls **g2f);
 
 #endif	/* KGD_KFD_INTERFACE_H_INCLUDED */
diff --git a/drivers/gpu/drm/radeon/radeon_kfd.c b/drivers/gpu/drm/radeon/radeon_kfd.c
index 9a4d69e..83c2704 100644
--- a/drivers/gpu/drm/radeon/radeon_kfd.c
+++ b/drivers/gpu/drm/radeon/radeon_kfd.c
@@ -142,7 +142,7 @@ bool radeon_kfd_init(void)
 	if (kgd2kfd_init_p == NULL)
 		return false;
 
-	if (!kgd2kfd_init_p(KFD_INTERFACE_VERSION, &kgd2kfd)) {
+	if (kgd2kfd_init_p(KFD_INTERFACE_VERSION, &kgd2kfd)) {
 		symbol_put(kgd2kfd_init);
 		kgd2kfd = NULL;
 
@@ -151,7 +151,7 @@ bool radeon_kfd_init(void)
 
 	return true;
 #elif defined(CONFIG_HSA_AMD)
-	if (!kgd2kfd_init(KFD_INTERFACE_VERSION, &kgd2kfd)) {
+	if (kgd2kfd_init(KFD_INTERFACE_VERSION, &kgd2kfd)) {
 		kgd2kfd = NULL;
 
 		return false;
-- 
2.5.0

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

[PATCH 2/3] drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded

[Oded Gabbay]

radeon must load only after amdkfd's loading has been completed. If that
is not enforced, then radeon's call into amdkfd's functions will cause a
kernel BUG.

When radeon and amdkfd are built as kernel modules, that rule is
enforced by the kernel's modules loading mechanism. When radeon and
amdkfd are built inside the kernel image, that rule is enforced by
ordering in the drm Makefile (amdkfd before radeon).

Instead of using drm Makefile ordering, we can now use deferred
loading as amdkfd now returns -EPROBE_DEFER in kgd2kfd_init() when it is
not yet loaded.

This patch defers radeon loading by propagating -EPROBE_DEFER to the
kernel's drivers loading infrastructure. That will put radeon into the
pending drivers list (see description in dd.c). Once amdkfd is loaded,
a call to kgd2kfd_init() will return successfully and radeon will be
able to load.

Signed-off-by: Oded Gabbay <oded.gabbay@gmail.com>
---
 drivers/gpu/drm/radeon/radeon_drv.c | 10 ++++++++--
 drivers/gpu/drm/radeon/radeon_kfd.c | 25 ++++++++++++-------------
 drivers/gpu/drm/radeon/radeon_kfd.h |  2 +-
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index e266ffc..4147350 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -319,6 +319,14 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 {
 	int ret;
 
+	/*
+	 * Initialize amdkfd before starting radeon. If it was not loaded yet,
+	 * defer radeon probing
+	 */
+	ret = radeon_kfd_init();
+	if (ret == -EPROBE_DEFER)
+		return ret;
+
 	/* Get rid of things like offb */
 	ret = radeon_kick_out_firmware_fb(pdev);
 	if (ret)
@@ -570,8 +578,6 @@ static int __init radeon_init(void)
 		return -EINVAL;
 	}
 
-	radeon_kfd_init();
-
 	/* let modprobe override vga console setting */
 	return drm_pci_init(driver, pdriver);
 }
diff --git a/drivers/gpu/drm/radeon/radeon_kfd.c b/drivers/gpu/drm/radeon/radeon_kfd.c
index 83c2704..87a9ebb 100644
--- a/drivers/gpu/drm/radeon/radeon_kfd.c
+++ b/drivers/gpu/drm/radeon/radeon_kfd.c
@@ -132,35 +132,34 @@ static const struct kfd2kgd_calls kfd2kgd = {
 
 static const struct kgd2kfd_calls *kgd2kfd;
 
-bool radeon_kfd_init(void)
+int radeon_kfd_init(void)
 {
+	int ret;
+
 #if defined(CONFIG_HSA_AMD_MODULE)
-	bool (*kgd2kfd_init_p)(unsigned, const struct kgd2kfd_calls**);
+	int (*kgd2kfd_init_p)(unsigned, const struct kgd2kfd_calls**);
 
 	kgd2kfd_init_p = symbol_request(kgd2kfd_init);
 
 	if (kgd2kfd_init_p == NULL)
-		return false;
+		return -ENOENT;
 
-	if (kgd2kfd_init_p(KFD_INTERFACE_VERSION, &kgd2kfd)) {
+	ret = kgd2kfd_init_p(KFD_INTERFACE_VERSION, &kgd2kfd);
+	if (ret) {
 		symbol_put(kgd2kfd_init);
 		kgd2kfd = NULL;
-
-		return false;
 	}
 
-	return true;
 #elif defined(CONFIG_HSA_AMD)
-	if (kgd2kfd_init(KFD_INTERFACE_VERSION, &kgd2kfd)) {
+	ret = kgd2kfd_init(KFD_INTERFACE_VERSION, &kgd2kfd);
+	if (ret)
 		kgd2kfd = NULL;
 
-		return false;
-	}
-
-	return true;
 #else
-	return false;
+	ret = -ENOENT;
 #endif
+
+	return ret;
 }
 
 void radeon_kfd_fini(void)
diff --git a/drivers/gpu/drm/radeon/radeon_kfd.h b/drivers/gpu/drm/radeon/radeon_kfd.h
index 1103f90..9df1fea 100644
--- a/drivers/gpu/drm/radeon/radeon_kfd.h
+++ b/drivers/gpu/drm/radeon/radeon_kfd.h
@@ -33,7 +33,7 @@
 
 struct radeon_device;
 
-bool radeon_kfd_init(void);
+int radeon_kfd_init(void);
 void radeon_kfd_fini(void);
 
 void radeon_kfd_suspend(struct radeon_device *rdev);
-- 
2.5.0

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

[PATCH 3/3] drm/amdgpu: Return -EPROBE_DEFER when amdkfd not loaded

[Oded Gabbay]

amdgpu must load only after amdkfd's loading has been completed. If that
is not enforced, then amdgpu's call into amdkfd's functions will cause a
kernel BUG.

When amdgpu and amdkfd are built as kernel modules, that rule is enforced
by the kernel's modules loading mechanism. When amdgpu and amdkfd are
built inside the kernel image, that rule is enforced by ordering in the
drm Makefile (amdkfd before amdgpu).

Instead of using drm Makefile ordering, we can now use deferred loading
as amdkfd now returns -EPROBE_DEFER in kgd2kfd_init() when it is not yet
loaded.

This patch defers amdgpu loading by propagating -EPROBE_DEFER to the
kernel's drivers loading infrastructure. That will put amdgpu into the
pending drivers list (see description in dd.c). Once amdkfd is loaded,
a call to kgd2kfd_init() will return successfully and amdgpu will be able
to load.

Signed-off-by: Oded Gabbay <oded.gabbay@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 57 +++++++++++-------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 10 ++++--
 3 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 44ba8be..32809f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -30,25 +30,38 @@ const struct kfd2kgd_calls *kfd2kgd;
 const struct kgd2kfd_calls *kgd2kfd;
 bool (*kgd2kfd_init_p)(unsigned, const struct kgd2kfd_calls**);
 
-bool amdgpu_amdkfd_init(void)
+int amdgpu_amdkfd_init(void)
 {
+	int ret;
+
 #if defined(CONFIG_HSA_AMD_MODULE)
-	bool (*kgd2kfd_init_p)(unsigned, const struct kgd2kfd_calls**);
+	int (*kgd2kfd_init_p)(unsigned, const struct kgd2kfd_calls**);
 
 	kgd2kfd_init_p = symbol_request(kgd2kfd_init);
 
 	if (kgd2kfd_init_p == NULL)
-		return false;
+		return -ENOENT;
+
+	ret = kgd2kfd_init_p(KFD_INTERFACE_VERSION, &kgd2kfd);
+	if (ret) {
+		symbol_put(kgd2kfd_init);
+		kgd2kfd = NULL;
+	}
+
+#elif defined(CONFIG_HSA_AMD)
+	ret = kgd2kfd_init(KFD_INTERFACE_VERSION, &kgd2kfd);
+	if (ret)
+		kgd2kfd = NULL;
+
+#else
+	ret = -ENOENT;
 #endif
-	return true;
+
+	return ret;
 }
 
 bool amdgpu_amdkfd_load_interface(struct amdgpu_device *rdev)
 {
-#if defined(CONFIG_HSA_AMD_MODULE)
-	bool (*kgd2kfd_init_p)(unsigned, const struct kgd2kfd_calls**);
-#endif
-
 	switch (rdev->asic_type) {
 #ifdef CONFIG_DRM_AMDGPU_CIK
 	case CHIP_KAVERI:
@@ -62,35 +75,7 @@ bool amdgpu_amdkfd_load_interface(struct amdgpu_device *rdev)
 		return false;
 	}
 
-#if defined(CONFIG_HSA_AMD_MODULE)
-	kgd2kfd_init_p = symbol_request(kgd2kfd_init);
-
-	if (kgd2kfd_init_p == NULL) {
-		kfd2kgd = NULL;
-		return false;
-	}
-
-	if (kgd2kfd_init_p(KFD_INTERFACE_VERSION, &kgd2kfd)) {
-		symbol_put(kgd2kfd_init);
-		kfd2kgd = NULL;
-		kgd2kfd = NULL;
-
-		return false;
-	}
-
 	return true;
-#elif defined(CONFIG_HSA_AMD)
-	if (kgd2kfd_init(KFD_INTERFACE_VERSION, &kgd2kfd)) {
-		kfd2kgd = NULL;
-		kgd2kfd = NULL;
-		return false;
-	}
-
-	return true;
-#else
-	kfd2kgd = NULL;
-	return false;
-#endif
 }
 
 void amdgpu_amdkfd_fini(void)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index a8be765..de530f68d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -36,7 +36,7 @@ struct kgd_mem {
 	void *cpu_ptr;
 };
 
-bool amdgpu_amdkfd_init(void);
+int amdgpu_amdkfd_init(void);
 void amdgpu_amdkfd_fini(void);
 
 bool amdgpu_amdkfd_load_interface(struct amdgpu_device *rdev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 9c1af89..178519a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -322,6 +322,14 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
 		return -ENODEV;
 	}
 
+	/*
+	 * Initialize amdkfd before starting radeon. If it was not loaded yet,
+	 * defer radeon probing
+	 */
+	ret = amdgpu_amdkfd_init();
+	if (ret == -EPROBE_DEFER)
+		return ret;
+
 	/* Get rid of things like offb */
 	ret = amdgpu_kick_out_firmware_fb(pdev);
 	if (ret)
@@ -564,8 +572,6 @@ static int __init amdgpu_init(void)
 	driver->num_ioctls = amdgpu_max_kms_ioctl;
 	amdgpu_register_atpx_handler();
 
-	amdgpu_amdkfd_init();
-
 	/* let modprobe override vga console setting */
 	return drm_pci_init(driver, pdriver);
 }
-- 
2.5.0

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

Re: [PATCH 0/3] Deferr load of radeon/amdgpu until amdkfd is loaded

[Daniel Vetter]

On Sun, Feb 14, 2016 at 11:16:52AM +0200, Oded Gabbay wrote:
> Following Daniel's request, I spent some time removing the hard requirement 
> that radeon and amdgpu will always appear _after_ amdkfd in the drm Makefile.
> 
> This was done by modifing radeon/amdgpu to defer their loading if they detect 
> that amdkfd is not loaded yet, in case the drivers are built inside the 
> kernel image. 
> 
> See the patch's individiual commit messages for more explanation.
> 
> This patch-set was tested on a KAVERI machine, with multiple configurations:
> 
> 1. radeon + amdgpu (CIK disabled) + amdkfd as kernel modules
> 2. radeon + amdgpu (CIK disabled) + amdkfd inside the kernel image
> 3. amdgpu (CIK enabled) + amdkfd inside the kernel image (radeon not compiled)
> 4. amdgpu (CIK enabled) inside the kernel image (radeon + amdkfd not compiled)
> 5. radeon + amdgpu (CIK disabled) as kernel modules (amdkfd not compiled)

Care to throw one patch on top (maybe on top of the patch floating around)
to reorder amdkfd to be alphabetical? Just to make sure this doesn't get
broken again accidentally. Or maybe just pick up the other patch and adapt
it so it's all in one series.

Thanks, Daniel
> 
> Thanks,
> 
>     Oded
> 
> Oded Gabbay (3):
>   drm/amdkfd: Track when module's init is complete
>   drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded
>   drm/amdgpu: Return -EPROBE_DEFER when amdkfd not loaded
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c      | 57 +++++++++----------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h      |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c         | 10 ++++-
>  drivers/gpu/drm/amd/amdkfd/kfd_module.c         | 15 +++++--
>  drivers/gpu/drm/amd/include/kgd_kfd_interface.h |  2 +-
>  drivers/gpu/drm/radeon/radeon_drv.c             | 10 ++++-
>  drivers/gpu/drm/radeon/radeon_kfd.c             | 25 ++++++-----
>  drivers/gpu/drm/radeon/radeon_kfd.h             |  2 +-
>  8 files changed, 64 insertions(+), 59 deletions(-)
> 
> -- 
> 2.5.0
> 

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

Re: [PATCH 0/3] Deferr load of radeon/amdgpu until amdkfd is loaded

[Oded Gabbay]

On Sun, Feb 14, 2016 at 2:58 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Sun, Feb 14, 2016 at 11:16:52AM +0200, Oded Gabbay wrote:
>> Following Daniel's request, I spent some time removing the hard requirement
>> that radeon and amdgpu will always appear _after_ amdkfd in the drm Makefile.
>>
>> This was done by modifing radeon/amdgpu to defer their loading if they detect
>> that amdkfd is not loaded yet, in case the drivers are built inside the
>> kernel image.
>>
>> See the patch's individiual commit messages for more explanation.
>>
>> This patch-set was tested on a KAVERI machine, with multiple configurations:
>>
>> 1. radeon + amdgpu (CIK disabled) + amdkfd as kernel modules
>> 2. radeon + amdgpu (CIK disabled) + amdkfd inside the kernel image
>> 3. amdgpu (CIK enabled) + amdkfd inside the kernel image (radeon not compiled)
>> 4. amdgpu (CIK enabled) inside the kernel image (radeon + amdkfd not compiled)
>> 5. radeon + amdgpu (CIK disabled) as kernel modules (amdkfd not compiled)
>
> Care to throw one patch on top (maybe on top of the patch floating around)
> to reorder amdkfd to be alphabetical? Just to make sure this doesn't get
> broken again accidentally. Or maybe just pick up the other patch and adapt
> it so it's all in one series.
>
> Thanks, Daniel

Hi Daniel,

I thought about it and I think I prefer to leave the current order as
it is, for the reason that I observed the boot-up process is a little
bit faster when the deferred probing doesn't occur. This is probably
because all the moves between pending drivers list and active driver
list.

Although this patch-set ensure that the kernel will boot successfully
with no regard to the order of amdkfd/radeon/amdgpu in the drm
makefile, I think that if the current order gives us a bit less boot
time then it is better to keep things as they are.

Thanks,

      Oded

>>
>> Thanks,
>>
>>     Oded
>>
>> Oded Gabbay (3):
>>   drm/amdkfd: Track when module's init is complete
>>   drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded
>>   drm/amdgpu: Return -EPROBE_DEFER when amdkfd not loaded
>>
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c      | 57 +++++++++----------------
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h      |  2 +-
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c         | 10 ++++-
>>  drivers/gpu/drm/amd/amdkfd/kfd_module.c         | 15 +++++--
>>  drivers/gpu/drm/amd/include/kgd_kfd_interface.h |  2 +-
>>  drivers/gpu/drm/radeon/radeon_drv.c             | 10 ++++-
>>  drivers/gpu/drm/radeon/radeon_kfd.c             | 25 ++++++-----
>>  drivers/gpu/drm/radeon/radeon_kfd.h             |  2 +-
>>  8 files changed, 64 insertions(+), 59 deletions(-)
>>
>> --
>> 2.5.0
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] Deferr load of radeon/amdgpu until amdkfd is loaded

[Alex Deucher]

On Sun, Feb 14, 2016 at 4:16 AM, Oded Gabbay <oded.gabbay@gmail.com> wrote:
> Following Daniel's request, I spent some time removing the hard requirement
> that radeon and amdgpu will always appear _after_ amdkfd in the drm Makefile.
>
> This was done by modifing radeon/amdgpu to defer their loading if they detect
> that amdkfd is not loaded yet, in case the drivers are built inside the
> kernel image.
>
> See the patch's individiual commit messages for more explanation.
>
> This patch-set was tested on a KAVERI machine, with multiple configurations:
>
> 1. radeon + amdgpu (CIK disabled) + amdkfd as kernel modules
> 2. radeon + amdgpu (CIK disabled) + amdkfd inside the kernel image
> 3. amdgpu (CIK enabled) + amdkfd inside the kernel image (radeon not compiled)
> 4. amdgpu (CIK enabled) inside the kernel image (radeon + amdkfd not compiled)
> 5. radeon + amdgpu (CIK disabled) as kernel modules (amdkfd not compiled)

For the series:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>


>
> Thanks,
>
>     Oded
>
> Oded Gabbay (3):
>   drm/amdkfd: Track when module's init is complete
>   drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded
>   drm/amdgpu: Return -EPROBE_DEFER when amdkfd not loaded
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c      | 57 +++++++++----------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h      |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c         | 10 ++++-
>  drivers/gpu/drm/amd/amdkfd/kfd_module.c         | 15 +++++--
>  drivers/gpu/drm/amd/include/kgd_kfd_interface.h |  2 +-
>  drivers/gpu/drm/radeon/radeon_drv.c             | 10 ++++-
>  drivers/gpu/drm/radeon/radeon_kfd.c             | 25 ++++++-----
>  drivers/gpu/drm/radeon/radeon_kfd.h             |  2 +-
>  8 files changed, 64 insertions(+), 59 deletions(-)
>
> --
> 2.5.0
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] Deferr load of radeon/amdgpu until amdkfd is loaded

[Xinliang Liu]

On 15 February 2016 at 19:04, Oded Gabbay <oded.gabbay@gmail.com> wrote:
> On Sun, Feb 14, 2016 at 2:58 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Sun, Feb 14, 2016 at 11:16:52AM +0200, Oded Gabbay wrote:
>>> Following Daniel's request, I spent some time removing the hard requirement
>>> that radeon and amdgpu will always appear _after_ amdkfd in the drm Makefile.
>>>
>>> This was done by modifing radeon/amdgpu to defer their loading if they detect
>>> that amdkfd is not loaded yet, in case the drivers are built inside the
>>> kernel image.
>>>
>>> See the patch's individiual commit messages for more explanation.
>>>
>>> This patch-set was tested on a KAVERI machine, with multiple configurations:
>>>
>>> 1. radeon + amdgpu (CIK disabled) + amdkfd as kernel modules
>>> 2. radeon + amdgpu (CIK disabled) + amdkfd inside the kernel image
>>> 3. amdgpu (CIK enabled) + amdkfd inside the kernel image (radeon not compiled)
>>> 4. amdgpu (CIK enabled) inside the kernel image (radeon + amdkfd not compiled)
>>> 5. radeon + amdgpu (CIK disabled) as kernel modules (amdkfd not compiled)
>>
>> Care to throw one patch on top (maybe on top of the patch floating around)
>> to reorder amdkfd to be alphabetical? Just to make sure this doesn't get
>> broken again accidentally. Or maybe just pick up the other patch and adapt
>> it so it's all in one series.
>>
>> Thanks, Daniel
>
> Hi Daniel,
>
> I thought about it and I think I prefer to leave the current order as
> it is, for the reason that I observed the boot-up process is a little
> bit faster when the deferred probing doesn't occur. This is probably
> because all the moves between pending drivers list and active driver
> list.
>
> Although this patch-set ensure that the kernel will boot successfully
> with no regard to the order of amdkfd/radeon/amdgpu in the drm
> makefile, I think that if the current order gives us a bit less boot
> time then it is better to keep things as they are.
>
> Thanks,
>
>       Oded
>
>>>
>>> Thanks,
>>>
>>>     Oded
>>>
>>> Oded Gabbay (3):
>>>   drm/amdkfd: Track when module's init is complete
>>>   drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded
>>>   drm/amdgpu: Return -EPROBE_DEFER when amdkfd not loaded
>>>
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c      | 57 +++++++++----------------
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h      |  2 +-
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c         | 10 ++++-
>>>  drivers/gpu/drm/amd/amdkfd/kfd_module.c         | 15 +++++--
>>>  drivers/gpu/drm/amd/include/kgd_kfd_interface.h |  2 +-
>>>  drivers/gpu/drm/radeon/radeon_drv.c             | 10 ++++-
>>>  drivers/gpu/drm/radeon/radeon_kfd.c             | 25 ++++++-----
>>>  drivers/gpu/drm/radeon/radeon_kfd.h             |  2 +-
>>>  8 files changed, 64 insertions(+), 59 deletions(-)
>>>
>>> --
>>> 2.5.0
>>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] Deferr load of radeon/amdgpu until amdkfd is loaded

[Xinliang Liu]

On 15 February 2016 at 19:04, Oded Gabbay <oded.gabbay@gmail.com> wrote:
> On Sun, Feb 14, 2016 at 2:58 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Sun, Feb 14, 2016 at 11:16:52AM +0200, Oded Gabbay wrote:
>>> Following Daniel's request, I spent some time removing the hard requirement
>>> that radeon and amdgpu will always appear _after_ amdkfd in the drm Makefile.
>>>
>>> This was done by modifing radeon/amdgpu to defer their loading if they detect
>>> that amdkfd is not loaded yet, in case the drivers are built inside the
>>> kernel image.
>>>
>>> See the patch's individiual commit messages for more explanation.
>>>
>>> This patch-set was tested on a KAVERI machine, with multiple configurations:
>>>
>>> 1. radeon + amdgpu (CIK disabled) + amdkfd as kernel modules
>>> 2. radeon + amdgpu (CIK disabled) + amdkfd inside the kernel image
>>> 3. amdgpu (CIK enabled) + amdkfd inside the kernel image (radeon not compiled)
>>> 4. amdgpu (CIK enabled) inside the kernel image (radeon + amdkfd not compiled)
>>> 5. radeon + amdgpu (CIK disabled) as kernel modules (amdkfd not compiled)
>>
>> Care to throw one patch on top (maybe on top of the patch floating around)
>> to reorder amdkfd to be alphabetical? Just to make sure this doesn't get
>> broken again accidentally. Or maybe just pick up the other patch and adapt
>> it so it's all in one series.
>>
>> Thanks, Daniel
>
> Hi Daniel,
>
> I thought about it and I think I prefer to leave the current order as
> it is, for the reason that I observed the boot-up process is a little
> bit faster when the deferred probing doesn't occur. This is probably
> because all the moves between pending drivers list and active driver
> list.
>
> Although this patch-set ensure that the kernel will boot successfully
> with no regard to the order of amdkfd/radeon/amdgpu in the drm

So, my drm make clean up patch should keep amdkfd in front of radeon/amdgpu?

Best,
-xinliang

> makefile, I think that if the current order gives us a bit less boot
> time then it is better to keep things as they are.
>
> Thanks,
>
>       Oded
>
>>>
>>> Thanks,
>>>
>>>     Oded
>>>
>>> Oded Gabbay (3):
>>>   drm/amdkfd: Track when module's init is complete
>>>   drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded
>>>   drm/amdgpu: Return -EPROBE_DEFER when amdkfd not loaded
>>>
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c      | 57 +++++++++----------------
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h      |  2 +-
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c         | 10 ++++-
>>>  drivers/gpu/drm/amd/amdkfd/kfd_module.c         | 15 +++++--
>>>  drivers/gpu/drm/amd/include/kgd_kfd_interface.h |  2 +-
>>>  drivers/gpu/drm/radeon/radeon_drv.c             | 10 ++++-
>>>  drivers/gpu/drm/radeon/radeon_kfd.c             | 25 ++++++-----
>>>  drivers/gpu/drm/radeon/radeon_kfd.h             |  2 +-
>>>  8 files changed, 64 insertions(+), 59 deletions(-)
>>>
>>> --
>>> 2.5.0
>>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] Deferr load of radeon/amdgpu until amdkfd is loaded

[Oded Gabbay]

On Tue, Feb 23, 2016 at 5:10 AM, Xinliang Liu <xinliang.liu@linaro.org> wrote:
> On 15 February 2016 at 19:04, Oded Gabbay <oded.gabbay@gmail.com> wrote:
>> On Sun, Feb 14, 2016 at 2:58 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>>> On Sun, Feb 14, 2016 at 11:16:52AM +0200, Oded Gabbay wrote:
>>>> Following Daniel's request, I spent some time removing the hard requirement
>>>> that radeon and amdgpu will always appear _after_ amdkfd in the drm Makefile.
>>>>
>>>> This was done by modifing radeon/amdgpu to defer their loading if they detect
>>>> that amdkfd is not loaded yet, in case the drivers are built inside the
>>>> kernel image.
>>>>
>>>> See the patch's individiual commit messages for more explanation.
>>>>
>>>> This patch-set was tested on a KAVERI machine, with multiple configurations:
>>>>
>>>> 1. radeon + amdgpu (CIK disabled) + amdkfd as kernel modules
>>>> 2. radeon + amdgpu (CIK disabled) + amdkfd inside the kernel image
>>>> 3. amdgpu (CIK enabled) + amdkfd inside the kernel image (radeon not compiled)
>>>> 4. amdgpu (CIK enabled) inside the kernel image (radeon + amdkfd not compiled)
>>>> 5. radeon + amdgpu (CIK disabled) as kernel modules (amdkfd not compiled)
>>>
>>> Care to throw one patch on top (maybe on top of the patch floating around)
>>> to reorder amdkfd to be alphabetical? Just to make sure this doesn't get
>>> broken again accidentally. Or maybe just pick up the other patch and adapt
>>> it so it's all in one series.
>>>
>>> Thanks, Daniel
>>
>> Hi Daniel,
>>
>> I thought about it and I think I prefer to leave the current order as
>> it is, for the reason that I observed the boot-up process is a little
>> bit faster when the deferred probing doesn't occur. This is probably
>> because all the moves between pending drivers list and active driver
>> list.
>>
>> Although this patch-set ensure that the kernel will boot successfully
>> with no regard to the order of amdkfd/radeon/amdgpu in the drm
>
> So, my drm make clean up patch should keep amdkfd in front of radeon/amdgpu?
>
> Best,
> -xinliang

As I wrote to Daniel, I think that for the sake of a faster boot time,
we should keep amdkfd before radeon/amdgpu. This patch is to make sure
that if someone will change it without us watching, everything will
still work (and that's why its an important patch as Daniel said)

Oded

>
>> makefile, I think that if the current order gives us a bit less boot
>> time then it is better to keep things as they are.
>>
>> Thanks,
>>
>>       Oded
>>
>>>>
>>>> Thanks,
>>>>
>>>>     Oded
>>>>
>>>> Oded Gabbay (3):
>>>>   drm/amdkfd: Track when module's init is complete
>>>>   drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded
>>>>   drm/amdgpu: Return -EPROBE_DEFER when amdkfd not loaded
>>>>
>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c      | 57 +++++++++----------------
>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h      |  2 +-
>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c         | 10 ++++-
>>>>  drivers/gpu/drm/amd/amdkfd/kfd_module.c         | 15 +++++--
>>>>  drivers/gpu/drm/amd/include/kgd_kfd_interface.h |  2 +-
>>>>  drivers/gpu/drm/radeon/radeon_drv.c             | 10 ++++-
>>>>  drivers/gpu/drm/radeon/radeon_kfd.c             | 25 ++++++-----
>>>>  drivers/gpu/drm/radeon/radeon_kfd.h             |  2 +-
>>>>  8 files changed, 64 insertions(+), 59 deletions(-)
>>>>
>>>> --
>>>> 2.5.0
>>>>
>>>
>>> --
>>> Daniel Vetter
>>> Software Engineer, Intel Corporation
>>> http://blog.ffwll.ch
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] Deferr load of radeon/amdgpu until amdkfd is loaded

[Xinliang Liu]

On 23 February 2016 at 14:51, Oded Gabbay <oded.gabbay@gmail.com> wrote:
> On Tue, Feb 23, 2016 at 5:10 AM, Xinliang Liu <xinliang.liu@linaro.org> wrote:
>> On 15 February 2016 at 19:04, Oded Gabbay <oded.gabbay@gmail.com> wrote:
>>> On Sun, Feb 14, 2016 at 2:58 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>>>> On Sun, Feb 14, 2016 at 11:16:52AM +0200, Oded Gabbay wrote:
>>>>> Following Daniel's request, I spent some time removing the hard requirement
>>>>> that radeon and amdgpu will always appear _after_ amdkfd in the drm Makefile.
>>>>>
>>>>> This was done by modifing radeon/amdgpu to defer their loading if they detect
>>>>> that amdkfd is not loaded yet, in case the drivers are built inside the
>>>>> kernel image.
>>>>>
>>>>> See the patch's individiual commit messages for more explanation.
>>>>>
>>>>> This patch-set was tested on a KAVERI machine, with multiple configurations:
>>>>>
>>>>> 1. radeon + amdgpu (CIK disabled) + amdkfd as kernel modules
>>>>> 2. radeon + amdgpu (CIK disabled) + amdkfd inside the kernel image
>>>>> 3. amdgpu (CIK enabled) + amdkfd inside the kernel image (radeon not compiled)
>>>>> 4. amdgpu (CIK enabled) inside the kernel image (radeon + amdkfd not compiled)
>>>>> 5. radeon + amdgpu (CIK disabled) as kernel modules (amdkfd not compiled)
>>>>
>>>> Care to throw one patch on top (maybe on top of the patch floating around)
>>>> to reorder amdkfd to be alphabetical? Just to make sure this doesn't get
>>>> broken again accidentally. Or maybe just pick up the other patch and adapt
>>>> it so it's all in one series.
>>>>
>>>> Thanks, Daniel
>>>
>>> Hi Daniel,
>>>
>>> I thought about it and I think I prefer to leave the current order as
>>> it is, for the reason that I observed the boot-up process is a little
>>> bit faster when the deferred probing doesn't occur. This is probably
>>> because all the moves between pending drivers list and active driver
>>> list.
>>>
>>> Although this patch-set ensure that the kernel will boot successfully
>>> with no regard to the order of amdkfd/radeon/amdgpu in the drm
>>
>> So, my drm make clean up patch should keep amdkfd in front of radeon/amdgpu?
>>
>> Best,
>> -xinliang
>
> As I wrote to Daniel, I think that for the sake of a faster boot time,
> we should keep amdkfd before radeon/amdgpu. This patch is to make sure
> that if someone will change it without us watching, everything will
> still work (and that's why its an important patch as Daniel said)
>

OK, got it.

> Oded
>
>>
>>> makefile, I think that if the current order gives us a bit less boot
>>> time then it is better to keep things as they are.
>>>
>>> Thanks,
>>>
>>>       Oded
>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>>     Oded
>>>>>
>>>>> Oded Gabbay (3):
>>>>>   drm/amdkfd: Track when module's init is complete
>>>>>   drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded
>>>>>   drm/amdgpu: Return -EPROBE_DEFER when amdkfd not loaded
>>>>>
>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c      | 57 +++++++++----------------
>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h      |  2 +-
>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c         | 10 ++++-
>>>>>  drivers/gpu/drm/amd/amdkfd/kfd_module.c         | 15 +++++--
>>>>>  drivers/gpu/drm/amd/include/kgd_kfd_interface.h |  2 +-
>>>>>  drivers/gpu/drm/radeon/radeon_drv.c             | 10 ++++-
>>>>>  drivers/gpu/drm/radeon/radeon_kfd.c             | 25 ++++++-----
>>>>>  drivers/gpu/drm/radeon/radeon_kfd.h             |  2 +-
>>>>>  8 files changed, 64 insertions(+), 59 deletions(-)
>>>>>
>>>>> --
>>>>> 2.5.0
>>>>>
>>>>
>>>> --
>>>> Daniel Vetter
>>>> Software Engineer, Intel Corporation
>>>> http://blog.ffwll.ch
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel