Re: [PATCH] arm64: dts: qcom: hamoa: Set MDP MMCX corners to LOW_SVS per Windows

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The commit incorrectly applies a Windows ACPI sleep/idle-state voltage vote as the active-state voltage requirement for all MDP MMCX operating frequencies, severely breaking the DVFS curve and risking hardware instability.
- [Medium] Board-specific power optimizations derived from a single device's ACPI tables are inappropriately applied globally to the SoC base device tree.
- [Low] The commit message claims to update 7 MMCX OPP packages, but the implementation differs.
--

commit 4a04fd599f313f3ace74e7b33f9120e9295f92fa
Author: Daniel J Blueman <daniel@quora.org>
arm64: dts: qcom: hamoa: Set MDP MMCX corners to LOW_SVS per Windows

This commit updates the MDP MMCX operating performance points to use the
LOW_SVS voltage corner based on Windows ACPI DSDT F-state tables.

> On the Lenovo Slim 7x with Qualcomm X1 E80100, the current Windows ACPI
> DSDT votes LOW_SVS (0x40) for all 7 MMCX OPP packages in the _SB.PEP0.G0MD

[Severity: Low]
Is this description accurate? The commit message states it updates 7 MMCX OPP
packages, but the mdp_opp_table in hamoa.dtsi only contains 5 OPP entries and
this patch modifies 4 of them.

> F-state EXIT package:
> 
> "FSTATE",
> Zero,
> Package (...)
> {
>    "EXIT",
>    Package (0x02)
>    {
>        "NPARESOURCE",
>        Package (0x03)
>        {
>            One,
>            "/arc/client/rail_mmcx",
>            0x40
>        }
>    },
>    ...
> },
> 
> Apply the same in Linux for power saving opportunity.

[Severity: High]
Does this apply an idle state voltage requirement to active operating
performance points?

The F-state EXIT package defines the power requirements when a device
transitions out of a functional state into idle. By mapping this low power
state to active OPPs up to 575MHz, could this regression severely undervolt the
MDP?

During the multi-monitor high-resolution validation, the DisplayPort
controllers may have independently cast high voltage votes on the shared
RPMHPD_MMCX power domain, masking the MDP's lower vote. Would a user
connecting a low-resolution display experience regressions like hard lockups or
visual artifacts due to this voltage drop?

> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> @@ -5657,22 +5657,22 @@ opp-200000000 {
>  
>  					opp-325000000 {
>  						opp-hz = /bits/ 64 <325000000>;
> -						required-opps = <&rpmhpd_opp_svs>;
> +						required-opps = <&rpmhpd_opp_low_svs>;
>  					};

[Severity: Medium]
Since this voltage optimization was derived from the ACPI tables of the Lenovo
Slim 7x, should it be applied as an override in the board-specific .dts file
rather than globally in the SoC base device tree?

Could applying this to the base hamoa.dtsi cause regressions across all
Snapdragon X Elite/Plus devices, which might have different power delivery
designs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524115704.9442-1-daniel@quora.org?part=1