From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/msm/dpu: Add Milos support
Date: Tue, 05 May 2026 09:35:35 +1000
Message-ID: <review-patch8-20260501-milos-mdss-v3-8-58bfc58c0e13@fairphone.com>
In-Reply-To: <20260501-milos-mdss-v3-8-58bfc58c0e13@fairphone.com>
References: <20260501-milos-mdss-v3-0-58bfc58c0e13@fairphone.com> <20260501-milos-mdss-v3-8-58bfc58c0e13@fairphone.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

This is the largest and most complex patch. Creates the DPU 10.2 catalog for Milos. Several observations:

**DSC sub-blocks -- unusual ctl register length:**

```c
static const struct dpu_dsc_sub_blks milos_dsc_sblk_0 = {
	.enc = {.name = "enc", .base = 0x100, .len = 0x100},
	.ctl = {.name = "ctl", .base = 0xF00, .len = 0x80},
};
```

The `.ctl.len = 0x80` (128 bytes) is significantly larger than any other platform: the default is `0x10`, SM8750 uses `0x24`. The `.enc.len = 0x100` matches SM8750. Is the 0x80 ctl length verified from the hardware documentation? It's not wrong per se, but it stands out as an outlier. All this register length does is gate how much of the register space the driver will consider "owned" by this sub-block, so over-sizing is usually harmless, but worth confirming.

**VBIF configuration:**

```c
static const struct dpu_vbif_cfg milos_vbif = {
	...
	.qos_rt_tbl = {
		.npriority_lvl = ARRAY_SIZE(sdm845_rt_pri_lvl),
		.priority_lvl = sdm845_rt_pri_lvl,
	},
```

This creates a new VBIF config that is structurally identical to `sm8650_vbif` but uses `sdm845_rt_pri_lvl` (`{3, 3, 4, 4, 5, 5, 6, 6}`) instead of `sm8650_rt_pri_lvl` (`{4, 4, 5, 5, 5, 5, 5, 6}`). Every DPU 9.0+ SoC in-tree uses `sm8650_vbif` or `sm8550_vbif` (both with their own higher priority levels). Using the SDM845 RT priorities for a DPU 10.2 SoC is a deliberate choice -- presumably reflecting the mid-range tier having fewer AXI paths and different QoS requirements. This seems fine given it's tested on hardware.

**Performance data TODO/FIXME markers:**

```c
	/* FIXME: lut tables */
	.danger_lut_tbl = {0x3ffff, 0x3ffff, 0x0},
	...
	/* TODO: macrotile-qseed is different from macrotile */
```

These indicate the perf tuning is not finalized. The QoS LUT tables reference `sc7180_qos_*` which matches what SM8650 does (so this is the established pattern, not a concern). The danger/safe LUT values are placeholder-like; they should be verified against the hardware programming guide when available.

**Hardware topology -- scaled down from SM8650:**
- 1 VIG (vs 4 on SM8650), 3 DMA (vs 6), 3 LMs (vs 6), 1 DSPP (vs 4), 4 CTLs (vs 6)
- LM_0 stands alone with DSPP_0, LM_2+LM_3 are paired for 3D merge but without DSPPs
- Single DSI + DP interface (vs dual on SM8650)

This is a reasonable cut-down for a mid-range SoC focused on single-panel use.

**Missing Reviewed-by tags**: This is the only driver patch without any Reviewed-by, which is worth noting given it's the most complex patch in the series.

---
Generated by Claude Code Patch Reviewer