[PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Liu, Shaoyun]

Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for 
peer device

Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
Signed-off-by: shaoyunl 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 11 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index a82c3b1..a0f56e4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -666,6 +666,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
struct amdgpu_device *adev = dev->dev_private;
struct drm_amdgpu_gem_op *args = data;
struct drm_gem_object *gobj;
+   struct amdgpu_vm_bo_base *base;
+   struct amdgpu_bo_va *bo_va;
struct amdgpu_bo *robj;
int r;
 
@@ -704,6 +706,15 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
amdgpu_bo_unreserve(robj);
break;
}
+   for (base = robj->vm_bo; base; base = base->next) {
+   bo_va = container_of(base, struct amdgpu_bo_va, base);
+   if (bo_va && bo_va->is_xgmi) {
+   r = -EINVAL;
+   amdgpu_bo_unreserve(robj);
+   goto out;
+   }
+   }
+
robj->preferred_domains = args->value & (AMDGPU_GEM_DOMAIN_VRAM 
|
AMDGPU_GEM_DOMAIN_GTT |
AMDGPU_GEM_DOMAIN_CPU);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 76eee7e..8ed23d2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2048,7 +2048,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
amdgpu_device *adev,
INIT_LIST_HEAD(_va->valids);
INIT_LIST_HEAD(_va->invalids);
 
-   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
+   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
+   (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)) {
bo_va->is_xgmi = true;
mutex_lock(>vm_manager.lock_pstate);
/* Power up XGMI if it can be potentially used */
-- 
2.7.4

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

Re: [PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Kuehling, Felix]

On 2019-03-26 2:54 p.m., Liu, Shaoyun wrote:
> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for 
> peer device
>
> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
> Signed-off-by: shaoyunl 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 9 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 3 ++-
>   2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index a82c3b1..3c7ee71 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -664,8 +664,10 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void 
> *data,
>   struct drm_file *filp)
>   {
>   struct amdgpu_device *adev = dev->dev_private;
> + struct amdgpu_fpriv *fpriv = filp->driver_priv;
>   struct drm_amdgpu_gem_op *args = data;
>   struct drm_gem_object *gobj;
> + struct amdgpu_bo_va *bo_va;
>   struct amdgpu_bo *robj;
>   int r;
>   
> @@ -704,6 +706,13 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void 
> *data,
>   amdgpu_bo_unreserve(robj);
>   break;
>   }
> + bo_va = amdgpu_vm_bo_find(>vm, robj);
> + if (bo_va && bo_va->is_xgmi) {
> + r = -EINVAL;
> + amdgpu_bo_unreserve(robj);
> + break;
> + }
> +

Hmm, from the other discussion, GEM doesn't really support P2P of VRAM 
BOs between GPUs right now. The only way this function can affect a BO 
that's P2P shared is, if the BO is allocated with GEM and then imported 
into KFD. In that case you'll need to take into account mappings of the 
imported BO in all the KFD VMs, not the VMs in the fpriv->vm.

In other words, you need to find all bo_vas of the BO in all VMs and for 
each one check, whether it has is_xgmi set.

Regards,
   Felix


