Skip to contentSkip to conversation

drm/amdgpu: increase the MAX ring number

PATCH In review Evan Quan 4 participants 10 messages Dec 13, 2018 – Dec 14, 2018 3 messages not in the archive

Subsystems (2)
DRM drivers Radeon and amdgpu DRM drivers
lore
Share & tools
lore

Evan Quan

author
#0 lore raw

PATCH 1/4 drm/amdgpu: increase the MAX ring number

As two more SDMA page queue rings are added on Vega20.

Change-Id I8a3d8fbc924f7c24aaebf17b3f329a4a38fe5a56
Signed-off-by Evan Quan <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h +1 -1
index 4b31c96a2a2c..fe657788024f 100644
@@ -31,7 +31,7 @@
 #endif
 
 /* max number of rings */
-#define AMDGPU_MAX_RINGS		21
+#define AMDGPU_MAX_RINGS		23
 #define AMDGPU_MAX_GFX_RINGS		1
 #define AMDGPU_MAX_COMPUTE_RINGS	8
 #define AMDGPU_MAX_VCE_RINGS		3
Signature · 6 lines
2.19.2

_______________________________________________
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): [email protected]

author
#2 lore raw

PATCH 2/4 drm/amdgpu: update the vm invalidation engine layout

We need new invalidation engine layout due to new SDMA page queues added.

Change-Id I2f3861689bffb9828c9eae744a7a0de4963ac2b6
Signed-off-by Evan Quan <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 47 ++++++++++++++-------------
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h | 10 ++++++
 2 files changed, 35 insertions(+), 22 deletions(-)
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +25 -22
index dc4dadc3575c..092a4d111557 100644
@@ -719,37 +719,40 @@ static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev)
 	}
 }
 
-static int gmc_v9_0_late_init(void *handle)
+static int gmc_v9_0_allocate_vm_inv_eng(struct amdgpu_device *adev)
 {
-	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-	/*
-	 * The latest engine allocation on gfx9 is:
-	 * Engine 0, 1: idle
-	 * Engine 2, 3: firmware
-	 * Engine 4~13: amdgpu ring, subject to change when ring number changes
-	 * Engine 14~15: idle
-	 * Engine 16: kfd tlb invalidation
-	 * Engine 17: Gart flushes
-	 */
-	unsigned vm_inv_eng[AMDGPU_MAX_VMHUBS] = { 4, 4 };
+	struct amdgpu_ring *ring;
+	unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS] =
+		{GFXHUB_FREE_VM_INV_ENGS_BITMAP, MMHUB_FREE_VM_INV_ENGS_BITMAP};
 	unsigned i;
-	int r;
+	unsigned vmhub, inv_eng;
 
-	if (!gmc_v9_0_keep_stolen_memory(adev))
-		amdgpu_bo_late_init(adev);
+	for (i = 0; i < adev->num_rings; ++i) {
+		ring = adev->rings[i];
+		vmhub = ring->funcs->vmhub;
+
+		inv_eng = ffs(vm_inv_engs[vmhub]);
+		BUG_ON(!inv_eng);
 
-	for(i = 0; i < adev->num_rings; ++i) {
-		struct amdgpu_ring *ring = adev->rings[i];
-		unsigned vmhub = ring->funcs->vmhub;
+		ring->vm_inv_eng = inv_eng - 1;
+		change_bit(inv_eng - 1, (unsigned long *)(&vm_inv_engs[vmhub]));
 
-		ring->vm_inv_eng = vm_inv_eng[vmhub]++;
 		dev_info(adev->dev, "ring %s uses VM inv eng %u on hub %u\n",
 			 ring->name, ring->vm_inv_eng, ring->funcs->vmhub);
 	}
 
-	/* Engine 16 is used for KFD and 17 for GART flushes */
-	for(i = 0; i < AMDGPU_MAX_VMHUBS; ++i)
-		BUG_ON(vm_inv_eng[i] > 16);
+	return 0;
+}
+
+static int gmc_v9_0_late_init(void *handle)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	if (!gmc_v9_0_keep_stolen_memory(adev))
+		amdgpu_bo_late_init(adev);
+
+	gmc_v9_0_allocate_vm_inv_eng(adev);
 
 	if (adev->asic_type == CHIP_VEGA10 && !amdgpu_sriov_vf(adev)) {
 		r = gmc_v9_0_ecc_available(adev);
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h +10 -0
index b030ca5ea107..5c8deac65580 100644
@@ -24,6 +24,16 @@
 #ifndef __GMC_V9_0_H__
 #define __GMC_V9_0_H__
 
+	/*
+	 * The latest engine allocation on gfx9 is:
+	 * Engine 2, 3: firmware
+	 * Engine 0, 1, 4~16: amdgpu ring,
+	 *                    subject to change when ring number changes
+	 * Engine 17: Gart flushes
+	 */
+#define GFXHUB_FREE_VM_INV_ENGS_BITMAP		0x1FFF3
+#define MMHUB_FREE_VM_INV_ENGS_BITMAP		0x1FFF3
+
 extern const struct amd_ip_funcs gmc_v9_0_ip_funcs;
 extern const struct amdgpu_ip_block_version gmc_v9_0_ip_block;
Signature · 6 lines
2.19.2

_______________________________________________
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): [email protected]

#4 lore raw

Re: [PATCH 2/4] drm/amdgpu: update the vm invalidation engine layout

Am 13.12.18 um 05:09 schrieb Evan Quan:

Quoted 5 lines

We need new invalidation engine layout due to new SDMA page queues added.

Change-Id I2f3861689bffb9828c9eae744a7a0de4963ac2b6
Signed-off-by Evan Quan <[email protected]>

Wow, that looks really nice. Patch is Reviewed-by: Christian König <[email protected]>

Quoted 83 lines
---
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 47 ++++++++++++++-------------
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h | 10 ++++++
  2 files changed, 35 insertions(+), 22 deletions(-)
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +25 -22
index dc4dadc3575c..092a4d111557 100644
@@ -719,37 +719,40 @@ static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev)
  	}
  }
 
-static int gmc_v9_0_late_init(void *handle)
+static int gmc_v9_0_allocate_vm_inv_eng(struct amdgpu_device *adev)
  {
-	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-	/*
-	 * The latest engine allocation on gfx9 is:
-	 * Engine 0, 1: idle
-	 * Engine 2, 3: firmware
-	 * Engine 4~13: amdgpu ring, subject to change when ring number changes
-	 * Engine 14~15: idle
-	 * Engine 16: kfd tlb invalidation
-	 * Engine 17: Gart flushes
-	 */
-	unsigned vm_inv_eng[AMDGPU_MAX_VMHUBS] = { 4, 4 };
+	struct amdgpu_ring *ring;
+	unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS] =
+		{GFXHUB_FREE_VM_INV_ENGS_BITMAP, MMHUB_FREE_VM_INV_ENGS_BITMAP};
  	unsigned i;
-	int r;
+	unsigned vmhub, inv_eng;
 
