Claude review: drm/msm/snapshot: fix dumping of the unaligned regions

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

Patch Review

**Understanding the old bug:**

Before this patch, `msm_disp_snapshot_add_block()` pre-aligned the size:
```c
new_blk->size = ALIGN(len, REG_DUMP_ALIGN);
```
Then `msm_disp_state_dump_regs()` received this already-aligned value as `aligned_len` and treated it as a count in units of `REG_DUMP_ALIGN`:
```c
len_padded = aligned_len * REG_DUMP_ALIGN;  // 16x overallocation!
num_rows = aligned_len / REG_DUMP_ALIGN;
end_addr = base_addr + aligned_len;
```
So if the caller passed `len = 0x200` (512 bytes), it was aligned to `0x200` (no-op since already aligned), then the dump function computed `len_padded = 0x200 * 16 = 0x2000` -- a 16x overallocation. Meanwhile `num_rows = 0x200 / 16 = 32` and `end_addr = base_addr + 0x200` were coincidentally correct for aligned inputs, masking the arithmetic confusion.

For unaligned DSI regions (e.g., a ctrl_size that isn't a multiple of 16), the `ALIGN()` in `msm_disp_snapshot_add_block` would round up, but the dump function's `end_addr = base_addr + aligned_len` would then read past the actual MMIO region, which could cause bus errors or read garbage registers.

**Fix analysis:**

The fix correctly:

1. **Stores the raw length** in `new_blk->size = len` -- no premature alignment.

2. **Aligns internally in `msm_disp_state_dump_regs()`**:
   ```c
   len_padded = round_up(len, REG_DUMP_ALIGN);  // for kvzalloc size
   num_rows = DIV_ROUND_UP(len, REG_DUMP_ALIGN); // iterate all rows including partial
   end_addr = base_addr + len;                    // true end of MMIO region
   ```
   The dump function already had per-register `addr < end_addr` guards, so the partial last row is handled correctly -- registers past the true end read as 0 (the guard returns 0, and kvzalloc zeroed the buffer).

3. **Handles partial rows in printing**: The new tail code in `msm_disp_state_print_regs()` prints only the registers that were actually within the region:
   ```c
   if (addr != end_addr) {
       drm_printf(p, "0x%lx : %08x", ...);
       if (addr + 0x4 < end_addr) ...
       if (addr + 0x8 < end_addr) ...
       drm_printf(p, "\n");
   }
   ```

**Minor observations:**

- **`num_rows` in print function not updated**: In `msm_disp_state_print_regs()`, line 62 still uses integer division `num_rows = len / REG_DUMP_ALIGN` (truncating). This is intentional and correct -- the for-loop handles only complete rows, and the new tail block handles the remainder. Good.

- **`block->size` now stores raw len**: `msm_disp_state_print_regs` is called with `block->size` at line 101. Since `size` now stores the unaligned value, both the dump and print functions receive the same raw length, and the `kvzalloc` allocation in the dump function uses `round_up(len, REG_DUMP_ALIGN)`, so the buffer is large enough for the print function to read `dump_addr[i * 4 + N]` indices up to the last partial row without going out of bounds. Correct.

- **First register unconditionally printed in tail**: The tail code unconditionally prints `dump_addr[i * 4]` when `addr != end_addr`. Since `addr` only reaches this point if `addr < end_addr` (because `num_rows = len / REG_DUMP_ALIGN` means `addr = base_addr + num_rows * REG_DUMP_ALIGN`, and if `addr != end_addr` then there are remaining bytes), there is always at least one register to print. Correct.

- **Declaration order change**: Moving `end_addr` declaration and putting `int i` after it is a minor style cleanup (reverse Christmas tree). Fine.

**No issues found. Patch is correct.**

---
Generated by Claude Code Patch Reviewer