[PATCH v3 26/32] drm/exynos: Consolidate suspend/resume in drm_drv

Inki Dae inki.dae at samsung.com
Thu Dec 19 08:48:49 PST 2013


Hi,

Below is my comments.

Thanks for your contributions,
Inki Dae

2013/10/30 Sean Paul <seanpaul at chromium.org>:
> 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(-)
>
> 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>

Remove above include because it doesn't need runtime pm support. For
this, see the below comments.

>  #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;

Could you tell me why you want back to old state? I think
connector->dpms should have DRM_MODE_DPMS_OFF after the above
connector's dpms callback is called, not previous state.

> +       }
> +       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);

Just DRM_MODE_DPMS_ON instead of connector->dpms. Is there any reason
to send connector->dpms as a argument?

> +       }
> +
> +       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);

It seems that this exynos_drm_drv module doesn't need to support
runtime pm, and it's enough for each sub driver supports the runtime
pm .

Above pm_runtime_get_sync call wouldn't do what you expect because
crtc, encoder and connector of each sub driver aren't created yet at
this moment. And crtc and connector devices will be enabled by dpms
callback of each drivers when load() is called:
drm_fb_helper_initial_config function in exynos_drm_fbdev_init(). So
it would be enough for exynos_drm_drv module considers only
suspend-to-ram.

> +
>         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;

Unnecessary codes.

> +
> +       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;

Unnecessary codes.

> +
> +       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)
> +};

Wouldn't need above all runtime pm related codes.

> +
>  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
> +        */
>         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_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> index 90dacaf..7d79b6c 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -142,31 +142,6 @@ static struct exynos_drm_display vidi_display = {
>         .ops = &vidi_display_ops,
>  };
>
> -static void vidi_dpms(struct exynos_drm_manager *mgr, int mode)
> -{
> -       struct vidi_context *ctx = mgr->ctx;
> -
> -       DRM_DEBUG_KMS("%d\n", mode);
> -
> -       mutex_lock(&ctx->lock);
> -
> -       switch (mode) {
> -       case DRM_MODE_DPMS_ON:
> -               /* TODO. */
> -               break;
> -       case DRM_MODE_DPMS_STANDBY:
> -       case DRM_MODE_DPMS_SUSPEND:
> -       case DRM_MODE_DPMS_OFF:
> -               /* TODO. */
> -               break;
> -       default:
> -               DRM_DEBUG_KMS("unspecified mode %d\n", mode);
> -               break;
> -       }
> -
> -       mutex_unlock(&ctx->lock);
> -}
> -
>  static void vidi_apply(struct exynos_drm_manager *mgr)
>  {
>         struct vidi_context *ctx = mgr->ctx;
> @@ -321,6 +296,55 @@ static void vidi_win_disable(struct exynos_drm_manager *mgr, int zpos)
>         /* TODO. */
>  }
>
> +static int vidi_power_on(struct exynos_drm_manager *mgr, bool enable)
> +{
> +       struct vidi_context *ctx = mgr->ctx;
> +
> +       DRM_DEBUG_KMS("%s\n", __FILE__);
> +
> +       if (enable != false && enable != true)
> +               return -EINVAL;
> +
> +       if (enable) {
> +               ctx->suspended = false;
> +
> +               /* if vblank was enabled status, enable it again. */
> +               if (test_and_clear_bit(0, &ctx->irq_flags))
> +                       vidi_enable_vblank(mgr);
> +
> +               vidi_apply(mgr);
> +       } else {
> +               ctx->suspended = true;
> +       }
> +
> +       return 0;
> +}
> +
> +static void vidi_dpms(struct exynos_drm_manager *mgr, int mode)
> +{
> +       struct vidi_context *ctx = mgr->ctx;
> +
> +       DRM_DEBUG_KMS("%d\n", mode);
> +
> +       mutex_lock(&ctx->lock);
> +
> +       switch (mode) {
> +       case DRM_MODE_DPMS_ON:
> +               vidi_power_on(mgr, true);
> +               break;
> +       case DRM_MODE_DPMS_STANDBY:
> +       case DRM_MODE_DPMS_SUSPEND:
> +       case DRM_MODE_DPMS_OFF:
> +               vidi_power_on(mgr, false);
> +               break;
> +       default:
> +               DRM_DEBUG_KMS("unspecified mode %d\n", mode);
> +               break;
> +       }
> +
> +       mutex_unlock(&ctx->lock);
> +}
> +
>  static int vidi_mgr_initialize(struct exynos_drm_manager *mgr,
>                         struct drm_device *drm_dev, int pipe)
>  {
> @@ -392,30 +416,6 @@ static void vidi_fake_vblank_handler(struct work_struct *work)
>         exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
>  }
>
> -static int vidi_power_on(struct exynos_drm_manager *mgr, bool enable)
> -{
> -       struct vidi_context *ctx = mgr->ctx;
> -
> -       DRM_DEBUG_KMS("%s\n", __FILE__);
> -
> -       if (enable != false && enable != true)
> -               return -EINVAL;
> -
> -       if (enable) {
> -               ctx->suspended = false;
> -
> -               /* if vblank was enabled status, enable it again. */
> -               if (test_and_clear_bit(0, &ctx->irq_flags))
> -                       vidi_enable_vblank(mgr);
> -
> -               vidi_apply(mgr);
> -       } else {
> -               ctx->suspended = true;
> -       }
> -
> -       return 0;
> -}
> -
>  static int vidi_show_connection(struct device *dev,
>                                 struct device_attribute *attr, char *buf)
>  {
> @@ -582,32 +582,11 @@ static int vidi_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> -#ifdef CONFIG_PM_SLEEP
> -static int vidi_suspend(struct device *dev)
> -{
> -       struct exynos_drm_manager *mgr = get_vidi_mgr(dev);
> -
> -       return vidi_power_on(mgr, false);
> -}
> -
> -static int vidi_resume(struct device *dev)
> -{
> -       struct exynos_drm_manager *mgr = get_vidi_mgr(dev);
> -
> -       return vidi_power_on(mgr, true);
> -}
> -#endif
> -
> -static const struct dev_pm_ops vidi_pm_ops = {
> -       SET_SYSTEM_SLEEP_PM_OPS(vidi_suspend, vidi_resume)
> -};
> -
>  struct platform_driver vidi_driver = {
>         .probe          = vidi_probe,
>         .remove         = vidi_remove,
>         .driver         = {
>                 .name   = "exynos-drm-vidi",
>                 .owner  = THIS_MODULE,
> -               .pm     = &vidi_pm_ops,
>         },
>  };
> 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
> @@ -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>
> @@ -1770,7 +1769,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:
> @@ -1779,20 +1777,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);
> @@ -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);
> -
>         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;
>  }
>
> -#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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel


More information about the dri-devel mailing list