[PATCH 0/3] GPU workload hints for better performance

[PATCH 0/3] 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.

Arvind Yadav (3):
  drm/amdgpu: Add new api to switch on/off power profile mode
  drm/amdgpu: Switch on/off GPU workload profile
  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_device.c    |   2 +
 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  | 156 ++++++++++++++++++
 drivers/gpu/drm/amd/include/amdgpu_workload.h |  44 +++++
 7 files changed, 213 insertions(+), 13 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 1/3] drm/amdgpu: Add new api to switch on/off power profile mode

[Arvind Yadav]

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

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/Makefile           |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 156 ++++++++++++++++++
 drivers/gpu/drm/amd/include/amdgpu_workload.h |  44 +++++
 5 files changed, 206 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..0ec18b8fe29f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3672,6 +3672,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 
 	INIT_WORK(&adev->xgmi_reset_work, amdgpu_device_xgmi_reset_func);
 
+	amdgpu_smu_workload_init(adev);
+
 	adev->gfx.gfx_off_req_count = 1;
 	adev->gfx.gfx_off_residency = 0;
 	adev->gfx.gfx_off_entrycount = 0;
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..ce0339d75c12
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
@@ -0,0 +1,156 @@
+// 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"
+
+/* 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)
+{
+	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 void
+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 == 0) {
+		/* Set the bit for the submitted workload profile */
+		adev->smu_workload.submit_workload_status |= (1 << profile);
+		atomic_inc(&adev->smu_workload.power_profile_ref[profile]);
+	} else {
+		DRM_ERROR("Failed to set power profile, error %d\n", ret);
+	}
+
+}
+
+static void
+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 == 0) {
+		 /* Clear the bit for the submitted workload profile */
+		adev->smu_workload.submit_workload_status &= ~(1 << profile);
+	} else
+		DRM_ERROR("Failed to clear power profile, error %d\n", ret);
+
+}
+
+static void amdgpu_smu_idle_work_handler(struct work_struct *work)
+{
+
+	struct amdgpu_smu_workload *wl = container_of(work,
+						      struct amdgpu_smu_workload,
+						      smu_delayed_work.work);
+	struct amdgpu_device *adev = wl->adev;
+	bool reschedule = false;
+
+	mutex_lock(&adev->smu_workload.workload_lock);
+	for (int index  = fls(adev->smu_workload.submit_workload_status);
+	     index >= 0; index--) {
+		if (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
+		    adev->smu_workload.submit_workload_status & (1 << index)) {
+			amdgpu_power_profile_clear(adev, index);
+		} else if (atomic_read(&adev->smu_workload.power_profile_ref[index]))
+			reschedule = true;
+	}
+
+	if (reschedule)
+		schedule_delayed_work(&adev->smu_workload.smu_delayed_work,
+				      SMU_IDLE_TIMEOUT);
+
+	mutex_unlock(&adev->smu_workload.workload_lock);
+}
+
+void amdgpu_put_workload_profile(struct amdgpu_device *adev,
+				 uint32_t ring_type)
+{
+
+	enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
+
+	if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
+		return;
+
+	mutex_lock(&adev->smu_workload.workload_lock);
+	atomic_dec(&adev->smu_workload.power_profile_ref[profile]);
+	schedule_delayed_work(&adev->smu_workload.smu_delayed_work, SMU_IDLE_TIMEOUT);
+	mutex_unlock(&adev->smu_workload.workload_lock);
+}
+
+void amdgpu_set_workload_profile(struct amdgpu_device *adev,
+				 uint32_t ring_type)
+{
+	enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
+
+	if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
+		return;
+
+	mutex_lock(&adev->smu_workload.workload_lock);
+	cancel_delayed_work_sync(&adev->smu_workload.smu_delayed_work);
+
+	amdgpu_power_profile_set(adev, profile);
+
+	/* Clear the already finished jobs of higher power profile*/
+	for (int index = fls(adev->smu_workload.submit_workload_status);
+	     index > profile; index--) {
+		if (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
+		    adev->smu_workload.submit_workload_status & (1 << index)) {
+			amdgpu_power_profile_clear(adev, index);
+		}
+	}
+
+	mutex_unlock(&adev->smu_workload.workload_lock);
+}
+
+void amdgpu_smu_workload_init(struct amdgpu_device *adev)
+{
+	struct amdgpu_smu_workload wl;
+
+	wl.adev = adev;
+	wl.submit_workload_status = 0;
+	adev->smu_workload = wl;
+
+	mutex_init(&adev->smu_workload.workload_lock);
+	INIT_DELAYED_WORK(&adev->smu_workload.smu_delayed_work, amdgpu_smu_idle_work_handler);
+}
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..09804c3d2869
--- /dev/null
+++ b/drivers/gpu/drm/amd/include/amdgpu_workload.h
@@ -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.
+ *
+ */
+
+#ifndef _AMDGPU_WORKLOAD_H_
+#define _AMDGPU_WORKLOAD_H_
+
+struct amdgpu_smu_workload {
+	struct amdgpu_device	*adev;
+	struct mutex		workload_lock;
+	struct delayed_work	smu_delayed_work;
+	uint32_t		submit_workload_status;
+	atomic_t		power_profile_ref[PP_SMC_POWER_PROFILE_COUNT];
+};
+
+void amdgpu_set_workload_profile(struct amdgpu_device *adev,
+				 uint32_t ring_type);
+
+void amdgpu_put_workload_profile(struct amdgpu_device *adev,
+				 uint32_t ring_type);
+
+void amdgpu_smu_workload_init(struct amdgpu_device *adev);
+
+#endif
-- 
2.34.1

[PATCH 2/3] drm/amdgpu: Switch on/off 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.

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_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..8457d2468fd8 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_put_workload_profile(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_set_workload_profile(ring->adev, ring->funcs->type);
+
 	job->job_run_counter++;
 	amdgpu_job_free_resources(job);
 
-- 
2.34.1

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

[Arvind Yadav]

This reverts commit 5ce71f59bb9bd3d8a09b96afdbc92975cb6dc303.

Reason for revert: New amdgpu_smu* 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: 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_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 1/3] drm/amdgpu: Add new api to switch on/off power profile mode

[Christian König]

Am 14.08.23 um 09:34 schrieb Arvind Yadav:
> This patch adds a function which will allow to
> change the GPU power profile based on a submitted job.
> This can optimize the power performance when the
> workload is on.
>
> 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/Makefile           |   2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   2 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 156 ++++++++++++++++++
>   drivers/gpu/drm/amd/include/amdgpu_workload.h |  44 +++++
>   5 files changed, 206 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..0ec18b8fe29f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3672,6 +3672,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   
>   	INIT_WORK(&adev->xgmi_reset_work, amdgpu_device_xgmi_reset_func);
>   
> +	amdgpu_smu_workload_init(adev);
> +
>   	adev->gfx.gfx_off_req_count = 1;
>   	adev->gfx.gfx_off_residency = 0;
>   	adev->gfx.gfx_off_entrycount = 0;
> 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..ce0339d75c12
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
> @@ -0,0 +1,156 @@
> +// 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"
> +
> +/* 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)
> +{
> +	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 void
> +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 == 0) {

Please double check the coding style, stuff like this is usually 
complained about by the automated checkers.

Apart from that Alex needs to take a look if this here makes sense or not.

Regards,
Christian.

> +		/* Set the bit for the submitted workload profile */
> +		adev->smu_workload.submit_workload_status |= (1 << profile);
> +		atomic_inc(&adev->smu_workload.power_profile_ref[profile]);
> +	} else {
> +		DRM_ERROR("Failed to set power profile, error %d\n", ret);
> +	}
> +
> +}
> +
> +static void
> +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 == 0) {
> +		 /* Clear the bit for the submitted workload profile */
> +		adev->smu_workload.submit_workload_status &= ~(1 << profile);
> +	} else
> +		DRM_ERROR("Failed to clear power profile, error %d\n", ret);
> +
> +}
> +
> +static void amdgpu_smu_idle_work_handler(struct work_struct *work)
> +{
> +
> +	struct amdgpu_smu_workload *wl = container_of(work,
> +						      struct amdgpu_smu_workload,
> +						      smu_delayed_work.work);
> +	struct amdgpu_device *adev = wl->adev;
> +	bool reschedule = false;
> +
> +	mutex_lock(&adev->smu_workload.workload_lock);
> +	for (int index  = fls(adev->smu_workload.submit_workload_status);
> +	     index >= 0; index--) {
> +		if (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
> +		    adev->smu_workload.submit_workload_status & (1 << index)) {
> +			amdgpu_power_profile_clear(adev, index);
> +		} else if (atomic_read(&adev->smu_workload.power_profile_ref[index]))
> +			reschedule = true;
> +	}
> +
> +	if (reschedule)
> +		schedule_delayed_work(&adev->smu_workload.smu_delayed_work,
> +				      SMU_IDLE_TIMEOUT);
> +
> +	mutex_unlock(&adev->smu_workload.workload_lock);
> +}
> +
> +void amdgpu_put_workload_profile(struct amdgpu_device *adev,
> +				 uint32_t ring_type)
> +{
> +
> +	enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
> +
> +	if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
> +		return;
> +
> +	mutex_lock(&adev->smu_workload.workload_lock);
> +	atomic_dec(&adev->smu_workload.power_profile_ref[profile]);
> +	schedule_delayed_work(&adev->smu_workload.smu_delayed_work, SMU_IDLE_TIMEOUT);
> +	mutex_unlock(&adev->smu_workload.workload_lock);
> +}
> +
> +void amdgpu_set_workload_profile(struct amdgpu_device *adev,
> +				 uint32_t ring_type)
> +{
> +	enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
> +
> +	if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
> +		return;
> +
> +	mutex_lock(&adev->smu_workload.workload_lock);
> +	cancel_delayed_work_sync(&adev->smu_workload.smu_delayed_work);
> +
> +	amdgpu_power_profile_set(adev, profile);
> +
> +	/* Clear the already finished jobs of higher power profile*/
> +	for (int index = fls(adev->smu_workload.submit_workload_status);
> +	     index > profile; index--) {
> +		if (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
> +		    adev->smu_workload.submit_workload_status & (1 << index)) {
> +			amdgpu_power_profile_clear(adev, index);
> +		}
> +	}
> +
> +	mutex_unlock(&adev->smu_workload.workload_lock);
> +}
> +
> +void amdgpu_smu_workload_init(struct amdgpu_device *adev)
> +{
> +	struct amdgpu_smu_workload wl;
> +
> +	wl.adev = adev;
> +	wl.submit_workload_status = 0;
> +	adev->smu_workload = wl;
> +
> +	mutex_init(&adev->smu_workload.workload_lock);
> +	INIT_DELAYED_WORK(&adev->smu_workload.smu_delayed_work, amdgpu_smu_idle_work_handler);
> +}
> 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..09804c3d2869
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/include/amdgpu_workload.h
> @@ -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.
> + *
> + */
> +
> +#ifndef _AMDGPU_WORKLOAD_H_
> +#define _AMDGPU_WORKLOAD_H_
> +
> +struct amdgpu_smu_workload {
> +	struct amdgpu_device	*adev;
> +	struct mutex		workload_lock;
> +	struct delayed_work	smu_delayed_work;
> +	uint32_t		submit_workload_status;
> +	atomic_t		power_profile_ref[PP_SMC_POWER_PROFILE_COUNT];
> +};
> +
> +void amdgpu_set_workload_profile(struct amdgpu_device *adev,
> +				 uint32_t ring_type);
> +
> +void amdgpu_put_workload_profile(struct amdgpu_device *adev,
> +				 uint32_t ring_type);
> +
> +void amdgpu_smu_workload_init(struct amdgpu_device *adev);
> +
> +#endif

Re: [PATCH 1/3] drm/amdgpu: Add new api to switch on/off power profile mode

[Yadav, Arvind]

On 8/14/2023 5:35 PM, Christian König wrote:
> Am 14.08.23 um 09:34 schrieb Arvind Yadav:
>> This patch adds a function which will allow to
>> change the GPU power profile based on a submitted job.
>> This can optimize the power performance when the
>> workload is on.
>>
>> 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/Makefile           |   2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   2 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 156 ++++++++++++++++++
>>   drivers/gpu/drm/amd/include/amdgpu_workload.h |  44 +++++
>>   5 files changed, 206 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..0ec18b8fe29f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -3672,6 +3672,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>>         INIT_WORK(&adev->xgmi_reset_work, 
>> amdgpu_device_xgmi_reset_func);
>>   +    amdgpu_smu_workload_init(adev);
>> +
>>       adev->gfx.gfx_off_req_count = 1;
>>       adev->gfx.gfx_off_residency = 0;
>>       adev->gfx.gfx_off_entrycount = 0;
>> 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..ce0339d75c12
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
>> @@ -0,0 +1,156 @@
>> +// 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"
>> +
>> +/* 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)
>> +{
>> +    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 void
>> +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 == 0) {
>
> Please double check the coding style, stuff like this is usually 
> complained about by the automated checkers.
>
I have run the checkpatch.pl and did not get any warning for this, But 
sure I will change this.

Thank you,

~Arvind

> Apart from that Alex needs to take a look if this here makes sense or 
> not.
>
> Regards,
> Christian.
>
>> +        /* Set the bit for the submitted workload profile */
>> +        adev->smu_workload.submit_workload_status |= (1 << profile);
>> + atomic_inc(&adev->smu_workload.power_profile_ref[profile]);
>> +    } else {
>> +        DRM_ERROR("Failed to set power profile, error %d\n", ret);
>> +    }
>> +
>> +}
>> +
>> +static void
>> +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 == 0) {
>> +         /* Clear the bit for the submitted workload profile */
>> +        adev->smu_workload.submit_workload_status &= ~(1 << profile);
>> +    } else
>> +        DRM_ERROR("Failed to clear power profile, error %d\n", ret);
>> +
>> +}
>> +
>> +static void amdgpu_smu_idle_work_handler(struct work_struct *work)
>> +{
>> +
>> +    struct amdgpu_smu_workload *wl = container_of(work,
>> +                              struct amdgpu_smu_workload,
>> +                              smu_delayed_work.work);
>> +    struct amdgpu_device *adev = wl->adev;
>> +    bool reschedule = false;
>> +
>> +    mutex_lock(&adev->smu_workload.workload_lock);
>> +    for (int index  = fls(adev->smu_workload.submit_workload_status);
>> +         index >= 0; index--) {
>> +        if 
>> (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
>> +            adev->smu_workload.submit_workload_status & (1 << index)) {
>> +            amdgpu_power_profile_clear(adev, index);
>> +        } else if 
>> (atomic_read(&adev->smu_workload.power_profile_ref[index]))
>> +            reschedule = true;
>> +    }
>> +
>> +    if (reschedule)
>> + schedule_delayed_work(&adev->smu_workload.smu_delayed_work,
>> +                      SMU_IDLE_TIMEOUT);
>> +
>> +    mutex_unlock(&adev->smu_workload.workload_lock);
>> +}
>> +
>> +void amdgpu_put_workload_profile(struct amdgpu_device *adev,
>> +                 uint32_t ring_type)
>> +{
>> +
>> +    enum PP_SMC_POWER_PROFILE profile = 
>> ring_to_power_profile(ring_type);
>> +
>> +    if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
>> +        return;
>> +
>> +    mutex_lock(&adev->smu_workload.workload_lock);
>> + atomic_dec(&adev->smu_workload.power_profile_ref[profile]);
>> + schedule_delayed_work(&adev->smu_workload.smu_delayed_work, 
>> SMU_IDLE_TIMEOUT);
>> +    mutex_unlock(&adev->smu_workload.workload_lock);
>> +}
>> +
>> +void amdgpu_set_workload_profile(struct amdgpu_device *adev,
>> +                 uint32_t ring_type)
>> +{
>> +    enum PP_SMC_POWER_PROFILE profile = 
>> ring_to_power_profile(ring_type);
>> +
>> +    if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
>> +        return;
>> +
>> +    mutex_lock(&adev->smu_workload.workload_lock);
>> + cancel_delayed_work_sync(&adev->smu_workload.smu_delayed_work);
>> +
>> +    amdgpu_power_profile_set(adev, profile);
>> +
>> +    /* Clear the already finished jobs of higher power profile*/
>> +    for (int index = fls(adev->smu_workload.submit_workload_status);
>> +         index > profile; index--) {
>> +        if 
>> (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
>> +            adev->smu_workload.submit_workload_status & (1 << index)) {
>> +            amdgpu_power_profile_clear(adev, index);
>> +        }
>> +    }
>> +
>> +    mutex_unlock(&adev->smu_workload.workload_lock);
>> +}
>> +
>> +void amdgpu_smu_workload_init(struct amdgpu_device *adev)
>> +{
>> +    struct amdgpu_smu_workload wl;
>> +
>> +    wl.adev = adev;
>> +    wl.submit_workload_status = 0;
>> +    adev->smu_workload = wl;
>> +
>> +    mutex_init(&adev->smu_workload.workload_lock);
>> + INIT_DELAYED_WORK(&adev->smu_workload.smu_delayed_work, 
>> amdgpu_smu_idle_work_handler);
>> +}
>> 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..09804c3d2869
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/include/amdgpu_workload.h
>> @@ -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.
>> + *
>> + */
>> +
>> +#ifndef _AMDGPU_WORKLOAD_H_
>> +#define _AMDGPU_WORKLOAD_H_
>> +
>> +struct amdgpu_smu_workload {
>> +    struct amdgpu_device    *adev;
>> +    struct mutex        workload_lock;
>> +    struct delayed_work    smu_delayed_work;
>> +    uint32_t        submit_workload_status;
>> +    atomic_t power_profile_ref[PP_SMC_POWER_PROFILE_COUNT];
>> +};
>> +
>> +void amdgpu_set_workload_profile(struct amdgpu_device *adev,
>> +                 uint32_t ring_type);
>> +
>> +void amdgpu_put_workload_profile(struct amdgpu_device *adev,
>> +                 uint32_t ring_type);
>> +
>> +void amdgpu_smu_workload_init(struct amdgpu_device *adev);
>> +
>> +#endif
>

Re: [PATCH 1/3] drm/amdgpu: Add new api to switch on/off power profile mode

[Alex Deucher]

On Mon, Aug 14, 2023 at 3:35 AM Arvind Yadav <Arvind.Yadav@amd.com> wrote:
>
> This patch adds a function which will allow to
> change the GPU power profile based on a submitted job.
> This can optimize the power performance when the
> workload is on.

A few minor comments inline below.  One thing to double check is that
we properly cancel this work before a suspend or driver unload.  We
need to make sure this is taken care of before we take down the SMU.

Alex

>
> 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/Makefile           |   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   2 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 156 ++++++++++++++++++
>  drivers/gpu/drm/amd/include/amdgpu_workload.h |  44 +++++
>  5 files changed, 206 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..0ec18b8fe29f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3672,6 +3672,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>
>         INIT_WORK(&adev->xgmi_reset_work, amdgpu_device_xgmi_reset_func);
>
> +       amdgpu_smu_workload_init(adev);
> +
>         adev->gfx.gfx_off_req_count = 1;
>         adev->gfx.gfx_off_residency = 0;
>         adev->gfx.gfx_off_entrycount = 0;
> 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..ce0339d75c12
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
> @@ -0,0 +1,156 @@
> +// 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"
> +
> +/* 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)
> +{
> +       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 void
> +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 == 0) {
> +               /* Set the bit for the submitted workload profile */
> +               adev->smu_workload.submit_workload_status |= (1 << profile);
> +               atomic_inc(&adev->smu_workload.power_profile_ref[profile]);
> +       } else {
> +               DRM_ERROR("Failed to set power profile, error %d\n", ret);
> +       }
> +
> +}
> +
> +static void
> +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 == 0) {
> +                /* Clear the bit for the submitted workload profile */
> +               adev->smu_workload.submit_workload_status &= ~(1 << profile);
> +       } else
> +               DRM_ERROR("Failed to clear power profile, error %d\n", ret);
> +
> +}
> +
> +static void amdgpu_smu_idle_work_handler(struct work_struct *work)

How about amdgpu_power_profile_idle_work_handler() for consistency?

> +{
> +
> +       struct amdgpu_smu_workload *wl = container_of(work,
> +                                                     struct amdgpu_smu_workload,
> +                                                     smu_delayed_work.work);
> +       struct amdgpu_device *adev = wl->adev;
> +       bool reschedule = false;
> +
> +       mutex_lock(&adev->smu_workload.workload_lock);
> +       for (int index  = fls(adev->smu_workload.submit_workload_status);
> +            index >= 0; index--) {
> +               if (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
> +                   adev->smu_workload.submit_workload_status & (1 << index)) {
> +                       amdgpu_power_profile_clear(adev, index);
> +               } else if (atomic_read(&adev->smu_workload.power_profile_ref[index]))
> +                       reschedule = true;
> +       }
> +
> +       if (reschedule)
> +               schedule_delayed_work(&adev->smu_workload.smu_delayed_work,
> +                                     SMU_IDLE_TIMEOUT);
> +
> +       mutex_unlock(&adev->smu_workload.workload_lock);
> +}
> +
> +void amdgpu_put_workload_profile(struct amdgpu_device *adev,

amdgpu_workload_profile_put() for consistency.

> +                                uint32_t ring_type)
> +{
> +
> +       enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
> +
> +       if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
> +               return;
> +
> +       mutex_lock(&adev->smu_workload.workload_lock);
> +       atomic_dec(&adev->smu_workload.power_profile_ref[profile]);
> +       schedule_delayed_work(&adev->smu_workload.smu_delayed_work, SMU_IDLE_TIMEOUT);
> +       mutex_unlock(&adev->smu_workload.workload_lock);
> +}
> +
> +void amdgpu_set_workload_profile(struct amdgpu_device *adev,
> +                                uint32_t ring_type)

amdgpu_workload_profile_set() for consistency.

> +{
> +       enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
> +
> +       if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
> +               return;
> +
> +       mutex_lock(&adev->smu_workload.workload_lock);
> +       cancel_delayed_work_sync(&adev->smu_workload.smu_delayed_work);
> +
> +       amdgpu_power_profile_set(adev, profile);
> +
> +       /* Clear the already finished jobs of higher power profile*/
> +       for (int index = fls(adev->smu_workload.submit_workload_status);
> +            index > profile; index--) {
> +               if (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
> +                   adev->smu_workload.submit_workload_status & (1 << index)) {
> +                       amdgpu_power_profile_clear(adev, index);
> +               }
> +       }
> +
> +       mutex_unlock(&adev->smu_workload.workload_lock);
> +}
> +
> +void amdgpu_smu_workload_init(struct amdgpu_device *adev)

amdgpu_workload_profile_init() for consistency.

> +{
> +       struct amdgpu_smu_workload wl;
> +
> +       wl.adev = adev;
> +       wl.submit_workload_status = 0;
> +       adev->smu_workload = wl;
> +
> +       mutex_init(&adev->smu_workload.workload_lock);
> +       INIT_DELAYED_WORK(&adev->smu_workload.smu_delayed_work, amdgpu_smu_idle_work_handler);
> +}
> 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..09804c3d2869
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/include/amdgpu_workload.h
> @@ -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.
> + *
> + */
> +
> +#ifndef _AMDGPU_WORKLOAD_H_
> +#define _AMDGPU_WORKLOAD_H_
> +
> +struct amdgpu_smu_workload {
> +       struct amdgpu_device    *adev;
> +       struct mutex            workload_lock;
> +       struct delayed_work     smu_delayed_work;
> +       uint32_t                submit_workload_status;
> +       atomic_t                power_profile_ref[PP_SMC_POWER_PROFILE_COUNT];
> +};
> +
> +void amdgpu_set_workload_profile(struct amdgpu_device *adev,
> +                                uint32_t ring_type);
> +
> +void amdgpu_put_workload_profile(struct amdgpu_device *adev,
> +                                uint32_t ring_type);
> +
> +void amdgpu_smu_workload_init(struct amdgpu_device *adev);
> +
> +#endif
> --
> 2.34.1
>

Re: [PATCH 1/3] drm/amdgpu: Add new api to switch on/off power profile mode

[Shashank Sharma]

Hey Arvind,

On 14/08/2023 09:34, Arvind Yadav wrote:
> This patch adds a function which will allow to
> change the GPU power profile based on a submitted job.
> This can optimize the power performance when the
> workload is on.
>
> 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/Makefile           |   2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   2 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 156 ++++++++++++++++++
>   drivers/gpu/drm/amd/include/amdgpu_workload.h |  44 +++++
>   5 files changed, 206 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..0ec18b8fe29f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3672,6 +3672,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   
>   	INIT_WORK(&adev->xgmi_reset_work, amdgpu_device_xgmi_reset_func);
>   
> +	amdgpu_smu_workload_init(adev);
> +
>   	adev->gfx.gfx_off_req_count = 1;
>   	adev->gfx.gfx_off_residency = 0;
>   	adev->gfx.gfx_off_entrycount = 0;
> 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..ce0339d75c12
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
> @@ -0,0 +1,156 @@
> +// 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"
> +
> +/* 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)
> +{
> +	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 void
> +amdgpu_power_profile_set(struct amdgpu_device *adev,
> +			 enum PP_SMC_POWER_PROFILE profile)
This function expects the caller to hold the smu_workload_mutex, may be 
we should document it.
> +{
> +	int ret = amdgpu_dpm_switch_power_profile(adev, profile, true);
I think we should pass this return value to caller instead of keeping 
the function void.
> +
> +	if (ret == 0) {
> +		/* Set the bit for the submitted workload profile */
> +		adev->smu_workload.submit_workload_status |= (1 << profile);
> +		atomic_inc(&adev->smu_workload.power_profile_ref[profile]);
> +	} else {
> +		DRM_ERROR("Failed to set power profile, error %d\n", ret);
This can be a warning instead of error.
> +	}
> +
> +}
> +
> +static void
> +amdgpu_power_profile_clear(struct amdgpu_device *adev,
> +			   enum PP_SMC_POWER_PROFILE profile)
> +{
> +	int ret = amdgpu_dpm_switch_power_profile(adev, profile, false);
same for return value here as well.
> +
> +	if (ret == 0) {
> +		 /* Clear the bit for the submitted workload profile */
> +		adev->smu_workload.submit_workload_status &= ~(1 << profile);
> +	} else
> +		DRM_ERROR("Failed to clear power profile, error %d\n", ret);
> +
> +}
> +
> +static void amdgpu_smu_idle_work_handler(struct work_struct *work)
> +{
> +
> +	struct amdgpu_smu_workload *wl = container_of(work,
> +						      struct amdgpu_smu_workload,
> +						      smu_delayed_work.work);
> +	struct amdgpu_device *adev = wl->adev;
> +	bool reschedule = false;
> +
> +	mutex_lock(&adev->smu_workload.workload_lock);
> +	for (int index  = fls(adev->smu_workload.submit_workload_status);
This can be kept outside the for() for better readability and alignment.
> +	     index >= 0; index--) {
> +		if (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
> +		    adev->smu_workload.submit_workload_status & (1 << index)) {
> +			amdgpu_power_profile_clear(adev, index);
> +		} else if (atomic_read(&adev->smu_workload.power_profile_ref[index]))
> +			reschedule = true;
> +	}
> +

This block can be re-arranged a bit for better readability, pls consider:

for () {

     atomic_t val = 
atomic_read(&adev->smu_workload.power_profile_ref[index];

     if (val) {

         reschedule = true;

         break;

     } else {

         if (adev->smu_workload.submit_workload_status & (1 << index))

             amdgpu_power_profile_clear(adev, index);

     }

}

> +	if (reschedule)
> +		schedule_delayed_work(&adev->smu_workload.smu_delayed_work,
> +				      SMU_IDLE_TIMEOUT);
pls check the return value of work
> +
> +	mutex_unlock(&adev->smu_workload.workload_lock);
> +}
> +
> +void amdgpu_put_workload_profile(struct amdgpu_device *adev,
> +				 uint32_t ring_type)
> +{
> +
> +	enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
> +
> +	if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
> +		return;
> +
> +	mutex_lock(&adev->smu_workload.workload_lock);
> +	atomic_dec(&adev->smu_workload.power_profile_ref[profile]);
> +	schedule_delayed_work(&adev->smu_workload.smu_delayed_work, SMU_IDLE_TIMEOUT);
> +	mutex_unlock(&adev->smu_workload.workload_lock);
> +}
> +
> +void amdgpu_set_workload_profile(struct amdgpu_device *adev,
> +				 uint32_t ring_type)
I would prefer if you can split this patch into two, one just to set 
profile, other to clear profile and schedule work.
> +{
> +	enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
> +
> +	if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
> +		return;
> +
> +	mutex_lock(&adev->smu_workload.workload_lock);
> +	cancel_delayed_work_sync(&adev->smu_workload.smu_delayed_work);
Please check the return value here and proceed only when we were able to 
cancel successfully.
> +
> +	amdgpu_power_profile_set(adev, profile);
> +
> +	/* Clear the already finished jobs of higher power profile*/
> +	for (int index = fls(adev->smu_workload.submit_workload_status);
> +	     index > profile; index--) {
> +		if (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
> +		    adev->smu_workload.submit_workload_status & (1 << index)) {
> +			amdgpu_power_profile_clear(adev, index);
> +		}
> +	}
> +
> +	mutex_unlock(&adev->smu_workload.workload_lock);
> +}
> +
> +void amdgpu_smu_workload_init(struct amdgpu_device *adev)
> +{
> +	struct amdgpu_smu_workload wl;
> +
> +	wl.adev = adev;
> +	wl.submit_workload_status = 0;
> +	adev->smu_workload = wl;

Why do we need variable wl at all, which is a local variable of the 
stack ? You can just do:

adev->smu_workload.adev = adev;
adev->smu_workload.submit_workload_status = 0;

> +
> +	mutex_init(&adev->smu_workload.workload_lock);
> +	INIT_DELAYED_WORK(&adev->smu_workload.smu_delayed_work, amdgpu_smu_idle_work_handler);

Are we missing the respective amdgpu_smu_workload_fini which will 
destroy the mutex ?

- Shashank

> +}
> 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..09804c3d2869
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/include/amdgpu_workload.h
> @@ -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.
> + *
> + */
> +
> +#ifndef _AMDGPU_WORKLOAD_H_
> +#define _AMDGPU_WORKLOAD_H_
> +
> +struct amdgpu_smu_workload {
> +	struct amdgpu_device	*adev;
> +	struct mutex		workload_lock;
> +	struct delayed_work	smu_delayed_work;
> +	uint32_t		submit_workload_status;
> +	atomic_t		power_profile_ref[PP_SMC_POWER_PROFILE_COUNT];
> +};
> +
> +void amdgpu_set_workload_profile(struct amdgpu_device *adev,
> +				 uint32_t ring_type);
> +
> +void amdgpu_put_workload_profile(struct amdgpu_device *adev,
> +				 uint32_t ring_type);
> +
> +void amdgpu_smu_workload_init(struct amdgpu_device *adev);
> +
> +#endif

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

[Alex Deucher]

KFD also changes the profile when queues are active.  Please make sure
that is properly taken into account as well.

Alex

On Mon, Aug 14, 2023 at 3:36 AM Arvind Yadav <Arvind.Yadav@amd.com> wrote:
>
> This reverts commit 5ce71f59bb9bd3d8a09b96afdbc92975cb6dc303.
>
> Reason for revert: New amdgpu_smu* 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: 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_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 3/3] Revert "drm/amd/amdgpu: switch on/off vcn power profile mode"

[Shashank Sharma]

Ah, Thanks for pointing that out Alex.

@Arvind, please refer to the patch 
(https://patchwork.freedesktop.org/patch/504854/?series=109060&rev=4) in 
previous series of SMU workload hints with UAPI (here: 
https://patchwork.freedesktop.org/series/109060/)

Regards

Shashank

On 14/08/2023 17:20, Alex Deucher wrote:
> KFD also changes the profile when queues are active.  Please make sure
> that is properly taken into account as well.
>
> Alex
>
> On Mon, Aug 14, 2023 at 3:36 AM Arvind Yadav <Arvind.Yadav@amd.com> wrote:
>> This reverts commit 5ce71f59bb9bd3d8a09b96afdbc92975cb6dc303.
>>
>> Reason for revert: New amdgpu_smu* 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: 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_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 1/3] drm/amdgpu: Add new api to switch on/off power profile mode

[Yadav, Arvind]

On 8/14/2023 8:03 PM, Alex Deucher wrote:
> On Mon, Aug 14, 2023 at 3:35 AM Arvind Yadav <Arvind.Yadav@amd.com> wrote:
>> This patch adds a function which will allow to
>> change the GPU power profile based on a submitted job.
>> This can optimize the power performance when the
>> workload is on.
> A few minor comments inline below.  One thing to double check is that
> we properly cancel this work before a suspend or driver unload.  We
> need to make sure this is taken care of before we take down the SMU.
>
> Alex

Noted,

>> 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/Makefile           |   2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   2 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 156 ++++++++++++++++++
>>   drivers/gpu/drm/amd/include/amdgpu_workload.h |  44 +++++
>>   5 files changed, 206 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..0ec18b8fe29f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -3672,6 +3672,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>>
>>          INIT_WORK(&adev->xgmi_reset_work, amdgpu_device_xgmi_reset_func);
>>
>> +       amdgpu_smu_workload_init(adev);
>> +
>>          adev->gfx.gfx_off_req_count = 1;
>>          adev->gfx.gfx_off_residency = 0;
>>          adev->gfx.gfx_off_entrycount = 0;
>> 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..ce0339d75c12
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
>> @@ -0,0 +1,156 @@
>> +// 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"
>> +
>> +/* 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)
>> +{
>> +       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 void
>> +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 == 0) {
>> +               /* Set the bit for the submitted workload profile */
>> +               adev->smu_workload.submit_workload_status |= (1 << profile);
>> +               atomic_inc(&adev->smu_workload.power_profile_ref[profile]);
>> +       } else {
>> +               DRM_ERROR("Failed to set power profile, error %d\n", ret);
>> +       }
>> +
>> +}
>> +
>> +static void
>> +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 == 0) {
>> +                /* Clear the bit for the submitted workload profile */
>> +               adev->smu_workload.submit_workload_status &= ~(1 << profile);
>> +       } else
>> +               DRM_ERROR("Failed to clear power profile, error %d\n", ret);
>> +
>> +}
>> +
>> +static void amdgpu_smu_idle_work_handler(struct work_struct *work)
> How about amdgpu_power_profile_idle_work_handler() for consistency?
>
>> +{
>> +
>> +       struct amdgpu_smu_workload *wl = container_of(work,
>> +                                                     struct amdgpu_smu_workload,
>> +                                                     smu_delayed_work.work);
>> +       struct amdgpu_device *adev = wl->adev;
>> +       bool reschedule = false;
>> +
>> +       mutex_lock(&adev->smu_workload.workload_lock);
>> +       for (int index  = fls(adev->smu_workload.submit_workload_status);
>> +            index >= 0; index--) {
>> +               if (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
>> +                   adev->smu_workload.submit_workload_status & (1 << index)) {
>> +                       amdgpu_power_profile_clear(adev, index);
>> +               } else if (atomic_read(&adev->smu_workload.power_profile_ref[index]))
>> +                       reschedule = true;
>> +       }
>> +
>> +       if (reschedule)
>> +               schedule_delayed_work(&adev->smu_workload.smu_delayed_work,
>> +                                     SMU_IDLE_TIMEOUT);
>> +
>> +       mutex_unlock(&adev->smu_workload.workload_lock);
>> +}
>> +
>> +void amdgpu_put_workload_profile(struct amdgpu_device *adev,
> amdgpu_workload_profile_put() for consistency.
>
>> +                                uint32_t ring_type)
>> +{
>> +
>> +       enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
>> +
>> +       if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
>> +               return;
>> +
>> +       mutex_lock(&adev->smu_workload.workload_lock);
>> +       atomic_dec(&adev->smu_workload.power_profile_ref[profile]);
>> +       schedule_delayed_work(&adev->smu_workload.smu_delayed_work, SMU_IDLE_TIMEOUT);
>> +       mutex_unlock(&adev->smu_workload.workload_lock);
>> +}
>> +
>> +void amdgpu_set_workload_profile(struct amdgpu_device *adev,
>> +                                uint32_t ring_type)
> amdgpu_workload_profile_set() for consistency.
Noted, I will change the function name.
>> +{
>> +       enum PP_SMC_POWER_PROFILE profile = ring_to_power_profile(ring_type);
>> +
>> +       if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
>> +               return;
>> +
>> +       mutex_lock(&adev->smu_workload.workload_lock);
>> +       cancel_delayed_work_sync(&adev->smu_workload.smu_delayed_work);
>> +
>> +       amdgpu_power_profile_set(adev, profile);
>> +
>> +       /* Clear the already finished jobs of higher power profile*/
>> +       for (int index = fls(adev->smu_workload.submit_workload_status);
>> +            index > profile; index--) {
>> +               if (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
>> +                   adev->smu_workload.submit_workload_status & (1 << index)) {
>> +                       amdgpu_power_profile_clear(adev, index);
>> +               }
>> +       }
>> +
>> +       mutex_unlock(&adev->smu_workload.workload_lock);
>> +}
>> +
>> +void amdgpu_smu_workload_init(struct amdgpu_device *adev)
> amdgpu_workload_profile_init() for consistency.

Noted,

~Arvind

>> +{
>> +       struct amdgpu_smu_workload wl;
>> +
>> +       wl.adev = adev;
>> +       wl.submit_workload_status = 0;
>> +       adev->smu_workload = wl;
>> +
>> +       mutex_init(&adev->smu_workload.workload_lock);
>> +       INIT_DELAYED_WORK(&adev->smu_workload.smu_delayed_work, amdgpu_smu_idle_work_handler);
>> +}
>> 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..09804c3d2869
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/include/amdgpu_workload.h
>> @@ -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.
>> + *
>> + */
>> +
>> +#ifndef _AMDGPU_WORKLOAD_H_
>> +#define _AMDGPU_WORKLOAD_H_
>> +
>> +struct amdgpu_smu_workload {
>> +       struct amdgpu_device    *adev;
>> +       struct mutex            workload_lock;
>> +       struct delayed_work     smu_delayed_work;
>> +       uint32_t                submit_workload_status;
>> +       atomic_t                power_profile_ref[PP_SMC_POWER_PROFILE_COUNT];
>> +};
>> +
>> +void amdgpu_set_workload_profile(struct amdgpu_device *adev,
>> +                                uint32_t ring_type);
>> +
>> +void amdgpu_put_workload_profile(struct amdgpu_device *adev,
>> +                                uint32_t ring_type);
>> +
>> +void amdgpu_smu_workload_init(struct amdgpu_device *adev);
>> +
>> +#endif
>> --
>> 2.34.1
>>

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

[Yadav, Arvind]

On 8/14/2023 9:35 PM, Shashank Sharma wrote:
> Ah, Thanks for pointing that out Alex.
>
> @Arvind, please refer to the patch 
> (https://patchwork.freedesktop.org/patch/504854/?series=109060&rev=4) 
> in previous series of SMU workload hints with UAPI (here: 
> https://patchwork.freedesktop.org/series/109060/)
>
Thank you Shashank and Alex. I will handle the KFD.

Regards,

Arvind

> Regards
>
> Shashank
>
> On 14/08/2023 17:20, Alex Deucher wrote:
>> KFD also changes the profile when queues are active.  Please make sure
>> that is properly taken into account as well.
>>
>> Alex
>>
>> On Mon, Aug 14, 2023 at 3:36 AM Arvind Yadav <Arvind.Yadav@amd.com> 
>> wrote:
>>> This reverts commit 5ce71f59bb9bd3d8a09b96afdbc92975cb6dc303.
>>>
>>> Reason for revert: New amdgpu_smu* 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: 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_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 1/3] drm/amdgpu: Add new api to switch on/off power profile mode

[Yadav, Arvind]

On 8/14/2023 8:28 PM, Shashank Sharma wrote:
> Hey Arvind,
>
> On 14/08/2023 09:34, Arvind Yadav wrote:
>> This patch adds a function which will allow to
>> change the GPU power profile based on a submitted job.
>> This can optimize the power performance when the
>> workload is on.
>>
>> 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/Makefile           |   2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   2 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c  | 156 ++++++++++++++++++
>>   drivers/gpu/drm/amd/include/amdgpu_workload.h |  44 +++++
>>   5 files changed, 206 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..0ec18b8fe29f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -3672,6 +3672,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>>         INIT_WORK(&adev->xgmi_reset_work, 
>> amdgpu_device_xgmi_reset_func);
>>   +    amdgpu_smu_workload_init(adev);
>> +
>>       adev->gfx.gfx_off_req_count = 1;
>>       adev->gfx.gfx_off_residency = 0;
>>       adev->gfx.gfx_off_entrycount = 0;
>> 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..ce0339d75c12
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_workload.c
>> @@ -0,0 +1,156 @@
>> +// 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"
>> +
>> +/* 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)
>> +{
>> +    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 void
>> +amdgpu_power_profile_set(struct amdgpu_device *adev,
>> +             enum PP_SMC_POWER_PROFILE profile)
> This function expects the caller to hold the smu_workload_mutex, may 
> be we should document it.
>> +{
>> +    int ret = amdgpu_dpm_switch_power_profile(adev, profile, true);
> I think we should pass this return value to caller instead of keeping 
> the function void.
Noted,
>> +
>> +    if (ret == 0) {
>> +        /* Set the bit for the submitted workload profile */
>> +        adev->smu_workload.submit_workload_status |= (1 << profile);
>> + atomic_inc(&adev->smu_workload.power_profile_ref[profile]);
>> +    } else {
>> +        DRM_ERROR("Failed to set power profile, error %d\n", ret);
> This can be a warning instead of error.
Noted,
>> +    }
>> +
>> +}
>> +
>> +static void
>> +amdgpu_power_profile_clear(struct amdgpu_device *adev,
>> +               enum PP_SMC_POWER_PROFILE profile)
>> +{
>> +    int ret = amdgpu_dpm_switch_power_profile(adev, profile, false);
> same for return value here as well.
Noted,
>
>> +
>> +    if (ret == 0) {
>> +         /* Clear the bit for the submitted workload profile */
>> +        adev->smu_workload.submit_workload_status &= ~(1 << profile);
>> +    } else
>> +        DRM_ERROR("Failed to clear power profile, error %d\n", ret);
>> +
>> +}
>> +
>> +static void amdgpu_smu_idle_work_handler(struct work_struct *work)
>> +{
>> +
>> +    struct amdgpu_smu_workload *wl = container_of(work,
>> +                              struct amdgpu_smu_workload,
>> +                              smu_delayed_work.work);
>> +    struct amdgpu_device *adev = wl->adev;
>> +    bool reschedule = false;
>> +
>> +    mutex_lock(&adev->smu_workload.workload_lock);
>> +    for (int index  = fls(adev->smu_workload.submit_workload_status);
> This can be kept outside the for() for better readability and alignment.
Noted,
>> +         index >= 0; index--) {
>> +        if 
>> (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
>> +            adev->smu_workload.submit_workload_status & (1 << index)) {
>> +            amdgpu_power_profile_clear(adev, index);
>> +        } else if 
>> (atomic_read(&adev->smu_workload.power_profile_ref[index]))
>> +            reschedule = true;
>> +    }
>> +
>
> This block can be re-arranged a bit for better readability, pls consider:
>
> for () {
>
>     atomic_t val = 
> atomic_read(&adev->smu_workload.power_profile_ref[index];
>
>     if (val) {
>
>         reschedule = true;
>
>         break;
>
>     } else {
>
>         if (adev->smu_workload.submit_workload_status & (1 << index))
>
>             amdgpu_power_profile_clear(adev, index);
>
>     }
>
> }
>
Noted,
>> +    if (reschedule)
>> + schedule_delayed_work(&adev->smu_workload.smu_delayed_work,
>> +                      SMU_IDLE_TIMEOUT);
> pls check the return value of work
Noted,
>> +
>> +    mutex_unlock(&adev->smu_workload.workload_lock);
>> +}
>> +
>> +void amdgpu_put_workload_profile(struct amdgpu_device *adev,
>> +                 uint32_t ring_type)
>> +{
>> +
>> +    enum PP_SMC_POWER_PROFILE profile = 
>> ring_to_power_profile(ring_type);
>> +
>> +    if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
>> +        return;
>> +
>> +    mutex_lock(&adev->smu_workload.workload_lock);
>> + atomic_dec(&adev->smu_workload.power_profile_ref[profile]);
>> + schedule_delayed_work(&adev->smu_workload.smu_delayed_work, 
>> SMU_IDLE_TIMEOUT);
>> +    mutex_unlock(&adev->smu_workload.workload_lock);
>> +}
>> +
>> +void amdgpu_set_workload_profile(struct amdgpu_device *adev,
>> +                 uint32_t ring_type)
> I would prefer if you can split this patch into two, one just to set 
> profile, other to clear profile and schedule work.
Noted,
>> +{
>> +    enum PP_SMC_POWER_PROFILE profile = 
>> ring_to_power_profile(ring_type);
>> +
>> +    if (profile == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
>> +        return;
>> +
>> +    mutex_lock(&adev->smu_workload.workload_lock);
>> + cancel_delayed_work_sync(&adev->smu_workload.smu_delayed_work);
> Please check the return value here and proceed only when we were able 
> to cancel successfully.
Noted,
>> +
>> +    amdgpu_power_profile_set(adev, profile);
>> +
>> +    /* Clear the already finished jobs of higher power profile*/
>> +    for (int index = fls(adev->smu_workload.submit_workload_status);
>> +         index > profile; index--) {
>> +        if 
>> (!atomic_read(&adev->smu_workload.power_profile_ref[index]) &&
>> +            adev->smu_workload.submit_workload_status & (1 << index)) {
>> +            amdgpu_power_profile_clear(adev, index);
>> +        }
>> +    }
>> +
>> +    mutex_unlock(&adev->smu_workload.workload_lock);
>> +}
>> +
>> +void amdgpu_smu_workload_init(struct amdgpu_device *adev)
>> +{
>> +    struct amdgpu_smu_workload wl;
>> +
>> +    wl.adev = adev;
>> +    wl.submit_workload_status = 0;
>> +    adev->smu_workload = wl;
>
> Why do we need variable wl at all, which is a local variable of the 
> stack ? You can just do:
Noted,
>
> adev->smu_workload.adev = adev;
> adev->smu_workload.submit_workload_status = 0;
>
>> +
>> +    mutex_init(&adev->smu_workload.workload_lock);
>> + INIT_DELAYED_WORK(&adev->smu_workload.smu_delayed_work, 
>> amdgpu_smu_idle_work_handler);
>
> Are we missing the respective amdgpu_smu_workload_fini which will 
> destroy the mutex ?
>
Noted,

Regards,

~Arvind

> - Shashank
>
>> +}
>> 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..09804c3d2869
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/include/amdgpu_workload.h
>> @@ -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.
>> + *
>> + */
>> +
>> +#ifndef _AMDGPU_WORKLOAD_H_
>> +#define _AMDGPU_WORKLOAD_H_
>> +
>> +struct amdgpu_smu_workload {
>> +    struct amdgpu_device    *adev;
>> +    struct mutex        workload_lock;
>> +    struct delayed_work    smu_delayed_work;
>> +    uint32_t        submit_workload_status;
>> +    atomic_t power_profile_ref[PP_SMC_POWER_PROFILE_COUNT];
>> +};
>> +
>> +void amdgpu_set_workload_profile(struct amdgpu_device *adev,
>> +                 uint32_t ring_type);
>> +
>> +void amdgpu_put_workload_profile(struct amdgpu_device *adev,
>> +                 uint32_t ring_type);
>> +
>> +void amdgpu_smu_workload_init(struct amdgpu_device *adev);
>> +
>> +#endif