>   robj->preferred_domains = args->value & (AMDGPU_GEM_DOMAIN_VRAM 
> |
>   AMDGPU_GEM_DOMAIN_GTT |
>   AMDGPU_GEM_DOMAIN_CPU);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 76eee7e..f08dda2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2048,7 +2048,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
> amdgpu_device *adev,
>   INIT_LIST_HEAD(_va->valids);
>   INIT_LIST_HEAD(_va->invalids);
>   
> - if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
> + if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
> + (bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM)) {
>   bo_va->is_xgmi = true;
>   mutex_lock(>vm_manager.lock_pstate);
>   /* Power up XGMI if it can be potentially used */
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Set correct context adev for gem object

[Koenig, Christian]

Am 26.03.19 um 20:33 schrieb Liu, Shaoyun:
> OK , just ignore it . BTW , if you think it's a hack currently used in
> KFD  to share the memory among GPUs,  what's your plane to use the peer
> GPU through XGMI from GEM  point of view ?

We just need to unpack the root bo while doing the XGMI check, e.g. we 
have a BO structure in the importing device and use this one while doing 
the mapping.

Christian.

>
> Regards
>
> shaoyun.liu
>
> On 2019-03-26 2:56 p.m., Christian König wrote:
>> Am 26.03.19 um 16:51 schrieb Liu, Shaoyun:
>>> The context device pointer could be different of the object been
>>> acctually allocated
>> Actually that is unnecessary, cause the GEM adev is always identical
>> to the file_priv.
>>
>> E.g. we don't support the hack using in the KFD for BO sharing here.
>>
>>> Change-Id: I7b2338858126d75350b65ff04d9bb419e1eae15c
>>> Signed-off-by: shaoyunl 
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> index 9ee8d7a..a82c3b1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> @@ -117,8 +117,8 @@ int amdgpu_gem_object_open(struct drm_gem_object
>>> *obj,
>>>   struct drm_file *file_priv)
>>>    {
>>>    struct amdgpu_bo *abo = gem_to_amdgpu_bo(obj);
>>> -    struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
>>>    struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
>>> +    struct amdgpu_device *adev = fpriv->ctx_mgr.adev;
>>>    struct amdgpu_vm *vm = >vm;
>>>    struct amdgpu_bo_va *bo_va;
>>>    struct mm_struct *mm;
>>> @@ -150,8 +150,8 @@ void amdgpu_gem_object_close(struct
>>> drm_gem_object *obj,
>>>     struct drm_file *file_priv)
>>>    {
>>>    struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>>> -    struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>>    struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
>>> +    struct amdgpu_device *adev = fpriv->ctx_mgr.adev;
>>>    struct amdgpu_vm *vm = >vm;
>>>      struct amdgpu_bo_list_entry vm_pd;

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

Re: [PATCH] drm/amdgpu: Set correct context adev for gem object

[Liu, Shaoyun]

OK , just ignore it . BTW , if you think it's a hack currently used in 
KFD  to share the memory among GPUs,  what's your plane to use the peer 
GPU through XGMI from GEM  point of view ?

Regards

shaoyun.liu

On 2019-03-26 2:56 p.m., Christian König wrote:
> Am 26.03.19 um 16:51 schrieb Liu, Shaoyun:
>> The context device pointer could be different of the object been 
>> acctually allocated
>
> Actually that is unnecessary, cause the GEM adev is always identical 
> to the file_priv.
>
> E.g. we don't support the hack using in the KFD for BO sharing here.
>
>>
>> Change-Id: I7b2338858126d75350b65ff04d9bb419e1eae15c
>> Signed-off-by: shaoyunl 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index 9ee8d7a..a82c3b1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -117,8 +117,8 @@ int amdgpu_gem_object_open(struct drm_gem_object 
>> *obj,
>>  struct drm_file *file_priv)
>>   {
>>   struct amdgpu_bo *abo = gem_to_amdgpu_bo(obj);
>> -    struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
>>   struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
>> +    struct amdgpu_device *adev = fpriv->ctx_mgr.adev;
>>   struct amdgpu_vm *vm = >vm;
>>   struct amdgpu_bo_va *bo_va;
>>   struct mm_struct *mm;
>> @@ -150,8 +150,8 @@ void amdgpu_gem_object_close(struct 
>> drm_gem_object *obj,
>>    struct drm_file *file_priv)
>>   {
>>   struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>> -    struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>   struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
>> +    struct amdgpu_device *adev = fpriv->ctx_mgr.adev;
>>   struct amdgpu_vm *vm = >vm;
>>     struct amdgpu_bo_list_entry vm_pd;
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Christian König]

Am 26.03.19 um 19:54 schrieb Liu, Shaoyun:

Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for 
peer device

Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
Signed-off-by: shaoyunl 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 9 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 3 ++-
  2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index a82c3b1..3c7ee71 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -664,8 +664,10 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
  {
struct amdgpu_device *adev = dev->dev_private;
+   struct amdgpu_fpriv *fpriv = filp->driver_priv;
struct drm_amdgpu_gem_op *args = data;
struct drm_gem_object *gobj;
+   struct amdgpu_bo_va *bo_va;
struct amdgpu_bo *robj;
int r;
  
@@ -704,6 +706,13 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,

amdgpu_bo_unreserve(robj);
break;
}
+   bo_va = amdgpu_vm_bo_find(>vm, robj);
+   if (bo_va && bo_va->is_xgmi) {
+   r = -EINVAL;
+   amdgpu_bo_unreserve(robj);
+   break;
+   }
+
robj->preferred_domains = args->value & (AMDGPU_GEM_DOMAIN_VRAM 
|
AMDGPU_GEM_DOMAIN_GTT |
AMDGPU_GEM_DOMAIN_CPU);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 76eee7e..f08dda2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2048,7 +2048,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
amdgpu_device *adev,
INIT_LIST_HEAD(_va->valids);
INIT_LIST_HEAD(_va->invalids);
  
-	if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {

+   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
+   (bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM)) {


This is a bit mask, so better change this to "bo->preferred_domains & 
AMDGPU_GEM_DOMAIN_VRAM".


With that done the patch is Reviewed-by: Christian König 
.


Christian.


bo_va->is_xgmi = true;
mutex_lock(>vm_manager.lock_pstate);
/* Power up XGMI if it can be potentially used */


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

Re: WARNING at amdgpu_vm_sdma.c:85

[Christian König]

Mhm, that is actually harmless. We somehow end up trying to submit a 
zero sized IB.


Going to double check tomorrow how this can happen.

Is the anything bad happening when you just remove the warning?

Christian.

Am 26.03.19 um 18:30 schrieb Michel Dänzer:

On 2019-03-26 4:21 p.m., Michel Dänzer wrote:

I'm hitting some badness with piglit today, see the attached dmesg
excerpt. I was away for a couple of days, but looks like it might be
related to the VM changes? I'll try bisecting.

Bisected to:

602b5050d839c40572bbb19e9b17a37a150c1e3c is the first bad commit
commit 602b5050d839c40572bbb19e9b17a37a150c1e3c
Author: Christian König 
Date:   Mon Mar 18 14:26:24 2019 +0100

 drm/amdgpu: use the new VM backend for PDEs

 And remove the existing code when it is unused.

 Signed-off-by: Christian König 
 Reviewed-by: Chunming Zhou 
 Reviewed-by: Felix Kuehling 


The problem doesn't happen with this commit and the corresponding PTE
one reverted on current amd-staging-drm-next.




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

Re: [PATCH] drm/amdgpu: Set correct context adev for gem object

[Christian König]

Am 26.03.19 um 16:51 schrieb Liu, Shaoyun:

The context device pointer could be different of the object been acctually 
allocated


Actually that is unnecessary, cause the GEM adev is always identical to 
the file_priv.


E.g. we don't support the hack using in the KFD for BO sharing here.



Change-Id: I7b2338858126d75350b65ff04d9bb419e1eae15c
Signed-off-by: shaoyunl 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9ee8d7a..a82c3b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -117,8 +117,8 @@ int amdgpu_gem_object_open(struct drm_gem_object *obj,
   struct drm_file *file_priv)
  {
struct amdgpu_bo *abo = gem_to_amdgpu_bo(obj);
-   struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
+   struct amdgpu_device *adev = fpriv->ctx_mgr.adev;
struct amdgpu_vm *vm = >vm;
struct amdgpu_bo_va *bo_va;
struct mm_struct *mm;
@@ -150,8 +150,8 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
 struct drm_file *file_priv)
  {
struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
-   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
+   struct amdgpu_device *adev = fpriv->ctx_mgr.adev;
struct amdgpu_vm *vm = >vm;
  
  	struct amdgpu_bo_list_entry vm_pd;


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

[PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Liu, Shaoyun]

Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for 
peer device

Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
Signed-off-by: shaoyunl 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 9 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 3 ++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index a82c3b1..3c7ee71 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -664,8 +664,10 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
 {
struct amdgpu_device *adev = dev->dev_private;
+   struct amdgpu_fpriv *fpriv = filp->driver_priv;
struct drm_amdgpu_gem_op *args = data;
struct drm_gem_object *gobj;
+   struct amdgpu_bo_va *bo_va;
struct amdgpu_bo *robj;
int r;
 
@@ -704,6 +706,13 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
amdgpu_bo_unreserve(robj);
break;
}
+   bo_va = amdgpu_vm_bo_find(>vm, robj);
+   if (bo_va && bo_va->is_xgmi) {
+   r = -EINVAL;
+   amdgpu_bo_unreserve(robj);
+   break;
+   }
+
robj->preferred_domains = args->value & (AMDGPU_GEM_DOMAIN_VRAM 
|
AMDGPU_GEM_DOMAIN_GTT |
AMDGPU_GEM_DOMAIN_CPU);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 76eee7e..f08dda2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2048,7 +2048,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
amdgpu_device *adev,
INIT_LIST_HEAD(_va->valids);
INIT_LIST_HEAD(_va->invalids);
 
-   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
+   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
+   (bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM)) {
bo_va->is_xgmi = true;
mutex_lock(>vm_manager.lock_pstate);
/* Power up XGMI if it can be potentially used */
-- 
2.7.4

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

Re: [PATCH] drm/amdgpu: XGMI pstate switch initial support

[Christian König]

Am 26.03.19 um 16:43 schrieb Liu, Shaoyun:

Can we simply failed the set placement call  if we find the bo_va is
existing and  bo_va->is_xgmi flag is set ?  The original call also
failed when it detect it's shared and  the  placement is in VRAM.


Possible, alternative we could just move setting the flag again into 
amdgpu_vm_bo_update().


My concern was to not have the flag in the mapping structure, but 
setting/clearing it when the update is made is still possible.


Down side is that you then end up with multiple code paths for disabling 
XGMI.


Christian.



Regards

shaoyun.liu


On 2019-03-26 10:00 a.m., Kuehling, Felix wrote:

On 2019-03-26 9:15 a.m., Liu, Shaoyun wrote:

I think in a real usage  ( tensorflow ex),  it's rarely only the
system memory (no vram) will be mapped for peer access.

With that argument you could simplify your change and just power up XGMI
as soon as a KFD process starts. Because that's effectively what happens
with your change as it is now, if you don't check the memory type. Every
KFD process maps the signal page (in system memory) to all GPUs. So you
will always increment the xgmi_map_count even before the first VRAM BO
is allocated, let alone mapped to multiple GPUs.



Anyway, how about add preferred_domain check for xgmi ? I think even
user use ioctl to change the preferred_domain,  bo_add should still be
called before the real mapping.

amdgpu_gem_op_ioctl with AMDGPU_GEM_OP_SET_PLACEMENT doesn't call
bo_add. You'd have to add something in amdgpu_gem_op_ioctl to
re-evaluate all bo_vas of the BO when its placement changes, update the
bo_va->is_xgmi flag, and if necessary the xgmi_map_counter.

Regards,
     Felix



Regards
Shaoyun.liu

*From:* amd-gfx  on behalf of
Kuehling, Felix 
*Sent:* March 25, 2019 6:28:32 PM
*To:* Liu, Shaoyun; amd-gfx@lists.freedesktop.org
*Subject:* Re: [PATCH] drm/amdgpu: XGMI pstate switch initial support
I don't see any check for the memory type. As far as I can tell you'll
power up XGMI even for system memory mappings. See inline.

On 2019-03-22 3:28 p.m., Liu, Shaoyun wrote:

Driver vote low to high pstate switch whenever there is an outstanding
XGMI mapping request. Driver vote high to low pstate when all the
outstanding XGMI mapping is terminated.

Change-Id: I197501f853c47f844055c0e28c0ac00a1ff06607
Signed-off-by: shaoyunl 
---
     drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 
     drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 ++
     drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 21 +
     drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  4 
     drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   | 16 +++-
     drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   | 10 ++
     6 files changed, 56 insertions(+), 1 deletion(-)

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

b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

index ec9562d..c4c61e9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2018,6 +2018,10 @@ static void

amdgpu_device_ip_late_init_func_handler(struct work_struct *work)

      r = amdgpu_device_enable_mgpu_fan_boost();
      if (r)
      DRM_ERROR("enable mgpu fan boost failed (%d).\n", r);
+
+ /*set to low pstate by default */
+ amdgpu_xgmi_set_pstate(adev, 0);
+
     }

     static void amdgpu_device_delay_enable_gfx_off(struct work_struct

*work)

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

b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h

index 220a6a7..c430e82 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -72,6 +72,8 @@ struct amdgpu_bo_va {

      /* If the mappings are cleared or filled */
      bool    cleared;
+
+ bool    is_xgmi;
     };

     struct amdgpu_bo {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 729da1c..a7247d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -34,6 +34,7 @@
     #include "amdgpu_trace.h"
     #include "amdgpu_amdkfd.h"
     #include "amdgpu_gmc.h"
+#include "amdgpu_xgmi.h"

     /**
      * DOC: GPUVM
@@ -2072,6 +2073,15 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct

amdgpu_device *adev,

      INIT_LIST_HEAD(_va->valids);
      INIT_LIST_HEAD(_va->invalids);

+ if (bo && amdgpu_xgmi_same_hive(adev,

amdgpu_ttm_adev(bo->tbo.bdev))) {

+ bo_va->is_xgmi = true;

You're setting this to true even for system memory BOs that don't
involve XGMI mappings. That means you'll power up XGMI unnecessarily in
many cases because KFD processes always have system memory mappings that
are mapped to all GPUs (e.g. the signal page).

Regards,
     Felix



+ mutex_lock(>vm_manager.lock_pstate);
+ /* Power up XGMI if it can be 

Re: [PATCH 3/4] drm/amd/display: In VRR mode, do DRM core vblank handling at end of vblank. (v2)

[Kazlauskas, Nicholas]

On 3/22/19 4:04 PM, Mario Kleiner wrote:
> In VRR mode, proper vblank/pageflip timestamps can only be computed
> after the display scanout position has left front-porch. Therefore
> delay calls to drm_crtc_handle_vblank(), and thereby calls to
> drm_update_vblank_count() and pageflip event delivery, to after the
> end of front-porch when in VRR mode.
> 
> We add a new vupdate irq, which triggers at the end of the vupdate
> interval, ie. at the end of vblank, and calls the core vblank handler
> function. The new irq handler is not executed in standard non-VRR
> mode, so vblank handling for fixed refresh rate mode is identical
> to the past implementation.
> 
> v2: Implement feedback by Nicholas and Paul Menzel.
> 
> Signed-off-by: Mario Kleiner 
> Acked-by: Harry Wentland 

Reviewed-by: Nicholas Kazlauskas 

This patch seems reasonable to me for the most part. I did testing on 
the previous version and seems to do what it intends to.

Harry made good points about the potential problems associated with 
VUPDATE - it's dependent on the timing for the mode and could 
potentially even be before line 0, but I think that's unlikely to happen 
for any monitor that supports adaptive sync.

I'd like to look into moving all of this over to VLINE instead at some 
point for a guarantee on when these events get sent to userspace but for 
now this is probably good enough for resolving these timestamp issues.

There still needs to be that other patch preceding your first one that 
addresses the freesync config being updated after it's checked for 
keeping the vblank reference.

I should be able to find some time this week to write up a patch that 
fixes that problem and send it to amd-gfx if you haven't already taken a 
look at the issue.

Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h   |   1 +
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 128 +-
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   9 ++
>   .../drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c |  22 +++
>   .../dc/irq/dce110/irq_service_dce110.c|   7 +-
>   .../dc/irq/dce120/irq_service_dce120.c|   7 +-
>   .../display/dc/irq/dce80/irq_service_dce80.c  |   6 +-
>   .../display/dc/irq/dcn10/irq_service_dcn10.c  |  40 --
>   8 files changed, 204 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 6e71749cb3bb..6294316f24c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -827,6 +827,7 @@ struct amdgpu_device {
>   /* For pre-DCE11. DCE11 and later are in "struct amdgpu_device->dm" */
>   struct work_struct  hotplug_work;
>   struct amdgpu_irq_src   crtc_irq;
> + struct amdgpu_irq_src   vupdate_irq;
>   struct amdgpu_irq_src   pageflip_irq;
>   struct amdgpu_irq_src   hpd_irq;
>   
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index aabd23fa0766..fe207988d0b2 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -315,6 +315,32 @@ static void dm_pflip_high_irq(void *interrupt_params)
>   drm_crtc_vblank_put(_crtc->base);
>   }
>   
> +static void dm_vupdate_high_irq(void *interrupt_params)
> +{
> + struct common_irq_params *irq_params = interrupt_params;
> + struct amdgpu_device *adev = irq_params->adev;
> + struct amdgpu_crtc *acrtc;
> + struct dm_crtc_state *acrtc_state;
> +
> + acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - 
> IRQ_TYPE_VUPDATE);
> +
> + if (acrtc) {
> + acrtc_state = to_dm_crtc_state(acrtc->base.state);
> +
> + DRM_DEBUG_DRIVER("crtc:%d, vupdate-vrr:%d\n", acrtc->crtc_id,
> +  amdgpu_dm_vrr_active(acrtc_state));
> +
> + /* Core vblank handling is done here after end of front-porch in
> +  * vrr mode, as vblank timestamping will give valid results
> +  * while now done after front-porch. This will also deliver
> +  * page-flip completion events that have been queued to us
> +  * if a pageflip happened inside front-porch.
> +  */
> + if (amdgpu_dm_vrr_active(acrtc_state))
> + drm_crtc_handle_vblank(>base);
> + }
> +}
> +
>   static void dm_crtc_high_irq(void *interrupt_params)
>   {
>   struct common_irq_params *irq_params = interrupt_params;
> @@ -325,11 +351,24 @@ static void dm_crtc_high_irq(void *interrupt_params)
>   acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - 
> IRQ_TYPE_VBLANK);
>   
>   if (acrtc) {
> - drm_crtc_handle_vblank(>base);
> - amdgpu_dm_crtc_handle_crc_irq(>base);
> -
>   acrtc_state = to_dm_crtc_state(acrtc->base.state);
>   
> + 

Re: WARNING at amdgpu_vm_sdma.c:85

[Michel Dänzer]

On 2019-03-26 4:21 p.m., Michel Dänzer wrote:
> 
> I'm hitting some badness with piglit today, see the attached dmesg
> excerpt. I was away for a couple of days, but looks like it might be
> related to the VM changes? I'll try bisecting.

Bisected to:

602b5050d839c40572bbb19e9b17a37a150c1e3c is the first bad commit
commit 602b5050d839c40572bbb19e9b17a37a150c1e3c
Author: Christian König 
Date:   Mon Mar 18 14:26:24 2019 +0100

drm/amdgpu: use the new VM backend for PDEs

And remove the existing code when it is unused.

Signed-off-by: Christian König 
Reviewed-by: Chunming Zhou 
Reviewed-by: Felix Kuehling 


The problem doesn't happen with this commit and the corresponding PTE
one reverted on current amd-staging-drm-next.


-- 
Earthling Michel Dänzer   |  https://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: XGMI pstate switch initial support

[Christian König]

The problem is that userspace could add it first and then change the domain.

Pretty unlikely thought,
Christian.

Am 26.03.19 um 14:15 schrieb Liu, Shaoyun:
I think in a real usage  ( tensorflow ex),  it's rarely only the 
system memory (no vram) will be mapped for peer access.
Anyway, how about add preferred_domain check for xgmi ? I think even 
user use ioctl to change the preferred_domain,  bo_add should still be 
called before the real mapping.


Regards
Shaoyun.liu

*From:* amd-gfx  on behalf of 
Kuehling, Felix 

*Sent:* March 25, 2019 6:28:32 PM
*To:* Liu, Shaoyun; amd-gfx@lists.freedesktop.org
*Subject:* Re: [PATCH] drm/amdgpu: XGMI pstate switch initial support
I don't see any check for the memory type. As far as I can tell you'll
power up XGMI even for system memory mappings. See inline.

On 2019-03-22 3:28 p.m., Liu, Shaoyun wrote:
> Driver vote low to high pstate switch whenever there is an outstanding
> XGMI mapping request. Driver vote high to low pstate when all the
> outstanding XGMI mapping is terminated.
>
> Change-Id: I197501f853c47f844055c0e28c0ac00a1ff06607
> Signed-off-by: shaoyunl 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 21 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  4 
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   | 16 +++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   | 10 ++
>   6 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> index ec9562d..c4c61e9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2018,6 +2018,10 @@ static void 
amdgpu_device_ip_late_init_func_handler(struct work_struct *work)

>    r = amdgpu_device_enable_mgpu_fan_boost();
>    if (r)
>    DRM_ERROR("enable mgpu fan boost failed (%d).\n", r);
> +
> + /*set to low pstate by default */
> + amdgpu_xgmi_set_pstate(adev, 0);
> +
>   }
>
>   static void amdgpu_device_delay_enable_gfx_off(struct work_struct 
*work)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h

> index 220a6a7..c430e82 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -72,6 +72,8 @@ struct amdgpu_bo_va {
>
>    /* If the mappings are cleared or filled */
>    bool    cleared;
> +
> + bool    is_xgmi;
>   };
>
>   struct amdgpu_bo {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

> index 729da1c..a7247d5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -34,6 +34,7 @@
>   #include "amdgpu_trace.h"
>   #include "amdgpu_amdkfd.h"
>   #include "amdgpu_gmc.h"
> +#include "amdgpu_xgmi.h"
>
>   /**
>    * DOC: GPUVM
> @@ -2072,6 +2073,15 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
amdgpu_device *adev,

>    INIT_LIST_HEAD(_va->valids);
>    INIT_LIST_HEAD(_va->invalids);
>
> + if (bo && amdgpu_xgmi_same_hive(adev, 
amdgpu_ttm_adev(bo->tbo.bdev))) {

> + bo_va->is_xgmi = true;

You're setting this to true even for system memory BOs that don't
involve XGMI mappings. That means you'll power up XGMI unnecessarily in
many cases because KFD processes always have system memory mappings that
are mapped to all GPUs (e.g. the signal page).

Regards,
   Felix


> + mutex_lock(>vm_manager.lock_pstate);
> + /* Power up XGMI if it can be potentially used */
> + if (++adev->vm_manager.xgmi_map_counter == 1)
> + amdgpu_xgmi_set_pstate(adev, 1);
> + mutex_unlock(>vm_manager.lock_pstate);
> + }
> +
>    return bo_va;
>   }
>
> @@ -2490,6 +2500,14 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>    }
>
>    dma_fence_put(bo_va->last_pt_update);
> +
> + if (bo && bo_va->is_xgmi) {
> + mutex_lock(>vm_manager.lock_pstate);
> + if (--adev->vm_manager.xgmi_map_counter == 0)
> + amdgpu_xgmi_set_pstate(adev, 0);
> + mutex_unlock(>vm_manager.lock_pstate);
> + }
> +
>    kfree(bo_va);
>   }
>
> @@ -2997,6 +3015,9 @@ void amdgpu_vm_manager_init(struct 
amdgpu_device *adev)

>
>    idr_init(>vm_manager.pasid_idr);
> spin_lock_init(>vm_manager.pasid_lock);
> +
> + adev->vm_manager.xgmi_map_counter = 0;
> + mutex_init(>vm_manager.lock_pstate);
>   }
>
>   /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h

> index 520122b..f586b38 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -324,6 +324,10 @@ struct 

[PATCH] drm/amdgpu: Set correct context adev for gem object

[Liu, Shaoyun]

The context device pointer could be different of the object been acctually 
allocated

Change-Id: I7b2338858126d75350b65ff04d9bb419e1eae15c
Signed-off-by: shaoyunl 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9ee8d7a..a82c3b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -117,8 +117,8 @@ int amdgpu_gem_object_open(struct drm_gem_object *obj,
   struct drm_file *file_priv)
 {
struct amdgpu_bo *abo = gem_to_amdgpu_bo(obj);
-   struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
+   struct amdgpu_device *adev = fpriv->ctx_mgr.adev;
struct amdgpu_vm *vm = >vm;
struct amdgpu_bo_va *bo_va;
struct mm_struct *mm;
@@ -150,8 +150,8 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
 struct drm_file *file_priv)
 {
struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
-   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
+   struct amdgpu_device *adev = fpriv->ctx_mgr.adev;
struct amdgpu_vm *vm = >vm;
 
struct amdgpu_bo_list_entry vm_pd;
-- 
2.7.4

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

Re: [PATCH] drm/amdgpu: XGMI pstate switch initial support

[Liu, Shaoyun]

Can we simply failed the set placement call  if we find the bo_va is 
existing and  bo_va->is_xgmi flag is set ?  The original call also 
failed when it detect it's shared and  the  placement is in VRAM.

Regards

shaoyun.liu


On 2019-03-26 10:00 a.m., Kuehling, Felix wrote:
> On 2019-03-26 9:15 a.m., Liu, Shaoyun wrote:
>> I think in a real usage  ( tensorflow ex),  it's rarely only the
>> system memory (no vram) will be mapped for peer access.
> With that argument you could simplify your change and just power up XGMI
> as soon as a KFD process starts. Because that's effectively what happens
> with your change as it is now, if you don't check the memory type. Every
> KFD process maps the signal page (in system memory) to all GPUs. So you
> will always increment the xgmi_map_count even before the first VRAM BO
> is allocated, let alone mapped to multiple GPUs.
>
>
>> Anyway, how about add preferred_domain check for xgmi ? I think even
>> user use ioctl to change the preferred_domain,  bo_add should still be
>> called before the real mapping.
> amdgpu_gem_op_ioctl with AMDGPU_GEM_OP_SET_PLACEMENT doesn't call
> bo_add. You'd have to add something in amdgpu_gem_op_ioctl to
> re-evaluate all bo_vas of the BO when its placement changes, update the
> bo_va->is_xgmi flag, and if necessary the xgmi_map_counter.
>
> Regards,
>     Felix
>
>
>> Regards
>> Shaoyun.liu
>> 
>> *From:* amd-gfx  on behalf of
>> Kuehling, Felix 
>> *Sent:* March 25, 2019 6:28:32 PM
>> *To:* Liu, Shaoyun; amd-gfx@lists.freedesktop.org
>> *Subject:* Re: [PATCH] drm/amdgpu: XGMI pstate switch initial support
>> I don't see any check for the memory type. As far as I can tell you'll
>> power up XGMI even for system memory mappings. See inline.
>>
>> On 2019-03-22 3:28 p.m., Liu, Shaoyun wrote:
>>> Driver vote low to high pstate switch whenever there is an outstanding
>>> XGMI mapping request. Driver vote high to low pstate when all the
>>> outstanding XGMI mapping is terminated.
>>>
>>> Change-Id: I197501f853c47f844055c0e28c0ac00a1ff06607
>>> Signed-off-by: shaoyunl 
>>> ---
>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 
>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 ++
>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 21 +
>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  4 
>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   | 16 +++-
>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   | 10 ++
>>>     6 files changed, 56 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index ec9562d..c4c61e9 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -2018,6 +2018,10 @@ static void
>> amdgpu_device_ip_late_init_func_handler(struct work_struct *work)
>>>      r = amdgpu_device_enable_mgpu_fan_boost();
>>>      if (r)
>>>      DRM_ERROR("enable mgpu fan boost failed (%d).\n", r);
>>> +
>>> + /*set to low pstate by default */
>>> + amdgpu_xgmi_set_pstate(adev, 0);
>>> +
>>>     }
>>>
>>>     static void amdgpu_device_delay_enable_gfx_off(struct work_struct
>> *work)
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> index 220a6a7..c430e82 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> @@ -72,6 +72,8 @@ struct amdgpu_bo_va {
>>>
>>>      /* If the mappings are cleared or filled */
>>>      bool    cleared;
>>> +
>>> + bool    is_xgmi;
>>>     };
>>>
>>>     struct amdgpu_bo {
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 729da1c..a7247d5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -34,6 +34,7 @@
>>>     #include "amdgpu_trace.h"
>>>     #include "amdgpu_amdkfd.h"
>>>     #include "amdgpu_gmc.h"
>>> +#include "amdgpu_xgmi.h"
>>>
>>>     /**
>>>      * DOC: GPUVM
>>> @@ -2072,6 +2073,15 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct
>> amdgpu_device *adev,
>>>      INIT_LIST_HEAD(_va->valids);
>>>      INIT_LIST_HEAD(_va->invalids);
>>>
>>> + if (bo && amdgpu_xgmi_same_hive(adev,
>> amdgpu_ttm_adev(bo->tbo.bdev))) {
>>> + bo_va->is_xgmi = true;
>> You're setting this to true even for system memory BOs that don't
>> involve XGMI mappings. That means you'll power up XGMI unnecessarily in
>> many cases because KFD processes always have system memory mappings that
>> are mapped to all GPUs (e.g. the signal page).
>>
>> Regards,
>>     Felix
>>
>>
>>> + mutex_lock(>vm_manager.lock_pstate);
>>> + /* Power up XGMI if it can be potentially used */
>>> + if 

WARNING at amdgpu_vm_sdma.c:85

[Michel Dänzer]

I'm hitting some badness with piglit today, see the attached dmesg
excerpt. I was away for a couple of days, but looks like it might be
related to the VM changes? I'll try bisecting.


-- 
Earthling Michel Dänzer   |  https://www.amd.com
Libre software enthusiast | Mesa and X developer
Mar 26 13:39:22 kaveri kernel: [ 2583.307294] WARNING: CPU: 8 PID: 6574 at drivers/gpu/drm//amd/amdgpu/amdgpu_vm_sdma.c:85 amdgpu_vm_sdma_commit+0x358/0x480 [amdgpu]
Mar 26 13:39:22 kaveri kernel: [ 2583.307670] Modules linked in: fuse(E) ipt_MASQUERADE(E) nf_conntrack_netlink(E) nfnetlink(E) xfrm_user(E) xfrm_algo(E) iptable_nat(E) nf_nat_ipv4(E) xt_addrtype(E) iptable_filter(E) bpfilter(E) xt_conntrack(E) nf_nat(E) nf_conntrack(E) nf_defrag_ipv6(E) nf_defrag_ipv4(E) libcrc32c(E) br_netfilter(E) bridge(E) stp(E) llc(E) overlay(E) lz4(E) lz4_compress(E) cpufreq_powersave(E) cpufreq_userspace(E) cpufreq_conservative(E) amdgpu(OE) gpu_sched(OE) binfmt_misc(E) nls_ascii(E) nls_cp437(E) vfat(E) fat(E) edac_mce_amd(E) kvm(E) irqbypass(E) radeon(OE) snd_hda_codec_realtek(E) snd_hda_codec_generic(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) crct10dif_pclmul(E) crc32_pclmul(E) ttm(OE) snd_hda_intel(E) snd_hda_codec(E) drm_kms_helper(OE) ghash_clmulni_intel(E) snd_hda_core(E) efi_pstore(E) wmi_bmof(E) snd_hwdep(E) drm(OE) realtek(E) aesni_intel(E) snd_pcm(E) i2c_algo_bit(E) aes_x86_64(E) snd_timer(E) crypto_simd(E) r8169(E) fb_sys_fops(E) sg(E) cryptd(E) syscopyarea(E) sp5100_tco(E) glue_helper(E)
Mar 26 13:39:22 kaveri kernel: [ 2583.307738]  ccp(E) sysfillrect(E) snd(E) pcspkr(E) efivars(E) rng_core(E) sysimgblt(E) libphy(E) i2c_piix4(E) soundcore(E) k10temp(E) wmi(E) pcc_cpufreq(E) button(E) acpi_cpufreq(E) tcp_bbr(E) sch_fq(E) sunrpc(E) nct6775(E) hwmon_vid(E) efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) fscrypto(E) dm_mod(E) raid10(E) raid1(E) raid0(E) multipath(E) linear(E) md_mod(E) sd_mod(E) evdev(E) hid_generic(E) usbhid(E) hid(E) ahci(E) libahci(E) xhci_pci(E) libata(E) xhci_hcd(E) crc32c_intel(E) scsi_mod(E) usbcore(E) gpio_amdpt(E) gpio_generic(E)
Mar 26 13:39:22 kaveri kernel: [ 2583.307804] CPU: 8 PID: 6574 Comm: shader_run:cs0 Tainted: G   OE 5.0.0-rc1-00651-g4a93cf78b903 #121
Mar 26 13:39:22 kaveri kernel: [ 2583.307811] Hardware name: Micro-Star International Co., Ltd. MS-7A34/B350 TOMAHAWK (MS-7A34), BIOS 1.80 09/13/2017
Mar 26 13:39:22 kaveri kernel: [ 2583.307935] RIP: 0010:amdgpu_vm_sdma_commit+0x358/0x480 [amdgpu]
Mar 26 13:39:22 kaveri kernel: [ 2583.307943] Code: 48 c1 ea 03 80 3c 02 00 0f 85 34 01 00 00 48 8b 7b 18 e8 8b 5b 28 00 eb a1 48 89 df e8 a1 47 c5 cf eb 97 0f 0b e9 d3 fe ff ff <0f> 0b e9 fa fd ff ff e8 fc 8b 23 cf e9 8e fe ff ff 48 89 44 24 10
Mar 26 13:39:22 kaveri kernel: [ 2583.307949] RSP: 0018:88835a2ef4e8 EFLAGS: 00010246
Mar 26 13:39:22 kaveri kernel: [ 2583.307956] RAX: 888341c8af18 RBX: 88835a2ef6c0 RCX: 
Mar 26 13:39:22 kaveri kernel: [ 2583.307962] RDX: 888341c8afe8 RSI: 88814063fbb0 RDI: 88814063fbb8
Mar 26 13:39:22 kaveri kernel: [ 2583.307967] RBP: 11106b45dea0 R08: 1110280c7f77 R09: ed1068390aee
Mar 26 13:39:22 kaveri kernel: [ 2583.307973] R10: ed1068390aee R11: 888341c85773 R12: 88835a2ef6e0
Mar 26 13:39:22 kaveri kernel: [ 2583.307979] R13: 888367d2c4e0 R14: 888360bfa200 R15: 88835a2ef6c8
Mar 26 13:39:22 kaveri kernel: [ 2583.307985] FS:  7fc032275700() GS:88837e00() knlGS:
Mar 26 13:39:22 kaveri kernel: [ 2583.307990] CS:  0010 DS:  ES:  CR0: 80050033
Mar 26 13:39:22 kaveri kernel: [ 2583.307995] CR2: 7f80c450d000 CR3: 00015d8e6000 CR4: 003406e0
Mar 26 13:39:22 kaveri kernel: [ 2583.308000] Call Trace:
Mar 26 13:39:22 kaveri kernel: [ 2583.308128]  ? amdgpu_vm_cpu_prepare+0xc0/0xc0 [amdgpu]
Mar 26 13:39:22 kaveri kernel: [ 2583.308255]  ? amdgpu_vm_sdma_prepare+0x170/0x2e0 [amdgpu]
Mar 26 13:39:22 kaveri kernel: [ 2583.308382]  amdgpu_vm_update_directories+0x664/0xa60 [amdgpu]
Mar 26 13:39:22 kaveri kernel: [ 2583.308511]  ? amdgpu_vm_map_gart+0x40/0x40 [amdgpu]
Mar 26 13:39:22 kaveri kernel: [ 2583.308633]  ? amdgpu_vm_handle_moved+0x28c/0x360 [amdgpu]
Mar 26 13:39:22 kaveri kernel: [ 2583.308649]  ? lock_downgrade+0x5d0/0x5d0
Mar 26 13:39:22 kaveri kernel: [ 2583.308657]  ? rwlock_bug.part.2+0x90/0x90
Mar 26 13:39:22 kaveri kernel: [ 2583.308788]  ? amdgpu_vm_handle_moved+0x28c/0x360 [amdgpu]
Mar 26 13:39:22 kaveri kernel: [ 2583.308917]  amdgpu_cs_ioctl+0x2f57/0x4850 [amdgpu]
Mar 26 13:39:22 kaveri kernel: [ 2583.309047]  ? amdgpu_cs_find_mapping+0x3c0/0x3c0 [amdgpu]
Mar 26 13:39:22 kaveri kernel: [ 2583.309055]  ? __switch_to_asm+0x40/0x70
Mar 26 13:39:22 kaveri kernel: [ 2583.309067]  ? __lock_acquire+0x5d6/0x4650
Mar 26 13:39:22 kaveri kernel: [ 2583.309076]  ? futex_wait_queue_me+0x1c3/0x510
Mar 26 13:39:22 kaveri 

Re: [PATCH 16/21] drm/radeon: Use drm_fb_helper_fill_info

[Noralf Trønnes]

Den 26.03.2019 14.20, skrev Daniel Vetter:
> This should not result in any changes.
> 
> v2: Rebase
> 
> Signed-off-by: Daniel Vetter 
> Cc: Alex Deucher 
> Cc: "Christian König" 
> Cc: "David (ChunMing) Zhou" 
> Cc: amd-gfx@lists.freedesktop.org
> ---

Acked-by: Noralf Trønnes 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Limit gpu max clock for ryzen 2400g

[Lauri Ehrenpreis]

Hi!

Thanx for helping, but I have tried those methods and they didn't work on
Ryzen 2400G.

--
Lauri

On Tue, Mar 26, 2019 at 2:57 PM Russell, Kent  wrote:

> Hi Lauri,
>
>
>
> There’s a more efficient method using the Power Profiles (and optionally,
> the ROCM-SMI tool, found at https://github.com/RadeonOpenCompute/ROC-smi),
> or the pp_sclk mask, depending on what exactly you want. I’ll list out the
> methods here and the rocm-smi and non-SMI commands to do it. I’ll assume
> that this GPU is card0 (it may be card1, card2, etc, depending on what GPUs
> are installed on your system; “rocm-smi -i” or “cat
> /sys/class/drm/card?/device/device will give you the GPU IDs of all of the
> cards, then you can figure out which one you want to use)
>
>
>
>1. Mask the SCLKs . pp_dpm_sclk allows you to set a mask of what
>levels to use.
>   1. First, read the values (“rocm-smi --showclkfrq” , or “cat
>   /sys/class/drm/card0/device/pp_dpm_sclk”) and see the supported DPM 
> levels
>   for your card.
>   2. Mask off the levels that you don’t want. E.g. If you only want
>   to use levels 0-6 (and thus skip level 7), you can do either ‘rocm-smi
>   --setsclk 0 1 2 3 4 5 6’ or ‘echo manual >
>   /sys/class/drm/card0/device/power_dpm_force_performance_level && echo 
> “0 1
>   2 3 4 5 6” > /sys/class/drm/card0/device/pp_dpm_sclk’ . This will set 
> DPM
>   to only use levels 0-6 and skip level 7. You can do this for any
>   combination of levels or a single level (“0 2 5”, “1 2 7”, “5”, etc). 
> That
>   will tell it to only use the specified DPM levels and will persist until
>   reboot, or until the power_dpm_force_performance is set back to ‘auto’ .
>2. Set the specific DPM level values manually:
>   1. First, you’ll need to enable the Power Profile Overdrive
>   functionality. The easiest way is to add 
> “amdgpu.ppfeaturemask=0x”
>   to your linux command line parameters (by editing /boot/grub/grub.cfg
>   manually, editing /etc/default/grub and doing an update-grub, or 
> manually
>   entering the kernel parameter in the GRUB menu before booting).
>   2. Once that’s enabled, you should see the following file:
>   /sys/class/drm/card0/device/pp_od_clk_voltage.
>   3. Check the current DPM level information with “rocm-smi -S” or
>   “cat /sys/class/drm/card0/device/pp_od_clk_voltage file” . Now that we 
> have
>   that, you can see the supported SCLK and voltages for each level.
>   4. You can use the rocm-smi tool to manually change the levels
>   through “rocm-smi --setslevel # MHZ VLT”, where:
>
>i.   #
> is the level (level 7 is probably the one you want, but you can do it for
> all of them)
>
>  ii.  MHZ
> is the speed in MHz
>
>iii.  VLT
> is the voltage in mV.
>
>1. Honestly, you can probably just copy the highest level that you’re
>   comfortable with and set that for all of the levels that exceed the 
> values
>   that you desire. So if you want to keep it to whatever level 6 is, just 
> set
>   level 7 to have the same values as level 6 (that way you don’t have to 
> muck
>   with voltages and such). Or if 5 is the highest that you want, set 
> level 6
>   and level 7 to match level 5
>
>
>
> Hopefully that helps. It also means that you don’t have to constantly try
> to build your own kernel with a change to cap the SCLK cherry-picked on
> tpo. Please let me know if you have any questions at all!
>
>
>
> Kent
>
>
>
> *From:* amd-gfx  *On Behalf Of *Lauri
> Ehrenpreis
> *Sent:* Friday, March 22, 2019 6:18 AM
> *To:* amd-gfx list 
> *Subject:* Limit gpu max clock for ryzen 2400g
>
>
>
> Hi!
>
>
>
> Is there a way how to limit gpu max clock rate? Currently I can either
> leave the clock to automatic mode or force it to specific level
> via /sys/class/drm/card0/device/pp_dpm_sclk. But ideally I would like the
> clock to be automatically regulated but specify a different upper limit for
> power saving reasons.
>
>
>
> --
>
> Lauri
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: XGMI pstate switch initial support

[Kuehling, Felix]

On 2019-03-26 9:15 a.m., Liu, Shaoyun wrote:
> I think in a real usage  ( tensorflow ex),  it's rarely only the 
> system memory (no vram) will be mapped for peer access.

With that argument you could simplify your change and just power up XGMI 
as soon as a KFD process starts. Because that's effectively what happens 
with your change as it is now, if you don't check the memory type. Every 
KFD process maps the signal page (in system memory) to all GPUs. So you 
will always increment the xgmi_map_count even before the first VRAM BO 
is allocated, let alone mapped to multiple GPUs.


> Anyway, how about add preferred_domain check for xgmi ? I think even 
> user use ioctl to change the preferred_domain,  bo_add should still be 
> called before the real mapping.

amdgpu_gem_op_ioctl with AMDGPU_GEM_OP_SET_PLACEMENT doesn't call 
bo_add. You'd have to add something in amdgpu_gem_op_ioctl to 
re-evaluate all bo_vas of the BO when its placement changes, update the 
bo_va->is_xgmi flag, and if necessary the xgmi_map_counter.

Regards,
   Felix


>
> Regards
> Shaoyun.liu
> 
> *From:* amd-gfx  on behalf of 
> Kuehling, Felix 
> *Sent:* March 25, 2019 6:28:32 PM
> *To:* Liu, Shaoyun; amd-gfx@lists.freedesktop.org
> *Subject:* Re: [PATCH] drm/amdgpu: XGMI pstate switch initial support
> I don't see any check for the memory type. As far as I can tell you'll
> power up XGMI even for system memory mappings. See inline.
>
> On 2019-03-22 3:28 p.m., Liu, Shaoyun wrote:
> > Driver vote low to high pstate switch whenever there is an outstanding
> > XGMI mapping request. Driver vote high to low pstate when all the
> > outstanding XGMI mapping is terminated.
> >
> > Change-Id: I197501f853c47f844055c0e28c0ac00a1ff06607
> > Signed-off-by: shaoyunl 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 ++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 21 +
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  4 
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   | 16 +++-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   | 10 ++
> >   6 files changed, 56 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index ec9562d..c4c61e9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -2018,6 +2018,10 @@ static void 
> amdgpu_device_ip_late_init_func_handler(struct work_struct *work)
> >    r = amdgpu_device_enable_mgpu_fan_boost();
> >    if (r)
> >    DRM_ERROR("enable mgpu fan boost failed (%d).\n", r);
> > +
> > + /*set to low pstate by default */
> > + amdgpu_xgmi_set_pstate(adev, 0);
> > +
> >   }
> >
> >   static void amdgpu_device_delay_enable_gfx_off(struct work_struct 
> *work)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > index 220a6a7..c430e82 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > @@ -72,6 +72,8 @@ struct amdgpu_bo_va {
> >
> >    /* If the mappings are cleared or filled */
> >    bool    cleared;
> > +
> > + bool    is_xgmi;
> >   };
> >
> >   struct amdgpu_bo {
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > index 729da1c..a7247d5 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -34,6 +34,7 @@
> >   #include "amdgpu_trace.h"
> >   #include "amdgpu_amdkfd.h"
> >   #include "amdgpu_gmc.h"
> > +#include "amdgpu_xgmi.h"
> >
> >   /**
> >    * DOC: GPUVM
> > @@ -2072,6 +2073,15 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
> amdgpu_device *adev,
> >    INIT_LIST_HEAD(_va->valids);
> >    INIT_LIST_HEAD(_va->invalids);
> >
> > + if (bo && amdgpu_xgmi_same_hive(adev, 
> amdgpu_ttm_adev(bo->tbo.bdev))) {
> > + bo_va->is_xgmi = true;
>
> You're setting this to true even for system memory BOs that don't
> involve XGMI mappings. That means you'll power up XGMI unnecessarily in
> many cases because KFD processes always have system memory mappings that
> are mapped to all GPUs (e.g. the signal page).
>
> Regards,
>    Felix
>
>
> > + mutex_lock(>vm_manager.lock_pstate);
> > + /* Power up XGMI if it can be potentially used */
> > + if (++adev->vm_manager.xgmi_map_counter == 1)
> > + amdgpu_xgmi_set_pstate(adev, 1);
> > + mutex_unlock(>vm_manager.lock_pstate);
> > + }
> > +
> >    return bo_va;
> >   }
> >
> > @@ -2490,6 +2500,14 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
> >    }
> >
> >    dma_fence_put(bo_va->last_pt_update);
> > +
> > + if (bo && 

[PATCH 16/21] drm/radeon: Use drm_fb_helper_fill_info

[Daniel Vetter]

This should not result in any changes.

v2: Rebase

Signed-off-by: Daniel Vetter 
Cc: Alex Deucher 
Cc: "Christian König" 
Cc: "David (ChunMing) Zhou" 
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/radeon/radeon_fb.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fb.c 
b/drivers/gpu/drm/radeon/radeon_fb.c
index d50bff20f7de..1298b84cb1c7 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -42,7 +42,7 @@
  * the helper contains a pointer to radeon framebuffer baseclass.
  */
 struct radeon_fbdev {
-   struct drm_fb_helper helper;
+   struct drm_fb_helper helper; /* must be first */
struct drm_framebuffer fb;
struct radeon_device *rdev;
 };
@@ -247,8 +247,6 @@ static int radeonfb_create(struct drm_fb_helper *helper,
/* radeon resume is fragile and needs a vt switch to help it along */
info->skip_vt_switch = false;
 
-   info->par = rfbdev;
-
ret = radeon_framebuffer_init(rdev->ddev, >fb, _cmd, gobj);
if (ret) {
DRM_ERROR("failed to initialize framebuffer %d\n", ret);
@@ -262,10 +260,6 @@ static int radeonfb_create(struct drm_fb_helper *helper,
 
memset_io(rbo->kptr, 0x0, radeon_bo_size(rbo));
 
-   strcpy(info->fix.id, "radeondrmfb");
-
-   drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
-
info->fbops = _ops;
 
tmp = radeon_bo_gpu_offset(rbo) - rdev->mc.vram_start;
@@ -274,7 +268,7 @@ static int radeonfb_create(struct drm_fb_helper *helper,
info->screen_base = rbo->kptr;
info->screen_size = radeon_bo_size(rbo);
 
-   drm_fb_helper_fill_var(info, >helper, sizes->fb_width, 
sizes->fb_height);
+   drm_fb_helper_fill_info(info, >helper, sizes);
 
/* setup aperture base/size for vesafb takeover */
info->apertures->ranges[0].base = rdev->ddev->mode_config.fb_base;
-- 
2.20.1

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

Re: [PATCH] drm/amdgpu: XGMI pstate switch initial support

[Liu, Shaoyun]

I think in a real usage  ( tensorflow ex),  it's rarely only the system memory 
(no vram) will be mapped for peer access.
Anyway, how about add preferred_domain check for xgmi ? I think even user use 
ioctl to change the preferred_domain,  bo_add should still be called before the 
real mapping.

Regards
Shaoyun.liu

From: amd-gfx  on behalf of Kuehling, 
Felix 
Sent: March 25, 2019 6:28:32 PM
To: Liu, Shaoyun; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: XGMI pstate switch initial support

I don't see any check for the memory type. As far as I can tell you'll
power up XGMI even for system memory mappings. See inline.

On 2019-03-22 3:28 p.m., Liu, Shaoyun wrote:
> Driver vote low to high pstate switch whenever there is an outstanding
> XGMI mapping request. Driver vote high to low pstate when all the
> outstanding XGMI mapping is terminated.
>
> Change-Id: I197501f853c47f844055c0e28c0ac00a1ff06607
> Signed-off-by: shaoyunl 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 21 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  4 
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   | 16 +++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   | 10 ++
>   6 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index ec9562d..c4c61e9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2018,6 +2018,10 @@ static void 
> amdgpu_device_ip_late_init_func_handler(struct work_struct *work)
>r = amdgpu_device_enable_mgpu_fan_boost();
>if (r)
>DRM_ERROR("enable mgpu fan boost failed (%d).\n", r);
> +
> + /*set to low pstate by default */
> + amdgpu_xgmi_set_pstate(adev, 0);
> +
>   }
>
>   static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 220a6a7..c430e82 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -72,6 +72,8 @@ struct amdgpu_bo_va {
>
>/* If the mappings are cleared or filled */
>boolcleared;
> +
> + boolis_xgmi;
>   };
>
>   struct amdgpu_bo {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 729da1c..a7247d5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -34,6 +34,7 @@
>   #include "amdgpu_trace.h"
>   #include "amdgpu_amdkfd.h"
>   #include "amdgpu_gmc.h"
> +#include "amdgpu_xgmi.h"
>
>   /**
>* DOC: GPUVM
> @@ -2072,6 +2073,15 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
> amdgpu_device *adev,
>INIT_LIST_HEAD(_va->valids);
>INIT_LIST_HEAD(_va->invalids);
>
> + if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
> + bo_va->is_xgmi = true;

You're setting this to true even for system memory BOs that don't
involve XGMI mappings. That means you'll power up XGMI unnecessarily in
many cases because KFD processes always have system memory mappings that
are mapped to all GPUs (e.g. the signal page).

Regards,
   Felix


> + mutex_lock(>vm_manager.lock_pstate);
> + /* Power up XGMI if it can be potentially used */
> + if (++adev->vm_manager.xgmi_map_counter == 1)
> + amdgpu_xgmi_set_pstate(adev, 1);
> + mutex_unlock(>vm_manager.lock_pstate);
> + }
> +
>return bo_va;
>   }
>
> @@ -2490,6 +2500,14 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>}
>
>dma_fence_put(bo_va->last_pt_update);
> +
> + if (bo && bo_va->is_xgmi) {
> + mutex_lock(>vm_manager.lock_pstate);
> + if (--adev->vm_manager.xgmi_map_counter == 0)
> + amdgpu_xgmi_set_pstate(adev, 0);
> + mutex_unlock(>vm_manager.lock_pstate);
> + }
> +
>kfree(bo_va);
>   }
>
> @@ -2997,6 +3015,9 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
>
>idr_init(>vm_manager.pasid_idr);
>spin_lock_init(>vm_manager.pasid_lock);
> +
> + adev->vm_manager.xgmi_map_counter = 0;
> + mutex_init(>vm_manager.lock_pstate);
>   }
>
>   /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 520122b..f586b38 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -324,6 +324,10 @@ struct amdgpu_vm_manager {
> */
>struct idr  pasid_idr;
>spinlock_t  pasid_lock;
> 

RE: [PATCH] drm/amd/powerplay: update current profile mode only when it's really applied

[Russell, Kent]

That makes sense.

Reviewed-by: Kent Russell 


> -Original Message-
> From: amd-gfx  On Behalf Of Evan
> Quan
> Sent: Tuesday, March 26, 2019 5:34 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan ; Russell, Kent
> 
> Subject: [PATCH] drm/amd/powerplay: update current profile mode only
> when it's really applied
> 
> No need to update current profile mode if the new profile mode does not
> take effect in fact.
> 
> Change-Id: If24474d201840bfaefacc189f6a6ff6856695dff
> Signed-off-by: Evan Quan 
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  9 +
> drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 13 +++---
> ---
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> index ed6c638700f5..85a536924571 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> @@ -4904,13 +4904,12 @@ static int
> vega10_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, ui
>   uint8_t FPS;
>   uint8_t use_rlc_busy;
>   uint8_t min_active_level;
> -
> - hwmgr->power_profile_mode = input[size];
> + uint32_t power_profile_mode = input[size];
> 
>   smum_send_msg_to_smc_with_parameter(hwmgr,
> PPSMC_MSG_SetWorkloadMask,
> - 1< >power_profile_mode);
> + 1 << power_profile_mode);
> 
> - if (hwmgr->power_profile_mode ==
> PP_SMC_POWER_PROFILE_CUSTOM) {
> + if (power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) {
>   if (size == 0 || size > 4)
>   return -EINVAL;
> 
> @@ -4924,6 +4923,8 @@ static int vega10_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>   use_rlc_busy << 16 |
> min_active_level<<24);
>   }
> 
> + hwmgr->power_profile_mode = power_profile_mode;
> +
>   return 0;
>  }
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> index 664544e7fcdc..3f349ada8de0 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> @@ -3819,15 +3819,14 @@ static int
> vega20_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, ui
> {
>   DpmActivityMonitorCoeffInt_t activity_monitor;
>   int workload_type, result = 0;
> + uint32_t power_profile_mode = input[size];
> 
> - hwmgr->power_profile_mode = input[size];
> -
> - if (hwmgr->power_profile_mode >
> PP_SMC_POWER_PROFILE_CUSTOM) {
> - pr_err("Invalid power profile mode %d\n", hwmgr-
> >power_profile_mode);
> + if (power_profile_mode > PP_SMC_POWER_PROFILE_CUSTOM) {
> + pr_err("Invalid power profile mode %d\n",
> power_profile_mode);
>   return -EINVAL;
>   }
> 
> - if (hwmgr->power_profile_mode ==
> PP_SMC_POWER_PROFILE_CUSTOM) {
> + if (power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) {
>   if (size < 10)
>   return -EINVAL;
> 
> @@ -3895,10 +3894,12 @@ static int
> vega20_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, ui
> 
>   /* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */
>   workload_type =
> - conv_power_profile_to_pplib_workload(hwmgr-
> >power_profile_mode);
> +
>   conv_power_profile_to_pplib_workload(power_profile_mode);
>   smum_send_msg_to_smc_with_parameter(hwmgr,
> PPSMC_MSG_SetWorkloadMask,
>   1 << workload_type);
> 
> + hwmgr->power_profile_mode = power_profile_mode;
> +
>   return 0;
>  }
> 
> --
> 2.21.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 2/2] drm/amd/powerplay: correct data type to avoid overflow

[Deucher, Alexander]

Series is:
Reviewed-by: Alex Deucher 

From: amd-gfx  on behalf of Quan, Evan 

Sent: Tuesday, March 26, 2019 6:15 AM
To: Quan, Evan; amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH 2/2] drm/amd/powerplay: correct data type to avoid overflow

Ping..

> -Original Message-
> From: Evan Quan 
> Sent: 2019年3月23日 2:07
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan 
> Subject: [PATCH 2/2] drm/amd/powerplay: correct data type to avoid
> overflow
>
> Avoid left shift overflow.
>
> Change-Id: If03f4f4d440b6d742d8eaa23d0bae6ddd21c01ea
> Signed-off-by: Evan Quan 
> ---
>  drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h
> b/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h
> index b90089a4fb6a..195c4ae67058 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h
> @@ -165,7 +165,7 @@
>  #define FEATURE_DS_FCLK_MASK(1 << FEATURE_DS_FCLK_BIT
> )
>  #define FEATURE_DS_MP1CLK_MASK  (1 <<
> FEATURE_DS_MP1CLK_BIT  )
>  #define FEATURE_DS_MP0CLK_MASK  (1 <<
> FEATURE_DS_MP0CLK_BIT  )
> -#define FEATURE_XGMI_MASK   (1 << FEATURE_XGMI_BIT   
> )
> +#define FEATURE_XGMI_MASK   (1ULL <<
> FEATURE_XGMI_BIT   )
>  #define FEATURE_ECC_MASK(1ULL << FEATURE_ECC_BIT 
>)
>
>  #define DPM_OVERRIDE_DISABLE_SOCCLK_PID 0x0001
> --
> 2.21.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] drm/amd/powerplay: fix possible hang with 3+ 4K monitors

[Deucher, Alexander]

Acked-by: Alex Deucher 

From: amd-gfx  on behalf of Evan Quan 

Sent: Tuesday, March 26, 2019 6:13 AM
To: amd-gfx@lists.freedesktop.org
Cc: Quan, Evan
Subject: [PATCH] drm/amd/powerplay: fix possible hang with 3+ 4K monitors

If DAL requires to force MCLK high, the FCLK will be
forced to high also.

Change-Id: Iaff8956ca1faafaf904f0bec108f566e8bbf6a64
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index 3f349ada8de0..38dbec3caa01 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -3471,6 +3471,7 @@ static int vega20_apply_clocks_adjust_rules(struct 
pp_hwmgr *hwmgr)
 struct vega20_single_dpm_table *dpm_table;
 bool vblank_too_short = false;
 bool disable_mclk_switching;
+   bool disable_fclk_switching;
 uint32_t i, latency;

 disable_mclk_switching = ((1 < hwmgr->display_config->num_display) &&
@@ -3546,13 +3547,20 @@ static int vega20_apply_clocks_adjust_rules(struct 
pp_hwmgr *hwmgr)
 if (hwmgr->display_config->nb_pstate_switch_disable)
 dpm_table->dpm_state.hard_min_level = 
dpm_table->dpm_levels[dpm_table->count - 1].value;

+   if ((disable_mclk_switching &&
+   (dpm_table->dpm_state.hard_min_level == 
dpm_table->dpm_levels[dpm_table->count - 1].value)) ||
+hwmgr->display_config->min_mem_set_clock / 100 >= 
dpm_table->dpm_levels[dpm_table->count - 1].value)
+   disable_fclk_switching = true;
+   else
+   disable_fclk_switching = false;
+
 /* fclk */
 dpm_table = &(data->dpm_table.fclk_table);
 dpm_table->dpm_state.soft_min_level = dpm_table->dpm_levels[0].value;
 dpm_table->dpm_state.soft_max_level = VG20_CLOCK_MAX_DEFAULT;
 dpm_table->dpm_state.hard_min_level = dpm_table->dpm_levels[0].value;
 dpm_table->dpm_state.hard_max_level = VG20_CLOCK_MAX_DEFAULT;
-   if (hwmgr->display_config->nb_pstate_switch_disable)
+   if (hwmgr->display_config->nb_pstate_switch_disable || 
disable_fclk_switching)
 dpm_table->dpm_state.soft_min_level = 
dpm_table->dpm_levels[dpm_table->count - 1].value;

 /* vclk */
--
2.21.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: Limit gpu max clock for ryzen 2400g

[Russell, Kent]

Hi Lauri,

There’s a more efficient method using the Power Profiles (and optionally, the 
ROCM-SMI tool, found at https://github.com/RadeonOpenCompute/ROC-smi), or the 
pp_sclk mask, depending on what exactly you want. I’ll list out the methods 
here and the rocm-smi and non-SMI commands to do it. I’ll assume that this GPU 
is card0 (it may be card1, card2, etc, depending on what GPUs are installed on 
your system; “rocm-smi -i” or “cat /sys/class/drm/card?/device/device will give 
you the GPU IDs of all of the cards, then you can figure out which one you want 
to use)


  1.  Mask the SCLKs . pp_dpm_sclk allows you to set a mask of what levels to 
use.
 *   First, read the values (“rocm-smi --showclkfrq” , or “cat 
/sys/class/drm/card0/device/pp_dpm_sclk”) and see the supported DPM levels for 
your card.
 *   Mask off the levels that you don’t want. E.g. If you only want to use 
levels 0-6 (and thus skip level 7), you can do either ‘rocm-smi --setsclk 0 1 2 
3 4 5 6’ or ‘echo manual > 
/sys/class/drm/card0/device/power_dpm_force_performance_level && echo “0 1 2 3 
4 5 6” > /sys/class/drm/card0/device/pp_dpm_sclk’ . This will set DPM to only 
use levels 0-6 and skip level 7. You can do this for any combination of levels 
or a single level (“0 2 5”, “1 2 7”, “5”, etc). That will tell it to only use 
the specified DPM levels and will persist until reboot, or until the 
power_dpm_force_performance is set back to ‘auto’ .
  2.  Set the specific DPM level values manually:
 *   First, you’ll need to enable the Power Profile Overdrive 
functionality. The easiest way is to add “amdgpu.ppfeaturemask=0x” to 
your linux command line parameters (by editing /boot/grub/grub.cfg manually, 
editing /etc/default/grub and doing an update-grub, or manually entering the 
kernel parameter in the GRUB menu before booting).
 *   Once that’s enabled, you should see the following file: 
/sys/class/drm/card0/device/pp_od_clk_voltage.
 *   Check the current DPM level information with “rocm-smi -S” or “cat 
/sys/class/drm/card0/device/pp_od_clk_voltage file” . Now that we have that, 
you can see the supported SCLK and voltages for each level.
 *   You can use the rocm-smi tool to manually change the levels through 
“rocm-smi --setslevel # MHZ VLT”, where:

   i.   # is 
the level (level 7 is probably the one you want, but you can do it for all of 
them)

 ii.  MHZ is 
the speed in MHz

   iii.  VLT is the 
voltage in mV.

 *   Honestly, you can probably just copy the highest level that you’re 
comfortable with and set that for all of the levels that exceed the values that 
you desire. So if you want to keep it to whatever level 6 is, just set level 7 
to have the same values as level 6 (that way you don’t have to muck with 
voltages and such). Or if 5 is the highest that you want, set level 6 and level 
7 to match level 5

Hopefully that helps. It also means that you don’t have to constantly try to 
build your own kernel with a change to cap the SCLK cherry-picked on tpo. 
Please let me know if you have any questions at all!

Kent

From: amd-gfx  On Behalf Of Lauri 
Ehrenpreis
Sent: Friday, March 22, 2019 6:18 AM
To: amd-gfx list 
Subject: Limit gpu max clock for ryzen 2400g

Hi!

Is there a way how to limit gpu max clock rate? Currently I can either leave 
the clock to automatic mode or force it to specific level via 
/sys/class/drm/card0/device/pp_dpm_sclk. But ideally I would like the clock to 
be automatically regulated but specify a different upper limit for power saving 
reasons.

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

RE: [PATCH 2/2] drm/amd/powerplay: correct data type to avoid overflow

[Quan, Evan]

Ping..

> -Original Message-
> From: Evan Quan 
> Sent: 2019年3月23日 2:07
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan 
> Subject: [PATCH 2/2] drm/amd/powerplay: correct data type to avoid
> overflow
> 
> Avoid left shift overflow.
> 
> Change-Id: If03f4f4d440b6d742d8eaa23d0bae6ddd21c01ea
> Signed-off-by: Evan Quan 
> ---
>  drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h
> b/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h
> index b90089a4fb6a..195c4ae67058 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h
> @@ -165,7 +165,7 @@
>  #define FEATURE_DS_FCLK_MASK(1 << FEATURE_DS_FCLK_BIT
> )
>  #define FEATURE_DS_MP1CLK_MASK  (1 <<
> FEATURE_DS_MP1CLK_BIT  )
>  #define FEATURE_DS_MP0CLK_MASK  (1 <<
> FEATURE_DS_MP0CLK_BIT  )
> -#define FEATURE_XGMI_MASK   (1 << FEATURE_XGMI_BIT   
> )
> +#define FEATURE_XGMI_MASK   (1ULL <<
> FEATURE_XGMI_BIT   )
>  #define FEATURE_ECC_MASK(1ULL << FEATURE_ECC_BIT 
>)
> 
>  #define DPM_OVERRIDE_DISABLE_SOCCLK_PID 0x0001
> --
> 2.21.0

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

RE: [PATCH 1/2] drm/amd/powerplay: add ECC feature bit

[Quan, Evan]

Ping..

> -Original Message-
> From: Evan Quan 
> Sent: 2019年3月23日 2:06
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan 
> Subject: [PATCH 1/2] drm/amd/powerplay: add ECC feature bit
> 
> It's OK to have this feature bit with old SMU firmwares.
> But the feature should be disabled on them.
> 
> Change-Id: I6fb4869ef454ea7b6d01cf368b457be01eeb5058
> Signed-off-by: Evan Quan 
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c  | 10
> +-  drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h  |
> 1 +  drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h |  3 ++-
>  3 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> index fac7a5df7c27..49be888fcd50 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> @@ -91,6 +91,12 @@ static void vega20_set_default_registry_data(struct
> pp_hwmgr *hwmgr)
>*   MP0CLK DS
>*/
>   data->registry_data.disallowed_features = 0xE0041C00;
> + /* ECC feature should be disabled on old SMUs */
> + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetSmuVersion);
> + hwmgr->smu_version = smum_get_argument(hwmgr);
> + if (hwmgr->smu_version < 0x282100)
> + data->registry_data.disallowed_features |=
> FEATURE_ECC_MASK;
> +
>   data->registry_data.od_state_in_dc_support = 0;
>   data->registry_data.thermal_support = 1;
>   data->registry_data.skip_baco_hardware = 0; @@ -357,6 +363,7 @@
> static void vega20_init_dpm_defaults(struct pp_hwmgr *hwmgr)
>   data->smu_features[GNLD_DS_MP1CLK].smu_feature_id =
> FEATURE_DS_MP1CLK_BIT;
>   data->smu_features[GNLD_DS_MP0CLK].smu_feature_id =
> FEATURE_DS_MP0CLK_BIT;
>   data->smu_features[GNLD_XGMI].smu_feature_id =
> FEATURE_XGMI_BIT;
> + data->smu_features[GNLD_ECC].smu_feature_id =
> FEATURE_ECC_BIT;
> 
>   for (i = 0; i < GNLD_FEATURES_MAX; i++) {
>   data->smu_features[i].smu_feature_bitmap = @@ -3048,7
> +3055,8 @@ static int vega20_get_ppfeature_status(struct pp_hwmgr
> *hwmgr, char *buf)
>   "FCLK_DS",
>   "MP1CLK_DS",
>   "MP0CLK_DS",
> - "XGMI"};
> + "XGMI",
> + "ECC"};
>   static const char *output_title[] = {
>   "FEATURES",
>   "BITMASK",
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
> index a46cdeb7da70..c3890b5e076c 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
> @@ -81,6 +81,7 @@ enum {
>   GNLD_DS_MP1CLK,
>   GNLD_DS_MP0CLK,
>   GNLD_XGMI,
> + GNLD_ECC,
> 
>   GNLD_FEATURES_MAX
>  };
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h
> b/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h
> index 63d5cf691549..b90089a4fb6a 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if.h
> @@ -99,7 +99,7 @@
>  #define FEATURE_DS_MP1CLK_BIT   30
>  #define FEATURE_DS_MP0CLK_BIT   31
>  #define FEATURE_XGMI_BIT32
> -#define FEATURE_SPARE_33_BIT33
> +#define FEATURE_ECC_BIT 33
>  #define FEATURE_SPARE_34_BIT34
>  #define FEATURE_SPARE_35_BIT35
>  #define FEATURE_SPARE_36_BIT36
> @@ -166,6 +166,7 @@
>  #define FEATURE_DS_MP1CLK_MASK  (1 <<
> FEATURE_DS_MP1CLK_BIT  )
>  #define FEATURE_DS_MP0CLK_MASK  (1 <<
> FEATURE_DS_MP0CLK_BIT  )
>  #define FEATURE_XGMI_MASK   (1 << FEATURE_XGMI_BIT   
> )
> +#define FEATURE_ECC_MASK(1ULL << FEATURE_ECC_BIT 
>)
> 
>  #define DPM_OVERRIDE_DISABLE_SOCCLK_PID 0x0001
>  #define DPM_OVERRIDE_DISABLE_UCLK_PID   0x0002
> --
> 2.21.0

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

