[PATCH] drm/radeon: Add HAINAN clock adjustment

[PATCH] drm/radeon: Add HAINAN clock adjustment

[decce6]

This patch limits the clock speeds of the AMD Radeon R5 M420 GPU from
850/1000MHz (core/memory) to 800/950 MHz, making it work stably. This
patch is for radeon.

Signed-off-by: decce6 <decce6@proton.me>
---
 drivers/gpu/drm/radeon/si_dpm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/radeon/si_dpm.c b/drivers/gpu/drm/radeon/si_dpm.c
index 9deb91970d4d..f12227145ef0 100644
--- a/drivers/gpu/drm/radeon/si_dpm.c
+++ b/drivers/gpu/drm/radeon/si_dpm.c
@@ -2925,6 +2925,11 @@ static void si_apply_state_adjust_rules(struct radeon_device *rdev,
 			max_sclk = 60000;
 			max_mclk = 80000;
 		}
+		if ((rdev->pdev->device == 0x666f) &&
+		    (rdev->pdev->revision == 0x00)) {
+			max_sclk = 80000;
+			max_mclk = 95000;
+		}
 	} else if (rdev->family == CHIP_OLAND) {
 		if ((rdev->pdev->revision == 0xC7) ||
 		    (rdev->pdev->revision == 0x80) ||
-- 
2.43.0

Claude review: drm/radeon: Add HAINAN clock adjustment

[Claude Code Review Bot]

Overall Series Review

Subject: drm/radeon: Add HAINAN clock adjustment
Author: decce6 <decce6@proton.me>
Patches: 1
Reviewed: 2026-02-11T16:33:48.316837

---

This is a single-patch series that adds hardware-specific clock speed limiting for the AMD Radeon R5 M420 GPU (HAINAN chip family). The patch attempts to address stability issues by reducing maximum clock frequencies.

**Series-level concerns:**

1. **Missing justification**: The commit message lacks critical information about why these specific frequencies were chosen and how stability issues were diagnosed
2. **Lack of testing details**: No information about testing methodology, duration, or workloads used to validate the fix
3. **Potential hardware variation**: No consideration of whether all R5 M420 GPUs need this limitation or if this is specific to certain board designs
4. **Missing documentation**: No explanation of the stability symptoms that motivated this change

**Architecture concerns:**

- The patch uses PCI device ID and revision matching, which is appropriate for hardware-specific workarounds
- The placement in `si_apply_state_adjust_rules()` is correct for the radeon driver architecture
- However, this approach may be too broad if only specific board implementations require the limitation

---
Generated by Claude Code Patch Reviewer

Claude review: drm/radeon: Add HAINAN clock adjustment

[Claude Code Review Bot]

Patch Review

**Subject line issues:**
- Title is vague - should be more specific: "drm/radeon: Limit R5 M420 clocks to fix stability on HAINAN"

**Commit message analysis:**

```
This patch limits the clock speeds of the AMD Radeon R5 M420 GPU from
850/1000MHz (core/memory) to 800/950 MHz, making it work stably. This
patch is for radeon.
```

**Problems:**
1. States "This patch..." (redundant - commit messages should use imperative mood)
2. "This patch is for radeon" adds no value
3. Missing critical context:
   - What are the symptoms of instability? (artifacts, crashes, hangs?)
   - How was the issue reproduced and validated?
   - Why these specific frequency values?
   - Is this specific to certain board vendors or all R5 M420?
   - Has this been tested on multiple systems?

**Better commit message would be:**
```
drm/radeon: Limit R5 M420 clocks to fix stability issues

The AMD Radeon R5 M420 (HAINAN, device 0x666f revision 0x00) experiences
[specific symptoms: crashes/artifacts/hangs] when running at default clock
speeds of 850/1000 MHz (core/memory).

Limit maximum clocks to 800/950 MHz to ensure stable operation. These
values were determined through [testing methodology] and have been validated
on [number] systems over [duration].

[If known: Link to bug report or add Closes: tag]
```

**Code review:**

```c
+		if ((rdev->pdev->device == 0x666f) &&
+		    (rdev->pdev->revision == 0x00)) {
+			max_sclk = 80000;
+			max_mclk = 95000;
+		}
```

**Issues:**

1. **Magic numbers**: The device ID `0x666f` should be defined as a constant with a meaningful name (though this matches existing code style in the file)

2. **Inconsistent units**: The comment in the commit message says "800/950 MHz" but the code uses `80000/95000` - these represent 10 kHz units, so:
   - 80000 * 10 kHz = 800 MHz (core) ✓
   - 95000 * 10 kHz = 950 MHz (memory) ✓
   The math is correct but could confuse reviewers

3. **Whitespace**: Uses `=09` (tab encoding in quoted-printable) - appears to match surrounding code style

4. **Location**: Correctly placed within the `CHIP_HAINAN` conditional block

5. **Logic**: The double-parentheses style `((condition))` matches the existing code pattern in this function

6. **Completeness**: Only checks revision 0x00 - are other revisions of this device ID affected? This needs clarification or testing data

**Technical questions:**

1. Is device 0x666f revision 0x00 the only problematic variant?
2. Should this be configurable via module parameter for users who want to try higher clocks?
3. Has AMD provided any guidance on safe clock speeds for this part?
4. Could this be thermal-related rather than a silicon issue?
5. Should there be a pr_info() message when this workaround is applied?

**Code correctness:** The code itself appears functionally correct and follows the existing pattern.

**Testing validation needed:**
- Multiple R5 M420 systems (different vendors/board designs)
- Extended stress testing (hours/days)
- Different workloads (3D, compute, video decode)
- Temperature monitoring during testing

**Recommendation:** **CHANGES REQUESTED**

The functional change appears reasonable, but the patch needs:
1. Substantially improved commit message with proper justification
2. Testing details and validation data
3. Clarification on whether all R5 M420 GPUs need this or specific board variants
4. Consider adding a debug message when the limitation is applied
5. Confirmation that other revisions don't need similar treatment

---
Generated by Claude Code Patch Reviewer