[PATCH v3 0/7] GPU workload hints for better performance

[PATCH v3 0/7] GPU workload hints for better performance

[Arvind Yadav]

AMDGPU SOCs supports dynamic workload based power profiles, which can
provide fine-tuned performance for a particular type of workload.
This patch series adds an interface to set/reset these power profiles
based on the submitted job. The driver can dynamically switch
the power profiles based on submitted job. This can optimize the power
performance when the particular workload is on. 

v2:
- Splitting workload_profile_set and workload_profile_put
  into two separate patches.
- Addressed review comment.
- Added new suspend function.
- Added patch to switches the GPU workload mode for KFD. 

v3:
- Addressed all review comment.
- Changed the function name from *_set() to *_get().
- Now clearing all the profile in work handler.
- Added *_clear_all function to clear all the power profile.


Arvind Yadav (7):
  drm/amdgpu: Added init/fini functions for workload
  drm/amdgpu: Add new function to set GPU power profile
  drm/amdgpu: Add new function to put GPU power profile
  drm/amdgpu: Add suspend function to clear the GPU power profile.
  drm/amdgpu: Set/Reset GPU workload profile
  drm/amdgpu: switch workload context to/from compute
  Revert "drm/amd/amdgpu: switch on/off vcn power profile mode"

 drivers/gpu/drm/amd/amdgpu/Makefile           |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c    |   8 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   6 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |   5 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c       |  14 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 226 ++++++++++++++++++
 drivers/gpu/drm/amd/include/amdgpu_workload.h |  61 +++++
 8 files changed, 309 insertions(+), 16 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
 create mode 100644 drivers/gpu/drm/amd/include/amdgpu_workload.h

-- 
2.34.1

[PATCH v3 1/7] drm/amdgpu: Added init/fini functions for workload

[Arvind Yadav]

The'struct amdgpu_smu_workload' initialization/cleanup
functions is added by this patch.

v2:
- Splitting big patch into separate patches.
- Added new fini function.

v3:
- Addressed review comment to change 'power_profile_work'
  instead of 'smu_delayed_work'.

Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Shashank Sharma <shashank.sharma@amd.com>
Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/Makefile           |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |  4 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 44 +++++++++++++++
 drivers/gpu/drm/amd/include/amdgpu_workload.h | 53 +++++++++++++++++++
 5 files changed, 105 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
 create mode 100644 drivers/gpu/drm/amd/include/amdgpu_workload.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index 415a7fa395c4..6a9e187d61e1 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -60,7 +60,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 	amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
 	amdgpu_fw_attestation.o amdgpu_securedisplay.o \
 	amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o \
-	amdgpu_ring_mux.o
+	amdgpu_ring_mux.o amdgpu_workload.o
 
 amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 02b827785e39..1939fa1af8a6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -107,6 +107,7 @@
 #include "amdgpu_fdinfo.h"
 #include "amdgpu_mca.h"
 #include "amdgpu_ras.h"
+#include "amdgpu_workload.h"
 
 #define MAX_GPU_INSTANCE		16
 
@@ -1050,6 +1051,8 @@ struct amdgpu_device {
 
 	bool                            job_hang;
 	bool                            dc_enabled;
+
+	struct amdgpu_smu_workload	smu_workload;
 };
 
 static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 5c7d40873ee2..cd3bf641b630 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2243,6 +2243,8 @@ static int amdgpu_device_ip_early_init(struct amdgpu_device *adev)
 	adev->cg_flags &= amdgpu_cg_mask;
 	adev->pg_flags &= amdgpu_pg_mask;
 
+	amdgpu_workload_profile_init(adev);
+
 	return 0;
 }
 
@@ -2890,6 +2892,8 @@ static int amdgpu_device_ip_fini(struct amdgpu_device *adev)
 {
 	int i, r;
 
+	amdgpu_workload_profile_fini(adev);
+
 	if (amdgpu_sriov_vf(adev) && adev->virt.ras_init_done)
 		amdgpu_virt_release_ras_err_handler_data(adev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
new file mode 100644
index 000000000000..32166f482f77
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2023 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#include "amdgpu.h"
+
+void amdgpu_workload_profile_init(struct amdgpu_device *adev)
+{
+	adev->smu_workload.adev = adev;
+	adev->smu_workload.submit_workload_status = 0;
+	adev->smu_workload.initialized = true;
+
+	mutex_init(&adev->smu_workload.workload_lock);
+}
+
+void amdgpu_workload_profile_fini(struct amdgpu_device *adev)
+{
+	if (!adev->smu_workload.initialized)
+		return;
+
+	adev->smu_workload.submit_workload_status = 0;
+	adev->smu_workload.initialized = false;
+	mutex_destroy(&adev->smu_workload.workload_lock);
+}
diff --git a/drivers/gpu/drm/amd/include/amdgpu_workload.h b/drivers/gpu/drm/amd/include/amdgpu_workload.h
new file mode 100644
index 000000000000..dc12448764a3
--- /dev/null
+++ b/drivers/gpu/drm/amd/include/amdgpu_workload.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright 2023 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef _AMDGPU_WORKLOAD_H_
+#define _AMDGPU_WORKLOAD_H_
+
+struct amdgpu_smu_workload {
+	struct amdgpu_device	*adev;
+	struct mutex		workload_lock;
+	struct delayed_work	power_profile_work;
+	unsigned long		submit_workload_status;
+	bool			initialized;
+	atomic_t		power_profile_ref[PP_SMC_POWER_PROFILE_COUNT];
+};
+
+/* Workload mode names */
+static const char * const amdgpu_workload_mode_name[] = {
+	"Default",
+	"3D",
+	"Powersaving",
+	"Video",
+	"VR",
+	"Compute",
+	"Custom",
+	"Window3D"
+};
+
+void amdgpu_workload_profile_init(struct amdgpu_device *adev);
+
+void amdgpu_workload_profile_fini(struct amdgpu_device *adev);
+
+#endif
-- 
2.34.1

[PATCH v3 2/7] drm/amdgpu: Add new function to set GPU power profile

[Arvind Yadav]

This patch adds a function which will change the GPU
power profile based on a submitted job. This can optimize
the power performance when the workload is on.

v2:
- Splitting workload_profile_set and workload_profile_put
  into two separate patches.
- Addressed review comment.

v3:
- Adressed all the review comment.
- Changing the function name from *_set() to *_get().
- Now setting a power profile when refcount is zero.

Cc: Shashank Sharma <shashank.sharma@amd.com>
Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 59 +++++++++++++++++++
 drivers/gpu/drm/amd/include/amdgpu_workload.h |  3 +
 2 files changed, 62 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
index 32166f482f77..67eacaac6c9b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
@@ -24,6 +24,65 @@
 
 #include "amdgpu.h"
 
+static enum PP_SMC_POWER_PROFILE
+ring_to_power_profile(uint32_t ring_type)
+{
+	switch (ring_type) {
+	case AMDGPU_RING_TYPE_GFX:
+		return PP_SMC_POWER_PROFILE_FULLSCREEN3D;
+	case AMDGPU_RING_TYPE_COMPUTE:
+		return PP_SMC_POWER_PROFILE_COMPUTE;
+	case AMDGPU_RING_TYPE_UVD:
+	case AMDGPU_RING_TYPE_VCE:
+	case AMDGPU_RING_TYPE_UVD_ENC:
+	case AMDGPU_RING_TYPE_VCN_DEC:
+	case AMDGPU_RING_TYPE_VCN_ENC:
+	case AMDGPU_RING_TYPE_VCN_JPEG:
+		return PP_SMC_POWER_PROFILE_VIDEO;
+	default:
+		return PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
+	}
+}
+
+static int
+amdgpu_power_profile_set(struct amdgpu_device *adev,
+			 enum PP_SMC_POWER_PROFILE profile)
+{
+	int ret = amdgpu_dpm_switch_power_profile(adev, profile, true);
+
+	if (!ret) {
+		/* Set the bit for the submitted workload profile */
+		set_bit(profile, &adev->smu_workload.submit_workload_status);
+	}
+
+	return ret;
+}
+
+void amdgpu_workload_profile_get(struct amdgpu_device *adev,
+				 uint32_t ring_type)
+{
+	struct amdgpu_smu_workload *workload = &adev->smu_workload;
+	enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
+	int ret, refcount;
+
+	if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
+		return;
+
+	mutex_lock(&workload->workload_lock);
+
+	refcount = atomic_read(&workload->power_profile_ref[profile]);
+	if (!refcount) {
+		ret = amdgpu_power_profile_set(adev, profile);
+		if (ret) {
+			DRM_WARN("Failed to set workload profile to %s, error = %d\n",
+				 amdgpu_workload_mode_name[profile], ret);
+		}
+	}
+
+	atomic_inc(&adev->smu_workload.power_profile_ref[profile]);
+	mutex_unlock(&workload->workload_lock);
+}
+
 void amdgpu_workload_profile_init(struct amdgpu_device *adev)
 {
 	adev->smu_workload.adev = adev;
diff --git a/drivers/gpu/drm/amd/include/amdgpu_workload.h b/drivers/gpu/drm/amd/include/amdgpu_workload.h
index dc12448764a3..5fc0bc2a74a4 100644
--- a/drivers/gpu/drm/amd/include/amdgpu_workload.h
+++ b/drivers/gpu/drm/amd/include/amdgpu_workload.h
@@ -46,6 +46,9 @@ static const char * const amdgpu_workload_mode_name[] = {
 	"Window3D"
 };
 
+void amdgpu_workload_profile_get(struct amdgpu_device *adev,
+				 uint32_t ring_type);
+
 void amdgpu_workload_profile_init(struct amdgpu_device *adev);
 
 void amdgpu_workload_profile_fini(struct amdgpu_device *adev);
-- 
2.34.1

[PATCH v3 3/7] drm/amdgpu: Add new function to put GPU power profile

[Arvind Yadav]

This patch adds a function which will clear the GPU
power profile after job finished.

This is how it works:
- schedular will set the GPU power profile based on ring_type.
- Schedular will clear the GPU Power profile once job finished.
- Here, the *_workload_profile_set function will set the GPU
  power profile and the *_workload_profile_put function will
  schedule the smu_delayed_work task after 100ms delay. This
  smu_delayed_work task will clear a GPU power profile if any
  new jobs are not scheduled within 100 ms. But if any new job
  comes within 100ms then the *_workload_profile_set function
  will cancel this work and set the GPU power profile based on
  preferences.

v2:
- Splitting workload_profile_set and workload_profile_put
  into two separate patches.
- Addressed review comment.

v3:
- Adressed all the review comment.
- Now clearing all the profile in work handler.
- Added *_clear_all function to clear all the power profile.
- scheduling delay work to clear the power profile when refcount
  becomes zero.

Cc: Shashank Sharma <shashank.sharma@amd.com>
Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 118 +++++++++++++++++-
 drivers/gpu/drm/amd/include/amdgpu_workload.h |   3 +
 2 files changed, 120 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
index 67eacaac6c9b..fbe86ee5b8bf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
@@ -24,6 +24,9 @@
 
 #include "amdgpu.h"
 
+/* 100 millsecond timeout */
+#define SMU_IDLE_TIMEOUT	msecs_to_jiffies(100)
+
 static enum PP_SMC_POWER_PROFILE
 ring_to_power_profile(uint32_t ring_type)
 {
@@ -58,16 +61,111 @@ amdgpu_power_profile_set(struct amdgpu_device *adev,
 	return ret;
 }
 
+static int
+amdgpu_power_profile_clear(struct amdgpu_device *adev,
+			   enum PP_SMC_POWER_PROFILE profile)
+{
+	int ret = amdgpu_dpm_switch_power_profile(adev, profile, false);
+
+	if (!ret) {
+		/* Clear the bit for the submitted workload profile */
+		clear_bit(profile, &adev->smu_workload.submit_workload_status);
+	}
+
+	return ret;
+}
+
+static void
+amdgpu_power_profile_clear_all(struct amdgpu_device *adev,
+			       struct amdgpu_smu_workload *workload)
+{
+	int ret;
+	int profile = PP_SMC_POWER_PROFILE_COMPUTE;
+
+	cancel_delayed_work_sync(&workload->power_profile_work);
+	mutex_lock(&workload->workload_lock);
+
+	/* Clear all the GPU power profile*/
+	for (; profile > 0; profile--) {
+		atomic_set(&workload->power_profile_ref[profile], 0);
+		ret = amdgpu_power_profile_clear(adev, profile);
+		if (ret) {
+			DRM_WARN("Failed to clear workload %s,error = %d\n",
+				 amdgpu_workload_mode_name[profile], ret);
+		}
+	}
+
+	workload->submit_workload_status = 0;
+	mutex_unlock(&workload->workload_lock);
+}
+
+static void
+amdgpu_power_profile_idle_work_handler(struct work_struct *work)
+{
+
+	struct amdgpu_smu_workload *workload = container_of(work,
+						      struct amdgpu_smu_workload,
+						      power_profile_work.work);
+	struct amdgpu_device *adev = workload->adev;
+	int ret;
+	int profile;
+
+	mutex_lock(&workload->workload_lock);
+
+	/* Clear all the GPU power profile*/
+	for_each_set_bit(profile, &workload->submit_workload_status,
+			 PP_SMC_POWER_PROFILE_CUSTOM) {
+		if (!atomic_read(&workload->power_profile_ref[profile])) {
+			ret = amdgpu_power_profile_clear(adev, profile);
+			if (ret) {
+				DRM_WARN("Failed to clear workload %s,error = %d\n",
+					 amdgpu_workload_mode_name[profile], ret);
+			}
+		}
+	}
+
+	mutex_unlock(&workload->workload_lock);
+}
+
+void amdgpu_workload_profile_put(struct amdgpu_device *adev,
+				 uint32_t ring_type)
+{
+	struct amdgpu_smu_workload *workload = &adev->smu_workload;
+	enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
+	int refcount;
+
+	if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
+		return;
+
+	mutex_lock(&workload->workload_lock);
+
+	refcount = atomic_read(&workload->power_profile_ref[profile]);
+	if (!refcount) {
+		DRM_WARN("Power profile %s ref. count error\n",
+			 amdgpu_workload_mode_name[profile]);
+	} else {
+		if (refcount == 1)
+			schedule_delayed_work(&workload->power_profile_work,
+					      SMU_IDLE_TIMEOUT);
+
+		atomic_dec(&workload->power_profile_ref[profile]);
+	}
+
+	mutex_unlock(&workload->workload_lock);
+}
+
 void amdgpu_workload_profile_get(struct amdgpu_device *adev,
 				 uint32_t ring_type)
 {
 	struct amdgpu_smu_workload *workload = &adev->smu_workload;
 	enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
 	int ret, refcount;
+	int index;
 
 	if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
 		return;
 
+	cancel_delayed_work_sync(&workload->power_profile_work);
 	mutex_lock(&workload->workload_lock);
 
 	refcount = atomic_read(&workload->power_profile_ref[profile]);
@@ -80,6 +178,22 @@ void amdgpu_workload_profile_get(struct amdgpu_device *adev,
 	}
 
 	atomic_inc(&adev->smu_workload.power_profile_ref[profile]);
+
+	/* As we cancelled the delayed work, check and clear the pending
+	 * power profiles set by previous jobs which are now done.
+	 */
+	for_each_set_bit(index, &workload->submit_workload_status,
+			 PP_SMC_POWER_PROFILE_CUSTOM) {
+		if (!atomic_read(&workload->power_profile_ref[index]) &&
+		    (index != profile)) {
+			ret = amdgpu_power_profile_clear(adev, index);
+			if (ret) {
+				DRM_WARN("Failed to clear workload %s, err = %d\n",
+					 amdgpu_workload_mode_name[profile], ret);
+			}
+		}
+	}
+
 	mutex_unlock(&workload->workload_lock);
 }
 
@@ -90,6 +204,8 @@ void amdgpu_workload_profile_init(struct amdgpu_device *adev)
 	adev->smu_workload.initialized = true;
 
 	mutex_init(&adev->smu_workload.workload_lock);
+	INIT_DELAYED_WORK(&adev->smu_workload.power_profile_work,
+			  amdgpu_power_profile_idle_work_handler);
 }
 
 void amdgpu_workload_profile_fini(struct amdgpu_device *adev)
@@ -97,7 +213,7 @@ void amdgpu_workload_profile_fini(struct amdgpu_device *adev)
 	if (!adev->smu_workload.initialized)
 		return;
 
-	adev->smu_workload.submit_workload_status = 0;
+	amdgpu_power_profile_clear_all(adev, &adev->smu_workload);
 	adev->smu_workload.initialized = false;
 	mutex_destroy(&adev->smu_workload.workload_lock);
 }
diff --git a/drivers/gpu/drm/amd/include/amdgpu_workload.h b/drivers/gpu/drm/amd/include/amdgpu_workload.h
index 5fc0bc2a74a4..596a962800e9 100644
--- a/drivers/gpu/drm/amd/include/amdgpu_workload.h
+++ b/drivers/gpu/drm/amd/include/amdgpu_workload.h
@@ -46,6 +46,9 @@ static const char * const amdgpu_workload_mode_name[] = {
 	"Window3D"
 };
 
+void amdgpu_workload_profile_put(struct amdgpu_device *adev,
+				 uint32_t ring_type);
+
 void amdgpu_workload_profile_get(struct amdgpu_device *adev,
 				 uint32_t ring_type);
 
-- 
2.34.1

[PATCH v3 4/7] drm/amdgpu: Add suspend function to clear the GPU power profile.

[Arvind Yadav]

This patch adds a suspend function that will clear the GPU
power profile before going into suspend state.

v2:
- Add the new suspend function based on review comment.

v3:
- Adressed the review comment.
- Now clearing all the profile in work handler.

Cc: Shashank Sharma <shashank.sharma@amd.com>
Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    | 2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 7 +++++++
 drivers/gpu/drm/amd/include/amdgpu_workload.h | 2 ++
 3 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index cd3bf641b630..3b70e657b439 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4212,6 +4212,8 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
 
 	amdgpu_ras_suspend(adev);
 
+	amdgpu_workload_profile_suspend(adev);
+
 	amdgpu_device_ip_suspend_phase1(adev);
 
 	if (!adev->in_s0ix)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
index fbe86ee5b8bf..0ba0000b274a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
@@ -197,6 +197,13 @@ void amdgpu_workload_profile_get(struct amdgpu_device *adev,
 	mutex_unlock(&workload->workload_lock);
 }
 
+void amdgpu_workload_profile_suspend(struct amdgpu_device *adev)
+{
+	struct amdgpu_smu_workload *workload = &adev->smu_workload;
+
+	amdgpu_power_profile_clear_all(adev, workload);
+}
+
 void amdgpu_workload_profile_init(struct amdgpu_device *adev)
 {
 	adev->smu_workload.adev = adev;
diff --git a/drivers/gpu/drm/amd/include/amdgpu_workload.h b/drivers/gpu/drm/amd/include/amdgpu_workload.h
index 596a962800e9..34b96a5e7b50 100644
--- a/drivers/gpu/drm/amd/include/amdgpu_workload.h
+++ b/drivers/gpu/drm/amd/include/amdgpu_workload.h
@@ -52,6 +52,8 @@ void amdgpu_workload_profile_put(struct amdgpu_device *adev,
 void amdgpu_workload_profile_get(struct amdgpu_device *adev,
 				 uint32_t ring_type);
 
+void amdgpu_workload_profile_suspend(struct amdgpu_device *adev);
+
 void amdgpu_workload_profile_init(struct amdgpu_device *adev);
 
 void amdgpu_workload_profile_fini(struct amdgpu_device *adev);
-- 
2.34.1

[PATCH v3 5/7] drm/amdgpu: Set/Reset GPU workload profile

[Arvind Yadav]

This patch is to switch the GPU workload profile based
on the submitted job. The workload profile is reset to
default when the job is done.

v3:
- Addressed the review comment about changing the function
  name from *_set() to *_get().

Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Shashank Sharma <shashank.sharma@amd.com>
Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index c3d9d75143f4..c5032762d497 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -176,6 +176,9 @@ void amdgpu_job_free_resources(struct amdgpu_job *job)
 static void amdgpu_job_free_cb(struct drm_sched_job *s_job)
 {
 	struct amdgpu_job *job = to_amdgpu_job(s_job);
+	struct amdgpu_ring *ring = to_amdgpu_ring(s_job->sched);
+
+	amdgpu_workload_profile_put(ring->adev, ring->funcs->type);
 
 	drm_sched_job_cleanup(s_job);
 
@@ -295,6 +298,8 @@ static struct dma_fence *amdgpu_job_run(struct drm_sched_job *sched_job)
 			DRM_ERROR("Error scheduling IBs (%d)\n", r);
 	}
 
+	amdgpu_workload_profile_get(adev, ring->funcs->type);
+
 	job->job_run_counter++;
 	amdgpu_job_free_resources(job);
 
-- 
2.34.1

[PATCH v3 6/7] drm/amdgpu: switch workload context to/from compute

[Arvind Yadav]

This patch switches the GPU workload mode to/from
compute mode, while submitting compute workload.

v3:
- Addressed the review comment about changing the
  function name from *_set() to *_get().

Cc: Christian Koenig <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Shashank Sharma <shashank.sharma@amd.com>
Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 0385f7f69278..fa939ac17120 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -713,9 +713,11 @@ void amdgpu_amdkfd_set_compute_idle(struct amdgpu_device *adev, bool idle)
 		pr_debug("GFXOFF is %s\n", idle ? "enabled" : "disabled");
 		amdgpu_gfx_off_ctrl(adev, idle);
 	}
-	amdgpu_dpm_switch_power_profile(adev,
-					PP_SMC_POWER_PROFILE_COMPUTE,
-					!idle);
+
+	if (idle)
+		amdgpu_workload_profile_put(adev, AMDGPU_RING_TYPE_COMPUTE);
+	else
+		amdgpu_workload_profile_get(adev, AMDGPU_RING_TYPE_COMPUTE);
 }
 
 bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device *adev, u32 vmid)
-- 
2.34.1

[PATCH v3 7/7] Revert "drm/amd/amdgpu: switch on/off vcn power profile mode"

[Arvind Yadav]

This reverts commit 5ce71f59bb9bd3d8a09b96afdbc92975cb6dc303.

Reason for revert: New  amdgpu_workload_profile* api is added
to switch on/off profile mode. These new api will allow to
change the GPU power profile based on a submitted job.

Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Acked-by: Shashank Sharma <shashank.sharma@amd.com>
Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 2d94f1b63bd6..70777fcfa626 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -363,7 +363,6 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
 		container_of(work, struct amdgpu_device, vcn.idle_work.work);
 	unsigned int fences = 0, fence[AMDGPU_MAX_VCN_INSTANCES] = {0};
 	unsigned int i, j;
-	int r = 0;
 
 	for (j = 0; j < adev->vcn.num_vcn_inst; ++j) {
 		if (adev->vcn.harvest_config & (1 << j))
@@ -392,10 +391,6 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
 	if (!fences && !atomic_read(&adev->vcn.total_submission_cnt)) {
 		amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
 		       AMD_PG_STATE_GATE);
-		r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
-				false);
-		if (r)
-			dev_warn(adev->dev, "(%d) failed to disable video power profile mode\n", r);
 	} else {
 		schedule_delayed_work(&adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
 	}
@@ -404,16 +399,11 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
 void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
 {
 	struct amdgpu_device *adev = ring->adev;
-	int r = 0;
 
 	atomic_inc(&adev->vcn.total_submission_cnt);
 
-	if (!cancel_delayed_work_sync(&adev->vcn.idle_work)) {
-		r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
-				true);
-		if (r)
-			dev_warn(adev->dev, "(%d) failed to switch to video power profile mode\n", r);
-	}
+	if (!cancel_delayed_work_sync(&adev->vcn.idle_work))
+		amdgpu_gfx_off_ctrl(adev, false);
 
 	mutex_lock(&adev->vcn.vcn_pg_lock);
 	amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
-- 
2.34.1

Re: [PATCH v3 0/7] GPU workload hints for better performance

[Lazar, Lijo]

[-- Attachment #1: Type: text/plain, Size: 3805 bytes --]

[AMD Official Use Only - General]

As mentioned with an older version of this series, this is an 'abuse' of power profile interface.

This series is oversimplifying what PMFW algorithms are supposed to be doing. Whatever this series is doing, FW can do it better.

To explain in simpler terms - it just tries to boost a profile based on ring type without even knowing how much of activity a job can trigger on a particular ring. A job scheduled to a GFX ring doesn't deserve a profile boost unless it can create a certain level of activity. In CPU terms, a job scheduled to a processor doesn't mean it deserves a frequency boost of that CPU.  At minimum it depends on more details like whether that job is compute bound or memory bound or memory bound.

While FW algorithms are designed to do that, this series tries to trivialise all such things.

Unless you are able to show the tangible benefits in some terms like performance, power, or performance per watt,  I don't think this should be the default behaviour where driver tries to override FW just based on job submissions to rings.

Thanks,
Lijo
________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Arvind Yadav <Arvind.Yadav@amd.com>
Sent: Monday, August 28, 2023 5:56:07 PM
To: Koenig, Christian <Christian.Koenig@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; airlied@gmail.com <airlied@gmail.com>; daniel@ffwll.ch <daniel@ffwll.ch>; Kuehling, Felix <Felix.Kuehling@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Yadav, Arvind <Arvind.Yadav@amd.com>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>
Subject: [PATCH v3 0/7] GPU workload hints for better performance

AMDGPU SOCs supports dynamic workload based power profiles, which can
provide fine-tuned performance for a particular type of workload.
This patch series adds an interface to set/reset these power profiles
based on the submitted job. The driver can dynamically switch
the power profiles based on submitted job. This can optimize the power
performance when the particular workload is on.

v2:
- Splitting workload_profile_set and workload_profile_put
  into two separate patches.
- Addressed review comment.
- Added new suspend function.
- Added patch to switches the GPU workload mode for KFD.

v3:
- Addressed all review comment.
- Changed the function name from *_set() to *_get().
- Now clearing all the profile in work handler.
- Added *_clear_all function to clear all the power profile.


Arvind Yadav (7):
  drm/amdgpu: Added init/fini functions for workload
  drm/amdgpu: Add new function to set GPU power profile
  drm/amdgpu: Add new function to put GPU power profile
  drm/amdgpu: Add suspend function to clear the GPU power profile.
  drm/amdgpu: Set/Reset GPU workload profile
  drm/amdgpu: switch workload context to/from compute
  Revert "drm/amd/amdgpu: switch on/off vcn power profile mode"

 drivers/gpu/drm/amd/amdgpu/Makefile           |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c    |   8 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   6 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |   5 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c       |  14 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 226 ++++++++++++++++++
 drivers/gpu/drm/amd/include/amdgpu_workload.h |  61 +++++
 8 files changed, 309 insertions(+), 16 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
 create mode 100644 drivers/gpu/drm/amd/include/amdgpu_workload.h

--
2.34.1


[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 20153 bytes --]

Re: [PATCH v3 0/7] GPU workload hints for better performance

[Helen Mae Koike Fornazier]

On Monday, August 28, 2023 09:26 -03, Arvind Yadav <Arvind.Yadav@amd.com> wrote:

> AMDGPU SOCs supports dynamic workload based power profiles, which can
> provide fine-tuned performance for a particular type of workload.
> This patch series adds an interface to set/reset these power profiles
> based on the submitted job. The driver can dynamically switch
> the power profiles based on submitted job. This can optimize the power
> performance when the particular workload is on. 

Hi Arvind,

Would you mind to test your patchset with drm-ci ? There is a amdgpu
test there and I would love to get your feedback of the ci.

You basically just need to apply the ci patch which is available on
https://cgit.freedesktop.org/drm/drm/log/?h=topic/drm-ci

There are instruction on the docs, but in short: to configure it, you push
your branch to gitlab.freedesktop.org, go to the settings and change the
CI/CD configuration file from .gitlab-ci.yml to drivers/gpu/drm/ci/gitlab-ci.yml,
and you can trigger a pipeline on your branch to get tests running.

(by the time of this writing, gitlab.fdo is under maintenance but should
be up soonish).

Thank you!
Helen

> 
> v2:
> - Splitting workload_profile_set and workload_profile_put
>   into two separate patches.
> - Addressed review comment.
> - Added new suspend function.
> - Added patch to switches the GPU workload mode for KFD. 
> 
> v3:
> - Addressed all review comment.
> - Changed the function name from *_set() to *_get().
> - Now clearing all the profile in work handler.
> - Added *_clear_all function to clear all the power profile.
> 
> 
> Arvind Yadav (7):
>   drm/amdgpu: Added init/fini functions for workload
>   drm/amdgpu: Add new function to set GPU power profile
>   drm/amdgpu: Add new function to put GPU power profile
>   drm/amdgpu: Add suspend function to clear the GPU power profile.
>   drm/amdgpu: Set/Reset GPU workload profile
>   drm/amdgpu: switch workload context to/from compute
>   Revert "drm/amd/amdgpu: switch on/off vcn power profile mode"
> 
>  drivers/gpu/drm/amd/amdgpu/Makefile           |   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c    |   8 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   6 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |   5 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c       |  14 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 226 ++++++++++++++++++
>  drivers/gpu/drm/amd/include/amdgpu_workload.h |  61 +++++
>  8 files changed, 309 insertions(+), 16 deletions(-)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
>  create mode 100644 drivers/gpu/drm/amd/include/amdgpu_workload.h
> 
> -- 
> 2.34.1
>

Re: [PATCH v3 0/7] GPU workload hints for better performance

[Yadav, Arvind]

On 8/28/2023 9:13 PM, Helen Mae Koike Fornazier wrote:
> On Monday, August 28, 2023 09:26 -03, Arvind Yadav <Arvind.Yadav@amd.com> wrote:
>
>> AMDGPU SOCs supports dynamic workload based power profiles, which can
>> provide fine-tuned performance for a particular type of workload.
>> This patch series adds an interface to set/reset these power profiles
>> based on the submitted job. The driver can dynamically switch
>> the power profiles based on submitted job. This can optimize the power
>> performance when the particular workload is on.
> Hi Arvind,
>
> Would you mind to test your patchset with drm-ci ? There is a amdgpu
> test there and I would love to get your feedback of the ci.
>
> You basically just need to apply the ci patch which is available on
> https://cgit.freedesktop.org/drm/drm/log/?h=topic/drm-ci
>
> There are instruction on the docs, but in short: to configure it, you push
> your branch to gitlab.freedesktop.org, go to the settings and change the
> CI/CD configuration file from .gitlab-ci.yml to drivers/gpu/drm/ci/gitlab-ci.yml,
> and you can trigger a pipeline on your branch to get tests running.
>
> (by the time of this writing, gitlab.fdo is under maintenance but should
> be up soonish).

Hi Helen,

I tried the steps as mentioned by you but looks like something is 
missing and build itself is failing.

https://gitlab.freedesktop.org/ArvindYadav/drm-next/-/commits/smu_workload

Regards,
~Arvind

> Thank you!
> Helen
>
>> v2:
>> - Splitting workload_profile_set and workload_profile_put
>>    into two separate patches.
>> - Addressed review comment.
>> - Added new suspend function.
>> - Added patch to switches the GPU workload mode for KFD.
>>
>> v3:
>> - Addressed all review comment.
>> - Changed the function name from *_set() to *_get().
>> - Now clearing all the profile in work handler.
>> - Added *_clear_all function to clear all the power profile.
>>
>>
>> Arvind Yadav (7):
>>    drm/amdgpu: Added init/fini functions for workload
>>    drm/amdgpu: Add new function to set GPU power profile
>>    drm/amdgpu: Add new function to put GPU power profile
>>    drm/amdgpu: Add suspend function to clear the GPU power profile.
>>    drm/amdgpu: Set/Reset GPU workload profile
>>    drm/amdgpu: switch workload context to/from compute
>>    Revert "drm/amd/amdgpu: switch on/off vcn power profile mode"
>>
>>   drivers/gpu/drm/amd/amdgpu/Makefile           |   2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c    |   8 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   6 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |   5 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c       |  14 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 226 ++++++++++++++++++
>>   drivers/gpu/drm/amd/include/amdgpu_workload.h |  61 +++++
>>   8 files changed, 309 insertions(+), 16 deletions(-)
>>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
>>   create mode 100644 drivers/gpu/drm/amd/include/amdgpu_workload.h
>>
>> -- 
>> 2.34.1
>>

Re: [PATCH v3 0/7] GPU workload hints for better performance

[Helen Koike]

On 28/08/2023 17:14, Yadav, Arvind wrote:
> 
> On 8/28/2023 9:13 PM, Helen Mae Koike Fornazier wrote:
>> On Monday, August 28, 2023 09:26 -03, Arvind Yadav 
>> <Arvind.Yadav@amd.com> wrote:
>>
>>> AMDGPU SOCs supports dynamic workload based power profiles, which can
>>> provide fine-tuned performance for a particular type of workload.
>>> This patch series adds an interface to set/reset these power profiles
>>> based on the submitted job. The driver can dynamically switch
>>> the power profiles based on submitted job. This can optimize the power
>>> performance when the particular workload is on.
>> Hi Arvind,
>>
>> Would you mind to test your patchset with drm-ci ? There is a amdgpu
>> test there and I would love to get your feedback of the ci.
>>
>> You basically just need to apply the ci patch which is available on
>> https://cgit.freedesktop.org/drm/drm/log/?h=topic/drm-ci
>>
>> There are instruction on the docs, but in short: to configure it, you 
>> push
>> your branch to gitlab.freedesktop.org, go to the settings and change the
>> CI/CD configuration file from .gitlab-ci.yml to 
>> drivers/gpu/drm/ci/gitlab-ci.yml,
>> and you can trigger a pipeline on your branch to get tests running.
>>
>> (by the time of this writing, gitlab.fdo is under maintenance but should
>> be up soonish).
> 
> Hi Helen,
> 
> I tried the steps as mentioned by you but looks like something is 
> missing and build itself is failing.
> 
> https://gitlab.freedesktop.org/ArvindYadav/drm-next/-/commits/smu_workload

Thanks for your feedback!

You need to apply this patch 
https://gitlab.freedesktop.org/ArvindYadav/drm-next/-/commit/cc6dcff192d07f9fe82645fbc4213c97e872156b

This patch adds the file drivers/gpu/drm/ci/gitlab-ci.yml for you.

And you can drop the patch where gitlab added the ci template.

I replied here too 
https://gitlab.freedesktop.org/ArvindYadav/drm-next/-/commit/cc6dcff192d07f9fe82645fbc4213c97e872156b

Could you try again with that patch?

Thanks a lot!
Helen


> 
> Regards,
> ~Arvind
> 
>> Thank you!
>> Helen
>>
>>> v2:
>>> - Splitting workload_profile_set and workload_profile_put
>>>    into two separate patches.
>>> - Addressed review comment.
>>> - Added new suspend function.
>>> - Added patch to switches the GPU workload mode for KFD.
>>>
>>> v3:
>>> - Addressed all review comment.
>>> - Changed the function name from *_set() to *_get().
>>> - Now clearing all the profile in work handler.
>>> - Added *_clear_all function to clear all the power profile.
>>>
>>>
>>> Arvind Yadav (7):
>>>    drm/amdgpu: Added init/fini functions for workload
>>>    drm/amdgpu: Add new function to set GPU power profile
>>>    drm/amdgpu: Add new function to put GPU power profile
>>>    drm/amdgpu: Add suspend function to clear the GPU power profile.
>>>    drm/amdgpu: Set/Reset GPU workload profile
>>>    drm/amdgpu: switch workload context to/from compute
>>>    Revert "drm/amd/amdgpu: switch on/off vcn power profile mode"
>>>
>>>   drivers/gpu/drm/amd/amdgpu/Makefile           |   2 +-
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c    |   8 +-
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   6 +
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |   5 +
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c       |  14 +-
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 226 ++++++++++++++++++
>>>   drivers/gpu/drm/amd/include/amdgpu_workload.h |  61 +++++
>>>   8 files changed, 309 insertions(+), 16 deletions(-)
>>>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
>>>   create mode 100644 drivers/gpu/drm/amd/include/amdgpu_workload.h
>>>
>>> -- 
>>> 2.34.1
>>>

Re: [PATCH v3 5/7] drm/amdgpu: Set/Reset GPU workload profile

[Christian König]

Am 28.08.23 um 14:26 schrieb Arvind Yadav:
> This patch is to switch the GPU workload profile based
> on the submitted job. The workload profile is reset to
> default when the job is done.
>
> v3:
> - Addressed the review comment about changing the function
>    name from *_set() to *_get().

That looks like a really bad idea in general. This are the high level 
functions, but what you want to use are the low level functions for each 
ring.

Take a look at amdgpu_ring_begin_use()/_end_use() instead.

Christian.

>
> Cc: Christian Koenig <christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Reviewed-by: Shashank Sharma <shashank.sharma@amd.com>
> Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index c3d9d75143f4..c5032762d497 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -176,6 +176,9 @@ void amdgpu_job_free_resources(struct amdgpu_job *job)
>   static void amdgpu_job_free_cb(struct drm_sched_job *s_job)
>   {
>   	struct amdgpu_job *job = to_amdgpu_job(s_job);
> +	struct amdgpu_ring *ring = to_amdgpu_ring(s_job->sched);
> +
> +	amdgpu_workload_profile_put(ring->adev, ring->funcs->type);
>   
>   	drm_sched_job_cleanup(s_job);
>   
> @@ -295,6 +298,8 @@ static struct dma_fence *amdgpu_job_run(struct drm_sched_job *sched_job)
>   			DRM_ERROR("Error scheduling IBs (%d)\n", r);
>   	}
>   
> +	amdgpu_workload_profile_get(adev, ring->funcs->type);
> +
>   	job->job_run_counter++;
>   	amdgpu_job_free_resources(job);
>   

Re: [PATCH v3 0/7] GPU workload hints for better performance

[Michel Dänzer]

On 8/28/23 17:02, Lazar, Lijo wrote:
> [AMD Official Use Only - General]
> 
> 
> As mentioned with an older version of this series, this is an 'abuse' of power profile interface.
> 
> This series is oversimplifying what PMFW algorithms are supposed to be doing. Whatever this series is doing, FW can do it better.
> 
> To explain in simpler terms - it just tries to boost a profile based on ring type without even knowing how much of activity a job can trigger on a particular ring. A job scheduled to a GFX ring doesn't deserve a profile boost unless it can create a certain level of activity. In CPU terms, a job scheduled to a processor doesn't mean it deserves a frequency boost of that CPU.  At minimum it depends on more details like whether that job is compute bound or memory bound or memory bound. 
> 
> While FW algorithms are designed to do that, this series tries to trivialise all such things.
> 
> Unless you are able to show the tangible benefits in some terms like performance, power, or performance per watt,  I don't think this should be the default behaviour where driver tries to override FW just based on job submissions to rings.

I know at least one tangible benefit this would have: a snappier GNOME desktop with lower input → output latency on many laptops. The bootup default profile doesn't work well for that IME.

It should also help for issues like
https://gitlab.freedesktop.org/drm/amd/-/issues/1500 .

That said, I agree this approach is very aggressive. I think it might be acceptable with AC power, not sure about on battery though. (There might be better performance/power profile mechanisms to hook into than AC vs battery)


-- 
Earthling Michel Dänzer            |                  https://redhat.com
Libre software enthusiast          |         Mesa and Xwayland developer