diff mbox

[RFC,v2,3/3] ARM: dts: exynos5420: add async-bridge clocks to disp1 power domain

Message ID 1423220154-5333-1-git-send-email-a.hajda@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrzej Hajda Feb. 6, 2015, 10:55 a.m. UTC
FIMD and MIXER IPs in disp1 power domain have async-bridges (to GSCALER),
therefore their clocks should be enabled during power domain switch.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
Hi,

This is 2nd version of the patch. After decrypting manual and discussion
with Marek I guess this set of clocks is more apropriate - async-bridges
are present in MIXER and FIMD, so their clocks should be enabled.

The 1st version worked for me due to fact I have forgot to remove
clk_ignore_unused kernel boot option during tests ;)

Regards
Andrzej
---
 arch/arm/boot/dts/exynos5420.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Javier Martinez Canillas Feb. 6, 2015, 11:27 a.m. UTC | #1
Hello Andrzej,

On 02/06/2015 11:55 AM, Andrzej Hajda wrote:
> FIMD and MIXER IPs in disp1 power domain have async-bridges (to GSCALER),
> therefore their clocks should be enabled during power domain switch.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> Hi,
> 
> This is 2nd version of the patch. After decrypting manual and discussion
> with Marek I guess this set of clocks is more apropriate - async-bridges
> are present in MIXER and FIMD, so their clocks should be enabled.
>

I just tested this version on my Exynos5420 Peach Pit and the power domain
failed error does not happen anymore even after enabling and disabling the
HDMI display several times.

Thanks a lot for fixing this since the dependency was not clear to me from
reading the manual.

> The 1st version worked for me due to fact I have forgot to remove
> clk_ignore_unused kernel boot option during tests ;)
>

Yes, that has bitten me too many times as well :)

> Regards
> Andrzej
> ---

Best regards,
Javier
Javier Martinez Canillas March 3, 2015, 8:12 a.m. UTC | #2
Hello Kukjin,

On 02/06/2015 12:27 PM, Javier Martinez Canillas wrote:
> Hello Andrzej,
> 
> On 02/06/2015 11:55 AM, Andrzej Hajda wrote:
>> FIMD and MIXER IPs in disp1 power domain have async-bridges (to GSCALER),
>> therefore their clocks should be enabled during power domain switch.
>> 
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> Hi,
>> 
>> This is 2nd version of the patch. After decrypting manual and discussion
>> with Marek I guess this set of clocks is more apropriate - async-bridges
>> are present in MIXER and FIMD, so their clocks should be enabled.
>>
> 
> I just tested this version on my Exynos5420 Peach Pit and the power domain
> failed error does not happen anymore even after enabling and disabling the
> HDMI display several times.
> 
> Thanks a lot for fixing this since the dependency was not clear to me from
> reading the manual.
> 
>> The 1st version worked for me due to fact I have forgot to remove
>> clk_ignore_unused kernel boot option during tests ;)
>>
> 
> Yes, that has bitten me too many times as well :)
> 
>> Regards
>> Andrzej
>> ---
> 
> Best regards,
> Javier
> 

Could you please pick this patch and patches 1/2 and 2/3 from the series [0]?

These are needed to fix DISP1 power domain issues and HDMI on Exynos542x.

Thanks a lot and best regards,
Javier

[0]: https://lkml.org/lkml/2015/2/5/265
On 06/02/15 11:55, Andrzej Hajda wrote:
> FIMD and MIXER IPs in disp1 power domain have async-bridges (to GSCALER),
> therefore their clocks should be enabled during power domain switch.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5420.dtsi | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index e1fa800..58579f5 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -293,9 +293,11 @@
>  			 <&clock CLK_MOUT_SW_ACLK300>,
>  			 <&clock CLK_MOUT_USER_ACLK300_DISP1>,
>  			 <&clock CLK_MOUT_SW_ACLK400>,
> -			 <&clock CLK_MOUT_USER_ACLK400_DISP1>;
> +			 <&clock CLK_MOUT_USER_ACLK400_DISP1>,
> +			 <&clock CLK_FIMD1>, <&clock CLK_MIXER>;
>  		clock-names = "oscclk", "pclk0", "clk0",
> -			      "pclk1", "clk1", "pclk2", "clk2";
> +			      "pclk1", "clk1", "pclk2", "clk2",
> +			      "asb0", "asb1";
>  	};

In general I don't like those clock/clock-names properties in the power
domain nodes, since the power domains are not really consumers of those
clocks. However these clocks are essential for the exynos power domains
operation. There are more dependencies between the clocks and the power
domains which adding of those properties does not cover. And we'll need
to address those dependencies somehow.
Anyway, the subject patch looks OK to me, given that support for clocks/
clock-names in the exynos power domain device nodes has been merged
for quite long already.
The entire feature has been merged without PM or clk subsystem
maintainer ACK, I don't see a reason not to merge this small addition
of more clocks, especially that it fixes a real bug.

Please feel free to add:
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
diff mbox

Patch

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index e1fa800..58579f5 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -293,9 +293,11 @@ 
 			 <&clock CLK_MOUT_SW_ACLK300>,
 			 <&clock CLK_MOUT_USER_ACLK300_DISP1>,
 			 <&clock CLK_MOUT_SW_ACLK400>,
-			 <&clock CLK_MOUT_USER_ACLK400_DISP1>;
+			 <&clock CLK_MOUT_USER_ACLK400_DISP1>,
+			 <&clock CLK_FIMD1>, <&clock CLK_MIXER>;
 		clock-names = "oscclk", "pclk0", "clk0",
-			      "pclk1", "clk1", "pclk2", "clk2";
+			      "pclk1", "clk1", "pclk2", "clk2",
+			      "asb0", "asb1";
 	};
 
 	pinctrl_0: pinctrl@13400000 {