Message-ID: <bb9d4495-ada4-446a-80e3-6b9ddc676f48@ideasonboard.com>
Date: Thu, 29 Aug 2024 14:55:07 +0300
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Ulf Hansson <ulf.hansson@...aro.org>, Abel Vesa <abel.vesa@...aro.org>,
 Saravana Kannan <saravanak@...gle.com>, Stephen Boyd <sboyd@...nel.org>
Cc: Nishanth Menon <nm@...com>, Tero Kristo <kristo@...nel.org>,
 Santosh Shilimkar <ssantosh@...nel.org>, Dave Gerlach <d-gerlach@...com>,
 J Keerthy <j-keerthy@...com>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Santosh Shilimkar <santosh.shilimkar@...cle.com>,
 linux-arm-kernel@...ts.infradead.org, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, Devarsh Thakkar <devarsht@...com>
Subject: Re: [PATCH RFC 2/2] pmdomain: ti-sci: Support retaining PD boot time
 state

Hi Ulf,

On 03/05/2024 16:45, Ulf Hansson wrote:
> + Abel, Saravanna, Stephen
> 
> On Mon, 15 Apr 2024 at 19:17, Tomi Valkeinen
> <tomi.valkeinen@...asonboard.com> wrote:
>>
>> On 15/04/2024 19:00, Tomi Valkeinen wrote:
>>> Add a new flag, TI_SCI_PD_KEEP_BOOT_STATE, which can be set in the dts
>>> when referring to power domains. When this flag is set, the ti-sci
>>> driver will check if the PD is currently enabled in the HW, and if so,
>>> set the GENPD_FLAG_ALWAYS_ON flag so that the PD will stay enabled.
>>>
>>> The main issue I'm trying to solve here is this:
>>>
>>> If the Display Subsystem (DSS) has been enabled by the bootloader, the
>>> related PD has also been enabled in the HW. When the tidss driver
>>> probes, the driver framework will automatically enable the PD. While
>>> executing the probe function it is very common for the probe to return
>>> EPROBE_DEFER, and, in rarer cases, an actual error. When this happens
>>> (probe() returns an error), the driver framework will automatically
>>> disable the related PD.
>>>
>>> Powering off the PD while the DSS is enabled and displaying a picture
>>> will cause the DSS HW to enter a bad state, from which (afaik) it can't
>>> be woken up except with full power-cycle. Trying to access the DSS in
>>> this state (e.g. when retrying the probe) will usually cause the board
>>> to hang sooner or later.
>>>
>>> Even if we wouldn't have this board-hangs issue, it's nice to be able to
>>> keep the DSS PD enabled: we want to keep the DSS enabled when the
>>> bootloader has enabled the screen. If, instead, we disable the PD at the
>>> first EPROBE_DEFER, the screen will (probably) go black.
>>
>> A few things occurred to me. The driver is supposed to clear the
>> GENPD_FLAG_ALWAYS_ON when the driver has probed successfully. There are
>> two possible issues with that:
>>
>> - Afaics, there's no API to do that, and currently I just clear the bit
>> in genpd->flags. There's a clear race there, so some locking would be
>> required.
>>
>> - This uses the GENPD_FLAG_ALWAYS_ON flag to say "PD is always on, until
>> the driver has started". If the PD would have GENPD_FLAG_ALWAYS_ON set
>> for other reasons, the driver would still go and clear the flag, which
>> might break things.
>>
>> Also, unrelated to the above and not a problem in practice at the very
>> moment, but I think clocks should also be dealt with somehow. Something,
>> at early-ish boot stage, should mark the relevant clocks as in use, so
>> that there's no chance they would be turned off when the main kernel has
>> started (the main display driver is often a module).
>>
>> It would be nice to deal with all the above in a single place. I wonder
>> if the tidss driver itself could somehow be split into two parts, an
>> early part that would probe with minimal dependencies, mainly to reserve
>> the core resources without doing any kind of DRM init. And a main part
>> which would (somehow) finish the initialization at a later point, when
>> we have the filesystem (for firmware) and the other bridge/panel drivers
>> have probed.
>>
>> That can be somewhat achieved with simplefb or simpledrm, though, but we
>> can't do any TI DSS specific things there, and it also creates a
>> requirement to have either of those drivers built-in, and the related DT
>> nodes to be added.
> 
> Without going into too much detail, this and similar problems have
> been discussed in the past. With the fw_devlink and the ->sync_state()
> callback we are getting closer to a solution, but for genpd a solution
> is still pending.
> 
> If you want to read up on earlier discussions and join us moving
> forward, that would be great. The last attempt for genpd to move this
> forward was posted by Abel Vesa:
> https://lore.kernel.org/linux-pm/20230621144019.3219858-1-abel.vesa@linaro.org/
> 
> Beyond that, we have also discussed various solutions at the last LPC
> in Richmond. I think the consensus at that point was that Saravana
> targeted to post something for clocks - and when that was done, we
> should do the similar thing for genpd. Anyway, I have looped them into
> this thread, so they can share any updates on their side of the
> matter.

Do you know if there's been any recent work related to this? I tried to 
look around on the lists, but nothing caught my eye.

  Tomi

Powered by blists - more mailing lists