| Message ID | 20231030132257.85379-4-angelogioacchino.delregno@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panfrost: Turn off clocks and regulators in PM | expand |
On 30/10/2023 13:22, AngeloGioacchino Del Regno wrote: > Some platforms/SoCs can power off the GPU entirely by completely cutting > off power, greatly enhancing battery time during system suspend: add a > new pm_feature GPU_PM_VREG_OFF to allow turning off the GPU regulators > during full suspend only on selected platforms. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_device.c | 19 ++++++++++++++++++- > drivers/gpu/drm/panfrost/panfrost_device.h | 2 ++ > 2 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c > index 2022ed76a620..51b22eb0971d 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c > @@ -431,10 +431,21 @@ 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_VREG_OFF)) { > + unsigned long freq = pfdev->pfdevfreq.fast_rate; > + struct dev_pm_opp *opp; > + > + opp = dev_pm_opp_find_freq_ceil(dev, &freq); > + if (IS_ERR(opp)) > + return PTR_ERR(opp); > + dev_pm_opp_put(opp); > + dev_pm_opp_set_opp(dev, opp); These lines look the wrong way round - surely we want to hang onto the reference until the set_opp call is made? Steve > + } > + > if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) { > ret = clk_enable(pfdev->clock); > if (ret) > - return ret; > + goto err_clk; > > if (pfdev->bus_clock) { > ret = clk_enable(pfdev->bus_clock); > @@ -455,6 +466,9 @@ static int panfrost_device_resume(struct device *dev) > err_bus_clk: > if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) > clk_disable(pfdev->clock); > +err_clk: > + if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) > + dev_pm_opp_set_opp(dev, NULL); > return ret; > } > > @@ -474,6 +488,9 @@ static int panfrost_device_suspend(struct device *dev) > clk_disable(pfdev->bus_clock); > } > > + if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) > + dev_pm_opp_set_opp(dev, NULL); > + > return 0; > } > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h > index d7f179eb8ea3..0fc558db6bfd 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -28,9 +28,11 @@ struct panfrost_perfcnt; > /** > * 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 > */ > enum panfrost_gpu_pm { > GPU_PM_CLK_DIS, > + GPU_PM_VREG_OFF, > }; > > struct panfrost_features {
Il 30/10/23 15:57, Steven Price ha scritto: > On 30/10/2023 13:22, AngeloGioacchino Del Regno wrote: >> Some platforms/SoCs can power off the GPU entirely by completely cutting >> off power, greatly enhancing battery time during system suspend: add a >> new pm_feature GPU_PM_VREG_OFF to allow turning off the GPU regulators >> during full suspend only on selected platforms. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> --- >> drivers/gpu/drm/panfrost/panfrost_device.c | 19 ++++++++++++++++++- >> drivers/gpu/drm/panfrost/panfrost_device.h | 2 ++ >> 2 files changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c >> index 2022ed76a620..51b22eb0971d 100644 >> --- a/drivers/gpu/drm/panfrost/panfrost_device.c >> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c >> @@ -431,10 +431,21 @@ 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_VREG_OFF)) { >> + unsigned long freq = pfdev->pfdevfreq.fast_rate; >> + struct dev_pm_opp *opp; >> + >> + opp = dev_pm_opp_find_freq_ceil(dev, &freq); >> + if (IS_ERR(opp)) >> + return PTR_ERR(opp); >> + dev_pm_opp_put(opp); >> + dev_pm_opp_set_opp(dev, opp); > > These lines look the wrong way round - surely we want to hang onto the > reference until the set_opp call is made? > Whoops! Yes, thanks, this happened during a cleanup for whatever reason. I'll invert those for v2. Angelo > Steve > >> + } >> + >> if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) { >> ret = clk_enable(pfdev->clock); >> if (ret) >> - return ret; >> + goto err_clk; >> >> if (pfdev->bus_clock) { >> ret = clk_enable(pfdev->bus_clock); >> @@ -455,6 +466,9 @@ static int panfrost_device_resume(struct device *dev) >> err_bus_clk: >> if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) >> clk_disable(pfdev->clock); >> +err_clk: >> + if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) >> + dev_pm_opp_set_opp(dev, NULL); >> return ret; >> } >> >> @@ -474,6 +488,9 @@ static int panfrost_device_suspend(struct device *dev) >> clk_disable(pfdev->bus_clock); >> } >> >> + if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) >> + dev_pm_opp_set_opp(dev, NULL); >> + >> return 0; >> } >> >> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h >> index d7f179eb8ea3..0fc558db6bfd 100644 >> --- a/drivers/gpu/drm/panfrost/panfrost_device.h >> +++ b/drivers/gpu/drm/panfrost/panfrost_device.h >> @@ -28,9 +28,11 @@ struct panfrost_perfcnt; >> /** >> * 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 >> */ >> enum panfrost_gpu_pm { >> GPU_PM_CLK_DIS, >> + GPU_PM_VREG_OFF, >> }; >> >> struct panfrost_features { > > _______________________________________________ > Kernel mailing list -- kernel@mailman.collabora.com > To unsubscribe send an email to kernel-leave@mailman.collabora.com
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 2022ed76a620..51b22eb0971d 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -431,10 +431,21 @@ 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_VREG_OFF)) { + unsigned long freq = pfdev->pfdevfreq.fast_rate; + struct dev_pm_opp *opp; + + opp = dev_pm_opp_find_freq_ceil(dev, &freq); + if (IS_ERR(opp)) + return PTR_ERR(opp); + dev_pm_opp_put(opp); + dev_pm_opp_set_opp(dev, opp); + } + if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) { ret = clk_enable(pfdev->clock); if (ret) - return ret; + goto err_clk; if (pfdev->bus_clock) { ret = clk_enable(pfdev->bus_clock); @@ -455,6 +466,9 @@ static int panfrost_device_resume(struct device *dev) err_bus_clk: if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) clk_disable(pfdev->clock); +err_clk: + if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) + dev_pm_opp_set_opp(dev, NULL); return ret; } @@ -474,6 +488,9 @@ static int panfrost_device_suspend(struct device *dev) clk_disable(pfdev->bus_clock); } + if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) + dev_pm_opp_set_opp(dev, NULL); + return 0; } diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index d7f179eb8ea3..0fc558db6bfd 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -28,9 +28,11 @@ struct panfrost_perfcnt; /** * 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 */ enum panfrost_gpu_pm { GPU_PM_CLK_DIS, + GPU_PM_VREG_OFF, }; struct panfrost_features {
Some platforms/SoCs can power off the GPU entirely by completely cutting off power, greatly enhancing battery time during system suspend: add a new pm_feature GPU_PM_VREG_OFF to allow turning off the GPU regulators during full suspend only on selected platforms. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_device.c | 19 ++++++++++++++++++- drivers/gpu/drm/panfrost/panfrost_device.h | 2 ++ 2 files changed, 20 insertions(+), 1 deletion(-)