public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] 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.
@ 2026-05-10 18:54 Gilles Risch
  2026-05-16  5:26 ` Claude review: " Claude Code Review Bot
                   ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Gilles Risch @ 2026-05-10 18:54 UTC (permalink / raw)
  To: alexander.deucher; +Cc: dri-devel, amd-gfx, Gilles Risch

Fixes freedesktop issue 164
Link: https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/-/issues/164
---
 drivers/gpu/drm/radeon/atombios_crtc.c     | 4 ++--
 drivers/gpu/drm/radeon/atombios_encoders.c | 9 +++++----
 drivers/gpu/drm/radeon/radeon.h            | 1 +
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 2fc0334e0..3c6d33273 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -580,7 +580,7 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
 			radeon_crtc->pll_flags |= (/*RADEON_PLL_USE_FRAC_FB_DIV |*/
 				RADEON_PLL_PREFER_CLOSEST_LOWER);
 
-		if (ASIC_IS_DCE32(rdev) && mode->clock > 200000)	/* range limits??? */
+		if (ASIC_IS_DCE31(rdev) && mode->clock > 200000)	/* range limits??? */
 			radeon_crtc->pll_flags |= RADEON_PLL_PREFER_HIGH_FB_DIV;
 		else
 			radeon_crtc->pll_flags |= RADEON_PLL_PREFER_LOW_REF_DIV;
@@ -594,7 +594,7 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
 		if (((rdev->family == CHIP_RS780) || (rdev->family == CHIP_RS880))
 		    && !radeon_crtc->ss_enabled)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
-		if (ASIC_IS_DCE32(rdev) && mode->clock > 165000)
+		if (ASIC_IS_DCE31(rdev) && mode->clock > 165000)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
 	} else {
 		radeon_crtc->pll_flags |= RADEON_PLL_LEGACY;
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index 5cfd8fcfa..4e984973c 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -2123,12 +2123,13 @@ int radeon_atom_pick_dig_encoder(struct drm_encoder *encoder, int fe_idx)
 	}
 
 	/*
-	 * On DCE32 any encoder can drive any block so usually just use crtc id,
-	 * but Apple thinks different at least on iMac10,1 and iMac11,2, so there use linkb,
-	 * otherwise the internal eDP panel will stay dark.
+	 * On DCE31 and DCE32 any encoder can drive any block so usually just use crtc id,
+	 * but Apple thinks different at least on iMac10,1, iMac11,1 and iMac11,2,
+	 * so there use linkb, otherwise the internal eDP panel will stay dark.
 	 */
-	if (ASIC_IS_DCE32(rdev)) {
+	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;
 		else
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 527b9d19d..6b7c0abe4 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2625,6 +2625,7 @@ void r100_pll_errata_after_index(struct radeon_device *rdev);
 			    (rdev->family == CHIP_RS740)  ||	\
 			    (rdev->family >= CHIP_R600))
 #define ASIC_IS_DCE3(rdev) ((rdev->family >= CHIP_RV620))
+#define ASIC_IS_DCE31(rdev) ((rdev->family >= CHIP_RV770))
 #define ASIC_IS_DCE32(rdev) ((rdev->family >= CHIP_RV730))
 #define ASIC_IS_DCE4(rdev) ((rdev->family >= CHIP_CEDAR))
 #define ASIC_IS_DCE41(rdev) ((rdev->family >= CHIP_PALM) && \
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* 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.
  2026-05-10 18:54 [PATCH] 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 Gilles Risch
@ 2026-05-16  5:26 ` Claude Code Review Bot
  2026-05-16  5:26 ` Claude Code Review Bot
  2026-05-16  9:24 ` [PATCH v2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
  2 siblings, 0 replies; 28+ messages in thread
From: Claude Code Review Bot @ 2026-05-16  5:26 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: 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.
Author: Gilles Risch <gilles.risch@gmail.com>
Patches: 1
Reviewed: 2026-05-16T15:26:36.767296

---

This is a single patch fixing display issues on the Apple iMac11,1 (late 2009, ATI Mobility Radeon HD 4850 / RV770). The core problem being fixed is real: the RV770 chip is DCE 3.2 hardware, but the existing `ASIC_IS_DCE32` macro (`>= CHIP_RV730`) accidentally excludes RV770 because `CHIP_RV770` precedes `CHIP_RV730` in the `radeon_family` enum. This causes PLL misconfiguration and wrong encoder selection on RV770-based Apple machines.

However, the patch has **significant issues** with its approach, naming, commit message formatting, and missing sign-off. It should not be applied in its current form.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 28+ messages in thread

* 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.
  2026-05-10 18:54 [PATCH] 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 Gilles Risch
  2026-05-16  5:26 ` Claude review: " Claude Code Review Bot
@ 2026-05-16  5:26 ` Claude Code Review Bot
  2026-05-16  9:24 ` [PATCH v2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
  2 siblings, 0 replies; 28+ messages in thread
From: Claude Code Review Bot @ 2026-05-16  5:26 UTC (permalink / raw)
  To: dri-devel-reviews

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-10 18:54 [PATCH] 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 Gilles Risch
  2026-05-16  5:26 ` Claude review: " Claude Code Review Bot
  2026-05-16  5:26 ` Claude Code Review Bot
@ 2026-05-16  9:24 ` Gilles Risch
  2026-05-16 18:09   ` Lukas Wunner
                     ` (2 more replies)
  2 siblings, 3 replies; 28+ messages in thread
From: Gilles Risch @ 2026-05-16  9:24 UTC (permalink / raw)
  To: alexander.deucher; +Cc: amd-gfx, dri-devel, Gilles Risch

The Apple iMac11,1 (late 2009) has an integrated ATI Mobility Radeon
HD 4850 (RV770/M98L) with a 2560x1440 internal panel connected via an
internal DisplayPort path. This machine suffers from a similar problem
as the iMac10,1 (late 2009) and the iMac11,2 (mid 2010). Without this
fix the display stays dark under KMS. Two issues are addressed:

1. The RV770 implements DCE3.1 and not DCE3.2. ASIC_IS_DCE32() starts at
   CHIP_RV730 which is newer than RV770, so the RV770 never matched the
   DCE3.2 PLL and encoder logic. Introduce ASIC_IS_DCE31() starting at
   CHIP_RV770 to fix this.

2. Apple routed the internal display through Link B of the DIG encoder
   instead of Link A, as observed in the kernel display connector log.
   The same quirk already exists for iMac10,1 and iMac11,2 - iMac11,1
   was simply missing from the list.

Note: resume from suspend still results in a dark screen as the DP
re-driver chips on the mainboard lose their state during power-off.
This will be addressed in a follow-up patch.

Fixes freedesktop issue 164
Link: https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/-/issues/164

Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
---
v2: No code changes. Fixed commit message:
    - Added drm/radeon: prefix to subject line
    - Added empty line between subject and body
    - Added technical description of the changes

 drivers/gpu/drm/radeon/atombios_crtc.c     | 4 ++--
 drivers/gpu/drm/radeon/atombios_encoders.c | 9 +++++----
 drivers/gpu/drm/radeon/radeon.h            | 1 +
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 2fc0334e0d6c..3c6d332739e3 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -580,7 +580,7 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
 			radeon_crtc->pll_flags |= (/*RADEON_PLL_USE_FRAC_FB_DIV |*/
 				RADEON_PLL_PREFER_CLOSEST_LOWER);
 
-		if (ASIC_IS_DCE32(rdev) && mode->clock > 200000)	/* range limits??? */
+		if (ASIC_IS_DCE31(rdev) && mode->clock > 200000)	/* range limits??? */
 			radeon_crtc->pll_flags |= RADEON_PLL_PREFER_HIGH_FB_DIV;
 		else
 			radeon_crtc->pll_flags |= RADEON_PLL_PREFER_LOW_REF_DIV;
@@ -594,7 +594,7 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
 		if (((rdev->family == CHIP_RS780) || (rdev->family == CHIP_RS880))
 		    && !radeon_crtc->ss_enabled)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
-		if (ASIC_IS_DCE32(rdev) && mode->clock > 165000)
+		if (ASIC_IS_DCE31(rdev) && mode->clock > 165000)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
 	} else {
 		radeon_crtc->pll_flags |= RADEON_PLL_LEGACY;
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index 5cfd8fcfa5e8..4e984973c043 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -2123,12 +2123,13 @@ int radeon_atom_pick_dig_encoder(struct drm_encoder *encoder, int fe_idx)
 	}
 
 	/*
-	 * On DCE32 any encoder can drive any block so usually just use crtc id,
-	 * but Apple thinks different at least on iMac10,1 and iMac11,2, so there use linkb,
-	 * otherwise the internal eDP panel will stay dark.
+	 * On DCE31 and DCE32 any encoder can drive any block so usually just use crtc id,
+	 * but Apple thinks different at least on iMac10,1, iMac11,1 and iMac11,2,
+	 * so there use linkb, otherwise the internal eDP panel will stay dark.
 	 */
-	if (ASIC_IS_DCE32(rdev)) {
+	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;
 		else
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 527b9d19d730..6b7c0abe49fb 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2625,6 +2625,7 @@ void r100_pll_errata_after_index(struct radeon_device *rdev);
 			    (rdev->family == CHIP_RS740)  ||	\
 			    (rdev->family >= CHIP_R600))
 #define ASIC_IS_DCE3(rdev) ((rdev->family >= CHIP_RV620))
+#define ASIC_IS_DCE31(rdev) ((rdev->family >= CHIP_RV770))
 #define ASIC_IS_DCE32(rdev) ((rdev->family >= CHIP_RV730))
 #define ASIC_IS_DCE4(rdev) ((rdev->family >= CHIP_CEDAR))
 #define ASIC_IS_DCE41(rdev) ((rdev->family >= CHIP_PALM) && \
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-16  9:24 ` [PATCH v2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
@ 2026-05-16 18:09   ` Lukas Wunner
  2026-05-17 13:53     ` Gilles Risch
  2026-05-18  6:36     ` Claude review: " Claude Code Review Bot
  2026-05-16 18:52   ` [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Gilles Risch
  2026-05-18  6:57   ` Claude review: drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Claude Code Review Bot
  2 siblings, 2 replies; 28+ messages in thread
From: Lukas Wunner @ 2026-05-16 18:09 UTC (permalink / raw)
  To: Gilles Risch; +Cc: alexander.deucher, amd-gfx, dri-devel

On Sat, May 16, 2026 at 11:24:21AM +0200, Gilles Risch wrote:
> The Apple iMac11,1 (late 2009) has an integrated ATI Mobility Radeon
> HD 4850 (RV770/M98L) with a 2560x1440 internal panel connected via an
> internal DisplayPort path. This machine suffers from a similar problem
> as the iMac10,1 (late 2009) and the iMac11,2 (mid 2010). Without this
> fix the display stays dark under KMS. Two issues are addressed:
> 
> 1. The RV770 implements DCE3.1 and not DCE3.2. ASIC_IS_DCE32() starts at
>    CHIP_RV730 which is newer than RV770, so the RV770 never matched the
>    DCE3.2 PLL and encoder logic. Introduce ASIC_IS_DCE31() starting at
>    CHIP_RV770 to fix this.
> 
> 2. Apple routed the internal display through Link B of the DIG encoder
>    instead of Link A, as observed in the kernel display connector log.
>    The same quirk already exists for iMac10,1 and iMac11,2 - iMac11,1
>    was simply missing from the list.

Not an expert in the radeon driver or AMD chip revisions, but if it
helps improve support for older Macs I'll be happy to offer my:

Reviewed-by: Lukas Wunner <lukas@wunner.de>

It's not immediately obvious that CHIP_RV770 precedes CHIP_RV730
in the enum and that hence ASIC_IS_DCE31() subsumes ASIC_IS_DCE32().

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems
  2026-05-16  9:24 ` [PATCH v2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
  2026-05-16 18:09   ` Lukas Wunner
@ 2026-05-16 18:52   ` Gilles Risch
  2026-05-17  3:28     ` kernel test robot
                       ` (3 more replies)
  2026-05-18  6:57   ` Claude review: drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Claude Code Review Bot
  2 siblings, 4 replies; 28+ messages in thread
