* [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function"
@ 2025-05-05 13:04 Alex Deucher
2025-05-05 13:04 ` [PATCH 2/7] drm/amdgpu/hdp4: use memcfg register to post the write for HDP flush Alex Deucher
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: Alex Deucher @ 2025-05-05 13:04 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher
This reverts commit 18a878fd8aef0ec21648a3782f55a79790cd4073.
Revert this temporarily to make it easier to fix a regression
in the HDP handling.
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c | 21 ---------------------
drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h | 2 --
drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 13 ++++++++++++-
drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c | 13 ++++++++++++-
drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c | 13 ++++++++++++-
drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c | 13 ++++++++++++-
6 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
index 7fd8f09c28e66..b6cf801939aa5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
@@ -22,7 +22,6 @@
*/
#include "amdgpu.h"
#include "amdgpu_ras.h"
-#include <uapi/linux/kfd_ioctl.h>
int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
{
@@ -47,23 +46,3 @@ int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
/* hdp ras follows amdgpu_ras_block_late_init_default for late init */
return 0;
}
-
-void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
- struct amdgpu_ring *ring)
-{
- if (!ring || !ring->funcs->emit_wreg) {
- WREG32((adev->rmmio_remap.reg_offset +
- KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
- 2,
- 0);
- RREG32((adev->rmmio_remap.reg_offset +
- KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
- 2);
- } else {
- amdgpu_ring_emit_wreg(ring,
- (adev->rmmio_remap.reg_offset +
- KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
- 2,
- 0);
- }
-}
\ No newline at end of file
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
index 4cfd932b7e91e..7b8a6152dc8d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
@@ -44,6 +44,4 @@ struct amdgpu_hdp {
};
int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev);
-void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
- struct amdgpu_ring *ring);
#endif /* __AMDGPU_HDP_H__ */
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
index e6c0d86d34865..f1dc13b3ab38e 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
@@ -36,6 +36,17 @@
#define HDP_MEM_POWER_CTRL__RC_MEM_POWER_LS_EN_MASK 0x00020000L
#define mmHDP_MEM_POWER_CTRL_BASE_IDX 0
+static void hdp_v4_0_flush_hdp(struct amdgpu_device *adev,
+ struct amdgpu_ring *ring)
+{
+ if (!ring || !ring->funcs->emit_wreg) {
+ WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ } else {
+ amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ }
+}
+
static void hdp_v4_0_invalidate_hdp(struct amdgpu_device *adev,
struct amdgpu_ring *ring)
{
@@ -169,7 +180,7 @@ struct amdgpu_hdp_ras hdp_v4_0_ras = {
};
const struct amdgpu_hdp_funcs hdp_v4_0_funcs = {
- .flush_hdp = amdgpu_hdp_generic_flush,
+ .flush_hdp = hdp_v4_0_flush_hdp,
.invalidate_hdp = hdp_v4_0_invalidate_hdp,
.update_clock_gating = hdp_v4_0_update_clock_gating,
.get_clock_gating_state = hdp_v4_0_get_clockgating_state,
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
index 8bc001dc9f631..43195c0797480 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
@@ -27,6 +27,17 @@
#include "hdp/hdp_5_0_0_sh_mask.h"
#include <uapi/linux/kfd_ioctl.h>
+static void hdp_v5_0_flush_hdp(struct amdgpu_device *adev,
+ struct amdgpu_ring *ring)
+{
+ if (!ring || !ring->funcs->emit_wreg) {
+ WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ } else {
+ amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ }
+}
+
static void hdp_v5_0_invalidate_hdp(struct amdgpu_device *adev,
struct amdgpu_ring *ring)
{
@@ -206,7 +217,7 @@ static void hdp_v5_0_init_registers(struct amdgpu_device *adev)
}
const struct amdgpu_hdp_funcs hdp_v5_0_funcs = {
- .flush_hdp = amdgpu_hdp_generic_flush,
+ .flush_hdp = hdp_v5_0_flush_hdp,
.invalidate_hdp = hdp_v5_0_invalidate_hdp,
.update_clock_gating = hdp_v5_0_update_clock_gating,
.get_clock_gating_state = hdp_v5_0_get_clockgating_state,
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
index ec20daf4272c5..a88d25a06c29b 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
@@ -30,6 +30,17 @@
#define regHDP_CLK_CNTL_V6_1 0xd5
#define regHDP_CLK_CNTL_V6_1_BASE_IDX 0
+static void hdp_v6_0_flush_hdp(struct amdgpu_device *adev,
+ struct amdgpu_ring *ring)
+{
+ if (!ring || !ring->funcs->emit_wreg) {
+ WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ } else {
+ amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ }
+}
+
static void hdp_v6_0_update_clock_gating(struct amdgpu_device *adev,
bool enable)
{
@@ -138,7 +149,7 @@ static void hdp_v6_0_get_clockgating_state(struct amdgpu_device *adev,
}
const struct amdgpu_hdp_funcs hdp_v6_0_funcs = {
- .flush_hdp = amdgpu_hdp_generic_flush,
+ .flush_hdp = hdp_v6_0_flush_hdp,
.update_clock_gating = hdp_v6_0_update_clock_gating,
.get_clock_gating_state = hdp_v6_0_get_clockgating_state,
};
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
index ed1debc035073..49f7eb4fbd117 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
@@ -27,6 +27,17 @@
#include "hdp/hdp_7_0_0_sh_mask.h"
#include <uapi/linux/kfd_ioctl.h>
+static void hdp_v7_0_flush_hdp(struct amdgpu_device *adev,
+ struct amdgpu_ring *ring)
+{
+ if (!ring || !ring->funcs->emit_wreg) {
+ WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ } else {
+ amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ }
+}
+
static void hdp_v7_0_update_clock_gating(struct amdgpu_device *adev,
bool enable)
{
@@ -126,7 +137,7 @@ static void hdp_v7_0_get_clockgating_state(struct amdgpu_device *adev,
}
const struct amdgpu_hdp_funcs hdp_v7_0_funcs = {
- .flush_hdp = amdgpu_hdp_generic_flush,
+ .flush_hdp = hdp_v7_0_flush_hdp,
.update_clock_gating = hdp_v7_0_update_clock_gating,
.get_clock_gating_state = hdp_v7_0_get_clockgating_state,
};
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/7] drm/amdgpu/hdp4: use memcfg register to post the write for HDP flush
2025-05-05 13:04 [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
@ 2025-05-05 13:04 ` Alex Deucher
2025-05-05 13:04 ` [PATCH 3/7] drm/amdgpu/hdp5: " Alex Deucher
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Alex Deucher @ 2025-05-05 13:04 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Alexey Klimov
Reading back the remapped HDP flush register seems to cause
problems on some platforms. All we need is a read, so read back
the memcfg register.
Fixes: c9b8dcabb52a ("drm/amdgpu/hdp4.0: do a posting read when flushing HDP")
Reported-by: Alexey Klimov <alexey.klimov@linaro.org>
Link: https://lists.freedesktop.org/archives/amd-gfx/2025-April/123150.html
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4119
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3908
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
index f1dc13b3ab38e..cbbeadeb53f72 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
@@ -41,7 +41,12 @@ static void hdp_v4_0_flush_hdp(struct amdgpu_device *adev,
{
if (!ring || !ring->funcs->emit_wreg) {
WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
- RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ /* We just need to read back a register to post the write.
+ * Reading back the remapped register causes problems on
+ * some platforms so just read back the memory size register.
+ */
+ if (adev->nbio.funcs->get_memsize)
+ adev->nbio.funcs->get_memsize(adev);
} else {
amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
}
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/7] drm/amdgpu/hdp5: use memcfg register to post the write for HDP flush
2025-05-05 13:04 [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
2025-05-05 13:04 ` [PATCH 2/7] drm/amdgpu/hdp4: use memcfg register to post the write for HDP flush Alex Deucher
@ 2025-05-05 13:04 ` Alex Deucher
2025-05-05 13:04 ` [PATCH 4/7] drm/amdgpu/hdp5.2: " Alex Deucher
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Alex Deucher @ 2025-05-05 13:04 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Alexey Klimov
Reading back the remapped HDP flush register seems to cause
problems on some platforms. All we need is a read, so read back
the memcfg register.
Fixes: cf424020e040 ("drm/amdgpu/hdp5.0: do a posting read when flushing HDP")
Reported-by: Alexey Klimov <alexey.klimov@linaro.org>
Link: https://lists.freedesktop.org/archives/amd-gfx/2025-April/123150.html
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4119
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3908
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
index 43195c0797480..086a647308df0 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
@@ -32,7 +32,12 @@ static void hdp_v5_0_flush_hdp(struct amdgpu_device *adev,
{
if (!ring || !ring->funcs->emit_wreg) {
WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
- RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ /* We just need to read back a register to post the write.
+ * Reading back the remapped register causes problems on
+ * some platforms so just read back the memory size register.
+ */
+ if (adev->nbio.funcs->get_memsize)
+ adev->nbio.funcs->get_memsize(adev);
} else {
amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
}
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/7] drm/amdgpu/hdp5.2: use memcfg register to post the write for HDP flush
2025-05-05 13:04 [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
2025-05-05 13:04 ` [PATCH 2/7] drm/amdgpu/hdp4: use memcfg register to post the write for HDP flush Alex Deucher
2025-05-05 13:04 ` [PATCH 3/7] drm/amdgpu/hdp5: " Alex Deucher
@ 2025-05-05 13:04 ` Alex Deucher
2025-05-05 13:04 ` [PATCH 5/7] drm/amdgpu/hdp6: " Alex Deucher
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Alex Deucher @ 2025-05-05 13:04 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Alexey Klimov
Reading back the remapped HDP flush register seems to cause
problems on some platforms. All we need is a read, so read back
the memcfg register.
Fixes: f756dbac1ce1 ("drm/amdgpu/hdp5.2: do a posting read when flushing HDP")
Reported-by: Alexey Klimov <alexey.klimov@linaro.org>
Link: https://lists.freedesktop.org/archives/amd-gfx/2025-April/123150.html
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4119
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3908
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/hdp_v5_2.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v5_2.c b/drivers/gpu/drm/amd/amdgpu/hdp_v5_2.c
index fcb8dd2876bcc..40940b4ab4007 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v5_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v5_2.c
@@ -33,7 +33,17 @@ static void hdp_v5_2_flush_hdp(struct amdgpu_device *adev,
if (!ring || !ring->funcs->emit_wreg) {
WREG32_NO_KIQ((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2,
0);
- RREG32_NO_KIQ((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ if (amdgpu_sriov_vf(adev)) {
+ /* this is fine because SR_IOV doesn't remap the register */
+ RREG32_NO_KIQ((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ } else {
+ /* We just need to read back a register to post the write.
+ * Reading back the remapped register causes problems on
+ * some platforms so just read back the memory size register.
+ */
+ if (adev->nbio.funcs->get_memsize)
+ adev->nbio.funcs->get_memsize(adev);
+ }
} else {
amdgpu_ring_emit_wreg(ring,
(adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2,
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/7] drm/amdgpu/hdp6: use memcfg register to post the write for HDP flush
2025-05-05 13:04 [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
` (2 preceding siblings ...)
2025-05-05 13:04 ` [PATCH 4/7] drm/amdgpu/hdp5.2: " Alex Deucher
@ 2025-05-05 13:04 ` Alex Deucher
2025-05-05 13:04 ` [PATCH 6/7] drm/amdgpu/hdp7: " Alex Deucher
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Alex Deucher @ 2025-05-05 13:04 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Alexey Klimov
Reading back the remapped HDP flush register seems to cause
problems on some platforms. All we need is a read, so read back
the memcfg register.
Fixes: abe1cbaec6cf ("drm/amdgpu/hdp6.0: do a posting read when flushing HDP")
Reported-by: Alexey Klimov <alexey.klimov@linaro.org>
Link: https://lists.freedesktop.org/archives/amd-gfx/2025-April/123150.html
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4119
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3908
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
index a88d25a06c29b..6ccd31c8bc692 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
@@ -35,7 +35,12 @@ static void hdp_v6_0_flush_hdp(struct amdgpu_device *adev,
{
if (!ring || !ring->funcs->emit_wreg) {
WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
- RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ /* We just need to read back a register to post the write.
+ * Reading back the remapped register causes problems on
+ * some platforms so just read back the memory size register.
+ */
+ if (adev->nbio.funcs->get_memsize)
+ adev->nbio.funcs->get_memsize(adev);
} else {
amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
}
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/7] drm/amdgpu/hdp7: use memcfg register to post the write for HDP flush
2025-05-05 13:04 [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
` (3 preceding siblings ...)
2025-05-05 13:04 ` [PATCH 5/7] drm/amdgpu/hdp6: " Alex Deucher
@ 2025-05-05 13:04 ` Alex Deucher
2025-05-05 13:04 ` [PATCH 7/7] Reapply: drm/amdgpu: Use generic hdp flush function Alex Deucher
2025-05-06 15:17 ` [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
6 siblings, 0 replies; 13+ messages in thread
From: Alex Deucher @ 2025-05-05 13:04 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Alexey Klimov
Reading back the remapped HDP flush register seems to cause
problems on some platforms. All we need is a read, so read back
the memcfg register.
Fixes: 689275140cb8 ("drm/amdgpu/hdp7.0: do a posting read when flushing HDP")
Reported-by: Alexey Klimov <alexey.klimov@linaro.org>
Link: https://lists.freedesktop.org/archives/amd-gfx/2025-April/123150.html
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4119
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3908
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
index 49f7eb4fbd117..2c9239a22f398 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
@@ -32,7 +32,12 @@ static void hdp_v7_0_flush_hdp(struct amdgpu_device *adev,
{
if (!ring || !ring->funcs->emit_wreg) {
WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
- RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ /* We just need to read back a register to post the write.
+ * Reading back the remapped register causes problems on
+ * some platforms so just read back the memory size register.
+ */
+ if (adev->nbio.funcs->get_memsize)
+ adev->nbio.funcs->get_memsize(adev);
} else {
amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
}
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 7/7] Reapply: drm/amdgpu: Use generic hdp flush function
2025-05-05 13:04 [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
` (4 preceding siblings ...)
2025-05-05 13:04 ` [PATCH 6/7] drm/amdgpu/hdp7: " Alex Deucher
@ 2025-05-05 13:04 ` Alex Deucher
2025-05-06 15:17 ` [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
6 siblings, 0 replies; 13+ messages in thread
From: Alex Deucher @ 2025-05-05 13:04 UTC (permalink / raw)
To: amd-gfx; +Cc: Lijo Lazar, Alex Deucher
From: Lijo Lazar <lijo.lazar@amd.com>
Except HDP v5.2 all use a common logic for HDP flush. Use a generic
function. HDP v5.2 forces NO_KIQ logic, revisit it later.
Reapply after fixing up an HDP regression.
Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c | 21 +++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h | 2 ++
drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 18 +-----------------
drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c | 18 +-----------------
drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c | 18 +-----------------
drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c | 18 +-----------------
6 files changed, 27 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
index b6cf801939aa5..7fd8f09c28e66 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
@@ -22,6 +22,7 @@
*/
#include "amdgpu.h"
#include "amdgpu_ras.h"
+#include <uapi/linux/kfd_ioctl.h>
int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
{
@@ -46,3 +47,23 @@ int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
/* hdp ras follows amdgpu_ras_block_late_init_default for late init */
return 0;
}
+
+void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
+ struct amdgpu_ring *ring)
+{
+ if (!ring || !ring->funcs->emit_wreg) {
+ WREG32((adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
+ 2,
+ 0);
+ RREG32((adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
+ 2);
+ } else {
+ amdgpu_ring_emit_wreg(ring,
+ (adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
+ 2,
+ 0);
+ }
+}
\ No newline at end of file
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
index 7b8a6152dc8d9..4cfd932b7e91e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
@@ -44,4 +44,6 @@ struct amdgpu_hdp {
};
int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev);
+void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
+ struct amdgpu_ring *ring);
#endif /* __AMDGPU_HDP_H__ */
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
index cbbeadeb53f72..e6c0d86d34865 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
@@ -36,22 +36,6 @@
#define HDP_MEM_POWER_CTRL__RC_MEM_POWER_LS_EN_MASK 0x00020000L
#define mmHDP_MEM_POWER_CTRL_BASE_IDX 0
-static void hdp_v4_0_flush_hdp(struct amdgpu_device *adev,
- struct amdgpu_ring *ring)
-{
- if (!ring || !ring->funcs->emit_wreg) {
- WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
- /* We just need to read back a register to post the write.
- * Reading back the remapped register causes problems on
- * some platforms so just read back the memory size register.
- */
- if (adev->nbio.funcs->get_memsize)
- adev->nbio.funcs->get_memsize(adev);
- } else {
- amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
- }
-}
-
static void hdp_v4_0_invalidate_hdp(struct amdgpu_device *adev,
struct amdgpu_ring *ring)
{
@@ -185,7 +169,7 @@ struct amdgpu_hdp_ras hdp_v4_0_ras = {
};
const struct amdgpu_hdp_funcs hdp_v4_0_funcs = {
- .flush_hdp = hdp_v4_0_flush_hdp,
+ .flush_hdp = amdgpu_hdp_generic_flush,
.invalidate_hdp = hdp_v4_0_invalidate_hdp,
.update_clock_gating = hdp_v4_0_update_clock_gating,
.get_clock_gating_state = hdp_v4_0_get_clockgating_state,
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
index 086a647308df0..8bc001dc9f631 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
@@ -27,22 +27,6 @@
#include "hdp/hdp_5_0_0_sh_mask.h"
#include <uapi/linux/kfd_ioctl.h>
-static void hdp_v5_0_flush_hdp(struct amdgpu_device *adev,
- struct amdgpu_ring *ring)
-{
- if (!ring || !ring->funcs->emit_wreg) {
- WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
- /* We just need to read back a register to post the write.
- * Reading back the remapped register causes problems on
- * some platforms so just read back the memory size register.
- */
- if (adev->nbio.funcs->get_memsize)
- adev->nbio.funcs->get_memsize(adev);
- } else {
- amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
- }
-}
-
static void hdp_v5_0_invalidate_hdp(struct amdgpu_device *adev,
struct amdgpu_ring *ring)
{
@@ -222,7 +206,7 @@ static void hdp_v5_0_init_registers(struct amdgpu_device *adev)
}
const struct amdgpu_hdp_funcs hdp_v5_0_funcs = {
- .flush_hdp = hdp_v5_0_flush_hdp,
+ .flush_hdp = amdgpu_hdp_generic_flush,
.invalidate_hdp = hdp_v5_0_invalidate_hdp,
.update_clock_gating = hdp_v5_0_update_clock_gating,
.get_clock_gating_state = hdp_v5_0_get_clockgating_state,
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
index 6ccd31c8bc692..ec20daf4272c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
@@ -30,22 +30,6 @@
#define regHDP_CLK_CNTL_V6_1 0xd5
#define regHDP_CLK_CNTL_V6_1_BASE_IDX 0
-static void hdp_v6_0_flush_hdp(struct amdgpu_device *adev,
- struct amdgpu_ring *ring)
-{
- if (!ring || !ring->funcs->emit_wreg) {
- WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
- /* We just need to read back a register to post the write.
- * Reading back the remapped register causes problems on
- * some platforms so just read back the memory size register.
- */
- if (adev->nbio.funcs->get_memsize)
- adev->nbio.funcs->get_memsize(adev);
- } else {
- amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
- }
-}
-
static void hdp_v6_0_update_clock_gating(struct amdgpu_device *adev,
bool enable)
{
@@ -154,7 +138,7 @@ static void hdp_v6_0_get_clockgating_state(struct amdgpu_device *adev,
}
const struct amdgpu_hdp_funcs hdp_v6_0_funcs = {
- .flush_hdp = hdp_v6_0_flush_hdp,
+ .flush_hdp = amdgpu_hdp_generic_flush,
.update_clock_gating = hdp_v6_0_update_clock_gating,
.get_clock_gating_state = hdp_v6_0_get_clockgating_state,
};
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
index 2c9239a22f398..ed1debc035073 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
@@ -27,22 +27,6 @@
#include "hdp/hdp_7_0_0_sh_mask.h"
#include <uapi/linux/kfd_ioctl.h>
-static void hdp_v7_0_flush_hdp(struct amdgpu_device *adev,
- struct amdgpu_ring *ring)
-{
- if (!ring || !ring->funcs->emit_wreg) {
- WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
- /* We just need to read back a register to post the write.
- * Reading back the remapped register causes problems on
- * some platforms so just read back the memory size register.
- */
- if (adev->nbio.funcs->get_memsize)
- adev->nbio.funcs->get_memsize(adev);
- } else {
- amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
- }
-}
-
static void hdp_v7_0_update_clock_gating(struct amdgpu_device *adev,
bool enable)
{
@@ -142,7 +126,7 @@ static void hdp_v7_0_get_clockgating_state(struct amdgpu_device *adev,
}
const struct amdgpu_hdp_funcs hdp_v7_0_funcs = {
- .flush_hdp = hdp_v7_0_flush_hdp,
+ .flush_hdp = amdgpu_hdp_generic_flush,
.update_clock_gating = hdp_v7_0_update_clock_gating,
.get_clock_gating_state = hdp_v7_0_get_clockgating_state,
};
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function"
2025-05-05 13:04 [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
` (5 preceding siblings ...)
2025-05-05 13:04 ` [PATCH 7/7] Reapply: drm/amdgpu: Use generic hdp flush function Alex Deucher
@ 2025-05-06 15:17 ` Alex Deucher
2025-05-07 15:13 ` Alex Deucher
2025-05-07 15:27 ` Felix Kuehling
6 siblings, 2 replies; 13+ messages in thread
From: Alex Deucher @ 2025-05-06 15:17 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx
Ping on this series? This fixes a regression.
Alex
On Mon, May 5, 2025 at 9:05 AM Alex Deucher <alexander.deucher@amd.com> wrote:
>
> This reverts commit 18a878fd8aef0ec21648a3782f55a79790cd4073.
>
> Revert this temporarily to make it easier to fix a regression
> in the HDP handling.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c | 21 ---------------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h | 2 --
> drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 13 ++++++++++++-
> drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c | 13 ++++++++++++-
> drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c | 13 ++++++++++++-
> drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c | 13 ++++++++++++-
> 6 files changed, 48 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
> index 7fd8f09c28e66..b6cf801939aa5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
> @@ -22,7 +22,6 @@
> */
> #include "amdgpu.h"
> #include "amdgpu_ras.h"
> -#include <uapi/linux/kfd_ioctl.h>
>
> int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
> {
> @@ -47,23 +46,3 @@ int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
> /* hdp ras follows amdgpu_ras_block_late_init_default for late init */
> return 0;
> }
> -
> -void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
> - struct amdgpu_ring *ring)
> -{
> - if (!ring || !ring->funcs->emit_wreg) {
> - WREG32((adev->rmmio_remap.reg_offset +
> - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
> - 2,
> - 0);
> - RREG32((adev->rmmio_remap.reg_offset +
> - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
> - 2);
> - } else {
> - amdgpu_ring_emit_wreg(ring,
> - (adev->rmmio_remap.reg_offset +
> - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
> - 2,
> - 0);
> - }
> -}
> \ No newline at end of file
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> index 4cfd932b7e91e..7b8a6152dc8d9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> @@ -44,6 +44,4 @@ struct amdgpu_hdp {
> };
>
> int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev);
> -void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
> - struct amdgpu_ring *ring);
> #endif /* __AMDGPU_HDP_H__ */
> diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> index e6c0d86d34865..f1dc13b3ab38e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> @@ -36,6 +36,17 @@
> #define HDP_MEM_POWER_CTRL__RC_MEM_POWER_LS_EN_MASK 0x00020000L
> #define mmHDP_MEM_POWER_CTRL_BASE_IDX 0
>
> +static void hdp_v4_0_flush_hdp(struct amdgpu_device *adev,
> + struct amdgpu_ring *ring)
> +{
> + if (!ring || !ring->funcs->emit_wreg) {
> + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> + } else {
> + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> + }
> +}
> +
> static void hdp_v4_0_invalidate_hdp(struct amdgpu_device *adev,
> struct amdgpu_ring *ring)
> {
> @@ -169,7 +180,7 @@ struct amdgpu_hdp_ras hdp_v4_0_ras = {
> };
>
> const struct amdgpu_hdp_funcs hdp_v4_0_funcs = {
> - .flush_hdp = amdgpu_hdp_generic_flush,
> + .flush_hdp = hdp_v4_0_flush_hdp,
> .invalidate_hdp = hdp_v4_0_invalidate_hdp,
> .update_clock_gating = hdp_v4_0_update_clock_gating,
> .get_clock_gating_state = hdp_v4_0_get_clockgating_state,
> diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
> index 8bc001dc9f631..43195c0797480 100644
> --- a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
> @@ -27,6 +27,17 @@
> #include "hdp/hdp_5_0_0_sh_mask.h"
> #include <uapi/linux/kfd_ioctl.h>
>
> +static void hdp_v5_0_flush_hdp(struct amdgpu_device *adev,
> + struct amdgpu_ring *ring)
> +{
> + if (!ring || !ring->funcs->emit_wreg) {
> + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> + } else {
> + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> + }
> +}
> +
> static void hdp_v5_0_invalidate_hdp(struct amdgpu_device *adev,
> struct amdgpu_ring *ring)
> {
> @@ -206,7 +217,7 @@ static void hdp_v5_0_init_registers(struct amdgpu_device *adev)
> }
>
> const struct amdgpu_hdp_funcs hdp_v5_0_funcs = {
> - .flush_hdp = amdgpu_hdp_generic_flush,
> + .flush_hdp = hdp_v5_0_flush_hdp,
> .invalidate_hdp = hdp_v5_0_invalidate_hdp,
> .update_clock_gating = hdp_v5_0_update_clock_gating,
> .get_clock_gating_state = hdp_v5_0_get_clockgating_state,
> diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
> index ec20daf4272c5..a88d25a06c29b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
> @@ -30,6 +30,17 @@
> #define regHDP_CLK_CNTL_V6_1 0xd5
> #define regHDP_CLK_CNTL_V6_1_BASE_IDX 0
>
> +static void hdp_v6_0_flush_hdp(struct amdgpu_device *adev,
> + struct amdgpu_ring *ring)
> +{
> + if (!ring || !ring->funcs->emit_wreg) {
> + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> + } else {
> + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> + }
> +}
> +
> static void hdp_v6_0_update_clock_gating(struct amdgpu_device *adev,
> bool enable)
> {
> @@ -138,7 +149,7 @@ static void hdp_v6_0_get_clockgating_state(struct amdgpu_device *adev,
> }
>
> const struct amdgpu_hdp_funcs hdp_v6_0_funcs = {
> - .flush_hdp = amdgpu_hdp_generic_flush,
> + .flush_hdp = hdp_v6_0_flush_hdp,
> .update_clock_gating = hdp_v6_0_update_clock_gating,
> .get_clock_gating_state = hdp_v6_0_get_clockgating_state,
> };
> diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
> index ed1debc035073..49f7eb4fbd117 100644
> --- a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
> @@ -27,6 +27,17 @@
> #include "hdp/hdp_7_0_0_sh_mask.h"
> #include <uapi/linux/kfd_ioctl.h>
>
> +static void hdp_v7_0_flush_hdp(struct amdgpu_device *adev,
> + struct amdgpu_ring *ring)
> +{
> + if (!ring || !ring->funcs->emit_wreg) {
> + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> + } else {
> + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> + }
> +}
> +
> static void hdp_v7_0_update_clock_gating(struct amdgpu_device *adev,
> bool enable)
> {
> @@ -126,7 +137,7 @@ static void hdp_v7_0_get_clockgating_state(struct amdgpu_device *adev,
> }
>
> const struct amdgpu_hdp_funcs hdp_v7_0_funcs = {
> - .flush_hdp = amdgpu_hdp_generic_flush,
> + .flush_hdp = hdp_v7_0_flush_hdp,
> .update_clock_gating = hdp_v7_0_update_clock_gating,
> .get_clock_gating_state = hdp_v7_0_get_clockgating_state,
> };
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function"
2025-05-06 15:17 ` [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
@ 2025-05-07 15:13 ` Alex Deucher
2025-05-07 16:43 ` Lazar, Lijo
2025-05-07 15:27 ` Felix Kuehling
1 sibling, 1 reply; 13+ messages in thread
From: Alex Deucher @ 2025-05-07 15:13 UTC (permalink / raw)
To: Alex Deucher, Lazar, Lijo, Kuehling, Felix, Christian Koenig; +Cc: amd-gfx
Ping again on this series?
Alex
On Tue, May 6, 2025 at 11:17 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> Ping on this series? This fixes a regression.
>
> Alex
>
> On Mon, May 5, 2025 at 9:05 AM Alex Deucher <alexander.deucher@amd.com> wrote:
> >
> > This reverts commit 18a878fd8aef0ec21648a3782f55a79790cd4073.
> >
> > Revert this temporarily to make it easier to fix a regression
> > in the HDP handling.
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c | 21 ---------------------
> > drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h | 2 --
> > drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 13 ++++++++++++-
> > drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c | 13 ++++++++++++-
> > drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c | 13 ++++++++++++-
> > drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c | 13 ++++++++++++-
> > 6 files changed, 48 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
> > index 7fd8f09c28e66..b6cf801939aa5 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
> > @@ -22,7 +22,6 @@
> > */
> > #include "amdgpu.h"
> > #include "amdgpu_ras.h"
> > -#include <uapi/linux/kfd_ioctl.h>
> >
> > int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
> > {
> > @@ -47,23 +46,3 @@ int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
> > /* hdp ras follows amdgpu_ras_block_late_init_default for late init */
> > return 0;
> > }
> > -
> > -void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
> > - struct amdgpu_ring *ring)
> > -{
> > - if (!ring || !ring->funcs->emit_wreg) {
> > - WREG32((adev->rmmio_remap.reg_offset +
> > - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
> > - 2,
> > - 0);
> > - RREG32((adev->rmmio_remap.reg_offset +
> > - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
> > - 2);
> > - } else {
> > - amdgpu_ring_emit_wreg(ring,
> > - (adev->rmmio_remap.reg_offset +
> > - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
> > - 2,
> > - 0);
> > - }
> > -}
> > \ No newline at end of file
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> > index 4cfd932b7e91e..7b8a6152dc8d9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> > @@ -44,6 +44,4 @@ struct amdgpu_hdp {
> > };
> >
> > int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev);
> > -void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
> > - struct amdgpu_ring *ring);
> > #endif /* __AMDGPU_HDP_H__ */
> > diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> > index e6c0d86d34865..f1dc13b3ab38e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> > @@ -36,6 +36,17 @@
> > #define HDP_MEM_POWER_CTRL__RC_MEM_POWER_LS_EN_MASK 0x00020000L
> > #define mmHDP_MEM_POWER_CTRL_BASE_IDX 0
> >
> > +static void hdp_v4_0_flush_hdp(struct amdgpu_device *adev,
> > + struct amdgpu_ring *ring)
> > +{
> > + if (!ring || !ring->funcs->emit_wreg) {
> > + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> > + } else {
> > + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + }
> > +}
> > +
> > static void hdp_v4_0_invalidate_hdp(struct amdgpu_device *adev,
> > struct amdgpu_ring *ring)
> > {
> > @@ -169,7 +180,7 @@ struct amdgpu_hdp_ras hdp_v4_0_ras = {
> > };
> >
> > const struct amdgpu_hdp_funcs hdp_v4_0_funcs = {
> > - .flush_hdp = amdgpu_hdp_generic_flush,
> > + .flush_hdp = hdp_v4_0_flush_hdp,
> > .invalidate_hdp = hdp_v4_0_invalidate_hdp,
> > .update_clock_gating = hdp_v4_0_update_clock_gating,
> > .get_clock_gating_state = hdp_v4_0_get_clockgating_state,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
> > index 8bc001dc9f631..43195c0797480 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
> > @@ -27,6 +27,17 @@
> > #include "hdp/hdp_5_0_0_sh_mask.h"
> > #include <uapi/linux/kfd_ioctl.h>
> >
> > +static void hdp_v5_0_flush_hdp(struct amdgpu_device *adev,
> > + struct amdgpu_ring *ring)
> > +{
> > + if (!ring || !ring->funcs->emit_wreg) {
> > + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> > + } else {
> > + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + }
> > +}
> > +
> > static void hdp_v5_0_invalidate_hdp(struct amdgpu_device *adev,
> > struct amdgpu_ring *ring)
> > {
> > @@ -206,7 +217,7 @@ static void hdp_v5_0_init_registers(struct amdgpu_device *adev)
> > }
> >
> > const struct amdgpu_hdp_funcs hdp_v5_0_funcs = {
> > - .flush_hdp = amdgpu_hdp_generic_flush,
> > + .flush_hdp = hdp_v5_0_flush_hdp,
> > .invalidate_hdp = hdp_v5_0_invalidate_hdp,
> > .update_clock_gating = hdp_v5_0_update_clock_gating,
> > .get_clock_gating_state = hdp_v5_0_get_clockgating_state,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
> > index ec20daf4272c5..a88d25a06c29b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
> > @@ -30,6 +30,17 @@
> > #define regHDP_CLK_CNTL_V6_1 0xd5
> > #define regHDP_CLK_CNTL_V6_1_BASE_IDX 0
> >
> > +static void hdp_v6_0_flush_hdp(struct amdgpu_device *adev,
> > + struct amdgpu_ring *ring)
> > +{
> > + if (!ring || !ring->funcs->emit_wreg) {
> > + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> > + } else {
> > + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + }
> > +}
> > +
> > static void hdp_v6_0_update_clock_gating(struct amdgpu_device *adev,
> > bool enable)
> > {
> > @@ -138,7 +149,7 @@ static void hdp_v6_0_get_clockgating_state(struct amdgpu_device *adev,
> > }
> >
> > const struct amdgpu_hdp_funcs hdp_v6_0_funcs = {
> > - .flush_hdp = amdgpu_hdp_generic_flush,
> > + .flush_hdp = hdp_v6_0_flush_hdp,
> > .update_clock_gating = hdp_v6_0_update_clock_gating,
> > .get_clock_gating_state = hdp_v6_0_get_clockgating_state,
> > };
> > diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
> > index ed1debc035073..49f7eb4fbd117 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
> > @@ -27,6 +27,17 @@
> > #include "hdp/hdp_7_0_0_sh_mask.h"
> > #include <uapi/linux/kfd_ioctl.h>
> >
> > +static void hdp_v7_0_flush_hdp(struct amdgpu_device *adev,
> > + struct amdgpu_ring *ring)
> > +{
> > + if (!ring || !ring->funcs->emit_wreg) {
> > + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> > + } else {
> > + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + }
> > +}
> > +
> > static void hdp_v7_0_update_clock_gating(struct amdgpu_device *adev,
> > bool enable)
> > {
> > @@ -126,7 +137,7 @@ static void hdp_v7_0_get_clockgating_state(struct amdgpu_device *adev,
> > }
> >
> > const struct amdgpu_hdp_funcs hdp_v7_0_funcs = {
> > - .flush_hdp = amdgpu_hdp_generic_flush,
> > + .flush_hdp = hdp_v7_0_flush_hdp,
> > .update_clock_gating = hdp_v7_0_update_clock_gating,
> > .get_clock_gating_state = hdp_v7_0_get_clockgating_state,
> > };
> > --
> > 2.49.0
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function"
@ 2025-05-07 15:27 Alex Deucher
0 siblings, 0 replies; 13+ messages in thread
From: Alex Deucher @ 2025-05-07 15:27 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher
This reverts commit 18a878fd8aef0ec21648a3782f55a79790cd4073.
Revert this temporarily to make it easier to fix a regression
in the HDP handling.
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c | 21 ---------------------
drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h | 2 --
drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 13 ++++++++++++-
drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c | 13 ++++++++++++-
drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c | 13 ++++++++++++-
drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c | 13 ++++++++++++-
6 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
index 7fd8f09c28e66..b6cf801939aa5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
@@ -22,7 +22,6 @@
*/
#include "amdgpu.h"
#include "amdgpu_ras.h"
-#include <uapi/linux/kfd_ioctl.h>
int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
{
@@ -47,23 +46,3 @@ int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
/* hdp ras follows amdgpu_ras_block_late_init_default for late init */
return 0;
}
-
-void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
- struct amdgpu_ring *ring)
-{
- if (!ring || !ring->funcs->emit_wreg) {
- WREG32((adev->rmmio_remap.reg_offset +
- KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
- 2,
- 0);
- RREG32((adev->rmmio_remap.reg_offset +
- KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
- 2);
- } else {
- amdgpu_ring_emit_wreg(ring,
- (adev->rmmio_remap.reg_offset +
- KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
- 2,
- 0);
- }
-}
\ No newline at end of file
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
index 4cfd932b7e91e..7b8a6152dc8d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
@@ -44,6 +44,4 @@ struct amdgpu_hdp {
};
int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev);
-void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
- struct amdgpu_ring *ring);
#endif /* __AMDGPU_HDP_H__ */
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
index e6c0d86d34865..f1dc13b3ab38e 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
@@ -36,6 +36,17 @@
#define HDP_MEM_POWER_CTRL__RC_MEM_POWER_LS_EN_MASK 0x00020000L
#define mmHDP_MEM_POWER_CTRL_BASE_IDX 0
+static void hdp_v4_0_flush_hdp(struct amdgpu_device *adev,
+ struct amdgpu_ring *ring)
+{
+ if (!ring || !ring->funcs->emit_wreg) {
+ WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ } else {
+ amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ }
+}
+
static void hdp_v4_0_invalidate_hdp(struct amdgpu_device *adev,
struct amdgpu_ring *ring)
{
@@ -169,7 +180,7 @@ struct amdgpu_hdp_ras hdp_v4_0_ras = {
};
const struct amdgpu_hdp_funcs hdp_v4_0_funcs = {
- .flush_hdp = amdgpu_hdp_generic_flush,
+ .flush_hdp = hdp_v4_0_flush_hdp,
.invalidate_hdp = hdp_v4_0_invalidate_hdp,
.update_clock_gating = hdp_v4_0_update_clock_gating,
.get_clock_gating_state = hdp_v4_0_get_clockgating_state,
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
index 8bc001dc9f631..43195c0797480 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
@@ -27,6 +27,17 @@
#include "hdp/hdp_5_0_0_sh_mask.h"
#include <uapi/linux/kfd_ioctl.h>
+static void hdp_v5_0_flush_hdp(struct amdgpu_device *adev,
+ struct amdgpu_ring *ring)
+{
+ if (!ring || !ring->funcs->emit_wreg) {
+ WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ } else {
+ amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ }
+}
+
static void hdp_v5_0_invalidate_hdp(struct amdgpu_device *adev,
struct amdgpu_ring *ring)
{
@@ -206,7 +217,7 @@ static void hdp_v5_0_init_registers(struct amdgpu_device *adev)
}
const struct amdgpu_hdp_funcs hdp_v5_0_funcs = {
- .flush_hdp = amdgpu_hdp_generic_flush,
+ .flush_hdp = hdp_v5_0_flush_hdp,
.invalidate_hdp = hdp_v5_0_invalidate_hdp,
.update_clock_gating = hdp_v5_0_update_clock_gating,
.get_clock_gating_state = hdp_v5_0_get_clockgating_state,
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
index ec20daf4272c5..a88d25a06c29b 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
@@ -30,6 +30,17 @@
#define regHDP_CLK_CNTL_V6_1 0xd5
#define regHDP_CLK_CNTL_V6_1_BASE_IDX 0
+static void hdp_v6_0_flush_hdp(struct amdgpu_device *adev,
+ struct amdgpu_ring *ring)
+{
+ if (!ring || !ring->funcs->emit_wreg) {
+ WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ } else {
+ amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ }
+}
+
static void hdp_v6_0_update_clock_gating(struct amdgpu_device *adev,
bool enable)
{
@@ -138,7 +149,7 @@ static void hdp_v6_0_get_clockgating_state(struct amdgpu_device *adev,
}
const struct amdgpu_hdp_funcs hdp_v6_0_funcs = {
- .flush_hdp = amdgpu_hdp_generic_flush,
+ .flush_hdp = hdp_v6_0_flush_hdp,
.update_clock_gating = hdp_v6_0_update_clock_gating,
.get_clock_gating_state = hdp_v6_0_get_clockgating_state,
};
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
index ed1debc035073..49f7eb4fbd117 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
@@ -27,6 +27,17 @@
#include "hdp/hdp_7_0_0_sh_mask.h"
#include <uapi/linux/kfd_ioctl.h>
+static void hdp_v7_0_flush_hdp(struct amdgpu_device *adev,
+ struct amdgpu_ring *ring)
+{
+ if (!ring || !ring->funcs->emit_wreg) {
+ WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
+ } else {
+ amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
+ }
+}
+
static void hdp_v7_0_update_clock_gating(struct amdgpu_device *adev,
bool enable)
{
@@ -126,7 +137,7 @@ static void hdp_v7_0_get_clockgating_state(struct amdgpu_device *adev,
}
const struct amdgpu_hdp_funcs hdp_v7_0_funcs = {
- .flush_hdp = amdgpu_hdp_generic_flush,
+ .flush_hdp = hdp_v7_0_flush_hdp,
.update_clock_gating = hdp_v7_0_update_clock_gating,
.get_clock_gating_state = hdp_v7_0_get_clockgating_state,
};
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function"
2025-05-06 15:17 ` [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
2025-05-07 15:13 ` Alex Deucher
@ 2025-05-07 15:27 ` Felix Kuehling
1 sibling, 0 replies; 13+ messages in thread
From: Felix Kuehling @ 2025-05-07 15:27 UTC (permalink / raw)
To: Alex Deucher, Alex Deucher; +Cc: amd-gfx
On 2025-05-06 11:17, Alex Deucher wrote:
> Ping on this series? This fixes a regression.
>
> Alex
>
> On Mon, May 5, 2025 at 9:05 AM Alex Deucher <alexander.deucher@amd.com> wrote:
>> This reverts commit 18a878fd8aef0ec21648a3782f55a79790cd4073.
>>
>> Revert this temporarily to make it easier to fix a regression
>> in the HDP handling.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
The series is
Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c | 21 ---------------------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h | 2 --
>> drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 13 ++++++++++++-
>> drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c | 13 ++++++++++++-
>> drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c | 13 ++++++++++++-
>> drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c | 13 ++++++++++++-
>> 6 files changed, 48 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
>> index 7fd8f09c28e66..b6cf801939aa5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
>> @@ -22,7 +22,6 @@
>> */
>> #include "amdgpu.h"
>> #include "amdgpu_ras.h"
>> -#include <uapi/linux/kfd_ioctl.h>
>>
>> int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
>> {
>> @@ -47,23 +46,3 @@ int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
>> /* hdp ras follows amdgpu_ras_block_late_init_default for late init */
>> return 0;
>> }
>> -
>> -void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
>> - struct amdgpu_ring *ring)
>> -{
>> - if (!ring || !ring->funcs->emit_wreg) {
>> - WREG32((adev->rmmio_remap.reg_offset +
>> - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
>> - 2,
>> - 0);
>> - RREG32((adev->rmmio_remap.reg_offset +
>> - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
>> - 2);
>> - } else {
>> - amdgpu_ring_emit_wreg(ring,
>> - (adev->rmmio_remap.reg_offset +
>> - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
>> - 2,
>> - 0);
>> - }
>> -}
>> \ No newline at end of file
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
>> index 4cfd932b7e91e..7b8a6152dc8d9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
>> @@ -44,6 +44,4 @@ struct amdgpu_hdp {
>> };
>>
>> int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev);
>> -void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
>> - struct amdgpu_ring *ring);
>> #endif /* __AMDGPU_HDP_H__ */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
>> index e6c0d86d34865..f1dc13b3ab38e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
>> @@ -36,6 +36,17 @@
>> #define HDP_MEM_POWER_CTRL__RC_MEM_POWER_LS_EN_MASK 0x00020000L
>> #define mmHDP_MEM_POWER_CTRL_BASE_IDX 0
>>
>> +static void hdp_v4_0_flush_hdp(struct amdgpu_device *adev,
>> + struct amdgpu_ring *ring)
>> +{
>> + if (!ring || !ring->funcs->emit_wreg) {
>> + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
>> + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
>> + } else {
>> + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
>> + }
>> +}
>> +
>> static void hdp_v4_0_invalidate_hdp(struct amdgpu_device *adev,
>> struct amdgpu_ring *ring)
>> {
>> @@ -169,7 +180,7 @@ struct amdgpu_hdp_ras hdp_v4_0_ras = {
>> };
>>
>> const struct amdgpu_hdp_funcs hdp_v4_0_funcs = {
>> - .flush_hdp = amdgpu_hdp_generic_flush,
>> + .flush_hdp = hdp_v4_0_flush_hdp,
>> .invalidate_hdp = hdp_v4_0_invalidate_hdp,
>> .update_clock_gating = hdp_v4_0_update_clock_gating,
>> .get_clock_gating_state = hdp_v4_0_get_clockgating_state,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
>> index 8bc001dc9f631..43195c0797480 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
>> @@ -27,6 +27,17 @@
>> #include "hdp/hdp_5_0_0_sh_mask.h"
>> #include <uapi/linux/kfd_ioctl.h>
>>
>> +static void hdp_v5_0_flush_hdp(struct amdgpu_device *adev,
>> + struct amdgpu_ring *ring)
>> +{
>> + if (!ring || !ring->funcs->emit_wreg) {
>> + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
>> + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
>> + } else {
>> + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
>> + }
>> +}
>> +
>> static void hdp_v5_0_invalidate_hdp(struct amdgpu_device *adev,
>> struct amdgpu_ring *ring)
>> {
>> @@ -206,7 +217,7 @@ static void hdp_v5_0_init_registers(struct amdgpu_device *adev)
>> }
>>
>> const struct amdgpu_hdp_funcs hdp_v5_0_funcs = {
>> - .flush_hdp = amdgpu_hdp_generic_flush,
>> + .flush_hdp = hdp_v5_0_flush_hdp,
>> .invalidate_hdp = hdp_v5_0_invalidate_hdp,
>> .update_clock_gating = hdp_v5_0_update_clock_gating,
>> .get_clock_gating_state = hdp_v5_0_get_clockgating_state,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
>> index ec20daf4272c5..a88d25a06c29b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
>> @@ -30,6 +30,17 @@
>> #define regHDP_CLK_CNTL_V6_1 0xd5
>> #define regHDP_CLK_CNTL_V6_1_BASE_IDX 0
>>
>> +static void hdp_v6_0_flush_hdp(struct amdgpu_device *adev,
>> + struct amdgpu_ring *ring)
>> +{
>> + if (!ring || !ring->funcs->emit_wreg) {
>> + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
>> + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
>> + } else {
>> + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
>> + }
>> +}
>> +
>> static void hdp_v6_0_update_clock_gating(struct amdgpu_device *adev,
>> bool enable)
>> {
>> @@ -138,7 +149,7 @@ static void hdp_v6_0_get_clockgating_state(struct amdgpu_device *adev,
>> }
>>
>> const struct amdgpu_hdp_funcs hdp_v6_0_funcs = {
>> - .flush_hdp = amdgpu_hdp_generic_flush,
>> + .flush_hdp = hdp_v6_0_flush_hdp,
>> .update_clock_gating = hdp_v6_0_update_clock_gating,
>> .get_clock_gating_state = hdp_v6_0_get_clockgating_state,
>> };
>> diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
>> index ed1debc035073..49f7eb4fbd117 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
>> @@ -27,6 +27,17 @@
>> #include "hdp/hdp_7_0_0_sh_mask.h"
>> #include <uapi/linux/kfd_ioctl.h>
>>
>> +static void hdp_v7_0_flush_hdp(struct amdgpu_device *adev,
>> + struct amdgpu_ring *ring)
>> +{
>> + if (!ring || !ring->funcs->emit_wreg) {
>> + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
>> + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
>> + } else {
>> + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
>> + }
>> +}
>> +
>> static void hdp_v7_0_update_clock_gating(struct amdgpu_device *adev,
>> bool enable)
>> {
>> @@ -126,7 +137,7 @@ static void hdp_v7_0_get_clockgating_state(struct amdgpu_device *adev,
>> }
>>
>> const struct amdgpu_hdp_funcs hdp_v7_0_funcs = {
>> - .flush_hdp = amdgpu_hdp_generic_flush,
>> + .flush_hdp = hdp_v7_0_flush_hdp,
>> .update_clock_gating = hdp_v7_0_update_clock_gating,
>> .get_clock_gating_state = hdp_v7_0_get_clockgating_state,
>> };
>> --
>> 2.49.0
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function"
2025-05-07 15:13 ` Alex Deucher
@ 2025-05-07 16:43 ` Lazar, Lijo
2025-05-07 16:49 ` Alex Deucher
0 siblings, 1 reply; 13+ messages in thread
From: Lazar, Lijo @ 2025-05-07 16:43 UTC (permalink / raw)
To: Alex Deucher, Deucher, Alexander, Kuehling, Felix,
Koenig, Christian
Cc: amd-gfx@lists.freedesktop.org
[-- Attachment #1: Type: text/plain, Size: 9748 bytes --]
[AMD Official Use Only - AMD Internal Distribution Only]
The flush reg is read/write. If the reads are not working on those platforms, I think the writes may be dropped silently.
Thanks,
Lijo
________________________________
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Wednesday, May 7, 2025 8:43:51 PM
To: Deucher, Alexander <Alexander.Deucher@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>
Cc: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function"
Ping again on this series?
Alex
On Tue, May 6, 2025 at 11:17 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> Ping on this series? This fixes a regression.
>
> Alex
>
> On Mon, May 5, 2025 at 9:05 AM Alex Deucher <alexander.deucher@amd.com> wrote:
> >
> > This reverts commit 18a878fd8aef0ec21648a3782f55a79790cd4073.
> >
> > Revert this temporarily to make it easier to fix a regression
> > in the HDP handling.
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c | 21 ---------------------
> > drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h | 2 --
> > drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 13 ++++++++++++-
> > drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c | 13 ++++++++++++-
> > drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c | 13 ++++++++++++-
> > drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c | 13 ++++++++++++-
> > 6 files changed, 48 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
> > index 7fd8f09c28e66..b6cf801939aa5 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
> > @@ -22,7 +22,6 @@
> > */
> > #include "amdgpu.h"
> > #include "amdgpu_ras.h"
> > -#include <uapi/linux/kfd_ioctl.h>
> >
> > int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
> > {
> > @@ -47,23 +46,3 @@ int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
> > /* hdp ras follows amdgpu_ras_block_late_init_default for late init */
> > return 0;
> > }
> > -
> > -void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
> > - struct amdgpu_ring *ring)
> > -{
> > - if (!ring || !ring->funcs->emit_wreg) {
> > - WREG32((adev->rmmio_remap.reg_offset +
> > - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
> > - 2,
> > - 0);
> > - RREG32((adev->rmmio_remap.reg_offset +
> > - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
> > - 2);
> > - } else {
> > - amdgpu_ring_emit_wreg(ring,
> > - (adev->rmmio_remap.reg_offset +
> > - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
> > - 2,
> > - 0);
> > - }
> > -}
> > \ No newline at end of file
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> > index 4cfd932b7e91e..7b8a6152dc8d9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> > @@ -44,6 +44,4 @@ struct amdgpu_hdp {
> > };
> >
> > int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev);
> > -void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
> > - struct amdgpu_ring *ring);
> > #endif /* __AMDGPU_HDP_H__ */
> > diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> > index e6c0d86d34865..f1dc13b3ab38e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> > @@ -36,6 +36,17 @@
> > #define HDP_MEM_POWER_CTRL__RC_MEM_POWER_LS_EN_MASK 0x00020000L
> > #define mmHDP_MEM_POWER_CTRL_BASE_IDX 0
> >
> > +static void hdp_v4_0_flush_hdp(struct amdgpu_device *adev,
> > + struct amdgpu_ring *ring)
> > +{
> > + if (!ring || !ring->funcs->emit_wreg) {
> > + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> > + } else {
> > + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + }
> > +}
> > +
> > static void hdp_v4_0_invalidate_hdp(struct amdgpu_device *adev,
> > struct amdgpu_ring *ring)
> > {
> > @@ -169,7 +180,7 @@ struct amdgpu_hdp_ras hdp_v4_0_ras = {
> > };
> >
> > const struct amdgpu_hdp_funcs hdp_v4_0_funcs = {
> > - .flush_hdp = amdgpu_hdp_generic_flush,
> > + .flush_hdp = hdp_v4_0_flush_hdp,
> > .invalidate_hdp = hdp_v4_0_invalidate_hdp,
> > .update_clock_gating = hdp_v4_0_update_clock_gating,
> > .get_clock_gating_state = hdp_v4_0_get_clockgating_state,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
> > index 8bc001dc9f631..43195c0797480 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
> > @@ -27,6 +27,17 @@
> > #include "hdp/hdp_5_0_0_sh_mask.h"
> > #include <uapi/linux/kfd_ioctl.h>
> >
> > +static void hdp_v5_0_flush_hdp(struct amdgpu_device *adev,
> > + struct amdgpu_ring *ring)
> > +{
> > + if (!ring || !ring->funcs->emit_wreg) {
> > + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> > + } else {
> > + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + }
> > +}
> > +
> > static void hdp_v5_0_invalidate_hdp(struct amdgpu_device *adev,
> > struct amdgpu_ring *ring)
> > {
> > @@ -206,7 +217,7 @@ static void hdp_v5_0_init_registers(struct amdgpu_device *adev)
> > }
> >
> > const struct amdgpu_hdp_funcs hdp_v5_0_funcs = {
> > - .flush_hdp = amdgpu_hdp_generic_flush,
> > + .flush_hdp = hdp_v5_0_flush_hdp,
> > .invalidate_hdp = hdp_v5_0_invalidate_hdp,
> > .update_clock_gating = hdp_v5_0_update_clock_gating,
> > .get_clock_gating_state = hdp_v5_0_get_clockgating_state,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
> > index ec20daf4272c5..a88d25a06c29b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
> > @@ -30,6 +30,17 @@
> > #define regHDP_CLK_CNTL_V6_1 0xd5
> > #define regHDP_CLK_CNTL_V6_1_BASE_IDX 0
> >
> > +static void hdp_v6_0_flush_hdp(struct amdgpu_device *adev,
> > + struct amdgpu_ring *ring)
> > +{
> > + if (!ring || !ring->funcs->emit_wreg) {
> > + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> > + } else {
> > + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + }
> > +}
> > +
> > static void hdp_v6_0_update_clock_gating(struct amdgpu_device *adev,
> > bool enable)
> > {
> > @@ -138,7 +149,7 @@ static void hdp_v6_0_get_clockgating_state(struct amdgpu_device *adev,
> > }
> >
> > const struct amdgpu_hdp_funcs hdp_v6_0_funcs = {
> > - .flush_hdp = amdgpu_hdp_generic_flush,
> > + .flush_hdp = hdp_v6_0_flush_hdp,
> > .update_clock_gating = hdp_v6_0_update_clock_gating,
> > .get_clock_gating_state = hdp_v6_0_get_clockgating_state,
> > };
> > diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
> > index ed1debc035073..49f7eb4fbd117 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
> > @@ -27,6 +27,17 @@
> > #include "hdp/hdp_7_0_0_sh_mask.h"
> > #include <uapi/linux/kfd_ioctl.h>
> >
> > +static void hdp_v7_0_flush_hdp(struct amdgpu_device *adev,
> > + struct amdgpu_ring *ring)
> > +{
> > + if (!ring || !ring->funcs->emit_wreg) {
> > + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> > + } else {
> > + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > + }
> > +}
> > +
> > static void hdp_v7_0_update_clock_gating(struct amdgpu_device *adev,
> > bool enable)
> > {
> > @@ -126,7 +137,7 @@ static void hdp_v7_0_get_clockgating_state(struct amdgpu_device *adev,
> > }
> >
> > const struct amdgpu_hdp_funcs hdp_v7_0_funcs = {
> > - .flush_hdp = amdgpu_hdp_generic_flush,
> > + .flush_hdp = hdp_v7_0_flush_hdp,
> > .update_clock_gating = hdp_v7_0_update_clock_gating,
> > .get_clock_gating_state = hdp_v7_0_get_clockgating_state,
> > };
> > --
> > 2.49.0
> >
[-- Attachment #2: Type: text/html, Size: 18598 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function"
2025-05-07 16:43 ` Lazar, Lijo
@ 2025-05-07 16:49 ` Alex Deucher
0 siblings, 0 replies; 13+ messages in thread
From: Alex Deucher @ 2025-05-07 16:49 UTC (permalink / raw)
To: Lazar, Lijo
Cc: Deucher, Alexander, Kuehling, Felix, Koenig, Christian,
amd-gfx@lists.freedesktop.org
On Wed, May 7, 2025 at 12:43 PM Lazar, Lijo <Lijo.Lazar@amd.com> wrote:
>
> [AMD Official Use Only - AMD Internal Distribution Only]
>
>
> The flush reg is read/write. If the reads are not working on those platforms, I think the writes may be dropped silently.
The problem only happens when we remap the register to the unused MMIO
space. In that case, we end up with a PCI AER when reading the
remapped register. Seems to happen on x86 platforms as well. Writes
don't seem to be a problem.
Alex
>
> Thanks,
> Lijo
> ________________________________
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Wednesday, May 7, 2025 8:43:51 PM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>
> Cc: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function"
>
> Ping again on this series?
>
> Alex
>
> On Tue, May 6, 2025 at 11:17 AM Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > Ping on this series? This fixes a regression.
> >
> > Alex
> >
> > On Mon, May 5, 2025 at 9:05 AM Alex Deucher <alexander.deucher@amd.com> wrote:
> > >
> > > This reverts commit 18a878fd8aef0ec21648a3782f55a79790cd4073.
> > >
> > > Revert this temporarily to make it easier to fix a regression
> > > in the HDP handling.
> > >
> > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c | 21 ---------------------
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h | 2 --
> > > drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 13 ++++++++++++-
> > > drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c | 13 ++++++++++++-
> > > drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c | 13 ++++++++++++-
> > > drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c | 13 ++++++++++++-
> > > 6 files changed, 48 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
> > > index 7fd8f09c28e66..b6cf801939aa5 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
> > > @@ -22,7 +22,6 @@
> > > */
> > > #include "amdgpu.h"
> > > #include "amdgpu_ras.h"
> > > -#include <uapi/linux/kfd_ioctl.h>
> > >
> > > int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
> > > {
> > > @@ -47,23 +46,3 @@ int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev)
> > > /* hdp ras follows amdgpu_ras_block_late_init_default for late init */
> > > return 0;
> > > }
> > > -
> > > -void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
> > > - struct amdgpu_ring *ring)
> > > -{
> > > - if (!ring || !ring->funcs->emit_wreg) {
> > > - WREG32((adev->rmmio_remap.reg_offset +
> > > - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
> > > - 2,
> > > - 0);
> > > - RREG32((adev->rmmio_remap.reg_offset +
> > > - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
> > > - 2);
> > > - } else {
> > > - amdgpu_ring_emit_wreg(ring,
> > > - (adev->rmmio_remap.reg_offset +
> > > - KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >>
> > > - 2,
> > > - 0);
> > > - }
> > > -}
> > > \ No newline at end of file
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> > > index 4cfd932b7e91e..7b8a6152dc8d9 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> > > @@ -44,6 +44,4 @@ struct amdgpu_hdp {
> > > };
> > >
> > > int amdgpu_hdp_ras_sw_init(struct amdgpu_device *adev);
> > > -void amdgpu_hdp_generic_flush(struct amdgpu_device *adev,
> > > - struct amdgpu_ring *ring);
> > > #endif /* __AMDGPU_HDP_H__ */
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> > > index e6c0d86d34865..f1dc13b3ab38e 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> > > @@ -36,6 +36,17 @@
> > > #define HDP_MEM_POWER_CTRL__RC_MEM_POWER_LS_EN_MASK 0x00020000L
> > > #define mmHDP_MEM_POWER_CTRL_BASE_IDX 0
> > >
> > > +static void hdp_v4_0_flush_hdp(struct amdgpu_device *adev,
> > > + struct amdgpu_ring *ring)
> > > +{
> > > + if (!ring || !ring->funcs->emit_wreg) {
> > > + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > > + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> > > + } else {
> > > + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > > + }
> > > +}
> > > +
> > > static void hdp_v4_0_invalidate_hdp(struct amdgpu_device *adev,
> > > struct amdgpu_ring *ring)
> > > {
> > > @@ -169,7 +180,7 @@ struct amdgpu_hdp_ras hdp_v4_0_ras = {
> > > };
> > >
> > > const struct amdgpu_hdp_funcs hdp_v4_0_funcs = {
> > > - .flush_hdp = amdgpu_hdp_generic_flush,
> > > + .flush_hdp = hdp_v4_0_flush_hdp,
> > > .invalidate_hdp = hdp_v4_0_invalidate_hdp,
> > > .update_clock_gating = hdp_v4_0_update_clock_gating,
> > > .get_clock_gating_state = hdp_v4_0_get_clockgating_state,
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
> > > index 8bc001dc9f631..43195c0797480 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c
> > > @@ -27,6 +27,17 @@
> > > #include "hdp/hdp_5_0_0_sh_mask.h"
> > > #include <uapi/linux/kfd_ioctl.h>
> > >
> > > +static void hdp_v5_0_flush_hdp(struct amdgpu_device *adev,
> > > + struct amdgpu_ring *ring)
> > > +{
> > > + if (!ring || !ring->funcs->emit_wreg) {
> > > + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > > + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> > > + } else {
> > > + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > > + }
> > > +}
> > > +
> > > static void hdp_v5_0_invalidate_hdp(struct amdgpu_device *adev,
> > > struct amdgpu_ring *ring)
> > > {
> > > @@ -206,7 +217,7 @@ static void hdp_v5_0_init_registers(struct amdgpu_device *adev)
> > > }
> > >
> > > const struct amdgpu_hdp_funcs hdp_v5_0_funcs = {
> > > - .flush_hdp = amdgpu_hdp_generic_flush,
> > > + .flush_hdp = hdp_v5_0_flush_hdp,
> > > .invalidate_hdp = hdp_v5_0_invalidate_hdp,
> > > .update_clock_gating = hdp_v5_0_update_clock_gating,
> > > .get_clock_gating_state = hdp_v5_0_get_clockgating_state,
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
> > > index ec20daf4272c5..a88d25a06c29b 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v6_0.c
> > > @@ -30,6 +30,17 @@
> > > #define regHDP_CLK_CNTL_V6_1 0xd5
> > > #define regHDP_CLK_CNTL_V6_1_BASE_IDX 0
> > >
> > > +static void hdp_v6_0_flush_hdp(struct amdgpu_device *adev,
> > > + struct amdgpu_ring *ring)
> > > +{
> > > + if (!ring || !ring->funcs->emit_wreg) {
> > > + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > > + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> > > + } else {
> > > + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > > + }
> > > +}
> > > +
> > > static void hdp_v6_0_update_clock_gating(struct amdgpu_device *adev,
> > > bool enable)
> > > {
> > > @@ -138,7 +149,7 @@ static void hdp_v6_0_get_clockgating_state(struct amdgpu_device *adev,
> > > }
> > >
> > > const struct amdgpu_hdp_funcs hdp_v6_0_funcs = {
> > > - .flush_hdp = amdgpu_hdp_generic_flush,
> > > + .flush_hdp = hdp_v6_0_flush_hdp,
> > > .update_clock_gating = hdp_v6_0_update_clock_gating,
> > > .get_clock_gating_state = hdp_v6_0_get_clockgating_state,
> > > };
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
> > > index ed1debc035073..49f7eb4fbd117 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v7_0.c
> > > @@ -27,6 +27,17 @@
> > > #include "hdp/hdp_7_0_0_sh_mask.h"
> > > #include <uapi/linux/kfd_ioctl.h>
> > >
> > > +static void hdp_v7_0_flush_hdp(struct amdgpu_device *adev,
> > > + struct amdgpu_ring *ring)
> > > +{
> > > + if (!ring || !ring->funcs->emit_wreg) {
> > > + WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > > + RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
> > > + } else {
> > > + amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
> > > + }
> > > +}
> > > +
> > > static void hdp_v7_0_update_clock_gating(struct amdgpu_device *adev,
> > > bool enable)
> > > {
> > > @@ -126,7 +137,7 @@ static void hdp_v7_0_get_clockgating_state(struct amdgpu_device *adev,
> > > }
> > >
> > > const struct amdgpu_hdp_funcs hdp_v7_0_funcs = {
> > > - .flush_hdp = amdgpu_hdp_generic_flush,
> > > + .flush_hdp = hdp_v7_0_flush_hdp,
> > > .update_clock_gating = hdp_v7_0_update_clock_gating,
> > > .get_clock_gating_state = hdp_v7_0_get_clockgating_state,
> > > };
> > > --
> > > 2.49.0
> > >
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-05-07 16:49 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-05 13:04 [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
2025-05-05 13:04 ` [PATCH 2/7] drm/amdgpu/hdp4: use memcfg register to post the write for HDP flush Alex Deucher
2025-05-05 13:04 ` [PATCH 3/7] drm/amdgpu/hdp5: " Alex Deucher
2025-05-05 13:04 ` [PATCH 4/7] drm/amdgpu/hdp5.2: " Alex Deucher
2025-05-05 13:04 ` [PATCH 5/7] drm/amdgpu/hdp6: " Alex Deucher
2025-05-05 13:04 ` [PATCH 6/7] drm/amdgpu/hdp7: " Alex Deucher
2025-05-05 13:04 ` [PATCH 7/7] Reapply: drm/amdgpu: Use generic hdp flush function Alex Deucher
2025-05-06 15:17 ` [PATCH 1/7] Revert "drm/amdgpu: Use generic hdp flush function" Alex Deucher
2025-05-07 15:13 ` Alex Deucher
2025-05-07 16:43 ` Lazar, Lijo
2025-05-07 16:49 ` Alex Deucher
2025-05-07 15:27 ` Felix Kuehling
-- strict thread matches above, loose matches on Subject: below --
2025-05-07 15:27 Alex Deucher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox