RE: [PATCH 07/10] drm/amdgpu: add concurrent baco reset support for XGMI

[Ma, Le]

[AMD Official Use Only - Internal Distribution Only]

Not sure it's same issue as I observed.

If you have an XGMI setup, use the latest drm-next and the PMFW I used on my 
XGMI system(I just sent you the vega20_smc.bin through mail). And then give 
another attempt.

About the strict time interval, I remember the XGMI node EnterBaco message will 
fail when interval is around millisecond.

Regards,
Ma Le

From: Grodzovsky, Andrey 
Sent: Tuesday, December 10, 2019 6:01 AM
To: Ma, Le ; amd-gfx@lists.freedesktop.org; Zhou1, Tao 
; Deucher, Alexander ; Li, Dennis 
; Zhang, Hawking 
Cc: Chen, Guchun 
Subject: Re: [PATCH 07/10] drm/amdgpu: add concurrent baco reset support for 
XGMI


I reproduced the issue on my side - i consistently  observe amdgpu: [powerplay] 
Failed to send message 0x58, response 0x0 - Baco exit failure - do you know 
what is the strict time interval within which all the Baco enter/Exit messages 
needs to be sent to all the nodes in the hive ?

Andrey
On 12/9/19 6:34 AM, Ma, Le wrote:

[AMD Official Use Only - Internal Distribution Only]

Hi Andrey,

I tried your patches on my 2P XGMI platform. The baco can work at most time, 
and randomly got following error:
[ 1701.542298] amdgpu: [powerplay] Failed to send message 0x25, response 0x0

This error usually means some sync issue exist for xgmi baco case. Feel free to 
debug your patches on my XGMI platform.

Regards,
Ma Le

From: Grodzovsky, Andrey 

Sent: Saturday, December 7, 2019 5:51 AM
To: Ma, Le ; 
amd-gfx@lists.freedesktop.org; Zhou1, Tao 
; Deucher, Alexander 
; Li, Dennis 
; Zhang, Hawking 

Cc: Chen, Guchun 
Subject: Re: [PATCH 07/10] drm/amdgpu: add concurrent baco reset support for 
XGMI


Hey Ma, attached a solution - it's just compiled as I still can't make my XGMI 
setup work (with bridge connected only one device is visible to the system 
while the other is not). Please try it on your system if you have a chance.

Andrey
On 12/4/19 10:14 PM, Ma, Le wrote:

AFAIK it's enough for even single one node in the hive to to fail the enter the 
BACO state on time to fail the entire hive reset procedure, no ?
[Le]: Yeah, agree that. I've been thinking that make all nodes entering baco 
simultaneously can reduce the possibility of node failure to enter/exit BACO 
risk. For example, in an XGMI hive with 8 nodes, the total time interval of 8 
nodes enter/exit BACO on 8 CPUs is less than the interval that 8 nodes enter 
BACO serially and exit BACO serially depending on one CPU with yield 
capability. This interval is usually strict for BACO feature itself. Anyway, we 
need more looping test later on any method we will choose.

Any way - I see our discussion blocks your entire patch set - I think you can 
go ahead and commit yours way (I think you got an RB from Hawking) and I will 
look then and see if I can implement my method and if it works will just revert 
your patch.

[Le]: OK, fine.

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

Re: [PATCH] drm/amdgpu: avoid using invalidate semaphore for picasso(v2)

[Huang, Ray]

On Tue, Dec 10, 2019 at 10:55:13AM +0800, Zhu, Changfeng wrote:
> From: changzhu 
> 
> It may cause timeout waiting for sem acquire in VM flush when using
> invalidate semaphore for picasso. So it needs to avoid using invalidate
> semaphore for piasso.
> 
> Change-Id: I6dc552bde180919cd5ba6c81c6d9e3f800043b03
> Signed-off-by: changzhu 

Reviewed-by: Huang Rui 

