Skip to contentSkip to conversation

drm/amdgpu: enable paging queue doorbell support

PATCH In review Yang, Philip 3 participants 5 messages Nov 15, 2018 4 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/1 drm/amdgpu: enable paging queue doorbell support

paging queues doorbell index use existing assignment sDMA_HI_PRI_ENGINE0/1 index, and increase SDMA_DOORBELL_RANGE size from 2 dwords to 4 dwords to enable the new doorbell index.

Change-Id I9adb965f16ee4089d261d9a22231337739184e49
Signed-off-by Philip Yang <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 25 +++++++++++++++++--------
 3 files changed, 19 insertions(+), 10 deletions(-)
drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c +1 -1
index 6f9c54978cc1..0eb42c29ecac 100644
@@ -80,7 +80,7 @@ static void nbio_v6_1_sdma_doorbell_range(struct amdgpu_device *adev, int instan
 
 	if (use_doorbell) {
 		doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, OFFSET, doorbell_index);
-		doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 2);
+		doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 4);
 	} else
 		doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 0);
drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c +1 -1
index f8cee95d61cc..9342ee03d7d4 100644
@@ -76,7 +76,7 @@ static void nbio_v7_4_sdma_doorbell_range(struct amdgpu_device *adev, int instan
 
 	if (use_doorbell) {
 		doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, OFFSET, doorbell_index);
-		doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 2);
+		doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 4);
 	} else
 		doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 0);
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +17 -8
index f4490cdd9804..96c9e83204b7 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 index is already enabled 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);

@@ -1504,9 +1502,6 @@ 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");
-
 		if (adev->asic_type == CHIP_VEGA10)
 			ring->doorbell_index = (i == 0) ?
 				(AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset

@@ -1516,6 +1511,8 @@ static int sdma_v4_0_sw_init(void *handle)
 				(AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset
 				: (AMDGPU_DOORBELL64_sDMA_ENGINE1 << 1); // get DWORD offset
 
+		DRM_INFO("use_doorbell being set to: [%s] doorbell index %d\n",
+			ring->use_doorbell?"true":"false", ring->doorbell_index);
 
 		sprintf(ring->name, "sdma%d", i);
 		r = amdgpu_ring_init(adev, ring, 1024,

@@ -1529,7 +1526,19 @@ 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;
+
+			if (adev->asic_type == CHIP_VEGA10)
+				ring->doorbell_index = (i == 0) ?
+					(AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 << 1)
+					: (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 << 1);
+			else
+				ring->doorbell_index = (i == 0) ?
+					(AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 << 1)
+					: (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 << 1);
+
+			DRM_INFO("paging queue doorbell being set to: [%s] doorbell index %d\n",
+				ring->use_doorbell?"true":"false", ring->doorbell_index);
 
 			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]

#2 lore raw
Quoted 7 lines · Philip

paging queues doorbell index use existing assignment sDMA_HI_PRI_ENGINE0/1 index, and increase SDMA_DOORBELL_RANGE size from 2 dwords to 4 dwords to enable the new doorbell index.

Change-Id I9adb965f16ee4089d261d9a22231337739184e49
Signed-off-by Philip Yang <[email protected]>

Is there a specific fw version requirement for this? If so, we need to add a check. Also will this break SR-IOV due to the doorbell mapping requirements for other OSes? Have we resolved that yet?

Quoted 61 lines
---
 drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 25 +++++++++++++++++--------
 3 files changed, 19 insertions(+), 10 deletions(-)
drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c +1 -1
index 6f9c54978cc1..0eb42c29ecac 100644
@@ -80,7 +80,7 @@ static void nbio_v6_1_sdma_doorbell_range(struct amdgpu_device *adev, int instan
 
        if (use_doorbell) {
                doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, OFFSET, doorbell_index);
-               doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 2);
+               doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 4);
        } else
                doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 0);
drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c +1 -1
index f8cee95d61cc..9342ee03d7d4 100644
@@ -76,7 +76,7 @@ static void nbio_v7_4_sdma_doorbell_range(struct amdgpu_device *adev, int instan
 
        if (use_doorbell) {
                doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, OFFSET, doorbell_index);
-               doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 2);
+               doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 4);
        } else
                doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 0);
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +2 -7
index f4490cdd9804..96c9e83204b7 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 index is already enabled 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);

@@ -1504,9 +1502,6 @@ 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");
-
                if (adev->asic_type == CHIP_VEGA10)
                        ring->doorbell_index = (i == 0) ?
                                (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c static int sdma_v4_0_sw_init(void *handle) · line 1516

Quoted 6 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +2 -0
@@ -1516,6 +1511,8 @@ static int sdma_v4_0_sw_init(void *handle)
                                (AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset
                                : (AMDGPU_DOORBELL64_sDMA_ENGINE1 << 1); // get DWORD offset
 
+               DRM_INFO("use_doorbell being set to: [%s] doorbell index %d\n",
+                       ring->use_doorbell?"true":"false", ring->doorbell_index);

I think we can drop these messages in general. Looks like leftover debugging output.

Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +1 -1
                sprintf(ring->name, "sdma%d", i);
                r = amdgpu_ring_init(adev, ring, 1024,

@@ -1529,7 +1526,19 @@ 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;

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c static int sdma_v4_0_sw_init(void *handle) · line 1539

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +10 -0
+
+                       if (adev->asic_type == CHIP_VEGA10)
+                               ring->doorbell_index = (i == 0) ?
+                                       (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 << 1)
+                                       : (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 << 1);
+                       else
+                               ring->doorbell_index = (i == 0) ?
+                                       (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 << 1)
+                                       : (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 << 1);
+

Copy paste typo? Seems like the if and else are the same.

Alex

Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +2 -1
+                       DRM_INFO("paging queue doorbell being set to: [%s] doorbell index %d\n",
+                               ring->use_doorbell?"true":"false", ring->doorbell_index);
 
                        sprintf(ring->name, "page%d", i);
                        r = amdgpu_ring_init(adev, ring, 1024,
--

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): CADnq5_NjEQa2E-pXtLn-rDK3TmuMb+9Ye=3jbmchrqif_M3eVw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

author
#4 lore raw
Quoted 11 lines · Alex Deucher
Quoted 6 lines · Philip

paging queues doorbell index use existing assignment sDMA_HI_PRI_ENGINE0/1 index, and increase SDMA_DOORBELL_RANGE size from 2 dwords to 4 dwords to enable the new doorbell index.

Change-Id I9adb965f16ee4089d261d9a22231337739184e49
Signed-off-by Philip Yang <[email protected]>

Is there a specific fw version requirement for this? If so, we need to add a check. Also will this break SR-IOV due to the doorbell mapping requirements for other OSes? Have we resolved that yet?

Yes, new FW is required to enable paging queue, it's good idea to check FW version
when we enable paging queue later after new FW is checked in. paging queue is not enabled yet.
This change will not take effect until paging queue is enabled.

This does not change doorbell mapping so it will not break SR-IOV. My understanding is Doorbell
mapping is good for vega10/12/20, but need update for future chip.

Quoted 64 lines
Quoted 64 lines
---
  drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 25 +++++++++++++++++--------
  3 files changed, 19 insertions(+), 10 deletions(-)
drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c +1 -1
index 6f9c54978cc1..0eb42c29ecac 100644
@@ -80,7 +80,7 @@ static void nbio_v6_1_sdma_doorbell_range(struct amdgpu_device *adev, int instan
 
         if (use_doorbell) {
                 doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, OFFSET, doorbell_index);
-               doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 2);
+               doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 4);
         } else
                 doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 0);
drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c +1 -1
index f8cee95d61cc..9342ee03d7d4 100644
@@ -76,7 +76,7 @@ static void nbio_v7_4_sdma_doorbell_range(struct amdgpu_device *adev, int instan
 
         if (use_doorbell) {
                 doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, OFFSET, doorbell_index);
-               doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 2);
+               doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 4);
         } else
                 doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 0);
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +2 -7
index f4490cdd9804..96c9e83204b7 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 index is already enabled 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);

