[PATCH 00/15] drm/ast: Clean up access to MMIO registers

[PATCH 00/15] drm/ast: Clean up access to MMIO registers

[Thomas Zimmermann]

Aspeed devices up to Gen6 contain internal memory that is accessible
from the host via the P2A bridge. The helpers ast_moutdwm() and
ast_mindwm() pick a 64-KiB segment of the memory to expose to the
host, which then has the content available. Content is either registers
of the various IP blocks or on-board system memory.

Clean up the driver code involved and provide macro constants for the
registers used by the ast driver. Go through the driver and replace
the magic values with the constants.

The first patch of the series fixes a bug that has been found while
working on these changes. The DP501 writes the content of register
SCU2C to the wrong address. The fix is a separate patch to ease
backporting.

Smoke-tested on AST2100, AST2300, AST2500 and AST2600.

Thomas Zimmermann (15):
  drm/ast: dp501: Fix initialization of SCU2C
  drm/ast: Move 32-bit register-access helpers to ast_drv.{c,h}
  drm/ast: Use constants for AHBC registers
  drm/ast: Use constants for MCR registers
  drm/ast: Use constants for SCU registers
  drm/ast: Use constants for A2P registers
  drm/ast: Use constants for WDT registers
  drm/ast: Use constants for SDRAM registers
  drm/ast: Store register addresses in struct ast_dramstruct
  drm/ast: Gen1: Fix open-coded register access
  drm/ast: Gen2: Fix open-coded register access
  drm/ast: Gen4: Fix open-coded register access
  drm/ast: Gen6: Fix open-coded register access
  drm/ast: dp501: Fix open-coded register access
  drm/ast: Fix open-coded scu_rev access

 drivers/gpu/drm/ast/ast_2000.c  |  61 +++--
 drivers/gpu/drm/ast/ast_2100.c  | 292 +++++++++++------------
 drivers/gpu/drm/ast/ast_2300.c  | 398 ++++++++++++++++----------------
 drivers/gpu/drm/ast/ast_2500.c  | 319 ++++++++++++-------------
 drivers/gpu/drm/ast/ast_dp501.c | 146 ++++++------
 drivers/gpu/drm/ast/ast_drv.c   |  84 ++++++-
 drivers/gpu/drm/ast/ast_drv.h   |  60 +++--
 drivers/gpu/drm/ast/ast_post.c  |  48 +---
 drivers/gpu/drm/ast/ast_post.h  |  20 +-
 drivers/gpu/drm/ast/ast_reg.h   | 155 +++++++++++++
 10 files changed, 886 insertions(+), 697 deletions(-)


base-commit: 5cb7d14d9bd2061386a0192a4649626a3e3a5ec3
-- 
2.53.0

[PATCH 01/15] drm/ast: dp501: Fix initialization of SCU2C

[Thomas Zimmermann]

Ast's DP501 initialization reads the register SCU2C at offset 0x1202c
and tries to set it to source data from VGA. But writes the update to
offset 0x0, with unknown results. Write the result to SCU instead.

The bug only happens in ast_init_analog(). There's similar code in
ast_init_dvo(), which works correctly.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 83c6620bae3f ("drm/ast: initial DP501 support (v0.2)")
Cc: Dave Airlie <airlied@redhat.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Jocelyn Falempe <jfalempe@redhat.com>
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v3.16+
---
 drivers/gpu/drm/ast/ast_dp501.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 9e19d8c17730..677c52c0d99a 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -436,7 +436,7 @@ static void ast_init_analog(struct ast_device *ast)
 	/* Finally, clear bits [17:16] of SCU2c */
 	data = ast_read32(ast, 0x1202c);
 	data &= 0xfffcffff;
-	ast_write32(ast, 0, data);
+	ast_write32(ast, 0x1202c, data);
 
 	/* Disable DVO */
 	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xa3, 0xcf, 0x00);
-- 
2.53.0

Claude review: drm/ast: dp501: Fix initialization of SCU2C

[Claude Code Review Bot]

Patch Review

Good bugfix. The existing code at `ast_dp501.c:439` writes to address 0 instead of 0x1202c:
```c
-	ast_write32(ast, 0, data);
+	ast_write32(ast, 0x1202c, data);
```
This has been broken since the original commit `83c6620bae3f` (v3.16). Correctly tagged with Fixes and Cc: stable. Clean and obvious fix.

---
Generated by Claude Code Patch Reviewer

[PATCH 02/15] drm/ast: Move 32-bit register-access helpers to ast_drv.{c, h}

[Thomas Zimmermann]

The helpers ast_mindwm() and ast_moutdwm() access the I/O memory of
the various IP modules on the Aspeed device. This is based on the
"P-Bus to AHB Bridge" interface.

Reimplement the access function with properly defined constants and
helper macros.

- Define P2A constants for the related registers and addresses. The P2A
interface is located in the memory range at [0x00000000, 0x00010000].

- Memory access is segmented. An address' upper 16-bit select the
memory segment, the lower 16-bit select the offset within the segment.
Implement segment selection in a shared helper __ast_segsel(). Validate
that the segment hs been changes. This logic has previously been part
of __ast_moudwm() and __ast_mindwm(). Relax the CPU while busy-waiting.

- Put intra-segment reads and writes in the helpers __ast_rdseg32()
and __ast_wrseg32(). The helpers set the segment offset automatically.

- Reimplement the existing interfaces on put of these helpers.

Put the new implementation next to the other I/O helpers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_drv.c  | 59 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/ast/ast_drv.h  | 59 +++++++++++++++++++++-------------
 drivers/gpu/drm/ast/ast_post.c | 38 ----------------------
 drivers/gpu/drm/ast/ast_post.h |  3 --
 drivers/gpu/drm/ast/ast_reg.h  | 12 +++++++
 5 files changed, 108 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index b9a9b050b546..05ec3542ab62 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -47,6 +47,65 @@ static int ast_modeset = -1;
 MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
 module_param_named(modeset, ast_modeset, int, 0400);
 
