Re: [PATCH] drm/amdkfd: Consolidate duplicated bo alloc flags

2020-03-09 Thread Felix Kuehling

On 2020-03-06 18:30, Yong Zhao wrote:

ALLOC_MEM_FLAGS_* used are the same as the KFD_IOC_ALLOC_MEM_FLAGS_*,
but they are interweavedly used in kernel driver, resulting in bad
readability. For example, KFD_IOC_ALLOC_MEM_FLAGS_COHERENT is not
referenced in kernel, and it functions implicitly in kernel through
ALLOC_MEM_FLAGS_COHERENT, causing unnecessary confusion.

Replace all occurrences of ALLOC_MEM_FLAGS_* with
KFD_IOC_ALLOC_MEM_FLAGS_* to solve the problem.

Change-Id: Iced6ed3698167296c97b14e7e4569883859d619c
Signed-off-by: Yong Zhao 


Reviewed-by: Felix Kuehling 



---
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c|  6 ++--
  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 29 ++-
  drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 13 +
  .../gpu/drm/amd/include/kgd_kfd_interface.h   | 21 --
  4 files changed, 27 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 726c91ab6761..abfbe89e805e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -29,6 +29,7 @@
  #include 
  #include 
  #include "amdgpu_xgmi.h"
+#include 
  
  static const unsigned int compute_vmid_bitmap = 0xFF00;
  
@@ -501,10 +502,11 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, int dma_buf_fd,

   metadata_size, &metadata_flags);
if (flags) {
*flags = (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
-   ALLOC_MEM_FLAGS_VRAM : ALLOC_MEM_FLAGS_GTT;
+   KFD_IOC_ALLOC_MEM_FLAGS_VRAM
+   : KFD_IOC_ALLOC_MEM_FLAGS_GTT;
  
  		if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)

-   *flags |= ALLOC_MEM_FLAGS_PUBLIC;
+   *flags |= KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC;
}
  
  out_put:

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index e4481caed648..9dff792c9290 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -29,6 +29,7 @@
  #include "amdgpu_vm.h"
  #include "amdgpu_amdkfd.h"
  #include "amdgpu_dma_buf.h"
+#include 
  
  /* BO flag to indicate a KFD userptr BO */

  #define AMDGPU_AMDKFD_USERPTR_BO (1ULL << 63)
@@ -400,18 +401,18 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct 
amdgpu_sync *sync)
  static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem *mem)
  {
struct amdgpu_device *bo_adev = amdgpu_ttm_adev(mem->bo->tbo.bdev);
-   bool coherent = mem->alloc_flags & ALLOC_MEM_FLAGS_COHERENT;
+   bool coherent = mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_COHERENT;
uint32_t mapping_flags;
  
  	mapping_flags = AMDGPU_VM_PAGE_READABLE;

-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE)
mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE)
mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
  
  	switch (adev->asic_type) {

case CHIP_ARCTURUS:
-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_VRAM) {
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
if (bo_adev == adev)
mapping_flags |= coherent ?
AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW;
@@ -1160,24 +1161,24 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
/*
 * Check on which domain to allocate BO
 */
-   if (flags & ALLOC_MEM_FLAGS_VRAM) {
+   if (flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
domain = alloc_domain = AMDGPU_GEM_DOMAIN_VRAM;
alloc_flags = AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
-   alloc_flags |= (flags & ALLOC_MEM_FLAGS_PUBLIC) ?
+   alloc_flags |= (flags & KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC) ?
AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED :
AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
-   } else if (flags & ALLOC_MEM_FLAGS_GTT) {
+   } else if (flags & KFD_IOC_ALLOC_MEM_FLAGS_GTT) {
domain = alloc_domain = AMDGPU_GEM_DOMAIN_GTT;
alloc_flags = 0;
-   } else if (flags & ALLOC_MEM_FLAGS_USERPTR) {
+   } else if (flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
domain = AMDGPU_GEM_DOMAIN_GTT;
alloc_domain = AMDGPU_GEM_DOMAIN_CPU;
alloc_flags = 0;
if (!offset || !*offset)
return -EINVAL;
user_addr = untagged_addr(*offset);
-   } else if (flags & (ALLOC_MEM_FLAGS_DOORBELL |
-   ALLOC_MEM_FLAGS_MMIO_REMAP)) {
+   

[PATCH] drm/amdkfd: Consolidate duplicated bo alloc flags

2020-03-06 Thread Yong Zhao
ALLOC_MEM_FLAGS_* used are the same as the KFD_IOC_ALLOC_MEM_FLAGS_*,
but they are interweavedly used in kernel driver, resulting in bad
readability. For example, KFD_IOC_ALLOC_MEM_FLAGS_COHERENT is not
referenced in kernel, and it functions implicitly in kernel through
ALLOC_MEM_FLAGS_COHERENT, causing unnecessary confusion.

Replace all occurrences of ALLOC_MEM_FLAGS_* with
KFD_IOC_ALLOC_MEM_FLAGS_* to solve the problem.

Change-Id: Iced6ed3698167296c97b14e7e4569883859d619c
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c|  6 ++--
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 29 ++-
 drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 13 +
 .../gpu/drm/amd/include/kgd_kfd_interface.h   | 21 --
 4 files changed, 27 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 726c91ab6761..abfbe89e805e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include "amdgpu_xgmi.h"
+#include 
 
 static const unsigned int compute_vmid_bitmap = 0xFF00;
 
@@ -501,10 +502,11 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, 
int dma_buf_fd,
   metadata_size, &metadata_flags);
if (flags) {
*flags = (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
-   ALLOC_MEM_FLAGS_VRAM : ALLOC_MEM_FLAGS_GTT;
+   KFD_IOC_ALLOC_MEM_FLAGS_VRAM
+   : KFD_IOC_ALLOC_MEM_FLAGS_GTT;
 
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
-   *flags |= ALLOC_MEM_FLAGS_PUBLIC;
+   *flags |= KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC;
}
 
 out_put:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index e4481caed648..9dff792c9290 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -29,6 +29,7 @@
 #include "amdgpu_vm.h"
 #include "amdgpu_amdkfd.h"
 #include "amdgpu_dma_buf.h"
+#include 
 
 /* BO flag to indicate a KFD userptr BO */
 #define AMDGPU_AMDKFD_USERPTR_BO (1ULL << 63)
@@ -400,18 +401,18 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct 
amdgpu_sync *sync)
 static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem *mem)
 {
struct amdgpu_device *bo_adev = amdgpu_ttm_adev(mem->bo->tbo.bdev);
-   bool coherent = mem->alloc_flags & ALLOC_MEM_FLAGS_COHERENT;
+   bool coherent = mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_COHERENT;
uint32_t mapping_flags;
 
mapping_flags = AMDGPU_VM_PAGE_READABLE;
-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE)
mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE)
mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
 
