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

[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 <shaoyun.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9ee8d7a..82dc2b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -31,6 +31,7 @@
 #include <drm/amdgpu_drm.h>
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_xgmi.h"
 
 void amdgpu_gem_object_free(struct drm_gem_object *gobj)
 {
@@ -666,6 +667,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 +707,16 @@ 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 &&
+			    amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev))) {
+				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 3d221f0..eb242a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
 	INIT_LIST_HEAD(&bo_va->valids);
 	INIT_LIST_HEAD(&bo_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(&adev->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-28 1:55 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 <shaoyun.liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
>   2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 9ee8d7a..82dc2b6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -31,6 +31,7 @@
>   #include <drm/amdgpu_drm.h>
>   #include "amdgpu.h"
>   #include "amdgpu_display.h"
> +#include "amdgpu_xgmi.h"
>   
>   void amdgpu_gem_object_free(struct drm_gem_object *gobj)
>   {
> @@ -666,6 +667,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 +707,16 @@ 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 &&
> +			    amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev))) {

adev and robj->tbo.bdev are the same in each loop iteration. Shouldn't 
we get one of the devices from the bo_va or amdgpu_vm_bo_base?

Regards,
   Felix


> +				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 3d221f0..eb242a1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>   	INIT_LIST_HEAD(&bo_va->valids);
>   	INIT_LIST_HEAD(&bo_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(&adev->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: Add preferred_domain check when determine XGMI state

[Liu, Shaoyun]

I think we only care about the context device (adev) and the real device 
this bo been allocated(robj->tbo.bdev).  The bo_va or base  don't have  
the device pointer directly, it have  a pointer to bo which should be 
the  same  as robj here.  We  can move the same_hive  check out of the 
loop .

Regards

shaoyun.liu


On 2019-03-28 3:18 p.m., Kuehling, Felix wrote:
> On 2019-03-28 1:55 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 <shaoyun.liu@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +++++++++++++
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
>>    2 files changed, 15 insertions(+), 1 deletion(-)
>> I think we only care about the context device (adev)  and  the real device this bo been allocated ,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index 9ee8d7a..82dc2b6 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -31,6 +31,7 @@
>>    #include <drm/amdgpu_drm.h>
>>    #include "amdgpu.h"
>>    #include "amdgpu_display.h"
>> +#include "amdgpu_xgmi.h"
>>    
>>    void amdgpu_gem_object_free(struct drm_gem_object *gobj)
>>    {
>> @@ -666,6 +667,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 +707,16 @@ 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 &&
>> +			    amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev))) {
> adev and robj->tbo.bdev are the same in each loop iteration. Shouldn't
> we get one of the devices from the bo_va or amdgpu_vm_bo_base?
>
> Regards,
>     Felix
>
>
>> +				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 3d221f0..eb242a1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>>    	INIT_LIST_HEAD(&bo_va->valids);
>>    	INIT_LIST_HEAD(&bo_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(&adev->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
_______________________________________________
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-28 3:47 p.m., Liu, Shaoyun wrote:
> I think we only care about the context device (adev) and the real device
> this bo been allocated(robj->tbo.bdev).  The bo_va or base  don't have
> the device pointer directly, it have  a pointer to bo which should be
> the  same  as robj here.  We  can move the same_hive  check out of the
> loop .

That doesn't make sense. The "same hive" check only makes sense if one 
of the devices is the one where the memory is physically located 
(robj->tbo.bdev), and the other one is where the memory is accessed 
from. That only makes sense inside the loop. The amdgpu_vm_bo_base 
should tell you the device that's mapping and potentially accessing the 
memory over XGMI. You could get it like this:

     mapping_adev = base->vm->root.base.bo->tbo.bdev;

Regards,
   Felix


>
> Regards
>
> shaoyun.liu
>
>
> On 2019-03-28 3:18 p.m., Kuehling, Felix wrote:
>> On 2019-03-28 1:55 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 <shaoyun.liu@amd.com>
>>> ---
>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +++++++++++++
>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
>>>     2 files changed, 15 insertions(+), 1 deletion(-)
>>> I think we only care about the context device (adev)  and  the real device this bo been allocated ,
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> index 9ee8d7a..82dc2b6 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> @@ -31,6 +31,7 @@
>>>     #include <drm/amdgpu_drm.h>
>>>     #include "amdgpu.h"
>>>     #include "amdgpu_display.h"
>>> +#include "amdgpu_xgmi.h"
>>>     
>>>     void amdgpu_gem_object_free(struct drm_gem_object *gobj)
>>>     {
>>> @@ -666,6 +667,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 +707,16 @@ 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 &&
>>> +			    amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev))) {
>> adev and robj->tbo.bdev are the same in each loop iteration. Shouldn't
>> we get one of the devices from the bo_va or amdgpu_vm_bo_base?
>>
>> Regards,
>>      Felix
>>
>>
>>> +				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 3d221f0..eb242a1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>>>     	INIT_LIST_HEAD(&bo_va->valids);
>>>     	INIT_LIST_HEAD(&bo_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(&adev->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
_______________________________________________
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 28.03.19 um 18:55 schrieb Liu, Shaoyun:
> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for peer device
>
> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
> Signed-off-by: shaoyunl <shaoyun.liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
>   2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 9ee8d7a..82dc2b6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -31,6 +31,7 @@
>   #include <drm/amdgpu_drm.h>
>   #include "amdgpu.h"
>   #include "amdgpu_display.h"
> +#include "amdgpu_xgmi.h"
>   
>   void amdgpu_gem_object_free(struct drm_gem_object *gobj)
>   {
> @@ -666,6 +667,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 +707,16 @@ 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 can never be NULL here, apart from that looks good to me.

Christian.

> +			    amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev))) {
> +				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 3d221f0..eb242a1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>   	INIT_LIST_HEAD(&bo_va->valids);
>   	INIT_LIST_HEAD(&bo_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(&adev->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: Add preferred_domain check when determine XGMI state

[Christian König]

Am 28.03.19 um 21:39 schrieb Kuehling, Felix:
> On 2019-03-28 4:38 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 <shaoyun.liu@amd.com>
> This patch is Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
>
> Please also give Christian a chance to review this one before you submit.

Looks good to me as well, feel free to also add my rb.

Christian.

>
> Thanks,
>     Felix
>
>
>> ---
>>    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 9ee8d7a..7b84036 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -31,6 +31,7 @@
>>    #include <drm/amdgpu_drm.h>
>>    #include "amdgpu.h"
>>    #include "amdgpu_display.h"
>> +#include "amdgpu_xgmi.h"
>>    
>>    void amdgpu_gem_object_free(struct drm_gem_object *gobj)
>>    {
>> @@ -666,6 +667,7 @@ 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 *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)
>> +			if (amdgpu_xgmi_same_hive(amdgpu_ttm_adev(robj->tbo.bdev),
>> +				amdgpu_ttm_adev(base->vm->root.base.bo->tbo.bdev))) {
>> +				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 3d221f0..eb242a1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>>    	INIT_LIST_HEAD(&bo_va->valids);
>>    	INIT_LIST_HEAD(&bo_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(&adev->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

_______________________________________________
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-28 4:38 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 <shaoyun.liu@amd.com>

This patch is Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>

Please also give Christian a chance to review this one before you submit.

Thanks,
   Felix


> ---
>   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 9ee8d7a..7b84036 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -31,6 +31,7 @@
>   #include <drm/amdgpu_drm.h>
>   #include "amdgpu.h"
>   #include "amdgpu_display.h"
> +#include "amdgpu_xgmi.h"
>   
>   void amdgpu_gem_object_free(struct drm_gem_object *gobj)
>   {
> @@ -666,6 +667,7 @@ 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 *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)
> +			if (amdgpu_xgmi_same_hive(amdgpu_ttm_adev(robj->tbo.bdev),
> +				amdgpu_ttm_adev(base->vm->root.base.bo->tbo.bdev))) {
> +				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 3d221f0..eb242a1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>   	INIT_LIST_HEAD(&bo_va->valids);
>   	INIT_LIST_HEAD(&bo_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(&adev->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

[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 <shaoyun.liu@amd.com>
---
 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 9ee8d7a..7b84036 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -31,6 +31,7 @@
 #include <drm/amdgpu_drm.h>
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_xgmi.h"
 
 void amdgpu_gem_object_free(struct drm_gem_object *gobj)
 {
@@ -666,6 +667,7 @@ 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 *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)
+			if (amdgpu_xgmi_same_hive(amdgpu_ttm_adev(robj->tbo.bdev),
+				amdgpu_ttm_adev(base->vm->root.base.bo->tbo.bdev))) {
+				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 3d221f0..eb242a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
 	INIT_LIST_HEAD(&bo_va->valids);
 	INIT_LIST_HEAD(&bo_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(&adev->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

[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 <shaoyun.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 14 ++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9ee8d7a..6a076d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -31,6 +31,7 @@
 #include <drm/amdgpu_drm.h>
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_xgmi.h"
 
 void amdgpu_gem_object_free(struct drm_gem_object *gobj)
 {
@@ -666,6 +667,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 +707,17 @@ 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 &&
+			    amdgpu_xgmi_same_hive(amdgpu_ttm_adev(robj->tbo.bdev),
+					amdgpu_ttm_adev(bo_va->base.vm->root.base.bo->tbo.bdev))) {
+				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 3d221f0..eb242a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
 	INIT_LIST_HEAD(&bo_va->valids);
 	INIT_LIST_HEAD(&bo_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(&adev->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

[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 <shaoyun.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9ee8d7a..7716ada 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -31,6 +31,7 @@
 #include <drm/amdgpu_drm.h>
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_xgmi.h"
 
 void amdgpu_gem_object_free(struct drm_gem_object *gobj)
 {
@@ -666,6 +667,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 +707,16 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
 			amdgpu_bo_unreserve(robj);
 			break;
 		}
+		if (amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev)))
+			for (base = robj->vm_bo; base; base = base->next) {
+				bo_va = container_of(base, struct amdgpu_bo_va, base);
+				if (bo_va) {
+					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 3d221f0..eb242a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
 	INIT_LIST_HEAD(&bo_va->valids);
 	INIT_LIST_HEAD(&bo_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(&adev->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

[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 <shaoyun.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9ee8d7a..7716ada 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -31,6 +31,7 @@
 #include <drm/amdgpu_drm.h>
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_xgmi.h"
 
 void amdgpu_gem_object_free(struct drm_gem_object *gobj)
 {
@@ -666,6 +667,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 +707,16 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
 			amdgpu_bo_unreserve(robj);
 			break;
 		}
+		if (amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev)))
+			for (base = robj->vm_bo; base; base = base->next) {
+				bo_va = container_of(base, struct amdgpu_bo_va, base);
+				if (bo_va) {
+					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 3d221f0..eb242a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
 	INIT_LIST_HEAD(&bo_va->valids);
 	INIT_LIST_HEAD(&bo_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(&adev->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

[Liu, Shaoyun]

Even the preferred_domains been changed for bo that original is none xgmi here , it doesn't means driver must change the XGMI status for the bo_va. From current design, only the bo later been added to the  compute VM (through bo_add) will be enable the XGMI mapping. Without that call , the XGMI mapping will be disabled.
Remove the XGMI checking may affect other sharing mechanism.

Shaoyun.liu
  

On 2019-03-27 11:52 a.m., Kuehling, Felix wrote:
> On 2019-03-26 4:35 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 <shaoyun.liu@amd.com>
>> ---
>>    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) {
> The point here is to prevent transitions where a mapping moves from
> is_xgmi => !is_xgmi or from !is_xgmi => is_xgmi, because that would
> potentially require a XGMI pstate change.
>
> This check catches the case of is_xgmi => !is_xgmi, but not the other
> way around. I think you should drop the is_xgmi condition here. Any BO
> that's shared between multiple GPUs should return -EINVAL here because
> moving it could result in an XGMI pstate change. It doesn't matter
> whether it's currently using XGMI or not.
>
> Regards,
>     Felix
>
>> +				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(&bo_va->valids);
>>    	INIT_LIST_HEAD(&bo_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(&adev->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: Add preferred_domain check when determine XGMI state

[Kuehling, Felix]

On 2019-03-26 4:35 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 <shaoyun.liu@amd.com>
> ---
>   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) {

The point here is to prevent transitions where a mapping moves from 
is_xgmi => !is_xgmi or from !is_xgmi => is_xgmi, because that would 
potentially require a XGMI pstate change.

This check catches the case of is_xgmi => !is_xgmi, but not the other 
way around. I think you should drop the is_xgmi condition here. Any BO 
that's shared between multiple GPUs should return -EINVAL here because 
moving it could result in an XGMI pstate change. It doesn't matter 
whether it's currently using XGMI or not.

Regards,
   Felix

> +				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(&bo_va->valids);
>   	INIT_LIST_HEAD(&bo_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(&adev->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: Add preferred_domain check when determine XGMI state

[Koenig, Christian]

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

Yeah, feel free to add my RB.

Christian.

Am 27.03.2019 16:43 schrieb "Liu, Shaoyun" <Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>:

bool  is_xgmi is added in bo_va structure , this change won't increase the size of   struct amdgpu_vm_bo_base. Anyway , can I said it's reviewed by you now?

shaoyun.liu

On 2019-03-27 11:39 a.m., Koenig, Christian wrote:
Yeah, that one is intentionally very small because it is used so often.

When it grows over the next power of two by this change we need to figure out a different approach.

Christian.

Am 27.03.2019 16:34 schrieb "Liu, Shaoyun" <Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org><mailto:Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>:

you mean struct amdgpu_vm_bo_base ? It only  has few pointer and  a list_head , should be  pretty small .

struct amdgpu_vm_bo_base {
    /* constant after initialization */
    struct amdgpu_vm        *vm;
    struct amdgpu_bo        *bo;

    /* protected by bo being reserved */
    struct amdgpu_vm_bo_base    *next;

    /* protected by spinlock */
    struct list_head        vm_status;

    /* protected by the BO being reserved */
    bool                moved;
};

shaoyun.liu

On 2019-03-27 11:30 a.m., Koenig, Christian wrote:
Ah, of course. In this case feel free to go ahead.
 BTW: What's the size of the structure now?

Christian.

Am 27.03.2019 16:22 schrieb "Liu, Shaoyun" <Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org><mailto:Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>:
The  next in struct amdgpu_vm_bo_base  is not a  list_head,  I'm not
sure whether we can use list_for_each_entry here.

Regards

shaoyun.liu

On 2019-03-27 3:41 a.m., Christian König wrote:
> Am 26.03.19 um 21:35 schrieb Liu, Shaoyun:
>> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram
>> memory for peer device
>>
>> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
>> Signed-off-by: shaoyunl <shaoyun.liu-5C7GfCeVMHo@public.gmane.org><mailto:shaoyun.liu-5C7GfCeVMHo@public.gmane.org>
>> ---
>>   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);
>
> Please use list_for_each_entry here instead of open coding it.
>
> Christian.
>
>> +            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(&bo_va->valids);
>>       INIT_LIST_HEAD(&bo_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(&adev->vm_manager.lock_pstate);
>>           /* Power up XGMI if it can be potentially used */
>



_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

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

_______________________________________________
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

[Liu, Shaoyun]

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

bool  is_xgmi is added in bo_va structure , this change won't increase the size of   struct amdgpu_vm_bo_base. Anyway , can I said it's reviewed by you now?

shaoyun.liu

On 2019-03-27 11:39 a.m., Koenig, Christian wrote:
Yeah, that one is intentionally very small because it is used so often.

When it grows over the next power of two by this change we need to figure out a different approach.

Christian.

Am 27.03.2019 16:34 schrieb "Liu, Shaoyun" <Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org><mailto:Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>:

you mean struct amdgpu_vm_bo_base ? It only  has few pointer and  a list_head , should be  pretty small .

struct amdgpu_vm_bo_base {
    /* constant after initialization */
    struct amdgpu_vm        *vm;
    struct amdgpu_bo        *bo;

    /* protected by bo being reserved */
    struct amdgpu_vm_bo_base    *next;

    /* protected by spinlock */
    struct list_head        vm_status;

    /* protected by the BO being reserved */
    bool                moved;
};

shaoyun.liu

On 2019-03-27 11:30 a.m., Koenig, Christian wrote:
Ah, of course. In this case feel free to go ahead.
 BTW: What's the size of the structure now?

Christian.

Am 27.03.2019 16:22 schrieb "Liu, Shaoyun" <Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org><mailto:Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>:
The  next in struct amdgpu_vm_bo_base  is not a  list_head,  I'm not
sure whether we can use list_for_each_entry here.

Regards

shaoyun.liu

On 2019-03-27 3:41 a.m., Christian König wrote:
> Am 26.03.19 um 21:35 schrieb Liu, Shaoyun:
>> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram
>> memory for peer device
>>
>> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
>> Signed-off-by: shaoyunl <shaoyun.liu-5C7GfCeVMHo@public.gmane.org><mailto:shaoyun.liu-5C7GfCeVMHo@public.gmane.org>
>> ---
>>   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);
>
> Please use list_for_each_entry here instead of open coding it.
>
> Christian.
>
>> +            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(&bo_va->valids);
>>       INIT_LIST_HEAD(&bo_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(&adev->vm_manager.lock_pstate);
>>           /* Power up XGMI if it can be potentially used */
>



_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

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

_______________________________________________
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

[Koenig, Christian]

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

Yeah, that one is intentionally very small because it is used so often.

When it grows over the next power of two by this change we need to figure out a different approach.

Christian.

Am 27.03.2019 16:34 schrieb "Liu, Shaoyun" <Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>:

you mean struct amdgpu_vm_bo_base ? It only  has few pointer and  a list_head , should be  pretty small .

struct amdgpu_vm_bo_base {
    /* constant after initialization */
    struct amdgpu_vm        *vm;
    struct amdgpu_bo        *bo;

    /* protected by bo being reserved */
    struct amdgpu_vm_bo_base    *next;

    /* protected by spinlock */
    struct list_head        vm_status;

    /* protected by the BO being reserved */
    bool                moved;
};

shaoyun.liu

On 2019-03-27 11:30 a.m., Koenig, Christian wrote:
Ah, of course. In this case feel free to go ahead.
 BTW: What's the size of the structure now?

Christian.

Am 27.03.2019 16:22 schrieb "Liu, Shaoyun" <Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org><mailto:Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>:
The  next in struct amdgpu_vm_bo_base  is not a  list_head,  I'm not
sure whether we can use list_for_each_entry here.

Regards

shaoyun.liu

On 2019-03-27 3:41 a.m., Christian König wrote:
> Am 26.03.19 um 21:35 schrieb Liu, Shaoyun:
>> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram
>> memory for peer device
>>
>> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
>> Signed-off-by: shaoyunl <shaoyun.liu-5C7GfCeVMHo@public.gmane.org><mailto:shaoyun.liu-5C7GfCeVMHo@public.gmane.org>
>> ---
>>   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);
>
> Please use list_for_each_entry here instead of open coding it.
>
> Christian.
>
>> +            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(&bo_va->valids);
>>       INIT_LIST_HEAD(&bo_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(&adev->vm_manager.lock_pstate);
>>           /* Power up XGMI if it can be potentially used */
>

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

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

_______________________________________________
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

[Liu, Shaoyun]

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

you mean struct amdgpu_vm_bo_base ? It only  has few pointer and  a list_head , should be  pretty small .

struct amdgpu_vm_bo_base {
    /* constant after initialization */
    struct amdgpu_vm        *vm;
    struct amdgpu_bo        *bo;

    /* protected by bo being reserved */
    struct amdgpu_vm_bo_base    *next;

    /* protected by spinlock */
    struct list_head        vm_status;

    /* protected by the BO being reserved */
    bool                moved;
};

shaoyun.liu

On 2019-03-27 11:30 a.m., Koenig, Christian wrote:
Ah, of course. In this case feel free to go ahead.
 BTW: What's the size of the structure now?

Christian.

Am 27.03.2019 16:22 schrieb "Liu, Shaoyun" <Shaoyun.Liu@amd.com><mailto:Shaoyun.Liu@amd.com>:
The  next in struct amdgpu_vm_bo_base  is not a  list_head,  I'm not
sure whether we can use list_for_each_entry here.

Regards

shaoyun.liu

On 2019-03-27 3:41 a.m., Christian König wrote:
> Am 26.03.19 um 21:35 schrieb Liu, Shaoyun:
>> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram
>> memory for peer device
>>
>> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
>> Signed-off-by: shaoyunl <shaoyun.liu@amd.com><mailto:shaoyun.liu@amd.com>
>> ---
>>   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);
>
> Please use list_for_each_entry here instead of open coding it.
>
> Christian.
>
>> +            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(&bo_va->valids);
>>       INIT_LIST_HEAD(&bo_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(&adev->vm_manager.lock_pstate);
>>           /* Power up XGMI if it can be potentially used */
>

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

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

_______________________________________________
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

[Koenig, Christian]

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

Ah, of course. In this case feel free to go ahead.
 BTW: What's the size of the structure now?

Christian.

Am 27.03.2019 16:22 schrieb "Liu, Shaoyun" <Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>:
The  next in struct amdgpu_vm_bo_base  is not a  list_head,  I'm not
sure whether we can use list_for_each_entry here.

Regards

shaoyun.liu

On 2019-03-27 3:41 a.m., Christian König wrote:
> Am 26.03.19 um 21:35 schrieb Liu, Shaoyun:
>> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram
>> memory for peer device
>>
>> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
>> Signed-off-by: shaoyunl <shaoyun.liu-5C7GfCeVMHo@public.gmane.org>
>> ---
>>   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);
>
> Please use list_for_each_entry here instead of open coding it.
>
> Christian.
>
>> +            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(&bo_va->valids);
>>       INIT_LIST_HEAD(&bo_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(&adev->vm_manager.lock_pstate);
>>           /* Power up XGMI if it can be potentially used */
>

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

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

_______________________________________________
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

[Liu, Shaoyun]

The  next in struct amdgpu_vm_bo_base  is not a  list_head,  I'm not 
sure whether we can use list_for_each_entry here.

Regards

shaoyun.liu

On 2019-03-27 3:41 a.m., Christian König wrote:
> Am 26.03.19 um 21:35 schrieb Liu, Shaoyun:
>> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram 
>> memory for peer device
>>
>> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
>> Signed-off-by: shaoyunl <shaoyun.liu@amd.com>
>> ---
>>   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);
>
> Please use list_for_each_entry here instead of open coding it.
>
> Christian.
>
>> +            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(&bo_va->valids);
>>       INIT_LIST_HEAD(&bo_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(&adev->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: Add preferred_domain check when determine XGMI state

[Christian König]

Am 26.03.19 um 21:35 schrieb Liu, Shaoyun:
> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for peer device
>
> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
> Signed-off-by: shaoyunl <shaoyun.liu@amd.com>
> ---
>   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);

Please use list_for_each_entry here instead of open coding it.

Christian.

> +			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(&bo_va->valids);
>   	INIT_LIST_HEAD(&bo_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(&adev->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

[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 <shaoyun.liu@amd.com>
---
 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(&bo_va->valids);
 	INIT_LIST_HEAD(&bo_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(&adev->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 <shaoyun.liu@amd.com>
> ---
>   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(&fpriv->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(&bo_va->valids);
>   	INIT_LIST_HEAD(&bo_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(&adev->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: 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 <shaoyun.liu@amd.com>
> ---
>   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(&fpriv->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(&bo_va->valids);
>   	INIT_LIST_HEAD(&bo_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.koenig@amd.com>.

Christian.

>   		bo_va->is_xgmi = true;
>   		mutex_lock(&adev->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

[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 <shaoyun.liu@amd.com>
---
 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(&fpriv->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(&bo_va->valids);
 	INIT_LIST_HEAD(&bo_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(&adev->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