From: Gilles Risch @ 2026-05-16 18:52 UTC (permalink / raw)
  To: alexander.deucher; +Cc: amd-gfx, dri-devel, Gilles Risch

After suspend/resume the internal eDP display on iMac11,1 (and
potentially other DCE3.1 systems) stays dark because
atombios_set_edp_panel_power() skips panel power control for
anything older than DCE4.

Fix this by:
- Extending atombios_set_edp_panel_power() to also handle DCE3.1
  by changing the !ASIC_IS_DCE4() guard to !ASIC_IS_DCE31().
  HPD polling works correctly on DCE3.1 (verified: HPD is asserted
  at iteration 0 on iMac11,1).
- Issuing ATOM_ENCODER_CMD_DP_VIDEO_ON/OFF for DCE3.1 in addition
  to DCE4+.

Tested on iMac11,1 (Mobility Radeon HD 4850, RV770/DCE3.1).

Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
---
 drivers/gpu/drm/radeon/atombios_encoders.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index 4e984973c043..b62fd713efcf 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -1383,7 +1383,7 @@ atombios_set_edp_panel_power(struct drm_connector *connector, int action)
 	if (connector->connector_type != DRM_MODE_CONNECTOR_eDP)
 		goto done;
 
-	if (!ASIC_IS_DCE4(rdev))
+	if (!ASIC_IS_DCE31(rdev))
 		goto done;
 
 	if ((action != ATOM_TRANSMITTER_ACTION_POWER_ON) &&
@@ -1707,7 +1707,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
 		if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector) {
 			/* DP_SET_POWER_D0 is set in radeon_dp_link_train */
 			radeon_dp_link_train(encoder, connector);
-			if (ASIC_IS_DCE4(rdev))
+			if (ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev))
 				atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
 		}
 		if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) {
@@ -1724,7 +1724,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
 	case DRM_MODE_DPMS_SUSPEND:
 	case DRM_MODE_DPMS_OFF:
 
-		if (ASIC_IS_DCE4(rdev)) {
+		if (ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev)) {
 			if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector)
 				atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_OFF, 0);
 		}
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems
  2026-05-16 18:52   ` [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Gilles Risch
@ 2026-05-17  3:28     ` kernel test robot
  2026-06-04  3:37       ` Claude review: " Claude Code Review Bot
  2026-05-17  5:12     ` kernel test robot
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 28+ messages in thread
From: kernel test robot @ 2026-05-17  3:28 UTC (permalink / raw)
  To: Gilles Risch, alexander.deucher
  Cc: llvm, oe-kbuild-all, amd-gfx, dri-devel, Gilles Risch

Hi Gilles,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v7.1-rc3 next-20260508]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Gilles-Risch/drm-radeon-fix-eDP-resume-from-suspend-on-iMac11-1-DCE3-1-systems/20260517-025343
base:   https://gitlab.freedesktop.org/drm/misc/kernel.git drm-misc-next
patch link:    https://lore.kernel.org/r/20260516185226.3005-1-gilles.risch%40gmail.com
patch subject: [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems
config: riscv-randconfig-001-20260517 (https://download.01.org/0day-ci/archive/20260517/202605171126.1g7CRkTZ-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 5bac06718f502014fade905512f1d26d578a18f3)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260517/202605171126.1g7CRkTZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605171126.1g7CRkTZ-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/radeon/atombios_encoders.c:1386:7: error: call to undeclared function 'ASIC_IS_DCE31'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    1386 |         if (!ASIC_IS_DCE31(rdev))
         |              ^
   drivers/gpu/drm/radeon/atombios_encoders.c:1710:30: error: call to undeclared function 'ASIC_IS_DCE31'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    1710 |                         if (ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev))
         |                                                   ^
   drivers/gpu/drm/radeon/atombios_encoders.c:1727:29: error: call to undeclared function 'ASIC_IS_DCE31'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    1727 |                 if (ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev)) {
         |                                           ^
   3 errors generated.


vim +/ASIC_IS_DCE31 +1386 drivers/gpu/drm/radeon/atombios_encoders.c

  1372	
  1373	bool
  1374	atombios_set_edp_panel_power(struct drm_connector *connector, int action)
  1375	{
  1376		struct radeon_connector *radeon_connector = to_radeon_connector(connector);
  1377		struct drm_device *dev = radeon_connector->base.dev;
  1378		struct radeon_device *rdev = dev->dev_private;
  1379		union dig_transmitter_control args;
  1380		int index = GetIndexIntoMasterTable(COMMAND, UNIPHYTransmitterControl);
  1381		uint8_t frev, crev;
  1382	
  1383		if (connector->connector_type != DRM_MODE_CONNECTOR_eDP)
  1384			goto done;
  1385	
> 1386		if (!ASIC_IS_DCE31(rdev))
  1387			goto done;
  1388	
  1389		if ((action != ATOM_TRANSMITTER_ACTION_POWER_ON) &&
  1390		    (action != ATOM_TRANSMITTER_ACTION_POWER_OFF))
  1391			goto done;
  1392	
  1393		if (!atom_parse_cmd_header(rdev->mode_info.atom_context, index, &frev, &crev))
  1394			goto done;
  1395	
  1396		memset(&args, 0, sizeof(args));
  1397	
  1398		args.v1.ucAction = action;
  1399	
  1400		atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
  1401	
  1402		/* wait for the panel to power up */
  1403		if (action == ATOM_TRANSMITTER_ACTION_POWER_ON) {
  1404			int i;
  1405	
  1406			for (i = 0; i < 300; i++) {
  1407				if (radeon_hpd_sense(rdev, radeon_connector->hpd.hpd))
  1408					return true;
  1409				mdelay(1);
  1410			}
  1411			return false;
  1412		}
  1413	done:
  1414		return true;
  1415	}
  1416	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems
  2026-05-16 18:52   ` [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Gilles Risch
  2026-05-17  3:28     ` kernel test robot
@ 2026-05-17  5:12     ` kernel test robot
  2026-05-18 14:13     ` Alex Deucher
  2026-06-04  3:37     ` Claude review: drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Claude Code Review Bot
  3 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2026-05-17  5:12 UTC (permalink / raw)
  To: Gilles Risch, alexander.deucher
  Cc: oe-kbuild-all, amd-gfx, dri-devel, Gilles Risch

Hi Gilles,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v7.1-rc3 next-20260508]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Gilles-Risch/drm-radeon-fix-eDP-resume-from-suspend-on-iMac11-1-DCE3-1-systems/20260517-025343
base:   https://gitlab.freedesktop.org/drm/misc/kernel.git drm-misc-next
patch link:    https://lore.kernel.org/r/20260516185226.3005-1-gilles.risch%40gmail.com
patch subject: [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems
config: riscv-randconfig-002-20260517 (https://download.01.org/0day-ci/archive/20260517/202605171301.73YzX9wA-lkp@intel.com/config)
compiler: riscv32-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260517/202605171301.73YzX9wA-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605171301.73YzX9wA-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gpu/drm/radeon/atombios_encoders.c: In function 'atombios_set_edp_panel_power':
>> drivers/gpu/drm/radeon/atombios_encoders.c:1386:14: error: implicit declaration of function 'ASIC_IS_DCE31'; did you mean 'ASIC_IS_DCE3'? [-Wimplicit-function-declaration]
    1386 |         if (!ASIC_IS_DCE31(rdev))
         |              ^~~~~~~~~~~~~
         |              ASIC_IS_DCE3


vim +1386 drivers/gpu/drm/radeon/atombios_encoders.c

  1372	
  1373	bool
  1374	atombios_set_edp_panel_power(struct drm_connector *connector, int action)
  1375	{
  1376		struct radeon_connector *radeon_connector = to_radeon_connector(connector);
  1377		struct drm_device *dev = radeon_connector->base.dev;
  1378		struct radeon_device *rdev = dev->dev_private;
  1379		union dig_transmitter_control args;
  1380		int index = GetIndexIntoMasterTable(COMMAND, UNIPHYTransmitterControl);
  1381		uint8_t frev, crev;
  1382	
  1383		if (connector->connector_type != DRM_MODE_CONNECTOR_eDP)
  1384			goto done;
  1385	
> 1386		if (!ASIC_IS_DCE31(rdev))
  1387			goto done;
  1388	
  1389		if ((action != ATOM_TRANSMITTER_ACTION_POWER_ON) &&
  1390		    (action != ATOM_TRANSMITTER_ACTION_POWER_OFF))
  1391			goto done;
  1392	
  1393		if (!atom_parse_cmd_header(rdev->mode_info.atom_context, index, &frev, &crev))
  1394			goto done;
  1395	
  1396		memset(&args, 0, sizeof(args));
  1397	
  1398		args.v1.ucAction = action;
  1399	
  1400		atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
  1401	
  1402		/* wait for the panel to power up */
  1403		if (action == ATOM_TRANSMITTER_ACTION_POWER_ON) {
  1404			int i;
  1405	
  1406			for (i = 0; i < 300; i++) {
  1407				if (radeon_hpd_sense(rdev, radeon_connector->hpd.hpd))
  1408					return true;
  1409				mdelay(1);
  1410			}
  1411			return false;
  1412		}
  1413	done:
  1414		return true;
  1415	}
  1416	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-16 18:09   ` Lukas Wunner
@ 2026-05-17 13:53     ` Gilles Risch
  2026-05-17 14:05       ` Lukas Wunner
  2026-05-18  6:36     ` Claude review: " Claude Code Review Bot
  1 sibling, 1 reply; 28+ messages in thread
From: Gilles Risch @ 2026-05-17 13:53 UTC (permalink / raw)
  To: Lukas Wunner; +Cc: alexander.deucher, amd-gfx, dri-devel

Good afternoon Lukas,

thanks for the review.

The initial launch date can either be verified in the TeraScale 
architecture comparison table on Wikipedia [1] or directly in the kernel 
source at drivers/gpu/drm/radeon/radeon_family.h where CHIP_RV770 (line 
74) precedes CHIP_RV730 (line 75).

The test robot mentions that it cannot find ASIC_IS_DCE31(); this is 
because both patches build on one another. Should I submit both patches 
as a series?

Kind regards,
Gilles

On 16.05.26 20:09, Lukas Wunner wrote:
> On Sat, May 16, 2026 at 11:24:21AM +0200, Gilles Risch wrote:
>> The Apple iMac11,1 (late 2009) has an integrated ATI Mobility Radeon
>> HD 4850 (RV770/M98L) with a 2560x1440 internal panel connected via an
>> internal DisplayPort path. This machine suffers from a similar problem
>> as the iMac10,1 (late 2009) and the iMac11,2 (mid 2010). Without this
>> fix the display stays dark under KMS. Two issues are addressed:
>>
>> 1. The RV770 implements DCE3.1 and not DCE3.2. ASIC_IS_DCE32() starts at
>>     CHIP_RV730 which is newer than RV770, so the RV770 never matched the
>>     DCE3.2 PLL and encoder logic. Introduce ASIC_IS_DCE31() starting at
>>     CHIP_RV770 to fix this.
>>
>> 2. Apple routed the internal display through Link B of the DIG encoder
>>     instead of Link A, as observed in the kernel display connector log.
>>     The same quirk already exists for iMac10,1 and iMac11,2 - iMac11,1
>>     was simply missing from the list.
> Not an expert in the radeon driver or AMD chip revisions, but if it
> helps improve support for older Macs I'll be happy to offer my:
>
> Reviewed-by: Lukas Wunner <lukas@wunner.de>
>
> It's not immediately obvious that CHIP_RV770 precedes CHIP_RV730
> in the enum and that hence ASIC_IS_DCE31() subsumes ASIC_IS_DCE32().
>
> Thanks,
>
> Lukas



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-17 13:53     ` Gilles Risch
@ 2026-05-17 14:05       ` Lukas Wunner
  0 siblings, 0 replies; 28+ messages in thread
From: Lukas Wunner @ 2026-05-17 14:05 UTC (permalink / raw)
  To: Gilles Risch; +Cc: alexander.deucher, amd-gfx, dri-devel

On Sun, May 17, 2026 at 03:53:51PM +0200, Gilles Risch wrote:
> The test robot mentions that it cannot find ASIC_IS_DCE31(); this is because
> both patches build on one another. Should I submit both patches as a series?

Yes, either that or wait for the first patch to be applied and show up in
the drm-next repo.  Then 0-day should not complain.

Usually people are annoyed if patches are resubmitted in short cadence,
so the polite thing to do is wait for 1 or 2 weeks before respinning
so that people get a chance to provide further comments.

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Claude review: Re: [PATCH v2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-16 18:09   ` Lukas Wunner
  2026-05-17 13:53     ` Gilles Risch
@ 2026-05-18  6:36     ` Claude Code Review Bot
  1 sibling, 0 replies; 28+ messages in thread
From: Claude Code Review Bot @ 2026-05-18  6:36 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Mostly good, with a logical redundancy.**

**Panel power guard in atombios_set_edp_panel_power():**

```c
-	if (!ASIC_IS_DCE4(rdev))
+	if (!ASIC_IS_DCE31(rdev))
 		goto done;
```

This is the key fix — without it, the eDP panel power-on sequence is skipped entirely for DCE3.1. The connector type check (`!= DRM_MODE_CONNECTOR_eDP`) above this line already gates entry, so extending to DCE3.1 won't affect non-eDP systems. The author reports HPD polling works correctly on iMac11,1.

**DP video on/off changes:**

```c
-			if (ASIC_IS_DCE4(rdev))
+			if (ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev))
 				atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
```

```c
-		if (ASIC_IS_DCE4(rdev)) {
+		if (ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev)) {
```

**Issue: Logical redundancy.** Since `ASIC_IS_DCE31()` is `>= CHIP_RV770` and `ASIC_IS_DCE4()` is `>= CHIP_CEDAR`, and `CHIP_CEDAR > CHIP_RV770` in the enum, every chip matching DCE4 also matches DCE31. Therefore `ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev)` simplifies to just `ASIC_IS_DCE31(rdev)`. The `ASIC_IS_DCE4` term is redundant.

However, this has a subtle semantic consequence: the original code only sent `DP_VIDEO_ON`/`DP_VIDEO_OFF` for DCE4+. By changing to `ASIC_IS_DCE31(rdev)` (even with the redundant `||`), this now also sends these commands for **DCE3.2** hardware (RV730, RV710, RV740) — not just RV770. The author should confirm this is intentional and safe for DCE3.2 systems, or narrow the condition if only RV770 needs it.

**Other issues:**

1. **Missing `Fixes:` tag** — same as patch 2.

2. **Missing `Signed-off-by` chain context.** This patch depends on patch v2 (it uses `ASIC_IS_DCE31` introduced there and its `diff --git` index line `4e984973c043` matches the post-v2 state). The dependency should be explicitly stated via an `In-Reply-To:` to the v2 patch or a cover letter, and the two should be submitted as a proper series (`[PATCH v2 1/2]` and `[PATCH v2 2/2]`).

3. **The `ATOM_ENCODER_CMD_DP_VIDEO_ON/OFF` on DCE3.1** — the author should confirm the ATOM BIOS table on RV770 actually implements these encoder commands. If the VBIOS table doesn't handle them, `atom_execute_table()` might silently succeed or could misbehave. The author says it's tested on iMac11,1, which is good, but broader DCE3.1/3.2 implications should be considered.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Claude review: drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-16  9:24 ` [PATCH v2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
  2026-05-16 18:09   ` Lukas Wunner
  2026-05-16 18:52   ` [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Gilles Risch
@ 2026-05-18  6:57   ` Claude Code Review Bot
  2 siblings, 0 replies; 28+ messages in thread
From: Claude Code Review Bot @ 2026-05-18  6:57 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Looks correct.** The new macro is well-placed and the logic follows the existing DCE versioning scheme.

**Issue 1 — Missing `Fixes:` tag:** The commit message references "Fixes freedesktop issue 164" in prose, but does not include a kernel-style `Fixes:` tag pointing to the commit that introduced the regression (or the original DCE3.2 logic that omitted RV770). If this is a fix for existing behavior, it should have:
```
Fixes: <sha1> ("drm/radeon: ...")
Cc: stable@vger.kernel.org
```
If there's no single commit to blame (the logic was always wrong), a `Cc: stable` is still appropriate since this is a hardware enablement fix for a longstanding bug.

**Issue 2 — PLL flag changes affect all RV770 systems, not just iMac:**

```c
-		if (ASIC_IS_DCE32(rdev) && mode->clock > 200000)
+		if (ASIC_IS_DCE31(rdev) && mode->clock > 200000)
```

This change means *every* RV770-based system (not just the iMac11,1) will now get `RADEON_PLL_PREFER_HIGH_FB_DIV` for high clocks and `RADEON_PLL_USE_FRAC_FB_DIV` for clocks > 165000. While this is likely the correct behavior for DCE3.1 hardware in general, the commit message frames this purely as an iMac fix. The author should confirm this is intentional for all RV770 cards, or if it should be gated behind a DMI check. Given that line 588 already has `if (rdev->family < CHIP_RV770)` as a boundary, this is likely fine — but it should be stated explicitly in the commit message.

**Issue 3 — Audit of remaining `ASIC_IS_DCE32` call sites:** There are additional `ASIC_IS_DCE32()` users in `r600.c` (interrupt handling, HDMI, HPD) and `radeon_audio.c`. The author should confirm these do not also need the DCE3.1 treatment. If they are truly DCE3.2-specific features, the commit message should note this.

**Minor — Subject line wrapping:** The subject line appears wrapped across multiple header lines in the mbox (`Subject: [PATCH v2] drm/radeon: fix internal display on iMac11,\n 1 (RV770/DCE3.1)`). This may be an email client artifact, but the author should verify `git log --oneline` shows it correctly.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems
  2026-05-16 18:52   ` [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Gilles Risch
  2026-05-17  3:28     ` kernel test robot
  2026-05-17  5:12     ` kernel test robot
@ 2026-05-18 14:13     ` Alex Deucher
  2026-05-18 14:40       ` Gilles Risch
  2026-06-04  3:37     ` Claude review: drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Claude Code Review Bot
  3 siblings, 1 reply; 28+ messages in thread
From: Alex Deucher @ 2026-05-18 14:13 UTC (permalink / raw)
  To: Gilles Risch; +Cc: alexander.deucher, amd-gfx, dri-devel

[-- Attachment #1: Type: text/plain, Size: 2843 bytes --]

On Sat, May 16, 2026 at 2:52 PM Gilles Risch <gilles.risch@gmail.com> wrote:
>
> After suspend/resume the internal eDP display on iMac11,1 (and
> potentially other DCE3.1 systems) stays dark because
> atombios_set_edp_panel_power() skips panel power control for
> anything older than DCE4.
>
> Fix this by:
> - Extending atombios_set_edp_panel_power() to also handle DCE3.1
>   by changing the !ASIC_IS_DCE4() guard to !ASIC_IS_DCE31().
>   HPD polling works correctly on DCE3.1 (verified: HPD is asserted
>   at iteration 0 on iMac11,1).
> - Issuing ATOM_ENCODER_CMD_DP_VIDEO_ON/OFF for DCE3.1 in addition
>   to DCE4+.
>
> Tested on iMac11,1 (Mobility Radeon HD 4850, RV770/DCE3.1).
>
> Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
> ---
>  drivers/gpu/drm/radeon/atombios_encoders.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
> index 4e984973c043..b62fd713efcf 100644
> --- a/drivers/gpu/drm/radeon/atombios_encoders.c
> +++ b/drivers/gpu/drm/radeon/atombios_encoders.c
> @@ -1383,7 +1383,7 @@ atombios_set_edp_panel_power(struct drm_connector *connector, int action)
>         if (connector->connector_type != DRM_MODE_CONNECTOR_eDP)
>                 goto done;
>
> -       if (!ASIC_IS_DCE4(rdev))
> +       if (!ASIC_IS_DCE31(rdev))
>                 goto done;
>
>         if ((action != ATOM_TRANSMITTER_ACTION_POWER_ON) &&
> @@ -1707,7 +1707,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
>                 if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector) {
>                         /* DP_SET_POWER_D0 is set in radeon_dp_link_train */
>                         radeon_dp_link_train(encoder, connector);
> -                       if (ASIC_IS_DCE4(rdev))
> +                       if (ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev))
>                                 atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
>                 }
>                 if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) {
> @@ -1724,7 +1724,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
>         case DRM_MODE_DPMS_SUSPEND:
>         case DRM_MODE_DPMS_OFF:
>
> -               if (ASIC_IS_DCE4(rdev)) {
> +               if (ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev)) {
>                         if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector)
>                                 atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_OFF, 0);
>                 }

These changes don't make sense and will break other DCE3.c boards.   I
had the attached patch from you in one of my old branches, does it fix
the issue?

Alex

[-- Attachment #2: 0001-drm-radeon-Fix-eDP-for-single-display-iMac11-1.patch --]
[-- Type: text/x-patch, Size: 3753 bytes --]

From ca4fc2fd58dcd3bf9f6afd8d37b27a1cfcc42337 Mon Sep 17 00:00:00 2001
From: Gilles Risch <gilles.risch@gmail.com>
Date: Fri, 17 Nov 2023 20:48:24 +0100
Subject: [PATCH] drm/radeon: Fix eDP for single-display iMac11,1

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.

Fixes freedesktop issue 164
Link: https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/-/issues/164

Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/atombios_crtc.c     | 4 ++--
 drivers/gpu/drm/radeon/atombios_encoders.c | 9 +++++----
 drivers/gpu/drm/radeon/radeon.h            | 1 +
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 2fc0334e0d6c5..3c6d332739e3c 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -580,7 +580,7 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
 			radeon_crtc->pll_flags |= (/*RADEON_PLL_USE_FRAC_FB_DIV |*/
 				RADEON_PLL_PREFER_CLOSEST_LOWER);
 
-		if (ASIC_IS_DCE32(rdev) && mode->clock > 200000)	/* range limits??? */
+		if (ASIC_IS_DCE31(rdev) && mode->clock > 200000)	/* range limits??? */
 			radeon_crtc->pll_flags |= RADEON_PLL_PREFER_HIGH_FB_DIV;
 		else
 			radeon_crtc->pll_flags |= RADEON_PLL_PREFER_LOW_REF_DIV;
@@ -594,7 +594,7 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
 		if (((rdev->family == CHIP_RS780) || (rdev->family == CHIP_RS880))
 		    && !radeon_crtc->ss_enabled)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
-		if (ASIC_IS_DCE32(rdev) && mode->clock > 165000)
+		if (ASIC_IS_DCE31(rdev) && mode->clock > 165000)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
 	} else {
 		radeon_crtc->pll_flags |= RADEON_PLL_LEGACY;
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index 3d9f47bc807af..37da7961ce792 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -2123,12 +2123,13 @@ int radeon_atom_pick_dig_encoder(struct drm_encoder *encoder, int fe_idx)
 	}
 
 	/*
-	 * On DCE32 any encoder can drive any block so usually just use crtc id,
-	 * but Apple thinks different at least on iMac10,1 and iMac11,2, so there use linkb,
-	 * otherwise the internal eDP panel will stay dark.
+	 * On DCE31 and DCE32 any encoder can drive any block so usually just use crtc id,
+	 * but Apple thinks different at least on iMac10,1, iMac11,1 and iMac11,2,
+	 * so there use linkb, otherwise the internal eDP panel will stay dark.
 	 */
-	if (ASIC_IS_DCE32(rdev)) {
+	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;
 		else
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 527b9d19d7306..6b7c0abe49fb7 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2625,6 +2625,7 @@ void r100_pll_errata_after_index(struct radeon_device *rdev);
 			    (rdev->family == CHIP_RS740)  ||	\
 			    (rdev->family >= CHIP_R600))
 #define ASIC_IS_DCE3(rdev) ((rdev->family >= CHIP_RV620))
+#define ASIC_IS_DCE31(rdev) ((rdev->family >= CHIP_RV770))
 #define ASIC_IS_DCE32(rdev) ((rdev->family >= CHIP_RV730))
 #define ASIC_IS_DCE4(rdev) ((rdev->family >= CHIP_CEDAR))
 #define ASIC_IS_DCE41(rdev) ((rdev->family >= CHIP_PALM) && \
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems
  2026-05-18 14:13     ` Alex Deucher
@ 2026-05-18 14:40       ` Gilles Risch
  2026-05-18 14:57         ` Alex Deucher
  0 siblings, 1 reply; 28+ messages in thread
From: Gilles Risch @ 2026-05-18 14:40 UTC (permalink / raw)
  To: Alex Deucher; +Cc: alexander.deucher, amd-gfx, dri-devel

Am Mo., 18. Mai 2026 um 16:13 Uhr schrieb Alex Deucher <alexdeucher@gmail.com>:
>
> On Sat, May 16, 2026 at 2:52 PM Gilles Risch <gilles.risch@gmail.com> wrote:
> >
> > After suspend/resume the internal eDP display on iMac11,1 (and
> > potentially other DCE3.1 systems) stays dark because
> > atombios_set_edp_panel_power() skips panel power control for
> > anything older than DCE4.
> >
> > Fix this by:
> > - Extending atombios_set_edp_panel_power() to also handle DCE3.1
> >   by changing the !ASIC_IS_DCE4() guard to !ASIC_IS_DCE31().
> >   HPD polling works correctly on DCE3.1 (verified: HPD is asserted
> >   at iteration 0 on iMac11,1).
> > - Issuing ATOM_ENCODER_CMD_DP_VIDEO_ON/OFF for DCE3.1 in addition
> >   to DCE4+.
> >
> > Tested on iMac11,1 (Mobility Radeon HD 4850, RV770/DCE3.1).
> >
> > Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
> > ---
> >  drivers/gpu/drm/radeon/atombios_encoders.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
> > index 4e984973c043..b62fd713efcf 100644
> > --- a/drivers/gpu/drm/radeon/atombios_encoders.c
> > +++ b/drivers/gpu/drm/radeon/atombios_encoders.c
> > @@ -1383,7 +1383,7 @@ atombios_set_edp_panel_power(struct drm_connector *connector, int action)
> >         if (connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> >                 goto done;
> >
> > -       if (!ASIC_IS_DCE4(rdev))
> > +       if (!ASIC_IS_DCE31(rdev))
> >                 goto done;
> >
> >         if ((action != ATOM_TRANSMITTER_ACTION_POWER_ON) &&
> > @@ -1707,7 +1707,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
> >                 if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector) {
> >                         /* DP_SET_POWER_D0 is set in radeon_dp_link_train */
> >                         radeon_dp_link_train(encoder, connector);
> > -                       if (ASIC_IS_DCE4(rdev))
> > +                       if (ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev))
> >                                 atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
> >                 }
> >                 if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) {
> > @@ -1724,7 +1724,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
> >         case DRM_MODE_DPMS_SUSPEND:
> >         case DRM_MODE_DPMS_OFF:
> >
> > -               if (ASIC_IS_DCE4(rdev)) {
> > +               if (ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev)) {
> >                         if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector)
> >                                 atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_OFF, 0);
> >                 }
>
> These changes don't make sense and will break other DCE3.c boards.   I
> had the attached patch from you in one of my old branches, does it fix
> the issue?
>
> Alex

While the initial attached patch resolved the dark screen issue after
a fresh boot, this follow-up patch addresses the same problem
occurring after resuming from suspend. Should I used dmi_match() to
avoid breaking other DCE3.c boards?

Regards,
Gilles

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems
  2026-05-18 14:40       ` Gilles Risch
@ 2026-05-18 14:57         ` Alex Deucher
  2026-05-18 21:14           ` [PATCH v3 0/2] drm/radeon: fix iMac11,1 dark display Gilles Risch
                             ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Alex Deucher @ 2026-05-18 14:57 UTC (permalink / raw)
  To: Gilles Risch; +Cc: alexander.deucher, amd-gfx, dri-devel

On Mon, May 18, 2026 at 10:40 AM Gilles Risch <gilles.risch@gmail.com> wrote:
>
> Am Mo., 18. Mai 2026 um 16:13 Uhr schrieb Alex Deucher <alexdeucher@gmail.com>:
> >
> > On Sat, May 16, 2026 at 2:52 PM Gilles Risch <gilles.risch@gmail.com> wrote:
> > >
> > > After suspend/resume the internal eDP display on iMac11,1 (and
> > > potentially other DCE3.1 systems) stays dark because
> > > atombios_set_edp_panel_power() skips panel power control for
> > > anything older than DCE4.
> > >
> > > Fix this by:
> > > - Extending atombios_set_edp_panel_power() to also handle DCE3.1
> > >   by changing the !ASIC_IS_DCE4() guard to !ASIC_IS_DCE31().
> > >   HPD polling works correctly on DCE3.1 (verified: HPD is asserted
> > >   at iteration 0 on iMac11,1).
> > > - Issuing ATOM_ENCODER_CMD_DP_VIDEO_ON/OFF for DCE3.1 in addition
> > >   to DCE4+.
> > >
> > > Tested on iMac11,1 (Mobility Radeon HD 4850, RV770/DCE3.1).
> > >
> > > Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
> > > ---
> > >  drivers/gpu/drm/radeon/atombios_encoders.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
> > > index 4e984973c043..b62fd713efcf 100644
> > > --- a/drivers/gpu/drm/radeon/atombios_encoders.c
> > > +++ b/drivers/gpu/drm/radeon/atombios_encoders.c
> > > @@ -1383,7 +1383,7 @@ atombios_set_edp_panel_power(struct drm_connector *connector, int action)
> > >         if (connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> > >                 goto done;
> > >
> > > -       if (!ASIC_IS_DCE4(rdev))
> > > +       if (!ASIC_IS_DCE31(rdev))
> > >                 goto done;
> > >
> > >         if ((action != ATOM_TRANSMITTER_ACTION_POWER_ON) &&
> > > @@ -1707,7 +1707,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
> > >                 if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector) {
> > >                         /* DP_SET_POWER_D0 is set in radeon_dp_link_train */
> > >                         radeon_dp_link_train(encoder, connector);
> > > -                       if (ASIC_IS_DCE4(rdev))
> > > +                       if (ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev))
> > >                                 atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
> > >                 }
> > >                 if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) {
> > > @@ -1724,7 +1724,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
> > >         case DRM_MODE_DPMS_SUSPEND:
> > >         case DRM_MODE_DPMS_OFF:
> > >
> > > -               if (ASIC_IS_DCE4(rdev)) {
> > > +               if (ASIC_IS_DCE4(rdev) || ASIC_IS_DCE31(rdev)) {
> > >                         if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector)
> > >                                 atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_OFF, 0);
> > >                 }
> >
> > These changes don't make sense and will break other DCE3.c boards.   I
> > had the attached patch from you in one of my old branches, does it fix
> > the issue?
> >
> > Alex
>
> While the initial attached patch resolved the dark screen issue after
> a fresh boot, this follow-up patch addresses the same problem
> occurring after resuming from suspend. Should I used dmi_match() to
> avoid breaking other DCE3.c boards?

Yes.  Even that is a hack.  You are effectively just skipping some of
the required programming sequences.  Presumably the mac is wired up
strangely or there are issues with its atom tables.  Ideally we'd sort
that out, but this hardware is pretty old so I'm ok with a hack.

Alex

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 0/2] drm/radeon: fix iMac11,1 dark display
  2026-05-18 14:57         ` Alex Deucher
@ 2026-05-18 21:14           ` Gilles Risch
  2026-06-04  3:37             ` Claude review: " Claude Code Review Bot
  2026-05-18 21:14           ` [PATCH v3 1/2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
  2026-05-18 21:14           ` [PATCH v3 2/2] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Gilles Risch
  2 siblings, 1 reply; 28+ messages in thread
From: Gilles Risch @ 2026-05-18 21:14 UTC (permalink / raw)
  To: alexander.deucher; +Cc: amd-gfx, dri-devel, lkp, Gilles Risch

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1487 bytes --]

Hello Alex,

thank you for the feedback. I've updated the patch to use dmi_match()
as suggested, limited to iMac11,1 only since that is the only machine
I have tested so far.

I will have access to other older Apple iMacs which also show display
issues under Linux. Given your comment about unusual wiring and
potentially broken atom tables on these machines, it would make sense
to investigate this more thoroughly in the long run. I would be happy
to help with testing if that would be useful.

The build error reported by the kernel test robot is also fixed ��� the
previous version was missing the dependency on the ASIC_IS_DCE31()
macro introduced in patch 1. Both patches are now submitted together
as a series.

Changes in v3:
- Submit both patches together as a series to fix the build dependency
- Use dmi_match(DMI_PRODUCT_NAME, "iMac11,1") instead of
  ASIC_IS_DCE31() to avoid breaking other DCE3.x boards
  (suggested by Alex Deucher)
- Limit fix to iMac11,1 only (only tested machine)

Tested on iMac11,1 with Debian Linux.

Gilles Risch (2):
  drm/radeon: fix internal display on iMac11,1 (RV770/DCE3.1)
  drm/radeon: fix eDP resume from suspend on iMac11,1 / DCE3.1 systems

 drivers/gpu/drm/radeon/atombios_crtc.c     |  4 ++--
 drivers/gpu/drm/radeon/atombios_encoders.c | 16 +++++++++-------
 drivers/gpu/drm/radeon/radeon.h            |  1 +
 3 files changed, 12 insertions(+), 9 deletions(-)


base-commit: 6916d5703ddf9a38f1f6c2cc793381a24ee914c6
-- 
2.47.3


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 1/2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-18 14:57         ` Alex Deucher
  2026-05-18 21:14           ` [PATCH v3 0/2] drm/radeon: fix iMac11,1 dark display Gilles Risch
@ 2026-05-18 21:14           ` Gilles Risch
  2026-05-19 13:35             ` Alex Deucher
  2026-05-18 21:14           ` [PATCH v3 2/2] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Gilles Risch
  2 siblings, 1 reply; 28+ messages in thread
From: Gilles Risch @ 2026-05-18 21:14 UTC (permalink / raw)
  To: alexander.deucher; +Cc: amd-gfx, dri-devel, lkp, Gilles Risch

The Apple iMac11,1 (late 2009) has an integrated ATI Mobility Radeon
HD 4850 (RV770/M98L) with a 2560x1440 internal panel connected via an
internal DisplayPort path. This machine suffers from a similar problem
as the iMac10,1 (late 2009) and the iMac11,2 (mid 2010). Without this
fix the display stays dark under KMS. Two issues are addressed:

1. The RV770 implements DCE3.1 and not DCE3.2. ASIC_IS_DCE32() starts at
   CHIP_RV730 which is newer than RV770, so the RV770 never matched the
   DCE3.2 PLL and encoder logic. Introduce ASIC_IS_DCE31() starting at
   CHIP_RV770 to fix this.

2. Apple routed the internal display through Link B of the DIG encoder
   instead of Link A, as observed in the kernel display connector log.
   The same quirk already exists for iMac10,1 and iMac11,2 - iMac11,1
   was simply missing from the list.

Note: resume from suspend still results in a dark screen as the DP
re-driver chips on the mainboard lose their state during power-off.
This will be addressed in a follow-up patch.

Fixes freedesktop issue 164
Link: https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/-/issues/164

Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
---
v3: No code changes. 

 drivers/gpu/drm/radeon/atombios_crtc.c     | 4 ++--
 drivers/gpu/drm/radeon/atombios_encoders.c | 9 +++++----
 drivers/gpu/drm/radeon/radeon.h            | 1 +
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 2fc0334e0d6c..3c6d332739e3 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -580,7 +580,7 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
 			radeon_crtc->pll_flags |= (/*RADEON_PLL_USE_FRAC_FB_DIV |*/
 				RADEON_PLL_PREFER_CLOSEST_LOWER);
 
-		if (ASIC_IS_DCE32(rdev) && mode->clock > 200000)	/* range limits??? */
+		if (ASIC_IS_DCE31(rdev) && mode->clock > 200000)	/* range limits??? */
 			radeon_crtc->pll_flags |= RADEON_PLL_PREFER_HIGH_FB_DIV;
 		else
 			radeon_crtc->pll_flags |= RADEON_PLL_PREFER_LOW_REF_DIV;
@@ -594,7 +594,7 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
 		if (((rdev->family == CHIP_RS780) || (rdev->family == CHIP_RS880))
 		    && !radeon_crtc->ss_enabled)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
-		if (ASIC_IS_DCE32(rdev) && mode->clock > 165000)
+		if (ASIC_IS_DCE31(rdev) && mode->clock > 165000)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
 	} else {
 		radeon_crtc->pll_flags |= RADEON_PLL_LEGACY;
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index 5cfd8fcfa5e8..4e984973c043 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -2123,12 +2123,13 @@ int radeon_atom_pick_dig_encoder(struct drm_encoder *encoder, int fe_idx)
 	}
 
 	/*
-	 * On DCE32 any encoder can drive any block so usually just use crtc id,
-	 * but Apple thinks different at least on iMac10,1 and iMac11,2, so there use linkb,
-	 * otherwise the internal eDP panel will stay dark.
+	 * On DCE31 and DCE32 any encoder can drive any block so usually just use crtc id,
+	 * but Apple thinks different at least on iMac10,1, iMac11,1 and iMac11,2,
+	 * so there use linkb, otherwise the internal eDP panel will stay dark.
 	 */
-	if (ASIC_IS_DCE32(rdev)) {
+	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;
 		else
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 527b9d19d730..6b7c0abe49fb 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2625,6 +2625,7 @@ void r100_pll_errata_after_index(struct radeon_device *rdev);
 			    (rdev->family == CHIP_RS740)  ||	\
 			    (rdev->family >= CHIP_R600))
 #define ASIC_IS_DCE3(rdev) ((rdev->family >= CHIP_RV620))
+#define ASIC_IS_DCE31(rdev) ((rdev->family >= CHIP_RV770))
 #define ASIC_IS_DCE32(rdev) ((rdev->family >= CHIP_RV730))
 #define ASIC_IS_DCE4(rdev) ((rdev->family >= CHIP_CEDAR))
 #define ASIC_IS_DCE41(rdev) ((rdev->family >= CHIP_PALM) && \
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 2/2] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems
  2026-05-18 14:57         ` Alex Deucher
  2026-05-18 21:14           ` [PATCH v3 0/2] drm/radeon: fix iMac11,1 dark display Gilles Risch
  2026-05-18 21:14           ` [PATCH v3 1/2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
@ 2026-05-18 21:14           ` Gilles Risch
  2026-05-19 13:42             ` Alex Deucher
  2 siblings, 1 reply; 28+ messages in thread
From: Gilles Risch @ 2026-05-18 21:14 UTC (permalink / raw)
  To: alexander.deucher; +Cc: amd-gfx, dri-devel, lkp, Gilles Risch

After suspend/resume the internal eDP display on Apple iMac11,1,
stays dark because atombios_set_edp_panel_power() skips panel
power control for anything older than DCE4, and
ATOM_ENCODER_CMD_DP_VIDEO_ON/OFF is required on ths iMac.

Tested on iMac11,1 (Mobility Radeon HD 4850, RV770/DCE3.1).

Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
---
 drivers/gpu/drm/radeon/atombios_encoders.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index 4e984973c043..0a699a887ee9 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -1384,7 +1384,8 @@ atombios_set_edp_panel_power(struct drm_connector *connector, int action)
 		goto done;
 
 	if (!ASIC_IS_DCE4(rdev))
-		goto done;
+	    if (!dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
+		    goto done;
 
 	if ((action != ATOM_TRANSMITTER_ACTION_POWER_ON) &&
 	    (action != ATOM_TRANSMITTER_ACTION_POWER_OFF))
@@ -1707,7 +1708,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
 		if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector) {
 			/* DP_SET_POWER_D0 is set in radeon_dp_link_train */
 			radeon_dp_link_train(encoder, connector);
-			if (ASIC_IS_DCE4(rdev))
+			if (ASIC_IS_DCE4(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
 				atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
 		}
 		if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) {
@@ -1724,7 +1725,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
 	case DRM_MODE_DPMS_SUSPEND:
 	case DRM_MODE_DPMS_OFF:
 
-		if (ASIC_IS_DCE4(rdev)) {
+		if (ASIC_IS_DCE4(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1")) {
 			if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector)
 				atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_OFF, 0);
 		}
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-18 21:14           ` [PATCH v3 1/2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
@ 2026-05-19 13:35             ` Alex Deucher
  0 siblings, 0 replies; 28+ messages in thread
From: Alex Deucher @ 2026-05-19 13:35 UTC (permalink / raw)
  To: Gilles Risch; +Cc: alexander.deucher, amd-gfx, dri-devel, lkp

On Tue, May 19, 2026 at 3:19 AM Gilles Risch <gilles.risch@gmail.com> wrote:
>
> The Apple iMac11,1 (late 2009) has an integrated ATI Mobility Radeon
> HD 4850 (RV770/M98L) with a 2560x1440 internal panel connected via an
> internal DisplayPort path. This machine suffers from a similar problem
> as the iMac10,1 (late 2009) and the iMac11,2 (mid 2010). Without this
> fix the display stays dark under KMS. Two issues are addressed:
>
> 1. The RV770 implements DCE3.1 and not DCE3.2. ASIC_IS_DCE32() starts at
>    CHIP_RV730 which is newer than RV770, so the RV770 never matched the
>    DCE3.2 PLL and encoder logic. Introduce ASIC_IS_DCE31() starting at
>    CHIP_RV770 to fix this.
>
> 2. Apple routed the internal display through Link B of the DIG encoder
>    instead of Link A, as observed in the kernel display connector log.
>    The same quirk already exists for iMac10,1 and iMac11,2 - iMac11,1
>    was simply missing from the list.
>
> Note: resume from suspend still results in a dark screen as the DP
> re-driver chips on the mainboard lose their state during power-off.
> This will be addressed in a follow-up patch.
>
> Fixes freedesktop issue 164
> Link: https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/-/issues/164
>
> Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
> ---
> v3: No code changes.
>
>  drivers/gpu/drm/radeon/atombios_crtc.c     | 4 ++--
>  drivers/gpu/drm/radeon/atombios_encoders.c | 9 +++++----
>  drivers/gpu/drm/radeon/radeon.h            | 1 +
>  3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
> index 2fc0334e0d6c..3c6d332739e3 100644
> --- a/drivers/gpu/drm/radeon/atombios_crtc.c
> +++ b/drivers/gpu/drm/radeon/atombios_crtc.c
> @@ -580,7 +580,7 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
>                         radeon_crtc->pll_flags |= (/*RADEON_PLL_USE_FRAC_FB_DIV |*/
>                                 RADEON_PLL_PREFER_CLOSEST_LOWER);
>
> -               if (ASIC_IS_DCE32(rdev) && mode->clock > 200000)        /* range limits??? */
> +               if (ASIC_IS_DCE31(rdev) && mode->clock > 200000)        /* range limits??? */
>                         radeon_crtc->pll_flags |= RADEON_PLL_PREFER_HIGH_FB_DIV;
>                 else
>                         radeon_crtc->pll_flags |= RADEON_PLL_PREFER_LOW_REF_DIV;
> @@ -594,7 +594,7 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
>                 if (((rdev->family == CHIP_RS780) || (rdev->family == CHIP_RS880))
>                     && !radeon_crtc->ss_enabled)
>                         radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
> -               if (ASIC_IS_DCE32(rdev) && mode->clock > 165000)
> +               if (ASIC_IS_DCE31(rdev) && mode->clock > 165000)

Can you switch these to dmi matches as well?  This change will also
change the behavior for other boards with respect to the PLL
calculations which are often pretty sensitive and may cause
regressions.

Alex

>                         radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
>         } else {
>                 radeon_crtc->pll_flags |= RADEON_PLL_LEGACY;
> diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
> index 5cfd8fcfa5e8..4e984973c043 100644
> --- a/drivers/gpu/drm/radeon/atombios_encoders.c
> +++ b/drivers/gpu/drm/radeon/atombios_encoders.c
> @@ -2123,12 +2123,13 @@ int radeon_atom_pick_dig_encoder(struct drm_encoder *encoder, int fe_idx)
>         }
>
>         /*
> -        * On DCE32 any encoder can drive any block so usually just use crtc id,
> -        * but Apple thinks different at least on iMac10,1 and iMac11,2, so there use linkb,
> -        * otherwise the internal eDP panel will stay dark.
> +        * On DCE31 and DCE32 any encoder can drive any block so usually just use crtc id,
> +        * but Apple thinks different at least on iMac10,1, iMac11,1 and iMac11,2,
> +        * so there use linkb, otherwise the internal eDP panel will stay dark.
>          */
> -       if (ASIC_IS_DCE32(rdev)) {
> +       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;
>                 else
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 527b9d19d730..6b7c0abe49fb 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2625,6 +2625,7 @@ void r100_pll_errata_after_index(struct radeon_device *rdev);
>                             (rdev->family == CHIP_RS740)  ||    \
>                             (rdev->family >= CHIP_R600))
>  #define ASIC_IS_DCE3(rdev) ((rdev->family >= CHIP_RV620))
> +#define ASIC_IS_DCE31(rdev) ((rdev->family >= CHIP_RV770))
>  #define ASIC_IS_DCE32(rdev) ((rdev->family >= CHIP_RV730))
>  #define ASIC_IS_DCE4(rdev) ((rdev->family >= CHIP_CEDAR))
>  #define ASIC_IS_DCE41(rdev) ((rdev->family >= CHIP_PALM) && \
> --
> 2.47.3
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 2/2] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems
  2026-05-18 21:14           ` [PATCH v3 2/2] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Gilles Risch
@ 2026-05-19 13:42             ` Alex Deucher
  2026-05-27 22:20               ` [PATCH v4] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
  0 siblings, 1 reply; 28+ messages in thread
From: Alex Deucher @ 2026-05-19 13:42 UTC (permalink / raw)
  To: Gilles Risch; +Cc: alexander.deucher, amd-gfx, dri-devel, lkp

On Tue, May 19, 2026 at 3:44 AM Gilles Risch <gilles.risch@gmail.com> wrote:
>
> After suspend/resume the internal eDP display on Apple iMac11,1,
> stays dark because atombios_set_edp_panel_power() skips panel
> power control for anything older than DCE4, and
> ATOM_ENCODER_CMD_DP_VIDEO_ON/OFF is required on ths iMac.
>
> Tested on iMac11,1 (Mobility Radeon HD 4850, RV770/DCE3.1).
>
> Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
> ---
>  drivers/gpu/drm/radeon/atombios_encoders.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
> index 4e984973c043..0a699a887ee9 100644
> --- a/drivers/gpu/drm/radeon/atombios_encoders.c
> +++ b/drivers/gpu/drm/radeon/atombios_encoders.c
> @@ -1384,7 +1384,8 @@ atombios_set_edp_panel_power(struct drm_connector *connector, int action)
>                 goto done;
>
>         if (!ASIC_IS_DCE4(rdev))
> -               goto done;
> +           if (!dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
> +                   goto done;

This logic is harder to follow.  I would just add an additional check.  E.g.,

         if (!ASIC_IS_DCE4(rdev))
               goto done;

          if (!dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
                  goto done;

Other than that, looks good to me.

>
>         if ((action != ATOM_TRANSMITTER_ACTION_POWER_ON) &&
>             (action != ATOM_TRANSMITTER_ACTION_POWER_OFF))
> @@ -1707,7 +1708,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
>                 if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector) {
>                         /* DP_SET_POWER_D0 is set in radeon_dp_link_train */
>                         radeon_dp_link_train(encoder, connector);
> -                       if (ASIC_IS_DCE4(rdev))
> +                       if (ASIC_IS_DCE4(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
>                                 atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
>                 }
>                 if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) {
> @@ -1724,7 +1725,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
>         case DRM_MODE_DPMS_SUSPEND:
>         case DRM_MODE_DPMS_OFF:
>
> -               if (ASIC_IS_DCE4(rdev)) {
> +               if (ASIC_IS_DCE4(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1")) {
>                         if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector)
>                                 atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_OFF, 0);
>                 }
> --
> 2.47.3
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v4] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-19 13:42             ` Alex Deucher
@ 2026-05-27 22:20               ` Gilles Risch
  2026-05-28 18:53                 ` Alex Deucher
  0 siblings, 1 reply; 28+ messages in thread
From: Gilles Risch @ 2026-05-27 22:20 UTC (permalink / raw)
  To: alexander.deucher; +Cc: amd-gfx, dri-devel, Gilles Risch

The Apple iMac11,1 (27-inch, Late 2009) uses a Mobility Radeon HD 4850
(RV770/DCE3.1) with a 2560x1440 internal panel on an internal
DisplayPort path. Without this fix the display stays dark under KMS.

The same problem already exists for iMac10,1 and iMac11,2. Three fixes:

1. Introduce ASIC_IS_DCE31() for CHIP_RV770 and newer. Note that
   CHIP_RV770 precedes CHIP_RV730 in the enum, so ASIC_IS_DCE31()
   subsumes ASIC_IS_DCE32().

2. Extend the Link B encoder quirk from ASIC_IS_DCE32() to
   ASIC_IS_DCE31() and add iMac11,1 to the DMI list.

3. The 2560x1440 panel needs RADEON_PLL_USE_FRAC_FB_DIV and
   ATOM_ENCODER_CMD_DP_VIDEO_ON. These are limited to iMac11,1
   via dmi_match() to avoid affecting other DCE3.1 boards.

Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
---
 drivers/gpu/drm/radeon/atombios_crtc.c     |  5 ++++-
 drivers/gpu/drm/radeon/atombios_encoders.c | 11 ++++++-----
 drivers/gpu/drm/radeon/radeon.h            |  1 +
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 2fc0334e0..075eba2d4 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -24,6 +24,8 @@
  *          Alex Deucher
  */
 
+#include <linux/dmi.h>
+
 #include <drm/drm_fixed.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
@@ -594,7 +596,8 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
 		if (((rdev->family == CHIP_RS780) || (rdev->family == CHIP_RS880))
 		    && !radeon_crtc->ss_enabled)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
-		if (ASIC_IS_DCE32(rdev) && mode->clock > 165000)
+		if ((ASIC_IS_DCE32(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
+		    && mode->clock > 165000)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
 	} else {
 		radeon_crtc->pll_flags |= RADEON_PLL_LEGACY;
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index 5cfd8fcfa..4de60559f 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -1707,7 +1707,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
 		if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector) {
 			/* DP_SET_POWER_D0 is set in radeon_dp_link_train */
 			radeon_dp_link_train(encoder, connector);
-			if (ASIC_IS_DCE4(rdev))
+			if (ASIC_IS_DCE4(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
 				atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
 		}
 		if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) {
@@ -2123,12 +2123,13 @@ int radeon_atom_pick_dig_encoder(struct drm_encoder *encoder, int fe_idx)
 	}
 
 	/*
-	 * On DCE32 any encoder can drive any block so usually just use crtc id,
-	 * but Apple thinks different at least on iMac10,1 and iMac11,2, so there use linkb,
-	 * otherwise the internal eDP panel will stay dark.
+	 * On DCE31 and DCE32 any encoder can drive any block so usually just use crtc id,
+	 * but Apple thinks different at least on iMac10,1, iMac11,1 and iMac11,2,
+	 * so there use linkb, otherwise the internal eDP panel will stay dark.
 	 */
-	if (ASIC_IS_DCE32(rdev)) {
+	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;
 		else
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 527b9d19d..6b7c0abe4 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2625,6 +2625,7 @@ void r100_pll_errata_after_index(struct radeon_device *rdev);
 			    (rdev->family == CHIP_RS740)  ||	\
 			    (rdev->family >= CHIP_R600))
 #define ASIC_IS_DCE3(rdev) ((rdev->family >= CHIP_RV620))
+#define ASIC_IS_DCE31(rdev) ((rdev->family >= CHIP_RV770))
 #define ASIC_IS_DCE32(rdev) ((rdev->family >= CHIP_RV730))
 #define ASIC_IS_DCE4(rdev) ((rdev->family >= CHIP_CEDAR))
 #define ASIC_IS_DCE41(rdev) ((rdev->family >= CHIP_PALM) && \

base-commit: e7ae89a0c97ce2b68b0983cd01eda67cf373517d
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v4] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-27 22:20               ` [PATCH v4] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
@ 2026-05-28 18:53                 ` Alex Deucher
  2026-05-29  7:43                   ` Gilles Risch
  0 siblings, 1 reply; 28+ messages in thread
From: Alex Deucher @ 2026-05-28 18:53 UTC (permalink / raw)
  To: Gilles Risch; +Cc: alexander.deucher, amd-gfx, dri-devel

On Thu, May 28, 2026 at 3:39 AM Gilles Risch <gilles.risch@gmail.com> wrote:
>
> The Apple iMac11,1 (27-inch, Late 2009) uses a Mobility Radeon HD 4850
> (RV770/DCE3.1) with a 2560x1440 internal panel on an internal
> DisplayPort path. Without this fix the display stays dark under KMS.
>
> The same problem already exists for iMac10,1 and iMac11,2. Three fixes:
>
> 1. Introduce ASIC_IS_DCE31() for CHIP_RV770 and newer. Note that
>    CHIP_RV770 precedes CHIP_RV730 in the enum, so ASIC_IS_DCE31()
>    subsumes ASIC_IS_DCE32().
>
> 2. Extend the Link B encoder quirk from ASIC_IS_DCE32() to
>    ASIC_IS_DCE31() and add iMac11,1 to the DMI list.
>
> 3. The 2560x1440 panel needs RADEON_PLL_USE_FRAC_FB_DIV and
>    ATOM_ENCODER_CMD_DP_VIDEO_ON. These are limited to iMac11,1
>    via dmi_match() to avoid affecting other DCE3.1 boards.
>
> Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
> ---
>  drivers/gpu/drm/radeon/atombios_crtc.c     |  5 ++++-
>  drivers/gpu/drm/radeon/atombios_encoders.c | 11 ++++++-----
>  drivers/gpu/drm/radeon/radeon.h            |  1 +
>  3 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
> index 2fc0334e0..075eba2d4 100644
> --- a/drivers/gpu/drm/radeon/atombios_crtc.c
> +++ b/drivers/gpu/drm/radeon/atombios_crtc.c
> @@ -24,6 +24,8 @@
>   *          Alex Deucher
>   */
>
> +#include <linux/dmi.h>
> +
>  #include <drm/drm_fixed.h>
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_framebuffer.h>
> @@ -594,7 +596,8 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
>                 if (((rdev->family == CHIP_RS780) || (rdev->family == CHIP_RS880))
>                     && !radeon_crtc->ss_enabled)
>                         radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
> -               if (ASIC_IS_DCE32(rdev) && mode->clock > 165000)
> +               if ((ASIC_IS_DCE32(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
> +                   && mode->clock > 165000)
>                         radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
>         } else {
>                 radeon_crtc->pll_flags |= RADEON_PLL_LEGACY;
> diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
> index 5cfd8fcfa..4de60559f 100644
> --- a/drivers/gpu/drm/radeon/atombios_encoders.c
> +++ b/drivers/gpu/drm/radeon/atombios_encoders.c
> @@ -1707,7 +1707,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
>                 if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector) {
>                         /* DP_SET_POWER_D0 is set in radeon_dp_link_train */
>                         radeon_dp_link_train(encoder, connector);
> -                       if (ASIC_IS_DCE4(rdev))
> +                       if (ASIC_IS_DCE4(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
>                                 atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
>                 }
>                 if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) {
> @@ -2123,12 +2123,13 @@ int radeon_atom_pick_dig_encoder(struct drm_encoder *encoder, int fe_idx)
>         }
>
>         /*
> -        * On DCE32 any encoder can drive any block so usually just use crtc id,
> -        * but Apple thinks different at least on iMac10,1 and iMac11,2, so there use linkb,
> -        * otherwise the internal eDP panel will stay dark.
> +        * On DCE31 and DCE32 any encoder can drive any block so usually just use crtc id,
> +        * but Apple thinks different at least on iMac10,1, iMac11,1 and iMac11,2,
> +        * so there use linkb, otherwise the internal eDP panel will stay dark.
>          */
> -       if (ASIC_IS_DCE32(rdev)) {
> +       if (ASIC_IS_DCE31(rdev)) {

This will change the routing for DCE3.1 as well.  Probably best to do
something like:

if (dmi_match(DMI_PRODUCT_NAME, "iMac11,1")) {
    enc_idx = (dig->linkb) ? 1 : 0;
    goto assigned;
}

before or after the DCE32() block.

>                 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;
>                 else
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 527b9d19d..6b7c0abe4 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2625,6 +2625,7 @@ void r100_pll_errata_after_index(struct radeon_device *rdev);
>                             (rdev->family == CHIP_RS740)  ||    \
>                             (rdev->family >= CHIP_R600))
>  #define ASIC_IS_DCE3(rdev) ((rdev->family >= CHIP_RV620))
> +#define ASIC_IS_DCE31(rdev) ((rdev->family >= CHIP_RV770))
>  #define ASIC_IS_DCE32(rdev) ((rdev->family >= CHIP_RV730))
>  #define ASIC_IS_DCE4(rdev) ((rdev->family >= CHIP_CEDAR))
>  #define ASIC_IS_DCE41(rdev) ((rdev->family >= CHIP_PALM) && \
>
> base-commit: e7ae89a0c97ce2b68b0983cd01eda67cf373517d
> --
> 2.47.3
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-28 18:53                 ` Alex Deucher
@ 2026-05-29  7:43                   ` Gilles Risch
  2026-05-29 13:31                     ` Alex Deucher
  0 siblings, 1 reply; 28+ messages in thread
From: Gilles Risch @ 2026-05-29  7:43 UTC (permalink / raw)
  To: Alex Deucher; +Cc: alexander.deucher, amd-gfx, dri-devel

Good morning,

I'll make that adjustment, shouldn't I just regroup all three iMac
machines in one if statement?

Am Do., 28. Mai 2026 um 20:53 Uhr schrieb Alex Deucher <alexdeucher@gmail.com>:
>
> On Thu, May 28, 2026 at 3:39 AM Gilles Risch <gilles.risch@gmail.com> wrote:
> >
> > The Apple iMac11,1 (27-inch, Late 2009) uses a Mobility Radeon HD 4850
> > (RV770/DCE3.1) with a 2560x1440 internal panel on an internal
> > DisplayPort path. Without this fix the display stays dark under KMS.
> >
> > The same problem already exists for iMac10,1 and iMac11,2. Three fixes:
> >
> > 1. Introduce ASIC_IS_DCE31() for CHIP_RV770 and newer. Note that
> >    CHIP_RV770 precedes CHIP_RV730 in the enum, so ASIC_IS_DCE31()
> >    subsumes ASIC_IS_DCE32().
> >
> > 2. Extend the Link B encoder quirk from ASIC_IS_DCE32() to
> >    ASIC_IS_DCE31() and add iMac11,1 to the DMI list.
> >
> > 3. The 2560x1440 panel needs RADEON_PLL_USE_FRAC_FB_DIV and
> >    ATOM_ENCODER_CMD_DP_VIDEO_ON. These are limited to iMac11,1
> >    via dmi_match() to avoid affecting other DCE3.1 boards.
> >
> > Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
> > ---
> >  drivers/gpu/drm/radeon/atombios_crtc.c     |  5 ++++-
> >  drivers/gpu/drm/radeon/atombios_encoders.c | 11 ++++++-----
> >  drivers/gpu/drm/radeon/radeon.h            |  1 +
> >  3 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
> > index 2fc0334e0..075eba2d4 100644
> > --- a/drivers/gpu/drm/radeon/atombios_crtc.c
> > +++ b/drivers/gpu/drm/radeon/atombios_crtc.c
> > @@ -24,6 +24,8 @@
> >   *          Alex Deucher
> >   */
> >
> > +#include <linux/dmi.h>
> > +
> >  #include <drm/drm_fixed.h>
> >  #include <drm/drm_fourcc.h>
> >  #include <drm/drm_framebuffer.h>
> > @@ -594,7 +596,8 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
> >                 if (((rdev->family == CHIP_RS780) || (rdev->family == CHIP_RS880))
> >                     && !radeon_crtc->ss_enabled)
> >                         radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
> > -               if (ASIC_IS_DCE32(rdev) && mode->clock > 165000)
> > +               if ((ASIC_IS_DCE32(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
> > +                   && mode->clock > 165000)
> >                         radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
> >         } else {
> >                 radeon_crtc->pll_flags |= RADEON_PLL_LEGACY;
> > diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
> > index 5cfd8fcfa..4de60559f 100644
> > --- a/drivers/gpu/drm/radeon/atombios_encoders.c
> > +++ b/drivers/gpu/drm/radeon/atombios_encoders.c
> > @@ -1707,7 +1707,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
> >                 if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector) {
> >                         /* DP_SET_POWER_D0 is set in radeon_dp_link_train */
> >                         radeon_dp_link_train(encoder, connector);
> > -                       if (ASIC_IS_DCE4(rdev))
> > +                       if (ASIC_IS_DCE4(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
> >                                 atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
> >                 }
> >                 if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) {
> > @@ -2123,12 +2123,13 @@ int radeon_atom_pick_dig_encoder(struct drm_encoder *encoder, int fe_idx)
> >         }
> >
> >         /*
> > -        * On DCE32 any encoder can drive any block so usually just use crtc id,
> > -        * but Apple thinks different at least on iMac10,1 and iMac11,2, so there use linkb,
> > -        * otherwise the internal eDP panel will stay dark.
> > +        * On DCE31 and DCE32 any encoder can drive any block so usually just use crtc id,
> > +        * but Apple thinks different at least on iMac10,1, iMac11,1 and iMac11,2,
> > +        * so there use linkb, otherwise the internal eDP panel will stay dark.
> >          */
> > -       if (ASIC_IS_DCE32(rdev)) {
> > +       if (ASIC_IS_DCE31(rdev)) {
>
> This will change the routing for DCE3.1 as well.  Probably best to do
> something like:
>
> if (dmi_match(DMI_PRODUCT_NAME, "iMac11,1")) {
>     enc_idx = (dig->linkb) ? 1 : 0;
>     goto assigned;
> }
>
> before or after the DCE32() block.
>
> >                 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;
> >                 else
> > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> > index 527b9d19d..6b7c0abe4 100644
> > --- a/drivers/gpu/drm/radeon/radeon.h
> > +++ b/drivers/gpu/drm/radeon/radeon.h
> > @@ -2625,6 +2625,7 @@ void r100_pll_errata_after_index(struct radeon_device *rdev);
> >                             (rdev->family == CHIP_RS740)  ||    \
> >                             (rdev->family >= CHIP_R600))
> >  #define ASIC_IS_DCE3(rdev) ((rdev->family >= CHIP_RV620))
> > +#define ASIC_IS_DCE31(rdev) ((rdev->family >= CHIP_RV770))
> >  #define ASIC_IS_DCE32(rdev) ((rdev->family >= CHIP_RV730))
> >  #define ASIC_IS_DCE4(rdev) ((rdev->family >= CHIP_CEDAR))
> >  #define ASIC_IS_DCE41(rdev) ((rdev->family >= CHIP_PALM) && \
> >
> > base-commit: e7ae89a0c97ce2b68b0983cd01eda67cf373517d
> > --
> > 2.47.3
> >



-- 
Gilles

Sent from my Commodore C=64

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-29  7:43                   ` Gilles Risch
@ 2026-05-29 13:31                     ` Alex Deucher
  2026-06-01 21:19                       ` [PATCH v5] " Gilles Risch
  0 siblings, 1 reply; 28+ messages in thread
From: Alex Deucher @ 2026-05-29 13:31 UTC (permalink / raw)
  To: Gilles Risch; +Cc: alexander.deucher, amd-gfx, dri-devel

On Fri, May 29, 2026 at 3:43 AM Gilles Risch <gilles.risch@gmail.com> wrote:
>
> Good morning,
>
> I'll make that adjustment, shouldn't I just regroup all three iMac
> machines in one if statement?

Yeah, that works too.

Alex

>
> Am Do., 28. Mai 2026 um 20:53 Uhr schrieb Alex Deucher <alexdeucher@gmail.com>:
> >
> > On Thu, May 28, 2026 at 3:39 AM Gilles Risch <gilles.risch@gmail.com> wrote:
> > >
> > > The Apple iMac11,1 (27-inch, Late 2009) uses a Mobility Radeon HD 4850
> > > (RV770/DCE3.1) with a 2560x1440 internal panel on an internal
> > > DisplayPort path. Without this fix the display stays dark under KMS.
> > >
> > > The same problem already exists for iMac10,1 and iMac11,2. Three fixes:
> > >
> > > 1. Introduce ASIC_IS_DCE31() for CHIP_RV770 and newer. Note that
> > >    CHIP_RV770 precedes CHIP_RV730 in the enum, so ASIC_IS_DCE31()
> > >    subsumes ASIC_IS_DCE32().
> > >
> > > 2. Extend the Link B encoder quirk from ASIC_IS_DCE32() to
> > >    ASIC_IS_DCE31() and add iMac11,1 to the DMI list.
> > >
> > > 3. The 2560x1440 panel needs RADEON_PLL_USE_FRAC_FB_DIV and
> > >    ATOM_ENCODER_CMD_DP_VIDEO_ON. These are limited to iMac11,1
> > >    via dmi_match() to avoid affecting other DCE3.1 boards.
> > >
> > > Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
> > > ---
> > >  drivers/gpu/drm/radeon/atombios_crtc.c     |  5 ++++-
> > >  drivers/gpu/drm/radeon/atombios_encoders.c | 11 ++++++-----
> > >  drivers/gpu/drm/radeon/radeon.h            |  1 +
> > >  3 files changed, 11 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
> > > index 2fc0334e0..075eba2d4 100644
> > > --- a/drivers/gpu/drm/radeon/atombios_crtc.c
> > > +++ b/drivers/gpu/drm/radeon/atombios_crtc.c
> > > @@ -24,6 +24,8 @@
> > >   *          Alex Deucher
> > >   */
> > >
> > > +#include <linux/dmi.h>
> > > +
> > >  #include <drm/drm_fixed.h>
> > >  #include <drm/drm_fourcc.h>
> > >  #include <drm/drm_framebuffer.h>
> > > @@ -594,7 +596,8 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
> > >                 if (((rdev->family == CHIP_RS780) || (rdev->family == CHIP_RS880))
> > >                     && !radeon_crtc->ss_enabled)
> > >                         radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
> > > -               if (ASIC_IS_DCE32(rdev) && mode->clock > 165000)
> > > +               if ((ASIC_IS_DCE32(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
> > > +                   && mode->clock > 165000)
> > >                         radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
> > >         } else {
> > >                 radeon_crtc->pll_flags |= RADEON_PLL_LEGACY;
> > > diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
> > > index 5cfd8fcfa..4de60559f 100644
> > > --- a/drivers/gpu/drm/radeon/atombios_encoders.c
> > > +++ b/drivers/gpu/drm/radeon/atombios_encoders.c
> > > @@ -1707,7 +1707,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
> > >                 if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector) {
> > >                         /* DP_SET_POWER_D0 is set in radeon_dp_link_train */
> > >                         radeon_dp_link_train(encoder, connector);
> > > -                       if (ASIC_IS_DCE4(rdev))
> > > +                       if (ASIC_IS_DCE4(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
> > >                                 atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
> > >                 }
> > >                 if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) {
> > > @@ -2123,12 +2123,13 @@ int radeon_atom_pick_dig_encoder(struct drm_encoder *encoder, int fe_idx)
> > >         }
> > >
> > >         /*
> > > -        * On DCE32 any encoder can drive any block so usually just use crtc id,
> > > -        * but Apple thinks different at least on iMac10,1 and iMac11,2, so there use linkb,
> > > -        * otherwise the internal eDP panel will stay dark.
> > > +        * On DCE31 and DCE32 any encoder can drive any block so usually just use crtc id,
> > > +        * but Apple thinks different at least on iMac10,1, iMac11,1 and iMac11,2,
> > > +        * so there use linkb, otherwise the internal eDP panel will stay dark.
> > >          */
> > > -       if (ASIC_IS_DCE32(rdev)) {
> > > +       if (ASIC_IS_DCE31(rdev)) {
> >
> > This will change the routing for DCE3.1 as well.  Probably best to do
> > something like:
> >
> > if (dmi_match(DMI_PRODUCT_NAME, "iMac11,1")) {
> >     enc_idx = (dig->linkb) ? 1 : 0;
> >     goto assigned;
> > }
> >
> > before or after the DCE32() block.
> >
> > >                 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;
> > >                 else
> > > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> > > index 527b9d19d..6b7c0abe4 100644
> > > --- a/drivers/gpu/drm/radeon/radeon.h
> > > +++ b/drivers/gpu/drm/radeon/radeon.h
> > > @@ -2625,6 +2625,7 @@ void r100_pll_errata_after_index(struct radeon_device *rdev);
> > >                             (rdev->family == CHIP_RS740)  ||    \
> > >                             (rdev->family >= CHIP_R600))
> > >  #define ASIC_IS_DCE3(rdev) ((rdev->family >= CHIP_RV620))
> > > +#define ASIC_IS_DCE31(rdev) ((rdev->family >= CHIP_RV770))
> > >  #define ASIC_IS_DCE32(rdev) ((rdev->family >= CHIP_RV730))
> > >  #define ASIC_IS_DCE4(rdev) ((rdev->family >= CHIP_CEDAR))
> > >  #define ASIC_IS_DCE41(rdev) ((rdev->family >= CHIP_PALM) && \
> > >
> > > base-commit: e7ae89a0c97ce2b68b0983cd01eda67cf373517d
> > > --
> > > 2.47.3
> > >
>
>
>
> --
> Gilles
>
> Sent from my Commodore C=64

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v5] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1)
  2026-05-29 13:31                     ` Alex Deucher
@ 2026-06-01 21:19                       ` Gilles Risch
  0 siblings, 0 replies; 28+ messages in thread
From: Gilles Risch @ 2026-06-01 21:19 UTC (permalink / raw)
  To: alexander.deucher; +Cc: amd-gfx, dri-devel, Gilles Risch

The Apple iMac11,1 (27-inch, Late 2009) uses a Mobility Radeon HD 4850
(RV770/DCE3.1) with a 2560x1440 internal panel on an internal
DisplayPort path. Without this fix the display stays dark under KMS.

This machine suffers from the same issue as iMac10,1 and iMac11,2:
Apple routes the internal display through Link B of the DIG encoder
instead of Link A. Add iMac11,1 to the existing DMI quirk and move
the Apple-specific encoder assignment into its own block, independent
of the DCE version check.

Additionally, the 2560x1440 panel requires RADEON_PLL_USE_FRAC_FB_DIV
and ATOM_ENCODER_CMD_DP_VIDEO_ON, limited to iMac11,1 via dmi_match()
to avoid affecting other boards.

Signed-off-by: Gilles Risch <gilles.risch@gmail.com>
---
 drivers/gpu/drm/radeon/atombios_crtc.c     |  5 ++++-
 drivers/gpu/drm/radeon/atombios_encoders.c | 23 ++++++++++++----------
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 2fc0334e0..075eba2d4 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -24,6 +24,8 @@
  *          Alex Deucher
  */
 
+#include <linux/dmi.h>
+
 #include <drm/drm_fixed.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
@@ -594,7 +596,8 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
 		if (((rdev->family == CHIP_RS780) || (rdev->family == CHIP_RS880))
 		    && !radeon_crtc->ss_enabled)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
-		if (ASIC_IS_DCE32(rdev) && mode->clock > 165000)
+		if ((ASIC_IS_DCE32(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
+		    && mode->clock > 165000)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
 	} else {
 		radeon_crtc->pll_flags |= RADEON_PLL_LEGACY;
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index 5cfd8fcfa..8b3f8303a 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -1707,7 +1707,7 @@ radeon_atom_encoder_dpms_dig(struct drm_encoder *encoder, int mode)
 		if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector) {
 			/* DP_SET_POWER_D0 is set in radeon_dp_link_train */
 			radeon_dp_link_train(encoder, connector);
-			if (ASIC_IS_DCE4(rdev))
+			if (ASIC_IS_DCE4(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
 				atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
 		}
 		if (radeon_encoder->devices & (ATOM_DEVICE_LCD_SUPPORT)) {
@@ -2123,17 +2123,20 @@ int radeon_atom_pick_dig_encoder(struct drm_encoder *encoder, int fe_idx)
 	}
 
 	/*
-	 * On DCE32 any encoder can drive any block so usually just use crtc id,
-	 * but Apple thinks different at least on iMac10,1 and iMac11,2, so there use linkb,
-	 * otherwise the internal eDP panel will stay dark.
+	 * Apple routes the internal eDP panel through Link B of the DIG encoder
+	 * instead of Link A on the iMac10,1, iMac11,1 and iMac11,2.
+	 * Use linkb to avoid a dark display.
 	 */
-	if (ASIC_IS_DCE32(rdev)) {
-		if (dmi_match(DMI_PRODUCT_NAME, "iMac10,1") ||
-		    dmi_match(DMI_PRODUCT_NAME, "iMac11,2"))
-			enc_idx = (dig->linkb) ? 1 : 0;
-		else
-			enc_idx = radeon_crtc->crtc_id;
+	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;
+		goto assigned;
+	}
 
+	/* on DCE32 and encoder can driver any block so just crtc id */
+	if (ASIC_IS_DCE32(rdev)) {
+		enc_idx = radeon_crtc->crtc_id;
 		goto assigned;
 	}
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Claude review: drm/radeon: fix iMac11,1 dark display
  2026-05-18 21:14           ` [PATCH v3 0/2] drm/radeon: fix iMac11,1 dark display Gilles Risch
@ 2026-06-04  3:37             ` Claude Code Review Bot
  0 siblings, 0 replies; 28+ messages in thread
From: Claude Code Review Bot @ 2026-06-04  3:37 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/radeon: fix iMac11,1 dark display
Author: Gilles Risch <gilles.risch@gmail.com>
Patches: 21
Reviewed: 2026-06-04T13:37:51.633170

---

This mbox contains the evolution of a fix for the Apple iMac11,1 (Late 2009, Mobility Radeon HD 4850/RV770) dark display under KMS, spanning v1 through v5. The root cause is well-understood: the RV770 is DCE3.1 but the existing ASIC_IS_DCE32() macro starts at CHIP_RV730, which comes *after* CHIP_RV770 in the enum, so the RV770 never matched the DCE3.2 PLL and encoder logic paths. Additionally, Apple routes the internal eDP panel through Link B instead of Link A.

The author iterated productively in response to maintainer feedback: v1-v3 used a broad `ASIC_IS_DCE31()` macro that could affect other DCE3.1 boards; v4-v5 narrowed the fix to `dmi_match()` for iMac11,1 specifically. The v5 further improves the encoder picker restructuring, separating the Apple DMI quirk from the DCE32 version logic entirely.

**The latest patch (v5) is the one that should be reviewed for merge.** It is a well-scoped, low-risk quirk for a specific Apple machine. A few minor issues remain.

The earlier patches (v1 through v4) are superseded and should be disregarded.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Claude review: drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems
  2026-05-16 18:52   ` [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Gilles Risch
                       ` (2 preceding siblings ...)
  2026-05-18 14:13     ` Alex Deucher
@ 2026-06-04  3:37     ` Claude Code Review Bot
  3 siblings, 0 replies; 28+ messages in thread
From: Claude Code Review Bot @ 2026-06-04  3:37 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

Superseded by v4/v5. The v3 2/2 patch narrowed the resume fix to `dmi_match("iMac11,1")`, which was the right direction. However:

In `atombios_set_edp_panel_power()` (v3 2/2):
```c
	if (!ASIC_IS_DCE4(rdev))
	    if (!dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
		    goto done;
```
This has an **indentation/style issue** — the nested if without braces is confusing and uses spaces instead of tabs. It should be:
```c
	if (!ASIC_IS_DCE4(rdev) && !dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
		goto done;
```

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Claude review: Re: [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems
  2026-05-17  3:28     ` kernel test robot
@ 2026-06-04  3:37       ` Claude Code Review Bot
  0 siblings, 0 replies; 28+ messages in thread
From: Claude Code Review Bot @ 2026-06-04  3:37 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

Superseded by v5. Combined everything into a single patch. Still introduces `ASIC_IS_DCE31()` in `radeon.h` but only uses it in the encoder picker — other uses are `dmi_match()`.

### PATCH 6 (LATEST): [PATCH v5] drm/radeon: fix internal display on iMac11,1 (RV770/DCE3.1)

This is the version that matters. Three changes across two files:

**1. atombios_crtc.c — PLL fractional feedback divider quirk**

```c
+#include <linux/dmi.h>
```
Correct — needed for `dmi_match()`, not previously included in this file.

```c
-		if (ASIC_IS_DCE32(rdev) && mode->clock > 165000)
+		if ((ASIC_IS_DCE32(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
+		    && mode->clock > 165000)
 			radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
```
This enables the fractional feedback divider for high-clock modes on the iMac11,1's 2560x1440 panel. The 165000 kHz threshold corresponds to single-link DVI limits. This is correct — the iMac11,1's RV770 (DCE3.1) was excluded from the DCE3.2 check because `CHIP_RV770 < CHIP_RV730` in the enum.

Note: the other DCE32 check at line ~583 (`ASIC_IS_DCE32(rdev) && mode->clock > 200000` for `RADEON_PLL_PREFER_HIGH_FB_DIV`) is intentionally left untouched. This seems reasonable if the iMac11,1 works without the high-FB-div preference.

**2. atombios_encoders.c — DP_VIDEO_ON quirk**

```c
-			if (ASIC_IS_DCE4(rdev))
+			if (ASIC_IS_DCE4(rdev) || dmi_match(DMI_PRODUCT_NAME, "iMac11,1"))
 				atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_ON, 0);
```
Enables the DP video-on command for the iMac11,1. Reasonable for initial display bring-up.

**Concern: DP_VIDEO_OFF not patched.** The corresponding `DPMS_OFF` path at line 1727:
```c
		if (ASIC_IS_DCE4(rdev)) {
			if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(encoder)) && connector)
				atombios_dig_encoder_setup(encoder, ATOM_ENCODER_CMD_DP_VIDEO_OFF, 0);
		}
```
is not updated with the same `dmi_match()`. In earlier versions (v3 2/2), both ON and OFF were patched. If the encoder needs an explicit VIDEO_ON to light up, symmetry suggests it should also get VIDEO_OFF when powering down, otherwise the encoder may be left in an inconsistent state on DPMS off or suspend. The author should clarify whether omitting the OFF path is intentional or an oversight.

**3. atombios_encoders.c — Encoder picker restructure**

```c
-	if (ASIC_IS_DCE32(rdev)) {
-		if (dmi_match(DMI_PRODUCT_NAME, "iMac10,1") ||
-		    dmi_match(DMI_PRODUCT_NAME, "iMac11,2"))
-			enc_idx = (dig->linkb) ? 1 : 0;
-		else
-			enc_idx = radeon_crtc->crtc_id;
-
+	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;
 		goto assigned;
 	}
+
+	/* on DCE32 and encoder can driver any block so just crtc id */
+	if (ASIC_IS_DCE32(rdev)) {
+		enc_idx = radeon_crtc->crtc_id;
+		goto assigned;
+	}
```

This restructuring is the cleanest part of the patch. It:
- Separates the Apple DMI quirk from the DCE version check
- Adds iMac11,1 to the quirk list
- Preserves identical behavior for all existing paths:
  - Apple iMac10,1/iMac11,2 (DCE3.2): still gets linkb (now matched by DMI before the DCE32 check)
  - Non-Apple DCE3.2: still gets `crtc_id`
  - iMac11,1 (DCE3.1): **now correctly gets linkb** (previously fell through to DCE3 LVTMA logic)
  - Non-Apple DCE3.1: unchanged (DMI fails, DCE32 fails, falls to DCE3 logic)

**Minor issue: typo in comment.** The new comment reads:
```c
	/* on DCE32 and encoder can driver any block so just crtc id */
```
Should be: `/* on DCE32 any encoder can drive any block so just use crtc id */`

**Other observations:**

- **Missing Fixes/Cc-stable tags**: This fixes a real dark-display regression. A `Cc: stable@vger.kernel.org` tag would be appropriate. A `Fixes:` tag pointing to the commit that introduced the DCE32 encoder picker logic would help stable backports.

- **No `atombios_set_edp_panel_power()` change**: The v5 does not extend the eDP panel power function to iMac11,1. The earlier v3 series included this as patch 2/2 for suspend/resume. The commit message no longer mentions suspend/resume, so this is apparently deferred. This is a reasonable scoping decision — fix initial display first, tackle suspend/resume separately.

- **The ASIC_IS_DCE31 macro is no longer added**: v5 dropped this entirely in favor of targeted `dmi_match()` calls, which is the safer approach for a legacy driver where broad DCE3.1 changes could regress other hardware.

**Summary**: v5 is a well-targeted, low-risk quirk fix. The encoder picker restructure is clean and correct. Two items need attention before merge: the comment typo, and clarification on whether the missing `DP_VIDEO_OFF` dmi_match is intentional.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2026-06-04  3:37 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-10 18:54 [PATCH] 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 Gilles Risch
2026-05-16  5:26 ` Claude review: " Claude Code Review Bot
2026-05-16  5:26 ` Claude Code Review Bot
2026-05-16  9:24 ` [PATCH v2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
2026-05-16 18:09   ` Lukas Wunner
2026-05-17 13:53     ` Gilles Risch
2026-05-17 14:05       ` Lukas Wunner
2026-05-18  6:36     ` Claude review: " Claude Code Review Bot
2026-05-16 18:52   ` [PATCH] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Gilles Risch
2026-05-17  3:28     ` kernel test robot
2026-06-04  3:37       ` Claude review: " Claude Code Review Bot
2026-05-17  5:12     ` kernel test robot
2026-05-18 14:13     ` Alex Deucher
2026-05-18 14:40       ` Gilles Risch
2026-05-18 14:57         ` Alex Deucher
2026-05-18 21:14           ` [PATCH v3 0/2] drm/radeon: fix iMac11,1 dark display Gilles Risch
2026-06-04  3:37             ` Claude review: " Claude Code Review Bot
2026-05-18 21:14           ` [PATCH v3 1/2] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
2026-05-19 13:35             ` Alex Deucher
2026-05-18 21:14           ` [PATCH v3 2/2] drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Gilles Risch
2026-05-19 13:42             ` Alex Deucher
2026-05-27 22:20               ` [PATCH v4] drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Gilles Risch
2026-05-28 18:53                 ` Alex Deucher
2026-05-29  7:43                   ` Gilles Risch
2026-05-29 13:31                     ` Alex Deucher
2026-06-01 21:19                       ` [PATCH v5] " Gilles Risch
2026-06-04  3:37     ` Claude review: drm/radeon: fix eDP resume from suspend on iMac11, 1 / DCE3.1 systems Claude Code Review Bot
2026-05-18  6:57   ` Claude review: drm/radeon: fix internal display on iMac11, 1 (RV770/DCE3.1) Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox