From 26b6ae2d211baa80c9b22bd3dcbf559fdf7fd7a9 Mon Sep 17 00:00:00 2001 From: Veera Sundaram Sankaran Date: Thu, 22 Oct 2020 11:07:31 -0700 Subject: [PATCH 1/3] disp: msm: sde: fix Trusted-ui state transition check Fix the checks done as part of the trsuted-ui transition. Allow full validation to go through whenever the new vm request or old vm request is not NONE. Split the checks and the vm acquire, so vm acquire can be done after all necessary checks are over. Change-Id: I91165010ad110193c2ca3947af8c6504cd259919 Signed-off-by: Veera Sundaram Sankaran --- msm/sde/sde_kms.c | 68 +++++++++++++++++++++++++++++------------ msm/sde/sde_vm_common.c | 8 +---- 2 files changed, 49 insertions(+), 27 deletions(-) diff --git a/msm/sde/sde_kms.c b/msm/sde/sde_kms.c index 8e5fe0c25892..6c720ceb24e6 100644 --- a/msm/sde/sde_kms.c +++ b/msm/sde/sde_kms.c @@ -2502,6 +2502,12 @@ static int sde_kms_check_vm_request(struct msm_kms *kms, if (!vm_ops) return 0; + if (!vm_ops->vm_request_valid || !vm_ops->vm_owns_hw || + !vm_ops->vm_acquire) + return -EINVAL; + + sde_vm_lock(sde_kms); + for_each_oldnew_crtc_in_state(state, crtc, old_cstate, new_cstate, i) { struct sde_crtc_state *old_state = NULL, *new_state = NULL; @@ -2516,12 +2522,27 @@ static int sde_kms_check_vm_request(struct msm_kms *kms, old_vm_req = sde_crtc_get_property(old_state, CRTC_PROP_VM_REQ_STATE); - /** + /* * No active request if the transition is from * VM_REQ_NONE to VM_REQ_NONE */ - if (new_vm_req || old_vm_req) - vm_req_active = true; + if (old_vm_req || new_vm_req) { + rc = vm_ops->vm_request_valid(sde_kms, + old_vm_req, new_vm_req); + if (rc) { + SDE_ERROR( + "VM transition check failed; o_state:%d, n_state:%d, hw_owner:%d, rc:%d\n", + old_vm_req, new_vm_req, + vm_ops->vm_owns_hw(sde_kms), rc); + goto end; + } else if (old_vm_req == VM_REQ_ACQUIRE && + new_vm_req == VM_REQ_NONE) { + SDE_DEBUG( + "VM transition valid; ignore further checks\n"); + } else { + vm_req_active = true; + } + } idle_pc_state = sde_crtc_get_property(new_state, CRTC_PROP_IDLE_PC_STATE); @@ -2530,6 +2551,10 @@ static int sde_kms_check_vm_request(struct msm_kms *kms, commit_crtc_cnt++; } + /* return early if no active vm request */ + if (!vm_req_active) + goto end; + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { if (!crtc->state->active) continue; @@ -2539,36 +2564,39 @@ static int sde_kms_check_vm_request(struct msm_kms *kms, } /* Check for single crtc commits only on valid VM requests */ - if (vm_req_active && active_crtc && global_active_crtc && + if (active_crtc && global_active_crtc && (commit_crtc_cnt > sde_kms->catalog->max_trusted_vm_displays || global_crtc_cnt > sde_kms->catalog->max_trusted_vm_displays || active_crtc != global_active_crtc)) { SDE_ERROR( - "failed to switch VM due to CRTC concurrencies: MAX_CNT: %d active_cnt: %d global_cnt: %d active_crtc: %d global_crtc: %d\n", + "VM switch failed; MAX:%d a_cnt:%d g_cnt:%d a_crtc:%d g_crtc:%d\n", sde_kms->catalog->max_trusted_vm_displays, - commit_crtc_cnt, global_crtc_cnt, active_crtc, - global_active_crtc); - return -E2BIG; + commit_crtc_cnt, global_crtc_cnt, DRMID(active_crtc), + DRMID(global_active_crtc)); + rc = -E2BIG; + goto end; } - if (!vm_req_active) - return 0; - /* disable idle-pc before releasing the HW */ if ((new_vm_req == VM_REQ_RELEASE) && (idle_pc_state == IDLE_PC_ENABLE)) { - SDE_ERROR("failed to switch VM since idle-pc is enabled\n"); - return -EINVAL; + SDE_ERROR("VM switch failed since idle-pc is enabled\n"); + rc = -EINVAL; + goto end; } - sde_vm_lock(sde_kms); + if ((new_vm_req == VM_REQ_ACQUIRE) && !vm_ops->vm_owns_hw(sde_kms)) { + rc = vm_ops->vm_acquire(sde_kms); + if (rc) { + SDE_ERROR( + "VM acquire failed; o_state:%d, n_state:%d, hw_owner:%d, rc:%d\n", + old_vm_req, new_vm_req, + vm_ops->vm_owns_hw(sde_kms), rc); + goto end; + } + } - if (vm_ops->vm_request_valid) - rc = vm_ops->vm_request_valid(sde_kms, old_vm_req, new_vm_req); - if (rc) - SDE_ERROR( - "failed to complete vm transition request. old_state = %d, new_state = %d, hw_ownership: %d\n", - old_vm_req, new_vm_req, vm_ops->vm_owns_hw(sde_kms)); +end: sde_vm_unlock(sde_kms); return rc; diff --git a/msm/sde/sde_vm_common.c b/msm/sde/sde_vm_common.c index d48ce18ec0c3..5026a8aa64c2 100644 --- a/msm/sde/sde_vm_common.c +++ b/msm/sde/sde_vm_common.c @@ -306,14 +306,8 @@ int sde_vm_request_valid(struct sde_kms *sde_kms, rc = -EINVAL; break; case VM_REQ_ACQUIRE: - if (old_state != VM_REQ_RELEASE) { + if (old_state != VM_REQ_RELEASE) rc = -EINVAL; - } else if (!vm_ops->vm_owns_hw(sde_kms)) { - if (vm_ops->vm_acquire) - rc = vm_ops->vm_acquire(sde_kms); - else - rc = -EINVAL; - } break; default: SDE_ERROR("invalid vm request\n"); From 303c324c43ad3c71e9315f41fbde86bb1b17478f Mon Sep 17 00:00:00 2001 From: Veera Sundaram Sankaran Date: Tue, 20 Oct 2020 11:07:05 -0700 Subject: [PATCH 2/3] disp: msm: sde: fix crtc_state iteration during vm release Iterate over the crtc old/new state and decide which ones to process by checking the active state of both during the vm release in error cases. Change-Id: Iab20e89792c53fb72e0c00f1fa5091616c7afbf3 Signed-off-by: Veera Sundaram Sankaran --- msm/sde/sde_kms.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/msm/sde/sde_kms.c b/msm/sde/sde_kms.c index 6c720ceb24e6..b6e43ba51042 100644 --- a/msm/sde/sde_kms.c +++ b/msm/sde/sde_kms.c @@ -2689,18 +2689,21 @@ static void sde_kms_vm_res_release(struct msm_kms *kms, struct drm_atomic_state *state) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_cstate, *old_cstate; struct sde_vm_ops *vm_ops; enum sde_crtc_vm_req vm_req; struct sde_kms *sde_kms = to_sde_kms(kms); int i; - for_each_new_crtc_in_state(state, crtc, crtc_state, i) { - struct sde_crtc_state *cstate; + for_each_oldnew_crtc_in_state(state, crtc, old_cstate, new_cstate, i) { + struct sde_crtc_state *new_state; - cstate = to_sde_crtc_state(state->crtcs[0].new_state); + if (!new_cstate->active && !old_cstate->active) + continue; - vm_req = sde_crtc_get_property(cstate, CRTC_PROP_VM_REQ_STATE); + new_state = to_sde_crtc_state(new_cstate); + vm_req = sde_crtc_get_property(new_state, + CRTC_PROP_VM_REQ_STATE); if (vm_req != VM_REQ_ACQUIRE) return; } From a61f2493982abbfb1e815da5f020e16e288b3a72 Mon Sep 17 00:00:00 2001 From: Veera Sundaram Sankaran Date: Mon, 12 Oct 2020 21:43:21 -0700 Subject: [PATCH 3/3] disp: msm: sde: limit encoders attached to crtc during trusted-ui Add checks to block transition to trusted-vm when multiple encoders are attached to a crtc. This ensures, concurrent writeback is disabled before trusted-ui usecase. Additionally, avoid enabling / disabling the IRQs associated with the cwb encoder during all the transitions to avoid any unbalanced calls. Change-Id: I022077018ac9b7dfb62506cfaddcb60cb8b35ed8 Signed-off-by: Veera Sundaram Sankaran --- msm/sde/sde_kms.c | 72 ++++++++++++++++++++++++++++++++++------------- msm/sde/sde_vm.h | 2 ++ 2 files changed, 54 insertions(+), 20 deletions(-) diff --git a/msm/sde/sde_kms.c b/msm/sde/sde_kms.c index b6e43ba51042..7b5bf289f159 100644 --- a/msm/sde/sde_kms.c +++ b/msm/sde/sde_kms.c @@ -1027,8 +1027,13 @@ int sde_kms_vm_primary_prepare_commit(struct sde_kms *sde_kms, sde_kms->hw_intr->ops.clear_all_irqs(sde_kms->hw_intr); /* enable the display path IRQ's */ - drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask) + 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, true); + } /* Schedule ESD work */ list_for_each_entry(connector, &ddev->mode_config.connector_list, head) @@ -1317,8 +1322,13 @@ int sde_kms_vm_trusted_post_commit(struct sde_kms *sde_kms, /* 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) + 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); + } list_for_each_entry(plane, &ddev->mode_config.plane_list, head) sde_plane_set_sid(plane, 0); @@ -1359,8 +1369,13 @@ int sde_kms_vm_pre_release(struct sde_kms *sde_kms, } /* disable SDE irq's */ - drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask) + 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); @@ -2483,13 +2498,16 @@ static int sde_kms_check_vm_request(struct msm_kms *kms, struct sde_kms *sde_kms; struct drm_device *dev; struct drm_crtc *crtc; - struct drm_crtc_state *new_cstate, *old_cstate; + struct drm_encoder *encoder; + struct drm_crtc_state *new_cstate, *old_cstate, *active_cstate; uint32_t i, commit_crtc_cnt = 0, global_crtc_cnt = 0; + uint32_t crtc_encoder_cnt = 0; struct drm_crtc *active_crtc = NULL, *global_active_crtc = NULL; enum sde_crtc_vm_req old_vm_req = VM_REQ_NONE, new_vm_req = VM_REQ_NONE; struct sde_vm_ops *vm_ops; bool vm_req_active = false; enum sde_crtc_idle_pc_state idle_pc_state; + struct sde_mdss_cfg *catalog; int rc = 0; if (!kms || !state) @@ -2497,6 +2515,7 @@ static int sde_kms_check_vm_request(struct msm_kms *kms, sde_kms = to_sde_kms(kms); dev = sde_kms->dev; + catalog = sde_kms->catalog; vm_ops = sde_vm_get_ops(sde_kms); if (!vm_ops) @@ -2548,6 +2567,7 @@ static int sde_kms_check_vm_request(struct msm_kms *kms, CRTC_PROP_IDLE_PC_STATE); active_crtc = crtc; + active_cstate = new_cstate; commit_crtc_cnt++; } @@ -2563,24 +2583,36 @@ static int sde_kms_check_vm_request(struct msm_kms *kms, global_active_crtc = crtc; } - /* Check for single crtc commits only on valid VM requests */ - if (active_crtc && global_active_crtc && - (commit_crtc_cnt > sde_kms->catalog->max_trusted_vm_displays || - global_crtc_cnt > sde_kms->catalog->max_trusted_vm_displays || - active_crtc != global_active_crtc)) { - SDE_ERROR( - "VM switch failed; MAX:%d a_cnt:%d g_cnt:%d a_crtc:%d g_crtc:%d\n", - sde_kms->catalog->max_trusted_vm_displays, - commit_crtc_cnt, global_crtc_cnt, DRMID(active_crtc), - DRMID(global_active_crtc)); - rc = -E2BIG; - goto end; + if (active_crtc) { + drm_for_each_encoder_mask(encoder, active_crtc->dev, + active_cstate->encoder_mask) + crtc_encoder_cnt++; } - /* disable idle-pc before releasing the HW */ - if ((new_vm_req == VM_REQ_RELEASE) && - (idle_pc_state == IDLE_PC_ENABLE)) { - SDE_ERROR("VM switch failed since idle-pc is enabled\n"); + /* Check for single crtc commits only on valid VM requests */ + if (active_crtc && global_active_crtc && + (commit_crtc_cnt > catalog->max_trusted_vm_displays || + global_crtc_cnt > catalog->max_trusted_vm_displays || + active_crtc != global_active_crtc)) { + SDE_ERROR( + "VM switch failed; MAX:%d a_cnt:%d g_cnt:%d a_crtc:%d g_crtc:%d\n", + catalog->max_trusted_vm_displays, + commit_crtc_cnt, global_crtc_cnt, DRMID(active_crtc), + DRMID(global_active_crtc)); + rc = -E2BIG; + goto end; + + } else if ((new_vm_req == VM_REQ_RELEASE) && + ((idle_pc_state == IDLE_PC_ENABLE) || + (crtc_encoder_cnt > TRUSTED_VM_MAX_ENCODER_PER_CRTC))) { + /* + * disable idle-pc before releasing the HW + * allow only specified number of encoders on a given crtc + */ + SDE_ERROR( + "VM switch failed; idle-pc:%d max:%d encoder_cnt:%d\n", + idle_pc_state, TRUSTED_VM_MAX_ENCODER_PER_CRTC, + crtc_encoder_cnt); rc = -EINVAL; goto end; } diff --git a/msm/sde/sde_vm.h b/msm/sde/sde_vm.h index 6095da8c4b97..238452072d70 100644 --- a/msm/sde/sde_vm.h +++ b/msm/sde/sde_vm.h @@ -8,6 +8,8 @@ #include "msm_drv.h" +#define TRUSTED_VM_MAX_ENCODER_PER_CRTC 1 + struct sde_kms; /**