> ---
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 28 +++
>  1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 231ea9762cb5..601667246a1c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -464,8 +464,11 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device 
> *adev, uint32_t vmid,
>*/
>  
>   /* TODO: It needs to continue working on debugging with semaphore for 
> GFXHUB as well. */
> - if (vmhub == AMDGPU_MMHUB_0 ||
> - vmhub == AMDGPU_MMHUB_1) {
> + if ((vmhub == AMDGPU_MMHUB_0 ||
> +  vmhub == AMDGPU_MMHUB_1) &&
> + (!(adev->asic_type == CHIP_RAVEN &&
> +adev->rev_id < 0x8 &&
> +adev->pdev->device == 0x15d8))) {
>   for (j = 0; j < adev->usec_timeout; j++) {
>   /* a read return value of 1 means semaphore acuqire */
>   tmp = RREG32_NO_KIQ(hub->vm_inv_eng0_sem + eng);
> @@ -495,8 +498,11 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device 
> *adev, uint32_t vmid,
>   }
>  
>   /* TODO: It needs to continue working on debugging with semaphore for 
> GFXHUB as well. */
> - if (vmhub == AMDGPU_MMHUB_0 ||
> - vmhub == AMDGPU_MMHUB_1)
> + if ((vmhub == AMDGPU_MMHUB_0 ||
> +  vmhub == AMDGPU_MMHUB_1) &&
> + (!(adev->asic_type == CHIP_RAVEN &&
> +adev->rev_id < 0x8 &&
> +adev->pdev->device == 0x15d8)))
>   /*
>* add semaphore release after invalidation,
>* write with 0 means semaphore release
> @@ -527,8 +533,11 @@ static uint64_t gmc_v9_0_emit_flush_gpu_tlb(struct 
> amdgpu_ring *ring,
>*/
>  
>   /* TODO: It needs to continue working on debugging with semaphore for 
> GFXHUB as well. */
> - if (ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
> - ring->funcs->vmhub == AMDGPU_MMHUB_1)
> + if ((ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
> +  ring->funcs->vmhub == AMDGPU_MMHUB_1) &&
> + (!(adev->asic_type == CHIP_RAVEN &&
> +adev->rev_id < 0x8 &&
> +adev->pdev->device == 0x15d8)))
>   /* a read return value of 1 means semaphore acuqire */
>   amdgpu_ring_emit_reg_wait(ring,
> hub->vm_inv_eng0_sem + eng, 0x1, 0x1);
> @@ -544,8 +553,11 @@ static uint64_t gmc_v9_0_emit_flush_gpu_tlb(struct 
> amdgpu_ring *ring,
>   req, 1 << vmid);
>  
>   /* TODO: It needs to continue working on debugging with semaphore for 
> GFXHUB as well. */
> - if (ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
> - ring->funcs->vmhub == AMDGPU_MMHUB_1)
> + if ((ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
> +  ring->funcs->vmhub == AMDGPU_MMHUB_1) &&
> + (!(adev->asic_type == CHIP_RAVEN &&
> +adev->rev_id < 0x8 &&
> +adev->pdev->device == 0x15d8)))
>   /*
>* add semaphore release after invalidation,
>* write with 0 means semaphore release
> -- 
> 2.17.1
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: avoid using invalidate semaphore for picasso(v2)

[Changfeng.Zhu]

From: changzhu 

It may cause timeout waiting for sem acquire in VM flush when using
invalidate semaphore for picasso. So it needs to avoid using invalidate
semaphore for piasso.

Change-Id: I6dc552bde180919cd5ba6c81c6d9e3f800043b03
Signed-off-by: changzhu 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 28 +++
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 231ea9762cb5..601667246a1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -464,8 +464,11 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device 
*adev, uint32_t vmid,
 */
 
/* TODO: It needs to continue working on debugging with semaphore for 
GFXHUB as well. */
-   if (vmhub == AMDGPU_MMHUB_0 ||
-   vmhub == AMDGPU_MMHUB_1) {
+   if ((vmhub == AMDGPU_MMHUB_0 ||
+vmhub == AMDGPU_MMHUB_1) &&
+   (!(adev->asic_type == CHIP_RAVEN &&
+  adev->rev_id < 0x8 &&
+  adev->pdev->device == 0x15d8))) {
for (j = 0; j < adev->usec_timeout; j++) {
/* a read return value of 1 means semaphore acuqire */
tmp = RREG32_NO_KIQ(hub->vm_inv_eng0_sem + eng);
@@ -495,8 +498,11 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device 
*adev, uint32_t vmid,
}
 
/* TODO: It needs to continue working on debugging with semaphore for 
GFXHUB as well. */
-   if (vmhub == AMDGPU_MMHUB_0 ||
-   vmhub == AMDGPU_MMHUB_1)
+   if ((vmhub == AMDGPU_MMHUB_0 ||
+vmhub == AMDGPU_MMHUB_1) &&
+   (!(adev->asic_type == CHIP_RAVEN &&
+  adev->rev_id < 0x8 &&
+  adev->pdev->device == 0x15d8)))
/*
 * add semaphore release after invalidation,
 * write with 0 means semaphore release
@@ -527,8 +533,11 @@ static uint64_t gmc_v9_0_emit_flush_gpu_tlb(struct 
amdgpu_ring *ring,
 */
 
/* TODO: It needs to continue working on debugging with semaphore for 
GFXHUB as well. */
-   if (ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
-   ring->funcs->vmhub == AMDGPU_MMHUB_1)
+   if ((ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
+ring->funcs->vmhub == AMDGPU_MMHUB_1) &&
+   (!(adev->asic_type == CHIP_RAVEN &&
+  adev->rev_id < 0x8 &&
+  adev->pdev->device == 0x15d8)))
/* a read return value of 1 means semaphore acuqire */
amdgpu_ring_emit_reg_wait(ring,
  hub->vm_inv_eng0_sem + eng, 0x1, 0x1);
@@ -544,8 +553,11 @@ static uint64_t gmc_v9_0_emit_flush_gpu_tlb(struct 
amdgpu_ring *ring,
req, 1 << vmid);
 
/* TODO: It needs to continue working on debugging with semaphore for 
GFXHUB as well. */
-   if (ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
-   ring->funcs->vmhub == AMDGPU_MMHUB_1)
+   if ((ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
+ring->funcs->vmhub == AMDGPU_MMHUB_1) &&
+   (!(adev->asic_type == CHIP_RAVEN &&
+  adev->rev_id < 0x8 &&
+  adev->pdev->device == 0x15d8)))
/*
 * add semaphore release after invalidation,
 * write with 0 means semaphore release
-- 
2.17.1

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

RE: [PATCH 07/10] drm/amdgpu: add concurrent baco reset support for XGMI

[Ma, Le]

[AMD Official Use Only - Internal Distribution Only]

I'm fine with your solution if synchronization time interval satisfies BACO 
requirements and loop test can pass on XGMI system.

Regards,
Ma Le

From: Grodzovsky, Andrey 
Sent: Monday, December 9, 2019 11:52 PM
To: Ma, Le ; amd-gfx@lists.freedesktop.org; Zhou1, Tao 
; Deucher, Alexander ; Li, Dennis 
; Zhang, Hawking 
Cc: Chen, Guchun 
Subject: Re: [PATCH 07/10] drm/amdgpu: add concurrent baco reset support for 
XGMI


Thanks a lot Ma for trying - I think I have to have my own system to debug this 
so I will keep trying enabling XGMI - i still think the is the right and the 
generic solution for multiple nodes reset synchronization and in fact the 
barrier should also be used for synchronizing PSP mode 1 XGMI reset too.

Andrey
On 12/9/19 6:34 AM, Ma, Le wrote:

[AMD Official Use Only - Internal Distribution Only]

Hi Andrey,

I tried your patches on my 2P XGMI platform. The baco can work at most time, 
and randomly got following error:
[ 1701.542298] amdgpu: [powerplay] Failed to send message 0x25, response 0x0

This error usually means some sync issue exist for xgmi baco case. Feel free to 
debug your patches on my XGMI platform.

Regards,
Ma Le

From: Grodzovsky, Andrey 

Sent: Saturday, December 7, 2019 5:51 AM
To: Ma, Le ; 
amd-gfx@lists.freedesktop.org; Zhou1, Tao 
; Deucher, Alexander 
; Li, Dennis 
; Zhang, Hawking 

Cc: Chen, Guchun 
Subject: Re: [PATCH 07/10] drm/amdgpu: add concurrent baco reset support for 
XGMI


Hey Ma, attached a solution - it's just compiled as I still can't make my XGMI 
setup work (with bridge connected only one device is visible to the system 
while the other is not). Please try it on your system if you have a chance.

Andrey
On 12/4/19 10:14 PM, Ma, Le wrote:

AFAIK it's enough for even single one node in the hive to to fail the enter the 
BACO state on time to fail the entire hive reset procedure, no ?
[Le]: Yeah, agree that. I've been thinking that make all nodes entering baco 
simultaneously can reduce the possibility of node failure to enter/exit BACO 
risk. For example, in an XGMI hive with 8 nodes, the total time interval of 8 
nodes enter/exit BACO on 8 CPUs is less than the interval that 8 nodes enter 
BACO serially and exit BACO serially depending on one CPU with yield 
capability. This interval is usually strict for BACO feature itself. Anyway, we 
need more looping test later on any method we will choose.

Any way - I see our discussion blocks your entire patch set - I think you can 
go ahead and commit yours way (I think you got an RB from Hawking) and I will 
look then and see if I can implement my method and if it works will just revert 
your patch.

[Le]: OK, fine.

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

RE: [PATCH] drm/amd/powerplay: avoid null pointer

[Quan, Evan]

Thanks. Reviewed-by: Evan Quan 

> -Original Message-
> From: amd-gfx  On Behalf Of Yintian
> Tao
> Sent: Tuesday, December 10, 2019 1:27 AM
> To: Deucher, Alexander ; Feng, Kenneth
> 
> Cc: amd-gfx@lists.freedesktop.org; Tao, Yintian 
> Subject: [PATCH] drm/amd/powerplay: avoid null pointer
> 
> because some asics have no smu.ppt_funcs we need add one check for it
> otherwise it will raise null pointer problem.
> 
> Signed-off-by: Yintian Tao 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index a21ee035ca57..b8a42ebb2f5a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -160,7 +160,8 @@ static ssize_t amdgpu_get_dpm_state(struct device
> *dev,
>   enum amd_pm_state_type pm;
> 
>   if (is_support_sw_smu(adev)) {
> - if (adev->smu.ppt_funcs->get_current_power_state)
> + if (adev->smu.ppt_funcs &&
> + adev->smu.ppt_funcs->get_current_power_state)
>   pm = smu_get_current_power_state(>smu);
>   else
>   pm = adev->pm.dpm.user_state;
> --
> 2.17.1
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.free
> desktop.org%2Fmailman%2Flistinfo%2Famd-
> gfxdata=02%7C01%7Cevan.quan%40amd.com%7Cc3d9c9c42b8648b0c2
> d708d77cccf3f3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6371
> 15092081521686sdata=qDCT%2BPXFgCAu9GDTRpNjW890IVBEXuWMHx
> dN9OZZIsE%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 07/10] drm/amdgpu: add concurrent baco reset support for XGMI

[Andrey Grodzovsky]

I reproduced the issue on my side - i consistently  observe amdgpu: 
[powerplay] Failed to send message 0x58, response 0x0 - Baco exit 
failure - do you know what is the strict time interval within which all 
the Baco enter/Exit messages needs to be sent to all the nodes in the hive ?


Andrey

On 12/9/19 6:34 AM, Ma, Le wrote:


[AMD Official Use Only - Internal Distribution Only]


Hi Andrey,

I tried your patches on my 2P XGMI platform. The baco can work at most 
time, and randomly got following error:


[ 1701.542298] amdgpu: [powerplay] Failed to send message 0x25, 
response 0x0


This error usually means some sync issue exist for xgmi baco case. 
Feel free to debug your patches on my XGMI platform.


Regards,

Ma Le

*From:*Grodzovsky, Andrey 
*Sent:* Saturday, December 7, 2019 5:51 AM
*To:* Ma, Le ; amd-gfx@lists.freedesktop.org; Zhou1, 
Tao ; Deucher, Alexander 
; Li, Dennis ; Zhang, 
Hawking 

*Cc:* Chen, Guchun 
*Subject:* Re: [PATCH 07/10] drm/amdgpu: add concurrent baco reset 
support for XGMI


Hey Ma, attached a solution - it's just compiled as I still can't make 
my XGMI setup work (with bridge connected only one device is visible 
to the system while the other is not). Please try it on your system if 
you have a chance.


Andrey

On 12/4/19 10:14 PM, Ma, Le wrote:

AFAIK it's enough for even single one node in the hive to to fail
the enter the BACO state on time to fail the entire hive reset
procedure, no ?

[Le]: Yeah, agree that. I’ve been thinking that make all nodes
entering baco simultaneously can reduce the possibility of node
failure to enter/exit BACO risk. For example, in an XGMI hive with
8 nodes, the total time interval of 8 nodes enter/exit BACO on 8
CPUs is less than the interval that 8 nodes enter BACO serially
and exit BACO serially depending on one CPU with yield capability.
This interval is usually strict for BACO feature itself. Anyway,
we need more looping test later on any method we will choose.

Any way - I see our discussion blocks your entire patch set - I
think you can go ahead and commit yours way (I think you got an RB
from Hawking) and I will look then and see if I can implement my
method and if it works will just revert your patch.

[Le]: OK, fine.

Andrey

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

Re: [PATCH 2/2] drm/amdgpu: fix JPEG instance checking when ctx init

[Deucher, Alexander]

forgot to add RB for the series.

Alex

From: amd-gfx  on behalf of Deucher, 
Alexander 
Sent: Monday, December 9, 2019 4:45 PM
To: Liu, Leo ; amd-gfx@lists.freedesktop.org 

Subject: Re: [PATCH 2/2] drm/amdgpu: fix JPEG instance checking when ctx init


[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Alex Deucher 

From: amd-gfx  on behalf of Leo Liu 

Sent: Monday, December 9, 2019 4:10 PM
To: amd-gfx@lists.freedesktop.org 
Cc: Liu, Leo 
Subject: [PATCH 2/2] drm/amdgpu: fix JPEG instance checking when ctx init

Fixes: 0388aee76("drm/amdgpu: use the JPEG structure for
general driver support")

Signed-off-by: Leo Liu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index a0d3d7b756eb..db4b6283c28c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -170,7 +170,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
 break;
 case AMDGPU_HW_IP_VCN_JPEG:
 for (j = 0; j < adev->jpeg.num_jpeg_inst; ++j) {
-   if (adev->vcn.harvest_config & (1 << j))
+   if (adev->jpeg.harvest_config & (1 << j))
 continue;
 rings[num_rings++] = 
>jpeg.inst[j].ring_dec;
 }
--
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Calexander.deucher%40amd.com%7Ca5db83292d3844d8955908d77cec5306%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637115227328960060sdata=mWsj0v7AxwKQVL1FzSn%2F6QhASdd4NxUQmMl9pCX7vTQ%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/4] drm/scheduler: rework entity creation

[Nirmoy Das]

Entity currently keeps a copy of run_queue list and modify it in
drm_sched_entity_set_priority(). Entities shouldn't modify run_queue
list. Use drm_gpu_scheduler list instead of drm_sched_rq list
in drm_sched_entity struct. In this way we can select a runqueue based
on entity/ctx's priority for a  drm scheduler.

Signed-off-by: Nirmoy Das 
Reviewed-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c  |  7 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  |  8 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c  |  7 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c  |  7 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 14 +++--
 drivers/gpu/drm/etnaviv/etnaviv_drv.c|  7 ++-
 drivers/gpu/drm/lima/lima_sched.c|  5 +-
 drivers/gpu/drm/panfrost/panfrost_job.c  |  8 ++-
 drivers/gpu/drm/scheduler/sched_entity.c | 74 ++--
 drivers/gpu/drm/v3d/v3d_drv.c|  8 ++-
 include/drm/gpu_scheduler.h  |  8 ++-
 11 files changed, 78 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index a0d3d7b756eb..1d6850af9908 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -122,7 +122,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
 
for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
-   struct drm_sched_rq *rqs[AMDGPU_MAX_RINGS];
+   struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
unsigned num_rings = 0;
unsigned num_rqs = 0;
 
@@ -181,12 +181,13 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
if (!rings[j]->adev)
continue;
 
-   rqs[num_rqs++] = [j]->sched.sched_rq[priority];
+   sched_list[num_rqs++] = [j]->sched;
}
 
for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j)
r = drm_sched_entity_init(>entities[i][j].entity,
- rqs, num_rqs, >guilty);
+ priority, sched_list,
+ num_rqs, >guilty);
if (r)
goto error_cleanup_entities;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 19ffe00d9072..2b6e35893918 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1957,11 +1957,13 @@ void amdgpu_ttm_set_buffer_funcs_status(struct 
amdgpu_device *adev, bool enable)
 
if (enable) {
struct amdgpu_ring *ring;
-   struct drm_sched_rq *rq;
+   struct drm_gpu_scheduler *sched;
 
ring = adev->mman.buffer_funcs_ring;
-   rq = >sched.sched_rq[DRM_SCHED_PRIORITY_KERNEL];
-   r = drm_sched_entity_init(>mman.entity, , 1, NULL);
+   sched = >sched;
+   r = drm_sched_entity_init(>mman.entity,
+ DRM_SCHED_PRIORITY_KERNEL, ,
+ 1, NULL);
if (r) {
DRM_ERROR("Failed setting up TTM BO move entity (%d)\n",
  r);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index e324bfe6c58f..a1a110f5513d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -330,12 +330,13 @@ int amdgpu_uvd_sw_fini(struct amdgpu_device *adev)
 int amdgpu_uvd_entity_init(struct amdgpu_device *adev)
 {
struct amdgpu_ring *ring;
-   struct drm_sched_rq *rq;
+   struct drm_gpu_scheduler *sched;
int r;
 
ring = >uvd.inst[0].ring;
-   rq = >sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
-   r = drm_sched_entity_init(>uvd.entity, , 1, NULL);
+   sched = >sched;
+   r = drm_sched_entity_init(>uvd.entity, DRM_SCHED_PRIORITY_NORMAL,
+ , 1, NULL);
if (r) {
DRM_ERROR("Failed setting up UVD kernel entity.\n");
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 46b590af2fd2..ceb0dbf685f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -240,12 +240,13 @@ int amdgpu_vce_sw_fini(struct amdgpu_device *adev)
 int amdgpu_vce_entity_init(struct amdgpu_device *adev)
 {
struct amdgpu_ring *ring;
-   struct drm_sched_rq *rq;
+   struct drm_gpu_scheduler *sched;
int r;
 
ring = >vce.ring[0];
-   rq = >sched.sched_rq[DRM_SCHED_PRIORITY_NORMAL];
-   r = drm_sched_entity_init(>vce.entity, , 1, NULL);
+   sched = >sched;
+   r = 

[PATCH 4/4] drm/scheduler: do not keep a copy of sched list

[Nirmoy Das]

entity should not keep copy and maintain sched list for
itself.

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/scheduler/sched_entity.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/sched_entity.c 
b/drivers/gpu/drm/scheduler/sched_entity.c
index f9b6ce29c58f..a5f729f421f8 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -67,17 +67,10 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
entity->guilty = guilty;
entity->num_sched_list = num_sched_list;
entity->priority = priority;
-   entity->sched_list =  kcalloc(num_sched_list,
- sizeof(struct drm_gpu_scheduler *), 
GFP_KERNEL);
+   entity->sched_list =  sched_list;
 
-   if(!entity->sched_list)
-   return -ENOMEM;
 
init_completion(>entity_idle);
-
-   for (i = 0; i < num_sched_list; i++)
-   entity->sched_list[i] = sched_list[i];
-
if (num_sched_list)
entity->rq = >sched_list[0]->sched_rq[entity->priority];
 
@@ -312,7 +305,6 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
 
dma_fence_put(entity->last_scheduled);
entity->last_scheduled = NULL;
-   kfree(entity->sched_list);
 }
 EXPORT_SYMBOL(drm_sched_entity_fini);
 
-- 
2.24.0

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

[PATCH 2/4] drm/amdgpu: replace vm_pte's run-queue list with drm gpu scheds list

[Nirmoy Das]

drm_sched_entity_init() takes drm gpu scheduler list instead of
drm_sched_rq list. This makes conversion of drm_sched_rq list
to drm gpu scheduler list unnecessary

Signed-off-by: Nirmoy Das 
Reviewed-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  4 ++--
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c  |  8 +++-
 drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c |  8 +++-
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c |  8 +++-
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c |  5 ++---
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c |  8 +++-
 drivers/gpu/drm/amd/amdgpu/si_dma.c|  8 +++-
 9 files changed, 24 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f85007382093..cf4953c4e2cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2779,7 +2779,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
adev->mman.buffer_funcs = NULL;
adev->mman.buffer_funcs_ring = NULL;
adev->vm_manager.vm_pte_funcs = NULL;
-   adev->vm_manager.vm_pte_num_rqs = 0;
+   adev->vm_manager.vm_pte_num_scheds = 0;
adev->gmc.gmc_funcs = NULL;
adev->fence_context = dma_fence_context_alloc(AMDGPU_MAX_RINGS);
bitmap_zero(adev->gfx.pipe_reserve_bitmap, AMDGPU_MAX_COMPUTE_QUEUES);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 5e78db30c722..0e1ed8ef2ce7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2687,7 +2687,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
 {
struct amdgpu_bo_param bp;
struct amdgpu_bo *root;
-   struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
int r, i;
 
vm->va = RB_ROOT_CACHED;
@@ -2701,19 +2700,17 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
spin_lock_init(>invalidated_lock);
INIT_LIST_HEAD(>freed);
 
-   for (i = 0; i < adev->vm_manager.vm_pte_num_rqs; i++)
-   sched_list[i] = adev->vm_manager.vm_pte_rqs[i]->sched;
 
/* create scheduler entities for page table updates */
r = drm_sched_entity_init(>direct, DRM_SCHED_PRIORITY_NORMAL,
- sched_list, adev->vm_manager.vm_pte_num_rqs,
- NULL);
+ adev->vm_manager.vm_pte_scheds,
+ adev->vm_manager.vm_pte_num_scheds, NULL);
if (r)
return r;
 
r = drm_sched_entity_init(>delayed, DRM_SCHED_PRIORITY_NORMAL,
- sched_list, adev->vm_manager.vm_pte_num_rqs,
- NULL);
+ adev->vm_manager.vm_pte_scheds,
+ adev->vm_manager.vm_pte_num_scheds, NULL);
if (r)
goto error_free_direct;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 76fcf853035c..5eaba8645a43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -322,8 +322,8 @@ struct amdgpu_vm_manager {
u64 vram_base_offset;
/* vm pte handling */
const struct amdgpu_vm_pte_funcs*vm_pte_funcs;
-   struct drm_sched_rq *vm_pte_rqs[AMDGPU_MAX_RINGS];
-   unsignedvm_pte_num_rqs;
+   struct drm_gpu_scheduler
*vm_pte_scheds[AMDGPU_MAX_RINGS];
+   unsignedvm_pte_num_scheds;
struct amdgpu_ring  *page_fault;
 
/* partial resident texture handling */
diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c 
b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
index 82cdb8f57bfd..1f22a8d0f7f3 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
@@ -1373,16 +1373,14 @@ static const struct amdgpu_vm_pte_funcs 
cik_sdma_vm_pte_funcs = {
 
 static void cik_sdma_set_vm_pte_funcs(struct amdgpu_device *adev)
 {
-   struct drm_gpu_scheduler *sched;
unsigned i;
 
adev->vm_manager.vm_pte_funcs = _sdma_vm_pte_funcs;
for (i = 0; i < adev->sdma.num_instances; i++) {
-   sched = >sdma.instance[i].ring.sched;
-   adev->vm_manager.vm_pte_rqs[i] =
-   >sched_rq[DRM_SCHED_PRIORITY_KERNEL];
+   adev->vm_manager.vm_pte_scheds[i] =
+   >sdma.instance[i].ring.sched;
}
-   adev->vm_manager.vm_pte_num_rqs = adev->sdma.num_instances;
+   adev->vm_manager.vm_pte_num_scheds = adev->sdma.num_instances;
 }
 
 const struct 

[PATCH 3/4] amd/amdgpu: add sched list to IPs with multiple run-queues

[Nirmoy Das]

This sched list can be passed on to entity creation routine
instead of manually creating such sched list on every context creation.

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c| 69 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 44 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h|  4 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h   |  2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h   |  2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h|  9 ++-
 6 files changed, 85 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 1d6850af9908..c1fc75299b7d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -74,7 +74,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
   struct amdgpu_ctx *ctx)
 {
unsigned num_entities = amdgpu_ctx_total_num_entities();
-   unsigned i, j, k;
+   unsigned i, j;
int r;
 
if (priority < 0 || priority >= DRM_SCHED_PRIORITY_MAX)
@@ -121,73 +121,56 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
ctx->override_priority = DRM_SCHED_PRIORITY_UNSET;
 
for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
-   struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
-   struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS];
-   unsigned num_rings = 0;
-   unsigned num_rqs = 0;
+   struct drm_gpu_scheduler **sched_list;
+   struct drm_gpu_scheduler *sched;
+   unsigned num_scheds = 0;
 
switch (i) {
case AMDGPU_HW_IP_GFX:
-   rings[0] = >gfx.gfx_ring[0];
-   num_rings = 1;
+   sched_list = adev->gfx.gfx_sched_list;
+   num_scheds = 1;
break;
case AMDGPU_HW_IP_COMPUTE:
-   for (j = 0; j < adev->gfx.num_compute_rings; ++j)
-   rings[j] = >gfx.compute_ring[j];
-   num_rings = adev->gfx.num_compute_rings;
+   sched_list = adev->gfx.compute_sched_list;
+   num_scheds = adev->gfx.num_compute_rings;
break;
case AMDGPU_HW_IP_DMA:
-   for (j = 0; j < adev->sdma.num_instances; ++j)
-   rings[j] = >sdma.instance[j].ring;
-   num_rings = adev->sdma.num_instances;
+   sched_list = adev->sdma.sdma_sched_list;
+   num_scheds = adev->sdma.num_instances;
break;
case AMDGPU_HW_IP_UVD:
-   rings[0] = >uvd.inst[0].ring;
-   num_rings = 1;
+   sched = >uvd.inst[0].ring.sched;
+   sched_list = 
+   num_scheds = 1;
break;
case AMDGPU_HW_IP_VCE:
-   rings[0] = >vce.ring[0];
-   num_rings = 1;
+   sched = >vce.ring[0].sched;
+   sched_list = 
+   num_scheds = 1;
break;
case AMDGPU_HW_IP_UVD_ENC:
-   rings[0] = >uvd.inst[0].ring_enc[0];
-   num_rings = 1;
+   sched = >uvd.inst[0].ring_enc[0].sched;
+   sched_list = 
+   num_scheds = 1;
break;
case AMDGPU_HW_IP_VCN_DEC:
-   for (j = 0; j < adev->vcn.num_vcn_inst; ++j) {
-   if (adev->vcn.harvest_config & (1 << j))
-   continue;
-   rings[num_rings++] = 
>vcn.inst[j].ring_dec;
-   }
+   sched_list = adev->vcn.vcn_dec_sched_list;
+   num_scheds =  adev->vcn.num_vcn_dec_sched_list;
break;
case AMDGPU_HW_IP_VCN_ENC:
-   for (j = 0; j < adev->vcn.num_vcn_inst; ++j) {
-   if (adev->vcn.harvest_config & (1 << j))
-   continue;
-   for (k = 0; k < adev->vcn.num_enc_rings; ++k)
-   rings[num_rings++] = 
>vcn.inst[j].ring_enc[k];
-   }
+   sched_list = adev->vcn.vcn_enc_sched_list;
+   num_scheds =  adev->vcn.num_vcn_enc_sched_list;
break;
case AMDGPU_HW_IP_VCN_JPEG:
-   for (j = 0; j < adev->jpeg.num_jpeg_inst; ++j) {
-   if (adev->vcn.harvest_config & (1 << j))
-   

Re: [PATCH 2/2] drm/amdgpu: fix JPEG instance checking when ctx init

[Deucher, Alexander]

[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Alex Deucher 

From: amd-gfx  on behalf of Leo Liu 

Sent: Monday, December 9, 2019 4:10 PM
To: amd-gfx@lists.freedesktop.org 
Cc: Liu, Leo 
Subject: [PATCH 2/2] drm/amdgpu: fix JPEG instance checking when ctx init

Fixes: 0388aee76("drm/amdgpu: use the JPEG structure for
general driver support")

Signed-off-by: Leo Liu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index a0d3d7b756eb..db4b6283c28c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -170,7 +170,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
 break;
 case AMDGPU_HW_IP_VCN_JPEG:
 for (j = 0; j < adev->jpeg.num_jpeg_inst; ++j) {
-   if (adev->vcn.harvest_config & (1 << j))
+   if (adev->jpeg.harvest_config & (1 << j))
 continue;
 rings[num_rings++] = 
>jpeg.inst[j].ring_dec;
 }
--
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Calexander.deucher%40amd.com%7Ca5db83292d3844d8955908d77cec5306%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637115227328960060sdata=mWsj0v7AxwKQVL1FzSn%2F6QhASdd4NxUQmMl9pCX7vTQ%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: fix VCN2.x number of irq types

[James Zhu]

Reviewed-by: James Zhu  for the series

On 2019-12-09 4:10 p.m., Leo Liu wrote:

The JPEG irq type has been moved to its own structure

Signed-off-by: Leo Liu 
---
  drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 2 +-
  drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index 5649190cb629..d76ece38c97b 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -1788,7 +1788,7 @@ static const struct amdgpu_irq_src_funcs 
vcn_v2_0_irq_funcs = {
  
  static void vcn_v2_0_set_irq_funcs(struct amdgpu_device *adev)

  {
-   adev->vcn.inst->irq.num_types = adev->vcn.num_enc_rings + 2;
+   adev->vcn.inst->irq.num_types = adev->vcn.num_enc_rings + 1;
adev->vcn.inst->irq.funcs = _v2_0_irq_funcs;
  }
  
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c

index 42d6b9f0553b..f67fca38c1a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -1138,7 +1138,7 @@ static void vcn_v2_5_set_irq_funcs(struct amdgpu_device 
*adev)
for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
if (adev->vcn.harvest_config & (1 << i))
continue;
-   adev->vcn.inst[i].irq.num_types = adev->vcn.num_enc_rings + 2;
+   adev->vcn.inst[i].irq.num_types = adev->vcn.num_enc_rings + 1;
adev->vcn.inst[i].irq.funcs = _v2_5_irq_funcs;
}
  }

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

[PATCH 2/2] drm/amdgpu: fix JPEG instance checking when ctx init

[Leo Liu]

Fixes: 0388aee76("drm/amdgpu: use the JPEG structure for
general driver support")

Signed-off-by: Leo Liu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index a0d3d7b756eb..db4b6283c28c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -170,7 +170,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
break;
case AMDGPU_HW_IP_VCN_JPEG:
for (j = 0; j < adev->jpeg.num_jpeg_inst; ++j) {
-   if (adev->vcn.harvest_config & (1 << j))
+   if (adev->jpeg.harvest_config & (1 << j))
continue;
rings[num_rings++] = 
>jpeg.inst[j].ring_dec;
}
-- 
2.17.1

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

[PATCH 1/2] drm/amdgpu: fix VCN2.x number of irq types

[Leo Liu]

The JPEG irq type has been moved to its own structure

Signed-off-by: Leo Liu 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index 5649190cb629..d76ece38c97b 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -1788,7 +1788,7 @@ static const struct amdgpu_irq_src_funcs 
vcn_v2_0_irq_funcs = {
 
 static void vcn_v2_0_set_irq_funcs(struct amdgpu_device *adev)
 {
-   adev->vcn.inst->irq.num_types = adev->vcn.num_enc_rings + 2;
+   adev->vcn.inst->irq.num_types = adev->vcn.num_enc_rings + 1;
adev->vcn.inst->irq.funcs = _v2_0_irq_funcs;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index 42d6b9f0553b..f67fca38c1a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -1138,7 +1138,7 @@ static void vcn_v2_5_set_irq_funcs(struct amdgpu_device 
*adev)
for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
if (adev->vcn.harvest_config & (1 << i))
continue;
-   adev->vcn.inst[i].irq.num_types = adev->vcn.num_enc_rings + 2;
+   adev->vcn.inst[i].irq.num_types = adev->vcn.num_enc_rings + 1;
adev->vcn.inst[i].irq.funcs = _v2_5_irq_funcs;
}
 }
-- 
2.17.1

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

Re: [PATCH 1/3] tests/amdgpu: Fix various warnings

[Christian König]

Am 09.12.19 um 19:57 schrieb Luben Tuikov:

This patch fixes the following warnings:
-Wformat=
-Wmaybe-uninitialized
-Wmisleading-indentation
-Wstringop-truncation
-Wunused-function
-Wunused-variable

It also removes forward declarations and moves
global functions to the bottom, keeping locals
at the top, in ras_tests.c.

Signed-off-by: Luben Tuikov 


Acked-by: Christian König  for the series.


---
  amdgpu/amdgpu_bo.c   |   2 -
  tests/amdgpu/basic_tests.c   |   7 +-
  tests/amdgpu/bo_tests.c  |   9 +-
  tests/amdgpu/cs_tests.c  |   1 +
  tests/amdgpu/ras_tests.c | 241 +--
  tests/amdgpu/syncobj_tests.c |   2 +-
  6 files changed, 124 insertions(+), 138 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index c54687ed..d6ea0e74 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -396,7 +396,6 @@ unlock:
  drm_public int amdgpu_get_fb_id(amdgpu_device_handle dev, unsigned int *fb_id)
  {
drmModeResPtr mode_res;
-   int count_crtcs;
drmModeCrtcPtr mode_crtc;
int current_id = 0;
int r = 0;
@@ -421,7 +420,6 @@ drm_public int amdgpu_get_fb_id(amdgpu_device_handle dev, 
unsigned int *fb_id)
if (!mode_res)
return EFAULT;
  
-	count_crtcs = mode_res->count_crtcs;

for (i = 0; i < mode_res->count_crtcs; i++) {
mode_crtc = drmModeGetCrtc(fd, mode_res->crtcs[i]);
if (mode_crtc) {
diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c
index c3c09702..cce0640a 100644
--- a/tests/amdgpu/basic_tests.c
+++ b/tests/amdgpu/basic_tests.c
@@ -3231,7 +3231,7 @@ static void amdgpu_memcpy_draw(amdgpu_device_handle 
device_handle,
int bo_cmd_size = 4096;
struct amdgpu_cs_request ibs_request = {0};
struct amdgpu_cs_ib_info ib_info= {0};
-   uint32_t hang_state, hangs, expired;
+   uint32_t expired;
amdgpu_bo_list_handle bo_list;
struct amdgpu_cs_fence fence_status = {0};
  
@@ -3479,10 +3479,11 @@ amdgpu_direct_gma_bo_alloc_and_map(amdgpu_device_handle dev, amdgpu_device_handl

goto error_va_map;
}
  
-	if (cpu)

+   if (cpu) {
r = amdgpu_bo_cpu_map(buf_handle_import, cpu);
if(r)
goto error_va_map;
+   }
  
  	*bo = buf_handle;

*bo_peer = buf_handle_import;
@@ -3610,7 +3611,7 @@ static void amdgpu_direct_gma_mmap(amdgpu_device_handle 
dev,
amdgpu_bo_handle *buf_handle_import;
volatile uint8_t **ptr;
struct drm_amdgpu_capability cap;
-   uint64_t size = 4096, phys_addr, remain;
+   uint64_t size = 4096, remain;
int i, j, r, tst_loop = 20;
  
  	buf_handle = calloc(tst_loop, sizeof(*buf_handle));

diff --git a/tests/amdgpu/bo_tests.c b/tests/amdgpu/bo_tests.c
index 27048c88..7fcabb85 100644
--- a/tests/amdgpu/bo_tests.c
+++ b/tests/amdgpu/bo_tests.c
@@ -325,8 +325,7 @@ static void amdgpu_bo_find_by_cpu_mapping(void)
  }
  static void amdgpu_get_fb_id_and_handle(void)
  {
-   uint32_t *ptr;
-   int i, r;
+   int r;
unsigned int fb_id;
struct amdgpu_bo_import_result output;
  
@@ -352,7 +351,7 @@ static void amdgpu_bo_ssg(void)

int i, j, fd;
uint64_t pattern = 0xdeadbeef12345678, out;
void *buf;
-   bool write_is_ok;
+   bool write_is_ok = false;
  
  	CU_ASSERT(!amdgpu_query_capability(device_handle, ));

if(!(cap.flag & AMDGPU_CAPABILITY_SSG_FLAG)) {
@@ -363,7 +362,7 @@ static void amdgpu_bo_ssg(void)
if (buf_size > cap.direct_gma_size << 20)
buf_size = cap.direct_gma_size << 20;
  
-	printf("SSG read/write block size 0x%x\n", buf_size);

+   printf("SSG read/write block size 0x%lx\n", buf_size);
  
  	CU_ASSERT((fd = open(in_file, O_WRONLY | O_CREAT, S_IRWXU)) >= 0);

for (i = 0; i < buf_size; i += sizeof(pattern)) {
@@ -413,7 +412,7 @@ static void amdgpu_bo_ssg(void)
for (i = 0; i < 3; i++) {
struct timespec ts1, ts2;
double a, b, c;
-   bool write_is_same;
+   bool write_is_same = false;
  
  		CU_ASSERT((fd = open(out_file, O_WRONLY | O_CREAT | O_DIRECT, S_IRWXU)) >= 0);
  
diff --git a/tests/amdgpu/cs_tests.c b/tests/amdgpu/cs_tests.c

index c0903a2a..10124c15 100644
--- a/tests/amdgpu/cs_tests.c
+++ b/tests/amdgpu/cs_tests.c
@@ -362,6 +362,7 @@ static void amdgpu_cs_uvd_decode(void)
bs_addr = fb_addr + 4*1024;
dpb_addr = ALIGN(bs_addr + sizeof(uvd_bitstream), 4*1024);
  
+	ctx_addr = 0;

if (family_id >= AMDGPU_FAMILY_VI) {
if ((family_id == AMDGPU_FAMILY_AI) ||
(chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A ||
diff --git a/tests/amdgpu/ras_tests.c b/tests/amdgpu/ras_tests.c
index c1c543c1..d714be73 100644
--- a/tests/amdgpu/ras_tests.c
+++ b/tests/amdgpu/ras_tests.c
@@ -522,124 +522,6 @@ static int 

[PATCH V4] drm: Add support for DP 1.4 Compliance edid corruption test

[Jerry (Fangzhi) Zuo]

Unlike DP 1.2 edid corruption test, DP 1.4 requires to calculate
real CRC value of the last edid data block, and write it back.
Current edid CRC calculates routine adds the last CRC byte,
and check if non-zero.

This behavior is not accurate; actually, we need to return
the actual CRC value when corruption is detected.
This commit changes this issue by returning the calculated CRC,
and initiate the required sequence.

Change since v4
- Fix for CI.CHECKPATCH

Change since v3
- Fix a minor typo.

Change since v2
- Rewrite checksum computation routine to avoid duplicated code.
- Rename to avoid confusion.

Change since v1
- Have separate routine for returning real CRC.

Signed-off-by: Jerry (Fangzhi) Zuo 
---
 drivers/gpu/drm/drm_dp_helper.c | 35 +
 drivers/gpu/drm/drm_edid.c  | 23 ++
 include/drm/drm_connector.h |  6 ++
 include/drm/drm_dp_helper.h |  3 +++
 4 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 2c7870aef469..85a777ce98ba 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -351,6 +351,41 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 }
 EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
 
+/**
+ * drm_dp_send_real_edid_checksum() - send back real edid checksum value
+ * @aux: DisplayPort AUX channel
+ * @real_edid_checksum: real edid checksum for the last block
+ *
+ * Returns true on success
+ */
+bool drm_dp_send_real_edid_checksum(struct drm_dp_aux *aux,
+u8 real_edid_checksum)
+{
+   u8 link_edid_read = 0, auto_test_req = 0, test_resp = 0;
+
+   drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, _test_req, 1);
+   auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
+
+   drm_dp_dpcd_read(aux, DP_TEST_REQUEST, _edid_read, 1);
+   link_edid_read &= DP_TEST_LINK_EDID_READ;
+
+   if (!auto_test_req || !link_edid_read) {
+   DRM_DEBUG_KMS("Source DUT does not support TEST_EDID_READ\n");
+   return false;
+   }
+
+   drm_dp_dpcd_write(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, _test_req, 1);
+
+   /* send back checksum for the last edid extension block data */
+   drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, _edid_checksum, 1);
+
+   test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
+   drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, _resp, 1);
+
+   return true;
+}
+EXPORT_SYMBOL(drm_dp_send_real_edid_checksum);
+
 /**
  * drm_dp_downstream_max_clock() - extract branch device max
  * pixel rate for legacy VGA
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5b33b7cfd645..0e35405ecc74 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1450,11 +1450,22 @@ static int validate_displayid(u8 *displayid, int 
length, int idx);
 static int drm_edid_block_checksum(const u8 *raw_edid)
 {
int i;
-   u8 csum = 0;
-   for (i = 0; i < EDID_LENGTH; i++)
+   u8 csum = 0, crc = 0;
+
+   for (i = 0; i < EDID_LENGTH - 1; i++)
csum += raw_edid[i];
 
-   return csum;
+   crc = 0x100 - csum;
+
+   return crc;
+}
+
+static bool drm_edid_block_checksum_diff(const u8 *raw_edid, u8 real_checksum)
+{
+   if (raw_edid[EDID_LENGTH - 1] != real_checksum)
+   return true;
+   else
+   return false;
 }
 
 static bool drm_edid_is_zero(const u8 *in_edid, int length)
@@ -1512,7 +1523,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool 
print_bad_edid,
}
 
csum = drm_edid_block_checksum(raw_edid);
-   if (csum) {
+   if (drm_edid_block_checksum_diff(raw_edid, csum)) {
if (edid_corrupt)
*edid_corrupt = true;
 
@@ -1653,6 +1664,7 @@ static void connector_bad_edid(struct drm_connector 
*connector,
   u8 *edid, int num_blocks)
 {
int i;
+   u8 num_of_ext = edid[0x7e];
 
if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS))
return;
@@ -1675,6 +1687,9 @@ static void connector_bad_edid(struct drm_connector 
*connector,
   prefix, DUMP_PREFIX_NONE, 16, 1,
   block, EDID_LENGTH, false);
}
+
+   /* Calculate real checksum for the last edid extension block data */
+   connector->real_edid_checksum = drm_edid_block_checksum(edid + 
num_of_ext * EDID_LENGTH);
 }
 
 /* Get override or firmware EDID */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 5f8c3389d46f..b07d8276a58c 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1349,6 +1349,12 @@ struct drm_connector {
 * rev1.1 4.2.2.6
 */
bool edid_corrupt;
+   /**
+* @real_edid_checksum: real edid checksum for corrupted edid block.
+* 

[PATCH 3/3] tests/amdgpu: Fix buffer overflow

[Luben Tuikov]

This patch fixes the following warning:
-Wformat-overflow=

Signed-off-by: Luben Tuikov 
---
 tests/amdgpu/meson.build |  1 +
 tests/amdgpu/ras_tests.c | 41 
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/tests/amdgpu/meson.build b/tests/amdgpu/meson.build
index 2a48b43f..faaf0ee6 100644
--- a/tests/amdgpu/meson.build
+++ b/tests/amdgpu/meson.build
@@ -28,6 +28,7 @@ if dep_cunit.found()
 ),
 dependencies : [dep_cunit, dep_threads],
 include_directories : [inc_root, inc_drm, 
include_directories('../../amdgpu')],
+link_args : ['-lbsd'],
 link_with : [libdrm, libdrm_amdgpu],
 install : with_install_tests,
   )
diff --git a/tests/amdgpu/ras_tests.c b/tests/amdgpu/ras_tests.c
index f745166b..c96cde45 100644
--- a/tests/amdgpu/ras_tests.c
+++ b/tests/amdgpu/ras_tests.c
@@ -30,6 +30,10 @@
 #include 
 #include 
 #include "xf86drm.h"
+#include 
+#include 
+
+#define PATH_SIZE PATH_MAX
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
 
@@ -498,7 +502,7 @@ static int get_file_contents(char *file, char *buf, int 
size);
 
 static int amdgpu_ras_lookup_id(drmDevicePtr device)
 {
-   char path[1024];
+   char path[PATH_SIZE];
char str[128];
drmPciBusInfo info;
int i;
@@ -525,8 +529,8 @@ static int amdgpu_ras_lookup_id(drmDevicePtr device)
 //helpers
 
 static int test_card;
-static char sysfs_path[1024];
-static char debugfs_path[1024];
+static char sysfs_path[PATH_SIZE];
+static char debugfs_path[PATH_SIZE];
 static uint32_t ras_mask;
 static amdgpu_device_handle device_handle;
 
@@ -605,10 +609,11 @@ static int amdgpu_ras_is_feature_supported(enum 
amdgpu_ras_block block)
 
 static int amdgpu_ras_invoke(struct ras_debug_if *data)
 {
-   char path[1024];
+   char path[PATH_SIZE];
int ret;
 
-   sprintf(path, "%s%s", get_ras_debugfs_root(), "ras_ctrl");
+   snprintf(path, sizeof(path), "%s", get_ras_debugfs_root());
+   strlcpy(path, "ras_ctrl", sizeof(path));
 
ret = set_file_contents(path, (char *)data, sizeof(*data))
- sizeof(*data);
@@ -619,14 +624,16 @@ static int amdgpu_ras_query_err_count(enum 
amdgpu_ras_block block,
unsigned long *ue, unsigned long *ce)
 {
char buf[64];
-   char name[1024];
+   char name[PATH_SIZE];
 
*ue = *ce = 0;
 
if (amdgpu_ras_is_feature_supported(block) <= 0)
return -1;
 
-   sprintf(name, "%s%s%s", get_ras_sysfs_root(), ras_block_str(block), 
"_err_count");
+   snprintf(name, sizeof(name), "%s", get_ras_sysfs_root());
+   strlcpy(name, ras_block_str(block), sizeof(name));
+   strlcpy(name, "_err_count", sizeof(name));
 
if (is_file_ok(name, O_RDONLY))
return 0;
@@ -837,7 +844,7 @@ static void amdgpu_ras_basic_test(void)
int i;
int j;
uint32_t features;
-   char path[1024];
+   char path[PATH_SIZE];
 
ret = is_file_ok("/sys/module/amdgpu/parameters/ras_mask", O_RDONLY);
CU_ASSERT_EQUAL(ret, 0);
@@ -849,11 +856,15 @@ static void amdgpu_ras_basic_test(void)
sizeof(features), );
CU_ASSERT_EQUAL(ret, 0);
 
-   sprintf(path, "%s%s", get_ras_debugfs_root(), "ras_ctrl");
+   snprintf(path, sizeof(path), "%s", get_ras_debugfs_root());
+   strlcpy(path, "ras_ctrl", sizeof(path));
+
ret = is_file_ok(path, O_WRONLY);
CU_ASSERT_EQUAL(ret, 0);
 
-   sprintf(path, "%s%s", get_ras_sysfs_root(), "features");
+   snprintf(path, sizeof(path), "%s", get_ras_sysfs_root());
+   strlcpy(path, "features", sizeof(path));
+
ret = is_file_ok(path, O_RDONLY);
CU_ASSERT_EQUAL(ret, 0);
 
@@ -865,11 +876,17 @@ static void amdgpu_ras_basic_test(void)
if (!((1 << j) & ras_block_mask_basic))
continue;
 
-   sprintf(path, "%s%s%s", get_ras_sysfs_root(), 
ras_block_str(j), "_err_count");
+   snprintf(path, sizeof(path), "%s", 
get_ras_sysfs_root());
+   strlcpy(path, ras_block_str(j), sizeof(path));
+   strlcpy(path, "_err_count", sizeof(path));
+
ret = is_file_ok(path, O_RDONLY);
CU_ASSERT_EQUAL(ret, 0);
 
-   sprintf(path, "%s%s%s", get_ras_debugfs_root(), 
ras_block_str(j), "_err_inject");
+   snprintf(path, sizeof(path), "%s", 
get_ras_debugfs_root());
+   strlcpy(path, ras_block_str(j), sizeof(path));
+   strlcpy(path, "_err_inject", sizeof(path));
+
ret = is_file_ok(path, O_WRONLY);
CU_ASSERT_EQUAL(ret, 0);
}
-- 
2.24.0.390.g083378cc35

___
amd-gfx 

[PATCH 2/3] tests/amdgpu: Fix unused function warning (v2)

[Luben Tuikov]

This patch fixes:
-Wunused-function

v2: Always enable amdgpu_ras_test().

Signed-off-by: Luben Tuikov 
---
 tests/amdgpu/ras_tests.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/amdgpu/ras_tests.c b/tests/amdgpu/ras_tests.c
index d714be73..f745166b 100644
--- a/tests/amdgpu/ras_tests.c
+++ b/tests/amdgpu/ras_tests.c
@@ -881,9 +881,7 @@ CU_TestInfo ras_tests[] = {
{ "ras query test", amdgpu_ras_query_test },
{ "ras inject test",amdgpu_ras_inject_test },
{ "ras disable test",   amdgpu_ras_disable_test },
-#if 0
{ "ras enable test",amdgpu_ras_enable_test },
-#endif
CU_TEST_INFO_NULL,
 };
 
-- 
2.24.0.390.g083378cc35

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

[PATCH 1/3] tests/amdgpu: Fix various warnings

[Luben Tuikov]

This patch fixes the following warnings:
-Wformat=
-Wmaybe-uninitialized
-Wmisleading-indentation
-Wstringop-truncation
-Wunused-function
-Wunused-variable

It also removes forward declarations and moves
global functions to the bottom, keeping locals
at the top, in ras_tests.c.

Signed-off-by: Luben Tuikov 
---
 amdgpu/amdgpu_bo.c   |   2 -
 tests/amdgpu/basic_tests.c   |   7 +-
 tests/amdgpu/bo_tests.c  |   9 +-
 tests/amdgpu/cs_tests.c  |   1 +
 tests/amdgpu/ras_tests.c | 241 +--
 tests/amdgpu/syncobj_tests.c |   2 +-
 6 files changed, 124 insertions(+), 138 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index c54687ed..d6ea0e74 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -396,7 +396,6 @@ unlock:
 drm_public int amdgpu_get_fb_id(amdgpu_device_handle dev, unsigned int *fb_id)
 {
drmModeResPtr mode_res;
-   int count_crtcs;
drmModeCrtcPtr mode_crtc;
int current_id = 0;
int r = 0;
@@ -421,7 +420,6 @@ drm_public int amdgpu_get_fb_id(amdgpu_device_handle dev, 
unsigned int *fb_id)
if (!mode_res)
return EFAULT;
 
-   count_crtcs = mode_res->count_crtcs;
for (i = 0; i < mode_res->count_crtcs; i++) {
mode_crtc = drmModeGetCrtc(fd, mode_res->crtcs[i]);
if (mode_crtc) {
diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c
index c3c09702..cce0640a 100644
--- a/tests/amdgpu/basic_tests.c
+++ b/tests/amdgpu/basic_tests.c
@@ -3231,7 +3231,7 @@ static void amdgpu_memcpy_draw(amdgpu_device_handle 
device_handle,
int bo_cmd_size = 4096;
struct amdgpu_cs_request ibs_request = {0};
struct amdgpu_cs_ib_info ib_info= {0};
-   uint32_t hang_state, hangs, expired;
+   uint32_t expired;
amdgpu_bo_list_handle bo_list;
struct amdgpu_cs_fence fence_status = {0};
 
@@ -3479,10 +3479,11 @@ amdgpu_direct_gma_bo_alloc_and_map(amdgpu_device_handle 
dev, amdgpu_device_handl
goto error_va_map;
}
 
-   if (cpu)
+   if (cpu) {
r = amdgpu_bo_cpu_map(buf_handle_import, cpu);
if(r)
goto error_va_map;
+   }
 
*bo = buf_handle;
*bo_peer = buf_handle_import;
@@ -3610,7 +3611,7 @@ static void amdgpu_direct_gma_mmap(amdgpu_device_handle 
dev,
amdgpu_bo_handle *buf_handle_import;
volatile uint8_t **ptr;
struct drm_amdgpu_capability cap;
-   uint64_t size = 4096, phys_addr, remain;
+   uint64_t size = 4096, remain;
int i, j, r, tst_loop = 20;
 
buf_handle = calloc(tst_loop, sizeof(*buf_handle));
diff --git a/tests/amdgpu/bo_tests.c b/tests/amdgpu/bo_tests.c
index 27048c88..7fcabb85 100644
--- a/tests/amdgpu/bo_tests.c
+++ b/tests/amdgpu/bo_tests.c
@@ -325,8 +325,7 @@ static void amdgpu_bo_find_by_cpu_mapping(void)
 }
 static void amdgpu_get_fb_id_and_handle(void)
 {
-   uint32_t *ptr;
-   int i, r;
+   int r;
unsigned int fb_id;
struct amdgpu_bo_import_result output;
 
@@ -352,7 +351,7 @@ static void amdgpu_bo_ssg(void)
int i, j, fd;
uint64_t pattern = 0xdeadbeef12345678, out;
void *buf;
-   bool write_is_ok;
+   bool write_is_ok = false;
 
CU_ASSERT(!amdgpu_query_capability(device_handle, ));
if(!(cap.flag & AMDGPU_CAPABILITY_SSG_FLAG)) {
@@ -363,7 +362,7 @@ static void amdgpu_bo_ssg(void)
if (buf_size > cap.direct_gma_size << 20)
buf_size = cap.direct_gma_size << 20;
 
-   printf("SSG read/write block size 0x%x\n", buf_size);
+   printf("SSG read/write block size 0x%lx\n", buf_size);
 
CU_ASSERT((fd = open(in_file, O_WRONLY | O_CREAT, S_IRWXU)) >= 0);
for (i = 0; i < buf_size; i += sizeof(pattern)) {
@@ -413,7 +412,7 @@ static void amdgpu_bo_ssg(void)
for (i = 0; i < 3; i++) {
struct timespec ts1, ts2;
double a, b, c;
-   bool write_is_same;
+   bool write_is_same = false;
 
CU_ASSERT((fd = open(out_file, O_WRONLY | O_CREAT | O_DIRECT, 
S_IRWXU)) >= 0);
 
diff --git a/tests/amdgpu/cs_tests.c b/tests/amdgpu/cs_tests.c
index c0903a2a..10124c15 100644
--- a/tests/amdgpu/cs_tests.c
+++ b/tests/amdgpu/cs_tests.c
@@ -362,6 +362,7 @@ static void amdgpu_cs_uvd_decode(void)
bs_addr = fb_addr + 4*1024;
dpb_addr = ALIGN(bs_addr + sizeof(uvd_bitstream), 4*1024);
 
+   ctx_addr = 0;
if (family_id >= AMDGPU_FAMILY_VI) {
if ((family_id == AMDGPU_FAMILY_AI) ||
(chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A ||
diff --git a/tests/amdgpu/ras_tests.c b/tests/amdgpu/ras_tests.c
index c1c543c1..d714be73 100644
--- a/tests/amdgpu/ras_tests.c
+++ b/tests/amdgpu/ras_tests.c
@@ -522,124 +522,6 @@ static int amdgpu_ras_lookup_id(drmDevicePtr device)
return -1;
 }
 
-CU_BOOL 

[PATCH] drm/amd/powerplay: avoid null pointer

[Yintian Tao]

because some asics have no smu.ppt_funcs
we need add one check for it otherwise
it will raise null pointer problem.

Signed-off-by: Yintian Tao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index a21ee035ca57..b8a42ebb2f5a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -160,7 +160,8 @@ static ssize_t amdgpu_get_dpm_state(struct device *dev,
enum amd_pm_state_type pm;
 
if (is_support_sw_smu(adev)) {
-   if (adev->smu.ppt_funcs->get_current_power_state)
+   if (adev->smu.ppt_funcs &&
+   adev->smu.ppt_funcs->get_current_power_state)
pm = smu_get_current_power_state(>smu);
else
pm = adev->pm.dpm.user_state;
-- 
2.17.1

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

Re: [PATCH] drm/amd/powerplay: enable pp one vf mode for vega10

[Alex Deucher]

On Mon, Dec 9, 2019 at 12:16 PM Tao, Yintian  wrote:
>
> Hi  Alex
>
> Many thanks for your review. Please see inline comments. I will submit the v3 
> patch according to your comment. Thanks again.
>
> Best Regards
> Yintian Tao
>
> -Original Message-
> From: Alex Deucher 
> Sent: 2019年12月10日 0:52
> To: Tao, Yintian 
> Cc: Deucher, Alexander ; Feng, Kenneth 
> ; amd-gfx list 
> Subject: Re: [PATCH] drm/amd/powerplay: enable pp one vf mode for vega10
>
> On Mon, Dec 9, 2019 at 5:39 AM Yintian Tao  wrote:
> >
> > Originally, due to the restriction from PSP and SMU, VF has
> > to send message to hypervisor driver to handle powerplay
> > change which is complicated and redundant. Currently, SMU
> > and PSP can support VF to directly handle powerplay
> > change by itself. Therefore, the old code about the handshake
> > between VF and PF to handle powerplay will be removed and VF
> > will use new the registers below to handshake with SMU.
> > mmMP1_SMN_C2PMSG_101: register to handle SMU message
> > mmMP1_SMN_C2PMSG_102: register to handle SMU parameter
> > mmMP1_SMN_C2PMSG_103: register to handle SMU response
> >
> > v2: remove module parameter pp_one_vf
> >
> > Signed-off-by: Yintian Tao 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|  13 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |   4 -
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 121 +++---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c  |  51 --
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h  |  14 +-
> >  drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c |  78 -
> >  drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h |   4 -
> >  drivers/gpu/drm/amd/amdgpu/soc15.c|   8 +-
> >  drivers/gpu/drm/amd/powerplay/amd_powerplay.c |   4 +-
> >  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c|  24 +--
> >  .../drm/amd/powerplay/hwmgr/hardwaremanager.c |  15 +-
> >  drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c   |   7 +
> >  drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c  |  30 ++--
> >  .../drm/amd/powerplay/hwmgr/vega10_hwmgr.c| 157 --
> >  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h |   1 +
> >  .../drm/amd/powerplay/smumgr/smu9_smumgr.c|  55 --
> >  .../drm/amd/powerplay/smumgr/vega10_smumgr.c  |   6 +
> >  17 files changed, 280 insertions(+), 312 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index b9ca7e728d3e..30eb42593a20 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -1880,6 +1880,9 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
> > *adev)
> > }
> > }
> >
> > +   if (amdgpu_sriov_vf(adev))
> > +   amdgpu_virt_init_data_exchange(adev);
> > +
> > r = amdgpu_ib_pool_init(adev);
> > if (r) {
> > dev_err(adev->dev, "IB initialization failed (%d).\n", r);
> > @@ -1922,8 +1925,6 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
> > *adev)
> >
> >  init_failed:
> > if (amdgpu_sriov_vf(adev)) {
> > -   if (!r)
> > -   amdgpu_virt_init_data_exchange(adev);
> > amdgpu_virt_release_full_gpu(adev, true);
> > }
> >
> > @@ -2819,7 +2820,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> > mutex_init(>virt.vf_errors.lock);
> > hash_init(adev->mn_hash);
> > mutex_init(>lock_reset);
> > -   mutex_init(>virt.dpm_mutex);
> > mutex_init(>psp.mutex);
> >
> > r = amdgpu_device_check_arguments(adev);
> > @@ -3040,9 +3040,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> >
> > amdgpu_fbdev_init(adev);
> >
> > -   if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev))
> > -   amdgpu_pm_virt_sysfs_init(adev);
> > -
> > r = amdgpu_pm_sysfs_init(adev);
> > if (r) {
> > adev->pm_sysfs_en = false;
> > @@ -3187,8 +3184,6 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
> > iounmap(adev->rmmio);
> > adev->rmmio = NULL;
> > amdgpu_device_doorbell_fini(adev);
> > -   if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev))
> > -   amdgpu_pm_virt_sysfs_fini(adev);
> >
> > amdgpu_debugfs_regs_cleanup(adev);
> > device_remove_file(adev->dev, _attr_pcie_replay_count);
> > @@ -3669,6 +3664,7 @@ static int amdgpu_device_reset_sriov(struct 
> > amdgpu_device *adev,
> > if (r)
> > goto error;
> >
> > +   amdgpu_virt_init_data_exchange(adev);
> > /* we need recover gart prior to run SMC/CP/SDMA resume */
> > amdgpu_gtt_mgr_recover(>mman.bdev.man[TTM_PL_TT]);
> >
> > @@ -3686,7 +3682,6 @@ static int amdgpu_device_reset_sriov(struct 
> > amdgpu_device *adev,
> > amdgpu_amdkfd_post_reset(adev);
> >
> >  error:
> > -   amdgpu_virt_init_data_exchange(adev);
> > 

[PATCH] drm/amd/powerplay: enable pp one vf mode for vega10

[Yintian Tao]

Originally, due to the restriction from PSP and SMU, VF has
to send message to hypervisor driver to handle powerplay
change which is complicated and redundant. Currently, SMU
and PSP can support VF to directly handle powerplay
change by itself. Therefore, the old code about the handshake
between VF and PF to handle powerplay will be removed and VF
will use new the registers below to handshake with SMU.
mmMP1_SMN_C2PMSG_101: register to handle SMU message
mmMP1_SMN_C2PMSG_102: register to handle SMU parameter
mmMP1_SMN_C2PMSG_103: register to handle SMU response

v2: remove module parameter pp_one_vf
v3: separte one fix into another patch and fix the parens

Signed-off-by: Yintian Tao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|  13 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |   4 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 118 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c  |  51 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h  |  14 +-
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c |  78 -
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h |   4 -
 drivers/gpu/drm/amd/amdgpu/soc15.c|   8 +-
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c |   4 +-
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c|  24 +--
 .../drm/amd/powerplay/hwmgr/hardwaremanager.c |  15 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c   |   7 +
 drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c  |  30 ++--
 .../drm/amd/powerplay/hwmgr/vega10_hwmgr.c| 157 --
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h |   1 +
 .../drm/amd/powerplay/smumgr/smu9_smumgr.c|  56 +--
 .../drm/amd/powerplay/smumgr/vega10_smumgr.c  |   6 +
 17 files changed, 279 insertions(+), 311 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index b9ca7e728d3e..30eb42593a20 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1880,6 +1880,9 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)
}
}
 
+   if (amdgpu_sriov_vf(adev))
+   amdgpu_virt_init_data_exchange(adev);
+
r = amdgpu_ib_pool_init(adev);
if (r) {
dev_err(adev->dev, "IB initialization failed (%d).\n", r);
@@ -1922,8 +1925,6 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)
 
 init_failed:
if (amdgpu_sriov_vf(adev)) {
-   if (!r)
-   amdgpu_virt_init_data_exchange(adev);
amdgpu_virt_release_full_gpu(adev, true);
}
 
@@ -2819,7 +2820,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
mutex_init(>virt.vf_errors.lock);
hash_init(adev->mn_hash);
mutex_init(>lock_reset);
-   mutex_init(>virt.dpm_mutex);
mutex_init(>psp.mutex);
 
r = amdgpu_device_check_arguments(adev);
@@ -3040,9 +3040,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 
amdgpu_fbdev_init(adev);
 
-   if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev))
-   amdgpu_pm_virt_sysfs_init(adev);
-
r = amdgpu_pm_sysfs_init(adev);
if (r) {
adev->pm_sysfs_en = false;
@@ -3187,8 +3184,6 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
iounmap(adev->rmmio);
adev->rmmio = NULL;
amdgpu_device_doorbell_fini(adev);
-   if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev))
-   amdgpu_pm_virt_sysfs_fini(adev);
 
amdgpu_debugfs_regs_cleanup(adev);
device_remove_file(adev->dev, _attr_pcie_replay_count);
@@ -3669,6 +3664,7 @@ static int amdgpu_device_reset_sriov(struct amdgpu_device 
*adev,
if (r)
goto error;
 
+   amdgpu_virt_init_data_exchange(adev);
/* we need recover gart prior to run SMC/CP/SDMA resume */
amdgpu_gtt_mgr_recover(>mman.bdev.man[TTM_PL_TT]);
 
@@ -3686,7 +3682,6 @@ static int amdgpu_device_reset_sriov(struct amdgpu_device 
*adev,
amdgpu_amdkfd_post_reset(adev);
 
 error:
-   amdgpu_virt_init_data_exchange(adev);
amdgpu_virt_release_full_gpu(adev, true);
if (!r && adev->virt.gim_feature & AMDGIM_FEATURE_GIM_FLR_VRAMLOST) {
amdgpu_inc_vram_lost(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 5ec1415d1755..3a0ea9096498 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -703,10 +703,6 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
if (adev->pm.dpm_enabled) {
dev_info.max_engine_clock = amdgpu_dpm_get_sclk(adev, 
false) * 10;
dev_info.max_memory_clock = amdgpu_dpm_get_mclk(adev, 
false) * 10;
-   } else if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev) &&
-  adev->virt.ops->get_pp_clk) {
-   

Re: [PATCH] drm/amd/powerplay: enable pp one vf mode for vega10

[Alex Deucher]

On Mon, Dec 9, 2019 at 5:39 AM Yintian Tao  wrote:
>
> Originally, due to the restriction from PSP and SMU, VF has
> to send message to hypervisor driver to handle powerplay
> change which is complicated and redundant. Currently, SMU
> and PSP can support VF to directly handle powerplay
> change by itself. Therefore, the old code about the handshake
> between VF and PF to handle powerplay will be removed and VF
> will use new the registers below to handshake with SMU.
> mmMP1_SMN_C2PMSG_101: register to handle SMU message
> mmMP1_SMN_C2PMSG_102: register to handle SMU parameter
> mmMP1_SMN_C2PMSG_103: register to handle SMU response
>
> v2: remove module parameter pp_one_vf
>
> Signed-off-by: Yintian Tao 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|  13 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |   4 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 121 +++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c  |  51 --
>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h  |  14 +-
>  drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c |  78 -
>  drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h |   4 -
>  drivers/gpu/drm/amd/amdgpu/soc15.c|   8 +-
>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c |   4 +-
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c|  24 +--
>  .../drm/amd/powerplay/hwmgr/hardwaremanager.c |  15 +-
>  drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c   |   7 +
>  drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c  |  30 ++--
>  .../drm/amd/powerplay/hwmgr/vega10_hwmgr.c| 157 --
>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h |   1 +
>  .../drm/amd/powerplay/smumgr/smu9_smumgr.c|  55 --
>  .../drm/amd/powerplay/smumgr/vega10_smumgr.c  |   6 +
>  17 files changed, 280 insertions(+), 312 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index b9ca7e728d3e..30eb42593a20 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1880,6 +1880,9 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
> *adev)
> }
> }
>
> +   if (amdgpu_sriov_vf(adev))
> +   amdgpu_virt_init_data_exchange(adev);
> +
> r = amdgpu_ib_pool_init(adev);
> if (r) {
> dev_err(adev->dev, "IB initialization failed (%d).\n", r);
> @@ -1922,8 +1925,6 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
> *adev)
>
>  init_failed:
> if (amdgpu_sriov_vf(adev)) {
> -   if (!r)
> -   amdgpu_virt_init_data_exchange(adev);
> amdgpu_virt_release_full_gpu(adev, true);
> }
>
> @@ -2819,7 +2820,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> mutex_init(>virt.vf_errors.lock);
> hash_init(adev->mn_hash);
> mutex_init(>lock_reset);
> -   mutex_init(>virt.dpm_mutex);
> mutex_init(>psp.mutex);
>
> r = amdgpu_device_check_arguments(adev);
> @@ -3040,9 +3040,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>
> amdgpu_fbdev_init(adev);
>
> -   if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev))
> -   amdgpu_pm_virt_sysfs_init(adev);
> -
> r = amdgpu_pm_sysfs_init(adev);
> if (r) {
> adev->pm_sysfs_en = false;
> @@ -3187,8 +3184,6 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
> iounmap(adev->rmmio);
> adev->rmmio = NULL;
> amdgpu_device_doorbell_fini(adev);
> -   if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev))
> -   amdgpu_pm_virt_sysfs_fini(adev);
>
> amdgpu_debugfs_regs_cleanup(adev);
> device_remove_file(adev->dev, _attr_pcie_replay_count);
> @@ -3669,6 +3664,7 @@ static int amdgpu_device_reset_sriov(struct 
> amdgpu_device *adev,
> if (r)
> goto error;
>
> +   amdgpu_virt_init_data_exchange(adev);
> /* we need recover gart prior to run SMC/CP/SDMA resume */
> amdgpu_gtt_mgr_recover(>mman.bdev.man[TTM_PL_TT]);
>
> @@ -3686,7 +3682,6 @@ static int amdgpu_device_reset_sriov(struct 
> amdgpu_device *adev,
> amdgpu_amdkfd_post_reset(adev);
>
>  error:
> -   amdgpu_virt_init_data_exchange(adev);
> amdgpu_virt_release_full_gpu(adev, true);
> if (!r && adev->virt.gim_feature & AMDGIM_FEATURE_GIM_FLR_VRAMLOST) {
> amdgpu_inc_vram_lost(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 5ec1415d1755..3a0ea9096498 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -703,10 +703,6 @@ static int amdgpu_info_ioctl(struct drm_device *dev, 
> void *data, struct drm_file
> if (adev->pm.dpm_enabled) {
> dev_info.max_engine_clock = amdgpu_dpm_get_sclk(adev, 
> false) * 10;
> 

Re: [PATCH 07/10] drm/amdgpu: add concurrent baco reset support for XGMI

[Andrey Grodzovsky]

Thanks a lot Ma for trying - I think I have to have my own system to 
debug this so I will keep trying enabling XGMI - i still think the is 
the right and the generic solution for multiple nodes reset 
synchronization and in fact the barrier should also be used for 
synchronizing PSP mode 1 XGMI reset too.


Andrey

On 12/9/19 6:34 AM, Ma, Le wrote:


[AMD Official Use Only - Internal Distribution Only]


Hi Andrey,

I tried your patches on my 2P XGMI platform. The baco can work at most 
time, and randomly got following error:


[ 1701.542298] amdgpu: [powerplay] Failed to send message 0x25, 
response 0x0


This error usually means some sync issue exist for xgmi baco case. 
Feel free to debug your patches on my XGMI platform.


Regards,

Ma Le

*From:*Grodzovsky, Andrey 
*Sent:* Saturday, December 7, 2019 5:51 AM
*To:* Ma, Le ; amd-gfx@lists.freedesktop.org; Zhou1, 
Tao ; Deucher, Alexander 
; Li, Dennis ; Zhang, 
Hawking 

*Cc:* Chen, Guchun 
*Subject:* Re: [PATCH 07/10] drm/amdgpu: add concurrent baco reset 
support for XGMI


Hey Ma, attached a solution - it's just compiled as I still can't make 
my XGMI setup work (with bridge connected only one device is visible 
to the system while the other is not). Please try it on your system if 
you have a chance.


Andrey

On 12/4/19 10:14 PM, Ma, Le wrote:

AFAIK it's enough for even single one node in the hive to to fail
the enter the BACO state on time to fail the entire hive reset
procedure, no ?

[Le]: Yeah, agree that. I’ve been thinking that make all nodes
entering baco simultaneously can reduce the possibility of node
failure to enter/exit BACO risk. For example, in an XGMI hive with
8 nodes, the total time interval of 8 nodes enter/exit BACO on 8
CPUs is less than the interval that 8 nodes enter BACO serially
and exit BACO serially depending on one CPU with yield capability.
This interval is usually strict for BACO feature itself. Anyway,
we need more looping test later on any method we will choose.

Any way - I see our discussion blocks your entire patch set - I
think you can go ahead and commit yours way (I think you got an RB
from Hawking) and I will look then and see if I can implement my
method and if it works will just revert your patch.

[Le]: OK, fine.

Andrey

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

Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list

[Christian König]

Yeah, that won't work very well.

During bring-up we also often have the case that we can't correctly 
initialize all engines, e.g. only the first SDMA comes up etc.


So better stick with the initial approach of constructing the scheduler 
array for each engine type which needs it.


Regards,
Christian.

Am 09.12.19 um 15:09 schrieb Nirmoy:

I can see one issue with this. I am ignoring/removing changes from

commit 2a84e48e9712ea8591a10dd59d59ccab3d54efd6 drm/amdgpu: Only add 
rqs for initialized rings.


I wonder if we can handle that differently.

Regards,

Nirmoy

On 12/9/19 2:56 PM, Nirmoy wrote:

Hi Christian,

I got a different idea, a bit more simple let me know what do you 
think about it:


diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

index 50bab33cba39..8de4de4f7a43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -870,6 +870,7 @@ struct amdgpu_device {
    u64 fence_context;
    unsigned    num_rings;
    struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
+  struct drm_gpu_scheduler *rings_sched_list[AMDGPU_MAX_RINGS];
    bool    ib_pool_ready;
    struct amdgpu_sa_manager    ring_tmp_bo;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c

index 1d6850af9908..52b3a5d85a1d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -122,9 +122,8 @@ static int amdgpu_ctx_init(struct amdgpu_device 
*adev,


    for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
    struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
-   struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS]
+  struct drm_gpu_scheduler **sched_list;
    unsigned num_rings = 0;
-   unsigned num_rqs = 0;

    switch (i) {
    case AMDGPU_HW_IP_GFX:
@@ -177,17 +176,11 @@ static int amdgpu_ctx_init(struct amdgpu_device 
*adev,

    break;
    }

-   for (j = 0; j < num_rings; ++j) {
-   if (!rings[j]->adev)
-   continue;
-
-   sched_list[num_rqs++] = [j]->sched;
-   }
-
+  sched_list= adev->rings_sched_list+rings[0]->idx;
    for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j)
    r = 
drm_sched_entity_init(>entities[i][j].entity,

  priority, sched_list,
- num_rqs, 
>guilty);
+    num_rings, 
>guilty);

    if (r)
    goto error_cleanup_entities;
    }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c

index 377fe20bce23..e8cfa357e445 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -480,6 +480,8 @@ int amdgpu_fence_driver_init_ring(struct 
amdgpu_ring *ring,

  ring->name);
    return r;
    }
+
+   adev->rings_sched_list[ring->idx] = >sched;
    }

    return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

index bd9ed33bab43..bfe36199ffed 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1744,8 +1744,11 @@ static int sdma_v4_0_sw_init(void *handle)
 AMDGPU_SDMA_IRQ_INSTANCE0 + i);
    if (r)
    return r;
+   }
+
+   if (adev->sdma.has_page_queue) {
+   for (i = 0; i < adev->sdma.num_instances; i++) {

-   if (adev->sdma.has_page_queue) {
    ring = >sdma.instance[i].page;
    ring->ring_obj = NULL;
    ring->use_doorbell = true;

It relies on contiguous ring initialization that's why I had to 
change  sdma_v4_0.c so that we do ring_init(sdma0, sdma1, page0, page1}


instead of ring_init{sdma0, page0, sdma1, page1}


Regards,

Nirmoy

On 12/9/19 1:20 PM, Christian König wrote:
Yes, you need to do this for the SDMA as well but in general that 
looks like the idea I had in mind as well.


I would do it like this:

1. Change the special case when you only get one scheduler for an 
entity to drop the pointer to the scheduler list.
    This way we always use the same scheduler for the entity and can 
pass in the array on the stack.


2. Change all callers which use more than one scheduler in the list 
to pass in pointers which are not allocated on the stack.
    This obviously also means that we build the list of schedulers 
for each type only once during device init and not for each context 
init.


3. Make the scheduler 

Re: [PATCH] drm/amd/powerplay: clear VBIOS scratchs on baco exit V2

[Alex Deucher]

On Sun, Dec 8, 2019 at 8:42 PM Evan Quan  wrote:
>
> This is needed for coming asic init on performing gpu reset.
>
> V2: use non-asic specific programing way
>
> Change-Id: If3671a24d239e3d288665fadaa2c40c87d5da40b
> Signed-off-by: Evan Quan 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c 
> b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index f5469ad43929..7781d245f8ef 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1676,10 +1676,17 @@ int smu_v11_0_baco_set_state(struct smu_context *smu, 
> enum smu_baco_state state)
> }
> } else {
> ret = smu_send_smc_msg(smu, SMU_MSG_ExitBaco);
> +   if (ret)
> +   goto out;
> +
> bif_doorbell_intr_cntl = REG_SET_FIELD(bif_doorbell_intr_cntl,
> BIF_DOORBELL_INT_CNTL,
> DOORBELL_INTERRUPT_DISABLE, 
> 0);
> WREG32_SOC15(NBIO, 0, mmBIF_DOORBELL_INT_CNTL, 
> bif_doorbell_intr_cntl);
> +
> +   /* clear vbios scratch 6 and 7 for coming asic reinit */
> +   WREG32(adev->bios_scratch_reg_offset + 6, 0);
> +   WREG32(adev->bios_scratch_reg_offset + 7, 0);
> }
> if (ret)
> goto out;
> --
> 2.24.0
>
> ___
> 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 4/4] drm/scheduler: do not keep a copy of sched list

[Nirmoy]

I can see  one issue with this. I am ignoring/removing changes from

commit 2a84e48e9712ea8591a10dd59d59ccab3d54efd6 drm/amdgpu: Only add rqs 
for initialized rings.


I wonder if we can handle that differently.

Regards,

Nirmoy

On 12/9/19 2:56 PM, Nirmoy wrote:

Hi Christian,

I got a different idea, a bit more simple let me know what do you 
think about it:


diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

index 50bab33cba39..8de4de4f7a43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -870,6 +870,7 @@ struct amdgpu_device {
    u64 fence_context;
    unsigned    num_rings;
    struct amdgpu_ring  *rings[AMDGPU_MAX_RINGS];
+  struct drm_gpu_scheduler *rings_sched_list[AMDGPU_MAX_RINGS];
    bool    ib_pool_ready;
    struct amdgpu_sa_manager    ring_tmp_bo;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c

index 1d6850af9908..52b3a5d85a1d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -122,9 +122,8 @@ static int amdgpu_ctx_init(struct amdgpu_device 
*adev,


    for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
    struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
-   struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS]
+  struct drm_gpu_scheduler **sched_list;
    unsigned num_rings = 0;
-   unsigned num_rqs = 0;

    switch (i) {
    case AMDGPU_HW_IP_GFX:
@@ -177,17 +176,11 @@ static int amdgpu_ctx_init(struct amdgpu_device 
*adev,

    break;
    }

-   for (j = 0; j < num_rings; ++j) {
-   if (!rings[j]->adev)
-   continue;
-
-   sched_list[num_rqs++] = [j]->sched;
-   }
-
+  sched_list= adev->rings_sched_list+rings[0]->idx;
    for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j)
    r = 
drm_sched_entity_init(>entities[i][j].entity,

  priority, sched_list,
- num_rqs, >guilty);
+    num_rings, 
>guilty);

    if (r)
    goto error_cleanup_entities;
    }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c

index 377fe20bce23..e8cfa357e445 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -480,6 +480,8 @@ int amdgpu_fence_driver_init_ring(struct 
amdgpu_ring *ring,

  ring->name);
    return r;
    }
+
+   adev->rings_sched_list[ring->idx] = >sched;
    }

    return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

index bd9ed33bab43..bfe36199ffed 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1744,8 +1744,11 @@ static int sdma_v4_0_sw_init(void *handle)
 AMDGPU_SDMA_IRQ_INSTANCE0 + i);
    if (r)
    return r;
+   }
+
+   if (adev->sdma.has_page_queue) {
+   for (i = 0; i < adev->sdma.num_instances; i++) {

-   if (adev->sdma.has_page_queue) {
    ring = >sdma.instance[i].page;
    ring->ring_obj = NULL;
    ring->use_doorbell = true;

It relies on contiguous ring initialization that's why I had to 
change  sdma_v4_0.c so that we do ring_init(sdma0, sdma1, page0, page1}


instead of ring_init{sdma0, page0, sdma1, page1}


Regards,

Nirmoy

On 12/9/19 1:20 PM, Christian König wrote:
Yes, you need to do this for the SDMA as well but in general that 
looks like the idea I had in mind as well.


I would do it like this:

1. Change the special case when you only get one scheduler for an 
entity to drop the pointer to the scheduler list.
    This way we always use the same scheduler for the entity and can 
pass in the array on the stack.


2. Change all callers which use more than one scheduler in the list 
to pass in pointers which are not allocated on the stack.
    This obviously also means that we build the list of schedulers 
for each type only once during device init and not for each context 
init.


3. Make the scheduler list const and drop the kcalloc()/kfree() from 
the entity code.


Regards,
Christian.

Am 08.12.19 um 20:57 schrieb Nirmoy:


On 12/6/19 8:41 PM, Christian König wrote:

Am 06.12.19 um 18:33 schrieb Nirmoy Das:

entity should not keep copy and maintain sched list for
itself.


That is a good step, but we need to take this 

Re: [PATCH v8 11/17] drm/dp_mst: Add DSC enablement helpers to DRM

[Mikita Lipski]

On 12/6/19 7:24 PM, Lyude Paul wrote:

Nice! All I've got is a couple of typos I noticed and one question, this looks
great :)


Thanks! I'll clean it up. The response to the question is below.


On Tue, 2019-12-03 at 09:35 -0500, mikita.lip...@amd.com wrote:
From: Mikita Lipski 

Adding a helper function to be called by
drivers outside of DRM to enable DSC on
the MST ports.

Function is called to recalculate VCPI allocation
if DSC is enabled and raise the DSC flag to enable.
In case of disabling DSC the flag is set to false
and recalculation of VCPI slots is expected to be done
in encoder's atomic_check.

v2: squash separate functions into one and call it per
port

Cc: Harry Wentland 
Cc: Lyude Paul 
Signed-off-by: Mikita Lipski 
---
  drivers/gpu/drm/drm_dp_mst_topology.c | 61 +++
  include/drm/drm_dp_mst_helper.h   |  5 +++
  2 files changed, 66 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index f1d883960831..5e549f48ffb8 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4742,6 +4742,67 @@ drm_dp_mst_atomic_check_topology_state(struct
drm_dp_mst_topology_mgr *mgr,
return 0;
  }
  
+/**

+ * drm_dp_mst_atomic_enable_dsc - Set DSC Enable Flag to On/Off
+ * @state: Pointer to the new drm_atomic_state
+ * @pointer: Pointer to the affected MST Port

Typo here


+ * @pbn: Newly recalculated bw required for link with DSC enabled
+ * @pbn_div: Divider to calculate correct number of pbn per slot
+ * @enable: Boolean flag enabling or disabling DSC on the port
+ *
+ * This function enables DSC on the given Port
+ * by recalculating its vcpi from pbn provided
+ * and sets dsc_enable flag to keep track of which
+ * ports have DSC enabled
+ *
+ */
+int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state,
+struct drm_dp_mst_port *port,
+int pbn, int pbn_div,
+bool enable)
+{
+   struct drm_dp_mst_topology_state *mst_state;
+   struct drm_dp_vcpi_allocation *pos;
+   bool found = false;
+   int vcpi = 0;
+
+   mst_state = drm_atomic_get_mst_topology_state(state, port->mgr);
+
+   if (IS_ERR(mst_state))
+   return PTR_ERR(mst_state);
+
+   list_for_each_entry(pos, _state->vcpis, next) {
+   if (pos->port == port) {
+   found = true;
+   break;
+   }
+   }
+
+   if (!found) {
+   DRM_DEBUG_ATOMIC("[MST PORT:%p] Couldn't find VCPI allocation
in mst state %p\n",
+port, mst_state);
+   return -EINVAL;
+   }


Just double checking-does this handle the case where we're enabling DSC on a
port that didn't previously have a VCPI allocation because it wasn't enabled
previously? Or do we not need to handle that here


Because we call encoder atomic check previously to allocate VCPI slots - 
the port should have VCPI allocation before enabling DSC even if it 
wasn't enabled previously.
Therefore, I was thinking, that if encoder atomic check fails to 
allocate VCPI slots for the port - we shouldn't enable DSC on it and 
probably should fail atomic check if that is even requested.


Assuming you did the right thing here, with the small typo fixes:

Reviewed-by: Lyude Paul 


+
+   if (pos->dsc_enabled == enable) {
+   DRM_DEBUG_ATOMIC("[MST PORT:%p] DSC flag is already set to %d,
returning %d VCPI slots\n",
+port, enable, pos->vcpi);
+   vcpi = pos->vcpi;
+   }
+
+   if (enable) {
+   vcpi = drm_dp_atomic_find_vcpi_slots(state, port->mgr, port,
pbn, pbn_div);
+   DRM_DEBUG_ATOMIC("[MST PORT:%p] Enabling DSC flag,
reallocating %d VCPI slots on the port\n",
+port, vcpi);
+   if (vcpi < 0)
+   return -EINVAL;
+   }
+
+   pos->dsc_enabled = enable;
+
+   return vcpi;
+}
+EXPORT_SYMBOL(drm_dp_mst_atomic_enable_dsc);
  /**
   * drm_dp_mst_atomic_check - Check that the new state of an MST topology in
an
   * atomic update is valid
diff --git a/include/drm/drm_dp_mst_helper.h
b/include/drm/drm_dp_mst_helper.h
index 0f813d6346aa..830c94b7f45d 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -502,6 +502,7 @@ struct drm_dp_payload {
  struct drm_dp_vcpi_allocation {
struct drm_dp_mst_port *port;
int vcpi;
+   bool dsc_enabled;
struct list_head next;
  };
  
@@ -773,6 +774,10 @@ drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state

*state,
  struct drm_dp_mst_topology_mgr *mgr,
  struct drm_dp_mst_port *port, int pbn,
  int pbn_div);
+int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state,
+ 

Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list

[Nirmoy]

Hi Christian,

I got a different idea, a bit more simple let me know what do you think 
about it:


diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

index 50bab33cba39..8de4de4f7a43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -870,6 +870,7 @@ struct amdgpu_device {
    u64 fence_context;
    unsigned    num_rings;
    struct amdgpu_ring  *rings[AMDGPU_MAX_RINGS];
+  struct drm_gpu_scheduler *rings_sched_list[AMDGPU_MAX_RINGS];
    bool    ib_pool_ready;
    struct amdgpu_sa_manager    ring_tmp_bo;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c

index 1d6850af9908..52b3a5d85a1d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -122,9 +122,8 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,

    for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
    struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
-   struct drm_gpu_scheduler *sched_list[AMDGPU_MAX_RINGS]
+  struct drm_gpu_scheduler **sched_list;
    unsigned num_rings = 0;
-   unsigned num_rqs = 0;

    switch (i) {
    case AMDGPU_HW_IP_GFX:
@@ -177,17 +176,11 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
    break;
    }

-   for (j = 0; j < num_rings; ++j) {
-   if (!rings[j]->adev)
-   continue;
-
-   sched_list[num_rqs++] = [j]->sched;
-   }
-
+  sched_list= adev->rings_sched_list+rings[0]->idx;
    for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j)
    r = 
drm_sched_entity_init(>entities[i][j].entity,

  priority, sched_list,
- num_rqs, >guilty);
+    num_rings, >guilty);
    if (r)
    goto error_cleanup_entities;
    }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c

index 377fe20bce23..e8cfa357e445 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -480,6 +480,8 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring 
*ring,

  ring->name);
    return r;
    }
+
+   adev->rings_sched_list[ring->idx] = >sched;
    }

    return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

index bd9ed33bab43..bfe36199ffed 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1744,8 +1744,11 @@ static int sdma_v4_0_sw_init(void *handle)
 AMDGPU_SDMA_IRQ_INSTANCE0 + i);
    if (r)
    return r;
+   }
+
+   if (adev->sdma.has_page_queue) {
+   for (i = 0; i < adev->sdma.num_instances; i++) {

-   if (adev->sdma.has_page_queue) {
    ring = >sdma.instance[i].page;
    ring->ring_obj = NULL;
    ring->use_doorbell = true;

It relies on contiguous ring initialization that's why I had to change  
sdma_v4_0.c so that we do ring_init(sdma0, sdma1, page0, page1}


instead of ring_init{sdma0, page0, sdma1, page1}


Regards,

Nirmoy

On 12/9/19 1:20 PM, Christian König wrote:
Yes, you need to do this for the SDMA as well but in general that 
looks like the idea I had in mind as well.


I would do it like this:

1. Change the special case when you only get one scheduler for an 
entity to drop the pointer to the scheduler list.
    This way we always use the same scheduler for the entity and can 
pass in the array on the stack.


2. Change all callers which use more than one scheduler in the list to 
pass in pointers which are not allocated on the stack.
    This obviously also means that we build the list of schedulers for 
each type only once during device init and not for each context init.


3. Make the scheduler list const and drop the kcalloc()/kfree() from 
the entity code.


Regards,
Christian.

Am 08.12.19 um 20:57 schrieb Nirmoy:


On 12/6/19 8:41 PM, Christian König wrote:

Am 06.12.19 um 18:33 schrieb Nirmoy Das:

entity should not keep copy and maintain sched list for
itself.


That is a good step, but we need to take this further.


How about  something like ?

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h

index 0ae0a2715b0d..a71ee084b47a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -269,8 

Re: [PATCH 4/4] drm/scheduler: do not keep a copy of sched list

[Christian König]

Yes, you need to do this for the SDMA as well but in general that looks 
like the idea I had in mind as well.


I would do it like this:

1. Change the special case when you only get one scheduler for an entity 
to drop the pointer to the scheduler list.
    This way we always use the same scheduler for the entity and can 
pass in the array on the stack.


2. Change all callers which use more than one scheduler in the list to 
pass in pointers which are not allocated on the stack.
    This obviously also means that we build the list of schedulers for 
each type only once during device init and not for each context init.


3. Make the scheduler list const and drop the kcalloc()/kfree() from the 
entity code.


Regards,
Christian.

Am 08.12.19 um 20:57 schrieb Nirmoy:


On 12/6/19 8:41 PM, Christian König wrote:

Am 06.12.19 um 18:33 schrieb Nirmoy Das:

entity should not keep copy and maintain sched list for
itself.


That is a good step, but we need to take this further.


How about  something like ?

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h

index 0ae0a2715b0d..a71ee084b47a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -269,8 +269,10 @@ struct amdgpu_gfx {
    bool    me_fw_write_wait;
    bool    cp_fw_write_wait;
    struct amdgpu_ring gfx_ring[AMDGPU_MAX_GFX_RINGS];
+   struct drm_gpu_scheduler *gfx_sched_list[AMDGPU_MAX_GFX_RINGS];
    unsigned    num_gfx_rings;
    struct amdgpu_ring compute_ring[AMDGPU_MAX_COMPUTE_RINGS];
+   struct drm_gpu_scheduler 
*compute_sched_list[AMDGPU_MAX_COMPUTE_RINGS];

    unsigned    num_compute_rings;
    struct amdgpu_irq_src   eop_irq;
    struct amdgpu_irq_src   priv_reg_irq;


Regards,

Nirmoy



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

RE: [PATCH 07/10] drm/amdgpu: add concurrent baco reset support for XGMI

[Ma, Le]

[AMD Official Use Only - Internal Distribution Only]

Hi Andrey,

I tried your patches on my 2P XGMI platform. The baco can work at most time, 
and randomly got following error:
[ 1701.542298] amdgpu: [powerplay] Failed to send message 0x25, response 0x0

This error usually means some sync issue exist for xgmi baco case. Feel free to 
debug your patches on my XGMI platform.

Regards,
Ma Le

From: Grodzovsky, Andrey 
Sent: Saturday, December 7, 2019 5:51 AM
To: Ma, Le ; amd-gfx@lists.freedesktop.org; Zhou1, Tao 
; Deucher, Alexander ; Li, Dennis 
; Zhang, Hawking 
Cc: Chen, Guchun 
Subject: Re: [PATCH 07/10] drm/amdgpu: add concurrent baco reset support for 
XGMI


Hey Ma, attached a solution - it's just compiled as I still can't make my XGMI 
setup work (with bridge connected only one device is visible to the system 
while the other is not). Please try it on your system if you have a chance.

Andrey
On 12/4/19 10:14 PM, Ma, Le wrote:

AFAIK it's enough for even single one node in the hive to to fail the enter the 
BACO state on time to fail the entire hive reset procedure, no ?
[Le]: Yeah, agree that. I've been thinking that make all nodes entering baco 
simultaneously can reduce the possibility of node failure to enter/exit BACO 
risk. For example, in an XGMI hive with 8 nodes, the total time interval of 8 
nodes enter/exit BACO on 8 CPUs is less than the interval that 8 nodes enter 
BACO serially and exit BACO serially depending on one CPU with yield 
capability. This interval is usually strict for BACO feature itself. Anyway, we 
need more looping test later on any method we will choose.

Any way - I see our discussion blocks your entire patch set - I think you can 
go ahead and commit yours way (I think you got an RB from Hawking) and I will 
look then and see if I can implement my method and if it works will just revert 
your patch.

[Le]: OK, fine.

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

[PATCH] drm/amd/powerplay: enable pp one vf mode for vega10

[Yintian Tao]

Originally, due to the restriction from PSP and SMU, VF has
to send message to hypervisor driver to handle powerplay
change which is complicated and redundant. Currently, SMU
and PSP can support VF to directly handle powerplay
change by itself. Therefore, the old code about the handshake
between VF and PF to handle powerplay will be removed and VF
will use new the registers below to handshake with SMU.
mmMP1_SMN_C2PMSG_101: register to handle SMU message
mmMP1_SMN_C2PMSG_102: register to handle SMU parameter
mmMP1_SMN_C2PMSG_103: register to handle SMU response

v2: remove module parameter pp_one_vf

Signed-off-by: Yintian Tao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|  13 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |   4 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 121 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c  |  51 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h  |  14 +-
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c |  78 -
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h |   4 -
 drivers/gpu/drm/amd/amdgpu/soc15.c|   8 +-
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c |   4 +-
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c|  24 +--
 .../drm/amd/powerplay/hwmgr/hardwaremanager.c |  15 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c   |   7 +
 drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c  |  30 ++--
 .../drm/amd/powerplay/hwmgr/vega10_hwmgr.c| 157 --
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h |   1 +
 .../drm/amd/powerplay/smumgr/smu9_smumgr.c|  55 --
 .../drm/amd/powerplay/smumgr/vega10_smumgr.c  |   6 +
 17 files changed, 280 insertions(+), 312 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index b9ca7e728d3e..30eb42593a20 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1880,6 +1880,9 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)
}
}
 
+   if (amdgpu_sriov_vf(adev))
+   amdgpu_virt_init_data_exchange(adev);
+
r = amdgpu_ib_pool_init(adev);
if (r) {
dev_err(adev->dev, "IB initialization failed (%d).\n", r);
@@ -1922,8 +1925,6 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)
 
 init_failed:
if (amdgpu_sriov_vf(adev)) {
-   if (!r)
-   amdgpu_virt_init_data_exchange(adev);
amdgpu_virt_release_full_gpu(adev, true);
}
 
@@ -2819,7 +2820,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
mutex_init(>virt.vf_errors.lock);
hash_init(adev->mn_hash);
mutex_init(>lock_reset);
-   mutex_init(>virt.dpm_mutex);
mutex_init(>psp.mutex);
 
r = amdgpu_device_check_arguments(adev);
@@ -3040,9 +3040,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 
amdgpu_fbdev_init(adev);
 
-   if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev))
-   amdgpu_pm_virt_sysfs_init(adev);
-
r = amdgpu_pm_sysfs_init(adev);
if (r) {
adev->pm_sysfs_en = false;
@@ -3187,8 +3184,6 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
iounmap(adev->rmmio);
adev->rmmio = NULL;
amdgpu_device_doorbell_fini(adev);
-   if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev))
-   amdgpu_pm_virt_sysfs_fini(adev);
 
amdgpu_debugfs_regs_cleanup(adev);
device_remove_file(adev->dev, _attr_pcie_replay_count);
@@ -3669,6 +3664,7 @@ static int amdgpu_device_reset_sriov(struct amdgpu_device 
*adev,
if (r)
goto error;
 
+   amdgpu_virt_init_data_exchange(adev);
/* we need recover gart prior to run SMC/CP/SDMA resume */
amdgpu_gtt_mgr_recover(>mman.bdev.man[TTM_PL_TT]);
 
@@ -3686,7 +3682,6 @@ static int amdgpu_device_reset_sriov(struct amdgpu_device 
*adev,
amdgpu_amdkfd_post_reset(adev);
 
 error:
-   amdgpu_virt_init_data_exchange(adev);
amdgpu_virt_release_full_gpu(adev, true);
if (!r && adev->virt.gim_feature & AMDGIM_FEATURE_GIM_FLR_VRAMLOST) {
amdgpu_inc_vram_lost(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 5ec1415d1755..3a0ea9096498 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -703,10 +703,6 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
if (adev->pm.dpm_enabled) {
dev_info.max_engine_clock = amdgpu_dpm_get_sclk(adev, 
false) * 10;
dev_info.max_memory_clock = amdgpu_dpm_get_mclk(adev, 
false) * 10;
-   } else if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev) &&
-  adev->virt.ops->get_pp_clk) {
-   dev_info.max_engine_clock = 

[PATCH] drm/amd/powerplay: enable pp one vf mode for vega10

[Yintian Tao]

Originally, due to the restriction from PSP and SMU, VF has
to send message to hypervisor driver to handle powerplay
change which is complicated and redundant. Currently, SMU
and PSP can support VF to directly handle powerplay
change by itself. Therefore, the old code about the handshake
between VF and PF to handle powerplay will be removed and VF
will use new the registers below to handshake with SMU.
mmMP1_SMN_C2PMSG_101: register to handle SMU message
mmMP1_SMN_C2PMSG_102: register to handle SMU parameter
mmMP1_SMN_C2PMSG_103: register to handle SMU response

Signed-off-by: Yintian Tao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|   6 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |   7 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |   4 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 121 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c  |  51 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h  |  14 +-
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c |  78 -
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h |   4 -
 drivers/gpu/drm/amd/amdgpu/soc15.c|   2 +-
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c |   7 +-
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c|  24 +--
 .../drm/amd/powerplay/hwmgr/hardwaremanager.c |  15 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c   |   4 +
 drivers/gpu/drm/amd/powerplay/hwmgr/pp_psm.c  |  30 ++--
 .../drm/amd/powerplay/hwmgr/vega10_hwmgr.c| 157 --
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h |   1 +
 .../drm/amd/powerplay/smumgr/smu9_smumgr.c|  55 --
 .../drm/amd/powerplay/smumgr/vega10_smumgr.c  |   6 +
 19 files changed, 281 insertions(+), 306 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 50bab33cba39..3f00c2e0b0e1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -114,6 +114,7 @@ struct amdgpu_mgpu_info
 /*
  * Modules parameters.
  */
+extern int amdgpu_pp_one_vf;
 extern int amdgpu_modeset;
 extern int amdgpu_vram_limit;
 extern int amdgpu_vis_vram_limit;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index b9ca7e728d3e..62fa5f074ebd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2819,7 +2819,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
mutex_init(>virt.vf_errors.lock);
hash_init(adev->mn_hash);
mutex_init(>lock_reset);
-   mutex_init(>virt.dpm_mutex);
mutex_init(>psp.mutex);
 
r = amdgpu_device_check_arguments(adev);
@@ -3040,9 +3039,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 
amdgpu_fbdev_init(adev);
 
-   if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev))
-   amdgpu_pm_virt_sysfs_init(adev);
-
r = amdgpu_pm_sysfs_init(adev);
if (r) {
adev->pm_sysfs_en = false;
@@ -3187,8 +3183,6 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
iounmap(adev->rmmio);
adev->rmmio = NULL;
amdgpu_device_doorbell_fini(adev);
-   if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev))
-   amdgpu_pm_virt_sysfs_fini(adev);
 
amdgpu_debugfs_regs_cleanup(adev);
device_remove_file(adev->dev, _attr_pcie_replay_count);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index d1e9946ac218..f6a4642cf9b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -135,6 +135,7 @@ int amdgpu_lbpw = -1;
 int amdgpu_compute_multipipe = -1;
 int amdgpu_gpu_recovery = -1; /* auto */
 int amdgpu_emu_mode = 0;
+int amdgpu_pp_one_vf;
 uint amdgpu_smu_memory_pool_size = 0;
 /* FBC (bit 0) disabled by default*/
 uint amdgpu_dc_feature_mask = 0;
@@ -152,6 +153,12 @@ struct amdgpu_mgpu_info mgpu_info = {
 int amdgpu_ras_enable = -1;
 uint amdgpu_ras_mask = 0x;
 
+/**
+ * DOC: pp_one_vf (int)
+ * Enable VF to adjust the powerplay. The default is 0 (disable it).
+ */
+MODULE_PARM_DESC(pp_one_vf, "One vf mode support (0 = disable (default), 1 = 
enable)");
+module_param_named(pp_one_vf, amdgpu_pp_one_vf, int, 0600);
 /**
  * DOC: vramlimit (int)
  * Restrict the total amount of VRAM in MiB for testing.  The default is 0 
(Use full VRAM).
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 5ec1415d1755..3a0ea9096498 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -703,10 +703,6 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
if (adev->pm.dpm_enabled) {
dev_info.max_engine_clock = amdgpu_dpm_get_sclk(adev, 
false) * 10;
dev_info.max_memory_clock = amdgpu_dpm_get_mclk(adev, 
false) *