Claude review: The Apple iMac11, 1 (late 2009) has an integrated ATI Mobility Radeon HD 4850. This machine suffers from a similar problem as the iMac10, 1 (late 2009) and the iMac11, 2 (mid 2010). This small patch fixes the issue on this machine.

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Commit message formatting issues:**

- The subject line contains the entire commit description, which is against kernel patch conventions. It should be a short imperative line like `drm/radeon: Fix display on iMac11,1 with HD 4850` with details in the body.
- **Missing `Signed-off-by:` line** — required by the kernel's Developer Certificate of Origin policy. The patch cannot be accepted without it.
- The `Fixes:` tag format is wrong. It references a freedesktop.org xf86-video-ati issue, not a kernel commit. If a kernel commit introduced the regression, that commit hash should be cited with the standard `Fixes: <hash> ("subject")` format.

**Incorrect macro definition — the central problem:**

```c
#define ASIC_IS_DCE31(rdev) ((rdev->family >= CHIP_RV770))
#define ASIC_IS_DCE32(rdev) ((rdev->family >= CHIP_RV730))
```

From `radeon_family.h`, the enum ordering is:
```
CHIP_RS780  = 72  (DCE 3.1)
CHIP_RS880  = 73  (DCE 3.1)
CHIP_RV770  = 74  (DCE 3.2)
CHIP_RV730  = 75  (DCE 3.2)
```

The new `ASIC_IS_DCE31` defined as `>= CHIP_RV770` does **not** match any actual DCE 3.1 hardware (RS780, RS880). It only matches DCE 3.2 hardware — it is effectively a corrected version of `ASIC_IS_DCE32`. The naming is misleading and will confuse future maintainers.

The real bug is that `ASIC_IS_DCE32` itself is wrong: it should be `>= CHIP_RV770` instead of `>= CHIP_RV730`, since RV770 is DCE 3.2 hardware. The correct fix is to fix `ASIC_IS_DCE32` directly, not introduce a new macro with a confusing name. This would also fix the other ~7 callsites in `r600.c` and `radeon_audio.c` that currently exclude RV770 from DCE 3.2 logic.

**PLL changes are broader than necessary:**

```c
if (ASIC_IS_DCE31(rdev) && mode->clock > 200000)	/* range limits??? */
    radeon_crtc->pll_flags |= RADEON_PLL_PREFER_HIGH_FB_DIV;
```

and:

```c
if (ASIC_IS_DCE31(rdev) && mode->clock > 165000)
    radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
```

These changes affect **all** RV770 systems, not just Apple iMac11,1. Since RV770 genuinely is DCE 3.2 hardware, these PLL flags arguably should apply to it — but this represents a behavior change for all RV770 users and should be explicitly acknowledged and tested. It's worth noting that line 588 already uses `rdev->family < CHIP_RV770` as a boundary, suggesting the code already knows RV770 belongs with the newer chips.

**Encoder selection change is correctly scoped:**

```c
if (ASIC_IS_DCE31(rdev)) {
    if (dmi_match(DMI_PRODUCT_NAME, "iMac10,1") ||
        dmi_match(DMI_PRODUCT_NAME, "iMac11,1") ||
        dmi_match(DMI_PRODUCT_NAME, "iMac11,2"))
        enc_idx = (dig->linkb) ? 1 : 0;
```

This part is fine — the DMI matching gates the Apple-specific quirk so only the listed machines are affected. Adding `iMac11,1` here is the right thing to do. The comment update is also appropriate.

**Recommended approach for a v2:**

1. Fix `ASIC_IS_DCE32` to be `>= CHIP_RV770` instead of creating a new `ASIC_IS_DCE31` macro. This fixes the root cause (RV770 being wrongly excluded from DCE 3.2) and cleans up all callsites at once.
2. Add the `iMac11,1` DMI match in `atombios_encoders.c` — this is unambiguously correct.
3. Use a proper short subject line: `drm/radeon: Fix display on Apple iMac11,1 (HD 4850/RV770)`.
4. Add `Signed-off-by:`.
5. Write a proper commit message explaining that `ASIC_IS_DCE32` incorrectly excluded RV770 due to the enum ordering.

---
Generated by Claude Code Patch Reviewer