| Message ID | 20181005210403.54615-4-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 05:04:02PM -0400, Bruce Wang wrote: > 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 for > dpu_crtc_enable/disable. This patch removes the suspend logic from the crtc and > removes the relevant tracing from dpu_trace. Since we removed all calls > to dpu_kms_is_suspend_state, we can remove that function and the > suspend_state field of dpu_kms as well. > > v2: added patch to patchset > v3: reworded commit body and moved deletion of dpu_kms_is_suspend_state and > suspend_state to this patch > > Signed-off-by: Bruce Wang <bzwang@chromium.org> Reviewed-by: Sean Paul <seanpaul@chromium.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 48 ++--------------------- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 - > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 16 -------- > drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 15 +------ > 4 files changed, 6 insertions(+), 75 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..ac604055a824 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 > @@ -951,9 +922,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 +934,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 +1000,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 +1255,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_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 > * > 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 >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index d4530d60767b..ac604055a824 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 @@ -951,9 +922,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 +934,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 +1000,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 +1255,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_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 * 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)
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 for dpu_crtc_enable/disable. This patch removes the suspend logic from the crtc and removes the relevant tracing from dpu_trace. Since we removed all calls to dpu_kms_is_suspend_state, we can remove that function and the suspend_state field of dpu_kms as well. v2: added patch to patchset v3: reworded commit body and moved deletion of dpu_kms_is_suspend_state and suspend_state to this patch Signed-off-by: Bruce Wang <bzwang@chromium.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 48 ++--------------------- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 - drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 16 -------- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 15 +------ 4 files changed, 6 insertions(+), 75 deletions(-)