From 9c77f40c14bc397a759337e1880e0edf08a858df Mon Sep 17 00:00:00 2001 From: Steve Cohen Date: Fri, 9 Jul 2021 19:31:03 -0400 Subject: [PATCH 1/3] 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 | 8 ++- msm/sde/sde_encoder.c | 13 +++- msm/sde/sde_encoder.h | 7 +++ msm/sde/sde_kms.c | 143 +++++++++++++++++++++--------------------- 5 files changed, 116 insertions(+), 73 deletions(-) diff --git a/msm/sde/sde_crtc.c b/msm/sde/sde_crtc.c index 9caa41328e80..24b773058f52 100644 --- a/msm/sde/sde_crtc.c +++ b/msm/sde/sde_crtc.c @@ -6626,6 +6626,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 4071e4841485..75fb998ed06d 100644 --- a/msm/sde/sde_crtc.h +++ b/msm/sde/sde_crtc.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2015-2021 The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark @@ -966,4 +966,10 @@ void sde_crtc_reset_sw_state(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 3eeb518c1a63..9c103530d916 100644 --- a/msm/sde/sde_encoder.c +++ b/msm/sde/sde_encoder.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2014-2021, The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark @@ -1687,6 +1687,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 3bd432072c8b..a1c138c16999 100644 --- a/msm/sde/sde_encoder.h +++ b/msm/sde/sde_encoder.h @@ -1,4 +1,5 @@ /* + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark @@ -597,6 +598,12 @@ static inline u32 sde_encoder_get_dfps_maxfps(struct drm_encoder *drm_enc) */ void sde_encoder_virt_reset(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 4c5705ce81b3..85c5e20efb82 100644 --- a/msm/sde/sde_kms.c +++ b/msm/sde/sde_kms.c @@ -1034,6 +1034,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; @@ -1070,9 +1071,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); @@ -1282,6 +1285,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) { @@ -1289,7 +1358,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; @@ -1311,24 +1379,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) @@ -1339,62 +1396,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; - struct msm_drm_private *priv; - - ddev = sde_kms->dev; - - crtc = sde_kms_vm_get_vm_crtc(state); - if (!crtc) - return 0; - - priv = crtc->dev->dev_private; - /* 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); - - /* - * Flush event thread queue for any pending events as vblank work - * might get scheduled from drm_crtc_vblank_off - */ - kthread_flush_worker(&priv->event_thread[crtc->index].worker); - - /* 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) { @@ -1421,7 +1422,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; From 38a180a911b4b69cf4eeb2902407ea27dd5033f5 Mon Sep 17 00:00:00 2001 From: Jayaprakash Madisetty Date: Wed, 16 Feb 2022 19:50:08 +0530 Subject: [PATCH 2/3] disp: msm: cancel all delayed_works before triggering msm_lastclose This patch cancels all the delayed_off_works if scheduled and flushes the display threads for completion during msm_lastclose. The commit from msm_lastclose client modeset to disable any crtcs if enabled is always scheduled on primary crtc_commit thread. In the current issue, delayed_off_work is scheduled on secondary display crtc_commit thread and primary crtc_commit thread is scheduled to turn off active crtcs from msm_lastclose leading to null dereference access of sde_enc's cur_master. This race is avoided by serializing the operations in msm_lastclose. Change-Id: I30cc95b925c8134f0064816ebe2cfdb86a49fb36 Signed-off-by: Jayaprakash Madisetty --- msm/msm_drv.c | 21 +++++++++++++++++++++ msm/msm_drv.h | 2 ++ msm/sde/sde_kms.c | 22 ++++++++++++---------- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/msm/msm_drv.c b/msm/msm_drv.c index a522ca30d78b..9fece7fd6ce4 100644 --- a/msm/msm_drv.c +++ b/msm/msm_drv.c @@ -954,6 +954,25 @@ mdss_init_fail: return ret; } +void msm_atomic_flush_display_threads(struct msm_drm_private *priv) +{ + int i; + + if (!priv) { + SDE_ERROR("invalid private data\n"); + return; + } + + for (i = 0; i < priv->num_crtcs; i++) { + if (priv->disp_thread[i].thread) + kthread_flush_worker(&priv->disp_thread[i].worker); + if (priv->event_thread[i].thread) + kthread_flush_worker(&priv->event_thread[i].worker); + } + + kthread_flush_worker(&priv->pp_event_worker); +} + /* * DRM operations: */ @@ -1077,6 +1096,8 @@ static void msm_lastclose(struct drm_device *dev) DRM_INFO("wait for crtc mask 0x%x failed, commit anyway...\n", priv->pending_crtcs); + msm_atomic_flush_display_threads(priv); + if (priv->fbdev) { rc = drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); if (rc) diff --git a/msm/msm_drv.h b/msm/msm_drv.h index 438c9abbec32..364b615a1ba9 100644 --- a/msm/msm_drv.h +++ b/msm/msm_drv.h @@ -973,6 +973,8 @@ struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev); void msm_atomic_state_clear(struct drm_atomic_state *state); void msm_atomic_state_free(struct drm_atomic_state *state); +void msm_atomic_flush_display_threads(struct msm_drm_private *priv); + int msm_gem_init_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma, int npages); void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, diff --git a/msm/sde/sde_kms.c b/msm/sde/sde_kms.c index 85c5e20efb82..728cf9eec651 100644 --- a/msm/sde/sde_kms.c +++ b/msm/sde/sde_kms.c @@ -2570,6 +2570,11 @@ error: drm_framebuffer_put(fb); } + drm_for_each_crtc(crtc, dev) { + if (!ret && crtc_mask & drm_crtc_mask(crtc)) + sde_kms_cancel_delayed_work(crtc); + } + end: return ret; } @@ -3723,6 +3728,7 @@ static int sde_kms_trigger_null_flush(struct msm_kms *kms) { struct sde_kms *sde_kms; struct sde_splash_display *splash_display; + struct drm_crtc *crtc; int i, rc = 0; if (!kms) { @@ -3740,10 +3746,14 @@ static int sde_kms_trigger_null_flush(struct msm_kms *kms) splash_display = &sde_kms->splash_data.splash_display[i]; if (splash_display->cont_splash_enabled && splash_display->encoder) { + crtc = splash_display->encoder->crtc; SDE_DEBUG("triggering null commit on enc:%d\n", DRMID(splash_display->encoder)); SDE_EVT32(DRMID(splash_display->encoder), SDE_EVTLOG_FUNC_ENTRY); rc = _sde_kms_null_commit(sde_kms->dev, splash_display->encoder); + + if (!rc && crtc) + sde_kms_cancel_delayed_work(crtc); } } @@ -3804,7 +3814,7 @@ static inline int _sde_kms_pm_deepsleep_helper(struct sde_kms *sde_kms, static void _sde_kms_pm_suspend_idle_helper(struct sde_kms *sde_kms, struct device *dev) { - int i, ret, crtc_id = 0; + int ret, crtc_id = 0; struct drm_device *ddev = dev_get_drvdata(dev); struct drm_connector *conn; struct drm_connector_list_iter conn_iter; @@ -3841,15 +3851,7 @@ static void _sde_kms_pm_suspend_idle_helper(struct sde_kms *sde_kms, } drm_connector_list_iter_end(&conn_iter); - for (i = 0; i < priv->num_crtcs; i++) { - if (priv->disp_thread[i].thread) - kthread_flush_worker( - &priv->disp_thread[i].worker); - if (priv->event_thread[i].thread) - kthread_flush_worker( - &priv->event_thread[i].worker); - } - kthread_flush_worker(&priv->pp_event_worker); + msm_atomic_flush_display_threads(priv); } static int sde_kms_pm_suspend(struct device *dev) From 74e59e97a7730248f2193b4e8479da75ca6f7d65 Mon Sep 17 00:00:00 2001 From: Jayaprakash Madisetty Date: Thu, 14 Apr 2022 18:09:07 +0530 Subject: [PATCH 3/3] disp: msm: sde: skip msm_lastclose if display is stuck in splash This change skips msm_lastclose, when splash enabled builtin-displays equals number of actual displays and are stuck in continuous splash. It fixes the issue seen with change commit id 548b17185e95 ("disp: msm: send power_on event in dual display composer kill scenario"). Change-Id: I1f5417d8945db621dc20ab0a9cc0146eabae5e22 Signed-off-by: Jayaprakash Madisetty --- msm/msm_drv.c | 4 +--- msm/sde/sde_kms.c | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/msm/msm_drv.c b/msm/msm_drv.c index 9fece7fd6ce4..f091be5a252c 100644 --- a/msm/msm_drv.c +++ b/msm/msm_drv.c @@ -1069,10 +1069,8 @@ static void msm_lastclose(struct drm_device *dev) priv->pending_crtcs); rc = kms->funcs->trigger_null_flush(kms); - if (rc) { - DRM_ERROR("null flush commit failure during lastclose\n"); + if (rc) return; - } } /* diff --git a/msm/sde/sde_kms.c b/msm/sde/sde_kms.c index 728cf9eec651..2fa69f6af04d 100644 --- a/msm/sde/sde_kms.c +++ b/msm/sde/sde_kms.c @@ -3738,10 +3738,17 @@ static int sde_kms_trigger_null_flush(struct msm_kms *kms) sde_kms = to_sde_kms(kms); - if (!sde_kms->splash_data.num_splash_displays || - sde_kms->dsi_display_count == sde_kms->splash_data.num_splash_displays) - return rc; + /* If splash handoff is done, early return*/ + if (!sde_kms->splash_data.num_splash_displays) + return 0; + /* If all builtin-displays are having cont splash enabled, ignore lastclose*/ + if (sde_kms->dsi_display_count == sde_kms->splash_data.num_splash_displays) + return -EINVAL; + + /* Trigger NULL flush if built-in secondary/primary is stuck in splash + * while the primary/secondary is running respectively before lastclose. + */ for (i = 0; i < MAX_DSI_DISPLAYS; i++) { splash_display = &sde_kms->splash_data.splash_display[i]; @@ -3754,10 +3761,12 @@ static int sde_kms_trigger_null_flush(struct msm_kms *kms) if (!rc && crtc) sde_kms_cancel_delayed_work(crtc); + if (rc) + DRM_ERROR("null flush commit failure during lastclose\n"); } } - return rc; + return 0; } #ifdef CONFIG_DEEPSLEEP