Re: [PATCH v3 0/8] Add TH1520 GPU support with power sequencing

[Drew Fustini <drew@pdp7.com>]

On Fri, May 30, 2025 at 12:23:47AM +0200, Michal Wilczynski wrote:
> This patch series introduces support for the Imagination IMG BXM-4-64
> GPU found on the T-HEAD TH1520 SoC. A key aspect of this support is
> managing the GPU's complex power-up and power-down sequence, which
> involves multiple clocks and resets.
> 
> The TH1520 GPU requires a specific sequence to be followed for its
> clocks and resets to ensure correct operation. Initial discussions and
> an earlier version of this series explored managing this via the generic
> power domain (genpd) framework. However, following further discussions
> with kernel maintainers [1], the approach has been reworked to utilize
> the dedicated power sequencing (pwrseq) framework.
> 
> This revised series now employs a new pwrseq provider driver
> (pwrseq-thead-gpu.c) specifically for the TH1520 GPU. This driver
> encapsulates the SoC specific power sequence details. The Imagination
> GPU driver (pvr_device.c) is updated to act as a consumer of this power
> sequencer, requesting the "gpu-power" target. The sequencer driver,
> during its match phase with the GPU device, acquires the necessary clock
> and reset handles from the GPU device node to perform the full sequence.
> 
> This approach aligns with the goal of abstracting SoC specific power
> management details away from generic device drivers and leverages the
> pwrseq framework as recommended.
> 
> The series is structured as follows:
> 
> Patch 1: Adds device tree bindings for the new T-HEAD TH1520 GPU
>          power sequencer provider.
> Patch 2: Introduces the pwrseq-thead-gpu driver to manage the GPU's
>          power-on/off sequence.
> Patch 3: Updates the Imagination DRM driver to utilize the pwrseq
>          framework for TH1520 GPU power management.
> Patch 4: Adds the TH1520 GPU compatible string to the Imagination
>          GPU DT bindings.
> Patch 5: Adds the missing reset controller header include in the
>          TH1520 DTS include file.
> Patch 6: Adds the device tree node for the GPU power sequencer to
>          the TH1520 DTS include file.
> Patch 7: Adds the GPU device tree node for the IMG BXM-4-64 GPU to
>          the TH1520 DTS include file.
> Patch 8: Enables compilation of the drm/imagination on the RISC-V
>          architecture
> 
> This patchset finishes the work started in bigger series [2] by adding
> all the remaining GPU power sequencing piece. After this patchset the GPU
> probes correctly.
> 
> This series supersedes the previous genpd based approach. Testing on
> T-HEAD TH1520 SoC indicates the new pwrseq based solution works
> correctly.
> 
> This time it's based on linux-next, as there are dependent patches not
> yet merged, but present in linux-next like clock and reset patches.
> 
> An open point in Patch 7/8 concerns the GPU memory clock (gpu_mem_clk),
> defined as a fixed-clock. The specific hardware frequency for this clock
> on the TH1520 could not be determined from available public
> documentation. Consequently, clock-frequency = <0>; has been used as a
> placeholder to enable driver functionality.
> 
> Link to v2 of this series - [3].
> 
> v3:
> 
>  - re-worked cover letter completely
>  - complete architectural rework from using extended genpd callbacks to a
>    dedicated pwrseq provider driver
>  - introduced pwrseq-thead-gpu.c and associated DT bindings
>    (thead,th1520-gpu-pwrseq)
>  - the Imagination driver now calls devm_pwrseq_get() and uses
>    pwrseq_power_on() / pwrseq_power_off() for the TH1520 GPU
>  - removed the platform_resources_managed flag from dev_pm_info and
>    associated logic
>  - the new pwrseq driver's match() function now acquires consumer-specific
>    resources (GPU clocks, GPU core reset) directly from the consumer device
> 
> 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/CAPDyKFpi6_CD++a9sbGBvJCuBSQS6YcpNttkRQhQMTWy1yyrRg@mail.gmail.com/
> [2] - https://lore.kernel.org/all/20250219140239.1378758-1-m.wilczynski@samsung.com/
> [3] - https://lore.kernel.org/all/20250414-apr_14_for_sending-v2-0-70c5af2af96c@samsung.com/
> 
> ---
> Michal Wilczynski (8):
>       dt-bindings: power: Add T-HEAD TH1520 GPU power sequencer
>       power: sequencing: Add T-HEAD TH1520 GPU power sequencer driver
>       drm/imagination: Use pwrseq for TH1520 GPU power management
>       dt-bindings: gpu: Add TH1520 GPU compatible to Imagination bindings
>       riscv: dts: thead: th1520: Add missing reset controller header include
>       riscv: dts: thead: Add GPU power sequencer node
>       riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node
>       drm/imagination: Enable PowerVR driver for RISC-V
> 
>  .../devicetree/bindings/gpu/img,powervr-rogue.yaml |   9 +-
>  .../bindings/power/thead,th1520-pwrseq.yaml        |  42 +++++
>  MAINTAINERS                                        |   2 +
>  arch/riscv/boot/dts/thead/th1520.dtsi              |  29 ++++
>  drivers/gpu/drm/imagination/Kconfig                |   3 +-
>  drivers/gpu/drm/imagination/pvr_device.c           |  33 +++-
>  drivers/gpu/drm/imagination/pvr_device.h           |   6 +
>  drivers/gpu/drm/imagination/pvr_power.c            |  82 +++++----
>  drivers/power/sequencing/Kconfig                   |   8 +
>  drivers/power/sequencing/Makefile                  |   1 +
>  drivers/power/sequencing/pwrseq-thead-gpu.c        | 183 +++++++++++++++++++++
>  11 files changed, 363 insertions(+), 35 deletions(-)
> ---
> base-commit: 49473fe7fdb5fbbe5bbfa51083792c17df63d317
> change-id: 20250414-apr_14_for_sending-5b3917817acc
> 
> Best regards,
> -- 
> Michal Wilczynski <m.wilczynski@samsung.com>
> 

Thank you for continuing to work on this series.

I applied it to next-20250530 and the boot hangs:

<snip>
[    0.895622] mmc0: new HS400 MMC card at address 0001
[    0.902638] mmcblk0: mmc0:0001 8GTF4R 7.28 GiB
[    0.915454]  mmcblk0: p1 p2 p3
[    0.916613] debug_vm_pgtable: [debug_vm_pgtable         ]: Validating architecture page table helpers
[    0.920107] mmcblk0boot0: mmc0:0001 8GTF4R 4.00 MiB
[    0.936592] mmcblk0boot1: mmc0:0001 8GTF4R 4.00 MiB
[    0.944986] mmcblk0rpmb: mmc0:0001 8GTF4R 512 KiB, chardev (243:0)
[    0.947700] mmc1: new UHS-I speed DDR50 SDHC card at address aaaa
[    0.961368] mmcblk1: mmc1:aaaa SU16G 14.8 GiB
[    0.969639]  mmcblk1: p1 p2 p3
[    0.986688] printk: legacy console [ttyS0] disabled
[    0.992468] ffe7014000.serial: ttyS0 at MMIO 0xffe7014000 (irq = 23, base_baud = 6250000) is a 16550A
[    1.002085] printk: legacy console [ttyS0] enabled
[    1.002085] printk: legacy console [ttyS0] enabled
[    1.011784] printk: legacy bootconsole [uart0] disabled
[    1.011784] printk: legacy bootconsole [uart0] disabled
[    1.024633] stackdepot: allocating hash table of 524288 entries via kvcalloc
<no more output>

I pasted the full boot log [1]. I have clk_ignore_unused in the kernel
cmdline so I don't think it is related to disabling clocks. Boot does
complete okay if I set the gpu node status to disabled.

Any ideas of what might fix the boot hang?

Thanks,
Drew

[1] https://gist.github.com/pdp7/44bd6de63fb9274a66a705ad807690b6