Skip to contentSkip to conversation

drm/amdgpu: conditionally compile amdgpu's amdkfd files

PATCH Superseded Oded Gabbay 2 participants 4 messages May 18, 2018 3 messages not in the archive

Subsystems (3)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 drm/amdgpu: conditionally compile amdgpu's amdkfd files

In case CONFIG_HSA_AMD is not chosen, there is no need to compile amdkfd files that reside inside amdgpu dirver. In addition, because amdkfd depends on x86_64 architecture and amdgpu is not, compiling amdkfd files under i386 architecture can cause compiler errors and warnings.

This patch modifies amdgpu's makefile to build amdkfd files only if CONFIG_HSA_AMD is chosen. The only file to be compiled unconditionally is amdgpu_amdkfd.c

Direct calls from amdgpu driver proper to functions in other amdgpu_amdkfd_*.c files were changed to calls to functions inside amdgpu_amdkfd.c. These functions call the original functions using a function pointer to allow compilation without the original functions.

v2:
Instead of using function pointers, use stub functions that are compiled only if amdkfd is not compiled.

---
 drivers/gpu/drm/amd/amdgpu/Makefile        | 13 +++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 46 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 12 ++++----
 3 files changed, 62 insertions(+), 9 deletions(-)
drivers/gpu/drm/amd/amdgpu/Makefile +10 -3
index 68e9f584c570..e03ee41cedcb 100644
@@ -56,8 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 
 # add asic specific block
 amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
-	ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o \
-	amdgpu_amdkfd_gfx_v7.o
+	ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
 
 amdgpu-$(CONFIG_DRM_AMDGPU_SI)+= si.o gmc_v6_0.o gfx_v6_0.o si_ih.o si_dma.o dce_v6_0.o si_dpm.o si_smc.o
 
@@ -130,13 +129,21 @@ amdgpu-y += \
 	vcn_v1_0.o
 
 # add amdkfd interfaces
+amdgpu-y += amdgpu_amdkfd.o
+
+ifneq ($(CONFIG_HSA_AMD),)
 amdgpu-y += \
-	 amdgpu_amdkfd.o \
 	 amdgpu_amdkfd_fence.o \
 	 amdgpu_amdkfd_gpuvm.o \
 	 amdgpu_amdkfd_gfx_v8.o \
 	 amdgpu_amdkfd_gfx_v9.o
 
+ifneq ($(CONFIG_DRM_AMDGPU_CIK),)
+amdgpu-y += amdgpu_amdkfd_gfx_v7.o
+endif
+
+endif
+
 # add cgs
 amdgpu-y += amdgpu_cgs.o
 
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +46 -0

63 lines not on this page · raw message

drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h +6 -6
index 12367a9951e8..a8418a3f4e9d 100644
@@ -156,14 +156,14 @@ uint64_t amdgpu_amdkfd_get_vram_usage(struct kgd_dev *kgd);
 
 /* GPUVM API */
 int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, void **vm,
-					  void **process_info,
-					  struct dma_fence **ef);
+					void **process_info,
+					struct dma_fence **ef);
 int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
-					   struct file *filp,
-					   void **vm, void **process_info,
-					   struct dma_fence **ef);
+					struct file *filp,
+					void **vm, void **process_info,
+					struct dma_fence **ef);
 void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
-				    struct amdgpu_vm *vm);
+				struct amdgpu_vm *vm);
 void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm);
 uint32_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm);
 int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
Signature · 6 lines
2.14.3

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 20180518194250.3813-1-oded.gabbay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

#2 lore raw

Two more comments inline. One cosmetic, one real issue. With that fixed, this patch is Reviewed-by: Felix Kuehling <[email protected]>

Regards,
  Felix

Quoted 5 lines · Oded Gabbay

In case CONFIG_HSA_AMD is not chosen, there is no need to compile amdkfd files that reside inside amdgpu dirver. In addition, because amdkfd depends on x86_64 architecture and amdgpu is not, compiling amdkfd files under i386 architecture can cause compiler errors and warnings.

Quoted 8 lines

This patch modifies amdgpu's makefile to build amdkfd files only if CONFIG_HSA_AMD is chosen. The only file to be compiled unconditionally is amdgpu_amdkfd.c

Direct calls from amdgpu driver proper to functions in other amdgpu_amdkfd_*.c files were changed to calls to functions inside amdgpu_amdkfd.c. These functions call the original functions using a function pointer to allow compilation without the original functions.

