Unconditionally calling of_genpd_sync_state() causes issues on
platforms with child domains as the parent domain will be turned off
before the child domain was even registered during boot.
This in particular is an issue for the upcoming Google gs101 support -
all operations on child domains registered after the parent domain
misbehave.
Add a flag to the probe data to be able to sync_state conditionally
only, and enable that flag on the two platforms currently supported by
this driver.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
v2:
* use bool for need_early_sync_state (Krzysztof)
---
drivers/pmdomain/samsung/exynos-pm-domains.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/pmdomain/samsung/exynos-pm-domains.c b/drivers/pmdomain/samsung/exynos-pm-domains.c
index 638d286b57f716140b2401092415644a6805870e..15a1582aa92103a07335eb681600d9415369fefd 100644
--- a/drivers/pmdomain/samsung/exynos-pm-domains.c
+++ b/drivers/pmdomain/samsung/exynos-pm-domains.c
@@ -20,6 +20,7 @@
struct exynos_pm_domain_config {
/* Value for LOCAL_PWR_CFG and STATUS fields for each domain */
u32 local_pwr_cfg;
+ bool need_early_sync_state;
};
/*
@@ -69,10 +70,12 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
static const struct exynos_pm_domain_config exynos4210_cfg = {
.local_pwr_cfg = 0x7,
+ .need_early_sync_state = true,
};
static const struct exynos_pm_domain_config exynos5433_cfg = {
.local_pwr_cfg = 0xf,
+ .need_early_sync_state = true,
};
static const struct of_device_id exynos_pm_domain_of_match[] = {
@@ -179,7 +182,7 @@ static int exynos_pd_probe(struct platform_device *pdev)
* reset during boot. As a temporary hack to manage this, let's enforce
* a sync_state.
*/
- if (!ret)
+ if (pm_domain_cfg->need_early_sync_state && !ret)
of_genpd_sync_state(np);
pm_runtime_enable(dev);
--
2.51.0.788.g6d19910ace-goog
On Thu, 16 Oct 2025 at 17:58, André Draszik <andre.draszik@linaro.org> wrote:
>
> Unconditionally calling of_genpd_sync_state() causes issues on
> platforms with child domains as the parent domain will be turned off
> before the child domain was even registered during boot.
>
> This in particular is an issue for the upcoming Google gs101 support -
> all operations on child domains registered after the parent domain
> misbehave.
>
> Add a flag to the probe data to be able to sync_state conditionally
> only, and enable that flag on the two platforms currently supported by
> this driver.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
>
> ---
> v2:
> * use bool for need_early_sync_state (Krzysztof)
> ---
> drivers/pmdomain/samsung/exynos-pm-domains.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pmdomain/samsung/exynos-pm-domains.c b/drivers/pmdomain/samsung/exynos-pm-domains.c
> index 638d286b57f716140b2401092415644a6805870e..15a1582aa92103a07335eb681600d9415369fefd 100644
> --- a/drivers/pmdomain/samsung/exynos-pm-domains.c
> +++ b/drivers/pmdomain/samsung/exynos-pm-domains.c
> @@ -20,6 +20,7 @@
> struct exynos_pm_domain_config {
> /* Value for LOCAL_PWR_CFG and STATUS fields for each domain */
> u32 local_pwr_cfg;
> + bool need_early_sync_state;
> };
>
> /*
> @@ -69,10 +70,12 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
>
> static const struct exynos_pm_domain_config exynos4210_cfg = {
> .local_pwr_cfg = 0x7,
> + .need_early_sync_state = true,
> };
>
> static const struct exynos_pm_domain_config exynos5433_cfg = {
> .local_pwr_cfg = 0xf,
> + .need_early_sync_state = true,
> };
>
> static const struct of_device_id exynos_pm_domain_of_match[] = {
> @@ -179,7 +182,7 @@ static int exynos_pd_probe(struct platform_device *pdev)
> * reset during boot. As a temporary hack to manage this, let's enforce
> * a sync_state.
> */
> - if (!ret)
> + if (pm_domain_cfg->need_early_sync_state && !ret)
> of_genpd_sync_state(np);
The call to of_genpd_sync_state() was intended as a temporary solution here.
Potentially, if we would be able to distinguish what PM domain that is
causing the problem on the Exynos platforms, we could set
GENPD_FLAG_NO_STAY_ON for that genpd instead.
>
> pm_runtime_enable(dev);
>
> --
> 2.51.0.788.g6d19910ace-goog
>
The above said, $subject patch is still fine as is for me.
Kind regards
Uffe
On 22.10.2025 13:06, Ulf Hansson wrote:
> On Thu, 16 Oct 2025 at 17:58, André Draszik <andre.draszik@linaro.org> wrote:
>> Unconditionally calling of_genpd_sync_state() causes issues on
>> platforms with child domains as the parent domain will be turned off
>> before the child domain was even registered during boot.
>>
>> This in particular is an issue for the upcoming Google gs101 support -
>> all operations on child domains registered after the parent domain
>> misbehave.
>>
>> Add a flag to the probe data to be able to sync_state conditionally
>> only, and enable that flag on the two platforms currently supported by
>> this driver.
>>
>> Signed-off-by: André Draszik <andre.draszik@linaro.org>
>>
>> ---
>> v2:
>> * use bool for need_early_sync_state (Krzysztof)
>> ---
>> drivers/pmdomain/samsung/exynos-pm-domains.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pmdomain/samsung/exynos-pm-domains.c b/drivers/pmdomain/samsung/exynos-pm-domains.c
>> index 638d286b57f716140b2401092415644a6805870e..15a1582aa92103a07335eb681600d9415369fefd 100644
>> --- a/drivers/pmdomain/samsung/exynos-pm-domains.c
>> +++ b/drivers/pmdomain/samsung/exynos-pm-domains.c
>> @@ -20,6 +20,7 @@
>> struct exynos_pm_domain_config {
>> /* Value for LOCAL_PWR_CFG and STATUS fields for each domain */
>> u32 local_pwr_cfg;
>> + bool need_early_sync_state;
>> };
>>
>> /*
>> @@ -69,10 +70,12 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
>>
>> static const struct exynos_pm_domain_config exynos4210_cfg = {
>> .local_pwr_cfg = 0x7,
>> + .need_early_sync_state = true,
>> };
>>
>> static const struct exynos_pm_domain_config exynos5433_cfg = {
>> .local_pwr_cfg = 0xf,
>> + .need_early_sync_state = true,
>> };
>>
>> static const struct of_device_id exynos_pm_domain_of_match[] = {
>> @@ -179,7 +182,7 @@ static int exynos_pd_probe(struct platform_device *pdev)
>> * reset during boot. As a temporary hack to manage this, let's enforce
>> * a sync_state.
>> */
>> - if (!ret)
>> + if (pm_domain_cfg->need_early_sync_state && !ret)
>> of_genpd_sync_state(np);
> The call to of_genpd_sync_state() was intended as a temporary solution here.
>
> Potentially, if we would be able to distinguish what PM domain that is
> causing the problem on the Exynos platforms, we could set
> GENPD_FLAG_NO_STAY_ON for that genpd instead.
Well, this of_genpd_sync_state() "workaround" has to be applied only to
the power domain of the display controller device. It can be replaced by
the following check on the legacy Exynos systems:
if (IS_ENABLED(CONFIG_ARM) &&
of_device_is_compatible(np, "samsung,exynos4210-pd") &&
(strstr(pd->pd.name, "LCD") || strstr(pd->pd.name, "DISP")))
pd->pd.flags = GENPD_FLAG_NO_STAY_ON;
I assume that this information cannot be coded in device tree to make it
somehow generic...
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On Wed, 22 Oct 2025 at 20:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> On 22.10.2025 13:06, Ulf Hansson wrote:
> > On Thu, 16 Oct 2025 at 17:58, André Draszik <andre.draszik@linaro.org> wrote:
> >> Unconditionally calling of_genpd_sync_state() causes issues on
> >> platforms with child domains as the parent domain will be turned off
> >> before the child domain was even registered during boot.
> >>
> >> This in particular is an issue for the upcoming Google gs101 support -
> >> all operations on child domains registered after the parent domain
> >> misbehave.
> >>
> >> Add a flag to the probe data to be able to sync_state conditionally
> >> only, and enable that flag on the two platforms currently supported by
> >> this driver.
> >>
> >> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> >>
> >> ---
> >> v2:
> >> * use bool for need_early_sync_state (Krzysztof)
> >> ---
> >> drivers/pmdomain/samsung/exynos-pm-domains.c | 5 ++++-
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/pmdomain/samsung/exynos-pm-domains.c b/drivers/pmdomain/samsung/exynos-pm-domains.c
> >> index 638d286b57f716140b2401092415644a6805870e..15a1582aa92103a07335eb681600d9415369fefd 100644
> >> --- a/drivers/pmdomain/samsung/exynos-pm-domains.c
> >> +++ b/drivers/pmdomain/samsung/exynos-pm-domains.c
> >> @@ -20,6 +20,7 @@
> >> struct exynos_pm_domain_config {
> >> /* Value for LOCAL_PWR_CFG and STATUS fields for each domain */
> >> u32 local_pwr_cfg;
> >> + bool need_early_sync_state;
> >> };
> >>
> >> /*
> >> @@ -69,10 +70,12 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
> >>
> >> static const struct exynos_pm_domain_config exynos4210_cfg = {
> >> .local_pwr_cfg = 0x7,
> >> + .need_early_sync_state = true,
> >> };
> >>
> >> static const struct exynos_pm_domain_config exynos5433_cfg = {
> >> .local_pwr_cfg = 0xf,
> >> + .need_early_sync_state = true,
> >> };
> >>
> >> static const struct of_device_id exynos_pm_domain_of_match[] = {
> >> @@ -179,7 +182,7 @@ static int exynos_pd_probe(struct platform_device *pdev)
> >> * reset during boot. As a temporary hack to manage this, let's enforce
> >> * a sync_state.
> >> */
> >> - if (!ret)
> >> + if (pm_domain_cfg->need_early_sync_state && !ret)
> >> of_genpd_sync_state(np);
> > The call to of_genpd_sync_state() was intended as a temporary solution here.
> >
> > Potentially, if we would be able to distinguish what PM domain that is
> > causing the problem on the Exynos platforms, we could set
> > GENPD_FLAG_NO_STAY_ON for that genpd instead.
>
> Well, this of_genpd_sync_state() "workaround" has to be applied only to
> the power domain of the display controller device. It can be replaced by
> the following check on the legacy Exynos systems:
>
> if (IS_ENABLED(CONFIG_ARM) &&
> of_device_is_compatible(np, "samsung,exynos4210-pd") &&
> (strstr(pd->pd.name, "LCD") || strstr(pd->pd.name, "DISP")))
> pd->pd.flags = GENPD_FLAG_NO_STAY_ON;
Oh wait, perhaps better to just power-off these PM domains before
calling pm_genpd_init(), if that can be done safely?
At least that would guarantee the reset to happen before the display
driver gets probed. Instead of relying on genpd_power_off_unused()
(late_initcall_sync) to do it.
>
> I assume that this information cannot be coded in device tree to make it
> somehow generic...
Right, in principle we would need a new DT property for a power-domain
provider, like "broken-hw-reset", because we don't have a reset-line
to pull.
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
Kind regards
Uffe
On 23.10.2025 12:02, Ulf Hansson wrote:
> On Wed, 22 Oct 2025 at 20:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> On 22.10.2025 13:06, Ulf Hansson wrote:
>>> On Thu, 16 Oct 2025 at 17:58, André Draszik <andre.draszik@linaro.org> wrote:
>>>> Unconditionally calling of_genpd_sync_state() causes issues on
>>>> platforms with child domains as the parent domain will be turned off
>>>> before the child domain was even registered during boot.
>>>>
>>>> This in particular is an issue for the upcoming Google gs101 support -
>>>> all operations on child domains registered after the parent domain
>>>> misbehave.
>>>>
>>>> Add a flag to the probe data to be able to sync_state conditionally
>>>> only, and enable that flag on the two platforms currently supported by
>>>> this driver.
>>>>
>>>> Signed-off-by: André Draszik <andre.draszik@linaro.org>
>>>>
>>>> ---
>>>> v2:
>>>> * use bool for need_early_sync_state (Krzysztof)
>>>> ---
>>>> drivers/pmdomain/samsung/exynos-pm-domains.c | 5 ++++-
>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pmdomain/samsung/exynos-pm-domains.c b/drivers/pmdomain/samsung/exynos-pm-domains.c
>>>> index 638d286b57f716140b2401092415644a6805870e..15a1582aa92103a07335eb681600d9415369fefd 100644
>>>> --- a/drivers/pmdomain/samsung/exynos-pm-domains.c
>>>> +++ b/drivers/pmdomain/samsung/exynos-pm-domains.c
>>>> @@ -20,6 +20,7 @@
>>>> struct exynos_pm_domain_config {
>>>> /* Value for LOCAL_PWR_CFG and STATUS fields for each domain */
>>>> u32 local_pwr_cfg;
>>>> + bool need_early_sync_state;
>>>> };
>>>>
>>>> /*
>>>> @@ -69,10 +70,12 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
>>>>
>>>> static const struct exynos_pm_domain_config exynos4210_cfg = {
>>>> .local_pwr_cfg = 0x7,
>>>> + .need_early_sync_state = true,
>>>> };
>>>>
>>>> static const struct exynos_pm_domain_config exynos5433_cfg = {
>>>> .local_pwr_cfg = 0xf,
>>>> + .need_early_sync_state = true,
>>>> };
>>>>
>>>> static const struct of_device_id exynos_pm_domain_of_match[] = {
>>>> @@ -179,7 +182,7 @@ static int exynos_pd_probe(struct platform_device *pdev)
>>>> * reset during boot. As a temporary hack to manage this, let's enforce
>>>> * a sync_state.
>>>> */
>>>> - if (!ret)
>>>> + if (pm_domain_cfg->need_early_sync_state && !ret)
>>>> of_genpd_sync_state(np);
>>> The call to of_genpd_sync_state() was intended as a temporary solution here.
>>>
>>> Potentially, if we would be able to distinguish what PM domain that is
>>> causing the problem on the Exynos platforms, we could set
>>> GENPD_FLAG_NO_STAY_ON for that genpd instead.
>> Well, this of_genpd_sync_state() "workaround" has to be applied only to
>> the power domain of the display controller device. It can be replaced by
>> the following check on the legacy Exynos systems:
>>
>> if (IS_ENABLED(CONFIG_ARM) &&
>> of_device_is_compatible(np, "samsung,exynos4210-pd") &&
>> (strstr(pd->pd.name, "LCD") || strstr(pd->pd.name, "DISP")))
>> pd->pd.flags = GENPD_FLAG_NO_STAY_ON;
> Oh wait, perhaps better to just power-off these PM domains before
> calling pm_genpd_init(), if that can be done safely?
>
> At least that would guarantee the reset to happen before the display
> driver gets probed. Instead of relying on genpd_power_off_unused()
> (late_initcall_sync) to do it.
Well, yes, this works too:
if ((of_device_is_compatible(np, "samsung,exynos4210-pd") &&
(strstr(pd->pd.name, "LCD") || strstr(pd->pd.name, "DISP"))))
exynos_pd_power_off(&pd->pd);
>> I assume that this information cannot be coded in device tree to make it
>> somehow generic...
> Right, in principle we would need a new DT property for a power-domain
> provider, like "broken-hw-reset", because we don't have a reset-line
> to pull.
It is not a matter of broken reset at all. It is a matter of software
configuration and the lack of 'protocol' to pass the information that
the display controller is configured to display splash screen from the
system memory at given address and newly instantiated drivers must to be
aware of that.
Turning display-related power domain off simply resets all that
configuration, so drivers can start from good known 'unconfigured' state.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On Thu, 23 Oct 2025 at 14:17, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> On 23.10.2025 12:02, Ulf Hansson wrote:
> > On Wed, 22 Oct 2025 at 20:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >> On 22.10.2025 13:06, Ulf Hansson wrote:
> >>> On Thu, 16 Oct 2025 at 17:58, André Draszik <andre.draszik@linaro.org> wrote:
> >>>> Unconditionally calling of_genpd_sync_state() causes issues on
> >>>> platforms with child domains as the parent domain will be turned off
> >>>> before the child domain was even registered during boot.
> >>>>
> >>>> This in particular is an issue for the upcoming Google gs101 support -
> >>>> all operations on child domains registered after the parent domain
> >>>> misbehave.
> >>>>
> >>>> Add a flag to the probe data to be able to sync_state conditionally
> >>>> only, and enable that flag on the two platforms currently supported by
> >>>> this driver.
> >>>>
> >>>> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> >>>>
> >>>> ---
> >>>> v2:
> >>>> * use bool for need_early_sync_state (Krzysztof)
> >>>> ---
> >>>> drivers/pmdomain/samsung/exynos-pm-domains.c | 5 ++++-
> >>>> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/pmdomain/samsung/exynos-pm-domains.c b/drivers/pmdomain/samsung/exynos-pm-domains.c
> >>>> index 638d286b57f716140b2401092415644a6805870e..15a1582aa92103a07335eb681600d9415369fefd 100644
> >>>> --- a/drivers/pmdomain/samsung/exynos-pm-domains.c
> >>>> +++ b/drivers/pmdomain/samsung/exynos-pm-domains.c
> >>>> @@ -20,6 +20,7 @@
> >>>> struct exynos_pm_domain_config {
> >>>> /* Value for LOCAL_PWR_CFG and STATUS fields for each domain */
> >>>> u32 local_pwr_cfg;
> >>>> + bool need_early_sync_state;
> >>>> };
> >>>>
> >>>> /*
> >>>> @@ -69,10 +70,12 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
> >>>>
> >>>> static const struct exynos_pm_domain_config exynos4210_cfg = {
> >>>> .local_pwr_cfg = 0x7,
> >>>> + .need_early_sync_state = true,
> >>>> };
> >>>>
> >>>> static const struct exynos_pm_domain_config exynos5433_cfg = {
> >>>> .local_pwr_cfg = 0xf,
> >>>> + .need_early_sync_state = true,
> >>>> };
> >>>>
> >>>> static const struct of_device_id exynos_pm_domain_of_match[] = {
> >>>> @@ -179,7 +182,7 @@ static int exynos_pd_probe(struct platform_device *pdev)
> >>>> * reset during boot. As a temporary hack to manage this, let's enforce
> >>>> * a sync_state.
> >>>> */
> >>>> - if (!ret)
> >>>> + if (pm_domain_cfg->need_early_sync_state && !ret)
> >>>> of_genpd_sync_state(np);
> >>> The call to of_genpd_sync_state() was intended as a temporary solution here.
> >>>
> >>> Potentially, if we would be able to distinguish what PM domain that is
> >>> causing the problem on the Exynos platforms, we could set
> >>> GENPD_FLAG_NO_STAY_ON for that genpd instead.
> >> Well, this of_genpd_sync_state() "workaround" has to be applied only to
> >> the power domain of the display controller device. It can be replaced by
> >> the following check on the legacy Exynos systems:
> >>
> >> if (IS_ENABLED(CONFIG_ARM) &&
> >> of_device_is_compatible(np, "samsung,exynos4210-pd") &&
> >> (strstr(pd->pd.name, "LCD") || strstr(pd->pd.name, "DISP")))
> >> pd->pd.flags = GENPD_FLAG_NO_STAY_ON;
> > Oh wait, perhaps better to just power-off these PM domains before
> > calling pm_genpd_init(), if that can be done safely?
> >
> > At least that would guarantee the reset to happen before the display
> > driver gets probed. Instead of relying on genpd_power_off_unused()
> > (late_initcall_sync) to do it.
>
> Well, yes, this works too:
>
> if ((of_device_is_compatible(np, "samsung,exynos4210-pd") &&
> (strstr(pd->pd.name, "LCD") || strstr(pd->pd.name, "DISP"))))
> exynos_pd_power_off(&pd->pd);
Okay!
Should we (you or me?) send a patch and change the code like this?
>
> >> I assume that this information cannot be coded in device tree to make it
> >> somehow generic...
> > Right, in principle we would need a new DT property for a power-domain
> > provider, like "broken-hw-reset", because we don't have a reset-line
> > to pull.
>
> It is not a matter of broken reset at all. It is a matter of software
> configuration and the lack of 'protocol' to pass the information that
> the display controller is configured to display splash screen from the
> system memory at given address and newly instantiated drivers must to be
> aware of that.
>
> Turning display-related power domain off simply resets all that
> configuration, so drivers can start from good known 'unconfigured' state.
Alright, thanks for clarifying! Yeah, a DT property would be wrong for this.
In fact, there are some drivers [1] that make use of the genpd on/off
notifiers to manage a reset of its device's PM domain. It works, but
it's a bit messy. Perhaps it could be simplified with some new helper
functions, if more consumers drivers need something similar.
Kind regards
Uffe
[1]
commit c11fa1204fe9 ("drm/msm/a6xx: Use genpd notifier to ensure
cx-gdsc collapse")
© 2016 - 2025 Red Hat, Inc.