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 [thread overview]
Message-ID: <review-patch8-20260501-milos-mdss-v3-8-58bfc58c0e13@fairphone.com> (raw)
In-Reply-To: <20260501-milos-mdss-v3-8-58bfc58c0e13@fairphone.com>
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
next prev parent reply other threads:[~2026-05-04 23:35 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-01 7:14 [PATCH v3 0/9] MDSS / DPU bringup for Milos Luca Weiss
2026-05-01 7:14 ` [PATCH v3 1/9] dt-bindings: display: msm-dsi-phy-7nm: document the Milos DSI PHY Luca Weiss
2026-05-04 23:35 ` Claude review: " Claude Code Review Bot
2026-05-01 7:14 ` [PATCH v3 2/9] dt-bindings: display: msm-dsi-controller-main: document the Milos DSI Controller Luca Weiss
2026-05-04 23:35 ` Claude review: " Claude Code Review Bot
2026-05-01 7:14 ` [PATCH v3 3/9] dt-bindings: display: msm: document the Milos DPU Luca Weiss
2026-05-04 23:35 ` Claude review: " Claude Code Review Bot
2026-05-01 7:14 ` [PATCH v3 4/9] dt-bindings: display: msm: document the Milos Mobile Display Subsystem Luca Weiss
2026-05-03 12:28 ` Krzysztof Kozlowski
2026-05-04 23:35 ` Claude review: " Claude Code Review Bot
2026-05-01 7:14 ` [PATCH v3 5/9] soc: qcom: ubwc: Add config for Milos Luca Weiss
2026-05-04 23:35 ` Claude review: " Claude Code Review Bot
2026-05-01 7:14 ` [PATCH v3 6/9] drm/msm/dsi: add support for DSI-PHY on Milos Luca Weiss
2026-05-04 23:35 ` Claude review: " Claude Code Review Bot
2026-05-01 7:14 ` [PATCH v3 7/9] drm/msm: mdss: Add Milos support Luca Weiss
2026-05-04 23:35 ` Claude review: " Claude Code Review Bot
2026-05-01 7:14 ` [PATCH v3 8/9] drm/msm/dpu: " Luca Weiss
2026-05-04 23:35 ` Claude Code Review Bot [this message]
2026-05-01 7:14 ` [PATCH v3 9/9] arm64: dts: qcom: milos: Add display (MDSS) Luca Weiss
2026-05-04 23:35 ` Claude review: " Claude Code Review Bot
2026-05-04 23:35 ` Claude review: MDSS / DPU bringup for Milos Claude Code Review Bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=review-patch8-20260501-milos-mdss-v3-8-58bfc58c0e13@fairphone.com \
--to=claude-review@example.com \
--cc=dri-devel-reviews@example.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox