All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Upstream debugger feature for GFX v9.4.3
@ 2023-07-06 18:19 Eric Huang
  2023-07-06 18:19 ` [PATCH 1/6] drm/amdkfd: add kfd2kgd debugger callbacks for GC v9.4.3 Eric Huang
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Eric Huang @ 2023-07-06 18:19 UTC (permalink / raw)
  To: amd-gfx; +Cc: Eric Huang, Jonathan.Kim

Eric Huang (2):
  drm/amdkfd: enable grace period for xcc instance
  drm/amdkfd: always keep trap enabled for GC v9.4.3

Jonathan Kim (4):
  drm/amdkfd: add kfd2kgd debugger callbacks for GC v9.4.3
  drm/amdkfd: restore debugger additional info for gfx v9_4_3
  drm/amdkfd: enable watch points globally for gfx943
  drm/amdkfd: add multi-process debugging support for GC v9.4.3

 .../drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c  |  10 +-
 .../drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.h  |  30 ++++
 .../drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c   | 152 +++++++++++++++++-
 .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c    |   9 +-
 .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.h    |  10 +-
 .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v11.c    |   3 +-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c |  15 +-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h |  10 +-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |   2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_debug.c        |  12 +-
 drivers/gpu/drm/amd/amdkfd/kfd_debug.h        |   5 +-
 .../drm/amd/amdkfd/kfd_device_queue_manager.c |   9 +-
 .../drm/amd/amdkfd/kfd_device_queue_manager.h |   2 +-
 .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   |  32 ++--
 .../drm/amd/amdkfd/kfd_packet_manager_v9.c    |  10 +-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |   2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_process.c      |   6 +-
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c     |  10 +-
 drivers/gpu/drm/amd/amdkfd/kfd_topology.h     |   3 +
 .../gpu/drm/amd/include/kgd_kfd_interface.h   |   9 +-
 20 files changed, 284 insertions(+), 57 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.h

-- 
2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/6] drm/amdkfd: add kfd2kgd debugger callbacks for GC v9.4.3
  2023-07-06 18:19 [PATCH 0/6] Upstream debugger feature for GFX v9.4.3 Eric Huang
@ 2023-07-06 18:19 ` Eric Huang
  2023-07-07 14:37   ` Kim, Jonathan
  2023-07-06 18:19 ` [PATCH 2/6] drm/amdkfd: restore debugger additional info for gfx v9_4_3 Eric Huang
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Eric Huang @ 2023-07-06 18:19 UTC (permalink / raw)
  To: amd-gfx; +Cc: Eric Huang, Jonathan Kim

From: Jonathan Kim <jonathan.kim@amd.com>

Implement the similarities as GC v9.4.2, and the difference
for GC v9.4.3 HW spec, i.e. xcc instance.

Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
---
 .../drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c  |  10 +-
 .../drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.h  |  30 ++++
 .../drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c   | 152 +++++++++++++++++-
 .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c    |   9 +-
 .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.h    |  10 +-
 .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v11.c    |   3 +-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c |  15 +-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h |  10 +-
 drivers/gpu/drm/amd/amdkfd/kfd_debug.c        |   3 +-
 .../drm/amd/amdkfd/kfd_device_queue_manager.c |   2 +-
 .../drm/amd/amdkfd/kfd_packet_manager_v9.c    |   3 +-
 .../gpu/drm/amd/include/kgd_kfd_interface.h   |   9 +-
 12 files changed, 230 insertions(+), 26 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c
index 60f9e027fb66..7d7eaed68531 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c
@@ -23,6 +23,7 @@
 #include "amdgpu_amdkfd.h"
 #include "amdgpu_amdkfd_arcturus.h"
 #include "amdgpu_amdkfd_gfx_v9.h"
+#include "amdgpu_amdkfd_aldebaran.h"
 #include "gc/gc_9_4_2_offset.h"
 #include "gc/gc_9_4_2_sh_mask.h"
 #include <uapi/linux/kfd_ioctl.h>
@@ -36,7 +37,7 @@
  * initialize the debug mode registers after it has disabled GFX off during the
  * debug session.
  */
-static uint32_t kgd_aldebaran_enable_debug_trap(struct amdgpu_device *adev,
+uint32_t kgd_aldebaran_enable_debug_trap(struct amdgpu_device *adev,
 					    bool restore_dbg_registers,
 					    uint32_t vmid)
 {
@@ -50,7 +51,7 @@ static uint32_t kgd_aldebaran_enable_debug_trap(struct amdgpu_device *adev,
 }
 
 /* returns TRAP_EN, EXCP_EN and EXCP_REPLACE. */
-static uint32_t kgd_aldebaran_disable_debug_trap(struct amdgpu_device *adev,
+uint32_t kgd_aldebaran_disable_debug_trap(struct amdgpu_device *adev,
 						bool keep_trap_enabled,
 						uint32_t vmid)
 {
@@ -107,7 +108,7 @@ static uint32_t kgd_aldebaran_set_wave_launch_trap_override(struct amdgpu_device
 	return data;
 }
 
-static uint32_t kgd_aldebaran_set_wave_launch_mode(struct amdgpu_device *adev,
+uint32_t kgd_aldebaran_set_wave_launch_mode(struct amdgpu_device *adev,
 					uint8_t wave_launch_mode,
 					uint32_t vmid)
 {
@@ -125,7 +126,8 @@ static uint32_t kgd_gfx_aldebaran_set_address_watch(
 					uint32_t watch_address_mask,
 					uint32_t watch_id,
 					uint32_t watch_mode,
-					uint32_t debug_vmid)
+					uint32_t debug_vmid,
+					uint32_t inst )
 {
 	uint32_t watch_address_high;
 	uint32_t watch_address_low;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.h
new file mode 100644
index 000000000000..ed349ff397bd
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.h
@@ -0,0 +1,30 @@
+/*
+ * 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.
+ */
+uint32_t kgd_aldebaran_enable_debug_trap(struct amdgpu_device *adev,
+					bool restore_dbg_registers,
+					uint32_t vmid);
+uint32_t kgd_aldebaran_disable_debug_trap(struct amdgpu_device *adev,
+					bool keep_trap_enabled,
+					uint32_t vmid);
+uint32_t kgd_aldebaran_set_wave_launch_mode(struct amdgpu_device *adev,
+					uint8_t wave_launch_mode,
+					uint32_t vmid);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c
index 5b4b7f8b92a5..1880162a4581 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c
@@ -22,6 +22,7 @@
 #include "amdgpu.h"
 #include "amdgpu_amdkfd.h"
 #include "amdgpu_amdkfd_gfx_v9.h"
+#include "amdgpu_amdkfd_aldebaran.h"
 #include "gc/gc_9_4_3_offset.h"
 #include "gc/gc_9_4_3_sh_mask.h"
 #include "athub/athub_1_8_0_offset.h"
@@ -32,6 +33,7 @@
 #include "soc15.h"
 #include "sdma/sdma_4_4_2_offset.h"
 #include "sdma/sdma_4_4_2_sh_mask.h"
+#include <uapi/linux/kfd_ioctl.h>
 
 static inline struct v9_sdma_mqd *get_sdma_mqd(void *mqd)
 {
@@ -361,6 +363,142 @@ static int kgd_gfx_v9_4_3_hqd_load(struct amdgpu_device *adev, void *mqd,
 	return 0;
 }
 
+static int kgd_gfx_v9_4_3_validate_trap_override_request(
+				struct amdgpu_device *adev,
+				uint32_t trap_override,
+				uint32_t *trap_mask_supported)
+{
+	*trap_mask_supported &= KFD_DBG_TRAP_MASK_FP_INVALID |
+	                        KFD_DBG_TRAP_MASK_FP_INPUT_DENORMAL |
+	                        KFD_DBG_TRAP_MASK_FP_DIVIDE_BY_ZERO |
+	                        KFD_DBG_TRAP_MASK_FP_OVERFLOW |
+	                        KFD_DBG_TRAP_MASK_FP_UNDERFLOW |
+	                        KFD_DBG_TRAP_MASK_FP_INEXACT |
+	                        KFD_DBG_TRAP_MASK_INT_DIVIDE_BY_ZERO |
+	                        KFD_DBG_TRAP_MASK_DBG_ADDRESS_WATCH |
+	                        KFD_DBG_TRAP_MASK_DBG_MEMORY_VIOLATION |
+	                        KFD_DBG_TRAP_MASK_TRAP_ON_WAVE_START |
+	                        KFD_DBG_TRAP_MASK_TRAP_ON_WAVE_END;
+
+	if (trap_override != KFD_DBG_TRAP_OVERRIDE_OR &&
+			trap_override != KFD_DBG_TRAP_OVERRIDE_REPLACE)
+		return -EPERM;
+
+	return 0;
+}
+
+static uint32_t trap_mask_map_sw_to_hw(uint32_t mask)
+{
+	uint32_t trap_on_start = (mask & KFD_DBG_TRAP_MASK_TRAP_ON_WAVE_START) ? 1 : 0;
+	uint32_t trap_on_end = (mask & KFD_DBG_TRAP_MASK_TRAP_ON_WAVE_END) ? 1 : 0;
+	uint32_t excp_en = mask & (KFD_DBG_TRAP_MASK_FP_INVALID |
+				KFD_DBG_TRAP_MASK_FP_INPUT_DENORMAL |
+				KFD_DBG_TRAP_MASK_FP_DIVIDE_BY_ZERO |
+				KFD_DBG_TRAP_MASK_FP_OVERFLOW |
+				KFD_DBG_TRAP_MASK_FP_UNDERFLOW |
+				KFD_DBG_TRAP_MASK_FP_INEXACT |
+				KFD_DBG_TRAP_MASK_INT_DIVIDE_BY_ZERO |
+				KFD_DBG_TRAP_MASK_DBG_ADDRESS_WATCH |
+				KFD_DBG_TRAP_MASK_DBG_MEMORY_VIOLATION);
+	uint32_t ret;
+
+	ret = REG_SET_FIELD(0, SPI_GDBG_PER_VMID_CNTL, EXCP_EN, excp_en);
+	ret = REG_SET_FIELD(ret, SPI_GDBG_PER_VMID_CNTL, TRAP_ON_START, trap_on_start);
+	ret = REG_SET_FIELD(ret, SPI_GDBG_PER_VMID_CNTL, TRAP_ON_END, trap_on_end);
+
+	return ret;
+}
+
+static uint32_t trap_mask_map_hw_to_sw(uint32_t mask)
+{
+	uint32_t ret = REG_GET_FIELD(mask, SPI_GDBG_PER_VMID_CNTL, EXCP_EN);
+
+	if (REG_GET_FIELD(mask, SPI_GDBG_PER_VMID_CNTL, TRAP_ON_START))
+		ret |= KFD_DBG_TRAP_MASK_TRAP_ON_WAVE_START;
+
+	if (REG_GET_FIELD(mask, SPI_GDBG_PER_VMID_CNTL, TRAP_ON_END))
+		ret |= KFD_DBG_TRAP_MASK_TRAP_ON_WAVE_END;
+
+	return ret;
+}
+
+/* returns TRAP_EN, EXCP_EN and EXCP_REPLACE. */
+static uint32_t kgd_gfx_v9_4_3_set_wave_launch_trap_override(
+				struct amdgpu_device *adev,
+				uint32_t vmid,
+				uint32_t trap_override,
+				uint32_t trap_mask_bits,
+				uint32_t trap_mask_request,
+				uint32_t *trap_mask_prev,
+				uint32_t kfd_dbg_trap_cntl_prev)
+
+{
+	uint32_t data = 0;
+
+	*trap_mask_prev = trap_mask_map_hw_to_sw(kfd_dbg_trap_cntl_prev);
+
+	data = (trap_mask_bits & trap_mask_request) |
+	       (*trap_mask_prev & ~trap_mask_request);
+	data = trap_mask_map_sw_to_hw(data);
+
+	data = REG_SET_FIELD(data, SPI_GDBG_PER_VMID_CNTL, TRAP_EN, 1);
+	data = REG_SET_FIELD(data, SPI_GDBG_PER_VMID_CNTL, EXCP_REPLACE, trap_override);
+
+	return data;
+}
+
+#define TCP_WATCH_STRIDE (regTCP_WATCH1_ADDR_H - regTCP_WATCH0_ADDR_H)
+static uint32_t kgd_gfx_v9_4_3_set_address_watch(
+				struct amdgpu_device *adev,
+				uint64_t watch_address,
+				uint32_t watch_address_mask,
+				uint32_t watch_id,
+				uint32_t watch_mode,
+				uint32_t debug_vmid,
+				uint32_t inst)
+{
+	uint32_t watch_address_high;
+	uint32_t watch_address_low;
+	uint32_t watch_address_cntl;
+
+	watch_address_cntl = 0;
+	watch_address_low = lower_32_bits(watch_address);
+	watch_address_high = upper_32_bits(watch_address) & 0xffff;
+
+	watch_address_cntl = REG_SET_FIELD(watch_address_cntl,
+			TCP_WATCH0_CNTL,
+			MODE,
+			watch_mode);
+
+	watch_address_cntl = REG_SET_FIELD(watch_address_cntl,
+			TCP_WATCH0_CNTL,
+			MASK,
+			watch_address_mask >> 7);
+
+	watch_address_cntl = REG_SET_FIELD(watch_address_cntl,
+			TCP_WATCH0_CNTL,
+			VALID,
+			1);
+
+	WREG32_RLC((SOC15_REG_OFFSET(GC, GET_INST(GC, inst),
+			regTCP_WATCH0_ADDR_H) +
+			(watch_id * TCP_WATCH_STRIDE)),
+			watch_address_high);
+
+	WREG32_RLC((SOC15_REG_OFFSET(GC, GET_INST(GC, inst),
+			regTCP_WATCH0_ADDR_L) +
+			(watch_id * TCP_WATCH_STRIDE)),
+			watch_address_low);
+
+	return watch_address_cntl;
+}
+
+static uint32_t kgd_gfx_v9_4_3_clear_address_watch(struct amdgpu_device *adev,
+				uint32_t watch_id)
+{
+	return 0;
+}
+
 const struct kfd2kgd_calls gc_9_4_3_kfd2kgd = {
 	.program_sh_mem_settings = kgd_gfx_v9_program_sh_mem_settings,
 	.set_pasid_vmid_mapping = kgd_gfx_v9_4_3_set_pasid_vmid_mapping,
@@ -380,5 +518,17 @@ const struct kfd2kgd_calls gc_9_4_3_kfd2kgd = {
 	.set_vm_context_page_table_base =
 				kgd_gfx_v9_set_vm_context_page_table_base,
 	.program_trap_handler_settings =
-				kgd_gfx_v9_program_trap_handler_settings
+				kgd_gfx_v9_program_trap_handler_settings,
+	.build_grace_period_packet_info =
+				kgd_gfx_v9_build_grace_period_packet_info,
+	.get_iq_wait_times = kgd_gfx_v9_get_iq_wait_times,
+	.enable_debug_trap = kgd_aldebaran_enable_debug_trap,
+	.disable_debug_trap = kgd_aldebaran_disable_debug_trap,
+	.validate_trap_override_request =
+			kgd_gfx_v9_4_3_validate_trap_override_request,
+	.set_wave_launch_trap_override =
+			kgd_gfx_v9_4_3_set_wave_launch_trap_override,
+	.set_wave_launch_mode = kgd_aldebaran_set_wave_launch_mode,
+	.set_address_watch = kgd_gfx_v9_4_3_set_address_watch,
+	.clear_address_watch = kgd_gfx_v9_4_3_clear_address_watch
 };
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
index 8ad7a7779e14..f1f2c24de081 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
@@ -886,7 +886,8 @@ uint32_t kgd_gfx_v10_set_address_watch(struct amdgpu_device *adev,
 					uint32_t watch_address_mask,
 					uint32_t watch_id,
 					uint32_t watch_mode,
-					uint32_t debug_vmid)
+					uint32_t debug_vmid,
+					uint32_t inst)
 {
 	uint32_t watch_address_high;
 	uint32_t watch_address_low;
@@ -968,7 +969,8 @@ uint32_t kgd_gfx_v10_clear_address_watch(struct amdgpu_device *adev,
  *     deq_retry_wait_time      -- Wait Count for Global Wave Syncs.
  */
 void kgd_gfx_v10_get_iq_wait_times(struct amdgpu_device *adev,
-					uint32_t *wait_times)
+					uint32_t *wait_times,
+					uint32_t inst)
 
 {
 	*wait_times = RREG32(SOC15_REG_OFFSET(GC, 0, mmCP_IQ_WAIT_TIME2));
@@ -978,7 +980,8 @@ void kgd_gfx_v10_build_grace_period_packet_info(struct amdgpu_device *adev,
 						uint32_t wait_times,
 						uint32_t grace_period,
 						uint32_t *reg_offset,
-						uint32_t *reg_data)
+						uint32_t *reg_data,
+						uint32_t inst)
 {
 	*reg_data = wait_times;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.h
index e6b70196071a..ecaead24e8c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.h
@@ -44,12 +44,16 @@ uint32_t kgd_gfx_v10_set_address_watch(struct amdgpu_device *adev,
 					uint32_t watch_address_mask,
 					uint32_t watch_id,
 					uint32_t watch_mode,
-					uint32_t debug_vmid);
+					uint32_t debug_vmid,
+					uint32_t inst);
 uint32_t kgd_gfx_v10_clear_address_watch(struct amdgpu_device *adev,
 					uint32_t watch_id);
-void kgd_gfx_v10_get_iq_wait_times(struct amdgpu_device *adev, uint32_t *wait_times);
+void kgd_gfx_v10_get_iq_wait_times(struct amdgpu_device *adev,
+				uint32_t *wait_times,
+				uint32_t inst);
 void kgd_gfx_v10_build_grace_period_packet_info(struct amdgpu_device *adev,
 					       uint32_t wait_times,
 					       uint32_t grace_period,
 					       uint32_t *reg_offset,
-					       uint32_t *reg_data);
+					       uint32_t *reg_data,
+					       uint32_t inst);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v11.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v11.c
index 91c3574ebed3..77ca5cbfb601 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v11.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v11.c
@@ -743,7 +743,8 @@ static uint32_t kgd_gfx_v11_set_address_watch(struct amdgpu_device *adev,
 					uint32_t watch_address_mask,
 					uint32_t watch_id,
 					uint32_t watch_mode,
-					uint32_t debug_vmid)
+					uint32_t debug_vmid,
+					uint32_t inst)
 {
 	uint32_t watch_address_high;
 	uint32_t watch_address_low;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
index 51d93fb13ea3..7b1eea493377 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
@@ -822,7 +822,8 @@ uint32_t kgd_gfx_v9_set_address_watch(struct amdgpu_device *adev,
 					uint32_t watch_address_mask,
 					uint32_t watch_id,
 					uint32_t watch_mode,
-					uint32_t debug_vmid)
+					uint32_t debug_vmid,
+					uint32_t inst)
 {
 	uint32_t watch_address_high;
 	uint32_t watch_address_low;
@@ -903,10 +904,12 @@ uint32_t kgd_gfx_v9_clear_address_watch(struct amdgpu_device *adev,
  *     deq_retry_wait_time      -- Wait Count for Global Wave Syncs.
  */
 void kgd_gfx_v9_get_iq_wait_times(struct amdgpu_device *adev,
-					uint32_t *wait_times)
+					uint32_t *wait_times,
+					uint32_t inst)
 
 {
-	*wait_times = RREG32(SOC15_REG_OFFSET(GC, 0, mmCP_IQ_WAIT_TIME2));
+	*wait_times = RREG32(SOC15_REG_OFFSET(GC, GET_INST(GC, inst),
+			mmCP_IQ_WAIT_TIME2));
 }
 
 void kgd_gfx_v9_set_vm_context_page_table_base(struct amdgpu_device *adev,
@@ -1100,7 +1103,8 @@ void kgd_gfx_v9_build_grace_period_packet_info(struct amdgpu_device *adev,
 		uint32_t wait_times,
 		uint32_t grace_period,
 		uint32_t *reg_offset,
-		uint32_t *reg_data)
+		uint32_t *reg_data,
+		uint32_t inst)
 {
 	*reg_data = wait_times;
 
@@ -1116,7 +1120,8 @@ void kgd_gfx_v9_build_grace_period_packet_info(struct amdgpu_device *adev,
 			SCH_WAVE,
 			grace_period);
 
-	*reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_IQ_WAIT_TIME2);
+	*reg_offset = SOC15_REG_OFFSET(GC, GET_INST(GC, inst),
+			mmCP_IQ_WAIT_TIME2);
 }
 
 void kgd_gfx_v9_program_trap_handler_settings(struct amdgpu_device *adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
index 5f54bff0db49..936e501908ce 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
@@ -89,12 +89,16 @@ uint32_t kgd_gfx_v9_set_address_watch(struct amdgpu_device *adev,
 					uint32_t watch_address_mask,
 					uint32_t watch_id,
 					uint32_t watch_mode,
-					uint32_t debug_vmid);
+					uint32_t debug_vmid,
+					uint32_t inst);
 uint32_t kgd_gfx_v9_clear_address_watch(struct amdgpu_device *adev,
 					uint32_t watch_id);
-void kgd_gfx_v9_get_iq_wait_times(struct amdgpu_device *adev, uint32_t *wait_times);
+void kgd_gfx_v9_get_iq_wait_times(struct amdgpu_device *adev,
+				uint32_t *wait_times,
+				uint32_t inst);
 void kgd_gfx_v9_build_grace_period_packet_info(struct amdgpu_device *adev,
 					       uint32_t wait_times,
 					       uint32_t grace_period,
 					       uint32_t *reg_offset,
-					       uint32_t *reg_data);
+					       uint32_t *reg_data,
+					       uint32_t inst);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
index fff3ccc04fa9..24083db44724 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
@@ -466,7 +466,8 @@ int kfd_dbg_trap_set_dev_address_watch(struct kfd_process_device *pdd,
 				watch_address_mask,
 				*watch_id,
 				watch_mode,
-				pdd->dev->vm_info.last_vmid_kfd);
+				pdd->dev->vm_info.last_vmid_kfd,
+				0);
 	amdgpu_gfx_off_ctrl(pdd->dev->adev, true);
 
 	if (!pdd->dev->kfd->shared_resources.enable_mes)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index f515cb8f30ca..a2bff3f01359 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1621,7 +1621,7 @@ static int initialize_cpsch(struct device_queue_manager *dqm)
 
 	if (dqm->dev->kfd2kgd->get_iq_wait_times)
 		dqm->dev->kfd2kgd->get_iq_wait_times(dqm->dev->adev,
-					&dqm->wait_times);
+					&dqm->wait_times, 0);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
index 29a2d0499b67..8fda16e6fee6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
@@ -298,7 +298,8 @@ static int pm_set_grace_period_v9(struct packet_manager *pm,
 			pm->dqm->wait_times,
 			grace_period,
 			&reg_offset,
-			&reg_data);
+			&reg_data,
+			0);
 
 	if (grace_period == USE_DEFAULT_GRACE_PERIOD)
 		reg_data = pm->dqm->wait_times;
diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
index d0df3381539f..8433f99f6667 100644
--- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
@@ -315,16 +315,19 @@ struct kfd2kgd_calls {
 					uint32_t watch_address_mask,
 					uint32_t watch_id,
 					uint32_t watch_mode,
-					uint32_t debug_vmid);
+					uint32_t debug_vmid,
+					uint32_t inst);
 	uint32_t (*clear_address_watch)(struct amdgpu_device *adev,
 			uint32_t watch_id);
 	void (*get_iq_wait_times)(struct amdgpu_device *adev,
-			uint32_t *wait_times);
+			uint32_t *wait_times,
+			uint32_t inst);
 	void (*build_grace_period_packet_info)(struct amdgpu_device *adev,
 			uint32_t wait_times,
 			uint32_t grace_period,
 			uint32_t *reg_offset,
-			uint32_t *reg_data);
+			uint32_t *reg_data,
+			uint32_t inst);
 	void (*get_cu_occupancy)(struct amdgpu_device *adev, int pasid,
 			int *wave_cnt, int *max_waves_per_cu, uint32_t inst);
 	void (*program_trap_handler_settings)(struct amdgpu_device *adev,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/6] drm/amdkfd: restore debugger additional info for gfx v9_4_3
  2023-07-06 18:19 [PATCH 0/6] Upstream debugger feature for GFX v9.4.3 Eric Huang
  2023-07-06 18:19 ` [PATCH 1/6] drm/amdkfd: add kfd2kgd debugger callbacks for GC v9.4.3 Eric Huang
@ 2023-07-06 18:19 ` Eric Huang
  2023-07-06 18:19 ` [PATCH 3/6] drm/amdkfd: enable watch points globally for gfx943 Eric Huang
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Eric Huang @ 2023-07-06 18:19 UTC (permalink / raw)
  To: amd-gfx; +Cc: Amber Lin, Eric Huang, Jonathan Kim, Harish Kasiviswanathan

From: Jonathan Kim <jonathan.kim@amd.com>

The additional information that the KFD reports to the debugger was
destroyed when the following commit was merged:
"drm/amdkfd: convert switches to IP version checking"

Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
Reviewed-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com>
Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
Acked-by: Amber Lin <amber.lin@amd.com>
Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
Reviewed-by: Jonathan Kim <jonathan.kim@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 10 ++++++++--
 drivers/gpu/drm/amd/amdkfd/kfd_topology.h |  3 +++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index 61fc62f3e003..1a4cdee86759 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -1932,8 +1932,14 @@ static void kfd_topology_set_capabilities(struct kfd_topology_device *dev)
 			HSA_CAP_TRAP_DEBUG_WAVE_LAUNCH_MODE_SUPPORTED;
 
 	if (KFD_GC_VERSION(dev->gpu) < IP_VERSION(10, 0, 0)) {
-		dev->node_props.debug_prop |= HSA_DBG_WATCH_ADDR_MASK_LO_BIT_GFX9 |
-						HSA_DBG_WATCH_ADDR_MASK_HI_BIT;
+		if (KFD_GC_VERSION(dev->gpu) == IP_VERSION(9, 4, 3))
+			dev->node_props.debug_prop |=
+				HSA_DBG_WATCH_ADDR_MASK_LO_BIT_GFX9_4_3 |
+				HSA_DBG_WATCH_ADDR_MASK_HI_BIT_GFX9_4_3;
+		else
+			dev->node_props.debug_prop |=
+				HSA_DBG_WATCH_ADDR_MASK_LO_BIT_GFX9 |
+				HSA_DBG_WATCH_ADDR_MASK_HI_BIT;
 
 		if (KFD_GC_VERSION(dev->gpu) < IP_VERSION(9, 4, 2))
 			dev->node_props.debug_prop |=
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
index cba2cd5ed9d1..dea32a9e5506 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
@@ -32,9 +32,12 @@
 #define KFD_TOPOLOGY_PUBLIC_NAME_SIZE 32
 
 #define HSA_DBG_WATCH_ADDR_MASK_LO_BIT_GFX9	6
+#define HSA_DBG_WATCH_ADDR_MASK_LO_BIT_GFX9_4_3 7
 #define HSA_DBG_WATCH_ADDR_MASK_LO_BIT_GFX10	7
 #define HSA_DBG_WATCH_ADDR_MASK_HI_BIT  \
 			(29 << HSA_DBG_WATCH_ADDR_MASK_HI_BIT_SHIFT)
+#define HSA_DBG_WATCH_ADDR_MASK_HI_BIT_GFX9_4_3 \
+			(30 << HSA_DBG_WATCH_ADDR_MASK_HI_BIT_SHIFT)
 
 struct kfd_node_properties {
 	uint64_t hive_id;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/6] drm/amdkfd: enable watch points globally for gfx943
  2023-07-06 18:19 [PATCH 0/6] Upstream debugger feature for GFX v9.4.3 Eric Huang
  2023-07-06 18:19 ` [PATCH 1/6] drm/amdkfd: add kfd2kgd debugger callbacks for GC v9.4.3 Eric Huang
  2023-07-06 18:19 ` [PATCH 2/6] drm/amdkfd: restore debugger additional info for gfx v9_4_3 Eric Huang
@ 2023-07-06 18:19 ` Eric Huang
  2023-07-07 15:04   ` Kim, Jonathan
  2023-07-06 18:19 ` [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance Eric Huang
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Eric Huang @ 2023-07-06 18:19 UTC (permalink / raw)
  To: amd-gfx; +Cc: Eric Huang, Felix Kuehling, Jonathan Kim

From: Jonathan Kim <jonathan.kim@amd.com>

Set watch points for all xcc instances on GFX943.

Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_debug.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
index 24083db44724..190b03efe5ff 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
@@ -446,7 +446,8 @@ int kfd_dbg_trap_set_dev_address_watch(struct kfd_process_device *pdd,
 					uint32_t *watch_id,
 					uint32_t watch_mode)
 {
-	int r = kfd_dbg_get_dev_watch_id(pdd, watch_id);
+	int xcc_id, r = kfd_dbg_get_dev_watch_id(pdd, watch_id);
+	uint32_t xcc_mask = pdd->dev->xcc_mask;
 
 	if (r)
 		return r;
@@ -460,14 +461,15 @@ int kfd_dbg_trap_set_dev_address_watch(struct kfd_process_device *pdd,
 	}
 
 	amdgpu_gfx_off_ctrl(pdd->dev->adev, false);
-	pdd->watch_points[*watch_id] = pdd->dev->kfd2kgd->set_address_watch(
+	for_each_inst(xcc_id, xcc_mask)
+		pdd->watch_points[*watch_id] = pdd->dev->kfd2kgd->set_address_watch(
 				pdd->dev->adev,
 				watch_address,
 				watch_address_mask,
 				*watch_id,
 				watch_mode,
 				pdd->dev->vm_info.last_vmid_kfd,
-				0);
+				xcc_id);
 	amdgpu_gfx_off_ctrl(pdd->dev->adev, true);
 
 	if (!pdd->dev->kfd->shared_resources.enable_mes)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
  2023-07-06 18:19 [PATCH 0/6] Upstream debugger feature for GFX v9.4.3 Eric Huang
                   ` (2 preceding siblings ...)
  2023-07-06 18:19 ` [PATCH 3/6] drm/amdkfd: enable watch points globally for gfx943 Eric Huang
@ 2023-07-06 18:19 ` Eric Huang
  2023-07-07 14:59   ` Kim, Jonathan
  2023-07-06 18:19 ` [PATCH 5/6] drm/amdkfd: always keep trap enabled for GC v9.4.3 Eric Huang
  2023-07-06 18:19 ` [PATCH 6/6] drm/amdkfd: add multi-process debugging support " Eric Huang
  5 siblings, 1 reply; 17+ messages in thread
From: Eric Huang @ 2023-07-06 18:19 UTC (permalink / raw)
  To: amd-gfx; +Cc: Eric Huang, Jonathan.Kim

each xcc instance needs to get iq wait time and set
grace period accordingly.

Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
---
 .../drm/amd/amdkfd/kfd_device_queue_manager.c |  9 ++++--
 .../drm/amd/amdkfd/kfd_device_queue_manager.h |  2 +-
 .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   | 32 +++++++++++--------
 .../drm/amd/amdkfd/kfd_packet_manager_v9.c    |  9 +++---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  2 +-
 5 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index a2bff3f01359..0f12c1989e14 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1606,6 +1606,8 @@ static int set_sched_resources(struct device_queue_manager *dqm)
 
 static int initialize_cpsch(struct device_queue_manager *dqm)
 {
+	uint32_t xcc_id, xcc_mask = dqm->dev->xcc_mask;
+
 	pr_debug("num of pipes: %d\n", get_pipes_per_mec(dqm));
 
 	mutex_init(&dqm->lock_hidden);
@@ -1620,8 +1622,11 @@ static int initialize_cpsch(struct device_queue_manager *dqm)
 	init_sdma_bitmaps(dqm);
 
 	if (dqm->dev->kfd2kgd->get_iq_wait_times)
-		dqm->dev->kfd2kgd->get_iq_wait_times(dqm->dev->adev,
-					&dqm->wait_times, 0);
+		for_each_inst(xcc_id, xcc_mask)
+			dqm->dev->kfd2kgd->get_iq_wait_times(
+					dqm->dev->adev,
+					&dqm->wait_times[xcc_id],
+					xcc_id);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
index 7dd4b177219d..62a6dc8d3032 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
@@ -262,7 +262,7 @@ struct device_queue_manager {
 	/* used for GFX 9.4.3 only */
 	uint32_t		current_logical_xcc_start;
 
-	uint32_t		wait_times;
+	uint32_t		wait_times[32];
 
 	wait_queue_head_t	destroy_wait;
 };
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
index 401096c103b2..f37ab4b6d88c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
@@ -374,27 +374,31 @@ int pm_update_grace_period(struct packet_manager *pm, uint32_t grace_period)
 {
 	int retval = 0;
 	uint32_t *buffer, size;
+	uint32_t xcc_id, xcc_mask = pm->dqm->dev->xcc_mask;
 
 	size = pm->pmf->set_grace_period_size;
 
 	mutex_lock(&pm->lock);
 
 	if (size) {
-		kq_acquire_packet_buffer(pm->priv_queue,
-			size / sizeof(uint32_t),
-			(unsigned int **)&buffer);
-
-		if (!buffer) {
-			pr_err("Failed to allocate buffer on kernel queue\n");
-			retval = -ENOMEM;
-			goto out;
-		}
+		for_each_inst(xcc_id, xcc_mask) {
+			kq_acquire_packet_buffer(pm->priv_queue,
+					size / sizeof(uint32_t),
+					(unsigned int **)&buffer);
 
-		retval = pm->pmf->set_grace_period(pm, buffer, grace_period);
-		if (!retval)
-			kq_submit_packet(pm->priv_queue);
-		else
-			kq_rollback_packet(pm->priv_queue);
+			if (!buffer) {
+				pr_err("Failed to allocate buffer on kernel queue\n");
+				retval = -ENOMEM;
+				goto out;
+			}
+
+			retval = pm->pmf->set_grace_period(pm, buffer,
+					grace_period, xcc_id);
+			if (!retval)
+				kq_submit_packet(pm->priv_queue);
+			else
+				kq_rollback_packet(pm->priv_queue);
+		}
 	}
 
 out:
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
index 8fda16e6fee6..a9443d661957 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
@@ -287,7 +287,8 @@ static int pm_map_queues_v9(struct packet_manager *pm, uint32_t *buffer,
 
 static int pm_set_grace_period_v9(struct packet_manager *pm,
 		uint32_t *buffer,
-		uint32_t grace_period)
+		uint32_t grace_period,
+		uint32_t inst)
 {
 	struct pm4_mec_write_data_mmio *packet;
 	uint32_t reg_offset = 0;
@@ -295,14 +296,14 @@ static int pm_set_grace_period_v9(struct packet_manager *pm,
 
 	pm->dqm->dev->kfd2kgd->build_grace_period_packet_info(
 			pm->dqm->dev->adev,
-			pm->dqm->wait_times,
+			pm->dqm->wait_times[inst],
 			grace_period,
 			&reg_offset,
 			&reg_data,
-			0);
+			inst);
 
 	if (grace_period == USE_DEFAULT_GRACE_PERIOD)
-		reg_data = pm->dqm->wait_times;
+		reg_data = pm->dqm->wait_times[inst];
 
 	packet = (struct pm4_mec_write_data_mmio *)buffer;
 	memset(buffer, 0, sizeof(struct pm4_mec_write_data_mmio));
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index d4c9ee3f9953..22c4a403ddd7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1400,7 +1400,7 @@ struct packet_manager_funcs {
 			enum kfd_unmap_queues_filter mode,
 			uint32_t filter_param, bool reset);
 	int (*set_grace_period)(struct packet_manager *pm, uint32_t *buffer,
-			uint32_t grace_period);
+			uint32_t grace_period, uint32_t inst);
 	int (*query_status)(struct packet_manager *pm, uint32_t *buffer,
 			uint64_t fence_address,	uint64_t fence_value);
 	int (*release_mem)(uint64_t gpu_addr, uint32_t *buffer);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/6] drm/amdkfd: always keep trap enabled for GC v9.4.3
  2023-07-06 18:19 [PATCH 0/6] Upstream debugger feature for GFX v9.4.3 Eric Huang
                   ` (3 preceding siblings ...)
  2023-07-06 18:19 ` [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance Eric Huang
@ 2023-07-06 18:19 ` Eric Huang
  2023-07-07 14:39   ` Kim, Jonathan
  2023-07-06 18:19 ` [PATCH 6/6] drm/amdkfd: add multi-process debugging support " Eric Huang
  5 siblings, 1 reply; 17+ messages in thread
From: Eric Huang @ 2023-07-06 18:19 UTC (permalink / raw)
  To: amd-gfx; +Cc: Eric Huang, Jonathan.Kim

To set TTMP setup on by default.

Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_debug.c   | 3 ++-
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index cf1db0ab3471..47c5d16677d6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -2842,7 +2842,7 @@ static int runtime_disable(struct kfd_process *p)
 			pdd->spi_dbg_override =
 					pdd->dev->kfd2kgd->disable_debug_trap(
 					pdd->dev->adev,
-					false,
+					KFD_GC_VERSION(pdd->dev) == IP_VERSION(9, 4, 3),
 					pdd->dev->vm_info.last_vmid_kfd);
 
 			if (!pdd->dev->kfd->shared_resources.enable_mes)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
index 190b03efe5ff..4cb9b3b18065 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
@@ -591,7 +591,8 @@ void kfd_dbg_trap_deactivate(struct kfd_process *target, bool unwind, int unwind
 		pdd->spi_dbg_override =
 				pdd->dev->kfd2kgd->disable_debug_trap(
 				pdd->dev->adev,
-				target->runtime_info.ttmp_setup,
+				KFD_GC_VERSION(pdd->dev) == IP_VERSION(9, 4, 3) ?
+					true : target->runtime_info.ttmp_setup,
 				pdd->dev->vm_info.last_vmid_kfd);
 		amdgpu_gfx_off_ctrl(pdd->dev->adev, true);
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index ba04a4baecf2..91ae9121e2bf 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1644,9 +1644,9 @@ struct kfd_process_device *kfd_create_process_device_data(struct kfd_node *dev,
 	p->pdds[p->n_pdds++] = pdd;
 	if (kfd_dbg_is_per_vmid_supported(pdd->dev))
 		pdd->spi_dbg_override = pdd->dev->kfd2kgd->disable_debug_trap(
-							pdd->dev->adev,
-							false,
-							0);
+				pdd->dev->adev,
+				KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 3),
+				0);
 
 	/* Init idr used for memory handle translation */
 	idr_init(&pdd->alloc_idr);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/6] drm/amdkfd: add multi-process debugging support for GC v9.4.3
  2023-07-06 18:19 [PATCH 0/6] Upstream debugger feature for GFX v9.4.3 Eric Huang
                   ` (4 preceding siblings ...)
  2023-07-06 18:19 ` [PATCH 5/6] drm/amdkfd: always keep trap enabled for GC v9.4.3 Eric Huang
@ 2023-07-06 18:19 ` Eric Huang
  2023-07-07 15:06   ` Kim, Jonathan
  5 siblings, 1 reply; 17+ messages in thread
From: Eric Huang @ 2023-07-06 18:19 UTC (permalink / raw)
  To: amd-gfx; +Cc: Eric Huang, Felix Kuehling, Jonathan Kim

From: Jonathan Kim <jonathan.kim@amd.com>

Similar to GC v9.4.2, GC v9.4.3 should use the 5-Dword extended
MAP_PROCESS packet to support multi-process debugging.  Update the
mutli-process debug support list so that the KFD updates the runlist
on debug mode setting and that it allocates enough GTT memory during
KFD device initialization.

Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_debug.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
index a289e59ceb79..a0afc6a7b6c4 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
@@ -76,8 +76,9 @@ int kfd_dbg_send_exception_to_runtime(struct kfd_process *p,
 
 static inline bool kfd_dbg_is_per_vmid_supported(struct kfd_node *dev)
 {
-	return KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
-	       KFD_GC_VERSION(dev) >= IP_VERSION(11, 0, 0);
+	return (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
+	        KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 3) ||
+	        KFD_GC_VERSION(dev) >= IP_VERSION(11, 0, 0));
 }
 
 void debug_event_write_work_handler(struct work_struct *work);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* RE: [PATCH 1/6] drm/amdkfd: add kfd2kgd debugger callbacks for GC v9.4.3
  2023-07-06 18:19 ` [PATCH 1/6] drm/amdkfd: add kfd2kgd debugger callbacks for GC v9.4.3 Eric Huang
@ 2023-07-07 14:37   ` Kim, Jonathan
  0 siblings, 0 replies; 17+ messages in thread
From: Kim, Jonathan @ 2023-07-07 14:37 UTC (permalink / raw)
  To: Huang, JinHuiEric, amd-gfx

[AMD Official Use Only - General]

> -----Original Message-----
> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> Sent: Thursday, July 6, 2023 2:19 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Kim, Jonathan <Jonathan.Kim@amd.com>; Kim, Jonathan
> <Jonathan.Kim@amd.com>; Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> Subject: [PATCH 1/6] drm/amdkfd: add kfd2kgd debugger callbacks for GC
> v9.4.3
>
> From: Jonathan Kim <jonathan.kim@amd.com>
>
> Implement the similarities as GC v9.4.2, and the difference
> for GC v9.4.3 HW spec, i.e. xcc instance.
>
> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
> ---
>  .../drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c  |  10 +-
>  .../drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.h  |  30 ++++
>  .../drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c   | 152
> +++++++++++++++++-
>  .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c    |   9 +-
>  .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.h    |  10 +-
>  .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v11.c    |   3 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c |  15 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h |  10 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_debug.c        |   3 +-
>  .../drm/amd/amdkfd/kfd_device_queue_manager.c |   2 +-
>  .../drm/amd/amdkfd/kfd_packet_manager_v9.c    |   3 +-
>  .../gpu/drm/amd/include/kgd_kfd_interface.h   |   9 +-
>  12 files changed, 230 insertions(+), 26 deletions(-)
>  create mode 100644
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c
> index 60f9e027fb66..7d7eaed68531 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c
> @@ -23,6 +23,7 @@
>  #include "amdgpu_amdkfd.h"
>  #include "amdgpu_amdkfd_arcturus.h"
>  #include "amdgpu_amdkfd_gfx_v9.h"
> +#include "amdgpu_amdkfd_aldebaran.h"
>  #include "gc/gc_9_4_2_offset.h"
>  #include "gc/gc_9_4_2_sh_mask.h"
>  #include <uapi/linux/kfd_ioctl.h>
> @@ -36,7 +37,7 @@
>   * initialize the debug mode registers after it has disabled GFX off during the
>   * debug session.
>   */
> -static uint32_t kgd_aldebaran_enable_debug_trap(struct amdgpu_device
> *adev,
> +uint32_t kgd_aldebaran_enable_debug_trap(struct amdgpu_device *adev,
>                                           bool restore_dbg_registers,
>                                           uint32_t vmid)
>  {
> @@ -50,7 +51,7 @@ static uint32_t
> kgd_aldebaran_enable_debug_trap(struct amdgpu_device *adev,
>  }
>
>  /* returns TRAP_EN, EXCP_EN and EXCP_REPLACE. */
> -static uint32_t kgd_aldebaran_disable_debug_trap(struct amdgpu_device
> *adev,
> +uint32_t kgd_aldebaran_disable_debug_trap(struct amdgpu_device *adev,
>                                               bool keep_trap_enabled,
>                                               uint32_t vmid)
>  {
> @@ -107,7 +108,7 @@ static uint32_t
> kgd_aldebaran_set_wave_launch_trap_override(struct amdgpu_device
>       return data;
>  }
>
> -static uint32_t kgd_aldebaran_set_wave_launch_mode(struct
> amdgpu_device *adev,
> +uint32_t kgd_aldebaran_set_wave_launch_mode(struct amdgpu_device
> *adev,
>                                       uint8_t wave_launch_mode,
>                                       uint32_t vmid)
>  {
> @@ -125,7 +126,8 @@ static uint32_t
> kgd_gfx_aldebaran_set_address_watch(
>                                       uint32_t watch_address_mask,
>                                       uint32_t watch_id,
>                                       uint32_t watch_mode,
> -                                     uint32_t debug_vmid)
> +                                     uint32_t debug_vmid,
> +                                     uint32_t inst )
>  {
>       uint32_t watch_address_high;
>       uint32_t watch_address_low;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.h
> new file mode 100644
> index 000000000000..ed349ff397bd
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.h
> @@ -0,0 +1,30 @@
> +/*
> + * 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.
> + */
> +uint32_t kgd_aldebaran_enable_debug_trap(struct amdgpu_device *adev,
> +                                     bool restore_dbg_registers,
> +                                     uint32_t vmid);
> +uint32_t kgd_aldebaran_disable_debug_trap(struct amdgpu_device *adev,
> +                                     bool keep_trap_enabled,
> +                                     uint32_t vmid);
> +uint32_t kgd_aldebaran_set_wave_launch_mode(struct amdgpu_device
> *adev,
> +                                     uint8_t wave_launch_mode,
> +                                     uint32_t vmid);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c
> index 5b4b7f8b92a5..1880162a4581 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c
> @@ -22,6 +22,7 @@
>  #include "amdgpu.h"
>  #include "amdgpu_amdkfd.h"
>  #include "amdgpu_amdkfd_gfx_v9.h"
> +#include "amdgpu_amdkfd_aldebaran.h"
>  #include "gc/gc_9_4_3_offset.h"
>  #include "gc/gc_9_4_3_sh_mask.h"
>  #include "athub/athub_1_8_0_offset.h"
> @@ -32,6 +33,7 @@
>  #include "soc15.h"
>  #include "sdma/sdma_4_4_2_offset.h"
>  #include "sdma/sdma_4_4_2_sh_mask.h"
> +#include <uapi/linux/kfd_ioctl.h>
>
>  static inline struct v9_sdma_mqd *get_sdma_mqd(void *mqd)
>  {
> @@ -361,6 +363,142 @@ static int kgd_gfx_v9_4_3_hqd_load(struct
> amdgpu_device *adev, void *mqd,
>       return 0;
>  }
>
> +static int kgd_gfx_v9_4_3_validate_trap_override_request(
> +                             struct amdgpu_device *adev,
> +                             uint32_t trap_override,
> +                             uint32_t *trap_mask_supported)
> +{
> +     *trap_mask_supported &= KFD_DBG_TRAP_MASK_FP_INVALID |
> +                             KFD_DBG_TRAP_MASK_FP_INPUT_DENORMAL |
> +                             KFD_DBG_TRAP_MASK_FP_DIVIDE_BY_ZERO |
> +                             KFD_DBG_TRAP_MASK_FP_OVERFLOW |
> +                             KFD_DBG_TRAP_MASK_FP_UNDERFLOW |
> +                             KFD_DBG_TRAP_MASK_FP_INEXACT |
> +                             KFD_DBG_TRAP_MASK_INT_DIVIDE_BY_ZERO |
> +                             KFD_DBG_TRAP_MASK_DBG_ADDRESS_WATCH |
> +                             KFD_DBG_TRAP_MASK_DBG_MEMORY_VIOLATION |
> +                             KFD_DBG_TRAP_MASK_TRAP_ON_WAVE_START |
> +                             KFD_DBG_TRAP_MASK_TRAP_ON_WAVE_END;
> +
> +     if (trap_override != KFD_DBG_TRAP_OVERRIDE_OR &&
> +                     trap_override !=
> KFD_DBG_TRAP_OVERRIDE_REPLACE)
> +             return -EPERM;
> +
> +     return 0;
> +}
> +
> +static uint32_t trap_mask_map_sw_to_hw(uint32_t mask)
> +{
> +     uint32_t trap_on_start = (mask &
> KFD_DBG_TRAP_MASK_TRAP_ON_WAVE_START) ? 1 : 0;
> +     uint32_t trap_on_end = (mask &
> KFD_DBG_TRAP_MASK_TRAP_ON_WAVE_END) ? 1 : 0;
> +     uint32_t excp_en = mask & (KFD_DBG_TRAP_MASK_FP_INVALID |
> +
>       KFD_DBG_TRAP_MASK_FP_INPUT_DENORMAL |
> +                             KFD_DBG_TRAP_MASK_FP_DIVIDE_BY_ZERO |
> +                             KFD_DBG_TRAP_MASK_FP_OVERFLOW |
> +                             KFD_DBG_TRAP_MASK_FP_UNDERFLOW |
> +                             KFD_DBG_TRAP_MASK_FP_INEXACT |
> +                             KFD_DBG_TRAP_MASK_INT_DIVIDE_BY_ZERO
> |
> +
>       KFD_DBG_TRAP_MASK_DBG_ADDRESS_WATCH |
> +
>       KFD_DBG_TRAP_MASK_DBG_MEMORY_VIOLATION);
> +     uint32_t ret;
> +
> +     ret = REG_SET_FIELD(0, SPI_GDBG_PER_VMID_CNTL, EXCP_EN,
> excp_en);
> +     ret = REG_SET_FIELD(ret, SPI_GDBG_PER_VMID_CNTL,
> TRAP_ON_START, trap_on_start);
> +     ret = REG_SET_FIELD(ret, SPI_GDBG_PER_VMID_CNTL,
> TRAP_ON_END, trap_on_end);
> +
> +     return ret;
> +}
> +
> +static uint32_t trap_mask_map_hw_to_sw(uint32_t mask)
> +{
> +     uint32_t ret = REG_GET_FIELD(mask, SPI_GDBG_PER_VMID_CNTL,
> EXCP_EN);
> +
> +     if (REG_GET_FIELD(mask, SPI_GDBG_PER_VMID_CNTL,
> TRAP_ON_START))
> +             ret |= KFD_DBG_TRAP_MASK_TRAP_ON_WAVE_START;
> +
> +     if (REG_GET_FIELD(mask, SPI_GDBG_PER_VMID_CNTL,
> TRAP_ON_END))
> +             ret |= KFD_DBG_TRAP_MASK_TRAP_ON_WAVE_END;
> +
> +     return ret;
> +}
> +
> +/* returns TRAP_EN, EXCP_EN and EXCP_REPLACE. */
> +static uint32_t kgd_gfx_v9_4_3_set_wave_launch_trap_override(
> +                             struct amdgpu_device *adev,
> +                             uint32_t vmid,
> +                             uint32_t trap_override,
> +                             uint32_t trap_mask_bits,
> +                             uint32_t trap_mask_request,
> +                             uint32_t *trap_mask_prev,
> +                             uint32_t kfd_dbg_trap_cntl_prev)
> +
> +{
> +     uint32_t data = 0;
> +
> +     *trap_mask_prev =
> trap_mask_map_hw_to_sw(kfd_dbg_trap_cntl_prev);
> +
> +     data = (trap_mask_bits & trap_mask_request) |
> +            (*trap_mask_prev & ~trap_mask_request);
> +     data = trap_mask_map_sw_to_hw(data);
> +
> +     data = REG_SET_FIELD(data, SPI_GDBG_PER_VMID_CNTL, TRAP_EN,
> 1);
> +     data = REG_SET_FIELD(data, SPI_GDBG_PER_VMID_CNTL,
> EXCP_REPLACE, trap_override);
> +
> +     return data;
> +}
> +
> +#define TCP_WATCH_STRIDE (regTCP_WATCH1_ADDR_H -
> regTCP_WATCH0_ADDR_H)
> +static uint32_t kgd_gfx_v9_4_3_set_address_watch(
> +                             struct amdgpu_device *adev,
> +                             uint64_t watch_address,
> +                             uint32_t watch_address_mask,
> +                             uint32_t watch_id,
> +                             uint32_t watch_mode,
> +                             uint32_t debug_vmid,
> +                             uint32_t inst)
> +{
> +     uint32_t watch_address_high;
> +     uint32_t watch_address_low;
> +     uint32_t watch_address_cntl;
> +
> +     watch_address_cntl = 0;
> +     watch_address_low = lower_32_bits(watch_address);
> +     watch_address_high = upper_32_bits(watch_address) & 0xffff;
> +
> +     watch_address_cntl = REG_SET_FIELD(watch_address_cntl,
> +                     TCP_WATCH0_CNTL,
> +                     MODE,
> +                     watch_mode);
> +
> +     watch_address_cntl = REG_SET_FIELD(watch_address_cntl,
> +                     TCP_WATCH0_CNTL,
> +                     MASK,
> +                     watch_address_mask >> 7);
> +
> +     watch_address_cntl = REG_SET_FIELD(watch_address_cntl,
> +                     TCP_WATCH0_CNTL,
> +                     VALID,
> +                     1);
> +
> +     WREG32_RLC((SOC15_REG_OFFSET(GC, GET_INST(GC, inst),
> +                     regTCP_WATCH0_ADDR_H) +
> +                     (watch_id * TCP_WATCH_STRIDE)),
> +                     watch_address_high);
> +
> +     WREG32_RLC((SOC15_REG_OFFSET(GC, GET_INST(GC, inst),
> +                     regTCP_WATCH0_ADDR_L) +
> +                     (watch_id * TCP_WATCH_STRIDE)),
> +                     watch_address_low);
> +
> +     return watch_address_cntl;
> +}
> +
> +static uint32_t kgd_gfx_v9_4_3_clear_address_watch(struct amdgpu_device
> *adev,
> +                             uint32_t watch_id)
> +{
> +     return 0;
> +}
> +
>  const struct kfd2kgd_calls gc_9_4_3_kfd2kgd = {
>       .program_sh_mem_settings =
> kgd_gfx_v9_program_sh_mem_settings,
>       .set_pasid_vmid_mapping =
> kgd_gfx_v9_4_3_set_pasid_vmid_mapping,
> @@ -380,5 +518,17 @@ const struct kfd2kgd_calls gc_9_4_3_kfd2kgd = {
>       .set_vm_context_page_table_base =
>
>       kgd_gfx_v9_set_vm_context_page_table_base,
>       .program_trap_handler_settings =
> -                             kgd_gfx_v9_program_trap_handler_settings
> +                             kgd_gfx_v9_program_trap_handler_settings,
> +     .build_grace_period_packet_info =
> +                             kgd_gfx_v9_build_grace_period_packet_info,
> +     .get_iq_wait_times = kgd_gfx_v9_get_iq_wait_times,
> +     .enable_debug_trap = kgd_aldebaran_enable_debug_trap,
> +     .disable_debug_trap = kgd_aldebaran_disable_debug_trap,

Let's implement disable_debug_trap here instead of referencing GC 9.4.2.
That way we can drop patch 5.

Thanks,

Jon

> +     .validate_trap_override_request =
> +                     kgd_gfx_v9_4_3_validate_trap_override_request,
> +     .set_wave_launch_trap_override =
> +                     kgd_gfx_v9_4_3_set_wave_launch_trap_override,
> +     .set_wave_launch_mode = kgd_aldebaran_set_wave_launch_mode,
> +     .set_address_watch = kgd_gfx_v9_4_3_set_address_watch,
> +     .clear_address_watch = kgd_gfx_v9_4_3_clear_address_watch
>  };
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> index 8ad7a7779e14..f1f2c24de081 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> @@ -886,7 +886,8 @@ uint32_t kgd_gfx_v10_set_address_watch(struct
> amdgpu_device *adev,
>                                       uint32_t watch_address_mask,
>                                       uint32_t watch_id,
>                                       uint32_t watch_mode,
> -                                     uint32_t debug_vmid)
> +                                     uint32_t debug_vmid,
> +                                     uint32_t inst)
>  {
>       uint32_t watch_address_high;
>       uint32_t watch_address_low;
> @@ -968,7 +969,8 @@ uint32_t kgd_gfx_v10_clear_address_watch(struct
> amdgpu_device *adev,
>   *     deq_retry_wait_time      -- Wait Count for Global Wave Syncs.
>   */
>  void kgd_gfx_v10_get_iq_wait_times(struct amdgpu_device *adev,
> -                                     uint32_t *wait_times)
> +                                     uint32_t *wait_times,
> +                                     uint32_t inst)
>
>  {
>       *wait_times = RREG32(SOC15_REG_OFFSET(GC, 0,
> mmCP_IQ_WAIT_TIME2));
> @@ -978,7 +980,8 @@ void
> kgd_gfx_v10_build_grace_period_packet_info(struct amdgpu_device *adev,
>                                               uint32_t wait_times,
>                                               uint32_t grace_period,
>                                               uint32_t *reg_offset,
> -                                             uint32_t *reg_data)
> +                                             uint32_t *reg_data,
> +                                             uint32_t inst)
>  {
>       *reg_data = wait_times;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.h
> index e6b70196071a..ecaead24e8c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.h
> @@ -44,12 +44,16 @@ uint32_t kgd_gfx_v10_set_address_watch(struct
> amdgpu_device *adev,
>                                       uint32_t watch_address_mask,
>                                       uint32_t watch_id,
>                                       uint32_t watch_mode,
> -                                     uint32_t debug_vmid);
> +                                     uint32_t debug_vmid,
> +                                     uint32_t inst);
>  uint32_t kgd_gfx_v10_clear_address_watch(struct amdgpu_device *adev,
>                                       uint32_t watch_id);
> -void kgd_gfx_v10_get_iq_wait_times(struct amdgpu_device *adev, uint32_t
> *wait_times);
> +void kgd_gfx_v10_get_iq_wait_times(struct amdgpu_device *adev,
> +                             uint32_t *wait_times,
> +                             uint32_t inst);
>  void kgd_gfx_v10_build_grace_period_packet_info(struct amdgpu_device
> *adev,
>                                              uint32_t wait_times,
>                                              uint32_t grace_period,
>                                              uint32_t *reg_offset,
> -                                            uint32_t *reg_data);
> +                                            uint32_t *reg_data,
> +                                            uint32_t inst);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v11.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v11.c
> index 91c3574ebed3..77ca5cbfb601 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v11.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v11.c
> @@ -743,7 +743,8 @@ static uint32_t kgd_gfx_v11_set_address_watch(struct
> amdgpu_device *adev,
>                                       uint32_t watch_address_mask,
>                                       uint32_t watch_id,
>                                       uint32_t watch_mode,
> -                                     uint32_t debug_vmid)
> +                                     uint32_t debug_vmid,
> +                                     uint32_t inst)
>  {
>       uint32_t watch_address_high;
>       uint32_t watch_address_low;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> index 51d93fb13ea3..7b1eea493377 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> @@ -822,7 +822,8 @@ uint32_t kgd_gfx_v9_set_address_watch(struct
> amdgpu_device *adev,
>                                       uint32_t watch_address_mask,
>                                       uint32_t watch_id,
>                                       uint32_t watch_mode,
> -                                     uint32_t debug_vmid)
> +                                     uint32_t debug_vmid,
> +                                     uint32_t inst)
>  {
>       uint32_t watch_address_high;
>       uint32_t watch_address_low;
> @@ -903,10 +904,12 @@ uint32_t kgd_gfx_v9_clear_address_watch(struct
> amdgpu_device *adev,
>   *     deq_retry_wait_time      -- Wait Count for Global Wave Syncs.
>   */
>  void kgd_gfx_v9_get_iq_wait_times(struct amdgpu_device *adev,
> -                                     uint32_t *wait_times)
> +                                     uint32_t *wait_times,
> +                                     uint32_t inst)
>
>  {
> -     *wait_times = RREG32(SOC15_REG_OFFSET(GC, 0,
> mmCP_IQ_WAIT_TIME2));
> +     *wait_times = RREG32(SOC15_REG_OFFSET(GC, GET_INST(GC, inst),
> +                     mmCP_IQ_WAIT_TIME2));
>  }
>
>  void kgd_gfx_v9_set_vm_context_page_table_base(struct amdgpu_device
> *adev,
> @@ -1100,7 +1103,8 @@ void
> kgd_gfx_v9_build_grace_period_packet_info(struct amdgpu_device *adev,
>               uint32_t wait_times,
>               uint32_t grace_period,
>               uint32_t *reg_offset,
> -             uint32_t *reg_data)
> +             uint32_t *reg_data,
> +             uint32_t inst)
>  {
>       *reg_data = wait_times;
>
> @@ -1116,7 +1120,8 @@ void
> kgd_gfx_v9_build_grace_period_packet_info(struct amdgpu_device *adev,
>                       SCH_WAVE,
>                       grace_period);
>
> -     *reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_IQ_WAIT_TIME2);
> +     *reg_offset = SOC15_REG_OFFSET(GC, GET_INST(GC, inst),
> +                     mmCP_IQ_WAIT_TIME2);
>  }
>
>  void kgd_gfx_v9_program_trap_handler_settings(struct amdgpu_device
> *adev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> index 5f54bff0db49..936e501908ce 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> @@ -89,12 +89,16 @@ uint32_t kgd_gfx_v9_set_address_watch(struct
> amdgpu_device *adev,
>                                       uint32_t watch_address_mask,
>                                       uint32_t watch_id,
>                                       uint32_t watch_mode,
> -                                     uint32_t debug_vmid);
> +                                     uint32_t debug_vmid,
> +                                     uint32_t inst);
>  uint32_t kgd_gfx_v9_clear_address_watch(struct amdgpu_device *adev,
>                                       uint32_t watch_id);
> -void kgd_gfx_v9_get_iq_wait_times(struct amdgpu_device *adev, uint32_t
> *wait_times);
> +void kgd_gfx_v9_get_iq_wait_times(struct amdgpu_device *adev,
> +                             uint32_t *wait_times,
> +                             uint32_t inst);
>  void kgd_gfx_v9_build_grace_period_packet_info(struct amdgpu_device
> *adev,
>                                              uint32_t wait_times,
>                                              uint32_t grace_period,
>                                              uint32_t *reg_offset,
> -                                            uint32_t *reg_data);
> +                                            uint32_t *reg_data,
> +                                            uint32_t inst);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> index fff3ccc04fa9..24083db44724 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> @@ -466,7 +466,8 @@ int kfd_dbg_trap_set_dev_address_watch(struct
> kfd_process_device *pdd,
>                               watch_address_mask,
>                               *watch_id,
>                               watch_mode,
> -                             pdd->dev->vm_info.last_vmid_kfd);
> +                             pdd->dev->vm_info.last_vmid_kfd,
> +                             0);
>       amdgpu_gfx_off_ctrl(pdd->dev->adev, true);
>
>       if (!pdd->dev->kfd->shared_resources.enable_mes)
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index f515cb8f30ca..a2bff3f01359 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -1621,7 +1621,7 @@ static int initialize_cpsch(struct
> device_queue_manager *dqm)
>
>       if (dqm->dev->kfd2kgd->get_iq_wait_times)
>               dqm->dev->kfd2kgd->get_iq_wait_times(dqm->dev->adev,
> -                                     &dqm->wait_times);
> +                                     &dqm->wait_times, 0);
>       return 0;
>  }
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> index 29a2d0499b67..8fda16e6fee6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> @@ -298,7 +298,8 @@ static int pm_set_grace_period_v9(struct
> packet_manager *pm,
>                       pm->dqm->wait_times,
>                       grace_period,
>                       &reg_offset,
> -                     &reg_data);
> +                     &reg_data,
> +                     0);
>
>       if (grace_period == USE_DEFAULT_GRACE_PERIOD)
>               reg_data = pm->dqm->wait_times;
> diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> index d0df3381539f..8433f99f6667 100644
> --- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> @@ -315,16 +315,19 @@ struct kfd2kgd_calls {
>                                       uint32_t watch_address_mask,
>                                       uint32_t watch_id,
>                                       uint32_t watch_mode,
> -                                     uint32_t debug_vmid);
> +                                     uint32_t debug_vmid,
> +                                     uint32_t inst);
>       uint32_t (*clear_address_watch)(struct amdgpu_device *adev,
>                       uint32_t watch_id);
>       void (*get_iq_wait_times)(struct amdgpu_device *adev,
> -                     uint32_t *wait_times);
> +                     uint32_t *wait_times,
> +                     uint32_t inst);
>       void (*build_grace_period_packet_info)(struct amdgpu_device *adev,
>                       uint32_t wait_times,
>                       uint32_t grace_period,
>                       uint32_t *reg_offset,
> -                     uint32_t *reg_data);
> +                     uint32_t *reg_data,
> +                     uint32_t inst);
>       void (*get_cu_occupancy)(struct amdgpu_device *adev, int pasid,
>                       int *wave_cnt, int *max_waves_per_cu, uint32_t
> inst);
>       void (*program_trap_handler_settings)(struct amdgpu_device *adev,
> --
> 2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 5/6] drm/amdkfd: always keep trap enabled for GC v9.4.3
  2023-07-06 18:19 ` [PATCH 5/6] drm/amdkfd: always keep trap enabled for GC v9.4.3 Eric Huang
@ 2023-07-07 14:39   ` Kim, Jonathan
  0 siblings, 0 replies; 17+ messages in thread
From: Kim, Jonathan @ 2023-07-07 14:39 UTC (permalink / raw)
  To: Huang, JinHuiEric, amd-gfx

[Public]

If we implement this in the GC 9.4.3 KGD disable call in patch 1 (see comments for that one), then it will look less awkward and we can drop this.

Thanks,

Jon

> -----Original Message-----
> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> Sent: Thursday, July 6, 2023 2:19 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Kim, Jonathan <Jonathan.Kim@amd.com>; Huang, JinHuiEric
> <JinHuiEric.Huang@amd.com>
> Subject: [PATCH 5/6] drm/amdkfd: always keep trap enabled for GC v9.4.3
>
> To set TTMP setup on by default.
>
> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_debug.c   | 3 ++-
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 6 +++---
>  3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index cf1db0ab3471..47c5d16677d6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -2842,7 +2842,7 @@ static int runtime_disable(struct kfd_process *p)
>                       pdd->spi_dbg_override =
>                                       pdd->dev->kfd2kgd-
> >disable_debug_trap(
>                                       pdd->dev->adev,
> -                                     false,
> +                                     KFD_GC_VERSION(pdd->dev) ==
> IP_VERSION(9, 4, 3),
>                                       pdd->dev->vm_info.last_vmid_kfd);
>
>                       if (!pdd->dev->kfd->shared_resources.enable_mes)
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> index 190b03efe5ff..4cb9b3b18065 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> @@ -591,7 +591,8 @@ void kfd_dbg_trap_deactivate(struct kfd_process
> *target, bool unwind, int unwind
>               pdd->spi_dbg_override =
>                               pdd->dev->kfd2kgd->disable_debug_trap(
>                               pdd->dev->adev,
> -                             target->runtime_info.ttmp_setup,
> +                             KFD_GC_VERSION(pdd->dev) ==
> IP_VERSION(9, 4, 3) ?
> +                                     true : target-
> >runtime_info.ttmp_setup,
>                               pdd->dev->vm_info.last_vmid_kfd);
>               amdgpu_gfx_off_ctrl(pdd->dev->adev, true);
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index ba04a4baecf2..91ae9121e2bf 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1644,9 +1644,9 @@ struct kfd_process_device
> *kfd_create_process_device_data(struct kfd_node *dev,
>       p->pdds[p->n_pdds++] = pdd;
>       if (kfd_dbg_is_per_vmid_supported(pdd->dev))
>               pdd->spi_dbg_override = pdd->dev->kfd2kgd-
> >disable_debug_trap(
> -                                                     pdd->dev->adev,
> -                                                     false,
> -                                                     0);
> +                             pdd->dev->adev,
> +                             KFD_GC_VERSION(dev) == IP_VERSION(9, 4,
> 3),
> +                             0);
>
>       /* Init idr used for memory handle translation */
>       idr_init(&pdd->alloc_idr);
> --
> 2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
  2023-07-06 18:19 ` [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance Eric Huang
@ 2023-07-07 14:59   ` Kim, Jonathan
  2023-07-07 15:45     ` Eric Huang
  0 siblings, 1 reply; 17+ messages in thread
From: Kim, Jonathan @ 2023-07-07 14:59 UTC (permalink / raw)
  To: Huang, JinHuiEric, amd-gfx

[Public]

> -----Original Message-----
> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> Sent: Thursday, July 6, 2023 2:19 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Kim, Jonathan <Jonathan.Kim@amd.com>; Huang, JinHuiEric
> <JinHuiEric.Huang@amd.com>
> Subject: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
>
> each xcc instance needs to get iq wait time and set
> grace period accordingly.
>
> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
> ---
>  .../drm/amd/amdkfd/kfd_device_queue_manager.c |  9 ++++--
>  .../drm/amd/amdkfd/kfd_device_queue_manager.h |  2 +-
>  .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   | 32 +++++++++++--------
>  .../drm/amd/amdkfd/kfd_packet_manager_v9.c    |  9 +++---
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  2 +-
>  5 files changed, 32 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index a2bff3f01359..0f12c1989e14 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -1606,6 +1606,8 @@ static int set_sched_resources(struct
> device_queue_manager *dqm)
>
>  static int initialize_cpsch(struct device_queue_manager *dqm)
>  {
> +     uint32_t xcc_id, xcc_mask = dqm->dev->xcc_mask;
> +
>       pr_debug("num of pipes: %d\n", get_pipes_per_mec(dqm));
>
>       mutex_init(&dqm->lock_hidden);
> @@ -1620,8 +1622,11 @@ static int initialize_cpsch(struct
> device_queue_manager *dqm)
>       init_sdma_bitmaps(dqm);
>
>       if (dqm->dev->kfd2kgd->get_iq_wait_times)
> -             dqm->dev->kfd2kgd->get_iq_wait_times(dqm->dev->adev,
> -                                     &dqm->wait_times, 0);
> +             for_each_inst(xcc_id, xcc_mask)
> +                     dqm->dev->kfd2kgd->get_iq_wait_times(
> +                                     dqm->dev->adev,
> +                                     &dqm->wait_times[xcc_id],
> +                                     xcc_id);
>       return 0;
>  }
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> index 7dd4b177219d..62a6dc8d3032 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> @@ -262,7 +262,7 @@ struct device_queue_manager {
>       /* used for GFX 9.4.3 only */
>       uint32_t                current_logical_xcc_start;
>
> -     uint32_t                wait_times;
> +     uint32_t                wait_times[32];

I think wait_times[16] should be sufficient.  We only get the hamming weight of 16 bits for NUM_XCC and I believe the xcc_mask is declared as a uint16_t in the KGD portion anyway.  We may as well align to that.

>
>       wait_queue_head_t       destroy_wait;
>  };
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> index 401096c103b2..f37ab4b6d88c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> @@ -374,27 +374,31 @@ int pm_update_grace_period(struct
> packet_manager *pm, uint32_t grace_period)
>  {
>       int retval = 0;
>       uint32_t *buffer, size;
> +     uint32_t xcc_id, xcc_mask = pm->dqm->dev->xcc_mask;
>
>       size = pm->pmf->set_grace_period_size;
>
>       mutex_lock(&pm->lock);
>
>       if (size) {
> -             kq_acquire_packet_buffer(pm->priv_queue,
> -                     size / sizeof(uint32_t),
> -                     (unsigned int **)&buffer);
> -
> -             if (!buffer) {
> -                     pr_err("Failed to allocate buffer on kernel queue\n");
> -                     retval = -ENOMEM;
> -                     goto out;
> -             }
> +             for_each_inst(xcc_id, xcc_mask) {
> +                     kq_acquire_packet_buffer(pm->priv_queue,
> +                                     size / sizeof(uint32_t),
> +                                     (unsigned int **)&buffer);
>
> -             retval = pm->pmf->set_grace_period(pm, buffer,
> grace_period);
> -             if (!retval)
> -                     kq_submit_packet(pm->priv_queue);
> -             else
> -                     kq_rollback_packet(pm->priv_queue);
> +                     if (!buffer) {
> +                             pr_err("Failed to allocate buffer on kernel
> queue\n");
> +                             retval = -ENOMEM;
> +                             goto out;
> +                     }
> +
> +                     retval = pm->pmf->set_grace_period(pm, buffer,
> +                                     grace_period, xcc_id);
> +                     if (!retval)
> +                             kq_submit_packet(pm->priv_queue);
> +                     else
> +                             kq_rollback_packet(pm->priv_queue);

In the event of partial success do we need to roll back (i.e. resubmit default grace period) on failure?
I believe the default grace period is put in place for better CWSR performance in normal mode, so leaving fast preemption settings on failure could impact performance.

Thanks,

Jon

> +             }
>       }
>
>  out:
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> index 8fda16e6fee6..a9443d661957 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> @@ -287,7 +287,8 @@ static int pm_map_queues_v9(struct packet_manager
> *pm, uint32_t *buffer,
>
>  static int pm_set_grace_period_v9(struct packet_manager *pm,
>               uint32_t *buffer,
> -             uint32_t grace_period)
> +             uint32_t grace_period,
> +             uint32_t inst)
>  {
>       struct pm4_mec_write_data_mmio *packet;
>       uint32_t reg_offset = 0;
> @@ -295,14 +296,14 @@ static int pm_set_grace_period_v9(struct
> packet_manager *pm,
>
>       pm->dqm->dev->kfd2kgd->build_grace_period_packet_info(
>                       pm->dqm->dev->adev,
> -                     pm->dqm->wait_times,
> +                     pm->dqm->wait_times[inst],
>                       grace_period,
>                       &reg_offset,
>                       &reg_data,
> -                     0);
> +                     inst);
>
>       if (grace_period == USE_DEFAULT_GRACE_PERIOD)
> -             reg_data = pm->dqm->wait_times;
> +             reg_data = pm->dqm->wait_times[inst];
>
>       packet = (struct pm4_mec_write_data_mmio *)buffer;
>       memset(buffer, 0, sizeof(struct pm4_mec_write_data_mmio));
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index d4c9ee3f9953..22c4a403ddd7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -1400,7 +1400,7 @@ struct packet_manager_funcs {
>                       enum kfd_unmap_queues_filter mode,
>                       uint32_t filter_param, bool reset);
>       int (*set_grace_period)(struct packet_manager *pm, uint32_t *buffer,
> -                     uint32_t grace_period);
> +                     uint32_t grace_period, uint32_t inst);
>       int (*query_status)(struct packet_manager *pm, uint32_t *buffer,
>                       uint64_t fence_address, uint64_t
> fence_value);
>       int (*release_mem)(uint64_t gpu_addr, uint32_t *buffer);
> --
> 2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 3/6] drm/amdkfd: enable watch points globally for gfx943
  2023-07-06 18:19 ` [PATCH 3/6] drm/amdkfd: enable watch points globally for gfx943 Eric Huang
@ 2023-07-07 15:04   ` Kim, Jonathan
  0 siblings, 0 replies; 17+ messages in thread
From: Kim, Jonathan @ 2023-07-07 15:04 UTC (permalink / raw)
  To: Huang, JinHuiEric, amd-gfx; +Cc: Kuehling, Felix

[Public]

> -----Original Message-----
> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> Sent: Thursday, July 6, 2023 2:19 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Kim, Jonathan <Jonathan.Kim@amd.com>; Kim, Jonathan
> <Jonathan.Kim@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>;
> Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> Subject: [PATCH 3/6] drm/amdkfd: enable watch points globally for gfx943
>
> From: Jonathan Kim <jonathan.kim@amd.com>
>
> Set watch points for all xcc instances on GFX943.
>
> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
> Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>

This patch is Reviewed-by: Jonathan Kim <jonathan.kim@amd.com>

> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_debug.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> index 24083db44724..190b03efe5ff 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> @@ -446,7 +446,8 @@ int kfd_dbg_trap_set_dev_address_watch(struct
> kfd_process_device *pdd,
>                                       uint32_t *watch_id,
>                                       uint32_t watch_mode)
>  {
> -     int r = kfd_dbg_get_dev_watch_id(pdd, watch_id);
> +     int xcc_id, r = kfd_dbg_get_dev_watch_id(pdd, watch_id);
> +     uint32_t xcc_mask = pdd->dev->xcc_mask;
>
>       if (r)
>               return r;
> @@ -460,14 +461,15 @@ int kfd_dbg_trap_set_dev_address_watch(struct
> kfd_process_device *pdd,
>       }
>
>       amdgpu_gfx_off_ctrl(pdd->dev->adev, false);
> -     pdd->watch_points[*watch_id] = pdd->dev->kfd2kgd-
> >set_address_watch(
> +     for_each_inst(xcc_id, xcc_mask)
> +             pdd->watch_points[*watch_id] = pdd->dev->kfd2kgd-
> >set_address_watch(
>                               pdd->dev->adev,
>                               watch_address,
>                               watch_address_mask,
>                               *watch_id,
>                               watch_mode,
>                               pdd->dev->vm_info.last_vmid_kfd,
> -                             0);
> +                             xcc_id);
>       amdgpu_gfx_off_ctrl(pdd->dev->adev, true);
>
>       if (!pdd->dev->kfd->shared_resources.enable_mes)
> --
> 2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 6/6] drm/amdkfd: add multi-process debugging support for GC v9.4.3
  2023-07-06 18:19 ` [PATCH 6/6] drm/amdkfd: add multi-process debugging support " Eric Huang
@ 2023-07-07 15:06   ` Kim, Jonathan
  0 siblings, 0 replies; 17+ messages in thread
From: Kim, Jonathan @ 2023-07-07 15:06 UTC (permalink / raw)
  To: Huang, JinHuiEric, amd-gfx; +Cc: Kuehling, Felix

[Public]

> -----Original Message-----
> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> Sent: Thursday, July 6, 2023 2:19 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Kim, Jonathan <Jonathan.Kim@amd.com>; Kim, Jonathan
> <Jonathan.Kim@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>;
> Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> Subject: [PATCH 6/6] drm/amdkfd: add multi-process debugging support for
> GC v9.4.3
>
> From: Jonathan Kim <jonathan.kim@amd.com>
>
> Similar to GC v9.4.2, GC v9.4.3 should use the 5-Dword extended
> MAP_PROCESS packet to support multi-process debugging.  Update the
> mutli-process debug support list so that the KFD updates the runlist
> on debug mode setting and that it allocates enough GTT memory during
> KFD device initialization.
>
> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
> Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>

This patch is Reviewed-by: Jonathan Kim <jonathan.kim@amd.com>

> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_debug.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> index a289e59ceb79..a0afc6a7b6c4 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> @@ -76,8 +76,9 @@ int kfd_dbg_send_exception_to_runtime(struct
> kfd_process *p,
>
>  static inline bool kfd_dbg_is_per_vmid_supported(struct kfd_node *dev)
>  {
> -     return KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
> -            KFD_GC_VERSION(dev) >= IP_VERSION(11, 0, 0);
> +     return (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
> +             KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 3) ||
> +             KFD_GC_VERSION(dev) >= IP_VERSION(11, 0, 0));
>  }
>
>  void debug_event_write_work_handler(struct work_struct *work);
> --
> 2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
  2023-07-07 14:59   ` Kim, Jonathan
@ 2023-07-07 15:45     ` Eric Huang
  2023-07-07 15:56       ` Kim, Jonathan
  0 siblings, 1 reply; 17+ messages in thread
From: Eric Huang @ 2023-07-07 15:45 UTC (permalink / raw)
  To: Kim, Jonathan, amd-gfx


On 2023-07-07 10:59, Kim, Jonathan wrote:
> [Public]
>
>> -----Original Message-----
>> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
>> Sent: Thursday, July 6, 2023 2:19 PM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Kim, Jonathan <Jonathan.Kim@amd.com>; Huang, JinHuiEric
>> <JinHuiEric.Huang@amd.com>
>> Subject: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
>>
>> each xcc instance needs to get iq wait time and set
>> grace period accordingly.
>>
>> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
>> ---
>>   .../drm/amd/amdkfd/kfd_device_queue_manager.c |  9 ++++--
>>   .../drm/amd/amdkfd/kfd_device_queue_manager.h |  2 +-
>>   .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   | 32 +++++++++++--------
>>   .../drm/amd/amdkfd/kfd_packet_manager_v9.c    |  9 +++---
>>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  2 +-
>>   5 files changed, 32 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> index a2bff3f01359..0f12c1989e14 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> @@ -1606,6 +1606,8 @@ static int set_sched_resources(struct
>> device_queue_manager *dqm)
>>
>>   static int initialize_cpsch(struct device_queue_manager *dqm)
>>   {
>> +     uint32_t xcc_id, xcc_mask = dqm->dev->xcc_mask;
>> +
>>        pr_debug("num of pipes: %d\n", get_pipes_per_mec(dqm));
>>
>>        mutex_init(&dqm->lock_hidden);
>> @@ -1620,8 +1622,11 @@ static int initialize_cpsch(struct
>> device_queue_manager *dqm)
>>        init_sdma_bitmaps(dqm);
>>
>>        if (dqm->dev->kfd2kgd->get_iq_wait_times)
>> -             dqm->dev->kfd2kgd->get_iq_wait_times(dqm->dev->adev,
>> -                                     &dqm->wait_times, 0);
>> +             for_each_inst(xcc_id, xcc_mask)
>> +                     dqm->dev->kfd2kgd->get_iq_wait_times(
>> +                                     dqm->dev->adev,
>> +                                     &dqm->wait_times[xcc_id],
>> +                                     xcc_id);
>>        return 0;
>>   }
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
>> index 7dd4b177219d..62a6dc8d3032 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
>> @@ -262,7 +262,7 @@ struct device_queue_manager {
>>        /* used for GFX 9.4.3 only */
>>        uint32_t                current_logical_xcc_start;
>>
>> -     uint32_t                wait_times;
>> +     uint32_t                wait_times[32];
> I think wait_times[16] should be sufficient.  We only get the hamming weight of 16 bits for NUM_XCC and I believe the xcc_mask is declared as a uint16_t in the KGD portion anyway.  We may as well align to that.
>
>>        wait_queue_head_t       destroy_wait;
>>   };
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
>> index 401096c103b2..f37ab4b6d88c 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
>> @@ -374,27 +374,31 @@ int pm_update_grace_period(struct
>> packet_manager *pm, uint32_t grace_period)
>>   {
>>        int retval = 0;
>>        uint32_t *buffer, size;
>> +     uint32_t xcc_id, xcc_mask = pm->dqm->dev->xcc_mask;
>>
>>        size = pm->pmf->set_grace_period_size;
>>
>>        mutex_lock(&pm->lock);
>>
>>        if (size) {
>> -             kq_acquire_packet_buffer(pm->priv_queue,
>> -                     size / sizeof(uint32_t),
>> -                     (unsigned int **)&buffer);
>> -
>> -             if (!buffer) {
>> -                     pr_err("Failed to allocate buffer on kernel queue\n");
>> -                     retval = -ENOMEM;
>> -                     goto out;
>> -             }
>> +             for_each_inst(xcc_id, xcc_mask) {
>> +                     kq_acquire_packet_buffer(pm->priv_queue,
>> +                                     size / sizeof(uint32_t),
>> +                                     (unsigned int **)&buffer);
>>
>> -             retval = pm->pmf->set_grace_period(pm, buffer,
>> grace_period);
>> -             if (!retval)
>> -                     kq_submit_packet(pm->priv_queue);
>> -             else
>> -                     kq_rollback_packet(pm->priv_queue);
>> +                     if (!buffer) {
>> +                             pr_err("Failed to allocate buffer on kernel
>> queue\n");
>> +                             retval = -ENOMEM;
>> +                             goto out;
>> +                     }
>> +
>> +                     retval = pm->pmf->set_grace_period(pm, buffer,
>> +                                     grace_period, xcc_id);
>> +                     if (!retval)
>> +                             kq_submit_packet(pm->priv_queue);
>> +                     else
>> +                             kq_rollback_packet(pm->priv_queue);
> In the event of partial success do we need to roll back (i.e. resubmit default grace period) on failure?
The function pm_set_grace_period_v9 always return 0, and it is not 
complicate operation, it should be always successful. Partial success 
will not be the case we should care about at this moment.

Regards,
Eric
> I believe the default grace period is put in place for better CWSR performance in normal mode, so leaving fast preemption settings on failure could impact performance.
>
> Thanks,
>
> Jon
>
>> +             }
>>        }
>>
>>   out:
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>> index 8fda16e6fee6..a9443d661957 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>> @@ -287,7 +287,8 @@ static int pm_map_queues_v9(struct packet_manager
>> *pm, uint32_t *buffer,
>>
>>   static int pm_set_grace_period_v9(struct packet_manager *pm,
>>                uint32_t *buffer,
>> -             uint32_t grace_period)
>> +             uint32_t grace_period,
>> +             uint32_t inst)
>>   {
>>        struct pm4_mec_write_data_mmio *packet;
>>        uint32_t reg_offset = 0;
>> @@ -295,14 +296,14 @@ static int pm_set_grace_period_v9(struct
>> packet_manager *pm,
>>
>>        pm->dqm->dev->kfd2kgd->build_grace_period_packet_info(
>>                        pm->dqm->dev->adev,
>> -                     pm->dqm->wait_times,
>> +                     pm->dqm->wait_times[inst],
>>                        grace_period,
>>                        &reg_offset,
>>                        &reg_data,
>> -                     0);
>> +                     inst);
>>
>>        if (grace_period == USE_DEFAULT_GRACE_PERIOD)
>> -             reg_data = pm->dqm->wait_times;
>> +             reg_data = pm->dqm->wait_times[inst];
>>
>>        packet = (struct pm4_mec_write_data_mmio *)buffer;
>>        memset(buffer, 0, sizeof(struct pm4_mec_write_data_mmio));
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> index d4c9ee3f9953..22c4a403ddd7 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> @@ -1400,7 +1400,7 @@ struct packet_manager_funcs {
>>                        enum kfd_unmap_queues_filter mode,
>>                        uint32_t filter_param, bool reset);
>>        int (*set_grace_period)(struct packet_manager *pm, uint32_t *buffer,
>> -                     uint32_t grace_period);
>> +                     uint32_t grace_period, uint32_t inst);
>>        int (*query_status)(struct packet_manager *pm, uint32_t *buffer,
>>                        uint64_t fence_address, uint64_t
>> fence_value);
>>        int (*release_mem)(uint64_t gpu_addr, uint32_t *buffer);
>> --
>> 2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
  2023-07-07 15:45     ` Eric Huang
@ 2023-07-07 15:56       ` Kim, Jonathan
  2023-07-07 16:44         ` Eric Huang
  0 siblings, 1 reply; 17+ messages in thread
From: Kim, Jonathan @ 2023-07-07 15:56 UTC (permalink / raw)
  To: Huang, JinHuiEric, amd-gfx; +Cc: Joshi, Mukul

[Public]

> -----Original Message-----
> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> Sent: Friday, July 7, 2023 11:46 AM
> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
>
>
> On 2023-07-07 10:59, Kim, Jonathan wrote:
> > [Public]
> >
> >> -----Original Message-----
> >> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> >> Sent: Thursday, July 6, 2023 2:19 PM
> >> To: amd-gfx@lists.freedesktop.org
> >> Cc: Kim, Jonathan <Jonathan.Kim@amd.com>; Huang, JinHuiEric
> >> <JinHuiEric.Huang@amd.com>
> >> Subject: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
> >>
> >> each xcc instance needs to get iq wait time and set
> >> grace period accordingly.
> >>
> >> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
> >> ---
> >>   .../drm/amd/amdkfd/kfd_device_queue_manager.c |  9 ++++--
> >>   .../drm/amd/amdkfd/kfd_device_queue_manager.h |  2 +-
> >>   .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   | 32 +++++++++++-------
> -
> >>   .../drm/amd/amdkfd/kfd_packet_manager_v9.c    |  9 +++---
> >>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  2 +-
> >>   5 files changed, 32 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> >> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> >> index a2bff3f01359..0f12c1989e14 100644
> >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> >> @@ -1606,6 +1606,8 @@ static int set_sched_resources(struct
> >> device_queue_manager *dqm)
> >>
> >>   static int initialize_cpsch(struct device_queue_manager *dqm)
> >>   {
> >> +     uint32_t xcc_id, xcc_mask = dqm->dev->xcc_mask;
> >> +
> >>        pr_debug("num of pipes: %d\n", get_pipes_per_mec(dqm));
> >>
> >>        mutex_init(&dqm->lock_hidden);
> >> @@ -1620,8 +1622,11 @@ static int initialize_cpsch(struct
> >> device_queue_manager *dqm)
> >>        init_sdma_bitmaps(dqm);
> >>
> >>        if (dqm->dev->kfd2kgd->get_iq_wait_times)
> >> -             dqm->dev->kfd2kgd->get_iq_wait_times(dqm->dev->adev,
> >> -                                     &dqm->wait_times, 0);
> >> +             for_each_inst(xcc_id, xcc_mask)
> >> +                     dqm->dev->kfd2kgd->get_iq_wait_times(
> >> +                                     dqm->dev->adev,
> >> +                                     &dqm->wait_times[xcc_id],
> >> +                                     xcc_id);
> >>        return 0;
> >>   }
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> >> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> >> index 7dd4b177219d..62a6dc8d3032 100644
> >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> >> @@ -262,7 +262,7 @@ struct device_queue_manager {
> >>        /* used for GFX 9.4.3 only */
> >>        uint32_t                current_logical_xcc_start;
> >>
> >> -     uint32_t                wait_times;
> >> +     uint32_t                wait_times[32];
> > I think wait_times[16] should be sufficient.  We only get the hamming
> weight of 16 bits for NUM_XCC and I believe the xcc_mask is declared as a
> uint16_t in the KGD portion anyway.  We may as well align to that.
> >
> >>        wait_queue_head_t       destroy_wait;
> >>   };
> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> >> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> >> index 401096c103b2..f37ab4b6d88c 100644
> >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> >> @@ -374,27 +374,31 @@ int pm_update_grace_period(struct
> >> packet_manager *pm, uint32_t grace_period)
> >>   {
> >>        int retval = 0;
> >>        uint32_t *buffer, size;
> >> +     uint32_t xcc_id, xcc_mask = pm->dqm->dev->xcc_mask;
> >>
> >>        size = pm->pmf->set_grace_period_size;
> >>
> >>        mutex_lock(&pm->lock);
> >>
> >>        if (size) {
> >> -             kq_acquire_packet_buffer(pm->priv_queue,
> >> -                     size / sizeof(uint32_t),
> >> -                     (unsigned int **)&buffer);
> >> -
> >> -             if (!buffer) {
> >> -                     pr_err("Failed to allocate buffer on kernel queue\n");
> >> -                     retval = -ENOMEM;
> >> -                     goto out;
> >> -             }
> >> +             for_each_inst(xcc_id, xcc_mask) {
> >> +                     kq_acquire_packet_buffer(pm->priv_queue,
> >> +                                     size / sizeof(uint32_t),
> >> +                                     (unsigned int **)&buffer);
> >>
> >> -             retval = pm->pmf->set_grace_period(pm, buffer,
> >> grace_period);
> >> -             if (!retval)
> >> -                     kq_submit_packet(pm->priv_queue);
> >> -             else
> >> -                     kq_rollback_packet(pm->priv_queue);
> >> +                     if (!buffer) {
> >> +                             pr_err("Failed to allocate buffer on kernel
> >> queue\n");
> >> +                             retval = -ENOMEM;
> >> +                             goto out;
> >> +                     }
> >> +
> >> +                     retval = pm->pmf->set_grace_period(pm, buffer,
> >> +                                     grace_period, xcc_id);
> >> +                     if (!retval)
> >> +                             kq_submit_packet(pm->priv_queue);
> >> +                     else
> >> +                             kq_rollback_packet(pm->priv_queue);
> > In the event of partial success do we need to roll back (i.e. resubmit default
> grace period) on failure?
> The function pm_set_grace_period_v9 always return 0, and it is not
> complicate operation, it should be always successful. Partial success
> will not be the case we should care about at this moment.

There's a roll back logic already built-in prior to this that's now only partial with this patch.
Either way, after a side discussion with Mukul, it looks like the CP register to set the default grace period is done by the master XCC per partition so we'd can just stick to the current way of setting the grace period as long as we reference the start_xcc_id correctly on read and write.
Plus the suspend call will set the grace period per partition, so the wait_times array wasn't required in the first place.

Thanks,

Jon

>
> Regards,
> Eric
> > I believe the default grace period is put in place for better CWSR
> performance in normal mode, so leaving fast preemption settings on failure
> could impact performance.
> >
> > Thanks,
> >
> > Jon
> >
> >> +             }
> >>        }
> >>
> >>   out:
> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> >> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> >> index 8fda16e6fee6..a9443d661957 100644
> >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> >> @@ -287,7 +287,8 @@ static int pm_map_queues_v9(struct
> packet_manager
> >> *pm, uint32_t *buffer,
> >>
> >>   static int pm_set_grace_period_v9(struct packet_manager *pm,
> >>                uint32_t *buffer,
> >> -             uint32_t grace_period)
> >> +             uint32_t grace_period,
> >> +             uint32_t inst)
> >>   {
> >>        struct pm4_mec_write_data_mmio *packet;
> >>        uint32_t reg_offset = 0;
> >> @@ -295,14 +296,14 @@ static int pm_set_grace_period_v9(struct
> >> packet_manager *pm,
> >>
> >>        pm->dqm->dev->kfd2kgd->build_grace_period_packet_info(
> >>                        pm->dqm->dev->adev,
> >> -                     pm->dqm->wait_times,
> >> +                     pm->dqm->wait_times[inst],
> >>                        grace_period,
> >>                        &reg_offset,
> >>                        &reg_data,
> >> -                     0);
> >> +                     inst);
> >>
> >>        if (grace_period == USE_DEFAULT_GRACE_PERIOD)
> >> -             reg_data = pm->dqm->wait_times;
> >> +             reg_data = pm->dqm->wait_times[inst];
> >>
> >>        packet = (struct pm4_mec_write_data_mmio *)buffer;
> >>        memset(buffer, 0, sizeof(struct pm4_mec_write_data_mmio));
> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> >> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> >> index d4c9ee3f9953..22c4a403ddd7 100644
> >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> >> @@ -1400,7 +1400,7 @@ struct packet_manager_funcs {
> >>                        enum kfd_unmap_queues_filter mode,
> >>                        uint32_t filter_param, bool reset);
> >>        int (*set_grace_period)(struct packet_manager *pm, uint32_t *buffer,
> >> -                     uint32_t grace_period);
> >> +                     uint32_t grace_period, uint32_t inst);
> >>        int (*query_status)(struct packet_manager *pm, uint32_t *buffer,
> >>                        uint64_t fence_address, uint64_t
> >> fence_value);
> >>        int (*release_mem)(uint64_t gpu_addr, uint32_t *buffer);
> >> --
> >> 2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
  2023-07-07 15:56       ` Kim, Jonathan
@ 2023-07-07 16:44         ` Eric Huang
  2023-07-07 17:06           ` Kim, Jonathan
  0 siblings, 1 reply; 17+ messages in thread
From: Eric Huang @ 2023-07-07 16:44 UTC (permalink / raw)
  To: Kim, Jonathan, amd-gfx; +Cc: Joshi, Mukul


On 2023-07-07 11:56, Kim, Jonathan wrote:
> [Public]
>
>> -----Original Message-----
>> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
>> Sent: Friday, July 7, 2023 11:46 AM
>> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
>>
>>
>> On 2023-07-07 10:59, Kim, Jonathan wrote:
>>> [Public]
>>>
>>>> -----Original Message-----
>>>> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
>>>> Sent: Thursday, July 6, 2023 2:19 PM
>>>> To: amd-gfx@lists.freedesktop.org
>>>> Cc: Kim, Jonathan <Jonathan.Kim@amd.com>; Huang, JinHuiEric
>>>> <JinHuiEric.Huang@amd.com>
>>>> Subject: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
>>>>
>>>> each xcc instance needs to get iq wait time and set
>>>> grace period accordingly.
>>>>
>>>> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
>>>> ---
>>>>    .../drm/amd/amdkfd/kfd_device_queue_manager.c |  9 ++++--
>>>>    .../drm/amd/amdkfd/kfd_device_queue_manager.h |  2 +-
>>>>    .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   | 32 +++++++++++-------
>> -
>>>>    .../drm/amd/amdkfd/kfd_packet_manager_v9.c    |  9 +++---
>>>>    drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  2 +-
>>>>    5 files changed, 32 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>>> index a2bff3f01359..0f12c1989e14 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>>> @@ -1606,6 +1606,8 @@ static int set_sched_resources(struct
>>>> device_queue_manager *dqm)
>>>>
>>>>    static int initialize_cpsch(struct device_queue_manager *dqm)
>>>>    {
>>>> +     uint32_t xcc_id, xcc_mask = dqm->dev->xcc_mask;
>>>> +
>>>>         pr_debug("num of pipes: %d\n", get_pipes_per_mec(dqm));
>>>>
>>>>         mutex_init(&dqm->lock_hidden);
>>>> @@ -1620,8 +1622,11 @@ static int initialize_cpsch(struct
>>>> device_queue_manager *dqm)
>>>>         init_sdma_bitmaps(dqm);
>>>>
>>>>         if (dqm->dev->kfd2kgd->get_iq_wait_times)
>>>> -             dqm->dev->kfd2kgd->get_iq_wait_times(dqm->dev->adev,
>>>> -                                     &dqm->wait_times, 0);
>>>> +             for_each_inst(xcc_id, xcc_mask)
>>>> +                     dqm->dev->kfd2kgd->get_iq_wait_times(
>>>> +                                     dqm->dev->adev,
>>>> +                                     &dqm->wait_times[xcc_id],
>>>> +                                     xcc_id);
>>>>         return 0;
>>>>    }
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
>>>> index 7dd4b177219d..62a6dc8d3032 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
>>>> @@ -262,7 +262,7 @@ struct device_queue_manager {
>>>>         /* used for GFX 9.4.3 only */
>>>>         uint32_t                current_logical_xcc_start;
>>>>
>>>> -     uint32_t                wait_times;
>>>> +     uint32_t                wait_times[32];
>>> I think wait_times[16] should be sufficient.  We only get the hamming
>> weight of 16 bits for NUM_XCC and I believe the xcc_mask is declared as a
>> uint16_t in the KGD portion anyway.  We may as well align to that.
>>>>         wait_queue_head_t       destroy_wait;
>>>>    };
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
>>>> index 401096c103b2..f37ab4b6d88c 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
>>>> @@ -374,27 +374,31 @@ int pm_update_grace_period(struct
>>>> packet_manager *pm, uint32_t grace_period)
>>>>    {
>>>>         int retval = 0;
>>>>         uint32_t *buffer, size;
>>>> +     uint32_t xcc_id, xcc_mask = pm->dqm->dev->xcc_mask;
>>>>
>>>>         size = pm->pmf->set_grace_period_size;
>>>>
>>>>         mutex_lock(&pm->lock);
>>>>
>>>>         if (size) {
>>>> -             kq_acquire_packet_buffer(pm->priv_queue,
>>>> -                     size / sizeof(uint32_t),
>>>> -                     (unsigned int **)&buffer);
>>>> -
>>>> -             if (!buffer) {
>>>> -                     pr_err("Failed to allocate buffer on kernel queue\n");
>>>> -                     retval = -ENOMEM;
>>>> -                     goto out;
>>>> -             }
>>>> +             for_each_inst(xcc_id, xcc_mask) {
>>>> +                     kq_acquire_packet_buffer(pm->priv_queue,
>>>> +                                     size / sizeof(uint32_t),
>>>> +                                     (unsigned int **)&buffer);
>>>>
>>>> -             retval = pm->pmf->set_grace_period(pm, buffer,
>>>> grace_period);
>>>> -             if (!retval)
>>>> -                     kq_submit_packet(pm->priv_queue);
>>>> -             else
>>>> -                     kq_rollback_packet(pm->priv_queue);
>>>> +                     if (!buffer) {
>>>> +                             pr_err("Failed to allocate buffer on kernel
>>>> queue\n");
>>>> +                             retval = -ENOMEM;
>>>> +                             goto out;
>>>> +                     }
>>>> +
>>>> +                     retval = pm->pmf->set_grace_period(pm, buffer,
>>>> +                                     grace_period, xcc_id);
>>>> +                     if (!retval)
>>>> +                             kq_submit_packet(pm->priv_queue);
>>>> +                     else
>>>> +                             kq_rollback_packet(pm->priv_queue);
>>> In the event of partial success do we need to roll back (i.e. resubmit default
>> grace period) on failure?
>> The function pm_set_grace_period_v9 always return 0, and it is not
>> complicate operation, it should be always successful. Partial success
>> will not be the case we should care about at this moment.
> There's a roll back logic already built-in prior to this that's now only partial with this patch.
> Either way, after a side discussion with Mukul, it looks like the CP register to set the default grace period is done by the master XCC per partition so we'd can just stick to the current way of setting the grace period as long as we reference the start_xcc_id correctly on read and write.
The start_xcc_id is not defined in amd_staging_drm_next. If we assume 0 
is always the first instance number, the instance parameter in function 
get_iq_wait_times and build_grace_period_packet_info will be not needed.

Regards,
Eric
> Plus the suspend call will set the grace period per partition, so the wait_times array wasn't required in the first place.
>
> Thanks,
>
> Jon
>
>> Regards,
>> Eric
>>> I believe the default grace period is put in place for better CWSR
>> performance in normal mode, so leaving fast preemption settings on failure
>> could impact performance.
>>> Thanks,
>>>
>>> Jon
>>>
>>>> +             }
>>>>         }
>>>>
>>>>    out:
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>>>> index 8fda16e6fee6..a9443d661957 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>>>> @@ -287,7 +287,8 @@ static int pm_map_queues_v9(struct
>> packet_manager
>>>> *pm, uint32_t *buffer,
>>>>
>>>>    static int pm_set_grace_period_v9(struct packet_manager *pm,
>>>>                 uint32_t *buffer,
>>>> -             uint32_t grace_period)
>>>> +             uint32_t grace_period,
>>>> +             uint32_t inst)
>>>>    {
>>>>         struct pm4_mec_write_data_mmio *packet;
>>>>         uint32_t reg_offset = 0;
>>>> @@ -295,14 +296,14 @@ static int pm_set_grace_period_v9(struct
>>>> packet_manager *pm,
>>>>
>>>>         pm->dqm->dev->kfd2kgd->build_grace_period_packet_info(
>>>>                         pm->dqm->dev->adev,
>>>> -                     pm->dqm->wait_times,
>>>> +                     pm->dqm->wait_times[inst],
>>>>                         grace_period,
>>>>                         &reg_offset,
>>>>                         &reg_data,
>>>> -                     0);
>>>> +                     inst);
>>>>
>>>>         if (grace_period == USE_DEFAULT_GRACE_PERIOD)
>>>> -             reg_data = pm->dqm->wait_times;
>>>> +             reg_data = pm->dqm->wait_times[inst];
>>>>
>>>>         packet = (struct pm4_mec_write_data_mmio *)buffer;
>>>>         memset(buffer, 0, sizeof(struct pm4_mec_write_data_mmio));
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>>> index d4c9ee3f9953..22c4a403ddd7 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>>> @@ -1400,7 +1400,7 @@ struct packet_manager_funcs {
>>>>                         enum kfd_unmap_queues_filter mode,
>>>>                         uint32_t filter_param, bool reset);
>>>>         int (*set_grace_period)(struct packet_manager *pm, uint32_t *buffer,
>>>> -                     uint32_t grace_period);
>>>> +                     uint32_t grace_period, uint32_t inst);
>>>>         int (*query_status)(struct packet_manager *pm, uint32_t *buffer,
>>>>                         uint64_t fence_address, uint64_t
>>>> fence_value);
>>>>         int (*release_mem)(uint64_t gpu_addr, uint32_t *buffer);
>>>> --
>>>> 2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
  2023-07-07 16:44         ` Eric Huang
@ 2023-07-07 17:06           ` Kim, Jonathan
  2023-07-07 19:08             ` Kim, Jonathan
  0 siblings, 1 reply; 17+ messages in thread
From: Kim, Jonathan @ 2023-07-07 17:06 UTC (permalink / raw)
  To: Huang, JinHuiEric, amd-gfx; +Cc: Joshi, Mukul

[Public]

> -----Original Message-----
> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> Sent: Friday, July 7, 2023 12:44 PM
> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Joshi, Mukul <Mukul.Joshi@amd.com>
> Subject: Re: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
>
>
> On 2023-07-07 11:56, Kim, Jonathan wrote:
> > [Public]
> >
> >> -----Original Message-----
> >> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> >> Sent: Friday, July 7, 2023 11:46 AM
> >> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-
> gfx@lists.freedesktop.org
> >> Subject: Re: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
> >>
> >>
> >> On 2023-07-07 10:59, Kim, Jonathan wrote:
> >>> [Public]
> >>>
> >>>> -----Original Message-----
> >>>> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> >>>> Sent: Thursday, July 6, 2023 2:19 PM
> >>>> To: amd-gfx@lists.freedesktop.org
> >>>> Cc: Kim, Jonathan <Jonathan.Kim@amd.com>; Huang, JinHuiEric
> >>>> <JinHuiEric.Huang@amd.com>
> >>>> Subject: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
> >>>>
> >>>> each xcc instance needs to get iq wait time and set
> >>>> grace period accordingly.
> >>>>
> >>>> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
> >>>> ---
> >>>>    .../drm/amd/amdkfd/kfd_device_queue_manager.c |  9 ++++--
> >>>>    .../drm/amd/amdkfd/kfd_device_queue_manager.h |  2 +-
> >>>>    .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   | 32 +++++++++++---
> ----
> >> -
> >>>>    .../drm/amd/amdkfd/kfd_packet_manager_v9.c    |  9 +++---
> >>>>    drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  2 +-
> >>>>    5 files changed, 32 insertions(+), 22 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> >>>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> >>>> index a2bff3f01359..0f12c1989e14 100644
> >>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> >>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> >>>> @@ -1606,6 +1606,8 @@ static int set_sched_resources(struct
> >>>> device_queue_manager *dqm)
> >>>>
> >>>>    static int initialize_cpsch(struct device_queue_manager *dqm)
> >>>>    {
> >>>> +     uint32_t xcc_id, xcc_mask = dqm->dev->xcc_mask;
> >>>> +
> >>>>         pr_debug("num of pipes: %d\n", get_pipes_per_mec(dqm));
> >>>>
> >>>>         mutex_init(&dqm->lock_hidden);
> >>>> @@ -1620,8 +1622,11 @@ static int initialize_cpsch(struct
> >>>> device_queue_manager *dqm)
> >>>>         init_sdma_bitmaps(dqm);
> >>>>
> >>>>         if (dqm->dev->kfd2kgd->get_iq_wait_times)
> >>>> -             dqm->dev->kfd2kgd->get_iq_wait_times(dqm->dev->adev,
> >>>> -                                     &dqm->wait_times, 0);
> >>>> +             for_each_inst(xcc_id, xcc_mask)
> >>>> +                     dqm->dev->kfd2kgd->get_iq_wait_times(
> >>>> +                                     dqm->dev->adev,
> >>>> +                                     &dqm->wait_times[xcc_id],
> >>>> +                                     xcc_id);
> >>>>         return 0;
> >>>>    }
> >>>>
> >>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> >>>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> >>>> index 7dd4b177219d..62a6dc8d3032 100644
> >>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> >>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> >>>> @@ -262,7 +262,7 @@ struct device_queue_manager {
> >>>>         /* used for GFX 9.4.3 only */
> >>>>         uint32_t                current_logical_xcc_start;
> >>>>
> >>>> -     uint32_t                wait_times;
> >>>> +     uint32_t                wait_times[32];
> >>> I think wait_times[16] should be sufficient.  We only get the hamming
> >> weight of 16 bits for NUM_XCC and I believe the xcc_mask is declared as a
> >> uint16_t in the KGD portion anyway.  We may as well align to that.
> >>>>         wait_queue_head_t       destroy_wait;
> >>>>    };
> >>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> >>>> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> >>>> index 401096c103b2..f37ab4b6d88c 100644
> >>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> >>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> >>>> @@ -374,27 +374,31 @@ int pm_update_grace_period(struct
> >>>> packet_manager *pm, uint32_t grace_period)
> >>>>    {
> >>>>         int retval = 0;
> >>>>         uint32_t *buffer, size;
> >>>> +     uint32_t xcc_id, xcc_mask = pm->dqm->dev->xcc_mask;
> >>>>
> >>>>         size = pm->pmf->set_grace_period_size;
> >>>>
> >>>>         mutex_lock(&pm->lock);
> >>>>
> >>>>         if (size) {
> >>>> -             kq_acquire_packet_buffer(pm->priv_queue,
> >>>> -                     size / sizeof(uint32_t),
> >>>> -                     (unsigned int **)&buffer);
> >>>> -
> >>>> -             if (!buffer) {
> >>>> -                     pr_err("Failed to allocate buffer on kernel queue\n");
> >>>> -                     retval = -ENOMEM;
> >>>> -                     goto out;
> >>>> -             }
> >>>> +             for_each_inst(xcc_id, xcc_mask) {
> >>>> +                     kq_acquire_packet_buffer(pm->priv_queue,
> >>>> +                                     size / sizeof(uint32_t),
> >>>> +                                     (unsigned int **)&buffer);
> >>>>
> >>>> -             retval = pm->pmf->set_grace_period(pm, buffer,
> >>>> grace_period);
> >>>> -             if (!retval)
> >>>> -                     kq_submit_packet(pm->priv_queue);
> >>>> -             else
> >>>> -                     kq_rollback_packet(pm->priv_queue);
> >>>> +                     if (!buffer) {
> >>>> +                             pr_err("Failed to allocate buffer on kernel
> >>>> queue\n");
> >>>> +                             retval = -ENOMEM;
> >>>> +                             goto out;
> >>>> +                     }
> >>>> +
> >>>> +                     retval = pm->pmf->set_grace_period(pm, buffer,
> >>>> +                                     grace_period, xcc_id);
> >>>> +                     if (!retval)
> >>>> +                             kq_submit_packet(pm->priv_queue);
> >>>> +                     else
> >>>> +                             kq_rollback_packet(pm->priv_queue);
> >>> In the event of partial success do we need to roll back (i.e. resubmit
> default
> >> grace period) on failure?
> >> The function pm_set_grace_period_v9 always return 0, and it is not
> >> complicate operation, it should be always successful. Partial success
> >> will not be the case we should care about at this moment.
> > There's a roll back logic already built-in prior to this that's now only partial
> with this patch.
> > Either way, after a side discussion with Mukul, it looks like the CP register
> to set the default grace period is done by the master XCC per partition so
> we'd can just stick to the current way of setting the grace period as long as
> we reference the start_xcc_id correctly on read and write.
> The start_xcc_id is not defined in amd_staging_drm_next. If we assume 0
> is always the first instance number, the instance parameter in function
> get_iq_wait_times and build_grace_period_packet_info will be not needed.

I see. Each logical partition should have its own mask.
As long as we rely on GET_INST then you are correct.

Thanks,

Jon

>
> Regards,
> Eric
> > Plus the suspend call will set the grace period per partition, so the
> wait_times array wasn't required in the first place.
> >
> > Thanks,
> >
> > Jon
> >
> >> Regards,
> >> Eric
> >>> I believe the default grace period is put in place for better CWSR
> >> performance in normal mode, so leaving fast preemption settings on
> failure
> >> could impact performance.
> >>> Thanks,
> >>>
> >>> Jon
> >>>
> >>>> +             }
> >>>>         }
> >>>>
> >>>>    out:
> >>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> >>>> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> >>>> index 8fda16e6fee6..a9443d661957 100644
> >>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> >>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> >>>> @@ -287,7 +287,8 @@ static int pm_map_queues_v9(struct
> >> packet_manager
> >>>> *pm, uint32_t *buffer,
> >>>>
> >>>>    static int pm_set_grace_period_v9(struct packet_manager *pm,
> >>>>                 uint32_t *buffer,
> >>>> -             uint32_t grace_period)
> >>>> +             uint32_t grace_period,
> >>>> +             uint32_t inst)
> >>>>    {
> >>>>         struct pm4_mec_write_data_mmio *packet;
> >>>>         uint32_t reg_offset = 0;
> >>>> @@ -295,14 +296,14 @@ static int pm_set_grace_period_v9(struct
> >>>> packet_manager *pm,
> >>>>
> >>>>         pm->dqm->dev->kfd2kgd->build_grace_period_packet_info(
> >>>>                         pm->dqm->dev->adev,
> >>>> -                     pm->dqm->wait_times,
> >>>> +                     pm->dqm->wait_times[inst],
> >>>>                         grace_period,
> >>>>                         &reg_offset,
> >>>>                         &reg_data,
> >>>> -                     0);
> >>>> +                     inst);
> >>>>
> >>>>         if (grace_period == USE_DEFAULT_GRACE_PERIOD)
> >>>> -             reg_data = pm->dqm->wait_times;
> >>>> +             reg_data = pm->dqm->wait_times[inst];
> >>>>
> >>>>         packet = (struct pm4_mec_write_data_mmio *)buffer;
> >>>>         memset(buffer, 0, sizeof(struct pm4_mec_write_data_mmio));
> >>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> >>>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> >>>> index d4c9ee3f9953..22c4a403ddd7 100644
> >>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> >>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> >>>> @@ -1400,7 +1400,7 @@ struct packet_manager_funcs {
> >>>>                         enum kfd_unmap_queues_filter mode,
> >>>>                         uint32_t filter_param, bool reset);
> >>>>         int (*set_grace_period)(struct packet_manager *pm, uint32_t
> *buffer,
> >>>> -                     uint32_t grace_period);
> >>>> +                     uint32_t grace_period, uint32_t inst);
> >>>>         int (*query_status)(struct packet_manager *pm, uint32_t *buffer,
> >>>>                         uint64_t fence_address, uint64_t
> >>>> fence_value);
> >>>>         int (*release_mem)(uint64_t gpu_addr, uint32_t *buffer);
> >>>> --
> >>>> 2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
  2023-07-07 17:06           ` Kim, Jonathan
@ 2023-07-07 19:08             ` Kim, Jonathan
  0 siblings, 0 replies; 17+ messages in thread
From: Kim, Jonathan @ 2023-07-07 19:08 UTC (permalink / raw)
  To: Huang, JinHuiEric, amd-gfx; +Cc: Joshi, Mukul

[Public]

> -----Original Message-----
> From: Kim, Jonathan
> Sent: Friday, July 7, 2023 1:06 PM
> To: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: Joshi, Mukul <Mukul.Joshi@amd.com>
> Subject: RE: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
>
>
>
> > -----Original Message-----
> > From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> > Sent: Friday, July 7, 2023 12:44 PM
> > To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-
> gfx@lists.freedesktop.org
> > Cc: Joshi, Mukul <Mukul.Joshi@amd.com>
> > Subject: Re: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
> >
> >
> > On 2023-07-07 11:56, Kim, Jonathan wrote:
> > > [Public]
> > >
> > >> -----Original Message-----
> > >> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> > >> Sent: Friday, July 7, 2023 11:46 AM
> > >> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-
> > gfx@lists.freedesktop.org
> > >> Subject: Re: [PATCH 4/6] drm/amdkfd: enable grace period for xcc
> instance
> > >>
> > >>
> > >> On 2023-07-07 10:59, Kim, Jonathan wrote:
> > >>> [Public]
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Huang, JinHuiEric <JinHuiEric.Huang@amd.com>
> > >>>> Sent: Thursday, July 6, 2023 2:19 PM
> > >>>> To: amd-gfx@lists.freedesktop.org
> > >>>> Cc: Kim, Jonathan <Jonathan.Kim@amd.com>; Huang, JinHuiEric
> > >>>> <JinHuiEric.Huang@amd.com>
> > >>>> Subject: [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance
> > >>>>
> > >>>> each xcc instance needs to get iq wait time and set
> > >>>> grace period accordingly.
> > >>>>
> > >>>> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
> > >>>> ---
> > >>>>    .../drm/amd/amdkfd/kfd_device_queue_manager.c |  9 ++++--
> > >>>>    .../drm/amd/amdkfd/kfd_device_queue_manager.h |  2 +-
> > >>>>    .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   | 32 +++++++++++-
> --
> > ----
> > >> -
> > >>>>    .../drm/amd/amdkfd/kfd_packet_manager_v9.c    |  9 +++---
> > >>>>    drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  2 +-
> > >>>>    5 files changed, 32 insertions(+), 22 deletions(-)
> > >>>>
> > >>>> diff --git
> a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > >>>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > >>>> index a2bff3f01359..0f12c1989e14 100644
> > >>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > >>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > >>>> @@ -1606,6 +1606,8 @@ static int set_sched_resources(struct
> > >>>> device_queue_manager *dqm)
> > >>>>
> > >>>>    static int initialize_cpsch(struct device_queue_manager *dqm)
> > >>>>    {
> > >>>> +     uint32_t xcc_id, xcc_mask = dqm->dev->xcc_mask;
> > >>>> +
> > >>>>         pr_debug("num of pipes: %d\n", get_pipes_per_mec(dqm));
> > >>>>
> > >>>>         mutex_init(&dqm->lock_hidden);
> > >>>> @@ -1620,8 +1622,11 @@ static int initialize_cpsch(struct
> > >>>> device_queue_manager *dqm)
> > >>>>         init_sdma_bitmaps(dqm);
> > >>>>
> > >>>>         if (dqm->dev->kfd2kgd->get_iq_wait_times)
> > >>>> -             dqm->dev->kfd2kgd->get_iq_wait_times(dqm->dev->adev,
> > >>>> -                                     &dqm->wait_times, 0);
> > >>>> +             for_each_inst(xcc_id, xcc_mask)
> > >>>> +                     dqm->dev->kfd2kgd->get_iq_wait_times(
> > >>>> +                                     dqm->dev->adev,
> > >>>> +                                     &dqm->wait_times[xcc_id],
> > >>>> +                                     xcc_id);
> > >>>>         return 0;
> > >>>>    }
> > >>>>
> > >>>> diff --git
> a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> > >>>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> > >>>> index 7dd4b177219d..62a6dc8d3032 100644
> > >>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> > >>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> > >>>> @@ -262,7 +262,7 @@ struct device_queue_manager {
> > >>>>         /* used for GFX 9.4.3 only */
> > >>>>         uint32_t                current_logical_xcc_start;
> > >>>>
> > >>>> -     uint32_t                wait_times;
> > >>>> +     uint32_t                wait_times[32];
> > >>> I think wait_times[16] should be sufficient.  We only get the hamming
> > >> weight of 16 bits for NUM_XCC and I believe the xcc_mask is declared as
> a
> > >> uint16_t in the KGD portion anyway.  We may as well align to that.
> > >>>>         wait_queue_head_t       destroy_wait;
> > >>>>    };
> > >>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> > >>>> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> > >>>> index 401096c103b2..f37ab4b6d88c 100644
> > >>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> > >>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> > >>>> @@ -374,27 +374,31 @@ int pm_update_grace_period(struct
> > >>>> packet_manager *pm, uint32_t grace_period)
> > >>>>    {
> > >>>>         int retval = 0;
> > >>>>         uint32_t *buffer, size;
> > >>>> +     uint32_t xcc_id, xcc_mask = pm->dqm->dev->xcc_mask;
> > >>>>
> > >>>>         size = pm->pmf->set_grace_period_size;
> > >>>>
> > >>>>         mutex_lock(&pm->lock);
> > >>>>
> > >>>>         if (size) {
> > >>>> -             kq_acquire_packet_buffer(pm->priv_queue,
> > >>>> -                     size / sizeof(uint32_t),
> > >>>> -                     (unsigned int **)&buffer);
> > >>>> -
> > >>>> -             if (!buffer) {
> > >>>> -                     pr_err("Failed to allocate buffer on kernel queue\n");
> > >>>> -                     retval = -ENOMEM;
> > >>>> -                     goto out;
> > >>>> -             }
> > >>>> +             for_each_inst(xcc_id, xcc_mask) {
> > >>>> +                     kq_acquire_packet_buffer(pm->priv_queue,
> > >>>> +                                     size / sizeof(uint32_t),
> > >>>> +                                     (unsigned int **)&buffer);
> > >>>>
> > >>>> -             retval = pm->pmf->set_grace_period(pm, buffer,
> > >>>> grace_period);
> > >>>> -             if (!retval)
> > >>>> -                     kq_submit_packet(pm->priv_queue);
> > >>>> -             else
> > >>>> -                     kq_rollback_packet(pm->priv_queue);
> > >>>> +                     if (!buffer) {
> > >>>> +                             pr_err("Failed to allocate buffer on kernel
> > >>>> queue\n");
> > >>>> +                             retval = -ENOMEM;
> > >>>> +                             goto out;
> > >>>> +                     }
> > >>>> +
> > >>>> +                     retval = pm->pmf->set_grace_period(pm, buffer,
> > >>>> +                                     grace_period, xcc_id);
> > >>>> +                     if (!retval)
> > >>>> +                             kq_submit_packet(pm->priv_queue);
> > >>>> +                     else
> > >>>> +                             kq_rollback_packet(pm->priv_queue);
> > >>> In the event of partial success do we need to roll back (i.e. resubmit
> > default
> > >> grace period) on failure?
> > >> The function pm_set_grace_period_v9 always return 0, and it is not
> > >> complicate operation, it should be always successful. Partial success
> > >> will not be the case we should care about at this moment.
> > > There's a roll back logic already built-in prior to this that's now only
> partial
> > with this patch.
> > > Either way, after a side discussion with Mukul, it looks like the CP register
> > to set the default grace period is done by the master XCC per partition so
> > we'd can just stick to the current way of setting the grace period as long as
> > we reference the start_xcc_id correctly on read and write.
> > The start_xcc_id is not defined in amd_staging_drm_next. If we assume 0
> > is always the first instance number, the instance parameter in function
> > get_iq_wait_times and build_grace_period_packet_info will be not needed.
>
> I see. Each logical partition should have its own mask.
> As long as we rely on GET_INST then you are correct.

Sorry for the churn Eric.
So after more investigation and confirming with Mukul, we need to wait for a couple of his missing patches as xcc_mask have a unique bit-wise position start per partition.  Grace period read/write will indeed require proper instancing.

Thanks,

Jon

>
> Thanks,
>
> Jon
>
> >
> > Regards,
> > Eric
> > > Plus the suspend call will set the grace period per partition, so the
> > wait_times array wasn't required in the first place.
> > >
> > > Thanks,
> > >
> > > Jon
> > >
> > >> Regards,
> > >> Eric
> > >>> I believe the default grace period is put in place for better CWSR
> > >> performance in normal mode, so leaving fast preemption settings on
> > failure
> > >> could impact performance.
> > >>> Thanks,
> > >>>
> > >>> Jon
> > >>>
> > >>>> +             }
> > >>>>         }
> > >>>>
> > >>>>    out:
> > >>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> > >>>> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> > >>>> index 8fda16e6fee6..a9443d661957 100644
> > >>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> > >>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> > >>>> @@ -287,7 +287,8 @@ static int pm_map_queues_v9(struct
> > >> packet_manager
> > >>>> *pm, uint32_t *buffer,
> > >>>>
> > >>>>    static int pm_set_grace_period_v9(struct packet_manager *pm,
> > >>>>                 uint32_t *buffer,
> > >>>> -             uint32_t grace_period)
> > >>>> +             uint32_t grace_period,
> > >>>> +             uint32_t inst)
> > >>>>    {
> > >>>>         struct pm4_mec_write_data_mmio *packet;
> > >>>>         uint32_t reg_offset = 0;
> > >>>> @@ -295,14 +296,14 @@ static int pm_set_grace_period_v9(struct
> > >>>> packet_manager *pm,
> > >>>>
> > >>>>         pm->dqm->dev->kfd2kgd->build_grace_period_packet_info(
> > >>>>                         pm->dqm->dev->adev,
> > >>>> -                     pm->dqm->wait_times,
> > >>>> +                     pm->dqm->wait_times[inst],
> > >>>>                         grace_period,
> > >>>>                         &reg_offset,
> > >>>>                         &reg_data,
> > >>>> -                     0);
> > >>>> +                     inst);
> > >>>>
> > >>>>         if (grace_period == USE_DEFAULT_GRACE_PERIOD)
> > >>>> -             reg_data = pm->dqm->wait_times;
> > >>>> +             reg_data = pm->dqm->wait_times[inst];
> > >>>>
> > >>>>         packet = (struct pm4_mec_write_data_mmio *)buffer;
> > >>>>         memset(buffer, 0, sizeof(struct pm4_mec_write_data_mmio));
> > >>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > >>>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > >>>> index d4c9ee3f9953..22c4a403ddd7 100644
> > >>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > >>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > >>>> @@ -1400,7 +1400,7 @@ struct packet_manager_funcs {
> > >>>>                         enum kfd_unmap_queues_filter mode,
> > >>>>                         uint32_t filter_param, bool reset);
> > >>>>         int (*set_grace_period)(struct packet_manager *pm, uint32_t
> > *buffer,
> > >>>> -                     uint32_t grace_period);
> > >>>> +                     uint32_t grace_period, uint32_t inst);
> > >>>>         int (*query_status)(struct packet_manager *pm, uint32_t *buffer,
> > >>>>                         uint64_t fence_address, uint64_t
> > >>>> fence_value);
> > >>>>         int (*release_mem)(uint64_t gpu_addr, uint32_t *buffer);
> > >>>> --
> > >>>> 2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2023-07-07 19:08 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-06 18:19 [PATCH 0/6] Upstream debugger feature for GFX v9.4.3 Eric Huang
2023-07-06 18:19 ` [PATCH 1/6] drm/amdkfd: add kfd2kgd debugger callbacks for GC v9.4.3 Eric Huang
2023-07-07 14:37   ` Kim, Jonathan
2023-07-06 18:19 ` [PATCH 2/6] drm/amdkfd: restore debugger additional info for gfx v9_4_3 Eric Huang
2023-07-06 18:19 ` [PATCH 3/6] drm/amdkfd: enable watch points globally for gfx943 Eric Huang
2023-07-07 15:04   ` Kim, Jonathan
2023-07-06 18:19 ` [PATCH 4/6] drm/amdkfd: enable grace period for xcc instance Eric Huang
2023-07-07 14:59   ` Kim, Jonathan
2023-07-07 15:45     ` Eric Huang
2023-07-07 15:56       ` Kim, Jonathan
2023-07-07 16:44         ` Eric Huang
2023-07-07 17:06           ` Kim, Jonathan
2023-07-07 19:08             ` Kim, Jonathan
2023-07-06 18:19 ` [PATCH 5/6] drm/amdkfd: always keep trap enabled for GC v9.4.3 Eric Huang
2023-07-07 14:39   ` Kim, Jonathan
2023-07-06 18:19 ` [PATCH 6/6] drm/amdkfd: add multi-process debugging support " Eric Huang
2023-07-07 15:06   ` Kim, Jonathan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.