-	if (!gmc_v9_0_keep_stolen_memory(adev))
-		amdgpu_bo_late_init(adev);
+	for (i = 0; i < adev->num_rings; ++i) {
+		ring = adev->rings[i];
+		vmhub = ring->funcs->vmhub;
+
+		inv_eng = ffs(vm_inv_engs[vmhub]);
+		BUG_ON(!inv_eng);
 
-	for(i = 0; i < adev->num_rings; ++i) {
-		struct amdgpu_ring *ring = adev->rings[i];
-		unsigned vmhub = ring->funcs->vmhub;
+		ring->vm_inv_eng = inv_eng - 1;
+		change_bit(inv_eng - 1, (unsigned long *)(&vm_inv_engs[vmhub]));
 
-		ring->vm_inv_eng = vm_inv_eng[vmhub]++;
  		dev_info(adev->dev, "ring %s uses VM inv eng %u on hub %u\n",
  			 ring->name, ring->vm_inv_eng, ring->funcs->vmhub);
  	}
 
-	/* Engine 16 is used for KFD and 17 for GART flushes */
-	for(i = 0; i < AMDGPU_MAX_VMHUBS; ++i)
-		BUG_ON(vm_inv_eng[i] > 16);
+	return 0;
+}
+
+static int gmc_v9_0_late_init(void *handle)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	if (!gmc_v9_0_keep_stolen_memory(adev))
+		amdgpu_bo_late_init(adev);
+
+	gmc_v9_0_allocate_vm_inv_eng(adev);
 
  	if (adev->asic_type == CHIP_VEGA10 && !amdgpu_sriov_vf(adev)) {
  		r = gmc_v9_0_ecc_available(adev);
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h +3 -0
index b030ca5ea107..5c8deac65580 100644
@@ -24,6 +24,16 @@
  #ifndef __GMC_V9_0_H__
  #define __GMC_V9_0_H__
 
+	/*
+	 * The latest engine allocation on gfx9 is:
+	 * Engine 2, 3: firmware

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h line 39

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h +7 -0
+	 * Engine 0, 1, 4~16: amdgpu ring,
+	 *                    subject to change when ring number changes
+	 * Engine 17: Gart flushes
+	 */
+#define GFXHUB_FREE_VM_INV_ENGS_BITMAP		0x1FFF3
+#define MMHUB_FREE_VM_INV_ENGS_BITMAP		0x1FFF3
+
  extern const struct amd_ip_funcs gmc_v9_0_ip_funcs;
  extern const struct amdgpu_ip_block_version gmc_v9_0_ip_block;

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

Reviewed-by Oak Zeng
#5 lore raw

RE: [PATCH 2/4] drm/amdgpu: update the vm invalidation engine layout

In reply to a message not in the archive
Reviewed-by Oak Zeng <[email protected]>

Regards,
Oak

-----Original Message-----
From: Evan Quan <[email protected]>
Sent: Wednesday, December 12, 2018 11:10 PM
To: [email protected]
Cc: Koenig, Christian <[email protected]>; Zeng, Oak <[email protected]>; Deucher, Alexander <[email protected]>; Quan, Evan <[email protected]>
Subject: [PATCH 2/4] drm/amdgpu: update the vm invalidation engine layout

We need new invalidation engine layout due to new SDMA page queues added.

Change-Id I2f3861689bffb9828c9eae744a7a0de4963ac2b6
Signed-off-by Evan Quan <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 47 ++++++++++++++-------------  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h | 10 ++++++
 2 files changed, 35 insertions(+), 22 deletions(-)
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +24 -22
index dc4dadc3575c..092a4d111557 100644
@@ -719,37 +719,40 @@ static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev)
 	}
 }
 
-static int gmc_v9_0_late_init(void *handle)
+static int gmc_v9_0_allocate_vm_inv_eng(struct amdgpu_device *adev)
 {
-	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-	/*
-	 * The latest engine allocation on gfx9 is:
-	 * Engine 0, 1: idle
-	 * Engine 2, 3: firmware
-	 * Engine 4~13: amdgpu ring, subject to change when ring number changes
-	 * Engine 14~15: idle
-	 * Engine 16: kfd tlb invalidation
-	 * Engine 17: Gart flushes
-	 */
-	unsigned vm_inv_eng[AMDGPU_MAX_VMHUBS] = { 4, 4 };
+	struct amdgpu_ring *ring;
+	unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS] =
+		{GFXHUB_FREE_VM_INV_ENGS_BITMAP, MMHUB_FREE_VM_INV_ENGS_BITMAP};
 	unsigned i;
-	int r;
+	unsigned vmhub, inv_eng;
 
