From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/msm/dpu: Add support for Eliza SoC
Date: Wed, 04 Mar 2026 07:15:20 +1000 [thread overview]
Message-ID: <review-patch7-20260303-drm-display-eliza-v1-7-814121dbb2bf@oss.qualcomm.com> (raw)
In-Reply-To: <20260303-drm-display-eliza-v1-7-814121dbb2bf@oss.qualcomm.com>
Patch Review
This is the main catalog patch adding `dpu_12_4_eliza.h` (365 lines). It defines the DPU v12.4 hardware blocks. Observations:
**Bug -- `.cwb` array mismatch:**
```c
.cwb_count = ARRAY_SIZE(eliza_cwb),
.cwb = sm8650_cwb,
```
The local `eliza_cwb[]` array is defined with 4 entries having `.len = 0x20`, but the config struct points to `sm8650_cwb` whose entries have `.len = 0x8`. This means the `eliza_cwb` array data is defined but never actually used. This is a pre-existing pattern -- SM8750, Glymur, and Kaanapali catalogs all exhibit the same mismatch (they all define local CWB arrays but point `.cwb` at `sm8650_cwb`). This should ideally be `.cwb = eliza_cwb` to use the correct register lengths. **However**, since this mirrors existing upstream code, this might be intentional or a known issue being tracked separately. Worth a comment from the maintainers.
**DSC with unpaired dce_1:**
```c
static const struct dpu_dsc_cfg eliza_dsc[] = {
{ .name = "dce_0_0", .id = DSC_0, .base = 0x80000, ... .sblk = &sm8750_dsc_sblk_0, },
{ .name = "dce_0_1", .id = DSC_1, .base = 0x80000, ... .sblk = &sm8750_dsc_sblk_1, },
{ .name = "dce_1_0", .id = DSC_2, .base = 0x81000, ... .sblk = &sm8750_dsc_sblk_0, },
};
```
DCE 0 has both hard slices (DSC_0 + DSC_1) but DCE 1 only has one hard slice (DSC_2 without DSC_3). The comment says "Each display compression engine (DCE) contains dual hard slice DSC encoders," yet dce_1 only has one. If this matches the actual hardware, the comment inherited from SM8750 is now slightly misleading. If the hardware actually has the second slice, it should be included.
**Unverified MDP base/len:**
```c
static const struct dpu_mdp_cfg eliza_mdp = {
.name = "top_0",
.base = 0, .len = 0x494, // not verified
```
The `// not verified` comment is honest, but for a production patch this should ideally be verified before merging.
**Perf data uses sc7180 QoS tables:**
```c
.qos_lut_tbl = {
{.nentry = ARRAY_SIZE(sc7180_qos_linear),
.entries = sc7180_qos_linear },
...
/* TODO: macrotile-qseed is different from macrotile */
},
```
The TODO comment and reuse of sc7180 QoS tables is consistent with what SM8750 does, so this is fine for initial bringup.
**LM_3 has no DSPP:**
```c
{ .name = "lm_3", .id = LM_3,
.base = 0x47000, .len = 0x400,
...
.pingpong = PINGPONG_3,
}, // no .dspp field
```
While this is consistent with the cover letter saying DSPP3 was removed (only DSPP 0-2 remain), LM_3 having no DSPP assignment will limit its functionality. This appears intentional.
The remaining catalog additions (`dpu_hw_catalog.c`, `dpu_hw_catalog.h`, `dpu_kms.c`) are mechanical one-liner additions that look correct.
---
Generated by Claude Code Patch Reviewer
next prev parent reply other threads:[~2026-03-03 21:15 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-03 13:07 [PATCH 0/8] drm/msm: Add Qualcomm Eliza SoC support Krzysztof Kozlowski
2026-03-03 13:07 ` [PATCH 1/8] dt-bindings: display/msm: dp-controller: Add Eliza SoC Krzysztof Kozlowski
2026-03-03 13:47 ` Dmitry Baryshkov
2026-03-03 14:02 ` Krzysztof Kozlowski
2026-03-03 21:15 ` Claude review: " Claude Code Review Bot
2026-03-03 13:07 ` [PATCH 2/8] dt-bindings: display/msm: dsi-phy-7nm: " Krzysztof Kozlowski
2026-03-03 13:47 ` Dmitry Baryshkov
2026-03-03 21:15 ` Claude review: " Claude Code Review Bot
2026-03-03 13:07 ` [PATCH 3/8] dt-bindings: display/msm: dsi-controller-main: " Krzysztof Kozlowski
2026-03-03 13:48 ` Dmitry Baryshkov
2026-03-03 21:15 ` Claude review: " Claude Code Review Bot
2026-03-03 13:07 ` [PATCH 4/8] dt-bindings: display/msm: qcom,sm8650-dpu: " Krzysztof Kozlowski
2026-03-03 13:49 ` [PATCH 4/8] dt-bindings: display/msm: qcom, sm8650-dpu: " Dmitry Baryshkov
2026-03-03 21:15 ` Claude review: dt-bindings: display/msm: qcom,sm8650-dpu: " Claude Code Review Bot
2026-03-03 13:07 ` [PATCH 5/8] dt-bindings: display/msm: qcom,eliza-mdss: " Krzysztof Kozlowski
2026-03-03 13:49 ` [PATCH 5/8] dt-bindings: display/msm: qcom, eliza-mdss: " Dmitry Baryshkov
2026-03-03 15:56 ` [PATCH 5/8] dt-bindings: display/msm: qcom,eliza-mdss: " Rob Herring (Arm)
2026-03-03 16:09 ` [PATCH 5/8] dt-bindings: display/msm: qcom, eliza-mdss: " Krzysztof Kozlowski
2026-03-03 21:15 ` Claude review: dt-bindings: display/msm: qcom,eliza-mdss: " Claude Code Review Bot
2026-03-03 13:07 ` [PATCH 6/8] soc: qcom: ubwc: Add configuration " Krzysztof Kozlowski
2026-03-03 13:12 ` Konrad Dybcio
2026-03-03 14:03 ` Krzysztof Kozlowski
2026-03-03 21:15 ` Claude review: " Claude Code Review Bot
2026-03-03 13:07 ` [PATCH 7/8] drm/msm/dpu: Add support for " Krzysztof Kozlowski
2026-03-03 14:08 ` Dmitry Baryshkov
2026-03-03 14:09 ` Krzysztof Kozlowski
2026-03-03 14:10 ` Dmitry Baryshkov
2026-03-03 16:22 ` Krzysztof Kozlowski
2026-03-03 21:15 ` Claude Code Review Bot [this message]
2026-03-03 13:07 ` [PATCH 8/8] drm/msm/mdss: " Krzysztof Kozlowski
2026-03-03 14:11 ` Dmitry Baryshkov
2026-03-03 21:15 ` Claude review: " Claude Code Review Bot
2026-03-03 21:15 ` Claude review: drm/msm: Add Qualcomm Eliza SoC support Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-03-04 12:58 [PATCH v2 0/8] " Krzysztof Kozlowski
2026-03-04 12:58 ` [PATCH v2 7/8] drm/msm/dpu: Add support for Eliza SoC Krzysztof Kozlowski
2026-03-05 3:31 ` Claude review: " 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-patch7-20260303-drm-display-eliza-v1-7-814121dbb2bf@oss.qualcomm.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