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@xxxxxxx> > > --- > > 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! / > --------------- > ¯\_(ツ)_/¯ -- Regards, Lowry _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