-	if (!gmc_v9_0_keep_stolen_memory(adev))
-		amdgpu_bo_late_init(adev);
+	for (i = 0; i < adev->num_rings; ++i) {
+		ring = adev->rings[i];
+		vmhub = ring->funcs->vmhub;
+
+		inv_eng = ffs(vm_inv_engs[vmhub]);
+		BUG_ON(!inv_eng);
 
-	for(i = 0; i < adev->num_rings; ++i) {
-		struct amdgpu_ring *ring = adev->rings[i];
-		unsigned vmhub = ring->funcs->vmhub;
+		ring->vm_inv_eng = inv_eng - 1;
+		change_bit(inv_eng - 1, (unsigned long *)(&vm_inv_engs[vmhub]));
 
-		ring->vm_inv_eng = vm_inv_eng[vmhub]++;
 		dev_info(adev->dev, "ring %s uses VM inv eng %u on hub %u\n",
 			 ring->name, ring->vm_inv_eng, ring->funcs->vmhub);
 	}
 
-	/* Engine 16 is used for KFD and 17 for GART flushes */
-	for(i = 0; i < AMDGPU_MAX_VMHUBS; ++i)
-		BUG_ON(vm_inv_eng[i] > 16);
+	return 0;
+}
+
+static int gmc_v9_0_late_init(void *handle) {
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	if (!gmc_v9_0_keep_stolen_memory(adev))
+		amdgpu_bo_late_init(adev);
+
+	gmc_v9_0_allocate_vm_inv_eng(adev);
 
 	if (adev->asic_type == CHIP_VEGA10 && !amdgpu_sriov_vf(adev)) {
 		r = gmc_v9_0_ecc_available(adev);
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h +10 -1
index b030ca5ea107..5c8deac65580 100644
@@ -24,6 +24,16 @@
 #ifndef __GMC_V9_0_H__
 #define __GMC_V9_0_H__
 
+	/*
+	 * The latest engine allocation on gfx9 is:
+	 * Engine 2, 3: firmware
+	 * Engine 0, 1, 4~16: amdgpu ring,
+	 *                    subject to change when ring number changes
+	 * Engine 17: Gart flushes
+	 */
+#define GFXHUB_FREE_VM_INV_ENGS_BITMAP		0x1FFF3
+#define MMHUB_FREE_VM_INV_ENGS_BITMAP		0x1FFF3
+
 extern const struct amd_ip_funcs gmc_v9_0_ip_funcs;  extern const struct amdgpu_ip_block_version gmc_v9_0_ip_block;
 
--

2.19.2

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

#6 lore raw

Re: [PATCH 2/4] drm/amdgpu: update the vm invalidation engine layout

In reply to a message not in the archive

Some nit-picks inline.

Quoted 15 lines · Evan Quan

We need new invalidation engine layout due to new SDMA page queues added.

Change-Id I2f3861689bffb9828c9eae744a7a0de4963ac2b6
Signed-off-by Evan Quan <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 47 ++++++++++++++-------------
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h | 10 ++++++
 2 files changed, 35 insertions(+), 22 deletions(-)
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +0 -0
index dc4dadc3575c..092a4d111557 100644

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev) · line 722

Quoted 5 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +1 -1
@@ -719,37 +719,40 @@ static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev)
 	}
 }
 
-static int gmc_v9_0_late_init(void *handle)
+static int gmc_v9_0_allocate_vm_inv_eng(struct amdgpu_device *adev)

The function returns int. But only ever returns 0 and the caller ignores the return value.

Quoted 17 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +3 -12
 {
-	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-	/*
-	 * The latest engine allocation on gfx9 is:
-	 * Engine 0, 1: idle
-	 * Engine 2, 3: firmware
-	 * Engine 4~13: amdgpu ring, subject to change when ring number changes
-	 * Engine 14~15: idle
-	 * Engine 16: kfd tlb invalidation
-	 * Engine 17: Gart flushes
-	 */
-	unsigned vm_inv_eng[AMDGPU_MAX_VMHUBS] = { 4, 4 };
+	struct amdgpu_ring *ring;
+	unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS] =
+		{GFXHUB_FREE_VM_INV_ENGS_BITMAP, MMHUB_FREE_VM_INV_ENGS_BITMAP};
 	unsigned i;
-	int r;

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev) · line 735

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +7 -2
+	unsigned vmhub, inv_eng;
 
-	if (!gmc_v9_0_keep_stolen_memory(adev))
-		amdgpu_bo_late_init(adev);
+	for (i = 0; i < adev->num_rings; ++i) {
+		ring = adev->rings[i];
+		vmhub = ring->funcs->vmhub;
+
+		inv_eng = ffs(vm_inv_engs[vmhub]);
+		BUG_ON(!inv_eng);

Adding new BUG_ONs is discouraged. checkpatch.pl always warns about these. Errors that can be handled more gracefully shouldn't use a BUG_ON. E.g. use a WARN_ON and return an error code.

Quoted 6 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +2 -3
-	for(i = 0; i < adev->num_rings; ++i) {
-		struct amdgpu_ring *ring = adev->rings[i];
-		unsigned vmhub = ring->funcs->vmhub;
+		ring->vm_inv_eng = inv_eng - 1;
+		change_bit(inv_eng - 1, (unsigned long *)(&vm_inv_engs[vmhub]));

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev) · line 744

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +1 -4
 
-		ring->vm_inv_eng = vm_inv_eng[vmhub]++;
 		dev_info(adev->dev, "ring %s uses VM inv eng %u on hub %u\n",
 			 ring->name, ring->vm_inv_eng, ring->funcs->vmhub);
 	}
 
