Re: [PATCH v2 26/26] drm/exynos: Consolidate suspend/resume in drm_drv

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 16, 2013 at 3:26 PM, Sean Paul <seanpaul@xxxxxxxxxxxx> 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
>
>  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 97 ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 91 +++++-------------------------
>  drivers/gpu/drm/exynos/exynos_hdmi.c     | 82 +--------------------------
>  drivers/gpu/drm/exynos/exynos_mixer.c    | 75 +++++-------------------
>  4 files changed, 127 insertions(+), 218 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 03caa3a..91d6863 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -11,6 +11,7 @@
>   * option) any later version.
>   */
>
> +#include <linux/pm_runtime.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc_helper.h>
>
> @@ -51,6 +52,7 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
>                 return -ENOMEM;
>
>         INIT_LIST_HEAD(&private->pageflip_event_list);
> +       dev_set_drvdata(dev->dev, dev);
>         dev->dev_private = (void *)private;
>
>         /*
> @@ -155,6 +157,41 @@ static int exynos_drm_unload(struct drm_device *dev)
>         return 0;
>  }
>
> +static int exynos_drm_suspend(struct drm_device *dev, pm_message_t state)
> +{
> +       struct drm_connector *connector;
> +
> +       drm_modeset_lock_all(dev);
> +       list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> +               int old_dpms = connector->dpms;
> +
> +               if (connector->funcs->dpms)
> +                       connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);
> +
> +               /* Set the old mode back to the connector for resume */
> +               connector->dpms = old_dpms;
> +       }
> +       drm_modeset_unlock_all(dev);
> +
> +       return 0;
> +}
> +
> +static int exynos_drm_resume(struct drm_device *dev)
> +{
> +       struct drm_connector *connector;
> +
> +       drm_modeset_lock_all(dev);
> +       list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> +               if (connector->funcs->dpms)
> +                       connector->funcs->dpms(connector, connector->dpms);
> +       }
> +
> +       drm_helper_resume_force_mode(dev);
> +       drm_modeset_unlock_all(dev);
> +
> +       return 0;
> +}
> +
>  static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
>  {
>         struct drm_exynos_file_private *file_priv;
> @@ -262,6 +299,8 @@ static struct drm_driver exynos_drm_driver = {
>                                         DRIVER_GEM | DRIVER_PRIME,
>         .load                   = exynos_drm_load,
>         .unload                 = exynos_drm_unload,
> +       .suspend                = exynos_drm_suspend,
> +       .resume                 = exynos_drm_resume,
>         .open                   = exynos_drm_open,
>         .preclose               = exynos_drm_preclose,
>         .lastclose              = exynos_drm_lastclose,
> @@ -293,6 +332,9 @@ static int exynos_drm_platform_probe(struct platform_device *pdev)
>  {
>         pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>
> +       pm_runtime_enable(&pdev->dev);
> +       pm_runtime_get_sync(&pdev->dev);
> +
>         return drm_platform_init(&exynos_drm_driver, pdev);
>  }
>
> @@ -303,12 +345,67 @@ static int exynos_drm_platform_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> +#ifdef CONFIG_PM_SLEEP
> +static int exynos_drm_sys_suspend(struct device *dev)
> +{
> +       struct drm_device *drm_dev = dev_get_drvdata(dev);
> +       pm_message_t message;
> +
> +       if (pm_runtime_suspended(dev))
> +               return 0;
> +
> +       message.event = PM_EVENT_SUSPEND;
> +       return exynos_drm_suspend(drm_dev, message);
> +}
> +
> +static int exynos_drm_sys_resume(struct device *dev)
> +{
> +       struct drm_device *drm_dev = dev_get_drvdata(dev);
> +
> +       if (pm_runtime_suspended(dev))
> +               return 0;
> +
> +       return exynos_drm_resume(drm_dev);
> +}
> +#endif
> +
> +#ifdef CONFIG_PM_RUNTIME
> +static int exynos_drm_runtime_suspend(struct device *dev)
> +{
> +       struct drm_device *drm_dev = dev_get_drvdata(dev);
> +       pm_message_t message;
> +
> +       if (pm_runtime_suspended(dev))
> +               return 0;
> +
> +       message.event = PM_EVENT_SUSPEND;
> +       return exynos_drm_suspend(drm_dev, message);
> +}
> +
> +static int exynos_drm_runtime_resume(struct device *dev)
> +{
> +       struct drm_device *drm_dev = dev_get_drvdata(dev);
> +
> +       if (!pm_runtime_suspended(dev))
> +               return 0;
> +
> +       return exynos_drm_resume(drm_dev);
> +}
> +#endif
> +
> +static const struct dev_pm_ops exynos_drm_pm_ops = {
> +       SET_SYSTEM_SLEEP_PM_OPS(exynos_drm_sys_suspend, exynos_drm_sys_resume)
> +       SET_RUNTIME_PM_OPS(exynos_drm_runtime_suspend,
> +                       exynos_drm_runtime_resume, NULL)
> +};
> +
>  static struct platform_driver exynos_drm_platform_driver = {
>         .probe          = exynos_drm_platform_probe,
>         .remove         = exynos_drm_platform_remove,
>         .driver         = {
>                 .owner  = THIS_MODULE,
>                 .name   = "exynos-drm",
> +               .pm     = &exynos_drm_pm_ops,
>         },
>  };
>
> 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;
>  }
>
>  static void fimd_dpms(struct exynos_drm_manager *mgr, int mode)
>  {
> -       struct fimd_context *ctx = mgr->ctx;
> -
>         DRM_DEBUG_KMS("%s, %d\n", __FILE__, mode);
>
>         switch (mode) {
>         case DRM_MODE_DPMS_ON:
> -               /*
> -                * enable fimd hardware only if suspended status.
> -                *
> -                * P.S. fimd_dpms function would be called at booting time so
> -                * clk_enable could be called double time.
> -                */
> -               if (ctx->suspended)
> -                       pm_runtime_get_sync(ctx->dev);
> +               fimd_poweron(mgr);
>                 break;
>         case DRM_MODE_DPMS_STANDBY:
>         case DRM_MODE_DPMS_SUSPEND:
>         case DRM_MODE_DPMS_OFF:
> -               if (!ctx->suspended)
> -                       pm_runtime_put_sync(ctx->dev);
> +               fimd_poweroff(mgr);
>                 break;
>         default:
>                 DRM_DEBUG_KMS("unspecified mode %d\n", mode);
> @@ -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.
> +        *
> +        * Tracked in crbug.com/264312

Argh, this (and the similar comment below) slipped by me, sorry about that.

Can you remove, or would you like to to send a new version?

Sean

> +        */
>         pm_runtime_enable(dev);
> -       pm_runtime_get_sync(dev);
>
>         for (win = 0; win < WINDOWS_NR; win++)
>                 fimd_clear_win(ctx, win);
> @@ -961,84 +961,23 @@ static int fimd_probe(struct platform_device *pdev)
>
>  static int fimd_remove(struct platform_device *pdev)
>  {
> -       struct device *dev = &pdev->dev;
>         struct exynos_drm_manager *mgr = platform_get_drvdata(pdev);
> -       struct fimd_context *ctx = mgr->ctx;
>
>         exynos_drm_manager_unregister(&fimd_manager);
>
> -       if (ctx->suspended)
> -               goto out;
> -
> -       pm_runtime_set_suspended(dev);
> -       pm_runtime_put_sync(dev);
> +       fimd_dpms(mgr, DRM_MODE_DPMS_OFF);
>
> -out:
> -       pm_runtime_disable(dev);
> +       pm_runtime_disable(&pdev->dev);
>
>         return 0;
>  }
>
> -#ifdef CONFIG_PM_SLEEP
> -static int fimd_suspend(struct device *dev)
> -{
> -       struct exynos_drm_manager *mgr = get_fimd_manager(dev);
> -
> -       /*
> -        * do not use pm_runtime_suspend(). if pm_runtime_suspend() is
> -        * called here, an error would be returned by that interface
> -        * because the usage_count of pm runtime is more than 1.
> -        */
> -       if (!pm_runtime_suspended(dev))
> -               return fimd_poweroff(mgr);
> -
> -       return 0;
> -}
> -
> -static int fimd_resume(struct device *dev)
> -{
> -       struct exynos_drm_manager *mgr = get_fimd_manager(dev);
> -
> -       /*
> -        * if entered to sleep when lcd panel was on, the usage_count
> -        * of pm runtime would still be 1 so in this case, fimd driver
> -        * should be on directly not drawing on pm runtime interface.
> -        */
> -       if (pm_runtime_suspended(dev))
> -               return 0;
> -
> -       return fimd_poweron(mgr);
> -}
> -#endif
> -
> -#ifdef CONFIG_PM_RUNTIME
> -static int fimd_runtime_suspend(struct device *dev)
> -{
> -       struct exynos_drm_manager *mgr = get_fimd_manager(dev);
> -
> -       return fimd_poweroff(mgr);
> -}
> -
> -static int fimd_runtime_resume(struct device *dev)
> -{
> -       struct exynos_drm_manager *mgr = get_fimd_manager(dev);
> -
> -       return fimd_poweron(mgr);
> -}
> -#endif
> -
> -static const struct dev_pm_ops fimd_pm_ops = {
> -       SET_SYSTEM_SLEEP_PM_OPS(fimd_suspend, fimd_resume)
> -       SET_RUNTIME_PM_OPS(fimd_runtime_suspend, fimd_runtime_resume, NULL)
> -};
> -
>  struct platform_driver fimd_driver = {
>         .probe          = fimd_probe,
>         .remove         = fimd_remove,
>         .driver         = {
>                 .name   = "exynos4-fb",
>                 .owner  = THIS_MODULE,
> -               .pm     = &fimd_pm_ops,
>                 .of_match_table = fimd_driver_dt_match,
>         },
>  };
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index afc83c9..130b109 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -28,7 +28,6 @@
>  #include <linux/interrupt.h>
>  #include <linux/irq.h>
>  #include <linux/delay.h>
> -#include <linux/pm_runtime.h>
>  #include <linux/clk.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/io.h>
> @@ -1771,7 +1770,6 @@ static void hdmi_poweroff(struct exynos_drm_display *display)
>         regulator_bulk_disable(res->regul_count, res->regul_bulk);
>
>         mutex_lock(&hdata->hdmi_mutex);
> -
>         hdata->powered = false;
>
>  out:
> @@ -1780,20 +1778,16 @@ out:
>
>  static void hdmi_dpms(struct exynos_drm_display *display, int mode)
>  {
> -       struct hdmi_context *hdata = display->ctx;
> -
>         DRM_DEBUG_KMS("mode %d\n", mode);
>
>         switch (mode) {
>         case DRM_MODE_DPMS_ON:
> -               if (pm_runtime_suspended(hdata->dev))
> -                       pm_runtime_get_sync(hdata->dev);
> +               hdmi_poweron(display);
>                 break;
>         case DRM_MODE_DPMS_STANDBY:
>         case DRM_MODE_DPMS_SUSPEND:
>         case DRM_MODE_DPMS_OFF:
> -               if (!pm_runtime_suspended(hdata->dev))
> -                       pm_runtime_put_sync(hdata->dev);
> +               hdmi_poweroff(display);
>                 break;
>         default:
>                 DRM_DEBUG_KMS("unknown dpms mode: %d\n", mode);
> @@ -2036,8 +2030,6 @@ static int hdmi_probe(struct platform_device *pdev)
>         hdmi_display.ctx = hdata;
>         exynos_drm_display_register(&hdmi_display);
>
> -       pm_runtime_enable(dev);
> -
>         return 0;
>
>  err_hdmiphy:
> @@ -2053,88 +2045,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;
>  }
>
> -#ifdef CONFIG_PM_SLEEP
> -static int hdmi_suspend(struct device *dev)
> -{
> -       struct exynos_drm_display *display = get_hdmi_display(dev);
> -       struct hdmi_context *hdata = display->ctx;
> -
> -       disable_irq(hdata->irq);
> -
> -       hdata->hpd = false;
> -       if (hdata->drm_dev)
> -               drm_helper_hpd_irq_event(hdata->drm_dev);
> -
> -       if (pm_runtime_suspended(dev)) {
> -               DRM_DEBUG_KMS("Already suspended\n");
> -               return 0;
> -       }
> -
> -       hdmi_poweroff(display);
> -
> -       return 0;
> -}
> -
> -static int hdmi_resume(struct device *dev)
> -{
> -       struct exynos_drm_display *display = get_hdmi_display(dev);
> -       struct hdmi_context *hdata = display->ctx;
> -
> -       hdata->hpd = gpio_get_value(hdata->hpd_gpio);
> -
> -       enable_irq(hdata->irq);
> -
> -       if (!pm_runtime_suspended(dev)) {
> -               DRM_DEBUG_KMS("Already resumed\n");
> -               return 0;
> -       }
> -
> -       hdmi_poweron(display);
> -
> -       return 0;
> -}
> -#endif
> -
> -#ifdef CONFIG_PM_RUNTIME
> -static int hdmi_runtime_suspend(struct device *dev)
> -{
> -       struct exynos_drm_display *display = get_hdmi_display(dev);
> -
> -       hdmi_poweroff(display);
> -
> -       return 0;
> -}
> -
> -static int hdmi_runtime_resume(struct device *dev)
> -{
> -       struct exynos_drm_display *display = get_hdmi_display(dev);
> -
> -       hdmi_poweron(display);
> -
> -       return 0;
> -}
> -#endif
> -
> -static const struct dev_pm_ops hdmi_pm_ops = {
> -       SET_SYSTEM_SLEEP_PM_OPS(hdmi_suspend, hdmi_resume)
> -       SET_RUNTIME_PM_OPS(hdmi_runtime_suspend, hdmi_runtime_resume, NULL)
> -};
> -
>  struct platform_driver hdmi_driver = {
>         .probe          = hdmi_probe,
>         .remove         = hdmi_remove,
>         .driver         = {
>                 .name   = "exynos-hdmi",
>                 .owner  = THIS_MODULE,
> -               .pm     = &hdmi_pm_ops,
>                 .of_match_table = hdmi_match_types,
>         },
>  };
> 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
> @@ -902,6 +902,8 @@ static void mixer_poweron(struct exynos_drm_manager *mgr)
>         ctx->powered = true;
>         mutex_unlock(&ctx->mixer_mutex);
>
> +       pm_runtime_get_sync(ctx->dev);
> +
>         clk_prepare_enable(res->mixer);
>         if (ctx->vp_enabled) {
>                 clk_prepare_enable(res->vp);
> @@ -934,6 +936,8 @@ static void mixer_poweroff(struct exynos_drm_manager *mgr)
>                 clk_disable_unprepare(res->sclk_mixer);
>         }
>
> +       pm_runtime_put_sync(ctx->dev);
> +
>         mutex_lock(&ctx->mixer_mutex);
>         ctx->powered = false;
>
> @@ -943,18 +947,14 @@ out:
>
>  static void mixer_dpms(struct exynos_drm_manager *mgr, int mode)
>  {
> -       struct mixer_context *mixer_ctx = mgr->ctx;
> -
>         switch (mode) {
>         case DRM_MODE_DPMS_ON:
> -               if (pm_runtime_suspended(mixer_ctx->dev))
> -                       pm_runtime_get_sync(mixer_ctx->dev);
> +               mixer_poweron(mgr);
>                 break;
>         case DRM_MODE_DPMS_STANDBY:
>         case DRM_MODE_DPMS_SUSPEND:
>         case DRM_MODE_DPMS_OFF:
> -               if (!pm_runtime_suspended(mixer_ctx->dev))
> -                       pm_runtime_put_sync(mixer_ctx->dev);
> +               mixer_poweroff(mgr);
>                 break;
>         default:
>                 DRM_DEBUG_KMS("unknown dpms mode: %d\n", mode);
> @@ -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.
> +        *
> +        * Tracked in crbug.com/264312
> +        */
>         pm_runtime_enable(dev);
>
>         return 0;
> @@ -1258,66 +1265,10 @@ static int mixer_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> -#ifdef CONFIG_PM_SLEEP
> -static int mixer_suspend(struct device *dev)
> -{
> -       struct exynos_drm_manager *mgr = get_mixer_manager(dev);
> -
> -       if (pm_runtime_suspended(dev)) {
> -               DRM_DEBUG_KMS("Already suspended\n");
> -               return 0;
> -       }
> -
> -       mixer_poweroff(mgr);
> -
> -       return 0;
> -}
> -
> -static int mixer_resume(struct device *dev)
> -{
> -       struct exynos_drm_manager *mgr = get_mixer_manager(dev);
> -
> -       if (!pm_runtime_suspended(dev)) {
> -               DRM_DEBUG_KMS("Already resumed\n");
> -               return 0;
> -       }
> -
> -       mixer_poweron(mgr);
> -
> -       return 0;
> -}
> -#endif
> -
> -#ifdef CONFIG_PM_RUNTIME
> -static int mixer_runtime_suspend(struct device *dev)
> -{
> -       struct exynos_drm_manager *mgr = get_mixer_manager(dev);
> -
> -       mixer_poweroff(mgr);
> -
> -       return 0;
> -}
> -
> -static int mixer_runtime_resume(struct device *dev)
> -{
> -       struct exynos_drm_manager *mgr = get_mixer_manager(dev);
> -
> -       mixer_poweron(mgr);
> -
> -       return 0;
> -}
> -#endif
> -
> -static const struct dev_pm_ops mixer_pm_ops = {
> -       SET_SYSTEM_SLEEP_PM_OPS(mixer_suspend, mixer_resume)
> -       SET_RUNTIME_PM_OPS(mixer_runtime_suspend, mixer_runtime_resume, NULL)
> -};
> -
>  struct platform_driver mixer_driver = {
>         .driver = {
>                 .name = "exynos-mixer",
>                 .owner = THIS_MODULE,
> -               .pm = &mixer_pm_ops,
>                 .of_match_table = mixer_match_types,
>         },
>         .probe = mixer_probe,
> --
> 1.8.4
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux