Re: [PATCH] drm/amdkfd: Add more comments on GFX9 user CP queue MQD workaround

2020-03-05 Thread Christian König 

Am 04.03.20 um 20:40 schrieb Yong Zhao:

Because too many things are involved in this workaround, we need more
comments to avoid pitfalls.

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


Acked-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  5 -
  .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c| 18 +++---
  2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 1947a326de57..10f6f4b21b44 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1041,7 +1041,10 @@ int amdgpu_ttm_gart_bind(struct amdgpu_device *adev,
if (r)
goto gart_bind_fail;
  
-		/* Patch mtype of the second part BO */

+   /* The memory type of the first page defaults to UC. Now
+* modify the memory type to NC from the second page of
+* the BO onward.
+*/
flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
flags |= AMDGPU_PTE_MTYPE_VG10(AMDGPU_MTYPE_NC);
  
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c

index 436b7f518979..5b11190ff6e6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
@@ -87,9 +87,21 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
int retval;
struct kfd_mem_obj *mqd_mem_obj = NULL;
  
-	/* From V9,  for CWSR, the control stack is located on the next page

-* boundary after the mqd, we will use the gtt allocation function
-* instead of sub-allocation function.
+   /* For V9 only, due to a HW bug, the control stack of a user mode
+* compute queue needs to be allocated just behind the page boundary
+* of its regular MQD buffer. So we allocate an enlarged MQD buffer:
+* the first page of the buffer serves as the regular MQD buffer
+* purpose and the remaining is for control stack. Although the two
+* parts are in the same buffer object, they need different memory
+* types: MQD part needs UC (uncached) as usual, while control stack
+* needs NC (non coherent), which is different from the UC type which
+* is used when control stack is allocated in user space.
+*
+* Because of all those, we use the gtt allocation function instead
+* of sub-allocation function for this enlarged MQD buffer. Moreover,
+* in order to achieve two memory types in a single buffer object, we
+* pass a special bo flag AMDGPU_GEM_CREATE_MQD_GFX9 to instruct
+* amdgpu memory functions to do so.
 */
if (kfd->cwsr_enabled && (q->type == KFD_QUEUE_TYPE_COMPUTE)) {
mqd_mem_obj = kzalloc(sizeof(struct kfd_mem_obj), GFP_KERNEL);


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdkfd: Add more comments on GFX9 user CP queue MQD workaround

2020-03-04 Thread philip yang 

Reviewed-by: Philip Yang 

On 2020-03-04 2:40 p.m., Yong Zhao wrote:

Because too many things are involved in this workaround, we need more
comments to avoid pitfalls.

Change-Id: I5d7917296dd5f5edb45921118cf8e7d778d40de1
Signed-off-by: Yong Zhao 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  5 -
  .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c| 18 +++---
  2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 1947a326de57..10f6f4b21b44 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1041,7 +1041,10 @@ int amdgpu_ttm_gart_bind(struct amdgpu_device *adev,
if (r)
goto gart_bind_fail;
  
-		/* Patch mtype of the second part BO */

+   /* The memory type of the first page defaults to UC. Now
+* modify the memory type to NC from the second page of
+* the BO onward.
+*/
flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
flags |= AMDGPU_PTE_MTYPE_VG10(AMDGPU_MTYPE_NC);
  
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c

index 436b7f518979..5b11190ff6e6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
@@ -87,9 +87,21 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
int retval;
struct kfd_mem_obj *mqd_mem_obj = NULL;
  
-	/* From V9,  for CWSR, the control stack is located on the next page

-* boundary after the mqd, we will use the gtt allocation function
-* instead of sub-allocation function.
+   /* For V9 only, due to a HW bug, the control stack of a user mode
+* compute queue needs to be allocated just behind the page boundary
+* of its regular MQD buffer. So we allocate an enlarged MQD buffer:
+* the first page of the buffer serves as the regular MQD buffer
+* purpose and the remaining is for control stack. Although the two
+* parts are in the same buffer object, they need different memory
+* types: MQD part needs UC (uncached) as usual, while control stack
+* needs NC (non coherent), which is different from the UC type which
+* is used when control stack is allocated in user space.
+*
+* Because of all those, we use the gtt allocation function instead
+* of sub-allocation function for this enlarged MQD buffer. Moreover,
+* in order to achieve two memory types in a single buffer object, we
+* pass a special bo flag AMDGPU_GEM_CREATE_MQD_GFX9 to instruct
+* amdgpu memory functions to do so.
 */
if (kfd->cwsr_enabled && (q->type == KFD_QUEUE_TYPE_COMPUTE)) {
mqd_mem_obj = kzalloc(sizeof(struct kfd_mem_obj), GFP_KERNEL);


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdkfd: Add more comments on GFX9 user CP queue MQD workaround

2020-03-04 Thread Yong Zhao 
Because too many things are involved in this workaround, we need more
comments to avoid pitfalls.

Change-Id: I5d7917296dd5f5edb45921118cf8e7d778d40de1
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  5 -
 .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c| 18 +++---
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 1947a326de57..10f6f4b21b44 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1041,7 +1041,10 @@ int amdgpu_ttm_gart_bind(struct amdgpu_device *adev,
if (r)
goto gart_bind_fail;
 
-   /* Patch mtype of the second part BO */
+   /* The memory type of the first page defaults to UC. Now
+* modify the memory type to NC from the second page of
+* the BO onward.
+*/
flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
flags |= AMDGPU_PTE_MTYPE_VG10(AMDGPU_MTYPE_NC);
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
index 436b7f518979..5b11190ff6e6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
@@ -87,9 +87,21 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
int retval;
struct kfd_mem_obj *mqd_mem_obj = NULL;
 
-   /* From V9,  for CWSR, the control stack is located on the next page
-* boundary after the mqd, we will use the gtt allocation function
-* instead of sub-allocation function.
+   /* For V9 only, due to a HW bug, the control stack of a user mode
+* compute queue needs to be allocated just behind the page boundary
+* of its regular MQD buffer. So we allocate an enlarged MQD buffer:
+* the first page of the buffer serves as the regular MQD buffer
+* purpose and the remaining is for control stack. Although the two
+* parts are in the same buffer object, they need different memory
+* types: MQD part needs UC (uncached) as usual, while control stack
+* needs NC (non coherent), which is different from the UC type which
+* is used when control stack is allocated in user space.
+*
+* Because of all those, we use the gtt allocation function instead
+* of sub-allocation function for this enlarged MQD buffer. Moreover,
+* in order to achieve two memory types in a single buffer object, we
+* pass a special bo flag AMDGPU_GEM_CREATE_MQD_GFX9 to instruct
+* amdgpu memory functions to do so.
 */
if (kfd->cwsr_enabled && (q->type == KFD_QUEUE_TYPE_COMPUTE)) {
mqd_mem_obj = kzalloc(sizeof(struct kfd_mem_obj), GFP_KERNEL);
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdkfd: Add more comments on GFX9 user CP queue MQD workaround

2020-02-26 Thread Yong Zhao 
Because too many things are involved in this workaround, we need more
comments to avoid pitfalls.

Change-Id: I5d7917296dd5f5edb45921118cf8e7d778d40de1
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  5 -
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c | 17 ++---
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 660867cf2597..a6c8e4cfc051 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1041,7 +1041,10 @@ int amdgpu_ttm_gart_bind(struct amdgpu_device *adev,
if (r)
goto gart_bind_fail;
 
-   /* Patch mtype of the second part BO */
+   /* The memory type of the first page defaults to UC. Now
+* modify the memory type to NC from the second page of
+* the BO onward.
+*/
flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
flags |= AMDGPU_PTE_MTYPE_VG10(AMDGPU_MTYPE_NC);
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
index 436b7f518979..ff2e84872721 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
@@ -87,9 +87,20 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
int retval;
struct kfd_mem_obj *mqd_mem_obj = NULL;
 
-   /* From V9,  for CWSR, the control stack is located on the next page
-* boundary after the mqd, we will use the gtt allocation function
-* instead of sub-allocation function.
+   /* For V9 only, due to a HW bug, the control stack of a user mode
+* compute queue needs to be allocated just behind the page boundary
+* of its regular MQD buffer. So we allocate an enlarged MQD buffer:
+* the first page of the buffer serves as the regular MQD buffer
+* purpose and the remaining is for control stack. Although the two
+* parts are in the same buffer object, they need different memory
+* type: MQD part needs UC (uncached) as usual, while control stack
+* needs NC (non coherent).
+*
+* Because of all those, we use the gtt allocation function instead
+* of sub-allocation function for this enlarged MQD buffer. Moreover,
+* in order to achieve two memory types in a single buffer object, we
+* pass a special bo flag AMDGPU_GEM_CREATE_MQD_GFX9 to instruct
+* amdgpu memory functions to do so.
 */
if (kfd->cwsr_enabled && (q->type == KFD_QUEUE_TYPE_COMPUTE)) {
mqd_mem_obj = kzalloc(sizeof(struct kfd_mem_obj), GFP_KERNEL);
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdkfd: Add more comments on GFX9 user CP queue MQD workaround

2020-02-26 Thread Yong Zhao 
Because two many things are involved in this workaround, we need more
comments to avoid pitfalls.

Change-Id: I5d7917296dd5f5edb45921118cf8e7d778d40de1
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  5 -
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c | 16 +---
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 660867cf2597..a6c8e4cfc051 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1041,7 +1041,10 @@ int amdgpu_ttm_gart_bind(struct amdgpu_device *adev,
if (r)
goto gart_bind_fail;
 
-   /* Patch mtype of the second part BO */
+   /* The memory type of the first page defaults to UC. Now
+* modify the memory type to NC from the second page of
+* the BO onward.
+*/
flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
flags |= AMDGPU_PTE_MTYPE_VG10(AMDGPU_MTYPE_NC);
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
index 436b7f518979..06a3d9ead510 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
@@ -87,9 +87,19 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
int retval;
struct kfd_mem_obj *mqd_mem_obj = NULL;
 
-   /* From V9,  for CWSR, the control stack is located on the next page
-* boundary after the mqd, we will use the gtt allocation function
-* instead of sub-allocation function.
+   /* For V9 only, due to a HW bug, the control stack of user mode
+* compute queues needs to be allocated just behind the page boundary
+* of its MQD buffer. So we allocate a enlarged MQD buffer: the first
+* page of which serves as the regular MQD buffer purpose. Although
+* the two parts are in the same buffer object, they need different
+* memory type: MQD part needs UC (uncached) as usual, while control
+* stack needs NC (non coherent).
+*
+* Because of all those, we use the gtt allocation function instead
+* of sub-allocation function for this enlarged MQD buffer. Moreover,
+* in order to achieve two memory types in a single buffer object, we
+* pass a special bo flag AMDGPU_GEM_CREATE_MQD_GFX9 to instruct
+* amdgpu memory functions to do so.
 */
if (kfd->cwsr_enabled && (q->type == KFD_QUEUE_TYPE_COMPUTE)) {
mqd_mem_obj = kzalloc(sizeof(struct kfd_mem_obj), GFP_KERNEL);
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx