[PATCH] drm/ast: DisplayPort edid supports 256 bytes

[PATCH] drm/ast: DisplayPort edid supports 256 bytes

[Jammy Huang]

DisplayPort supports edid at most 256 bytes. Thus, allow it to fetch
edid block 0 and 1.

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
ASPEED DisplayPort's EDID size can be 256 bytes at most. Thus, EDID
blocks fetched can be 0 and 1.
---
 drivers/gpu/drm/ast/ast_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index 9d07dad358c..c938e1d6b1d 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -88,7 +88,7 @@ static int ast_astdp_read_edid_block(void *data, u8 *buf, unsigned int block, si
 	int ret = 0;
 	unsigned int i;
 
-	if (block > 0)
+	if (block > 1)
 		return -EIO; /* extension headers not supported */
 
 	/*

---
base-commit: 5ee8dbf54602dc340d6235b1d6aa17c0f283f48c
change-id: 20260313-upstream_ast_dp_edid-5fe6adf7ad36

Best regards,
-- 
Jammy Huang <jammy_huang@aspeedtech.com>

Claude review: drm/ast: DisplayPort edid supports 256 bytes

[Claude Code Review Bot]

Overall Series Review

Subject: drm/ast: DisplayPort edid supports 256 bytes
Author: Jammy Huang <jammy_huang@aspeedtech.com>
Patches: 1
Reviewed: 2026-03-14T07:11:37.010240

---

This is a single-patch series that aims to enable reading EDID extension blocks (block 1) over the ASPEED DisplayPort interface. The change itself is a one-line fix (`block > 0` → `block > 1`), but there is a significant functional problem: the code at line 157-168 unconditionally zeroes out the extension count byte and adjusts the checksum in block 0, which contradicts the goal of this patch. If block 1 is now allowed, the EDID block 0 should actually report that an extension exists.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/ast: DisplayPort edid supports 256 bytes

[Claude Code Review Bot]

Patch Review

**The guard change is correct in isolation:**

```c
-	if (block > 0)
+	if (block > 1)
 		return -EIO; /* extension headers not supported */
```

Changing the limit from 0 to 1 makes sense given the hardware can address up to offset 63 (i.e., 256 bytes / 4 = 64 dwords). The offset bounds check at line 110 (`if (offset >= 64)`) would correctly allow block 0 and block 1 (offsets 0–63).

**However, there is a critical conflict with the extension-count zeroing logic at lines 157-168:**

```c
	if (i == 31) {
		/*
		 * For 128-bytes EDID_1.3,
		 * 1. Add the value of Bytes-126 to Bytes-127.
		 *		The Bytes-127 is Checksum. Sum of all 128bytes should
		 *		equal 0	(mod 256).
		 * 2. Modify Bytes-126 to be 0.
		 *		The Bytes-126 indicates the Number of extensions to
		 *		follow. 0 represents noextensions.
		 */
		ediddata[3] = ediddata[3] + ediddata[2];
		ediddata[2] = 0;
	}
```

This code runs for **every** call to `ast_astdp_read_edid_block`, including when `block == 0`. It forces the extension count (byte 126) to 0 and adjusts the checksum accordingly. This means:

1. When the DRM EDID parser reads block 0, it will see extension count = 0 and **never request block 1**. The `block > 1` guard change becomes dead code.
2. For this patch to actually work, this zeroing logic needs to be conditioned on something (e.g., only applied when the hardware EDID truly has no valid extension), or removed entirely so the real extension count is reported.

**The comment on the guard is now stale:**

```c
	return -EIO; /* extension headers not supported */
```

If extension block 1 is now supported, this comment should be updated to something like `/* only blocks 0 and 1 supported */`.

**Additional observation:** The `i == 31` check is checking the loop variable `i` which increments by 4, so `i == 31` will never be true — `i` takes values 0, 4, 8, ..., 28, 32, ... This appears to be a pre-existing bug. It seems like the intent was `i == 124` (the 32nd dword read, i.e., offset 31 when `block == 0`, covering bytes 124–127 which include the extension count at byte 126 and checksum at byte 127). This needs investigation to confirm whether this code path is actually ever reached, and if not, whether the extension-zeroing is happening at all today.

**Summary:** The patch as submitted either (a) has no effect because the extension count is zeroed so block 1 is never requested, or (b) the zeroing code is itself dead due to the `i == 31` bug, in which case the patch may work but the dead zeroing code should be cleaned up. Either way, additional work is needed before this can be merged. The author should clarify the interaction with the extension-count modification logic and submit a v2 that addresses it.

---
Generated by Claude Code Patch Reviewer