[4/7] Exynos: add support for 'domain-always-on' property

Message ID 1404202211-15788-5-git-send-email-m.szyprowski@samsung.com
State New
Headers show

Commit Message

Marek Szyprowski July 1, 2014, 8:10 a.m.
This patch adds support for domain-always-on property to Exynos power
domain driver. Domains with this property as always kept enabled.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 Documentation/devicetree/bindings/arm/exynos/power_domain.txt | 2 ++
 arch/arm/mach-exynos/pm_domains.c                             | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

Comments

Tobias Jakobi July 1, 2014, 8:52 a.m. | #1
Hello Marek,

I think you had a similar patch in the tizen tree, but according to
Tomasz Figa, it was considered a hack. I don't quite see how this is
different.

Also, if I have been following the discussion correctly, then the
powerdomain issue essentially is about the question which SoC block
needs the LCD0 domain and how the proper power on/off sequences should
look like.

At least the mixer power issue, which I pointed out some time ago, seems
to be deal with now:
https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?h=exynos-drm-next&id=381be025ac1a6dc8efebdf146ced0d4a6007f77b

With best wishes,
Tobias




Marek Szyprowski wrote:
> This patch adds support for domain-always-on property to Exynos power
> domain driver. Domains with this property as always kept enabled.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  Documentation/devicetree/bindings/arm/exynos/power_domain.txt | 2 ++
>  arch/arm/mach-exynos/pm_domains.c                             | 6 +++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
> index 5216b419016a..b25d9b1ce471 100644
> --- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
> +++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
> @@ -8,6 +8,8 @@ Required Properties:
>      * samsung,exynos4210-pd - for exynos4210 type power domain.
>  - reg: physical base address of the controller and length of memory mapped
>      region.
> +Optional properties:
> +- domain-always-on:	keeps the domain always enabled
>  
>  Node of a device using power domains must have a samsung,power-domain property
>  defined with a phandle to respective power domain.
> diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-exynos/pm_domains.c
> index fe6570ebbdde..279b008de02f 100644
> --- a/arch/arm/mach-exynos/pm_domains.c
> +++ b/arch/arm/mach-exynos/pm_domains.c
> @@ -151,6 +151,7 @@ static __init int exynos4_pm_init_power_domain(void)
>  	struct device_node *np;
>  
>  	for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
> +		struct dev_power_governor *gov = NULL;
>  		struct exynos_pm_domain *pd;
>  		int on;
>  
> @@ -163,6 +164,9 @@ static __init int exynos4_pm_init_power_domain(void)
>  			return -ENOMEM;
>  		}
>  
> +		if (of_property_read_bool(np, "domain-always-on"))
> +			gov = &pm_domain_always_on_gov;
> +
>  		pd->pd.name = kstrdup(np->name, GFP_KERNEL);
>  		pd->name = pd->pd.name;
>  		pd->base = of_iomap(np, 0);
> @@ -174,7 +178,7 @@ static __init int exynos4_pm_init_power_domain(void)
>  
>  		on = __raw_readl(pd->base + 0x4) & S5P_INT_LOCAL_PWR_EN;
>  
> -		pm_genpd_init(&pd->pd, NULL, !on);
> +		pm_genpd_init(&pd->pd, gov, !on);
>  	}
>  
>  	bus_register_notifier(&platform_bus_type, &platform_nb);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Szyprowski July 1, 2014, 9:08 a.m. | #2
Hello,

On 2014-07-01 10:52, Tobias Jakobi wrote:
> Hello Marek,
>
> I think you had a similar patch in the tizen tree, but according to
> Tomasz Figa, it was considered a hack. I don't quite see how this is
> different.
>
> Also, if I have been following the discussion correctly, then the
> powerdomain issue essentially is about the question which SoC block
> needs the LCD0 domain and how the proper power on/off sequences should
> look like.
>
> At least the mixer power issue, which I pointed out some time ago, seems
> to be deal with now:
> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?h=exynos-drm-next&id=381be025ac1a6dc8efebdf146ced0d4a6007f77b

Well, that patch solves power on/off sequence issue with mixer and hdmi,
but it didn't solve the issue with additional managing of power domain
on/off. You can check that if you remove always on property, system will
freeze when hdmi cable is connected for the second time. I've investigated
it for some time, but right now I didn't find any 100% reliable solution
other than keeping the power domain enabled all the time. At least for
now, this patch lets you use HDMI without any stability issues.

I've only found that there are still at least 2 issues with power domains.
One is Mixer/Video Processor dependency on LCD0 domain, second is the proper
power on/off sequence of HDMI/Mixer and TV domain. Forcing both domains to
'always on' workarounds both issues for now. Right now I have no better
idea.

Later, once the proper sequence is found we can remove those properties
from Odroid DTS.

Best regards
Tobias Jakobi July 1, 2014, 5:54 p.m. | #3
Hi,


Marek Szyprowski wrote:
> Hello,
> 
> On 2014-07-01 10:52, Tobias Jakobi wrote:
>> Hello Marek,
>>
>> I think you had a similar patch in the tizen tree, but according to
>> Tomasz Figa, it was considered a hack. I don't quite see how this is
>> different.
>>
>> Also, if I have been following the discussion correctly, then the
>> powerdomain issue essentially is about the question which SoC block
>> needs the LCD0 domain and how the proper power on/off sequences should
>> look like.
>>
>> At least the mixer power issue, which I pointed out some time ago, seems
>> to be deal with now:
>> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?h=exynos-drm-next&id=381be025ac1a6dc8efebdf146ced0d4a6007f77b
>>
> 
> Well, that patch solves power on/off sequence issue with mixer and hdmi,
> but it didn't solve the issue with additional managing of power domain
> on/off. You can check that if you remove always on property, system will
> freeze when hdmi cable is connected for the second time. I've investigated
> it for some time, but right now I didn't find any 100% reliable solution
> other than keeping the power domain enabled all the time. At least for
> now, this patch lets you use HDMI without any stability issues.
Hmm, I have also applied a similar hack to force TV and LCD0 pd on on my
system, but I didn't experience this behaviour (cable reconnecting).
Guess I have to recheck with some more recent tree to make sure.


> I've only found that there are still at least 2 issues with power domains.
> One is Mixer/Video Processor dependency on LCD0 domain, second is the
> proper
> power on/off sequence of HDMI/Mixer and TV domain. Forcing both domains to
> 'always on' workarounds both issues for now. Right now I have no better
> idea.
I had the impression that the patch from above, plus this one
https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?h=exynos-drm-next&id=245f98f269714c08dc6d66d021d166cf36059bc4
were supposed to fix these issues. I haven't tested them yet, because of
lack of time, but Rahul's patch appears to fix mixer poweroff, and
Inki's patch the sequencing (VP, Mixer and last but not least, the HDMI).


> Later, once the proper sequence is found we can remove those properties
> from Odroid DTS.
I dunno, because you would have to support the property to the end of
time then, even though it's not used anymore. Isn't that the kind of
thing that shouldn't end up in DT bindings specs?



> 
> Best regards


With best wishes,
Tobias
Joonyoung Shim July 2, 2014, 7:28 a.m. | #4
Hi Marek,

On 07/01/2014 06:08 PM, Marek Szyprowski wrote:
> Hello,
>
> On 2014-07-01 10:52, Tobias Jakobi wrote:
>> Hello Marek,
>>
>> I think you had a similar patch in the tizen tree, but according to
>> Tomasz Figa, it was considered a hack. I don't quite see how this is
>> different.
>>
>> Also, if I have been following the discussion correctly, then the
>> powerdomain issue essentially is about the question which SoC block
>> needs the LCD0 domain and how the proper power on/off sequences should
>> look like.
>>
>> At least the mixer power issue, which I pointed out some time ago, seems
>> to be deal with now:
>> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?h=exynos-drm-next&id=381be025ac1a6dc8efebdf146ced0d4a6007f77b 
>>
>
> Well, that patch solves power on/off sequence issue with mixer and hdmi,
> but it didn't solve the issue with additional managing of power domain
> on/off. You can check that if you remove always on property, system will
> freeze when hdmi cable is connected for the second time. I've 
> investigated
> it for some time, but right now I didn't find any 100% reliable solution
> other than keeping the power domain enabled all the time. At least for
> now, this patch lets you use HDMI without any stability issues.
>
> I've only found that there are still at least 2 issues with power 
> domains.
> One is Mixer/Video Processor dependency on LCD0 domain, second is the 
> proper

To express dependency of LCD0 domain and TV domain, how about make
master and slave hierarchy between the pm domains using
pm_genpd_add_subdomain?

> power on/off sequence of HDMI/Mixer and TV domain. Forcing both 
> domains to
> 'always on' workarounds both issues for now. Right now I have no better
> idea.
>
> Later, once the proper sequence is found we can remove those properties
> from Odroid DTS.
>
> Best regards

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Szyprowski July 2, 2014, 7:56 a.m. | #5
Hello,

On 2014-07-01 19:54, Tobias Jakobi wrote:
> Marek Szyprowski wrote:
>> On 2014-07-01 10:52, Tobias Jakobi wrote:
>>> I think you had a similar patch in the tizen tree, but according to
>>> Tomasz Figa, it was considered a hack. I don't quite see how this is
>>> different.
>>>
>>> Also, if I have been following the discussion correctly, then the
>>> powerdomain issue essentially is about the question which SoC block
>>> needs the LCD0 domain and how the proper power on/off sequences should
>>> look like.
>>>
>>> At least the mixer power issue, which I pointed out some time ago, seems
>>> to be deal with now:
>>> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?h=exynos-drm-next&id=381be025ac1a6dc8efebdf146ced0d4a6007f77b
>> Well, that patch solves power on/off sequence issue with mixer and hdmi,
>> but it didn't solve the issue with additional managing of power domain
>> on/off. You can check that if you remove always on property, system will
>> freeze when hdmi cable is connected for the second time. I've investigated
>> it for some time, but right now I didn't find any 100% reliable solution
>> other than keeping the power domain enabled all the time. At least for
>> now, this patch lets you use HDMI without any stability issues.
> Hmm, I have also applied a similar hack to force TV and LCD0 pd on on my
> system, but I didn't experience this behaviour (cable reconnecting).
> Guess I have to recheck with some more recent tree to make sure.
>
>
>> I've only found that there are still at least 2 issues with power domains.
>> One is Mixer/Video Processor dependency on LCD0 domain, second is the
>> proper
>> power on/off sequence of HDMI/Mixer and TV domain. Forcing both domains to
>> 'always on' workarounds both issues for now. Right now I have no better
>> idea.
> I had the impression that the patch from above, plus this one
> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?h=exynos-drm-next&id=245f98f269714c08dc6d66d021d166cf36059bc4
> were supposed to fix these issues. I haven't tested them yet, because of
> lack of time, but Rahul's patch appears to fix mixer poweroff, and
> Inki's patch the sequencing (VP, Mixer and last but not least, the HDMI).

You can check it directly from the git branch I've prepared:
https://git.linaro.org/people/marek.szyprowski/linux-srpol.git/shortlog/refs/heads/v3.16-odroid

If you disable this always-on property from tv pd domain (you can keep lcd0
domain enabled to simplify things), the system will freeze. I really have
no idea how to fix this.

>
>> Later, once the proper sequence is found we can remove those properties
>> from Odroid DTS.
> I dunno, because you would have to support the property to the end of
> time then, even though it's not used anymore. Isn't that the kind of
> thing that shouldn't end up in DT bindings specs?

I think that there have been recently similar issue with one of Exyno5 SoCs,
where one of the domain has been removed from DTS just to keep it enabled
all the time (assuming that bootloader left it enabled). See commit
8b3ca983a69ab28bab257bdbaa9eaa1b8830d7b7 ("ARM: dts: Remove g2d_pd node for
exynos5420"). So this property can be used for that case too.

Best regards

Patch

diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
index 5216b419016a..b25d9b1ce471 100644
--- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
+++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
@@ -8,6 +8,8 @@  Required Properties:
     * samsung,exynos4210-pd - for exynos4210 type power domain.
 - reg: physical base address of the controller and length of memory mapped
     region.
+Optional properties:
+- domain-always-on:	keeps the domain always enabled
 
 Node of a device using power domains must have a samsung,power-domain property
 defined with a phandle to respective power domain.
diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-exynos/pm_domains.c
index fe6570ebbdde..279b008de02f 100644
--- a/arch/arm/mach-exynos/pm_domains.c
+++ b/arch/arm/mach-exynos/pm_domains.c
@@ -151,6 +151,7 @@  static __init int exynos4_pm_init_power_domain(void)
 	struct device_node *np;
 
 	for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
+		struct dev_power_governor *gov = NULL;
 		struct exynos_pm_domain *pd;
 		int on;
 
@@ -163,6 +164,9 @@  static __init int exynos4_pm_init_power_domain(void)
 			return -ENOMEM;
 		}
 
+		if (of_property_read_bool(np, "domain-always-on"))
+			gov = &pm_domain_always_on_gov;
+
 		pd->pd.name = kstrdup(np->name, GFP_KERNEL);
 		pd->name = pd->pd.name;
 		pd->base = of_iomap(np, 0);
@@ -174,7 +178,7 @@  static __init int exynos4_pm_init_power_domain(void)
 
 		on = __raw_readl(pd->base + 0x4) & S5P_INT_LOCAL_PWR_EN;
 
-		pm_genpd_init(&pd->pd, NULL, !on);
+		pm_genpd_init(&pd->pd, gov, !on);
 	}
 
 	bus_register_notifier(&platform_bus_type, &platform_nb);