[PATCH] drm/amd/powerplay: fix possible hang with 3+ 4K monitors

[Evan Quan]

If DAL requires to force MCLK high, the FCLK will be
forced to high also.

Change-Id: Iaff8956ca1faafaf904f0bec108f566e8bbf6a64
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index 3f349ada8de0..38dbec3caa01 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -3471,6 +3471,7 @@ static int vega20_apply_clocks_adjust_rules(struct 
pp_hwmgr *hwmgr)
struct vega20_single_dpm_table *dpm_table;
bool vblank_too_short = false;
bool disable_mclk_switching;
+   bool disable_fclk_switching;
uint32_t i, latency;
 
disable_mclk_switching = ((1 < hwmgr->display_config->num_display) &&
@@ -3546,13 +3547,20 @@ static int vega20_apply_clocks_adjust_rules(struct 
pp_hwmgr *hwmgr)
if (hwmgr->display_config->nb_pstate_switch_disable)
dpm_table->dpm_state.hard_min_level = 
dpm_table->dpm_levels[dpm_table->count - 1].value;
 
+   if ((disable_mclk_switching &&
+   (dpm_table->dpm_state.hard_min_level == 
dpm_table->dpm_levels[dpm_table->count - 1].value)) ||
+hwmgr->display_config->min_mem_set_clock / 100 >= 
dpm_table->dpm_levels[dpm_table->count - 1].value)
+   disable_fclk_switching = true;
+   else
+   disable_fclk_switching = false;
+
/* fclk */
dpm_table = &(data->dpm_table.fclk_table);
dpm_table->dpm_state.soft_min_level = dpm_table->dpm_levels[0].value;
dpm_table->dpm_state.soft_max_level = VG20_CLOCK_MAX_DEFAULT;
dpm_table->dpm_state.hard_min_level = dpm_table->dpm_levels[0].value;
dpm_table->dpm_state.hard_max_level = VG20_CLOCK_MAX_DEFAULT;
-   if (hwmgr->display_config->nb_pstate_switch_disable)
+   if (hwmgr->display_config->nb_pstate_switch_disable || 
disable_fclk_switching)
dpm_table->dpm_state.soft_min_level = 
dpm_table->dpm_levels[dpm_table->count - 1].value;
 
/* vclk */
-- 
2.21.0

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

Re: [PATCH v2] gpu: radeon: fix a potential NULL-pointer dereference

[Michel Dänzer]

On 2019-03-25 10:05 p.m., Kangjie Lu wrote:
> In case alloc_workqueue fails, the fix frees memory and
> returns -ENOMEM to avoid potential NULL pointer dereference.
> 
> Signed-off-by: Kangjie Lu 
> ---
> v2: use radeon_crtc_destroy to properly clean up resources as
> suggested by Michel Dänzer 
> 
>  [...]
> 
> @@ -671,13 +671,18 @@ static void radeon_crtc_init(struct drm_device *dev, 
> int index)
>  
>   radeon_crtc = kzalloc(sizeof(struct radeon_crtc) + (RADEONFB_CONN_LIMIT 
> * sizeof(struct drm_connector *)), GFP_KERNEL);
>   if (radeon_crtc == NULL)
> - return;
> + return -ENOMEM;
>  
>   drm_crtc_init(dev, _crtc->base, _crtc_funcs);
>  
>   drm_mode_crtc_set_gamma_size(_crtc->base, 256);
>   radeon_crtc->crtc_id = index;
>   radeon_crtc->flip_queue = alloc_workqueue("radeon-crtc", WQ_HIGHPRI, 0);
> + if (!radeon_crtc->flip_queue) {
> + DRM_ERROR("failed to allocate the flip queue\n");
> + radeon_crtc_destroy(_crtc->base);
> + return -ENOMEM;
> + }

radeon_crtc_destroy currently unconditionally calls:

destroy_workqueue(radeon_crtc->flip_queue);

AFAICT destroy_workqueue will blow up if NULL is passed to it, so
radeon_crtc_destroy needs to check for that.


-- 
Earthling Michel Dänzer   |  https://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 1/2] drm/amdgpu: move VM table mapping into the backend as well

[Liu, Monk]

I cannot apply your patch ... report error on line 85, what branch are you 
based on ? I assume it should be amd-staging-drm-next 

/Monk

-Original Message-
From: Christian König  
Sent: Monday, March 25, 2019 8:23 PM
To: Liu, Monk ; amd-...@freedesktop.org
Subject: [PATCH 1/2] drm/amdgpu: move VM table mapping into the backend as well

Clean that up further and also fix another case where the BO wasn't kmapped for 
CPU based updates.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 31 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c  | 11   
drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 20 +
 4 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index af1a7020c3ab..c9c8309a4d3f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -660,17 +660,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
if (bo->tbo.type != ttm_bo_type_kernel) {
amdgpu_vm_bo_moved(bo_base);
} else {
-   if (vm->use_cpu_for_update)
-   r = amdgpu_bo_kmap(bo, NULL);
-   else
-   r = amdgpu_ttm_alloc_gart(>tbo);
-   if (r)
-   break;
-   if (bo->shadow) {
-   r = amdgpu_ttm_alloc_gart(>shadow->tbo);
-   if (r)
-   break;
-   }
+   vm->update_funcs->map_table(bo);
amdgpu_vm_bo_relocated(bo_base);
}
}
@@ -752,22 +742,17 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
if (r)
return r;
 
-   r = amdgpu_ttm_alloc_gart(>tbo);
-   if (r)
-   return r;
-
if (bo->shadow) {
r = ttm_bo_validate(>shadow->tbo, >shadow->placement,
);
if (r)
return r;
-
-   r = amdgpu_ttm_alloc_gart(>shadow->tbo);
-   if (r)
-   return r;
-
}
 
+   r = vm->update_funcs->map_table(bo);
+   if (r)
+   return r;
+
memset(, 0, sizeof(params));
params.adev = adev;
params.vm = vm;
@@ -878,12 +863,6 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
if (r)
return r;
 
-   if (vm->use_cpu_for_update) {
-   r = amdgpu_bo_kmap(pt, NULL);
-   if (r)
-   goto error_free_pt;
-   }
-
/* Keep a reference to the root directory to avoid
 * freeing them up in the wrong order.
 */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 520122be798b..3ec875c0cc76 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -215,7 +215,7 @@ struct amdgpu_vm_update_params {  };
 
 struct amdgpu_vm_update_funcs {
-
+   int (*map_table)(struct amdgpu_bo *bo);
int (*prepare)(struct amdgpu_vm_update_params *p, void * owner,
   struct dma_fence *exclusive);
int (*update)(struct amdgpu_vm_update_params *p, diff --git 
a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
index 9d53982021de..5222d165abfc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
@@ -24,6 +24,16 @@
 #include "amdgpu_object.h"
 #include "amdgpu_trace.h"
 
+/**
+ * amdgpu_vm_cpu_map_table - make sure new PDs/PTs are kmapped
+ *
+ * @table: newly allocated or validated PD/PT  */ static int 
+amdgpu_vm_cpu_map_table(struct amdgpu_bo *table) {
+   return amdgpu_bo_kmap(table, NULL);
+}
+
 /**
  * amdgpu_vm_cpu_prepare - prepare page table update with the CPU
  *
@@ -110,6 +120,7 @@ static int amdgpu_vm_cpu_commit(struct 
amdgpu_vm_update_params *p,  }
 
 const struct amdgpu_vm_update_funcs amdgpu_vm_cpu_funcs = {
+   .map_table = amdgpu_vm_cpu_map_table,
.prepare = amdgpu_vm_cpu_prepare,
.update = amdgpu_vm_cpu_update,
.commit = amdgpu_vm_cpu_commit
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
index e4bacdb44c68..4bccd69fe30d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
@@ -28,6 +28,25 @@
 #define AMDGPU_VM_SDMA_MIN_NUM_DW  256u
 #define AMDGPU_VM_SDMA_MAX_NUM_DW  (16u * 1024u)
 
+/**
+ * amdgpu_vm_sdma_map_table - make sure new PDs/PTs are GTT mapped
+ *
+ * @table: newly allocated or validated PD/PT  */ 

RE: [PATCH 1/3] drm/amdgpu: Stop returning EINVAL during profile switch

[Russell, Kent]

Indeed, it allowed me to apply the profile even with no previous profile set 
(so it basically just applied all zeros). I’ll rebase my 2 patches on your 
patch once that is pushed to drm-next.

Kent

From: Quan, Evan
Sent: Tuesday, March 26, 2019 5:10 AM
To: Russell, Kent ; amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH 1/3] drm/amdgpu: Stop returning EINVAL during profile switch

That seems a bug. I will fix that.
It’s OK to store previous CUSTOM profile settings in driver and switch back to 
that with no need to pass in settings again. That makes sense.
But you may need to update the patch(to check whether there is previous custom 
profile settings).

Regards,
Evan
From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 On Behalf Of Russell, Kent
Sent: 2019年3月26日 0:00
To: Quan, Evan mailto:evan.q...@amd.com>>; 
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/amdgpu: Stop returning EINVAL during profile switch

I set CUSTOM to a weird value like "1 2 3 4 5 6" , then switched to VR profile, 
then switched back to CUSTOM (just with "echo 6 > profile), and it kept my 
custom values. Before this patch, it throws EINVAL but still switches it to 
CUSTOM with the saved values, on both vega10 and vega20.

Kent

KENT RUSSELL
Sr. Software Engineer | Linux Compute Kernel
1 Commerce Valley Drive East
Markham, ON L3T 7X6
O +(1) 289-695-2122 | Ext 72122

From: Quan, Evan
Sent: Monday, March 25, 2019 10:14:03 AM
To: Russell, Kent; 
amd-gfx@lists.freedesktop.org
Cc: Russell, Kent
Subject: RE: [PATCH 1/3] drm/amdgpu: Stop returning EINVAL during profile switch

How can you tell whether it was already applied before or 1st time to switch to 
compute mode?
For the latter case, other settings/parameters do need.

Regards,
Evan
> -Original Message-
> From: amd-gfx 
> mailto:amd-gfx-boun...@lists.freedesktop.org>>
>  On Behalf Of
> Russell, Kent
> Sent: 2019年3月25日 20:42
> To: amd-gfx@lists.freedesktop.org
> Cc: Russell, Kent mailto:kent.russ...@amd.com>>
> Subject: [PATCH 1/3] drm/amdgpu: Stop returning EINVAL during profile
> switch
>
> Don't return an error after switching to the CUSTOM profile, if there aren't
> any CUSTOM values passed in. The CUSTOM profile is already applied, so if
> no other arguments are passed, just return instead of throwing -EINVAL
> when successful
>
> Change-Id: I114cc9783226ee9ebb146863897e951527a85e20
> Signed-off-by: Kent Russell 
> mailto:kent.russ...@amd.com>>
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 6 +-
> drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 5 -
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> index ed6c638700f5..a98b927282ac 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> @@ -4911,7 +4911,11 @@ static int vega10_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>1< >power_profile_mode);
>
>if (hwmgr->power_profile_mode ==
> PP_SMC_POWER_PROFILE_CUSTOM) {
> - if (size == 0 || size > 4)
> + /* If size = 0, then just return, it was applied above */
> + if (size == 0)
> + return 0;
> +
> + if (size != 4)
>return -EINVAL;
>
>data->custom_profile_mode[0] = busy_set_point = input[0];
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> index 664544e7fcdc..225b2102c82c 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> @@ -3828,8 +3828,11 @@ static int vega20_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>}
>
>if (hwmgr->power_profile_mode ==
> PP_SMC_POWER_PROFILE_CUSTOM) {
> - if (size < 10)
> + if (size < 10 && size != 0)
>return -EINVAL;
> + /* If size = 0, then just return, it was applied above */
> + if (size == 0)
> + return 0;
>
>result = vega20_get_activity_monitor_coeff(hwmgr,
>(uint8_t *)(_monitor),
> --
> 2.17.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amd/powerplay: update current profile mode only when it's really applied

[Evan Quan]

No need to update current profile mode if the new profile mode
does not take effect in fact.

Change-Id: If24474d201840bfaefacc189f6a6ff6856695dff
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  9 +
 drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 13 +++--
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index ed6c638700f5..85a536924571 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -4904,13 +4904,12 @@ static int vega10_set_power_profile_mode(struct 
pp_hwmgr *hwmgr, long *input, ui
uint8_t FPS;
uint8_t use_rlc_busy;
uint8_t min_active_level;
-
-   hwmgr->power_profile_mode = input[size];
+   uint32_t power_profile_mode = input[size];
 
smum_send_msg_to_smc_with_parameter(hwmgr, PPSMC_MSG_SetWorkloadMask,
-   1power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) {
+   if (power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) {
if (size == 0 || size > 4)
return -EINVAL;
 
@@ -4924,6 +4923,8 @@ static int vega10_set_power_profile_mode(struct pp_hwmgr 
*hwmgr, long *input, ui
use_rlc_busy << 16 | 
min_active_level<<24);
}
 
+   hwmgr->power_profile_mode = power_profile_mode;
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index 664544e7fcdc..3f349ada8de0 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -3819,15 +3819,14 @@ static int vega20_set_power_profile_mode(struct 
pp_hwmgr *hwmgr, long *input, ui
 {
DpmActivityMonitorCoeffInt_t activity_monitor;
int workload_type, result = 0;
+   uint32_t power_profile_mode = input[size];
 
-   hwmgr->power_profile_mode = input[size];
-
-   if (hwmgr->power_profile_mode > PP_SMC_POWER_PROFILE_CUSTOM) {
-   pr_err("Invalid power profile mode %d\n", 
hwmgr->power_profile_mode);
+   if (power_profile_mode > PP_SMC_POWER_PROFILE_CUSTOM) {
+   pr_err("Invalid power profile mode %d\n", power_profile_mode);
return -EINVAL;
}
 
-   if (hwmgr->power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) {
+   if (power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) {
if (size < 10)
return -EINVAL;
 
@@ -3895,10 +3894,12 @@ static int vega20_set_power_profile_mode(struct 
pp_hwmgr *hwmgr, long *input, ui
 
/* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */
workload_type =
-   conv_power_profile_to_pplib_workload(hwmgr->power_profile_mode);
+   conv_power_profile_to_pplib_workload(power_profile_mode);
smum_send_msg_to_smc_with_parameter(hwmgr, PPSMC_MSG_SetWorkloadMask,
1 << workload_type);
 
+   hwmgr->power_profile_mode = power_profile_mode;
+
return 0;
 }
 
-- 
2.21.0

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

RE: [PATCH 1/3] drm/amdgpu: Stop returning EINVAL during profile switch

[Quan, Evan]

That seems a bug. I will fix that.
It’s OK to store previous CUSTOM profile settings in driver and switch back to 
that with no need to pass in settings again. That makes sense.
But you may need to update the patch(to check whether there is previous custom 
profile settings).

Regards,
Evan
From: amd-gfx  On Behalf Of Russell, Kent
Sent: 2019年3月26日 0:00
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/amdgpu: Stop returning EINVAL during profile switch

I set CUSTOM to a weird value like "1 2 3 4 5 6" , then switched to VR profile, 
then switched back to CUSTOM (just with "echo 6 > profile), and it kept my 
custom values. Before this patch, it throws EINVAL but still switches it to 
CUSTOM with the saved values, on both vega10 and vega20.

Kent

KENT RUSSELL
Sr. Software Engineer | Linux Compute Kernel
1 Commerce Valley Drive East
Markham, ON L3T 7X6
O +(1) 289-695-2122 | Ext 72122

From: Quan, Evan
Sent: Monday, March 25, 2019 10:14:03 AM
To: Russell, Kent; 
amd-gfx@lists.freedesktop.org
Cc: Russell, Kent
Subject: RE: [PATCH 1/3] drm/amdgpu: Stop returning EINVAL during profile switch

How can you tell whether it was already applied before or 1st time to switch to 
compute mode?
For the latter case, other settings/parameters do need.

Regards,
Evan
> -Original Message-
> From: amd-gfx 
> mailto:amd-gfx-boun...@lists.freedesktop.org>>
>  On Behalf Of
> Russell, Kent
> Sent: 2019年3月25日 20:42
> To: amd-gfx@lists.freedesktop.org
> Cc: Russell, Kent mailto:kent.russ...@amd.com>>
> Subject: [PATCH 1/3] drm/amdgpu: Stop returning EINVAL during profile
> switch
>
> Don't return an error after switching to the CUSTOM profile, if there aren't
> any CUSTOM values passed in. The CUSTOM profile is already applied, so if
> no other arguments are passed, just return instead of throwing -EINVAL
> when successful
>
> Change-Id: I114cc9783226ee9ebb146863897e951527a85e20
> Signed-off-by: Kent Russell 
> mailto:kent.russ...@amd.com>>
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 6 +-
> drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 5 -
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> index ed6c638700f5..a98b927282ac 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> @@ -4911,7 +4911,11 @@ static int vega10_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>1< >power_profile_mode);
>
>if (hwmgr->power_profile_mode ==
> PP_SMC_POWER_PROFILE_CUSTOM) {
> - if (size == 0 || size > 4)
> + /* If size = 0, then just return, it was applied above */
> + if (size == 0)
> + return 0;
> +
> + if (size != 4)
>return -EINVAL;
>
>data->custom_profile_mode[0] = busy_set_point = input[0];
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> index 664544e7fcdc..225b2102c82c 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> @@ -3828,8 +3828,11 @@ static int vega20_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>}
>
>if (hwmgr->power_profile_mode ==
> PP_SMC_POWER_PROFILE_CUSTOM) {
> - if (size < 10)
> + if (size < 10 && size != 0)
>return -EINVAL;
> + /* If size = 0, then just return, it was applied above */
> + if (size == 0)
> + return 0;
>
>result = vega20_get_activity_monitor_coeff(hwmgr,
>(uint8_t *)(_monitor),
> --
> 2.17.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx