Re: [EXTERNAL] Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Matt Coster <Matt.Coster@imgtec.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4801 bytes --]

On 12/02/2026 15:56, Thorsten Leemhuis wrote:
> On 2/12/26 15:38, Marek Vasut wrote:
>> On 2/12/26 10:00 AM, Matt Coster wrote:
>>> On 11/02/2026 19:17, Marek Vasut wrote:
>>>> On 1/23/26 2:50 PM, Geert Uytterhoeven wrote:
>>>>> On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com>
>>>>> wrote:
>>>>>> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
>>>>>>> Call the dev_pm_domain_attach_list() and dev_pm_domain_detach_list()
>>>>>>> helpers instead of open-coding multi PM Domain handling.
>>>>>>>
>>>>>>> This changes behavior slightly:
>>>>>>>     - The new handling is also applied in case of a single PM Domain,
>>>>>>>     - PM Domains are now referred to by index instead of by name, but
>>>>>>>       "make dtbs_check" enforces the actual naming and ordering
>>>>>>> anyway,
>>>>>>>     - There are no longer device links created between virtual domain
>>>>>>>       devices, only between virtual devices and the parent device.
>>>>>>
>>>>>> We still need this guarantee, both at start and end of day. In the
>>>>>> current implementation dev_pm_domain_attach_list() iterates forwards,
>>>>>> but so does dev_pm_domain_detach_list(). Even if we changed that, I'd
>>>>>> prefer not to rely on the implementation details when we can
>>>>>> declare the
>>>>>> dependencies explicitly.
>>>>>
>>>>> Note that on R-Car, the PM Domains are nested (see e.g.
>>>>> r8a7795_areas[]),
>>>>> so they are always (un)powered in the correct order.  But that may not
>>>>> be the case in the integration on other SoCs.
>>>>>
>>>>>> We had/have a patch (attached) kicking around internally to use the
>>>>>> *_list() functions but keep the inter-domain links in place; it got
>>>>>> held
>>>>>> up by discussions as to whether we actually need those dependencies
>>>>>> for
>>>>>> the hardware to behave correctly. Your patch spurred me to run around
>>>>>> the office and nag people a bit, and it seems we really do need to
>>>>>> care
>>>>>> about the ordering.
>>>>>
>>>>> OK.
>>>>>
>>>>>> Can you add the links back in for a V2 or I can properly send the
>>>>>> attached patch instead, I don't mind either way.
>>>>>
>>>>> Please move forward with your patch, you are the expert.
>>>>> I prefer not to be blamed for any breakage ;-)
>>>>
>>>> Has there been any progress on fixing this kernel crash ?
>>>>
>>>> There are already two proposed solutions, but no fix is upstream.
>>>
>>> Yes and no. Our patch to use dev_pm_domain_attach_list() has landed in
>>> drm-misc-next as commit e19cc5ab347e3 ("drm/imagination: Use
>>> dev_pm_domain_attach_list()"), but this does not fix the underlying
>>> issue of missing synchronization in the PM core[1] is still unresolved
>>> as far as I'm aware.
>>
>> OK, but the pvr driver can currently easily crash the kernel on boot if
>> firmware is missing, so that should be fixed soon, right ?
> 
> Well, drm-misc-next afaik means that the above mentioned fix would only
> be merged in 7.1, which is ~4 months away, which is not really "soon"
> I'd say. Or did I misjudge this?

The above isn't really a "fix" per se, it's just an enhancement. The
underlying crash can still happen. We could still pick it into
drm-misc-fixes and have it in the next -rc plus backported to stable,
but I'm not sure I see the value.

> 
>> I added the regressions list onto CC, because this seems like a problem
>> worth tracking.
> 
> Noticed that and wondered what change caused the regression. Did not
> find a answer in a quick search on lore[1]. Because if it's a
> regression, we maybe should just revert the culprit for now according to
> Linus:
> https://lore.kernel.org/lkml/CAHk-=wi86AosXs66-yi54+mpQjPu0upxB8ZAfG+LsMyJmcuMSA@mail.gmail.com/ 

From our side at least, I don't believe this is a regression at all. We
haven't been able to reproduce this issue on any of the platforms we
have available (although we did stumble on a related bugfix[2] while
trying).

My current understanding of the situation is that the fix proposed by
Marek in the Reneasas driver[3] works, but is not suitable since
pm_runtime_barrier() should be inserted by the caller, not the power
driver. But it seems that's not always possible (particularly when using
devm), so I don't really understand where we go from here. I don't see
anything we're doing substantially differently (before or after the
commit I mentioned above) from anybody else.

Cheers,
Matt

[2]: TBC
[3]: https://lore.kernel.org/r/0e9f963b-00e0-43d1-b567-cb10b8f66df1@mailbox.org/

> 
> Ciao, Thorsten
> 
> [1] I guess this was the initial report from Geert?
> https://lore.kernel.org/all/CAMuHMdWapT40hV3c+CSBqFOW05aWcV1a6v_NiJYgoYi0i9_PDQ@mail.gmail.com/ 


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]