[PATCH v2 0/5] GPU workload hints for better performance

[PATCH v2 0/5] GPU workload hints for better performance

[Shashank Sharma]

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 workload type hints. A user can set a hint of workload
type being submistted to GPU, and the driver can dynamically switch
the power profiles which is best suited to this kind of workload. 

Currently supported workload profiles are:
"None", "3D", "Video", "VR", "Compute"

V2: This version addresses the review comment from Christian about
chaning the design to set workload mode in a more dynamic method
than during the context creation.

Shashank Sharma (5):
  drm/amdgpu: add UAPI for workload hints to ctx ioctl
  drm/amdgpu: add new functions to set GPU power profile
  drm/amdgpu: set GPU workload via ctx IOCTL
  drm/amdgpu: switch GPU workload profile
  drm/amdgpu: switch workload context to/from compute

 drivers/gpu/drm/amd/amdgpu/Makefile           |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c    | 14 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c        |  2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c       | 56 ++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h       |  1 +
 .../gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c  | 93 +++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       | 15 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h       |  3 +
 .../gpu/drm/amd/include/amdgpu_ctx_workload.h | 54 +++++++++++
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h       |  5 +
 include/uapi/drm/amdgpu_drm.h                 | 18 ++++
 12 files changed, 258 insertions(+), 6 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
 create mode 100644 drivers/gpu/drm/amd/include/amdgpu_ctx_workload.h

-- 
2.34.1

[PATCH v2 1/5] drm/amdgpu: add UAPI for workload hints to ctx ioctl

[Shashank Sharma]

Allow the user to specify a workload hint to the kernel.
We can use these to tweak the dpm heuristics to better match
the workload for improved performance.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
---
 include/uapi/drm/amdgpu_drm.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index c2c9c674a223..da5019a6e233 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -212,6 +212,8 @@ union drm_amdgpu_bo_list {
 #define AMDGPU_CTX_OP_QUERY_STATE2	4
 #define AMDGPU_CTX_OP_GET_STABLE_PSTATE	5
 #define AMDGPU_CTX_OP_SET_STABLE_PSTATE	6
+#define AMDGPU_CTX_OP_GET_WORKLOAD_PROFILE	7
+#define AMDGPU_CTX_OP_SET_WORKLOAD_PROFILE	8
 
 /* GPU reset status */
 #define AMDGPU_CTX_NO_RESET		0
@@ -252,6 +254,17 @@ union drm_amdgpu_bo_list {
 #define AMDGPU_CTX_STABLE_PSTATE_MIN_MCLK  3
 #define AMDGPU_CTX_STABLE_PSTATE_PEAK  4
 
+/* GPU workload hints, flag bits 8-15 */
+#define AMDGPU_CTX_WORKLOAD_HINT_SHIFT     8
+#define AMDGPU_CTX_WORKLOAD_HINT_MASK      (0xff << AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
+#define AMDGPU_CTX_WORKLOAD_HINT_NONE      (0 << AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
+#define AMDGPU_CTX_WORKLOAD_HINT_3D        (1 << AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
+#define AMDGPU_CTX_WORKLOAD_HINT_VIDEO     (2 << AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
+#define AMDGPU_CTX_WORKLOAD_HINT_VR        (3 << AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
+#define AMDGPU_CTX_WORKLOAD_HINT_COMPUTE   (4 << AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
+#define AMDGPU_CTX_WORKLOAD_HINT_MAX	   AMDGPU_CTX_WORKLOAD_HINT_COMPUTE
+#define AMDGPU_CTX_WORKLOAD_INDEX(n)	   (n >> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
+
 struct drm_amdgpu_ctx_in {
 	/** AMDGPU_CTX_OP_* */
 	__u32	op;
@@ -281,6 +294,11 @@ union drm_amdgpu_ctx_out {
 			__u32	flags;
 			__u32	_pad;
 		} pstate;
+
+		struct {
+			__u32	flags;
+			__u32	_pad;
+		} workload;
 };
 
 union drm_amdgpu_ctx {
-- 
2.34.1

[PATCH v2 2/5] drm/amdgpu: add new functions to set GPU power profile

[Shashank Sharma]

This patch adds new functions which will allow a user to
change the GPU power profile based a GPU workload hint
flag.

Cc: Alex Deucher <alexandar.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/Makefile           |  2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c  | 97 +++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |  1 +
 .../gpu/drm/amd/include/amdgpu_ctx_workload.h | 54 +++++++++++
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h       |  5 +
 5 files changed, 158 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
 create mode 100644 drivers/gpu/drm/amd/include/amdgpu_ctx_workload.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index 5a283d12f8e1..34679c657ecc 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -50,7 +50,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 	atombios_dp.o amdgpu_afmt.o amdgpu_trace_points.o \
 	atombios_encoders.o amdgpu_sa.o atombios_i2c.o \
 	amdgpu_dma_buf.o amdgpu_vm.o amdgpu_vm_pt.o amdgpu_ib.o amdgpu_pll.o \
-	amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \
+	amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_ctx_workload.o amdgpu_sync.o \
 	amdgpu_gtt_mgr.o amdgpu_preempt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o \
 	amdgpu_atomfirmware.o amdgpu_vf_error.o amdgpu_sched.o \
 	amdgpu_debugfs.o amdgpu_ids.o amdgpu_gmc.o \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
new file mode 100644
index 000000000000..a11cf29bc388
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
@@ -0,0 +1,97 @@
+/*
+ * Copyright 2022 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 <drm/drm.h>
+#include "kgd_pp_interface.h"
+#include "amdgpu_ctx_workload.h"
+
+static enum PP_SMC_POWER_PROFILE
+amdgpu_workload_to_power_profile(uint32_t hint)
+{
+	switch (hint) {
+	case AMDGPU_CTX_WORKLOAD_HINT_NONE:
+	default:
+		return PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
+
+	case AMDGPU_CTX_WORKLOAD_HINT_3D:
+		return PP_SMC_POWER_PROFILE_FULLSCREEN3D;
+	case AMDGPU_CTX_WORKLOAD_HINT_VIDEO:
+		return PP_SMC_POWER_PROFILE_VIDEO;
+	case AMDGPU_CTX_WORKLOAD_HINT_VR:
+		return PP_SMC_POWER_PROFILE_VR;
+	case AMDGPU_CTX_WORKLOAD_HINT_COMPUTE:
+		return PP_SMC_POWER_PROFILE_COMPUTE;
+	}
+}
+
+int amdgpu_set_workload_profile(struct amdgpu_device *adev,
+				uint32_t hint)
+{
+	int ret = 0;
+	enum PP_SMC_POWER_PROFILE profile =
+			amdgpu_workload_to_power_profile(hint);
+
+	if (adev->pm.workload_mode == hint)
+		return 0;
+
+	mutex_lock(&adev->pm.smu_workload_lock);
+
+	if (adev->pm.workload_mode == hint)
+		goto unlock;
+
+	ret = amdgpu_dpm_switch_power_profile(adev, profile, 1);
+	if (!ret)
+		adev->pm.workload_mode = hint;
+	atomic_inc(&adev->pm.workload_switch_ref);
+
+unlock:
+	mutex_unlock(&adev->pm.smu_workload_lock);
+	return ret;
+}
+
+int amdgpu_clear_workload_profile(struct amdgpu_device *adev,
+				  uint32_t hint)
+{
+	int ret = 0;
+	enum PP_SMC_POWER_PROFILE profile =
+			amdgpu_workload_to_power_profile(hint);
+
+	if (hint == AMDGPU_CTX_WORKLOAD_HINT_NONE)
+		return 0;
+
+	/* Do not reset GPU power profile if another reset is coming */
+	if (atomic_dec_return(&adev->pm.workload_switch_ref) > 0)
+		return 0;
+
+	mutex_lock(&adev->pm.smu_workload_lock);
+
+	if (adev->pm.workload_mode != hint)
+		goto unlock;
+
+	ret = amdgpu_dpm_switch_power_profile(adev, profile, 0);
+	if (!ret)
+		adev->pm.workload_mode = AMDGPU_CTX_WORKLOAD_HINT_NONE;
+
+unlock:
+	mutex_unlock(&adev->pm.smu_workload_lock);
+	return ret;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index be7aff2d4a57..1f0f64662c04 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3554,6 +3554,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	mutex_init(&adev->psp.mutex);
 	mutex_init(&adev->notifier_lock);
 	mutex_init(&adev->pm.stable_pstate_ctx_lock);
+	mutex_init(&adev->pm.smu_workload_lock);
 	mutex_init(&adev->benchmark_mutex);
 
 	amdgpu_device_init_apu_flags(adev);
diff --git a/drivers/gpu/drm/amd/include/amdgpu_ctx_workload.h b/drivers/gpu/drm/amd/include/amdgpu_ctx_workload.h
new file mode 100644
index 000000000000..6060fc53c3b0
--- /dev/null
+++ b/drivers/gpu/drm/amd/include/amdgpu_ctx_workload.h
@@ -0,0 +1,54 @@
+/*
+ * Copyright 2022 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_CTX_WL_H_
+#define _AMDGPU_CTX_WL_H_
+#include <drm/amdgpu_drm.h>
+#include "amdgpu.h"
+
+/* Workload mode names */
+static const char * const amdgpu_workload_mode_name[] = {
+	"None",
+	"3D",
+	"Video",
+	"VR",
+	"Compute",
+	"Unknown",
+};
+
+static inline const
+char *amdgpu_workload_profile_name(uint32_t profile)
+{
+	if (profile >= AMDGPU_CTX_WORKLOAD_HINT_NONE &&
+		profile < AMDGPU_CTX_WORKLOAD_HINT_MAX)
+		return amdgpu_workload_mode_name[AMDGPU_CTX_WORKLOAD_INDEX(profile)];
+
+	return amdgpu_workload_mode_name[AMDGPU_CTX_WORKLOAD_HINT_MAX];
+}
+
+int amdgpu_clear_workload_profile(struct amdgpu_device *adev,
+				uint32_t hint);
+
+int amdgpu_set_workload_profile(struct amdgpu_device *adev,
+				uint32_t hint);
+
+#endif
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
index 65624d091ed2..565131f789d0 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
@@ -361,6 +361,11 @@ struct amdgpu_pm {
 	struct mutex            stable_pstate_ctx_lock;
 	struct amdgpu_ctx       *stable_pstate_ctx;
 
+	/* SMU workload mode */
+	struct mutex smu_workload_lock;
+	uint32_t workload_mode;
+	atomic_t workload_switch_ref;
+
 	struct config_table_setting config_table;
 	/* runtime mode */
 	enum amdgpu_runpm_mode rpm_mode;
-- 
2.34.1

[PATCH v2 3/5] drm/amdgpu: set GPU workload via ctx IOCTL

[Shashank Sharma]

This patch adds new IOCTL flags in amdgpu_context_IOCTL to get
and set GPU workload profile. These calls will allow a user to
switch to a GPU power profile which might be better suitable to
its workload type. The currently supported workload types are:
    "None": Default workload profile
    "3D": Workload profile for 3D rendering work
    "Video": Workload profile for Media/Encode/Decode work
    "VR": Workload profile for VR rendering work
    "Compute": Workload profile for Compute work

The workload hint flag is saved in GPU context, and then its
applied when we actually run the job.

Cc: Alex Deucher <alexandar.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 56 ++++++++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h |  1 +
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 8ee4e8491f39..5a116e5bb6a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -27,6 +27,7 @@
 #include "amdgpu.h"
 #include "amdgpu_sched.h"
 #include "amdgpu_ras.h"
+#include "amdgpu_ctx_workload.h"
 #include <linux/nospec.h>
 
 #define to_amdgpu_ctx_entity(e)	\
@@ -328,7 +329,7 @@ static int amdgpu_ctx_init(struct amdgpu_ctx_mgr *mgr, int32_t priority,
 		return r;
 
 	ctx->stable_pstate = current_stable_pstate;
-
+	ctx->workload_mode = AMDGPU_CTX_WORKLOAD_HINT_NONE;
 	return 0;
 }
 
@@ -633,11 +634,41 @@ static int amdgpu_ctx_stable_pstate(struct amdgpu_device *adev,
 	return r;
 }
 
+static int amdgpu_ctx_workload_profile(struct amdgpu_device *adev,
+				       struct amdgpu_fpriv *fpriv, uint32_t id,
+				       bool set, u32 *profile)
+{
+	struct amdgpu_ctx *ctx;
+	struct amdgpu_ctx_mgr *mgr;
+	u32 workload_hint;
+
+	if (!fpriv)
+		return -EINVAL;
+
+	mgr = &fpriv->ctx_mgr;
+	mutex_lock(&mgr->lock);
+	ctx = idr_find(&mgr->ctx_handles, id);
+	if (!ctx) {
+		mutex_unlock(&mgr->lock);
+		return -EINVAL;
+	}
+
+	if (set) {
+		workload_hint = *profile;
+		ctx->workload_mode = workload_hint;
+	} else {
+		*profile = adev->pm.workload_mode;
+	}
+
+	mutex_unlock(&mgr->lock);
+	return 0;
+}
+
 int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
 		     struct drm_file *filp)
 {
 	int r;
-	uint32_t id, stable_pstate;
+	uint32_t id, stable_pstate, wl_hint;
 	int32_t priority;
 
 	union drm_amdgpu_ctx *args = data;
@@ -681,6 +712,27 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
 			return -EINVAL;
 		r = amdgpu_ctx_stable_pstate(adev, fpriv, id, true, &stable_pstate);
 		break;
+	case AMDGPU_CTX_OP_GET_WORKLOAD_PROFILE:
+		if (args->in.flags & ~AMDGPU_CTX_WORKLOAD_HINT_MASK)
+			return -EINVAL;
+		r = amdgpu_ctx_workload_profile(adev, fpriv, id, false, &wl_hint);
+		if (!r)
+			args->out.workload.flags = wl_hint;
+		break;
+	case AMDGPU_CTX_OP_SET_WORKLOAD_PROFILE:
+		if (args->in.flags & ~AMDGPU_CTX_WORKLOAD_HINT_MASK)
+			return -EINVAL;
+		wl_hint = args->in.flags & AMDGPU_CTX_WORKLOAD_HINT_MASK;
+		if (wl_hint > AMDGPU_CTX_WORKLOAD_HINT_MAX)
+			return -EINVAL;
+		r = amdgpu_ctx_workload_profile(adev, fpriv, id, true, &wl_hint);
+		if (r)
+			DRM_ERROR("Failed to set workload profile to %s\n",
+				amdgpu_workload_profile_name(wl_hint));
+		else
+			DRM_DEBUG_DRIVER("Workload profile set to %s\n",
+				amdgpu_workload_profile_name(wl_hint));
+		break;
 	default:
 		return -EINVAL;
 	}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
index cc7c8afff414..6c8032c3291a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
@@ -58,6 +58,7 @@ struct amdgpu_ctx {
 	unsigned long			ras_counter_ce;
 	unsigned long			ras_counter_ue;
 	uint32_t			stable_pstate;
+	uint32_t			workload_mode;
 };
 
 struct amdgpu_ctx_mgr {
-- 
2.34.1

[PATCH v2 4/5] drm/amdgpu: switch GPU workload profile

[Shashank Sharma]

This patch and switches the GPU workload based profile based
on the workload hint information saved in the workload context.
The workload profile is reset to NONE when the job is done.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c           |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c |  4 ----
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c          | 15 +++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h          |  3 +++
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index b7bae833c804..de906a42144f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -237,6 +237,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs
 		goto free_all_kdata;
 	}
 
+	p->job->workload_mode = p->ctx->workload_mode;
+
 	if (p->uf_entry.tv.bo)
 		p->job->uf_addr = uf_offset;
 	kvfree(chunk_array);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
index a11cf29bc388..625114804121 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
@@ -55,15 +55,11 @@ int amdgpu_set_workload_profile(struct amdgpu_device *adev,
 
 	mutex_lock(&adev->pm.smu_workload_lock);
 
-	if (adev->pm.workload_mode == hint)
-		goto unlock;
-
 	ret = amdgpu_dpm_switch_power_profile(adev, profile, 1);
 	if (!ret)
 		adev->pm.workload_mode = hint;
 	atomic_inc(&adev->pm.workload_switch_ref);
 
-unlock:
 	mutex_unlock(&adev->pm.smu_workload_lock);
 	return ret;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index c2fd6f3076a6..9300e86ee7c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -30,6 +30,7 @@
 #include "amdgpu.h"
 #include "amdgpu_trace.h"
 #include "amdgpu_reset.h"
+#include "amdgpu_ctx_workload.h"
 
 static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
 {
@@ -144,6 +145,14 @@ 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);
+
+	if (job->workload_mode != AMDGPU_CTX_WORKLOAD_HINT_NONE) {
+		if (amdgpu_clear_workload_profile(ring->adev, job->workload_mode))
+			DRM_WARN("Failed to come out of workload profile %s\n",
+				amdgpu_workload_profile_name(job->workload_mode));
+		job->workload_mode = AMDGPU_CTX_WORKLOAD_HINT_NONE;
+	}
 
 	drm_sched_job_cleanup(s_job);
 
@@ -256,6 +265,12 @@ static struct dma_fence *amdgpu_job_run(struct drm_sched_job *sched_job)
 			DRM_ERROR("Error scheduling IBs (%d)\n", r);
 	}
 
+	if (job->workload_mode != AMDGPU_CTX_WORKLOAD_HINT_NONE) {
+		if (amdgpu_set_workload_profile(ring->adev, job->workload_mode))
+			DRM_WARN("Failed to set workload profile to %s\n",
+				  amdgpu_workload_profile_name(job->workload_mode));
+	}
+
 	job->job_run_counter++;
 	amdgpu_job_free_resources(job);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index babc0af751c2..573e8692c814 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -68,6 +68,9 @@ struct amdgpu_job {
 	/* job_run_counter >= 1 means a resubmit job */
 	uint32_t		job_run_counter;
 
+	/* workload mode hint for pm */
+	uint32_t		workload_mode;
+
 	uint32_t		num_ibs;
 	struct amdgpu_ib	ibs[];
 };
-- 
2.34.1

[PATCH v2 5/5] drm/amdgpu: switch workload context to/from compute

[Shashank Sharma]

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

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 5e53a5293935..1caed319a448 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -34,6 +34,7 @@
 #include "amdgpu_ras.h"
 #include "amdgpu_umc.h"
 #include "amdgpu_reset.h"
+#include "amdgpu_ctx_workload.h"
 
 /* Total memory size in system memory and all GPU VRAM. Used to
  * estimate worst case amount of memory to reserve for page tables
@@ -703,9 +704,16 @@ int amdgpu_amdkfd_submit_ib(struct amdgpu_device *adev,
 
 void amdgpu_amdkfd_set_compute_idle(struct amdgpu_device *adev, bool idle)
 {
-	amdgpu_dpm_switch_power_profile(adev,
-					PP_SMC_POWER_PROFILE_COMPUTE,
-					!idle);
+	int ret;
+
+	if (idle)
+		ret = amdgpu_clear_workload_profile(adev, AMDGPU_CTX_WORKLOAD_HINT_COMPUTE);
+	else
+		ret = amdgpu_set_workload_profile(adev, AMDGPU_CTX_WORKLOAD_HINT_COMPUTE);
+
+	if (ret)
+		drm_warn(&adev->ddev, "Failed to %s power profile to compute mode\n",
+			 idle ? "reset" : "set");
 }
 
 bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device *adev, u32 vmid)
-- 
2.34.1

Re: [PATCH v2 1/5] drm/amdgpu: add UAPI for workload hints to ctx ioctl

[Christian König]

Am 26.09.22 um 17:02 schrieb Shashank Sharma:
> Allow the user to specify a workload hint to the kernel.
> We can use these to tweak the dpm heuristics to better match
> the workload for improved performance.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
> ---
>   include/uapi/drm/amdgpu_drm.h | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index c2c9c674a223..da5019a6e233 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -212,6 +212,8 @@ union drm_amdgpu_bo_list {
>   #define AMDGPU_CTX_OP_QUERY_STATE2	4
>   #define AMDGPU_CTX_OP_GET_STABLE_PSTATE	5
>   #define AMDGPU_CTX_OP_SET_STABLE_PSTATE	6
> +#define AMDGPU_CTX_OP_GET_WORKLOAD_PROFILE	7

Do we really have an use case for getting the profile or is that just 
added for completeness?

At base minimum we need a test-case for both to exercise the UAPI.

Regards,
Christian.

> +#define AMDGPU_CTX_OP_SET_WORKLOAD_PROFILE	8
>   
>   /* GPU reset status */
>   #define AMDGPU_CTX_NO_RESET		0
> @@ -252,6 +254,17 @@ union drm_amdgpu_bo_list {
>   #define AMDGPU_CTX_STABLE_PSTATE_MIN_MCLK  3
>   #define AMDGPU_CTX_STABLE_PSTATE_PEAK  4
>   
> +/* GPU workload hints, flag bits 8-15 */
> +#define AMDGPU_CTX_WORKLOAD_HINT_SHIFT     8
> +#define AMDGPU_CTX_WORKLOAD_HINT_MASK      (0xff << AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
> +#define AMDGPU_CTX_WORKLOAD_HINT_NONE      (0 << AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
> +#define AMDGPU_CTX_WORKLOAD_HINT_3D        (1 << AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
> +#define AMDGPU_CTX_WORKLOAD_HINT_VIDEO     (2 << AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
> +#define AMDGPU_CTX_WORKLOAD_HINT_VR        (3 << AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
> +#define AMDGPU_CTX_WORKLOAD_HINT_COMPUTE   (4 << AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
> +#define AMDGPU_CTX_WORKLOAD_HINT_MAX	   AMDGPU_CTX_WORKLOAD_HINT_COMPUTE
> +#define AMDGPU_CTX_WORKLOAD_INDEX(n)	   (n >> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
> +
>   struct drm_amdgpu_ctx_in {
>   	/** AMDGPU_CTX_OP_* */
>   	__u32	op;
> @@ -281,6 +294,11 @@ union drm_amdgpu_ctx_out {
>   			__u32	flags;
>   			__u32	_pad;
>   		} pstate;
> +
> +		struct {
> +			__u32	flags;
> +			__u32	_pad;
> +		} workload;
>   };
>   
>   union drm_amdgpu_ctx {

Re: [PATCH v2 1/5] drm/amdgpu: add UAPI for workload hints to ctx ioctl

[Sharma, Shashank]

Hello Christian,

On 9/26/2022 5:10 PM, Christian König wrote:
> Am 26.09.22 um 17:02 schrieb Shashank Sharma:
>> Allow the user to specify a workload hint to the kernel.
>> We can use these to tweak the dpm heuristics to better match
>> the workload for improved performance.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>> ---
>>   include/uapi/drm/amdgpu_drm.h | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/include/uapi/drm/amdgpu_drm.h 
>> b/include/uapi/drm/amdgpu_drm.h
>> index c2c9c674a223..da5019a6e233 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -212,6 +212,8 @@ union drm_amdgpu_bo_list {
>>   #define AMDGPU_CTX_OP_QUERY_STATE2    4
>>   #define AMDGPU_CTX_OP_GET_STABLE_PSTATE    5
>>   #define AMDGPU_CTX_OP_SET_STABLE_PSTATE    6
>> +#define AMDGPU_CTX_OP_GET_WORKLOAD_PROFILE    7
> 
> Do we really have an use case for getting the profile or is that just 
> added for completeness?
> To be honest, there is no direct use case for a get(). We have 
self-reset in kernel to clear the profile, so this is just for the sake 
of completeness.

> At base minimum we need a test-case for both to exercise the UAPI.
> 

Agree, I have already added some test cases in libdrm/tests/amdgpu for 
my local testing, I will start publishing them once we have an agreement 
on the UAPI and kernel design.

- Shashank

> Regards,
> Christian.
> 
>> +#define AMDGPU_CTX_OP_SET_WORKLOAD_PROFILE    8
>>   /* GPU reset status */
>>   #define AMDGPU_CTX_NO_RESET        0
>> @@ -252,6 +254,17 @@ union drm_amdgpu_bo_list {
>>   #define AMDGPU_CTX_STABLE_PSTATE_MIN_MCLK  3
>>   #define AMDGPU_CTX_STABLE_PSTATE_PEAK  4
>> +/* GPU workload hints, flag bits 8-15 */
>> +#define AMDGPU_CTX_WORKLOAD_HINT_SHIFT     8
>> +#define AMDGPU_CTX_WORKLOAD_HINT_MASK      (0xff << 
>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>> +#define AMDGPU_CTX_WORKLOAD_HINT_NONE      (0 << 
>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>> +#define AMDGPU_CTX_WORKLOAD_HINT_3D        (1 << 
>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>> +#define AMDGPU_CTX_WORKLOAD_HINT_VIDEO     (2 << 
>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>> +#define AMDGPU_CTX_WORKLOAD_HINT_VR        (3 << 
>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>> +#define AMDGPU_CTX_WORKLOAD_HINT_COMPUTE   (4 << 
>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>> +#define AMDGPU_CTX_WORKLOAD_HINT_MAX       
>> AMDGPU_CTX_WORKLOAD_HINT_COMPUTE
>> +#define AMDGPU_CTX_WORKLOAD_INDEX(n)       (n >> 
>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>> +
>>   struct drm_amdgpu_ctx_in {
>>       /** AMDGPU_CTX_OP_* */
>>       __u32    op;
>> @@ -281,6 +294,11 @@ union drm_amdgpu_ctx_out {
>>               __u32    flags;
>>               __u32    _pad;
>>           } pstate;
>> +
>> +        struct {
>> +            __u32    flags;
>> +            __u32    _pad;
>> +        } workload;
>>   };
>>   union drm_amdgpu_ctx {
> 

Re: [PATCH v2 1/5] drm/amdgpu: add UAPI for workload hints to ctx ioctl

[Christian König]

Am 26.09.22 um 17:14 schrieb Sharma, Shashank:
>
> Hello Christian,
>
> On 9/26/2022 5:10 PM, Christian König wrote:
>> Am 26.09.22 um 17:02 schrieb Shashank Sharma:
>>> Allow the user to specify a workload hint to the kernel.
>>> We can use these to tweak the dpm heuristics to better match
>>> the workload for improved performance.
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>>> ---
>>>   include/uapi/drm/amdgpu_drm.h | 18 ++++++++++++++++++
>>>   1 file changed, 18 insertions(+)
>>>
>>> diff --git a/include/uapi/drm/amdgpu_drm.h 
>>> b/include/uapi/drm/amdgpu_drm.h
>>> index c2c9c674a223..da5019a6e233 100644
>>> --- a/include/uapi/drm/amdgpu_drm.h
>>> +++ b/include/uapi/drm/amdgpu_drm.h
>>> @@ -212,6 +212,8 @@ union drm_amdgpu_bo_list {
>>>   #define AMDGPU_CTX_OP_QUERY_STATE2    4
>>>   #define AMDGPU_CTX_OP_GET_STABLE_PSTATE    5
>>>   #define AMDGPU_CTX_OP_SET_STABLE_PSTATE    6
>>> +#define AMDGPU_CTX_OP_GET_WORKLOAD_PROFILE    7
>>
>> Do we really have an use case for getting the profile or is that just 
>> added for completeness?
>> To be honest, there is no direct use case for a get(). We have 
> self-reset in kernel to clear the profile, so this is just for the 
> sake of completeness.

The problem is we usually need an userspace use case to justify 
upstreaming of an UAPI.

We already had a couple of cases where we only upstreamed UAPI for the 
sake of completeness (set/get pair for example) and then years later 
found out that the getter is completely broken for years because nobody 
used it.

So if we don't really need it I would try to avoid it.

Christian.

>
>> At base minimum we need a test-case for both to exercise the UAPI.
>>
>
> Agree, I have already added some test cases in libdrm/tests/amdgpu for 
> my local testing, I will start publishing them once we have an 
> agreement on the UAPI and kernel design.
>
> - Shashank
>
>> Regards,
>> Christian.
>>
>>> +#define AMDGPU_CTX_OP_SET_WORKLOAD_PROFILE    8
>>>   /* GPU reset status */
>>>   #define AMDGPU_CTX_NO_RESET        0
>>> @@ -252,6 +254,17 @@ union drm_amdgpu_bo_list {
>>>   #define AMDGPU_CTX_STABLE_PSTATE_MIN_MCLK  3
>>>   #define AMDGPU_CTX_STABLE_PSTATE_PEAK  4
>>> +/* GPU workload hints, flag bits 8-15 */
>>> +#define AMDGPU_CTX_WORKLOAD_HINT_SHIFT     8
>>> +#define AMDGPU_CTX_WORKLOAD_HINT_MASK      (0xff << 
>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>> +#define AMDGPU_CTX_WORKLOAD_HINT_NONE      (0 << 
>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>> +#define AMDGPU_CTX_WORKLOAD_HINT_3D        (1 << 
>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>> +#define AMDGPU_CTX_WORKLOAD_HINT_VIDEO     (2 << 
>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>> +#define AMDGPU_CTX_WORKLOAD_HINT_VR        (3 << 
>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>> +#define AMDGPU_CTX_WORKLOAD_HINT_COMPUTE   (4 << 
>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>> +#define AMDGPU_CTX_WORKLOAD_HINT_MAX AMDGPU_CTX_WORKLOAD_HINT_COMPUTE
>>> +#define AMDGPU_CTX_WORKLOAD_INDEX(n)       (n >> 
>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>> +
>>>   struct drm_amdgpu_ctx_in {
>>>       /** AMDGPU_CTX_OP_* */
>>>       __u32    op;
>>> @@ -281,6 +294,11 @@ union drm_amdgpu_ctx_out {
>>>               __u32    flags;
>>>               __u32    _pad;
>>>           } pstate;
>>> +
>>> +        struct {
>>> +            __u32    flags;
>>> +            __u32    _pad;
>>> +        } workload;
>>>   };
>>>   union drm_amdgpu_ctx {
>>

Re: [PATCH v2 1/5] drm/amdgpu: add UAPI for workload hints to ctx ioctl

[Sharma, Shashank]

On 9/26/2022 5:19 PM, Christian König wrote:
> Am 26.09.22 um 17:14 schrieb Sharma, Shashank:
>>
>> Hello Christian,
>>
>> On 9/26/2022 5:10 PM, Christian König wrote:
>>> Am 26.09.22 um 17:02 schrieb Shashank Sharma:
>>>> Allow the user to specify a workload hint to the kernel.
>>>> We can use these to tweak the dpm heuristics to better match
>>>> the workload for improved performance.
>>>>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>>>> ---
>>>>   include/uapi/drm/amdgpu_drm.h | 18 ++++++++++++++++++
>>>>   1 file changed, 18 insertions(+)
>>>>
>>>> diff --git a/include/uapi/drm/amdgpu_drm.h 
>>>> b/include/uapi/drm/amdgpu_drm.h
>>>> index c2c9c674a223..da5019a6e233 100644
>>>> --- a/include/uapi/drm/amdgpu_drm.h
>>>> +++ b/include/uapi/drm/amdgpu_drm.h
>>>> @@ -212,6 +212,8 @@ union drm_amdgpu_bo_list {
>>>>   #define AMDGPU_CTX_OP_QUERY_STATE2    4
>>>>   #define AMDGPU_CTX_OP_GET_STABLE_PSTATE    5
>>>>   #define AMDGPU_CTX_OP_SET_STABLE_PSTATE    6
>>>> +#define AMDGPU_CTX_OP_GET_WORKLOAD_PROFILE    7
>>>
>>> Do we really have an use case for getting the profile or is that just 
>>> added for completeness?
>>> To be honest, there is no direct use case for a get(). We have 
>> self-reset in kernel to clear the profile, so this is just for the 
>> sake of completeness.
> 
> The problem is we usually need an userspace use case to justify 
> upstreaming of an UAPI.
> 
> We already had a couple of cases where we only upstreamed UAPI for the 
> sake of completeness (set/get pair for example) and then years later 
> found out that the getter is completely broken for years because nobody 
> used it.
> 
> So if we don't really need it I would try to avoid it.
> 
> Christian.

Makes sense, I can remove get API as UAPI and just keep it kernel internal.

- Shashank

> 
>>
>>> At base minimum we need a test-case for both to exercise the UAPI.
>>>
>>
>> Agree, I have already added some test cases in libdrm/tests/amdgpu for 
>> my local testing, I will start publishing them once we have an 
>> agreement on the UAPI and kernel design.
>>
>> - Shashank
>>
>>> Regards,
>>> Christian.
>>>
>>>> +#define AMDGPU_CTX_OP_SET_WORKLOAD_PROFILE    8
>>>>   /* GPU reset status */
>>>>   #define AMDGPU_CTX_NO_RESET        0
>>>> @@ -252,6 +254,17 @@ union drm_amdgpu_bo_list {
>>>>   #define AMDGPU_CTX_STABLE_PSTATE_MIN_MCLK  3
>>>>   #define AMDGPU_CTX_STABLE_PSTATE_PEAK  4
>>>> +/* GPU workload hints, flag bits 8-15 */
>>>> +#define AMDGPU_CTX_WORKLOAD_HINT_SHIFT     8
>>>> +#define AMDGPU_CTX_WORKLOAD_HINT_MASK      (0xff << 
>>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>>> +#define AMDGPU_CTX_WORKLOAD_HINT_NONE      (0 << 
>>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>>> +#define AMDGPU_CTX_WORKLOAD_HINT_3D        (1 << 
>>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>>> +#define AMDGPU_CTX_WORKLOAD_HINT_VIDEO     (2 << 
>>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>>> +#define AMDGPU_CTX_WORKLOAD_HINT_VR        (3 << 
>>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>>> +#define AMDGPU_CTX_WORKLOAD_HINT_COMPUTE   (4 << 
>>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>>> +#define AMDGPU_CTX_WORKLOAD_HINT_MAX AMDGPU_CTX_WORKLOAD_HINT_COMPUTE
>>>> +#define AMDGPU_CTX_WORKLOAD_INDEX(n)       (n >> 
>>>> AMDGPU_CTX_WORKLOAD_HINT_SHIFT)
>>>> +
>>>>   struct drm_amdgpu_ctx_in {
>>>>       /** AMDGPU_CTX_OP_* */
>>>>       __u32    op;
>>>> @@ -281,6 +294,11 @@ union drm_amdgpu_ctx_out {
>>>>               __u32    flags;
>>>>               __u32    _pad;
>>>>           } pstate;
>>>> +
>>>> +        struct {
>>>> +            __u32    flags;
>>>> +            __u32    _pad;
>>>> +        } workload;
>>>>   };
>>>>   union drm_amdgpu_ctx {
>>>
>