-	/* Engine 16 is used for KFD and 17 for GART flushes */
-	for(i = 0; i < AMDGPU_MAX_VMHUBS; ++i)
-		BUG_ON(vm_inv_eng[i] > 16);
+	return 0;

This is the only return statement. Maybe this could be a void function. Unless you decide to turn the BUG_ON into a WARN with an error return.

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev) · line 755

Quoted 11 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +11 -0
+}
+
+static int gmc_v9_0_late_init(void *handle)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	if (!gmc_v9_0_keep_stolen_memory(adev))
+		amdgpu_bo_late_init(adev);
+
+	gmc_v9_0_allocate_vm_inv_eng(adev);

You're ignoring the return value. Either, make the function void, or handle potential error returns.

Regards,
  Felix

Quoted 14 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +0 -0
 	if (adev->asic_type == CHIP_VEGA10 && !amdgpu_sriov_vf(adev)) {
 		r = gmc_v9_0_ecc_available(adev);
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h +3 -0
index b030ca5ea107..5c8deac65580 100644
@@ -24,6 +24,16 @@
 #ifndef __GMC_V9_0_H__
 #define __GMC_V9_0_H__
 
+	/*
+	 * The latest engine allocation on gfx9 is:
+	 * Engine 2, 3: firmware

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h line 39

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h +7 -0
+	 * Engine 0, 1, 4~16: amdgpu ring,
+	 *                    subject to change when ring number changes
+	 * Engine 17: Gart flushes
+	 */
+#define GFXHUB_FREE_VM_INV_ENGS_BITMAP		0x1FFF3
+#define MMHUB_FREE_VM_INV_ENGS_BITMAP		0x1FFF3
+
 extern const struct amd_ip_funcs gmc_v9_0_ip_funcs;
 extern const struct amdgpu_ip_block_version gmc_v9_0_ip_block;

_______________________________________________
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): 37469029-e5b3-6dab-1f61-250a326f8169-5C7GfCeVMHo@public.gmane.org

author
#8 lore raw

RE: [PATCH 2/4] drm/amdgpu: update the vm invalidation engine layout

Addressed this in the new patch which was just sent out.

Thanks,
Evan

Quoted 130 lines

-----Original Message-----
From: Kuehling, Felix
Sent: 2018年12月14日 1:09
To: [email protected]; Quan, Evan <[email protected]>; Deucher, Alexander <[email protected]>; Zeng, Oak <[email protected]>; Koenig, Christian <[email protected]>
Subject: Re: [PATCH 2/4] drm/amdgpu: update the vm invalidation engine layout

Some nit-picks inline.

Quoted 8 lines · Evan Quan

We need new invalidation engine layout due to new SDMA page queues added.

Change-Id I2f3861689bffb9828c9eae744a7a0de4963ac2b6
Signed-off-by Evan Quan <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 47

++++++++++++++------------- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h

|

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool

Quoted 9 lines

10 ++++++

 2 files changed, 35 insertions(+), 22 deletions(-)
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +0 -0
index dc4dadc3575c..092a4d111557 100644
@@ -719,37 +719,40 @@ static bool

gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev)

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev) · line 722

Quoted 5 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +1 -1
 	}
 }
 
-static int gmc_v9_0_late_init(void *handle)
+static int gmc_v9_0_allocate_vm_inv_eng(struct amdgpu_device *adev)

The function returns int. But only ever returns 0 and the caller ignores the return value.

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev)