switch (adev->asic_type) {
case CHIP_ARCTURUS:
-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_VRAM) {
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
if (bo_adev == adev)
mapping_flags |= coherent ?
AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW;
@@ -1160,24 +1161,24 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
/*
 * Check on which domain to allocate BO
 */
-   if (flags & ALLOC_MEM_FLAGS_VRAM) {
+   if (flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
domain = alloc_domain = AMDGPU_GEM_DOMAIN_VRAM;
alloc_flags = AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
-   alloc_flags |= (flags & ALLOC_MEM_FLAGS_PUBLIC) ?
+   alloc_flags |= (flags & KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC) ?
AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED :
AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
-   } else if (flags & ALLOC_MEM_FLAGS_GTT) {
+   } else if (flags & KFD_IOC_ALLOC_MEM_FLAGS_GTT) {
domain = alloc_domain = AMDGPU_GEM_DOMAIN_GTT;
alloc_flags = 0;
-   } else if (flags & ALLOC_MEM_FLAGS_USERPTR) {
+   } else if (flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
domain = AMDGPU_GEM_DOMAIN_GTT;
alloc_domain = AMDGPU_GEM_DOMAIN_CPU;
alloc_flags = 0;
if (!offset || !*offset)
return -EINVAL;
user_addr = untagged_addr(*offset);
-   } else if (flags & (ALLOC_MEM_FLAGS_DOORBELL |
-   ALLOC_MEM_FLAGS_MMIO_REMAP)) {
+   } else if (flags & (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
+   KF

Re: [PATCH] drm/amdkfd: Consolidate duplicated bo alloc flags

2020-03-05 Thread Zhao, Yong
[AMD Official Use Only - Internal Distribution Only]

Okay, I will change back to its original format.

Yong


From: Kuehling, Felix 
Sent: Thursday, March 5, 2020 3:49 PM
To: amd-gfx@lists.freedesktop.org ; Zhao, Yong 

Subject: Re: [PATCH] drm/amdkfd: Consolidate duplicated bo alloc flags

On 2020-03-04 3:21 p.m., Yong Zhao wrote:
> ALLOC_MEM_FLAGS_* used are the same as the KFD_IOC_ALLOC_MEM_FLAGS_*,
> but they are interweavedly used in kernel driver, resulting in bad
> readability. For example, KFD_IOC_ALLOC_MEM_FLAGS_COHERENT is totally
> not referenced in kernel, and it functions in the kernel through
> ALLOC_MEM_FLAGS_COHERENT, causing unnecessary confusion.
>
> Replace all occurrences of ALLOC_MEM_FLAGS_* by
> KFD_IOC_ALLOC_MEM_FLAGS_* to solve the problem.
>
> Change-Id: Iced6ed3698167296c97b14e7e4569883859d619c
> Signed-off-by: Yong Zhao 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c|  9 +++--
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 38 +++
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 13 ---
>   .../gpu/drm/amd/include/kgd_kfd_interface.h   | 21 --
>   4 files changed, 36 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 726c91ab6761..affaa0d4b636 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -29,6 +29,7 @@
>   #include 
>   #include 
>   #include "amdgpu_xgmi.h"
> +#include 
>
>   static const unsigned int compute_vmid_bitmap = 0xFF00;
>
> @@ -500,11 +501,13 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, 
> int dma_buf_fd,
>r = amdgpu_bo_get_metadata(bo, metadata_buffer, buffer_size,
>   metadata_size, &metadata_flags);
>if (flags) {
> - *flags = (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
> - ALLOC_MEM_FLAGS_VRAM : ALLOC_MEM_FLAGS_GTT;
> + if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)
> + *flags = KFD_IOC_ALLOC_MEM_FLAGS_VRAM;
> + else
> + *flags = KFD_IOC_ALLOC_MEM_FLAGS_GTT;

You're sneaking in some personal preference (changing the trinary (cond
? a : b) operator to if-else) with the renaming change. Personally I
find the trinary operator just as readable and more concise.


>
>if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
> - *flags |= ALLOC_MEM_FLAGS_PUBLIC;
> + *flags |= KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC;
>}
>
>   out_put:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index e4481caed648..c81fe7011e88 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -29,6 +29,7 @@
>   #include "amdgpu_vm.h"
>   #include "amdgpu_amdkfd.h"
>   #include "amdgpu_dma_buf.h"
> +#include 
>
>   /* BO flag to indicate a KFD userptr BO */
>   #define AMDGPU_AMDKFD_USERPTR_BO (1ULL << 63)
> @@ -400,18 +401,18 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct 
> amdgpu_sync *sync)
>   static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem 
> *mem)
>   {
>struct amdgpu_device *bo_adev = amdgpu_ttm_adev(mem->bo->tbo.bdev);
> - bool coherent = mem->alloc_flags & ALLOC_MEM_FLAGS_COHERENT;
> + bool coherent = mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_COHERENT;
>uint32_t mapping_flags;
>
>mapping_flags = AMDGPU_VM_PAGE_READABLE;
> - if (mem->alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> + if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE)
>mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> - if (mem->alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> + if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE)
>mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
>
>switch (adev->asic_type) {
>case CHIP_ARCTURUS:
> - if (mem->alloc_flags & ALLOC_MEM_FLAGS_VRAM) {
> + if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
>if (bo_adev == adev)
>mapping_flags |= coherent ?
>AMDGPU_VM_MTYPE_CC : 
> AMDGPU_VM_MTYPE_RW;
> @@ -1160,24 +1161,24 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>/*
> * Chec

Re: [PATCH] drm/amdkfd: Consolidate duplicated bo alloc flags

2020-03-05 Thread Felix Kuehling

On 2020-03-04 3:21 p.m., Yong Zhao wrote:

ALLOC_MEM_FLAGS_* used are the same as the KFD_IOC_ALLOC_MEM_FLAGS_*,
but they are interweavedly used in kernel driver, resulting in bad
readability. For example, KFD_IOC_ALLOC_MEM_FLAGS_COHERENT is totally
not referenced in kernel, and it functions in the kernel through
ALLOC_MEM_FLAGS_COHERENT, causing unnecessary confusion.

Replace all occurrences of ALLOC_MEM_FLAGS_* by
KFD_IOC_ALLOC_MEM_FLAGS_* to solve the problem.

Change-Id: Iced6ed3698167296c97b14e7e4569883859d619c
Signed-off-by: Yong Zhao 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c|  9 +++--
  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 38 +++
  drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 13 ---
  .../gpu/drm/amd/include/kgd_kfd_interface.h   | 21 --
  4 files changed, 36 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 726c91ab6761..affaa0d4b636 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -29,6 +29,7 @@
  #include 
  #include 
  #include "amdgpu_xgmi.h"
+#include 
  
  static const unsigned int compute_vmid_bitmap = 0xFF00;
  
@@ -500,11 +501,13 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, int dma_buf_fd,

r = amdgpu_bo_get_metadata(bo, metadata_buffer, buffer_size,
   metadata_size, &metadata_flags);
if (flags) {
-   *flags = (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
-   ALLOC_MEM_FLAGS_VRAM : ALLOC_MEM_FLAGS_GTT;
+   if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)
+   *flags = KFD_IOC_ALLOC_MEM_FLAGS_VRAM;
+   else
+   *flags = KFD_IOC_ALLOC_MEM_FLAGS_GTT;


You're sneaking in some personal preference (changing the trinary (cond 
? a : b) operator to if-else) with the renaming change. Personally I 
find the trinary operator just as readable and more concise.



  
  		if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)

-   *flags |= ALLOC_MEM_FLAGS_PUBLIC;
+   *flags |= KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC;
}
  
  out_put:

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index e4481caed648..c81fe7011e88 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -29,6 +29,7 @@
  #include "amdgpu_vm.h"
  #include "amdgpu_amdkfd.h"
  #include "amdgpu_dma_buf.h"
+#include 
  
  /* BO flag to indicate a KFD userptr BO */

  #define AMDGPU_AMDKFD_USERPTR_BO (1ULL << 63)
@@ -400,18 +401,18 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct 
amdgpu_sync *sync)
  static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem *mem)
  {
struct amdgpu_device *bo_adev = amdgpu_ttm_adev(mem->bo->tbo.bdev);
-   bool coherent = mem->alloc_flags & ALLOC_MEM_FLAGS_COHERENT;
+   bool coherent = mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_COHERENT;
uint32_t mapping_flags;
  
  	mapping_flags = AMDGPU_VM_PAGE_READABLE;

-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE)
mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE)
mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
  
  	switch (adev->asic_type) {

case CHIP_ARCTURUS:
-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_VRAM) {
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
if (bo_adev == adev)
mapping_flags |= coherent ?
AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW;
@@ -1160,24 +1161,24 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
/*
 * Check on which domain to allocate BO
 */
-   if (flags & ALLOC_MEM_FLAGS_VRAM) {
+   if (flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
domain = alloc_domain = AMDGPU_GEM_DOMAIN_VRAM;
alloc_flags = AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
-   alloc_flags |= (flags & ALLOC_MEM_FLAGS_PUBLIC) ?
+   alloc_flags |= (flags & KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC) ?
AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED :
AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
-   } else if (flags & ALLOC_MEM_FLAGS_GTT) {
+   } else if (flags & KFD_IOC_ALLOC_MEM_FLAGS_GTT) {
domain = alloc_domain = AMDGPU_GEM_DOMAIN_GTT;
alloc_flags = 0;
-   } else if (flags & ALLOC_MEM_FLAGS_USERPTR) {
+   } else if (flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
domain = AMDGPU_GEM_DOMAIN_GTT

[PATCH] drm/amdkfd: Consolidate duplicated bo alloc flags

2020-03-04 Thread Yong Zhao
ALLOC_MEM_FLAGS_* used are the same as the KFD_IOC_ALLOC_MEM_FLAGS_*,
but they are interweavedly used in kernel driver, resulting in bad
readability. For example, KFD_IOC_ALLOC_MEM_FLAGS_COHERENT is totally
not referenced in kernel, and it functions in the kernel through
ALLOC_MEM_FLAGS_COHERENT, causing unnecessary confusion.

Replace all occurrences of ALLOC_MEM_FLAGS_* by
KFD_IOC_ALLOC_MEM_FLAGS_* to solve the problem.

Change-Id: Iced6ed3698167296c97b14e7e4569883859d619c
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c|  9 +++--
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 38 +++
 drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 13 ---
 .../gpu/drm/amd/include/kgd_kfd_interface.h   | 21 --
 4 files changed, 36 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 726c91ab6761..affaa0d4b636 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include "amdgpu_xgmi.h"
+#include 
 
 static const unsigned int compute_vmid_bitmap = 0xFF00;
 
@@ -500,11 +501,13 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, 
int dma_buf_fd,
r = amdgpu_bo_get_metadata(bo, metadata_buffer, buffer_size,
   metadata_size, &metadata_flags);
if (flags) {
-   *flags = (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
-   ALLOC_MEM_FLAGS_VRAM : ALLOC_MEM_FLAGS_GTT;
+   if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)
+   *flags = KFD_IOC_ALLOC_MEM_FLAGS_VRAM;
+   else
+   *flags = KFD_IOC_ALLOC_MEM_FLAGS_GTT;
 
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
-   *flags |= ALLOC_MEM_FLAGS_PUBLIC;
+   *flags |= KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC;
}
 
 out_put:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index e4481caed648..c81fe7011e88 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -29,6 +29,7 @@
 #include "amdgpu_vm.h"
 #include "amdgpu_amdkfd.h"
 #include "amdgpu_dma_buf.h"
+#include 
 
 /* BO flag to indicate a KFD userptr BO */
 #define AMDGPU_AMDKFD_USERPTR_BO (1ULL << 63)
@@ -400,18 +401,18 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct 
amdgpu_sync *sync)
 static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem *mem)
 {
struct amdgpu_device *bo_adev = amdgpu_ttm_adev(mem->bo->tbo.bdev);
-   bool coherent = mem->alloc_flags & ALLOC_MEM_FLAGS_COHERENT;
+   bool coherent = mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_COHERENT;
uint32_t mapping_flags;
 
mapping_flags = AMDGPU_VM_PAGE_READABLE;
-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE)
mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE)
mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
 
