From a42fd877c7dbf69712d811dc91b73f8b498736ea Mon Sep 17 00:00:00 2001 From: Steve Cohen Date: Fri, 9 Jul 2021 19:31:03 -0400 Subject: [PATCH] disp: msm: sde: cancel delayed work items during TUI transition Delayed work items may touch HW registers. If these work items run while HW is not owned by this VM it will lead to invalid access. This happens in video mode as HAL does not disable idle power-collapse in this mode. It can also happen with ESD status if lastclose or TUI transition failure occurs. Although there is a contract with user mode to turn off certain features, kernel cannot rely on it to always do the right thing. Prevent potential crashes from certain corner cases by cancelling all delayed work items when the HW ownership is transferred. Change-Id: I08da17f2ce72bf2fddf71924c3e8edd2e2715be8 Signed-off-by: Steve Cohen --- msm/sde/sde_crtc.c | 18 ++++++ msm/sde/sde_crtc.h | 6 ++ msm/sde/sde_encoder.c | 11 ++++ msm/sde/sde_encoder.h | 6 ++ msm/sde/sde_kms.c | 135 ++++++++++++++++++++++-------------------- 5 files changed, 113 insertions(+), 63 deletions(-) diff --git a/msm/sde/sde_crtc.c b/msm/sde/sde_crtc.c index 3b145cce59..7ce84677bd 100644 --- a/msm/sde/sde_crtc.c +++ b/msm/sde/sde_crtc.c @@ -7102,6 +7102,24 @@ static void __sde_crtc_idle_notify_work(struct kthread_work *work) } } +void sde_crtc_cancel_delayed_work(struct drm_crtc *crtc) +{ + struct sde_crtc *sde_crtc; + struct sde_crtc_state *cstate; + bool idle_status; + bool cache_status; + + if (!crtc || !crtc->state) + return; + + sde_crtc = to_sde_crtc(crtc); + cstate = to_sde_crtc_state(crtc->state); + + idle_status = kthread_cancel_delayed_work_sync(&sde_crtc->idle_notify_work); + cache_status = kthread_cancel_delayed_work_sync(&sde_crtc->static_cache_read_work); + SDE_EVT32(DRMID(crtc), idle_status, cache_status); +} + /* initialize crtc */ struct drm_crtc *sde_crtc_init(struct drm_device *dev, struct drm_plane *plane) { diff --git a/msm/sde/sde_crtc.h b/msm/sde/sde_crtc.h index 17b738f39c..662ef1c3ec 100644 --- a/msm/sde/sde_crtc.h +++ b/msm/sde/sde_crtc.h @@ -1049,4 +1049,10 @@ void sde_crtc_disable_cp_features(struct drm_crtc *crtc); */ void _sde_crtc_clear_dim_layers_v1(struct drm_crtc_state *state); +/** + * sde_crtc_cancel_delayed_work - cancel any pending work items for a given crtc + * @crtc: Pointer to DRM crtc object + */ +void sde_crtc_cancel_delayed_work(struct drm_crtc *crtc); + #endif /* _SDE_CRTC_H_ */ diff --git a/msm/sde/sde_encoder.c b/msm/sde/sde_encoder.c index 1591c35520..1fe64d66d7 100644 --- a/msm/sde/sde_encoder.c +++ b/msm/sde/sde_encoder.c @@ -1880,6 +1880,17 @@ static void _sde_encoder_rc_cancel_delayed(struct sde_encoder_virt *sde_enc, sw_event); } +void sde_encoder_cancel_delayed_work(struct drm_encoder *encoder) +{ + struct sde_encoder_virt *sde_enc; + + if (!encoder) + return; + + sde_enc = to_sde_encoder_virt(encoder); + _sde_encoder_rc_cancel_delayed(sde_enc, 0); +} + static void _sde_encoder_rc_kickoff_delayed(struct sde_encoder_virt *sde_enc, u32 sw_event) { diff --git a/msm/sde/sde_encoder.h b/msm/sde/sde_encoder.h index 0025722665..5529e2bd3a 100644 --- a/msm/sde/sde_encoder.h +++ b/msm/sde/sde_encoder.h @@ -613,6 +613,12 @@ void sde_encoder_virt_reset(struct drm_encoder *drm_enc); */ ktime_t sde_encoder_calc_last_vsync_timestamp(struct drm_encoder *drm_enc); +/** + * sde_encoder_cancel_delayed_work - cancel delayed off work for encoder + * @drm_enc: Pointer to drm encoder structure + */ +void sde_encoder_cancel_delayed_work(struct drm_encoder *encoder); + /** * sde_encoder_get_kms - retrieve the kms from encoder * @drm_enc: Pointer to drm encoder structure diff --git a/msm/sde/sde_kms.c b/msm/sde/sde_kms.c index 911a42ea02..20cff2a923 100644 --- a/msm/sde/sde_kms.c +++ b/msm/sde/sde_kms.c @@ -1049,6 +1049,7 @@ int sde_kms_vm_primary_prepare_commit(struct sde_kms *sde_kms, struct drm_connector *connector; struct sde_vm_ops *vm_ops; struct sde_crtc_state *cstate; + struct drm_connector_list_iter iter; enum sde_crtc_vm_req vm_req; int rc = 0; @@ -1085,9 +1086,11 @@ int sde_kms_vm_primary_prepare_commit(struct sde_kms *sde_kms, } /* Schedule ESD work */ - list_for_each_entry(connector, &ddev->mode_config.connector_list, head) + drm_connector_list_iter_begin(ddev, &iter); + drm_for_each_connector_iter(connector, &iter) if (drm_connector_mask(connector) & crtc->state->connector_mask) sde_connector_schedule_status_work(connector, true); + drm_connector_list_iter_end(&iter); /* enable vblank events */ drm_crtc_vblank_on(crtc); @@ -1296,6 +1299,72 @@ static void _sde_kms_release_splash_resource(struct sde_kms *sde_kms, } } +static void sde_kms_cancel_delayed_work(struct drm_crtc *crtc) +{ + struct drm_connector *connector; + struct drm_connector_list_iter iter; + struct drm_encoder *encoder; + + /* Cancel CRTC work */ + sde_crtc_cancel_delayed_work(crtc); + + /* Cancel ESD work */ + drm_connector_list_iter_begin(crtc->dev, &iter); + drm_for_each_connector_iter(connector, &iter) + if (drm_connector_mask(connector) & crtc->state->connector_mask) + sde_connector_schedule_status_work(connector, false); + drm_connector_list_iter_end(&iter); + + /* Cancel Idle-PC work */ + drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask) { + if (sde_encoder_in_clone_mode(encoder)) + continue; + + sde_encoder_cancel_delayed_work(encoder); + } +} + +int sde_kms_vm_pre_release(struct sde_kms *sde_kms, + struct drm_atomic_state *state, bool is_primary) +{ + struct drm_crtc *crtc; + struct drm_encoder *encoder; + int rc = 0; + + crtc = sde_kms_vm_get_vm_crtc(state); + if (!crtc) + return 0; + + /* if vm_req is enabled, once CRTC on the commit is guaranteed */ + sde_kms_wait_for_frame_transfer_complete(&sde_kms->base, crtc); + + sde_kms_cancel_delayed_work(crtc); + + /* disable SDE irq's */ + drm_for_each_encoder_mask(encoder, crtc->dev, + crtc->state->encoder_mask) { + if (sde_encoder_in_clone_mode(encoder)) + continue; + + sde_encoder_irq_control(encoder, false); + } + + if (is_primary) { + /* disable IRQ line */ + sde_irq_update(&sde_kms->base, false); + + /* disable vblank events */ + drm_crtc_vblank_off(crtc); + + /* reset sw state */ + sde_crtc_reset_sw_state(crtc); + } + + sde_dbg_set_hw_ownership_status(false); + + return rc; +} + int sde_kms_vm_trusted_post_commit(struct sde_kms *sde_kms, struct drm_atomic_state *state) { @@ -1303,7 +1372,6 @@ int sde_kms_vm_trusted_post_commit(struct sde_kms *sde_kms, struct drm_device *ddev; struct drm_crtc *crtc; struct drm_plane *plane; - struct drm_encoder *encoder; struct sde_crtc_state *cstate; struct drm_crtc_state *new_cstate; enum sde_crtc_vm_req vm_req; @@ -1325,24 +1393,13 @@ int sde_kms_vm_trusted_post_commit(struct sde_kms *sde_kms, if (vm_req != VM_REQ_RELEASE) return 0; - /* if vm_req is enabled, once CRTC on the commit is guaranteed */ - sde_kms_wait_for_frame_transfer_complete(&sde_kms->base, crtc); - - drm_for_each_encoder_mask(encoder, crtc->dev, - crtc->state->encoder_mask) { - if (sde_encoder_in_clone_mode(encoder)) - continue; - - sde_encoder_irq_control(encoder, false); - } + sde_kms_vm_pre_release(sde_kms, state, false); list_for_each_entry(plane, &ddev->mode_config.plane_list, head) sde_plane_set_sid(plane, 0); sde_hw_set_lutdma_sid(sde_kms->hw_sid, 0); - sde_dbg_set_hw_ownership_status(false); - sde_vm_lock(sde_kms); if (vm_ops->vm_release) @@ -1353,54 +1410,6 @@ int sde_kms_vm_trusted_post_commit(struct sde_kms *sde_kms, return rc; } -int sde_kms_vm_pre_release(struct sde_kms *sde_kms, - struct drm_atomic_state *state) -{ - struct drm_device *ddev; - struct drm_crtc *crtc; - struct drm_encoder *encoder; - struct drm_connector *connector; - int rc = 0; - - ddev = sde_kms->dev; - - crtc = sde_kms_vm_get_vm_crtc(state); - if (!crtc) - return 0; - - /* if vm_req is enabled, once CRTC on the commit is guaranteed */ - sde_kms_wait_for_frame_transfer_complete(&sde_kms->base, crtc); - - /* disable ESD work */ - list_for_each_entry(connector, - &ddev->mode_config.connector_list, head) { - if (drm_connector_mask(connector) & crtc->state->connector_mask) - sde_connector_schedule_status_work(connector, false); - } - - /* disable SDE irq's */ - drm_for_each_encoder_mask(encoder, crtc->dev, - crtc->state->encoder_mask) { - if (sde_encoder_in_clone_mode(encoder)) - continue; - - sde_encoder_irq_control(encoder, false); - } - - /* disable IRQ line */ - sde_irq_update(&sde_kms->base, false); - - /* disable vblank events */ - drm_crtc_vblank_off(crtc); - - /* reset sw state */ - sde_crtc_reset_sw_state(crtc); - - sde_dbg_set_hw_ownership_status(false); - - return rc; -} - int sde_kms_vm_primary_post_commit(struct sde_kms *sde_kms, struct drm_atomic_state *state) { @@ -1427,7 +1436,7 @@ int sde_kms_vm_primary_post_commit(struct sde_kms *sde_kms, return 0; /* handle SDE pre-release */ - rc = sde_kms_vm_pre_release(sde_kms, state); + rc = sde_kms_vm_pre_release(sde_kms, state, true); if (rc) { SDE_ERROR("sde vm pre_release failed, rc=%d\n", rc); goto exit;