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 at chromium.org>
> ---
>
> 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
