From 1834bd53befc8897a675267a67b247690f7c7feb Mon Sep 17 00:00:00 2001 From: Jayaprakash Madisetty Date: Sat, 22 May 2021 17:16:25 +0530 Subject: [PATCH 1/6] disp: msm: sde: set ctl preference for built in displays Add changes to set CTL id preference for primary and secondary builtin displays. This change check for preference if already set from DT. If not preference will be set during connector init. This avoids CTL datapath switches on secondary when there is dual display usecase running concurrently with dp connect/disconnect. In current issue scenario, due to the secondary ctl datapath switch CTL_LAYER programming is happened on CTL_2 but CTL_FLUSH is happened on CTL_1 causing pp_done timeout issues. Change-Id: I5f1f52b7673740c48b249ab4d36e80b7a1d3db96 Signed-off-by: Jayaprakash Madisetty --- msm/sde/sde_connector.c | 3 +++ msm/sde/sde_hw_catalog.c | 39 +++++++++++++++++++++++++++++++++++++++ msm/sde/sde_hw_catalog.h | 8 ++++++++ msm/sde/sde_rm.c | 21 +++++++++++++++------ 4 files changed, 65 insertions(+), 6 deletions(-) diff --git a/msm/sde/sde_connector.c b/msm/sde/sde_connector.c index eeda022f1da7..a6de23aab43b 100644 --- a/msm/sde/sde_connector.c +++ b/msm/sde/sde_connector.c @@ -2994,6 +2994,9 @@ struct drm_connector *sde_connector_init(struct drm_device *dev, _sde_connector_lm_preference(c_conn, sde_kms, display_info.display_type); + sde_hw_ctl_set_preference(sde_kms->catalog, + display_info.display_type); + SDE_DEBUG("connector %d attach encoder %d\n", c_conn->base.base.id, encoder->base.id); diff --git a/msm/sde/sde_hw_catalog.c b/msm/sde/sde_hw_catalog.c index 74896a8edb2c..ce5caac2a667 100644 --- a/msm/sde/sde_hw_catalog.c +++ b/msm/sde/sde_hw_catalog.c @@ -1919,6 +1919,8 @@ static int sde_ctl_parse_dt(struct device_node *np, ctl_prop[HW_DISP].prop_name, i, &disp_pref); if (disp_pref && !strcmp(disp_pref, "primary")) set_bit(SDE_CTL_PRIMARY_PREF, &ctl->features); + if (disp_pref && !strcmp(disp_pref, "secondary")) + set_bit(SDE_CTL_SECONDARY_PREF, &ctl->features); if ((i < MAX_SPLIT_DISPLAY_CTL) && !(IS_SDE_CTL_REV_100(sde_cfg->ctl_rev))) set_bit(SDE_CTL_SPLIT_DISPLAY, &ctl->features); @@ -1937,6 +1939,43 @@ static int sde_ctl_parse_dt(struct device_node *np, return 0; } +void sde_hw_ctl_set_preference(struct sde_mdss_cfg *sde_cfg, + uint32_t disp_type) +{ + u32 i; + + if (!IS_SDE_CTL_REV_100(sde_cfg->ctl_rev)) + return; + + if (disp_type == SDE_CONNECTOR_PRIMARY) { + for (i = 0; i < sde_cfg->ctl_count; i++) { + /* Exit if already set in dt file*/ + if (sde_cfg->ctl[i].features & BIT(SDE_CTL_PRIMARY_PREF)) + return; + } + for (i = 0; i < sde_cfg->ctl_count; i++) { + /* Set preference here*/ + if (!(sde_cfg->ctl[i].features & BIT(SDE_CTL_SECONDARY_PREF))) { + set_bit(SDE_CTL_PRIMARY_PREF, &sde_cfg->ctl[i].features); + return; + } + } + } else if (disp_type == SDE_CONNECTOR_SECONDARY) { + for (i = 0; i < sde_cfg->ctl_count; i++) { + /* Exit if already set in dt file*/ + if (sde_cfg->ctl[i].features & BIT(SDE_CTL_SECONDARY_PREF)) + return; + } + for (i = 0; i < sde_cfg->ctl_count; i++) { + /* Set preference here*/ + if (!(sde_cfg->ctl[i].features & BIT(SDE_CTL_PRIMARY_PREF))) { + set_bit(SDE_CTL_SECONDARY_PREF, &sde_cfg->ctl[i].features); + return; + } + } + } +} + void sde_hw_mixer_set_preference(struct sde_mdss_cfg *sde_cfg, u32 num_lm, uint32_t disp_type) { diff --git a/msm/sde/sde_hw_catalog.h b/msm/sde/sde_hw_catalog.h index 6bfaf2bf5080..dd9d93d9f01f 100644 --- a/msm/sde/sde_hw_catalog.h +++ b/msm/sde/sde_hw_catalog.h @@ -465,6 +465,7 @@ enum { SDE_CTL_SPLIT_DISPLAY = 0x1, SDE_CTL_PINGPONG_SPLIT, SDE_CTL_PRIMARY_PREF, + SDE_CTL_SECONDARY_PREF, SDE_CTL_ACTIVE_CFG, SDE_CTL_UIDLE, SDE_CTL_UNIFIED_DSPP_FLUSH, @@ -1657,6 +1658,13 @@ struct sde_mdss_cfg *sde_hw_catalog_init(struct drm_device *dev); */ void sde_hw_catalog_deinit(struct sde_mdss_cfg *sde_cfg); +/** + * sde_hw_ctl_set_preference - set CTL preference for display + * @sde_cfg: pointer to mdss cfg + * @disp_type: type of display primary/secondary + */ +void sde_hw_ctl_set_preference(struct sde_mdss_cfg *sde_cfg, + uint32_t disp_type); /** * sde_hw_catalog_irq_offset_list_delete - delete the irq_offset_list * maintained by the catalog diff --git a/msm/sde/sde_rm.c b/msm/sde/sde_rm.c index f5221aae3f26..5314b3c28fdd 100644 --- a/msm/sde/sde_rm.c +++ b/msm/sde/sde_rm.c @@ -1227,7 +1227,8 @@ static int _sde_rm_reserve_ctls( while (_sde_rm_get_hw_locked(rm, &iter)) { const struct sde_hw_ctl *ctl = to_sde_hw_ctl(iter.blk->hw); unsigned long features = ctl->caps->features; - bool has_split_display, has_ppsplit, primary_pref; + bool has_split_display, has_ppsplit, primary_pref, secondary_pref; + bool is_conn_primary, is_conn_secondary; if (RESERVED_BY_OTHER(iter.blk, rsvp)) continue; @@ -1235,6 +1236,11 @@ static int _sde_rm_reserve_ctls( has_split_display = BIT(SDE_CTL_SPLIT_DISPLAY) & features; has_ppsplit = BIT(SDE_CTL_PINGPONG_SPLIT) & features; primary_pref = BIT(SDE_CTL_PRIMARY_PREF) & features; + secondary_pref = BIT(SDE_CTL_SECONDARY_PREF) & features; + is_conn_primary = (reqs->hw_res.display_type == + SDE_CONNECTOR_PRIMARY) ? true : false; + is_conn_secondary = (reqs->hw_res.display_type == + SDE_CONNECTOR_SECONDARY) ? true : false; SDE_DEBUG("ctl %d caps 0x%lX\n", iter.blk->id, features); @@ -1242,18 +1248,21 @@ static int _sde_rm_reserve_ctls( * bypass rest feature checks on finding CTL preferred * for primary displays. */ - if (!primary_pref && !_ctl_ids) { + if (!primary_pref && !secondary_pref && !_ctl_ids) { if (top->needs_split_display != has_split_display) continue; if (top->top_name == SDE_RM_TOPOLOGY_PPSPLIT && !has_ppsplit) continue; - } else if (!(reqs->hw_res.display_type == - SDE_CONNECTOR_PRIMARY && primary_pref) && !_ctl_ids) { + + if (is_conn_primary || is_conn_secondary) + continue; + } else if (!((is_conn_primary && primary_pref) || + (is_conn_secondary && secondary_pref)) && !_ctl_ids) { SDE_DEBUG( - "display pref not met. display_type: %d primary_pref: %d\n", - reqs->hw_res.display_type, primary_pref); + "display pref not met. display_type: %d features: 0x%lX\n", + reqs->hw_res.display_type, features); continue; } From 9d88585e1d132eece2877c43a3356151a8db7028 Mon Sep 17 00:00:00 2001 From: Krishna Manikandan Date: Wed, 23 Jun 2021 11:47:16 +0530 Subject: [PATCH 2/6] disp: msm: sde: add null check for drm file in msm_release Drm file is not set to NULL after freeing it from drm release. This can result in use-after-free issues in some scenarios. Add a mutex lock and other proper null checks to prevent such issues. Change-Id: Ic35b0a76166b0f47a354b1737e6f4c3ac1437ed4 Signed-off-by: Krishna Manikandan --- msm/msm_drv.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/msm/msm_drv.c b/msm/msm_drv.c index 00c321821e43..5597b8d72da1 100644 --- a/msm/msm_drv.c +++ b/msm/msm_drv.c @@ -83,6 +83,8 @@ (ktime_compare_safe(exp_ktime, cur_ktime) > 0));\ } while (0) +static DEFINE_MUTEX(msm_release_lock); + static void msm_fb_output_poll_changed(struct drm_device *dev) { struct msm_drm_private *priv = NULL; @@ -1487,13 +1489,25 @@ void msm_mode_object_event_notify(struct drm_mode_object *obj, static int msm_release(struct inode *inode, struct file *filp) { struct drm_file *file_priv = filp->private_data; - struct drm_minor *minor = file_priv->minor; - struct drm_device *dev = minor->dev; - struct msm_drm_private *priv = dev->dev_private; + struct drm_minor *minor; + struct drm_device *dev; + struct msm_drm_private *priv; struct msm_drm_event *node, *temp, *tmp_node; u32 count; unsigned long flags; LIST_HEAD(tmp_head); + int ret = 0; + + mutex_lock(&msm_release_lock); + + if (!file_priv) { + ret = -EINVAL; + goto end; + } + + minor = file_priv->minor; + dev = minor->dev; + priv = dev->dev_private; spin_lock_irqsave(&dev->event_lock, flags); list_for_each_entry_safe(node, temp, &priv->client_event_list, @@ -1531,7 +1545,11 @@ static int msm_release(struct inode *inode, struct file *filp) if (drm_is_current_master(file_priv)) msm_preclose(dev, file_priv); - return drm_release(inode, filp); + ret = drm_release(inode, filp); + filp->private_data = NULL; +end: + mutex_unlock(&msm_release_lock); + return ret; } /** From b34e02b6416d4a27e96f7954afb049069c8be592 Mon Sep 17 00:00:00 2001 From: Jayaprakash Madisetty Date: Thu, 24 Jun 2021 16:55:44 +0530 Subject: [PATCH 3/6] disp: msm: sde: enable merge_3d_active in ctl when mode_3d is set Add changes to set merge_3d_active in ctl datapath when mode3d is enabled without half panel partial update. Change-Id: I35c1f4c6045c4040cac730f23a6d9ce5f36cf5fb Signed-off-by: Jayaprakash Madisetty --- msm/sde/sde_encoder_dce.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/msm/sde/sde_encoder_dce.c b/msm/sde/sde_encoder_dce.c index 07bf4f6f6f7a..cde539fdd732 100644 --- a/msm/sde/sde_encoder_dce.c +++ b/msm/sde/sde_encoder_dce.c @@ -243,7 +243,7 @@ static void _dce_dsc_pipe_cfg(struct sde_hw_dsc *hw_dsc, if (mode_3d && disable_merge_3d && hw_pp->ops.reset_3d_mode) { SDE_DEBUG("disabling 3d mux \n"); hw_pp->ops.reset_3d_mode(hw_pp); - } else if (mode_3d && disable_merge_3d && hw_pp->ops.setup_3d_mode) { + } else if (mode_3d && !disable_merge_3d && hw_pp->ops.setup_3d_mode) { SDE_DEBUG("enabling 3d mux \n"); hw_pp->ops.setup_3d_mode(hw_pp, mode_3d); } @@ -313,7 +313,7 @@ static int _dce_dsc_setup_single(struct sde_encoder_virt *sde_enc, struct msm_display_dsc_info *dsc, unsigned long affected_displays, int index, const struct sde_rect *roi, int dsc_common_mode, - bool merge_3d, bool disable_merge_3d, bool mode_3d, + bool merge_3d, bool disable_merge_3d, enum sde_3d_blend_mode mode_3d, bool dsc_4hsmerge, bool half_panel_partial_update, int ich_res) { @@ -432,7 +432,7 @@ static int _dce_dsc_setup_helper(struct sde_encoder_virt *sde_enc, dsc_merge = ((num_dsc > num_intf) && !dsc->half_panel_pu) ? true : false; disable_merge_3d = (merge_3d && dsc->half_panel_pu) ? - false : true; + true : false; dsc_4hsmerge = (dsc_merge && num_dsc == 4 && num_intf == 1) ? true : false; From 79217384dbbe9d0a2f74f9e4c35bd447699428d2 Mon Sep 17 00:00:00 2001 From: Ritesh Kumar Date: Tue, 6 Jul 2021 16:02:24 +0530 Subject: [PATCH 4/6] disp: msm: dsi: Logging Improvement in dsi driver This change adds additional logs in dsi driver for easy debugging of issues related to command transfer. Change-Id: Ica784bed6c360b2760d6606d625837e23a22410c Signed-off-by: Ritesh Kumar --- msm/dsi/dsi_ctrl.c | 14 ++++++++++---- msm/dsi/dsi_ctrl_hw_cmn.c | 2 ++ msm/dsi/dsi_display.c | 3 +++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/msm/dsi/dsi_ctrl.c b/msm/dsi/dsi_ctrl.c index dcf19364cbff..d3d5cd20a203 100644 --- a/msm/dsi/dsi_ctrl.c +++ b/msm/dsi/dsi_ctrl.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2016-2020, The Linux Foundation. All rights reserved. + * Copyright (c) 2016-2021, The Linux Foundation. All rights reserved. */ #include @@ -1565,7 +1565,7 @@ static int dsi_message_tx(struct dsi_ctrl *dsi_ctrl, dsi_ctrl_validate_msg_flags(dsi_ctrl, msg, flags); - SDE_EVT32(dsi_ctrl->cell_index, SDE_EVTLOG_FUNC_ENTRY, flags); + SDE_EVT32(dsi_ctrl->cell_index, SDE_EVTLOG_FUNC_ENTRY, *flags, dsi_ctrl->cmd_len); if (dsi_ctrl->dma_wait_queued) dsi_ctrl_flush_cmd_dma_queue(dsi_ctrl); @@ -2959,7 +2959,10 @@ void dsi_ctrl_enable_status_interrupt(struct dsi_ctrl *dsi_ctrl, intr_idx >= DSI_STATUS_INTERRUPT_COUNT) return; - SDE_EVT32(dsi_ctrl->cell_index, SDE_EVTLOG_FUNC_ENTRY, intr_idx); + SDE_EVT32(dsi_ctrl->cell_index, intr_idx, + dsi_ctrl->irq_info.irq_num, dsi_ctrl->irq_info.irq_stat_mask, + dsi_ctrl->irq_info.irq_stat_refcount[intr_idx]); + spin_lock_irqsave(&dsi_ctrl->irq_info.irq_lock, flags); if (dsi_ctrl->irq_info.irq_stat_refcount[intr_idx] == 0) { @@ -2992,7 +2995,10 @@ void dsi_ctrl_disable_status_interrupt(struct dsi_ctrl *dsi_ctrl, if (!dsi_ctrl || intr_idx >= DSI_STATUS_INTERRUPT_COUNT) return; - SDE_EVT32_IRQ(dsi_ctrl->cell_index, SDE_EVTLOG_FUNC_ENTRY, intr_idx); + SDE_EVT32_IRQ(dsi_ctrl->cell_index, intr_idx, + dsi_ctrl->irq_info.irq_num, dsi_ctrl->irq_info.irq_stat_mask, + dsi_ctrl->irq_info.irq_stat_refcount[intr_idx]); + spin_lock_irqsave(&dsi_ctrl->irq_info.irq_lock, flags); if (dsi_ctrl->irq_info.irq_stat_refcount[intr_idx]) diff --git a/msm/dsi/dsi_ctrl_hw_cmn.c b/msm/dsi/dsi_ctrl_hw_cmn.c index 3f0affef1305..59281c045075 100644 --- a/msm/dsi/dsi_ctrl_hw_cmn.c +++ b/msm/dsi/dsi_ctrl_hw_cmn.c @@ -760,6 +760,8 @@ void dsi_ctrl_hw_cmn_kickoff_command(struct dsi_ctrl_hw *ctrl, if (!(flags & DSI_CTRL_HW_CMD_WAIT_FOR_TRIGGER)) DSI_W32(ctrl, DSI_CMD_MODE_DMA_SW_TRIGGER, 0x1); + + SDE_EVT32(ctrl->index, cmd->length, flags); } /** diff --git a/msm/dsi/dsi_display.c b/msm/dsi/dsi_display.c index 260be3e257f2..9cf97c164e70 100644 --- a/msm/dsi/dsi_display.c +++ b/msm/dsi/dsi_display.c @@ -1025,6 +1025,7 @@ int dsi_display_cmd_transfer(struct drm_connector *connector, return -EINVAL; } + SDE_EVT32(dsi_display->tx_cmd_buf_ndx, cmd_buf_len); DSI_DEBUG("[DSI] Display command transfer\n"); if ((cmd_buf[1]) || (cmd_buf[3] & MIPI_DSI_MSG_LASTCOMMAND)) @@ -1149,6 +1150,8 @@ int dsi_display_cmd_receive(void *display, const char *cmd_buf, return -EINVAL; } + SDE_EVT32(cmd_buf_len, recv_buf_len); + rc = dsi_display_cmd_prepare(cmd_buf, cmd_buf_len, &cmd, cmd_payload, MAX_CMD_PAYLOAD_SIZE); if (rc) { From 4b823cfd80b1ddc32f2a3f65872002693b0a1279 Mon Sep 17 00:00:00 2001 From: osaisruj Date: Sun, 27 Jun 2021 21:56:42 +0530 Subject: [PATCH 5/6] disp: msm: dsi: change pr_warn to pr_debug Change pr_warn to pr_debug to avoid extra logging. Change-Id: I4baf22083005641c77c2ca5a48157d1238b091a2 Signed-off-by: osaisruj --- msm/dsi/dsi_pll_5nm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msm/dsi/dsi_pll_5nm.c b/msm/dsi/dsi_pll_5nm.c index 332290ea8b42..f1c78cec1b22 100644 --- a/msm/dsi/dsi_pll_5nm.c +++ b/msm/dsi/dsi_pll_5nm.c @@ -492,7 +492,7 @@ static void dsi_pll_config_slave(struct dsi_pll_resource *rsc) rsc->slave = NULL; if (!orsc) { - pr_warn("slave PLL unavilable, assuming standalone config\n"); + pr_debug("slave PLL unavailable, assuming standalone config\n"); return; } From 909d927f883c0356a92b049999c9fe96b16a12fb Mon Sep 17 00:00:00 2001 From: Krishna Manikandan Date: Wed, 7 Jul 2021 09:59:13 +0530 Subject: [PATCH 6/6] disp: msm: sde: add dim layer check during secure transition If no planes are staged during non-secure to secure transition, dim layer should be staged at the highest blend stage. Add check to fail those commits where dim layer is not staged in such scenarios. Change-Id: Ifd49b00a8ff64547ea12751f6c765fd12e55629e Signed-off-by: Krishna Manikandan --- msm/sde/sde_crtc.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/msm/sde/sde_crtc.c b/msm/sde/sde_crtc.c index 089e182e0238..061d61c2b4e0 100644 --- a/msm/sde/sde_crtc.c +++ b/msm/sde/sde_crtc.c @@ -4413,6 +4413,9 @@ static int _sde_crtc_check_secure_blend_config(struct drm_crtc *crtc, { struct drm_plane *plane; int i; + struct drm_crtc_state *old_state = crtc->state; + struct sde_crtc_state *old_cstate = to_sde_crtc_state(old_state); + if (secure == SDE_DRM_SEC_ONLY) { /* * validate planes - only fb_sec_dir is allowed during sec_crtc @@ -4473,6 +4476,8 @@ static int _sde_crtc_check_secure_blend_config(struct drm_crtc *crtc, * - fail empty commit * - validate dim_layer or plane is staged in the supported * blendstage + * - fail if previous commit has no planes staged and + * no dim layer at highest blendstage. */ if (sde_kms->catalog->sui_supported_blendstage) { int sec_stage = cnt ? pstates[0].sde_pstate->stage : @@ -4490,6 +4495,14 @@ static int _sde_crtc_check_secure_blend_config(struct drm_crtc *crtc, cstate->num_dim_layers, sec_stage); return -EINVAL; } + + if (!old_state->plane_mask && + !old_cstate->num_dim_layers) { + SDE_ERROR( + "crtc%d: no dim layer in nonsecure to secure transition\n", + DRMID(crtc)); + return -EINVAL; + } } }