switch (adev->asic_type) {
case CHIP_ARCTURUS:
-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_VRAM) {
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
if (bo_adev == adev)
mapping_flags |= coherent ?
AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW;
@@ -1160,24 +1161,24 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
/*
 * Check on which domain to allocate BO
 */
-   if (flags & ALLOC_MEM_FLAGS_VRAM) {
+   if (flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
domain = alloc_domain = AMDGPU_GEM_DOMAIN_VRAM;
alloc_flags = AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
-   alloc_flags |= (flags & ALLOC_MEM_FLAGS_PUBLIC) ?
+   alloc_flags |= (flags & KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC) ?
AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED :
AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
-   } else if (flags & ALLOC_MEM_FLAGS_GTT) {
+   } else if (flags & KFD_IOC_ALLOC_MEM_FLAGS_GTT) {
domain = alloc_domain = AMDGPU_GEM_DOMAIN_GTT;
alloc_flags = 0;
-   } else if (flags & ALLOC_MEM_FLAGS_USERPTR) {
+   } else if (flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
domain = AMDGPU_GEM_DOMAIN_GTT;
alloc_domain = AMDGPU_GEM_DOMAIN_CPU;
alloc_flags = 0;
if (!offset || !*offset)
return -EINVAL;
user_addr = untagged_addr(*offset);
-   } else if (flags & (ALLOC_MEM_