* [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 1:56 ` Caesar Wang 0 siblings, 0 replies; 18+ messages in thread From: Caesar Wang @ 2016-06-30 1:56 UTC (permalink / raw) To: heiko Cc: dianders, jay.xu, briannorris, linux-kernel, linux-rockchip, linux-arm-kernel, devicetree, Elaine Zhang, Caesar Wang From: Elaine Zhang <zhangqing@rock-chips.com> In order to meet low power requirements, a power management unit (PMU) is designed for controlling power resources in RK3399. The RK3399 PMU is dedicated for managing the power of the whole chip. 1. add pd node for RK3399 Soc 2. create power domain tree 3. add qos node for domain >From the DT/binds and driver can get more detail information: The driver: drivers/soc/rockchip/pm_domains.c The document: Documentation/devicetree/bindings/soc/rockchip/power_domain.txt --- Tested on vop and gpu devices added for next kernel. PD: localhost / # cat sys/kernel/debug/pm_genpd/pm_genpd_summary domain status slaves /device runtime status ---------------------------------------------------------------------- pd_gpu on /devices/platform/ff9a0000.gpu active pd_vopl off /devices/platform/ff8f0000.vop suspended pd_vopb off /devices/platform/ff900000.vop suspended pd_vo off pd_vopb, pd_vopl pd_hdcp off ... pd_iep off pd_vcodec off pd_vdu off QOS: localhost / # cat sys/kernel/debug/pm_qos/ cpu_dma_latency network_latency memory_bandwidth network_throughput Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> Signed-off-by: Caesar Wang <wxt@rock-chips.com> --- arch/arm64/boot/dts/rockchip/rk3399.dtsi | 153 +++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index a6dd623..cc4035e 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi @@ -45,6 +45,7 @@ #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/pinctrl/rockchip.h> +#include <dt-bindings/power/rk3399-power.h> #include <dt-bindings/thermal/thermal.h> / { @@ -594,6 +595,158 @@ status = "disabled"; }; + qos_gpu: qos_gpu@ffae0000 { + compatible = "syscon"; + reg = <0x0 0xffae0000 0x0 0x20>; + }; + qos_video_m0: qos_video_m0@ffab8000 { + compatible = "syscon"; + reg = <0x0 0xffab8000 0x0 0x20>; + }; + qos_video_m1_r: qos_video_m1_r@ffac0000 { + compatible = "syscon"; + reg = <0x0 0xffac0000 0x0 0x20>; + }; + qos_video_m1_w: qos_video_m1_w@ffac0080 { + compatible = "syscon"; + reg = <0x0 0xffac0080 0x0 0x20>; + }; + qos_rga_r: qos_rga_r@ffab0000 { + compatible = "syscon"; + reg = <0x0 0xffab0000 0x0 0x20>; + }; + qos_rga_w: qos_rga_w@ffab0080 { + compatible = "syscon"; + reg = <0x0 0xffab0080 0x0 0x20>; + }; + qos_iep: qos_iep@ffa98000 { + compatible = "syscon"; + reg = <0x0 0xffa98000 0x0 0x20>; + }; + qos_vop_big_r: qos_vop_big_r@ffac8000 { + compatible = "syscon"; + reg = <0x0 0xffac8000 0x0 0x20>; + }; + qos_vop_big_w: qos_vop_big_w@ffac8080 { + compatible = "syscon"; + reg = <0x0 0xffac8080 0x0 0x20>; + }; + qos_vop_little: qos_vop_little@ffad0000 { + compatible = "syscon"; + reg = <0x0 0xffad0000 0x0 0x20>; + }; + qos_isp0_m0: qos_isp0_m0@ffaa0000 { + compatible = "syscon"; + reg = <0x0 0xffaa0000 0x0 0x20>; + }; + qos_isp0_m1: qos_isp0_m1@ffaa0080 { + compatible = "syscon"; + reg = <0x0 0xffaa0080 0x0 0x20>; + }; + qos_isp1_m0: qos_isp1_m0@ffaa8000 { + compatible = "syscon"; + reg = <0x0 0xffaa8000 0x0 0x20>; + }; + qos_isp1_m1: qos_isp1_m1@ffaa8080 { + compatible = "syscon"; + reg = <0x0 0xffaa8080 0x0 0x20>; + }; + qos_hdcp: qos_hdcp@ffa90000 { + compatible = "syscon"; + reg = <0x0 0xffa90000 0x0 0x20>; + }; + + pmu: power-management@ff310000 { + compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd"; + reg = <0x0 0xff310000 0x0 0x1000>; + + power: power-controller { + status = "okay"; + compatible = "rockchip,rk3399-power-controller"; + #power-domain-cells = <1>; + #address-cells = <1>; + #size-cells = <0>; + + pd_vdu { + reg = <RK3399_PD_VDU>; + clocks = <&cru ACLK_VDU>, + <&cru HCLK_VDU>; + pm_qos = <&qos_video_m1_r>, + <&qos_video_m1_w>; + }; + pd_vcodec { + reg = <RK3399_PD_VCODEC>; + clocks = <&cru ACLK_VCODEC>, + <&cru HCLK_VCODEC>; + pm_qos = <&qos_video_m0>; + }; + pd_iep { + reg = <RK3399_PD_IEP>; + clocks = <&cru ACLK_IEP>, + <&cru HCLK_IEP>; + pm_qos = <&qos_iep>; + }; + pd_rga { + reg = <RK3399_PD_RGA>; + clocks = <&cru ACLK_RGA>, + <&cru HCLK_RGA>; + pm_qos = <&qos_rga_r>, + <&qos_rga_w>; + }; + pd_vio { + reg = <RK3399_PD_VIO>; + #address-cells = <1>; + #size-cells = <0>; + + pd_isp0 { + reg = <RK3399_PD_ISP0>; + clocks = <&cru ACLK_ISP0>, + <&cru HCLK_ISP0>; + pm_qos = <&qos_isp0_m0>, + <&qos_isp0_m1>; + }; + pd_isp1 { + reg = <RK3399_PD_ISP1>; + clocks = <&cru ACLK_ISP1>, + <&cru HCLK_ISP1>; + pm_qos = <&qos_isp1_m0>, + <&qos_isp1_m1>; + }; + pd_hdcp { + reg = <RK3399_PD_HDCP>; + clocks = <&cru ACLK_HDCP>, + <&cru HCLK_HDCP>, + <&cru PCLK_HDCP>; + pm_qos = <&qos_hdcp>; + }; + pd_vo { + reg = <RK3399_PD_VO>; + #address-cells = <1>; + #size-cells = <0>; + + pd_vopb { + reg = <RK3399_PD_VOPB>; + clocks = <&cru ACLK_VOP0>, + <&cru HCLK_VOP0>; + pm_qos = <&qos_vop_big_r>, + <&qos_vop_big_w>; + }; + pd_vopl { + reg = <RK3399_PD_VOPL>; + clocks = <&cru ACLK_VOP1>, + <&cru HCLK_VOP1>; + pm_qos = <&qos_vop_little>; + }; + }; + }; + pd_gpu { + reg = <RK3399_PD_GPU>; + clocks = <&cru ACLK_GPU>; + pm_qos = <&qos_gpu>; + }; + }; + }; + pmugrf: syscon@ff320000 { compatible = "rockchip,rk3399-pmugrf", "syscon", "simple-mfd"; reg = <0x0 0xff320000 0x0 0x1000>; -- 1.9.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 1:56 ` Caesar Wang 0 siblings, 0 replies; 18+ messages in thread From: Caesar Wang @ 2016-06-30 1:56 UTC (permalink / raw) To: linux-arm-kernel From: Elaine Zhang <zhangqing@rock-chips.com> In order to meet low power requirements, a power management unit (PMU) is designed for controlling power resources in RK3399. The RK3399 PMU is dedicated for managing the power of the whole chip. 1. add pd node for RK3399 Soc 2. create power domain tree 3. add qos node for domain >From the DT/binds and driver can get more detail information: The driver: drivers/soc/rockchip/pm_domains.c The document: Documentation/devicetree/bindings/soc/rockchip/power_domain.txt --- Tested on vop and gpu devices added for next kernel. PD: localhost / # cat sys/kernel/debug/pm_genpd/pm_genpd_summary domain status slaves /device runtime status ---------------------------------------------------------------------- pd_gpu on /devices/platform/ff9a0000.gpu active pd_vopl off /devices/platform/ff8f0000.vop suspended pd_vopb off /devices/platform/ff900000.vop suspended pd_vo off pd_vopb, pd_vopl pd_hdcp off ... pd_iep off pd_vcodec off pd_vdu off QOS: localhost / # cat sys/kernel/debug/pm_qos/ cpu_dma_latency network_latency memory_bandwidth network_throughput Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> Signed-off-by: Caesar Wang <wxt@rock-chips.com> --- arch/arm64/boot/dts/rockchip/rk3399.dtsi | 153 +++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index a6dd623..cc4035e 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi @@ -45,6 +45,7 @@ #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/pinctrl/rockchip.h> +#include <dt-bindings/power/rk3399-power.h> #include <dt-bindings/thermal/thermal.h> / { @@ -594,6 +595,158 @@ status = "disabled"; }; + qos_gpu: qos_gpu at ffae0000 { + compatible = "syscon"; + reg = <0x0 0xffae0000 0x0 0x20>; + }; + qos_video_m0: qos_video_m0 at ffab8000 { + compatible = "syscon"; + reg = <0x0 0xffab8000 0x0 0x20>; + }; + qos_video_m1_r: qos_video_m1_r at ffac0000 { + compatible = "syscon"; + reg = <0x0 0xffac0000 0x0 0x20>; + }; + qos_video_m1_w: qos_video_m1_w at ffac0080 { + compatible = "syscon"; + reg = <0x0 0xffac0080 0x0 0x20>; + }; + qos_rga_r: qos_rga_r at ffab0000 { + compatible = "syscon"; + reg = <0x0 0xffab0000 0x0 0x20>; + }; + qos_rga_w: qos_rga_w at ffab0080 { + compatible = "syscon"; + reg = <0x0 0xffab0080 0x0 0x20>; + }; + qos_iep: qos_iep at ffa98000 { + compatible = "syscon"; + reg = <0x0 0xffa98000 0x0 0x20>; + }; + qos_vop_big_r: qos_vop_big_r at ffac8000 { + compatible = "syscon"; + reg = <0x0 0xffac8000 0x0 0x20>; + }; + qos_vop_big_w: qos_vop_big_w at ffac8080 { + compatible = "syscon"; + reg = <0x0 0xffac8080 0x0 0x20>; + }; + qos_vop_little: qos_vop_little at ffad0000 { + compatible = "syscon"; + reg = <0x0 0xffad0000 0x0 0x20>; + }; + qos_isp0_m0: qos_isp0_m0 at ffaa0000 { + compatible = "syscon"; + reg = <0x0 0xffaa0000 0x0 0x20>; + }; + qos_isp0_m1: qos_isp0_m1 at ffaa0080 { + compatible = "syscon"; + reg = <0x0 0xffaa0080 0x0 0x20>; + }; + qos_isp1_m0: qos_isp1_m0 at ffaa8000 { + compatible = "syscon"; + reg = <0x0 0xffaa8000 0x0 0x20>; + }; + qos_isp1_m1: qos_isp1_m1 at ffaa8080 { + compatible = "syscon"; + reg = <0x0 0xffaa8080 0x0 0x20>; + }; + qos_hdcp: qos_hdcp at ffa90000 { + compatible = "syscon"; + reg = <0x0 0xffa90000 0x0 0x20>; + }; + + pmu: power-management at ff310000 { + compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd"; + reg = <0x0 0xff310000 0x0 0x1000>; + + power: power-controller { + status = "okay"; + compatible = "rockchip,rk3399-power-controller"; + #power-domain-cells = <1>; + #address-cells = <1>; + #size-cells = <0>; + + pd_vdu { + reg = <RK3399_PD_VDU>; + clocks = <&cru ACLK_VDU>, + <&cru HCLK_VDU>; + pm_qos = <&qos_video_m1_r>, + <&qos_video_m1_w>; + }; + pd_vcodec { + reg = <RK3399_PD_VCODEC>; + clocks = <&cru ACLK_VCODEC>, + <&cru HCLK_VCODEC>; + pm_qos = <&qos_video_m0>; + }; + pd_iep { + reg = <RK3399_PD_IEP>; + clocks = <&cru ACLK_IEP>, + <&cru HCLK_IEP>; + pm_qos = <&qos_iep>; + }; + pd_rga { + reg = <RK3399_PD_RGA>; + clocks = <&cru ACLK_RGA>, + <&cru HCLK_RGA>; + pm_qos = <&qos_rga_r>, + <&qos_rga_w>; + }; + pd_vio { + reg = <RK3399_PD_VIO>; + #address-cells = <1>; + #size-cells = <0>; + + pd_isp0 { + reg = <RK3399_PD_ISP0>; + clocks = <&cru ACLK_ISP0>, + <&cru HCLK_ISP0>; + pm_qos = <&qos_isp0_m0>, + <&qos_isp0_m1>; + }; + pd_isp1 { + reg = <RK3399_PD_ISP1>; + clocks = <&cru ACLK_ISP1>, + <&cru HCLK_ISP1>; + pm_qos = <&qos_isp1_m0>, + <&qos_isp1_m1>; + }; + pd_hdcp { + reg = <RK3399_PD_HDCP>; + clocks = <&cru ACLK_HDCP>, + <&cru HCLK_HDCP>, + <&cru PCLK_HDCP>; + pm_qos = <&qos_hdcp>; + }; + pd_vo { + reg = <RK3399_PD_VO>; + #address-cells = <1>; + #size-cells = <0>; + + pd_vopb { + reg = <RK3399_PD_VOPB>; + clocks = <&cru ACLK_VOP0>, + <&cru HCLK_VOP0>; + pm_qos = <&qos_vop_big_r>, + <&qos_vop_big_w>; + }; + pd_vopl { + reg = <RK3399_PD_VOPL>; + clocks = <&cru ACLK_VOP1>, + <&cru HCLK_VOP1>; + pm_qos = <&qos_vop_little>; + }; + }; + }; + pd_gpu { + reg = <RK3399_PD_GPU>; + clocks = <&cru ACLK_GPU>; + pm_qos = <&qos_gpu>; + }; + }; + }; + pmugrf: syscon at ff320000 { compatible = "rockchip,rk3399-pmugrf", "syscon", "simple-mfd"; reg = <0x0 0xff320000 0x0 0x1000>; -- 1.9.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 1:56 ` Caesar Wang 0 siblings, 0 replies; 18+ messages in thread From: Caesar Wang @ 2016-06-30 1:56 UTC (permalink / raw) To: heiko Cc: dianders, jay.xu, briannorris, linux-kernel, linux-rockchip, linux-arm-kernel, devicetree, Elaine Zhang, Caesar Wang From: Elaine Zhang <zhangqing@rock-chips.com> In order to meet low power requirements, a power management unit (PMU) is designed for controlling power resources in RK3399. The RK3399 PMU is dedicated for managing the power of the whole chip. 1. add pd node for RK3399 Soc 2. create power domain tree 3. add qos node for domain From the DT/binds and driver can get more detail information: The driver: drivers/soc/rockchip/pm_domains.c The document: Documentation/devicetree/bindings/soc/rockchip/power_domain.txt --- Tested on vop and gpu devices added for next kernel. PD: localhost / # cat sys/kernel/debug/pm_genpd/pm_genpd_summary domain status slaves /device runtime status ---------------------------------------------------------------------- pd_gpu on /devices/platform/ff9a0000.gpu active pd_vopl off /devices/platform/ff8f0000.vop suspended pd_vopb off /devices/platform/ff900000.vop suspended pd_vo off pd_vopb, pd_vopl pd_hdcp off ... pd_iep off pd_vcodec off pd_vdu off QOS: localhost / # cat sys/kernel/debug/pm_qos/ cpu_dma_latency network_latency memory_bandwidth network_throughput Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> Signed-off-by: Caesar Wang <wxt@rock-chips.com> --- arch/arm64/boot/dts/rockchip/rk3399.dtsi | 153 +++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index a6dd623..cc4035e 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi @@ -45,6 +45,7 @@ #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/pinctrl/rockchip.h> +#include <dt-bindings/power/rk3399-power.h> #include <dt-bindings/thermal/thermal.h> / { @@ -594,6 +595,158 @@ status = "disabled"; }; + qos_gpu: qos_gpu@ffae0000 { + compatible = "syscon"; + reg = <0x0 0xffae0000 0x0 0x20>; + }; + qos_video_m0: qos_video_m0@ffab8000 { + compatible = "syscon"; + reg = <0x0 0xffab8000 0x0 0x20>; + }; + qos_video_m1_r: qos_video_m1_r@ffac0000 { + compatible = "syscon"; + reg = <0x0 0xffac0000 0x0 0x20>; + }; + qos_video_m1_w: qos_video_m1_w@ffac0080 { + compatible = "syscon"; + reg = <0x0 0xffac0080 0x0 0x20>; + }; + qos_rga_r: qos_rga_r@ffab0000 { + compatible = "syscon"; + reg = <0x0 0xffab0000 0x0 0x20>; + }; + qos_rga_w: qos_rga_w@ffab0080 { + compatible = "syscon"; + reg = <0x0 0xffab0080 0x0 0x20>; + }; + qos_iep: qos_iep@ffa98000 { + compatible = "syscon"; + reg = <0x0 0xffa98000 0x0 0x20>; + }; + qos_vop_big_r: qos_vop_big_r@ffac8000 { + compatible = "syscon"; + reg = <0x0 0xffac8000 0x0 0x20>; + }; + qos_vop_big_w: qos_vop_big_w@ffac8080 { + compatible = "syscon"; + reg = <0x0 0xffac8080 0x0 0x20>; + }; + qos_vop_little: qos_vop_little@ffad0000 { + compatible = "syscon"; + reg = <0x0 0xffad0000 0x0 0x20>; + }; + qos_isp0_m0: qos_isp0_m0@ffaa0000 { + compatible = "syscon"; + reg = <0x0 0xffaa0000 0x0 0x20>; + }; + qos_isp0_m1: qos_isp0_m1@ffaa0080 { + compatible = "syscon"; + reg = <0x0 0xffaa0080 0x0 0x20>; + }; + qos_isp1_m0: qos_isp1_m0@ffaa8000 { + compatible = "syscon"; + reg = <0x0 0xffaa8000 0x0 0x20>; + }; + qos_isp1_m1: qos_isp1_m1@ffaa8080 { + compatible = "syscon"; + reg = <0x0 0xffaa8080 0x0 0x20>; + }; + qos_hdcp: qos_hdcp@ffa90000 { + compatible = "syscon"; + reg = <0x0 0xffa90000 0x0 0x20>; + }; + + pmu: power-management@ff310000 { + compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd"; + reg = <0x0 0xff310000 0x0 0x1000>; + + power: power-controller { + status = "okay"; + compatible = "rockchip,rk3399-power-controller"; + #power-domain-cells = <1>; + #address-cells = <1>; + #size-cells = <0>; + + pd_vdu { + reg = <RK3399_PD_VDU>; + clocks = <&cru ACLK_VDU>, + <&cru HCLK_VDU>; + pm_qos = <&qos_video_m1_r>, + <&qos_video_m1_w>; + }; + pd_vcodec { + reg = <RK3399_PD_VCODEC>; + clocks = <&cru ACLK_VCODEC>, + <&cru HCLK_VCODEC>; + pm_qos = <&qos_video_m0>; + }; + pd_iep { + reg = <RK3399_PD_IEP>; + clocks = <&cru ACLK_IEP>, + <&cru HCLK_IEP>; + pm_qos = <&qos_iep>; + }; + pd_rga { + reg = <RK3399_PD_RGA>; + clocks = <&cru ACLK_RGA>, + <&cru HCLK_RGA>; + pm_qos = <&qos_rga_r>, + <&qos_rga_w>; + }; + pd_vio { + reg = <RK3399_PD_VIO>; + #address-cells = <1>; + #size-cells = <0>; + + pd_isp0 { + reg = <RK3399_PD_ISP0>; + clocks = <&cru ACLK_ISP0>, + <&cru HCLK_ISP0>; + pm_qos = <&qos_isp0_m0>, + <&qos_isp0_m1>; + }; + pd_isp1 { + reg = <RK3399_PD_ISP1>; + clocks = <&cru ACLK_ISP1>, + <&cru HCLK_ISP1>; + pm_qos = <&qos_isp1_m0>, + <&qos_isp1_m1>; + }; + pd_hdcp { + reg = <RK3399_PD_HDCP>; + clocks = <&cru ACLK_HDCP>, + <&cru HCLK_HDCP>, + <&cru PCLK_HDCP>; + pm_qos = <&qos_hdcp>; + }; + pd_vo { + reg = <RK3399_PD_VO>; + #address-cells = <1>; + #size-cells = <0>; + + pd_vopb { + reg = <RK3399_PD_VOPB>; + clocks = <&cru ACLK_VOP0>, + <&cru HCLK_VOP0>; + pm_qos = <&qos_vop_big_r>, + <&qos_vop_big_w>; + }; + pd_vopl { + reg = <RK3399_PD_VOPL>; + clocks = <&cru ACLK_VOP1>, + <&cru HCLK_VOP1>; + pm_qos = <&qos_vop_little>; + }; + }; + }; + pd_gpu { + reg = <RK3399_PD_GPU>; + clocks = <&cru ACLK_GPU>; + pm_qos = <&qos_gpu>; + }; + }; + }; + pmugrf: syscon@ff320000 { compatible = "rockchip,rk3399-pmugrf", "syscon", "simple-mfd"; reg = <0x0 0xff320000 0x0 0x1000>; -- 1.9.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 21:49 ` Doug Anderson 0 siblings, 0 replies; 18+ messages in thread From: Doug Anderson @ 2016-06-30 21:49 UTC (permalink / raw) To: Caesar Wang Cc: Heiko Stübner, Xu Jianqun, Brian Norris, linux-kernel, open list:ARM/Rockchip SoC..., linux-arm-kernel, devicetree, Elaine Zhang Caesar, On Wed, Jun 29, 2016 at 6:56 PM, Caesar Wang <wxt@rock-chips.com> wrote: > From: Elaine Zhang <zhangqing@rock-chips.com> > > In order to meet low power requirements, a power management unit (PMU) is > designed for controlling power resources in RK3399. The RK3399 PMU is > dedicated for managing the power of the whole chip. > > 1. add pd node for RK3399 Soc > 2. create power domain tree > 3. add qos node for domain > > From the DT/binds and driver can get more detail information: > The driver: > drivers/soc/rockchip/pm_domains.c > The document: > Documentation/devicetree/bindings/soc/rockchip/power_domain.txt > > --- > > Tested on vop and gpu devices added for next kernel. > PD: > localhost / # cat sys/kernel/debug/pm_genpd/pm_genpd_summary Nit: can you put a "/" before "sys" here and elsewhere in your patches? > domain status slaves > /device runtime status > ---------------------------------------------------------------------- > pd_gpu on > /devices/platform/ff9a0000.gpu active > pd_vopl off > /devices/platform/ff8f0000.vop suspended > pd_vopb off > /devices/platform/ff900000.vop suspended > pd_vo off pd_vopb, pd_vopl > pd_hdcp off > ... > pd_iep off > pd_vcodec off > pd_vdu off > > QOS: > localhost / # cat sys/kernel/debug/pm_qos/ > cpu_dma_latency network_latency > memory_bandwidth network_throughput What is this supposed to be showing exactly? You can't "cat" a directory, so maybe you meant "ls"? Also, each of these files contains the string "Empty!" and these files seem fairly unconnected to your patch. Those files exist both before and after your patch and nothing that I can see in the Rockchip QoS stuff hooks up to the generic Linux QoS infrastructure. The power domains just save and restore the QoS--they don't actually allow settting it. > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > Signed-off-by: Caesar Wang <wxt@rock-chips.com> > --- > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 153 +++++++++++++++++++++++++++++++ > 1 file changed, 153 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > index a6dd623..cc4035e 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > @@ -45,6 +45,7 @@ > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/pinctrl/rockchip.h> > +#include <dt-bindings/power/rk3399-power.h> > #include <dt-bindings/thermal/thermal.h> > > / { > @@ -594,6 +595,158 @@ > status = "disabled"; > }; > > + qos_gpu: qos_gpu@ffae0000 { > + compatible = "syscon"; > + reg = <0x0 0xffae0000 0x0 0x20>; > + }; > + qos_video_m0: qos_video_m0@ffab8000 { > + compatible = "syscon"; > + reg = <0x0 0xffab8000 0x0 0x20>; > + }; > + qos_video_m1_r: qos_video_m1_r@ffac0000 { > + compatible = "syscon"; > + reg = <0x0 0xffac0000 0x0 0x20>; > + }; > + qos_video_m1_w: qos_video_m1_w@ffac0080 { > + compatible = "syscon"; > + reg = <0x0 0xffac0080 0x0 0x20>; > + }; > + qos_rga_r: qos_rga_r@ffab0000 { > + compatible = "syscon"; > + reg = <0x0 0xffab0000 0x0 0x20>; > + }; > + qos_rga_w: qos_rga_w@ffab0080 { > + compatible = "syscon"; > + reg = <0x0 0xffab0080 0x0 0x20>; > + }; > + qos_iep: qos_iep@ffa98000 { > + compatible = "syscon"; > + reg = <0x0 0xffa98000 0x0 0x20>; > + }; > + qos_vop_big_r: qos_vop_big_r@ffac8000 { > + compatible = "syscon"; > + reg = <0x0 0xffac8000 0x0 0x20>; > + }; > + qos_vop_big_w: qos_vop_big_w@ffac8080 { > + compatible = "syscon"; > + reg = <0x0 0xffac8080 0x0 0x20>; > + }; > + qos_vop_little: qos_vop_little@ffad0000 { > + compatible = "syscon"; > + reg = <0x0 0xffad0000 0x0 0x20>; > + }; > + qos_isp0_m0: qos_isp0_m0@ffaa0000 { > + compatible = "syscon"; > + reg = <0x0 0xffaa0000 0x0 0x20>; > + }; > + qos_isp0_m1: qos_isp0_m1@ffaa0080 { > + compatible = "syscon"; > + reg = <0x0 0xffaa0080 0x0 0x20>; > + }; > + qos_isp1_m0: qos_isp1_m0@ffaa8000 { > + compatible = "syscon"; > + reg = <0x0 0xffaa8000 0x0 0x20>; > + }; > + qos_isp1_m1: qos_isp1_m1@ffaa8080 { > + compatible = "syscon"; > + reg = <0x0 0xffaa8080 0x0 0x20>; > + }; > + qos_hdcp: qos_hdcp@ffa90000 { > + compatible = "syscon"; > + reg = <0x0 0xffa90000 0x0 0x20>; > + }; A bit of a nit that your sorting is a bit random here. I know that your sorting order seems to match the TRM, but really we should just sort by base address. Speaking of the TRM, it seems like some entries are missing. QOS for everything above the GPU (like usb host1, for instance) is missing. Similarly the 3 entries in the TRM listed after hdcp. Should we put them all in so that we have them later if/when we need them? > + pmu: power-management@ff310000 { > + compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd"; > + reg = <0x0 0xff310000 0x0 0x1000>; > + > + power: power-controller { > + status = "okay"; > + compatible = "rockchip,rk3399-power-controller"; > + #power-domain-cells = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + pd_vdu { > + reg = <RK3399_PD_VDU>; > + clocks = <&cru ACLK_VDU>, > + <&cru HCLK_VDU>; > + pm_qos = <&qos_video_m1_r>, > + <&qos_video_m1_w>; > + }; > + pd_vcodec { > + reg = <RK3399_PD_VCODEC>; > + clocks = <&cru ACLK_VCODEC>, > + <&cru HCLK_VCODEC>; > + pm_qos = <&qos_video_m0>; > + }; > + pd_iep { > + reg = <RK3399_PD_IEP>; > + clocks = <&cru ACLK_IEP>, > + <&cru HCLK_IEP>; > + pm_qos = <&qos_iep>; > + }; > + pd_rga { > + reg = <RK3399_PD_RGA>; > + clocks = <&cru ACLK_RGA>, > + <&cru HCLK_RGA>; > + pm_qos = <&qos_rga_r>, > + <&qos_rga_w>; > + }; > + pd_vio { > + reg = <RK3399_PD_VIO>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + pd_isp0 { > + reg = <RK3399_PD_ISP0>; > + clocks = <&cru ACLK_ISP0>, > + <&cru HCLK_ISP0>; > + pm_qos = <&qos_isp0_m0>, > + <&qos_isp0_m1>; > + }; > + pd_isp1 { > + reg = <RK3399_PD_ISP1>; > + clocks = <&cru ACLK_ISP1>, > + <&cru HCLK_ISP1>; > + pm_qos = <&qos_isp1_m0>, > + <&qos_isp1_m1>; > + }; > + pd_hdcp { > + reg = <RK3399_PD_HDCP>; > + clocks = <&cru ACLK_HDCP>, > + <&cru HCLK_HDCP>, > + <&cru PCLK_HDCP>; > + pm_qos = <&qos_hdcp>; > + }; > + pd_vo { > + reg = <RK3399_PD_VO>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + pd_vopb { > + reg = <RK3399_PD_VOPB>; > + clocks = <&cru ACLK_VOP0>, > + <&cru HCLK_VOP0>; > + pm_qos = <&qos_vop_big_r>, > + <&qos_vop_big_w>; > + }; > + pd_vopl { > + reg = <RK3399_PD_VOPL>; > + clocks = <&cru ACLK_VOP1>, > + <&cru HCLK_VOP1>; > + pm_qos = <&qos_vop_little>; > + }; > + }; > + }; > + pd_gpu { > + reg = <RK3399_PD_GPU>; > + clocks = <&cru ACLK_GPU>; > + pm_qos = <&qos_gpu>; > + }; Again a nitty sort order question. Is there a reason not to make things alphabetical? AKA: pd_gpu, pd_iep, pd_rga, ... ...and inside pd_vio should be alphabetical too? In the TRM it looks like some of the power domains are grouped together (like all the domains under LOGIC or CENTERLOGIC). If keeping that grouping makes sense here then you should add a comment at the start of each group and sort the groups sanely (and sort within each group). It looks like there are also more power domains that you haven't listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you planning to add those in a followon patch? -Doug ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 21:49 ` Doug Anderson 0 siblings, 0 replies; 18+ messages in thread From: Doug Anderson @ 2016-06-30 21:49 UTC (permalink / raw) To: linux-arm-kernel Caesar, On Wed, Jun 29, 2016 at 6:56 PM, Caesar Wang <wxt@rock-chips.com> wrote: > From: Elaine Zhang <zhangqing@rock-chips.com> > > In order to meet low power requirements, a power management unit (PMU) is > designed for controlling power resources in RK3399. The RK3399 PMU is > dedicated for managing the power of the whole chip. > > 1. add pd node for RK3399 Soc > 2. create power domain tree > 3. add qos node for domain > > From the DT/binds and driver can get more detail information: > The driver: > drivers/soc/rockchip/pm_domains.c > The document: > Documentation/devicetree/bindings/soc/rockchip/power_domain.txt > > --- > > Tested on vop and gpu devices added for next kernel. > PD: > localhost / # cat sys/kernel/debug/pm_genpd/pm_genpd_summary Nit: can you put a "/" before "sys" here and elsewhere in your patches? > domain status slaves > /device runtime status > ---------------------------------------------------------------------- > pd_gpu on > /devices/platform/ff9a0000.gpu active > pd_vopl off > /devices/platform/ff8f0000.vop suspended > pd_vopb off > /devices/platform/ff900000.vop suspended > pd_vo off pd_vopb, pd_vopl > pd_hdcp off > ... > pd_iep off > pd_vcodec off > pd_vdu off > > QOS: > localhost / # cat sys/kernel/debug/pm_qos/ > cpu_dma_latency network_latency > memory_bandwidth network_throughput What is this supposed to be showing exactly? You can't "cat" a directory, so maybe you meant "ls"? Also, each of these files contains the string "Empty!" and these files seem fairly unconnected to your patch. Those files exist both before and after your patch and nothing that I can see in the Rockchip QoS stuff hooks up to the generic Linux QoS infrastructure. The power domains just save and restore the QoS--they don't actually allow settting it. > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > Signed-off-by: Caesar Wang <wxt@rock-chips.com> > --- > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 153 +++++++++++++++++++++++++++++++ > 1 file changed, 153 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > index a6dd623..cc4035e 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > @@ -45,6 +45,7 @@ > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/pinctrl/rockchip.h> > +#include <dt-bindings/power/rk3399-power.h> > #include <dt-bindings/thermal/thermal.h> > > / { > @@ -594,6 +595,158 @@ > status = "disabled"; > }; > > + qos_gpu: qos_gpu at ffae0000 { > + compatible = "syscon"; > + reg = <0x0 0xffae0000 0x0 0x20>; > + }; > + qos_video_m0: qos_video_m0 at ffab8000 { > + compatible = "syscon"; > + reg = <0x0 0xffab8000 0x0 0x20>; > + }; > + qos_video_m1_r: qos_video_m1_r at ffac0000 { > + compatible = "syscon"; > + reg = <0x0 0xffac0000 0x0 0x20>; > + }; > + qos_video_m1_w: qos_video_m1_w at ffac0080 { > + compatible = "syscon"; > + reg = <0x0 0xffac0080 0x0 0x20>; > + }; > + qos_rga_r: qos_rga_r at ffab0000 { > + compatible = "syscon"; > + reg = <0x0 0xffab0000 0x0 0x20>; > + }; > + qos_rga_w: qos_rga_w at ffab0080 { > + compatible = "syscon"; > + reg = <0x0 0xffab0080 0x0 0x20>; > + }; > + qos_iep: qos_iep at ffa98000 { > + compatible = "syscon"; > + reg = <0x0 0xffa98000 0x0 0x20>; > + }; > + qos_vop_big_r: qos_vop_big_r at ffac8000 { > + compatible = "syscon"; > + reg = <0x0 0xffac8000 0x0 0x20>; > + }; > + qos_vop_big_w: qos_vop_big_w at ffac8080 { > + compatible = "syscon"; > + reg = <0x0 0xffac8080 0x0 0x20>; > + }; > + qos_vop_little: qos_vop_little at ffad0000 { > + compatible = "syscon"; > + reg = <0x0 0xffad0000 0x0 0x20>; > + }; > + qos_isp0_m0: qos_isp0_m0 at ffaa0000 { > + compatible = "syscon"; > + reg = <0x0 0xffaa0000 0x0 0x20>; > + }; > + qos_isp0_m1: qos_isp0_m1 at ffaa0080 { > + compatible = "syscon"; > + reg = <0x0 0xffaa0080 0x0 0x20>; > + }; > + qos_isp1_m0: qos_isp1_m0 at ffaa8000 { > + compatible = "syscon"; > + reg = <0x0 0xffaa8000 0x0 0x20>; > + }; > + qos_isp1_m1: qos_isp1_m1 at ffaa8080 { > + compatible = "syscon"; > + reg = <0x0 0xffaa8080 0x0 0x20>; > + }; > + qos_hdcp: qos_hdcp at ffa90000 { > + compatible = "syscon"; > + reg = <0x0 0xffa90000 0x0 0x20>; > + }; A bit of a nit that your sorting is a bit random here. I know that your sorting order seems to match the TRM, but really we should just sort by base address. Speaking of the TRM, it seems like some entries are missing. QOS for everything above the GPU (like usb host1, for instance) is missing. Similarly the 3 entries in the TRM listed after hdcp. Should we put them all in so that we have them later if/when we need them? > + pmu: power-management at ff310000 { > + compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd"; > + reg = <0x0 0xff310000 0x0 0x1000>; > + > + power: power-controller { > + status = "okay"; > + compatible = "rockchip,rk3399-power-controller"; > + #power-domain-cells = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + pd_vdu { > + reg = <RK3399_PD_VDU>; > + clocks = <&cru ACLK_VDU>, > + <&cru HCLK_VDU>; > + pm_qos = <&qos_video_m1_r>, > + <&qos_video_m1_w>; > + }; > + pd_vcodec { > + reg = <RK3399_PD_VCODEC>; > + clocks = <&cru ACLK_VCODEC>, > + <&cru HCLK_VCODEC>; > + pm_qos = <&qos_video_m0>; > + }; > + pd_iep { > + reg = <RK3399_PD_IEP>; > + clocks = <&cru ACLK_IEP>, > + <&cru HCLK_IEP>; > + pm_qos = <&qos_iep>; > + }; > + pd_rga { > + reg = <RK3399_PD_RGA>; > + clocks = <&cru ACLK_RGA>, > + <&cru HCLK_RGA>; > + pm_qos = <&qos_rga_r>, > + <&qos_rga_w>; > + }; > + pd_vio { > + reg = <RK3399_PD_VIO>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + pd_isp0 { > + reg = <RK3399_PD_ISP0>; > + clocks = <&cru ACLK_ISP0>, > + <&cru HCLK_ISP0>; > + pm_qos = <&qos_isp0_m0>, > + <&qos_isp0_m1>; > + }; > + pd_isp1 { > + reg = <RK3399_PD_ISP1>; > + clocks = <&cru ACLK_ISP1>, > + <&cru HCLK_ISP1>; > + pm_qos = <&qos_isp1_m0>, > + <&qos_isp1_m1>; > + }; > + pd_hdcp { > + reg = <RK3399_PD_HDCP>; > + clocks = <&cru ACLK_HDCP>, > + <&cru HCLK_HDCP>, > + <&cru PCLK_HDCP>; > + pm_qos = <&qos_hdcp>; > + }; > + pd_vo { > + reg = <RK3399_PD_VO>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + pd_vopb { > + reg = <RK3399_PD_VOPB>; > + clocks = <&cru ACLK_VOP0>, > + <&cru HCLK_VOP0>; > + pm_qos = <&qos_vop_big_r>, > + <&qos_vop_big_w>; > + }; > + pd_vopl { > + reg = <RK3399_PD_VOPL>; > + clocks = <&cru ACLK_VOP1>, > + <&cru HCLK_VOP1>; > + pm_qos = <&qos_vop_little>; > + }; > + }; > + }; > + pd_gpu { > + reg = <RK3399_PD_GPU>; > + clocks = <&cru ACLK_GPU>; > + pm_qos = <&qos_gpu>; > + }; Again a nitty sort order question. Is there a reason not to make things alphabetical? AKA: pd_gpu, pd_iep, pd_rga, ... ...and inside pd_vio should be alphabetical too? In the TRM it looks like some of the power domains are grouped together (like all the domains under LOGIC or CENTERLOGIC). If keeping that grouping makes sense here then you should add a comment at the start of each group and sort the groups sanely (and sort within each group). It looks like there are also more power domains that you haven't listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you planning to add those in a followon patch? -Doug ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 21:49 ` Doug Anderson 0 siblings, 0 replies; 18+ messages in thread From: Doug Anderson @ 2016-06-30 21:49 UTC (permalink / raw) To: Caesar Wang Cc: Heiko Stübner, Xu Jianqun, Brian Norris, linux-kernel-u79uwXL29TY76Z2rM5mHXA, open list:ARM/Rockchip SoC..., linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, Elaine Zhang Caesar, On Wed, Jun 29, 2016 at 6:56 PM, Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote: > From: Elaine Zhang <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org> > > In order to meet low power requirements, a power management unit (PMU) is > designed for controlling power resources in RK3399. The RK3399 PMU is > dedicated for managing the power of the whole chip. > > 1. add pd node for RK3399 Soc > 2. create power domain tree > 3. add qos node for domain > > From the DT/binds and driver can get more detail information: > The driver: > drivers/soc/rockchip/pm_domains.c > The document: > Documentation/devicetree/bindings/soc/rockchip/power_domain.txt > > --- > > Tested on vop and gpu devices added for next kernel. > PD: > localhost / # cat sys/kernel/debug/pm_genpd/pm_genpd_summary Nit: can you put a "/" before "sys" here and elsewhere in your patches? > domain status slaves > /device runtime status > ---------------------------------------------------------------------- > pd_gpu on > /devices/platform/ff9a0000.gpu active > pd_vopl off > /devices/platform/ff8f0000.vop suspended > pd_vopb off > /devices/platform/ff900000.vop suspended > pd_vo off pd_vopb, pd_vopl > pd_hdcp off > ... > pd_iep off > pd_vcodec off > pd_vdu off > > QOS: > localhost / # cat sys/kernel/debug/pm_qos/ > cpu_dma_latency network_latency > memory_bandwidth network_throughput What is this supposed to be showing exactly? You can't "cat" a directory, so maybe you meant "ls"? Also, each of these files contains the string "Empty!" and these files seem fairly unconnected to your patch. Those files exist both before and after your patch and nothing that I can see in the Rockchip QoS stuff hooks up to the generic Linux QoS infrastructure. The power domains just save and restore the QoS--they don't actually allow settting it. > Signed-off-by: Elaine Zhang <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org> > Signed-off-by: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org> > --- > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 153 +++++++++++++++++++++++++++++++ > 1 file changed, 153 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > index a6dd623..cc4035e 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > @@ -45,6 +45,7 @@ > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/pinctrl/rockchip.h> > +#include <dt-bindings/power/rk3399-power.h> > #include <dt-bindings/thermal/thermal.h> > > / { > @@ -594,6 +595,158 @@ > status = "disabled"; > }; > > + qos_gpu: qos_gpu@ffae0000 { > + compatible = "syscon"; > + reg = <0x0 0xffae0000 0x0 0x20>; > + }; > + qos_video_m0: qos_video_m0@ffab8000 { > + compatible = "syscon"; > + reg = <0x0 0xffab8000 0x0 0x20>; > + }; > + qos_video_m1_r: qos_video_m1_r@ffac0000 { > + compatible = "syscon"; > + reg = <0x0 0xffac0000 0x0 0x20>; > + }; > + qos_video_m1_w: qos_video_m1_w@ffac0080 { > + compatible = "syscon"; > + reg = <0x0 0xffac0080 0x0 0x20>; > + }; > + qos_rga_r: qos_rga_r@ffab0000 { > + compatible = "syscon"; > + reg = <0x0 0xffab0000 0x0 0x20>; > + }; > + qos_rga_w: qos_rga_w@ffab0080 { > + compatible = "syscon"; > + reg = <0x0 0xffab0080 0x0 0x20>; > + }; > + qos_iep: qos_iep@ffa98000 { > + compatible = "syscon"; > + reg = <0x0 0xffa98000 0x0 0x20>; > + }; > + qos_vop_big_r: qos_vop_big_r@ffac8000 { > + compatible = "syscon"; > + reg = <0x0 0xffac8000 0x0 0x20>; > + }; > + qos_vop_big_w: qos_vop_big_w@ffac8080 { > + compatible = "syscon"; > + reg = <0x0 0xffac8080 0x0 0x20>; > + }; > + qos_vop_little: qos_vop_little@ffad0000 { > + compatible = "syscon"; > + reg = <0x0 0xffad0000 0x0 0x20>; > + }; > + qos_isp0_m0: qos_isp0_m0@ffaa0000 { > + compatible = "syscon"; > + reg = <0x0 0xffaa0000 0x0 0x20>; > + }; > + qos_isp0_m1: qos_isp0_m1@ffaa0080 { > + compatible = "syscon"; > + reg = <0x0 0xffaa0080 0x0 0x20>; > + }; > + qos_isp1_m0: qos_isp1_m0@ffaa8000 { > + compatible = "syscon"; > + reg = <0x0 0xffaa8000 0x0 0x20>; > + }; > + qos_isp1_m1: qos_isp1_m1@ffaa8080 { > + compatible = "syscon"; > + reg = <0x0 0xffaa8080 0x0 0x20>; > + }; > + qos_hdcp: qos_hdcp@ffa90000 { > + compatible = "syscon"; > + reg = <0x0 0xffa90000 0x0 0x20>; > + }; A bit of a nit that your sorting is a bit random here. I know that your sorting order seems to match the TRM, but really we should just sort by base address. Speaking of the TRM, it seems like some entries are missing. QOS for everything above the GPU (like usb host1, for instance) is missing. Similarly the 3 entries in the TRM listed after hdcp. Should we put them all in so that we have them later if/when we need them? > + pmu: power-management@ff310000 { > + compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd"; > + reg = <0x0 0xff310000 0x0 0x1000>; > + > + power: power-controller { > + status = "okay"; > + compatible = "rockchip,rk3399-power-controller"; > + #power-domain-cells = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + pd_vdu { > + reg = <RK3399_PD_VDU>; > + clocks = <&cru ACLK_VDU>, > + <&cru HCLK_VDU>; > + pm_qos = <&qos_video_m1_r>, > + <&qos_video_m1_w>; > + }; > + pd_vcodec { > + reg = <RK3399_PD_VCODEC>; > + clocks = <&cru ACLK_VCODEC>, > + <&cru HCLK_VCODEC>; > + pm_qos = <&qos_video_m0>; > + }; > + pd_iep { > + reg = <RK3399_PD_IEP>; > + clocks = <&cru ACLK_IEP>, > + <&cru HCLK_IEP>; > + pm_qos = <&qos_iep>; > + }; > + pd_rga { > + reg = <RK3399_PD_RGA>; > + clocks = <&cru ACLK_RGA>, > + <&cru HCLK_RGA>; > + pm_qos = <&qos_rga_r>, > + <&qos_rga_w>; > + }; > + pd_vio { > + reg = <RK3399_PD_VIO>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + pd_isp0 { > + reg = <RK3399_PD_ISP0>; > + clocks = <&cru ACLK_ISP0>, > + <&cru HCLK_ISP0>; > + pm_qos = <&qos_isp0_m0>, > + <&qos_isp0_m1>; > + }; > + pd_isp1 { > + reg = <RK3399_PD_ISP1>; > + clocks = <&cru ACLK_ISP1>, > + <&cru HCLK_ISP1>; > + pm_qos = <&qos_isp1_m0>, > + <&qos_isp1_m1>; > + }; > + pd_hdcp { > + reg = <RK3399_PD_HDCP>; > + clocks = <&cru ACLK_HDCP>, > + <&cru HCLK_HDCP>, > + <&cru PCLK_HDCP>; > + pm_qos = <&qos_hdcp>; > + }; > + pd_vo { > + reg = <RK3399_PD_VO>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + pd_vopb { > + reg = <RK3399_PD_VOPB>; > + clocks = <&cru ACLK_VOP0>, > + <&cru HCLK_VOP0>; > + pm_qos = <&qos_vop_big_r>, > + <&qos_vop_big_w>; > + }; > + pd_vopl { > + reg = <RK3399_PD_VOPL>; > + clocks = <&cru ACLK_VOP1>, > + <&cru HCLK_VOP1>; > + pm_qos = <&qos_vop_little>; > + }; > + }; > + }; > + pd_gpu { > + reg = <RK3399_PD_GPU>; > + clocks = <&cru ACLK_GPU>; > + pm_qos = <&qos_gpu>; > + }; Again a nitty sort order question. Is there a reason not to make things alphabetical? AKA: pd_gpu, pd_iep, pd_rga, ... ...and inside pd_vio should be alphabetical too? In the TRM it looks like some of the power domains are grouped together (like all the domains under LOGIC or CENTERLOGIC). If keeping that grouping makes sense here then you should add a comment at the start of each group and sort the groups sanely (and sort within each group). It looks like there are also more power domains that you haven't listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you planning to add those in a followon patch? -Doug -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 21:57 ` Heiko Stuebner 0 siblings, 0 replies; 18+ messages in thread From: Heiko Stuebner @ 2016-06-30 21:57 UTC (permalink / raw) To: Doug Anderson Cc: Caesar Wang, Xu Jianqun, Brian Norris, linux-kernel, open list:ARM/Rockchip SoC..., linux-arm-kernel, devicetree, Elaine Zhang Am Donnerstag, 30. Juni 2016, 14:49:41 schrieb Doug Anderson: > Caesar, > > On Wed, Jun 29, 2016 at 6:56 PM, Caesar Wang <wxt@rock-chips.com> wrote: > > From: Elaine Zhang <zhangqing@rock-chips.com> > > > > In order to meet low power requirements, a power management unit (PMU) > > is > > designed for controlling power resources in RK3399. The RK3399 PMU is > > dedicated for managing the power of the whole chip. > > > > 1. add pd node for RK3399 Soc > > 2. create power domain tree > > 3. add qos node for domain > > > > From the DT/binds and driver can get more detail information: > > The driver: > > drivers/soc/rockchip/pm_domains.c > > The document: > > Documentation/devicetree/bindings/soc/rockchip/power_domain.txt > > > > --- > > > > Tested on vop and gpu devices added for next kernel. > > PD: > > localhost / # cat sys/kernel/debug/pm_genpd/pm_genpd_summary > > Nit: can you put a "/" before "sys" here and elsewhere in your patches? > > > domain status slaves > > /device runtime status > > ---------------------------------------------------------------------- > > pd_gpu on > > /devices/platform/ff9a0000.gpu active > > pd_vopl off > > /devices/platform/ff8f0000.vop suspended > > pd_vopb off > > /devices/platform/ff900000.vop suspended > > pd_vo off pd_vopb, pd_vopl > > pd_hdcp off > > ... > > pd_iep off > > pd_vcodec off > > pd_vdu off > > > > QOS: > > localhost / # cat sys/kernel/debug/pm_qos/ > > cpu_dma_latency network_latency > > memory_bandwidth network_throughput > > What is this supposed to be showing exactly? You can't "cat" a > directory, so maybe you meant "ls"? > > Also, each of these files contains the string "Empty!" and these files > seem fairly unconnected to your patch. Those files exist both before > and after your patch and nothing that I can see in the Rockchip QoS > stuff hooks up to the generic Linux QoS infrastructure. The power > domains just save and restore the QoS--they don't actually allow > settting it. personally, I would just drop that debugfs-dump, as I don't see what we gain from it :-). > > > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > > Signed-off-by: Caesar Wang <wxt@rock-chips.com> > > --- [...] > > + pmu: power-management@ff310000 { > > + compatible = "rockchip,rk3399-pmu", "syscon", > > "simple-mfd"; + reg = <0x0 0xff310000 0x0 0x1000>; > > + > > + power: power-controller { > > + status = "okay"; > > + compatible = "rockchip,rk3399-power-controller"; > > + #power-domain-cells = <1>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pd_vdu { > > + reg = <RK3399_PD_VDU>; > > + clocks = <&cru ACLK_VDU>, > > + <&cru HCLK_VDU>; > > + pm_qos = <&qos_video_m1_r>, > > + <&qos_video_m1_w>; > > + }; > > + pd_vcodec { > > + reg = <RK3399_PD_VCODEC>; > > + clocks = <&cru ACLK_VCODEC>, > > + <&cru HCLK_VCODEC>; > > + pm_qos = <&qos_video_m0>; > > + }; > > + pd_iep { > > + reg = <RK3399_PD_IEP>; > > + clocks = <&cru ACLK_IEP>, > > + <&cru HCLK_IEP>; > > + pm_qos = <&qos_iep>; > > + }; > > + pd_rga { > > + reg = <RK3399_PD_RGA>; > > + clocks = <&cru ACLK_RGA>, > > + <&cru HCLK_RGA>; > > + pm_qos = <&qos_rga_r>, > > + <&qos_rga_w>; > > + }; > > + pd_vio { > > + reg = <RK3399_PD_VIO>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pd_isp0 { > > + reg = <RK3399_PD_ISP0>; > > + clocks = <&cru ACLK_ISP0>, > > + <&cru HCLK_ISP0>; > > + pm_qos = <&qos_isp0_m0>, > > + <&qos_isp0_m1>; > > + }; > > + pd_isp1 { > > + reg = <RK3399_PD_ISP1>; > > + clocks = <&cru ACLK_ISP1>, > > + <&cru HCLK_ISP1>; > > + pm_qos = <&qos_isp1_m0>, > > + <&qos_isp1_m1>; > > + }; > > + pd_hdcp { > > + reg = <RK3399_PD_HDCP>; > > + clocks = <&cru ACLK_HDCP>, > > + <&cru HCLK_HDCP>, > > + <&cru PCLK_HDCP>; > > + pm_qos = <&qos_hdcp>; > > + }; > > + pd_vo { > > + reg = <RK3399_PD_VO>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pd_vopb { > > + reg = <RK3399_PD_VOPB>; > > + clocks = <&cru > > ACLK_VOP0>, + > > <&cru HCLK_VOP0>; + > > pm_qos = <&qos_vop_big_r>, + > > <&qos_vop_big_w>; + > > }; > > + pd_vopl { > > + reg = <RK3399_PD_VOPL>; > > + clocks = <&cru > > ACLK_VOP1>, + > > <&cru HCLK_VOP1>; + > > pm_qos = <&qos_vop_little>; + }; > > + }; > > + }; > > + pd_gpu { > > + reg = <RK3399_PD_GPU>; > > + clocks = <&cru ACLK_GPU>; > > + pm_qos = <&qos_gpu>; > > + }; > > Again a nitty sort order question. Is there a reason not to make > things alphabetical? AKA: pd_gpu, pd_iep, pd_rga, ... > > ...and inside pd_vio should be alphabetical too? > > In the TRM it looks like some of the power domains are grouped > together (like all the domains under LOGIC or CENTERLOGIC). If > keeping that grouping makes sense here then you should add a comment > at the start of each group and sort the groups sanely (and sort within > each group). > > It looks like there are also more power domains that you haven't > listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you > planning to add those in a followon patch? that reminds me, nodes with a reg property should have the base address in the node name as well. Using the constant works nicely, as can be seen on the rk3288 where we have for example: pd_vio@RK3288_PD_VIO Heiko ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 21:57 ` Heiko Stuebner 0 siblings, 0 replies; 18+ messages in thread From: Heiko Stuebner @ 2016-06-30 21:57 UTC (permalink / raw) To: linux-arm-kernel Am Donnerstag, 30. Juni 2016, 14:49:41 schrieb Doug Anderson: > Caesar, > > On Wed, Jun 29, 2016 at 6:56 PM, Caesar Wang <wxt@rock-chips.com> wrote: > > From: Elaine Zhang <zhangqing@rock-chips.com> > > > > In order to meet low power requirements, a power management unit (PMU) > > is > > designed for controlling power resources in RK3399. The RK3399 PMU is > > dedicated for managing the power of the whole chip. > > > > 1. add pd node for RK3399 Soc > > 2. create power domain tree > > 3. add qos node for domain > > > > From the DT/binds and driver can get more detail information: > > The driver: > > drivers/soc/rockchip/pm_domains.c > > The document: > > Documentation/devicetree/bindings/soc/rockchip/power_domain.txt > > > > --- > > > > Tested on vop and gpu devices added for next kernel. > > PD: > > localhost / # cat sys/kernel/debug/pm_genpd/pm_genpd_summary > > Nit: can you put a "/" before "sys" here and elsewhere in your patches? > > > domain status slaves > > /device runtime status > > ---------------------------------------------------------------------- > > pd_gpu on > > /devices/platform/ff9a0000.gpu active > > pd_vopl off > > /devices/platform/ff8f0000.vop suspended > > pd_vopb off > > /devices/platform/ff900000.vop suspended > > pd_vo off pd_vopb, pd_vopl > > pd_hdcp off > > ... > > pd_iep off > > pd_vcodec off > > pd_vdu off > > > > QOS: > > localhost / # cat sys/kernel/debug/pm_qos/ > > cpu_dma_latency network_latency > > memory_bandwidth network_throughput > > What is this supposed to be showing exactly? You can't "cat" a > directory, so maybe you meant "ls"? > > Also, each of these files contains the string "Empty!" and these files > seem fairly unconnected to your patch. Those files exist both before > and after your patch and nothing that I can see in the Rockchip QoS > stuff hooks up to the generic Linux QoS infrastructure. The power > domains just save and restore the QoS--they don't actually allow > settting it. personally, I would just drop that debugfs-dump, as I don't see what we gain from it :-). > > > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > > Signed-off-by: Caesar Wang <wxt@rock-chips.com> > > --- [...] > > + pmu: power-management at ff310000 { > > + compatible = "rockchip,rk3399-pmu", "syscon", > > "simple-mfd"; + reg = <0x0 0xff310000 0x0 0x1000>; > > + > > + power: power-controller { > > + status = "okay"; > > + compatible = "rockchip,rk3399-power-controller"; > > + #power-domain-cells = <1>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pd_vdu { > > + reg = <RK3399_PD_VDU>; > > + clocks = <&cru ACLK_VDU>, > > + <&cru HCLK_VDU>; > > + pm_qos = <&qos_video_m1_r>, > > + <&qos_video_m1_w>; > > + }; > > + pd_vcodec { > > + reg = <RK3399_PD_VCODEC>; > > + clocks = <&cru ACLK_VCODEC>, > > + <&cru HCLK_VCODEC>; > > + pm_qos = <&qos_video_m0>; > > + }; > > + pd_iep { > > + reg = <RK3399_PD_IEP>; > > + clocks = <&cru ACLK_IEP>, > > + <&cru HCLK_IEP>; > > + pm_qos = <&qos_iep>; > > + }; > > + pd_rga { > > + reg = <RK3399_PD_RGA>; > > + clocks = <&cru ACLK_RGA>, > > + <&cru HCLK_RGA>; > > + pm_qos = <&qos_rga_r>, > > + <&qos_rga_w>; > > + }; > > + pd_vio { > > + reg = <RK3399_PD_VIO>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pd_isp0 { > > + reg = <RK3399_PD_ISP0>; > > + clocks = <&cru ACLK_ISP0>, > > + <&cru HCLK_ISP0>; > > + pm_qos = <&qos_isp0_m0>, > > + <&qos_isp0_m1>; > > + }; > > + pd_isp1 { > > + reg = <RK3399_PD_ISP1>; > > + clocks = <&cru ACLK_ISP1>, > > + <&cru HCLK_ISP1>; > > + pm_qos = <&qos_isp1_m0>, > > + <&qos_isp1_m1>; > > + }; > > + pd_hdcp { > > + reg = <RK3399_PD_HDCP>; > > + clocks = <&cru ACLK_HDCP>, > > + <&cru HCLK_HDCP>, > > + <&cru PCLK_HDCP>; > > + pm_qos = <&qos_hdcp>; > > + }; > > + pd_vo { > > + reg = <RK3399_PD_VO>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pd_vopb { > > + reg = <RK3399_PD_VOPB>; > > + clocks = <&cru > > ACLK_VOP0>, + > > <&cru HCLK_VOP0>; + > > pm_qos = <&qos_vop_big_r>, + > > <&qos_vop_big_w>; + > > }; > > + pd_vopl { > > + reg = <RK3399_PD_VOPL>; > > + clocks = <&cru > > ACLK_VOP1>, + > > <&cru HCLK_VOP1>; + > > pm_qos = <&qos_vop_little>; + }; > > + }; > > + }; > > + pd_gpu { > > + reg = <RK3399_PD_GPU>; > > + clocks = <&cru ACLK_GPU>; > > + pm_qos = <&qos_gpu>; > > + }; > > Again a nitty sort order question. Is there a reason not to make > things alphabetical? AKA: pd_gpu, pd_iep, pd_rga, ... > > ...and inside pd_vio should be alphabetical too? > > In the TRM it looks like some of the power domains are grouped > together (like all the domains under LOGIC or CENTERLOGIC). If > keeping that grouping makes sense here then you should add a comment > at the start of each group and sort the groups sanely (and sort within > each group). > > It looks like there are also more power domains that you haven't > listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you > planning to add those in a followon patch? that reminds me, nodes with a reg property should have the base address in the node name as well. Using the constant works nicely, as can be seen on the rk3288 where we have for example: pd_vio at RK3288_PD_VIO Heiko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 21:57 ` Heiko Stuebner 0 siblings, 0 replies; 18+ messages in thread From: Heiko Stuebner @ 2016-06-30 21:57 UTC (permalink / raw) To: Doug Anderson Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Elaine Zhang, Brian Norris, linux-kernel-u79uwXL29TY76Z2rM5mHXA, open list:ARM/Rockchip SoC..., Xu Jianqun, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Caesar Wang Am Donnerstag, 30. Juni 2016, 14:49:41 schrieb Doug Anderson: > Caesar, > > On Wed, Jun 29, 2016 at 6:56 PM, Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote: > > From: Elaine Zhang <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org> > > > > In order to meet low power requirements, a power management unit (PMU) > > is > > designed for controlling power resources in RK3399. The RK3399 PMU is > > dedicated for managing the power of the whole chip. > > > > 1. add pd node for RK3399 Soc > > 2. create power domain tree > > 3. add qos node for domain > > > > From the DT/binds and driver can get more detail information: > > The driver: > > drivers/soc/rockchip/pm_domains.c > > The document: > > Documentation/devicetree/bindings/soc/rockchip/power_domain.txt > > > > --- > > > > Tested on vop and gpu devices added for next kernel. > > PD: > > localhost / # cat sys/kernel/debug/pm_genpd/pm_genpd_summary > > Nit: can you put a "/" before "sys" here and elsewhere in your patches? > > > domain status slaves > > /device runtime status > > ---------------------------------------------------------------------- > > pd_gpu on > > /devices/platform/ff9a0000.gpu active > > pd_vopl off > > /devices/platform/ff8f0000.vop suspended > > pd_vopb off > > /devices/platform/ff900000.vop suspended > > pd_vo off pd_vopb, pd_vopl > > pd_hdcp off > > ... > > pd_iep off > > pd_vcodec off > > pd_vdu off > > > > QOS: > > localhost / # cat sys/kernel/debug/pm_qos/ > > cpu_dma_latency network_latency > > memory_bandwidth network_throughput > > What is this supposed to be showing exactly? You can't "cat" a > directory, so maybe you meant "ls"? > > Also, each of these files contains the string "Empty!" and these files > seem fairly unconnected to your patch. Those files exist both before > and after your patch and nothing that I can see in the Rockchip QoS > stuff hooks up to the generic Linux QoS infrastructure. The power > domains just save and restore the QoS--they don't actually allow > settting it. personally, I would just drop that debugfs-dump, as I don't see what we gain from it :-). > > > Signed-off-by: Elaine Zhang <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org> > > Signed-off-by: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org> > > --- [...] > > + pmu: power-management@ff310000 { > > + compatible = "rockchip,rk3399-pmu", "syscon", > > "simple-mfd"; + reg = <0x0 0xff310000 0x0 0x1000>; > > + > > + power: power-controller { > > + status = "okay"; > > + compatible = "rockchip,rk3399-power-controller"; > > + #power-domain-cells = <1>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pd_vdu { > > + reg = <RK3399_PD_VDU>; > > + clocks = <&cru ACLK_VDU>, > > + <&cru HCLK_VDU>; > > + pm_qos = <&qos_video_m1_r>, > > + <&qos_video_m1_w>; > > + }; > > + pd_vcodec { > > + reg = <RK3399_PD_VCODEC>; > > + clocks = <&cru ACLK_VCODEC>, > > + <&cru HCLK_VCODEC>; > > + pm_qos = <&qos_video_m0>; > > + }; > > + pd_iep { > > + reg = <RK3399_PD_IEP>; > > + clocks = <&cru ACLK_IEP>, > > + <&cru HCLK_IEP>; > > + pm_qos = <&qos_iep>; > > + }; > > + pd_rga { > > + reg = <RK3399_PD_RGA>; > > + clocks = <&cru ACLK_RGA>, > > + <&cru HCLK_RGA>; > > + pm_qos = <&qos_rga_r>, > > + <&qos_rga_w>; > > + }; > > + pd_vio { > > + reg = <RK3399_PD_VIO>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pd_isp0 { > > + reg = <RK3399_PD_ISP0>; > > + clocks = <&cru ACLK_ISP0>, > > + <&cru HCLK_ISP0>; > > + pm_qos = <&qos_isp0_m0>, > > + <&qos_isp0_m1>; > > + }; > > + pd_isp1 { > > + reg = <RK3399_PD_ISP1>; > > + clocks = <&cru ACLK_ISP1>, > > + <&cru HCLK_ISP1>; > > + pm_qos = <&qos_isp1_m0>, > > + <&qos_isp1_m1>; > > + }; > > + pd_hdcp { > > + reg = <RK3399_PD_HDCP>; > > + clocks = <&cru ACLK_HDCP>, > > + <&cru HCLK_HDCP>, > > + <&cru PCLK_HDCP>; > > + pm_qos = <&qos_hdcp>; > > + }; > > + pd_vo { > > + reg = <RK3399_PD_VO>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pd_vopb { > > + reg = <RK3399_PD_VOPB>; > > + clocks = <&cru > > ACLK_VOP0>, + > > <&cru HCLK_VOP0>; + > > pm_qos = <&qos_vop_big_r>, + > > <&qos_vop_big_w>; + > > }; > > + pd_vopl { > > + reg = <RK3399_PD_VOPL>; > > + clocks = <&cru > > ACLK_VOP1>, + > > <&cru HCLK_VOP1>; + > > pm_qos = <&qos_vop_little>; + }; > > + }; > > + }; > > + pd_gpu { > > + reg = <RK3399_PD_GPU>; > > + clocks = <&cru ACLK_GPU>; > > + pm_qos = <&qos_gpu>; > > + }; > > Again a nitty sort order question. Is there a reason not to make > things alphabetical? AKA: pd_gpu, pd_iep, pd_rga, ... > > ...and inside pd_vio should be alphabetical too? > > In the TRM it looks like some of the power domains are grouped > together (like all the domains under LOGIC or CENTERLOGIC). If > keeping that grouping makes sense here then you should add a comment > at the start of each group and sort the groups sanely (and sort within > each group). > > It looks like there are also more power domains that you haven't > listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you > planning to add those in a followon patch? that reminds me, nodes with a reg property should have the base address in the node name as well. Using the constant works nicely, as can be seen on the rk3288 where we have for example: pd_vio@RK3288_PD_VIO Heiko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 2016-06-30 21:57 ` Heiko Stuebner (?) @ 2016-06-30 22:32 ` Doug Anderson -1 siblings, 0 replies; 18+ messages in thread From: Doug Anderson @ 2016-06-30 22:32 UTC (permalink / raw) To: Heiko Stuebner Cc: Caesar Wang, Xu Jianqun, Brian Norris, linux-kernel, open list:ARM/Rockchip SoC..., linux-arm-kernel, devicetree, Elaine Zhang Hi, On Thu, Jun 30, 2016 at 2:57 PM, Heiko Stuebner <heiko@sntech.de> wrote: >> It looks like there are also more power domains that you haven't >> listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you >> planning to add those in a followon patch? > > that reminds me, nodes with a reg property should have the base address in > the node name as well. Using the constant works nicely, as can be seen on > the rk3288 where we have for example: > > pd_vio@RK3288_PD_VIO I was wondering about that. The device tree bindings are similarly missing the reg from the example. I'm curious: for sorting purposes, are you supposed to know the underlying integer and use that for sorting, or sort by the name of the #define? -Doug ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 22:32 ` Doug Anderson 0 siblings, 0 replies; 18+ messages in thread From: Doug Anderson @ 2016-06-30 22:32 UTC (permalink / raw) To: linux-arm-kernel Hi, On Thu, Jun 30, 2016 at 2:57 PM, Heiko Stuebner <heiko@sntech.de> wrote: >> It looks like there are also more power domains that you haven't >> listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you >> planning to add those in a followon patch? > > that reminds me, nodes with a reg property should have the base address in > the node name as well. Using the constant works nicely, as can be seen on > the rk3288 where we have for example: > > pd_vio at RK3288_PD_VIO I was wondering about that. The device tree bindings are similarly missing the reg from the example. I'm curious: for sorting purposes, are you supposed to know the underlying integer and use that for sorting, or sort by the name of the #define? -Doug ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 22:32 ` Doug Anderson 0 siblings, 0 replies; 18+ messages in thread From: Doug Anderson @ 2016-06-30 22:32 UTC (permalink / raw) To: Heiko Stuebner Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Elaine Zhang, Brian Norris, linux-kernel-u79uwXL29TY76Z2rM5mHXA, open list:ARM/Rockchip SoC..., Xu Jianqun, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Caesar Wang Hi, On Thu, Jun 30, 2016 at 2:57 PM, Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote: >> It looks like there are also more power domains that you haven't >> listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you >> planning to add those in a followon patch? > > that reminds me, nodes with a reg property should have the base address in > the node name as well. Using the constant works nicely, as can be seen on > the rk3288 where we have for example: > > pd_vio@RK3288_PD_VIO I was wondering about that. The device tree bindings are similarly missing the reg from the example. I'm curious: for sorting purposes, are you supposed to know the underlying integer and use that for sorting, or sort by the name of the #define? -Doug ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 22:50 ` Heiko Stuebner 0 siblings, 0 replies; 18+ messages in thread From: Heiko Stuebner @ 2016-06-30 22:50 UTC (permalink / raw) To: Doug Anderson Cc: Caesar Wang, Xu Jianqun, Brian Norris, linux-kernel, open list:ARM/Rockchip SoC..., linux-arm-kernel, devicetree, Elaine Zhang Am Donnerstag, 30. Juni 2016, 15:32:01 schrieb Doug Anderson: > Hi, > > On Thu, Jun 30, 2016 at 2:57 PM, Heiko Stuebner <heiko@sntech.de> wrote: > >> It looks like there are also more power domains that you haven't > >> listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you > >> planning to add those in a followon patch? > > > > that reminds me, nodes with a reg property should have the base address > > in the node name as well. Using the constant works nicely, as can be > > seen on> > > the rk3288 where we have for example: > > pd_vio@RK3288_PD_VIO > > I was wondering about that. The device tree bindings are similarly > missing the reg from the example. > > I'm curious: for sorting purposes, are you supposed to know the > underlying integer and use that for sorting, or sort by the name of > the #define? requiring the underlying integer sounds very cumbersome, especially as the sorting is only a style-thing. So personally I'd take the constants name as sorting criteria, as everything else would be somewhat counter-intuitive. Heiko ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 22:50 ` Heiko Stuebner 0 siblings, 0 replies; 18+ messages in thread From: Heiko Stuebner @ 2016-06-30 22:50 UTC (permalink / raw) To: linux-arm-kernel Am Donnerstag, 30. Juni 2016, 15:32:01 schrieb Doug Anderson: > Hi, > > On Thu, Jun 30, 2016 at 2:57 PM, Heiko Stuebner <heiko@sntech.de> wrote: > >> It looks like there are also more power domains that you haven't > >> listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you > >> planning to add those in a followon patch? > > > > that reminds me, nodes with a reg property should have the base address > > in the node name as well. Using the constant works nicely, as can be > > seen on> > > the rk3288 where we have for example: > > pd_vio at RK3288_PD_VIO > > I was wondering about that. The device tree bindings are similarly > missing the reg from the example. > > I'm curious: for sorting purposes, are you supposed to know the > underlying integer and use that for sorting, or sort by the name of > the #define? requiring the underlying integer sounds very cumbersome, especially as the sorting is only a style-thing. So personally I'd take the constants name as sorting criteria, as everything else would be somewhat counter-intuitive. Heiko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-06-30 22:50 ` Heiko Stuebner 0 siblings, 0 replies; 18+ messages in thread From: Heiko Stuebner @ 2016-06-30 22:50 UTC (permalink / raw) To: Doug Anderson Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Elaine Zhang, Brian Norris, linux-kernel-u79uwXL29TY76Z2rM5mHXA, open list:ARM/Rockchip SoC..., Xu Jianqun, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Caesar Wang Am Donnerstag, 30. Juni 2016, 15:32:01 schrieb Doug Anderson: > Hi, > > On Thu, Jun 30, 2016 at 2:57 PM, Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote: > >> It looks like there are also more power domains that you haven't > >> listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you > >> planning to add those in a followon patch? > > > > that reminds me, nodes with a reg property should have the base address > > in the node name as well. Using the constant works nicely, as can be > > seen on> > > the rk3288 where we have for example: > > pd_vio@RK3288_PD_VIO > > I was wondering about that. The device tree bindings are similarly > missing the reg from the example. > > I'm curious: for sorting purposes, are you supposed to know the > underlying integer and use that for sorting, or sort by the name of > the #define? requiring the underlying integer sounds very cumbersome, especially as the sorting is only a style-thing. So personally I'd take the constants name as sorting criteria, as everything else would be somewhat counter-intuitive. Heiko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 2016-06-30 21:57 ` Heiko Stuebner (?) @ 2016-07-01 2:11 ` Caesar Wang -1 siblings, 0 replies; 18+ messages in thread From: Caesar Wang @ 2016-07-01 2:11 UTC (permalink / raw) To: Heiko Stuebner, Doug Anderson Cc: Xu Jianqun, Brian Norris, linux-kernel, open list:ARM/Rockchip SoC..., linux-arm-kernel, devicetree, Elaine Zhang, huangtao Thanks your reviewing! On 2016年07月01日 05:57, Heiko Stuebner wrote: > Am Donnerstag, 30. Juni 2016, 14:49:41 schrieb Doug Anderson: >> Caesar, >> >> On Wed, Jun 29, 2016 at 6:56 PM, Caesar Wang <wxt@rock-chips.com> wrote: >>> From: Elaine Zhang <zhangqing@rock-chips.com> >>> >>> In order to meet low power requirements, a power management unit (PMU) >>> is >>> designed for controlling power resources in RK3399. The RK3399 PMU is >>> dedicated for managing the power of the whole chip. >>> >>> 1. add pd node for RK3399 Soc >>> 2. create power domain tree >>> 3. add qos node for domain >>> >>> From the DT/binds and driver can get more detail information: >>> The driver: >>> drivers/soc/rockchip/pm_domains.c >>> The document: >>> Documentation/devicetree/bindings/soc/rockchip/power_domain.txt >>> >>> --- >>> >>> Tested on vop and gpu devices added for next kernel. >>> PD: >>> localhost / # cat sys/kernel/debug/pm_genpd/pm_genpd_summary >> Nit: can you put a "/" before "sys" here and elsewhere in your patches? >> >>> domain status slaves >>> /device runtime status >>> ---------------------------------------------------------------------- >>> pd_gpu on >>> /devices/platform/ff9a0000.gpu active >>> pd_vopl off >>> /devices/platform/ff8f0000.vop suspended >>> pd_vopb off >>> /devices/platform/ff900000.vop suspended >>> pd_vo off pd_vopb, pd_vopl >>> pd_hdcp off >>> ... >>> pd_iep off >>> pd_vcodec off >>> pd_vdu off >>> >>> QOS: >>> localhost / # cat sys/kernel/debug/pm_qos/ >>> cpu_dma_latency network_latency >>> memory_bandwidth network_throughput >> What is this supposed to be showing exactly? You can't "cat" a >> directory, so maybe you meant "ls"? >> >> Also, each of these files contains the string "Empty!" and these files >> seem fairly unconnected to your patch. Those files exist both before >> and after your patch and nothing that I can see in the Rockchip QoS >> stuff hooks up to the generic Linux QoS infrastructure. The power >> domains just save and restore the QoS--they don't actually allow >> settting it. > personally, I would just drop that debugfs-dump, as I don't see what we gain > from it :-). Agreed, drop it. >>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> >>> Signed-off-by: Caesar Wang <wxt@rock-chips.com> >>> --- > [...] > >>> + pmu: power-management@ff310000 { >>> + compatible = "rockchip,rk3399-pmu", "syscon", >>> "simple-mfd"; + reg = <0x0 0xff310000 0x0 0x1000>; >>> + >>> + power: power-controller { >>> + status = "okay"; >>> + compatible = "rockchip,rk3399-power-controller"; >>> + #power-domain-cells = <1>; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + pd_vdu { >>> + reg = <RK3399_PD_VDU>; >>> + clocks = <&cru ACLK_VDU>, >>> + <&cru HCLK_VDU>; >>> + pm_qos = <&qos_video_m1_r>, >>> + <&qos_video_m1_w>; >>> + }; >>> + pd_vcodec { >>> + reg = <RK3399_PD_VCODEC>; >>> + clocks = <&cru ACLK_VCODEC>, >>> + <&cru HCLK_VCODEC>; >>> + pm_qos = <&qos_video_m0>; >>> + }; >>> + pd_iep { >>> + reg = <RK3399_PD_IEP>; >>> + clocks = <&cru ACLK_IEP>, >>> + <&cru HCLK_IEP>; >>> + pm_qos = <&qos_iep>; >>> + }; >>> + pd_rga { >>> + reg = <RK3399_PD_RGA>; >>> + clocks = <&cru ACLK_RGA>, >>> + <&cru HCLK_RGA>; >>> + pm_qos = <&qos_rga_r>, >>> + <&qos_rga_w>; >>> + }; >>> + pd_vio { >>> + reg = <RK3399_PD_VIO>; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + pd_isp0 { >>> + reg = <RK3399_PD_ISP0>; >>> + clocks = <&cru ACLK_ISP0>, >>> + <&cru HCLK_ISP0>; >>> + pm_qos = <&qos_isp0_m0>, >>> + <&qos_isp0_m1>; >>> + }; >>> + pd_isp1 { >>> + reg = <RK3399_PD_ISP1>; >>> + clocks = <&cru ACLK_ISP1>, >>> + <&cru HCLK_ISP1>; >>> + pm_qos = <&qos_isp1_m0>, >>> + <&qos_isp1_m1>; >>> + }; >>> + pd_hdcp { >>> + reg = <RK3399_PD_HDCP>; >>> + clocks = <&cru ACLK_HDCP>, >>> + <&cru HCLK_HDCP>, >>> + <&cru PCLK_HDCP>; >>> + pm_qos = <&qos_hdcp>; >>> + }; >>> + pd_vo { >>> + reg = <RK3399_PD_VO>; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + pd_vopb { >>> + reg = <RK3399_PD_VOPB>; >>> + clocks = <&cru >>> ACLK_VOP0>, + >>> <&cru HCLK_VOP0>; + >>> pm_qos = <&qos_vop_big_r>, + >>> <&qos_vop_big_w>; + >>> }; >>> + pd_vopl { >>> + reg = <RK3399_PD_VOPL>; >>> + clocks = <&cru >>> ACLK_VOP1>, + >>> <&cru HCLK_VOP1>; + >>> pm_qos = <&qos_vop_little>; + }; >>> + }; >>> + }; >>> + pd_gpu { >>> + reg = <RK3399_PD_GPU>; >>> + clocks = <&cru ACLK_GPU>; >>> + pm_qos = <&qos_gpu>; >>> + }; >> Again a nitty sort order question. Is there a reason not to make >> things alphabetical? AKA: pd_gpu, pd_iep, pd_rga, ... >> >> ...and inside pd_vio should be alphabetical too? >> >> In the TRM it looks like some of the power domains are grouped >> together (like all the domains under LOGIC or CENTERLOGIC). If >> keeping that grouping makes sense here then you should add a comment >> at the start of each group and sort the groups sanely (and sort within >> each group). Okay, let me see it. Thanks! >> >> It looks like there are also more power domains that you haven't >> listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you >> planning to add those in a followon patch? > that reminds me, nodes with a reg property should have the base address in > the node name as well. Using the constant works nicely, as can be seen on > the rk3288 where we have for example: > > pd_vio@RK3288_PD_VIO Agreed. > > > Heiko > > > -- caesar wang | software engineer | wxt@rock-chip.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-07-01 2:11 ` Caesar Wang 0 siblings, 0 replies; 18+ messages in thread From: Caesar Wang @ 2016-07-01 2:11 UTC (permalink / raw) To: linux-arm-kernel Thanks your reviewing! On 2016?07?01? 05:57, Heiko Stuebner wrote: > Am Donnerstag, 30. Juni 2016, 14:49:41 schrieb Doug Anderson: >> Caesar, >> >> On Wed, Jun 29, 2016 at 6:56 PM, Caesar Wang <wxt@rock-chips.com> wrote: >>> From: Elaine Zhang <zhangqing@rock-chips.com> >>> >>> In order to meet low power requirements, a power management unit (PMU) >>> is >>> designed for controlling power resources in RK3399. The RK3399 PMU is >>> dedicated for managing the power of the whole chip. >>> >>> 1. add pd node for RK3399 Soc >>> 2. create power domain tree >>> 3. add qos node for domain >>> >>> From the DT/binds and driver can get more detail information: >>> The driver: >>> drivers/soc/rockchip/pm_domains.c >>> The document: >>> Documentation/devicetree/bindings/soc/rockchip/power_domain.txt >>> >>> --- >>> >>> Tested on vop and gpu devices added for next kernel. >>> PD: >>> localhost / # cat sys/kernel/debug/pm_genpd/pm_genpd_summary >> Nit: can you put a "/" before "sys" here and elsewhere in your patches? >> >>> domain status slaves >>> /device runtime status >>> ---------------------------------------------------------------------- >>> pd_gpu on >>> /devices/platform/ff9a0000.gpu active >>> pd_vopl off >>> /devices/platform/ff8f0000.vop suspended >>> pd_vopb off >>> /devices/platform/ff900000.vop suspended >>> pd_vo off pd_vopb, pd_vopl >>> pd_hdcp off >>> ... >>> pd_iep off >>> pd_vcodec off >>> pd_vdu off >>> >>> QOS: >>> localhost / # cat sys/kernel/debug/pm_qos/ >>> cpu_dma_latency network_latency >>> memory_bandwidth network_throughput >> What is this supposed to be showing exactly? You can't "cat" a >> directory, so maybe you meant "ls"? >> >> Also, each of these files contains the string "Empty!" and these files >> seem fairly unconnected to your patch. Those files exist both before >> and after your patch and nothing that I can see in the Rockchip QoS >> stuff hooks up to the generic Linux QoS infrastructure. The power >> domains just save and restore the QoS--they don't actually allow >> settting it. > personally, I would just drop that debugfs-dump, as I don't see what we gain > from it :-). Agreed, drop it. >>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> >>> Signed-off-by: Caesar Wang <wxt@rock-chips.com> >>> --- > [...] > >>> + pmu: power-management at ff310000 { >>> + compatible = "rockchip,rk3399-pmu", "syscon", >>> "simple-mfd"; + reg = <0x0 0xff310000 0x0 0x1000>; >>> + >>> + power: power-controller { >>> + status = "okay"; >>> + compatible = "rockchip,rk3399-power-controller"; >>> + #power-domain-cells = <1>; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + pd_vdu { >>> + reg = <RK3399_PD_VDU>; >>> + clocks = <&cru ACLK_VDU>, >>> + <&cru HCLK_VDU>; >>> + pm_qos = <&qos_video_m1_r>, >>> + <&qos_video_m1_w>; >>> + }; >>> + pd_vcodec { >>> + reg = <RK3399_PD_VCODEC>; >>> + clocks = <&cru ACLK_VCODEC>, >>> + <&cru HCLK_VCODEC>; >>> + pm_qos = <&qos_video_m0>; >>> + }; >>> + pd_iep { >>> + reg = <RK3399_PD_IEP>; >>> + clocks = <&cru ACLK_IEP>, >>> + <&cru HCLK_IEP>; >>> + pm_qos = <&qos_iep>; >>> + }; >>> + pd_rga { >>> + reg = <RK3399_PD_RGA>; >>> + clocks = <&cru ACLK_RGA>, >>> + <&cru HCLK_RGA>; >>> + pm_qos = <&qos_rga_r>, >>> + <&qos_rga_w>; >>> + }; >>> + pd_vio { >>> + reg = <RK3399_PD_VIO>; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + pd_isp0 { >>> + reg = <RK3399_PD_ISP0>; >>> + clocks = <&cru ACLK_ISP0>, >>> + <&cru HCLK_ISP0>; >>> + pm_qos = <&qos_isp0_m0>, >>> + <&qos_isp0_m1>; >>> + }; >>> + pd_isp1 { >>> + reg = <RK3399_PD_ISP1>; >>> + clocks = <&cru ACLK_ISP1>, >>> + <&cru HCLK_ISP1>; >>> + pm_qos = <&qos_isp1_m0>, >>> + <&qos_isp1_m1>; >>> + }; >>> + pd_hdcp { >>> + reg = <RK3399_PD_HDCP>; >>> + clocks = <&cru ACLK_HDCP>, >>> + <&cru HCLK_HDCP>, >>> + <&cru PCLK_HDCP>; >>> + pm_qos = <&qos_hdcp>; >>> + }; >>> + pd_vo { >>> + reg = <RK3399_PD_VO>; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + pd_vopb { >>> + reg = <RK3399_PD_VOPB>; >>> + clocks = <&cru >>> ACLK_VOP0>, + >>> <&cru HCLK_VOP0>; + >>> pm_qos = <&qos_vop_big_r>, + >>> <&qos_vop_big_w>; + >>> }; >>> + pd_vopl { >>> + reg = <RK3399_PD_VOPL>; >>> + clocks = <&cru >>> ACLK_VOP1>, + >>> <&cru HCLK_VOP1>; + >>> pm_qos = <&qos_vop_little>; + }; >>> + }; >>> + }; >>> + pd_gpu { >>> + reg = <RK3399_PD_GPU>; >>> + clocks = <&cru ACLK_GPU>; >>> + pm_qos = <&qos_gpu>; >>> + }; >> Again a nitty sort order question. Is there a reason not to make >> things alphabetical? AKA: pd_gpu, pd_iep, pd_rga, ... >> >> ...and inside pd_vio should be alphabetical too? >> >> In the TRM it looks like some of the power domains are grouped >> together (like all the domains under LOGIC or CENTERLOGIC). If >> keeping that grouping makes sense here then you should add a comment >> at the start of each group and sort the groups sanely (and sort within >> each group). Okay, let me see it. Thanks! >> >> It looks like there are also more power domains that you haven't >> listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you >> planning to add those in a followon patch? > that reminds me, nodes with a reg property should have the base address in > the node name as well. Using the constant works nicely, as can be seen on > the rk3288 where we have for example: > > pd_vio at RK3288_PD_VIO Agreed. > > > Heiko > > > -- caesar wang | software engineer | wxt at rock-chip.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 @ 2016-07-01 2:11 ` Caesar Wang 0 siblings, 0 replies; 18+ messages in thread From: Caesar Wang @ 2016-07-01 2:11 UTC (permalink / raw) To: Heiko Stuebner, Doug Anderson Cc: huangtao, devicetree-u79uwXL29TY76Z2rM5mHXA, Elaine Zhang, Brian Norris, linux-kernel-u79uwXL29TY76Z2rM5mHXA, open list:ARM/Rockchip SoC..., Xu Jianqun, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Thanks your reviewing! On 2016年07月01日 05:57, Heiko Stuebner wrote: > Am Donnerstag, 30. Juni 2016, 14:49:41 schrieb Doug Anderson: >> Caesar, >> >> On Wed, Jun 29, 2016 at 6:56 PM, Caesar Wang <wxt@rock-chips.com> wrote: >>> From: Elaine Zhang <zhangqing@rock-chips.com> >>> >>> In order to meet low power requirements, a power management unit (PMU) >>> is >>> designed for controlling power resources in RK3399. The RK3399 PMU is >>> dedicated for managing the power of the whole chip. >>> >>> 1. add pd node for RK3399 Soc >>> 2. create power domain tree >>> 3. add qos node for domain >>> >>> From the DT/binds and driver can get more detail information: >>> The driver: >>> drivers/soc/rockchip/pm_domains.c >>> The document: >>> Documentation/devicetree/bindings/soc/rockchip/power_domain.txt >>> >>> --- >>> >>> Tested on vop and gpu devices added for next kernel. >>> PD: >>> localhost / # cat sys/kernel/debug/pm_genpd/pm_genpd_summary >> Nit: can you put a "/" before "sys" here and elsewhere in your patches? >> >>> domain status slaves >>> /device runtime status >>> ---------------------------------------------------------------------- >>> pd_gpu on >>> /devices/platform/ff9a0000.gpu active >>> pd_vopl off >>> /devices/platform/ff8f0000.vop suspended >>> pd_vopb off >>> /devices/platform/ff900000.vop suspended >>> pd_vo off pd_vopb, pd_vopl >>> pd_hdcp off >>> ... >>> pd_iep off >>> pd_vcodec off >>> pd_vdu off >>> >>> QOS: >>> localhost / # cat sys/kernel/debug/pm_qos/ >>> cpu_dma_latency network_latency >>> memory_bandwidth network_throughput >> What is this supposed to be showing exactly? You can't "cat" a >> directory, so maybe you meant "ls"? >> >> Also, each of these files contains the string "Empty!" and these files >> seem fairly unconnected to your patch. Those files exist both before >> and after your patch and nothing that I can see in the Rockchip QoS >> stuff hooks up to the generic Linux QoS infrastructure. The power >> domains just save and restore the QoS--they don't actually allow >> settting it. > personally, I would just drop that debugfs-dump, as I don't see what we gain > from it :-). Agreed, drop it. >>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> >>> Signed-off-by: Caesar Wang <wxt@rock-chips.com> >>> --- > [...] > >>> + pmu: power-management@ff310000 { >>> + compatible = "rockchip,rk3399-pmu", "syscon", >>> "simple-mfd"; + reg = <0x0 0xff310000 0x0 0x1000>; >>> + >>> + power: power-controller { >>> + status = "okay"; >>> + compatible = "rockchip,rk3399-power-controller"; >>> + #power-domain-cells = <1>; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + pd_vdu { >>> + reg = <RK3399_PD_VDU>; >>> + clocks = <&cru ACLK_VDU>, >>> + <&cru HCLK_VDU>; >>> + pm_qos = <&qos_video_m1_r>, >>> + <&qos_video_m1_w>; >>> + }; >>> + pd_vcodec { >>> + reg = <RK3399_PD_VCODEC>; >>> + clocks = <&cru ACLK_VCODEC>, >>> + <&cru HCLK_VCODEC>; >>> + pm_qos = <&qos_video_m0>; >>> + }; >>> + pd_iep { >>> + reg = <RK3399_PD_IEP>; >>> + clocks = <&cru ACLK_IEP>, >>> + <&cru HCLK_IEP>; >>> + pm_qos = <&qos_iep>; >>> + }; >>> + pd_rga { >>> + reg = <RK3399_PD_RGA>; >>> + clocks = <&cru ACLK_RGA>, >>> + <&cru HCLK_RGA>; >>> + pm_qos = <&qos_rga_r>, >>> + <&qos_rga_w>; >>> + }; >>> + pd_vio { >>> + reg = <RK3399_PD_VIO>; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + pd_isp0 { >>> + reg = <RK3399_PD_ISP0>; >>> + clocks = <&cru ACLK_ISP0>, >>> + <&cru HCLK_ISP0>; >>> + pm_qos = <&qos_isp0_m0>, >>> + <&qos_isp0_m1>; >>> + }; >>> + pd_isp1 { >>> + reg = <RK3399_PD_ISP1>; >>> + clocks = <&cru ACLK_ISP1>, >>> + <&cru HCLK_ISP1>; >>> + pm_qos = <&qos_isp1_m0>, >>> + <&qos_isp1_m1>; >>> + }; >>> + pd_hdcp { >>> + reg = <RK3399_PD_HDCP>; >>> + clocks = <&cru ACLK_HDCP>, >>> + <&cru HCLK_HDCP>, >>> + <&cru PCLK_HDCP>; >>> + pm_qos = <&qos_hdcp>; >>> + }; >>> + pd_vo { >>> + reg = <RK3399_PD_VO>; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + pd_vopb { >>> + reg = <RK3399_PD_VOPB>; >>> + clocks = <&cru >>> ACLK_VOP0>, + >>> <&cru HCLK_VOP0>; + >>> pm_qos = <&qos_vop_big_r>, + >>> <&qos_vop_big_w>; + >>> }; >>> + pd_vopl { >>> + reg = <RK3399_PD_VOPL>; >>> + clocks = <&cru >>> ACLK_VOP1>, + >>> <&cru HCLK_VOP1>; + >>> pm_qos = <&qos_vop_little>; + }; >>> + }; >>> + }; >>> + pd_gpu { >>> + reg = <RK3399_PD_GPU>; >>> + clocks = <&cru ACLK_GPU>; >>> + pm_qos = <&qos_gpu>; >>> + }; >> Again a nitty sort order question. Is there a reason not to make >> things alphabetical? AKA: pd_gpu, pd_iep, pd_rga, ... >> >> ...and inside pd_vio should be alphabetical too? >> >> In the TRM it looks like some of the power domains are grouped >> together (like all the domains under LOGIC or CENTERLOGIC). If >> keeping that grouping makes sense here then you should add a comment >> at the start of each group and sort the groups sanely (and sort within >> each group). Okay, let me see it. Thanks! >> >> It looks like there are also more power domains that you haven't >> listed here (like PD_GMAC, for instance, or PD_CORE_L). Are you >> planning to add those in a followon patch? > that reminds me, nodes with a reg property should have the base address in > the node name as well. Using the constant works nicely, as can be seen on > the rk3288 where we have for example: > > pd_vio@RK3288_PD_VIO Agreed. > > > Heiko > > > -- caesar wang | software engineer | wxt@rock-chip.com _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-07-01 2:12 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-30 1:56 [PATCH] arm64: dts: rockchip: add the power domain node for rk3399 Caesar Wang 2016-06-30 1:56 ` Caesar Wang 2016-06-30 1:56 ` Caesar Wang 2016-06-30 21:49 ` Doug Anderson 2016-06-30 21:49 ` Doug Anderson 2016-06-30 21:49 ` Doug Anderson 2016-06-30 21:57 ` Heiko Stuebner 2016-06-30 21:57 ` Heiko Stuebner 2016-06-30 21:57 ` Heiko Stuebner 2016-06-30 22:32 ` Doug Anderson 2016-06-30 22:32 ` Doug Anderson 2016-06-30 22:32 ` Doug Anderson 2016-06-30 22:50 ` Heiko Stuebner 2016-06-30 22:50 ` Heiko Stuebner 2016-06-30 22:50 ` Heiko Stuebner 2016-07-01 2:11 ` Caesar Wang 2016-07-01 2:11 ` Caesar Wang 2016-07-01 2:11 ` Caesar Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.