[PATCH v3 21/24] pmdomain: core: Leave powered-on genpds on until late_initcall_sync

[Marek Szyprowski]

On 01.07.2025 13:47, Ulf Hansson wrote:
> Powering-off a genpd that was on during boot, before all of its consumer
> devices have been probed, is certainly prone to problems.
>
> As a step to improve this situation, let's prevent these genpds from being
> powered-off until genpd_power_off_unused() gets called, which is a
> late_initcall_sync().
>
> Note that, this still doesn't guarantee that all the consumer devices has
> been probed before we allow to power-off the genpds. Yet, this should be a
> step in the right direction.
>
> Suggested-by: Saravana Kannan <saravanak at google.com>
> Tested-by: Hiago De Franco <hiago.franco at toradex.com> # Colibri iMX8X
> Tested-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com> # TI AM62A,Xilinx ZynqMP ZCU106
> Signed-off-by: Ulf Hansson <ulf.hansson at linaro.org>

This change has a side effect on some Exynos based boards, which have 
display and bootloader is configured to setup a splash screen on it. 
Since today's linux-next, those boards fails to boot, because of the 
IOMMU page fault.

This happens because the display controller is enabled and configured to 
perform the scanout from the spash-screen buffer until the respective 
driver will reset it in driver probe() function. This however doesn't 
work with IOMMU, which is being probed earlier than the display 
controller driver, what in turn causes IOMMU page fault once the IOMMU 
driver gets attached. This worked before applying this patch, because 
the power domain of display controller was simply turned off early 
effectively reseting the display controller.

This has been discussed a bit recently: 
https://lore.kernel.org/all/544ad69cba52a9b87447e3ac1c7fa8c3@disroot.org/ 
and I can add a workaround for this issue in the bootloaders of those 
boards, but this is something that has to be somehow addressed in a 
generic way.

> ---
>   drivers/pmdomain/core.c   | 10 ++++++++--
>   include/linux/pm_domain.h |  1 +
>   2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 5cef6de60c72..18951ed6295d 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -931,11 +931,12 @@ static void genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
>   	 * The domain is already in the "power off" state.
>   	 * System suspend is in progress.
>   	 * The domain is configured as always on.
> +	 * The domain was on at boot and still need to stay on.
>   	 * The domain has a subdomain being powered on.
>   	 */
>   	if (!genpd_status_on(genpd) || genpd->prepared_count > 0 ||
>   	    genpd_is_always_on(genpd) || genpd_is_rpm_always_on(genpd) ||
> -	    atomic_read(&genpd->sd_count) > 0)
> +	    genpd->stay_on || atomic_read(&genpd->sd_count) > 0)
>   		return;
>   
>   	/*
> @@ -1346,8 +1347,12 @@ static int __init genpd_power_off_unused(void)
>   	pr_info("genpd: Disabling unused power domains\n");
>   	mutex_lock(&gpd_list_lock);
>   
> -	list_for_each_entry(genpd, &gpd_list, gpd_list_node)
> +	list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
> +		genpd_lock(genpd);
> +		genpd->stay_on = false;
> +		genpd_unlock(genpd);
>   		genpd_queue_power_off_work(genpd);
> +	}
>   
>   	mutex_unlock(&gpd_list_lock);
>   
> @@ -2352,6 +2357,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
>   	INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
>   	atomic_set(&genpd->sd_count, 0);
>   	genpd->status = is_off ? GENPD_STATE_OFF : GENPD_STATE_ON;
> +	genpd->stay_on = !is_off;
>   	genpd->sync_state = GENPD_SYNC_STATE_OFF;
>   	genpd->device_count = 0;
>   	genpd->provider = NULL;
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index d68e07dadc99..99556589f45e 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -199,6 +199,7 @@ struct generic_pm_domain {
>   	unsigned int performance_state;	/* Aggregated max performance state */
>   	cpumask_var_t cpus;		/* A cpumask of the attached CPUs */
>   	bool synced_poweroff;		/* A consumer needs a synced poweroff */
> +	bool stay_on;			/* Stay powered-on during boot. */
>   	enum genpd_sync_state sync_state; /* How sync_state is managed. */
>   	int (*power_off)(struct generic_pm_domain *domain);
>   	int (*power_on)(struct generic_pm_domain *domain);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland