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 <pingli@codeaurora.org> Signed-off-by: UtsavBalar1231 <utsavbalar1231@gmail.com>
This commit is contained in:
parent
b87a34af2c
commit
25838aad41
@ -1314,7 +1314,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);
|
||||
@ -1344,12 +1344,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)
|
||||
@ -1357,7 +1358,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(struct sde_cp_node *prop_node,
|
||||
@ -2091,6 +2093,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->active_list);
|
||||
@ -2186,6 +2189,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);
|
||||
}
|
||||
@ -2976,45 +2980,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)
|
||||
@ -3024,11 +3003,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;
|
||||
}
|
||||
|
||||
@ -3038,15 +3019,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);
|
||||
|
@ -288,6 +288,7 @@ struct sde_crtc_misr_info {
|
||||
* @needs_hw_reset : Initiate a hw ctl reset
|
||||
* @comp_ratio : Compression ratio
|
||||
* @dspp_blob_info : blob containing dspp hw capability information
|
||||
* @hist_irq_idx : hist interrupt irq idx
|
||||
*/
|
||||
struct sde_crtc {
|
||||
struct drm_crtc base;
|
||||
@ -374,6 +375,7 @@ struct sde_crtc {
|
||||
struct mutex ltm_buffer_lock;
|
||||
spinlock_t ltm_lock;
|
||||
bool needs_hw_reset;
|
||||
int hist_irq_idx;
|
||||
|
||||
int comp_ratio;
|
||||
|
||||
|
@ -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 <drm/msm_drm_pp.h>
|
||||
@ -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);
|
||||
return;
|
||||
}
|
||||
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)
|
||||
|
Loading…
Reference in New Issue
Block a user