| Message ID | 1560750919-32255-1-git-send-email-lowry.li@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/komeda: Adds power management support | expand |
Hi Lowry, On Mon, Jun 17, 2019 at 06:55:49AM +0100, Lowry Li (Arm Technology China) wrote: > Adds runtime and system power management support in KMS kernel driver. > > Depends on: > - https://patchwork.freedesktop.org/series/61650/ > - https://patchwork.freedesktop.org/series/60083/ > > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com> > --- > drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 2 + > drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 30 +++++++++++++ > drivers/gpu/drm/arm/display/komeda/komeda_dev.h | 2 + > drivers/gpu/drm/arm/display/komeda/komeda_drv.c | 54 ++++++++++++++++++++++-- > 4 files changed, 85 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > index 66c5e0d..1b4ea8a 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > @@ -257,6 +257,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, > komeda_crtc_atomic_enable(struct drm_crtc *crtc, > struct drm_crtc_state *old) > { > + pm_runtime_get_sync(crtc->dev->dev); > komeda_crtc_prepare(to_kcrtc(crtc)); > drm_crtc_vblank_on(crtc); > komeda_crtc_do_flush(crtc, old); > @@ -330,6 +331,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, > > drm_crtc_vblank_off(crtc); > komeda_crtc_unprepare(kcrtc); > + pm_runtime_put(crtc->dev->dev); > } > > static void > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > index 405c64d..edd0943 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > @@ -308,3 +308,33 @@ void komeda_dev_destroy(struct komeda_dev *mdev) > > devm_kfree(dev, mdev); > } > + > +int komeda_dev_resume(struct komeda_dev *mdev) > +{ > + int ret = 0; > + > + if (mdev->iommu && mdev->funcs->connect_iommu) { > + ret = mdev->funcs->connect_iommu(mdev); > + if (ret < 0) { > + DRM_ERROR("connect iommu failed.\n"); > + return ret; > + } > + } > + > + return mdev->funcs->enable_irq(mdev); > +} > + > +int komeda_dev_suspend(struct komeda_dev *mdev) > +{ > + int ret = 0; > + > + if (mdev->iommu && mdev->funcs->disconnect_iommu) { > + ret = mdev->funcs->disconnect_iommu(mdev); > + if (ret < 0) { > + DRM_ERROR("disconnect iommu failed.\n"); > + return ret; > + } > + } > + > + return mdev->funcs->disable_irq(mdev); > +} > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h > index d1c86b6..096f9f7 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h > @@ -207,4 +207,6 @@ struct komeda_dev { > > struct komeda_dev *dev_to_mdev(struct device *dev); > > +int komeda_dev_resume(struct komeda_dev *mdev); > +int komeda_dev_suspend(struct komeda_dev *mdev); > #endif /*_KOMEDA_DEV_H_*/ > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c > index cfa5068..aa4cef1 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c > @@ -8,6 +8,7 @@ > #include <linux/kernel.h> > #include <linux/platform_device.h> > #include <linux/component.h> > +#include <linux/pm_runtime.h> > #include <drm/drm_of.h> > #include "komeda_dev.h" > #include "komeda_kms.h" > @@ -32,6 +33,9 @@ static void komeda_unbind(struct device *dev) > return; > > komeda_kms_detach(mdrv->kms); > + > + pm_runtime_disable(dev); > + > komeda_dev_destroy(mdrv->mdev); > > dev_set_drvdata(dev, NULL); > @@ -52,6 +56,9 @@ static int komeda_bind(struct device *dev) > err = PTR_ERR(mdrv->mdev); > goto free_mdrv; > } > + dev_set_drvdata(dev, mdrv); > + > + pm_runtime_enable(dev); > > mdrv->kms = komeda_kms_attach(mdrv->mdev); > if (IS_ERR(mdrv->kms)) { > @@ -59,11 +66,10 @@ static int komeda_bind(struct device *dev) > goto destroy_mdev; > } > > - dev_set_drvdata(dev, mdrv); > - > return 0; > > destroy_mdev: > + pm_runtime_disable(dev); > komeda_dev_destroy(mdrv->mdev); > > free_mdrv: > @@ -134,13 +140,55 @@ static int komeda_platform_remove(struct platform_device *pdev) > > MODULE_DEVICE_TABLE(of, komeda_of_match); > > +static int komeda_rt_pm_suspend(struct device *dev) > +{ > + dev_info(dev, "%s\n", __func__); > + return 0; > +} > + > +static int komeda_rt_pm_resume(struct device *dev) > +{ > + dev_info(dev, "%s\n", __func__); > + return 0; > +} Useful only for debugging? I think you've missed an opportunity here to turn off clocks and disable interrupts. > + > +static int __maybe_unused komeda_pm_suspend(struct device *dev) > +{ > + struct komeda_drv *mdrv = dev_get_drvdata(dev); > + struct drm_device *drm = &mdrv->kms->base; > + int res; > + > + dev_info(dev, "%s\n", __func__); > + res = drm_mode_config_helper_suspend(drm); > + > + komeda_dev_suspend(mdrv->mdev); > + > + return res; > +} > + > +static int __maybe_unused komeda_pm_resume(struct device *dev) > +{ > + struct komeda_drv *mdrv = dev_get_drvdata(dev); > + struct drm_device *drm = &mdrv->kms->base; > + > + dev_info(dev, "%s\n", __func__); I don't see the point of printing all these message on suspend/resume. Please convert them to DRM_DEBUG() if you want to have some sort of notices in the kernel log, otherwise please remove them. Best regards, Liviu > + komeda_dev_resume(mdrv->mdev); > + > + return drm_mode_config_helper_resume(drm); > +} > + > +static const struct dev_pm_ops komeda_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(komeda_pm_suspend, komeda_pm_resume) > + SET_RUNTIME_PM_OPS(komeda_rt_pm_suspend, komeda_rt_pm_resume, NULL) > +}; > + > static struct platform_driver komeda_platform_driver = { > .probe = komeda_platform_probe, > .remove = komeda_platform_remove, > .driver = { > .name = "komeda", > .of_match_table = komeda_of_match, > - .pm = NULL, > + .pm = &komeda_pm_ops, > }, > }; > > -- > 1.9.1 >
Hi Liviu, On Thu, Jun 20, 2019 at 12:15:22AM +0800, Liviu Dudau wrote: > Hi Lowry, > > On Mon, Jun 17, 2019 at 06:55:49AM +0100, Lowry Li (Arm Technology China) wrote: > > Adds runtime and system power management support in KMS kernel driver. > > > > Depends on: > > - https://patchwork.freedesktop.org/series/61650/ > > - https://patchwork.freedesktop.org/series/60083/ > > > > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com> > > --- > > drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 2 + > > drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 30 +++++++++++++ > > drivers/gpu/drm/arm/display/komeda/komeda_dev.h | 2 + > > drivers/gpu/drm/arm/display/komeda/komeda_drv.c | 54 ++++++++++++++++++++++-- > > 4 files changed, 85 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > index 66c5e0d..1b4ea8a 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > @@ -257,6 +257,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, > > komeda_crtc_atomic_enable(struct drm_crtc *crtc, > > struct drm_crtc_state *old) > > { > > + pm_runtime_get_sync(crtc->dev->dev); > > komeda_crtc_prepare(to_kcrtc(crtc)); > > drm_crtc_vblank_on(crtc); > > komeda_crtc_do_flush(crtc, old); > > @@ -330,6 +331,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, > > > > drm_crtc_vblank_off(crtc); > > komeda_crtc_unprepare(kcrtc); > > + pm_runtime_put(crtc->dev->dev); > > } > > > > static void > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > index 405c64d..edd0943 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > @@ -308,3 +308,33 @@ void komeda_dev_destroy(struct komeda_dev *mdev) > > > > devm_kfree(dev, mdev); > > } > > + > > +int komeda_dev_resume(struct komeda_dev *mdev) > > +{ > > + int ret = 0; > > + > > + if (mdev->iommu && mdev->funcs->connect_iommu) { > > + ret = mdev->funcs->connect_iommu(mdev); > > + if (ret < 0) { > > + DRM_ERROR("connect iommu failed.\n"); > > + return ret; > > + } > > + } > > + > > + return mdev->funcs->enable_irq(mdev); > > +} > > + > > +int komeda_dev_suspend(struct komeda_dev *mdev) > > +{ > > + int ret = 0; > > + > > + if (mdev->iommu && mdev->funcs->disconnect_iommu) { > > + ret = mdev->funcs->disconnect_iommu(mdev); > > + if (ret < 0) { > > + DRM_ERROR("disconnect iommu failed.\n"); > > + return ret; > > + } > > + } > > + > > + return mdev->funcs->disable_irq(mdev); > > +} > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h > > index d1c86b6..096f9f7 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h > > @@ -207,4 +207,6 @@ struct komeda_dev { > > > > struct komeda_dev *dev_to_mdev(struct device *dev); > > > > +int komeda_dev_resume(struct komeda_dev *mdev); > > +int komeda_dev_suspend(struct komeda_dev *mdev); > > #endif /*_KOMEDA_DEV_H_*/ > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c > > index cfa5068..aa4cef1 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c > > @@ -8,6 +8,7 @@ > > #include <linux/kernel.h> > > #include <linux/platform_device.h> > > #include <linux/component.h> > > +#include <linux/pm_runtime.h> > > #include <drm/drm_of.h> > > #include "komeda_dev.h" > > #include "komeda_kms.h" > > @@ -32,6 +33,9 @@ static void komeda_unbind(struct device *dev) > > return; > > > > komeda_kms_detach(mdrv->kms); > > + > > + pm_runtime_disable(dev); > > + > > komeda_dev_destroy(mdrv->mdev); > > > > dev_set_drvdata(dev, NULL); > > @@ -52,6 +56,9 @@ static int komeda_bind(struct device *dev) > > err = PTR_ERR(mdrv->mdev); > > goto free_mdrv; > > } > > + dev_set_drvdata(dev, mdrv); > > + > > + pm_runtime_enable(dev); > > > > mdrv->kms = komeda_kms_attach(mdrv->mdev); > > if (IS_ERR(mdrv->kms)) { > > @@ -59,11 +66,10 @@ static int komeda_bind(struct device *dev) > > goto destroy_mdev; > > } > > > > - dev_set_drvdata(dev, mdrv); > > - > > return 0; > > > > destroy_mdev: > > + pm_runtime_disable(dev); > > komeda_dev_destroy(mdrv->mdev); > > > > free_mdrv: > > @@ -134,13 +140,55 @@ static int komeda_platform_remove(struct platform_device *pdev) > > > > MODULE_DEVICE_TABLE(of, komeda_of_match); > > > > +static int komeda_rt_pm_suspend(struct device *dev) > > +{ > > + dev_info(dev, "%s\n", __func__); > > + return 0; > > +} > > + > > +static int komeda_rt_pm_resume(struct device *dev) > > +{ > > + dev_info(dev, "%s\n", __func__); > > + return 0; > > +} > > > Useful only for debugging? I think you've missed an opportunity here to turn > off clocks and disable interrupts. Since we have unified mclk/pclk/pipeline->aclk to one mclk, which will be turned on/off when crtc atomic enable/disable, no need to do those here. So runtime pm resume and suspend will be removed. > > + > > +static int __maybe_unused komeda_pm_suspend(struct device *dev) > > +{ > > + struct komeda_drv *mdrv = dev_get_drvdata(dev); > > + struct drm_device *drm = &mdrv->kms->base; > > + int res; > > + > > + dev_info(dev, "%s\n", __func__); > > + res = drm_mode_config_helper_suspend(drm); > > + > > + komeda_dev_suspend(mdrv->mdev); > > + > > + return res; > > +} > > + > > +static int __maybe_unused komeda_pm_resume(struct device *dev) > > +{ > > + struct komeda_drv *mdrv = dev_get_drvdata(dev); > > + struct drm_device *drm = &mdrv->kms->base; > > + > > + dev_info(dev, "%s\n", __func__); > > I don't see the point of printing all these message on suspend/resume. Please > convert them to DRM_DEBUG() if you want to have some sort of notices in the > kernel log, otherwise please remove them. > > Best regards, > Liviu Yes, will remove them. Thanks a lot. Best regards, Lowry > > > + komeda_dev_resume(mdrv->mdev); > > + > > + return drm_mode_config_helper_resume(drm); > > +} > > + > > +static const struct dev_pm_ops komeda_pm_ops = { > > + SET_SYSTEM_SLEEP_PM_OPS(komeda_pm_suspend, komeda_pm_resume) > > + SET_RUNTIME_PM_OPS(komeda_rt_pm_suspend, komeda_rt_pm_resume, NULL) > > +}; > > + > > static struct platform_driver komeda_platform_driver = { > > .probe = komeda_platform_probe, > > .remove = komeda_platform_remove, > > .driver = { > > .name = "komeda", > > .of_match_table = komeda_of_match, > > - .pm = NULL, > > + .pm = &komeda_pm_ops, > > }, > > }; > > > > -- > > 1.9.1 > > > > -- > ==================== > | I would like to | > | fix the world, | > | but they're not | > | giving me the | > \ source code! / > --------------- > ¯\_(ツ)_/¯
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c index 66c5e0d..1b4ea8a 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c @@ -257,6 +257,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, komeda_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old) { + pm_runtime_get_sync(crtc->dev->dev); komeda_crtc_prepare(to_kcrtc(crtc)); drm_crtc_vblank_on(crtc); komeda_crtc_do_flush(crtc, old); @@ -330,6 +331,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, drm_crtc_vblank_off(crtc); komeda_crtc_unprepare(kcrtc); + pm_runtime_put(crtc->dev->dev); } static void diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c index 405c64d..edd0943 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c @@ -308,3 +308,33 @@ void komeda_dev_destroy(struct komeda_dev *mdev) devm_kfree(dev, mdev); } + +int komeda_dev_resume(struct komeda_dev *mdev) +{ + int ret = 0; + + if (mdev->iommu && mdev->funcs->connect_iommu) { + ret = mdev->funcs->connect_iommu(mdev); + if (ret < 0) { + DRM_ERROR("connect iommu failed.\n"); + return ret; + } + } + + return mdev->funcs->enable_irq(mdev); +} + +int komeda_dev_suspend(struct komeda_dev *mdev) +{ + int ret = 0; + + if (mdev->iommu && mdev->funcs->disconnect_iommu) { + ret = mdev->funcs->disconnect_iommu(mdev); + if (ret < 0) { + DRM_ERROR("disconnect iommu failed.\n"); + return ret; + } + } + + return mdev->funcs->disable_irq(mdev); +} diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h index d1c86b6..096f9f7 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h @@ -207,4 +207,6 @@ struct komeda_dev { struct komeda_dev *dev_to_mdev(struct device *dev); +int komeda_dev_resume(struct komeda_dev *mdev); +int komeda_dev_suspend(struct komeda_dev *mdev); #endif /*_KOMEDA_DEV_H_*/ diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c index cfa5068..aa4cef1 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c @@ -8,6 +8,7 @@ #include <linux/kernel.h> #include <linux/platform_device.h> #include <linux/component.h> +#include <linux/pm_runtime.h> #include <drm/drm_of.h> #include "komeda_dev.h" #include "komeda_kms.h" @@ -32,6 +33,9 @@ static void komeda_unbind(struct device *dev) return; komeda_kms_detach(mdrv->kms); + + pm_runtime_disable(dev); + komeda_dev_destroy(mdrv->mdev); dev_set_drvdata(dev, NULL); @@ -52,6 +56,9 @@ static int komeda_bind(struct device *dev) err = PTR_ERR(mdrv->mdev); goto free_mdrv; } + dev_set_drvdata(dev, mdrv); + + pm_runtime_enable(dev); mdrv->kms = komeda_kms_attach(mdrv->mdev); if (IS_ERR(mdrv->kms)) { @@ -59,11 +66,10 @@ static int komeda_bind(struct device *dev) goto destroy_mdev; } - dev_set_drvdata(dev, mdrv); - return 0; destroy_mdev: + pm_runtime_disable(dev); komeda_dev_destroy(mdrv->mdev); free_mdrv: @@ -134,13 +140,55 @@ static int komeda_platform_remove(struct platform_device *pdev) MODULE_DEVICE_TABLE(of, komeda_of_match); +static int komeda_rt_pm_suspend(struct device *dev) +{ + dev_info(dev, "%s\n", __func__); + return 0; +} + +static int komeda_rt_pm_resume(struct device *dev) +{ + dev_info(dev, "%s\n", __func__); + return 0; +} + +static int __maybe_unused komeda_pm_suspend(struct device *dev) +{ + struct komeda_drv *mdrv = dev_get_drvdata(dev); + struct drm_device *drm = &mdrv->kms->base; + int res; + + dev_info(dev, "%s\n", __func__); + res = drm_mode_config_helper_suspend(drm); + + komeda_dev_suspend(mdrv->mdev); + + return res; +} + +static int __maybe_unused komeda_pm_resume(struct device *dev) +{ + struct komeda_drv *mdrv = dev_get_drvdata(dev); + struct drm_device *drm = &mdrv->kms->base; + + dev_info(dev, "%s\n", __func__); + komeda_dev_resume(mdrv->mdev); + + return drm_mode_config_helper_resume(drm); +} + +static const struct dev_pm_ops komeda_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(komeda_pm_suspend, komeda_pm_resume) + SET_RUNTIME_PM_OPS(komeda_rt_pm_suspend, komeda_rt_pm_resume, NULL) +}; + static struct platform_driver komeda_platform_driver = { .probe = komeda_platform_probe, .remove = komeda_platform_remove, .driver = { .name = "komeda", .of_match_table = komeda_of_match, - .pm = NULL, + .pm = &komeda_pm_ops, }, };
Adds runtime and system power management support in KMS kernel driver. Depends on: - https://patchwork.freedesktop.org/series/61650/ - https://patchwork.freedesktop.org/series/60083/ Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com> --- drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 2 + drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 30 +++++++++++++ drivers/gpu/drm/arm/display/komeda/komeda_dev.h | 2 + drivers/gpu/drm/arm/display/komeda/komeda_drv.c | 54 ++++++++++++++++++++++-- 4 files changed, 85 insertions(+), 3 deletions(-)