linux-kernel - Re: [PATCH v3 3/8] drm/imagination: Use pwrseq for TH1520 GPU power management

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250603-whispering-jaybird-of-thunder-f87867@kuoka>
Date: Tue, 3 Jun 2025 15:28:58 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Michal Wilczynski <m.wilczynski@...sung.com>
Cc: Drew Fustini <drew@...7.com>, Guo Ren <guoren@...nel.org>, 
	Fu Wei <wefu@...hat.com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Bartosz Golaszewski <brgl@...ev.pl>, Philipp Zabel <p.zabel@...gutronix.de>, 
	Frank Binns <frank.binns@...tec.com>, Matt Coster <matt.coster@...tec.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, 
	Alexandre Ghiti <alex@...ti.fr>, Ulf Hansson <ulf.hansson@...aro.org>, 
	Marek Szyprowski <m.szyprowski@...sung.com>, linux-riscv@...ts.infradead.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v3 3/8] drm/imagination: Use pwrseq for TH1520 GPU power
 management

On Fri, May 30, 2025 at 12:23:50AM GMT, Michal Wilczynski wrote:
> Update the Imagination PVR DRM driver to leverage the pwrseq framework
> for managing the power sequence of the GPU on the T-HEAD TH1520 SoC.
> 
> In pvr_device_init(), the driver now attempts to get a handle to the
> "gpu-power" sequencer target using devm_pwrseq_get(). If successful,
> the responsibility for powering on and off the GPU's core clocks and
> resets is delegated to the power sequencer. Consequently, the GPU
> driver conditionally skips acquiring the GPU reset line if the pwrseq
> handle is obtained, as the sequencer's match function will acquire it.
> Clock handles are still acquired by the GPU driver for other purposes
> like devfreq.
> 
> The runtime PM callbacks, pvr_power_device_resume() and
> pvr_power_device_suspend(), are modified to call pwrseq_power_on() and
> pwrseq_power_off() respectively when the sequencer is present.  If no
> sequencer is found, the driver falls back to its existing manual clock
> and reset management. A helper function,
> pvr_power_off_sequence_manual(), is introduced to encapsulate the manual
> power-down logic.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@...sung.com>
> ---
>  drivers/gpu/drm/imagination/Kconfig      |  1 +
>  drivers/gpu/drm/imagination/pvr_device.c | 33 +++++++++++--
>  drivers/gpu/drm/imagination/pvr_device.h |  6 +++
>  drivers/gpu/drm/imagination/pvr_power.c  | 82 +++++++++++++++++++++-----------
>  4 files changed, 89 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/Kconfig b/drivers/gpu/drm/imagination/Kconfig
> index 3bfa2ac212dccb73c53bdc2bc259bcba636e7cfc..737ace77c4f1247c687cc1fde2f139fc2e118c50 100644
> --- a/drivers/gpu/drm/imagination/Kconfig
> +++ b/drivers/gpu/drm/imagination/Kconfig
> @@ -11,6 +11,7 @@ config DRM_POWERVR
>  	select DRM_SCHED
>  	select DRM_GPUVM
>  	select FW_LOADER
> +  select POWER_SEQUENCING

Messed indent.

>  	help
>  	  Choose this option if you have a system that has an Imagination
>  	  Technologies PowerVR (Series 6 or later) or IMG GPU.
> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
> index 8b9ba4983c4cb5bc40342fcafc4259078bc70547..19d48bbc828cf2b8dbead602e90ff88780152124 100644
> --- a/drivers/gpu/drm/imagination/pvr_device.c
> +++ b/drivers/gpu/drm/imagination/pvr_device.c
> @@ -25,6 +25,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/pwrseq/consumer.h>
>  #include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/stddef.h>
> @@ -631,10 +632,34 @@ pvr_device_init(struct pvr_device *pvr_dev)
>  	if (err)
>  		return err;
>  
> -	/* Get the reset line for the GPU */
> -	err = pvr_device_reset_init(pvr_dev);
> -	if (err)
> -		return err;
> +	/*
> +	 * Try to get a power sequencer. If successful, it will handle clocks
> +	 * and resets. Otherwise, we fall back to managing them ourselves.
> +	 */
> +	pvr_dev->pwrseq = devm_pwrseq_get(dev, "gpu-power");
> +	if (IS_ERR(pvr_dev->pwrseq)) {
> +		int pwrseq_err = PTR_ERR(pvr_dev->pwrseq);
> +
> +		/*
> +		 * If the error is -EPROBE_DEFER, it's because the
> +		 * optional sequencer provider is not present
> +		 * and it's safe to fall back on manual power-up.

It is safe but why it is desirable? The rule is rather to defer the
probe, assuming this is probe path.

Best regards,
Krzysztof