Re: [PATCH v6 4/4] arm64: dts: qcom: qcs8550-ayaneo-pocket-ds: add the lower DSI panel

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The negative display bias rail (AVEE) is forced always-on, which causes continuous power drain and violates LCD power sequencing constraints.
- [Medium] Misuse of pinctrl 'output-high' to statically drive a panel/DSI enable GPIO, breaking runtime power management.
- [Medium] The sde_dsi1_suspend pinctrl state fails to disable the output high configuration, causing active power leakage during system suspend.
--

--- Patch [4]: [PATCH v6 4/4] arm64: dts: qcom: qcs8550-ayaneo-pocket-ds: add the lower DSI panel ---
commit 7b158fa2393a8ff0b401b5e960f82b14b4778a83
Author: Alexandre Hamamdjian <azkali.limited@gmail.com>

arm64: dts: qcom: qcs8550-ayaneo-pocket-ds: add the lower DSI panel

This commit adds support for the lower 768x1024 IPS LCD panel on the 
Pocket DS device. It configures the ST7703 panel along with its required 
SGM3804 bias supply, SY7758 backlight driver, and TCA6408 GPIO expander.

> diff --git a/arch/arm64/boot/dts/qcom/qcs8550-ayaneo-pocket-ds.dts b/arch/arm64/boot/dts/qcom/qcs8550-ayaneo-pocket-ds.dts
> --- a/arch/arm64/boot/dts/qcom/qcs8550-ayaneo-pocket-ds.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs8550-ayaneo-pocket-ds.dts
[ ... ]
> @@ -829,6 +840,38 @@ tca6408: gpio@20 {
[ ... ]
> +		sgm3804_neg: neg {
> +			regulator-name = "panel1_avee";
> +			regulator-min-microvolt = <5200000>;
> +			regulator-max-microvolt = <5200000>;
> +
> +			enable-gpios = <&tlmm 153 GPIO_ACTIVE_HIGH>;
> +
> +			regulator-always-on;

[Severity: High]
Does forcing this negative display bias rail (AVEE) to be always-on risk
permanent hardware damage?

Applying a continuous negative bias (-5.2V) to the LCD panel while the
positive and logic supplies (AVDD, IOVCC) are disabled during display
blanking or system suspend often violates the absolute maximum ratings
of LCD drivers. Over time, this could cause latch-up and hardware failure,
along with a continuous quiescent battery drain on the device.

Could this negative rail be modeled differently so it is toggled
synchronously with the panel's active/sleep lifecycle?

> +		};
> +	};
>  };
[ ... ]
> @@ -960,6 +1003,45 @@ &mdss_dp0_out {
>  	data-lanes = <0 1 2 3>;
>  };
>  
> +&mdss_dsi1 {
[ ... ]
> +	panel@0 {
> +		compatible = "ayaneo,pocket-ds-lower-panel";
> +		reg = <0>;
> +
> +		vcc-supply = <&sgm3804_pos>;
> +		iovcc-supply = <&vreg_panel1_iovcc>;
> +
> +		reset-gpios = <&tca6408 2 GPIO_ACTIVE_LOW>;
> +		backlight = <&backlight>;
> +		rotation = <270>;
> +
> +		pinctrl-0 = <&sde_dsi1_active>;
> +		pinctrl-1 = <&sde_dsi1_suspend>;
> +		pinctrl-names = "default", "sleep";

[Severity: Medium]
Will using the pinctrl output-high property (defined in sde_dsi1_active) 
to statically drive the panel enable GPIO (tlmm 137) break runtime power 
management?

Because DRM panels typically do not automatically transition to sleep
pinctrl states during runtime display blanking (for example, in
drm_panel_unprepare), the pin will remain asserted when the screen is
logically turned off.

If this pin controls the panel or a related signal, should it be modeled
as an explicit enable-gpios or fixed regulator instead so it toggles
correctly with the panel lifecycle?

> +
> +		port {
> +			panel1_in: endpoint {
> +				remote-endpoint = <&mdss_dsi1_out>;
> +			};
> +		};
> +	};
> +};
[ ... ]
> @@ -1558,6 +1640,21 @@ ts2_irq: ts2-irq-state {
>  		bias-disable;
>  	};
>  
> +	sde_dsi1_active: sde-dsi1-active-state {
> +		pins = "gpio137";
> +		function = "gpio";
> +		drive-strength = <8>;
> +		bias-disable;
> +		output-high;
> +	};
> +
> +	sde_dsi1_suspend: sde-dsi1-suspend-state {
> +		pins = "gpio137";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-down;

[Severity: Medium]
Does this suspend state cause an active power leakage during system suspend?

The active state configures the pin with output-high, but this suspend
state only defines bias-pull-down without an explicit output-low or
output-disable. 

When transitioning to suspend, the pin will remain actively driven high
while the internal pull-down resistor is concurrently enabled, which
creates a direct short-to-ground leakage path.

> +	};
> +
>  	ts2_reset: ts2-reset-state {
>  		pins = "gpio38";
>  		function = "gpio";

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-pocketds-v6-0-370161574ca2@gmail.com?part=4