+/*
+ * Register access
+ */
+
+/* Select R/W segment */
+static void __ast_selseg(void __iomem *regs, u32 r)
+{
+	u32 p2a04, p2a04_base;
+
+	p2a04 = r & AST_REG_P2A04_BASE_MASK;
+	__ast_write32(regs, AST_REG_P2A04, p2a04);
+	__ast_write32(regs, AST_REG_P2A00, AST_REG_P2A00_PROTECTION_KEY);
+
+	do {
+		cpu_relax();
+		p2a04_base = __ast_read32(regs, AST_REG_P2A04);
+		p2a04_base &= AST_REG_P2A04_BASE_MASK;
+	} while (p2a04_base != p2a04);
+}
+
+/* Read within segment */
+static u32 __ast_rdseg32(void __iomem *regs, u32 r)
+{
+	return __ast_read32(regs, AST_REG_P2A_ADDR(r));
+}
+
+/* Write within segment */
+static void __ast_wrseg32(void __iomem *regs, u32 r, u32 v)
+{
+	__ast_write32(regs, AST_REG_P2A_ADDR(r), v);
+}
+
+u32 __ast_mindwm(void __iomem *regs, u32 r)
+{
+	__ast_selseg(regs, r);
+
+	return __ast_rdseg32(regs, r);
+}
+
+void __ast_moutdwm(void __iomem *regs, u32 r, u32 v)
+{
+	__ast_selseg(regs, r);
+	__ast_wrseg32(regs, r, v);
+}
+
+u32 ast_mindwm(struct ast_device *ast, u32 r)
+{
+	return __ast_mindwm(ast->regs, r);
+}
+
+void ast_moutdwm(struct ast_device *ast, u32 r, u32 v)
+{
+	__ast_moutdwm(ast->regs, r, v);
+}
+
+/*
+ * AST device
+ */
+
 void ast_device_init(struct ast_device *ast,
 		     enum ast_chip chip,
 		     enum ast_config_mode config_mode,
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 787e38c6c17d..3eedf8239333 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -259,26 +259,20 @@ static inline bool __ast_gen_is_eq(struct ast_device *ast, unsigned long gen)
 #define IS_AST_GEN6(__ast)	__ast_gen_is_eq(__ast, 6)
 #define IS_AST_GEN7(__ast)	__ast_gen_is_eq(__ast, 7)
 
+/*
+ * MMIO access
+ */
+
 static inline u8 __ast_read8(const void __iomem *addr, u32 reg)
 {
 	return ioread8(addr + reg);
 }
 
-static inline u32 __ast_read32(const void __iomem *addr, u32 reg)
-{
-	return ioread32(addr + reg);
-}
-
 static inline void __ast_write8(void __iomem *addr, u32 reg, u8 val)
 {
 	iowrite8(val, addr + reg);
 }
 
-static inline void __ast_write32(void __iomem *addr, u32 reg, u32 val)
-{
-	iowrite32(val, addr + reg);
-}
-
 static inline u8 __ast_read8_i(void __iomem *addr, u32 reg, u8 index)
 {
 	__ast_write8(addr, reg, index);
@@ -307,16 +301,6 @@ static inline void __ast_write8_i_masked(void __iomem *addr, u32 reg, u8 index,
 	__ast_write8_i(addr, reg, index, tmp | val);
 }
 
-static inline u32 ast_read32(struct ast_device *ast, u32 reg)
-{
-	return __ast_read32(ast->regs, reg);
-}
-
-static inline void ast_write32(struct ast_device *ast, u32 reg, u32 val)
-{
-	__ast_write32(ast->regs, reg, val);
-}
-
 static inline u8 ast_io_read8(struct ast_device *ast, u32 reg)
 {
 	return __ast_read8(ast->ioregs, reg);
@@ -349,6 +333,39 @@ static inline void ast_set_index_reg_mask(struct ast_device *ast, u32 base, u8 i
 	__ast_write8_i_masked(ast->ioregs, base, index, preserve_mask, val);
 }
 
+/*
+ * Register access
+ */
+
+static inline u32 __ast_read32(const void __iomem *addr, u32 reg)
+{
+	return ioread32(addr + reg);
+}
+
+static inline void __ast_write32(void __iomem *addr, u32 reg, u32 val)
+{
+	iowrite32(val, addr + reg);
+}
+
+static inline u32 ast_read32(struct ast_device *ast, u32 reg)
+{
+	return __ast_read32(ast->regs, reg);
+}
+
+static inline void ast_write32(struct ast_device *ast, u32 reg, u32 val)
+{
+	__ast_write32(ast->regs, reg, val);
+}
+
+u32 __ast_mindwm(void __iomem *regs, u32 r);
+void __ast_moutdwm(void __iomem *regs, u32 r, u32 v);
+u32 ast_mindwm(struct ast_device *ast, u32 r);
+void ast_moutdwm(struct ast_device *ast, u32 r, u32 v);
+
+/*
+ * VBIOS
+ */
+
 struct ast_vbios_stdtable {
 	u8 misc;
 	u8 seq[4];
@@ -517,8 +534,6 @@ struct drm_device *ast_2600_device_create(struct pci_dev *pdev,
 
 /* ast post */
 int ast_post_gpu(struct ast_device *ast);
-u32 ast_mindwm(struct ast_device *ast, u32 r);
-void ast_moutdwm(struct ast_device *ast, u32 r, u32 v);
 
 int ast_vga_output_init(struct ast_device *ast);
 int ast_sil164_output_init(struct ast_device *ast);
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index b72914dbed38..5cec5d735b6a 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -34,44 +34,6 @@
 #include "ast_drv.h"
 #include "ast_post.h"
 
-u32 __ast_mindwm(void __iomem *regs, u32 r)
-{
-	u32 data;
-
-	__ast_write32(regs, 0xf004, r & 0xffff0000);
-	__ast_write32(regs, 0xf000, 0x1);
-
-	do {
-		data = __ast_read32(regs, 0xf004) & 0xffff0000;
-	} while (data != (r & 0xffff0000));
-
-	return __ast_read32(regs, 0x10000 + (r & 0x0000ffff));
-}
-
-void __ast_moutdwm(void __iomem *regs, u32 r, u32 v)
-{
-	u32 data;
-
-	__ast_write32(regs, 0xf004, r & 0xffff0000);
-	__ast_write32(regs, 0xf000, 0x1);
-
-	do {
-		data = __ast_read32(regs, 0xf004) & 0xffff0000;
-	} while (data != (r & 0xffff0000));
-
-	__ast_write32(regs, 0x10000 + (r & 0x0000ffff), v);
-}
-
-u32 ast_mindwm(struct ast_device *ast, u32 r)
-{
-	return __ast_mindwm(ast->regs, r);
-}
-
-void ast_moutdwm(struct ast_device *ast, u32 r, u32 v)
-{
-	__ast_moutdwm(ast->regs, r, v);
-}
-
 int ast_post_gpu(struct ast_device *ast)
 {
 	int ret;
diff --git a/drivers/gpu/drm/ast/ast_post.h b/drivers/gpu/drm/ast/ast_post.h
index aa5d247bebe8..41cd753b7f67 100644
--- a/drivers/gpu/drm/ast/ast_post.h
+++ b/drivers/gpu/drm/ast/ast_post.h
@@ -35,9 +35,6 @@ struct ast_dramstruct {
 #define AST_DRAMSTRUCT_IS(_entry, _name) \
 	((_entry)->index == __AST_DRAMSTRUCT_INDEX(_name))
 
-u32 __ast_mindwm(void __iomem *regs, u32 r);
-void __ast_moutdwm(void __iomem *regs, u32 r, u32 v);
-
 bool mmc_test(struct ast_device *ast, u32 datagen, u8 test_ctl);
 bool mmc_test_burst(struct ast_device *ast, u32 datagen);
 
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index 30578e3b07e4..ca9403efc7f9 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -75,4 +75,16 @@
 #define AST_IO_VGAIR1_R			(0x5A)
 #define AST_IO_VGAIR1_VREFRESH		BIT(3)
 
+/*
+ * P-Bus to AHB Bridge (0x00000000 - 0x0001ffff)
+ */
+
+#define AST_REG_P2A_BASE			(0x00000000)
+#define AST_REG_P2A(__offset)			(AST_REG_P2A_BASE + (__offset))
+#define AST_REG_P2A_ADDR(__addr)		AST_REG_P2A(0x10000 + ((__addr) & GENMASK(15, 0)))
+#define AST_REG_P2A00				AST_REG_P2A(0xf000)
+#define AST_REG_P2A00_PROTECTION_KEY		(0x01)
+#define AST_REG_P2A04				AST_REG_P2A(0xf004)
+#define AST_REG_P2A04_BASE_MASK			GENMASK(31, 16)
+
 #endif
-- 
2.53.0

Claude review: drm/ast: Move 32-bit register-access helpers to ast_drv.{c, h}

[Claude Code Review Bot]

Patch Review

Good refactoring. The reimplementation of `__ast_mindwm`/`__ast_moutdwm` with properly named sub-helpers (`__ast_selseg`, `__ast_rdseg32`, `__ast_wrseg32`) is cleaner and adds `cpu_relax()` to the busy-wait loop, which the original lacked.

One minor observation: the `__ast_selseg` helper calls `cpu_relax()` before the first read in the polling loop, whereas the original code did not relax at all. This is strictly better but moves from a tight spin to a relaxed spin.

The P2A register constants are well-defined:
```c
#define AST_REG_P2A00           AST_REG_P2A(0xf000)
#define AST_REG_P2A04           AST_REG_P2A(0xf004)
```

Note: The naming `P2A00` for offset `0xf000` is a bit confusing - the "00" suffix doesn't correspond to the offset. This follows the hardware register naming convention (P2A register 00), but a comment might help.

---
Generated by Claude Code Patch Reviewer

[PATCH 03/15] drm/ast: Use constants for AHBC registers

[Thomas Zimmermann]

AHBC is the Advanced High-Speed Bus Controller. AHBC registers are
located in the memory range within [0x1e600000, 0x1e61ffff]. Refer
to them with constants named AST_REG_AHBC<n>, where <n> is the byte
offset into the range.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2500.c |  6 +++---
 drivers/gpu/drm/ast/ast_reg.h  | 11 +++++++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2500.c b/drivers/gpu/drm/ast/ast_2500.c
index 2a52af0ded56..f751a27e7eb3 100644
--- a/drivers/gpu/drm/ast/ast_2500.c
+++ b/drivers/gpu/drm/ast/ast_2500.c
@@ -109,9 +109,9 @@ void ast_2500_patch_ahb(void __iomem *regs)
 	u32 data;
 
 	/* Clear bus lock condition */
-	__ast_moutdwm(regs, 0x1e600000, 0xAEED1A03);
-	__ast_moutdwm(regs, 0x1e600084, 0x00010000);
-	__ast_moutdwm(regs, 0x1e600088, 0x00000000);
+	__ast_moutdwm(regs, AST_REG_AHBC00, AST_REG_AHBC00_PROTECT_KEY);
+	__ast_moutdwm(regs, AST_REG_AHBC84, 0x00010000);
+	__ast_moutdwm(regs, AST_REG_AHBC88, 0x00000000);
 	__ast_moutdwm(regs, 0x1e6e2000, 0x1688A8A8);
 
 	data = __ast_mindwm(regs, 0x1e6e2070);
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index ca9403efc7f9..0bc033dbac55 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -87,4 +87,15 @@
 #define AST_REG_P2A04				AST_REG_P2A(0xf004)
 #define AST_REG_P2A04_BASE_MASK			GENMASK(31, 16)
 
+/*
+ * AHB Controller (0x1e600000 - 0x1e61ffff)
+ */
+
+#define AST_REG_AHBC_BASE			(0x1e600000)
+#define AST_REG_AHBC(__offset)			(AST_REG_AHBC_BASE + (__offset))
+#define AST_REG_AHBC00				AST_REG_AHBC(0x00)
+#define AST_REG_AHBC00_PROTECT_KEY		(0xaeed1a03)
+#define AST_REG_AHBC84				AST_REG_AHBC(0x84)
+#define AST_REG_AHBC88				AST_REG_AHBC(0x88)
+
 #endif
-- 
2.53.0

Claude review: drm/ast: Use constants for AHBC registers

[Claude Code Review Bot]

Patch Review

Straightforward. Introduces `AST_REG_AHBC*` constants for the AHB Controller and uses them in `ast_2500_patch_ahb()`. No issues.

---
Generated by Claude Code Patch Reviewer

[PATCH 04/15] drm/ast: Use constants for MCR registers

[Thomas Zimmermann]

SDRAM registers are located in the memory range at
[0x1e160000, 0x1e160fff]. Refer to them with constants named
AST_REG_MCR<n>, where n is the byte offset into the range.

Replacing the magic values in the ast driver was done with grep
and sed as shown below

  git grep -l \,\ 0x1e6e00 | xargs sed -i -e 's/, 0x1e6e00/, AST_REG_MCR/g'
  git grep -l \,\ 0x1E6E00 | xargs sed -i -e 's/, 0x1E6E00/, AST_REG_MCR/g'

plus some manual fixes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2000.c  |   2 +-
 drivers/gpu/drm/ast/ast_2100.c  |  30 +--
 drivers/gpu/drm/ast/ast_2300.c  | 330 ++++++++++++++++----------------
 drivers/gpu/drm/ast/ast_2500.c  | 224 +++++++++++-----------
 drivers/gpu/drm/ast/ast_dp501.c |   6 +-
 drivers/gpu/drm/ast/ast_drv.c   |   2 +-
 drivers/gpu/drm/ast/ast_post.c  |  10 +-
 drivers/gpu/drm/ast/ast_reg.h   |  71 +++++++
 8 files changed, 373 insertions(+), 302 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2000.c b/drivers/gpu/drm/ast/ast_2000.c
index fa3bc23ce098..204646bb1fa0 100644
--- a/drivers/gpu/drm/ast/ast_2000.c
+++ b/drivers/gpu/drm/ast/ast_2000.c
@@ -106,7 +106,7 @@ static void ast_post_chip_2000(struct ast_device *ast)
 
 	if ((j & 0x80) == 0) { /* VGA only */
 		dram_reg_info = ast2000_dram_table_data;
-		ast_write32(ast, 0xf004, 0x1e6e0000);
+		ast_write32(ast, 0xf004, AST_REG_MCR00);
 		ast_write32(ast, 0xf000, 0x1);
 		ast_write32(ast, 0x10100, 0xa8);
 
diff --git a/drivers/gpu/drm/ast/ast_2100.c b/drivers/gpu/drm/ast/ast_2100.c
index 05aeb0624d41..28cd36571b7f 100644
--- a/drivers/gpu/drm/ast/ast_2100.c
+++ b/drivers/gpu/drm/ast/ast_2100.c
@@ -43,7 +43,7 @@ static enum ast_dram_layout ast_2100_get_dram_layout_p2a(struct ast_device *ast)
 	u32 mcr_cfg;
 	enum ast_dram_layout dram_layout;
 
-	ast_write32(ast, 0xf004, 0x1e6e0000);
+	ast_write32(ast, 0xf004, AST_REG_MCR00);
 	ast_write32(ast, 0xf000, 0x1);
 	mcr_cfg = ast_read32(ast, 0x10004);
 
@@ -209,28 +209,28 @@ static u32 mmctestburst2_ast2150(struct ast_device *ast, u32 datagen)
 {
 	u32 data, timeout;
 
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000001 | (datagen << 3));
+	ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR70, 0x00000001 | (datagen << 3));
 	timeout = 0;
 	do {
-		data = ast_mindwm(ast, 0x1e6e0070) & 0x40;
+		data = ast_mindwm(ast, AST_REG_MCR70) & 0x40;
 		if (++timeout > TIMEOUT_AST2150) {
-			ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
+			ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
 			return 0xffffffff;
 		}
 	} while (!data);
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000003 | (datagen << 3));
+	ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR70, 0x00000003 | (datagen << 3));
 	timeout = 0;
 	do {
-		data = ast_mindwm(ast, 0x1e6e0070) & 0x40;
+		data = ast_mindwm(ast, AST_REG_MCR70) & 0x40;
 		if (++timeout > TIMEOUT_AST2150) {
-			ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
+			ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
 			return 0xffffffff;
 		}
 	} while (!data);
-	data = (ast_mindwm(ast, 0x1e6e0070) & 0x80) >> 7;
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
+	data = (ast_mindwm(ast, AST_REG_MCR70) & 0x80) >> 7;
+	ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
 	return data;
 }
 
@@ -249,7 +249,7 @@ static int cbrscan_ast2150(struct ast_device *ast, int busw)
 	u32 patcnt, loop;
 
 	for (patcnt = 0; patcnt < CBR_PATNUM_AST2150; patcnt++) {
-		ast_moutdwm(ast, 0x1e6e007c, pattern_AST2150[patcnt]);
+		ast_moutdwm(ast, AST_REG_MCR7C, pattern_AST2150[patcnt]);
 		for (loop = 0; loop < CBR_PASSNUM_AST2150; loop++) {
 			if (cbrtest_ast2150(ast))
 				break;
@@ -276,7 +276,7 @@ static void cbrdlli_ast2150(struct ast_device *ast, int busw)
 	passcnt = 0;
 
 	for (dlli = 0; dlli < 100; dlli++) {
-		ast_moutdwm(ast, 0x1e6e0068, dlli | (dlli << 8) | (dlli << 16) | (dlli << 24));
+		ast_moutdwm(ast, AST_REG_MCR68, dlli | (dlli << 8) | (dlli << 16) | (dlli << 24));
 		data = cbrscan_ast2150(ast, busw);
 		if (data != 0) {
 			if (data & 0x1) {
@@ -294,7 +294,7 @@ static void cbrdlli_ast2150(struct ast_device *ast, int busw)
 		goto cbr_start;
 
 	dlli = dll_min[0] + (((dll_max[0] - dll_min[0]) * 7) >> 4);
-	ast_moutdwm(ast, 0x1e6e0068, dlli | (dlli << 8) | (dlli << 16) | (dlli << 24));
+	ast_moutdwm(ast, AST_REG_MCR68, dlli | (dlli << 8) | (dlli << 16) | (dlli << 24));
 }
 
 static void ast_post_chip_2100(struct ast_device *ast)
@@ -312,7 +312,7 @@ static void ast_post_chip_2100(struct ast_device *ast)
 		else
 			dram_reg_info = ast1100_dram_table_data;
 
-		ast_write32(ast, 0xf004, 0x1e6e0000);
+		ast_write32(ast, 0xf004, AST_REG_MCR00);
 		ast_write32(ast, 0xf000, 0x1);
 		ast_write32(ast, 0x12000, 0x1688A8A8);
 		do {
diff --git a/drivers/gpu/drm/ast/ast_2300.c b/drivers/gpu/drm/ast/ast_2300.c
index 5f50d9f91ffd..56fe9e9f5c66 100644
--- a/drivers/gpu/drm/ast/ast_2300.c
+++ b/drivers/gpu/drm/ast/ast_2300.c
@@ -129,19 +129,19 @@ static u32 mmc_test2(struct ast_device *ast, u32 datagen, u8 test_ctl)
 {
 	u32 data, timeout;
 
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
-	ast_moutdwm(ast, 0x1e6e0070, (datagen << 3) | test_ctl);
+	ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR70, (datagen << 3) | test_ctl);
 	timeout = 0;
 	do {
-		data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
+		data = ast_mindwm(ast, AST_REG_MCR70) & 0x1000;
 		if (++timeout > TIMEOUT) {
-			ast_moutdwm(ast, 0x1e6e0070, 0x0);
+			ast_moutdwm(ast, AST_REG_MCR70, 0x0);
 			return 0xffffffff;
 		}
 	} while (!data);
-	data = ast_mindwm(ast, 0x1e6e0078);
+	data = ast_mindwm(ast, AST_REG_MCR78);
 	data = (data | (data >> 16)) & 0xffff;
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
 	return data;
 }
 
@@ -186,7 +186,7 @@ static int cbr_scan(struct ast_device *ast)
 
 	data2 = 3;
 	for (patcnt = 0; patcnt < CBR_PATNUM; patcnt++) {
-		ast_moutdwm(ast, 0x1e6e007c, pattern[patcnt]);
+		ast_moutdwm(ast, AST_REG_MCR7C, pattern[patcnt]);
 		for (loop = 0; loop < CBR_PASSNUM2; loop++) {
 			data = cbr_test(ast);
 			if (data != 0) {
@@ -222,7 +222,7 @@ static u32 cbr_scan2(struct ast_device *ast)
 
 	data2 = 0xffff;
 	for (patcnt = 0; patcnt < CBR_PATNUM; patcnt++) {
-		ast_moutdwm(ast, 0x1e6e007c, pattern[patcnt]);
+		ast_moutdwm(ast, AST_REG_MCR7C, pattern[patcnt]);
 		for (loop = 0; loop < CBR_PASSNUM2; loop++) {
 			data = cbr_test2(ast);
 			if (data != 0) {
@@ -252,7 +252,7 @@ static bool cbr_scan3(struct ast_device *ast)
 	u32 patcnt, loop;
 
 	for (patcnt = 0; patcnt < CBR_PATNUM; patcnt++) {
-		ast_moutdwm(ast, 0x1e6e007c, pattern[patcnt]);
+		ast_moutdwm(ast, AST_REG_MCR7C, pattern[patcnt]);
 		for (loop = 0; loop < 2; loop++) {
 			if (cbr_test3(ast))
 				break;
@@ -274,8 +274,8 @@ static bool finetuneDQI_L(struct ast_device *ast, struct ast2300_dram_param *par
 	}
 	passcnt = 0;
 	for (dlli = 0; dlli < 76; dlli++) {
-		ast_moutdwm(ast, 0x1E6E0068, 0x00001400 | (dlli << 16) | (dlli << 24));
-		ast_moutdwm(ast, 0x1E6E0074, CBR_SIZE1);
+		ast_moutdwm(ast, AST_REG_MCR68, 0x00001400 | (dlli << 16) | (dlli << 24));
+		ast_moutdwm(ast, AST_REG_MCR74, CBR_SIZE1);
 		data = cbr_scan2(ast);
 		if (data != 0) {
 			mask = 0x00010001;
@@ -330,7 +330,7 @@ static bool finetuneDQI_L(struct ast_device *ast, struct ast2300_dram_param *par
 			data |= dlli << 21;
 		}
 	}
-	ast_moutdwm(ast, 0x1E6E0080, data);
+	ast_moutdwm(ast, AST_REG_MCR80, data);
 
 	data = 0;
 	for (cnt = 8; cnt < 16; cnt++) {
@@ -354,7 +354,7 @@ static bool finetuneDQI_L(struct ast_device *ast, struct ast2300_dram_param *par
 			data |= dlli << 21;
 		}
 	}
-	ast_moutdwm(ast, 0x1E6E0084, data);
+	ast_moutdwm(ast, AST_REG_MCR84, data);
 	return status;
 } /* finetuneDQI_L */
 
@@ -367,10 +367,10 @@ static void finetuneDQSI(struct ast_device *ast)
 	char tag[2][76];
 
 	/* Disable DQI CBR */
-	reg_mcr0c  = ast_mindwm(ast, 0x1E6E000C);
-	reg_mcr18  = ast_mindwm(ast, 0x1E6E0018);
+	reg_mcr0c  = ast_mindwm(ast, AST_REG_MCR0C);
+	reg_mcr18  = ast_mindwm(ast, AST_REG_MCR18);
 	reg_mcr18 &= 0x0000ffff;
-	ast_moutdwm(ast, 0x1E6E0018, reg_mcr18);
+	ast_moutdwm(ast, AST_REG_MCR18, reg_mcr18);
 
 	for (dlli = 0; dlli < 76; dlli++) {
 		tag[0][dlli] = 0x0;
@@ -386,14 +386,14 @@ static void finetuneDQSI(struct ast_device *ast)
 		passcnt[0] = 0;
 		passcnt[1] = 0;
 		for (dqsip = 0; dqsip < 2; dqsip++) {
-			ast_moutdwm(ast, 0x1E6E000C, 0);
-			ast_moutdwm(ast, 0x1E6E0018, reg_mcr18 | (dqidly << 16) | (dqsip << 23));
-			ast_moutdwm(ast, 0x1E6E000C, reg_mcr0c);
+			ast_moutdwm(ast, AST_REG_MCR0C, 0);
+			ast_moutdwm(ast, AST_REG_MCR18, reg_mcr18 | (dqidly << 16) | (dqsip << 23));
+			ast_moutdwm(ast, AST_REG_MCR0C, reg_mcr0c);
 			for (dlli = 0; dlli < 76; dlli++) {
-				ast_moutdwm(ast, 0x1E6E0068,
+				ast_moutdwm(ast, AST_REG_MCR68,
 					    0x00001300 | (dlli << 16) | (dlli << 24));
-				ast_moutdwm(ast, 0x1E6E0070, 0);
-				ast_moutdwm(ast, 0x1E6E0074, CBR_SIZE0);
+				ast_moutdwm(ast, AST_REG_MCR70, 0);
+				ast_moutdwm(ast, AST_REG_MCR74, CBR_SIZE0);
 				if (cbr_scan3(ast)) {
 					if (dlli == 0)
 						break;
@@ -457,7 +457,7 @@ static void finetuneDQSI(struct ast_device *ast)
 		}
 	}
 	reg_mcr18 = reg_mcr18 | (g_dqidly << 16) | (g_dqsip << 23);
-	ast_moutdwm(ast, 0x1E6E0018, reg_mcr18);
+	ast_moutdwm(ast, AST_REG_MCR18, reg_mcr18);
 }
 
 static bool cbr_dll2(struct ast_device *ast, struct ast2300_dram_param *param)
@@ -476,8 +476,8 @@ static bool cbr_dll2(struct ast_device *ast, struct ast2300_dram_param *param)
 	dllmax[1] = 0x0;
 	passcnt = 0;
 	for (dlli = 0; dlli < 76; dlli++) {
-		ast_moutdwm(ast, 0x1E6E0068, 0x00001300 | (dlli << 16) | (dlli << 24));
-		ast_moutdwm(ast, 0x1E6E0074, CBR_SIZE2);
+		ast_moutdwm(ast, AST_REG_MCR68, 0x00001300 | (dlli << 16) | (dlli << 24));
+		ast_moutdwm(ast, AST_REG_MCR74, CBR_SIZE2);
 		data = cbr_scan(ast);
 		if (data != 0) {
 			if (data & 0x1) {
@@ -508,7 +508,7 @@ static bool cbr_dll2(struct ast_device *ast, struct ast2300_dram_param *param)
 	dlli  = (dllmin[1] + dllmax[1]) >> 1;
 	dlli <<= 8;
 	dlli += (dllmin[0] + dllmax[0]) >> 1;
-	ast_moutdwm(ast, 0x1E6E0068, ast_mindwm(ast, 0x1E720058) | (dlli << 16));
+	ast_moutdwm(ast, AST_REG_MCR68, ast_mindwm(ast, 0x1E720058) | (dlli << 16));
 	return status;
 } /* CBRDLL2 */
 
@@ -758,115 +758,115 @@ static void ddr3_init(struct ast_device *ast, struct ast2300_dram_param *param)
 	u32 data, data2, retry = 0;
 
 ddr3_init_start:
-	ast_moutdwm(ast, 0x1E6E0000, 0xFC600309);
-	ast_moutdwm(ast, 0x1E6E0018, 0x00000100);
-	ast_moutdwm(ast, 0x1E6E0024, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0034, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR00, AST_REG_MCR00_PROTECTION_KEY);
+	ast_moutdwm(ast, AST_REG_MCR18, 0x00000100);
+	ast_moutdwm(ast, AST_REG_MCR24, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR34, 0x00000000);
 	udelay(10);
-	ast_moutdwm(ast, 0x1E6E0064, param->reg_MADJ);
-	ast_moutdwm(ast, 0x1E6E0068, param->reg_SADJ);
+	ast_moutdwm(ast, AST_REG_MCR64, param->reg_MADJ);
+	ast_moutdwm(ast, AST_REG_MCR68, param->reg_SADJ);
 	udelay(10);
-	ast_moutdwm(ast, 0x1E6E0064, param->reg_MADJ | 0xC0000);
+	ast_moutdwm(ast, AST_REG_MCR64, param->reg_MADJ | 0xC0000);
 	udelay(10);
 
-	ast_moutdwm(ast, 0x1E6E0004, param->dram_config);
-	ast_moutdwm(ast, 0x1E6E0008, 0x90040f);
-	ast_moutdwm(ast, 0x1E6E0010, param->reg_AC1);
-	ast_moutdwm(ast, 0x1E6E0014, param->reg_AC2);
-	ast_moutdwm(ast, 0x1E6E0020, param->reg_DQSIC);
-	ast_moutdwm(ast, 0x1E6E0080, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0084, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0088, param->reg_DQIDLY);
-	ast_moutdwm(ast, 0x1E6E0018, 0x4000A170);
-	ast_moutdwm(ast, 0x1E6E0018, 0x00002370);
-	ast_moutdwm(ast, 0x1E6E0038, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0040, 0xFF444444);
-	ast_moutdwm(ast, 0x1E6E0044, 0x22222222);
-	ast_moutdwm(ast, 0x1E6E0048, 0x22222222);
-	ast_moutdwm(ast, 0x1E6E004C, 0x00000002);
-	ast_moutdwm(ast, 0x1E6E0050, 0x80000000);
-	ast_moutdwm(ast, 0x1E6E0050, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0054, 0);
-	ast_moutdwm(ast, 0x1E6E0060, param->reg_DRV);
-	ast_moutdwm(ast, 0x1E6E006C, param->reg_IOZ);
-	ast_moutdwm(ast, 0x1E6E0070, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0074, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0078, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E007C, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR04, param->dram_config);
+	ast_moutdwm(ast, AST_REG_MCR08, 0x90040f);
+	ast_moutdwm(ast, AST_REG_MCR10, param->reg_AC1);
+	ast_moutdwm(ast, AST_REG_MCR14, param->reg_AC2);
+	ast_moutdwm(ast, AST_REG_MCR20, param->reg_DQSIC);
+	ast_moutdwm(ast, AST_REG_MCR80, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR84, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR88, param->reg_DQIDLY);
+	ast_moutdwm(ast, AST_REG_MCR18, 0x4000A170);
+	ast_moutdwm(ast, AST_REG_MCR18, 0x00002370);
+	ast_moutdwm(ast, AST_REG_MCR38, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR40, 0xFF444444);
+	ast_moutdwm(ast, AST_REG_MCR44, 0x22222222);
+	ast_moutdwm(ast, AST_REG_MCR48, 0x22222222);
+	ast_moutdwm(ast, AST_REG_MCR4C, 0x00000002);
+	ast_moutdwm(ast, AST_REG_MCR50, 0x80000000);
+	ast_moutdwm(ast, AST_REG_MCR50, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR54, 0);
+	ast_moutdwm(ast, AST_REG_MCR60, param->reg_DRV);
+	ast_moutdwm(ast, AST_REG_MCR6C, param->reg_IOZ);
+	ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR74, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR78, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR7C, 0x00000000);
 	/* Wait MCLK2X lock to MCLK */
 	do {
-		data = ast_mindwm(ast, 0x1E6E001C);
+		data = ast_mindwm(ast, AST_REG_MCR1C);
 	} while (!(data & 0x08000000));
-	data = ast_mindwm(ast, 0x1E6E001C);
+	data = ast_mindwm(ast, AST_REG_MCR1C);
 	data = (data >> 8) & 0xff;
 	while ((data & 0x08) || ((data & 0x7) < 2) || (data < 4)) {
-		data2 = (ast_mindwm(ast, 0x1E6E0064) & 0xfff3ffff) + 4;
+		data2 = (ast_mindwm(ast, AST_REG_MCR64) & 0xfff3ffff) + 4;
 		if ((data2 & 0xff) > param->madj_max)
 			break;
-		ast_moutdwm(ast, 0x1E6E0064, data2);
+		ast_moutdwm(ast, AST_REG_MCR64, data2);
 		if (data2 & 0x00100000)
 			data2 = ((data2 & 0xff) >> 3) + 3;
 		else
 			data2 = ((data2 & 0xff) >> 2) + 5;
-		data = ast_mindwm(ast, 0x1E6E0068) & 0xffff00ff;
+		data = ast_mindwm(ast, AST_REG_MCR68) & 0xffff00ff;
 		data2 += data & 0xff;
 		data = data | (data2 << 8);
-		ast_moutdwm(ast, 0x1E6E0068, data);
+		ast_moutdwm(ast, AST_REG_MCR68, data);
 		udelay(10);
-		ast_moutdwm(ast, 0x1E6E0064, ast_mindwm(ast, 0x1E6E0064) | 0xC0000);
+		ast_moutdwm(ast, AST_REG_MCR64, ast_mindwm(ast, AST_REG_MCR64) | 0xC0000);
 		udelay(10);
-		data = ast_mindwm(ast, 0x1E6E0018) & 0xfffff1ff;
-		ast_moutdwm(ast, 0x1E6E0018, data);
+		data = ast_mindwm(ast, AST_REG_MCR18) & 0xfffff1ff;
+		ast_moutdwm(ast, AST_REG_MCR18, data);
 		data = data | 0x200;
-		ast_moutdwm(ast, 0x1E6E0018, data);
+		ast_moutdwm(ast, AST_REG_MCR18, data);
 		do {
-			data = ast_mindwm(ast, 0x1E6E001C);
+			data = ast_mindwm(ast, AST_REG_MCR1C);
 		} while (!(data & 0x08000000));
 
-		data = ast_mindwm(ast, 0x1E6E001C);
+		data = ast_mindwm(ast, AST_REG_MCR1C);
 		data = (data >> 8) & 0xff;
 	}
-	ast_moutdwm(ast, 0x1E720058, ast_mindwm(ast, 0x1E6E0068) & 0xffff);
-	data = ast_mindwm(ast, 0x1E6E0018) | 0xC00;
-	ast_moutdwm(ast, 0x1E6E0018, data);
+	ast_moutdwm(ast, 0x1E720058, ast_mindwm(ast, AST_REG_MCR68) & 0xffff);
+	data = ast_mindwm(ast, AST_REG_MCR18) | 0xC00;
+	ast_moutdwm(ast, AST_REG_MCR18, data);
 
-	ast_moutdwm(ast, 0x1E6E0034, 0x00000001);
-	ast_moutdwm(ast, 0x1E6E000C, 0x00000040);
+	ast_moutdwm(ast, AST_REG_MCR34, 0x00000001);
+	ast_moutdwm(ast, AST_REG_MCR0C, 0x00000040);
 	udelay(50);
 	/* Mode Register Setting */
-	ast_moutdwm(ast, 0x1E6E002C, param->reg_MRS | 0x100);
-	ast_moutdwm(ast, 0x1E6E0030, param->reg_EMRS);
-	ast_moutdwm(ast, 0x1E6E0028, 0x00000005);
-	ast_moutdwm(ast, 0x1E6E0028, 0x00000007);
-	ast_moutdwm(ast, 0x1E6E0028, 0x00000003);
-	ast_moutdwm(ast, 0x1E6E0028, 0x00000001);
-	ast_moutdwm(ast, 0x1E6E002C, param->reg_MRS);
-	ast_moutdwm(ast, 0x1E6E000C, 0x00005C08);
-	ast_moutdwm(ast, 0x1E6E0028, 0x00000001);
-
-	ast_moutdwm(ast, 0x1E6E000C, 0x00005C01);
+	ast_moutdwm(ast, AST_REG_MCR2C, param->reg_MRS | 0x100);
+	ast_moutdwm(ast, AST_REG_MCR30, param->reg_EMRS);
+	ast_moutdwm(ast, AST_REG_MCR28, 0x00000005);
+	ast_moutdwm(ast, AST_REG_MCR28, 0x00000007);
+	ast_moutdwm(ast, AST_REG_MCR28, 0x00000003);
+	ast_moutdwm(ast, AST_REG_MCR28, 0x00000001);
+	ast_moutdwm(ast, AST_REG_MCR2C, param->reg_MRS);
+	ast_moutdwm(ast, AST_REG_MCR0C, 0x00005C08);
+	ast_moutdwm(ast, AST_REG_MCR28, 0x00000001);
+
+	ast_moutdwm(ast, AST_REG_MCR0C, 0x00005C01);
 	data = 0;
 	if (param->wodt)
 		data = 0x300;
 	if (param->rodt)
 		data = data | 0x3000 | ((param->reg_AC2 & 0x60000) >> 3);
-	ast_moutdwm(ast, 0x1E6E0034, data | 0x3);
+	ast_moutdwm(ast, AST_REG_MCR34, data | 0x3);
 
 	/* Calibrate the DQSI delay */
 	if ((cbr_dll2(ast, param) == false) && (retry++ < 10))
 		goto ddr3_init_start;
 
-	ast_moutdwm(ast, 0x1E6E0120, param->reg_FREQ);
+	ast_moutdwm(ast, AST_REG_MCR120, param->reg_FREQ);
 	/* ECC Memory Initialization */
 #ifdef ECC
-	ast_moutdwm(ast, 0x1E6E007C, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0070, 0x221);
+	ast_moutdwm(ast, AST_REG_MCR7C, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR70, 0x221);
 	do {
-		data = ast_mindwm(ast, 0x1E6E0070);
+		data = ast_mindwm(ast, AST_REG_MCR70);
 	} while (!(data & 0x00001000));
-	ast_moutdwm(ast, 0x1E6E0070, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0050, 0x80000000);
-	ast_moutdwm(ast, 0x1E6E0050, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR50, 0x80000000);
+	ast_moutdwm(ast, AST_REG_MCR50, 0x00000000);
 #endif
 }
 
@@ -1121,104 +1121,104 @@ static void ddr2_init(struct ast_device *ast, struct ast2300_dram_param *param)
 	u32 data, data2, retry = 0;
 
 ddr2_init_start:
-	ast_moutdwm(ast, 0x1E6E0000, 0xFC600309);
-	ast_moutdwm(ast, 0x1E6E0018, 0x00000100);
-	ast_moutdwm(ast, 0x1E6E0024, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0064, param->reg_MADJ);
-	ast_moutdwm(ast, 0x1E6E0068, param->reg_SADJ);
+	ast_moutdwm(ast, AST_REG_MCR00, AST_REG_MCR00_PROTECTION_KEY);
+	ast_moutdwm(ast, AST_REG_MCR18, 0x00000100);
+	ast_moutdwm(ast, AST_REG_MCR24, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR64, param->reg_MADJ);
+	ast_moutdwm(ast, AST_REG_MCR68, param->reg_SADJ);
 	udelay(10);
-	ast_moutdwm(ast, 0x1E6E0064, param->reg_MADJ | 0xC0000);
+	ast_moutdwm(ast, AST_REG_MCR64, param->reg_MADJ | 0xC0000);
 	udelay(10);
 
-	ast_moutdwm(ast, 0x1E6E0004, param->dram_config);
-	ast_moutdwm(ast, 0x1E6E0008, 0x90040f);
-	ast_moutdwm(ast, 0x1E6E0010, param->reg_AC1);
-	ast_moutdwm(ast, 0x1E6E0014, param->reg_AC2);
-	ast_moutdwm(ast, 0x1E6E0020, param->reg_DQSIC);
-	ast_moutdwm(ast, 0x1E6E0080, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0084, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0088, param->reg_DQIDLY);
-	ast_moutdwm(ast, 0x1E6E0018, 0x4000A130);
-	ast_moutdwm(ast, 0x1E6E0018, 0x00002330);
-	ast_moutdwm(ast, 0x1E6E0038, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0040, 0xFF808000);
-	ast_moutdwm(ast, 0x1E6E0044, 0x88848466);
-	ast_moutdwm(ast, 0x1E6E0048, 0x44440008);
-	ast_moutdwm(ast, 0x1E6E004C, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0050, 0x80000000);
-	ast_moutdwm(ast, 0x1E6E0050, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0054, 0);
-	ast_moutdwm(ast, 0x1E6E0060, param->reg_DRV);
-	ast_moutdwm(ast, 0x1E6E006C, param->reg_IOZ);
-	ast_moutdwm(ast, 0x1E6E0070, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0074, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0078, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E007C, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR04, param->dram_config);
+	ast_moutdwm(ast, AST_REG_MCR08, 0x90040f);
+	ast_moutdwm(ast, AST_REG_MCR10, param->reg_AC1);
+	ast_moutdwm(ast, AST_REG_MCR14, param->reg_AC2);
+	ast_moutdwm(ast, AST_REG_MCR20, param->reg_DQSIC);
+	ast_moutdwm(ast, AST_REG_MCR80, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR84, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR88, param->reg_DQIDLY);
+	ast_moutdwm(ast, AST_REG_MCR18, 0x4000A130);
+	ast_moutdwm(ast, AST_REG_MCR18, 0x00002330);
+	ast_moutdwm(ast, AST_REG_MCR38, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR40, 0xFF808000);
+	ast_moutdwm(ast, AST_REG_MCR44, 0x88848466);
+	ast_moutdwm(ast, AST_REG_MCR48, 0x44440008);
+	ast_moutdwm(ast, AST_REG_MCR4C, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR50, 0x80000000);
+	ast_moutdwm(ast, AST_REG_MCR50, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR54, 0);
+	ast_moutdwm(ast, AST_REG_MCR60, param->reg_DRV);
+	ast_moutdwm(ast, AST_REG_MCR6C, param->reg_IOZ);
+	ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR74, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR78, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR7C, 0x00000000);
 
 	/* Wait MCLK2X lock to MCLK */
 	do {
-		data = ast_mindwm(ast, 0x1E6E001C);
+		data = ast_mindwm(ast, AST_REG_MCR1C);
 	} while (!(data & 0x08000000));
-	data = ast_mindwm(ast, 0x1E6E001C);
+	data = ast_mindwm(ast, AST_REG_MCR1C);
 	data = (data >> 8) & 0xff;
 	while ((data & 0x08) || ((data & 0x7) < 2) || (data < 4)) {
-		data2 = (ast_mindwm(ast, 0x1E6E0064) & 0xfff3ffff) + 4;
+		data2 = (ast_mindwm(ast, AST_REG_MCR64) & 0xfff3ffff) + 4;
 		if ((data2 & 0xff) > param->madj_max)
 			break;
-		ast_moutdwm(ast, 0x1E6E0064, data2);
+		ast_moutdwm(ast, AST_REG_MCR64, data2);
 		if (data2 & 0x00100000)
 			data2 = ((data2 & 0xff) >> 3) + 3;
 		else
 			data2 = ((data2 & 0xff) >> 2) + 5;
-		data = ast_mindwm(ast, 0x1E6E0068) & 0xffff00ff;
+		data = ast_mindwm(ast, AST_REG_MCR68) & 0xffff00ff;
 		data2 += data & 0xff;
 		data = data | (data2 << 8);
-		ast_moutdwm(ast, 0x1E6E0068, data);
+		ast_moutdwm(ast, AST_REG_MCR68, data);
 		udelay(10);
-		ast_moutdwm(ast, 0x1E6E0064, ast_mindwm(ast, 0x1E6E0064) | 0xC0000);
+		ast_moutdwm(ast, AST_REG_MCR64, ast_mindwm(ast, AST_REG_MCR64) | 0xC0000);
 		udelay(10);
-		data = ast_mindwm(ast, 0x1E6E0018) & 0xfffff1ff;
-		ast_moutdwm(ast, 0x1E6E0018, data);
+		data = ast_mindwm(ast, AST_REG_MCR18) & 0xfffff1ff;
+		ast_moutdwm(ast, AST_REG_MCR18, data);
 		data = data | 0x200;
-		ast_moutdwm(ast, 0x1E6E0018, data);
+		ast_moutdwm(ast, AST_REG_MCR18, data);
 		do {
-			data = ast_mindwm(ast, 0x1E6E001C);
+			data = ast_mindwm(ast, AST_REG_MCR1C);
 		} while (!(data & 0x08000000));
 
-		data = ast_mindwm(ast, 0x1E6E001C);
+		data = ast_mindwm(ast, AST_REG_MCR1C);
 		data = (data >> 8) & 0xff;
 	}
-	ast_moutdwm(ast, 0x1E720058, ast_mindwm(ast, 0x1E6E0008) & 0xffff);
-	data = ast_mindwm(ast, 0x1E6E0018) | 0xC00;
-	ast_moutdwm(ast, 0x1E6E0018, data);
+	ast_moutdwm(ast, 0x1E720058, ast_mindwm(ast, AST_REG_MCR08) & 0xffff);
+	data = ast_mindwm(ast, AST_REG_MCR18) | 0xC00;
+	ast_moutdwm(ast, AST_REG_MCR18, data);
 
-	ast_moutdwm(ast, 0x1E6E0034, 0x00000001);
-	ast_moutdwm(ast, 0x1E6E000C, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR34, 0x00000001);
+	ast_moutdwm(ast, AST_REG_MCR0C, 0x00000000);
 	udelay(50);
 	/* Mode Register Setting */
-	ast_moutdwm(ast, 0x1E6E002C, param->reg_MRS | 0x100);
-	ast_moutdwm(ast, 0x1E6E0030, param->reg_EMRS);
-	ast_moutdwm(ast, 0x1E6E0028, 0x00000005);
-	ast_moutdwm(ast, 0x1E6E0028, 0x00000007);
-	ast_moutdwm(ast, 0x1E6E0028, 0x00000003);
-	ast_moutdwm(ast, 0x1E6E0028, 0x00000001);
-
-	ast_moutdwm(ast, 0x1E6E000C, 0x00005C08);
-	ast_moutdwm(ast, 0x1E6E002C, param->reg_MRS);
-	ast_moutdwm(ast, 0x1E6E0028, 0x00000001);
-	ast_moutdwm(ast, 0x1E6E0030, param->reg_EMRS | 0x380);
-	ast_moutdwm(ast, 0x1E6E0028, 0x00000003);
-	ast_moutdwm(ast, 0x1E6E0030, param->reg_EMRS);
-	ast_moutdwm(ast, 0x1E6E0028, 0x00000003);
-
-	ast_moutdwm(ast, 0x1E6E000C, 0x7FFF5C01);
+	ast_moutdwm(ast, AST_REG_MCR2C, param->reg_MRS | 0x100);
+	ast_moutdwm(ast, AST_REG_MCR30, param->reg_EMRS);
+	ast_moutdwm(ast, AST_REG_MCR28, 0x00000005);
+	ast_moutdwm(ast, AST_REG_MCR28, 0x00000007);
+	ast_moutdwm(ast, AST_REG_MCR28, 0x00000003);
+	ast_moutdwm(ast, AST_REG_MCR28, 0x00000001);
+
+	ast_moutdwm(ast, AST_REG_MCR0C, 0x00005C08);
+	ast_moutdwm(ast, AST_REG_MCR2C, param->reg_MRS);
+	ast_moutdwm(ast, AST_REG_MCR28, 0x00000001);
+	ast_moutdwm(ast, AST_REG_MCR30, param->reg_EMRS | 0x380);
+	ast_moutdwm(ast, AST_REG_MCR28, 0x00000003);
+	ast_moutdwm(ast, AST_REG_MCR30, param->reg_EMRS);
+	ast_moutdwm(ast, AST_REG_MCR28, 0x00000003);
+
+	ast_moutdwm(ast, AST_REG_MCR0C, 0x7FFF5C01);
 	data = 0;
 	if (param->wodt)
 		data = 0x500;
 	if (param->rodt)
 		data = data | 0x3000 | ((param->reg_AC2 & 0x60000) >> 3);
-	ast_moutdwm(ast, 0x1E6E0034, data | 0x3);
-	ast_moutdwm(ast, 0x1E6E0120, param->reg_FREQ);
+	ast_moutdwm(ast, AST_REG_MCR34, data | 0x3);
+	ast_moutdwm(ast, AST_REG_MCR120, param->reg_FREQ);
 
 	/* Calibrate the DQSI delay */
 	if ((cbr_dll2(ast, param) == false) && (retry++ < 10))
@@ -1226,14 +1226,14 @@ static void ddr2_init(struct ast_device *ast, struct ast2300_dram_param *param)
 
 	/* ECC Memory Initialization */
 #ifdef ECC
-	ast_moutdwm(ast, 0x1E6E007C, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0070, 0x221);
+	ast_moutdwm(ast, AST_REG_MCR7C, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR70, 0x221);
 	do {
-		data = ast_mindwm(ast, 0x1E6E0070);
+		data = ast_mindwm(ast, AST_REG_MCR70);
 	} while (!(data & 0x00001000));
-	ast_moutdwm(ast, 0x1E6E0070, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0050, 0x80000000);
-	ast_moutdwm(ast, 0x1E6E0050, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR50, 0x80000000);
+	ast_moutdwm(ast, AST_REG_MCR50, 0x00000000);
 #endif
 }
 
@@ -1245,7 +1245,7 @@ static void ast_post_chip_2300(struct ast_device *ast)
 
 	reg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd0, 0xff);
 	if ((reg & 0x80) == 0) {/* vga only */
-		ast_write32(ast, 0xf004, 0x1e6e0000);
+		ast_write32(ast, 0xf004, AST_REG_MCR00);
 		ast_write32(ast, 0xf000, 0x1);
 		ast_write32(ast, 0x12000, 0x1688a8a8);
 		do {
diff --git a/drivers/gpu/drm/ast/ast_2500.c b/drivers/gpu/drm/ast/ast_2500.c
index f751a27e7eb3..4a9df920509f 100644
--- a/drivers/gpu/drm/ast/ast_2500.c
+++ b/drivers/gpu/drm/ast/ast_2500.c
@@ -148,8 +148,8 @@ static bool mmc_test_single_2500(struct ast_device *ast, u32 datagen)
 
 static bool cbr_test_2500(struct ast_device *ast)
 {
-	ast_moutdwm(ast, 0x1E6E0074, 0x0000FFFF);
-	ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
+	ast_moutdwm(ast, AST_REG_MCR74, 0x0000FFFF);
+	ast_moutdwm(ast, AST_REG_MCR7C, 0xFF00FF00);
 	if (!mmc_test_burst(ast, 0))
 		return false;
 	if (!mmc_test_single_2500(ast, 0))
@@ -159,8 +159,8 @@ static bool cbr_test_2500(struct ast_device *ast)
 
 static bool ddr_test_2500(struct ast_device *ast)
 {
-	ast_moutdwm(ast, 0x1E6E0074, 0x0000FFFF);
-	ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
+	ast_moutdwm(ast, AST_REG_MCR74, 0x0000FFFF);
+	ast_moutdwm(ast, AST_REG_MCR7C, 0xFF00FF00);
 	if (!mmc_test_burst(ast, 0))
 		return false;
 	if (!mmc_test_burst(ast, 1))
@@ -176,25 +176,25 @@ static bool ddr_test_2500(struct ast_device *ast)
 
 static void ddr_init_common_2500(struct ast_device *ast)
 {
-	ast_moutdwm(ast, 0x1E6E0034, 0x00020080);
-	ast_moutdwm(ast, 0x1E6E0008, 0x2003000F);
-	ast_moutdwm(ast, 0x1E6E0038, 0x00000FFF);
-	ast_moutdwm(ast, 0x1E6E0040, 0x88448844);
-	ast_moutdwm(ast, 0x1E6E0044, 0x24422288);
-	ast_moutdwm(ast, 0x1E6E0048, 0x22222222);
-	ast_moutdwm(ast, 0x1E6E004C, 0x22222222);
-	ast_moutdwm(ast, 0x1E6E0050, 0x80000000);
-	ast_moutdwm(ast, 0x1E6E0208, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0218, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0220, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0228, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0230, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E02A8, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E02B0, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0240, 0x86000000);
-	ast_moutdwm(ast, 0x1E6E0244, 0x00008600);
-	ast_moutdwm(ast, 0x1E6E0248, 0x80000000);
-	ast_moutdwm(ast, 0x1E6E024C, 0x80808080);
+	ast_moutdwm(ast, AST_REG_MCR34, 0x00020080);
+	ast_moutdwm(ast, AST_REG_MCR08, 0x2003000F);
+	ast_moutdwm(ast, AST_REG_MCR38, 0x00000FFF);
+	ast_moutdwm(ast, AST_REG_MCR40, 0x88448844);
+	ast_moutdwm(ast, AST_REG_MCR44, 0x24422288);
+	ast_moutdwm(ast, AST_REG_MCR48, 0x22222222);
+	ast_moutdwm(ast, AST_REG_MCR4C, 0x22222222);
+	ast_moutdwm(ast, AST_REG_MCR50, 0x80000000);
+	ast_moutdwm(ast, AST_REG_MCR208, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR218, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR220, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR228, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR230, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR2A8, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR2B0, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR240, 0x86000000);
+	ast_moutdwm(ast, AST_REG_MCR244, 0x00008600);
+	ast_moutdwm(ast, AST_REG_MCR248, 0x80000000);
+	ast_moutdwm(ast, AST_REG_MCR24C, 0x80808080);
 }
 
 static void ddr_phy_init_2500(struct ast_device *ast)
@@ -202,26 +202,26 @@ static void ddr_phy_init_2500(struct ast_device *ast)
 	u32 data, pass, timecnt;
 
 	pass = 0;
-	ast_moutdwm(ast, 0x1E6E0060, 0x00000005);
+	ast_moutdwm(ast, AST_REG_MCR60, 0x00000005);
 	while (!pass) {
 		for (timecnt = 0; timecnt < TIMEOUT; timecnt++) {
-			data = ast_mindwm(ast, 0x1E6E0060) & 0x1;
+			data = ast_mindwm(ast, AST_REG_MCR60) & 0x1;
 			if (!data)
 				break;
 		}
 		if (timecnt != TIMEOUT) {
-			data = ast_mindwm(ast, 0x1E6E0300) & 0x000A0000;
+			data = ast_mindwm(ast, AST_REG_MCR300) & 0x000A0000;
 			if (!data)
 				pass = 1;
 		}
 		if (!pass) {
-			ast_moutdwm(ast, 0x1E6E0060, 0x00000000);
+			ast_moutdwm(ast, AST_REG_MCR60, 0x00000000);
 			udelay(10); /* delay 10 us */
-			ast_moutdwm(ast, 0x1E6E0060, 0x00000005);
+			ast_moutdwm(ast, AST_REG_MCR60, 0x00000005);
 		}
 	}
 
-	ast_moutdwm(ast, 0x1E6E0060, 0x00000006);
+	ast_moutdwm(ast, AST_REG_MCR60, 0x00000006);
 }
 
 /*
@@ -235,8 +235,8 @@ static void check_dram_size_2500(struct ast_device *ast, u32 tRFC)
 {
 	u32 reg_04, reg_14;
 
-	reg_04 = ast_mindwm(ast, 0x1E6E0004) & 0xfffffffc;
-	reg_14 = ast_mindwm(ast, 0x1E6E0014) & 0xffffff00;
+	reg_04 = ast_mindwm(ast, AST_REG_MCR04) & 0xfffffffc;
+	reg_14 = ast_mindwm(ast, AST_REG_MCR14) & 0xffffff00;
 
 	ast_moutdwm(ast, 0xA0100000, 0x41424344);
 	ast_moutdwm(ast, 0x90100000, 0x35363738);
@@ -258,21 +258,21 @@ static void check_dram_size_2500(struct ast_device *ast, u32 tRFC)
 	} else {
 		reg_14 |= tRFC & 0xFF;
 	}
-	ast_moutdwm(ast, 0x1E6E0004, reg_04);
-	ast_moutdwm(ast, 0x1E6E0014, reg_14);
+	ast_moutdwm(ast, AST_REG_MCR04, reg_04);
+	ast_moutdwm(ast, AST_REG_MCR14, reg_14);
 }
 
 static void enable_cache_2500(struct ast_device *ast)
 {
 	u32 reg_04, data;
 
-	reg_04 = ast_mindwm(ast, 0x1E6E0004);
-	ast_moutdwm(ast, 0x1E6E0004, reg_04 | 0x1000);
+	reg_04 = ast_mindwm(ast, AST_REG_MCR04);
+	ast_moutdwm(ast, AST_REG_MCR04, reg_04 | 0x1000);
 
 	do
-		data = ast_mindwm(ast, 0x1E6E0004);
+		data = ast_mindwm(ast, AST_REG_MCR04);
 	while (!(data & 0x80000));
-	ast_moutdwm(ast, 0x1E6E0004, reg_04 | 0x400);
+	ast_moutdwm(ast, AST_REG_MCR04, reg_04 | 0x400);
 }
 
 static void set_mpll_2500(struct ast_device *ast)
@@ -280,13 +280,13 @@ static void set_mpll_2500(struct ast_device *ast)
 	u32 addr, data, param;
 
 	/* Reset MMC */
-	ast_moutdwm(ast, 0x1E6E0000, 0xFC600309);
-	ast_moutdwm(ast, 0x1E6E0034, 0x00020080);
+	ast_moutdwm(ast, AST_REG_MCR00, AST_REG_MCR00_PROTECTION_KEY);
+	ast_moutdwm(ast, AST_REG_MCR34, 0x00020080);
 	for (addr = 0x1e6e0004; addr < 0x1e6e0090;) {
 		ast_moutdwm(ast, addr, 0x0);
 		addr += 4;
 	}
-	ast_moutdwm(ast, 0x1E6E0034, 0x00020000);
+	ast_moutdwm(ast, AST_REG_MCR34, 0x00020000);
 
 	ast_moutdwm(ast, 0x1E6E2000, 0x1688A8A8);
 	data = ast_mindwm(ast, 0x1E6E2070) & 0x00800000;
@@ -310,50 +310,50 @@ static void reset_mmc_2500(struct ast_device *ast)
 	ast_moutdwm(ast, 0x1E78504C, 0x00000013);
 	mdelay(100);
 	ast_moutdwm(ast, 0x1E785054, 0x00000077);
-	ast_moutdwm(ast, 0x1E6E0000, 0xFC600309);
+	ast_moutdwm(ast, AST_REG_MCR00, AST_REG_MCR00_PROTECTION_KEY);
 }
 
 static void ddr3_init_2500(struct ast_device *ast, const u32 *ddr_table)
 {
-	ast_moutdwm(ast, 0x1E6E0004, 0x00000303);
-	ast_moutdwm(ast, 0x1E6E0010, ddr_table[REGIDX_010]);
-	ast_moutdwm(ast, 0x1E6E0014, ddr_table[REGIDX_014]);
-	ast_moutdwm(ast, 0x1E6E0018, ddr_table[REGIDX_018]);
-	ast_moutdwm(ast, 0x1E6E0020, ddr_table[REGIDX_020]);	     /* MODEREG4/6 */
-	ast_moutdwm(ast, 0x1E6E0024, ddr_table[REGIDX_024]);	     /* MODEREG5 */
-	ast_moutdwm(ast, 0x1E6E002C, ddr_table[REGIDX_02C] | 0x100); /* MODEREG0/2 */
-	ast_moutdwm(ast, 0x1E6E0030, ddr_table[REGIDX_030]);	     /* MODEREG1/3 */
+	ast_moutdwm(ast, AST_REG_MCR04, 0x00000303);
+	ast_moutdwm(ast, AST_REG_MCR10, ddr_table[REGIDX_010]);
+	ast_moutdwm(ast, AST_REG_MCR14, ddr_table[REGIDX_014]);
+	ast_moutdwm(ast, AST_REG_MCR18, ddr_table[REGIDX_018]);
+	ast_moutdwm(ast, AST_REG_MCR20, ddr_table[REGIDX_020]);	     /* MODEREG4/6 */
+	ast_moutdwm(ast, AST_REG_MCR24, ddr_table[REGIDX_024]);	     /* MODEREG5 */
+	ast_moutdwm(ast, AST_REG_MCR2C, ddr_table[REGIDX_02C] | 0x100); /* MODEREG0/2 */
+	ast_moutdwm(ast, AST_REG_MCR30, ddr_table[REGIDX_030]);	     /* MODEREG1/3 */
 
 	/* DDR PHY Setting */
-	ast_moutdwm(ast, 0x1E6E0200, 0x02492AAE);
-	ast_moutdwm(ast, 0x1E6E0204, 0x00001001);
-	ast_moutdwm(ast, 0x1E6E020C, 0x55E00B0B);
-	ast_moutdwm(ast, 0x1E6E0210, 0x20000000);
-	ast_moutdwm(ast, 0x1E6E0214, ddr_table[REGIDX_214]);
-	ast_moutdwm(ast, 0x1E6E02E0, ddr_table[REGIDX_2E0]);
-	ast_moutdwm(ast, 0x1E6E02E4, ddr_table[REGIDX_2E4]);
-	ast_moutdwm(ast, 0x1E6E02E8, ddr_table[REGIDX_2E8]);
-	ast_moutdwm(ast, 0x1E6E02EC, ddr_table[REGIDX_2EC]);
-	ast_moutdwm(ast, 0x1E6E02F0, ddr_table[REGIDX_2F0]);
-	ast_moutdwm(ast, 0x1E6E02F4, ddr_table[REGIDX_2F4]);
-	ast_moutdwm(ast, 0x1E6E02F8, ddr_table[REGIDX_2F8]);
-	ast_moutdwm(ast, 0x1E6E0290, 0x00100008);
-	ast_moutdwm(ast, 0x1E6E02C0, 0x00000006);
+	ast_moutdwm(ast, AST_REG_MCR200, 0x02492AAE);
+	ast_moutdwm(ast, AST_REG_MCR204, 0x00001001);
+	ast_moutdwm(ast, AST_REG_MCR20C, 0x55E00B0B);
+	ast_moutdwm(ast, AST_REG_MCR210, 0x20000000);
+	ast_moutdwm(ast, AST_REG_MCR214, ddr_table[REGIDX_214]);
+	ast_moutdwm(ast, AST_REG_MCR2E0, ddr_table[REGIDX_2E0]);
+	ast_moutdwm(ast, AST_REG_MCR2E4, ddr_table[REGIDX_2E4]);
+	ast_moutdwm(ast, AST_REG_MCR2E8, ddr_table[REGIDX_2E8]);
+	ast_moutdwm(ast, AST_REG_MCR2EC, ddr_table[REGIDX_2EC]);
+	ast_moutdwm(ast, AST_REG_MCR2F0, ddr_table[REGIDX_2F0]);
+	ast_moutdwm(ast, AST_REG_MCR2F4, ddr_table[REGIDX_2F4]);
+	ast_moutdwm(ast, AST_REG_MCR2F8, ddr_table[REGIDX_2F8]);
+	ast_moutdwm(ast, AST_REG_MCR290, 0x00100008);
+	ast_moutdwm(ast, AST_REG_MCR2C0, 0x00000006);
 
 	/* Controller Setting */
-	ast_moutdwm(ast, 0x1E6E0034, 0x00020091);
+	ast_moutdwm(ast, AST_REG_MCR34, 0x00020091);
 
 	/* Wait DDR PHY init done */
 	ddr_phy_init_2500(ast);
 
-	ast_moutdwm(ast, 0x1E6E0120, ddr_table[REGIDX_PLL]);
-	ast_moutdwm(ast, 0x1E6E000C, 0x42AA5C81);
-	ast_moutdwm(ast, 0x1E6E0034, 0x0001AF93);
+	ast_moutdwm(ast, AST_REG_MCR120, ddr_table[REGIDX_PLL]);
+	ast_moutdwm(ast, AST_REG_MCR0C, 0x42AA5C81);
+	ast_moutdwm(ast, AST_REG_MCR34, 0x0001AF93);
 
 	check_dram_size_2500(ast, ddr_table[REGIDX_RFC]);
 	enable_cache_2500(ast);
-	ast_moutdwm(ast, 0x1E6E001C, 0x00000008);
-	ast_moutdwm(ast, 0x1E6E0038, 0xFFFFFF00);
+	ast_moutdwm(ast, AST_REG_MCR1C, 0x00000008);
+	ast_moutdwm(ast, AST_REG_MCR38, 0xFFFFFF00);
 }
 
 static void ddr4_init_2500(struct ast_device *ast, const u32 *ddr_table)
@@ -363,34 +363,34 @@ static void ddr4_init_2500(struct ast_device *ast, const u32 *ddr_table)
 	u32 min_ddr_vref = 0, min_phy_vref = 0;
 	u32 max_ddr_vref = 0, max_phy_vref = 0;
 
-	ast_moutdwm(ast, 0x1E6E0004, 0x00000313);
-	ast_moutdwm(ast, 0x1E6E0010, ddr_table[REGIDX_010]);
-	ast_moutdwm(ast, 0x1E6E0014, ddr_table[REGIDX_014]);
-	ast_moutdwm(ast, 0x1E6E0018, ddr_table[REGIDX_018]);
-	ast_moutdwm(ast, 0x1E6E0020, ddr_table[REGIDX_020]);	     /* MODEREG4/6 */
-	ast_moutdwm(ast, 0x1E6E0024, ddr_table[REGIDX_024]);	     /* MODEREG5 */
-	ast_moutdwm(ast, 0x1E6E002C, ddr_table[REGIDX_02C] | 0x100); /* MODEREG0/2 */
-	ast_moutdwm(ast, 0x1E6E0030, ddr_table[REGIDX_030]);	     /* MODEREG1/3 */
+	ast_moutdwm(ast, AST_REG_MCR04, 0x00000313);
+	ast_moutdwm(ast, AST_REG_MCR10, ddr_table[REGIDX_010]);
+	ast_moutdwm(ast, AST_REG_MCR14, ddr_table[REGIDX_014]);
+	ast_moutdwm(ast, AST_REG_MCR18, ddr_table[REGIDX_018]);
+	ast_moutdwm(ast, AST_REG_MCR20, ddr_table[REGIDX_020]);	     /* MODEREG4/6 */
+	ast_moutdwm(ast, AST_REG_MCR24, ddr_table[REGIDX_024]);	     /* MODEREG5 */
+	ast_moutdwm(ast, AST_REG_MCR2C, ddr_table[REGIDX_02C] | 0x100); /* MODEREG0/2 */
+	ast_moutdwm(ast, AST_REG_MCR30, ddr_table[REGIDX_030]);	     /* MODEREG1/3 */
 
 	/* DDR PHY Setting */
-	ast_moutdwm(ast, 0x1E6E0200, 0x42492AAE);
-	ast_moutdwm(ast, 0x1E6E0204, 0x09002000);
-	ast_moutdwm(ast, 0x1E6E020C, 0x55E00B0B);
-	ast_moutdwm(ast, 0x1E6E0210, 0x20000000);
-	ast_moutdwm(ast, 0x1E6E0214, ddr_table[REGIDX_214]);
-	ast_moutdwm(ast, 0x1E6E02E0, ddr_table[REGIDX_2E0]);
-	ast_moutdwm(ast, 0x1E6E02E4, ddr_table[REGIDX_2E4]);
-	ast_moutdwm(ast, 0x1E6E02E8, ddr_table[REGIDX_2E8]);
-	ast_moutdwm(ast, 0x1E6E02EC, ddr_table[REGIDX_2EC]);
-	ast_moutdwm(ast, 0x1E6E02F0, ddr_table[REGIDX_2F0]);
-	ast_moutdwm(ast, 0x1E6E02F4, ddr_table[REGIDX_2F4]);
-	ast_moutdwm(ast, 0x1E6E02F8, ddr_table[REGIDX_2F8]);
-	ast_moutdwm(ast, 0x1E6E0290, 0x00100008);
-	ast_moutdwm(ast, 0x1E6E02C4, 0x3C183C3C);
-	ast_moutdwm(ast, 0x1E6E02C8, 0x00631E0E);
+	ast_moutdwm(ast, AST_REG_MCR200, 0x42492AAE);
+	ast_moutdwm(ast, AST_REG_MCR204, 0x09002000);
+	ast_moutdwm(ast, AST_REG_MCR20C, 0x55E00B0B);
+	ast_moutdwm(ast, AST_REG_MCR210, 0x20000000);
+	ast_moutdwm(ast, AST_REG_MCR214, ddr_table[REGIDX_214]);
+	ast_moutdwm(ast, AST_REG_MCR2E0, ddr_table[REGIDX_2E0]);
+	ast_moutdwm(ast, AST_REG_MCR2E4, ddr_table[REGIDX_2E4]);
+	ast_moutdwm(ast, AST_REG_MCR2E8, ddr_table[REGIDX_2E8]);
+	ast_moutdwm(ast, AST_REG_MCR2EC, ddr_table[REGIDX_2EC]);
+	ast_moutdwm(ast, AST_REG_MCR2F0, ddr_table[REGIDX_2F0]);
+	ast_moutdwm(ast, AST_REG_MCR2F4, ddr_table[REGIDX_2F4]);
+	ast_moutdwm(ast, AST_REG_MCR2F8, ddr_table[REGIDX_2F8]);
+	ast_moutdwm(ast, AST_REG_MCR290, 0x00100008);
+	ast_moutdwm(ast, AST_REG_MCR2C4, 0x3C183C3C);
+	ast_moutdwm(ast, AST_REG_MCR2C8, 0x00631E0E);
 
 	/* Controller Setting */
-	ast_moutdwm(ast, 0x1E6E0034, 0x0001A991);
+	ast_moutdwm(ast, AST_REG_MCR34, 0x0001A991);
 
 	/* Train PHY Vref first */
 	pass = 0;
@@ -398,17 +398,17 @@ static void ddr4_init_2500(struct ast_device *ast, const u32 *ddr_table)
 	for (retrycnt = 0; retrycnt < 4 && pass == 0; retrycnt++) {
 		max_phy_vref = 0x0;
 		pass = 0;
-		ast_moutdwm(ast, 0x1E6E02C0, 0x00001C06);
+		ast_moutdwm(ast, AST_REG_MCR2C0, 0x00001C06);
 		for (phy_vref = 0x40; phy_vref < 0x80; phy_vref++) {
-			ast_moutdwm(ast, 0x1E6E000C, 0x00000000);
-			ast_moutdwm(ast, 0x1E6E0060, 0x00000000);
-			ast_moutdwm(ast, 0x1E6E02CC, phy_vref | (phy_vref << 8));
+			ast_moutdwm(ast, AST_REG_MCR0C, 0x00000000);
+			ast_moutdwm(ast, AST_REG_MCR60, 0x00000000);
+			ast_moutdwm(ast, AST_REG_MCR2CC, phy_vref | (phy_vref << 8));
 			/* Fire DFI Init */
 			ddr_phy_init_2500(ast);
-			ast_moutdwm(ast, 0x1E6E000C, 0x00005C01);
+			ast_moutdwm(ast, AST_REG_MCR0C, 0x00005C01);
 			if (cbr_test_2500(ast)) {
 				pass++;
-				data = ast_mindwm(ast, 0x1E6E03D0);
+				data = ast_mindwm(ast, AST_REG_MCR3D0);
 				data2 = data >> 8;
 				data  = data & 0xff;
 				if (data > data2)
@@ -422,7 +422,7 @@ static void ddr4_init_2500(struct ast_device *ast, const u32 *ddr_table)
 			}
 		}
 	}
-	ast_moutdwm(ast, 0x1E6E02CC, min_phy_vref | (min_phy_vref << 8));
+	ast_moutdwm(ast, AST_REG_MCR2CC, min_phy_vref | (min_phy_vref << 8));
 
 	/* Train DDR Vref next */
 	pass = 0;
@@ -432,12 +432,12 @@ static void ddr4_init_2500(struct ast_device *ast, const u32 *ddr_table)
 		max_ddr_vref = 0x0;
 		pass = 0;
 		for (ddr_vref = 0x00; ddr_vref < 0x40; ddr_vref++) {
-			ast_moutdwm(ast, 0x1E6E000C, 0x00000000);
-			ast_moutdwm(ast, 0x1E6E0060, 0x00000000);
-			ast_moutdwm(ast, 0x1E6E02C0, 0x00000006 | (ddr_vref << 8));
+			ast_moutdwm(ast, AST_REG_MCR0C, 0x00000000);
+			ast_moutdwm(ast, AST_REG_MCR60, 0x00000000);
+			ast_moutdwm(ast, AST_REG_MCR2C0, 0x00000006 | (ddr_vref << 8));
 			/* Fire DFI Init */
 			ddr_phy_init_2500(ast);
-			ast_moutdwm(ast, 0x1E6E000C, 0x00005C01);
+			ast_moutdwm(ast, AST_REG_MCR0C, 0x00005C01);
 			if (cbr_test_2500(ast)) {
 				pass++;
 				if (min_ddr_vref > ddr_vref)
@@ -450,22 +450,22 @@ static void ddr4_init_2500(struct ast_device *ast, const u32 *ddr_table)
 		}
 	}
 
-	ast_moutdwm(ast, 0x1E6E000C, 0x00000000);
-	ast_moutdwm(ast, 0x1E6E0060, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR0C, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR60, 0x00000000);
 	ddr_vref = (min_ddr_vref + max_ddr_vref + 1) >> 1;
-	ast_moutdwm(ast, 0x1E6E02C0, 0x00000006 | (ddr_vref << 8));
+	ast_moutdwm(ast, AST_REG_MCR2C0, 0x00000006 | (ddr_vref << 8));
 
 	/* Wait DDR PHY init done */
 	ddr_phy_init_2500(ast);
 
-	ast_moutdwm(ast, 0x1E6E0120, ddr_table[REGIDX_PLL]);
-	ast_moutdwm(ast, 0x1E6E000C, 0x42AA5C81);
-	ast_moutdwm(ast, 0x1E6E0034, 0x0001AF93);
+	ast_moutdwm(ast, AST_REG_MCR120, ddr_table[REGIDX_PLL]);
+	ast_moutdwm(ast, AST_REG_MCR0C, 0x42AA5C81);
+	ast_moutdwm(ast, AST_REG_MCR34, 0x0001AF93);
 
 	check_dram_size_2500(ast, ddr_table[REGIDX_RFC]);
 	enable_cache_2500(ast);
-	ast_moutdwm(ast, 0x1E6E001C, 0x00000008);
-	ast_moutdwm(ast, 0x1E6E0038, 0xFFFFFF00);
+	ast_moutdwm(ast, AST_REG_MCR1C, 0x00000008);
+	ast_moutdwm(ast, AST_REG_MCR38, 0xFFFFFF00);
 }
 
 static bool ast_dram_init_2500(struct ast_device *ast)
diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 677c52c0d99a..8c2243cf8187 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -230,7 +230,7 @@ static bool ast_launch_m68k(struct ast_device *ast)
 		}
 		/* Get BootAddress */
 		ast_moutdwm(ast, 0x1e6e2000, 0x1688a8a8);
-		data = ast_mindwm(ast, 0x1e6e0004);
+		data = ast_mindwm(ast, AST_REG_MCR04);
 		switch (data & 0x03) {
 		case 0:
 			boot_address = 0x44000000;
@@ -348,7 +348,7 @@ static bool ast_init_dvo(struct ast_device *ast)
 {
 	u8 jreg;
 	u32 data;
-	ast_write32(ast, 0xf004, 0x1e6e0000);
+	ast_write32(ast, 0xf004, AST_REG_MCR00);
 	ast_write32(ast, 0xf000, 0x1);
 	ast_write32(ast, 0x12000, 0x1688a8a8);
 
@@ -425,7 +425,7 @@ static void ast_init_analog(struct ast_device *ast)
 	 * bridge. First configure the P2U to target the SCU
 	 * in case it isn't at this stage.
 	 */
-	ast_write32(ast, 0xf004, 0x1e6e0000);
+	ast_write32(ast, 0xf004, AST_REG_MCR00);
 	ast_write32(ast, 0xf000, 0x1);
 
 	/* Then unlock the SCU with the magic password */
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 05ec3542ab62..3da0cce0a3f6 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -268,7 +268,7 @@ static int ast_detect_chip(struct pci_dev *pdev,
 				config_mode = ast_use_p2a;
 
 				/* Read SCU7c (silicon revision register) */
-				__ast_write32(regs, 0xf004, 0x1e6e0000);
+				__ast_write32(regs, 0xf004, AST_REG_MCR00);
 				__ast_write32(regs, 0xf000, 0x1);
 				scu_rev = __ast_read32(regs, 0x1207c);
 			}
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 5cec5d735b6a..297230f1efaf 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -69,19 +69,19 @@ bool mmc_test(struct ast_device *ast, u32 datagen, u8 test_ctl)
 {
 	u32 data, timeout;
 
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
-	ast_moutdwm(ast, 0x1e6e0070, (datagen << 3) | test_ctl);
+	ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
+	ast_moutdwm(ast, AST_REG_MCR70, (datagen << 3) | test_ctl);
 	timeout = 0;
 	do {
-		data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
+		data = ast_mindwm(ast, AST_REG_MCR70) & 0x3000;
 		if (data & 0x2000)
 			return false;
 		if (++timeout > TIMEOUT) {
-			ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
+			ast_moutdwm(ast, AST_REG_MCR70, 0x00000000);
 			return false;
 		}
 	} while (!data);
-	ast_moutdwm(ast, 0x1e6e0070, 0x0);
+	ast_moutdwm(ast, AST_REG_MCR70, 0x0);
 	return true;
 }
 
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index 0bc033dbac55..a54733dc2675 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -98,4 +98,75 @@
 #define AST_REG_AHBC84				AST_REG_AHBC(0x84)
 #define AST_REG_AHBC88				AST_REG_AHBC(0x88)
 
+/*
+ * SDRAM Memory Controller (0x1e6e0000 - 0x1e6e0fff)
+ */
+
+#define AST_REG_MCR_BASE			(0x1e6e0000)
+#define AST_REG_MCR(__offset)			(AST_REG_MCR_BASE + (__offset))
+#define AST_REG_MCR00				AST_REG_MCR(0x00)
+#define AST_REG_MCR00_PROTECTION_KEY		(0xfc600309)
+#define AST_REG_MCR04				AST_REG_MCR(0x04)
+#define AST_REG_MCR08				AST_REG_MCR(0x08)
+#define AST_REG_MCR0C				AST_REG_MCR(0x0c)
+#define AST_REG_MCR10				AST_REG_MCR(0x10)
+#define AST_REG_MCR14				AST_REG_MCR(0x14)
+#define AST_REG_MCR18				AST_REG_MCR(0x18)
+#define AST_REG_MCR1C				AST_REG_MCR(0x1c)
+#define AST_REG_MCR20				AST_REG_MCR(0x20)
+#define AST_REG_MCR24				AST_REG_MCR(0x24)
+#define AST_REG_MCR28				AST_REG_MCR(0x28)
+#define AST_REG_MCR2C				AST_REG_MCR(0x2C)
+#define AST_REG_MCR30				AST_REG_MCR(0x30)
+#define AST_REG_MCR34				AST_REG_MCR(0x34)
+#define AST_REG_MCR38				AST_REG_MCR(0x38)
+#define AST_REG_MCR40				AST_REG_MCR(0x40)
+#define AST_REG_MCR44				AST_REG_MCR(0x44)
+#define AST_REG_MCR48				AST_REG_MCR(0x48)
+#define AST_REG_MCR4C				AST_REG_MCR(0x4C)
+#define AST_REG_MCR50				AST_REG_MCR(0x50)
+#define AST_REG_MCR54				AST_REG_MCR(0x54)
+#define AST_REG_MCR60				AST_REG_MCR(0x60)
+#define AST_REG_MCR64				AST_REG_MCR(0x64)
+#define AST_REG_MCR68				AST_REG_MCR(0x68)
+#define AST_REG_MCR6C				AST_REG_MCR(0x6c)
+#define AST_REG_MCR70				AST_REG_MCR(0x70)
+#define AST_REG_MCR74				AST_REG_MCR(0x74)
+#define AST_REG_MCR78				AST_REG_MCR(0x78)
+#define AST_REG_MCR7C				AST_REG_MCR(0x7c)
+#define AST_REG_MCR80				AST_REG_MCR(0x80)
+#define AST_REG_MCR84				AST_REG_MCR(0x84)
+#define AST_REG_MCR88				AST_REG_MCR(0x88)
+#define AST_REG_MCR120				AST_REG_MCR(0x120)
+#define AST_REG_MCR200				AST_REG_MCR(0x200)
+#define AST_REG_MCR204				AST_REG_MCR(0x204)
+#define AST_REG_MCR208				AST_REG_MCR(0x208)
+#define AST_REG_MCR20C				AST_REG_MCR(0x20C)
+#define AST_REG_MCR210				AST_REG_MCR(0x210)
+#define AST_REG_MCR214				AST_REG_MCR(0x214)
+#define AST_REG_MCR218				AST_REG_MCR(0x218)
+#define AST_REG_MCR220				AST_REG_MCR(0x220)
+#define AST_REG_MCR228				AST_REG_MCR(0x228)
+#define AST_REG_MCR230				AST_REG_MCR(0x230)
+#define AST_REG_MCR2A8				AST_REG_MCR(0x2a8)
+#define AST_REG_MCR2B0				AST_REG_MCR(0x2b0)
+#define AST_REG_MCR240				AST_REG_MCR(0x240)
+#define AST_REG_MCR244				AST_REG_MCR(0x244)
+#define AST_REG_MCR248				AST_REG_MCR(0x248)
+#define AST_REG_MCR24C				AST_REG_MCR(0x24c)
+#define AST_REG_MCR290				AST_REG_MCR(0x290)
+#define AST_REG_MCR2C0				AST_REG_MCR(0x2c0)
+#define AST_REG_MCR2C4				AST_REG_MCR(0x2c4)
+#define AST_REG_MCR2C8				AST_REG_MCR(0x2c8)
+#define AST_REG_MCR2CC				AST_REG_MCR(0x2cc)
+#define AST_REG_MCR2E0				AST_REG_MCR(0x2e0)
+#define AST_REG_MCR2E4				AST_REG_MCR(0x2e4)
+#define AST_REG_MCR2E8				AST_REG_MCR(0x2e8)
+#define AST_REG_MCR2EC				AST_REG_MCR(0x2ec)
+#define AST_REG_MCR2F0				AST_REG_MCR(0x2f0)
+#define AST_REG_MCR2F4				AST_REG_MCR(0x2f4)
+#define AST_REG_MCR2F8				AST_REG_MCR(0x2f8)
+#define AST_REG_MCR300				AST_REG_MCR(0x300)
+#define AST_REG_MCR3D0				AST_REG_MCR(0x3d0)
+
 #endif
-- 
2.53.0

Claude review: drm/ast: Use constants for MCR registers

[Claude Code Review Bot]

Patch Review

This is the largest mechanical patch (~300 line changes in ast_2300.c alone). The conversion was done via grep+sed plus manual fixes, which is appropriate for this kind of bulk replacement. The register constant definitions in `ast_reg.h` are well-organized.

Minor nit: In `set_mpll_2500()` there's a loop that still uses raw addresses:
```c
for (addr = 0x1e6e0004; addr < 0x1e6e0090;) {
    ast_moutdwm(ast, addr, 0x0);
    addr += 4;
}
```
This loop wasn't converted to use constants (it iterates through a range), which is fine - converting a loop like this would be awkward.

One concern: in `ast_dp501.c`, the P2A bridge setup at `ast_init_analog()` and `ast_init_dvo()` writes `AST_REG_MCR00` to register `0xf004`. This is the P2A base register being set to the MCR base, but using the MCR constant name for the *value* written is semantically correct (the MCR base address happens to equal `AST_REG_MCR00`).

However, in `ast_drv.c:ast_detect_chip()`, the same pattern `__ast_write32(regs, 0xf004, AST_REG_MCR00)` is used - where the intent is to set the P2A window base to the MCR region. Using `AST_REG_MCR00` as the *value* is technically correct but conceptually it's the base address being set. The later patches fix this by using `__ast_mindwm` which handles the segment setup internally.

---
Generated by Claude Code Patch Reviewer

[PATCH 05/15] drm/ast: Use constants for SCU registers

[Thomas Zimmermann]

SCU is the System Control Unit. SCU registers are located in the memory
range at [0x1e6e2000, 0x1e6e2fff]. Refer to them with constants named
AST_REG_SCU<n>, where <n> is the byte offset into the range.

Replacing the magic values in the ast driver was done with grep and sed
as shown below

  git grep -l \,\ 0x1e6e2 | xargs sed -i -e 's/, 0x1e6e2/, AST_REG_SCU/g'
  git grep -l \,\ 0x1E6E2 | xargs sed -i -e 's/, 0x1E6E2/, AST_REG_SCU/g'

plus some manual fixes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2300.c  | 50 ++++++++++++++++-----------------
 drivers/gpu/drm/ast/ast_2500.c  | 44 ++++++++++++++---------------
 drivers/gpu/drm/ast/ast_dp501.c | 20 ++++++-------
 drivers/gpu/drm/ast/ast_reg.h   | 19 +++++++++++++
 4 files changed, 76 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2300.c b/drivers/gpu/drm/ast/ast_2300.c
index 56fe9e9f5c66..1fe947178124 100644
--- a/drivers/gpu/drm/ast/ast_2300.c
+++ b/drivers/gpu/drm/ast/ast_2300.c
@@ -516,10 +516,10 @@ static void get_ddr3_info(struct ast_device *ast, struct ast2300_dram_param *par
 {
 	u32 trap, trap_AC2, trap_MRS;
 
-	ast_moutdwm(ast, 0x1E6E2000, 0x1688A8A8);
+	ast_moutdwm(ast, AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY);
 
 	/* Ger trap info */
-	trap = (ast_mindwm(ast, 0x1E6E2070) >> 25) & 0x3;
+	trap = (ast_mindwm(ast, AST_REG_SCU070) >> 25) & 0x3;
 	trap_AC2  = 0x00020000 + (trap << 16);
 	trap_AC2 |= 0x00300000 + ((trap & 0x2) << 19);
 	trap_MRS  = 0x00000010 + (trap << 4);
@@ -533,7 +533,7 @@ static void get_ddr3_info(struct ast_device *ast, struct ast2300_dram_param *par
 
 	switch (param->dram_freq) {
 	case 336:
-		ast_moutdwm(ast, 0x1E6E2020, 0x0190);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x0190);
 		param->wodt          = 0;
 		param->reg_AC1       = 0x22202725;
 		param->reg_AC2       = 0xAA007613 | trap_AC2;
@@ -561,7 +561,7 @@ static void get_ddr3_info(struct ast_device *ast, struct ast2300_dram_param *par
 		break;
 	default:
 	case 396:
-		ast_moutdwm(ast, 0x1E6E2020, 0x03F1);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x03F1);
 		param->wodt          = 1;
 		param->reg_AC1       = 0x33302825;
 		param->reg_AC2       = 0xCC009617 | trap_AC2;
@@ -591,7 +591,7 @@ static void get_ddr3_info(struct ast_device *ast, struct ast2300_dram_param *par
 		break;
 
 	case 408:
-		ast_moutdwm(ast, 0x1E6E2020, 0x01F0);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x01F0);
 		param->wodt          = 1;
 		param->reg_AC1       = 0x33302825;
 		param->reg_AC2       = 0xCC009617 | trap_AC2;
@@ -621,7 +621,7 @@ static void get_ddr3_info(struct ast_device *ast, struct ast2300_dram_param *par
 
 		break;
 	case 456:
-		ast_moutdwm(ast, 0x1E6E2020, 0x0230);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x0230);
 		param->wodt          = 0;
 		param->reg_AC1       = 0x33302926;
 		param->reg_AC2       = 0xCD44961A;
@@ -635,7 +635,7 @@ static void get_ddr3_info(struct ast_device *ast, struct ast2300_dram_param *par
 		param->dll2_finetune_step = 4;
 		break;
 	case 504:
-		ast_moutdwm(ast, 0x1E6E2020, 0x0270);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x0270);
 		param->wodt          = 1;
 		param->reg_AC1       = 0x33302926;
 		param->reg_AC2       = 0xDE44A61D;
@@ -649,7 +649,7 @@ static void get_ddr3_info(struct ast_device *ast, struct ast2300_dram_param *par
 		param->dll2_finetune_step = 4;
 		break;
 	case 528:
-		ast_moutdwm(ast, 0x1E6E2020, 0x0290);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x0290);
 		param->wodt          = 1;
 		param->rodt          = 1;
 		param->reg_AC1       = 0x33302926;
@@ -665,7 +665,7 @@ static void get_ddr3_info(struct ast_device *ast, struct ast2300_dram_param *par
 		param->dll2_finetune_step = 3;
 		break;
 	case 576:
-		ast_moutdwm(ast, 0x1E6E2020, 0x0140);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x0140);
 		param->reg_MADJ      = 0x00136868;
 		param->reg_SADJ      = 0x00004534;
 		param->wodt          = 1;
@@ -683,7 +683,7 @@ static void get_ddr3_info(struct ast_device *ast, struct ast2300_dram_param *par
 		param->dll2_finetune_step = 3;
 		break;
 	case 600:
-		ast_moutdwm(ast, 0x1E6E2020, 0x02E1);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x02E1);
 		param->reg_MADJ      = 0x00136868;
 		param->reg_SADJ      = 0x00004534;
 		param->wodt          = 1;
@@ -701,7 +701,7 @@ static void get_ddr3_info(struct ast_device *ast, struct ast2300_dram_param *par
 		param->dll2_finetune_step = 3;
 		break;
 	case 624:
-		ast_moutdwm(ast, 0x1E6E2020, 0x0160);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x0160);
 		param->reg_MADJ      = 0x00136868;
 		param->reg_SADJ      = 0x00004534;
 		param->wodt          = 1;
@@ -874,10 +874,10 @@ static void get_ddr2_info(struct ast_device *ast, struct ast2300_dram_param *par
 {
 	u32 trap, trap_AC2, trap_MRS;
 
-	ast_moutdwm(ast, 0x1E6E2000, 0x1688A8A8);
+	ast_moutdwm(ast, AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY);
 
 	/* Ger trap info */
-	trap = (ast_mindwm(ast, 0x1E6E2070) >> 25) & 0x3;
+	trap = (ast_mindwm(ast, AST_REG_SCU070) >> 25) & 0x3;
 	trap_AC2  = (trap << 20) | (trap << 16);
 	trap_AC2 += 0x00110000;
 	trap_MRS  = 0x00000040 | (trap << 4);
@@ -890,7 +890,7 @@ static void get_ddr2_info(struct ast_device *ast, struct ast2300_dram_param *par
 
 	switch (param->dram_freq) {
 	case 264:
-		ast_moutdwm(ast, 0x1E6E2020, 0x0130);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x0130);
 		param->wodt          = 0;
 		param->reg_AC1       = 0x11101513;
 		param->reg_AC2       = 0x78117011;
@@ -905,7 +905,7 @@ static void get_ddr2_info(struct ast_device *ast, struct ast2300_dram_param *par
 		param->dll2_finetune_step = 3;
 		break;
 	case 336:
-		ast_moutdwm(ast, 0x1E6E2020, 0x0190);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x0190);
 		param->wodt          = 1;
 		param->reg_AC1       = 0x22202613;
 		param->reg_AC2       = 0xAA009016 | trap_AC2;
@@ -936,7 +936,7 @@ static void get_ddr2_info(struct ast_device *ast, struct ast2300_dram_param *par
 		break;
 	default:
 	case 396:
-		ast_moutdwm(ast, 0x1E6E2020, 0x03F1);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x03F1);
 		param->wodt          = 1;
 		param->rodt          = 0;
 		param->reg_AC1       = 0x33302714;
@@ -970,7 +970,7 @@ static void get_ddr2_info(struct ast_device *ast, struct ast2300_dram_param *par
 		break;
 
 	case 408:
-		ast_moutdwm(ast, 0x1E6E2020, 0x01F0);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x01F0);
 		param->wodt          = 1;
 		param->rodt          = 0;
 		param->reg_AC1       = 0x33302714;
@@ -1003,7 +1003,7 @@ static void get_ddr2_info(struct ast_device *ast, struct ast2300_dram_param *par
 
 		break;
 	case 456:
-		ast_moutdwm(ast, 0x1E6E2020, 0x0230);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x0230);
 		param->wodt          = 0;
 		param->reg_AC1       = 0x33302815;
 		param->reg_AC2       = 0xCD44B01E;
@@ -1018,7 +1018,7 @@ static void get_ddr2_info(struct ast_device *ast, struct ast2300_dram_param *par
 		param->dll2_finetune_step = 3;
 		break;
 	case 504:
-		ast_moutdwm(ast, 0x1E6E2020, 0x0261);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x0261);
 		param->wodt          = 1;
 		param->rodt          = 1;
 		param->reg_AC1       = 0x33302815;
@@ -1034,7 +1034,7 @@ static void get_ddr2_info(struct ast_device *ast, struct ast2300_dram_param *par
 		param->dll2_finetune_step = 3;
 		break;
 	case 528:
-		ast_moutdwm(ast, 0x1E6E2020, 0x0120);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x0120);
 		param->wodt          = 1;
 		param->rodt          = 1;
 		param->reg_AC1       = 0x33302815;
@@ -1050,7 +1050,7 @@ static void get_ddr2_info(struct ast_device *ast, struct ast2300_dram_param *par
 		param->dll2_finetune_step = 3;
 		break;
 	case 552:
-		ast_moutdwm(ast, 0x1E6E2020, 0x02A1);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x02A1);
 		param->wodt          = 1;
 		param->rodt          = 1;
 		param->reg_AC1       = 0x43402915;
@@ -1066,7 +1066,7 @@ static void get_ddr2_info(struct ast_device *ast, struct ast2300_dram_param *par
 		param->dll2_finetune_step = 3;
 		break;
 	case 576:
-		ast_moutdwm(ast, 0x1E6E2020, 0x0140);
+		ast_moutdwm(ast, AST_REG_SCU020, 0x0140);
 		param->wodt          = 1;
 		param->rodt          = 1;
 		param->reg_AC1       = 0x43402915;
@@ -1264,7 +1264,7 @@ static void ast_post_chip_2300(struct ast_device *ast)
 
 		param.dram_freq = 396;
 		param.dram_type = AST_DDR3;
-		temp = ast_mindwm(ast, 0x1e6e2070);
+		temp = ast_mindwm(ast, AST_REG_SCU070);
 		if (temp & 0x01000000)
 			param.dram_type = AST_DDR2;
 		switch (temp & 0x18000000) {
@@ -1306,8 +1306,8 @@ static void ast_post_chip_2300(struct ast_device *ast)
 			ddr2_init(ast, &param);
 		}
 
-		temp = ast_mindwm(ast, 0x1e6e2040);
-		ast_moutdwm(ast, 0x1e6e2040, temp | 0x40);
+		temp = ast_mindwm(ast, AST_REG_SCU040);
+		ast_moutdwm(ast, AST_REG_SCU040, temp | 0x40);
 	}
 
 	/* wait ready */
diff --git a/drivers/gpu/drm/ast/ast_2500.c b/drivers/gpu/drm/ast/ast_2500.c
index 4a9df920509f..d141b34548a9 100644
--- a/drivers/gpu/drm/ast/ast_2500.c
+++ b/drivers/gpu/drm/ast/ast_2500.c
@@ -112,9 +112,9 @@ void ast_2500_patch_ahb(void __iomem *regs)
 	__ast_moutdwm(regs, AST_REG_AHBC00, AST_REG_AHBC00_PROTECT_KEY);
 	__ast_moutdwm(regs, AST_REG_AHBC84, 0x00010000);
 	__ast_moutdwm(regs, AST_REG_AHBC88, 0x00000000);
-	__ast_moutdwm(regs, 0x1e6e2000, 0x1688A8A8);
+	__ast_moutdwm(regs, AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY);
 
-	data = __ast_mindwm(regs, 0x1e6e2070);
+	data = __ast_mindwm(regs, AST_REG_SCU070);
 	if (data & 0x08000000) { /* check fast reset */
 		/*
 		 * If "Fast restet" is enabled for ARM-ICE debugger,
@@ -134,11 +134,11 @@ void ast_2500_patch_ahb(void __iomem *regs)
 	}
 
 	do {
-		__ast_moutdwm(regs, 0x1e6e2000, 0x1688A8A8);
-		data = __ast_mindwm(regs, 0x1e6e2000);
+		__ast_moutdwm(regs, AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY);
+		data = __ast_mindwm(regs, AST_REG_SCU000);
 	} while (data != 1);
 
-	__ast_moutdwm(regs, 0x1e6e207c, 0x08000000); /* clear fast reset */
+	__ast_moutdwm(regs, AST_REG_SCU07C, 0x08000000); /* clear fast reset */
 }
 
 static bool mmc_test_single_2500(struct ast_device *ast, u32 datagen)
@@ -288,17 +288,17 @@ static void set_mpll_2500(struct ast_device *ast)
 	}
 	ast_moutdwm(ast, AST_REG_MCR34, 0x00020000);
 
-	ast_moutdwm(ast, 0x1E6E2000, 0x1688A8A8);
-	data = ast_mindwm(ast, 0x1E6E2070) & 0x00800000;
+	ast_moutdwm(ast, AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY);
+	data = ast_mindwm(ast, AST_REG_SCU070) & 0x00800000;
 	if (data) {
 		/* CLKIN = 25MHz */
 		param = 0x930023E0;
-		ast_moutdwm(ast, 0x1E6E2160, 0x00011320);
+		ast_moutdwm(ast, AST_REG_SCU160, 0x00011320);
 	} else {
 		/* CLKIN = 24MHz */
 		param = 0x93002400;
 	}
-	ast_moutdwm(ast, 0x1E6E2020, param);
+	ast_moutdwm(ast, AST_REG_SCU020, param);
 	udelay(100);
 }
 
@@ -480,18 +480,18 @@ static bool ast_dram_init_2500(struct ast_device *ast)
 		reset_mmc_2500(ast);
 		ddr_init_common_2500(ast);
 
-		data = ast_mindwm(ast, 0x1E6E2070);
+		data = ast_mindwm(ast, AST_REG_SCU070);
 		if (data & 0x01000000)
 			ddr4_init_2500(ast, ast2500_ddr4_1600_timing_table);
 		else
 			ddr3_init_2500(ast, ast2500_ddr3_1600_timing_table);
 	} while (!ddr_test_2500(ast));
 
-	ast_moutdwm(ast, 0x1E6E2040, ast_mindwm(ast, 0x1E6E2040) | 0x41);
+	ast_moutdwm(ast, AST_REG_SCU040, ast_mindwm(ast, AST_REG_SCU040) | 0x41);
 
 	/* Patch code */
-	data = ast_mindwm(ast, 0x1E6E200C) & 0xF9FFFFFF;
-	ast_moutdwm(ast, 0x1E6E200C, data | 0x10000000);
+	data = ast_mindwm(ast, AST_REG_SCU00C) & 0xF9FFFFFF;
+	ast_moutdwm(ast, AST_REG_SCU00C, data | 0x10000000);
 
 	return true;
 }
@@ -524,17 +524,17 @@ static void ast_post_chip_2500(struct ast_device *ast)
 		 * SCU7C is Write clear reg to SCU70
 		 *	[23]:= write 1 and then SCU70[23] will be clear as 0b.
 		 */
-		ast_moutdwm(ast, 0x1E6E2090, 0x20000000);
-		ast_moutdwm(ast, 0x1E6E2094, 0x00004000);
-		if (ast_mindwm(ast, 0x1E6E2070) & 0x00800000) {
-			ast_moutdwm(ast, 0x1E6E207C, 0x00800000);
+		ast_moutdwm(ast, AST_REG_SCU090, 0x20000000);
+		ast_moutdwm(ast, AST_REG_SCU094, 0x00004000);
+		if (ast_mindwm(ast, AST_REG_SCU070) & 0x00800000) {
+			ast_moutdwm(ast, AST_REG_SCU07C, 0x00800000);
 			mdelay(100);
-			ast_moutdwm(ast, 0x1E6E2070, 0x00800000);
+			ast_moutdwm(ast, AST_REG_SCU070, 0x00800000);
 		}
 		/* Modify eSPI reset pin */
-		temp = ast_mindwm(ast, 0x1E6E2070);
+		temp = ast_mindwm(ast, AST_REG_SCU070);
 		if (temp & 0x02000000)
-			ast_moutdwm(ast, 0x1E6E207C, 0x00004000);
+			ast_moutdwm(ast, AST_REG_SCU07C, 0x00004000);
 
 		/* Slow down CPU/AHB CLK in VGA only mode */
 		temp = ast_read32(ast, 0x12008);
@@ -544,8 +544,8 @@ static void ast_post_chip_2500(struct ast_device *ast)
 		if (!ast_dram_init_2500(ast))
 			drm_err(dev, "DRAM init failed !\n");
 
-		temp = ast_mindwm(ast, 0x1e6e2040);
-		ast_moutdwm(ast, 0x1e6e2040, temp | 0x40);
+		temp = ast_mindwm(ast, AST_REG_SCU040);
+		ast_moutdwm(ast, AST_REG_SCU040, temp | 0x40);
 	}
 
 	/* wait ready */
diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 8c2243cf8187..23142119d733 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -183,7 +183,7 @@ static void ast_set_dp501_video_output(struct ast_device *ast, u8 mode)
 
 static u32 get_fw_base(struct ast_device *ast)
 {
-	return ast_mindwm(ast, 0x1e6e2104) & 0x7fffffff;
+	return ast_mindwm(ast, AST_REG_SCU104) & 0x7fffffff;
 }
 
 bool ast_backup_fw(struct ast_device *ast, u8 *addr, u32 size)
@@ -194,7 +194,7 @@ bool ast_backup_fw(struct ast_device *ast, u8 *addr, u32 size)
 	if (ast->config_mode != ast_use_p2a)
 		return false;
 
-	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
+	data = ast_mindwm(ast, AST_REG_SCU100) & 0x01;
 	if (data) {
 		boot_address = get_fw_base(ast);
 		for (i = 0; i < size; i += 4)
@@ -214,7 +214,7 @@ static bool ast_launch_m68k(struct ast_device *ast)
 	if (ast->config_mode != ast_use_p2a)
 		return false;
 
-	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
+	data = ast_mindwm(ast, AST_REG_SCU100) & 0x01;
 	if (!data) {
 
 		if (ast->dp501_fw_addr) {
@@ -229,7 +229,7 @@ static bool ast_launch_m68k(struct ast_device *ast)
 			len = ast->dp501_fw->size;
 		}
 		/* Get BootAddress */
-		ast_moutdwm(ast, 0x1e6e2000, 0x1688a8a8);
+		ast_moutdwm(ast, AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY);
 		data = ast_mindwm(ast, AST_REG_MCR04);
 		switch (data & 0x03) {
 		case 0:
@@ -255,16 +255,16 @@ static bool ast_launch_m68k(struct ast_device *ast)
 		}
 
 		/* Init SCU */
-		ast_moutdwm(ast, 0x1e6e2000, 0x1688a8a8);
+		ast_moutdwm(ast, AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY);
 
 		/* Launch FW */
-		ast_moutdwm(ast, 0x1e6e2104, 0x80000000 + boot_address);
-		ast_moutdwm(ast, 0x1e6e2100, 1);
+		ast_moutdwm(ast, AST_REG_SCU104, 0x80000000 + boot_address);
+		ast_moutdwm(ast, AST_REG_SCU100, 1);
 
 		/* Update Scratch */
-		data = ast_mindwm(ast, 0x1e6e2040) & 0xfffff1ff;		/* D[11:9] = 100b: UEFI handling */
-		data |= 0x800;
-		ast_moutdwm(ast, 0x1e6e2040, data);
+		data = ast_mindwm(ast, AST_REG_SCU040) & 0xfffff1ff;
+		data |= 0x800; /* D[11:9] = 100b: UEFI handling */
+		ast_moutdwm(ast, AST_REG_SCU040, data);
 
 		jreg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0x99, 0xfc); /* D[1:0]: Reserved Video Buffer */
 		jreg |= 0x02;
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index a54733dc2675..e8d4991186ca 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -169,4 +169,23 @@
 #define AST_REG_MCR300				AST_REG_MCR(0x300)
 #define AST_REG_MCR3D0				AST_REG_MCR(0x3d0)
 
+/*
+ * System Control Unit (0x1e6e2000 - 0x1e6e2fff)
+ */
+
+#define AST_REG_SCU_BASE			(0x1e6e2000)
+#define AST_REG_SCU(__offset)			(AST_REG_SCU_BASE + (__offset))
+#define AST_REG_SCU000				AST_REG_SCU(0x000)
+#define AST_REG_SCU000_PROTECTION_KEY		(0x1688a8a8)
+#define AST_REG_SCU00C				AST_REG_SCU(0x00c)
+#define AST_REG_SCU020				AST_REG_SCU(0x020)
+#define AST_REG_SCU040				AST_REG_SCU(0x040)
+#define AST_REG_SCU070				AST_REG_SCU(0x070)
+#define AST_REG_SCU07C				AST_REG_SCU(0x07c)
+#define AST_REG_SCU090				AST_REG_SCU(0x090)
+#define AST_REG_SCU094				AST_REG_SCU(0x094)
+#define AST_REG_SCU100				AST_REG_SCU(0x100)
+#define AST_REG_SCU104				AST_REG_SCU(0x104)
+#define AST_REG_SCU160				AST_REG_SCU(0x160)
+
 #endif
-- 
2.53.0

Claude review: drm/ast: Use constants for SCU registers

[Claude Code Review Bot]

Patch Review

Similar mechanical conversion as patch 4 but for SCU registers (0x1e6e2000 range). Well done. The `AST_REG_SCU000_PROTECTION_KEY` constant for the magic unlock value `0x1688a8a8` is a good improvement.

---
Generated by Claude Code Patch Reviewer

[PATCH 06/15] drm/ast: Use constants for A2P registers

[Thomas Zimmermann]

A2P is the AHB-to-P Bridge. A2P registers are located at the memory
range at [0x1e720000, 0x1e73ffff]. Refer to them with constants named
AST_REG_A2P<n>, where <n> is the byte offset into the range.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2300.c | 6 +++---
 drivers/gpu/drm/ast/ast_reg.h  | 8 ++++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2300.c b/drivers/gpu/drm/ast/ast_2300.c
index 1fe947178124..96d9909d942c 100644
--- a/drivers/gpu/drm/ast/ast_2300.c
+++ b/drivers/gpu/drm/ast/ast_2300.c
@@ -508,7 +508,7 @@ static bool cbr_dll2(struct ast_device *ast, struct ast2300_dram_param *param)
 	dlli  = (dllmin[1] + dllmax[1]) >> 1;
 	dlli <<= 8;
 	dlli += (dllmin[0] + dllmax[0]) >> 1;
-	ast_moutdwm(ast, AST_REG_MCR68, ast_mindwm(ast, 0x1E720058) | (dlli << 16));
+	ast_moutdwm(ast, AST_REG_MCR68, ast_mindwm(ast, AST_REG_A2P58) | (dlli << 16));
 	return status;
 } /* CBRDLL2 */
 
@@ -826,7 +826,7 @@ static void ddr3_init(struct ast_device *ast, struct ast2300_dram_param *param)
 		data = ast_mindwm(ast, AST_REG_MCR1C);
 		data = (data >> 8) & 0xff;
 	}
-	ast_moutdwm(ast, 0x1E720058, ast_mindwm(ast, AST_REG_MCR68) & 0xffff);
+	ast_moutdwm(ast, AST_REG_A2P58, ast_mindwm(ast, AST_REG_MCR68) & 0xffff);
 	data = ast_mindwm(ast, AST_REG_MCR18) | 0xC00;
 	ast_moutdwm(ast, AST_REG_MCR18, data);
 
@@ -1188,7 +1188,7 @@ static void ddr2_init(struct ast_device *ast, struct ast2300_dram_param *param)
 		data = ast_mindwm(ast, AST_REG_MCR1C);
 		data = (data >> 8) & 0xff;
 	}
-	ast_moutdwm(ast, 0x1E720058, ast_mindwm(ast, AST_REG_MCR08) & 0xffff);
+	ast_moutdwm(ast, AST_REG_A2P58, ast_mindwm(ast, AST_REG_MCR08) & 0xffff);
 	data = ast_mindwm(ast, AST_REG_MCR18) | 0xC00;
 	ast_moutdwm(ast, AST_REG_MCR18, data);
 
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index e8d4991186ca..730072a3c2cd 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -188,4 +188,12 @@
 #define AST_REG_SCU104				AST_REG_SCU(0x104)
 #define AST_REG_SCU160				AST_REG_SCU(0x160)
 
+/*
+ * AHB-to-P Bus Bridge (0x1e720000 - 0x1e73ffff)
+ */
+
+#define AST_REG_A2P_BASE			(0x1e720000)
+#define AST_REG_A2P(__offset)			(AST_REG_A2P_BASE + (__offset))
+#define AST_REG_A2P58				AST_REG_A2P(0x58)
+
 #endif
-- 
2.53.0

Claude review: drm/ast: Use constants for A2P registers

[Claude Code Review Bot]

Patch Review

Small and clean. Only 3 instances of `0x1E720058` are replaced with `AST_REG_A2P58`. No issues.

---
Generated by Claude Code Patch Reviewer

[PATCH 07/15] drm/ast: Use constants for WDT registers

[Thomas Zimmermann]

WDT is the Watchdog timer. WDC registers are located at the memory
range at [0x1e785000, 0x1e785fff]. There are currently up to 8
watchdog timers in the range, of which 2 are being used by AST2500.

Refer to them with macros named AST_REG_WDT<n>, where <n> is the byte
offset into the watchdog timer's memory range. Each macro also takes
the index of the watchdog timer to access. A new watchdog timer starts
at each 0x40 byte offset.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2500.c | 20 ++++++++++----------
 drivers/gpu/drm/ast/ast_reg.h  | 13 +++++++++++++
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2500.c b/drivers/gpu/drm/ast/ast_2500.c
index d141b34548a9..f0639f1ef062 100644
--- a/drivers/gpu/drm/ast/ast_2500.c
+++ b/drivers/gpu/drm/ast/ast_2500.c
@@ -127,9 +127,9 @@ void ast_2500_patch_ahb(void __iomem *regs)
 		 *	[1]:= 1:WDT will be cleeared and disabled after timeout occurs
 		 *	[0]:= 1:WDT enable
 		 */
-		__ast_moutdwm(regs, 0x1E785004, 0x00000010);
-		__ast_moutdwm(regs, 0x1E785008, 0x00004755);
-		__ast_moutdwm(regs, 0x1E78500c, 0x00000033);
+		__ast_moutdwm(regs, AST_REG_WDT04(0), 0x00000010);
+		__ast_moutdwm(regs, AST_REG_WDT08(0), 0x00004755);
+		__ast_moutdwm(regs, AST_REG_WDT0C(0), 0x00000033);
 		udelay(1000);
 	}
 
@@ -304,12 +304,12 @@ static void set_mpll_2500(struct ast_device *ast)
 
 static void reset_mmc_2500(struct ast_device *ast)
 {
-	ast_moutdwm(ast, 0x1E78505C, 0x00000004);
-	ast_moutdwm(ast, 0x1E785044, 0x00000001);
-	ast_moutdwm(ast, 0x1E785048, 0x00004755);
-	ast_moutdwm(ast, 0x1E78504C, 0x00000013);
+	ast_moutdwm(ast, AST_REG_WDT1C(1), 0x00000004);
+	ast_moutdwm(ast, AST_REG_WDT04(1), 0x00000001);
+	ast_moutdwm(ast, AST_REG_WDT08(1), 0x00004755);
+	ast_moutdwm(ast, AST_REG_WDT0C(1), 0x00000013);
 	mdelay(100);
-	ast_moutdwm(ast, 0x1E785054, 0x00000077);
+	ast_moutdwm(ast, AST_REG_WDT14(1), 0x00000077);
 	ast_moutdwm(ast, AST_REG_MCR00, AST_REG_MCR00_PROTECTION_KEY);
 }
 
@@ -508,8 +508,8 @@ static void ast_post_chip_2500(struct ast_device *ast)
 		ast_2500_patch_ahb(ast->regs);
 
 		/* Disable watchdog */
-		ast_moutdwm(ast, 0x1E78502C, 0x00000000);
-		ast_moutdwm(ast, 0x1E78504C, 0x00000000);
+		ast_moutdwm(ast, AST_REG_WDT2C(0), 0x00000000);
+		ast_moutdwm(ast, AST_REG_WDT0C(1), 0x00000000);
 
 		/*
 		 * Reset USB port to patch USB unknown device issue
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index 730072a3c2cd..5effe6897b51 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -196,4 +196,17 @@
 #define AST_REG_A2P(__offset)			(AST_REG_A2P_BASE + (__offset))
 #define AST_REG_A2P58				AST_REG_A2P(0x58)
 
+/*
+ * Watchdog timer (0x1e785000 - 0x1e785fff)
+ */
+
+#define AST_REG_WDT_BASE(__n)			(0x1e785000 + (__n) * 0x40)
+#define AST_REG_WDT(__n, __offset)		(AST_REG_WDT_BASE((__n)) + (__offset))
+#define AST_REG_WDT04(__n)			AST_REG_WDT((__n), 0x04)
+#define AST_REG_WDT08(__n)			AST_REG_WDT((__n), 0x08)
+#define AST_REG_WDT0C(__n)			AST_REG_WDT((__n), 0x0c)
+#define AST_REG_WDT14(__n)			AST_REG_WDT((__n), 0x14)
+#define AST_REG_WDT1C(__n)			AST_REG_WDT((__n), 0x1c)
+#define AST_REG_WDT2C(__n)			AST_REG_WDT((__n), 0x2c)
+
 #endif
-- 
2.53.0

Claude review: drm/ast: Use constants for WDT registers

[Claude Code Review Bot]

Patch Review

Good use of parameterized macros for the watchdog timer registers:
```c
#define AST_REG_WDT_BASE(__n)   (0x1e785000 + (__n) * 0x40)
#define AST_REG_WDT(__n, __offset) (AST_REG_WDT_BASE((__n)) + (__offset))
```

The WDT register spacing of 0x40 bytes per timer instance is cleanly abstracted. Verified the old addresses map correctly: WDT1 at 0x1e785040+ matches `AST_REG_WDT*(1)`.

---
Generated by Claude Code Patch Reviewer

[PATCH 08/15] drm/ast: Use constants for SDRAM registers

[Thomas Zimmermann]

Aspeed hardware allows for acceessing the SDRAM from the host. SDRAM
registers are located at the memory range at [0x80000000, 0xffffffff].

Refer to memory access with the macro AST_SDRAM(). Also add a TODO item
for the nonsensical documentation next to its caller.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2500.c | 17 ++++++++++-------
 drivers/gpu/drm/ast/ast_reg.h  |  7 +++++++
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2500.c b/drivers/gpu/drm/ast/ast_2500.c
index f0639f1ef062..39f401dd1f47 100644
--- a/drivers/gpu/drm/ast/ast_2500.c
+++ b/drivers/gpu/drm/ast/ast_2500.c
@@ -225,6 +225,9 @@ static void ddr_phy_init_2500(struct ast_device *ast)
 }
 
 /*
+ * TODO: Review and fix the comments. The function below only detects
+ *       up to 1 GiB of SDRAM.
+ *
  * Check DRAM Size
  * 1Gb : 0x80000000 ~ 0x87FFFFFF
  * 2Gb : 0x80000000 ~ 0x8FFFFFFF
@@ -238,21 +241,21 @@ static void check_dram_size_2500(struct ast_device *ast, u32 tRFC)
 	reg_04 = ast_mindwm(ast, AST_REG_MCR04) & 0xfffffffc;
 	reg_14 = ast_mindwm(ast, AST_REG_MCR14) & 0xffffff00;
 
-	ast_moutdwm(ast, 0xA0100000, 0x41424344);
-	ast_moutdwm(ast, 0x90100000, 0x35363738);
-	ast_moutdwm(ast, 0x88100000, 0x292A2B2C);
-	ast_moutdwm(ast, 0x80100000, 0x1D1E1F10);
+	ast_moutdwm(ast, AST_SDRAM(0x20100000), 0x41424344);
+	ast_moutdwm(ast, AST_SDRAM(0x10100000), 0x35363738);
+	ast_moutdwm(ast, AST_SDRAM(0x08100000), 0x292A2B2C);
+	ast_moutdwm(ast, AST_SDRAM(0x00100000), 0x1D1E1F10);
 
 	/* Check 8Gbit */
-	if (ast_mindwm(ast, 0xA0100000) == 0x41424344) {
+	if (ast_mindwm(ast, AST_SDRAM(0x20100000)) == 0x41424344) {
 		reg_04 |= 0x03;
 		reg_14 |= (tRFC >> 24) & 0xFF;
 		/* Check 4Gbit */
-	} else if (ast_mindwm(ast, 0x90100000) == 0x35363738) {
+	} else if (ast_mindwm(ast, AST_SDRAM(0x10100000)) == 0x35363738) {
 		reg_04 |= 0x02;
 		reg_14 |= (tRFC >> 16) & 0xFF;
 		/* Check 2Gbit */
-	} else if (ast_mindwm(ast, 0x88100000) == 0x292A2B2C) {
+	} else if (ast_mindwm(ast, AST_SDRAM(0x08100000)) == 0x292A2B2C) {
 		reg_04 |= 0x01;
 		reg_14 |= (tRFC >> 8) & 0xFF;
 	} else {
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index 5effe6897b51..78fabe2a9c81 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -209,4 +209,11 @@
 #define AST_REG_WDT1C(__n)			AST_REG_WDT((__n), 0x1c)
 #define AST_REG_WDT2C(__n)			AST_REG_WDT((__n), 0x2c)
 
+/*
+ * SDRAM (0x80000000 - 0xffffffff)
+ */
+
+#define AST_SDRAM_BASE				(0x80000000)
+#define AST_SDRAM(__offset)			(AST_SDRAM_BASE + (__offset))
+
 #endif
-- 
2.53.0

Claude review: drm/ast: Use constants for SDRAM registers

[Claude Code Review Bot]

Patch Review

Small patch introducing `AST_SDRAM()` macro. The TODO comment about the misleading function comments is a good annotation:
```c
+ * TODO: Review and fix the comments. The function below only detects
+ *       up to 1 GiB of SDRAM.
```

---
Generated by Claude Code Patch Reviewer

[PATCH 09/15] drm/ast: Store register addresses in struct ast_dramstruct

[Thomas Zimmermann]

Struct ast_dramstruct contains a 16-bit index field that either
contains a magic value or serves as index into the P2A address
segment at 0x1e600000. This segment serves MCR and SCU registers,
which the ast_dramstruct programs. It's fragile and relies upon
the ast_post_chip_*() functions to set up the segment correctly.

Replace the 16-bit index with a full 32-bit address of the SCU
and MCR addresses. Initialize the DRAM tables with full register
constants and write them out with ast_moutdwm(). This sets the
correct segment on each write.

Drop __AST_DRAMSTRUCT_DRAM_TYPE as it simply referred to MCR04.
Use the latter for initializing the DRAM tables.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2000.c |  43 ++++----
 drivers/gpu/drm/ast/ast_2100.c | 195 ++++++++++++++++-----------------
 drivers/gpu/drm/ast/ast_post.h |  17 +--
 drivers/gpu/drm/ast/ast_reg.h  |   4 +
 4 files changed, 131 insertions(+), 128 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2000.c b/drivers/gpu/drm/ast/ast_2000.c
index 204646bb1fa0..e683edf595e2 100644
--- a/drivers/gpu/drm/ast/ast_2000.c
+++ b/drivers/gpu/drm/ast/ast_2000.c
@@ -69,30 +69,30 @@ void ast_2000_set_def_ext_reg(struct ast_device *ast)
 }
 
 static const struct ast_dramstruct ast2000_dram_table_data[] = {
-	{ 0x0108, 0x00000000 },
-	{ 0x0120, 0x00004a21 },
+	AST_DRAMSTRUCT_REG(AST_REG_MCR108, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR120, 0x00004a21),
 	AST_DRAMSTRUCT_UDELAY(67u),
-	{ 0x0000, 0xFFFFFFFF },
-	AST_DRAMSTRUCT_INIT(DRAM_TYPE, 0x00000089),
-	{ 0x0008, 0x22331353 },
-	{ 0x000C, 0x0d07000b },
-	{ 0x0010, 0x11113333 },
-	{ 0x0020, 0x00110350 },
-	{ 0x0028, 0x1e0828f0 },
-	{ 0x0024, 0x00000001 },
-	{ 0x001C, 0x00000000 },
-	{ 0x0014, 0x00000003 },
+	AST_DRAMSTRUCT_REG(AST_REG_MCR00, 0xffffffff), /* FIXME: This locks the MCR registers. */
+	AST_DRAMSTRUCT_REG(AST_REG_MCR04, 0x00000089), /* DRAM type */
+	AST_DRAMSTRUCT_REG(AST_REG_MCR08, 0x22331353),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR0C, 0x0d07000b),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR10, 0x11113333),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR20, 0x00110350),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x1e0828f0),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR24, 0x00000001),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR1C, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR14, 0x00000003),
 	AST_DRAMSTRUCT_UDELAY(67u),
-	{ 0x0018, 0x00000131 },
-	{ 0x0014, 0x00000001 },
+	AST_DRAMSTRUCT_REG(AST_REG_MCR18, 0x00000131),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR14, 0x00000001),
 	AST_DRAMSTRUCT_UDELAY(67u),
-	{ 0x0018, 0x00000031 },
-	{ 0x0014, 0x00000001 },
+	AST_DRAMSTRUCT_REG(AST_REG_MCR18, 0x00000031),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR14, 0x00000001),
 	AST_DRAMSTRUCT_UDELAY(67u),
-	{ 0x0028, 0x1e0828f1 },
-	{ 0x0024, 0x00000003 },
-	{ 0x002C, 0x1f0f28fb },
-	{ 0x0030, 0xFFFFFE01 },
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x1e0828f1),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR24, 0x00000003),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR2C, 0x1f0f28fb),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR30, 0xfffffe01),
 	AST_DRAMSTRUCT_INVALID,
 };
 
@@ -119,8 +119,7 @@ static void ast_post_chip_2000(struct ast_device *ast)
 				for (i = 0; i < 15; i++)
 					udelay(dram_reg_info->data);
 			} else {
-				ast_write32(ast, 0x10000 + dram_reg_info->index,
-					    dram_reg_info->data);
+				ast_moutdwm(ast, dram_reg_info->index, dram_reg_info->data);
 			}
 			dram_reg_info++;
 		}
diff --git a/drivers/gpu/drm/ast/ast_2100.c b/drivers/gpu/drm/ast/ast_2100.c
index 28cd36571b7f..930e9f2d0ccf 100644
--- a/drivers/gpu/drm/ast/ast_2100.c
+++ b/drivers/gpu/drm/ast/ast_2100.c
@@ -72,108 +72,108 @@ static enum ast_dram_layout ast_2100_get_dram_layout_p2a(struct ast_device *ast)
  */
 
 static const struct ast_dramstruct ast1100_dram_table_data[] = {
-	{ 0x2000, 0x1688a8a8 },
-	{ 0x2020, 0x000041f0 },
+	AST_DRAMSTRUCT_REG(AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY),
+	AST_DRAMSTRUCT_REG(AST_REG_SCU020, 0x000041f0),
 	AST_DRAMSTRUCT_UDELAY(67u),
-	{ 0x0000, 0xfc600309 },
-	{ 0x006C, 0x00909090 },
-	{ 0x0064, 0x00050000 },
-	AST_DRAMSTRUCT_INIT(DRAM_TYPE, 0x00000585),
-	{ 0x0008, 0x0011030f },
-	{ 0x0010, 0x22201724 },
-	{ 0x0018, 0x1e29011a },
-	{ 0x0020, 0x00c82222 },
-	{ 0x0014, 0x01001523 },
-	{ 0x001C, 0x1024010d },
-	{ 0x0024, 0x00cb2522 },
-	{ 0x0038, 0xffffff82 },
-	{ 0x003C, 0x00000000 },
-	{ 0x0040, 0x00000000 },
-	{ 0x0044, 0x00000000 },
-	{ 0x0048, 0x00000000 },
-	{ 0x004C, 0x00000000 },
-	{ 0x0050, 0x00000000 },
-	{ 0x0054, 0x00000000 },
-	{ 0x0058, 0x00000000 },
-	{ 0x005C, 0x00000000 },
-	{ 0x0060, 0x032aa02a },
-	{ 0x0064, 0x002d3000 },
-	{ 0x0068, 0x00000000 },
-	{ 0x0070, 0x00000000 },
-	{ 0x0074, 0x00000000 },
-	{ 0x0078, 0x00000000 },
-	{ 0x007C, 0x00000000 },
-	{ 0x0034, 0x00000001 },
+	AST_DRAMSTRUCT_REG(AST_REG_MCR00, AST_REG_MCR00_PROTECTION_KEY),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR6C, 0x00909090),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR64, 0x00050000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR04, 0x00000585), // DRAM type
+	AST_DRAMSTRUCT_REG(AST_REG_MCR08, 0x0011030f),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR10, 0x22201724),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR18, 0x1e29011a),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR20, 0x00c82222),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR14, 0x01001523),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR1C, 0x1024010d),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR24, 0x00cb2522),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR38, 0xffffff82),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR3C, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR40, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR44, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR48, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR4C, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR50, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR54, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR58, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR5C, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR60, 0x032aa02a),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR64, 0x002d3000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR68, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR70, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR74, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR78, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR7C, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR34, 0x00000001),
 	AST_DRAMSTRUCT_UDELAY(67u),
-	{ 0x002C, 0x00000732 },
-	{ 0x0030, 0x00000040 },
-	{ 0x0028, 0x00000005 },
-	{ 0x0028, 0x00000007 },
-	{ 0x0028, 0x00000003 },
-	{ 0x0028, 0x00000001 },
-	{ 0x000C, 0x00005a08 },
-	{ 0x002C, 0x00000632 },
-	{ 0x0028, 0x00000001 },
-	{ 0x0030, 0x000003c0 },
-	{ 0x0028, 0x00000003 },
-	{ 0x0030, 0x00000040 },
-	{ 0x0028, 0x00000003 },
-	{ 0x000C, 0x00005a21 },
-	{ 0x0034, 0x00007c03 },
-	{ 0x0120, 0x00004c41 },
+	AST_DRAMSTRUCT_REG(AST_REG_MCR2C, 0x00000732),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR30, 0x00000040),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000005),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000007),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000003),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000001),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR0C, 0x00005a08),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR2C, 0x00000632),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000001),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR30, 0x000003c0),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000003),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR30, 0x00000040),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000003),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR0C, 0x00005a21),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR34, 0x00007c03),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR120, 0x00004c41),
 	AST_DRAMSTRUCT_INVALID,
 };
 
 static const struct ast_dramstruct ast2100_dram_table_data[] = {
-	{ 0x2000, 0x1688a8a8 },
-	{ 0x2020, 0x00004120 },
+	AST_DRAMSTRUCT_REG(AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY),
+	AST_DRAMSTRUCT_REG(AST_REG_SCU020, 0x00004120),
 	AST_DRAMSTRUCT_UDELAY(67u),
-	{ 0x0000, 0xfc600309 },
-	{ 0x006C, 0x00909090 },
-	{ 0x0064, 0x00070000 },
-	AST_DRAMSTRUCT_INIT(DRAM_TYPE, 0x00000489),
-	{ 0x0008, 0x0011030f },
-	{ 0x0010, 0x32302926 },
-	{ 0x0018, 0x274c0122 },
-	{ 0x0020, 0x00ce2222 },
-	{ 0x0014, 0x01001523 },
-	{ 0x001C, 0x1024010d },
-	{ 0x0024, 0x00cb2522 },
-	{ 0x0038, 0xffffff82 },
-	{ 0x003C, 0x00000000 },
-	{ 0x0040, 0x00000000 },
-	{ 0x0044, 0x00000000 },
-	{ 0x0048, 0x00000000 },
-	{ 0x004C, 0x00000000 },
-	{ 0x0050, 0x00000000 },
-	{ 0x0054, 0x00000000 },
-	{ 0x0058, 0x00000000 },
-	{ 0x005C, 0x00000000 },
-	{ 0x0060, 0x0f2aa02a },
-	{ 0x0064, 0x003f3005 },
-	{ 0x0068, 0x02020202 },
-	{ 0x0070, 0x00000000 },
-	{ 0x0074, 0x00000000 },
-	{ 0x0078, 0x00000000 },
-	{ 0x007C, 0x00000000 },
-	{ 0x0034, 0x00000001 },
+	AST_DRAMSTRUCT_REG(AST_REG_MCR00, AST_REG_MCR00_PROTECTION_KEY),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR6C, 0x00909090),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR64, 0x00070000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR04, 0x00000489), // DRAM type
+	AST_DRAMSTRUCT_REG(AST_REG_MCR08, 0x0011030f),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR10, 0x32302926),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR18, 0x274c0122),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR20, 0x00ce2222),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR14, 0x01001523),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR1C, 0x1024010d),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR24, 0x00cb2522),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR38, 0xffffff82),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR3C, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR40, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR44, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR48, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR4C, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR50, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR54, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR58, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR5C, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR60, 0x0f2aa02a),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR64, 0x003f3005),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR68, 0x02020202),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR70, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR74, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR78, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR7C, 0x00000000),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR34, 0x00000001),
 	AST_DRAMSTRUCT_UDELAY(67u),
-	{ 0x002C, 0x00000942 },
-	{ 0x0030, 0x00000040 },
-	{ 0x0028, 0x00000005 },
-	{ 0x0028, 0x00000007 },
-	{ 0x0028, 0x00000003 },
-	{ 0x0028, 0x00000001 },
-	{ 0x000C, 0x00005a08 },
-	{ 0x002C, 0x00000842 },
-	{ 0x0028, 0x00000001 },
-	{ 0x0030, 0x000003c0 },
-	{ 0x0028, 0x00000003 },
-	{ 0x0030, 0x00000040 },
-	{ 0x0028, 0x00000003 },
-	{ 0x000C, 0x00005a21 },
-	{ 0x0034, 0x00007c03 },
-	{ 0x0120, 0x00005061 },
+	AST_DRAMSTRUCT_REG(AST_REG_MCR2C, 0x00000942),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR30, 0x00000040),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000005),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000007),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000003),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000001),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR0C, 0x00005a08),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR2C, 0x00000842),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000001),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR30, 0x000003c0),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000003),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR30, 0x00000040),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR28, 0x00000003),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR0C, 0x00005a21),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR34, 0x00007c03),
+	AST_DRAMSTRUCT_REG(AST_REG_MCR120, 0x00005061),
 	AST_DRAMSTRUCT_INVALID,
 };
 
