[PATCH 1/1] ARM: dts: don't make DP a consumer of DISP1 on Exynos5250

[PATCH 1/1] ARM: dts: don't make DP a consumer of DISP1 on Exynos5250

[Javier Martinez Canillas]

By making the DP controller a consumer of DISP1, the PD is powered
off when the exynos-dp probe is deferred and powered on again when
the exynos-drm driver is probed.

But this causes the exynos-dp driver failing to obtain the stream
clock since the FIMD has been powered off with the DISP1 PD:

exynos-dp 145b0000.dp-controller: Timeout of video streamclk ok
exynos-dp 145b0000.dp-controller: unable to config video

The Exynos5250 documentation doesn't mention that the Display Port
Transmitter module is included in the DISP1 PD so the device should
not have a reference to this Power Domain.

This patch fixes video display on an Exynos5250 Snow Chromebook.

Fixes: 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250")
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Hello Andrzej and Marek,

I need this patch to have display working on an Snow Chromebook with
next20150312. I remember testing the mentioned patch that introduced
the regression when it was posted and I did not find any issues with
it. Do you know what could had caused this behavior change?

According to the Exynos5250 documentation I've access to, the modules
that are included in the DISP1 power domain are LCD controller (FIMD),
MIE1 VP, MIXER, TV Encoder and HDMI so I think this patch is correct
since it not only solves the regression but also models the HW better.

Please let me know if you think it should be fixed in a different way.

Best regards,
Javier

 arch/arm/boot/dts/exynos5250.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index adbde1adad95..ffb505171216 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -775,7 +775,6 @@
 	};
 
 	dp: dp-controller@145B0000 {
-		power-domains = <&pd_disp1>;
 		clocks = <&clock CLK_DP>;
 		clock-names = "dp";
 		phys = <&dp_phy>;
-- 
2.1.4

Re: [PATCH 1/1] ARM: dts: don't make DP a consumer of DISP1 on Exynos5250

[Andreas Färber]

Am 14.03.2015 um 09:11 schrieb Javier Martinez Canillas:
> By making the DP controller a consumer of DISP1, the PD is powered
> off when the exynos-dp probe is deferred and powered on again when
> the exynos-drm driver is probed.
> 
> But this causes the exynos-dp driver failing to obtain the stream
> clock since the FIMD has been powered off with the DISP1 PD:
> 
> exynos-dp 145b0000.dp-controller: Timeout of video streamclk ok
> exynos-dp 145b0000.dp-controller: unable to config video
> 
> The Exynos5250 documentation doesn't mention that the Display Port
> Transmitter module is included in the DISP1 PD so the device should
> not have a reference to this Power Domain.
> 
> This patch fixes video display on an Exynos5250 Snow Chromebook.
> 
> Fixes: 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250")
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
> 
> Hello Andrzej and Marek,
> 
> I need this patch to have display working on an Snow Chromebook

Tested-by: Andreas Färber <afaerber@suse.de>

This fixes the display on the Spring Chromebook as well!

Thanks,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [PATCH 1/1] ARM: dts: don't make DP a consumer of DISP1 on Exynos5250

[Javier Martinez Canillas]

Hello,

On 03/16/2015 01:15 PM, Andreas Färber wrote:
> Am 14.03.2015 um 09:11 schrieb Javier Martinez Canillas:
>> By making the DP controller a consumer of DISP1, the PD is powered
>> off when the exynos-dp probe is deferred and powered on again when
>> the exynos-drm driver is probed.
>> 
>> But this causes the exynos-dp driver failing to obtain the stream
>> clock since the FIMD has been powered off with the DISP1 PD:
>> 
>> exynos-dp 145b0000.dp-controller: Timeout of video streamclk ok
>> exynos-dp 145b0000.dp-controller: unable to config video
>> 
>> The Exynos5250 documentation doesn't mention that the Display Port
>> Transmitter module is included in the DISP1 PD so the device should
>> not have a reference to this Power Domain.
>> 
>> This patch fixes video display on an Exynos5250 Snow Chromebook.
>> 
>> Fixes: 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250")
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> ---
>> 
>> Hello Andrzej and Marek,
>> 
>> I need this patch to have display working on an Snow Chromebook
> 
> Tested-by: Andreas Färber <afaerber@suse.de>
> 
> This fixes the display on the Spring Chromebook as well!
>

Thanks for testing Andreas but this patch seems to only solve a symptom
rather than the root cause.

Since the error happens again when disabling and enabling the display:

# echo 1 > /sys/devices/platform/exynos-drm/graphics/fb0/blank
disp1-power-domain: Power-off latency exceeded, new value 225333 ns
# echo 0 > /sys/devices/platform/exynos-drm/graphics/fb0/blank
exynos-dp 145b0000.dp-controller: EDID data does not include any extensions.
exynos-dp 145b0000.dp-controller: EDID Read success!
exynos-dp 145b0000.dp-controller: Link Training Clock Recovery success
exynos-dp 145b0000.dp-controller: Link Training success!
exynos-dp 145b0000.dp-controller: Timeout of video streamclk ok
exynos-dp 145b0000.dp-controller: unable to config video

So what happens is that if the DISP1 power domain is powered off and
then powered on again, the display fails to be enabled.

Making the dp controller a consumer of the DISP1 pd only made this to
trigger on boot since the driver probe was deferred and the DISP1 pd
was turned off and on again on exynos-drm probe.

It seems that the kernel is not enabling everything that is needed for
the power domain and it is working on boot just because the bootloader
initialized everything properly.

This is similar to the problem we had in Exynos5420 and that was fixed
by Andrzej in the series "Fix power domains handling on exynos542x" [0].
So probably a similar solution is needed.

Andrzej,

I looked at the Exynos5250 manual and I didn't find anything obvious
that is missing in the DISP1 pd dev node but the asynchronous bridges
clocks needed on Exynos5420 was also not well documented so I don't
know if I'm missing something.

Do you know what could be missing here? Otherwise I think your patch
to add the DISP1 pd in the DT should be reverted to have display
working again on Exynos5250 boards.
 
> Thanks,
> Andreas
> 

Best regards,
Javier

[0]: https://lkml.org/lkml/2015/3/12/387

Re: [PATCH 1/1] ARM: dts: don't make DP a consumer of DISP1 on Exynos5250

[Javier Martinez Canillas]

Hello,

On 03/16/2015 03:16 PM, Javier Martinez Canillas wrote:
>> 
>> Tested-by: Andreas Färber <afaerber@suse.de>
>> 
>> This fixes the display on the Spring Chromebook as well!
>>
> 
> Thanks for testing Andreas but this patch seems to only solve a symptom
> rather than the root cause.
> 
> Since the error happens again when disabling and enabling the display:
> 
> # echo 1 > /sys/devices/platform/exynos-drm/graphics/fb0/blank
> disp1-power-domain: Power-off latency exceeded, new value 225333 ns
> # echo 0 > /sys/devices/platform/exynos-drm/graphics/fb0/blank
> exynos-dp 145b0000.dp-controller: EDID data does not include any extensions.
> exynos-dp 145b0000.dp-controller: EDID Read success!
> exynos-dp 145b0000.dp-controller: Link Training Clock Recovery success
> exynos-dp 145b0000.dp-controller: Link Training success!
> exynos-dp 145b0000.dp-controller: Timeout of video streamclk ok
> exynos-dp 145b0000.dp-controller: unable to config video
> 
> So what happens is that if the DISP1 power domain is powered off and
> then powered on again, the display fails to be enabled.
> 
> Making the dp controller a consumer of the DISP1 pd only made this to
> trigger on boot since the driver probe was deferred and the DISP1 pd
> was turned off and on again on exynos-drm probe.
> 
> It seems that the kernel is not enabling everything that is needed for
> the power domain and it is working on boot just because the bootloader
> initialized everything properly.
> 
> This is similar to the problem we had in Exynos5420 and that was fixed
> by Andrzej in the series "Fix power domains handling on exynos542x" [0].
> So probably a similar solution is needed.
> 

Actually, is more similar to the problem we had when trying to get
HDMI working on Exynos5420 that was solved by the following commits:

885601002998 clk: exynos5420: Add IDs for clocks used in DISP1 power domain
ea08de16eb1b ARM: dts: Add DISP1 power domain for exynos5420

> Andrzej,
> 
> I looked at the Exynos5250 manual and I didn't find anything obvious
> that is missing in the DISP1 pd dev node but the asynchronous bridges
> clocks needed on Exynos5420 was also not well documented so I don't
> know if I'm missing something.
> 
> Do you know what could be missing here? Otherwise I think your patch
> to add the DISP1 pd in the DT should be reverted to have display
> working again on Exynos5250 boards.
>

So my guess is that is missing the attached devices' parent and input clocks
to allow the Exynos PD driver to re-parent the devices input clocks when the
domain is powered off and on.

The Exynos5250 documentation is not as clear as the Exynos5420 on that regard
and also the clocks in the clk-exynos5250.c driver don't match exactly the
names used in the Exynos5250 manual I've access to. So is not clear to me what
are the needed clocks.

Best regards,
Javier

Re: [PATCH 1/1] ARM: dts: don't make DP a consumer of DISP1 on Exynos5250

[Javier Martinez Canillas]

Hello,

On 03/16/2015 03:16 PM, Javier Martinez Canillas wrote:
> Andrzej,
> 
> I looked at the Exynos5250 manual and I didn't find anything obvious
> that is missing in the DISP1 pd dev node but the asynchronous bridges
> clocks needed on Exynos5420 was also not well documented so I don't
> know if I'm missing something.
> 
> Do you know what could be missing here? Otherwise I think your patch
> to add the DISP1 pd in the DT should be reverted to have display
> working again on Exynos5250 boards.
> 

I wasn't able to find what's missing in the DISP1 pd definition so posted
a revert [0] to "ARM: dts: add display power domain for exynos5250".

Kukjin,

Please ignore $subject and pick [0] instead.

This will avoid Snow and Spring display to stop working and later the PD
can be added again once we know what was missing there.

Best regards,
Javier

[0]: https://lkml.org/lkml/2015/3/23/224