linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: ti: k3-am69-sk: remove assigned-clock-parents for unused VP
@ 2023-12-21 11:30 Jayesh Choudhary
  2023-12-21 12:28 ` Nishanth Menon
  2024-01-03 13:05 ` Tomi Valkeinen
  0 siblings, 2 replies; 4+ messages in thread
From: Jayesh Choudhary @ 2023-12-21 11:30 UTC (permalink / raw)
  To: nm, vigneshr
  Cc: kristo, robh+dt, krzysztof.kozlowski+dt, conor+dt, a-bhatia1,
	rogerq, sabiya.d, u-kumar1, devicetree, linux-kernel,
	linux-arm-kernel, j-choudhary

VP2 and VP3 are unused video ports and VP3 share the same parent
clock as VP1 causing issue with pixel clock setting for HDMI (VP1).
So remove the parent clocks for unused VPs.

Fixes: 6f8605fd7d11 ("arm64: dts: ti: k3-am69-sk: Add DP and HDMI support")
Reported-by: Nishanth Menon <nm@ti.com>
Closes: https://storage.kernelci.org/mainline/master/v6.7-rc6/arm64/defconfig/gcc-10/lab-ti/baseline-nfs-am69_sk-fs.txt
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---

Local testing log for HDMI on AM69-SK:
<https://gist.github.com/Jayesh2000/517395cd85eb28d65b8ee4568cefb809>

 arch/arm64/boot/dts/ti/k3-am69-sk.dts | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am69-sk.dts b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
index 8da591579868..370980eb59b0 100644
--- a/arch/arm64/boot/dts/ti/k3-am69-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
@@ -918,13 +918,9 @@ &dss {
 	pinctrl-names = "default";
 	pinctrl-0 = <&dss_vout0_pins_default>;
 	assigned-clocks = <&k3_clks 218 2>,
-			  <&k3_clks 218 5>,
-			  <&k3_clks 218 14>,
-			  <&k3_clks 218 18>;
+			  <&k3_clks 218 5>;
 	assigned-clock-parents = <&k3_clks 218 3>,
-				 <&k3_clks 218 7>,
-				 <&k3_clks 218 16>,
-				 <&k3_clks 218 22>;
+				 <&k3_clks 218 7>;
 };
 
 &serdes_wiz4 {
-- 
2.25.1


_______________________________________________
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] 4+ messages in thread

* Re: [PATCH] arm64: dts: ti: k3-am69-sk: remove assigned-clock-parents for unused VP
  2023-12-21 11:30 [PATCH] arm64: dts: ti: k3-am69-sk: remove assigned-clock-parents for unused VP Jayesh Choudhary
@ 2023-12-21 12:28 ` Nishanth Menon
  2024-01-03 13:05 ` Tomi Valkeinen
  1 sibling, 0 replies; 4+ messages in thread
From: Nishanth Menon @ 2023-12-21 12:28 UTC (permalink / raw)
  To: Jayesh Choudhary
  Cc: vigneshr, kristo, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	a-bhatia1, rogerq, sabiya.d, u-kumar1, devicetree, linux-kernel,
	linux-arm-kernel

On 17:00-20231221, Jayesh Choudhary wrote:
> VP2 and VP3 are unused video ports and VP3 share the same parent
> clock as VP1 causing issue with pixel clock setting for HDMI (VP1).
> So remove the parent clocks for unused VPs.
> 
> Fixes: 6f8605fd7d11 ("arm64: dts: ti: k3-am69-sk: Add DP and HDMI support")
> Reported-by: Nishanth Menon <nm@ti.com>
> Closes: https://storage.kernelci.org/mainline/master/v6.7-rc6/arm64/defconfig/gcc-10/lab-ti/baseline-nfs-am69_sk-fs.txt
> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> ---
> 
> Local testing log for HDMI on AM69-SK:
> <https://gist.github.com/Jayesh2000/517395cd85eb28d65b8ee4568cefb809>

Has this been always failing or just something introduced in rc6? I know
I noticed this in rc6.. so wondering..

> 
>  arch/arm64/boot/dts/ti/k3-am69-sk.dts | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am69-sk.dts b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
> index 8da591579868..370980eb59b0 100644
> --- a/arch/arm64/boot/dts/ti/k3-am69-sk.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
> @@ -918,13 +918,9 @@ &dss {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&dss_vout0_pins_default>;
>  	assigned-clocks = <&k3_clks 218 2>,
> -			  <&k3_clks 218 5>,
> -			  <&k3_clks 218 14>,
> -			  <&k3_clks 218 18>;
> +			  <&k3_clks 218 5>;
>  	assigned-clock-parents = <&k3_clks 218 3>,
> -				 <&k3_clks 218 7>,
> -				 <&k3_clks 218 16>,
> -				 <&k3_clks 218 22>;
> +				 <&k3_clks 218 7>;
>  };
>  
>  &serdes_wiz4 {
> -- 
> 2.25.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

_______________________________________________
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] 4+ messages in thread

* Re: [PATCH] arm64: dts: ti: k3-am69-sk: remove assigned-clock-parents for unused VP
  2023-12-21 11:30 [PATCH] arm64: dts: ti: k3-am69-sk: remove assigned-clock-parents for unused VP Jayesh Choudhary
  2023-12-21 12:28 ` Nishanth Menon
@ 2024-01-03 13:05 ` Tomi Valkeinen
  2024-02-01 10:40   ` Jayesh Choudhary
  1 sibling, 1 reply; 4+ messages in thread
From: Tomi Valkeinen @ 2024-01-03 13:05 UTC (permalink / raw)
  To: Jayesh Choudhary, nm, vigneshr
  Cc: kristo, robh+dt, krzysztof.kozlowski+dt, conor+dt, a-bhatia1,
	rogerq, sabiya.d, u-kumar1, devicetree, linux-kernel,
	linux-arm-kernel

Hi,

On 21/12/2023 13:30, Jayesh Choudhary wrote:
> VP2 and VP3 are unused video ports and VP3 share the same parent
> clock as VP1 causing issue with pixel clock setting for HDMI (VP1).
> So remove the parent clocks for unused VPs.
> 
> Fixes: 6f8605fd7d11 ("arm64: dts: ti: k3-am69-sk: Add DP and HDMI support")
> Reported-by: Nishanth Menon <nm@ti.com>
> Closes: https://storage.kernelci.org/mainline/master/v6.7-rc6/arm64/defconfig/gcc-10/lab-ti/baseline-nfs-am69_sk-fs.txt
> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> ---
> 
> Local testing log for HDMI on AM69-SK:
> <https://gist.github.com/Jayesh2000/517395cd85eb28d65b8ee4568cefb809>
> 
>   arch/arm64/boot/dts/ti/k3-am69-sk.dts | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am69-sk.dts b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
> index 8da591579868..370980eb59b0 100644
> --- a/arch/arm64/boot/dts/ti/k3-am69-sk.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
> @@ -918,13 +918,9 @@ &dss {
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&dss_vout0_pins_default>;
>   	assigned-clocks = <&k3_clks 218 2>,
> -			  <&k3_clks 218 5>,
> -			  <&k3_clks 218 14>,
> -			  <&k3_clks 218 18>;
> +			  <&k3_clks 218 5>;
>   	assigned-clock-parents = <&k3_clks 218 3>,
> -				 <&k3_clks 218 7>,
> -				 <&k3_clks 218 16>,
> -				 <&k3_clks 218 22>;
> +				 <&k3_clks 218 7>;
>   };
>   
>   &serdes_wiz4 {

The SK has two outputs, using VP0 and VP1, so the above kind of makes 
sense. Then again, setting up 4 clocks here really shouldn't break the 
SK, should it? The AM69 has 4 available VPs. How does one configure the 
clocks for a board that uses 4 VPs, or possibly a different selection of 
VPs?

I think the patch desc should explain why this doesn't work. Afaik, the 
dts is not wrong as such, but there's an underlying issue that breaks 
the clocking if all four clocks are set up here.

So, with the desc updated, as this fixes an issue and is not wrong:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

But I also feel this is dodging a firmware (?) issue.

  Tomi


_______________________________________________
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] 4+ messages in thread

* Re: [PATCH] arm64: dts: ti: k3-am69-sk: remove assigned-clock-parents for unused VP
  2024-01-03 13:05 ` Tomi Valkeinen
@ 2024-02-01 10:40   ` Jayesh Choudhary
  0 siblings, 0 replies; 4+ messages in thread
From: Jayesh Choudhary @ 2024-02-01 10:40 UTC (permalink / raw)
  To: Tomi Valkeinen, nm, vigneshr
  Cc: kristo, robh+dt, krzysztof.kozlowski+dt, conor+dt, a-bhatia1,
	rogerq, sabiya.d, u-kumar1, devicetree, linux-kernel,
	linux-arm-kernel

Hello Tomi,

On 03/01/24 18:35, Tomi Valkeinen wrote:
> Hi,
> 
> On 21/12/2023 13:30, Jayesh Choudhary wrote:
>> VP2 and VP3 are unused video ports and VP3 share the same parent
>> clock as VP1 causing issue with pixel clock setting for HDMI (VP1).
>> So remove the parent clocks for unused VPs.
>>
>> Fixes: 6f8605fd7d11 ("arm64: dts: ti: k3-am69-sk: Add DP and HDMI 
>> support")
>> Reported-by: Nishanth Menon <nm@ti.com>
>> Closes: 
>> https://storage.kernelci.org/mainline/master/v6.7-rc6/arm64/defconfig/gcc-10/lab-ti/baseline-nfs-am69_sk-fs.txt
>> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
>> ---
>>
>> Local testing log for HDMI on AM69-SK:
>> <https://gist.github.com/Jayesh2000/517395cd85eb28d65b8ee4568cefb809>
>>
>>   arch/arm64/boot/dts/ti/k3-am69-sk.dts | 8 ++------
>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am69-sk.dts 
>> b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
>> index 8da591579868..370980eb59b0 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am69-sk.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
>> @@ -918,13 +918,9 @@ &dss {
>>       pinctrl-names = "default";
>>       pinctrl-0 = <&dss_vout0_pins_default>;
>>       assigned-clocks = <&k3_clks 218 2>,
>> -              <&k3_clks 218 5>,
>> -              <&k3_clks 218 14>,
>> -              <&k3_clks 218 18>;
>> +              <&k3_clks 218 5>;
>>       assigned-clock-parents = <&k3_clks 218 3>,
>> -                 <&k3_clks 218 7>,
>> -                 <&k3_clks 218 16>,
>> -                 <&k3_clks 218 22>;
>> +                 <&k3_clks 218 7>;
>>   };
>>   &serdes_wiz4 {
> 
> The SK has two outputs, using VP0 and VP1, so the above kind of makes 
> sense. Then again, setting up 4 clocks here really shouldn't break the 
> SK, should it? The AM69 has 4 available VPs. How does one configure the 
> clocks for a board that uses 4 VPs, or possibly a different selection of 
> VPs?

> 
> I think the patch desc should explain why this doesn't work. Afaik, the 
> dts is not wrong as such, but there's an underlying issue that breaks 
> the clocking if all four clocks are set up here.
> 

I discussed this with firmware team, there is an issue with sibling
child clocks. If parent clock is shared, DM cannot set its rate.
The determine_rate and set_rate query are behaving unexpectedly in
this case. determine_rate is returning 0 and set_rate is setting it
to 1.8G even when VP can support max 600M.

Jayesh

> So, with the desc updated, as this fixes an issue and is not wrong:
> 
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> 
> But I also feel this is dodging a firmware (?) issue.
> 
>   Tomi
> 

_______________________________________________
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] 4+ messages in thread

end of thread, other threads:[~2024-02-01 10:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-21 11:30 [PATCH] arm64: dts: ti: k3-am69-sk: remove assigned-clock-parents for unused VP Jayesh Choudhary
2023-12-21 12:28 ` Nishanth Menon
2024-01-03 13:05 ` Tomi Valkeinen
2024-02-01 10:40   ` Jayesh Choudhary

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).