Claude review: soc/qcom/ubwc: rework UBWC configuration database

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

Overall Series Review

Subject: soc/qcom/ubwc: rework UBWC configuration database
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Patches: 25
Reviewed: 2026-03-09T08:32:14.521907

---

This 24-patch series by Dmitry Baryshkov (with one patch from Konrad Dybcio) systematically reworks the Qualcomm UBWC configuration database to establish the UBWC encoder version as the single source of truth from which all other UBWC parameters can be derived. The series follows a clean and well-structured approach:

1. **Foundation patches** (1-3): Fix MDSS UBWC programming to use HW revision instead of decoder version, define UBWC 3.1, and add helper functions.
2. **Driver migration** (4-13): Move all consumers (Adreno GPU, MDSS, DPU) to use the new helpers instead of raw struct fields.
3. **UBWC 3.1 adaptation** (14-17): Extend drivers to handle the newly-defined UBWC 3.1 version.
4. **Database simplification** (18-24): Progressively remove fields from `qcom_ubwc_cfg_data` (`ubwc_dec_version`, `ubwc_bank_spread`, `macrotile_mode`, `ubwc_swizzle`) and deduplicate the resulting identical configs.

**Strengths:**
- Well-ordered decomposition; each patch is self-contained and bisectable
- The end result dramatically simplifies `qcom_ubwc_cfg_data` from 6+ fields down to just `ubwc_enc_version`, `highest_bank_bit`, and `flags`
- The config database shrinks from ~22 per-SoC entries to ~14 generic entries, reducing maintenance burden
- Good use of inline helpers to derive values from the UBWC version, keeping the logic close to the header
- Patch 01 carries an appropriate Fixes tag for the incorrect decoder-version-based UBWC setup

**Concerns:**

1. **Patch 01 behavioral change in `msm_mdss_setup_ubwc_v4`**: The v4 function (MDSS HW rev 4.x) drops `ubwc_bank_spread` and masks swizzle with `& 0x1`, while the v5 function adds them. This is described as "correct" but the commit message doesn't fully explain why the old dec_20 function had bank_spread while the new v4 does not. More explanation of the MDSS HW register differences would be helpful.

2. **Patch 01 version-to-tag ladder duplication**: The version-to-tag ladder in `msm_mdss_setup_ubwc_v6` is immediately extracted to `qcom_ubwc_version_tag()` in patch 03/05, but patch 01 introduces it inline first. This creates transient code duplication. The series would be cleaner if the helper were introduced before or alongside patch 01.

3. **Patch 05 drops `UBWC_CTRL_2` write**: In the MDSS v6 setup, patch 05 removes the `writel_relaxed(ver, ...)` to `REG_MDSS_UBWC_CTRL_2` but keeps the `prediction_mode` write. This appears to be a bug -- the version tag should still be written to the register. The `qcom_ubwc_version_tag()` helper was called in the A8xx GPU path, but the MDSS path lost its CTRL_2 write entirely.

4. **UBWC 3.1 handling in `a8xx_set_ubwc_config`**: The switch statement in a8xx only has cases for `UBWC_3_0`, `UBWC_4_0`, `UBWC_5_0`, `UBWC_6_0`. After patch 15 introduces UBWC 3.1 for Adreno, `UBWC_3_1` would fall through to the `default` error case. This needs a `case UBWC_3_1:` added alongside `UBWC_3_0`.

5. **`qcom_ubwc_swizzle()` C99 mixed declarations**: The final version of `qcom_ubwc_swizzle()` declares `u32 ubwc_swizzle` after the early returns for UBWC 0 and UBWC 1.0. While this is valid C99/C11 and accepted by the kernel since 5.18, it's worth noting for style consistency.

6. **Lost TODO comments**: Patch 24 drops the `/* TODO: highest_bank_bit = 15 for LP_DDR4 */` comments from several configs during deduplication. These were reminders that the HBB should ideally use DRAM type detection. The comment on `highest_bank_bit` in the struct definition mentions this, but the per-SoC reminders are gone.

7. **Merge strategy**: The cover letter notes dependency on other SoC/UBWC patches going through linux-media. The proposed immutable tag approach is sensible but adds coordination overhead.

**Overall**: This is solid refactoring work that significantly improves the UBWC configuration architecture. The potential regression in patch 05 (missing UBWC_CTRL_2 write) needs investigation. With that addressed, the series is in good shape.

---

---
Generated by Claude Code Patch Reviewer