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

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

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