From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: 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.
Date: Sat, 16 May 2026 15:30:57 +1000 [thread overview]
Message-ID: <review-overall-20260510190505.4810-2-gilles.risch@gmail.com> (raw)
In-Reply-To: <20260510190505.4810-2-gilles.risch@gmail.com>
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:30:57.705528
---
This is a single-patch series that fixes a dark internal eDP panel on the Apple iMac11,1 (late 2009, ATI Mobility Radeon HD 4850 / RV770). The approach introduces a new `ASIC_IS_DCE31` macro to include the RV770 chip in code paths that previously only covered DCE 3.2 (RV730 and later), and adds the iMac11,1 to an existing Apple DMI quirk list for encoder selection.
The fix addresses a real hardware issue and the core logic is sound, but there are several problems with the submission that need addressing before it can be accepted:
1. **Severely malformed subject line** -- the entire patch description has been placed in the Subject header, violating standard kernel patch formatting.
2. **DCE version naming is questionable** -- historical AMD documentation associates DCE 3.1 with the RS780/RS880 IGPs, not RV770. The RV770 is widely understood to be DCE 3.2, and the existing `ASIC_IS_DCE32` macro appears to have a longstanding off-by-one in the chip enum that excludes it. Creating a new macro with a debatable name rather than fixing the existing one warrants discussion.
3. **PLL behavior changes affect all RV770 systems**, not just Apple iMacs -- this is not called out in the commit message and represents a regression risk.
4. **Thin commit message** with no technical explanation of the root cause.
---
---
Generated by Claude Code Patch Reviewer
next prev parent reply other threads:[~2026-05-16 5:30 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-10 19:05 [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:30 ` Claude Code Review Bot [this message]
2026-05-16 5:30 ` Claude review: " Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-05-10 18:54 [PATCH] " Gilles Risch
2026-05-16 5:26 ` Claude review: " Claude Code Review Bot
2026-05-16 5:26 ` Claude Code Review Bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=review-overall-20260510190505.4810-2-gilles.risch@gmail.com \
--to=claude-review@example.com \
--cc=dri-devel-reviews@example.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox