Re: [RFC PATCH v4 6/6] arm64: dts: rockchip: rk3576-rock-4d: enable NPU

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `vdd_npu_s0` regulator is forcibly kept `regulator-always-on`, defeating runtime dynamic power management for the NPU.
- [Medium] The patch only enables the first NPU core (`rknn_core_0`), leaving the second symmetric NPU core (`rknn_core_1`) disabled.
--

--- Patch [6]: [RFC PATCH v4 6/6] arm64: dts: rockchip: rk3576-rock-4d: enable NPU ---
commit 2ef1c8164923c68f245bae8c6ee995f5fd70d4d3
Author: Jiaxing Hu <[email protected]>

arm64: dts: rockchip: rk3576-rock-4d: enable NPU

Enable rknn_core_0 and its IOMMU on the Radxa ROCK 4D and supply the
core from vdd_npu_s0.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> index 272af1012ab03..965e0906bd07e 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts

[ ... ]

> @@ -442,6 +442,7 @@ regulator-state-mem {
>  			};
>  
>  			vdd_npu_s0: dcdc-reg2 {
> +				regulator-always-on;

[Severity: Medium]
Does keeping this regulator always-on defeat runtime dynamic power management
for the NPU?

The regulator will remain permanently powered, leading to unnecessary power
drain when the NPU is idle. Could we remove regulator-always-on and instead
assign this as the domain-supply for the NPU power domain so the generic
power domain framework can dynamically toggle the voltage?

>  				regulator-boot-on;
>  				regulator-enable-ramp-delay = <400>;
>  				regulator-min-microvolt = <550000>;

[ ... ]

> @@ -869,3 +870,12 @@ vp0_out_hdmi: endpoint@ROCKCHIP_VOP2_EP_HDMI0 {
>  		remote-endpoint = <&hdmi_in_vp0>;
>  	};
>  };
> +
> +&rknn_core_0 {
> +	npu-supply = <&vdd_npu_s0>;
> +	status = "okay";
> +};
> +
> +&rknn_mmu_0 {
> +	status = "okay";
> +};

[Severity: Medium]
Is there a reason to only enable the first NPU core (rknn_core_0) and leave
the second symmetric core (rknn_core_1) disabled?

Since both cores share the same voltage supply and power domains, leaving
rknn_core_1 and rknn_mmu_1 disabled limits the board to half of its
physical AI compute capacity.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6