@@ -1504,9 +1502,6 @@ 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");
-
                 if (adev->asic_type == CHIP_VEGA10)
                         ring->doorbell_index = (i == 0) ?
                                 (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset

@@ -1516,6 +1511,8 @@ static int sdma_v4_0_sw_init(void *handle)
                                 (AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset
                                 : (AMDGPU_DOORBELL64_sDMA_ENGINE1 << 1); // get DWORD offset
Quoted 6 lines

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c static int sdma_v4_0_sw_init(void *handle) · line 1516

Quoted 3 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +2 -0
+               DRM_INFO("use_doorbell being set to: [%s] doorbell index %d\n",
+                       ring->use_doorbell?"true":"false", ring->doorbell_index);

I think we can drop these messages in general. Looks like leftover debugging output.

Ok, change it to DRM_DEBUG

Quoted 20 lines
Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +1 -1
                 sprintf(ring->name, "sdma%d", i);
                 r = amdgpu_ring_init(adev, ring, 1024,

@@ -1529,7 +1526,19 @@ 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;

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c static int sdma_v4_0_sw_init(void *handle) · line 1539

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +10 -0
+
+                       if (adev->asic_type == CHIP_VEGA10)
+                               ring->doorbell_index = (i == 0) ?
+                                       (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 << 1)
+                                       : (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 << 1);
+                       else
+                               ring->doorbell_index = (i == 0) ?
+                                       (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 << 1)
+                                       : (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 << 1);
+

Copy paste typo? Seems like the if and else are the same.

Quoted 1 line

Alex

Yes, copy paste error. Thanks

Quoted 9 lines
Quoted 9 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +2 -1
+                       DRM_INFO("paging queue doorbell being set to: [%s] doorbell index %d\n",
+                               ring->use_doorbell?"true":"false", ring->doorbell_index);
 
                         sprintf(ring->name, "page%d", i);
                         r = amdgpu_ring_init(adev, ring, 1024,
--

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): 47ddc0e2-6164-ac44-608e-24f6f219f782-5C7GfCeVMHo@public.gmane.org

#6 lore raw

You changed the doorbell routing in NBIO. I think that won't work for SR-IOV, because it's not controlled by the guest OS there. We may need to disable paging queue doorbell on Vega10 and Vega12 with SRIOV. For Vega20 we plan to change the doorbell layout before it goes to production (Oak started working on that). So on Vega20 we should be able to enable the doorbell for the paging queue.

Regards,
Felix

-----Original Message-----
From: amd-gfx <[email protected]> On Behalf Of Yang, Philip
Sent: Thursday, November 15, 2018 12:54 PM
To: Alex Deucher <[email protected]>
Cc: amd-gfx list <[email protected]>
Subject: Re: [PATCH] drm/amdgpu: enable paging queue doorbell support

Quoted 11 lines · Alex Deucher
Quoted 6 lines · Philip

paging queues doorbell index use existing assignment sDMA_HI_PRI_ENGINE0/1 index, and increase SDMA_DOORBELL_RANGE size from 2 dwords to 4 dwords to enable the new doorbell index.

Change-Id I9adb965f16ee4089d261d9a22231337739184e49
Signed-off-by Philip Yang <[email protected]>

Is there a specific fw version requirement for this? If so, we need to add a check. Also will this break SR-IOV due to the doorbell mapping requirements for other OSes? Have we resolved that yet?

Yes, new FW is required to enable paging queue, it's good idea to check FW version when we enable paging queue later after new FW is checked in. paging queue is not enabled yet. This change will not take effect until paging queue is enabled.

This does not change doorbell mapping so it will not break SR-IOV. My understanding is Doorbell mapping is good for vega10/12/20, but need update for future chip.

Quoted 74 lines
Quoted 74 lines
---
  drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 25 +++++++++++++++++--------
  3 files changed, 19 insertions(+), 10 deletions(-)
drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c +0 -0
index 6f9c54978cc1..0eb42c29ecac 100644
@@ -80,7 +80,7 @@ static void nbio_v6_1_sdma_doorbell_range(struct

amdgpu_device *adev, int instan

         if (use_doorbell) {
                 doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, OFFSET, doorbell_index);
drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c +2 -1
-               doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 2);
+               doorbell_range = REG_SET_FIELD(doorbell_range, 
+ BIF_SDMA0_DOORBELL_RANGE, SIZE, 4);
         } else
                 doorbell_range = REG_SET_FIELD(doorbell_range,

BIF_SDMA0_DOORBELL_RANGE, SIZE, 0);

drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c +0 -0
index f8cee95d61cc..9342ee03d7d4 100644
@@ -76,7 +76,7 @@ static void nbio_v7_4_sdma_doorbell_range(struct

amdgpu_device *adev, int instan

         if (use_doorbell) {
                 doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, OFFSET, doorbell_index);
drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c +2 -1
-               doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 2);
+               doorbell_range = REG_SET_FIELD(doorbell_range, 
+ BIF_SDMA0_DOORBELL_RANGE, SIZE, 4);
         } else
                 doorbell_range = REG_SET_FIELD(doorbell_range,

BIF_SDMA0_DOORBELL_RANGE, SIZE, 0);

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +3 -7
index f4490cdd9804..96c9e83204b7 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 index is already enabled 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); @@ -1504,9 
+1502,6 @@ 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");
-
                 if (adev->asic_type == CHIP_VEGA10)
                         ring->doorbell_index = (i == 0) ?

(AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset @@ -1516,6 +1511,8 @@ static int sdma_v4_0_sw_init(void *handle)
(AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset
: (AMDGPU_DOORBELL64_sDMA_ENGINE1 <<
1); // get DWORD offset

Quoted 7 lines

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

Quoted 4 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +3 -0
+               DRM_INFO("use_doorbell being set to: [%s] doorbell index %d\n",
+                       ring->use_doorbell?"true":"false", 
+ ring->doorbell_index);

I think we can drop these messages in general. Looks like leftover debugging output.

Ok, change it to DRM_DEBUG

Quoted 21 lines
Quoted 9 lines
                 sprintf(ring->name, "sdma%d", i);
                 r = amdgpu_ring_init(adev, ring, 1024, @@ -1529,7 
+1526,19 @@ 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;
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +2 -1
-                       ring->use_doorbell = false;
+                       ring->use_doorbell = true;
+

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c static int sdma_v4_0_sw_init(void *handle) · line 1539

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +10 -0
+                       if (adev->asic_type == CHIP_VEGA10)
+                               ring->doorbell_index = (i == 0) ?
+                                       (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 << 1)
+                                       : (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 << 1);
+                       else
+                               ring->doorbell_index = (i == 0) ?
+                                       (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 << 1)
+                                       : 
+ (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 << 1);
+

Copy paste typo? Seems like the if and else are the same.

Quoted 1 line

Alex

Yes, copy paste error. Thanks

Quoted 10 lines
Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +3 -1
+                       DRM_INFO("paging queue doorbell being set to: [%s] doorbell index %d\n",
+                               ring->use_doorbell?"true":"false", 
+ ring->doorbell_index);
 
                         sprintf(ring->name, "page%d", i);
                         r = amdgpu_ring_init(adev, ring, 1024,
--

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
_______________________________________________
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): DM5PR12MB170795FFAFFF461CA6E3D96592DC0-2J9CzHegvk9TCtO+SvGBKwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org

author
#8 lore raw

Thanks, I will disable paging queue doorbell on Vega10 and Vega12 with SRIOV.

Philip

Quoted 142 lines · Felix

You changed the doorbell routing in NBIO. I think that won't work for SR-IOV, because it's not controlled by the guest OS there. We may need to disable paging queue doorbell on Vega10 and Vega12 with SRIOV. For Vega20 we plan to change the doorbell layout before it goes to production (Oak started working on that). So on Vega20 we should be able to enable the doorbell for the paging queue.

Regards,
Felix

-----Original Message-----
From: amd-gfx <[email protected]> On Behalf Of Yang, Philip
Sent: Thursday, November 15, 2018 12:54 PM
To: Alex Deucher <[email protected]>
Cc: amd-gfx list <[email protected]>
Subject: Re: [PATCH] drm/amdgpu: enable paging queue doorbell support

Quoted 11 lines · Alex Deucher
Quoted 6 lines · Philip

paging queues doorbell index use existing assignment sDMA_HI_PRI_ENGINE0/1 index, and increase SDMA_DOORBELL_RANGE size from 2 dwords to 4 dwords to enable the new doorbell index.

Change-Id I9adb965f16ee4089d261d9a22231337739184e49
Signed-off-by Philip Yang <[email protected]>

Is there a specific fw version requirement for this? If so, we need to add a check. Also will this break SR-IOV due to the doorbell mapping requirements for other OSes? Have we resolved that yet?

Yes, new FW is required to enable paging queue, it's good idea to check FW version when we enable paging queue later after new FW is checked in. paging queue is not enabled yet. This change will not take effect until paging queue is enabled.

This does not change doorbell mapping so it will not break SR-IOV. My understanding is Doorbell mapping is good for vega10/12/20, but need update for future chip.

Quoted 74 lines
Quoted 74 lines
---
   drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c |  2 +-
   drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c |  2 +-
   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 25 +++++++++++++++++--------
   3 files changed, 19 insertions(+), 10 deletions(-)
drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c +0 -0
index 6f9c54978cc1..0eb42c29ecac 100644
@@ -80,7 +80,7 @@ static void nbio_v6_1_sdma_doorbell_range(struct

amdgpu_device *adev, int instan

          if (use_doorbell) {
                  doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, OFFSET, doorbell_index);
drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c +2 -1
-               doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 2);
+               doorbell_range = REG_SET_FIELD(doorbell_range,
+ BIF_SDMA0_DOORBELL_RANGE, SIZE, 4);
          } else
                  doorbell_range = REG_SET_FIELD(doorbell_range,

BIF_SDMA0_DOORBELL_RANGE, SIZE, 0);

drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c +0 -0
index f8cee95d61cc..9342ee03d7d4 100644
@@ -76,7 +76,7 @@ static void nbio_v7_4_sdma_doorbell_range(struct

amdgpu_device *adev, int instan

          if (use_doorbell) {
                  doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, OFFSET, doorbell_index);
drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c +2 -1
-               doorbell_range = REG_SET_FIELD(doorbell_range, BIF_SDMA0_DOORBELL_RANGE, SIZE, 2);
+               doorbell_range = REG_SET_FIELD(doorbell_range,
+ BIF_SDMA0_DOORBELL_RANGE, SIZE, 4);
          } else
                  doorbell_range = REG_SET_FIELD(doorbell_range,

BIF_SDMA0_DOORBELL_RANGE, SIZE, 0);

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +3 -7
index f4490cdd9804..96c9e83204b7 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 index is already enabled 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); @@ -1504,9
+1502,6 @@ 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");
-
                  if (adev->asic_type == CHIP_VEGA10)
                          ring->doorbell_index = (i == 0) ?

(AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset @@ -1516,6 +1511,8 @@ static int sdma_v4_0_sw_init(void *handle)
(AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset
: (AMDGPU_DOORBELL64_sDMA_ENGINE1 <<
1); // get DWORD offset

Quoted 7 lines

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

Quoted 4 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +3 -0
+               DRM_INFO("use_doorbell being set to: [%s] doorbell index %d\n",
+                       ring->use_doorbell?"true":"false",
+ ring->doorbell_index);

I think we can drop these messages in general. Looks like leftover debugging output.

Ok, change it to DRM_DEBUG

Quoted 21 lines
Quoted 9 lines
                  sprintf(ring->name, "sdma%d", i);
                  r = amdgpu_ring_init(adev, ring, 1024, @@ -1529,7
+1526,19 @@ 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;
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +2 -1
-                       ring->use_doorbell = false;
+                       ring->use_doorbell = true;
+

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +10 -0
+                       if (adev->asic_type == CHIP_VEGA10)
+                               ring->doorbell_index = (i == 0) ?
+                                       (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 << 1)
+                                       : (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 << 1);
+                       else
+                               ring->doorbell_index = (i == 0) ?
+                                       (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 << 1)
+                                       :
+ (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 << 1);
+

Copy paste typo? Seems like the if and else are the same.

Quoted 1 line

Alex

Yes, copy paste error. Thanks

Quoted 9 lines
Quoted 9 lines
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +3 -1
+                       DRM_INFO("paging queue doorbell being set to: [%s] doorbell index %d\n",
+                               ring->use_doorbell?"true":"false",
+ ring->doorbell_index);
 
                          sprintf(ring->name, "page%d", i);
                          r = amdgpu_ring_init(adev, ring, 1024,
--

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