On 09/11/2023 10:25, AngeloGioacchino Del Regno wrote: > Currently, the GPU is being internally powered off for runtime suspend > and turned back on for runtime resume through commands sent to it, but > note that the GPU doesn't need to be clocked during the poweroff state, > hence it is possible to save some power on selected platforms. > > Add suspend and resume handlers for full system sleep and then add > a new panfrost_gpu_pm enumeration and a pm_features variable in the > panfrost_compatible structure: BIT(GPU_PM_CLK_DIS) will be used to > enable this power saving technique only on SoCs that are able to > safely use it. > > Note that this was implemented only for the system sleep case and not > for runtime PM because testing on one of my MediaTek platforms showed > issues when turning on and off clocks aggressively (in PM runtime) > resulting in a full system lockup. > > Doing this only for full system sleep never showed issues during my > testing by suspending and resuming the system continuously for more > than 100 cycles. > > Signed-off-by: AngeloGioacchino Del Regno > <angelogioacchino.delre...@collabora.com>
Reviewed-by: Steven Price <steven.pr...@arm.com> > --- > drivers/gpu/drm/panfrost/panfrost_device.c | 61 ++++++++++++++++++++-- > drivers/gpu/drm/panfrost/panfrost_device.h | 11 ++++ > 2 files changed, 68 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c > b/drivers/gpu/drm/panfrost/panfrost_device.c > index 28f7046e1b1a..b4ddbc3b8069 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c > @@ -403,7 +403,7 @@ void panfrost_device_reset(struct panfrost_device *pfdev) > panfrost_job_enable_interrupts(pfdev); > } > > -static int panfrost_device_resume(struct device *dev) > +static int panfrost_device_runtime_resume(struct device *dev) > { > struct panfrost_device *pfdev = dev_get_drvdata(dev); > > @@ -413,7 +413,7 @@ static int panfrost_device_resume(struct device *dev) > return 0; > } > > -static int panfrost_device_suspend(struct device *dev) > +static int panfrost_device_runtime_suspend(struct device *dev) > { > struct panfrost_device *pfdev = dev_get_drvdata(dev); > > @@ -426,5 +426,58 @@ static int panfrost_device_suspend(struct device *dev) > return 0; > } > > -EXPORT_GPL_RUNTIME_DEV_PM_OPS(panfrost_pm_ops, panfrost_device_suspend, > - panfrost_device_resume, NULL); > +static int panfrost_device_resume(struct device *dev) > +{ > + struct panfrost_device *pfdev = dev_get_drvdata(dev); > + int ret; > + > + if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) { > + ret = clk_enable(pfdev->clock); > + if (ret) > + return ret; > + > + if (pfdev->bus_clock) { > + ret = clk_enable(pfdev->bus_clock); > + if (ret) > + goto err_bus_clk; > + } > + } > + > + ret = pm_runtime_force_resume(dev); > + if (ret) > + goto err_resume; > + > + return 0; > + > +err_resume: > + if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS) && pfdev->bus_clock) > + clk_disable(pfdev->bus_clock); > +err_bus_clk: > + if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) > + clk_disable(pfdev->clock); > + return ret; > +} > + > +static int panfrost_device_suspend(struct device *dev) > +{ > + struct panfrost_device *pfdev = dev_get_drvdata(dev); > + int ret; > + > + ret = pm_runtime_force_suspend(dev); > + if (ret) > + return ret; > + > + if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) { > + if (pfdev->bus_clock) > + clk_disable(pfdev->bus_clock); > + > + clk_disable(pfdev->clock); > + } > + > + return 0; > +} > + > +EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = { > + RUNTIME_PM_OPS(panfrost_device_runtime_suspend, > panfrost_device_runtime_resume, NULL) > + SYSTEM_SLEEP_PM_OPS(panfrost_device_suspend, panfrost_device_resume) > +}; > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h > b/drivers/gpu/drm/panfrost/panfrost_device.h > index 1ef38f60d5dc..d7f179eb8ea3 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -25,6 +25,14 @@ struct panfrost_perfcnt; > #define NUM_JOB_SLOTS 3 > #define MAX_PM_DOMAINS 5 > > +/** > + * enum panfrost_gpu_pm - Supported kernel power management features > + * @GPU_PM_CLK_DIS: Allow disabling clocks during system suspend > + */ > +enum panfrost_gpu_pm { > + GPU_PM_CLK_DIS, > +}; > + > struct panfrost_features { > u16 id; > u16 revision; > @@ -75,6 +83,9 @@ struct panfrost_compatible { > > /* Vendor implementation quirks callback */ > void (*vendor_quirk)(struct panfrost_device *pfdev); > + > + /* Allowed PM features */ > + u8 pm_features; > }; > > struct panfrost_device {