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

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

[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 <Yong.Zhao@amd.com>
---
 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

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

[philip yang]

Reviewed-by: Philip Yang <philip.yang@amd.com>

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 <Yong.Zhao@amd.com>
> ---
>   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

[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 <Yong.Zhao@amd.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   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

[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 <Yong.Zhao@amd.com>
---
 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

[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 <Yong.Zhao@amd.com>
---
 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