All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate
@ 2022-06-07 21:15 ` Brian Norris
  0 siblings, 0 replies; 12+ messages in thread
From: Brian Norris @ 2022-06-07 21:15 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: linux-arm-kernel, linux-rockchip, Hans Verkuil, Nicolas Dufresne,
	Sebastian Fricke, devicetree, linux-kernel, Ezequiel Garcia,
	Brian Norris, stable

Before commit 9998943f6dfc ("media: rkvdec: Stop overclocking the
decoder"), the rkvdec driver was forcing the VDU clock rate. After that
commit, we rely on the default clock rate. That rate works OK on many
boards, with the default PLL settings (CPLL is 800MHz, VDU dividers
leave it at 400MHz); but some boards change PLL settings.

Assign the expected default clock rate explicitly, so that the rate is
consistent, regardless of PLL configuration.

This was particularly broken on RK3399 Gru Scarlet systems, where the
rk3399-gru-scarlet.dtsi assigns PLL_CPLL to 1.6 GHz, and so the VDU
clock ends up at 800 MHz (twice the expected rate), and causes video
artifacts and other issues.

Note: I assign the clock rate in the clock controller instead of the
vdec node, because there are multiple nodes that use this clock, and per
the clock.yaml specification:

  Configuring a clock's parent and rate through the device node that
  consumes the clock can be done only for clocks that have a single
  user. Specifying conflicting parent or rate configuration in multiple
  consumer nodes for a shared clock is forbidden.

  Configuration of common clocks, which affect multiple consumer devices
  can be similarly specified in the clock provider node.

Fixes: 9998943f6dfc ("media: rkvdec: Stop overclocking the decoder")
Cc: <stable@vger.kernel.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
This is a candidate for 5.19 IMO, since commit 9998943f6dfc landed in
5.19-rc1 and is being queued up for -stable as we speak.

 arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 4 +++-
 arch/arm64/boot/dts/rockchip/rk3399.dtsi             | 6 ++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
index 913d845eb51a..1977103a5ef4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
@@ -376,7 +376,8 @@ &cru {
 		<&cru ACLK_VIO>,
 		<&cru ACLK_GIC_PRE>,
 		<&cru PCLK_DDR>,
-		<&cru ACLK_HDCP>;
+		<&cru ACLK_HDCP>,
+		<&cru ACLK_VDU>;
 	assigned-clock-rates =
 		<600000000>, <1600000000>,
 		<1000000000>,
@@ -388,6 +389,7 @@ &cru {
 		<400000000>,
 		<200000000>,
 		<200000000>,
+		<400000000>,
 		<400000000>;
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index fbd0346624e6..9d5b0e8c9cca 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1462,7 +1462,8 @@ cru: clock-controller@ff760000 {
 			<&cru HCLK_PERILP1>, <&cru PCLK_PERILP1>,
 			<&cru ACLK_VIO>, <&cru ACLK_HDCP>,
 			<&cru ACLK_GIC_PRE>,
-			<&cru PCLK_DDR>;
+			<&cru PCLK_DDR>,
+			<&cru ACLK_VDU>;
 		assigned-clock-rates =
 			 <594000000>,  <800000000>,
 			<1000000000>,
@@ -1473,7 +1474,8 @@ cru: clock-controller@ff760000 {
 			 <100000000>,   <50000000>,
 			 <400000000>, <400000000>,
 			 <200000000>,
-			 <200000000>;
+			 <200000000>,
+			 <400000000>;
 	};
 
 	grf: syscon@ff770000 {
-- 
2.36.1.255.ge46751e96f-goog


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate
@ 2022-06-07 21:15 ` Brian Norris
  0 siblings, 0 replies; 12+ messages in thread
From: Brian Norris @ 2022-06-07 21:15 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: linux-arm-kernel, linux-rockchip, Hans Verkuil, Nicolas Dufresne,
	Sebastian Fricke, devicetree, linux-kernel, Ezequiel Garcia,
	Brian Norris, stable

Before commit 9998943f6dfc ("media: rkvdec: Stop overclocking the
decoder"), the rkvdec driver was forcing the VDU clock rate. After that
commit, we rely on the default clock rate. That rate works OK on many
boards, with the default PLL settings (CPLL is 800MHz, VDU dividers
leave it at 400MHz); but some boards change PLL settings.

Assign the expected default clock rate explicitly, so that the rate is
consistent, regardless of PLL configuration.

This was particularly broken on RK3399 Gru Scarlet systems, where the
rk3399-gru-scarlet.dtsi assigns PLL_CPLL to 1.6 GHz, and so the VDU
clock ends up at 800 MHz (twice the expected rate), and causes video
artifacts and other issues.

Note: I assign the clock rate in the clock controller instead of the
vdec node, because there are multiple nodes that use this clock, and per
the clock.yaml specification:

  Configuring a clock's parent and rate through the device node that
  consumes the clock can be done only for clocks that have a single
  user. Specifying conflicting parent or rate configuration in multiple
  consumer nodes for a shared clock is forbidden.

  Configuration of common clocks, which affect multiple consumer devices
  can be similarly specified in the clock provider node.

Fixes: 9998943f6dfc ("media: rkvdec: Stop overclocking the decoder")
Cc: <stable@vger.kernel.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
This is a candidate for 5.19 IMO, since commit 9998943f6dfc landed in
5.19-rc1 and is being queued up for -stable as we speak.

 arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 4 +++-
 arch/arm64/boot/dts/rockchip/rk3399.dtsi             | 6 ++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
index 913d845eb51a..1977103a5ef4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
@@ -376,7 +376,8 @@ &cru {
 		<&cru ACLK_VIO>,
 		<&cru ACLK_GIC_PRE>,
 		<&cru PCLK_DDR>,
-		<&cru ACLK_HDCP>;
+		<&cru ACLK_HDCP>,
+		<&cru ACLK_VDU>;
 	assigned-clock-rates =
 		<600000000>, <1600000000>,
 		<1000000000>,
@@ -388,6 +389,7 @@ &cru {
 		<400000000>,
 		<200000000>,
 		<200000000>,
+		<400000000>,
 		<400000000>;
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index fbd0346624e6..9d5b0e8c9cca 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1462,7 +1462,8 @@ cru: clock-controller@ff760000 {
 			<&cru HCLK_PERILP1>, <&cru PCLK_PERILP1>,
 			<&cru ACLK_VIO>, <&cru ACLK_HDCP>,
 			<&cru ACLK_GIC_PRE>,
-			<&cru PCLK_DDR>;
+			<&cru PCLK_DDR>,
+			<&cru ACLK_VDU>;
 		assigned-clock-rates =
 			 <594000000>,  <800000000>,
 			<1000000000>,
@@ -1473,7 +1474,8 @@ cru: clock-controller@ff760000 {
 			 <100000000>,   <50000000>,
 			 <400000000>, <400000000>,
 			 <200000000>,
-			 <200000000>;
+			 <200000000>,
+			 <400000000>;
 	};
 
 	grf: syscon@ff770000 {
-- 
2.36.1.255.ge46751e96f-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate
@ 2022-06-07 21:15 ` Brian Norris
  0 siblings, 0 replies; 12+ messages in thread
From: Brian Norris @ 2022-06-07 21:15 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: linux-arm-kernel, linux-rockchip, Hans Verkuil, Nicolas Dufresne,
	Sebastian Fricke, devicetree, linux-kernel, Ezequiel Garcia,
	Brian Norris, stable

Before commit 9998943f6dfc ("media: rkvdec: Stop overclocking the
decoder"), the rkvdec driver was forcing the VDU clock rate. After that
commit, we rely on the default clock rate. That rate works OK on many
boards, with the default PLL settings (CPLL is 800MHz, VDU dividers
leave it at 400MHz); but some boards change PLL settings.

Assign the expected default clock rate explicitly, so that the rate is
consistent, regardless of PLL configuration.

This was particularly broken on RK3399 Gru Scarlet systems, where the
rk3399-gru-scarlet.dtsi assigns PLL_CPLL to 1.6 GHz, and so the VDU
clock ends up at 800 MHz (twice the expected rate), and causes video
artifacts and other issues.

Note: I assign the clock rate in the clock controller instead of the
vdec node, because there are multiple nodes that use this clock, and per
the clock.yaml specification:

  Configuring a clock's parent and rate through the device node that
  consumes the clock can be done only for clocks that have a single
  user. Specifying conflicting parent or rate configuration in multiple
  consumer nodes for a shared clock is forbidden.

  Configuration of common clocks, which affect multiple consumer devices
  can be similarly specified in the clock provider node.

Fixes: 9998943f6dfc ("media: rkvdec: Stop overclocking the decoder")
Cc: <stable@vger.kernel.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
This is a candidate for 5.19 IMO, since commit 9998943f6dfc landed in
5.19-rc1 and is being queued up for -stable as we speak.

 arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 4 +++-
 arch/arm64/boot/dts/rockchip/rk3399.dtsi             | 6 ++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
index 913d845eb51a..1977103a5ef4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
@@ -376,7 +376,8 @@ &cru {
 		<&cru ACLK_VIO>,
 		<&cru ACLK_GIC_PRE>,
 		<&cru PCLK_DDR>,
-		<&cru ACLK_HDCP>;
+		<&cru ACLK_HDCP>,
+		<&cru ACLK_VDU>;
 	assigned-clock-rates =
 		<600000000>, <1600000000>,
 		<1000000000>,
@@ -388,6 +389,7 @@ &cru {
 		<400000000>,
 		<200000000>,
 		<200000000>,
+		<400000000>,
 		<400000000>;
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index fbd0346624e6..9d5b0e8c9cca 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1462,7 +1462,8 @@ cru: clock-controller@ff760000 {
 			<&cru HCLK_PERILP1>, <&cru PCLK_PERILP1>,
 			<&cru ACLK_VIO>, <&cru ACLK_HDCP>,
 			<&cru ACLK_GIC_PRE>,
-			<&cru PCLK_DDR>;
+			<&cru PCLK_DDR>,
+			<&cru ACLK_VDU>;
 		assigned-clock-rates =
 			 <594000000>,  <800000000>,
 			<1000000000>,
@@ -1473,7 +1474,8 @@ cru: clock-controller@ff760000 {
 			 <100000000>,   <50000000>,
 			 <400000000>, <400000000>,
 			 <200000000>,
-			 <200000000>;
+			 <200000000>,
+			 <400000000>;
 	};
 
 	grf: syscon@ff770000 {
-- 
2.36.1.255.ge46751e96f-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate
  2022-06-07 21:15 ` Brian Norris
  (?)
@ 2022-06-07 21:20   ` Nicolas Dufresne
  -1 siblings, 0 replies; 12+ messages in thread
From: Nicolas Dufresne @ 2022-06-07 21:20 UTC (permalink / raw)
  To: Brian Norris, Heiko Stuebner
  Cc: linux-arm-kernel, linux-rockchip, Hans Verkuil, Sebastian Fricke,
	devicetree, linux-kernel, Ezequiel Garcia, stable

Le mardi 07 juin 2022 à 14:15 -0700, Brian Norris a écrit :
> Before commit 9998943f6dfc ("media: rkvdec: Stop overclocking the
> decoder"), the rkvdec driver was forcing the VDU clock rate. After that
> commit, we rely on the default clock rate. That rate works OK on many
> boards, with the default PLL settings (CPLL is 800MHz, VDU dividers
> leave it at 400MHz); but some boards change PLL settings.
> 
> Assign the expected default clock rate explicitly, so that the rate is
> consistent, regardless of PLL configuration.
> 
> This was particularly broken on RK3399 Gru Scarlet systems, where the
> rk3399-gru-scarlet.dtsi assigns PLL_CPLL to 1.6 GHz, and so the VDU
> clock ends up at 800 MHz (twice the expected rate), and causes video
> artifacts and other issues.
> 
> Note: I assign the clock rate in the clock controller instead of the
> vdec node, because there are multiple nodes that use this clock, and per
> the clock.yaml specification:
> 
>   Configuring a clock's parent and rate through the device node that
>   consumes the clock can be done only for clocks that have a single
>   user. Specifying conflicting parent or rate configuration in multiple
>   consumer nodes for a shared clock is forbidden.
> 
>   Configuration of common clocks, which affect multiple consumer devices
>   can be similarly specified in the clock provider node.
> 
> Fixes: 9998943f6dfc ("media: rkvdec: Stop overclocking the decoder")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

My only doubt was if you really needed to duplicate that setting into gru-
scarlet.dtsi, but I've simply assumed the answer is yes, and that you already
checked that.

> ---
> This is a candidate for 5.19 IMO, since commit 9998943f6dfc landed in
> 5.19-rc1 and is being queued up for -stable as we speak.
> 
>  arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 4 +++-
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi             | 6 ++++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> index 913d845eb51a..1977103a5ef4 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> @@ -376,7 +376,8 @@ &cru {
>  		<&cru ACLK_VIO>,
>  		<&cru ACLK_GIC_PRE>,
>  		<&cru PCLK_DDR>,
> -		<&cru ACLK_HDCP>;
> +		<&cru ACLK_HDCP>,
> +		<&cru ACLK_VDU>;
>  	assigned-clock-rates =
>  		<600000000>, <1600000000>,
>  		<1000000000>,
> @@ -388,6 +389,7 @@ &cru {
>  		<400000000>,
>  		<200000000>,
>  		<200000000>,
> +		<400000000>,
>  		<400000000>;
>  };
>  
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index fbd0346624e6..9d5b0e8c9cca 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1462,7 +1462,8 @@ cru: clock-controller@ff760000 {
>  			<&cru HCLK_PERILP1>, <&cru PCLK_PERILP1>,
>  			<&cru ACLK_VIO>, <&cru ACLK_HDCP>,
>  			<&cru ACLK_GIC_PRE>,
> -			<&cru PCLK_DDR>;
> +			<&cru PCLK_DDR>,
> +			<&cru ACLK_VDU>;
>  		assigned-clock-rates =
>  			 <594000000>,  <800000000>,
>  			<1000000000>,
> @@ -1473,7 +1474,8 @@ cru: clock-controller@ff760000 {
>  			 <100000000>,   <50000000>,
>  			 <400000000>, <400000000>,
>  			 <200000000>,
> -			 <200000000>;
> +			 <200000000>,
> +			 <400000000>;
>  	};
>  
>  	grf: syscon@ff770000 {


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate
@ 2022-06-07 21:20   ` Nicolas Dufresne
  0 siblings, 0 replies; 12+ messages in thread
From: Nicolas Dufresne @ 2022-06-07 21:20 UTC (permalink / raw)
  To: Brian Norris, Heiko Stuebner
  Cc: linux-arm-kernel, linux-rockchip, Hans Verkuil, Sebastian Fricke,
	devicetree, linux-kernel, Ezequiel Garcia, stable

Le mardi 07 juin 2022 à 14:15 -0700, Brian Norris a écrit :
> Before commit 9998943f6dfc ("media: rkvdec: Stop overclocking the
> decoder"), the rkvdec driver was forcing the VDU clock rate. After that
> commit, we rely on the default clock rate. That rate works OK on many
> boards, with the default PLL settings (CPLL is 800MHz, VDU dividers
> leave it at 400MHz); but some boards change PLL settings.
> 
> Assign the expected default clock rate explicitly, so that the rate is
> consistent, regardless of PLL configuration.
> 
> This was particularly broken on RK3399 Gru Scarlet systems, where the
> rk3399-gru-scarlet.dtsi assigns PLL_CPLL to 1.6 GHz, and so the VDU
> clock ends up at 800 MHz (twice the expected rate), and causes video
> artifacts and other issues.
> 
> Note: I assign the clock rate in the clock controller instead of the
> vdec node, because there are multiple nodes that use this clock, and per
> the clock.yaml specification:
> 
>   Configuring a clock's parent and rate through the device node that
>   consumes the clock can be done only for clocks that have a single
>   user. Specifying conflicting parent or rate configuration in multiple
>   consumer nodes for a shared clock is forbidden.
> 
>   Configuration of common clocks, which affect multiple consumer devices
>   can be similarly specified in the clock provider node.
> 
> Fixes: 9998943f6dfc ("media: rkvdec: Stop overclocking the decoder")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

My only doubt was if you really needed to duplicate that setting into gru-
scarlet.dtsi, but I've simply assumed the answer is yes, and that you already
checked that.

> ---
> This is a candidate for 5.19 IMO, since commit 9998943f6dfc landed in
> 5.19-rc1 and is being queued up for -stable as we speak.
> 
>  arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 4 +++-
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi             | 6 ++++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> index 913d845eb51a..1977103a5ef4 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> @@ -376,7 +376,8 @@ &cru {
>  		<&cru ACLK_VIO>,
>  		<&cru ACLK_GIC_PRE>,
>  		<&cru PCLK_DDR>,
> -		<&cru ACLK_HDCP>;
> +		<&cru ACLK_HDCP>,
> +		<&cru ACLK_VDU>;
>  	assigned-clock-rates =
>  		<600000000>, <1600000000>,
>  		<1000000000>,
> @@ -388,6 +389,7 @@ &cru {
>  		<400000000>,
>  		<200000000>,
>  		<200000000>,
> +		<400000000>,
>  		<400000000>;
>  };
>  
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index fbd0346624e6..9d5b0e8c9cca 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1462,7 +1462,8 @@ cru: clock-controller@ff760000 {
>  			<&cru HCLK_PERILP1>, <&cru PCLK_PERILP1>,
>  			<&cru ACLK_VIO>, <&cru ACLK_HDCP>,
>  			<&cru ACLK_GIC_PRE>,
> -			<&cru PCLK_DDR>;
> +			<&cru PCLK_DDR>,
> +			<&cru ACLK_VDU>;
>  		assigned-clock-rates =
>  			 <594000000>,  <800000000>,
>  			<1000000000>,
> @@ -1473,7 +1474,8 @@ cru: clock-controller@ff760000 {
>  			 <100000000>,   <50000000>,
>  			 <400000000>, <400000000>,
>  			 <200000000>,
> -			 <200000000>;
> +			 <200000000>,
> +			 <400000000>;
>  	};
>  
>  	grf: syscon@ff770000 {


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate
@ 2022-06-07 21:20   ` Nicolas Dufresne
  0 siblings, 0 replies; 12+ messages in thread
From: Nicolas Dufresne @ 2022-06-07 21:20 UTC (permalink / raw)
  To: Brian Norris, Heiko Stuebner
  Cc: linux-arm-kernel, linux-rockchip, Hans Verkuil, Sebastian Fricke,
	devicetree, linux-kernel, Ezequiel Garcia, stable

Le mardi 07 juin 2022 à 14:15 -0700, Brian Norris a écrit :
> Before commit 9998943f6dfc ("media: rkvdec: Stop overclocking the
> decoder"), the rkvdec driver was forcing the VDU clock rate. After that
> commit, we rely on the default clock rate. That rate works OK on many
> boards, with the default PLL settings (CPLL is 800MHz, VDU dividers
> leave it at 400MHz); but some boards change PLL settings.
> 
> Assign the expected default clock rate explicitly, so that the rate is
> consistent, regardless of PLL configuration.
> 
> This was particularly broken on RK3399 Gru Scarlet systems, where the
> rk3399-gru-scarlet.dtsi assigns PLL_CPLL to 1.6 GHz, and so the VDU
> clock ends up at 800 MHz (twice the expected rate), and causes video
> artifacts and other issues.
> 
> Note: I assign the clock rate in the clock controller instead of the
> vdec node, because there are multiple nodes that use this clock, and per
> the clock.yaml specification:
> 
>   Configuring a clock's parent and rate through the device node that
>   consumes the clock can be done only for clocks that have a single
>   user. Specifying conflicting parent or rate configuration in multiple
>   consumer nodes for a shared clock is forbidden.
> 
>   Configuration of common clocks, which affect multiple consumer devices
>   can be similarly specified in the clock provider node.
> 
> Fixes: 9998943f6dfc ("media: rkvdec: Stop overclocking the decoder")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

My only doubt was if you really needed to duplicate that setting into gru-
scarlet.dtsi, but I've simply assumed the answer is yes, and that you already
checked that.

> ---
> This is a candidate for 5.19 IMO, since commit 9998943f6dfc landed in
> 5.19-rc1 and is being queued up for -stable as we speak.
> 
>  arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 4 +++-
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi             | 6 ++++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> index 913d845eb51a..1977103a5ef4 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> @@ -376,7 +376,8 @@ &cru {
>  		<&cru ACLK_VIO>,
>  		<&cru ACLK_GIC_PRE>,
>  		<&cru PCLK_DDR>,
> -		<&cru ACLK_HDCP>;
> +		<&cru ACLK_HDCP>,
> +		<&cru ACLK_VDU>;
>  	assigned-clock-rates =
>  		<600000000>, <1600000000>,
>  		<1000000000>,
> @@ -388,6 +389,7 @@ &cru {
>  		<400000000>,
>  		<200000000>,
>  		<200000000>,
> +		<400000000>,
>  		<400000000>;
>  };
>  
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index fbd0346624e6..9d5b0e8c9cca 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1462,7 +1462,8 @@ cru: clock-controller@ff760000 {
>  			<&cru HCLK_PERILP1>, <&cru PCLK_PERILP1>,
>  			<&cru ACLK_VIO>, <&cru ACLK_HDCP>,
>  			<&cru ACLK_GIC_PRE>,
> -			<&cru PCLK_DDR>;
> +			<&cru PCLK_DDR>,
> +			<&cru ACLK_VDU>;
>  		assigned-clock-rates =
>  			 <594000000>,  <800000000>,
>  			<1000000000>,
> @@ -1473,7 +1474,8 @@ cru: clock-controller@ff760000 {
>  			 <100000000>,   <50000000>,
>  			 <400000000>, <400000000>,
>  			 <200000000>,
> -			 <200000000>;
> +			 <200000000>,
> +			 <400000000>;
>  	};
>  
>  	grf: syscon@ff770000 {


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate
  2022-06-07 21:20   ` Nicolas Dufresne
  (?)
@ 2022-06-07 21:36     ` Brian Norris
  -1 siblings, 0 replies; 12+ messages in thread
From: Brian Norris @ 2022-06-07 21:36 UTC (permalink / raw)
  To: Nicolas Dufresne
  Cc: Heiko Stuebner, linux-arm-kernel, linux-rockchip, Hans Verkuil,
	Sebastian Fricke, devicetree, linux-kernel, Ezequiel Garcia,
	stable

On Tue, Jun 07, 2022 at 05:20:41PM -0400, Nicolas Dufresne wrote:
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Thanks!

> My only doubt was if you really needed to duplicate that setting into gru-
> scarlet.dtsi, but I've simply assumed the answer is yes, and that you already
> checked that.

I didn't explicitly test without modifying gru-scarlet.dtsi, but the
unfortunate nature of these long monolithic
assigned-clocks/assigned-clock-rates properties is that if you want to
add or override one element in the array, you have to repeat (or
override) all of them. And because rk3399-gru-scarlet.dtsi already
overrides some of them, every additional change has to be reflected
there.

That's why it would be much nicer to be able to distribute the
assignments to their various consumer nodes, but as noted in the commit
message (because you mentioned it to me out-of-band ;) ), we can't do
that.

Regards,
Brian

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate
@ 2022-06-07 21:36     ` Brian Norris
  0 siblings, 0 replies; 12+ messages in thread
From: Brian Norris @ 2022-06-07 21:36 UTC (permalink / raw)
  To: Nicolas Dufresne
  Cc: Heiko Stuebner, linux-arm-kernel, linux-rockchip, Hans Verkuil,
	Sebastian Fricke, devicetree, linux-kernel, Ezequiel Garcia,
	stable

On Tue, Jun 07, 2022 at 05:20:41PM -0400, Nicolas Dufresne wrote:
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Thanks!

> My only doubt was if you really needed to duplicate that setting into gru-
> scarlet.dtsi, but I've simply assumed the answer is yes, and that you already
> checked that.

I didn't explicitly test without modifying gru-scarlet.dtsi, but the
unfortunate nature of these long monolithic
assigned-clocks/assigned-clock-rates properties is that if you want to
add or override one element in the array, you have to repeat (or
override) all of them. And because rk3399-gru-scarlet.dtsi already
overrides some of them, every additional change has to be reflected
there.

That's why it would be much nicer to be able to distribute the
assignments to their various consumer nodes, but as noted in the commit
message (because you mentioned it to me out-of-band ;) ), we can't do
that.

Regards,
Brian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate
@ 2022-06-07 21:36     ` Brian Norris
  0 siblings, 0 replies; 12+ messages in thread
From: Brian Norris @ 2022-06-07 21:36 UTC (permalink / raw)
  To: Nicolas Dufresne
  Cc: Heiko Stuebner, linux-arm-kernel, linux-rockchip, Hans Verkuil,
	Sebastian Fricke, devicetree, linux-kernel, Ezequiel Garcia,
	stable

On Tue, Jun 07, 2022 at 05:20:41PM -0400, Nicolas Dufresne wrote:
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Thanks!

> My only doubt was if you really needed to duplicate that setting into gru-
> scarlet.dtsi, but I've simply assumed the answer is yes, and that you already
> checked that.

I didn't explicitly test without modifying gru-scarlet.dtsi, but the
unfortunate nature of these long monolithic
assigned-clocks/assigned-clock-rates properties is that if you want to
add or override one element in the array, you have to repeat (or
override) all of them. And because rk3399-gru-scarlet.dtsi already
overrides some of them, every additional change has to be reflected
there.

That's why it would be much nicer to be able to distribute the
assignments to their various consumer nodes, but as noted in the commit
message (because you mentioned it to me out-of-band ;) ), we can't do
that.

Regards,
Brian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate
  2022-06-07 21:15 ` Brian Norris
  (?)
@ 2022-06-11 15:20   ` Heiko Stuebner
  -1 siblings, 0 replies; 12+ messages in thread
From: Heiko Stuebner @ 2022-06-11 15:20 UTC (permalink / raw)
  To: Brian Norris
  Cc: Heiko Stuebner, Nicolas Dufresne, Sebastian Fricke,
	linux-rockchip, devicetree, linux-kernel, stable,
	linux-arm-kernel, Hans Verkuil, Ezequiel Garcia

On Tue, 7 Jun 2022 14:15:36 -0700, Brian Norris wrote:
> Before commit 9998943f6dfc ("media: rkvdec: Stop overclocking the
> decoder"), the rkvdec driver was forcing the VDU clock rate. After that
> commit, we rely on the default clock rate. That rate works OK on many
> boards, with the default PLL settings (CPLL is 800MHz, VDU dividers
> leave it at 400MHz); but some boards change PLL settings.
> 
> Assign the expected default clock rate explicitly, so that the rate is
> consistent, regardless of PLL configuration.
> 
> [...]

Applied, thanks!

[1/1] arm64: dts: rockchip: Assign RK3399 VDU clock rate
      commit: 2d56af33d4df94d2b76446ffc3e3654c42232f4b

      as fix for 5.19

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate
@ 2022-06-11 15:20   ` Heiko Stuebner
  0 siblings, 0 replies; 12+ messages in thread
From: Heiko Stuebner @ 2022-06-11 15:20 UTC (permalink / raw)
  To: Brian Norris
  Cc: Heiko Stuebner, Nicolas Dufresne, Sebastian Fricke,
	linux-rockchip, devicetree, linux-kernel, stable,
	linux-arm-kernel, Hans Verkuil, Ezequiel Garcia

On Tue, 7 Jun 2022 14:15:36 -0700, Brian Norris wrote:
> Before commit 9998943f6dfc ("media: rkvdec: Stop overclocking the
> decoder"), the rkvdec driver was forcing the VDU clock rate. After that
> commit, we rely on the default clock rate. That rate works OK on many
> boards, with the default PLL settings (CPLL is 800MHz, VDU dividers
> leave it at 400MHz); but some boards change PLL settings.
> 
> Assign the expected default clock rate explicitly, so that the rate is
> consistent, regardless of PLL configuration.
> 
> [...]

Applied, thanks!

[1/1] arm64: dts: rockchip: Assign RK3399 VDU clock rate
      commit: 2d56af33d4df94d2b76446ffc3e3654c42232f4b

      as fix for 5.19

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate
@ 2022-06-11 15:20   ` Heiko Stuebner
  0 siblings, 0 replies; 12+ messages in thread
From: Heiko Stuebner @ 2022-06-11 15:20 UTC (permalink / raw)
  To: Brian Norris
  Cc: Heiko Stuebner, Nicolas Dufresne, Sebastian Fricke,
	linux-rockchip, devicetree, linux-kernel, stable,
	linux-arm-kernel, Hans Verkuil, Ezequiel Garcia

On Tue, 7 Jun 2022 14:15:36 -0700, Brian Norris wrote:
> Before commit 9998943f6dfc ("media: rkvdec: Stop overclocking the
> decoder"), the rkvdec driver was forcing the VDU clock rate. After that
> commit, we rely on the default clock rate. That rate works OK on many
> boards, with the default PLL settings (CPLL is 800MHz, VDU dividers
> leave it at 400MHz); but some boards change PLL settings.
> 
> Assign the expected default clock rate explicitly, so that the rate is
> consistent, regardless of PLL configuration.
> 
> [...]

Applied, thanks!

[1/1] arm64: dts: rockchip: Assign RK3399 VDU clock rate
      commit: 2d56af33d4df94d2b76446ffc3e3654c42232f4b

      as fix for 5.19

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-06-11 15:21 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-07 21:15 [PATCH] arm64: dts: rockchip: Assign RK3399 VDU clock rate Brian Norris
2022-06-07 21:15 ` Brian Norris
2022-06-07 21:15 ` Brian Norris
2022-06-07 21:20 ` Nicolas Dufresne
2022-06-07 21:20   ` Nicolas Dufresne
2022-06-07 21:20   ` Nicolas Dufresne
2022-06-07 21:36   ` Brian Norris
2022-06-07 21:36     ` Brian Norris
2022-06-07 21:36     ` Brian Norris
2022-06-11 15:20 ` Heiko Stuebner
2022-06-11 15:20   ` Heiko Stuebner
2022-06-11 15:20   ` Heiko Stuebner

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.