@@ -328,7 +328,7 @@ static void ast_post_chip_2100(struct ast_device *ast)
 			if (AST_DRAMSTRUCT_IS(dram_reg_info, UDELAY)) {
 				for (i = 0; i < 15; i++)
 					udelay(dram_reg_info->data);
-			} else if (AST_DRAMSTRUCT_IS(dram_reg_info, DRAM_TYPE)) {
+			} else if (AST_DRAMSTRUCT_IS_REG(dram_reg_info, AST_REG_MCR04)) {
 				switch (dram_layout) {
 				case AST_DRAM_1Gx16:
 					data = 0x00000d89;
@@ -344,10 +344,9 @@ static void ast_post_chip_2100(struct ast_device *ast)
 				temp = ast_read32(ast, 0x12070);
 				temp &= 0xc;
 				temp <<= 2;
-				ast_write32(ast, 0x10000 + dram_reg_info->index, data | temp);
+				ast_moutdwm(ast, dram_reg_info->index, data | temp);
 			} else {
-				ast_write32(ast, 0x10000 + dram_reg_info->index,
-					    dram_reg_info->data);
+				ast_moutdwm(ast, dram_reg_info->index, dram_reg_info->data);
 			}
 			dram_reg_info++;
 		}
diff --git a/drivers/gpu/drm/ast/ast_post.h b/drivers/gpu/drm/ast/ast_post.h
index 41cd753b7f67..41b2db08a870 100644
--- a/drivers/gpu/drm/ast/ast_post.h
+++ b/drivers/gpu/drm/ast/ast_post.h
@@ -10,13 +10,10 @@ struct ast_device;
 
 /* DRAM timing tables */
 struct ast_dramstruct {
-	u16 index;
+	u32 index;
 	u32 data;
 };
 
