[PATCH v2 0/9] drm/amdgpu: Add jpeg devcoredump support

[PATCH v2 0/9] drm/amdgpu: Add jpeg devcoredump support

[Sathishkumar S]

This patch-series enables jpeg devcoredump support

V2: (Lijo)
 - add amdgpu_jpeg_reg_dump_init() for memory allocation and reg_list ref
 - use reg_list and reg_count from init() to dump and print registers
 - add amdgpu_jpeg_reg_dump_fini() to free memory and clear reg_list ref

Sathishkumar S (9):
  drm/amdgpu: Add a func for core specific reg offset
  drm/amdgpu: Add helper funcs for jpeg devcoredump
  drm/amdgpu: Enable devcoredump for JPEG4_0_3
  drm/amdgpu: Enable devcoredump for JPEG5_0_1
  drm/amdgpu: Enable devcoredump for JPEG4_0_0
  drm/amdgpu: Enable devcoredump for JPEG4_0_5
  drm/amdgpu: Enable devcoredump for JPEG3_0_0
  drm/amdgpu: Enable devcoredump for JPEG2_0_0
  drm/amdgpu: Enable devcoredump for JPEG2_5_0

 drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c | 80 +++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h | 10 +++
 drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.c   | 24 ++++++
 drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c   | 26 +++++++
 drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c   | 24 ++++++
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c   | 26 ++++++-
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c | 98 ++++++++++++++++--------
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c | 27 ++++++-
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c | 51 +++++++++++-
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h | 61 +++++++++++++++
 10 files changed, 393 insertions(+), 34 deletions(-)

-- 
2.25.1

[PATCH v2 1/9] drm/amdgpu: Add a func for core specific reg offset

[Sathishkumar S]

Add an inline function to calculate core specific register offsets for
JPEG v4.0.3 and reuse it, makes code more readable and easier to align.

Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c | 54 +++++++++++-------------
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
index 9459e8cc7413..bc21f12daea8 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
@@ -64,6 +64,14 @@ static inline bool jpeg_v4_0_3_normalizn_reqd(struct amdgpu_device *adev)
 	return (adev->jpeg.caps & AMDGPU_JPEG_CAPS(RRMT_ENABLED)) == 0;
 }
 
+static inline int jpeg_v4_0_3_core_reg_offset(u32 pipe)
+{
+	if (pipe)
+		return ((0x40 * pipe) - 0xc80);
+	else
+		return 0;
+}
+
 /**
  * jpeg_v4_0_3_early_init - set function pointers
  *
@@ -143,10 +151,8 @@ static int jpeg_v4_0_3_sw_init(struct amdgpu_ip_block *ip_block)
 			adev->jpeg.internal.jpeg_pitch[j] =
 				regUVD_JRBC0_UVD_JRBC_SCRATCH0_INTERNAL_OFFSET;
 			adev->jpeg.inst[i].external.jpeg_pitch[j] =
-				SOC15_REG_OFFSET1(
-					JPEG, jpeg_inst,
-					regUVD_JRBC0_UVD_JRBC_SCRATCH0,
-					(j ? (0x40 * j - 0xc80) : 0));
+				SOC15_REG_OFFSET1(JPEG, jpeg_inst, regUVD_JRBC0_UVD_JRBC_SCRATCH0,
+						  jpeg_v4_0_3_core_reg_offset(j));
 		}
 	}
 
@@ -521,7 +527,7 @@ static int jpeg_v4_0_3_start(struct amdgpu_device *adev)
 			 ~UVD_JMI_CNTL__SOFT_RESET_MASK);
 
 		for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
-			unsigned int reg_offset = (j?(0x40 * j - 0xc80):0);
+			int reg_offset = jpeg_v4_0_3_core_reg_offset(j);
 
 			ring = &adev->jpeg.inst[i].ring_dec[j];
 
@@ -616,9 +622,8 @@ static uint64_t jpeg_v4_0_3_dec_ring_get_rptr(struct amdgpu_ring *ring)
 {
 	struct amdgpu_device *adev = ring->adev;
 
-	return RREG32_SOC15_OFFSET(
-		JPEG, GET_INST(JPEG, ring->me), regUVD_JRBC0_UVD_JRBC_RB_RPTR,
-		ring->pipe ? (0x40 * ring->pipe - 0xc80) : 0);
+	return RREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, ring->me), regUVD_JRBC0_UVD_JRBC_RB_RPTR,
+				   jpeg_v4_0_3_core_reg_offset(ring->pipe));
 }
 
 /**
@@ -634,11 +639,9 @@ static uint64_t jpeg_v4_0_3_dec_ring_get_wptr(struct amdgpu_ring *ring)
 
 	if (ring->use_doorbell)
 		return adev->wb.wb[ring->wptr_offs];
-	else
-		return RREG32_SOC15_OFFSET(
-			JPEG, GET_INST(JPEG, ring->me),
-			regUVD_JRBC0_UVD_JRBC_RB_WPTR,
-			ring->pipe ? (0x40 * ring->pipe - 0xc80) : 0);
+
+	return RREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, ring->me), regUVD_JRBC0_UVD_JRBC_RB_WPTR,
+				   jpeg_v4_0_3_core_reg_offset(ring->pipe));
 }
 
 static void jpeg_v4_0_3_ring_emit_hdp_flush(struct amdgpu_ring *ring)
@@ -663,10 +666,8 @@ static void jpeg_v4_0_3_dec_ring_set_wptr(struct amdgpu_ring *ring)
 		adev->wb.wb[ring->wptr_offs] = lower_32_bits(ring->wptr);
 		WDOORBELL32(ring->doorbell_index, lower_32_bits(ring->wptr));
 	} else {
-		WREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, ring->me),
-				    regUVD_JRBC0_UVD_JRBC_RB_WPTR,
-				    (ring->pipe ? (0x40 * ring->pipe - 0xc80) :
-						  0),
+		WREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, ring->me), regUVD_JRBC0_UVD_JRBC_RB_WPTR,
+				    jpeg_v4_0_3_core_reg_offset(ring->pipe),
 				    lower_32_bits(ring->wptr));
 	}
 }
@@ -919,13 +920,9 @@ static bool jpeg_v4_0_3_is_idle(void *handle)
 
 	for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
 		for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
-			unsigned int reg_offset = (j?(0x40 * j - 0xc80):0);
-
-			ret &= ((RREG32_SOC15_OFFSET(
-					 JPEG, GET_INST(JPEG, i),
-					 regUVD_JRBC0_UVD_JRBC_STATUS,
-					 reg_offset) &
-				 UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK) ==
+			ret &= ((RREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, i),
+				regUVD_JRBC0_UVD_JRBC_STATUS, jpeg_v4_0_3_core_reg_offset(j)) &
+				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK) ==
 				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK);
 		}
 	}
@@ -941,13 +938,10 @@ static int jpeg_v4_0_3_wait_for_idle(struct amdgpu_ip_block *ip_block)
 
 	for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
 		for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
-			unsigned int reg_offset = (j?(0x40 * j - 0xc80):0);
-
-			ret &= SOC15_WAIT_ON_RREG_OFFSET(
-				JPEG, GET_INST(JPEG, i),
-				regUVD_JRBC0_UVD_JRBC_STATUS, reg_offset,
+			ret &= (SOC15_WAIT_ON_RREG_OFFSET(JPEG, GET_INST(JPEG, i),
+				regUVD_JRBC0_UVD_JRBC_STATUS, jpeg_v4_0_3_core_reg_offset(j),
 				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK,
-				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK);
+				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK));
 		}
 	}
 	return ret;
-- 
2.25.1

[PATCH v2 2/9] drm/amdgpu: Add helper funcs for jpeg devcoredump

[Sathishkumar S]

Add devcoredump helper functions that can be reused for all jpeg versions.

V2: (Lijo)
 - add amdgpu_jpeg_reg_dump_init() and amdgpu_jpeg_reg_dump_fini()
 - use reg_list and reg_count from init() to dump and print registers
 - memory allocation and freeing is moved to the init() and fini()

Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c | 80 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h | 10 +++
 2 files changed, 90 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
index b6d2eb049f54..0f9d81e27973 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
@@ -452,3 +452,83 @@ void amdgpu_jpeg_sysfs_reset_mask_fini(struct amdgpu_device *adev)
 			device_remove_file(adev->dev, &dev_attr_jpeg_reset_mask);
 	}
 }
+
+int amdgpu_jpeg_reg_dump_init(struct amdgpu_device *adev,
+			       const struct amdgpu_hwip_reg_entry *reg, u32 count)
+{
+	adev->jpeg.ip_dump = kcalloc(adev->jpeg.num_jpeg_inst * count,
+				     sizeof(uint32_t), GFP_KERNEL);
+	if (!adev->jpeg.ip_dump) {
+		DRM_ERROR("Failed to allocate memory for JPEG IP Dump\n");
+		return -ENOMEM;
+	}
+	adev->jpeg.reg_list = reg;
+	adev->jpeg.reg_count = count;
+
+	return 0;
+}
+
+void amdgpu_jpeg_reg_dump_fini(struct amdgpu_device *adev)
+{
+	kfree(adev->jpeg.ip_dump);
+	adev->jpeg.reg_list = NULL;
+	adev->jpeg.reg_count = 0;
+}
+
+void amdgpu_jpeg_dump_ip_state(struct amdgpu_ip_block *ip_block)
+{
+	struct amdgpu_device *adev = ip_block->adev;
+	u32 inst_off, inst_id, is_powered;
+	int i, j;
+
+	if (!adev->jpeg.ip_dump)
+		return;
+
+	for (i = 0; i < adev->jpeg.num_jpeg_inst; i++) {
+		if (adev->jpeg.harvest_config & (1 << i))
+			continue;
+
+		inst_id = GET_INST(JPEG, i);
+		inst_off = i * adev->jpeg.reg_count;
+		/* check power status from UVD_JPEG_POWER_STATUS */
+		adev->jpeg.ip_dump[inst_off] =
+			RREG32(SOC15_REG_ENTRY_OFFSET_INST(adev->jpeg.reg_list[0],
+							   inst_id));
+		is_powered = ((adev->jpeg.ip_dump[inst_off] & 0x1) != 1);
+
+		if (is_powered)
+			for (j = 1; j < adev->jpeg.reg_count; j++)
+				adev->jpeg.ip_dump[inst_off + j] =
+					RREG32(SOC15_REG_ENTRY_OFFSET_INST(adev->jpeg.reg_list[j],
+									   inst_id));
+	}
+}
+
+void amdgpu_jpeg_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p)
+{
+	struct amdgpu_device *adev = ip_block->adev;
+	u32 inst_off, is_powered;
+	int i, j;
+
+	if (!adev->jpeg.ip_dump)
+		return;
+
+	drm_printf(p, "num_instances:%d\n", adev->jpeg.num_jpeg_inst);
+	for (i = 0; i < adev->jpeg.num_jpeg_inst; i++) {
+		if (adev->jpeg.harvest_config & (1 << i)) {
+			drm_printf(p, "\nHarvested Instance:JPEG%d Skipping dump\n", i);
+			continue;
+		}
+
+		inst_off = i * adev->jpeg.reg_count;
+		is_powered = ((adev->jpeg.ip_dump[inst_off] & 0x1) != 1);
+
+		if (is_powered) {
+			drm_printf(p, "Active Instance:JPEG%d\n", i);
+			for (j = 0; j < adev->jpeg.reg_count; j++)
+				drm_printf(p, "%-50s \t 0x%08x\n", adev->jpeg.reg_list[j].reg_name,
+					   adev->jpeg.ip_dump[inst_off + j]);
+		} else
+			drm_printf(p, "\nInactive Instance:JPEG%d\n", i);
+	}
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
index eb2096dcf1a6..02886ec4466e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
@@ -92,6 +92,8 @@
 		*adev->jpeg.inst[inst_idx].dpg_sram_curr_addr++ = value;	\
 	} while (0)
 
+struct amdgpu_hwip_reg_entry;
+
 enum amdgpu_jpeg_caps {
 	AMDGPU_JPEG_RRMT_ENABLED,
 };
@@ -137,6 +139,9 @@ struct amdgpu_jpeg {
 	bool	indirect_sram;
 	uint32_t supported_reset;
 	uint32_t caps;
+	u32 *ip_dump;
+	u32 reg_count;
+	const struct amdgpu_hwip_reg_entry *reg_list;
 };
 
 int amdgpu_jpeg_sw_init(struct amdgpu_device *adev);
@@ -161,5 +166,10 @@ int amdgpu_jpeg_psp_update_sram(struct amdgpu_device *adev, int inst_idx,
 void amdgpu_debugfs_jpeg_sched_mask_init(struct amdgpu_device *adev);
 int amdgpu_jpeg_sysfs_reset_mask_init(struct amdgpu_device *adev);
 void amdgpu_jpeg_sysfs_reset_mask_fini(struct amdgpu_device *adev);
+int amdgpu_jpeg_reg_dump_init(struct amdgpu_device *adev,
+			       const struct amdgpu_hwip_reg_entry *reg, u32 count);
+void amdgpu_jpeg_reg_dump_fini(struct amdgpu_device *adev);
+void amdgpu_jpeg_dump_ip_state(struct amdgpu_ip_block *ip_block);
+void amdgpu_jpeg_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p);
 
 #endif /*__AMDGPU_JPEG_H__*/
-- 
2.25.1

[PATCH v2 3/9] drm/amdgpu: Enable devcoredump for JPEG4_0_3

[Sathishkumar S]

Add register list and enable devcoredump for JPEG4_0_3

V2: (Lijo)
 - remove version specific callbacks and use simplified helper functions

Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c | 44 ++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
index bc21f12daea8..6fc466af00e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
@@ -59,6 +59,42 @@ static int amdgpu_ih_srcid_jpeg[] = {
 	VCN_4_0__SRCID__JPEG7_DECODE
 };
 
+static const struct amdgpu_hwip_reg_entry jpeg_reg_list_4_0_3[] = {
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_POWER_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_INT_STAT),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_SYS_INT_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC0_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC0_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC0_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_ADDR_MODE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_GFX10_ADDR_CONFIG),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_Y_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_UV_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_PITCH),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_UV_PITCH),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC1_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC1_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC1_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC2_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC2_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC2_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC3_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC3_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC3_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC4_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC4_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC4_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC5_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC5_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC5_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC6_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC6_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC6_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC7_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC7_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC7_UVD_JRBC_STATUS),
+};
+
 static inline bool jpeg_v4_0_3_normalizn_reqd(struct amdgpu_device *adev)
 {
 	return (adev->jpeg.caps & AMDGPU_JPEG_CAPS(RRMT_ENABLED)) == 0;
@@ -164,6 +200,10 @@ static int jpeg_v4_0_3_sw_init(struct amdgpu_ip_block *ip_block)
 		}
 	}
 
+	r = amdgpu_jpeg_reg_dump_init(adev, jpeg_reg_list_4_0_3, ARRAY_SIZE(jpeg_reg_list_4_0_3));
+	if (r)
+		return r;
+
 	/* TODO: Add queue reset mask when FW fully supports it */
 	adev->jpeg.supported_reset =
 		amdgpu_get_soft_full_reset_mask(&adev->jpeg.inst[0].ring_dec[0]);
@@ -193,6 +233,8 @@ static int jpeg_v4_0_3_sw_fini(struct amdgpu_ip_block *ip_block)
 	amdgpu_jpeg_sysfs_reset_mask_fini(adev);
 	r = amdgpu_jpeg_sw_fini(adev);
 
+	amdgpu_jpeg_reg_dump_fini(adev);
+
 	return r;
 }
 
@@ -1066,6 +1108,8 @@ static const struct amd_ip_funcs jpeg_v4_0_3_ip_funcs = {
 	.wait_for_idle = jpeg_v4_0_3_wait_for_idle,
 	.set_clockgating_state = jpeg_v4_0_3_set_clockgating_state,
 	.set_powergating_state = jpeg_v4_0_3_set_powergating_state,
+	.dump_ip_state = amdgpu_jpeg_dump_ip_state,
+	.print_ip_state = amdgpu_jpeg_print_ip_state,
 };
 
 static const struct amdgpu_ring_funcs jpeg_v4_0_3_dec_ring_vm_funcs = {
-- 
2.25.1

[PATCH v2 4/9] drm/amdgpu: Enable devcoredump for JPEG5_0_1

[Sathishkumar S]

Add register list and enable devcoredump for JPEG5_0_1

V2: (Lijo)
- remove version specific callbacks and use simplified helper functions

Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c | 51 +++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h | 61 ++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
index 40d4c32a8c2a..210dd8bea1f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
@@ -52,6 +52,47 @@ static int amdgpu_ih_srcid_jpeg[] = {
 	VCN_5_0__SRCID__JPEG9_DECODE,
 };
 
+static const struct amdgpu_hwip_reg_entry jpeg_reg_list_5_0_1[] = {
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_POWER_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_INT_STAT),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC0_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC0_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC0_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_ADDR_MODE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_GFX10_ADDR_CONFIG),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_Y_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_UV_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_PITCH),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_UV_PITCH),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC1_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC1_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC1_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC2_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC2_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC2_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC3_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC3_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC3_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC4_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC4_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC4_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC5_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC5_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC5_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC6_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC6_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC6_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC7_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC7_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC7_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC8_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC8_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC8_UVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC9_UVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC9_UVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC9_UVD_JRBC_STATUS),
+};
+
 static int jpeg_v5_0_1_core_reg_offset(u32 pipe)
 {
 	if (pipe <= AMDGPU_MAX_JPEG_RINGS_4_0_3)
@@ -145,6 +186,10 @@ static int jpeg_v5_0_1_sw_init(struct amdgpu_ip_block *ip_block)
 		}
 	}
 
+	r = amdgpu_jpeg_reg_dump_init(adev, jpeg_reg_list_5_0_1, ARRAY_SIZE(jpeg_reg_list_5_0_1));
+	if (r)
+		return r;
+
 	return 0;
 }
 
@@ -166,6 +211,8 @@ static int jpeg_v5_0_1_sw_fini(struct amdgpu_ip_block *ip_block)
 
 	r = amdgpu_jpeg_sw_fini(adev);
 
+	amdgpu_jpeg_reg_dump_fini(adev);
+
 	return r;
 }
 
