* [PATCH 0/4] drm/msm: Use atomic helpers for PM suspend/resume @ 2018-10-05 15:48 Bruce Wang [not found] ` <20181005154843.172326-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Bruce Wang @ 2018-10-05 15:48 UTC (permalink / raw) To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA Cc: jsanka-sgV2jX0FEOL9JmXXK+q4OQ, jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, seanpaul-F7+t8E8rja9g9hUCZPvPmw, robdclark-Re5JQEeQqe8AvxtiuMwx3w, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ Patches 1-2: Changes msm_pm_resume implementation to use atomic helpers and removes the previously used dpu_kms_pm_suspend/resume functions. Patches 3-4: Removes reliance on knowledge about suspend state from crtc Bruce Wang (4): drm/msm/dpu: Remove dpu_kms_pm_suspend/resume drm/msm: Cut dpu_kms hooks from msm_pm_suspend/resume drm/msm/dpu: Remove suspend state tracking from crtc drm/msm/dpu: Remove suspend_state from dpu_kms drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 61 +---------- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 - drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 123 ---------------------- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 31 ------ drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 15 +-- drivers/gpu/drm/msm/msm_drv.c | 28 ++--- drivers/gpu/drm/msm/msm_kms.h | 3 - 7 files changed, 17 insertions(+), 246 deletions(-) -- 2.19.0.605.g01d371f741-goog _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20181005154843.172326-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]
* [PATCH 1/4] drm/msm/dpu: Remove dpu_kms_pm_suspend/resume [not found] ` <20181005154843.172326-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> @ 2018-10-05 15:48 ` Bruce Wang [not found] ` <20181005154843.172326-2-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2018-10-05 15:48 ` [PATCH 2/4] drm/msm: Cut dpu_kms hooks from msm_pm_suspend/resume Bruce Wang ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Bruce Wang @ 2018-10-05 15:48 UTC (permalink / raw) To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA Cc: jsanka-sgV2jX0FEOL9JmXXK+q4OQ, jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, seanpaul-F7+t8E8rja9g9hUCZPvPmw, robdclark-Re5JQEeQqe8AvxtiuMwx3w, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ 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(-) 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 _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <20181005154843.172326-2-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]
* Re: [PATCH 1/4] drm/msm/dpu: Remove dpu_kms_pm_suspend/resume [not found] ` <20181005154843.172326-2-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> @ 2018-10-05 17:27 ` Sean Paul 2018-10-05 17:35 ` Sean Paul 0 siblings, 1 reply; 10+ messages in thread From: Sean Paul @ 2018-10-05 17:27 UTC (permalink / raw) To: Bruce Wang Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ, robdclark-Re5JQEeQqe8AvxtiuMwx3w, seanpaul-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW 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 > -- Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] drm/msm/dpu: Remove dpu_kms_pm_suspend/resume 2018-10-05 17:27 ` Sean Paul @ 2018-10-05 17:35 ` Sean Paul 0 siblings, 0 replies; 10+ messages in thread From: Sean Paul @ 2018-10-05 17:35 UTC (permalink / raw) To: Bruce Wang Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ, robdclark-Re5JQEeQqe8AvxtiuMwx3w, seanpaul-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW 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 -- Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] drm/msm: Cut dpu_kms hooks from msm_pm_suspend/resume [not found] ` <20181005154843.172326-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2018-10-05 15:48 ` [PATCH 1/4] drm/msm/dpu: Remove dpu_kms_pm_suspend/resume Bruce Wang @ 2018-10-05 15:48 ` Bruce Wang [not found] ` <20181005154843.172326-3-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2018-10-05 15:48 ` [PATCH 3/4] drm/msm/dpu: Remove suspend state tracking from crtc Bruce Wang 2018-10-05 15:48 ` [PATCH 4/4] drm/msm/dpu: Remove suspend_state from dpu_kms Bruce Wang 3 siblings, 1 reply; 10+ messages in thread From: Bruce Wang @ 2018-10-05 15:48 UTC (permalink / raw) To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA Cc: jsanka-sgV2jX0FEOL9JmXXK+q4OQ, jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, seanpaul-F7+t8E8rja9g9hUCZPvPmw, robdclark-Re5JQEeQqe8AvxtiuMwx3w, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ 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; 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 -- 2.19.0.605.g01d371f741-goog _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <20181005154843.172326-3-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]
* Re: [PATCH 2/4] drm/msm: Cut dpu_kms hooks from msm_pm_suspend/resume [not found] ` <20181005154843.172326-3-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> @ 2018-10-05 17:34 ` Sean Paul 0 siblings, 0 replies; 10+ messages in thread From: Sean Paul @ 2018-10-05 17:34 UTC (permalink / raw) To: Bruce Wang Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ, robdclark-Re5JQEeQqe8AvxtiuMwx3w, seanpaul-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW 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 > -- Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] drm/msm/dpu: Remove suspend state tracking from crtc [not found] ` <20181005154843.172326-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2018-10-05 15:48 ` [PATCH 1/4] drm/msm/dpu: Remove dpu_kms_pm_suspend/resume Bruce Wang 2018-10-05 15:48 ` [PATCH 2/4] drm/msm: Cut dpu_kms hooks from msm_pm_suspend/resume Bruce Wang @ 2018-10-05 15:48 ` Bruce Wang [not found] ` <20181005154843.172326-4-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2018-10-05 15:48 ` [PATCH 4/4] drm/msm/dpu: Remove suspend_state from dpu_kms Bruce Wang 3 siblings, 1 reply; 10+ messages in thread From: Bruce Wang @ 2018-10-05 15:48 UTC (permalink / raw) To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA Cc: jsanka-sgV2jX0FEOL9JmXXK+q4OQ, jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, seanpaul-F7+t8E8rja9g9hUCZPvPmw, robdclark-Re5JQEeQqe8AvxtiuMwx3w, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ dpu_crtc_enable/disable/reset are only being called upon initialization/destruction of the device and as part of the PM suspend/resume handling. Since core suspend and resume calls are guarenteed to not overlap with one another we don't need to track whether or not we're in a suspended state from inside the crtc. This patch removes the suspend logic from the crtc, and removes the relevant tracing from dpu_trace. Signed-off-by: Bruce Wang <bzwang@chromium.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 61 ++--------------------- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 - drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 15 +----- 3 files changed, 7 insertions(+), 71 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index d4530d60767b..31e372554e0c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -815,35 +815,6 @@ static void _dpu_crtc_vblank_enable_no_lock( } } -/** - * _dpu_crtc_set_suspend - notify crtc of suspend enable/disable - * @crtc: Pointer to drm crtc object - * @enable: true to enable suspend, false to indicate resume - */ -static void _dpu_crtc_set_suspend(struct drm_crtc *crtc, bool enable) -{ - struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); - - DRM_DEBUG_KMS("crtc%d suspend = %d\n", crtc->base.id, enable); - - mutex_lock(&dpu_crtc->crtc_lock); - - /* - * If the vblank is enabled, release a power reference on suspend - * and take it back during resume (if it is still enabled). - */ - trace_dpu_crtc_set_suspend(DRMID(&dpu_crtc->base), enable, dpu_crtc); - if (dpu_crtc->suspend == enable) - DPU_DEBUG("crtc%d suspend already set to %d, ignoring update\n", - crtc->base.id, enable); - else if (dpu_crtc->enabled && dpu_crtc->vblank_requested) { - _dpu_crtc_vblank_enable_no_lock(dpu_crtc, !enable); - } - - dpu_crtc->suspend = enable; - mutex_unlock(&dpu_crtc->crtc_lock); -} - /** * dpu_crtc_duplicate_state - state duplicate hook * @crtc: Pointer to drm crtc structure @@ -881,25 +852,14 @@ static struct drm_crtc_state *dpu_crtc_duplicate_state(struct drm_crtc *crtc) */ static void dpu_crtc_reset(struct drm_crtc *crtc) { - struct dpu_crtc *dpu_crtc; struct dpu_crtc_state *cstate; - if (!crtc) { - DPU_ERROR("invalid crtc\n"); - return; - } - - /* revert suspend actions, if necessary */ - if (dpu_kms_is_suspend_state(crtc->dev)) - _dpu_crtc_set_suspend(crtc, false); - /* remove previous state, if present */ if (crtc->state) { dpu_crtc_destroy_state(crtc, crtc->state); - crtc->state = 0; + crtc->state = NULL; } - dpu_crtc = to_dpu_crtc(crtc); cstate = kzalloc(sizeof(*cstate), GFP_KERNEL); if (!cstate) { DPU_ERROR("failed to allocate state\n"); @@ -951,9 +911,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc) DRM_DEBUG_KMS("crtc%d\n", crtc->base.id); - if (dpu_kms_is_suspend_state(crtc->dev)) - _dpu_crtc_set_suspend(crtc, true); - /* Disable/save vblank irq handling */ drm_crtc_vblank_off(crtc); @@ -966,8 +923,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc) atomic_read(&dpu_crtc->frame_pending)); trace_dpu_crtc_disable(DRMID(crtc), false, dpu_crtc); - if (dpu_crtc->enabled && !dpu_crtc->suspend && - dpu_crtc->vblank_requested) { + if (dpu_crtc->enabled && dpu_crtc->vblank_requested) { _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false); } dpu_crtc->enabled = false; @@ -1033,8 +989,7 @@ static void dpu_crtc_enable(struct drm_crtc *crtc, mutex_lock(&dpu_crtc->crtc_lock); trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc); - if (!dpu_crtc->enabled && !dpu_crtc->suspend && - dpu_crtc->vblank_requested) { + if (!dpu_crtc->enabled && dpu_crtc->vblank_requested) { _dpu_crtc_vblank_enable_no_lock(dpu_crtc, true); } dpu_crtc->enabled = true; @@ -1289,17 +1244,11 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc, int dpu_crtc_vblank(struct drm_crtc *crtc, bool en) { - struct dpu_crtc *dpu_crtc; - - if (!crtc) { - DPU_ERROR("invalid crtc\n"); - return -EINVAL; - } - dpu_crtc = to_dpu_crtc(crtc); + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); mutex_lock(&dpu_crtc->crtc_lock); trace_dpu_crtc_vblank(DRMID(&dpu_crtc->base), en, dpu_crtc); - if (dpu_crtc->enabled && !dpu_crtc->suspend) { + if (dpu_crtc->enabled) { _dpu_crtc_vblank_enable_no_lock(dpu_crtc, en); } dpu_crtc->vblank_requested = en; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h index 3723b4830335..7dc181aabb4d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h @@ -133,7 +133,6 @@ struct dpu_crtc_frame_event { * @play_count : frame count between crtc enable and disable * @vblank_cb_time : ktime at vblank count reset * @vblank_requested : whether the user has requested vblank events - * @suspend : whether or not a suspend operation is in progress * @enabled : whether the DPU CRTC is currently enabled. updated in the * commit-thread, not state-swap time which is earlier, so * safe to make decisions on during VBLANK on/off work @@ -169,7 +168,6 @@ struct dpu_crtc { u64 play_count; ktime_t vblank_cb_time; bool vblank_requested; - bool suspend; bool enabled; struct list_head feature_list; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index 636b31b0d311..0c122e173892 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -749,7 +749,6 @@ TRACE_EVENT(dpu_crtc_vblank_enable, __field( uint32_t, enc_id ) __field( bool, enable ) __field( bool, enabled ) - __field( bool, suspend ) __field( bool, vblank_requested ) ), TP_fast_assign( @@ -757,15 +756,12 @@ TRACE_EVENT(dpu_crtc_vblank_enable, __entry->enc_id = enc_id; __entry->enable = enable; __entry->enabled = crtc->enabled; - __entry->suspend = crtc->suspend; __entry->vblank_requested = crtc->vblank_requested; ), - TP_printk("id:%u encoder:%u enable:%s state{enabled:%s suspend:%s " - "vblank_req:%s}", + TP_printk("id:%u encoder:%u enable:%s state{enabled:%s vblank_req:%s}", __entry->drm_id, __entry->enc_id, __entry->enable ? "true" : "false", __entry->enabled ? "true" : "false", - __entry->suspend ? "true" : "false", __entry->vblank_requested ? "true" : "false") ); @@ -776,26 +772,19 @@ DECLARE_EVENT_CLASS(dpu_crtc_enable_template, __field( uint32_t, drm_id ) __field( bool, enable ) __field( bool, enabled ) - __field( bool, suspend ) __field( bool, vblank_requested ) ), TP_fast_assign( __entry->drm_id = drm_id; __entry->enable = enable; __entry->enabled = crtc->enabled; - __entry->suspend = crtc->suspend; __entry->vblank_requested = crtc->vblank_requested; ), - TP_printk("id:%u enable:%s state{enabled:%s suspend:%s vblank_req:%s}", + TP_printk("id:%u enable:%s state{enabled:%s vblank_req:%s}", __entry->drm_id, __entry->enable ? "true" : "false", __entry->enabled ? "true" : "false", - __entry->suspend ? "true" : "false", __entry->vblank_requested ? "true" : "false") ); -DEFINE_EVENT(dpu_crtc_enable_template, dpu_crtc_set_suspend, - TP_PROTO(uint32_t drm_id, bool enable, struct dpu_crtc *crtc), - TP_ARGS(drm_id, enable, crtc) -); DEFINE_EVENT(dpu_crtc_enable_template, dpu_crtc_enable, TP_PROTO(uint32_t drm_id, bool enable, struct dpu_crtc *crtc), TP_ARGS(drm_id, enable, crtc) -- 2.19.0.605.g01d371f741-goog _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <20181005154843.172326-4-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]
* Re: [PATCH 3/4] drm/msm/dpu: Remove suspend state tracking from crtc [not found] ` <20181005154843.172326-4-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> @ 2018-10-05 18:04 ` Sean Paul 0 siblings, 0 replies; 10+ messages in thread From: Sean Paul @ 2018-10-05 18:04 UTC (permalink / raw) To: Bruce Wang Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ, robdclark-Re5JQEeQqe8AvxtiuMwx3w, seanpaul-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On Fri, Oct 05, 2018 at 11:48:42AM -0400, Bruce Wang wrote: > dpu_crtc_enable/disable/reset are only being called upon > initialization/destruction of the device and as part of the PM > suspend/resume handling. *and dpms/enable on/off > Since core suspend and resume calls > are guarenteed to not overlap with one another we don't need to *guaranteed It also might be more clear to just state "Since drm core's modeset locks serialize atomic commits we don't need to..." > track whether or not we're in a suspended state from inside the crtc. > This patch removes the suspend logic from the crtc, and removes the relevant > tracing from dpu_trace. > > Signed-off-by: Bruce Wang <bzwang@chromium.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 61 ++--------------------- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 - > drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 15 +----- > 3 files changed, 7 insertions(+), 71 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index d4530d60767b..31e372554e0c 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -815,35 +815,6 @@ static void _dpu_crtc_vblank_enable_no_lock( > } > } > > -/** > - * _dpu_crtc_set_suspend - notify crtc of suspend enable/disable > - * @crtc: Pointer to drm crtc object > - * @enable: true to enable suspend, false to indicate resume > - */ > -static void _dpu_crtc_set_suspend(struct drm_crtc *crtc, bool enable) > -{ > - struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); > - > - DRM_DEBUG_KMS("crtc%d suspend = %d\n", crtc->base.id, enable); > - > - mutex_lock(&dpu_crtc->crtc_lock); > - > - /* > - * If the vblank is enabled, release a power reference on suspend > - * and take it back during resume (if it is still enabled). > - */ > - trace_dpu_crtc_set_suspend(DRMID(&dpu_crtc->base), enable, dpu_crtc); > - if (dpu_crtc->suspend == enable) > - DPU_DEBUG("crtc%d suspend already set to %d, ignoring update\n", > - crtc->base.id, enable); > - else if (dpu_crtc->enabled && dpu_crtc->vblank_requested) { > - _dpu_crtc_vblank_enable_no_lock(dpu_crtc, !enable); > - } > - > - dpu_crtc->suspend = enable; > - mutex_unlock(&dpu_crtc->crtc_lock); > -} > - > /** > * dpu_crtc_duplicate_state - state duplicate hook > * @crtc: Pointer to drm crtc structure > @@ -881,25 +852,14 @@ static struct drm_crtc_state *dpu_crtc_duplicate_state(struct drm_crtc *crtc) > */ > static void dpu_crtc_reset(struct drm_crtc *crtc) > { Can you add a patch to the series to delete this function entirely and use drm_atomic_helper_crtc_reset in dpu_crtc_funcs instead? > - struct dpu_crtc *dpu_crtc; > struct dpu_crtc_state *cstate; > > - if (!crtc) { > - DPU_ERROR("invalid crtc\n"); > - return; > - } > - Since we're going to delete this function entirely, please also rollback the cleanups in it so the diff is a bit more easy to read. > - /* revert suspend actions, if necessary */ > - if (dpu_kms_is_suspend_state(crtc->dev)) > - _dpu_crtc_set_suspend(crtc, false); > - > /* remove previous state, if present */ > if (crtc->state) { > dpu_crtc_destroy_state(crtc, crtc->state); > - crtc->state = 0; > + crtc->state = NULL; > } > > - dpu_crtc = to_dpu_crtc(crtc); > cstate = kzalloc(sizeof(*cstate), GFP_KERNEL); > if (!cstate) { > DPU_ERROR("failed to allocate state\n"); > @@ -951,9 +911,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc) > > DRM_DEBUG_KMS("crtc%d\n", crtc->base.id); > > - if (dpu_kms_is_suspend_state(crtc->dev)) > - _dpu_crtc_set_suspend(crtc, true); > - > /* Disable/save vblank irq handling */ > drm_crtc_vblank_off(crtc); > > @@ -966,8 +923,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc) > atomic_read(&dpu_crtc->frame_pending)); > > trace_dpu_crtc_disable(DRMID(crtc), false, dpu_crtc); > - if (dpu_crtc->enabled && !dpu_crtc->suspend && > - dpu_crtc->vblank_requested) { > + if (dpu_crtc->enabled && dpu_crtc->vblank_requested) { > _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false); > } > dpu_crtc->enabled = false; > @@ -1033,8 +989,7 @@ static void dpu_crtc_enable(struct drm_crtc *crtc, > > mutex_lock(&dpu_crtc->crtc_lock); > trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc); > - if (!dpu_crtc->enabled && !dpu_crtc->suspend && > - dpu_crtc->vblank_requested) { > + if (!dpu_crtc->enabled && dpu_crtc->vblank_requested) { > _dpu_crtc_vblank_enable_no_lock(dpu_crtc, true); > } > dpu_crtc->enabled = true; > @@ -1289,17 +1244,11 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc, > > int dpu_crtc_vblank(struct drm_crtc *crtc, bool en) > { > - struct dpu_crtc *dpu_crtc; > - > - if (!crtc) { > - DPU_ERROR("invalid crtc\n"); > - return -EINVAL; > - } > - dpu_crtc = to_dpu_crtc(crtc); > + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); > > mutex_lock(&dpu_crtc->crtc_lock); > trace_dpu_crtc_vblank(DRMID(&dpu_crtc->base), en, dpu_crtc); > - if (dpu_crtc->enabled && !dpu_crtc->suspend) { > + if (dpu_crtc->enabled) { > _dpu_crtc_vblank_enable_no_lock(dpu_crtc, en); > } > dpu_crtc->vblank_requested = en; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h > index 3723b4830335..7dc181aabb4d 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h > @@ -133,7 +133,6 @@ struct dpu_crtc_frame_event { > * @play_count : frame count between crtc enable and disable > * @vblank_cb_time : ktime at vblank count reset > * @vblank_requested : whether the user has requested vblank events > - * @suspend : whether or not a suspend operation is in progress > * @enabled : whether the DPU CRTC is currently enabled. updated in the > * commit-thread, not state-swap time which is earlier, so > * safe to make decisions on during VBLANK on/off work > @@ -169,7 +168,6 @@ struct dpu_crtc { > u64 play_count; > ktime_t vblank_cb_time; > bool vblank_requested; > - bool suspend; > bool enabled; > > struct list_head feature_list; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > index 636b31b0d311..0c122e173892 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > @@ -749,7 +749,6 @@ TRACE_EVENT(dpu_crtc_vblank_enable, > __field( uint32_t, enc_id ) > __field( bool, enable ) > __field( bool, enabled ) > - __field( bool, suspend ) > __field( bool, vblank_requested ) > ), > TP_fast_assign( > @@ -757,15 +756,12 @@ TRACE_EVENT(dpu_crtc_vblank_enable, > __entry->enc_id = enc_id; > __entry->enable = enable; > __entry->enabled = crtc->enabled; > - __entry->suspend = crtc->suspend; > __entry->vblank_requested = crtc->vblank_requested; > ), > - TP_printk("id:%u encoder:%u enable:%s state{enabled:%s suspend:%s " > - "vblank_req:%s}", > + TP_printk("id:%u encoder:%u enable:%s state{enabled:%s vblank_req:%s}", > __entry->drm_id, __entry->enc_id, > __entry->enable ? "true" : "false", > __entry->enabled ? "true" : "false", > - __entry->suspend ? "true" : "false", > __entry->vblank_requested ? "true" : "false") > ); > > @@ -776,26 +772,19 @@ DECLARE_EVENT_CLASS(dpu_crtc_enable_template, > __field( uint32_t, drm_id ) > __field( bool, enable ) > __field( bool, enabled ) > - __field( bool, suspend ) > __field( bool, vblank_requested ) > ), > TP_fast_assign( > __entry->drm_id = drm_id; > __entry->enable = enable; > __entry->enabled = crtc->enabled; > - __entry->suspend = crtc->suspend; > __entry->vblank_requested = crtc->vblank_requested; > ), > - TP_printk("id:%u enable:%s state{enabled:%s suspend:%s vblank_req:%s}", > + TP_printk("id:%u enable:%s state{enabled:%s vblank_req:%s}", > __entry->drm_id, __entry->enable ? "true" : "false", > __entry->enabled ? "true" : "false", > - __entry->suspend ? "true" : "false", > __entry->vblank_requested ? "true" : "false") > ); > -DEFINE_EVENT(dpu_crtc_enable_template, dpu_crtc_set_suspend, > - TP_PROTO(uint32_t drm_id, bool enable, struct dpu_crtc *crtc), > - TP_ARGS(drm_id, enable, crtc) > -); > DEFINE_EVENT(dpu_crtc_enable_template, dpu_crtc_enable, > TP_PROTO(uint32_t drm_id, bool enable, struct dpu_crtc *crtc), > TP_ARGS(drm_id, enable, crtc) > -- > 2.19.0.605.g01d371f741-goog > -- Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] drm/msm/dpu: Remove suspend_state from dpu_kms [not found] ` <20181005154843.172326-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> ` (2 preceding siblings ...) 2018-10-05 15:48 ` [PATCH 3/4] drm/msm/dpu: Remove suspend state tracking from crtc Bruce Wang @ 2018-10-05 15:48 ` Bruce Wang [not found] ` <20181005154843.172326-5-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 3 siblings, 1 reply; 10+ messages in thread From: Bruce Wang @ 2018-10-05 15:48 UTC (permalink / raw) To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA Cc: jsanka-sgV2jX0FEOL9JmXXK+q4OQ, jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, seanpaul-F7+t8E8rja9g9hUCZPvPmw, robdclark-Re5JQEeQqe8AvxtiuMwx3w, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ The suspend_state field is no longer used and can be removed along with dpu_kms_is_suspend_state. Signed-off-by: Bruce Wang <bzwang@chromium.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 1991cbe424a3..2a3625eef6d3 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -135,9 +135,6 @@ struct dpu_kms { struct dpu_core_perf perf; - /* saved atomic state during system suspend */ - struct drm_atomic_state *suspend_state; - struct dpu_rm rm; bool rm_init; @@ -162,19 +159,6 @@ struct vsync_info { #define ddev_to_msm_kms(D) ((D) && (D)->dev_private ? \ ((struct msm_drm_private *)((D)->dev_private))->kms : NULL) -/** - * dpu_kms_is_suspend_state - whether or not the system is pm suspended - * @dev: Pointer to drm device - * Return: Suspend status - */ -static inline bool dpu_kms_is_suspend_state(struct drm_device *dev) -{ - if (!ddev_to_msm_kms(dev)) - return false; - - return to_dpu_kms(ddev_to_msm_kms(dev))->suspend_state != NULL; -} - /** * Debugfs functions - extra helper functions for debugfs support * -- 2.19.0.605.g01d371f741-goog _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <20181005154843.172326-5-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]
* Re: [PATCH 4/4] drm/msm/dpu: Remove suspend_state from dpu_kms [not found] ` <20181005154843.172326-5-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> @ 2018-10-05 18:05 ` Sean Paul 0 siblings, 0 replies; 10+ messages in thread From: Sean Paul @ 2018-10-05 18:05 UTC (permalink / raw) To: Bruce Wang Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ, robdclark-Re5JQEeQqe8AvxtiuMwx3w, seanpaul-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On Fri, Oct 05, 2018 at 11:48:43AM -0400, Bruce Wang wrote: > The suspend_state field is no longer used and can be removed along with > dpu_kms_is_suspend_state. > This can be rolled up into patch 3, since that's where the usage of these is being removed. Sean > Signed-off-by: Bruce Wang <bzwang@chromium.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 16 ---------------- > 1 file changed, 16 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > index 1991cbe424a3..2a3625eef6d3 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > @@ -135,9 +135,6 @@ struct dpu_kms { > > struct dpu_core_perf perf; > > - /* saved atomic state during system suspend */ > - struct drm_atomic_state *suspend_state; > - > struct dpu_rm rm; > bool rm_init; > > @@ -162,19 +159,6 @@ struct vsync_info { > #define ddev_to_msm_kms(D) ((D) && (D)->dev_private ? \ > ((struct msm_drm_private *)((D)->dev_private))->kms : NULL) > > -/** > - * dpu_kms_is_suspend_state - whether or not the system is pm suspended > - * @dev: Pointer to drm device > - * Return: Suspend status > - */ > -static inline bool dpu_kms_is_suspend_state(struct drm_device *dev) > -{ > - if (!ddev_to_msm_kms(dev)) > - return false; > - > - return to_dpu_kms(ddev_to_msm_kms(dev))->suspend_state != NULL; > -} > - > /** > * Debugfs functions - extra helper functions for debugfs support > * > -- > 2.19.0.605.g01d371f741-goog > -- Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-10-05 18:05 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-05 15:48 [PATCH 0/4] drm/msm: Use atomic helpers for PM suspend/resume Bruce Wang [not found] ` <20181005154843.172326-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2018-10-05 15:48 ` [PATCH 1/4] drm/msm/dpu: Remove dpu_kms_pm_suspend/resume Bruce Wang [not found] ` <20181005154843.172326-2-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2018-10-05 17:27 ` Sean Paul 2018-10-05 17:35 ` Sean Paul 2018-10-05 15:48 ` [PATCH 2/4] drm/msm: Cut dpu_kms hooks from msm_pm_suspend/resume Bruce Wang [not found] ` <20181005154843.172326-3-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2018-10-05 17:34 ` Sean Paul 2018-10-05 15:48 ` [PATCH 3/4] drm/msm/dpu: Remove suspend state tracking from crtc Bruce Wang [not found] ` <20181005154843.172326-4-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2018-10-05 18:04 ` Sean Paul 2018-10-05 15:48 ` [PATCH 4/4] drm/msm/dpu: Remove suspend_state from dpu_kms Bruce Wang [not found] ` <20181005154843.172326-5-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2018-10-05 18:05 ` Sean Paul
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.