* [PATCH 0/6] add missing DCE6 functions and uniformize value definitions
@ 2025-02-27 5:22 Alexandre Demers
2025-02-27 5:22 ` [PATCH 1/6] drm/amdgpu: add or move defines for DCE6 in sid.h Alexandre Demers
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Alexandre Demers @ 2025-02-27 5:22 UTC (permalink / raw)
To: amd-gfx
This series uniformizes some value definitions between DCE6, 8 and 10.
It also adds missing code for dce_v6_0_soft_reset() and dce_v6_0_set_crtc_vline_interrupt_state()
Alexandre Demers (6):
drm/amdgpu: add or move defines for DCE6 in sid.h
drm/amdgpu: add dce_v6_0_soft_reset() to DCE6
drm/amdgpu: complete dce_v6_0_set_crtc_vline_interrupt_state() in DCE6
drm/amdgpu: add some comments in DCE6
dmr/amdgpu: fix style in DCE6
drm/amdgpu: add defines for pin_offsets in DCE8
drivers/gpu/drm/amd/amdgpu/cikd.h | 9 ++
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 201 ++++++++++++++++++++------
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 14 +-
drivers/gpu/drm/amd/amdgpu/si_enums.h | 7 -
drivers/gpu/drm/amd/amdgpu/sid.h | 29 +++-
5 files changed, 193 insertions(+), 67 deletions(-)
--
2.48.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/6] drm/amdgpu: add or move defines for DCE6 in sid.h
2025-02-27 5:22 [PATCH 0/6] add missing DCE6 functions and uniformize value definitions Alexandre Demers
@ 2025-02-27 5:22 ` Alexandre Demers
2025-02-27 5:22 ` [PATCH 2/6] drm/amdgpu: add dce_v6_0_soft_reset() to DCE6 Alexandre Demers
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Alexandre Demers @ 2025-02-27 5:22 UTC (permalink / raw)
To: amd-gfx
For coherence with DCE8 et DCE10, add or move some values under sid.h.
Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
---
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 63 ++++++++++++++-------------
drivers/gpu/drm/amd/amdgpu/si_enums.h | 7 ---
drivers/gpu/drm/amd/amdgpu/sid.h | 29 +++++++++---
3 files changed, 55 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index eb7de9122d99..bd763fde1c50 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -32,6 +32,7 @@
#include "amdgpu.h"
#include "amdgpu_pm.h"
#include "amdgpu_i2c.h"
+#include "sid.h"
#include "atom.h"
#include "amdgpu_atombios.h"
#include "atombios_crtc.h"
@@ -59,31 +60,31 @@ static void dce_v6_0_set_irq_funcs(struct amdgpu_device *adev);
static const u32 crtc_offsets[6] =
{
- SI_CRTC0_REGISTER_OFFSET,
- SI_CRTC1_REGISTER_OFFSET,
- SI_CRTC2_REGISTER_OFFSET,
- SI_CRTC3_REGISTER_OFFSET,
- SI_CRTC4_REGISTER_OFFSET,
- SI_CRTC5_REGISTER_OFFSET
+ CRTC0_REGISTER_OFFSET,
+ CRTC1_REGISTER_OFFSET,
+ CRTC2_REGISTER_OFFSET,
+ CRTC3_REGISTER_OFFSET,
+ CRTC4_REGISTER_OFFSET,
+ CRTC5_REGISTER_OFFSET
};
static const u32 hpd_offsets[] =
{
- mmDC_HPD1_INT_STATUS - mmDC_HPD1_INT_STATUS,
- mmDC_HPD2_INT_STATUS - mmDC_HPD1_INT_STATUS,
- mmDC_HPD3_INT_STATUS - mmDC_HPD1_INT_STATUS,
- mmDC_HPD4_INT_STATUS - mmDC_HPD1_INT_STATUS,
- mmDC_HPD5_INT_STATUS - mmDC_HPD1_INT_STATUS,
- mmDC_HPD6_INT_STATUS - mmDC_HPD1_INT_STATUS,
+ HPD0_REGISTER_OFFSET,
+ HPD1_REGISTER_OFFSET,
+ HPD2_REGISTER_OFFSET,
+ HPD3_REGISTER_OFFSET,
+ HPD4_REGISTER_OFFSET,
+ HPD5_REGISTER_OFFSET
};
static const uint32_t dig_offsets[] = {
- SI_CRTC0_REGISTER_OFFSET,
- SI_CRTC1_REGISTER_OFFSET,
- SI_CRTC2_REGISTER_OFFSET,
- SI_CRTC3_REGISTER_OFFSET,
- SI_CRTC4_REGISTER_OFFSET,
- SI_CRTC5_REGISTER_OFFSET,
+ CRTC0_REGISTER_OFFSET,
+ CRTC1_REGISTER_OFFSET,
+ CRTC2_REGISTER_OFFSET,
+ CRTC3_REGISTER_OFFSET,
+ CRTC4_REGISTER_OFFSET,
+ CRTC5_REGISTER_OFFSET,
(0x13830 - 0x7030) >> 2,
};
@@ -1347,13 +1348,13 @@ static void dce_v6_0_audio_enable(struct amdgpu_device *adev,
static const u32 pin_offsets[7] =
{
- (0x1780 - 0x1780),
- (0x1786 - 0x1780),
- (0x178c - 0x1780),
- (0x1792 - 0x1780),
- (0x1798 - 0x1780),
- (0x179d - 0x1780),
- (0x17a4 - 0x1780),
+ AUD0_REGISTER_OFFSET,
+ AUD1_REGISTER_OFFSET,
+ AUD2_REGISTER_OFFSET,
+ AUD3_REGISTER_OFFSET,
+ AUD4_REGISTER_OFFSET,
+ AUD5_REGISTER_OFFSET,
+ AUD6_REGISTER_OFFSET,
};
static int dce_v6_0_audio_init(struct amdgpu_device *adev)
@@ -2862,22 +2863,22 @@ static void dce_v6_0_set_crtc_vblank_interrupt_state(struct amdgpu_device *adev,
switch (crtc) {
case 0:
- reg_block = SI_CRTC0_REGISTER_OFFSET;
+ reg_block = CRTC0_REGISTER_OFFSET;
break;
case 1:
- reg_block = SI_CRTC1_REGISTER_OFFSET;
+ reg_block = CRTC1_REGISTER_OFFSET;
break;
case 2:
- reg_block = SI_CRTC2_REGISTER_OFFSET;
+ reg_block = CRTC2_REGISTER_OFFSET;
break;
case 3:
- reg_block = SI_CRTC3_REGISTER_OFFSET;
+ reg_block = CRTC3_REGISTER_OFFSET;
break;
case 4:
- reg_block = SI_CRTC4_REGISTER_OFFSET;
+ reg_block = CRTC4_REGISTER_OFFSET;
break;
case 5:
- reg_block = SI_CRTC5_REGISTER_OFFSET;
+ reg_block = CRTC5_REGISTER_OFFSET;
break;
default:
DRM_DEBUG("invalid crtc %d\n", crtc);
diff --git a/drivers/gpu/drm/amd/amdgpu/si_enums.h b/drivers/gpu/drm/amd/amdgpu/si_enums.h
index 4e935baa7b91..cdf8eebaa392 100644
--- a/drivers/gpu/drm/amd/amdgpu/si_enums.h
+++ b/drivers/gpu/drm/amd/amdgpu/si_enums.h
@@ -121,13 +121,6 @@
#define CURSOR_UPDATE_LOCK (1 << 16)
#define CURSOR_DISABLE_MULTIPLE_UPDATE (1 << 24)
-#define SI_CRTC0_REGISTER_OFFSET 0
-#define SI_CRTC1_REGISTER_OFFSET 0x300
-#define SI_CRTC2_REGISTER_OFFSET 0x2600
-#define SI_CRTC3_REGISTER_OFFSET 0x2900
-#define SI_CRTC4_REGISTER_OFFSET 0x2c00
-#define SI_CRTC5_REGISTER_OFFSET 0x2f00
-
#define DMA0_REGISTER_OFFSET 0x000
#define DMA1_REGISTER_OFFSET 0x200
#define ES_AND_GS_AUTO 3
diff --git a/drivers/gpu/drm/amd/amdgpu/sid.h b/drivers/gpu/drm/amd/amdgpu/sid.h
index 9a39cbfe6db9..32a4e7d450c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/sid.h
+++ b/drivers/gpu/drm/amd/amdgpu/sid.h
@@ -1991,12 +1991,29 @@
//#dce stupp
/* display controller offsets used for crtc/cur/lut/grph/viewport/etc. */
-#define SI_CRTC0_REGISTER_OFFSET 0 //(0x6df0 - 0x6df0)/4
-#define SI_CRTC1_REGISTER_OFFSET 0x300 //(0x79f0 - 0x6df0)/4
-#define SI_CRTC2_REGISTER_OFFSET 0x2600 //(0x105f0 - 0x6df0)/4
-#define SI_CRTC3_REGISTER_OFFSET 0x2900 //(0x111f0 - 0x6df0)/4
-#define SI_CRTC4_REGISTER_OFFSET 0x2c00 //(0x11df0 - 0x6df0)/4
-#define SI_CRTC5_REGISTER_OFFSET 0x2f00 //(0x129f0 - 0x6df0)/4
+#define CRTC0_REGISTER_OFFSET (0x1b7c - 0x1b7c) //(0x6df0 - 0x6df0)/4
+#define CRTC1_REGISTER_OFFSET (0x1e7c - 0x1b7c) //(0x79f0 - 0x6df0)/4
+#define CRTC2_REGISTER_OFFSET (0x417c - 0x1b7c) //(0x105f0 - 0x6df0)/4
+#define CRTC3_REGISTER_OFFSET (0x447c - 0x1b7c) //(0x111f0 - 0x6df0)/4
+#define CRTC4_REGISTER_OFFSET (0x477c - 0x1b7c) //(0x11df0 - 0x6df0)/4
+#define CRTC5_REGISTER_OFFSET (0x4a7c - 0x1b7c) //(0x129f0 - 0x6df0)/4
+
+/* hpd instance offsets */
+#define HPD0_REGISTER_OFFSET (0x1807 - 0x1807)
+#define HPD1_REGISTER_OFFSET (0x180a - 0x1807)
+#define HPD2_REGISTER_OFFSET (0x180d - 0x1807)
+#define HPD3_REGISTER_OFFSET (0x1810 - 0x1807)
+#define HPD4_REGISTER_OFFSET (0x1813 - 0x1807)
+#define HPD5_REGISTER_OFFSET (0x1816 - 0x1807)
+
+/* audio endpt instance offsets */
+#define AUD0_REGISTER_OFFSET (0x1780 - 0x1780)
+#define AUD1_REGISTER_OFFSET (0x1786 - 0x1780)
+#define AUD2_REGISTER_OFFSET (0x178c - 0x1780)
+#define AUD3_REGISTER_OFFSET (0x1792 - 0x1780)
+#define AUD4_REGISTER_OFFSET (0x1798 - 0x1780)
+#define AUD5_REGISTER_OFFSET (0x179d - 0x1780)
+#define AUD6_REGISTER_OFFSET (0x17a4 - 0x1780)
#define CURSOR_WIDTH 64
#define CURSOR_HEIGHT 64
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/6] drm/amdgpu: add dce_v6_0_soft_reset() to DCE6
2025-02-27 5:22 [PATCH 0/6] add missing DCE6 functions and uniformize value definitions Alexandre Demers
2025-02-27 5:22 ` [PATCH 1/6] drm/amdgpu: add or move defines for DCE6 in sid.h Alexandre Demers
@ 2025-02-27 5:22 ` Alexandre Demers
2025-02-27 14:23 ` Alex Deucher
2025-02-27 5:22 ` [PATCH 3/6] drm/amdgpu: complete dce_v6_0_set_crtc_vline_interrupt_state() in DCE6 Alexandre Demers
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Alexandre Demers @ 2025-02-27 5:22 UTC (permalink / raw)
To: amd-gfx
DCE6 was missing soft reset, but it was easily identifiable under radeon.
This should be it, pretty much as it is done under DCE8 and DCE10.
Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
---
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 62 ++++++++++++++++++++++++---
1 file changed, 57 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index bd763fde1c50..254cb73324c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -371,27 +371,58 @@ static u32 dce_v6_0_hpd_get_gpio_reg(struct amdgpu_device *adev)
return mmDC_GPIO_HPD_A;
}
+static bool dce_v6_0_is_display_hung(struct amdgpu_device *adev)
+{
+ u32 crtc_hung = 0;
+ u32 crtc_status[6];
+ u32 i, j, tmp;
+
+ for (i = 0; i < adev->mode_info.num_crtc; i++) {
+ if (RREG32(mmCRTC_CONTROL + crtc_offsets[i]) & CRTC_CONTROL__CRTC_MASTER_EN_MASK) {
+ crtc_status[i] = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
+ crtc_hung |= (1 << i);
+ }
+ }
+
+ for (j = 0; j < 10; j++) {
+ for (i = 0; i < adev->mode_info.num_crtc; i++) {
+ if (crtc_hung & (1 << i)) {
+ tmp = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
+ if (tmp != crtc_status[i])
+ crtc_hung &= ~(1 << i);
+ }
+ }
+ if (crtc_hung == 0)
+ return false;
+ udelay(100);
+ }
+
+ return true;
+}
+
static void dce_v6_0_set_vga_render_state(struct amdgpu_device *adev,
bool render)
{
if (!render)
WREG32(mmVGA_RENDER_CONTROL,
RREG32(mmVGA_RENDER_CONTROL) & VGA_VSTATUS_CNTL);
-
}
static int dce_v6_0_get_num_crtc(struct amdgpu_device *adev)
{
+ int num_crtc = 0;
+
switch (adev->asic_type) {
case CHIP_TAHITI:
case CHIP_PITCAIRN:
case CHIP_VERDE:
- return 6;
+ num_crtc = 6;
case CHIP_OLAND:
- return 2;
+ num_crtc = 2;
default:
- return 0;
+ num_crtc = 0;
}
+ return num_crtc;
}
void dce_v6_0_disable_dce(struct amdgpu_device *adev)
@@ -2846,7 +2877,28 @@ static bool dce_v6_0_is_idle(void *handle)
static int dce_v6_0_soft_reset(struct amdgpu_ip_block *ip_block)
{
- DRM_INFO("xxxx: dce_v6_0_soft_reset --- no impl!!\n");
+ u32 srbm_soft_reset = 0, tmp;
+ struct amdgpu_device *adev = ip_block->adev;
+
+ if (dce_v6_0_is_display_hung(adev))
+ srbm_soft_reset |= SRBM_SOFT_RESET__SOFT_RESET_DC_MASK;
+
+ if (srbm_soft_reset) {
+ tmp = RREG32(mmSRBM_SOFT_RESET);
+ tmp |= srbm_soft_reset;
+ dev_info(adev->dev, "SRBM_SOFT_RESET=0x%08X\n", tmp);
+ WREG32(mmSRBM_SOFT_RESET, tmp);
+ tmp = RREG32(mmSRBM_SOFT_RESET);
+
+ udelay(50);
+
+ tmp &= ~srbm_soft_reset;
+ WREG32(mmSRBM_SOFT_RESET, tmp);
+ tmp = RREG32(mmSRBM_SOFT_RESET);
+
+ /* Wait a little for things to settle down */
+ udelay(50);
+ }
return 0;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/6] drm/amdgpu: complete dce_v6_0_set_crtc_vline_interrupt_state() in DCE6
2025-02-27 5:22 [PATCH 0/6] add missing DCE6 functions and uniformize value definitions Alexandre Demers
2025-02-27 5:22 ` [PATCH 1/6] drm/amdgpu: add or move defines for DCE6 in sid.h Alexandre Demers
2025-02-27 5:22 ` [PATCH 2/6] drm/amdgpu: add dce_v6_0_soft_reset() to DCE6 Alexandre Demers
@ 2025-02-27 5:22 ` Alexandre Demers
2025-02-27 14:22 ` Alex Deucher
2025-02-27 5:22 ` [PATCH 4/6] drm/amdgpu: add some comments " Alexandre Demers
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Alexandre Demers @ 2025-02-27 5:22 UTC (permalink / raw)
To: amd-gfx
dce_v6_0_set_crtc_vline_interrupt_state() was empty without any info to
inform the user.
Based on DCE8 and DCE10 code.
Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
---
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 44 +++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index 254cb73324c6..e805c4f9222c 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -2957,7 +2957,51 @@ static void dce_v6_0_set_crtc_vline_interrupt_state(struct amdgpu_device *adev,
int crtc,
enum amdgpu_interrupt_state state)
{
+ u32 reg_block, lb_interrupt_mask;
+ if (crtc >= adev->mode_info.num_crtc) {
+ DRM_DEBUG("invalid crtc %d\n", crtc);
+ return;
+ }
+
+ switch (crtc) {
+ case 0:
+ reg_block = CRTC0_REGISTER_OFFSET;
+ break;
+ case 1:
+ reg_block = CRTC1_REGISTER_OFFSET;
+ break;
+ case 2:
+ reg_block = CRTC2_REGISTER_OFFSET;
+ break;
+ case 3:
+ reg_block = CRTC3_REGISTER_OFFSET;
+ break;
+ case 4:
+ reg_block = CRTC4_REGISTER_OFFSET;
+ break;
+ case 5:
+ reg_block = CRTC5_REGISTER_OFFSET;
+ break;
+ default:
+ DRM_DEBUG("invalid crtc %d\n", crtc);
+ return;
+ }
+
+ switch (state) {
+ case AMDGPU_IRQ_STATE_DISABLE:
+ lb_interrupt_mask = RREG32(mmINT_MASK + reg_block);
+ lb_interrupt_mask &= ~VLINE_INT_MASK;
+ WREG32(mmINT_MASK + reg_block, lb_interrupt_mask);
+ break;
+ case AMDGPU_IRQ_STATE_ENABLE:
+ lb_interrupt_mask = RREG32(mmINT_MASK + reg_block);
+ lb_interrupt_mask |= VLINE_INT_MASK;
+ WREG32(mmINT_MASK + reg_block, lb_interrupt_mask);
+ break;
+ default:
+ break;
+ }
}
static int dce_v6_0_set_hpd_interrupt_state(struct amdgpu_device *adev,
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/6] drm/amdgpu: add some comments in DCE6
2025-02-27 5:22 [PATCH 0/6] add missing DCE6 functions and uniformize value definitions Alexandre Demers
` (2 preceding siblings ...)
2025-02-27 5:22 ` [PATCH 3/6] drm/amdgpu: complete dce_v6_0_set_crtc_vline_interrupt_state() in DCE6 Alexandre Demers
@ 2025-02-27 5:22 ` Alexandre Demers
2025-02-27 5:22 ` [PATCH 5/6] dmr/amdgpu: fix style " Alexandre Demers
2025-02-27 5:22 ` [PATCH 6/6] drm/amdgpu: add defines for pin_offsets in DCE8 Alexandre Demers
5 siblings, 0 replies; 15+ messages in thread
From: Alexandre Demers @ 2025-02-27 5:22 UTC (permalink / raw)
To: amd-gfx
Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
---
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index e805c4f9222c..fd2eb454a5d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -207,9 +207,9 @@ static void dce_v6_0_page_flip(struct amdgpu_device *adev,
/* update the scanout addresses */
WREG32(mmGRPH_PRIMARY_SURFACE_ADDRESS_HIGH + amdgpu_crtc->crtc_offset,
upper_32_bits(crtc_base));
+ /* writing to the low address triggers the update */
WREG32(mmGRPH_PRIMARY_SURFACE_ADDRESS + amdgpu_crtc->crtc_offset,
(u32)crtc_base);
-
/* post the write */
RREG32(mmGRPH_PRIMARY_SURFACE_ADDRESS + amdgpu_crtc->crtc_offset);
}
@@ -219,11 +219,11 @@ static int dce_v6_0_crtc_get_scanoutpos(struct amdgpu_device *adev, int crtc,
{
if ((crtc < 0) || (crtc >= adev->mode_info.num_crtc))
return -EINVAL;
+
*vbl = RREG32(mmCRTC_V_BLANK_START_END + crtc_offsets[crtc]);
*position = RREG32(mmCRTC_STATUS_POSITION + crtc_offsets[crtc]);
return 0;
-
}
/**
@@ -1038,6 +1038,18 @@ static void dce_v6_0_program_watermarks(struct amdgpu_device *adev,
}
/* watermark setup */
+/**
+ * dce_v6_0_line_buffer_adjust - Set up the line buffer
+ *
+ * @adev: amdgpu_device pointer
+ * @amdgpu_crtc: the selected display controller
+ * @mode: the current display mode on the selected display
+ * controller
+ *
+ * Setup up the line buffer allocation for
+ * the selected display controller (CIK).
+ * Returns the line buffer size in pixels.
+ */
static u32 dce_v6_0_line_buffer_adjust(struct amdgpu_device *adev,
struct amdgpu_crtc *amdgpu_crtc,
struct drm_display_mode *mode,
@@ -1418,6 +1430,8 @@ static int dce_v6_0_audio_init(struct amdgpu_device *adev)
adev->mode_info.audio.pin[i].connected = false;
adev->mode_info.audio.pin[i].offset = pin_offsets[i];
adev->mode_info.audio.pin[i].id = i;
+ /* disable audio. it will be set up later */
+ /* XXX remove once we switch to ip funcs */
dce_v6_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
}
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/6] dmr/amdgpu: fix style in DCE6
2025-02-27 5:22 [PATCH 0/6] add missing DCE6 functions and uniformize value definitions Alexandre Demers
` (3 preceding siblings ...)
2025-02-27 5:22 ` [PATCH 4/6] drm/amdgpu: add some comments " Alexandre Demers
@ 2025-02-27 5:22 ` Alexandre Demers
2025-02-27 5:22 ` [PATCH 6/6] drm/amdgpu: add defines for pin_offsets in DCE8 Alexandre Demers
5 siblings, 0 replies; 15+ messages in thread
From: Alexandre Demers @ 2025-02-27 5:22 UTC (permalink / raw)
To: amd-gfx
A few returns not where they should be.
Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
---
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index fd2eb454a5d8..4c3539e4960a 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -243,7 +243,8 @@ static bool dce_v6_0_hpd_sense(struct amdgpu_device *adev,
if (hpd >= adev->mode_info.num_hpd)
return connected;
- if (RREG32(mmDC_HPD1_INT_STATUS + hpd_offsets[hpd]) & DC_HPD1_INT_STATUS__DC_HPD1_SENSE_MASK)
+ if (RREG32(mmDC_HPD1_INT_STATUS + hpd_offsets[hpd]) &
+ DC_HPD1_INT_STATUS__DC_HPD1_SENSE_MASK)
connected = true;
return connected;
@@ -451,7 +452,6 @@ void dce_v6_0_disable_dce(struct amdgpu_device *adev)
static void dce_v6_0_program_fmt(struct drm_encoder *encoder)
{
-
struct drm_device *dev = encoder->dev;
struct amdgpu_device *adev = drm_to_adev(dev);
struct amdgpu_encoder *amdgpu_encoder = to_amdgpu_encoder(encoder);
@@ -927,8 +927,8 @@ static void dce_v6_0_program_watermarks(struct amdgpu_device *adev,
wm_high.dram_channels = dram_channels;
wm_high.num_heads = num_heads;
- if (adev->pm.dpm_enabled) {
/* watermark for low clocks */
+ if (adev->pm.dpm_enabled) {
wm_low.yclk =
amdgpu_dpm_get_mclk(adev, true) * 10;
wm_low.sclk =
@@ -3232,7 +3232,6 @@ static int dce_v6_0_hpd_irq(struct amdgpu_device *adev,
}
return 0;
-
}
static int dce_v6_0_set_clockgating_state(void *handle,
@@ -3365,8 +3364,7 @@ static void dce_v6_0_ext_commit(struct drm_encoder *encoder)
}
-static void
-dce_v6_0_ext_mode_set(struct drm_encoder *encoder,
+static void dce_v6_0_ext_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
{
@@ -3378,8 +3376,7 @@ static void dce_v6_0_ext_disable(struct drm_encoder *encoder)
}
-static void
-dce_v6_0_ext_dpms(struct drm_encoder *encoder, int mode)
+static void dce_v6_0_ext_dpms(struct drm_encoder *encoder, int mode)
{
}
@@ -3450,7 +3447,6 @@ static void dce_v6_0_encoder_add(struct amdgpu_device *adev,
amdgpu_encoder->devices |= supported_device;
return;
}
-
}
/* add a new one */
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 6/6] drm/amdgpu: add defines for pin_offsets in DCE8
2025-02-27 5:22 [PATCH 0/6] add missing DCE6 functions and uniformize value definitions Alexandre Demers
` (4 preceding siblings ...)
2025-02-27 5:22 ` [PATCH 5/6] dmr/amdgpu: fix style " Alexandre Demers
@ 2025-02-27 5:22 ` Alexandre Demers
5 siblings, 0 replies; 15+ messages in thread
From: Alexandre Demers @ 2025-02-27 5:22 UTC (permalink / raw)
To: amd-gfx
Define pin_offsets values in the same way it is done in DCE8
Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
---
drivers/gpu/drm/amd/amdgpu/cikd.h | 9 +++++++++
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 14 +++++++-------
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/cikd.h b/drivers/gpu/drm/amd/amdgpu/cikd.h
index 06088d52d81c..279288365940 100644
--- a/drivers/gpu/drm/amd/amdgpu/cikd.h
+++ b/drivers/gpu/drm/amd/amdgpu/cikd.h
@@ -51,6 +51,15 @@
#define HPD4_REGISTER_OFFSET (0x1813 - 0x1807)
#define HPD5_REGISTER_OFFSET (0x1816 - 0x1807)
+/* audio endpt instance offsets */
+#define AUD0_REGISTER_OFFSET (0x1780 - 0x1780)
+#define AUD1_REGISTER_OFFSET (0x1786 - 0x1780)
+#define AUD2_REGISTER_OFFSET (0x178c - 0x1780)
+#define AUD3_REGISTER_OFFSET (0x1792 - 0x1780)
+#define AUD4_REGISTER_OFFSET (0x1798 - 0x1780)
+#define AUD5_REGISTER_OFFSET (0x179d - 0x1780)
+#define AUD6_REGISTER_OFFSET (0x17a4 - 0x1780)
+
#define BONAIRE_GB_ADDR_CONFIG_GOLDEN 0x12010001
#define HAWAII_GB_ADDR_CONFIG_GOLDEN 0x12011003
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 04b79ff87f75..5e657b43a159 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1395,13 +1395,13 @@ static void dce_v8_0_audio_enable(struct amdgpu_device *adev,
}
static const u32 pin_offsets[7] = {
- (0x1780 - 0x1780),
- (0x1786 - 0x1780),
- (0x178c - 0x1780),
- (0x1792 - 0x1780),
- (0x1798 - 0x1780),
- (0x179d - 0x1780),
- (0x17a4 - 0x1780),
+ AUD0_REGISTER_OFFSET,
+ AUD1_REGISTER_OFFSET,
+ AUD2_REGISTER_OFFSET,
+ AUD3_REGISTER_OFFSET,
+ AUD4_REGISTER_OFFSET,
+ AUD5_REGISTER_OFFSET,
+ AUD6_REGISTER_OFFSET,
};
static int dce_v8_0_audio_init(struct amdgpu_device *adev)
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] drm/amdgpu: complete dce_v6_0_set_crtc_vline_interrupt_state() in DCE6
2025-02-27 5:22 ` [PATCH 3/6] drm/amdgpu: complete dce_v6_0_set_crtc_vline_interrupt_state() in DCE6 Alexandre Demers
@ 2025-02-27 14:22 ` Alex Deucher
0 siblings, 0 replies; 15+ messages in thread
From: Alex Deucher @ 2025-02-27 14:22 UTC (permalink / raw)
To: Alexandre Demers; +Cc: amd-gfx
On Thu, Feb 27, 2025 at 12:23 AM Alexandre Demers
<alexandre.f.demers@gmail.com> wrote:
>
> dce_v6_0_set_crtc_vline_interrupt_state() was empty without any info to
> inform the user.
Doesn't hurt to fill it in, but nothing uses the vline interrupt at
the moment. Might be better to just remove it from all of the non-DC
display code.
Alex
>
> Based on DCE8 and DCE10 code.
>
> Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> ---
> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 44 +++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 254cb73324c6..e805c4f9222c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -2957,7 +2957,51 @@ static void dce_v6_0_set_crtc_vline_interrupt_state(struct amdgpu_device *adev,
> int crtc,
> enum amdgpu_interrupt_state state)
> {
> + u32 reg_block, lb_interrupt_mask;
>
> + if (crtc >= adev->mode_info.num_crtc) {
> + DRM_DEBUG("invalid crtc %d\n", crtc);
> + return;
> + }
> +
> + switch (crtc) {
> + case 0:
> + reg_block = CRTC0_REGISTER_OFFSET;
> + break;
> + case 1:
> + reg_block = CRTC1_REGISTER_OFFSET;
> + break;
> + case 2:
> + reg_block = CRTC2_REGISTER_OFFSET;
> + break;
> + case 3:
> + reg_block = CRTC3_REGISTER_OFFSET;
> + break;
> + case 4:
> + reg_block = CRTC4_REGISTER_OFFSET;
> + break;
> + case 5:
> + reg_block = CRTC5_REGISTER_OFFSET;
> + break;
> + default:
> + DRM_DEBUG("invalid crtc %d\n", crtc);
> + return;
> + }
> +
> + switch (state) {
> + case AMDGPU_IRQ_STATE_DISABLE:
> + lb_interrupt_mask = RREG32(mmINT_MASK + reg_block);
> + lb_interrupt_mask &= ~VLINE_INT_MASK;
> + WREG32(mmINT_MASK + reg_block, lb_interrupt_mask);
> + break;
> + case AMDGPU_IRQ_STATE_ENABLE:
> + lb_interrupt_mask = RREG32(mmINT_MASK + reg_block);
> + lb_interrupt_mask |= VLINE_INT_MASK;
> + WREG32(mmINT_MASK + reg_block, lb_interrupt_mask);
> + break;
> + default:
> + break;
> + }
> }
>
> static int dce_v6_0_set_hpd_interrupt_state(struct amdgpu_device *adev,
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] drm/amdgpu: add dce_v6_0_soft_reset() to DCE6
2025-02-27 5:22 ` [PATCH 2/6] drm/amdgpu: add dce_v6_0_soft_reset() to DCE6 Alexandre Demers
@ 2025-02-27 14:23 ` Alex Deucher
2025-02-27 18:51 ` Alexandre Demers
0 siblings, 1 reply; 15+ messages in thread
From: Alex Deucher @ 2025-02-27 14:23 UTC (permalink / raw)
To: Alexandre Demers; +Cc: amd-gfx
On Thu, Feb 27, 2025 at 12:49 AM Alexandre Demers
<alexandre.f.demers@gmail.com> wrote:
>
> DCE6 was missing soft reset, but it was easily identifiable under radeon.
> This should be it, pretty much as it is done under DCE8 and DCE10.
>
> Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> ---
> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 62 ++++++++++++++++++++++++---
> 1 file changed, 57 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index bd763fde1c50..254cb73324c6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -371,27 +371,58 @@ static u32 dce_v6_0_hpd_get_gpio_reg(struct amdgpu_device *adev)
> return mmDC_GPIO_HPD_A;
> }
>
> +static bool dce_v6_0_is_display_hung(struct amdgpu_device *adev)
> +{
> + u32 crtc_hung = 0;
> + u32 crtc_status[6];
> + u32 i, j, tmp;
> +
> + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> + if (RREG32(mmCRTC_CONTROL + crtc_offsets[i]) & CRTC_CONTROL__CRTC_MASTER_EN_MASK) {
> + crtc_status[i] = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> + crtc_hung |= (1 << i);
> + }
> + }
> +
> + for (j = 0; j < 10; j++) {
> + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> + if (crtc_hung & (1 << i)) {
> + tmp = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> + if (tmp != crtc_status[i])
> + crtc_hung &= ~(1 << i);
> + }
> + }
> + if (crtc_hung == 0)
> + return false;
> + udelay(100);
> + }
> +
> + return true;
> +}
> +
> static void dce_v6_0_set_vga_render_state(struct amdgpu_device *adev,
> bool render)
> {
> if (!render)
> WREG32(mmVGA_RENDER_CONTROL,
> RREG32(mmVGA_RENDER_CONTROL) & VGA_VSTATUS_CNTL);
> -
> }
>
> static int dce_v6_0_get_num_crtc(struct amdgpu_device *adev)
> {
> + int num_crtc = 0;
> +
> switch (adev->asic_type) {
> case CHIP_TAHITI:
> case CHIP_PITCAIRN:
> case CHIP_VERDE:
> - return 6;
> + num_crtc = 6;
> case CHIP_OLAND:
> - return 2;
> + num_crtc = 2;
> default:
> - return 0;
> + num_crtc = 0;
> }
> + return num_crtc;
Any particular reason for this change? It just adds an extra variable.
Alex
> }
>
> void dce_v6_0_disable_dce(struct amdgpu_device *adev)
> @@ -2846,7 +2877,28 @@ static bool dce_v6_0_is_idle(void *handle)
>
> static int dce_v6_0_soft_reset(struct amdgpu_ip_block *ip_block)
> {
> - DRM_INFO("xxxx: dce_v6_0_soft_reset --- no impl!!\n");
> + u32 srbm_soft_reset = 0, tmp;
> + struct amdgpu_device *adev = ip_block->adev;
> +
> + if (dce_v6_0_is_display_hung(adev))
> + srbm_soft_reset |= SRBM_SOFT_RESET__SOFT_RESET_DC_MASK;
> +
> + if (srbm_soft_reset) {
> + tmp = RREG32(mmSRBM_SOFT_RESET);
> + tmp |= srbm_soft_reset;
> + dev_info(adev->dev, "SRBM_SOFT_RESET=0x%08X\n", tmp);
> + WREG32(mmSRBM_SOFT_RESET, tmp);
> + tmp = RREG32(mmSRBM_SOFT_RESET);
> +
> + udelay(50);
> +
> + tmp &= ~srbm_soft_reset;
> + WREG32(mmSRBM_SOFT_RESET, tmp);
> + tmp = RREG32(mmSRBM_SOFT_RESET);
> +
> + /* Wait a little for things to settle down */
> + udelay(50);
> + }
> return 0;
> }
>
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] drm/amdgpu: add dce_v6_0_soft_reset() to DCE6
2025-02-27 14:23 ` Alex Deucher
@ 2025-02-27 18:51 ` Alexandre Demers
2025-02-27 19:01 ` Alex Deucher
0 siblings, 1 reply; 15+ messages in thread
From: Alexandre Demers @ 2025-02-27 18:51 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx
On Thu, Feb 27, 2025 at 9:23 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Thu, Feb 27, 2025 at 12:49 AM Alexandre Demers
> <alexandre.f.demers@gmail.com> wrote:
> >
> > DCE6 was missing soft reset, but it was easily identifiable under radeon.
> > This should be it, pretty much as it is done under DCE8 and DCE10.
> >
> > Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 62 ++++++++++++++++++++++++---
> > 1 file changed, 57 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > index bd763fde1c50..254cb73324c6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > @@ -371,27 +371,58 @@ static u32 dce_v6_0_hpd_get_gpio_reg(struct amdgpu_device *adev)
> > return mmDC_GPIO_HPD_A;
> > }
> >
> > +static bool dce_v6_0_is_display_hung(struct amdgpu_device *adev)
> > +{
> > + u32 crtc_hung = 0;
> > + u32 crtc_status[6];
> > + u32 i, j, tmp;
> > +
> > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > + if (RREG32(mmCRTC_CONTROL + crtc_offsets[i]) & CRTC_CONTROL__CRTC_MASTER_EN_MASK) {
> > + crtc_status[i] = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > + crtc_hung |= (1 << i);
> > + }
> > + }
> > +
> > + for (j = 0; j < 10; j++) {
> > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > + if (crtc_hung & (1 << i)) {
> > + tmp = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > + if (tmp != crtc_status[i])
> > + crtc_hung &= ~(1 << i);
> > + }
> > + }
> > + if (crtc_hung == 0)
> > + return false;
> > + udelay(100);
> > + }
> > +
> > + return true;
> > +}
> > +
> > static void dce_v6_0_set_vga_render_state(struct amdgpu_device *adev,
> > bool render)
> > {
> > if (!render)
> > WREG32(mmVGA_RENDER_CONTROL,
> > RREG32(mmVGA_RENDER_CONTROL) & VGA_VSTATUS_CNTL);
> > -
> > }
> >
> > static int dce_v6_0_get_num_crtc(struct amdgpu_device *adev)
> > {
> > + int num_crtc = 0;
> > +
> > switch (adev->asic_type) {
> > case CHIP_TAHITI:
> > case CHIP_PITCAIRN:
> > case CHIP_VERDE:
> > - return 6;
> > + num_crtc = 6;
> > case CHIP_OLAND:
> > - return 2;
> > + num_crtc = 2;
> > default:
> > - return 0;
> > + num_crtc = 0;
> > }
> > + return num_crtc;
>
> Any particular reason for this change? It just adds an extra variable.
>
> Alex
Just for uniformisation with DCE8 and DCE10. We could also remove the
variable and use returns everywhere.
Any preferences?
Alexandre
>
> > }
> >
> > void dce_v6_0_disable_dce(struct amdgpu_device *adev)
> > @@ -2846,7 +2877,28 @@ static bool dce_v6_0_is_idle(void *handle)
> >
> > static int dce_v6_0_soft_reset(struct amdgpu_ip_block *ip_block)
> > {
> > - DRM_INFO("xxxx: dce_v6_0_soft_reset --- no impl!!\n");
> > + u32 srbm_soft_reset = 0, tmp;
> > + struct amdgpu_device *adev = ip_block->adev;
> > +
> > + if (dce_v6_0_is_display_hung(adev))
> > + srbm_soft_reset |= SRBM_SOFT_RESET__SOFT_RESET_DC_MASK;
> > +
> > + if (srbm_soft_reset) {
> > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > + tmp |= srbm_soft_reset;
> > + dev_info(adev->dev, "SRBM_SOFT_RESET=0x%08X\n", tmp);
> > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > +
> > + udelay(50);
> > +
> > + tmp &= ~srbm_soft_reset;
> > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > +
> > + /* Wait a little for things to settle down */
> > + udelay(50);
> > + }
> > return 0;
> > }
> >
> > --
> > 2.48.1
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] drm/amdgpu: add dce_v6_0_soft_reset() to DCE6
2025-02-27 18:51 ` Alexandre Demers
@ 2025-02-27 19:01 ` Alex Deucher
2025-02-27 19:05 ` Alex Deucher
2025-02-27 19:09 ` Alexandre Demers
0 siblings, 2 replies; 15+ messages in thread
From: Alex Deucher @ 2025-02-27 19:01 UTC (permalink / raw)
To: Alexandre Demers; +Cc: amd-gfx
On Thu, Feb 27, 2025 at 1:52 PM Alexandre Demers
<alexandre.f.demers@gmail.com> wrote:
>
> On Thu, Feb 27, 2025 at 9:23 AM Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > On Thu, Feb 27, 2025 at 12:49 AM Alexandre Demers
> > <alexandre.f.demers@gmail.com> wrote:
> > >
> > > DCE6 was missing soft reset, but it was easily identifiable under radeon.
> > > This should be it, pretty much as it is done under DCE8 and DCE10.
> > >
> > > Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 62 ++++++++++++++++++++++++---
> > > 1 file changed, 57 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > index bd763fde1c50..254cb73324c6 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > @@ -371,27 +371,58 @@ static u32 dce_v6_0_hpd_get_gpio_reg(struct amdgpu_device *adev)
> > > return mmDC_GPIO_HPD_A;
> > > }
> > >
> > > +static bool dce_v6_0_is_display_hung(struct amdgpu_device *adev)
> > > +{
> > > + u32 crtc_hung = 0;
> > > + u32 crtc_status[6];
> > > + u32 i, j, tmp;
> > > +
> > > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > > + if (RREG32(mmCRTC_CONTROL + crtc_offsets[i]) & CRTC_CONTROL__CRTC_MASTER_EN_MASK) {
> > > + crtc_status[i] = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > > + crtc_hung |= (1 << i);
> > > + }
> > > + }
> > > +
> > > + for (j = 0; j < 10; j++) {
> > > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > > + if (crtc_hung & (1 << i)) {
> > > + tmp = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > > + if (tmp != crtc_status[i])
> > > + crtc_hung &= ~(1 << i);
> > > + }
> > > + }
> > > + if (crtc_hung == 0)
> > > + return false;
> > > + udelay(100);
> > > + }
> > > +
> > > + return true;
> > > +}
> > > +
> > > static void dce_v6_0_set_vga_render_state(struct amdgpu_device *adev,
> > > bool render)
> > > {
> > > if (!render)
> > > WREG32(mmVGA_RENDER_CONTROL,
> > > RREG32(mmVGA_RENDER_CONTROL) & VGA_VSTATUS_CNTL);
> > > -
> > > }
> > >
> > > static int dce_v6_0_get_num_crtc(struct amdgpu_device *adev)
> > > {
> > > + int num_crtc = 0;
> > > +
> > > switch (adev->asic_type) {
> > > case CHIP_TAHITI:
> > > case CHIP_PITCAIRN:
> > > case CHIP_VERDE:
> > > - return 6;
> > > + num_crtc = 6;
> > > case CHIP_OLAND:
> > > - return 2;
> > > + num_crtc = 2;
> > > default:
> > > - return 0;
> > > + num_crtc = 0;
> > > }
> > > + return num_crtc;
> >
> > Any particular reason for this change? It just adds an extra variable.
> >
> > Alex
>
> Just for uniformisation with DCE8 and DCE10. We could also remove the
> variable and use returns everywhere.
>
> Any preferences?
ah, ok. I think the direct returns are cleaner.
Alex
> Alexandre
>
> >
> > > }
> > >
> > > void dce_v6_0_disable_dce(struct amdgpu_device *adev)
> > > @@ -2846,7 +2877,28 @@ static bool dce_v6_0_is_idle(void *handle)
> > >
> > > static int dce_v6_0_soft_reset(struct amdgpu_ip_block *ip_block)
> > > {
> > > - DRM_INFO("xxxx: dce_v6_0_soft_reset --- no impl!!\n");
> > > + u32 srbm_soft_reset = 0, tmp;
> > > + struct amdgpu_device *adev = ip_block->adev;
> > > +
> > > + if (dce_v6_0_is_display_hung(adev))
> > > + srbm_soft_reset |= SRBM_SOFT_RESET__SOFT_RESET_DC_MASK;
> > > +
> > > + if (srbm_soft_reset) {
> > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > + tmp |= srbm_soft_reset;
> > > + dev_info(adev->dev, "SRBM_SOFT_RESET=0x%08X\n", tmp);
> > > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > +
> > > + udelay(50);
> > > +
> > > + tmp &= ~srbm_soft_reset;
> > > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > +
> > > + /* Wait a little for things to settle down */
> > > + udelay(50);
> > > + }
> > > return 0;
> > > }
> > >
> > > --
> > > 2.48.1
> > >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] drm/amdgpu: add dce_v6_0_soft_reset() to DCE6
2025-02-27 19:01 ` Alex Deucher
@ 2025-02-27 19:05 ` Alex Deucher
2025-02-27 19:11 ` Alexandre Demers
2025-02-27 19:09 ` Alexandre Demers
1 sibling, 1 reply; 15+ messages in thread
From: Alex Deucher @ 2025-02-27 19:05 UTC (permalink / raw)
To: Alexandre Demers; +Cc: amd-gfx
On Thu, Feb 27, 2025 at 2:01 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Thu, Feb 27, 2025 at 1:52 PM Alexandre Demers
> <alexandre.f.demers@gmail.com> wrote:
> >
> > On Thu, Feb 27, 2025 at 9:23 AM Alex Deucher <alexdeucher@gmail.com> wrote:
> > >
> > > On Thu, Feb 27, 2025 at 12:49 AM Alexandre Demers
> > > <alexandre.f.demers@gmail.com> wrote:
> > > >
> > > > DCE6 was missing soft reset, but it was easily identifiable under radeon.
> > > > This should be it, pretty much as it is done under DCE8 and DCE10.
> > > >
> > > > Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> > > > ---
> > > > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 62 ++++++++++++++++++++++++---
> > > > 1 file changed, 57 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > index bd763fde1c50..254cb73324c6 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > @@ -371,27 +371,58 @@ static u32 dce_v6_0_hpd_get_gpio_reg(struct amdgpu_device *adev)
> > > > return mmDC_GPIO_HPD_A;
> > > > }
> > > >
> > > > +static bool dce_v6_0_is_display_hung(struct amdgpu_device *adev)
> > > > +{
> > > > + u32 crtc_hung = 0;
> > > > + u32 crtc_status[6];
> > > > + u32 i, j, tmp;
> > > > +
> > > > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > > > + if (RREG32(mmCRTC_CONTROL + crtc_offsets[i]) & CRTC_CONTROL__CRTC_MASTER_EN_MASK) {
> > > > + crtc_status[i] = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > > > + crtc_hung |= (1 << i);
> > > > + }
> > > > + }
> > > > +
> > > > + for (j = 0; j < 10; j++) {
> > > > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > > > + if (crtc_hung & (1 << i)) {
> > > > + tmp = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > > > + if (tmp != crtc_status[i])
> > > > + crtc_hung &= ~(1 << i);
> > > > + }
> > > > + }
> > > > + if (crtc_hung == 0)
> > > > + return false;
> > > > + udelay(100);
> > > > + }
> > > > +
> > > > + return true;
> > > > +}
> > > > +
> > > > static void dce_v6_0_set_vga_render_state(struct amdgpu_device *adev,
> > > > bool render)
> > > > {
> > > > if (!render)
> > > > WREG32(mmVGA_RENDER_CONTROL,
> > > > RREG32(mmVGA_RENDER_CONTROL) & VGA_VSTATUS_CNTL);
> > > > -
> > > > }
> > > >
> > > > static int dce_v6_0_get_num_crtc(struct amdgpu_device *adev)
> > > > {
> > > > + int num_crtc = 0;
> > > > +
> > > > switch (adev->asic_type) {
> > > > case CHIP_TAHITI:
> > > > case CHIP_PITCAIRN:
> > > > case CHIP_VERDE:
> > > > - return 6;
> > > > + num_crtc = 6;
> > > > case CHIP_OLAND:
> > > > - return 2;
> > > > + num_crtc = 2;
> > > > default:
> > > > - return 0;
> > > > + num_crtc = 0;
> > > > }
> > > > + return num_crtc;
> > >
> > > Any particular reason for this change? It just adds an extra variable.
> > >
> > > Alex
> >
> > Just for uniformisation with DCE8 and DCE10. We could also remove the
> > variable and use returns everywhere.
> >
> > Any preferences?
>
> ah, ok. I think the direct returns are cleaner.
I would maybe split up your patches into maybe 3 logical patch sets:
one to fix spelling typos and comments, one to make the DCE code more
uniform across versions, and another to add new DCE6 functionality.
Alex
>
> Alex
>
> > Alexandre
> >
> > >
> > > > }
> > > >
> > > > void dce_v6_0_disable_dce(struct amdgpu_device *adev)
> > > > @@ -2846,7 +2877,28 @@ static bool dce_v6_0_is_idle(void *handle)
> > > >
> > > > static int dce_v6_0_soft_reset(struct amdgpu_ip_block *ip_block)
> > > > {
> > > > - DRM_INFO("xxxx: dce_v6_0_soft_reset --- no impl!!\n");
> > > > + u32 srbm_soft_reset = 0, tmp;
> > > > + struct amdgpu_device *adev = ip_block->adev;
> > > > +
> > > > + if (dce_v6_0_is_display_hung(adev))
> > > > + srbm_soft_reset |= SRBM_SOFT_RESET__SOFT_RESET_DC_MASK;
> > > > +
> > > > + if (srbm_soft_reset) {
> > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > + tmp |= srbm_soft_reset;
> > > > + dev_info(adev->dev, "SRBM_SOFT_RESET=0x%08X\n", tmp);
> > > > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > +
> > > > + udelay(50);
> > > > +
> > > > + tmp &= ~srbm_soft_reset;
> > > > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > +
> > > > + /* Wait a little for things to settle down */
> > > > + udelay(50);
> > > > + }
> > > > return 0;
> > > > }
> > > >
> > > > --
> > > > 2.48.1
> > > >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] drm/amdgpu: add dce_v6_0_soft_reset() to DCE6
2025-02-27 19:01 ` Alex Deucher
2025-02-27 19:05 ` Alex Deucher
@ 2025-02-27 19:09 ` Alexandre Demers
1 sibling, 0 replies; 15+ messages in thread
From: Alexandre Demers @ 2025-02-27 19:09 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx
On Thu, Feb 27, 2025 at 2:01 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Thu, Feb 27, 2025 at 1:52 PM Alexandre Demers
> <alexandre.f.demers@gmail.com> wrote:
> >
> > On Thu, Feb 27, 2025 at 9:23 AM Alex Deucher <alexdeucher@gmail.com> wrote:
> > >
> > > On Thu, Feb 27, 2025 at 12:49 AM Alexandre Demers
> > > <alexandre.f.demers@gmail.com> wrote:
> > > >
> > > > DCE6 was missing soft reset, but it was easily identifiable under radeon.
> > > > This should be it, pretty much as it is done under DCE8 and DCE10.
> > > >
> > > > Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> > > > ---
> > > > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 62 ++++++++++++++++++++++++---
> > > > 1 file changed, 57 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > index bd763fde1c50..254cb73324c6 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > @@ -371,27 +371,58 @@ static u32 dce_v6_0_hpd_get_gpio_reg(struct amdgpu_device *adev)
> > > > return mmDC_GPIO_HPD_A;
> > > > }
> > > >
> > > > +static bool dce_v6_0_is_display_hung(struct amdgpu_device *adev)
> > > > +{
> > > > + u32 crtc_hung = 0;
> > > > + u32 crtc_status[6];
> > > > + u32 i, j, tmp;
> > > > +
> > > > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > > > + if (RREG32(mmCRTC_CONTROL + crtc_offsets[i]) & CRTC_CONTROL__CRTC_MASTER_EN_MASK) {
> > > > + crtc_status[i] = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > > > + crtc_hung |= (1 << i);
> > > > + }
> > > > + }
> > > > +
> > > > + for (j = 0; j < 10; j++) {
> > > > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > > > + if (crtc_hung & (1 << i)) {
> > > > + tmp = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > > > + if (tmp != crtc_status[i])
> > > > + crtc_hung &= ~(1 << i);
> > > > + }
> > > > + }
> > > > + if (crtc_hung == 0)
> > > > + return false;
> > > > + udelay(100);
> > > > + }
> > > > +
> > > > + return true;
> > > > +}
> > > > +
> > > > static void dce_v6_0_set_vga_render_state(struct amdgpu_device *adev,
> > > > bool render)
> > > > {
> > > > if (!render)
> > > > WREG32(mmVGA_RENDER_CONTROL,
> > > > RREG32(mmVGA_RENDER_CONTROL) & VGA_VSTATUS_CNTL);
> > > > -
> > > > }
> > > >
> > > > static int dce_v6_0_get_num_crtc(struct amdgpu_device *adev)
> > > > {
> > > > + int num_crtc = 0;
> > > > +
> > > > switch (adev->asic_type) {
> > > > case CHIP_TAHITI:
> > > > case CHIP_PITCAIRN:
> > > > case CHIP_VERDE:
> > > > - return 6;
> > > > + num_crtc = 6;
> > > > case CHIP_OLAND:
> > > > - return 2;
> > > > + num_crtc = 2;
> > > > default:
> > > > - return 0;
> > > > + num_crtc = 0;
> > > > }
> > > > + return num_crtc;
> > >
> > > Any particular reason for this change? It just adds an extra variable.
> > >
> > > Alex
> >
> > Just for uniformisation with DCE8 and DCE10. We could also remove the
> > variable and use returns everywhere.
> >
> > Any preferences?
>
> ah, ok. I think the direct returns are cleaner.
>
> Alex
Ok. Should I submit a new version or should this patch go through and
be changed everywhere in a different patch?
Alexandre
>
> > Alexandre
> >
> > >
> > > > }
> > > >
> > > > void dce_v6_0_disable_dce(struct amdgpu_device *adev)
> > > > @@ -2846,7 +2877,28 @@ static bool dce_v6_0_is_idle(void *handle)
> > > >
> > > > static int dce_v6_0_soft_reset(struct amdgpu_ip_block *ip_block)
> > > > {
> > > > - DRM_INFO("xxxx: dce_v6_0_soft_reset --- no impl!!\n");
> > > > + u32 srbm_soft_reset = 0, tmp;
> > > > + struct amdgpu_device *adev = ip_block->adev;
> > > > +
> > > > + if (dce_v6_0_is_display_hung(adev))
> > > > + srbm_soft_reset |= SRBM_SOFT_RESET__SOFT_RESET_DC_MASK;
> > > > +
> > > > + if (srbm_soft_reset) {
> > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > + tmp |= srbm_soft_reset;
> > > > + dev_info(adev->dev, "SRBM_SOFT_RESET=0x%08X\n", tmp);
> > > > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > +
> > > > + udelay(50);
> > > > +
> > > > + tmp &= ~srbm_soft_reset;
> > > > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > +
> > > > + /* Wait a little for things to settle down */
> > > > + udelay(50);
> > > > + }
> > > > return 0;
> > > > }
> > > >
> > > > --
> > > > 2.48.1
> > > >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] drm/amdgpu: add dce_v6_0_soft_reset() to DCE6
2025-02-27 19:05 ` Alex Deucher
@ 2025-02-27 19:11 ` Alexandre Demers
2025-02-28 14:32 ` Alex Deucher
0 siblings, 1 reply; 15+ messages in thread
From: Alexandre Demers @ 2025-02-27 19:11 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx
On Thu, Feb 27, 2025 at 2:05 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Thu, Feb 27, 2025 at 2:01 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > On Thu, Feb 27, 2025 at 1:52 PM Alexandre Demers
> > <alexandre.f.demers@gmail.com> wrote:
> > >
> > > On Thu, Feb 27, 2025 at 9:23 AM Alex Deucher <alexdeucher@gmail.com> wrote:
> > > >
> > > > On Thu, Feb 27, 2025 at 12:49 AM Alexandre Demers
> > > > <alexandre.f.demers@gmail.com> wrote:
> > > > >
> > > > > DCE6 was missing soft reset, but it was easily identifiable under radeon.
> > > > > This should be it, pretty much as it is done under DCE8 and DCE10.
> > > > >
> > > > > Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> > > > > ---
> > > > > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 62 ++++++++++++++++++++++++---
> > > > > 1 file changed, 57 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > > index bd763fde1c50..254cb73324c6 100644
> > > > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > > @@ -371,27 +371,58 @@ static u32 dce_v6_0_hpd_get_gpio_reg(struct amdgpu_device *adev)
> > > > > return mmDC_GPIO_HPD_A;
> > > > > }
> > > > >
> > > > > +static bool dce_v6_0_is_display_hung(struct amdgpu_device *adev)
> > > > > +{
> > > > > + u32 crtc_hung = 0;
> > > > > + u32 crtc_status[6];
> > > > > + u32 i, j, tmp;
> > > > > +
> > > > > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > > > > + if (RREG32(mmCRTC_CONTROL + crtc_offsets[i]) & CRTC_CONTROL__CRTC_MASTER_EN_MASK) {
> > > > > + crtc_status[i] = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > > > > + crtc_hung |= (1 << i);
> > > > > + }
> > > > > + }
> > > > > +
> > > > > + for (j = 0; j < 10; j++) {
> > > > > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > > > > + if (crtc_hung & (1 << i)) {
> > > > > + tmp = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > > > > + if (tmp != crtc_status[i])
> > > > > + crtc_hung &= ~(1 << i);
> > > > > + }
> > > > > + }
> > > > > + if (crtc_hung == 0)
> > > > > + return false;
> > > > > + udelay(100);
> > > > > + }
> > > > > +
> > > > > + return true;
> > > > > +}
> > > > > +
> > > > > static void dce_v6_0_set_vga_render_state(struct amdgpu_device *adev,
> > > > > bool render)
> > > > > {
> > > > > if (!render)
> > > > > WREG32(mmVGA_RENDER_CONTROL,
> > > > > RREG32(mmVGA_RENDER_CONTROL) & VGA_VSTATUS_CNTL);
> > > > > -
> > > > > }
> > > > >
> > > > > static int dce_v6_0_get_num_crtc(struct amdgpu_device *adev)
> > > > > {
> > > > > + int num_crtc = 0;
> > > > > +
> > > > > switch (adev->asic_type) {
> > > > > case CHIP_TAHITI:
> > > > > case CHIP_PITCAIRN:
> > > > > case CHIP_VERDE:
> > > > > - return 6;
> > > > > + num_crtc = 6;
> > > > > case CHIP_OLAND:
> > > > > - return 2;
> > > > > + num_crtc = 2;
> > > > > default:
> > > > > - return 0;
> > > > > + num_crtc = 0;
> > > > > }
> > > > > + return num_crtc;
> > > >
> > > > Any particular reason for this change? It just adds an extra variable.
> > > >
> > > > Alex
> > >
> > > Just for uniformisation with DCE8 and DCE10. We could also remove the
> > > variable and use returns everywhere.
> > >
> > > Any preferences?
> >
> > ah, ok. I think the direct returns are cleaner.
>
> I would maybe split up your patches into maybe 3 logical patch sets:
> one to fix spelling typos and comments, one to make the DCE code more
> uniform across versions, and another to add new DCE6 functionality.
>
> Alex
>
Ok, I'll split them and send new patch sets. Should they be identified as V2?
Alexandre
> >
> > Alex
> >
> > > Alexandre
> > >
> > > >
> > > > > }
> > > > >
> > > > > void dce_v6_0_disable_dce(struct amdgpu_device *adev)
> > > > > @@ -2846,7 +2877,28 @@ static bool dce_v6_0_is_idle(void *handle)
> > > > >
> > > > > static int dce_v6_0_soft_reset(struct amdgpu_ip_block *ip_block)
> > > > > {
> > > > > - DRM_INFO("xxxx: dce_v6_0_soft_reset --- no impl!!\n");
> > > > > + u32 srbm_soft_reset = 0, tmp;
> > > > > + struct amdgpu_device *adev = ip_block->adev;
> > > > > +
> > > > > + if (dce_v6_0_is_display_hung(adev))
> > > > > + srbm_soft_reset |= SRBM_SOFT_RESET__SOFT_RESET_DC_MASK;
> > > > > +
> > > > > + if (srbm_soft_reset) {
> > > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > > + tmp |= srbm_soft_reset;
> > > > > + dev_info(adev->dev, "SRBM_SOFT_RESET=0x%08X\n", tmp);
> > > > > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > > +
> > > > > + udelay(50);
> > > > > +
> > > > > + tmp &= ~srbm_soft_reset;
> > > > > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > > +
> > > > > + /* Wait a little for things to settle down */
> > > > > + udelay(50);
> > > > > + }
> > > > > return 0;
> > > > > }
> > > > >
> > > > > --
> > > > > 2.48.1
> > > > >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] drm/amdgpu: add dce_v6_0_soft_reset() to DCE6
2025-02-27 19:11 ` Alexandre Demers
@ 2025-02-28 14:32 ` Alex Deucher
0 siblings, 0 replies; 15+ messages in thread
From: Alex Deucher @ 2025-02-28 14:32 UTC (permalink / raw)
To: Alexandre Demers; +Cc: amd-gfx
On Thu, Feb 27, 2025 at 2:11 PM Alexandre Demers
<alexandre.f.demers@gmail.com> wrote:
>
> On Thu, Feb 27, 2025 at 2:05 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > On Thu, Feb 27, 2025 at 2:01 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> > >
> > > On Thu, Feb 27, 2025 at 1:52 PM Alexandre Demers
> > > <alexandre.f.demers@gmail.com> wrote:
> > > >
> > > > On Thu, Feb 27, 2025 at 9:23 AM Alex Deucher <alexdeucher@gmail.com> wrote:
> > > > >
> > > > > On Thu, Feb 27, 2025 at 12:49 AM Alexandre Demers
> > > > > <alexandre.f.demers@gmail.com> wrote:
> > > > > >
> > > > > > DCE6 was missing soft reset, but it was easily identifiable under radeon.
> > > > > > This should be it, pretty much as it is done under DCE8 and DCE10.
> > > > > >
> > > > > > Signed-off-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 62 ++++++++++++++++++++++++---
> > > > > > 1 file changed, 57 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > > > index bd763fde1c50..254cb73324c6 100644
> > > > > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > > > > > @@ -371,27 +371,58 @@ static u32 dce_v6_0_hpd_get_gpio_reg(struct amdgpu_device *adev)
> > > > > > return mmDC_GPIO_HPD_A;
> > > > > > }
> > > > > >
> > > > > > +static bool dce_v6_0_is_display_hung(struct amdgpu_device *adev)
> > > > > > +{
> > > > > > + u32 crtc_hung = 0;
> > > > > > + u32 crtc_status[6];
> > > > > > + u32 i, j, tmp;
> > > > > > +
> > > > > > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > > > > > + if (RREG32(mmCRTC_CONTROL + crtc_offsets[i]) & CRTC_CONTROL__CRTC_MASTER_EN_MASK) {
> > > > > > + crtc_status[i] = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > > > > > + crtc_hung |= (1 << i);
> > > > > > + }
> > > > > > + }
> > > > > > +
> > > > > > + for (j = 0; j < 10; j++) {
> > > > > > + for (i = 0; i < adev->mode_info.num_crtc; i++) {
> > > > > > + if (crtc_hung & (1 << i)) {
> > > > > > + tmp = RREG32(mmCRTC_STATUS_HV_COUNT + crtc_offsets[i]);
> > > > > > + if (tmp != crtc_status[i])
> > > > > > + crtc_hung &= ~(1 << i);
> > > > > > + }
> > > > > > + }
> > > > > > + if (crtc_hung == 0)
> > > > > > + return false;
> > > > > > + udelay(100);
> > > > > > + }
> > > > > > +
> > > > > > + return true;
> > > > > > +}
> > > > > > +
> > > > > > static void dce_v6_0_set_vga_render_state(struct amdgpu_device *adev,
> > > > > > bool render)
> > > > > > {
> > > > > > if (!render)
> > > > > > WREG32(mmVGA_RENDER_CONTROL,
> > > > > > RREG32(mmVGA_RENDER_CONTROL) & VGA_VSTATUS_CNTL);
> > > > > > -
> > > > > > }
> > > > > >
> > > > > > static int dce_v6_0_get_num_crtc(struct amdgpu_device *adev)
> > > > > > {
> > > > > > + int num_crtc = 0;
> > > > > > +
> > > > > > switch (adev->asic_type) {
> > > > > > case CHIP_TAHITI:
> > > > > > case CHIP_PITCAIRN:
> > > > > > case CHIP_VERDE:
> > > > > > - return 6;
> > > > > > + num_crtc = 6;
> > > > > > case CHIP_OLAND:
> > > > > > - return 2;
> > > > > > + num_crtc = 2;
> > > > > > default:
> > > > > > - return 0;
> > > > > > + num_crtc = 0;
> > > > > > }
> > > > > > + return num_crtc;
> > > > >
> > > > > Any particular reason for this change? It just adds an extra variable.
> > > > >
> > > > > Alex
> > > >
> > > > Just for uniformisation with DCE8 and DCE10. We could also remove the
> > > > variable and use returns everywhere.
> > > >
> > > > Any preferences?
> > >
> > > ah, ok. I think the direct returns are cleaner.
> >
> > I would maybe split up your patches into maybe 3 logical patch sets:
> > one to fix spelling typos and comments, one to make the DCE code more
> > uniform across versions, and another to add new DCE6 functionality.
> >
> > Alex
> >
> Ok, I'll split them and send new patch sets. Should they be identified as V2?
Yes, sounds good.
Thanks,
Alex
>
> Alexandre
> > >
> > > Alex
> > >
> > > > Alexandre
> > > >
> > > > >
> > > > > > }
> > > > > >
> > > > > > void dce_v6_0_disable_dce(struct amdgpu_device *adev)
> > > > > > @@ -2846,7 +2877,28 @@ static bool dce_v6_0_is_idle(void *handle)
> > > > > >
> > > > > > static int dce_v6_0_soft_reset(struct amdgpu_ip_block *ip_block)
> > > > > > {
> > > > > > - DRM_INFO("xxxx: dce_v6_0_soft_reset --- no impl!!\n");
> > > > > > + u32 srbm_soft_reset = 0, tmp;
> > > > > > + struct amdgpu_device *adev = ip_block->adev;
> > > > > > +
> > > > > > + if (dce_v6_0_is_display_hung(adev))
> > > > > > + srbm_soft_reset |= SRBM_SOFT_RESET__SOFT_RESET_DC_MASK;
> > > > > > +
> > > > > > + if (srbm_soft_reset) {
> > > > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > > > + tmp |= srbm_soft_reset;
> > > > > > + dev_info(adev->dev, "SRBM_SOFT_RESET=0x%08X\n", tmp);
> > > > > > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > > > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > > > +
> > > > > > + udelay(50);
> > > > > > +
> > > > > > + tmp &= ~srbm_soft_reset;
> > > > > > + WREG32(mmSRBM_SOFT_RESET, tmp);
> > > > > > + tmp = RREG32(mmSRBM_SOFT_RESET);
> > > > > > +
> > > > > > + /* Wait a little for things to settle down */
> > > > > > + udelay(50);
> > > > > > + }
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > --
> > > > > > 2.48.1
> > > > > >
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-02-28 14:32 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-27 5:22 [PATCH 0/6] add missing DCE6 functions and uniformize value definitions Alexandre Demers
2025-02-27 5:22 ` [PATCH 1/6] drm/amdgpu: add or move defines for DCE6 in sid.h Alexandre Demers
2025-02-27 5:22 ` [PATCH 2/6] drm/amdgpu: add dce_v6_0_soft_reset() to DCE6 Alexandre Demers
2025-02-27 14:23 ` Alex Deucher
2025-02-27 18:51 ` Alexandre Demers
2025-02-27 19:01 ` Alex Deucher
2025-02-27 19:05 ` Alex Deucher
2025-02-27 19:11 ` Alexandre Demers
2025-02-28 14:32 ` Alex Deucher
2025-02-27 19:09 ` Alexandre Demers
2025-02-27 5:22 ` [PATCH 3/6] drm/amdgpu: complete dce_v6_0_set_crtc_vline_interrupt_state() in DCE6 Alexandre Demers
2025-02-27 14:22 ` Alex Deucher
2025-02-27 5:22 ` [PATCH 4/6] drm/amdgpu: add some comments " Alexandre Demers
2025-02-27 5:22 ` [PATCH 5/6] dmr/amdgpu: fix style " Alexandre Demers
2025-02-27 5:22 ` [PATCH 6/6] drm/amdgpu: add defines for pin_offsets in DCE8 Alexandre Demers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox