Hi Sean, On Tuesday 29 of October 2013 12:13:12 Sean Paul wrote: > This patch removes all of the suspend/resume logic from the individual > drivers and consolidates it in drm_drv. This consolidation reduces the > number of functions which enable/disable the hardware to just one -- the > dpms callback. This ensures that we always power up/down in a consistent > manner. > > Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx> > --- > > Changes in v2: > - Added to the patchset > Changes in v3: > - Made appropriate changes to vidi as well (removed pm_ops) > > drivers/gpu/drm/exynos/exynos_drm_drv.c | 97 +++++++++++++++++++++++++ > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 91 ++++------------------- > drivers/gpu/drm/exynos/exynos_drm_vidi.c | 119 +++++++++++++------------------ > drivers/gpu/drm/exynos/exynos_hdmi.c | 82 +-------------------- > drivers/gpu/drm/exynos/exynos_mixer.c | 75 ++++--------------- > 5 files changed, 176 insertions(+), 288 deletions(-) [snip] > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index ba12916..208e013 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -741,6 +741,8 @@ static int fimd_poweron(struct exynos_drm_manager *mgr) > > ctx->suspended = false; > > + pm_runtime_get_sync(ctx->dev); > + > ret = clk_prepare_enable(ctx->bus_clk); > if (ret < 0) { > DRM_ERROR("Failed to prepare_enable the bus clk [%d]\n", ret); > @@ -794,32 +796,24 @@ static int fimd_poweroff(struct exynos_drm_manager *mgr) > clk_disable_unprepare(ctx->lcd_clk); > clk_disable_unprepare(ctx->bus_clk); > > + pm_runtime_put_sync(ctx->dev); > + > ctx->suspended = true; > return 0; > } [snip] > @@ -950,8 +944,14 @@ static int fimd_probe(struct platform_device *pdev) > fimd_manager.ctx = ctx; > exynos_drm_manager_register(&fimd_manager); > > + /* > + * We need to runtime pm to enable/disable sysmmu since it is a child of > + * this driver. Ideally, this would hang off the drm driver's runtime > + * operations, but we're not quite there yet. You also need runtime PM to control state of power domains. I don't think we should be going away of runtime PM API. Instead DPMS callbacks could simply call pm_runtime_get_sync/put() whenever the hardware is supposed to go up or down, just as done above in fimd_poweron/off(). This would allow any platform-specific PM logic placed outside of DRM subsystem (such as power domains and IOMMU) to operate. > + * > + * Tracked in crbug.com/264312 > + */ > pm_runtime_enable(dev); > - pm_runtime_get_sync(dev); > > for (win = 0; win < WINDOWS_NR; win++) > fimd_clear_win(ctx, win); [snip] > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c > index 2c127b9..c6561fe 100644 > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c [snip] > @@ -2030,8 +2024,6 @@ static int hdmi_probe(struct platform_device *pdev) > hdmi_display.ctx = hdata; > exynos_drm_display_register(&hdmi_display); > > - pm_runtime_enable(dev); > - That's plain wrong. You need runtime PM to enable LCD power domain in which the HDMI is placed. > return 0; > > err_hdmiphy: > @@ -2047,88 +2039,18 @@ static int hdmi_remove(struct platform_device *pdev) > struct exynos_drm_display *display = get_hdmi_display(dev); > struct hdmi_context *hdata = display->ctx; > > - pm_runtime_disable(dev); > - > put_device(&hdata->hdmiphy_port->dev); > put_device(&hdata->ddc_port->dev); > > return 0; > } [snip] > diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c > index 39aed3e..985391d 100644 > --- a/drivers/gpu/drm/exynos/exynos_mixer.c > +++ b/drivers/gpu/drm/exynos/exynos_mixer.c [snip] > @@ -1239,6 +1239,13 @@ static int mixer_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, &mixer_manager); > exynos_drm_manager_register(&mixer_manager); > > + /* > + * We need to runtime pm to enable/disable sysmmu since it is a child of > + * this driver. Ideally, this would hang off the drm driver's runtime > + * operations, but we're not quite there yet. Same comment as for FIMD and HDMI. Best regards, Tomasz _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel