Message-ID: <cb0f50a9-9e63-40ea-973e-937086d38136@ideasonboard.com>
Date: Tue, 16 Sep 2025 12:14:18 +0300
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Ulf Hansson <ulf.hansson@...aro.org>
Cc: Nishanth Menon <nm@...com>, Tero Kristo <kristo@...nel.org>,
 Santosh Shilimkar <ssantosh@...nel.org>,
 linux-arm-kernel@...ts.infradead.org, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org, Kevin Hilman <khilman@...libre.com>,
 vishalm@...com, sebin.francis@...com, d-gole@...com,
 Devarsh Thakkar <devarsht@...com>, Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [PATCH RFC v2] pmdomain: ti-sci: Set PD on/off state according to
 the HW state

Hi,

On 11/09/2025 17:38, Ulf Hansson wrote:
> On Mon, 8 Sept 2025 at 10:35, Tomi Valkeinen
> <tomi.valkeinen@...asonboard.com> wrote:
>>
>> At the moment the driver sets the power state of all the PDs it creates
>> to off, regardless of the actual HW state. This has two drawbacks:
>>
>> 1) The kernel cannot disable unused PDs automatically for power saving,
>>    as it thinks they are off already
>>
>> 2) A more specific case (but perhaps applicable to other scenarios
>>    also): bootloader enabled splash-screen cannot be kept on the screen.
>>
>> The issue in 2) is that the driver framework automatically enables the
>> device's PD before calling probe() and disables it after the probe().
>> This means that when the display subsystem (DSS) driver probes, but e.g.
>> fails due to deferred probing, the DSS PD gets turned off and the driver
>> cannot do anything to affect that.
>>
>> Solving the 2) requires more changes to actually keep the PD on during
>> the boot, but a prerequisite for it is to have the correct power state
>> for the PD.
>>
>> The downside with this patch is that it takes time to call the 'is_on'
>> op, and we need to call it for each PD. In my tests with AM62 SK, using
>> defconfig, I see an increase from ~3.5ms to ~7ms. However, the added
>> feature is valuable, so in my opinion it's worth it.
>>
>> The performance could probably be improved with a new firmware API which
>> returns the power states of all the PDs.
>>
>> There's also a related HW issue at play here: if the DSS IP is enabled
>> and active, and its PD is turned off without first disabling the DSS
>> display outputs, the DSS IP will hang and causes the kernel to halt if
>> and when the DSS driver accesses the DSS registers the next time.
>>
>> With the current upstream kernel, with this patch applied, this means
>> that if the bootloader enables the display, and the DSS driver is
>> compiled as a module, the kernel will at some point disable unused PDs,
>> including the DSS PD. When the DSS module is later loaded, it will hang
>> the kernel.
>>
>> The same issue is already there, even without this patch, as the DSS
>> driver may hit deferred probing, which causes the PD to be turned off,
>> and leading to kernel halt when the DSS driver is probed again. This
>> issue has been made quite rare with some arrangements in the DSS
>> driver's probe, but it's still there.
>>
>> With recent change from Ulf (e.g. commit 13a4b7fb6260 ("pmdomain: core:
>> Leave powered-on genpds on until late_initcall_sync")), the sync state
>> mechanism comes to rescue. It will keep the power domains enabled, until
>> the drivers have probed, or the sync-state is triggered via some other
>> mechanism (e.g. manually by the boot scripts).
>>
>> Reviewed-by: Kevin Hilman <khilman@...libre.com>
>> Tested-by: Kevin Hilman <khilman@...libre.com>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> 
> Makes perfect sense to me! Applied for next, thanks!

Thanks!

I just noticed that the patch was marked RFC. Just for the record, the
v1 was RFC, this wasn't.

 Tomi

Powered by blists - more mailing lists