Claude review: drm/imx/ipuv3: do not adjust hactive for non-8-pixel-aligned modes

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

Patch Review

**Positive aspects:**

1. The commit message is clear about the problem (visible skewing on 854x480 displays) and the rationale (IDMAC burst sizing is handled by `ipu_calculate_bursts()`).

2. The removal of the CRTC-side alignment in `ipuv3-crtc.c` is clean — it was stealing pixels from the front porch, which is problematic when the front porch is small.

3. The `ipu_calculate_bursts()` function at `ipuv3-plane.c:553-571` genuinely does handle arbitrary widths — it iterates to find burst sizes that fit within the stride, with no alignment assumptions.

4. Demoting the DC warning to `dev_dbg` is appropriate if this is no longer a problematic condition.

**Concerns:**

1. **DMFC wait4eot calculation with unaligned width.** The `ipu_dmfc_config_wait4eot()` function at `ipu-dmfc.c:135` uses `width` in a division:

   ```c
   if ((dmfc->slots * 64 * 4) / width > dmfc->data->max_fifo_lines)
   ```

   This is called with the same `width` from `ipu_src_rect_width()` at `ipuv3-plane.c:686`. With the 8-pixel alignment removed, this will now receive unaligned widths. The calculation still works mathematically (integer division), but the FIFO threshold decision changes slightly — worth confirming this doesn't cause FIFO overflow issues on the specific display configurations being targeted.

2. **Asymmetry between CRTC and plane width paths.** After this patch, the CRTC's `ipu_dc_init_sync()` will receive the raw (unaligned) `hactive` from `drm_display_mode_to_videomode()`, and the plane's IDMAC will also use the raw width. This is consistent, which is good. However, I want to confirm: is there any path where the plane `width` (from `ipu_src_rect_width`) could differ from the CRTC `hactive` passed to `ipu_dc_init_sync`? The commit message says "The IDMAC read width must match the DC/DI hactive exactly" — this was previously guaranteed by both sides aligning to 8 pixels. After the patch, both use raw values, so they should still match for fullscreen scanout, which is correct.

3. **The comment added in `ipu_src_rect_width()` references DMFC FIFO overflows/underflows:**

   ```c
   /*
    * Do not align width to 8 pixels here. The IDMAC read width
    * must match the DC/DI hactive exactly, otherwise the DMFC
    * FIFO overflows or underflows causing IDMAC timeouts or
    * display corruption. The DMA burst size is adapted to the
    * actual width by ipu_calculate_bursts().
    */
   ```

   This comment explains why the alignment is *removed* but could be slightly misleading — it implies that alignment *caused* DMFC FIFO issues, when the real problem is the mismatch between IDMAC and DC widths. Consider rewording to something like: "Use the exact source width without 8-pixel alignment. The DC/DI hactive is no longer rounded up, so the IDMAC read width must match it exactly to avoid DMFC FIFO overflows or underflows."

4. **Missing Fixes tag.** If this fixes a real user-visible display corruption on 854x480 panels, it should carry a `Fixes:` tag pointing to the commit that introduced the alignment. This would help with backporting to stable kernels.

5. **No `mode_fixup` / `atomic_check` validation.** The old code in `mode_set_nofb` blindly subtracted from `hfront_porch` without checking for underflow — that was itself a bug (if `hfront_porch < 6` for an 854-pixel mode, it would underflow). The patch removes this dangerous arithmetic, which is good. But there is no new validation in `mode_fixup` or `atomic_check` to reject modes that might be problematic for other reasons at non-aligned widths. This may be fine if the hardware truly handles any width, but it would be prudent to confirm on hardware.

6. **Minor style nit in ipu-dc.c:** The warning message change adds the `width` value, which is a good improvement for debuggability:

   ```c
   dev_dbg(priv->dev,
           "%s: hactive %u is not 8-pixel aligned\n",
           __func__, width);
   ```

   No issue here — just noting this is a nice touch.

**Overall assessment:** The patch is well-motivated and technically sound. The key insight — that `ipu_calculate_bursts()` handles arbitrary widths and that the foreground plane already works without alignment — provides good evidence that this is safe. I'd like to see a `Fixes:` tag and a slightly clearer comment, but neither is a blocking issue. The patch should be tested on both 8-pixel-aligned displays (regression) and non-aligned displays (854x480) to confirm no DMFC FIFO issues arise.

---
Generated by Claude Code Patch Reviewer