From b02eea56afd5186c8d53fa8743ebf477f1f6c2d4 Mon Sep 17 00:00:00 2001 From: Ritesh Kumar Date: Wed, 3 Mar 2021 18:57:11 +0530 Subject: [PATCH 1/2] disp: msm: dsi: update CPHY command mode clock calculation In CPHY, packet header and checksum is sent twice and SYNC is sent in between two headers. So, increase packet overhead used in clock calculation to 15 bytes. Packet Header: 8 bytes, CRC: 4 bytes, SYNC: 2 bytes and dcs command: 1 byte. Change-Id: I7a1160cbb57ba4f1faeb4b36a16c322e6069d58f Signed-off-by: Ritesh Kumar --- msm/dsi/dsi_panel.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/msm/dsi/dsi_panel.c b/msm/dsi/dsi_panel.c index 5d16e76ca7..6c3144966b 100644 --- a/msm/dsi/dsi_panel.c +++ b/msm/dsi/dsi_panel.c @@ -3875,13 +3875,18 @@ void dsi_panel_calc_dsi_transfer_time(struct dsi_host_common_cfg *config, struct dsi_mode_info *timing = &mode->timing; struct dsi_display_mode *display_mode; u32 jitter_numer, jitter_denom, prefill_lines; - u32 min_threshold_us, prefill_time_us, max_transfer_us; + u32 min_threshold_us, prefill_time_us, max_transfer_us, packet_overhead; u16 bpp; - /* Packet overlead in bits,2 bytes header + 2 bytes checksum - * + 1 byte dcs data command. + /* Packet overhead in bits, + * DPHY: 4 bytes header + 2 bytes checksum + 1 byte dcs data command. + * CPHY: 8 bytes header + 4 bytes checksum + 2 bytes SYNC + + * 1 byte dcs data command. */ - const u32 packet_overhead = 56; + if (config->phy_type & DSI_PHY_TYPE_CPHY) + packet_overhead = 120; + else + packet_overhead = 56; display_mode = container_of(timing, struct dsi_display_mode, timing); From 7ed51b95a204e01da742af6b0cb9df94445accb8 Mon Sep 17 00:00:00 2001 From: Ping Li Date: Mon, 23 Nov 2020 16:10:34 -0800 Subject: [PATCH 2/2] disp: msm: sde: fix potential race condition Move the hist irq handling out of callback function, i.e., the hw interrupt irq_lock context, to avoid dead lock between crtc spin_lock and irq_lock. This change also extends crtc spin_lock coverage in _sde_cp_crtc_enable_hist_irq to prevent null pointer dereference on event node, which can be deleted during crtc event de-registration. Change-Id: Iadaed54ab93c4c4abe065a8762d2addccb0c65c6 Signed-off-by: Ping Li --- msm/sde/sde_color_processing.c | 118 ++++++++++++++++--------- msm/sde/sde_crtc.h | 2 + msm/sde/sde_hw_color_processing_v1_7.c | 12 +-- 3 files changed, 85 insertions(+), 47 deletions(-) diff --git a/msm/sde/sde_color_processing.c b/msm/sde/sde_color_processing.c index 3ee567ca77..2588c40262 100644 --- a/msm/sde/sde_color_processing.c +++ b/msm/sde/sde_color_processing.c @@ -1503,7 +1503,7 @@ static void _sde_cp_crtc_enable_hist_irq(struct sde_crtc *sde_crtc) struct sde_hw_dspp *hw_dspp = NULL; struct sde_crtc_irq_info *node = NULL; int i, irq_idx, ret = 0; - unsigned long flags; + unsigned long flags, state_flags; if (!crtc_drm) { DRM_ERROR("invalid crtc %pK\n", crtc_drm); @@ -1533,12 +1533,13 @@ static void _sde_cp_crtc_enable_hist_irq(struct sde_crtc *sde_crtc) spin_lock_irqsave(&sde_crtc->spin_lock, flags); node = _sde_cp_get_intr_node(DRM_EVENT_HISTOGRAM, sde_crtc); - spin_unlock_irqrestore(&sde_crtc->spin_lock, flags); - if (!node) + if (!node) { + spin_unlock_irqrestore(&sde_crtc->spin_lock, flags); return; + } - spin_lock_irqsave(&node->state_lock, flags); + spin_lock_irqsave(&node->state_lock, state_flags); if (node->state == IRQ_DISABLED) { ret = sde_core_irq_enable(kms, &irq_idx, 1); if (ret) @@ -1546,7 +1547,8 @@ static void _sde_cp_crtc_enable_hist_irq(struct sde_crtc *sde_crtc) else node->state = IRQ_ENABLED; } - spin_unlock_irqrestore(&node->state_lock, flags); + spin_unlock_irqrestore(&node->state_lock, state_flags); + spin_unlock_irqrestore(&sde_crtc->spin_lock, flags); } static int _sde_cp_crtc_checkfeature(u32 feature, @@ -2543,6 +2545,7 @@ void sde_cp_crtc_destroy_properties(struct drm_crtc *crtc) } sde_crtc->ltm_buffer_cnt = 0; sde_crtc->ltm_hist_en = false; + sde_crtc->hist_irq_idx = -1; mutex_destroy(&sde_crtc->crtc_cp_lock); INIT_LIST_HEAD(&sde_crtc->cp_active_list); @@ -2685,6 +2688,7 @@ void sde_cp_crtc_clear(struct drm_crtc *crtc) } sde_crtc->ltm_buffer_cnt = 0; sde_crtc->ltm_hist_en = false; + sde_crtc->hist_irq_idx = -1; INIT_LIST_HEAD(&sde_crtc->ltm_buf_free); INIT_LIST_HEAD(&sde_crtc->ltm_buf_busy); } @@ -3519,45 +3523,20 @@ static void _sde_cp_hist_interrupt_cb(void *arg, int irq_idx) struct sde_crtc *crtc = arg; struct drm_crtc *crtc_drm = &crtc->base; struct sde_hw_dspp *hw_dspp; - struct sde_kms *kms; - struct sde_crtc_irq_info *node = NULL; + u32 lock_hist = 1; u32 i; - int ret = 0; - unsigned long flags; - - /* disable histogram irq */ - kms = get_kms(crtc_drm); - spin_lock_irqsave(&crtc->spin_lock, flags); - node = _sde_cp_get_intr_node(DRM_EVENT_HISTOGRAM, crtc); - spin_unlock_irqrestore(&crtc->spin_lock, flags); - - if (!node) { - DRM_DEBUG_DRIVER("cannot find histogram event node in crtc\n"); - return; - } - - spin_lock_irqsave(&node->state_lock, flags); - if (node->state == IRQ_ENABLED) { - if (sde_core_irq_disable_nolock(kms, irq_idx)) { - DRM_ERROR("failed to disable irq %d, ret %d\n", - irq_idx, ret); - spin_unlock_irqrestore(&node->state_lock, flags); - return; - } - node->state = IRQ_DISABLED; - } - spin_unlock_irqrestore(&node->state_lock, flags); /* lock histogram buffer */ for (i = 0; i < crtc->num_mixers; i++) { hw_dspp = crtc->mixers[i].hw_dspp; if (hw_dspp && hw_dspp->ops.lock_histogram) - hw_dspp->ops.lock_histogram(hw_dspp, NULL); + hw_dspp->ops.lock_histogram(hw_dspp, &lock_hist); } + crtc->hist_irq_idx = irq_idx; /* notify histogram event */ sde_crtc_event_queue(crtc_drm, _sde_cp_notify_hist_event, - NULL, true); + &crtc->hist_irq_idx, true); } static void _sde_cp_notify_hist_event(struct drm_crtc *crtc_drm, void *arg) @@ -3567,11 +3546,13 @@ static void _sde_cp_notify_hist_event(struct drm_crtc *crtc_drm, void *arg) struct drm_event event; struct drm_msm_hist *hist_data; struct sde_kms *kms; - int ret; - u32 i; + struct sde_crtc_irq_info *node = NULL; + unsigned long flags, state_flags; + int ret, irq_idx; + u32 i, lock_hist = 0; - if (!crtc_drm) { - DRM_ERROR("invalid crtc %pK\n", crtc_drm); + if (!crtc_drm || !arg) { + DRM_ERROR("invalid drm crtc %pK or arg %pK\n", crtc_drm, arg); return; } @@ -3581,15 +3562,70 @@ static void _sde_cp_notify_hist_event(struct drm_crtc *crtc_drm, void *arg) return; } - if (!crtc->hist_blob) - return; - kms = get_kms(crtc_drm); if (!kms || !kms->dev) { SDE_ERROR("invalid arg(s)\n"); return; } + /* disable histogram irq */ + spin_lock_irqsave(&crtc->spin_lock, flags); + node = _sde_cp_get_intr_node(DRM_EVENT_HISTOGRAM, crtc); + + if (!node) { + spin_unlock_irqrestore(&crtc->spin_lock, flags); + DRM_DEBUG_DRIVER("cannot find histogram event node in crtc\n"); + /* unlock histogram */ + ret = pm_runtime_get_sync(kms->dev->dev); + if (ret < 0) { + SDE_ERROR("failed to enable power resource %d\n", ret); + SDE_EVT32(ret, SDE_EVTLOG_ERROR); + return; + } + for (i = 0; i < crtc->num_mixers; i++) { + hw_dspp = crtc->mixers[i].hw_dspp; + if (hw_dspp && hw_dspp->ops.lock_histogram) + hw_dspp->ops.lock_histogram(hw_dspp, + &lock_hist); + } + pm_runtime_put_sync(kms->dev->dev); + return; + } + + irq_idx = *(int *)arg; + spin_lock_irqsave(&node->state_lock, state_flags); + if (node->state == IRQ_ENABLED) { + ret = sde_core_irq_disable_nolock(kms, irq_idx); + if (ret) { + DRM_ERROR("failed to disable irq %d, ret %d\n", + irq_idx, ret); + spin_unlock_irqrestore(&node->state_lock, state_flags); + spin_unlock_irqrestore(&crtc->spin_lock, flags); + ret = pm_runtime_get_sync(kms->dev->dev); + if (ret < 0) { + SDE_ERROR("failed to enable power %d\n", ret); + SDE_EVT32(ret, SDE_EVTLOG_ERROR); + return; + } + + /* unlock histogram */ + for (i = 0; i < crtc->num_mixers; i++) { + hw_dspp = crtc->mixers[i].hw_dspp; + if (hw_dspp && hw_dspp->ops.lock_histogram) + hw_dspp->ops.lock_histogram(hw_dspp, + &lock_hist); + } + pm_runtime_put_sync(kms->dev->dev); + return; + } + node->state = IRQ_DISABLED; + } + spin_unlock_irqrestore(&node->state_lock, state_flags); + spin_unlock_irqrestore(&crtc->spin_lock, flags); + + if (!crtc->hist_blob) + return; + ret = pm_runtime_get_sync(kms->dev->dev); if (ret < 0) { SDE_ERROR("failed to enable power resource %d\n", ret); diff --git a/msm/sde/sde_crtc.h b/msm/sde/sde_crtc.h index 4ea9337b66..c8326a775b 100644 --- a/msm/sde/sde_crtc.h +++ b/msm/sde/sde_crtc.h @@ -290,6 +290,7 @@ struct sde_crtc_misr_info { * @ltm_buffer_lock : muttx to protect ltm_buffers allcation and free * @ltm_lock : Spinlock to protect ltm buffer_cnt, hist_en and ltm lists * @needs_hw_reset : Initiate a hw ctl reset + * @hist_irq_idx : hist interrupt irq idx * @src_bpp : source bpp used to calculate compression ratio * @target_bpp : target bpp used to calculate compression ratio * @static_cache_read_work: delayed worker to transition cache state to read @@ -383,6 +384,7 @@ struct sde_crtc { struct mutex ltm_buffer_lock; spinlock_t ltm_lock; bool needs_hw_reset; + int hist_irq_idx; int src_bpp; int target_bpp; diff --git a/msm/sde/sde_hw_color_processing_v1_7.c b/msm/sde/sde_hw_color_processing_v1_7.c index e7920320fa..b39250b9c6 100644 --- a/msm/sde/sde_hw_color_processing_v1_7.c +++ b/msm/sde/sde_hw_color_processing_v1_7.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved. + * Copyright (c) 2016-2019, 2021, The Linux Foundation. All rights reserved. */ #include @@ -951,17 +951,17 @@ void sde_read_dspp_hist_v1_7(struct sde_hw_dspp *ctx, void *cfg) void sde_lock_dspp_hist_v1_7(struct sde_hw_dspp *ctx, void *cfg) { - u32 offset_ctl; + u32 offset_ctl, val; - if (!ctx) { - DRM_ERROR("invalid parameters ctx %pK", ctx); + if (!ctx || !cfg) { + DRM_ERROR("invalid parameters ctx %pK cfg %pK", ctx, cfg); return; } offset_ctl = ctx->cap->sblk->hist.base + PA_HIST_CTRL_DSPP_OFF; - /* lock hist buffer */ - SDE_REG_WRITE(&ctx->hw, offset_ctl, 1); + val = (*(u32 *)cfg) & 0x1; + SDE_REG_WRITE(&ctx->hw, offset_ctl, val); } void sde_setup_dspp_dither_v1_7(struct sde_hw_dspp *ctx, void *cfg)