From: sashiko-bot@kernel.org
To: "Nabige Aala" <nabige.aala@oss.qualcomm.com>
Cc: conor+dt@kernel.org, devicetree@vger.kernel.org, robh@kernel.org
Subject: Re: [PATCH 4/5] soc: qcom: ubwc: Add Shikra UBWC config
Date: Wed, 03 Jun 2026 15:30:12 +0000 [thread overview]
Message-ID: <20260603153013.2C9AA1F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260603-shikra-display-v1-4-aeac1b94faa7@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Assigning `no_ubwc_data` to the modern DPU-based Shikra platform causes an integer underflow in the MDSS display driver. `no_ubwc_data` zero-initializes `.highest_bank_bit`. For older MDP5 platforms, this is safe because `msm_mdss_enable()` skips UBWC setup entirely. However, Shikra is a modern DPU platform (MDSS HW ver 6.5), so `msm_mdss_enable()` unconditionally calls `msm_mdss_6x_setup_ubwc()`. This function executes `MDSS_UBWC_STATIC_HIGHEST_BANK_BIT(data->highest_bank_bit - 13)`. With `highest_bank_bit == 0`, this underflows to `-13`, writing garbage configuration bits to the `REG_MDSS_UBWC_STATIC` hardware register. Additionally, this introduces a latent kernel panic: if an Adreno GPU driver is later enabled for Shikra, `a6xx_set_ubwc_config()` will trigger an unconditional `BUG_ON(cfg->highest_bank_bit < 13)` during probe. The correct approach for modern platforms without UBWC is to supply a dummy configuration with `.highest_bank_bit >= 13` (as seen in `qcm2290_data` which sets it to 15).
--
commit 75083d2d954151a192078564961f45c04709ad91
Author: Nabige Aala <nabige.aala@oss.qualcomm.com>
soc: qcom: ubwc: Add Shikra UBWC config
Add UBWC configuration for the Shikra platform. Shikra has no UBWC
support and no highest_bank_bit setting, so use no_ubwc_data.
> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
> index 3fe47d8f0f638..01dca97e26714 100644
> --- a/drivers/soc/qcom/ubwc_config.c
> +++ b/drivers/soc/qcom/ubwc_config.c
> @@ -278,6 +278,7 @@ static const struct of_device_id qcom_ubwc_configs[] __maybe_unused = {
> { .compatible = "qcom,sdm660", .data = &msm8937_data },
> { .compatible = "qcom,sdm670", .data = &sdm670_data, },
> { .compatible = "qcom,sdm845", .data = &sdm845_data, },
> + { .compatible = "qcom,shikra", .data = &no_ubwc_data, },
[Severity: High]
Will using no_ubwc_data cause an integer underflow on modern DPU platforms?
The no_ubwc_data structure zero-initializes the highest_bank_bit field.
Because Shikra is a modern DPU platform, msm_mdss_enable() unconditionally
calls msm_mdss_6x_setup_ubwc() in drivers/gpu/drm/msm/msm_mdss.c, which
performs this calculation:
MDSS_UBWC_STATIC_HIGHEST_BANK_BIT(data->highest_bank_bit - 13);
If highest_bank_bit is 0, this results in -13 and writes corrupted bits
to the REG_MDSS_UBWC_STATIC hardware register.
Additionally, will this configuration trigger a kernel panic if an Adreno
GPU driver is later enabled? In a6xx_set_ubwc_config(), there is an
unconditional check:
BUG_ON(cfg->highest_bank_bit < 13);
Should this platform instead use a dummy configuration with highest_bank_bit
set to a value >= 13, similar to how qcm2290_data sets it to 15?
> { .compatible = "qcom,sm4250", .data = &sm6115_data, },
> { .compatible = "qcom,sm6115", .data = &sm6115_data, },
> { .compatible = "qcom,sm6125", .data = &sm6125_data, },
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-shikra-display-v1-0-aeac1b94faa7@oss.qualcomm.com?part=4
next prev parent reply other threads:[~2026-06-03 15:30 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-03 14:59 [PATCH 0/5] Subject: [PATCH 0/5] Add Shikra (QCM2290) display support Nabige Aala
2026-06-03 14:59 ` [PATCH 1/5] dt-bindings: display: msm: qcm2290: Add Shikra MDSS Nabige Aala
2026-06-03 15:12 ` sashiko-bot
2026-06-03 23:08 ` Dmitry Baryshkov
2026-06-03 14:59 ` [PATCH 2/5] drm/msm/mdss: Add Shikra support Nabige Aala
2026-06-03 23:09 ` Dmitry Baryshkov
2026-06-03 14:59 ` [PATCH 3/5] arm64: defconfig: Enable ILI7807S DSI panel driver Nabige Aala
2026-06-03 23:09 ` Dmitry Baryshkov
2026-06-03 14:59 ` [PATCH 4/5] soc: qcom: ubwc: Add Shikra UBWC config Nabige Aala
2026-06-03 15:30 ` sashiko-bot [this message]
2026-06-03 23:12 ` Dmitry Baryshkov
2026-06-03 14:59 ` [PATCH 5/5] iommu/arm-smmu: Add qcom,shikra-mdss to Qualcomm SMMU client tables Nabige Aala
2026-06-03 15:35 ` sashiko-bot
2026-06-03 23:11 ` Dmitry Baryshkov
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=20260603153013.2C9AA1F00893@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=nabige.aala@oss.qualcomm.com \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.