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

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

[Liu, Shaoyun]

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 <shaoyun.liu@amd.com>
---
 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(&bo_va->valids);
 	INIT_LIST_HEAD(&bo_va->invalids);
 
+	if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
+		bo_va->is_xgmi = true;
+		mutex_lock(&adev->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(&adev->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(&adev->vm_manager.lock_pstate);
+		if (--adev->vm_manager.xgmi_map_counter == 0)
+			amdgpu_xgmi_set_pstate(adev, 0);
+		mutex_unlock(&adev->vm_manager.lock_pstate);
+	}
+
 	kfree(bo_va);
 }
 
@@ -2997,6 +3015,9 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
 
 	idr_init(&adev->vm_manager.pasid_idr);
 	spin_lock_init(&adev->vm_manager.pasid_lock);
+
+	adev->vm_manager.xgmi_map_counter = 0;
+	mutex_init(&adev->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;
+
+	/* counter of mapped memory through xgmi */
+	uint32_t				xgmi_map_counter;
+	struct mutex				lock_pstate;
 };
 
 #define amdgpu_vm_copy_pte(adev, ib, pe, src, count) ((adev)->vm_manager.vm_pte_funcs->copy_pte((ib), (pe), (src), (count)))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index fcc4b05..3368347 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -200,12 +200,26 @@ struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
 
 	if (lock)
 		mutex_lock(&tmp->hive_lock);
-
+	tmp->pstate = -1;
 	mutex_unlock(&xgmi_mutex);
 
 	return tmp;
 }
 
+int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate)
+{
+	int ret = 0;
+	struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
+
+	if (!hive)
+		return 0;
+
+	if (hive->pstate == pstate)
+		return 0;
+	/* Todo : sent the message to SMU for pstate change */
+	return ret;
+}
+
 int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev)
 {
 	int ret = -EINVAL;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
index 24a3b03..3e9c91e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
@@ -33,11 +33,21 @@ struct amdgpu_hive_info {
 	struct kobject *kobj;
 	struct device_attribute dev_attr;
 	struct amdgpu_device *adev;
+	int pstate; /*0 -- low , 1 -- high , -1 unknown*/
 };
 
 struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lock);
 int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev);
 int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
 void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
+int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
+
+static inline bool amdgpu_xgmi_same_hive(struct amdgpu_device *adev,
+		struct amdgpu_device *bo_adev)
+{
+	return (adev != bo_adev &&
+		adev->gmc.xgmi.hive_id &&
+		adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id);
+}
 
 #endif
-- 
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 22.03.19 um 20:28 schrieb Liu, Shaoyun:
> 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 <shaoyun.liu@amd.com>

> +	int pstate; /*0 -- low , 1 -- high , -1 unknown*/
Maybe Alex or somebody else who is working on the power management stuff 
knows a better enum for this.

But at least from the VM side the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.
_______________________________________________
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]

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 <shaoyun.liu@amd.com>
> ---
>   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(&bo_va->valids);
>   	INIT_LIST_HEAD(&bo_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(&adev->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(&adev->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(&adev->vm_manager.lock_pstate);
> +		if (--adev->vm_manager.xgmi_map_counter == 0)
> +			amdgpu_xgmi_set_pstate(adev, 0);
> +		mutex_unlock(&adev->vm_manager.lock_pstate);
> +	}
> +
>   	kfree(bo_va);
>   }
>   
> @@ -2997,6 +3015,9 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
>   
>   	idr_init(&adev->vm_manager.pasid_idr);
>   	spin_lock_init(&adev->vm_manager.pasid_lock);
> +
> +	adev->vm_manager.xgmi_map_counter = 0;
> +	mutex_init(&adev->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;
> +
> +	/* counter of mapped memory through xgmi */
> +	uint32_t				xgmi_map_counter;
> +	struct mutex				lock_pstate;
>   };
>   
>   #define amdgpu_vm_copy_pte(adev, ib, pe, src, count) ((adev)->vm_manager.vm_pte_funcs->copy_pte((ib), (pe), (src), (count)))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index fcc4b05..3368347 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -200,12 +200,26 @@ struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
>   
>   	if (lock)
>   		mutex_lock(&tmp->hive_lock);
> -
> +	tmp->pstate = -1;
>   	mutex_unlock(&xgmi_mutex);
>   
>   	return tmp;
>   }
>   
> +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate)
> +{
> +	int ret = 0;
> +	struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
> +
> +	if (!hive)
> +		return 0;
> +
> +	if (hive->pstate == pstate)
> +		return 0;
> +	/* Todo : sent the message to SMU for pstate change */
> +	return ret;
> +}
> +
>   int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev)
>   {
>   	int ret = -EINVAL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> index 24a3b03..3e9c91e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> @@ -33,11 +33,21 @@ struct amdgpu_hive_info {
>   	struct kobject *kobj;
>   	struct device_attribute dev_attr;
>   	struct amdgpu_device *adev;
> +	int pstate; /*0 -- low , 1 -- high , -1 unknown*/
>   };
>   
>   struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lock);
>   int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev);
>   int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
>   void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
> +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
> +
> +static inline bool amdgpu_xgmi_same_hive(struct amdgpu_device *adev,
> +		struct amdgpu_device *bo_adev)
> +{
> +	return (adev != bo_adev &&
> +		adev->gmc.xgmi.hive_id &&
> +		adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id);
> +}
>   
>   #endif
_______________________________________________
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]

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

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 <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
Sent: March 25, 2019 6:28:32 PM
To: Liu, Shaoyun; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.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 <shaoyun.liu-5C7GfCeVMHo@public.gmane.org>
> ---
>   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(&bo_va->valids);
>        INIT_LIST_HEAD(&bo_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(&adev->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(&adev->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(&adev->vm_manager.lock_pstate);
> +             if (--adev->vm_manager.xgmi_map_counter == 0)
> +                     amdgpu_xgmi_set_pstate(adev, 0);
> +             mutex_unlock(&adev->vm_manager.lock_pstate);
> +     }
> +
>        kfree(bo_va);
>   }
>
> @@ -2997,6 +3015,9 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
>
>        idr_init(&adev->vm_manager.pasid_idr);
>        spin_lock_init(&adev->vm_manager.pasid_lock);
> +
> +     adev->vm_manager.xgmi_map_counter = 0;
> +     mutex_init(&adev->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;
> +
> +     /* counter of mapped memory through xgmi */
> +     uint32_t                                xgmi_map_counter;
> +     struct mutex                            lock_pstate;
>   };
>
>   #define amdgpu_vm_copy_pte(adev, ib, pe, src, count) ((adev)->vm_manager.vm_pte_funcs->copy_pte((ib), (pe), (src), (count)))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index fcc4b05..3368347 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -200,12 +200,26 @@ struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
>
>        if (lock)
>                mutex_lock(&tmp->hive_lock);
> -
> +     tmp->pstate = -1;
>        mutex_unlock(&xgmi_mutex);
>
>        return tmp;
>   }
>
> +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate)
> +{
> +     int ret = 0;
> +     struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
> +
> +     if (!hive)
> +             return 0;
> +
> +     if (hive->pstate == pstate)
> +             return 0;
> +     /* Todo : sent the message to SMU for pstate change */
> +     return ret;
> +}
> +
>   int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev)
>   {
>        int ret = -EINVAL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> index 24a3b03..3e9c91e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> @@ -33,11 +33,21 @@ struct amdgpu_hive_info {
>        struct kobject *kobj;
>        struct device_attribute dev_attr;
>        struct amdgpu_device *adev;
> +     int pstate; /*0 -- low , 1 -- high , -1 unknown*/
>   };
>
>   struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lock);
>   int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev);
>   int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
>   void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
> +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
> +
> +static inline bool amdgpu_xgmi_same_hive(struct amdgpu_device *adev,
> +             struct amdgpu_device *bo_adev)
> +{
> +     return (adev != bo_adev &&
> +             adev->gmc.xgmi.hive_id &&
> +             adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id);
> +}
>
>   #endif
_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 14412 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: 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 <amd-gfx-bounces@lists.freedesktop.org> on behalf of 
> Kuehling, Felix <Felix.Kuehling@amd.com>
> *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 <shaoyun.liu@amd.com>
> > ---
> >   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(&bo_va->valids);
> >        INIT_LIST_HEAD(&bo_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(&adev->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(&adev->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(&adev->vm_manager.lock_pstate);
> > +             if (--adev->vm_manager.xgmi_map_counter == 0)
> > +                     amdgpu_xgmi_set_pstate(adev, 0);
> > + mutex_unlock(&adev->vm_manager.lock_pstate);
> > +     }
> > +
> >        kfree(bo_va);
> >   }
> >
> > @@ -2997,6 +3015,9 @@ void amdgpu_vm_manager_init(struct 
> amdgpu_device *adev)
> >
> >        idr_init(&adev->vm_manager.pasid_idr);
> > spin_lock_init(&adev->vm_manager.pasid_lock);
> > +
> > +     adev->vm_manager.xgmi_map_counter = 0;
> > + mutex_init(&adev->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;
> > +
> > +     /* counter of mapped memory through xgmi */
> > +     uint32_t xgmi_map_counter;
> > +     struct mutex lock_pstate;
> >   };
> >
> >   #define amdgpu_vm_copy_pte(adev, ib, pe, src, count) 
> ((adev)->vm_manager.vm_pte_funcs->copy_pte((ib), (pe), (src), (count)))
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> > index fcc4b05..3368347 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> > @@ -200,12 +200,26 @@ struct amdgpu_hive_info 
> *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
> >
> >        if (lock)
> >                mutex_lock(&tmp->hive_lock);
> > -
> > +     tmp->pstate = -1;
> >        mutex_unlock(&xgmi_mutex);
> >
> >        return tmp;
> >   }
> >
> > +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate)
> > +{
> > +     int ret = 0;
> > +     struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
> > +
> > +     if (!hive)
> > +             return 0;
> > +
> > +     if (hive->pstate == pstate)
> > +             return 0;
> > +     /* Todo : sent the message to SMU for pstate change */
> > +     return ret;
> > +}
> > +
> >   int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, 
> struct amdgpu_device *adev)
> >   {
> >        int ret = -EINVAL;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> > index 24a3b03..3e9c91e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> > @@ -33,11 +33,21 @@ struct amdgpu_hive_info {
> >        struct kobject *kobj;
> >        struct device_attribute dev_attr;
> >        struct amdgpu_device *adev;
> > +     int pstate; /*0 -- low , 1 -- high , -1 unknown*/
> >   };
> >
> >   struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device 
> *adev, int lock);
> >   int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, 
> struct amdgpu_device *adev);
> >   int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
> >   void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
> > +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
> > +
> > +static inline bool amdgpu_xgmi_same_hive(struct amdgpu_device *adev,
> > +             struct amdgpu_device *bo_adev)
> > +{
> > +     return (adev != bo_adev &&
> > +             adev->gmc.xgmi.hive_id &&
> > +             adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id);
> > +}
> >
> >   #endif
> _______________________________________________
> 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: 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 <amd-gfx-bounces@lists.freedesktop.org> on behalf of
>> Kuehling, Felix <Felix.Kuehling@amd.com>
>> *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 <shaoyun.liu@amd.com>
>>> ---
>>>     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(&bo_va->valids);
>>>          INIT_LIST_HEAD(&bo_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(&adev->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(&adev->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(&adev->vm_manager.lock_pstate);
>>> +             if (--adev->vm_manager.xgmi_map_counter == 0)
>>> +                     amdgpu_xgmi_set_pstate(adev, 0);
>>> + mutex_unlock(&adev->vm_manager.lock_pstate);
>>> +     }
>>> +
>>>          kfree(bo_va);
>>>     }
>>>
>>> @@ -2997,6 +3015,9 @@ void amdgpu_vm_manager_init(struct
>> amdgpu_device *adev)
>>>          idr_init(&adev->vm_manager.pasid_idr);
>>> spin_lock_init(&adev->vm_manager.pasid_lock);
>>> +
>>> +     adev->vm_manager.xgmi_map_counter = 0;
>>> + mutex_init(&adev->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;
>>> +
>>> +     /* counter of mapped memory through xgmi */
>>> +     uint32_t xgmi_map_counter;
>>> +     struct mutex lock_pstate;
>>>     };
>>>
>>>     #define amdgpu_vm_copy_pte(adev, ib, pe, src, count)
>> ((adev)->vm_manager.vm_pte_funcs->copy_pte((ib), (pe), (src), (count)))
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>>> index fcc4b05..3368347 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>>> @@ -200,12 +200,26 @@ struct amdgpu_hive_info
>> *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
>>>          if (lock)
>>>                  mutex_lock(&tmp->hive_lock);
>>> -
>>> +     tmp->pstate = -1;
>>>          mutex_unlock(&xgmi_mutex);
>>>
>>>          return tmp;
>>>     }
>>>
>>> +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate)
>>> +{
>>> +     int ret = 0;
>>> +     struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
>>> +
>>> +     if (!hive)
>>> +             return 0;
>>> +
>>> +     if (hive->pstate == pstate)
>>> +             return 0;
>>> +     /* Todo : sent the message to SMU for pstate change */
>>> +     return ret;
>>> +}
>>> +
>>>     int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive,
>> struct amdgpu_device *adev)
>>>     {
>>>          int ret = -EINVAL;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
>>> index 24a3b03..3e9c91e 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
>>> @@ -33,11 +33,21 @@ struct amdgpu_hive_info {
>>>          struct kobject *kobj;
>>>          struct device_attribute dev_attr;
>>>          struct amdgpu_device *adev;
>>> +     int pstate; /*0 -- low , 1 -- high , -1 unknown*/
>>>     };
>>>
>>>     struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device
>> *adev, int lock);
>>>     int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive,
>> struct amdgpu_device *adev);
>>>     int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
>>>     void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
>>> +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
>>> +
>>> +static inline bool amdgpu_xgmi_same_hive(struct amdgpu_device *adev,
>>> +             struct amdgpu_device *bo_adev)
>>> +{
>>> +     return (adev != bo_adev &&
>>> +             adev->gmc.xgmi.hive_id &&
>>> +             adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id);
>>> +}
>>>
>>>     #endif
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Christian König]

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

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 <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of 
> Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
> *Sent:* March 25, 2019 6:28:32 PM
> *To:* Liu, Shaoyun; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.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 <shaoyun.liu-5C7GfCeVMHo@public.gmane.org>
> > ---
> >   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(&bo_va->valids);
> >        INIT_LIST_HEAD(&bo_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(&adev->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(&adev->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(&adev->vm_manager.lock_pstate);
> > +             if (--adev->vm_manager.xgmi_map_counter == 0)
> > +                     amdgpu_xgmi_set_pstate(adev, 0);
> > + mutex_unlock(&adev->vm_manager.lock_pstate);
> > +     }
> > +
> >        kfree(bo_va);
> >   }
> >
> > @@ -2997,6 +3015,9 @@ void amdgpu_vm_manager_init(struct 
> amdgpu_device *adev)
> >
> >        idr_init(&adev->vm_manager.pasid_idr);
> > spin_lock_init(&adev->vm_manager.pasid_lock);
> > +
> > +     adev->vm_manager.xgmi_map_counter = 0;
> > + mutex_init(&adev->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;
> > +
> > +     /* counter of mapped memory through xgmi */
> > +     uint32_t xgmi_map_counter;
> > +     struct mutex lock_pstate;
> >   };
> >
> >   #define amdgpu_vm_copy_pte(adev, ib, pe, src, count) 
> ((adev)->vm_manager.vm_pte_funcs->copy_pte((ib), (pe), (src), (count)))
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> > index fcc4b05..3368347 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> > @@ -200,12 +200,26 @@ struct amdgpu_hive_info 
> *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
> >
> >        if (lock)
> >                mutex_lock(&tmp->hive_lock);
> > -
> > +     tmp->pstate = -1;
> >        mutex_unlock(&xgmi_mutex);
> >
> >        return tmp;
> >   }
> >
> > +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate)
> > +{
> > +     int ret = 0;
> > +     struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
> > +
> > +     if (!hive)
> > +             return 0;
> > +
> > +     if (hive->pstate == pstate)
> > +             return 0;
> > +     /* Todo : sent the message to SMU for pstate change */
> > +     return ret;
> > +}
> > +
> >   int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, 
> struct amdgpu_device *adev)
> >   {
> >        int ret = -EINVAL;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> > index 24a3b03..3e9c91e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> > @@ -33,11 +33,21 @@ struct amdgpu_hive_info {
> >        struct kobject *kobj;
> >        struct device_attribute dev_attr;
> >        struct amdgpu_device *adev;
> > +     int pstate; /*0 -- low , 1 -- high , -1 unknown*/
> >   };
> >
> >   struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device 
> *adev, int lock);
> >   int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, 
> struct amdgpu_device *adev);
> >   int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
> >   void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
> > +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
> > +
> > +static inline bool amdgpu_xgmi_same_hive(struct amdgpu_device *adev,
> > +             struct amdgpu_device *bo_adev)
> > +{
> > +     return (adev != bo_adev &&
> > +             adev->gmc.xgmi.hive_id &&
> > +             adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id);
> > +}
> >
> >   #endif
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 15157 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: 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 <amd-gfx-bounces@lists.freedesktop.org> on behalf of
>>> Kuehling, Felix <Felix.Kuehling@amd.com>
>>> *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 <shaoyun.liu@amd.com>
>>>> ---
>>>>      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(&bo_va->valids);
>>>>           INIT_LIST_HEAD(&bo_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(&adev->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(&adev->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(&adev->vm_manager.lock_pstate);
>>>> +             if (--adev->vm_manager.xgmi_map_counter == 0)
>>>> +                     amdgpu_xgmi_set_pstate(adev, 0);
>>>> + mutex_unlock(&adev->vm_manager.lock_pstate);
>>>> +     }
>>>> +
>>>>           kfree(bo_va);
>>>>      }
>>>>
>>>> @@ -2997,6 +3015,9 @@ void amdgpu_vm_manager_init(struct
>>> amdgpu_device *adev)
>>>>           idr_init(&adev->vm_manager.pasid_idr);
>>>> spin_lock_init(&adev->vm_manager.pasid_lock);
>>>> +
>>>> +     adev->vm_manager.xgmi_map_counter = 0;
>>>> + mutex_init(&adev->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;
>>>> +
>>>> +     /* counter of mapped memory through xgmi */
>>>> +     uint32_t xgmi_map_counter;
>>>> +     struct mutex lock_pstate;
>>>>      };
>>>>
>>>>      #define amdgpu_vm_copy_pte(adev, ib, pe, src, count)
>>> ((adev)->vm_manager.vm_pte_funcs->copy_pte((ib), (pe), (src), (count)))
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>>>> index fcc4b05..3368347 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>>>> @@ -200,12 +200,26 @@ struct amdgpu_hive_info
>>> *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
>>>>           if (lock)
>>>>                   mutex_lock(&tmp->hive_lock);
>>>> -
>>>> +     tmp->pstate = -1;
>>>>           mutex_unlock(&xgmi_mutex);
>>>>
>>>>           return tmp;
>>>>      }
>>>>
>>>> +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate)
>>>> +{
>>>> +     int ret = 0;
>>>> +     struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
>>>> +
>>>> +     if (!hive)
>>>> +             return 0;
>>>> +
>>>> +     if (hive->pstate == pstate)
>>>> +             return 0;
>>>> +     /* Todo : sent the message to SMU for pstate change */
>>>> +     return ret;
>>>> +}
>>>> +
>>>>      int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive,
>>> struct amdgpu_device *adev)
>>>>      {
>>>>           int ret = -EINVAL;
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
>>>> index 24a3b03..3e9c91e 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
>>>> @@ -33,11 +33,21 @@ struct amdgpu_hive_info {
>>>>           struct kobject *kobj;
>>>>           struct device_attribute dev_attr;
>>>>           struct amdgpu_device *adev;
>>>> +     int pstate; /*0 -- low , 1 -- high , -1 unknown*/
>>>>      };
>>>>
>>>>      struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device
>>> *adev, int lock);
>>>>      int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive,
>>> struct amdgpu_device *adev);
>>>>      int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
>>>>      void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
>>>> +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
>>>> +
>>>> +static inline bool amdgpu_xgmi_same_hive(struct amdgpu_device *adev,
>>>> +             struct amdgpu_device *bo_adev)
>>>> +{
>>>> +     return (adev != bo_adev &&
>>>> +             adev->gmc.xgmi.hive_id &&
>>>> +             adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id);
>>>> +}
>>>>
>>>>      #endif
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
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 20.03.19 um 21:21 schrieb Liu, Shaoyun:
> 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.

Only a few style nit picks, but apart from that reviewed-by: Christian 
König <christian.koenig@amd.com>

>
> Change-Id: I197501f853c47f844055c0e28c0ac00a1ff06607
> Signed-off-by: shaoyunl <shaoyun.liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  5 +++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  5 +++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 26 ++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   | 16 +++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  2 ++
>   6 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 1db1921..c9c24d6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -932,6 +932,11 @@ struct amdgpu_device {
>   	struct work_struct		xgmi_reset_work;
>   
>   	bool                            in_baco_reset;
> +
> +	/* counter of mapped memory through xgmi */
> +	atomic_t			xgmi_map_counter;

This doesn't need to be an atomic any more when you protect it with a 
mutex anyway.

> +	struct mutex  			lock_pstate;

Better put both fields that into the VM manager structure.

> +
>   };
>   
>   static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device *bdev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 2065837..a5af885 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)
> @@ -2467,6 +2471,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   	mutex_init(&adev->virt.vf_errors.lock);
>   	hash_init(adev->mn_hash);
>   	mutex_init(&adev->lock_reset);
> +	mutex_init(&adev->lock_pstate);
>   
>   	amdgpu_device_check_arguments(adev);
>   
> 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 c8f0e4c..3639cf6 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
> @@ -2352,6 +2353,14 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>   	return 0;
>   }
>   
> +static bool amdgpu_xgmi_same_hive(struct amdgpu_device *adev,
> +		struct amdgpu_device *bo_adev)
> +{
> +	return (adev != bo_adev &&
> +		adev->gmc.xgmi.hive_id &&
> +		adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id);
> +}