Quoted 7 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +0 -6
 {
-	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-	/*
-	 * The latest engine allocation on gfx9 is:
-	 * Engine 0, 1: idle
-	 * Engine 2, 3: firmware
-	 * Engine 4~13: amdgpu ring, subject to change when ring number

changes

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev)

Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +3 -5
-	 * Engine 14~15: idle
-	 * Engine 16: kfd tlb invalidation
-	 * Engine 17: Gart flushes
-	 */
-	unsigned vm_inv_eng[AMDGPU_MAX_VMHUBS] = { 4, 4 };
+	struct amdgpu_ring *ring;
+	unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS] =
+		{GFXHUB_FREE_VM_INV_ENGS_BITMAP,

MMHUB_FREE_VM_INV_ENGS_BITMAP};

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev) · line 735

Quoted 12 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +7 -3
 	unsigned i;
-	int r;
+	unsigned vmhub, inv_eng;
 
-	if (!gmc_v9_0_keep_stolen_memory(adev))
-		amdgpu_bo_late_init(adev);
+	for (i = 0; i < adev->num_rings; ++i) {
+		ring = adev->rings[i];
+		vmhub = ring->funcs->vmhub;
+
+		inv_eng = ffs(vm_inv_engs[vmhub]);
+		BUG_ON(!inv_eng);

Adding new BUG_ONs is discouraged. checkpatch.pl always warns about these. Errors that can be handled more gracefully shouldn't use a BUG_ON. E.g. use a WARN_ON and return an error code.

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev)

Quoted 6 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +2 -3
-	for(i = 0; i < adev->num_rings; ++i) {
-		struct amdgpu_ring *ring = adev->rings[i];
-		unsigned vmhub = ring->funcs->vmhub;
+		ring->vm_inv_eng = inv_eng - 1;
+		change_bit(inv_eng - 1, (unsigned long

*)(&vm_inv_engs[vmhub]));

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool · line 738

Quoted 3 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +0 -1
-		ring->vm_inv_eng = vm_inv_eng[vmhub]++;
 		dev_info(adev->dev, "ring %s uses VM inv eng %u on

hub %u\n",

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev) · line 744

Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +1 -3
 			 ring->name, ring->vm_inv_eng, ring->funcs-
 vmhub);
 	}
 
-	/* Engine 16 is used for KFD and 17 for GART flushes */
-	for(i = 0; i < AMDGPU_MAX_VMHUBS; ++i)
-		BUG_ON(vm_inv_eng[i] > 16);
+	return 0;

This is the only return statement. Maybe this could be a void function. Unless you decide to turn the BUG_ON into a WARN with an error return.

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c static bool gmc_v9_0_keep_stolen_memory(struct amdgpu_device *adev) · line 755

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +10 -0
+}
+
+static int gmc_v9_0_late_init(void *handle) {
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	if (!gmc_v9_0_keep_stolen_memory(adev))
+		amdgpu_bo_late_init(adev);
+
+	gmc_v9_0_allocate_vm_inv_eng(adev);

You're ignoring the return value. Either, make the function void, or handle potential error returns.

Regards,
  Felix

Quoted 12 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +0 -0
 	if (adev->asic_type == CHIP_VEGA10 && !amdgpu_sriov_vf(adev)) {
 		r = gmc_v9_0_ecc_available(adev);
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h +0 -0
index b030ca5ea107..5c8deac65580 100644
@@ -24,6 +24,16 @@
 #ifndef __GMC_V9_0_H__
 #define __GMC_V9_0_H__
Quoted 13 lines

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h line 39

Quoted 13 lines
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h +10 -0
+	/*
+	 * The latest engine allocation on gfx9 is:
+	 * Engine 2, 3: firmware
+	 * Engine 0, 1, 4~16: amdgpu ring,
+	 *                    subject to change when ring number changes
+	 * Engine 17: Gart flushes
+	 */
+#define GFXHUB_FREE_VM_INV_ENGS_BITMAP		0x1FFF3
+#define MMHUB_FREE_VM_INV_ENGS_BITMAP		0x1FFF3
+
 extern const struct amd_ip_funcs gmc_v9_0_ip_funcs;  extern const
 struct amdgpu_ip_block_version gmc_v9_0_ip_block;

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

author
#9 lore raw

PATCH 3/4 drm/amdgpu: use different irq ring ID for Vega20 page queues

In reply to a message not in the archive

Vega20 uses ring id 1 for page queues EOP irq while previous ASICs take ring id 3.

Change-Id Id837fa934ab4a60f0360a33216413a8fc9987c56
Signed-off-by Evan Quan <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +4 -2
index 4b6d3e5c821f..d59addb47025 100644
@@ -1706,13 +1706,15 @@ static int sdma_v4_0_process_trap_irq(struct amdgpu_device *adev,
 		amdgpu_fence_process(&adev->sdma.instance[instance].ring);
 		break;
 	case 1:
-		/* XXX compute */
+		if (adev->asic_type == CHIP_VEGA20)
+			amdgpu_fence_process(&adev->sdma.instance[instance].page);
 		break;
 	case 2:
 		/* XXX compute */
 		break;
 	case 3:
-		amdgpu_fence_process(&adev->sdma.instance[instance].page);
+		if (adev->asic_type != CHIP_VEGA20)
+			amdgpu_fence_process(&adev->sdma.instance[instance].page);
 		break;
 	}
 	return 0;
Signature · 6 lines
2.19.2

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
author
#10 lore raw

PATCH 4/4 drm/amdgpu: enable Vega20 page queue support

In reply to a message not in the archive

Page queue is supported on Vega20 with SDMA firmware 123 onwards.

Change-Id I78d2c4a7ad9d200d89177fae2ad6073955e6bd8b
Signed-off-by Evan Quan <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +1 -2
index d59addb47025..fd0bfe140ee0 100644
@@ -1458,8 +1458,7 @@ static bool sdma_v4_0_fw_support_paging_queue(struct amdgpu_device *adev)
 		/*return fw_version >= 31;*/
 		return false;
 	case CHIP_VEGA20:
-		/*return fw_version >= 115;*/
-		return false;
+		return fw_version >= 123;
 	default:
 		return false;
 	}
Signature · 6 lines
2.19.2

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Reviewed-by Christian König
#11 lore raw
In reply to a message not in the archive

Am 13.12.18 um 05:09 schrieb Evan Quan:

Quoted 4 lines

As two more SDMA page queue rings are added on Vega20.

Change-Id I8a3d8fbc924f7c24aaebf17b3f329a4a38fe5a56
Signed-off-by Evan Quan <[email protected]>
Reviewed-by Christian König <[email protected]> for this one.

And Acked-by: Christian König <[email protected]> for patch #3 and #4.

Regards,
Christian.

Quoted 9 lines
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h +0 -0
index 4b31c96a2a2c..fe657788024f 100644

drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h line 37

Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h +1 -1
@@ -31,7 +31,7 @@
  #endif
 
  /* max number of rings */
-#define AMDGPU_MAX_RINGS		21
+#define AMDGPU_MAX_RINGS		23
  #define AMDGPU_MAX_GFX_RINGS		1
  #define AMDGPU_MAX_COMPUTE_RINGS	8
  #define AMDGPU_MAX_VCE_RINGS		3

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

Reviewed-by Oak Zeng
#12 lore raw
In reply to a message not in the archive
Reviewed-by Oak Zeng <[email protected]>

Regards,
Oak

-----Original Message-----
From: amd-gfx <[email protected]> On Behalf Of Evan Quan
Sent: Wednesday, December 12, 2018 11:10 PM
To: [email protected]
Cc: Deucher, Alexander <[email protected]>; Zeng, Oak <[email protected]>; Quan, Evan <[email protected]>; Koenig, Christian <[email protected]>
Subject: [PATCH 1/4] drm/amdgpu: increase the MAX ring number

As two more SDMA page queue rings are added on Vega20.

Change-Id I8a3d8fbc924f7c24aaebf17b3f329a4a38fe5a56
Signed-off-by Evan Quan <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h +1 -1
index 4b31c96a2a2c..fe657788024f 100644
@@ -31,7 +31,7 @@
 #endif
 
 /* max number of rings */
-#define AMDGPU_MAX_RINGS		21
+#define AMDGPU_MAX_RINGS		23
 #define AMDGPU_MAX_GFX_RINGS		1
 #define AMDGPU_MAX_COMPUTE_RINGS	8
 #define AMDGPU_MAX_VCE_RINGS		3
Signature · 10 lines
2.19.2

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