@@ -635,8 +682,8 @@ static const struct amd_ip_funcs jpeg_v5_0_1_ip_funcs = {
 	.post_soft_reset = NULL,
 	.set_clockgating_state = jpeg_v5_0_1_set_clockgating_state,
 	.set_powergating_state = jpeg_v5_0_1_set_powergating_state,
-	.dump_ip_state = NULL,
-	.print_ip_state = NULL,
+	.dump_ip_state = amdgpu_jpeg_dump_ip_state,
+	.print_ip_state = amdgpu_jpeg_print_ip_state,
 };
 
 static const struct amdgpu_ring_funcs jpeg_v5_0_1_dec_ring_vm_funcs = {
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
index 8ce146c00bb6..9de3272ef47f 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.h
@@ -26,4 +26,65 @@
 
 extern const struct amdgpu_ip_block_version jpeg_v5_0_1_ip_block;
 
+#define regUVD_JRBC0_UVD_JRBC_RB_WPTR             0x0640
+#define regUVD_JRBC0_UVD_JRBC_RB_WPTR_BASE_IDX    1
+#define regUVD_JRBC0_UVD_JRBC_STATUS              0x0649
+#define regUVD_JRBC0_UVD_JRBC_STATUS_BASE_IDX     1
+#define regUVD_JRBC0_UVD_JRBC_RB_RPTR             0x064a
+#define regUVD_JRBC0_UVD_JRBC_RB_RPTR_BASE_IDX    1
+#define regUVD_JRBC1_UVD_JRBC_RB_WPTR             0x0000
+#define regUVD_JRBC1_UVD_JRBC_RB_WPTR_BASE_IDX    0
+#define regUVD_JRBC1_UVD_JRBC_STATUS              0x0009
+#define regUVD_JRBC1_UVD_JRBC_STATUS_BASE_IDX     0
+#define regUVD_JRBC1_UVD_JRBC_RB_RPTR             0x000a
+#define regUVD_JRBC1_UVD_JRBC_RB_RPTR_BASE_IDX    0
+#define regUVD_JRBC2_UVD_JRBC_RB_WPTR             0x0040
+#define regUVD_JRBC2_UVD_JRBC_RB_WPTR_BASE_IDX    0
+#define regUVD_JRBC2_UVD_JRBC_STATUS              0x0049
+#define regUVD_JRBC2_UVD_JRBC_STATUS_BASE_IDX     0
+#define regUVD_JRBC2_UVD_JRBC_RB_RPTR             0x004a
+#define regUVD_JRBC2_UVD_JRBC_RB_RPTR_BASE_IDX    0
+#define regUVD_JRBC3_UVD_JRBC_RB_WPTR             0x0080
+#define regUVD_JRBC3_UVD_JRBC_RB_WPTR_BASE_IDX    0
+#define regUVD_JRBC3_UVD_JRBC_STATUS              0x0089
+#define regUVD_JRBC3_UVD_JRBC_STATUS_BASE_IDX     0
+#define regUVD_JRBC3_UVD_JRBC_RB_RPTR             0x008a
+#define regUVD_JRBC3_UVD_JRBC_RB_RPTR_BASE_IDX    0
+#define regUVD_JRBC4_UVD_JRBC_RB_WPTR             0x00c0
+#define regUVD_JRBC4_UVD_JRBC_RB_WPTR_BASE_IDX    0
+#define regUVD_JRBC4_UVD_JRBC_STATUS              0x00c9
+#define regUVD_JRBC4_UVD_JRBC_STATUS_BASE_IDX     0
+#define regUVD_JRBC4_UVD_JRBC_RB_RPTR             0x00ca
+#define regUVD_JRBC4_UVD_JRBC_RB_RPTR_BASE_IDX    0
+#define regUVD_JRBC5_UVD_JRBC_RB_WPTR             0x0100
+#define regUVD_JRBC5_UVD_JRBC_RB_WPTR_BASE_IDX    0
+#define regUVD_JRBC5_UVD_JRBC_STATUS              0x0109
+#define regUVD_JRBC5_UVD_JRBC_STATUS_BASE_IDX     0
+#define regUVD_JRBC5_UVD_JRBC_RB_RPTR             0x010a
+#define regUVD_JRBC5_UVD_JRBC_RB_RPTR_BASE_IDX    0
+#define regUVD_JRBC6_UVD_JRBC_RB_WPTR             0x0140
+#define regUVD_JRBC6_UVD_JRBC_RB_WPTR_BASE_IDX    0
+#define regUVD_JRBC6_UVD_JRBC_STATUS              0x0149
+#define regUVD_JRBC6_UVD_JRBC_STATUS_BASE_IDX     0
+#define regUVD_JRBC6_UVD_JRBC_RB_RPTR             0x014a
+#define regUVD_JRBC6_UVD_JRBC_RB_RPTR_BASE_IDX    0
+#define regUVD_JRBC7_UVD_JRBC_RB_WPTR             0x0180
+#define regUVD_JRBC7_UVD_JRBC_RB_WPTR_BASE_IDX    0
+#define regUVD_JRBC7_UVD_JRBC_STATUS              0x0189
+#define regUVD_JRBC7_UVD_JRBC_STATUS_BASE_IDX     0
+#define regUVD_JRBC7_UVD_JRBC_RB_RPTR             0x018a
+#define regUVD_JRBC7_UVD_JRBC_RB_RPTR_BASE_IDX    0
+#define regUVD_JRBC8_UVD_JRBC_RB_WPTR             0x01c0
+#define regUVD_JRBC8_UVD_JRBC_RB_WPTR_BASE_IDX    0
+#define regUVD_JRBC8_UVD_JRBC_STATUS              0x01c9
+#define regUVD_JRBC8_UVD_JRBC_STATUS_BASE_IDX     0
+#define regUVD_JRBC8_UVD_JRBC_RB_RPTR             0x01ca
+#define regUVD_JRBC8_UVD_JRBC_RB_RPTR_BASE_IDX    0
+#define regUVD_JRBC9_UVD_JRBC_RB_WPTR             0x0440
+#define regUVD_JRBC9_UVD_JRBC_RB_WPTR_BASE_IDX    1
+#define regUVD_JRBC9_UVD_JRBC_STATUS              0x0449
+#define regUVD_JRBC9_UVD_JRBC_STATUS_BASE_IDX     1
+#define regUVD_JRBC9_UVD_JRBC_RB_RPTR             0x044a
+#define regUVD_JRBC9_UVD_JRBC_RB_RPTR_BASE_IDX    1
+
 #endif /* __JPEG_V5_0_0_H__ */
-- 
2.25.1

[PATCH v2 5/9] drm/amdgpu: Enable devcoredump for JPEG4_0_0

[Sathishkumar S]

Add register list and enable devcoredump for JPEG4_0_0

V2: (Lijo)
- remove version specific callbacks and use simplified helper functions

Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
index 0aef1f64afd0..0ff0cb659394 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
@@ -36,13 +36,28 @@
 
 #define regUVD_JPEG_PITCH_INTERNAL_OFFSET                  0x401f
 
+static const struct amdgpu_hwip_reg_entry jpeg_reg_list_4_0[] = {
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_POWER_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_INT_STAT),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC_RB_CNTL),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC_RB_SIZE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_ADDR_MODE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_GFX10_ADDR_CONFIG),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_Y_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_UV_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_PITCH),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_UV_PITCH),
+};
+
 static int jpeg_v4_0_start_sriov(struct amdgpu_device *adev);
 static void jpeg_v4_0_set_dec_ring_funcs(struct amdgpu_device *adev);
 static void jpeg_v4_0_set_irq_funcs(struct amdgpu_device *adev);
 static int jpeg_v4_0_set_powergating_state(struct amdgpu_ip_block *ip_block,
 				enum amd_powergating_state state);
 static void jpeg_v4_0_set_ras_funcs(struct amdgpu_device *adev);
-
 static void jpeg_v4_0_dec_ring_set_wptr(struct amdgpu_ring *ring);
 
 /**
@@ -123,6 +138,11 @@ static int jpeg_v4_0_sw_init(struct amdgpu_ip_block *ip_block)
 	r = amdgpu_jpeg_ras_sw_init(adev);
 	if (r)
 		return r;
+
+	r = amdgpu_jpeg_reg_dump_init(adev, jpeg_reg_list_4_0, ARRAY_SIZE(jpeg_reg_list_4_0));
+	if (r)
+		return r;
+
 	/* TODO: Add queue reset mask when FW fully supports it */
 	adev->jpeg.supported_reset =
 		amdgpu_get_soft_full_reset_mask(&adev->jpeg.inst[0].ring_dec[0]);
@@ -152,6 +172,8 @@ static int jpeg_v4_0_sw_fini(struct amdgpu_ip_block *ip_block)
 	amdgpu_jpeg_sysfs_reset_mask_fini(adev);
 	r = amdgpu_jpeg_sw_fini(adev);
 
+	amdgpu_jpeg_reg_dump_fini(adev);
+
 	return r;
 }
 
@@ -717,6 +739,8 @@ static const struct amd_ip_funcs jpeg_v4_0_ip_funcs = {
 	.wait_for_idle = jpeg_v4_0_wait_for_idle,
 	.set_clockgating_state = jpeg_v4_0_set_clockgating_state,
 	.set_powergating_state = jpeg_v4_0_set_powergating_state,
+	.dump_ip_state = amdgpu_jpeg_dump_ip_state,
+	.print_ip_state = amdgpu_jpeg_print_ip_state,
 };
 
 static const struct amdgpu_ring_funcs jpeg_v4_0_dec_ring_vm_funcs = {
-- 
2.25.1

[PATCH v2 6/9] drm/amdgpu: Enable devcoredump for JPEG4_0_5

[Sathishkumar S]

Add register list and enable devcoredump for JPEG4_0_5

V2: (Lijo)
- remove version specific callbacks and use simplified helper functions

Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c | 27 +++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
index 6b3656984957..dabc9a2401c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
@@ -46,11 +46,26 @@
 #define regJPEG_CGC_GATE_INTERNAL_OFFSET		0x4160
 #define regUVD_NO_OP_INTERNAL_OFFSET			0x0029
 
+static const struct amdgpu_hwip_reg_entry jpeg_reg_list_4_0_5[] = {
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_POWER_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_INT_STAT),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC_RB_CNTL),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC_RB_SIZE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_ADDR_MODE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_GFX10_ADDR_CONFIG),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_Y_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regJPEG_DEC_UV_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_PITCH),
+	SOC15_REG_ENTRY_STR(JPEG, 0, regUVD_JPEG_UV_PITCH),
+};
+
 static void jpeg_v4_0_5_set_dec_ring_funcs(struct amdgpu_device *adev);
 static void jpeg_v4_0_5_set_irq_funcs(struct amdgpu_device *adev);
 static int jpeg_v4_0_5_set_powergating_state(struct amdgpu_ip_block *ip_block,
 				enum amd_powergating_state state);
-
 static void jpeg_v4_0_5_dec_ring_set_wptr(struct amdgpu_ring *ring);
 
 static int amdgpu_ih_clientid_jpeg[] = {
@@ -58,6 +73,8 @@ static int amdgpu_ih_clientid_jpeg[] = {
 	SOC15_IH_CLIENTID_VCN1
 };
 
+
+
 /**
  * jpeg_v4_0_5_early_init - set function pointers
  *
@@ -153,6 +170,10 @@ static int jpeg_v4_0_5_sw_init(struct amdgpu_ip_block *ip_block)
 		adev->jpeg.inst[i].external.jpeg_pitch[0] = SOC15_REG_OFFSET(JPEG, i, regUVD_JPEG_PITCH);
 	}
 
+	r = amdgpu_jpeg_reg_dump_init(adev, jpeg_reg_list_4_0_5, ARRAY_SIZE(jpeg_reg_list_4_0_5));
+	if (r)
+		return r;
+
 	/* TODO: Add queue reset mask when FW fully supports it */
 	adev->jpeg.supported_reset =
 		amdgpu_get_soft_full_reset_mask(&adev->jpeg.inst[0].ring_dec[0]);
@@ -182,6 +203,8 @@ static int jpeg_v4_0_5_sw_fini(struct amdgpu_ip_block *ip_block)
 	amdgpu_jpeg_sysfs_reset_mask_fini(adev);
 	r = amdgpu_jpeg_sw_fini(adev);
 
+	amdgpu_jpeg_reg_dump_fini(adev);
+
 	return r;
 }
 
@@ -759,6 +782,8 @@ static const struct amd_ip_funcs jpeg_v4_0_5_ip_funcs = {
 	.wait_for_idle = jpeg_v4_0_5_wait_for_idle,
 	.set_clockgating_state = jpeg_v4_0_5_set_clockgating_state,
 	.set_powergating_state = jpeg_v4_0_5_set_powergating_state,
+	.dump_ip_state = amdgpu_jpeg_dump_ip_state,
+	.print_ip_state = amdgpu_jpeg_print_ip_state,
 };
 
 static const struct amdgpu_ring_funcs jpeg_v4_0_5_dec_ring_vm_funcs = {
-- 
2.25.1

[PATCH v2 7/9] drm/amdgpu: Enable devcoredump for JPEG3_0_0

[Sathishkumar S]

Add register list and enable devcoredump for JPEG3_0_0

V2: (Lijo)
- remove version specific callbacks and use simplified helper functions

Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c
index 4eca65ea9053..ee5c30f5718a 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c
@@ -34,6 +34,22 @@
 
 #define mmUVD_JPEG_PITCH_INTERNAL_OFFSET	0x401f
 
+static const struct amdgpu_hwip_reg_entry jpeg_reg_list_3_0[] = {
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_POWER_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_INT_STAT),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_CNTL),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_SIZE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_ADDR_MODE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_GFX10_ADDR_CONFIG),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_Y_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_UV_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_PITCH),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_UV_PITCH),
+};
+
 static void jpeg_v3_0_set_dec_ring_funcs(struct amdgpu_device *adev);
 static void jpeg_v3_0_set_irq_funcs(struct amdgpu_device *adev);
 static int jpeg_v3_0_set_powergating_state(struct amdgpu_ip_block *ip_block,
@@ -112,6 +128,10 @@ static int jpeg_v3_0_sw_init(struct amdgpu_ip_block *ip_block)
 	adev->jpeg.internal.jpeg_pitch[0] = mmUVD_JPEG_PITCH_INTERNAL_OFFSET;
 	adev->jpeg.inst->external.jpeg_pitch[0] = SOC15_REG_OFFSET(JPEG, 0, mmUVD_JPEG_PITCH);
 
+	r = amdgpu_jpeg_reg_dump_init(adev, jpeg_reg_list_3_0, ARRAY_SIZE(jpeg_reg_list_3_0));
+	if (r)
+		return r;
+
 	return 0;
 }
 
@@ -133,6 +153,8 @@ static int jpeg_v3_0_sw_fini(struct amdgpu_ip_block *ip_block)
 
 	r = amdgpu_jpeg_sw_fini(adev);
 
+	amdgpu_jpeg_reg_dump_fini(adev);
+
 	return r;
 }
 
@@ -543,6 +565,8 @@ static const struct amd_ip_funcs jpeg_v3_0_ip_funcs = {
 	.wait_for_idle = jpeg_v3_0_wait_for_idle,
 	.set_clockgating_state = jpeg_v3_0_set_clockgating_state,
 	.set_powergating_state = jpeg_v3_0_set_powergating_state,
+	.dump_ip_state = amdgpu_jpeg_dump_ip_state,
+	.print_ip_state = amdgpu_jpeg_print_ip_state,
 };
 
 static const struct amdgpu_ring_funcs jpeg_v3_0_dec_ring_vm_funcs = {
-- 
2.25.1

[PATCH v2 8/9] drm/amdgpu: Enable devcoredump for JPEG2_0_0

[Sathishkumar S]

Add register list and enable devcoredump for JPEG2_0_0

V2: (Lijo)
- remove version specific callbacks and use simplified helper functions

Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.c
index 7c9251c03815..29bdab1c50c3 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.c
@@ -33,6 +33,22 @@
 #include "vcn/vcn_2_0_0_sh_mask.h"
 #include "ivsrcid/vcn/irqsrcs_vcn_2_0.h"
 
+static const struct amdgpu_hwip_reg_entry jpeg_reg_list_2_0[] = {
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_POWER_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_INT_STAT),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_CNTL),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_SIZE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_ADDR_MODE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_GFX10_ADDR_CONFIG),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_Y_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_UV_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_PITCH),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_UV_PITCH),
+};
+
 static void jpeg_v2_0_set_dec_ring_funcs(struct amdgpu_device *adev);
 static void jpeg_v2_0_set_irq_funcs(struct amdgpu_device *adev);
 static int jpeg_v2_0_set_powergating_state(struct amdgpu_ip_block *ip_block,
@@ -98,6 +114,10 @@ static int jpeg_v2_0_sw_init(struct amdgpu_ip_block *ip_block)
 	adev->jpeg.internal.jpeg_pitch[0] = mmUVD_JPEG_PITCH_INTERNAL_OFFSET;
 	adev->jpeg.inst->external.jpeg_pitch[0] = SOC15_REG_OFFSET(JPEG, 0, mmUVD_JPEG_PITCH);
 
+	r = amdgpu_jpeg_reg_dump_init(adev, jpeg_reg_list_2_0, ARRAY_SIZE(jpeg_reg_list_2_0));
+	if (r)
+		return r;
+
 	return 0;
 }
 
@@ -119,6 +139,8 @@ static int jpeg_v2_0_sw_fini(struct amdgpu_ip_block *ip_block)
 
 	r = amdgpu_jpeg_sw_fini(adev);
 
+	amdgpu_jpeg_reg_dump_fini(adev);
+
 	return r;
 }
 
@@ -752,6 +774,8 @@ static const struct amd_ip_funcs jpeg_v2_0_ip_funcs = {
 	.wait_for_idle = jpeg_v2_0_wait_for_idle,
 	.set_clockgating_state = jpeg_v2_0_set_clockgating_state,
 	.set_powergating_state = jpeg_v2_0_set_powergating_state,
+	.dump_ip_state = amdgpu_jpeg_dump_ip_state,
+	.print_ip_state = amdgpu_jpeg_print_ip_state,
 };
 
 static const struct amdgpu_ring_funcs jpeg_v2_0_dec_ring_vm_funcs = {
-- 
2.25.1

[PATCH v2 9/9] drm/amdgpu: Enable devcoredump for JPEG2_5_0

[Sathishkumar S]

Add register list and enable devcoredump for JPEG2_5_0

V2: (Lijo)
- remove version specific callbacks and use simplified helper functions

Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
index 11f6af2646e7..536a6101dcf9 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
@@ -36,6 +36,22 @@
 
 #define JPEG25_MAX_HW_INSTANCES_ARCTURUS			2
 
+static const struct amdgpu_hwip_reg_entry jpeg_reg_list_2_5[] = {
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_POWER_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_INT_STAT),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_RPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_WPTR),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_CNTL),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_SIZE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_STATUS),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_ADDR_MODE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_GFX10_ADDR_CONFIG),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_Y_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_UV_GFX10_TILING_SURFACE),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_PITCH),
+	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_UV_PITCH),
+};
+
 static void jpeg_v2_5_set_dec_ring_funcs(struct amdgpu_device *adev);
 static void jpeg_v2_5_set_irq_funcs(struct amdgpu_device *adev);
 static int jpeg_v2_5_set_powergating_state(struct amdgpu_ip_block *ip_block,
@@ -147,6 +163,10 @@ static int jpeg_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
 	if (r)
 		return r;
 
+	r = amdgpu_jpeg_reg_dump_init(adev, jpeg_reg_list_2_5, ARRAY_SIZE(jpeg_reg_list_2_5));
+	if (r)
+		return r;
+
 	return 0;
 }
 
@@ -168,6 +188,8 @@ static int jpeg_v2_5_sw_fini(struct amdgpu_ip_block *ip_block)
 
 	r = amdgpu_jpeg_sw_fini(adev);
 
+	amdgpu_jpeg_reg_dump_fini(adev);
+
 	return r;
 }
 
@@ -623,6 +645,8 @@ static const struct amd_ip_funcs jpeg_v2_5_ip_funcs = {
 	.wait_for_idle = jpeg_v2_5_wait_for_idle,
 	.set_clockgating_state = jpeg_v2_5_set_clockgating_state,
 	.set_powergating_state = jpeg_v2_5_set_powergating_state,
+	.dump_ip_state = amdgpu_jpeg_dump_ip_state,
+	.print_ip_state = amdgpu_jpeg_print_ip_state,
 };
 
 static const struct amd_ip_funcs jpeg_v2_6_ip_funcs = {
@@ -638,6 +662,8 @@ static const struct amd_ip_funcs jpeg_v2_6_ip_funcs = {
 	.wait_for_idle = jpeg_v2_5_wait_for_idle,
 	.set_clockgating_state = jpeg_v2_5_set_clockgating_state,
 	.set_powergating_state = jpeg_v2_5_set_powergating_state,
+	.dump_ip_state = amdgpu_jpeg_dump_ip_state,
+	.print_ip_state = amdgpu_jpeg_print_ip_state,
 };
 
 static const struct amdgpu_ring_funcs jpeg_v2_5_dec_ring_vm_funcs = {
-- 
2.25.1

Re: [PATCH v2 2/9] drm/amdgpu: Add helper funcs for jpeg devcoredump

[Lazar, Lijo]

On 1/29/2025 2:16 PM, Sathishkumar S wrote:
> Add devcoredump helper functions that can be reused for all jpeg versions.
> 
> V2: (Lijo)
>  - add amdgpu_jpeg_reg_dump_init() and amdgpu_jpeg_reg_dump_fini()
>  - use reg_list and reg_count from init() to dump and print registers
>  - memory allocation and freeing is moved to the init() and fini()
> 
> Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c | 80 ++++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h | 10 +++
>  2 files changed, 90 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
> index b6d2eb049f54..0f9d81e27973 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
> @@ -452,3 +452,83 @@ void amdgpu_jpeg_sysfs_reset_mask_fini(struct amdgpu_device *adev)
>  			device_remove_file(adev->dev, &dev_attr_jpeg_reset_mask);
>  	}
>  }
> +
> +int amdgpu_jpeg_reg_dump_init(struct amdgpu_device *adev,
> +			       const struct amdgpu_hwip_reg_entry *reg, u32 count)
> +{
> +	adev->jpeg.ip_dump = kcalloc(adev->jpeg.num_jpeg_inst * count,
> +				     sizeof(uint32_t), GFP_KERNEL);
> +	if (!adev->jpeg.ip_dump) {
> +		DRM_ERROR("Failed to allocate memory for JPEG IP Dump\n");
> +		return -ENOMEM;
> +	}
> +	adev->jpeg.reg_list = reg;
> +	adev->jpeg.reg_count = count;
> +
> +	return 0;
> +}
> +
> +void amdgpu_jpeg_reg_dump_fini(struct amdgpu_device *adev)
> +{
> +	kfree(adev->jpeg.ip_dump);
> +	adev->jpeg.reg_list = NULL;
> +	adev->jpeg.reg_count = 0;
> +}
> +
> +void amdgpu_jpeg_dump_ip_state(struct amdgpu_ip_block *ip_block)
> +{
> +	struct amdgpu_device *adev = ip_block->adev;
> +	u32 inst_off, inst_id, is_powered;
> +	int i, j;
> +
> +	if (!adev->jpeg.ip_dump)
> +		return;
> +
> +	for (i = 0; i < adev->jpeg.num_jpeg_inst; i++) {
> +		if (adev->jpeg.harvest_config & (1 << i))
> +			continue;
> +
> +		inst_id = GET_INST(JPEG, i);
> +		inst_off = i * adev->jpeg.reg_count;
> +		/* check power status from UVD_JPEG_POWER_STATUS */
> +		adev->jpeg.ip_dump[inst_off] =
> +			RREG32(SOC15_REG_ENTRY_OFFSET_INST(adev->jpeg.reg_list[0],
> +							   inst_id));
> +		is_powered = ((adev->jpeg.ip_dump[inst_off] & 0x1) != 1);
> +
> +		if (is_powered)
> +			for (j = 1; j < adev->jpeg.reg_count; j++)
> +				adev->jpeg.ip_dump[inst_off + j] =
> +					RREG32(SOC15_REG_ENTRY_OFFSET_INST(adev->jpeg.reg_list[j],
> +									   inst_id));
> +	}
> +}
> +
> +void amdgpu_jpeg_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p)
> +{
> +	struct amdgpu_device *adev = ip_block->adev;
> +	u32 inst_off, is_powered;
> +	int i, j;
> +
> +	if (!adev->jpeg.ip_dump)
> +		return;
> +
> +	drm_printf(p, "num_instances:%d\n", adev->jpeg.num_jpeg_inst);
> +	for (i = 0; i < adev->jpeg.num_jpeg_inst; i++) {
> +		if (adev->jpeg.harvest_config & (1 << i)) {
> +			drm_printf(p, "\nHarvested Instance:JPEG%d Skipping dump\n", i);
> +			continue;
> +		}
> +
> +		inst_off = i * adev->jpeg.reg_count;
> +		is_powered = ((adev->jpeg.ip_dump[inst_off] & 0x1) != 1);
> +
> +		if (is_powered) {
> +			drm_printf(p, "Active Instance:JPEG%d\n", i);
> +			for (j = 0; j < adev->jpeg.reg_count; j++)
> +				drm_printf(p, "%-50s \t 0x%08x\n", adev->jpeg.reg_list[j].reg_name,
> +					   adev->jpeg.ip_dump[inst_off + j]);
> +		} else
> +			drm_printf(p, "\nInactive Instance:JPEG%d\n", i);
> +	}
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
> index eb2096dcf1a6..02886ec4466e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
> @@ -92,6 +92,8 @@
>  		*adev->jpeg.inst[inst_idx].dpg_sram_curr_addr++ = value;	\
>  	} while (0)
>  
> +struct amdgpu_hwip_reg_entry;
> +
>  enum amdgpu_jpeg_caps {
>  	AMDGPU_JPEG_RRMT_ENABLED,
>  };
> @@ -137,6 +139,9 @@ struct amdgpu_jpeg {
>  	bool	indirect_sram;
>  	uint32_t supported_reset;
>  	uint32_t caps;
> +	u32 *ip_dump;
> +	u32 reg_count;
> +	const struct amdgpu_hwip_reg_entry *reg_list;
>  };
>  

Thanks, this is almost there. Personally, would still prefer something
like below and have an instance of this kept inside jpeg_inst (though I
see your point that jpeg_inst doesn't have an instance id and this will
also mean duplicating list pointer/num reg info in all instances).

amdgpu_jpeg_reg_dump {
	u32 *ip_dump;
	u32 reg_count;
	const struct amdgpu_hwip_reg_entry *reg_list;
};

Ignoring that -

With the current way,

amdgpu_jpeg_reg_dump_fini() may be called from within sw_fini(). Just
wanted to keep a wrapper fini() func to make sure all is cleaned up.
That would avoid calling this from every IP version.

Apart from those, leaving it to Leo or someone else from JPEG to take a
look.

Regards,
LIjo

>  int amdgpu_jpeg_sw_init(struct amdgpu_device *adev);
> @@ -161,5 +166,10 @@ int amdgpu_jpeg_psp_update_sram(struct amdgpu_device *adev, int inst_idx,
>  void amdgpu_debugfs_jpeg_sched_mask_init(struct amdgpu_device *adev);
>  int amdgpu_jpeg_sysfs_reset_mask_init(struct amdgpu_device *adev);
>  void amdgpu_jpeg_sysfs_reset_mask_fini(struct amdgpu_device *adev);
> +int amdgpu_jpeg_reg_dump_init(struct amdgpu_device *adev,
> +			       const struct amdgpu_hwip_reg_entry *reg, u32 count);
> +void amdgpu_jpeg_reg_dump_fini(struct amdgpu_device *adev);
> +void amdgpu_jpeg_dump_ip_state(struct amdgpu_ip_block *ip_block);
> +void amdgpu_jpeg_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p);
>  
>  #endif /*__AMDGPU_JPEG_H__*/

Re: [PATCH v2 2/9] drm/amdgpu: Add helper funcs for jpeg devcoredump

[Sundararaju, Sathishkumar]

On 1/29/2025 3:20 PM, Lazar, Lijo wrote:
>
> On 1/29/2025 2:16 PM, Sathishkumar S wrote:
>> Add devcoredump helper functions that can be reused for all jpeg versions.
>>
>> V2: (Lijo)
>>   - add amdgpu_jpeg_reg_dump_init() and amdgpu_jpeg_reg_dump_fini()
>>   - use reg_list and reg_count from init() to dump and print registers
>>   - memory allocation and freeing is moved to the init() and fini()
>>
>> Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c | 80 ++++++++++++++++++++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h | 10 +++
>>   2 files changed, 90 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
>> index b6d2eb049f54..0f9d81e27973 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
>> @@ -452,3 +452,83 @@ void amdgpu_jpeg_sysfs_reset_mask_fini(struct amdgpu_device *adev)
>>   			device_remove_file(adev->dev, &dev_attr_jpeg_reset_mask);
>>   	}
>>   }
>> +
>> +int amdgpu_jpeg_reg_dump_init(struct amdgpu_device *adev,
>> +			       const struct amdgpu_hwip_reg_entry *reg, u32 count)
>> +{
>> +	adev->jpeg.ip_dump = kcalloc(adev->jpeg.num_jpeg_inst * count,
>> +				     sizeof(uint32_t), GFP_KERNEL);
>> +	if (!adev->jpeg.ip_dump) {
>> +		DRM_ERROR("Failed to allocate memory for JPEG IP Dump\n");
>> +		return -ENOMEM;
>> +	}
>> +	adev->jpeg.reg_list = reg;
>> +	adev->jpeg.reg_count = count;
>> +
>> +	return 0;
>> +}
>> +
>> +void amdgpu_jpeg_reg_dump_fini(struct amdgpu_device *adev)
>> +{
>> +	kfree(adev->jpeg.ip_dump);
>> +	adev->jpeg.reg_list = NULL;
>> +	adev->jpeg.reg_count = 0;
>> +}
>> +
>> +void amdgpu_jpeg_dump_ip_state(struct amdgpu_ip_block *ip_block)
>> +{
>> +	struct amdgpu_device *adev = ip_block->adev;
>> +	u32 inst_off, inst_id, is_powered;
>> +	int i, j;
>> +
>> +	if (!adev->jpeg.ip_dump)
>> +		return;
>> +
>> +	for (i = 0; i < adev->jpeg.num_jpeg_inst; i++) {
>> +		if (adev->jpeg.harvest_config & (1 << i))
>> +			continue;
>> +
>> +		inst_id = GET_INST(JPEG, i);
>> +		inst_off = i * adev->jpeg.reg_count;
>> +		/* check power status from UVD_JPEG_POWER_STATUS */
>> +		adev->jpeg.ip_dump[inst_off] =
>> +			RREG32(SOC15_REG_ENTRY_OFFSET_INST(adev->jpeg.reg_list[0],
>> +							   inst_id));
>> +		is_powered = ((adev->jpeg.ip_dump[inst_off] & 0x1) != 1);
>> +
>> +		if (is_powered)
>> +			for (j = 1; j < adev->jpeg.reg_count; j++)
>> +				adev->jpeg.ip_dump[inst_off + j] =
>> +					RREG32(SOC15_REG_ENTRY_OFFSET_INST(adev->jpeg.reg_list[j],
>> +									   inst_id));
>> +	}
>> +}
>> +
>> +void amdgpu_jpeg_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p)
>> +{
>> +	struct amdgpu_device *adev = ip_block->adev;
>> +	u32 inst_off, is_powered;
>> +	int i, j;
>> +
>> +	if (!adev->jpeg.ip_dump)
>> +		return;
>> +
>> +	drm_printf(p, "num_instances:%d\n", adev->jpeg.num_jpeg_inst);
>> +	for (i = 0; i < adev->jpeg.num_jpeg_inst; i++) {
>> +		if (adev->jpeg.harvest_config & (1 << i)) {
>> +			drm_printf(p, "\nHarvested Instance:JPEG%d Skipping dump\n", i);
>> +			continue;
>> +		}
>> +
>> +		inst_off = i * adev->jpeg.reg_count;
>> +		is_powered = ((adev->jpeg.ip_dump[inst_off] & 0x1) != 1);
>> +
>> +		if (is_powered) {
>> +			drm_printf(p, "Active Instance:JPEG%d\n", i);
>> +			for (j = 0; j < adev->jpeg.reg_count; j++)
>> +				drm_printf(p, "%-50s \t 0x%08x\n", adev->jpeg.reg_list[j].reg_name,
>> +					   adev->jpeg.ip_dump[inst_off + j]);
>> +		} else
>> +			drm_printf(p, "\nInactive Instance:JPEG%d\n", i);
>> +	}
>> +}
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
>> index eb2096dcf1a6..02886ec4466e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
>> @@ -92,6 +92,8 @@
>>   		*adev->jpeg.inst[inst_idx].dpg_sram_curr_addr++ = value;	\
>>   	} while (0)
>>   
>> +struct amdgpu_hwip_reg_entry;
>> +
>>   enum amdgpu_jpeg_caps {
>>   	AMDGPU_JPEG_RRMT_ENABLED,
>>   };
>> @@ -137,6 +139,9 @@ struct amdgpu_jpeg {
>>   	bool	indirect_sram;
>>   	uint32_t supported_reset;
>>   	uint32_t caps;
>> +	u32 *ip_dump;
>> +	u32 reg_count;
>> +	const struct amdgpu_hwip_reg_entry *reg_list;
>>   };
>>   
> Thanks, this is almost there. Personally, would still prefer something
> like below and have an instance of this kept inside jpeg_inst (though I
> see your point that jpeg_inst doesn't have an instance id and this will
> also mean duplicating list pointer/num reg info in all instances).
The multiple copies is one reason I am trying to avoid this approach, 
and we can still print only
the affected instance registers in devcoredump in the future if support 
for it comes up.
>
> amdgpu_jpeg_reg_dump {
> 	u32 *ip_dump;
> 	u32 reg_count;
> 	const struct amdgpu_hwip_reg_entry *reg_list;
> };
>
> Ignoring that -
Thank you, would prefer to have single reference of the 
reg_list/reg_count and use inst_id.
>
> With the current way,
>
> amdgpu_jpeg_reg_dump_fini() may be called from within sw_fini(). Just
> wanted to keep a wrapper fini() func to make sure all is cleaned up.
> That would avoid calling this from every IP version.
Can we have it the current way instead ? as few ip_versions do not 
support devcoredump yet and calling
reg_dump_fini() for every ip version irrespective of the  support is 
redundant, if we add that in sw_fini().
>
> Apart from those, leaving it to Leo or someone else from JPEG to take a
> look.
Okay, thank you.

Regards,
Sathish
>
> Regards,
> LIjo
>
>>   int amdgpu_jpeg_sw_init(struct amdgpu_device *adev);
>> @@ -161,5 +166,10 @@ int amdgpu_jpeg_psp_update_sram(struct amdgpu_device *adev, int inst_idx,
>>   void amdgpu_debugfs_jpeg_sched_mask_init(struct amdgpu_device *adev);
>>   int amdgpu_jpeg_sysfs_reset_mask_init(struct amdgpu_device *adev);
>>   void amdgpu_jpeg_sysfs_reset_mask_fini(struct amdgpu_device *adev);
>> +int amdgpu_jpeg_reg_dump_init(struct amdgpu_device *adev,
>> +			       const struct amdgpu_hwip_reg_entry *reg, u32 count);
>> +void amdgpu_jpeg_reg_dump_fini(struct amdgpu_device *adev);
>> +void amdgpu_jpeg_dump_ip_state(struct amdgpu_ip_block *ip_block);
>> +void amdgpu_jpeg_print_ip_state(struct amdgpu_ip_block *ip_block, struct drm_printer *p);
>>   
>>   #endif /*__AMDGPU_JPEG_H__*/

Re: [PATCH v2 2/9] drm/amdgpu: Add helper funcs for jpeg devcoredump

[Lazar, Lijo]

On 1/29/2025 3:51 PM, Sundararaju, Sathishkumar wrote:
> 
> 
> 
> On 1/29/2025 3:20 PM, Lazar, Lijo wrote:
>>
>> On 1/29/2025 2:16 PM, Sathishkumar S wrote:
>>> Add devcoredump helper functions that can be reused for all jpeg
>>> versions.
>>>
>>> V2: (Lijo)
>>>   - add amdgpu_jpeg_reg_dump_init() and amdgpu_jpeg_reg_dump_fini()
>>>   - use reg_list and reg_count from init() to dump and print registers
>>>   - memory allocation and freeing is moved to the init() and fini()
>>>
>>> Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c | 80 ++++++++++++++++++++++++
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h | 10 +++
>>>   2 files changed, 90 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c b/drivers/gpu/
>>> drm/amd/amdgpu/amdgpu_jpeg.c
>>> index b6d2eb049f54..0f9d81e27973 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
>>> @@ -452,3 +452,83 @@ void amdgpu_jpeg_sysfs_reset_mask_fini(struct
>>> amdgpu_device *adev)
>>>               device_remove_file(adev->dev, &dev_attr_jpeg_reset_mask);
>>>       }
>>>   }
>>> +
>>> +int amdgpu_jpeg_reg_dump_init(struct amdgpu_device *adev,
>>> +                   const struct amdgpu_hwip_reg_entry *reg, u32 count)
>>> +{
>>> +    adev->jpeg.ip_dump = kcalloc(adev->jpeg.num_jpeg_inst * count,
>>> +                     sizeof(uint32_t), GFP_KERNEL);
>>> +    if (!adev->jpeg.ip_dump) {
>>> +        DRM_ERROR("Failed to allocate memory for JPEG IP Dump\n");
>>> +        return -ENOMEM;
>>> +    }
>>> +    adev->jpeg.reg_list = reg;
>>> +    adev->jpeg.reg_count = count;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +void amdgpu_jpeg_reg_dump_fini(struct amdgpu_device *adev)
>>> +{
>>> +    kfree(adev->jpeg.ip_dump);
>>> +    adev->jpeg.reg_list = NULL;
>>> +    adev->jpeg.reg_count = 0;
>>> +}
>>> +
>>> +void amdgpu_jpeg_dump_ip_state(struct amdgpu_ip_block *ip_block)
>>> +{
>>> +    struct amdgpu_device *adev = ip_block->adev;
>>> +    u32 inst_off, inst_id, is_powered;
>>> +    int i, j;
>>> +
>>> +    if (!adev->jpeg.ip_dump)
>>> +        return;
>>> +
>>> +    for (i = 0; i < adev->jpeg.num_jpeg_inst; i++) {
>>> +        if (adev->jpeg.harvest_config & (1 << i))
>>> +            continue;
>>> +
>>> +        inst_id = GET_INST(JPEG, i);
>>> +        inst_off = i * adev->jpeg.reg_count;
>>> +        /* check power status from UVD_JPEG_POWER_STATUS */
>>> +        adev->jpeg.ip_dump[inst_off] =
>>> +            RREG32(SOC15_REG_ENTRY_OFFSET_INST(adev->jpeg.reg_list[0],
>>> +                               inst_id));
>>> +        is_powered = ((adev->jpeg.ip_dump[inst_off] & 0x1) != 1);
>>> +
>>> +        if (is_powered)
>>> +            for (j = 1; j < adev->jpeg.reg_count; j++)
>>> +                adev->jpeg.ip_dump[inst_off + j] =
>>> +                    RREG32(SOC15_REG_ENTRY_OFFSET_INST(adev-
>>> >jpeg.reg_list[j],
>>> +                                       inst_id));
>>> +    }
>>> +}
>>> +
>>> +void amdgpu_jpeg_print_ip_state(struct amdgpu_ip_block *ip_block,
>>> struct drm_printer *p)
>>> +{
>>> +    struct amdgpu_device *adev = ip_block->adev;
>>> +    u32 inst_off, is_powered;
>>> +    int i, j;
>>> +
>>> +    if (!adev->jpeg.ip_dump)
>>> +        return;
>>> +
>>> +    drm_printf(p, "num_instances:%d\n", adev->jpeg.num_jpeg_inst);
>>> +    for (i = 0; i < adev->jpeg.num_jpeg_inst; i++) {
>>> +        if (adev->jpeg.harvest_config & (1 << i)) {
>>> +            drm_printf(p, "\nHarvested Instance:JPEG%d Skipping
>>> dump\n", i);
>>> +            continue;
>>> +        }
>>> +
>>> +        inst_off = i * adev->jpeg.reg_count;
>>> +        is_powered = ((adev->jpeg.ip_dump[inst_off] & 0x1) != 1);
>>> +
>>> +        if (is_powered) {
>>> +            drm_printf(p, "Active Instance:JPEG%d\n", i);
>>> +            for (j = 0; j < adev->jpeg.reg_count; j++)
>>> +                drm_printf(p, "%-50s \t 0x%08x\n", adev-
>>> >jpeg.reg_list[j].reg_name,
>>> +                       adev->jpeg.ip_dump[inst_off + j]);
>>> +        } else
>>> +            drm_printf(p, "\nInactive Instance:JPEG%d\n", i);
>>> +    }
>>> +}
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h b/drivers/gpu/
>>> drm/amd/amdgpu/amdgpu_jpeg.h
>>> index eb2096dcf1a6..02886ec4466e 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
>>> @@ -92,6 +92,8 @@
>>>           *adev->jpeg.inst[inst_idx].dpg_sram_curr_addr++ = value;    \
>>>       } while (0)
>>>   +struct amdgpu_hwip_reg_entry;
>>> +
>>>   enum amdgpu_jpeg_caps {
>>>       AMDGPU_JPEG_RRMT_ENABLED,
>>>   };
>>> @@ -137,6 +139,9 @@ struct amdgpu_jpeg {
>>>       bool    indirect_sram;
>>>       uint32_t supported_reset;
>>>       uint32_t caps;
>>> +    u32 *ip_dump;
>>> +    u32 reg_count;
>>> +    const struct amdgpu_hwip_reg_entry *reg_list;
>>>   };
>>>   
>> Thanks, this is almost there. Personally, would still prefer something
>> like below and have an instance of this kept inside jpeg_inst (though I
>> see your point that jpeg_inst doesn't have an instance id and this will
>> also mean duplicating list pointer/num reg info in all instances).
> The multiple copies is one reason I am trying to avoid this approach,
> and we can still print only
> the affected instance registers in devcoredump in the future if support
> for it comes up.
>>
>> amdgpu_jpeg_reg_dump {
>>     u32 *ip_dump;
>>     u32 reg_count;
>>     const struct amdgpu_hwip_reg_entry *reg_list;
>> };
>>
>> Ignoring that -
> Thank you, would prefer to have single reference of the reg_list/
> reg_count and use inst_id.
>>
>> With the current way,
>>
>> amdgpu_jpeg_reg_dump_fini() may be called from within sw_fini(). Just
>> wanted to keep a wrapper fini() func to make sure all is cleaned up.
>> That would avoid calling this from every IP version.
> Can we have it the current way instead ? as few ip_versions do not
> support devcoredump yet and calling
> reg_dump_fini() for every ip version irrespective of the  support is
> redundant, if we add that in sw_fini().

Just add a check for reg_list being non-null and that should take care.
Then this doesn't need to be taken care when more IP versions are added.
Also kfree on NULL is harmless.

Thanks,
Lijo

>>
>> Apart from those, leaving it to Leo or someone else from JPEG to take a
>> look.
> Okay, thank you.
> 
> Regards,
> Sathish
>>
>> Regards,
>> LIjo
>>
>>>   int amdgpu_jpeg_sw_init(struct amdgpu_device *adev);
>>> @@ -161,5 +166,10 @@ int amdgpu_jpeg_psp_update_sram(struct
>>> amdgpu_device *adev, int inst_idx,
>>>   void amdgpu_debugfs_jpeg_sched_mask_init(struct amdgpu_device *adev);
>>>   int amdgpu_jpeg_sysfs_reset_mask_init(struct amdgpu_device *adev);
>>>   void amdgpu_jpeg_sysfs_reset_mask_fini(struct amdgpu_device *adev);
>>> +int amdgpu_jpeg_reg_dump_init(struct amdgpu_device *adev,
>>> +                   const struct amdgpu_hwip_reg_entry *reg, u32 count);
>>> +void amdgpu_jpeg_reg_dump_fini(struct amdgpu_device *adev);
>>> +void amdgpu_jpeg_dump_ip_state(struct amdgpu_ip_block *ip_block);
>>> +void amdgpu_jpeg_print_ip_state(struct amdgpu_ip_block *ip_block,
>>> struct drm_printer *p);
>>>     #endif /*__AMDGPU_JPEG_H__*/
> 

Re: [PATCH v2 2/9] drm/amdgpu: Add helper funcs for jpeg devcoredump

[Sundararaju, Sathishkumar]

On 1/29/2025 4:15 PM, Lazar, Lijo wrote:
>
> On 1/29/2025 3:51 PM, Sundararaju, Sathishkumar wrote:
>>
>>
>> On 1/29/2025 3:20 PM, Lazar, Lijo wrote:
>>> On 1/29/2025 2:16 PM, Sathishkumar S wrote:
>>>> Add devcoredump helper functions that can be reused for all jpeg
>>>> versions.
>>>>
>>>> V2: (Lijo)
>>>>    - add amdgpu_jpeg_reg_dump_init() and amdgpu_jpeg_reg_dump_fini()
>>>>    - use reg_list and reg_count from init() to dump and print registers
>>>>    - memory allocation and freeing is moved to the init() and fini()
>>>>
>>>> Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c | 80 ++++++++++++++++++++++++
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h | 10 +++
>>>>    2 files changed, 90 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c b/drivers/gpu/
>>>> drm/amd/amdgpu/amdgpu_jpeg.c
>>>> index b6d2eb049f54..0f9d81e27973 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
>>>> @@ -452,3 +452,83 @@ void amdgpu_jpeg_sysfs_reset_mask_fini(struct
>>>> amdgpu_device *adev)
>>>>                device_remove_file(adev->dev, &dev_attr_jpeg_reset_mask);
>>>>        }
>>>>    }
>>>> +
>>>> +int amdgpu_jpeg_reg_dump_init(struct amdgpu_device *adev,
>>>> +                   const struct amdgpu_hwip_reg_entry *reg, u32 count)
>>>> +{
>>>> +    adev->jpeg.ip_dump = kcalloc(adev->jpeg.num_jpeg_inst * count,
>>>> +                     sizeof(uint32_t), GFP_KERNEL);
>>>> +    if (!adev->jpeg.ip_dump) {
>>>> +        DRM_ERROR("Failed to allocate memory for JPEG IP Dump\n");
>>>> +        return -ENOMEM;
>>>> +    }
>>>> +    adev->jpeg.reg_list = reg;
>>>> +    adev->jpeg.reg_count = count;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +void amdgpu_jpeg_reg_dump_fini(struct amdgpu_device *adev)
>>>> +{
>>>> +    kfree(adev->jpeg.ip_dump);
>>>> +    adev->jpeg.reg_list = NULL;
>>>> +    adev->jpeg.reg_count = 0;
>>>> +}
>>>> +
>>>> +void amdgpu_jpeg_dump_ip_state(struct amdgpu_ip_block *ip_block)
>>>> +{
>>>> +    struct amdgpu_device *adev = ip_block->adev;
>>>> +    u32 inst_off, inst_id, is_powered;
>>>> +    int i, j;
>>>> +
>>>> +    if (!adev->jpeg.ip_dump)
>>>> +        return;
>>>> +
>>>> +    for (i = 0; i < adev->jpeg.num_jpeg_inst; i++) {
>>>> +        if (adev->jpeg.harvest_config & (1 << i))
>>>> +            continue;
>>>> +
>>>> +        inst_id = GET_INST(JPEG, i);
>>>> +        inst_off = i * adev->jpeg.reg_count;
>>>> +        /* check power status from UVD_JPEG_POWER_STATUS */
>>>> +        adev->jpeg.ip_dump[inst_off] =
>>>> +            RREG32(SOC15_REG_ENTRY_OFFSET_INST(adev->jpeg.reg_list[0],
>>>> +                               inst_id));
>>>> +        is_powered = ((adev->jpeg.ip_dump[inst_off] & 0x1) != 1);
>>>> +
>>>> +        if (is_powered)
>>>> +            for (j = 1; j < adev->jpeg.reg_count; j++)
>>>> +                adev->jpeg.ip_dump[inst_off + j] =
>>>> +                    RREG32(SOC15_REG_ENTRY_OFFSET_INST(adev-
>>>>> jpeg.reg_list[j],
>>>> +                                       inst_id));
>>>> +    }
>>>> +}
>>>> +
>>>> +void amdgpu_jpeg_print_ip_state(struct amdgpu_ip_block *ip_block,
>>>> struct drm_printer *p)
>>>> +{
>>>> +    struct amdgpu_device *adev = ip_block->adev;
>>>> +    u32 inst_off, is_powered;
>>>> +    int i, j;
>>>> +
>>>> +    if (!adev->jpeg.ip_dump)
>>>> +        return;
>>>> +
>>>> +    drm_printf(p, "num_instances:%d\n", adev->jpeg.num_jpeg_inst);
>>>> +    for (i = 0; i < adev->jpeg.num_jpeg_inst; i++) {
>>>> +        if (adev->jpeg.harvest_config & (1 << i)) {
>>>> +            drm_printf(p, "\nHarvested Instance:JPEG%d Skipping
>>>> dump\n", i);
>>>> +            continue;
>>>> +        }
>>>> +
>>>> +        inst_off = i * adev->jpeg.reg_count;
>>>> +        is_powered = ((adev->jpeg.ip_dump[inst_off] & 0x1) != 1);
>>>> +
>>>> +        if (is_powered) {
>>>> +            drm_printf(p, "Active Instance:JPEG%d\n", i);
>>>> +            for (j = 0; j < adev->jpeg.reg_count; j++)
>>>> +                drm_printf(p, "%-50s \t 0x%08x\n", adev-
>>>>> jpeg.reg_list[j].reg_name,
>>>> +                       adev->jpeg.ip_dump[inst_off + j]);
>>>> +        } else
>>>> +            drm_printf(p, "\nInactive Instance:JPEG%d\n", i);
>>>> +    }
>>>> +}
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h b/drivers/gpu/
>>>> drm/amd/amdgpu/amdgpu_jpeg.h
>>>> index eb2096dcf1a6..02886ec4466e 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.h
>>>> @@ -92,6 +92,8 @@
>>>>            *adev->jpeg.inst[inst_idx].dpg_sram_curr_addr++ = value;    \
>>>>        } while (0)
>>>>    +struct amdgpu_hwip_reg_entry;
>>>> +
>>>>    enum amdgpu_jpeg_caps {
>>>>        AMDGPU_JPEG_RRMT_ENABLED,
>>>>    };
>>>> @@ -137,6 +139,9 @@ struct amdgpu_jpeg {
>>>>        bool    indirect_sram;
>>>>        uint32_t supported_reset;
>>>>        uint32_t caps;
>>>> +    u32 *ip_dump;
>>>> +    u32 reg_count;
>>>> +    const struct amdgpu_hwip_reg_entry *reg_list;
>>>>    };
>>>>    
>>> Thanks, this is almost there. Personally, would still prefer something
>>> like below and have an instance of this kept inside jpeg_inst (though I
>>> see your point that jpeg_inst doesn't have an instance id and this will
>>> also mean duplicating list pointer/num reg info in all instances).
>> The multiple copies is one reason I am trying to avoid this approach,
>> and we can still print only
>> the affected instance registers in devcoredump in the future if support
>> for it comes up.
>>> amdgpu_jpeg_reg_dump {
>>>      u32 *ip_dump;
>>>      u32 reg_count;
>>>      const struct amdgpu_hwip_reg_entry *reg_list;
>>> };
>>>
>>> Ignoring that -
>> Thank you, would prefer to have single reference of the reg_list/
>> reg_count and use inst_id.
>>> With the current way,
>>>
>>> amdgpu_jpeg_reg_dump_fini() may be called from within sw_fini(). Just
>>> wanted to keep a wrapper fini() func to make sure all is cleaned up.
>>> That would avoid calling this from every IP version.
>> Can we have it the current way instead ? as few ip_versions do not
>> support devcoredump yet and calling
>> reg_dump_fini() for every ip version irrespective of the  support is
>> redundant, if we add that in sw_fini().
> Just add a check for reg_list being non-null and that should take care.
> Then this doesn't need to be taken care when more IP versions are added.
> Also kfree on NULL is harmless.
>
> Thanks,
> Lijo
Any init() caller should be responsible for calling the corresponding 
fini() as well,
it is either good to have both amdgpu_jpeg_reg_dump_init()/fini() moved 
inside sw_init/sw_fini
or leave it to the helper function consumers to call both init() and 
fini() themselves.

Regards,
Sathish
>
>>> Apart from those, leaving it to Leo or someone else from JPEG to take a
>>> look.
>> Okay, thank you.
>>
>> Regards,
>> Sathish
>>> Regards,
>>> LIjo
>>>
>>>>    int amdgpu_jpeg_sw_init(struct amdgpu_device *adev);
>>>> @@ -161,5 +166,10 @@ int amdgpu_jpeg_psp_update_sram(struct
>>>> amdgpu_device *adev, int inst_idx,
>>>>    void amdgpu_debugfs_jpeg_sched_mask_init(struct amdgpu_device *adev);
>>>>    int amdgpu_jpeg_sysfs_reset_mask_init(struct amdgpu_device *adev);
>>>>    void amdgpu_jpeg_sysfs_reset_mask_fini(struct amdgpu_device *adev);
>>>> +int amdgpu_jpeg_reg_dump_init(struct amdgpu_device *adev,
>>>> +                   const struct amdgpu_hwip_reg_entry *reg, u32 count);
>>>> +void amdgpu_jpeg_reg_dump_fini(struct amdgpu_device *adev);
>>>> +void amdgpu_jpeg_dump_ip_state(struct amdgpu_ip_block *ip_block);
>>>> +void amdgpu_jpeg_print_ip_state(struct amdgpu_ip_block *ip_block,
>>>> struct drm_printer *p);
>>>>      #endif /*__AMDGPU_JPEG_H__*/

Re: [PATCH v2 1/9] drm/amdgpu: Add a func for core specific reg offset

[Leo Liu]

This patch is:

Reviewed-by: Leo Liu <leo.liu@amd.com>

On 1/29/25 03:46, Sathishkumar S wrote:
> Add an inline function to calculate core specific register offsets for
> JPEG v4.0.3 and reuse it, makes code more readable and easier to align.
>
> Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c | 54 +++++++++++-------------
>   1 file changed, 24 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
> index 9459e8cc7413..bc21f12daea8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
> @@ -64,6 +64,14 @@ static inline bool jpeg_v4_0_3_normalizn_reqd(struct amdgpu_device *adev)
>   	return (adev->jpeg.caps & AMDGPU_JPEG_CAPS(RRMT_ENABLED)) == 0;
>   }
>   
> +static inline int jpeg_v4_0_3_core_reg_offset(u32 pipe)
> +{
> +	if (pipe)
> +		return ((0x40 * pipe) - 0xc80);
> +	else
> +		return 0;
> +}
> +
>   /**
>    * jpeg_v4_0_3_early_init - set function pointers
>    *
> @@ -143,10 +151,8 @@ static int jpeg_v4_0_3_sw_init(struct amdgpu_ip_block *ip_block)
>   			adev->jpeg.internal.jpeg_pitch[j] =
>   				regUVD_JRBC0_UVD_JRBC_SCRATCH0_INTERNAL_OFFSET;
>   			adev->jpeg.inst[i].external.jpeg_pitch[j] =
> -				SOC15_REG_OFFSET1(
> -					JPEG, jpeg_inst,
> -					regUVD_JRBC0_UVD_JRBC_SCRATCH0,
> -					(j ? (0x40 * j - 0xc80) : 0));
> +				SOC15_REG_OFFSET1(JPEG, jpeg_inst, regUVD_JRBC0_UVD_JRBC_SCRATCH0,
> +						  jpeg_v4_0_3_core_reg_offset(j));
>   		}
>   	}
>   
> @@ -521,7 +527,7 @@ static int jpeg_v4_0_3_start(struct amdgpu_device *adev)
>   			 ~UVD_JMI_CNTL__SOFT_RESET_MASK);
>   
>   		for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
> -			unsigned int reg_offset = (j?(0x40 * j - 0xc80):0);
> +			int reg_offset = jpeg_v4_0_3_core_reg_offset(j);
>   
>   			ring = &adev->jpeg.inst[i].ring_dec[j];
>   
> @@ -616,9 +622,8 @@ static uint64_t jpeg_v4_0_3_dec_ring_get_rptr(struct amdgpu_ring *ring)
>   {
>   	struct amdgpu_device *adev = ring->adev;
>   
> -	return RREG32_SOC15_OFFSET(
> -		JPEG, GET_INST(JPEG, ring->me), regUVD_JRBC0_UVD_JRBC_RB_RPTR,
> -		ring->pipe ? (0x40 * ring->pipe - 0xc80) : 0);
> +	return RREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, ring->me), regUVD_JRBC0_UVD_JRBC_RB_RPTR,
> +				   jpeg_v4_0_3_core_reg_offset(ring->pipe));
>   }
>   
>   /**
> @@ -634,11 +639,9 @@ static uint64_t jpeg_v4_0_3_dec_ring_get_wptr(struct amdgpu_ring *ring)
>   
>   	if (ring->use_doorbell)
>   		return adev->wb.wb[ring->wptr_offs];
> -	else
> -		return RREG32_SOC15_OFFSET(
> -			JPEG, GET_INST(JPEG, ring->me),
> -			regUVD_JRBC0_UVD_JRBC_RB_WPTR,
> -			ring->pipe ? (0x40 * ring->pipe - 0xc80) : 0);
> +
> +	return RREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, ring->me), regUVD_JRBC0_UVD_JRBC_RB_WPTR,
> +				   jpeg_v4_0_3_core_reg_offset(ring->pipe));
>   }
>   
>   static void jpeg_v4_0_3_ring_emit_hdp_flush(struct amdgpu_ring *ring)
> @@ -663,10 +666,8 @@ static void jpeg_v4_0_3_dec_ring_set_wptr(struct amdgpu_ring *ring)
>   		adev->wb.wb[ring->wptr_offs] = lower_32_bits(ring->wptr);
>   		WDOORBELL32(ring->doorbell_index, lower_32_bits(ring->wptr));
>   	} else {
> -		WREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, ring->me),
> -				    regUVD_JRBC0_UVD_JRBC_RB_WPTR,
> -				    (ring->pipe ? (0x40 * ring->pipe - 0xc80) :
> -						  0),
> +		WREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, ring->me), regUVD_JRBC0_UVD_JRBC_RB_WPTR,
> +				    jpeg_v4_0_3_core_reg_offset(ring->pipe),
>   				    lower_32_bits(ring->wptr));
>   	}
>   }
> @@ -919,13 +920,9 @@ static bool jpeg_v4_0_3_is_idle(void *handle)
>   
>   	for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
>   		for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
> -			unsigned int reg_offset = (j?(0x40 * j - 0xc80):0);
> -
> -			ret &= ((RREG32_SOC15_OFFSET(
> -					 JPEG, GET_INST(JPEG, i),
> -					 regUVD_JRBC0_UVD_JRBC_STATUS,
> -					 reg_offset) &
> -				 UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK) ==
> +			ret &= ((RREG32_SOC15_OFFSET(JPEG, GET_INST(JPEG, i),
> +				regUVD_JRBC0_UVD_JRBC_STATUS, jpeg_v4_0_3_core_reg_offset(j)) &
> +				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK) ==
>   				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK);
>   		}
>   	}
> @@ -941,13 +938,10 @@ static int jpeg_v4_0_3_wait_for_idle(struct amdgpu_ip_block *ip_block)
>   
>   	for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
>   		for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
> -			unsigned int reg_offset = (j?(0x40 * j - 0xc80):0);
> -
> -			ret &= SOC15_WAIT_ON_RREG_OFFSET(
> -				JPEG, GET_INST(JPEG, i),
> -				regUVD_JRBC0_UVD_JRBC_STATUS, reg_offset,
> +			ret &= (SOC15_WAIT_ON_RREG_OFFSET(JPEG, GET_INST(JPEG, i),
> +				regUVD_JRBC0_UVD_JRBC_STATUS, jpeg_v4_0_3_core_reg_offset(j),
>   				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK,
> -				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK);
> +				UVD_JRBC0_UVD_JRBC_STATUS__RB_JOB_DONE_MASK));
>   		}
>   	}
>   	return ret;

Re: [PATCH v2 9/9] drm/amdgpu: Enable devcoredump for JPEG2_5_0

[Leo Liu]

Looks pretty good to me. One question that why this is not added to JPEG 
5_0_0? Since the patch 3-9 cover the rest of versions, they are:

Reviewed-by: Leo Liu <leo.liu@amd.com>

On 1/29/25 03:46, Sathishkumar S wrote:
> Add register list and enable devcoredump for JPEG2_5_0
>
> V2: (Lijo)
> - remove version specific callbacks and use simplified helper functions
>
> Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
> index 11f6af2646e7..536a6101dcf9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
> @@ -36,6 +36,22 @@
>   
>   #define JPEG25_MAX_HW_INSTANCES_ARCTURUS			2
>   
> +static const struct amdgpu_hwip_reg_entry jpeg_reg_list_2_5[] = {
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_POWER_STATUS),
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_INT_STAT),
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_RPTR),
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_WPTR),
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_CNTL),
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_SIZE),
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_STATUS),
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_ADDR_MODE),
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_GFX10_ADDR_CONFIG),
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_Y_GFX10_TILING_SURFACE),
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_UV_GFX10_TILING_SURFACE),
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_PITCH),
> +	SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_UV_PITCH),
> +};
> +
>   static void jpeg_v2_5_set_dec_ring_funcs(struct amdgpu_device *adev);
>   static void jpeg_v2_5_set_irq_funcs(struct amdgpu_device *adev);
>   static int jpeg_v2_5_set_powergating_state(struct amdgpu_ip_block *ip_block,
> @@ -147,6 +163,10 @@ static int jpeg_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
>   	if (r)
>   		return r;
>   
> +	r = amdgpu_jpeg_reg_dump_init(adev, jpeg_reg_list_2_5, ARRAY_SIZE(jpeg_reg_list_2_5));
> +	if (r)
> +		return r;
> +
>   	return 0;
>   }
>   
> @@ -168,6 +188,8 @@ static int jpeg_v2_5_sw_fini(struct amdgpu_ip_block *ip_block)
>   
>   	r = amdgpu_jpeg_sw_fini(adev);
>   
> +	amdgpu_jpeg_reg_dump_fini(adev);
> +
>   	return r;
>   }
>   
> @@ -623,6 +645,8 @@ static const struct amd_ip_funcs jpeg_v2_5_ip_funcs = {
>   	.wait_for_idle = jpeg_v2_5_wait_for_idle,
>   	.set_clockgating_state = jpeg_v2_5_set_clockgating_state,
>   	.set_powergating_state = jpeg_v2_5_set_powergating_state,
> +	.dump_ip_state = amdgpu_jpeg_dump_ip_state,
> +	.print_ip_state = amdgpu_jpeg_print_ip_state,
>   };
>   
>   static const struct amd_ip_funcs jpeg_v2_6_ip_funcs = {
> @@ -638,6 +662,8 @@ static const struct amd_ip_funcs jpeg_v2_6_ip_funcs = {
>   	.wait_for_idle = jpeg_v2_5_wait_for_idle,
>   	.set_clockgating_state = jpeg_v2_5_set_clockgating_state,
>   	.set_powergating_state = jpeg_v2_5_set_powergating_state,
> +	.dump_ip_state = amdgpu_jpeg_dump_ip_state,
> +	.print_ip_state = amdgpu_jpeg_print_ip_state,
>   };
>   
>   static const struct amdgpu_ring_funcs jpeg_v2_5_dec_ring_vm_funcs = {

Re: [PATCH v2 9/9] drm/amdgpu: Enable devcoredump for JPEG2_5_0

[Sundararaju, Sathishkumar]

Hi Leo,

Oh! I somehow missed it, I will enable it in 5_0_0 as well.
Initially I had some confusion around 4_0_5 and 5_0_0 as they both are 
in dpg mode, but after checking the existing register access I enabled 
it in 4_0_5, but missed to enable it in 5_0_0.

Regards,
Sathish

On 1/29/2025 10:06 PM, Leo Liu wrote:
> Looks pretty good to me. One question that why this is not added to 
> JPEG 5_0_0? Since the patch 3-9 cover the rest of versions, they are:
>
> Reviewed-by: Leo Liu <leo.liu@amd.com>
>
> On 1/29/25 03:46, Sathishkumar S wrote:
>> Add register list and enable devcoredump for JPEG2_5_0
>>
>> V2: (Lijo)
>> - remove version specific callbacks and use simplified helper functions
>>
>> Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c | 26 ++++++++++++++++++++++++++
>>   1 file changed, 26 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c 
>> b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
>> index 11f6af2646e7..536a6101dcf9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
>> @@ -36,6 +36,22 @@
>>     #define JPEG25_MAX_HW_INSTANCES_ARCTURUS            2
>>   +static const struct amdgpu_hwip_reg_entry jpeg_reg_list_2_5[] = {
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_POWER_STATUS),
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_INT_STAT),
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_RPTR),
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_WPTR),
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_CNTL),
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_RB_SIZE),
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JRBC_STATUS),
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_ADDR_MODE),
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_GFX10_ADDR_CONFIG),
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_Y_GFX10_TILING_SURFACE),
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmJPEG_DEC_UV_GFX10_TILING_SURFACE),
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_PITCH),
>> +    SOC15_REG_ENTRY_STR(JPEG, 0, mmUVD_JPEG_UV_PITCH),
>> +};
>> +
>>   static void jpeg_v2_5_set_dec_ring_funcs(struct amdgpu_device *adev);
>>   static void jpeg_v2_5_set_irq_funcs(struct amdgpu_device *adev);
>>   static int jpeg_v2_5_set_powergating_state(struct amdgpu_ip_block 
>> *ip_block,
>> @@ -147,6 +163,10 @@ static int jpeg_v2_5_sw_init(struct 
>> amdgpu_ip_block *ip_block)
>>       if (r)
>>           return r;
>>   +    r = amdgpu_jpeg_reg_dump_init(adev, jpeg_reg_list_2_5, 
>> ARRAY_SIZE(jpeg_reg_list_2_5));
>> +    if (r)
>> +        return r;
>> +
>>       return 0;
>>   }
>>   @@ -168,6 +188,8 @@ static int jpeg_v2_5_sw_fini(struct 
>> amdgpu_ip_block *ip_block)
>>         r = amdgpu_jpeg_sw_fini(adev);
>>   +    amdgpu_jpeg_reg_dump_fini(adev);
>> +
>>       return r;
>>   }
>>   @@ -623,6 +645,8 @@ static const struct amd_ip_funcs 
>> jpeg_v2_5_ip_funcs = {
>>       .wait_for_idle = jpeg_v2_5_wait_for_idle,
>>       .set_clockgating_state = jpeg_v2_5_set_clockgating_state,
>>       .set_powergating_state = jpeg_v2_5_set_powergating_state,
>> +    .dump_ip_state = amdgpu_jpeg_dump_ip_state,
>> +    .print_ip_state = amdgpu_jpeg_print_ip_state,
>>   };
>>     static const struct amd_ip_funcs jpeg_v2_6_ip_funcs = {
>> @@ -638,6 +662,8 @@ static const struct amd_ip_funcs 
>> jpeg_v2_6_ip_funcs = {
>>       .wait_for_idle = jpeg_v2_5_wait_for_idle,
>>       .set_clockgating_state = jpeg_v2_5_set_clockgating_state,
>>       .set_powergating_state = jpeg_v2_5_set_powergating_state,
>> +    .dump_ip_state = amdgpu_jpeg_dump_ip_state,
>> +    .print_ip_state = amdgpu_jpeg_print_ip_state,
>>   };
>>     static const struct amdgpu_ring_funcs jpeg_v2_5_dec_ring_vm_funcs 
>> = {