[v2,0/4] Add GPU clock/reset management for TH1520 in genpd

This small patch series adds clock and reset management for the GPU in
the T-HEAD TH1520 SoC through the generic power domain (genpd)
framework.

The TH1520 GPU requires a special sequence involving multiple clocks and
resets to safely bring it up. Coordinating this sequence properly is
necessary for correct GPU operation. Following discussions on the
mailing list with kernel maintainers [1], the recommended approach is to
model this complexity inside a power domain driver, keeping SoC specific
details out of the GPU driver, clock framework, and reset framework.

This series consists of four patches:
- Patch 1 introduces a new dev_pm_info flag to allow device drivers
  to detect when platform PM domains manage clocks or resets
- Patch 2 updates the AON firmware bindings to describe the GPU clkgen
  reset
- Patch 3 extends the TH1520 PM domain driver to handle GPU-specific
  clock and reset sequencing at runtime, using genpd start/stop and
  attach/detach callbacks
- Patch 4 updates the Imagination DRM driver to skip direct clock
  management when platform PM ownership is detected

This approach aligns with recent efforts to treat PM domains as
SoC-specific power management drivers, as presented at OSSEU 2024 [2].

This patchset continues the work started in bigger series [3] by moving
the GPU initialization sequence for the TH1520 SoC into a generic PM
domain driver, specifically handling clock and reset management as part
of GPU bring-up.

v2:

Extended the series by adding two new commits:
 - introduced a new platform_resources_managed flag in dev_pm_info along
   with helper functions, allowing drivers to detect when clocks and resets
   are managed by the platform
 - updated the DRM Imagination driver to skip claiming clocks when
   platform_resources_managed is set

Split the original bindings update:
 - the AON firmware bindings now only add the GPU clkgen reset (the GPU
   core reset remains handled by the GPU node)

Reworked the TH1520 PM domain driver to:
 - acquire GPU clocks and reset dynamically using attach_dev/detach_dev
   callbacks
 - handle clkgen reset internally, while GPU core reset is obtained from
   the consumer device node
 - added a check to enforce that only a single device can be attached to
   the GPU PM domain

[1] - https://lore.kernel.org/all/CAPDyKFqsJaTrF0tBSY-TjpqdVt5=6aPQHYfnDebtphfRZSU=-Q@mail.gmail.com/
[2] - https://osseu2024.sched.com/event/1ej38/the-case-for-an-soc-power-management-driver-stephen-boyd-google
[3] - https://lore.kernel.org/all/20250219140239.1378758-1-m.wilczynski@samsung.com/

---
Michal Wilczynski (4):
      PM: device: Introduce platform_resources_managed flag
      dt-bindings: firmware: thead,th1520: Add resets for GPU clkgen
      pmdomain: thead: Add GPU-specific clock and reset handling for TH1520
      drm/imagination: Skip clocks if platform PM manages resources

 .../bindings/firmware/thead,th1520-aon.yaml        |  11 ++
 drivers/gpu/drm/imagination/pvr_device.c           |  14 +-
 drivers/pmdomain/thead/th1520-pm-domains.c         | 199 +++++++++++++++++++++
 include/linux/device.h                             |  11 ++
 include/linux/pm.h                                 |   1 +
 5 files changed, 232 insertions(+), 4 deletions(-)
---
base-commit: 7c89da246c1268c8dbfc1c7f1edc55aabce45b43
change-id: 20250414-apr_14_for_sending-5b3917817acc

Best regards,

On Mon, Apr 14, 2025 at 08:52:58PM +0200, Michal Wilczynski wrote:
> Update the Imagination PVR driver to skip clock management during
> initialization if the platform PM has indicated that it manages platform
> resources.
> 
> This is necessary for platforms like the T-HEAD TH1520, where the GPU's
> clocks and resets are managed via a PM domain, and should not be
> manipulated directly by the GPU driver.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
>  drivers/gpu/drm/imagination/pvr_device.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
> index 1704c0268589bdeb65fa6535f9ec63182b0a3e94..f40468b99cf14da418aeecde086f009695ff877c 100644
> --- a/drivers/gpu/drm/imagination/pvr_device.c
> +++ b/drivers/gpu/drm/imagination/pvr_device.c
> @@ -504,10 +504,16 @@ pvr_device_init(struct pvr_device *pvr_dev)
>  	if (err)
>  		return err;
>  
> -	/* Enable and initialize clocks required for the device to operate. */
> -	err = pvr_device_clk_init(pvr_dev);
> -	if (err)
> -		return err;
> +	/*
> +	 * Only initialize clocks if they are not managed by the platform's
> +	 * PM domain.
> +	 */
> +	if (!device_platform_resources_pm_managed(dev)) {
> +		/* Enable and initialize clocks required for the device to operate. */
> +		err = pvr_device_clk_init(pvr_dev);
> +		if (err)
> +			return err;
> +	}

So, how does that work for devfreq? I can understand the rationale for
resets and the sys clock, but the core clock at least should really be
handled by the driver.

Maxime

On 15/04/2025 09:55, Maxime Ripard wrote:
> On Mon, Apr 14, 2025 at 08:52:58PM +0200, Michal Wilczynski wrote:
>> Update the Imagination PVR driver to skip clock management during
>> initialization if the platform PM has indicated that it manages platform
>> resources.
>>
>> This is necessary for platforms like the T-HEAD TH1520, where the GPU's
>> clocks and resets are managed via a PM domain, and should not be
>> manipulated directly by the GPU driver.
>>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>> ---
>>  drivers/gpu/drm/imagination/pvr_device.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
>> index 1704c0268589bdeb65fa6535f9ec63182b0a3e94..f40468b99cf14da418aeecde086f009695ff877c 100644
>> --- a/drivers/gpu/drm/imagination/pvr_device.c
>> +++ b/drivers/gpu/drm/imagination/pvr_device.c
>> @@ -504,10 +504,16 @@ pvr_device_init(struct pvr_device *pvr_dev)
>>  	if (err)
>>  		return err;
>>  
>> -	/* Enable and initialize clocks required for the device to operate. */
>> -	err = pvr_device_clk_init(pvr_dev);
>> -	if (err)
>> -		return err;
>> +	/*
>> +	 * Only initialize clocks if they are not managed by the platform's
>> +	 * PM domain.
>> +	 */
>> +	if (!device_platform_resources_pm_managed(dev)) {
>> +		/* Enable and initialize clocks required for the device to operate. */
>> +		err = pvr_device_clk_init(pvr_dev);
>> +		if (err)
>> +			return err;
>> +	}
> 
> So, how does that work for devfreq? I can understand the rationale for
> resets and the sys clock, but the core clock at least should really be
> handled by the driver.

I agree, this feels a bit "all or nothing" to me. There's only one clock
on this platform that has issues, we can still control the other two
just fine.

I thought fixed clocks were the standard mechanism for exposing
non-controllable clocks to device drivers?

Cheers,
Matt

> 
> Maxime

On 4/15/25 11:15, Matt Coster wrote:
> On 15/04/2025 09:55, Maxime Ripard wrote:
>> On Mon, Apr 14, 2025 at 08:52:58PM +0200, Michal Wilczynski wrote:
>>> Update the Imagination PVR driver to skip clock management during
>>> initialization if the platform PM has indicated that it manages platform
>>> resources.
>>>
>>> This is necessary for platforms like the T-HEAD TH1520, where the GPU's
>>> clocks and resets are managed via a PM domain, and should not be
>>> manipulated directly by the GPU driver.
>>>
>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>>> ---
>>>  drivers/gpu/drm/imagination/pvr_device.c | 14 ++++++++++----
>>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
>>> index 1704c0268589bdeb65fa6535f9ec63182b0a3e94..f40468b99cf14da418aeecde086f009695ff877c 100644
>>> --- a/drivers/gpu/drm/imagination/pvr_device.c
>>> +++ b/drivers/gpu/drm/imagination/pvr_device.c
>>> @@ -504,10 +504,16 @@ pvr_device_init(struct pvr_device *pvr_dev)
>>>  	if (err)
>>>  		return err;
>>>  
>>> -	/* Enable and initialize clocks required for the device to operate. */
>>> -	err = pvr_device_clk_init(pvr_dev);
>>> -	if (err)
>>> -		return err;
>>> +	/*
>>> +	 * Only initialize clocks if they are not managed by the platform's
>>> +	 * PM domain.
>>> +	 */
>>> +	if (!device_platform_resources_pm_managed(dev)) {
>>> +		/* Enable and initialize clocks required for the device to operate. */
>>> +		err = pvr_device_clk_init(pvr_dev);
>>> +		if (err)
>>> +			return err;
>>> +	}
>>
>> So, how does that work for devfreq? I can understand the rationale for
>> resets and the sys clock, but the core clock at least should really be
>> handled by the driver.

Hi Maxime, Matt,

Thanks for the feedback.

This commit is trying to prevent the pvr RUNTIME_PM_OPS from controlling the
clocks or resets, as there is a custom start/stop sequence needed for
the TH1520 SoC coded in patch 3 of this series.

static const struct dev_pm_ops pvr_pm_ops = {
	RUNTIME_PM_OPS(pvr_power_device_suspend, pvr_power_device_resume, pvr_power_device_idle)
};

So, if the core clock needs to be used for other purposes like devfreq,
we could move the device_platform_resources_pm_managed() check to the
pvr_power_* functions instead. This would prevent the clocks and resets
from being managed in runtime PM in the consumer driver, while still
allowing the GPU driver to access and control clocks like the core clock
as needed for other purposes.

That way, clocks could be safely shared between the PM domain driver and the
device driver, with generic PM driver controlling the start/stop
sequence for reset and clocks. We would probably need to find a
better name for the flag then, to more clearly reflect that it's about
delegating clock/reset PM runtime control, rather than full resource
ownership.

> 
> I agree, this feels a bit "all or nothing" to me. There's only one clock
> on this platform that has issues, we can still control the other two
> just fine.
> 
> I thought fixed clocks were the standard mechanism for exposing
> non-controllable clocks to device drivers?

That's correct — and it's not really about the MEM clock at this point.
The main goal is to ensure the custom power-up sequence for the TH1520
SoC is followed. That sequence is implemented in
th1520_gpu_domain_start() in patch 3 of this series.

Regards,
Michał

> 
> Cheers,
> Matt
> 
>>
>> Maxime
> 
>

On Mon, Apr 14, 2025 at 08:52:56PM +0200, Michal Wilczynski wrote:
> Extend the TH1520 AON firmware bindings to describe the GPU clkgen reset
> line, required for proper GPU clock and reset sequencing.
> 
> The T-HEAD TH1520 GPU requires coordinated management of two clocks
> (core and sys) and two resets (GPU core reset and GPU clkgen
> reset).  Only the clkgen reset is exposed at the AON level, to support
> SoC-specific initialization handled through a generic PM domain. The GPU
> core reset remains described in the GPU device node, as from the GPU
> driver's perspective, there is only a single reset line [1].
> 
> This follows upstream maintainers' recommendations [2] to abstract
> SoC specific details into the PM domain layer rather than exposing them
> to drivers directly.
> 
> [1] - https://lore.kernel.org/all/816db99d-7088-4c1a-af03-b9a825ac09dc@imgtec.com/
> [2] - https://lore.kernel.org/all/38d9650fc11a674c8b689d6bab937acf@kernel.org/
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
>  .../devicetree/bindings/firmware/thead,th1520-aon.yaml        | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
> index bbc183200400de7aadbb21fea21911f6f4227b09..6ea3029c222df9ba6ea7d423b92ba248cfb02cc0 100644
> --- a/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
> +++ b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
> @@ -32,6 +32,13 @@ properties:
>      items:
>        - const: aon
>  
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    items:
> +      - const: gpu-clkgen
> +
>    "#power-domain-cells":
>      const: 1
>  
> @@ -39,6 +46,8 @@ required:
>    - compatible
>    - mboxes
>    - mbox-names
> +  - resets
> +  - reset-names

Given these are new required properties, have you made sure in the
driver that their absence will not cause problems with older
devicetrees? I took a brief look at the driver, and it _looked_ like you
were failing if they were not there? It was a brief look though, tbf.

>    - "#power-domain-cells"
>  
>  additionalProperties: false
> @@ -49,5 +58,7 @@ examples:
>          compatible = "thead,th1520-aon";
>          mboxes = <&mbox_910t 1>;
>          mbox-names = "aon";
> +        resets = <&rst 0>;
> +        reset-names = "gpu-clkgen";
>          #power-domain-cells = <1>;
>      };
> 
> -- 
> 2.34.1
>

On 4/15/25 18:38, Conor Dooley wrote:
> On Mon, Apr 14, 2025 at 08:52:56PM +0200, Michal Wilczynski wrote:
>> Extend the TH1520 AON firmware bindings to describe the GPU clkgen reset
>> line, required for proper GPU clock and reset sequencing.
>>
>> The T-HEAD TH1520 GPU requires coordinated management of two clocks
>> (core and sys) and two resets (GPU core reset and GPU clkgen
>> reset).  Only the clkgen reset is exposed at the AON level, to support
>> SoC-specific initialization handled through a generic PM domain. The GPU
>> core reset remains described in the GPU device node, as from the GPU
>> driver's perspective, there is only a single reset line [1].
>>
>> This follows upstream maintainers' recommendations [2] to abstract
>> SoC specific details into the PM domain layer rather than exposing them
>> to drivers directly.
>>
>> [1] - https://lore.kernel.org/all/816db99d-7088-4c1a-af03-b9a825ac09dc@imgtec.com/
>> [2] - https://lore.kernel.org/all/38d9650fc11a674c8b689d6bab937acf@kernel.org/
>>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>> ---
>>  .../devicetree/bindings/firmware/thead,th1520-aon.yaml        | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
>> index bbc183200400de7aadbb21fea21911f6f4227b09..6ea3029c222df9ba6ea7d423b92ba248cfb02cc0 100644
>> --- a/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
>> +++ b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
>> @@ -32,6 +32,13 @@ properties:
>>      items:
>>        - const: aon
>>  
>> +  resets:
>> +    maxItems: 1
>> +
>> +  reset-names:
>> +    items:
>> +      - const: gpu-clkgen
>> +
>>    "#power-domain-cells":
>>      const: 1
>>  
>> @@ -39,6 +46,8 @@ required:
>>    - compatible
>>    - mboxes
>>    - mbox-names
>> +  - resets
>> +  - reset-names
> 
> Given these are new required properties, have you made sure in the
> driver that their absence will not cause problems with older
> devicetrees? I took a brief look at the driver, and it _looked_ like you
> were failing if they were not there? It was a brief look though, tbf.

Hi Conor,

Good point — but in this case, the devicetrees compatible with the
driver haven’t been merged upstream yet. In fact, the TH1520 PM domains
driver currently doesn’t even compile against mainline, since the
required commit [1] didn’t make it into 6.15.

That said, Drew has queued the DT changes for the next release [2], and
you’ve queued [1], so assuming this series lands in 6.16, there won’t be
any older devicetrees to support. As a result, I haven’t added a
fallback path in the driver for missing properties.

If, however this series doesn’t make it in for 6.16, then yes — we’d
need to revisit the driver and add a failure safe path for cases where
these properties aren’t present.

Thanks,
Michał

[1] - https://lore.kernel.org/all/20250407-synergy-staff-b1cec90ffe72@spud/
[2] - https://lore.kernel.org/all/Z%2F6p6MQDS8ZlQv5r@x1/

> 
>>    - "#power-domain-cells"
>>  
>>  additionalProperties: false
>> @@ -49,5 +58,7 @@ examples:
>>          compatible = "thead,th1520-aon";
>>          mboxes = <&mbox_910t 1>;
>>          mbox-names = "aon";
>> +        resets = <&rst 0>;
>> +        reset-names = "gpu-clkgen";
>>          #power-domain-cells = <1>;
>>      };
>>
>> -- 
>> 2.34.1
>>