Maybe put that into amdgpu_xgmi.h, could be useful in other cases as well.

Regards,
Christian.

> +
>   /**
>    * amdgpu_vm_bo_add - add a bo to a specific vm
>    *
> @@ -2383,6 +2392,15 @@ 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 (amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
> +		bo_va->is_xgmi = true;
> +		mutex_lock(&adev->lock_pstate);
> +		/* Power up XGMI if it can be potentially used */
> +		if (atomic_inc_return(&adev->xgmi_map_counter) == 1)
> +			amdgpu_xgmi_set_pstate(adev, 1);
> +		mutex_unlock(&adev->lock_pstate);
> +	}
> +
>   	return bo_va;
>   }
>   
> @@ -2801,6 +2819,14 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>   	}
>   
>   	dma_fence_put(bo_va->last_pt_update);
> +
> +	if (bo_va->is_xgmi) {
> +		mutex_lock(&adev->lock_pstate);
> +		if (atomic_dec_return(&adev->xgmi_map_counter) == 0)
> +			amdgpu_xgmi_set_pstate(adev, 0);
> +		mutex_unlock(&adev->lock_pstate);
> +	}
> +
>   	kfree(bo_va);
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index fcc4b05..3368347 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -200,12 +200,26 @@ struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
>   
>   	if (lock)
>   		mutex_lock(&tmp->hive_lock);
> -
> +	tmp->pstate = -1;
>   	mutex_unlock(&xgmi_mutex);
>   
>   	return tmp;
>   }
>   
> +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate)
> +{
> +	int ret = 0;
> +	struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
> +
> +	if (!hive)
> +		return 0;
> +
> +	if (hive->pstate == pstate)
> +		return 0;
> +	/* Todo : sent the message to SMU for pstate change */
> +	return ret;
> +}
> +
>   int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev)
>   {
>   	int ret = -EINVAL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> index 24a3b03..7e1710fc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> @@ -33,11 +33,13 @@ struct amdgpu_hive_info {
>   	struct kobject *kobj;
>   	struct device_attribute dev_attr;
>   	struct amdgpu_device *adev;
> +	int pstate; /*0 -- low , 1 -- high , -1 unknown*/
>   };
>   
>   struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lock);
>   int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev);
>   int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
>   void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
> +int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
>   
>   #endif

_______________________________________________
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]

ping

-----Original Message-----
From: Liu, Shaoyun <Shaoyun.Liu@amd.com> 
Sent: Wednesday, March 20, 2019 4:22 PM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Shaoyun <Shaoyun.Liu@amd.com>
Subject: [PATCH] drm/amdgpu: XGMI pstate switch initial support

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 <shaoyun.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  5 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  5 +++++  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 26 ++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   | 16 +++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  2 ++
 6 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 1db1921..c9c24d6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -932,6 +932,11 @@ struct amdgpu_device {
 	struct work_struct		xgmi_reset_work;
 
 	bool                            in_baco_reset;
+
+	/* counter of mapped memory through xgmi */
+	atomic_t			xgmi_map_counter;
+	struct mutex  			lock_pstate;
+
 };
 
 static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device *bdev) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2065837..a5af885 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) @@ -2467,6 +2471,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	mutex_init(&adev->virt.vf_errors.lock);
 	hash_init(adev->mn_hash);
 	mutex_init(&adev->lock_reset);
+	mutex_init(&adev->lock_pstate);
 
 	amdgpu_device_check_arguments(adev);
 
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 c8f0e4c..3639cf6 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
@@ -2352,6 +2353,14 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
 	return 0;
 }
 
+static bool amdgpu_xgmi_same_hive(struct amdgpu_device *adev,
+		struct amdgpu_device *bo_adev)
+{
+	return (adev != bo_adev &&
+		adev->gmc.xgmi.hive_id &&
+		adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id); }
+
 /**
  * amdgpu_vm_bo_add - add a bo to a specific vm
  *
@@ -2383,6 +2392,15 @@ 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 (amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
+		bo_va->is_xgmi = true;
+		mutex_lock(&adev->lock_pstate);
+		/* Power up XGMI if it can be potentially used */
+		if (atomic_inc_return(&adev->xgmi_map_counter) == 1)
+			amdgpu_xgmi_set_pstate(adev, 1);
+		mutex_unlock(&adev->lock_pstate);
+	}
+
 	return bo_va;
 }
 
@@ -2801,6 +2819,14 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
 	}
 
 	dma_fence_put(bo_va->last_pt_update);
+
+	if (bo_va->is_xgmi) {
+		mutex_lock(&adev->lock_pstate);
+		if (atomic_dec_return(&adev->xgmi_map_counter) == 0)
+			amdgpu_xgmi_set_pstate(adev, 0);
+		mutex_unlock(&adev->lock_pstate);
+	}
+
 	kfree(bo_va);
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index fcc4b05..3368347 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -200,12 +200,26 @@ struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
 
 	if (lock)
 		mutex_lock(&tmp->hive_lock);
-
+	tmp->pstate = -1;
 	mutex_unlock(&xgmi_mutex);
 
 	return tmp;
 }
 
+int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate) {
+	int ret = 0;
+	struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
+
+	if (!hive)
+		return 0;
+
+	if (hive->pstate == pstate)
+		return 0;
+	/* Todo : sent the message to SMU for pstate change */
+	return ret;
+}
+
 int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev)  {
 	int ret = -EINVAL;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
index 24a3b03..7e1710fc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
@@ -33,11 +33,13 @@ struct amdgpu_hive_info {
 	struct kobject *kobj;
 	struct device_attribute dev_attr;
 	struct amdgpu_device *adev;
+	int pstate; /*0 -- low , 1 -- high , -1 unknown*/
 };
 
 struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lock);  int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev);  int amdgpu_xgmi_add_device(struct amdgpu_device *adev);  void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
+int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
 
 #endif
--
2.7.4

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

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

[Liu, Shaoyun]

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 <shaoyun.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  5 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  5 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 26 ++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   | 16 +++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  2 ++
 6 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 1db1921..c9c24d6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -932,6 +932,11 @@ struct amdgpu_device {
 	struct work_struct		xgmi_reset_work;
 
 	bool                            in_baco_reset;
+
+	/* counter of mapped memory through xgmi */
+	atomic_t			xgmi_map_counter;
+	struct mutex  			lock_pstate;
+
 };
 
 static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device *bdev)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2065837..a5af885 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)
@@ -2467,6 +2471,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	mutex_init(&adev->virt.vf_errors.lock);
 	hash_init(adev->mn_hash);
 	mutex_init(&adev->lock_reset);
+	mutex_init(&adev->lock_pstate);
 
 	amdgpu_device_check_arguments(adev);
 
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 c8f0e4c..3639cf6 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
@@ -2352,6 +2353,14 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
 	return 0;
 }
 
+static bool amdgpu_xgmi_same_hive(struct amdgpu_device *adev,
+		struct amdgpu_device *bo_adev)
+{
+	return (adev != bo_adev &&
+		adev->gmc.xgmi.hive_id &&
+		adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id);
+}
+
 /**
  * amdgpu_vm_bo_add - add a bo to a specific vm
  *
@@ -2383,6 +2392,15 @@ 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 (amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
+		bo_va->is_xgmi = true;
+		mutex_lock(&adev->lock_pstate);
+		/* Power up XGMI if it can be potentially used */
+		if (atomic_inc_return(&adev->xgmi_map_counter) == 1)
+			amdgpu_xgmi_set_pstate(adev, 1);
+		mutex_unlock(&adev->lock_pstate);
+	}
+
 	return bo_va;
 }
 
@@ -2801,6 +2819,14 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
 	}
 
 	dma_fence_put(bo_va->last_pt_update);
+
+	if (bo_va->is_xgmi) {
+		mutex_lock(&adev->lock_pstate);
+		if (atomic_dec_return(&adev->xgmi_map_counter) == 0)
+			amdgpu_xgmi_set_pstate(adev, 0);
+		mutex_unlock(&adev->lock_pstate);
+	}
+
 	kfree(bo_va);
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index fcc4b05..3368347 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -200,12 +200,26 @@ struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
 
 	if (lock)
 		mutex_lock(&tmp->hive_lock);
-
+	tmp->pstate = -1;
 	mutex_unlock(&xgmi_mutex);
 
 	return tmp;
 }
 
+int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate)
+{
+	int ret = 0;
+	struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
+
+	if (!hive)
+		return 0;
+
+	if (hive->pstate == pstate)
+		return 0;
+	/* Todo : sent the message to SMU for pstate change */
+	return ret;
+}
+
 int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev)
 {
 	int ret = -EINVAL;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
index 24a3b03..7e1710fc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
@@ -33,11 +33,13 @@ struct amdgpu_hive_info {
 	struct kobject *kobj;
 	struct device_attribute dev_attr;
 	struct amdgpu_device *adev;
+	int pstate; /*0 -- low , 1 -- high , -1 unknown*/
 };
 
 struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lock);
 int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev);
 int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
 void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
+int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
 
 #endif
-- 
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

[Deucher, Alexander]

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

Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Liu, Shaoyun <Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>
Sent: Wednesday, March 6, 2019 11:03 AM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: RE: [PATCH] drm/amdgpu: XGMI pstate switch initial support

Ping

-----Original Message-----
From: Liu, Shaoyun <Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>
Sent: Tuesday, March 5, 2019 11:25 AM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: Liu, Shaoyun <Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>
Subject: [PATCH] drm/amdgpu: XGMI pstate switch initial support

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: I499fb1c389077632fe9cfce4b6dc9a33deff6875
Signed-off-by: shaoyunl <shaoyun.liu-5C7GfCeVMHo@public.gmane.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  4 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  3 +++  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 29 ++++++++++++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   | 15 +++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  2 ++
 6 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index f0dada9..c3c8392 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -930,6 +930,10 @@ struct amdgpu_device {

         int asic_reset_res;
         struct work_struct              xgmi_reset_work;
+
+       /* counter of mapped memory through xgmi */
+       atomic_t                        xgmi_map_counter;
+
 };

 static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device *bdev) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 00def57..f28abb3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2008,6 +2008,9 @@ 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..6f176bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -54,6 +54,7 @@ struct amdgpu_bo_va_mapping {
         uint64_t                        __subtree_last;
         uint64_t                        offset;
         uint64_t                        flags;
+       bool                            is_xgmi;
 };

 /* User space allocated BO in a VM */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index c0f315b..5765761 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
@@ -2013,8 +2014,9 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
         struct ttm_mem_reg *mem;
         struct drm_mm_node *nodes;
         struct dma_fence *exclusive, **last_update;
-       uint64_t flags;
         struct amdgpu_device *bo_adev = adev;
+       bool is_xgmi = false;
+       uint64_t flags;
         int r;

         if (clear || !bo) {
@@ -2036,6 +2038,10 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
         if (bo) {
                 flags = amdgpu_ttm_tt_pte_flags(adev, bo->tbo.ttm, mem);
                 bo_adev = amdgpu_ttm_adev(bo->tbo.bdev);
+               if (adev != bo_adev &&
+                   adev->gmc.xgmi.hive_id &&
+                   adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id)
+                       is_xgmi = true;
         } else {
                 flags = 0x0;
         }
@@ -2054,6 +2060,19 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
         }

         list_for_each_entry(mapping, &bo_va->invalids, list) {
+               if (mapping->is_xgmi != is_xgmi) {
+                       if (is_xgmi) {
+                               /* Adding an XGMI mapping to the PT */
+                               if (atomic_inc_return(&adev->xgmi_map_counter) == 1)
+                                       amdgpu_xgmi_set_pstate(adev, 1);
+                       } else {
+                               /* Removing an XGMI mapping from the PT */
+                               if (atomic_dec_return(&adev->xgmi_map_counter) == 0)
+                                       amdgpu_xgmi_set_pstate(adev, 0);
+                       }
+                       mapping->is_xgmi = is_xgmi;
+               }
+
                 r = amdgpu_vm_bo_split_mapping(adev, exclusive, pages_addr, vm,
                                                mapping, flags, bo_adev, nodes,
                                                last_update);
@@ -2271,6 +2290,13 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
                 r = amdgpu_vm_bo_update_mapping(adev, NULL, NULL, vm,
                                                 mapping->start, mapping->last,
                                                 init_pte_value, 0, &f);
+
+               if (mapping->is_xgmi) {
+                       /* Removing an XGMI mapping from the PT */
+                       if (atomic_dec_return(&adev->xgmi_map_counter) == 0)
+                               amdgpu_xgmi_set_pstate(adev, 0);
+               }
+
                 amdgpu_vm_free_mapping(adev, vm, mapping, f);
                 if (r) {
                         dma_fence_put(f);
@@ -2467,6 +2493,7 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
         mapping->last = eaddr;
         mapping->offset = offset;
         mapping->flags = flags;
+       mapping->is_xgmi = false;

         amdgpu_vm_bo_insert_map(adev, bo_va, mapping);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 407dd16c..cb403cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -72,6 +72,7 @@ struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
         mutex_init(&tmp->reset_lock);
         if (lock)
                 mutex_lock(&tmp->hive_lock);
+       tmp->pstate = -1;

         mutex_unlock(&xgmi_mutex);

@@ -182,3 +183,17 @@ void amdgpu_xgmi_remove_device(struct amdgpu_device *adev)
                 mutex_unlock(&hive->hive_lock);
         }
 }
+
+int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate) {
+       int ret = 0;
+       struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
+
+       if (!hive)
+               return 0;
+
+       if (hive->pstate == pstate)
+               return 0;
+       /* Todo : sent the message to SMU for pstate change */
+       return ret;
+}
\ No newline at end of file
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
index 14bc606..6859702 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
@@ -31,11 +31,13 @@ struct amdgpu_hive_info {
         int number_devices;
         struct mutex hive_lock,
                      reset_lock;
+       int pstate; /*0 -- low , 1 -- high , -1 unknown*/
 };

 struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lock);  int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev);  int amdgpu_xgmi_add_device(struct amdgpu_device *adev);  void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
+int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);

 #endif
--
2.7.4

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

[-- Attachment #1.2: Type: text/html, Size: 17968 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: XGMI pstate switch initial support

[Liu, Shaoyun]

Ping

-----Original Message-----
From: Liu, Shaoyun <Shaoyun.Liu@amd.com> 
Sent: Tuesday, March 5, 2019 11:25 AM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Shaoyun <Shaoyun.Liu@amd.com>
Subject: [PATCH] drm/amdgpu: XGMI pstate switch initial support

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: I499fb1c389077632fe9cfce4b6dc9a33deff6875
Signed-off-by: shaoyunl <shaoyun.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  4 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  3 +++  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 29 ++++++++++++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   | 15 +++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  2 ++
 6 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index f0dada9..c3c8392 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -930,6 +930,10 @@ struct amdgpu_device {
 
 	int asic_reset_res;
 	struct work_struct		xgmi_reset_work;
+
+	/* counter of mapped memory through xgmi */
+	atomic_t			xgmi_map_counter;
+
 };
 
 static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device *bdev) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 00def57..f28abb3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2008,6 +2008,9 @@ 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..6f176bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -54,6 +54,7 @@ struct amdgpu_bo_va_mapping {
 	uint64_t			__subtree_last;
 	uint64_t			offset;
 	uint64_t			flags;
+	bool				is_xgmi;
 };
 
 /* User space allocated BO in a VM */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index c0f315b..5765761 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
@@ -2013,8 +2014,9 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 	struct ttm_mem_reg *mem;
 	struct drm_mm_node *nodes;
 	struct dma_fence *exclusive, **last_update;
-	uint64_t flags;
 	struct amdgpu_device *bo_adev = adev;
+	bool is_xgmi = false;
+	uint64_t flags;
 	int r;
 
 	if (clear || !bo) {
@@ -2036,6 +2038,10 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 	if (bo) {
 		flags = amdgpu_ttm_tt_pte_flags(adev, bo->tbo.ttm, mem);
 		bo_adev = amdgpu_ttm_adev(bo->tbo.bdev);
+		if (adev != bo_adev &&
+		    adev->gmc.xgmi.hive_id &&
+		    adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id)
+			is_xgmi = true;
 	} else {
 		flags = 0x0;
 	}
@@ -2054,6 +2060,19 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 	}
 
 	list_for_each_entry(mapping, &bo_va->invalids, list) {
+		if (mapping->is_xgmi != is_xgmi) {
+			if (is_xgmi) {
+				/* Adding an XGMI mapping to the PT */
+				if (atomic_inc_return(&adev->xgmi_map_counter) == 1)
+					amdgpu_xgmi_set_pstate(adev, 1);
+			} else {
+				/* Removing an XGMI mapping from the PT */
+				if (atomic_dec_return(&adev->xgmi_map_counter) == 0)
+					amdgpu_xgmi_set_pstate(adev, 0);
+			}
+			mapping->is_xgmi = is_xgmi;
+		}
+
 		r = amdgpu_vm_bo_split_mapping(adev, exclusive, pages_addr, vm,
 					       mapping, flags, bo_adev, nodes,
 					       last_update);
@@ -2271,6 +2290,13 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
 		r = amdgpu_vm_bo_update_mapping(adev, NULL, NULL, vm,
 						mapping->start, mapping->last,
 						init_pte_value, 0, &f);
+
+		if (mapping->is_xgmi) {
+			/* Removing an XGMI mapping from the PT */
+			if (atomic_dec_return(&adev->xgmi_map_counter) == 0)
+				amdgpu_xgmi_set_pstate(adev, 0);
+		}
+
 		amdgpu_vm_free_mapping(adev, vm, mapping, f);
 		if (r) {
 			dma_fence_put(f);
@@ -2467,6 +2493,7 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
 	mapping->last = eaddr;
 	mapping->offset = offset;
 	mapping->flags = flags;
+	mapping->is_xgmi = false;
 
 	amdgpu_vm_bo_insert_map(adev, bo_va, mapping);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 407dd16c..cb403cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -72,6 +72,7 @@ struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
 	mutex_init(&tmp->reset_lock);
 	if (lock)
 		mutex_lock(&tmp->hive_lock);
+	tmp->pstate = -1;
 
 	mutex_unlock(&xgmi_mutex);
 
@@ -182,3 +183,17 @@ void amdgpu_xgmi_remove_device(struct amdgpu_device *adev)
 		mutex_unlock(&hive->hive_lock);
 	}
 }
+
+int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate) {
+	int ret = 0;
+	struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
+
+	if (!hive)
+		return 0;
+
+	if (hive->pstate == pstate)
+		return 0;
+	/* Todo : sent the message to SMU for pstate change */
+	return ret;
+}
\ No newline at end of file
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
index 14bc606..6859702 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
@@ -31,11 +31,13 @@ struct amdgpu_hive_info {
 	int number_devices;
 	struct mutex hive_lock,
 		     reset_lock;
+	int pstate; /*0 -- low , 1 -- high , -1 unknown*/
 };
 
 struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lock);  int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev);  int amdgpu_xgmi_add_device(struct amdgpu_device *adev);  void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
+int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
 
 #endif
--
2.7.4

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

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

[Liu, Shaoyun]

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: I499fb1c389077632fe9cfce4b6dc9a33deff6875
Signed-off-by: shaoyunl <shaoyun.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  4 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  3 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 29 ++++++++++++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   | 15 +++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  2 ++
 6 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index f0dada9..c3c8392 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -930,6 +930,10 @@ struct amdgpu_device {
 
 	int asic_reset_res;
 	struct work_struct		xgmi_reset_work;
+
+	/* counter of mapped memory through xgmi */
+	atomic_t			xgmi_map_counter;
+
 };
 
 static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device *bdev)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 00def57..f28abb3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2008,6 +2008,9 @@ 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..6f176bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -54,6 +54,7 @@ struct amdgpu_bo_va_mapping {
 	uint64_t			__subtree_last;
 	uint64_t			offset;
 	uint64_t			flags;
+	bool				is_xgmi;
 };
 
 /* User space allocated BO in a VM */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index c0f315b..5765761 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
@@ -2013,8 +2014,9 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 	struct ttm_mem_reg *mem;
 	struct drm_mm_node *nodes;
 	struct dma_fence *exclusive, **last_update;
-	uint64_t flags;
 	struct amdgpu_device *bo_adev = adev;
+	bool is_xgmi = false;
+	uint64_t flags;
 	int r;
 
 	if (clear || !bo) {
@@ -2036,6 +2038,10 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 	if (bo) {
 		flags = amdgpu_ttm_tt_pte_flags(adev, bo->tbo.ttm, mem);
 		bo_adev = amdgpu_ttm_adev(bo->tbo.bdev);
+		if (adev != bo_adev &&
+		    adev->gmc.xgmi.hive_id &&
+		    adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id)
+			is_xgmi = true;
 	} else {
 		flags = 0x0;
 	}
@@ -2054,6 +2060,19 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 	}
 
 	list_for_each_entry(mapping, &bo_va->invalids, list) {
+		if (mapping->is_xgmi != is_xgmi) {
+			if (is_xgmi) {
+				/* Adding an XGMI mapping to the PT */
+				if (atomic_inc_return(&adev->xgmi_map_counter) == 1)
+					amdgpu_xgmi_set_pstate(adev, 1);
+			} else {
+				/* Removing an XGMI mapping from the PT */
+				if (atomic_dec_return(&adev->xgmi_map_counter) == 0)
+					amdgpu_xgmi_set_pstate(adev, 0);
+			}
+			mapping->is_xgmi = is_xgmi;
+		}
+
 		r = amdgpu_vm_bo_split_mapping(adev, exclusive, pages_addr, vm,
 					       mapping, flags, bo_adev, nodes,
 					       last_update);
@@ -2271,6 +2290,13 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
 		r = amdgpu_vm_bo_update_mapping(adev, NULL, NULL, vm,
 						mapping->start, mapping->last,
 						init_pte_value, 0, &f);
+
+		if (mapping->is_xgmi) {
+			/* Removing an XGMI mapping from the PT */
+			if (atomic_dec_return(&adev->xgmi_map_counter) == 0)
+				amdgpu_xgmi_set_pstate(adev, 0);
+		}
+
 		amdgpu_vm_free_mapping(adev, vm, mapping, f);
 		if (r) {
 			dma_fence_put(f);
@@ -2467,6 +2493,7 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
 	mapping->last = eaddr;
 	mapping->offset = offset;
 	mapping->flags = flags;
+	mapping->is_xgmi = false;
 
 	amdgpu_vm_bo_insert_map(adev, bo_va, mapping);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 407dd16c..cb403cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -72,6 +72,7 @@ struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo
 	mutex_init(&tmp->reset_lock);
 	if (lock)
 		mutex_lock(&tmp->hive_lock);
+	tmp->pstate = -1;
 
 	mutex_unlock(&xgmi_mutex);
 
@@ -182,3 +183,17 @@ void amdgpu_xgmi_remove_device(struct amdgpu_device *adev)
 		mutex_unlock(&hive->hive_lock);
 	}
 }
+
+int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate)
+{
+	int ret = 0;
+	struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
+
+	if (!hive)
+		return 0;
+
+	if (hive->pstate == pstate)
+		return 0;
+	/* Todo : sent the message to SMU for pstate change */
+	return ret;
+}
\ No newline at end of file
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
index 14bc606..6859702 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
@@ -31,11 +31,13 @@ struct amdgpu_hive_info {
 	int number_devices;
 	struct mutex hive_lock,
 		     reset_lock;
+	int pstate; /*0 -- low , 1 -- high , -1 unknown*/
 };
 
 struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lock);
 int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct amdgpu_device *adev);
 int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
 void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
+int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int pstate);
 
 #endif
-- 
2.7.4

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