From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/msm/mdss: correct UBWC programming sequences Date: Mon, 09 Mar 2026 08:32:14 +1000 Message-ID: In-Reply-To: <20260306-ubwc-rework-v1-1-9cfdff12f2bb@oss.qualcomm.com> References: <20260306-ubwc-rework-v1-0-9cfdff12f2bb@oss.qualcomm.com> <20260306-ubwc-rework-v1-1-9cfdff12f2bb@oss.qualcomm.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review Rewrites UBWC register programming to use MDSS HW revision instead of decoder version. Introduces `MDSS_HW_VER()` macro and consolidates 5 setup functions into 3. The key dispatch change: ```c + hw_rev = readl_relaxed(msm_mdss->mmio + REG_MDSS_HW_VERSION); + + if (hw_rev >= MDSS_HW_VER(6, 0, 0)) + msm_mdss_setup_ubwc_v6(msm_mdss); + else if (hw_rev >= MDSS_HW_VER(5, 0, 0)) + msm_mdss_setup_ubwc_v5(msm_mdss); + else if (hw_rev >= MDSS_HW_VER(4, 0, 0)) + msm_mdss_setup_ubwc_v4(msm_mdss); + /* else UBWC 1.0 or none, no params to program */ ``` The `msm_mdss_setup_ubwc_v6` now includes an inline version-to-tag ladder and prediction_mode logic based on `ubwc_enc_version`. This is good -- it removes the incorrect assumption that MDSS registers track UBWC decoder versions. **Issue**: In `msm_mdss_setup_ubwc_v4`, the swizzle is masked with `& 0x1`: ```c + u32 value = MDSS_UBWC_STATIC_UBWC_SWIZZLE(data->ubwc_swizzle & 0x1) | ``` But in v5, the full swizzle value is used. The commit message doesn't explain this asymmetry. If this is intentional (MDSS v4 hardware only has a 1-bit swizzle field), it deserves a comment. Carries `Fixes:` tag, which is appropriate. --- Generated by Claude Code Patch Reviewer