Skip to contentSkip to conversation

drm/amdgpu: fix typo in function sdma_v4_0_page_resume

PATCH In review Yang, Philip 3 participants 8 messages Nov 19, 2018 – Nov 20, 2018 5 messages not in the archive

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

Yang, Philip

author
#0 lore raw

PATCH 1/3 drm/amdgpu: fix typo in function sdma_v4_0_page_resume

This looks like copy paste typo

Change-Id Iee3fd3a551650ec9199bc030a7886e92000b02e7
Signed-off-by Philip Yang <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +2 -4
index f4490cdd9804..3f6b7882dbd2 100644
@@ -925,11 +925,9 @@ static void sdma_v4_0_page_resume(struct amdgpu_device *adev, unsigned int i)
 					OFFSET, ring->doorbell_index);
 	WREG32_SDMA(i, mmSDMA0_PAGE_DOORBELL, doorbell);
 	WREG32_SDMA(i, mmSDMA0_PAGE_DOORBELL_OFFSET, doorbell_offset);
-	/* TODO: enable doorbell support */
-	/*adev->nbio_funcs->sdma_doorbell_range(adev, i, ring->use_doorbell,
-					      ring->doorbell_index);*/
 
-	sdma_v4_0_ring_set_wptr(ring);
+	/* paging queue doorbell range is setup at sdma_v4_0_gfx_resume */
+	sdma_v4_0_page_ring_set_wptr(ring);
 
 	/* set minor_ptr_update to 0 after wptr programed */
 	WREG32_SDMA(i, mmSDMA0_PAGE_MINOR_PTR_UPDATE, 0);
Signature · 6 lines
2.17.1

_______________________________________________
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/3 drm/amdgpu: enable paging queue doorbell support v4

Because increase SDMA_DOORBELL_RANGE to add new SDMA doorbell for paging queue will break SRIOV, instead we can reserve and map two doorbell pages for amdgpu, paging queues doorbell index use same index as SDMA gfx queues index but on second page.

For Vega20, after we change doorbell layout to increase SDMA doorbell for 8 SDMA RLC queues later, we could use new doorbell index for paging queue.

Change-Id I9adb965f16ee4089d261d9a22231337739184e49
Signed-off-by Philip Yang <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  6 +++++
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c     | 28 +++++++++++++++-------
 2 files changed, 25 insertions(+), 9 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +6 -0
index 590588a82471..cb06e6883fad 100644
@@ -534,6 +534,12 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
 	if (adev->doorbell.num_doorbells == 0)
 		return -EINVAL;
 
+	/* For Vega, reserve and map two pages on doorbell BAR since SDMA
+	 * paging queue doorbell use the second page
+	 */
+	if (adev->asic_type >= CHIP_VEGA10)
+		adev->doorbell.num_doorbells *= 2;
+
 	adev->doorbell.ptr = ioremap(adev->doorbell.base,
 				     adev->doorbell.num_doorbells *
 				     sizeof(u32));
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +19 -9
index 3f6b7882dbd2..4d873fd3242c 100644
@@ -1502,18 +1502,15 @@ static int sdma_v4_0_sw_init(void *handle)
 		ring->ring_obj = NULL;
 		ring->use_doorbell = true;
 
-		DRM_INFO("use_doorbell being set to: [%s]\n",
-				ring->use_doorbell?"true":"false");
-
+		/* doorbell size is 2 dwords, get DWORD offset */
 		if (adev->asic_type == CHIP_VEGA10)
 			ring->doorbell_index = (i == 0) ?
-				(AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset
-				: (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 << 1); // get DWORD offset
+				(AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1)
+				: (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 << 1);
 		else
 			ring->doorbell_index = (i == 0) ?
-				(AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset
-				: (AMDGPU_DOORBELL64_sDMA_ENGINE1 << 1); // get DWORD offset
-
+				(AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1)
+				: (AMDGPU_DOORBELL64_sDMA_ENGINE1 << 1);
 
 		sprintf(ring->name, "sdma%d", i);
 		r = amdgpu_ring_init(adev, ring, 1024,

@@ -1527,7 +1524,20 @@ static int sdma_v4_0_sw_init(void *handle)
 		if (adev->sdma.has_page_queue) {
 			ring = &adev->sdma.instance[i].page;
 			ring->ring_obj = NULL;
-			ring->use_doorbell = false;
+			ring->use_doorbell = true;
+
+			/* paging queue use same doorbell index/routing as gfx queue
+			 * with 0x400 (4096 dwords) offset on second doorbell page
+			 */
+			if (adev->asic_type == CHIP_VEGA10)
+				ring->doorbell_index = (i == 0) ?
+					(AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1)
+					: (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 << 1);
+			else
+				ring->doorbell_index = (i == 0) ?
+					(AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1)
+					: (AMDGPU_DOORBELL64_sDMA_ENGINE1 << 1);
+			ring->doorbell_index += 0x400;
 
 			sprintf(ring->name, "page%d", i);
 			r = amdgpu_ring_init(adev, ring, 1024,
Signature · 6 lines
2.17.1

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

Reviewed-by
#4 lore raw

Re: [PATCH 2/3] drm/amdgpu: enable paging queue doorbell support v4

Reviewed-by Alex Deucher <[email protected]>

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Yang, Philip <[email protected]>
Sent: Monday, November 19, 2018 3:20:07 PM
To: [email protected]
Cc: Yang, Philip
Subject: [PATCH 2/3] drm/amdgpu: enable paging queue doorbell support v4

Because increase SDMA_DOORBELL_RANGE to add new SDMA doorbell for paging queue will break SRIOV, instead we can reserve and map two doorbell pages for amdgpu, paging queues doorbell index use same index as SDMA gfx queues index but on second page.

For Vega20, after we change doorbell layout to increase SDMA doorbell for 8 SDMA RLC queues later, we could use new doorbell index for paging queue.

Change-Id I9adb965f16ee4089d261d9a22231337739184e49
Signed-off-by Philip Yang <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  6 +++++
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c     | 28 +++++++++++++++-------
 2 files changed, 25 insertions(+), 9 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +6 -0
index 590588a82471..cb06e6883fad 100644
@@ -534,6 +534,12 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
         if (adev->doorbell.num_doorbells == 0)
                 return -EINVAL;
 
+       /* For Vega, reserve and map two pages on doorbell BAR since SDMA
+        * paging queue doorbell use the second page
+        */
+       if (adev->asic_type >= CHIP_VEGA10)
+               adev->doorbell.num_doorbells *= 2;
+
         adev->doorbell.ptr = ioremap(adev->doorbell.base,
                                      adev->doorbell.num_doorbells *
                                      sizeof(u32));
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +19 -9
index 3f6b7882dbd2..4d873fd3242c 100644
@@ -1502,18 +1502,15 @@ static int sdma_v4_0_sw_init(void *handle)
                 ring->ring_obj = NULL;
                 ring->use_doorbell = true;
 
-               DRM_INFO("use_doorbell being set to: [%s]\n",
-                               ring->use_doorbell?"true":"false");
-
+               /* doorbell size is 2 dwords, get DWORD offset */
                 if (adev->asic_type == CHIP_VEGA10)
                         ring->doorbell_index = (i == 0) ?
-                               (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset
-                               : (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 << 1); // get DWORD offset
+                               (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1)
+                               : (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 << 1);
                 else
                         ring->doorbell_index = (i == 0) ?
-                               (AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset
-                               : (AMDGPU_DOORBELL64_sDMA_ENGINE1 << 1); // get DWORD offset
-
+                               (AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1)
+                               : (AMDGPU_DOORBELL64_sDMA_ENGINE1 << 1);
 
                 sprintf(ring->name, "sdma%d", i);
                 r = amdgpu_ring_init(adev, ring, 1024,

@@ -1527,7 +1524,20 @@ static int sdma_v4_0_sw_init(void *handle)
                 if (adev->sdma.has_page_queue) {
                         ring = &adev->sdma.instance[i].page;
                         ring->ring_obj = NULL;
-                       ring->use_doorbell = false;
+                       ring->use_doorbell = true;
+
+                       /* paging queue use same doorbell index/routing as gfx queue
+                        * with 0x400 (4096 dwords) offset on second doorbell page
+                        */
+                       if (adev->asic_type == CHIP_VEGA10)
+                               ring->doorbell_index = (i == 0) ?
+                                       (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1)
+                                       : (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 << 1);
+                       else
+                               ring->doorbell_index = (i == 0) ?
+                                       (AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1)
+                                       : (AMDGPU_DOORBELL64_sDMA_ENGINE1 << 1);
+                       ring->doorbell_index += 0x400;
 
                         sprintf(ring->name, "page%d", i);
                         r = amdgpu_ring_init(adev, ring, 1024,

--
2.17.1

_______________________________________________
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

author
#5 lore raw

PATCH 3/3 drm/amdgpu: enable paging queue based on FW version

In reply to a message not in the archive

Based SDMA fw version to enable has_page_queue support. Have to move sdma_v4_0_init_microcode from sw_init to early_init, to load firmware and init fw_version before set_ring/buffer/vm_pte_funcs use it.

Change-Id Ife5d4659d28bc2a7012b48947b27e929749d87c1
Signed-off-by Philip Yang <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 46 +++++++++++++++++---------
 1 file changed, 30 insertions(+), 16 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +30 -16
index 4d873fd3242c..0a3b68dd49a0 100644
@@ -1447,23 +1447,44 @@ static void sdma_v4_0_ring_emit_reg_wait(struct amdgpu_ring *ring, uint32_t reg,
 	sdma_v4_0_wait_reg_mem(ring, 0, 0, reg, 0, val, mask, 10);
 }
 
+static bool sdma_v4_0_fw_support_paging_queue(struct amdgpu_device *adev)
+{
+	uint fw_version = adev->sdma.instance[0].fw_version;
+
+	switch (adev->asic_type) {
+	case CHIP_VEGA10:
+		return fw_version >= 430;
+	case CHIP_VEGA12:
+		return fw_version >= 31;
+	case CHIP_VEGA20:
+		return fw_version >= 115;
+	default:
+		return false;
+	}
+}
+
 static int sdma_v4_0_early_init(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
 
-	if (adev->asic_type == CHIP_RAVEN) {
+	if (adev->asic_type == CHIP_RAVEN)
 		adev->sdma.num_instances = 1;
-		adev->sdma.has_page_queue = false;
-	} else {
+	else
 		adev->sdma.num_instances = 2;
-		/* TODO: Page queue breaks driver reload under SRIOV */
-		if ((adev->asic_type == CHIP_VEGA10) && amdgpu_sriov_vf((adev)))
-			adev->sdma.has_page_queue = false;
-		else if (adev->asic_type != CHIP_VEGA20 &&
-				adev->asic_type != CHIP_VEGA12)
-			adev->sdma.has_page_queue = true;
+
+	r = sdma_v4_0_init_microcode(adev);
+	if (r) {
+		DRM_ERROR("Failed to load sdma firmware!\n");
+		return r;
 	}
 
+	/* TODO: Page queue breaks driver reload under SRIOV */
+	if ((adev->asic_type == CHIP_VEGA10) && amdgpu_sriov_vf((adev)))
+		adev->sdma.has_page_queue = false;
+	else if (sdma_v4_0_fw_support_paging_queue(adev))
+		adev->sdma.has_page_queue = true;
+
 	sdma_v4_0_set_ring_funcs(adev);
 	sdma_v4_0_set_buffer_funcs(adev);
 	sdma_v4_0_set_vm_pte_funcs(adev);

@@ -1472,7 +1493,6 @@ static int sdma_v4_0_early_init(void *handle)
 	return 0;
 }
 
-
 static int sdma_v4_0_sw_init(void *handle)
 {
 	struct amdgpu_ring *ring;

@@ -1491,12 +1511,6 @@ static int sdma_v4_0_sw_init(void *handle)
 	if (r)
 		return r;
 
-	r = sdma_v4_0_init_microcode(adev);
-	if (r) {
-		DRM_ERROR("Failed to load sdma firmware!\n");
-		return r;
-	}
-
 	for (i = 0; i < adev->sdma.num_instances; i++) {
 		ring = &adev->sdma.instance[i].ring;
 		ring->ring_obj = NULL;
Signature · 6 lines
2.17.1

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

Acked-by Alex Deucher
#7 lore raw

RE: [PATCH 3/3] drm/amdgpu: enable paging queue based on FW version

Quoted 58 lines

-----Original Message-----
From: amd-gfx <[email protected]> On Behalf Of Yang, Philip
Sent: Monday, November 19, 2018 3:20 PM
To: [email protected]
Cc: Yang, Philip <[email protected]>
Subject: [PATCH 3/3] drm/amdgpu: enable paging queue based on FW version

Based SDMA fw version to enable has_page_queue support. Have to move sdma_v4_0_init_microcode from sw_init to early_init, to load firmware and init fw_version before set_ring/buffer/vm_pte_funcs use it.

Change-Id Ife5d4659d28bc2a7012b48947b27e929749d87c1
Signed-off-by Philip Yang <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 46 +++++++++++++++++----
-----
 1 file changed, 30 insertions(+), 16 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +19 -5
index 4d873fd3242c..0a3b68dd49a0 100644
@@ -1447,23 +1447,44 @@ static void
 sdma_v4_0_ring_emit_reg_wait(struct amdgpu_ring *ring, uint32_t reg,
 	sdma_v4_0_wait_reg_mem(ring, 0, 0, reg, 0, val, mask, 10);  }
 
+static bool sdma_v4_0_fw_support_paging_queue(struct amdgpu_device
+*adev) {
+	uint fw_version = adev->sdma.instance[0].fw_version;
+
+	switch (adev->asic_type) {
+	case CHIP_VEGA10:
+		return fw_version >= 430;
+	case CHIP_VEGA12:
+		return fw_version >= 31;
+	case CHIP_VEGA20:
+		return fw_version >= 115;
+	default:
+		return false;
+	}
+}
+
 static int sdma_v4_0_early_init(void *handle)  {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
 
-	if (adev->asic_type == CHIP_RAVEN) {
+	if (adev->asic_type == CHIP_RAVEN)
 		adev->sdma.num_instances = 1;
-		adev->sdma.has_page_queue = false;
-	} else {
+	else
 		adev->sdma.num_instances = 2;
-		/* TODO: Page queue breaks driver reload under SRIOV */
-		if ((adev->asic_type == CHIP_VEGA10) &&

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c static void · line 1479

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +5 -4
 amdgpu_sriov_vf((adev)))
-			adev->sdma.has_page_queue = false;
-		else if (adev->asic_type != CHIP_VEGA20 &&
-				adev->asic_type != CHIP_VEGA12)
-			adev->sdma.has_page_queue = true;
+
+	r = sdma_v4_0_init_microcode(adev);
+	if (r) {
+		DRM_ERROR("Failed to load sdma firmware!\n");
+		return r;

I think this should be ok. As long as you've verified that sdam_v4_0_init_microcode() doesn't depend on any other init from another module like psp. I took a quick look at the code and it seems like we should be ok.

Acked-by Alex Deucher <[email protected]>
Quoted 35 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +6 -0
 	}
 
+	/* TODO: Page queue breaks driver reload under SRIOV */
+	if ((adev->asic_type == CHIP_VEGA10) &&
 amdgpu_sriov_vf((adev)))
+		adev->sdma.has_page_queue = false;
+	else if (sdma_v4_0_fw_support_paging_queue(adev))
+		adev->sdma.has_page_queue = true;
+
 	sdma_v4_0_set_ring_funcs(adev);
 	sdma_v4_0_set_buffer_funcs(adev);
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +0 -8
 	sdma_v4_0_set_vm_pte_funcs(adev);

@@ -1472,7 +1493,6 @@ static int sdma_v4_0_early_init(void *handle)
 	return 0;
 }
 
-
 static int sdma_v4_0_sw_init(void *handle)  {
 	struct amdgpu_ring *ring;

@@ -1491,12 +1511,6 @@ static int sdma_v4_0_sw_init(void *handle)
 	if (r)
 		return r;
 
-	r = sdma_v4_0_init_microcode(adev);
-	if (r) {
-		DRM_ERROR("Failed to load sdma firmware!\n");
-		return r;
-	}
-
 	for (i = 0; i < adev->sdma.num_instances; i++) {
 		ring = &adev->sdma.instance[i].ring;
 		ring->ring_obj = NULL;
--

2.17.1

Quoted 4 lines

_______________________________________________
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

A message that is not in the archive (sent off-list or not yet received): BN6PR12MB18091E4BBC1BB24609AF383EF7D80-/b2+HYfkarSEx6ez0IUAagdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org

author
#9 lore raw

Re: [PATCH 3/3] drm/amdgpu: enable paging queue based on FW version

Quoted 69 lines · Alexander
Quoted 58 lines

-----Original Message-----
From: amd-gfx <[email protected]> On Behalf Of Yang, Philip
Sent: Monday, November 19, 2018 3:20 PM
To: [email protected]
Cc: Yang, Philip <[email protected]>
Subject: [PATCH 3/3] drm/amdgpu: enable paging queue based on FW version

Based SDMA fw version to enable has_page_queue support. Have to move sdma_v4_0_init_microcode from sw_init to early_init, to load firmware and init fw_version before set_ring/buffer/vm_pte_funcs use it.

Change-Id Ife5d4659d28bc2a7012b48947b27e929749d87c1
Signed-off-by Philip Yang <[email protected]>
---
  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 46 +++++++++++++++++----
-----
  1 file changed, 30 insertions(+), 16 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +19 -5
index 4d873fd3242c..0a3b68dd49a0 100644
@@ -1447,23 +1447,44 @@ static void
 sdma_v4_0_ring_emit_reg_wait(struct amdgpu_ring *ring, uint32_t reg,
  	sdma_v4_0_wait_reg_mem(ring, 0, 0, reg, 0, val, mask, 10);  }
 
+static bool sdma_v4_0_fw_support_paging_queue(struct amdgpu_device
+*adev) {
+	uint fw_version = adev->sdma.instance[0].fw_version;
+
+	switch (adev->asic_type) {
+	case CHIP_VEGA10:
+		return fw_version >= 430;
+	case CHIP_VEGA12:
+		return fw_version >= 31;
+	case CHIP_VEGA20:
+		return fw_version >= 115;
+	default:
+		return false;
+	}
+}
+
  static int sdma_v4_0_early_init(void *handle)  {
  	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
 
-	if (adev->asic_type == CHIP_RAVEN) {
+	if (adev->asic_type == CHIP_RAVEN)
  		adev->sdma.num_instances = 1;
-		adev->sdma.has_page_queue = false;
-	} else {
+	else
  		adev->sdma.num_instances = 2;
-		/* TODO: Page queue breaks driver reload under SRIOV */
-		if ((adev->asic_type == CHIP_VEGA10) &&

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c static void · line 1479

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +5 -4
 amdgpu_sriov_vf((adev)))
-			adev->sdma.has_page_queue = false;
-		else if (adev->asic_type != CHIP_VEGA20 &&
-				adev->asic_type != CHIP_VEGA12)
-			adev->sdma.has_page_queue = true;
+
+	r = sdma_v4_0_init_microcode(adev);
+	if (r) {
+		DRM_ERROR("Failed to load sdma firmware!\n");
+		return r;

I think this should be ok. As long as you've verified that sdam_v4_0_init_microcode() doesn't depend on any other init from another module like psp. I took a quick look at the code and it seems like we should be ok.

Quoted 1 line
Acked-by Alex Deucher <[email protected]>

sdma_v4_0_init_microcode() reads fw binary file and setup fw version etc data structure. It doesn't depend on other init so it is fine to move it from sw_init() to early_init(). sdma_v4_0_load_microcode() will start fw from sdma_v4_0_start() which depends on psp, gmc etc. This happens after early_init()->sw_init()->hw_init().

Philip

Quoted 36 lines
Quoted 36 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +6 -0
  	}
 
+	/* TODO: Page queue breaks driver reload under SRIOV */
+	if ((adev->asic_type == CHIP_VEGA10) &&
 amdgpu_sriov_vf((adev)))
+		adev->sdma.has_page_queue = false;
+	else if (sdma_v4_0_fw_support_paging_queue(adev))
+		adev->sdma.has_page_queue = true;
+
  	sdma_v4_0_set_ring_funcs(adev);
  	sdma_v4_0_set_buffer_funcs(adev);

  	sdma_v4_0_set_vm_pte_funcs(adev);
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +0 -8
@@ -1472,7 +1493,6 @@ static int sdma_v4_0_early_init(void *handle)
  	return 0;
  }
 
-
  static int sdma_v4_0_sw_init(void *handle)  {
  	struct amdgpu_ring *ring;

@@ -1491,12 +1511,6 @@ static int sdma_v4_0_sw_init(void *handle)
  	if (r)
  		return r;
 
-	r = sdma_v4_0_init_microcode(adev);
-	if (r) {
-		DRM_ERROR("Failed to load sdma firmware!\n");
-		return r;
-	}
-
  	for (i = 0; i < adev->sdma.num_instances; i++) {
  		ring = &adev->sdma.instance[i].ring;
  		ring->ring_obj = NULL;
--

2.17.1

Quoted 4 lines
Quoted 4 lines

_______________________________________________
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

A message that is not in the archive (sent off-list or not yet received): f9918fbe-7cab-e8ce-0ecb-a53d1ba5f0c0-5C7GfCeVMHo@public.gmane.org

#11 lore raw

Re: [PATCH 3/3] drm/amdgpu: enable paging queue based on FW version

Am 19.11.18 um 22:16 schrieb Yang, Philip:

Quoted 71 lines · Alexander
Quoted 69 lines
Quoted 58 lines

-----Original Message-----
From: amd-gfx <[email protected]> On Behalf Of Yang, Philip
Sent: Monday, November 19, 2018 3:20 PM
To: [email protected]
Cc: Yang, Philip <[email protected]>
Subject: [PATCH 3/3] drm/amdgpu: enable paging queue based on FW version

Based SDMA fw version to enable has_page_queue support. Have to move sdma_v4_0_init_microcode from sw_init to early_init, to load firmware and init fw_version before set_ring/buffer/vm_pte_funcs use it.

Change-Id Ife5d4659d28bc2a7012b48947b27e929749d87c1
Signed-off-by Philip Yang <[email protected]>
---
   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 46 +++++++++++++++++----
-----
   1 file changed, 30 insertions(+), 16 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +19 -5
index 4d873fd3242c..0a3b68dd49a0 100644
@@ -1447,23 +1447,44 @@ static void
 sdma_v4_0_ring_emit_reg_wait(struct amdgpu_ring *ring, uint32_t reg,
   	sdma_v4_0_wait_reg_mem(ring, 0, 0, reg, 0, val, mask, 10);  }
 
+static bool sdma_v4_0_fw_support_paging_queue(struct amdgpu_device
+*adev) {
+	uint fw_version = adev->sdma.instance[0].fw_version;
+
+	switch (adev->asic_type) {
+	case CHIP_VEGA10:
+		return fw_version >= 430;
+	case CHIP_VEGA12:
+		return fw_version >= 31;
+	case CHIP_VEGA20:
+		return fw_version >= 115;
+	default:
+		return false;
+	}
+}
+
   static int sdma_v4_0_early_init(void *handle)  {
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
 
-	if (adev->asic_type == CHIP_RAVEN) {
+	if (adev->asic_type == CHIP_RAVEN)
   		adev->sdma.num_instances = 1;
-		adev->sdma.has_page_queue = false;
-	} else {
+	else
   		adev->sdma.num_instances = 2;
-		/* TODO: Page queue breaks driver reload under SRIOV */
-		if ((adev->asic_type == CHIP_VEGA10) &&

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c static void · line 1479

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +5 -4
 amdgpu_sriov_vf((adev)))
-			adev->sdma.has_page_queue = false;
-		else if (adev->asic_type != CHIP_VEGA20 &&
-				adev->asic_type != CHIP_VEGA12)
-			adev->sdma.has_page_queue = true;
+
+	r = sdma_v4_0_init_microcode(adev);
+	if (r) {
+		DRM_ERROR("Failed to load sdma firmware!\n");
+		return r;

I think this should be ok. As long as you've verified that sdam_v4_0_init_microcode() doesn't depend on any other init from another module like psp. I took a quick look at the code and it seems like we should be ok.

Quoted 1 line
Acked-by Alex Deucher <[email protected]>
Quoted 5 lines

sdma_v4_0_init_microcode() reads fw binary file and setup fw version etc data structure. It doesn't depend on other init so it is fine to move it from sw_init() to early_init(). sdma_v4_0_load_microcode() will start fw from sdma_v4_0_start() which depends on psp, gmc etc. This happens after early_init()->sw_init()->hw_init().

Yeah, that sounds like it should work.

Patches #1 and #2 are Reviewed-by: Christian König <[email protected]>

Patch #3 is Acked-by: Christian König <[email protected]>

Thanks for taking care of that,
Christian.

Quoted 37 lines

Philip

Quoted 35 lines
Quoted 35 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +6 -0
   	}
 
+	/* TODO: Page queue breaks driver reload under SRIOV */
+	if ((adev->asic_type == CHIP_VEGA10) &&
 amdgpu_sriov_vf((adev)))
+		adev->sdma.has_page_queue = false;
+	else if (sdma_v4_0_fw_support_paging_queue(adev))
+		adev->sdma.has_page_queue = true;
+
   	sdma_v4_0_set_ring_funcs(adev);
   	sdma_v4_0_set_buffer_funcs(adev);

   	sdma_v4_0_set_vm_pte_funcs(adev);
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +0 -8
@@ -1472,7 +1493,6 @@ static int sdma_v4_0_early_init(void *handle)
   	return 0;
   }
 
-
   static int sdma_v4_0_sw_init(void *handle)  {
   	struct amdgpu_ring *ring;

@@ -1491,12 +1511,6 @@ static int sdma_v4_0_sw_init(void *handle)
   	if (r)
   		return r;
 
-	r = sdma_v4_0_init_microcode(adev);
-	if (r) {
-		DRM_ERROR("Failed to load sdma firmware!\n");
-		return r;
-	}
-
   	for (i = 0; i < adev->sdma.num_instances; i++) {
   		ring = &adev->sdma.instance[i].ring;
   		ring->ring_obj = NULL;
--

2.17.1

Quoted 8 lines
Quoted 4 lines
Quoted 4 lines

_______________________________________________
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

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

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

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Yang, Philip <[email protected]>
Sent: Monday, November 19, 2018 3:20:00 PM
To: [email protected]
Cc: Yang, Philip
Subject: [PATCH 1/3] drm/amdgpu: fix typo in function sdma_v4_0_page_resume

This looks like copy paste typo

Change-Id Iee3fd3a551650ec9199bc030a7886e92000b02e7
Signed-off-by Philip Yang <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +2 -4
index f4490cdd9804..3f6b7882dbd2 100644
@@ -925,11 +925,9 @@ static void sdma_v4_0_page_resume(struct amdgpu_device *adev, unsigned int i)
                                         OFFSET, ring->doorbell_index);
         WREG32_SDMA(i, mmSDMA0_PAGE_DOORBELL, doorbell);
         WREG32_SDMA(i, mmSDMA0_PAGE_DOORBELL_OFFSET, doorbell_offset);
-       /* TODO: enable doorbell support */
-       /*adev->nbio_funcs->sdma_doorbell_range(adev, i, ring->use_doorbell,
-                                             ring->doorbell_index);*/
 
-       sdma_v4_0_ring_set_wptr(ring);
+       /* paging queue doorbell range is setup at sdma_v4_0_gfx_resume */
+       sdma_v4_0_page_ring_set_wptr(ring);
 
         /* set minor_ptr_update to 0 after wptr programed */
         WREG32_SDMA(i, mmSDMA0_PAGE_MINOR_PTR_UPDATE, 0);

--
2.17.1

_______________________________________________
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