| Message ID | 20181005154843.172326-2-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:40AM -0400, Bruce Wang wrote: > PM resume was crashing during dpu_kms_pm_resume. This patch removes > dpu_kms_pm_suspend/resume so that msm_pm_suspend/resume uses the atomic > helpers instead (see next patch). This patch also removes > dpu_kms_is_suspend_blocked since it is never called. > > Signed-off-by: Bruce Wang <bzwang@chromium.org> Reviewed-by: Sean Paul <seanpaul@chromium.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 123 ------------------------ > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 15 --- > 2 files changed, 138 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..ff06b50dfc87 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -709,127 +709,6 @@ static void dpu_kms_destroy(struct msm_kms *kms) > _dpu_kms_hw_destroy(dpu_kms); > } > > -static int dpu_kms_pm_suspend(struct device *dev) > -{ > - struct drm_device *ddev; > - struct drm_modeset_acquire_ctx ctx; > - struct drm_atomic_state *state; > - struct dpu_kms *dpu_kms; > - int ret = 0, num_crtcs = 0; > - > - if (!dev) > - return -EINVAL; > - > - ddev = dev_get_drvdata(dev); > - if (!ddev || !ddev_to_msm_kms(ddev)) > - return -EINVAL; > - > - dpu_kms = to_dpu_kms(ddev_to_msm_kms(ddev)); > - > - /* disable hot-plug polling */ > - drm_kms_helper_poll_disable(ddev); > - > - /* acquire modeset lock(s) */ > - drm_modeset_acquire_init(&ctx, 0); > - > -retry: > - DPU_ATRACE_BEGIN("kms_pm_suspend"); > - > - ret = drm_modeset_lock_all_ctx(ddev, &ctx); > - if (ret) > - goto unlock; > - > - /* save current state for resume */ > - if (dpu_kms->suspend_state) > - drm_atomic_state_put(dpu_kms->suspend_state); > - dpu_kms->suspend_state = drm_atomic_helper_duplicate_state(ddev, &ctx); > - if (IS_ERR_OR_NULL(dpu_kms->suspend_state)) { > - DRM_ERROR("failed to back up suspend state\n"); > - dpu_kms->suspend_state = NULL; > - goto unlock; > - } > - > - /* create atomic state to disable all CRTCs */ > - state = drm_atomic_state_alloc(ddev); > - if (IS_ERR_OR_NULL(state)) { > - DRM_ERROR("failed to allocate crtc disable state\n"); > - goto unlock; > - } > - > - state->acquire_ctx = &ctx; > - > - /* check for nothing to do */ > - if (num_crtcs == 0) { > - DRM_DEBUG("all crtcs are already in the off state\n"); > - drm_atomic_state_put(state); > - goto suspended; > - } > - > - /* commit the "disable all" state */ > - ret = drm_atomic_commit(state); > - if (ret < 0) { > - DRM_ERROR("failed to disable crtcs, %d\n", ret); > - drm_atomic_state_put(state); > - goto unlock; > - } > - > -suspended: > - dpu_kms->suspend_block = true; > - > -unlock: > - if (ret == -EDEADLK) { > - drm_modeset_backoff(&ctx); > - goto retry; > - } > - drm_modeset_drop_locks(&ctx); > - drm_modeset_acquire_fini(&ctx); > - > - DPU_ATRACE_END("kms_pm_suspend"); > - return 0; > -} > - > -static int dpu_kms_pm_resume(struct device *dev) > -{ > - struct drm_device *ddev; > - struct dpu_kms *dpu_kms; > - int ret; > - > - if (!dev) > - return -EINVAL; > - > - ddev = dev_get_drvdata(dev); > - if (!ddev || !ddev_to_msm_kms(ddev)) > - return -EINVAL; > - > - dpu_kms = to_dpu_kms(ddev_to_msm_kms(ddev)); > - > - DPU_ATRACE_BEGIN("kms_pm_resume"); > - > - drm_mode_config_reset(ddev); > - > - drm_modeset_lock_all(ddev); > - > - dpu_kms->suspend_block = false; > - > - if (dpu_kms->suspend_state) { > - dpu_kms->suspend_state->acquire_ctx = > - ddev->mode_config.acquire_ctx; > - ret = drm_atomic_commit(dpu_kms->suspend_state); > - if (ret < 0) { > - DRM_ERROR("failed to restore state, %d\n", ret); > - drm_atomic_state_put(dpu_kms->suspend_state); > - } > - dpu_kms->suspend_state = NULL; > - } > - drm_modeset_unlock_all(ddev); > - > - /* enable hot-plug polling */ > - drm_kms_helper_poll_enable(ddev); > - > - DPU_ATRACE_END("kms_pm_resume"); > - return 0; > -} > - > static void _dpu_kms_set_encoder_mode(struct msm_kms *kms, > struct drm_encoder *encoder, > bool cmd_mode) > @@ -873,8 +752,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/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > index 66d466628e2b..1991cbe424a3 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > @@ -137,7 +137,6 @@ struct dpu_kms { > > /* saved atomic state during system suspend */ > struct drm_atomic_state *suspend_state; > - bool suspend_block; > > struct dpu_rm rm; > bool rm_init; > @@ -176,20 +175,6 @@ static inline bool dpu_kms_is_suspend_state(struct drm_device *dev) > return to_dpu_kms(ddev_to_msm_kms(dev))->suspend_state != NULL; > } > > -/** > - * dpu_kms_is_suspend_blocked - whether or not commits are blocked due to pm > - * suspend status > - * @dev: Pointer to drm device > - * Return: True if commits should be rejected due to pm suspend > - */ > -static inline bool dpu_kms_is_suspend_blocked(struct drm_device *dev) > -{ > - if (!dpu_kms_is_suspend_state(dev)) > - return false; > - > - return to_dpu_kms(ddev_to_msm_kms(dev))->suspend_block; > -} > - > /** > * Debugfs functions - extra helper functions for debugfs support > * > -- > 2.19.0.605.g01d371f741-goog >
On Fri, Oct 05, 2018 at 01:27:54PM -0400, Sean Paul wrote: > On Fri, Oct 05, 2018 at 11:48:40AM -0400, Bruce Wang wrote: > > PM resume was crashing during dpu_kms_pm_resume. This patch removes > > dpu_kms_pm_suspend/resume so that msm_pm_suspend/resume uses the atomic > > helpers instead (see next patch). This patch also removes > > dpu_kms_is_suspend_blocked since it is never called. > > > > Signed-off-by: Bruce Wang <bzwang@chromium.org> > > Reviewed-by: Sean Paul <seanpaul@chromium.org> One more thing. This series should have the prefix '[PATCH v2]' and should contain a 'Changes in v2:' section detailing what's different from the first version. Sean > > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 123 ------------------------ > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 15 --- > > 2 files changed, 138 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..ff06b50dfc87 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > @@ -709,127 +709,6 @@ static void dpu_kms_destroy(struct msm_kms *kms) > > _dpu_kms_hw_destroy(dpu_kms); > > } > > > > -static int dpu_kms_pm_suspend(struct device *dev) > > -{ > > - struct drm_device *ddev; > > - struct drm_modeset_acquire_ctx ctx; > > - struct drm_atomic_state *state; > > - struct dpu_kms *dpu_kms; > > - int ret = 0, num_crtcs = 0; > > - > > - if (!dev) > > - return -EINVAL; > > - > > - ddev = dev_get_drvdata(dev); > > - if (!ddev || !ddev_to_msm_kms(ddev)) > > - return -EINVAL; > > - > > - dpu_kms = to_dpu_kms(ddev_to_msm_kms(ddev)); > > - > > - /* disable hot-plug polling */ > > - drm_kms_helper_poll_disable(ddev); > > - > > - /* acquire modeset lock(s) */ > > - drm_modeset_acquire_init(&ctx, 0); > > - > > -retry: > > - DPU_ATRACE_BEGIN("kms_pm_suspend"); > > - > > - ret = drm_modeset_lock_all_ctx(ddev, &ctx); > > - if (ret) > > - goto unlock; > > - > > - /* save current state for resume */ > > - if (dpu_kms->suspend_state) > > - drm_atomic_state_put(dpu_kms->suspend_state); > > - dpu_kms->suspend_state = drm_atomic_helper_duplicate_state(ddev, &ctx); > > - if (IS_ERR_OR_NULL(dpu_kms->suspend_state)) { > > - DRM_ERROR("failed to back up suspend state\n"); > > - dpu_kms->suspend_state = NULL; > > - goto unlock; > > - } > > - > > - /* create atomic state to disable all CRTCs */ > > - state = drm_atomic_state_alloc(ddev); > > - if (IS_ERR_OR_NULL(state)) { > > - DRM_ERROR("failed to allocate crtc disable state\n"); > > - goto unlock; > > - } > > - > > - state->acquire_ctx = &ctx; > > - > > - /* check for nothing to do */ > > - if (num_crtcs == 0) { > > - DRM_DEBUG("all crtcs are already in the off state\n"); > > - drm_atomic_state_put(state); > > - goto suspended; > > - } > > - > > - /* commit the "disable all" state */ > > - ret = drm_atomic_commit(state); > > - if (ret < 0) { > > - DRM_ERROR("failed to disable crtcs, %d\n", ret); > > - drm_atomic_state_put(state); > > - goto unlock; > > - } > > - > > -suspended: > > - dpu_kms->suspend_block = true; > > - > > -unlock: > > - if (ret == -EDEADLK) { > > - drm_modeset_backoff(&ctx); > > - goto retry; > > - } > > - drm_modeset_drop_locks(&ctx); > > - drm_modeset_acquire_fini(&ctx); > > - > > - DPU_ATRACE_END("kms_pm_suspend"); > > - return 0; > > -} > > - > > -static int dpu_kms_pm_resume(struct device *dev) > > -{ > > - struct drm_device *ddev; > > - struct dpu_kms *dpu_kms; > > - int ret; > > - > > - if (!dev) > > - return -EINVAL; > > - > > - ddev = dev_get_drvdata(dev); > > - if (!ddev || !ddev_to_msm_kms(ddev)) > > - return -EINVAL; > > - > > - dpu_kms = to_dpu_kms(ddev_to_msm_kms(ddev)); > > - > > - DPU_ATRACE_BEGIN("kms_pm_resume"); > > - > > - drm_mode_config_reset(ddev); > > - > > - drm_modeset_lock_all(ddev); > > - > > - dpu_kms->suspend_block = false; > > - > > - if (dpu_kms->suspend_state) { > > - dpu_kms->suspend_state->acquire_ctx = > > - ddev->mode_config.acquire_ctx; > > - ret = drm_atomic_commit(dpu_kms->suspend_state); > > - if (ret < 0) { > > - DRM_ERROR("failed to restore state, %d\n", ret); > > - drm_atomic_state_put(dpu_kms->suspend_state); > > - } > > - dpu_kms->suspend_state = NULL; > > - } > > - drm_modeset_unlock_all(ddev); > > - > > - /* enable hot-plug polling */ > > - drm_kms_helper_poll_enable(ddev); > > - > > - DPU_ATRACE_END("kms_pm_resume"); > > - return 0; > > -} > > - > > static void _dpu_kms_set_encoder_mode(struct msm_kms *kms, > > struct drm_encoder *encoder, > > bool cmd_mode) > > @@ -873,8 +752,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/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > > index 66d466628e2b..1991cbe424a3 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > > @@ -137,7 +137,6 @@ struct dpu_kms { > > > > /* saved atomic state during system suspend */ > > struct drm_atomic_state *suspend_state; > > - bool suspend_block; > > > > struct dpu_rm rm; > > bool rm_init; > > @@ -176,20 +175,6 @@ static inline bool dpu_kms_is_suspend_state(struct drm_device *dev) > > return to_dpu_kms(ddev_to_msm_kms(dev))->suspend_state != NULL; > > } > > > > -/** > > - * dpu_kms_is_suspend_blocked - whether or not commits are blocked due to pm > > - * suspend status > > - * @dev: Pointer to drm device > > - * Return: True if commits should be rejected due to pm suspend > > - */ > > -static inline bool dpu_kms_is_suspend_blocked(struct drm_device *dev) > > -{ > > - if (!dpu_kms_is_suspend_state(dev)) > > - return false; > > - > > - return to_dpu_kms(ddev_to_msm_kms(dev))->suspend_block; > > -} > > - > > /** > > * Debugfs functions - extra helper functions for debugfs support > > * > > -- > > 2.19.0.605.g01d371f741-goog > > > > -- > Sean Paul, Software Engineer, Google / Chromium OS
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 0a683e65a9f3..ff06b50dfc87 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -709,127 +709,6 @@ static void dpu_kms_destroy(struct msm_kms *kms) _dpu_kms_hw_destroy(dpu_kms); } -static int dpu_kms_pm_suspend(struct device *dev) -{ - struct drm_device *ddev; - struct drm_modeset_acquire_ctx ctx; - struct drm_atomic_state *state; - struct dpu_kms *dpu_kms; - int ret = 0, num_crtcs = 0; - - if (!dev) - return -EINVAL; - - ddev = dev_get_drvdata(dev); - if (!ddev || !ddev_to_msm_kms(ddev)) - return -EINVAL; - - dpu_kms = to_dpu_kms(ddev_to_msm_kms(ddev)); - - /* disable hot-plug polling */ - drm_kms_helper_poll_disable(ddev); - - /* acquire modeset lock(s) */ - drm_modeset_acquire_init(&ctx, 0); - -retry: - DPU_ATRACE_BEGIN("kms_pm_suspend"); - - ret = drm_modeset_lock_all_ctx(ddev, &ctx); - if (ret) - goto unlock; - - /* save current state for resume */ - if (dpu_kms->suspend_state) - drm_atomic_state_put(dpu_kms->suspend_state); - dpu_kms->suspend_state = drm_atomic_helper_duplicate_state(ddev, &ctx); - if (IS_ERR_OR_NULL(dpu_kms->suspend_state)) { - DRM_ERROR("failed to back up suspend state\n"); - dpu_kms->suspend_state = NULL; - goto unlock; - } - - /* create atomic state to disable all CRTCs */ - state = drm_atomic_state_alloc(ddev); - if (IS_ERR_OR_NULL(state)) { - DRM_ERROR("failed to allocate crtc disable state\n"); - goto unlock; - } - - state->acquire_ctx = &ctx; - - /* check for nothing to do */ - if (num_crtcs == 0) { - DRM_DEBUG("all crtcs are already in the off state\n"); - drm_atomic_state_put(state); - goto suspended; - } - - /* commit the "disable all" state */ - ret = drm_atomic_commit(state); - if (ret < 0) { - DRM_ERROR("failed to disable crtcs, %d\n", ret); - drm_atomic_state_put(state); - goto unlock; - } - -suspended: - dpu_kms->suspend_block = true; - -unlock: - if (ret == -EDEADLK) { - drm_modeset_backoff(&ctx); - goto retry; - } - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); - - DPU_ATRACE_END("kms_pm_suspend"); - return 0; -} - -static int dpu_kms_pm_resume(struct device *dev) -{ - struct drm_device *ddev; - struct dpu_kms *dpu_kms; - int ret; - - if (!dev) - return -EINVAL; - - ddev = dev_get_drvdata(dev); - if (!ddev || !ddev_to_msm_kms(ddev)) - return -EINVAL; - - dpu_kms = to_dpu_kms(ddev_to_msm_kms(ddev)); - - DPU_ATRACE_BEGIN("kms_pm_resume"); - - drm_mode_config_reset(ddev); - - drm_modeset_lock_all(ddev); - - dpu_kms->suspend_block = false; - - if (dpu_kms->suspend_state) { - dpu_kms->suspend_state->acquire_ctx = - ddev->mode_config.acquire_ctx; - ret = drm_atomic_commit(dpu_kms->suspend_state); - if (ret < 0) { - DRM_ERROR("failed to restore state, %d\n", ret); - drm_atomic_state_put(dpu_kms->suspend_state); - } - dpu_kms->suspend_state = NULL; - } - drm_modeset_unlock_all(ddev); - - /* enable hot-plug polling */ - drm_kms_helper_poll_enable(ddev); - - DPU_ATRACE_END("kms_pm_resume"); - return 0; -} - static void _dpu_kms_set_encoder_mode(struct msm_kms *kms, struct drm_encoder *encoder, bool cmd_mode) @@ -873,8 +752,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/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 66d466628e2b..1991cbe424a3 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -137,7 +137,6 @@ struct dpu_kms { /* saved atomic state during system suspend */ struct drm_atomic_state *suspend_state; - bool suspend_block; struct dpu_rm rm; bool rm_init; @@ -176,20 +175,6 @@ static inline bool dpu_kms_is_suspend_state(struct drm_device *dev) return to_dpu_kms(ddev_to_msm_kms(dev))->suspend_state != NULL; } -/** - * dpu_kms_is_suspend_blocked - whether or not commits are blocked due to pm - * suspend status - * @dev: Pointer to drm device - * Return: True if commits should be rejected due to pm suspend - */ -static inline bool dpu_kms_is_suspend_blocked(struct drm_device *dev) -{ - if (!dpu_kms_is_suspend_state(dev)) - return false; - - return to_dpu_kms(ddev_to_msm_kms(dev))->suspend_block; -} - /** * Debugfs functions - extra helper functions for debugfs support *
PM resume was crashing during dpu_kms_pm_resume. This patch removes dpu_kms_pm_suspend/resume so that msm_pm_suspend/resume uses the atomic helpers instead (see next patch). This patch also removes dpu_kms_is_suspend_blocked since it is never called. Signed-off-by: Bruce Wang <bzwang@chromium.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 123 ------------------------ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 15 --- 2 files changed, 138 deletions(-)