On Fri, May 27, 2016 at 4:18 AM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote:
> To get KFD support in radeon we need the following
> initialization to happen in this order, their
> respective driver file that has its init routine
> listed next to it:
>
> 0. AMD IOMMUv1: arch/x86/kernel/pci-dma.c
> 1. AMD IOMMUv2: drivers/iommu/amd_iommu_v2.c
> 2. AMD KFD: drivers/gpu/drm/amd/amdkfd/kfd_module.c
> 3. AMD Radeon: drivers/gpu/drm/radeon/radeon_drv.c
Also AMD amdgpu (for VI+ APUs)
>
> Order is rather implicit, but these drivers can currently
> only do so much given the amount of leg room available.
> Below are the respective init routines and how they are
> initialized:
>
> arch/x86/kernel/pci-dma.c rootfs_initcall(pci_iommu_init);
> drivers/iommu/amd_iommu_v2.c module_init(amd_iommu_v2_init);
> drivers/gpu/drm/amd/amdkfd/kfd_module.c module_init(kfd_module_init);
> drivers/gpu/drm/radeon/radeon_drv.c module_init(radeon_init);
>
> When a driver is built-in module_init() folds to use
> device_initcall(), and we have the following possible
> orders:
>
> #define pure_initcall(fn) __define_initcall(fn, 0)
> #define core_initcall(fn) __define_initcall(fn, 1)
> #define postcore_initcall(fn)__define_initcall(fn, 2)
> #define arch_initcall(fn) __define_initcall(fn, 3)
> #define subsys_initcall(fn) __define_initcall(fn, 4)
> #define fs_initcall(fn) __define_initcall(fn, 5)
> ---------------------------------------------------------
> #define rootfs_initcall(fn) __define_initcall(fn, rootfs)
> #define device_initcall(fn) __define_initcall(fn, 6)
> #define late_initcall(fn) __define_initcall(fn, 7)
>
> Since we start off from rootfs_initcall(), it gives us 3 more
> levels of leg room to play with for order semantics, this isn't
> enough to address all required levels of dependencies, this
> is specially true given that AMD-KFD needs to be loaded before
> the radeon driver -- -but this it not enforced by symbols.
> If the AMD-KFD driver is not loaded prior to the radeon driver
> because otherwise the radeon driver will not initialize the
> AMD-KFD driver and you get no KFD functionality in userspace.
>
> Commit 1bacc894c227fad8a7 ("drivers: Move iommu/ before gpu/ in
> Makefile") works around some of the possibe races between
> the AMD IOMMU v2 and GPU drivers by changing the link order.
> This is fragile, however its the bets we can do, given that
> making the GPU drivers use late_initcall() would also implicate
> a similar race between them. That possible race is fortunatley
> addressed given that the drm Makefile currently has amdkfd
> linked prior to radeon:
>
> drivers/gpu/drm/Makefile
> ...
> obj-$(CONFIG_HSA_AMD) += amd/amdkfd/
> obj-$(CONFIG_DRM_RADEON)+= radeon/
> ...
>
> Changing amdkfd and radeon to late_initcall() however is
> still the right call in orde to annotate explicitly a
> delayed dependency requirement between the GPU drivers
> and the IOMMUs.
>
> We can't address the fragile nature of the link order
> right now, but in the future that might be possible.
>
> Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
> ---
>
> Please note, the changes to drivers/Makefile are just
> for the sake of forcing the possible race to occur,
> if this works well the actual [PATCH] submission will
> skip those changes as its pointless to remove those
> work arounds as it stands, due to the limited nature
> of the levels available for addressing requirements.
>
> Also, if you are aware of further dependency hell
> things like these -- please do let me know as I am
> interested in looking at addressing them.
>
> drivers/Makefile | 6 ++----
> drivers/gpu/drm/amd/amdkfd/kfd_module.c | 2 +-
> drivers/gpu/drm/radeon/radeon_drv.c | 2 +-
> 3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 0b6f3d60193d..0fbe3982041f 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -50,10 +50,7 @@ obj-$(CONFIG_RESET_CONTROLLER) += reset/
> obj-y += tty/
> obj-y += char/
>
> -# iommu/ comes before gpu as gpu are using iommu controllers
> -obj-$(CONFIG_IOMMU_SUPPORT) += iommu/
> -
> -# gpu/ comes after char for AGP vs DRM startup and after iommu
> +# gpu/ comes after char for AGP vs DRM startup
> obj-y += gpu/
>
> obj-$(CONFIG_CONNECTOR) += connector/
> @@ -147,6 +144,7 @@ obj-y += clk/
>
> obj-$(CONFIG_MAILBOX) += mailbox/
> obj-$(CONFIG_HWSPINLOCK) += hwspinlock/
> +obj-$(CONFIG_IOMMU_SUPPORT) += iommu/
> obj-$(CONFIG_REMOTEPROC) += remoteproc/
> obj-$(CONFIG_RPMSG) += rpmsg/
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_module.c b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
> index 850a5623661f..3d1dab8a31c7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_module.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
> @@ -141,7 +141,7 @@ static void __exit kfd_module_exit(void)
> dev_info(kfd_device, "Removed module\n");
> }
>
> -module_init(kfd_module_init);
> +late_initcall(kfd_module_init);
> module_exit(kfd_module_exit);
>
> MODULE_AUTHOR(KFD_DRIVER_AUTHOR);
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index b55aa740171f..1fa1b7f3a89c 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -609,7 +609,7 @@ static void __exit radeon_exit(void)
> radeon_unregister_atpx_handler();
> }
>
> -module_init(radeon_init);
> +late_initcall(radeon_init);
> module_exit(radeon_exit);
Need to modify also amdgpu module_init
>
> MODULE_AUTHOR(DRIVER_AUTHOR);
> --
> 2.8.2
>
I tested this on Kaveri, and amdkfd is working. For amdkfd that's
fine, but IMO that's not enough testing for radeon/amdgpu. I would
like to hear AMD's developers take on this.
Oded
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
