Re: [PATCH RFC v2] pmdomain: ti-sci: Set PD on/off state according to the HW state

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

Hi,

On 11/09/2025 17:38, Ulf Hansson wrote:
> On Mon, 8 Sept 2025 at 10:35, Tomi Valkeinen
> <tomi.valkeinen@ideasonboard.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@baylibre.com>
>> Tested-by: Kevin Hilman <khilman@baylibre.com>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.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