Claude review: drm/imx: Add support for i.MX94 DCIF

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

Patch Review

This is the main driver patch. Several observations:

**Potential issue — `params[i] < 0` is always false (dcif-crc.c:2033)**:
```c
ret = kstrtouint(opt, 10, &params[i]);
if (ret < 0)
    return ret;

if (params[i] < 0)
    return -EINVAL;
```
`params` is declared as `int params[4]` but `kstrtouint` parses unsigned integers. So this negative check is dead code — `kstrtouint` will never produce a negative value in `params[i]` (it would just fail to parse). Either use `kstrtoint` if you want to catch negatives, or remove the check.

**regmap locking disabled (dcif-drv.c:2962)**:
```c
static const struct regmap_config dcif_regmap_config = {
    ...
    .disable_locking = true,
};
```
Locking is disabled on the regmap, but CRC values are read from the IRQ handler under `spinlock_irqsave`, and regmap writes happen from both atomic commit paths and IRQ context. If all access is single-threaded or properly serialized by the DRM framework, this is fine, but it's worth flagging. The `dcif_crc_is_enabled` flag is accessed without explicit synchronization between the atomic commit path and the IRQ handler (it's written in `dcif_crtc_enable_crc_source`/`dcif_crtc_disable_crc_source` and read in `dcif_irq_handler` under `spinlock_irqsave`). The enable/disable functions are called from atomic flush which doesn't hold the event_lock spinlock, so there's a small race window.

**`dcif_crtc_mode_valid` compares `crtc_clock` vs constant (dcif-crtc.c:2339)**:
```c
#define DCIF_MAX_PIXEL_CLOCK		148500000

if (mode->crtc_clock > DCIF_MAX_PIXEL_CLOCK)
```
`mode->crtc_clock` is in kHz while `DCIF_MAX_PIXEL_CLOCK` is 148500000 (148.5 MHz in Hz, or 148500 kHz). This comparison is therefore wrong: `crtc_clock` in kHz would never exceed 148500000 kHz. The constant should be `148500` (kHz) instead of `148500000`. This is a **bug** — the mode_valid check will never reject any mode based on clock.

**Duplicate format switch in crtc and plane (dcif-crtc.c:2377-2423, dcif-plane.c:3465-3507)**: The pixel format switch statement is duplicated between `dcif_set_formats()` and `dcif_plane_atomic_update()`. Both set the same CTRLDESC0 register bits. When `dcif_crtc_atomic_enable` is called, it calls `dcif_crtc_mode_set_nofb` → `dcif_set_formats` which writes format to layer 0's CTRLDESC0, then later the plane's `atomic_update` also writes to CTRLDESC0 for the same layer. The plane update's write (line 3520) does a full `regmap_write` that would overwrite the format bits already set by `dcif_set_formats`. This should work correctly since both compute the same format value, but it's redundant.

**Overlay plane declares YUV rejection too late (dcif-plane.c:3509-3512)**:
```c
if (plane->type == DRM_PLANE_TYPE_OVERLAY && yuv_fmt == CTRLDESCL0_YUV_FORMAT_Y2UY1V) {
    dev_err(dcif->drm.dev, "Overlay plane could not support YUV format\n");
    return;
}
```
This check is in `atomic_update`, not `atomic_check`. Invalid formats should be rejected during `atomic_check`, not silently skipped during update. Also, the overlay format list `dcif_overlay_plane_formats` doesn't include any YUV formats, so this check would only trigger if the code has a bug elsewhere. If the intent is to protect against YUV on the overlay, the format lists already handle this correctly, making this check dead code (but in the wrong place regardless).

**`drm_bridge_put` usage on non-refcounted bridge (dcif-crtc.c:2658)**:
```c
struct drm_bridge *bridge __free(drm_bridge_put) = NULL;
```
`dcif_crtc_get_bridge()` calls `drm_bridge_chain_get_first_bridge()` which returns a bridge pointer without incrementing a refcount. Using `__free(drm_bridge_put)` on it would call `drm_bridge_put()` on a bridge that wasn't `drm_bridge_get()`'d, which could lead to a use-after-free. This was noted as reviewed by Luca Ceresoli for "bridge refcounting" but it looks like `drm_bridge_chain_get_first_bridge` doesn't take a reference. Worth verifying.

**`dcif_driver` should be `const` (dcif-drv.c:2925)**:
```c
static struct drm_driver dcif_driver = {
```
Minor: `dcif_driver` should be `static const struct drm_driver`.

---
Generated by Claude Code Patch Reviewer