| Message ID | 20181005154843.172326-3-bzwang@chromium.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
| Series | drm/msm: Use atomic helpers for PM suspend/resume | expand |
On Fri, Oct 05, 2018 at 11:48:41AM -0400, Bruce Wang wrote: > Removes the traces of the non-atomic helper calls in > msm_pm_suspend/resume since we just deleted those functions (see patch > 1). Also removes the drm_kms_helper_poll_disable/enable calls, since > the DRM_CONNECTOR_POLL_CONNECT flag is never set so periodic polling > doesn't happen anyways. > > Signed-off-by: Bruce Wang <bzwang@chromium.org> > --- > drivers/gpu/drm/msm/msm_drv.c | 28 ++++++++++------------------ > drivers/gpu/drm/msm/msm_kms.h | 3 --- > 2 files changed, 10 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index 4904d0d41409..f987df7f10df 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c > @@ -1069,37 +1069,29 @@ static int msm_pm_suspend(struct device *dev) > { > struct drm_device *ddev = dev_get_drvdata(dev); > struct msm_drm_private *priv = ddev->dev_private; > - struct msm_kms *kms = priv->kms; > - > - /* TODO: Use atomic helper suspend/resume */ > - if (kms && kms->funcs && kms->funcs->pm_suspend) > - return kms->funcs->pm_suspend(dev); > > - drm_kms_helper_poll_disable(ddev); > + if (!IS_ERR_OR_NULL(priv->pm_state)) > + return 0; Hmm, I don't think we want to completely abort suspending if it didn't succeed previously. It would be easier if we just scrubbed the value if it's not valid. So something like: if (WARN_ON(priv->pm_state)) drm_atomic_state_put(priv->pm_state); priv->pm_state = drm_atomic_helper_suspend(ddev); if (IS_ERR(priv->pm_state)) { int ret = PTR_ERR(priv->pm_state); DRM_ERROR("Failed to suspend dpu, %d\n", ret); return ret; } return 0; > > priv->pm_state = drm_atomic_helper_suspend(ddev); > - if (IS_ERR(priv->pm_state)) { > - drm_kms_helper_poll_enable(ddev); > - return PTR_ERR(priv->pm_state); > - } > > - return 0; > + return PTR_ERR_OR_ZERO(priv->pm_state); > } > > static int msm_pm_resume(struct device *dev) > { > struct drm_device *ddev = dev_get_drvdata(dev); > struct msm_drm_private *priv = ddev->dev_private; > - struct msm_kms *kms = priv->kms; > + int ret; > > - /* TODO: Use atomic helper suspend/resume */ > - if (kms && kms->funcs && kms->funcs->pm_resume) > - return kms->funcs->pm_resume(dev); > + if (IS_ERR_OR_NULL(priv->pm_state)) If you take my suggestion above, this can become: if (WARN_ON(!priv->pm_state)) return -ENOENT; > + return 0; > > - drm_atomic_helper_resume(ddev, priv->pm_state); > - drm_kms_helper_poll_enable(ddev); > + ret = drm_atomic_helper_resume(ddev, priv->pm_state); > + if (ret == 0) nit: This is usually expressed as: if (!ret) > + priv->pm_state = NULL; > > - return 0; > + return ret; > } > #endif > > diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h > index fd88cebb6adb..2b81b43a4bab 100644 > --- a/drivers/gpu/drm/msm/msm_kms.h > +++ b/drivers/gpu/drm/msm/msm_kms.h > @@ -67,9 +67,6 @@ struct msm_kms_funcs { > void (*set_encoder_mode)(struct msm_kms *kms, > struct drm_encoder *encoder, > bool cmd_mode); > - /* pm suspend/resume hooks */ > - int (*pm_suspend)(struct device *dev); > - int (*pm_resume)(struct device *dev); > /* cleanup: */ > void (*destroy)(struct msm_kms *kms); > #ifdef CONFIG_DEBUG_FS > -- > 2.19.0.605.g01d371f741-goog >
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 4904d0d41409..f987df7f10df 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1069,37 +1069,29 @@ static int msm_pm_suspend(struct device *dev) { struct drm_device *ddev = dev_get_drvdata(dev); struct msm_drm_private *priv = ddev->dev_private; - struct msm_kms *kms = priv->kms; - - /* TODO: Use atomic helper suspend/resume */ - if (kms && kms->funcs && kms->funcs->pm_suspend) - return kms->funcs->pm_suspend(dev); - drm_kms_helper_poll_disable(ddev); + if (!IS_ERR_OR_NULL(priv->pm_state)) + return 0; priv->pm_state = drm_atomic_helper_suspend(ddev); - if (IS_ERR(priv->pm_state)) { - drm_kms_helper_poll_enable(ddev); - return PTR_ERR(priv->pm_state); - } - return 0; + return PTR_ERR_OR_ZERO(priv->pm_state); } static int msm_pm_resume(struct device *dev) { struct drm_device *ddev = dev_get_drvdata(dev); struct msm_drm_private *priv = ddev->dev_private; - struct msm_kms *kms = priv->kms; + int ret; - /* TODO: Use atomic helper suspend/resume */ - if (kms && kms->funcs && kms->funcs->pm_resume) - return kms->funcs->pm_resume(dev); + if (IS_ERR_OR_NULL(priv->pm_state)) + return 0; - drm_atomic_helper_resume(ddev, priv->pm_state); - drm_kms_helper_poll_enable(ddev); + ret = drm_atomic_helper_resume(ddev, priv->pm_state); + if (ret == 0) + priv->pm_state = NULL; - return 0; + return ret; } #endif diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h index fd88cebb6adb..2b81b43a4bab 100644 --- a/drivers/gpu/drm/msm/msm_kms.h +++ b/drivers/gpu/drm/msm/msm_kms.h @@ -67,9 +67,6 @@ struct msm_kms_funcs { void (*set_encoder_mode)(struct msm_kms *kms, struct drm_encoder *encoder, bool cmd_mode); - /* pm suspend/resume hooks */ - int (*pm_suspend)(struct device *dev); - int (*pm_resume)(struct device *dev); /* cleanup: */ void (*destroy)(struct msm_kms *kms); #ifdef CONFIG_DEBUG_FS
Removes the traces of the non-atomic helper calls in msm_pm_suspend/resume since we just deleted those functions (see patch 1). Also removes the drm_kms_helper_poll_disable/enable calls, since the DRM_CONNECTOR_POLL_CONNECT flag is never set so periodic polling doesn't happen anyways. Signed-off-by: Bruce Wang <bzwang@chromium.org> --- drivers/gpu/drm/msm/msm_drv.c | 28 ++++++++++------------------ drivers/gpu/drm/msm/msm_kms.h | 3 --- 2 files changed, 10 insertions(+), 21 deletions(-)