The above paragraph needs to be updated.

Quoted 97 lines

v2:
Instead of using function pointers, use stub functions that are compiled only if amdkfd is not compiled.

---
 drivers/gpu/drm/amd/amdgpu/Makefile        | 13 +++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 46 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 12 ++++----
 3 files changed, 62 insertions(+), 9 deletions(-)
drivers/gpu/drm/amd/amdgpu/Makefile +10 -3
index 68e9f584c570..e03ee41cedcb 100644
@@ -56,8 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 
 # add asic specific block
 amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
-	ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o \
-	amdgpu_amdkfd_gfx_v7.o
+	ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
 
 amdgpu-$(CONFIG_DRM_AMDGPU_SI)+= si.o gmc_v6_0.o gfx_v6_0.o si_ih.o si_dma.o dce_v6_0.o si_dpm.o si_smc.o
 
@@ -130,13 +129,21 @@ amdgpu-y += \
 	vcn_v1_0.o
 
 # add amdkfd interfaces
+amdgpu-y += amdgpu_amdkfd.o
+
+ifneq ($(CONFIG_HSA_AMD),)
 amdgpu-y += \
-	 amdgpu_amdkfd.o \
 	 amdgpu_amdkfd_fence.o \
 	 amdgpu_amdkfd_gpuvm.o \
 	 amdgpu_amdkfd_gfx_v8.o \
 	 amdgpu_amdkfd_gfx_v9.o
 
+ifneq ($(CONFIG_DRM_AMDGPU_CIK),)
+amdgpu-y += amdgpu_amdkfd_gfx_v7.o
+endif
+
+endif
+
 # add cgs
 amdgpu-y += amdgpu_cgs.o
 
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +24 -0
index bd36ee9f7e6d..b0b39bd83f0f 100644
@@ -50,7 +50,9 @@ int amdgpu_amdkfd_init(void)
 		kgd2kfd = NULL;
 	}
 
+
 #elif defined(CONFIG_HSA_AMD)
+
 	ret = kgd2kfd_init(KFD_INTERFACE_VERSION, &kgd2kfd);
 	if (ret)
 		kgd2kfd = NULL;
@@ -58,7 +60,10 @@ int amdgpu_amdkfd_init(void)
 #else
 	ret = -ENOENT;
 #endif
+
+#if defined(CONFIG_HSA_AMD_MODULE) || defined(CONFIG_HSA_AMD)
 	amdgpu_amdkfd_gpuvm_init_mem_limits();
+#endif
 
 	return ret;
 }
@@ -464,3 +469,44 @@ bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device *adev, u32 vmid)
 
 	return false;
 }
+
+#if !defined(CONFIG_HSA_AMD_MODULE) && !defined(CONFIG_HSA_AMD)
+bool amdkfd_fence_check_mm(struct dma_fence *f, struct mm_struct *mm)
+{
+	return false;
+}
+
+void amdgpu_amdkfd_unreserve_system_memory_limit(struct amdgpu_bo *bo)
+{
+}
+
+void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
+					struct amdgpu_vm *vm)
+{
+}
+
+struct amdgpu_amdkfd_fence *to_amdgpu_amdkfd_fence(struct dma_fence *f)
+{
+	return NULL;

drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device *adev, u32 vmid) · line 500

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +10 -0
+}
+
+int amdgpu_amdkfd_evict_userptr(struct kgd_mem *mem, struct mm_struct *mm)
+{
+	return 0;
+}
+
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_7_get_functions(void)
+{
+	return NULL;

I think this will cause an oops in amdgpu_amdkfd_probe because that function doesn't handle kgd2kfd == NULL. You could remove amdgpu_amdkfd_gfx_*_get_functions and instead turn amdgpu_amdkfd_probe itself into a stub to simplify this. That's the only place where *_get_functions are called.

Quoted 27 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +12 -0
+}
+
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_8_0_get_functions(void)
+{
+	return NULL;
+}
+
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_9_0_get_functions(void)
+{
+	return NULL;
+}
+#endif
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h +2 -4
index 12367a9951e8..a8418a3f4e9d 100644
@@ -156,14 +156,14 @@ uint64_t amdgpu_amdkfd_get_vram_usage(struct kgd_dev *kgd);
 
 /* GPUVM API */
 int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, void **vm,
-					  void **process_info,
-					  struct dma_fence **ef);
+					void **process_info,
+					struct dma_fence **ef);
 int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
-					   struct file *filp,
-					   void **vm, void **process_info,

drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h uint64_t amdgpu_amdkfd_get_vram_usage(struct kgd_dev *kgd); · line 169

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h +4 -2
-					   struct dma_fence **ef);
+					struct file *filp,
+					void **vm, void **process_info,
+					struct dma_fence **ef);
 void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
-				    struct amdgpu_vm *vm);
+				struct amdgpu_vm *vm);
 void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm);
 uint32_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm);
 int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 6cf89c5a-61d9-041b-5296-ec57d8361a40-5C7GfCeVMHo@public.gmane.org

#4 lore raw
Quoted 16 lines · Felix Kuehling

Two more comments inline. One cosmetic, one real issue. With that fixed, this patch is Reviewed-by: Felix Kuehling <[email protected]>

Regards,
Felix

Quoted 9 lines · Oded Gabbay

In case CONFIG_HSA_AMD is not chosen, there is no need to compile amdkfd files that reside inside amdgpu dirver. In addition, because amdkfd depends on x86_64 architecture and amdgpu is not, compiling amdkfd files under i386 architecture can cause compiler errors and warnings.

This patch modifies amdgpu's makefile to build amdkfd files only if CONFIG_HSA_AMD is chosen. The only file to be compiled unconditionally is amdgpu_amdkfd.c

Quoted 6 lines
Quoted 4 lines

Direct calls from amdgpu driver proper to functions in other amdgpu_amdkfd_*.c files were changed to calls to functions inside amdgpu_amdkfd.c. These functions call the original functions using a function pointer to allow compilation without the original functions.

The above paragraph needs to be updated.

Yeah, I'll send v3.

Quoted 106 lines
Quoted 106 lines

v2:
Instead of using function pointers, use stub functions that are compiled only if amdkfd is not compiled.

---
 drivers/gpu/drm/amd/amdgpu/Makefile        | 13 +++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 46 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 12 ++++----
 3 files changed, 62 insertions(+), 9 deletions(-)
drivers/gpu/drm/amd/amdgpu/Makefile +10 -3
index 68e9f584c570..e03ee41cedcb 100644
@@ -56,8 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 
 # add asic specific block
 amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
-     ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o \
-     amdgpu_amdkfd_gfx_v7.o
+     ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
 
 amdgpu-$(CONFIG_DRM_AMDGPU_SI)+= si.o gmc_v6_0.o gfx_v6_0.o si_ih.o si_dma.o dce_v6_0.o si_dpm.o si_smc.o
 
@@ -130,13 +129,21 @@ amdgpu-y += \
      vcn_v1_0.o
 
 # add amdkfd interfaces
+amdgpu-y += amdgpu_amdkfd.o
+
+ifneq ($(CONFIG_HSA_AMD),)
 amdgpu-y += \
-      amdgpu_amdkfd.o \
       amdgpu_amdkfd_fence.o \
       amdgpu_amdkfd_gpuvm.o \
       amdgpu_amdkfd_gfx_v8.o \
       amdgpu_amdkfd_gfx_v9.o
 
+ifneq ($(CONFIG_DRM_AMDGPU_CIK),)
+amdgpu-y += amdgpu_amdkfd_gfx_v7.o
+endif
+
+endif
+
 # add cgs
 amdgpu-y += amdgpu_cgs.o
 
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +32 -0
index bd36ee9f7e6d..b0b39bd83f0f 100644
@@ -50,7 +50,9 @@ int amdgpu_amdkfd_init(void)
              kgd2kfd = NULL;
      }
 
+
 #elif defined(CONFIG_HSA_AMD)
+
      ret = kgd2kfd_init(KFD_INTERFACE_VERSION, &kgd2kfd);
      if (ret)
              kgd2kfd = NULL;
@@ -58,7 +60,10 @@ int amdgpu_amdkfd_init(void)
 #else
      ret = -ENOENT;
 #endif
+
+#if defined(CONFIG_HSA_AMD_MODULE) || defined(CONFIG_HSA_AMD)
      amdgpu_amdkfd_gpuvm_init_mem_limits();
+#endif
 
      return ret;
 }
@@ -464,3 +469,44 @@ bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device *adev, u32 vmid)
 
      return false;
 }
+
+#if !defined(CONFIG_HSA_AMD_MODULE) && !defined(CONFIG_HSA_AMD)
+bool amdkfd_fence_check_mm(struct dma_fence *f, struct mm_struct *mm)
+{
+     return false;
+}
+
+void amdgpu_amdkfd_unreserve_system_memory_limit(struct amdgpu_bo *bo)
+{
+}
+
+void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
+                                     struct amdgpu_vm *vm)
+{
+}
+
+struct amdgpu_amdkfd_fence *to_amdgpu_amdkfd_fence(struct dma_fence *f)
+{
+     return NULL;
+}
+
+int amdgpu_amdkfd_evict_userptr(struct kgd_mem *mem, struct mm_struct *mm)
+{
+     return 0;
+}
+
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_7_get_functions(void)
Quoted 8 lines

drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device *adev, u32 vmid) · line 500

Quoted 2 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +2 -0
+{
+     return NULL;

I think this will cause an oops in amdgpu_amdkfd_probe because that function doesn't handle kgd2kfd == NULL. You could remove amdgpu_amdkfd_gfx_*_get_functions and instead turn amdgpu_amdkfd_probe itself into a stub to simplify this. That's the only place where *_get_functions are called.

Actually this function will never be called if amdkfd is not compiled, because amdgpu_amdkfd_probe will exit immediately due to the if statement at the start of that function (which checks kgd2kfd is not NULL). I did see that kgd2kfd is not initialized to NULL in case amdkfd is not compiled and I fixed that in v3.
I prefer not to add stub for probe because then I will have to #ifdef around the real probe function and I prefer to minimize #ifdefs

Oded

Quoted 30 lines
Quoted 30 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +12 -0
+}
+
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_8_0_get_functions(void)
+{
+     return NULL;
+}
+
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_9_0_get_functions(void)
+{
+     return NULL;
+}
+#endif
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h +3 -5
index 12367a9951e8..a8418a3f4e9d 100644
@@ -156,14 +156,14 @@ uint64_t amdgpu_amdkfd_get_vram_usage(struct kgd_dev *kgd);
 
 /* GPUVM API */
 int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, void **vm,
-                                       void **process_info,
-                                       struct dma_fence **ef);
+                                     void **process_info,
+                                     struct dma_fence **ef);
 int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
-                                        struct file *filp,
-                                        void **vm, void **process_info,
-                                        struct dma_fence **ef);
+                                     struct file *filp,
Quoted 9 lines

drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h uint64_t amdgpu_amdkfd_get_vram_usage(struct kgd_dev *kgd); · line 169

Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h +3 -1
+                                     void **vm, void **process_info,
+                                     struct dma_fence **ef);
 void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
-                                 struct amdgpu_vm *vm);
+                             struct amdgpu_vm *vm);
 void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm);
 uint32_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm);
 int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): CAFCwf11LQ00m+VEHjjoTJirwx_N5L9gYAMULD3Bta9_pU6toZg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#6 lore raw
Quoted 8 lines · Oded Gabbay
Quoted 8 lines

drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device *adev, u32 vmid) · line 500

Quoted 3 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +3 -0
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_7_get_functions(void)
+{
+     return NULL;

I think this will cause an oops in amdgpu_amdkfd_probe because that function doesn't handle kgd2kfd == NULL. You could remove amdgpu_amdkfd_gfx_*_get_functions and instead turn amdgpu_amdkfd_probe itself into a stub to simplify this. That's the only place where *_get_functions are called.

Quoted 4 lines

Actually this function will never be called if amdkfd is not compiled, because amdgpu_amdkfd_probe will exit immediately due to the if statement at the start of that function (which checks kgd2kfd is not NULL).

Ah, I missed that.

Quoted 2 lines

I did see that kgd2kfd is not initialized to NULL in case
amdkfd is not compiled and I fixed that in v3.

kgd2kfd is global uninitialized data. That should be implicitly initialized to 0. I've got checkpatch warnings before for initializing global variables to 0 explicitly. Anyway, doing the initialization explicitly in the code like you did in v3 doesn't do any harm and it makes things more clear.

Thanks,
  Felix

Quoted 4 lines

I prefer not to add stub for probe because then I will have to #ifdef around the real probe function and I prefer to minimize #ifdefs

Oded

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx