| Message ID | 20181002150530.6407-1-bzwang@chromium.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
| Series | [1/2] drm/msm: Use atomic helpers for pm_suspend/resume | expand |
On Tue, Oct 02, 2018 at 11:05:29AM -0400, Bruce Wang wrote: > The dpu implementation of pm_resume were causing a crash. This patch > changes msm_pm_suspend and msm_pm_resume to use the atomic > helpers drm_atomic_helper_suspend and drm_atomic_helper_resume. > Removes the hooks needed for calling the dpu_kms implementations of > suspend/resume. Note that the poll_disable call is likely not needed as > of right now as the DRM_CONNECTOR_POLL_CONNECT flag is never set. > > Signed-off-by: Bruce Wang <bzwang@chromium.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 -- > drivers/gpu/drm/msm/msm_drv.c | 28 +++++++++---------------- > drivers/gpu/drm/msm/msm_kms.h | 3 --- > 3 files changed, 10 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index 0a683e65a9f3..e0142912d676 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -873,8 +873,6 @@ static const struct msm_kms_funcs kms_funcs = { > .check_modified_format = dpu_format_check_modified_format, > .get_format = dpu_get_msm_format, > .round_pixclk = dpu_kms_round_pixclk, > - .pm_suspend = dpu_kms_pm_suspend, > - .pm_resume = dpu_kms_pm_resume, Can you flip the order of these patches such that you remove the dpu implementation entirely first, and then remove the unused bits in msm core? That way the dpu and msm bits are clearly delineated. Sean > .destroy = dpu_kms_destroy, > .set_encoder_mode = _dpu_kms_set_encoder_mode, > #ifdef CONFIG_DEBUG_FS > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index c1abad8a8612..b8dc854c99f2 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 IS_ERR(priv->pm_state) ? PTR_ERR(priv->pm_state) : 0; > } > > 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 > -- > 2.19.0.605.g01d371f741-goog >
On Tue, Oct 02, 2018 at 11:05:29AM -0400, Bruce Wang wrote: > The dpu implementation of pm_resume were causing a crash. This patch > changes msm_pm_suspend and msm_pm_resume to use the atomic > helpers drm_atomic_helper_suspend and drm_atomic_helper_resume. > Removes the hooks needed for calling the dpu_kms implementations of > suspend/resume. Note that the poll_disable call is likely not needed as > of right now as the DRM_CONNECTOR_POLL_CONNECT flag is never set. > > Signed-off-by: Bruce Wang <bzwang@chromium.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 -- > drivers/gpu/drm/msm/msm_drv.c | 28 +++++++++---------------- > drivers/gpu/drm/msm/msm_kms.h | 3 --- > 3 files changed, 10 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index 0a683e65a9f3..e0142912d676 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -873,8 +873,6 @@ static const struct msm_kms_funcs kms_funcs = { > .check_modified_format = dpu_format_check_modified_format, > .get_format = dpu_get_msm_format, > .round_pixclk = dpu_kms_round_pixclk, > - .pm_suspend = dpu_kms_pm_suspend, > - .pm_resume = dpu_kms_pm_resume, > .destroy = dpu_kms_destroy, > .set_encoder_mode = _dpu_kms_set_encoder_mode, > #ifdef CONFIG_DEBUG_FS > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index c1abad8a8612..b8dc854c99f2 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 IS_ERR(priv->pm_state) ? PTR_ERR(priv->pm_state) : 0; Nit: this can be 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 > -- > 2.19.0.605.g01d371f741-goog >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 0a683e65a9f3..e0142912d676 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -873,8 +873,6 @@ static const struct msm_kms_funcs kms_funcs = { .check_modified_format = dpu_format_check_modified_format, .get_format = dpu_get_msm_format, .round_pixclk = dpu_kms_round_pixclk, - .pm_suspend = dpu_kms_pm_suspend, - .pm_resume = dpu_kms_pm_resume, .destroy = dpu_kms_destroy, .set_encoder_mode = _dpu_kms_set_encoder_mode, #ifdef CONFIG_DEBUG_FS diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index c1abad8a8612..b8dc854c99f2 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 IS_ERR(priv->pm_state) ? PTR_ERR(priv->pm_state) : 0; } 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
The dpu implementation of pm_resume were causing a crash. This patch changes msm_pm_suspend and msm_pm_resume to use the atomic helpers drm_atomic_helper_suspend and drm_atomic_helper_resume. Removes the hooks needed for calling the dpu_kms implementations of suspend/resume. Note that the poll_disable call is likely not needed as of right now as the DRM_CONNECTOR_POLL_CONNECT flag is never set. Signed-off-by: Bruce Wang <bzwang@chromium.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 -- drivers/gpu/drm/msm/msm_drv.c | 28 +++++++++---------------- drivers/gpu/drm/msm/msm_kms.h | 3 --- 3 files changed, 10 insertions(+), 23 deletions(-)