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

["Kuehling, Felix" <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>]

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