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: <20250327123628.3d33c68e@donnerap.manchester.arm.com>
Date: Thu, 27 Mar 2025 12:36:28 +0000
From: Andre Przywara <andre.przywara@....com>
To: Boris Brezillon <boris.brezillon@...labora.com>, Rob Herring
 <robh@...nel.org>, Steven Price <steven.price@....com>
Cc: Philippe Simons <simons.philippe@...il.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>, Philipp Zabel <p.zabel@...gutronix.de>,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 linux-sunxi@...ts.linux.dev, Jernej Škrabec
 <jernej.skrabec@...il.com>
Subject: Re: [PATCH 1/2] drm/panfrost: Add PM runtime flags

On Thu, 13 Mar 2025 00:23:18 +0100
Philippe Simons <simons.philippe@...il.com> wrote:

Hi Rob, Boris, Steven,

> When the GPU is the only device attached to a single power domain,
> core genpd disable and enable it when gpu enter and leave runtime suspend.
> 
> Some power-domain requires a sequence before disabled,
> and the reverse when enabled.
> 
> Add PM flags for CLK and RST, and implement in
> panfrost_device_runtime_suspend/resume.

So some Mali configuration and integration manual I am looking at says
that this sequence should be always observed, as the powerdown sequence
would include disabling the clocks first, then asserting the reset, then
turning the power switches off (and the inverse sequence on powerup).

So should we make this unconditional, not depending on implementation
specific flags?

And also I am wondering if panfrost_device_init() gets this wrong as well?
As I see it enabling clock first, then reset, then pm_domain, where it
should be exactly the opposite?

Cheers,
Andre

> 
> Signed-off-by: Philippe Simons <simons.philippe@...il.com>
> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c | 37 ++++++++++++++++++++++
>  drivers/gpu/drm/panfrost/panfrost_device.h |  4 +++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index a45e4addcc19..189ad2ad2b32 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -406,11 +406,38 @@ void panfrost_device_reset(struct panfrost_device *pfdev)
>  static int panfrost_device_runtime_resume(struct device *dev)
>  {
>  	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT_RST_ASRT)) {
> +		ret = reset_control_deassert(pfdev->rstc);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT_CLK_DIS)) {
> +		ret = clk_enable(pfdev->clock);
> +		if (ret)
> +			goto err_clk;
> +
> +		if (pfdev->bus_clock) {
> +			ret = clk_enable(pfdev->bus_clock);
> +			if (ret)
> +				goto err_bus_clk;
> +		}
> +	}
>  
>  	panfrost_device_reset(pfdev);
>  	panfrost_devfreq_resume(pfdev);
>  
>  	return 0;
> +
> +err_bus_clk:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT_CLK_DIS))
> +		clk_disable(pfdev->clock);
> +err_clk:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT_RST_ASRT))
> +		reset_control_assert(pfdev->rstc);
> +	return ret;
>  }
>  
>  static int panfrost_device_runtime_suspend(struct device *dev)
> @@ -426,6 +453,16 @@ static int panfrost_device_runtime_suspend(struct device *dev)
>  	panfrost_gpu_suspend_irq(pfdev);
>  	panfrost_gpu_power_off(pfdev);
>  
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT_CLK_DIS)) {
> +		if (pfdev->bus_clock)
> +			clk_disable(pfdev->bus_clock);
> +
> +		clk_disable(pfdev->clock);
> +	}
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT_RST_ASRT))
> +		reset_control_assert(pfdev->rstc);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
> index cffcb0ac7c11..f372d4819262 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.h
> @@ -36,10 +36,14 @@ enum panfrost_drv_comp_bits {
>   * enum panfrost_gpu_pm - Supported kernel power management features
>   * @GPU_PM_CLK_DIS:  Allow disabling clocks during system suspend
>   * @GPU_PM_VREG_OFF: Allow turning off regulators during system suspend
> + * @GPU_PM_RT_CLK_DIS: Allow disabling clocks during system runtime suspend
> + * @GPU_PM_RST_ASRT: Allow asserting the reset control during runtime suspend
>   */
>  enum panfrost_gpu_pm {
>  	GPU_PM_CLK_DIS,
>  	GPU_PM_VREG_OFF,
> +	GPU_PM_RT_CLK_DIS,
> +	GPU_PM_RT_RST_ASRT
>  };
>  
>  struct panfrost_features {


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