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

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

[Yang, Philip]

This looks like copy paste typo

Change-Id: Iee3fd3a551650ec9199bc030a7886e92000b02e7
Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index f4490cdd9804..3f6b7882dbd2 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -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
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Yang, Philip]

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 <Philip.Yang@amd.com>
---
 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(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 590588a82471..cb06e6883fad 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -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));
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 3f6b7882dbd2..4d873fd3242c 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -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
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Yang, Philip]

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 <Philip.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 46 +++++++++++++++++---------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 4d873fd3242c..0a3b68dd49a0 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -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;
-- 
2.17.1

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

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

[Deucher, Alexander]

[-- Attachment #1.1: Type: text/plain, Size: 1952 bytes --]

Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Yang, Philip <Philip.Yang-5C7GfCeVMHo@public.gmane.org>
Sent: Monday, November 19, 2018 3:20:00 PM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
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 <Philip.Yang-5C7GfCeVMHo@public.gmane.org>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index f4490cdd9804..3f6b7882dbd2 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -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
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 3708 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

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

[Deucher, Alexander]

[-- Attachment #1.1: Type: text/plain, Size: 5089 bytes --]

Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Yang, Philip <Philip.Yang-5C7GfCeVMHo@public.gmane.org>
Sent: Monday, November 19, 2018 3:20:07 PM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
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 <Philip.Yang-5C7GfCeVMHo@public.gmane.org>
---
 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(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 590588a82471..cb06e6883fad 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -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));
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 3f6b7882dbd2..4d873fd3242c 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -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
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 12211 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

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

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Yang, Philip
> Sent: Monday, November 19, 2018 3:20 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Yang, Philip <Philip.Yang@amd.com>
> 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 <Philip.Yang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 46 +++++++++++++++++----
> -----
>  1 file changed, 30 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 4d873fd3242c..0a3b68dd49a0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -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;

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 <alexander.deucher@amd.com>


>  	}
> 
> +	/* 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;
> --
> 2.17.1
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Yang, Philip]

On 2018-11-19 3:57 p.m., Deucher, Alexander wrote:
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>> Yang, Philip
>> Sent: Monday, November 19, 2018 3:20 PM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Yang, Philip <Philip.Yang@amd.com>
>> 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 <Philip.Yang@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 46 +++++++++++++++++----
>> -----
>>   1 file changed, 30 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> index 4d873fd3242c..0a3b68dd49a0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> @@ -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;
> 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 <alexander.deucher@amd.com>
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
>
>>   	}
>>
>> +	/* 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;
>> --
>> 2.17.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

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

[Christian König]

Am 19.11.18 um 22:16 schrieb Yang, Philip:
> On 2018-11-19 3:57 p.m., Deucher, Alexander wrote:
>>> -----Original Message-----
>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>> Yang, Philip
>>> Sent: Monday, November 19, 2018 3:20 PM
>>> To: amd-gfx@lists.freedesktop.org
>>> Cc: Yang, Philip <Philip.Yang@amd.com>
>>> 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 <Philip.Yang@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 46 +++++++++++++++++----
>>> -----
>>>    1 file changed, 30 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>>> index 4d873fd3242c..0a3b68dd49a0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>>> @@ -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;
>> 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 <alexander.deucher@amd.com>
> 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 
<christian.koenig@amd.com>

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

Thanks for taking care of that,
Christian.


>
> Philip
>>>    	}
>>>
>>> +	/* 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;
>>> --
>>> 2.17.1
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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