Re: [PATCH] arm64: dts: mediatek: mt8395-genio-1200-evk: Enable GPU

[AngeloGioacchino Del Regno]

Il 10/09/24 16:32, Pablo Sun ha scritto:
> Enable the Mali Valhall GPU on Genio 1200 EVK by providing regulator
> supply settings and enable the GPU node.
>
> In addition, set the GPU related regulator voltage range:
>
> 1. Set the recommended input voltage range of DVDD_GPU to (0.546V-0.787V),
>     based on Table 5-3 of MT8395 Application Processor Datasheet.
>     The regulator mt6315_7_vbuck1("Vgpu") connects to the DVDD_GPU input.
> 2. Set the input voltage of DVDD_SRAM_GPU, supplied by
>     mt6359_vsram_others_ldo_reg, to 0.75V and set it always on for GPU SRAM.
>
> This patch is tested by enabling CONFIG_DRM_PANFROST and
> on Genio 1200 EVK it probed with following dmesg:
>
> ```
> panfrost 13000000.gpu: clock rate = 700000092
> panfrost 13000000.gpu: mali-g57 id 0x9093 major 0x0 minor 0x1 status 0x0
> panfrost 13000000.gpu: features: 00000000,000019f7,
> 	               issues: 00000001,80000400
> panfrost 13000000.gpu: Features: L2:0x07120206 Shader:0x00000000
>                         Tiler:0x00000809 Mem:0x301
> 		       MMU:0x00002830 AS:0xff JS:0x7
> panfrost 13000000.gpu: shader_present=0x50045 l2_present=0x1
> [drm] Initialized panfrost 1.2.0 for 13000000.gpu on minor 0
> ```
>
> Signed-off-by: Pablo Sun <pablo.sun@xxxxxxxxxxxx>
> ---
>   .../boot/dts/mediatek/mt8395-genio-1200-evk.dts  | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
> index a06610fff8ad..9b7850b0b9b4 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
> @@ -194,6 +194,11 @@ eth_phy0: eth-phy0@1 {
>   	};
>   };
>   
> +&gpu {
> +	mali-supply = <&mt6315_7_vbuck1>;
> +	status = "okay";
> +};
> +
>   &i2c0 {
>   	clock-frequency = <400000>;
>   	pinctrl-0 = <&i2c0_pins>;
> @@ -407,6 +412,13 @@ &mt6359_vrf12_ldo_reg {
>   	regulator-always-on;
>   };
>   
> +/* for GPU SRAM */
> +&mt6359_vsram_others_ldo_reg {
> +	regulator-always-on;

No, that's not good. Like that, the GPU VSRAM will be subject to current leakage.

Remove the regulator-always-on property.
The right way of doing that is to add the vgpu to the mfg0's domain supply and
vsram to mfg1; that way all of the GPU regulators will be off at PM suspend time.

> +	regulator-min-microvolt = <750000>;
> +	regulator-max-microvolt = <750000>;
> +};
> +
>   &mt6359codec {
>   	mediatek,mic-type-0 = <1>; /* ACC */
>   	mediatek,mic-type-1 = <3>; /* DCC */
> @@ -839,8 +851,8 @@ regulators {
>   			mt6315_7_vbuck1: vbuck1 {
>   				regulator-compatible = "vbuck1";
>   				regulator-name = "Vgpu";
> -				regulator-min-microvolt = <300000>;
> -				regulator-max-microvolt = <1193750>;
> +				regulator-min-microvolt = <546000>;

I'm okay with this constraint but are you sure that MTK-SVS won't go any lower
than 0.546V?

Cheers,
Angelo

> +				regulator-max-microvolt = <787000>;
>   				regulator-enable-ramp-delay = <256>;
>   				regulator-allowed-modes = <0 1 2>;
>   			};