-/* hardware fields */
-#define __AST_DRAMSTRUCT_DRAM_TYPE      0x0004
-
 /* control commands */
 #define __AST_DRAMSTRUCT_UDELAY         0xff00
 #define __AST_DRAMSTRUCT_INVALID        0xffff
@@ -24,14 +21,18 @@ struct ast_dramstruct {
 #define __AST_DRAMSTRUCT_INDEX(_name) \
 	(__AST_DRAMSTRUCT_ ## _name)
 
-#define AST_DRAMSTRUCT_INIT(_name, _value) \
-	{ __AST_DRAMSTRUCT_INDEX(_name), (_value) }
+#define __AST_DRAMSTRUCT_INIT(_index, _value) \
+	{ (_index), (_value) }
 
+#define AST_DRAMSTRUCT_REG(_reg, _value) \
+	__AST_DRAMSTRUCT_INIT(_reg, _value)
 #define AST_DRAMSTRUCT_UDELAY(_usecs) \
-	AST_DRAMSTRUCT_INIT(UDELAY, _usecs)
+	__AST_DRAMSTRUCT_INIT(__AST_DRAMSTRUCT_UDELAY, _usecs)
 #define AST_DRAMSTRUCT_INVALID \
-	AST_DRAMSTRUCT_INIT(INVALID, U32_MAX)
+	__AST_DRAMSTRUCT_INIT(__AST_DRAMSTRUCT_INVALID, U32_MAX)
 
+#define AST_DRAMSTRUCT_IS_REG(_entry, _reg) \
+	((_entry)->index == (_reg))
 #define AST_DRAMSTRUCT_IS(_entry, _name) \
 	((_entry)->index == __AST_DRAMSTRUCT_INDEX(_name))
 
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index 78fabe2a9c81..a01af2bfbae6 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -120,12 +120,15 @@
 #define AST_REG_MCR30				AST_REG_MCR(0x30)
 #define AST_REG_MCR34				AST_REG_MCR(0x34)
 #define AST_REG_MCR38				AST_REG_MCR(0x38)
+#define AST_REG_MCR3C				AST_REG_MCR(0x3c)
 #define AST_REG_MCR40				AST_REG_MCR(0x40)
 #define AST_REG_MCR44				AST_REG_MCR(0x44)
 #define AST_REG_MCR48				AST_REG_MCR(0x48)
 #define AST_REG_MCR4C				AST_REG_MCR(0x4C)
 #define AST_REG_MCR50				AST_REG_MCR(0x50)
 #define AST_REG_MCR54				AST_REG_MCR(0x54)
+#define AST_REG_MCR58				AST_REG_MCR(0x58)
+#define AST_REG_MCR5C				AST_REG_MCR(0x5c)
 #define AST_REG_MCR60				AST_REG_MCR(0x60)
 #define AST_REG_MCR64				AST_REG_MCR(0x64)
 #define AST_REG_MCR68				AST_REG_MCR(0x68)
@@ -137,6 +140,7 @@
 #define AST_REG_MCR80				AST_REG_MCR(0x80)
 #define AST_REG_MCR84				AST_REG_MCR(0x84)
 #define AST_REG_MCR88				AST_REG_MCR(0x88)
+#define AST_REG_MCR108				AST_REG_MCR(0x108)
 #define AST_REG_MCR120				AST_REG_MCR(0x120)
 #define AST_REG_MCR200				AST_REG_MCR(0x200)
 #define AST_REG_MCR204				AST_REG_MCR(0x204)
-- 
2.53.0

Claude review: drm/ast: Store register addresses in struct ast_dramstruct

[Claude Code Review Bot]

Patch Review

This is a substantive change - widening `ast_dramstruct.index` from `u16` to `u32` to hold full register addresses instead of segment offsets. The approach is sound: it makes the DRAM tables self-contained with full addresses, removing the fragile dependency on the correct P2A segment being pre-selected.

The new `AST_DRAMSTRUCT_REG()` macro and `AST_DRAMSTRUCT_IS_REG()` are clean replacements. The removal of `__AST_DRAMSTRUCT_DRAM_TYPE` in favor of direct `AST_REG_MCR04` comparison is good.

Minor concern: the control commands `__AST_DRAMSTRUCT_UDELAY` (0xff00) and `__AST_DRAMSTRUCT_INVALID` (0xffff) are now in the same u32 namespace as real register addresses. These values don't collide with any AST register address, but a comment noting this invariant might be helpful for future maintenance. There's no realistic collision risk though.

Note: `// DRAM type` comments in the table use C++ style (`//`) rather than C style (`/* */`). The kernel coding style prefers `/* */`.

---
Generated by Claude Code Patch Reviewer

[PATCH 10/15] drm/ast: Gen1: Fix open-coded register access

[Thomas Zimmermann]

Replace all open-coded access to MCR registers in Gen1 with the
appropriate calls to ast_moutdwm() and ast_mindwm(). Use MCR register
constants.

For the poll loop on MCR100, add ast_moutdwm_poll(). The helper polls
the register until it has been updated to the given value. Relax the
CPU while busy-waiting.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2000.c | 18 +++++++-----------
 drivers/gpu/drm/ast/ast_drv.c  | 12 ++++++++++++
 drivers/gpu/drm/ast/ast_drv.h  |  1 +
 drivers/gpu/drm/ast/ast_reg.h  |  2 ++
 4 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2000.c b/drivers/gpu/drm/ast/ast_2000.c
index e683edf595e2..4cf951b3533d 100644
--- a/drivers/gpu/drm/ast/ast_2000.c
+++ b/drivers/gpu/drm/ast/ast_2000.c
@@ -99,20 +99,15 @@ static const struct ast_dramstruct ast2000_dram_table_data[] = {
 static void ast_post_chip_2000(struct ast_device *ast)
 {
 	u8 j;
-	u32 temp, i;
-	const struct ast_dramstruct *dram_reg_info;
+	u32 i;
 
 	j = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd0, 0xff);
 
 	if ((j & 0x80) == 0) { /* VGA only */
-		dram_reg_info = ast2000_dram_table_data;
-		ast_write32(ast, 0xf004, AST_REG_MCR00);
-		ast_write32(ast, 0xf000, 0x1);
-		ast_write32(ast, 0x10100, 0xa8);
+		const struct ast_dramstruct *dram_reg_info = ast2000_dram_table_data;
+		u32 mcr140;
 
-		do {
-			;
-		} while (ast_read32(ast, 0x10100) != 0xa8);
+		ast_moutdwm_poll(ast, AST_REG_MCR100, 0xa8, 0xa8);
 
 		while (!AST_DRAMSTRUCT_IS(dram_reg_info, INVALID)) {
 			if (AST_DRAMSTRUCT_IS(dram_reg_info, UDELAY)) {
@@ -124,8 +119,9 @@ static void ast_post_chip_2000(struct ast_device *ast)
 			dram_reg_info++;
 		}
 
-		temp = ast_read32(ast, 0x10140);
-		ast_write32(ast, 0x10140, temp | 0x40);
+		mcr140 = ast_mindwm(ast, AST_REG_MCR140);
+		mcr140 |= 0x00000040;
+		ast_moutdwm(ast, AST_REG_MCR140, mcr140);
 	}
 
 	/* wait ready */
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 3da0cce0a3f6..6fe549f16309 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -102,6 +102,18 @@ void ast_moutdwm(struct ast_device *ast, u32 r, u32 v)
 	__ast_moutdwm(ast->regs, r, v);
 }
 
+void ast_moutdwm_poll(struct ast_device *ast, u32 r, u32 v, u32 res)
+{
+	void __iomem *regs = ast->regs;
+
+	__ast_selseg(regs, r);
+	__ast_wrseg32(regs, r, v);
+
+	do {
+		cpu_relax();
+	} while (__ast_rdseg32(regs, r) != res);
+}
+
 /*
  * AST device
  */
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 3eedf8239333..4f221b848d68 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -361,6 +361,7 @@ u32 __ast_mindwm(void __iomem *regs, u32 r);
 void __ast_moutdwm(void __iomem *regs, u32 r, u32 v);
 u32 ast_mindwm(struct ast_device *ast, u32 r);
 void ast_moutdwm(struct ast_device *ast, u32 r, u32 v);
+void ast_moutdwm_poll(struct ast_device *ast, u32 r, u32 v, u32 res);
 
 /*
  * VBIOS
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index a01af2bfbae6..9ebdbbde9a47 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -140,8 +140,10 @@
 #define AST_REG_MCR80				AST_REG_MCR(0x80)
 #define AST_REG_MCR84				AST_REG_MCR(0x84)
 #define AST_REG_MCR88				AST_REG_MCR(0x88)
+#define AST_REG_MCR100				AST_REG_MCR(0x100)
 #define AST_REG_MCR108				AST_REG_MCR(0x108)
 #define AST_REG_MCR120				AST_REG_MCR(0x120)
+#define AST_REG_MCR140				AST_REG_MCR(0x140)
 #define AST_REG_MCR200				AST_REG_MCR(0x200)
 #define AST_REG_MCR204				AST_REG_MCR(0x204)
 #define AST_REG_MCR208				AST_REG_MCR(0x208)
-- 
2.53.0

Claude review: drm/ast: Gen1: Fix open-coded register access

[Claude Code Review Bot]

Patch Review

Good cleanup. The new `ast_moutdwm_poll()` helper is a useful addition:
```c
void ast_moutdwm_poll(struct ast_device *ast, u32 r, u32 v, u32 res)
```

This replaces the pattern of writing then busy-polling until the register reaches a target value. The `cpu_relax()` call in the poll loop is a nice improvement over the original tight `do { ; } while()` loops.

**Potential concern**: The original Gen1 code did:
```c
ast_write32(ast, 0xf004, AST_REG_MCR00);
ast_write32(ast, 0xf000, 0x1);
ast_write32(ast, 0x10100, 0xa8);
do { ; } while (ast_read32(ast, 0x10100) != 0xa8);
```
This writes directly to the P2A mapped region at offset 0x10100 (which is MCR100 after segment setup), and polls the same. The replacement uses `ast_moutdwm_poll(ast, AST_REG_MCR100, 0xa8, 0xa8)`, which will do a segment select + write + poll. This is functionally equivalent but the segment was previously set to MCR by the preceding `ast_write32(ast, 0xf004, ...)` call that is now removed. The `ast_moutdwm_poll` handles segment selection internally, so this is correct.

---
Generated by Claude Code Patch Reviewer

[PATCH 11/15] drm/ast: Gen2: Fix open-coded register access

[Thomas Zimmermann]

Replace all open-coded access to MCR and SCU registers in Gen2 with the
appropriate calls to ast_moutdwm() and ast_mindwm(). Use MCR and SCU
register constants. Name variables according to registers.

The values in MCR04 that control VRAM allocation do not look correct.
Leave a FIXME comment for later investigation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2100.c | 73 ++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2100.c b/drivers/gpu/drm/ast/ast_2100.c
index 930e9f2d0ccf..5cf8ad9c3446 100644
--- a/drivers/gpu/drm/ast/ast_2100.c
+++ b/drivers/gpu/drm/ast/ast_2100.c
@@ -40,21 +40,19 @@
 
 static enum ast_dram_layout ast_2100_get_dram_layout_p2a(struct ast_device *ast)
 {
-	u32 mcr_cfg;
+	u32 mcr04;
 	enum ast_dram_layout dram_layout;
 
-	ast_write32(ast, 0xf004, AST_REG_MCR00);
-	ast_write32(ast, 0xf000, 0x1);
-	mcr_cfg = ast_read32(ast, 0x10004);
+	mcr04 = ast_mindwm(ast, AST_REG_MCR04);
 
-	switch (mcr_cfg & 0x0c) {
+	switch (mcr04 & GENMASK(3, 2)) {
 	case 0:
 	case 4:
 	default:
 		dram_layout = AST_DRAM_512Mx16;
 		break;
 	case 8:
-		if (mcr_cfg & 0x40)
+		if (mcr04 & 0x40)
 			dram_layout = AST_DRAM_1Gx16;
 		else
 			dram_layout = AST_DRAM_512Mx32;
@@ -300,51 +298,52 @@ static void cbrdlli_ast2150(struct ast_device *ast, int busw)
 static void ast_post_chip_2100(struct ast_device *ast)
 {
 	u8 j;
-	u32 data, temp, i;
-	const struct ast_dramstruct *dram_reg_info;
-	enum ast_dram_layout dram_layout  = ast_2100_get_dram_layout_p2a(ast);
+	u32 i;
+	enum ast_dram_layout dram_layout = ast_2100_get_dram_layout_p2a(ast);
+	u32 mcr120;
+	u32 scu00c, scu040;
 
 	j = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd0, 0xff);
 
 	if ((j & 0x80) == 0) { /* VGA only */
+		const struct ast_dramstruct *dram_reg_info;
+
 		if (ast->chip == AST2100 || ast->chip == AST2200)
 			dram_reg_info = ast2100_dram_table_data;
 		else
 			dram_reg_info = ast1100_dram_table_data;
 
-		ast_write32(ast, 0xf004, AST_REG_MCR00);
-		ast_write32(ast, 0xf000, 0x1);
-		ast_write32(ast, 0x12000, 0x1688A8A8);
-		do {
-			;
-		} while (ast_read32(ast, 0x12000) != 0x01);
-
-		ast_write32(ast, 0x10000, 0xfc600309);
-		do {
-			;
-		} while (ast_read32(ast, 0x10000) != 0x01);
+		ast_moutdwm_poll(ast, AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY, 0x01);
+		ast_moutdwm_poll(ast, AST_REG_MCR00, AST_REG_MCR00_PROTECTION_KEY, 0x01);
 
 		while (!AST_DRAMSTRUCT_IS(dram_reg_info, INVALID)) {
 			if (AST_DRAMSTRUCT_IS(dram_reg_info, UDELAY)) {
 				for (i = 0; i < 15; i++)
 					udelay(dram_reg_info->data);
 			} else if (AST_DRAMSTRUCT_IS_REG(dram_reg_info, AST_REG_MCR04)) {
+				u32 mcr04;
+				u32 scu070;
+
 				switch (dram_layout) {
 				case AST_DRAM_1Gx16:
-					data = 0x00000d89;
+					mcr04 = 0x00000d89;
 					break;
 				case AST_DRAM_1Gx32:
-					data = 0x00000c8d;
+					mcr04 = 0x00000c8d;
 					break;
 				default:
-					data = dram_reg_info->data;
+					mcr04 = dram_reg_info->data;
 					break;
 				}
 
-				temp = ast_read32(ast, 0x12070);
-				temp &= 0xc;
-				temp <<= 2;
-				ast_moutdwm(ast, dram_reg_info->index, data | temp);
+				/*
+				 * FIXME: There might be bits already in MCR04[5:4]. Should
+				 *        we only do this in the default case?
+				 */
+				scu070 = ast_mindwm(ast, AST_REG_SCU070);
+				mcr04 |= (scu070 & GENMASK(3, 2)) << 2;
+
+				ast_moutdwm(ast, dram_reg_info->index, mcr04);
 			} else {
 				ast_moutdwm(ast, dram_reg_info->index, dram_reg_info->data);
 			}
@@ -352,19 +351,23 @@ static void ast_post_chip_2100(struct ast_device *ast)
 		}
 
 		/* AST 2100/2150 DRAM calibration */
-		data = ast_read32(ast, 0x10120);
-		if (data == 0x5061) { /* 266Mhz */
-			data = ast_read32(ast, 0x10004);
-			if (data & 0x40)
+		mcr120 = ast_mindwm(ast, AST_REG_MCR120);
+		if (mcr120 == 0x5061) { /* 266Mhz */
+			u32 mcr04 = ast_mindwm(ast, AST_REG_MCR04);
+
+			if (mcr04 & 0x40)
 				cbrdlli_ast2150(ast, 16); /* 16 bits */
 			else
 				cbrdlli_ast2150(ast, 32); /* 32 bits */
 		}
 
-		temp = ast_read32(ast, 0x1200c);
-		ast_write32(ast, 0x1200c, temp & 0xfffffffd);
-		temp = ast_read32(ast, 0x12040);
-		ast_write32(ast, 0x12040, temp | 0x40);
+		scu00c = ast_mindwm(ast, AST_REG_SCU00C);
+		scu00c &= 0xfffffffd;
+		ast_moutdwm(ast, AST_REG_SCU00C, scu00c);
+
+		scu040 = ast_mindwm(ast, AST_REG_SCU040);
+		scu040 |= 0x00000040;
+		ast_moutdwm(ast, AST_REG_SCU040, scu040);
 	}
 
 	/* wait ready */
-- 
2.53.0

Claude review: drm/ast: Gen2: Fix open-coded register access

[Claude Code Review Bot]

Patch Review

Good cleanup. The FIXME comment about MCR04[5:4] is a worthwhile observation:
```c
/*
 * FIXME: There might be bits already in MCR04[5:4]. Should
 *        we only do this in the default case?
 */
scu070 = ast_mindwm(ast, AST_REG_SCU070);
mcr04 |= (scu070 & GENMASK(3, 2)) << 2;
```

The `ast_moutdwm_poll` calls for SCU and MCR unlock correctly replace the old manual segment setup + busy-wait patterns.

---
Generated by Claude Code Patch Reviewer

[PATCH 12/15] drm/ast: Gen4: Fix open-coded register access

[Thomas Zimmermann]

Replace all open-coded access to MCR and SCU registers in Gen4 with the
appropriate calls to ast_moutdwm() and ast_mindwm(). Use MCR and SCU
register constants. Name variables according to registers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2300.c | 24 +++++++++---------------
 drivers/gpu/drm/ast/ast_reg.h  |  1 +
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2300.c b/drivers/gpu/drm/ast/ast_2300.c
index 96d9909d942c..cbeac6990c27 100644
--- a/drivers/gpu/drm/ast/ast_2300.c
+++ b/drivers/gpu/drm/ast/ast_2300.c
@@ -1245,22 +1245,15 @@ static void ast_post_chip_2300(struct ast_device *ast)
 
 	reg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd0, 0xff);
 	if ((reg & 0x80) == 0) {/* vga only */
-		ast_write32(ast, 0xf004, AST_REG_MCR00);
-		ast_write32(ast, 0xf000, 0x1);
-		ast_write32(ast, 0x12000, 0x1688a8a8);
-		do {
-			;
-		} while (ast_read32(ast, 0x12000) != 0x1);
+		u32 scu008, scu040;
 
-		ast_write32(ast, 0x10000, 0xfc600309);
-		do {
-			;
-		} while (ast_read32(ast, 0x10000) != 0x1);
+		ast_moutdwm_poll(ast, AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY, 0x01);
+		ast_moutdwm_poll(ast, AST_REG_MCR00, AST_REG_MCR00_PROTECTION_KEY, 0x01);
 
 		/* Slow down CPU/AHB CLK in VGA only mode */
-		temp = ast_read32(ast, 0x12008);
-		temp |= 0x73;
-		ast_write32(ast, 0x12008, temp);
+		scu008 = ast_mindwm(ast, AST_REG_SCU008);
+		scu008 |= 0x00000073;
+		ast_moutdwm(ast, AST_REG_SCU008, scu008);
 
 		param.dram_freq = 396;
 		param.dram_type = AST_DDR3;
@@ -1306,8 +1299,9 @@ static void ast_post_chip_2300(struct ast_device *ast)
 			ddr2_init(ast, &param);
 		}
 
-		temp = ast_mindwm(ast, AST_REG_SCU040);
-		ast_moutdwm(ast, AST_REG_SCU040, temp | 0x40);
+		scu040 = ast_mindwm(ast, AST_REG_SCU040);
+		scu040 |= 0x00000040;
+		ast_moutdwm(ast, AST_REG_SCU040, scu040);
 	}
 
 	/* wait ready */
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index 9ebdbbde9a47..c3473cad4545 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -183,6 +183,7 @@
 #define AST_REG_SCU(__offset)			(AST_REG_SCU_BASE + (__offset))
 #define AST_REG_SCU000				AST_REG_SCU(0x000)
 #define AST_REG_SCU000_PROTECTION_KEY		(0x1688a8a8)
+#define AST_REG_SCU008				AST_REG_SCU(0x008)
 #define AST_REG_SCU00C				AST_REG_SCU(0x00c)
 #define AST_REG_SCU020				AST_REG_SCU(0x020)
 #define AST_REG_SCU040				AST_REG_SCU(0x040)
-- 
2.53.0

Claude review: drm/ast: Gen4: Fix open-coded register access

[Claude Code Review Bot]

Patch Review

Clean conversion. Same pattern as the Gen1/Gen2 patches. No issues.

---
Generated by Claude Code Patch Reviewer

[PATCH 13/15] drm/ast: Gen6: Fix open-coded register access

[Thomas Zimmermann]

Replace all open-coded access to SCU registers in Gen6 with the
appropriate calls to ast_moutdwm() and ast_mindwm(). Use SCU register
constants. Name variables according to registers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2500.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2500.c b/drivers/gpu/drm/ast/ast_2500.c
index 39f401dd1f47..e442cb6a23d9 100644
--- a/drivers/gpu/drm/ast/ast_2500.c
+++ b/drivers/gpu/drm/ast/ast_2500.c
@@ -507,6 +507,8 @@ static void ast_post_chip_2500(struct ast_device *ast)
 
 	reg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd0, 0xff);
 	if ((reg & AST_IO_VGACRD0_VRAM_INIT_STATUS_MASK) == 0) {/* vga only */
+		u32 scu008;
+
 		/* Clear bus lock condition */
 		ast_2500_patch_ahb(ast->regs);
 
@@ -540,9 +542,9 @@ static void ast_post_chip_2500(struct ast_device *ast)
 			ast_moutdwm(ast, AST_REG_SCU07C, 0x00004000);
 
 		/* Slow down CPU/AHB CLK in VGA only mode */
-		temp = ast_read32(ast, 0x12008);
-		temp |= 0x73;
-		ast_write32(ast, 0x12008, temp);
+		scu008 = ast_mindwm(ast, AST_REG_SCU008);
+		scu008 |= 0x00000073;
+		ast_moutdwm(ast, AST_REG_SCU008, scu008);
 
 		if (!ast_dram_init_2500(ast))
 			drm_err(dev, "DRAM init failed !\n");
-- 
2.53.0

Claude review: drm/ast: Gen6: Fix open-coded register access

[Claude Code Review Bot]

Patch Review

Very small patch (3 lines changed). Replaces `ast_read32(ast, 0x12008)` with `ast_mindwm(ast, AST_REG_SCU008)`. No issues.

---
Generated by Claude Code Patch Reviewer

[PATCH 14/15] drm/ast: dp501: Fix open-coded register access

[Thomas Zimmermann]

Replace all open-coded access to SCU registers in DP501 support with
the appropriate calls to ast_moutdwm() and ast_mindwm(). Use SCU register
constants. Name variables according to registers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_dp501.c | 124 +++++++++++++++-----------------
 drivers/gpu/drm/ast/ast_reg.h   |   6 ++
 2 files changed, 64 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 23142119d733..98d8491ce6c9 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -347,68 +347,65 @@ static int ast_dp512_read_edid_block(void *data, u8 *buf, unsigned int block, si
 static bool ast_init_dvo(struct ast_device *ast)
 {
 	u8 jreg;
-	u32 data;
-	ast_write32(ast, 0xf004, AST_REG_MCR00);
-	ast_write32(ast, 0xf000, 0x1);
-	ast_write32(ast, 0x12000, 0x1688a8a8);
+	u32 scu02c;
+
+	ast_moutdwm(ast, AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY);
 
 	jreg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd0, 0xff);
 	if (!(jreg & 0x80)) {
+		u32 scu008;
+
 		/* Init SCU DVO Settings */
-		data = ast_read32(ast, 0x12008);
-		/* delay phase */
-		data &= 0xfffff8ff;
-		data |= 0x00000500;
-		ast_write32(ast, 0x12008, data);
+
+		scu008 = ast_mindwm(ast, AST_REG_SCU008);
+		scu008 &= 0xfffff8ff;
+		scu008 |= 0x00000500; /* delay phase */
+		ast_moutdwm(ast, AST_REG_SCU008, scu008);
 
 		if (IS_AST_GEN4(ast)) {
-			data = ast_read32(ast, 0x12084);
-			/* multi-pins for DVO single-edge */
-			data |= 0xfffe0000;
-			ast_write32(ast, 0x12084, data);
-
-			data = ast_read32(ast, 0x12088);
-			/* multi-pins for DVO single-edge */
-			data |= 0x000fffff;
-			ast_write32(ast, 0x12088, data);
-
-			data = ast_read32(ast, 0x12090);
-			/* multi-pins for DVO single-edge */
-			data &= 0xffffffcf;
-			data |= 0x00000020;
-			ast_write32(ast, 0x12090, data);
+			u32 scu084, scu088, scu090;
+
+			scu084 = ast_mindwm(ast, AST_REG_SCU084);
+			scu084 |= 0xfffe0000; /* multi-pins for DVO single-edge */
+			ast_moutdwm(ast, AST_REG_SCU084, scu084);
+
+			scu088 = ast_mindwm(ast, AST_REG_SCU088);
+			scu088 |= 0x000fffff; /* multi-pins for DVO single-edge */
+			ast_moutdwm(ast, AST_REG_SCU088, scu088);
+
+			scu090 = ast_mindwm(ast, AST_REG_SCU090);
+			scu090 &= 0xffffffcf;
+			scu090 |= 0x00000020; /* multi-pins for DVO single-edge */
+			ast_moutdwm(ast, AST_REG_SCU090, scu090);
 		} else { /* AST GEN5+ */
-			data = ast_read32(ast, 0x12088);
-			/* multi-pins for DVO single-edge */
-			data |= 0x30000000;
-			ast_write32(ast, 0x12088, data);
-
-			data = ast_read32(ast, 0x1208c);
-			/* multi-pins for DVO single-edge */
-			data |= 0x000000cf;
-			ast_write32(ast, 0x1208c, data);
-
-			data = ast_read32(ast, 0x120a4);
-			/* multi-pins for DVO single-edge */
-			data |= 0xffff0000;
-			ast_write32(ast, 0x120a4, data);
-
-			data = ast_read32(ast, 0x120a8);
-			/* multi-pins for DVO single-edge */
-			data |= 0x0000000f;
-			ast_write32(ast, 0x120a8, data);
-
-			data = ast_read32(ast, 0x12094);
-			/* multi-pins for DVO single-edge */
-			data |= 0x00000002;
-			ast_write32(ast, 0x12094, data);
+			u32 scu088, scu08c, scu0a4, scu0a8, scu094;
+
+			scu088 = ast_mindwm(ast, AST_REG_SCU088);
+			scu088 |= 0x30000000; /* multi-pins for DVO single-edge */
+			ast_moutdwm(ast, AST_REG_SCU088, scu088);
+
+			scu08c = ast_mindwm(ast, AST_REG_SCU08C);
+			scu08c |= 0x000000cf; /* multi-pins for DVO single-edge */
+			ast_moutdwm(ast, AST_REG_SCU08C, scu08c);
+
+			scu0a4 = ast_mindwm(ast, AST_REG_SCU0A4);
+			scu0a4 |= 0xffff0000; /* multi-pins for DVO single-edge */
+			ast_moutdwm(ast, AST_REG_SCU0A4, scu0a4);
+
+			scu0a8 = ast_mindwm(ast, AST_REG_SCU0A8);
+			scu0a8 |= 0x0000000f; /* multi-pins for DVO single-edge */
+			ast_moutdwm(ast, AST_REG_SCU0A8, scu0a8);
+
+			scu094 = ast_mindwm(ast, AST_REG_SCU094);
+			scu094 |= 0x00000002; /* multi-pins for DVO single-edge */
+			ast_moutdwm(ast, AST_REG_SCU094, scu094);
 		}
 	}
 
 	/* Force to DVO */
-	data = ast_read32(ast, 0x1202c);
-	data &= 0xfffbffff;
-	ast_write32(ast, 0x1202c, data);
+	scu02c = ast_mindwm(ast, AST_REG_SCU02C);
+	scu02c &= 0xfffbffff;
+	ast_moutdwm(ast, AST_REG_SCU02C, scu02c);
 
 	/* Init VGA DVO Settings */
 	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xa3, 0xcf, 0x80);
@@ -418,25 +415,20 @@ static bool ast_init_dvo(struct ast_device *ast)
 
 static void ast_init_analog(struct ast_device *ast)
 {
-	u32 data;
+	u32 scu02c;
 
 	/*
 	 * Set DAC source to VGA mode in SCU2C via the P2A
-	 * bridge. First configure the P2U to target the SCU
-	 * in case it isn't at this stage.
+	 * bridge.
 	 */
-	ast_write32(ast, 0xf004, AST_REG_MCR00);
-	ast_write32(ast, 0xf000, 0x1);
-
-	/* Then unlock the SCU with the magic password */
-	ast_write32(ast, 0x12000, 0x1688a8a8);
-	ast_write32(ast, 0x12000, 0x1688a8a8);
-	ast_write32(ast, 0x12000, 0x1688a8a8);
-
-	/* Finally, clear bits [17:16] of SCU2c */
-	data = ast_read32(ast, 0x1202c);
-	data &= 0xfffcffff;
-	ast_write32(ast, 0x1202c, data);
+
+	/* Unlock the SCU with the magic password */
+	ast_moutdwm_poll(ast, AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY, 0x01);
+
+	/* Clear bits [17:16] of SCU2C */
+	scu02c = ast_mindwm(ast, AST_REG_SCU02C);
+	scu02c &= 0xfffcffff;
+	ast_moutdwm(ast, AST_REG_SCU02C, scu02c);
 
 	/* Disable DVO */
 	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xa3, 0xcf, 0x00);
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index c3473cad4545..9d6ea0765990 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -186,11 +186,17 @@
 #define AST_REG_SCU008				AST_REG_SCU(0x008)
 #define AST_REG_SCU00C				AST_REG_SCU(0x00c)
 #define AST_REG_SCU020				AST_REG_SCU(0x020)
+#define AST_REG_SCU02C				AST_REG_SCU(0x02c)
 #define AST_REG_SCU040				AST_REG_SCU(0x040)
 #define AST_REG_SCU070				AST_REG_SCU(0x070)
 #define AST_REG_SCU07C				AST_REG_SCU(0x07c)
+#define AST_REG_SCU084				AST_REG_SCU(0x084)
+#define AST_REG_SCU088				AST_REG_SCU(0x088)
+#define AST_REG_SCU08C				AST_REG_SCU(0x08c)
 #define AST_REG_SCU090				AST_REG_SCU(0x090)
 #define AST_REG_SCU094				AST_REG_SCU(0x094)
+#define AST_REG_SCU0A4				AST_REG_SCU(0x0a4)
+#define AST_REG_SCU0A8				AST_REG_SCU(0x0a8)
 #define AST_REG_SCU100				AST_REG_SCU(0x100)
 #define AST_REG_SCU104				AST_REG_SCU(0x104)
 #define AST_REG_SCU160				AST_REG_SCU(0x160)
-- 
2.53.0

Claude review: drm/ast: dp501: Fix open-coded register access

[Claude Code Review Bot]

Patch Review

Good cleanup of `ast_init_dvo()` and `ast_init_analog()`. The conversion from `ast_read32`/`ast_write32` (which use raw P2A-mapped offsets) to `ast_mindwm`/`ast_moutdwm` (which handle segment selection) is correct.

**Important behavioral change in `ast_init_analog()`**: The old code wrote the SCU protection key 3 times:
```c
ast_write32(ast, 0x12000, 0x1688a8a8);
ast_write32(ast, 0x12000, 0x1688a8a8);
ast_write32(ast, 0x12000, 0x1688a8a8);
```
The new code uses `ast_moutdwm_poll`:
```c
ast_moutdwm_poll(ast, AST_REG_SCU000, AST_REG_SCU000_PROTECTION_KEY, 0x01);
```
This writes the key once and polls until the register reads 0x01 (unlocked). This is semantically different - the old code retried the write 3 times (perhaps as a reliability measure), while the new code writes once and polls. Since `ast_moutdwm_poll` is write-then-poll, if the hardware needs multiple writes, this could be a regression. However, since this was smoke-tested on hardware and the protection key mechanism typically just needs a single correct write, this should be fine.

Also in `ast_init_dvo()`: The old code had a P2A bridge setup sequence (`ast_write32(ast, 0xf004, AST_REG_MCR00)` etc.) that is removed because `ast_moutdwm` handles segment selection internally. But the old code wrote `0x1e6e0000` (MCR base) to the P2A window and then wrote `0x1688a8a8` to offset `0x12000` - which is SCU000 (0x1e6e2000) since 0x12000 = 0x10000 + 0x2000. Wait - that doesn't match: with P2A set to MCR (0x1e6e0000), the window at 0x10000 maps to 0x1e6e0000, so 0x12000 = 0x10000 + 0x2000 would be 0x1e6e2000 = SCU000. That's correct. The new code uses `ast_moutdwm(ast, AST_REG_SCU000, ...)` which will re-select the segment to SCU. Correct.

---
Generated by Claude Code Patch Reviewer

[PATCH 15/15] drm/ast: Fix open-coded scu_rev access

[Thomas Zimmermann]

Replace all open-coded access to P2A and SCU registers in the device
detection with the appropriate calls to ast_moutdwm() and ast_mindwm().
Use P2A and MCR register constants. Name variables according to registers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_drv.c | 13 ++++++-------
 drivers/gpu/drm/ast/ast_reg.h |  1 +
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 6fe549f16309..ba6cf5fa901c 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -242,7 +242,7 @@ static int ast_detect_chip(struct pci_dev *pdev,
 	enum ast_config_mode config_mode = ast_use_defaults;
 	uint32_t scu_rev = 0xffffffff;
 	enum ast_chip chip;
-	u32 data;
+	u32 data, p2a04, scu07c;
 	u8 vgacrd0, vgacrd1;
 
 	/*
@@ -275,14 +275,13 @@ static int ast_detect_chip(struct pci_dev *pdev,
 			}
 
 			/* Double check that it's actually working */
-			data = __ast_read32(regs, 0xf004);
-			if ((data != 0xffffffff) && (data != 0x00)) {
+			p2a04 = __ast_read32(regs, AST_REG_P2A04);
+			if (p2a04 != 0xffffffff && p2a04 != 0x00000000) {
 				config_mode = ast_use_p2a;
 
-				/* Read SCU7c (silicon revision register) */
-				__ast_write32(regs, 0xf004, AST_REG_MCR00);
-				__ast_write32(regs, 0xf000, 0x1);
-				scu_rev = __ast_read32(regs, 0x1207c);
+				/* Read SCU7C (silicon revision register) */
+				scu07c = __ast_mindwm(regs, AST_REG_SCU07C);
+				scu_rev = scu07c & AST_REG_SCU07C_CHIP_BONDING_MASK;
 			}
 		}
 	}
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index 9d6ea0765990..00e496d61551 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -190,6 +190,7 @@
 #define AST_REG_SCU040				AST_REG_SCU(0x040)
 #define AST_REG_SCU070				AST_REG_SCU(0x070)
 #define AST_REG_SCU07C				AST_REG_SCU(0x07c)
+#define AST_REG_SCU07C_CHIP_BONDING_MASK	GENMASK(15, 8)
 #define AST_REG_SCU084				AST_REG_SCU(0x084)
 #define AST_REG_SCU088				AST_REG_SCU(0x088)
 #define AST_REG_SCU08C				AST_REG_SCU(0x08c)
-- 
2.53.0

Claude review: drm/ast: Fix open-coded scu_rev access

[Claude Code Review Bot]

Patch Review

Clean final patch. Replaces the manual P2A setup for reading SCU7C with `__ast_mindwm`. The addition of `AST_REG_SCU07C_CHIP_BONDING_MASK` (`GENMASK(15, 8)`) is a nice touch for documenting which bits are actually used.

**Behavioral concern**: The old code read the full 32-bit `scu_rev` value and stored it as-is. The new code masks it:
```c
scu_rev = scu07c & AST_REG_SCU07C_CHIP_BONDING_MASK;
```
I'd want to verify that downstream consumers of `scu_rev` only care about bits [15:8]. Looking at the cover letter, this was smoke-tested, so it's likely correct, but this is a functional change worth noting - if any code path checks bits outside the mask, it would break.

---
Generated by Claude Code Patch Reviewer

Re: [PATCH 00/15] drm/ast: Clean up access to MMIO registers

[Thomas Zimmermann]

The series title should say 'non-MMIO registers'. :( The rest of the 
series is not affected by the typo.

Am 23.03.26 um 16:56 schrieb Thomas Zimmermann:
> Aspeed devices up to Gen6 contain internal memory that is accessible
> from the host via the P2A bridge. The helpers ast_moutdwm() and
> ast_mindwm() pick a 64-KiB segment of the memory to expose to the
> host, which then has the content available. Content is either registers
> of the various IP blocks or on-board system memory.
>
> Clean up the driver code involved and provide macro constants for the
> registers used by the ast driver. Go through the driver and replace
> the magic values with the constants.
>
> The first patch of the series fixes a bug that has been found while
> working on these changes. The DP501 writes the content of register
> SCU2C to the wrong address. The fix is a separate patch to ease
> backporting.
>
> Smoke-tested on AST2100, AST2300, AST2500 and AST2600.
>
> Thomas Zimmermann (15):
>    drm/ast: dp501: Fix initialization of SCU2C
>    drm/ast: Move 32-bit register-access helpers to ast_drv.{c,h}
>    drm/ast: Use constants for AHBC registers
>    drm/ast: Use constants for MCR registers
>    drm/ast: Use constants for SCU registers
>    drm/ast: Use constants for A2P registers
>    drm/ast: Use constants for WDT registers
>    drm/ast: Use constants for SDRAM registers
>    drm/ast: Store register addresses in struct ast_dramstruct
>    drm/ast: Gen1: Fix open-coded register access
>    drm/ast: Gen2: Fix open-coded register access
>    drm/ast: Gen4: Fix open-coded register access
>    drm/ast: Gen6: Fix open-coded register access
>    drm/ast: dp501: Fix open-coded register access
>    drm/ast: Fix open-coded scu_rev access
>
>   drivers/gpu/drm/ast/ast_2000.c  |  61 +++--
>   drivers/gpu/drm/ast/ast_2100.c  | 292 +++++++++++------------
>   drivers/gpu/drm/ast/ast_2300.c  | 398 ++++++++++++++++----------------
>   drivers/gpu/drm/ast/ast_2500.c  | 319 ++++++++++++-------------
>   drivers/gpu/drm/ast/ast_dp501.c | 146 ++++++------
>   drivers/gpu/drm/ast/ast_drv.c   |  84 ++++++-
>   drivers/gpu/drm/ast/ast_drv.h   |  60 +++--
>   drivers/gpu/drm/ast/ast_post.c  |  48 +---
>   drivers/gpu/drm/ast/ast_post.h  |  20 +-
>   drivers/gpu/drm/ast/ast_reg.h   | 155 +++++++++++++
>   10 files changed, 886 insertions(+), 697 deletions(-)
>
>
> base-commit: 5cb7d14d9bd2061386a0192a4649626a3e3a5ec3

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Claude review: drm/ast: Clean up access to MMIO registers

[Claude Code Review Bot]

Overall Series Review

Subject: drm/ast: Clean up access to MMIO registers
Author: Thomas Zimmermann <tzimmermann@suse.de>
Patches: 17
Reviewed: 2026-03-25T07:46:39.011091

---

This is a well-structured 15-patch cleanup series for the drm/ast driver by Thomas Zimmermann. It replaces hundreds of open-coded magic register addresses with named constants, consolidates the P2A bridge register access helpers, and fixes a real bug in patch 1. The series has been smoke-tested on AST2100, AST2300, AST2500, and AST2600.

The overall approach is sound - the driver was previously littered with raw hex addresses like `0x1e6e0000`, `0x1e6e2000`, etc., making the code hard to audit. The new `AST_REG_MCR*`, `AST_REG_SCU*`, `AST_REG_AHBC*`, etc. constants make register accesses self-documenting. The series is largely mechanical but done carefully, with a few substantive improvements (the new `ast_moutdwm_poll()` helper, the `ast_dramstruct` rework to use full 32-bit addresses).

I have a few concerns noted below but nothing that should block merging.

---
Generated by Claude Code Patch Reviewer