Re: [PATCH 3/4] arm64: dts: rockchip: Add GPU node for rk3568

Johan Jonker
2021-07-30T19:38:45Z

Hi Ezequiel,

Some comments. Have a look if it's useful.

On 7/30/21 6:45 PM, Ezequiel Garcia wrote:
> Rockchip SoCs RK3566 and RK3568 have a Mali Gondul core
> which is based on the Bifrost architecture. It has
> one shader core and two execution engines.
>
> Quoting the datasheet:
>
> Mali-G52 1-Core-2EE
> * Support 1600Mpix/s fill rate when 800MHz clock frequency
> * Support 38.4GLOPs when 800MHz clock frequency
>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> arch/arm64/boot/dts/rockchip/rk356x.dtsi | 47 ++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index bef747fb1fe2..f4f400792659 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -121,6 +121,35 @@ opp-1800000000 {
> };
> };
>

> + gpu_opp_table: opp-table2 {

gpu_opp_table: gpu-opp-table {

> + compatible = "operating-points-v2";
> +
> + opp-200000000 {
> + opp-hz = /bits/ 64 <200000000>;
> + opp-microvolt = <825000>;
> + };

Similar to cpu0_opp_table keep the same style and add an empty line
between nodes.

> + opp-300000000 {
> + opp-hz = /bits/ 64 <300000000>;
> + opp-microvolt = <825000>;
> + };
> + opp-400000000 {
> + opp-hz = /bits/ 64 <400000000>;
> + opp-microvolt = <825000>;
> + };
> + opp-600000000 {
> + opp-hz = /bits/ 64 <600000000>;
> + opp-microvolt = <825000>;
> + };
> + opp-700000000 {
> + opp-hz = /bits/ 64 <700000000>;
> + opp-microvolt = <900000>;
> + };
> + opp-800000000 {
> + opp-hz = /bits/ 64 <800000000>;
> + opp-microvolt = <1000000>;
> + };
> + };
> +
> firmware {
> scmi: scmi {
> compatible = "arm,scmi-smc";
> @@ -332,6 +361,24 @@ power-domain@RK3568_PD_RKVENC {
> };
> };
>
> + gpu: gpu@fde60000 {
> + compatible = "rockchip,rk3568-mali", "arm,mali-bifrost";
> + reg = <0x0 0xfde60000 0x0 0x4000>;

> +

remove empty lines

> + interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "job", "mmu", "gpu";

> +

dito

> + clocks = <&scmi_clk 1>, <&cru CLK_GPU>;
> + clock-names = "core", "bus";

Not sure if it's possible, but could you keep them all a little bit in
the same order/style as arm,mali-400?

From arm,mali-utgard.yaml:

clock-names:
items:
- const: bus
- const: core

> + operating-points-v2 = <&gpu_opp_table>;

> +

dito

> + #cooling-cells = <2>;
> + power-domains = <&power RK3568_PD_GPU>;
> + status = "disabled";
> + };
> +
> sdmmc2: mmc@fe000000 {
> compatible = "rockchip,rk3568-dw-mshc", "rockchip,rk3288-dw-mshc";
> reg = <0x0 0xfe000000 0x0 0x4000>;
>