From ac94b019c305e3111e762a83fb37b4f625b1039b Mon Sep 17 00:00:00 2001 From: Lakshmi Narayana Kalavala Date: Tue, 20 Feb 2018 17:55:26 -0800 Subject: [PATCH 1/4] drm/msm/sde: add debugfs nodes for underruns debug This change adds debugfs nodes for debugging underruns, after this change following commands are supported: 1. To enable the underrun ftrace: echo 1 > /d/tracing/events/sde/sde_encoder_underrun/enable 2. To enable feature that disables the ftraces once the underrun happens: echo 1 > /d/dri/0/debug/dbg_ctrl 3. To enable the panic in the device when underrun happens: echo 2 > /d/dri/0/debug/dbg_ctrl 4. To enable both of the above options: echo 3 > /d/dri/0/debug/dbg_ctrl Change-Id: Id9f407edb0908a5f8454f08d63c356dc8f04d353 Signed-off-by: Ingrid Gallardo Signed-off-by: Jayant Shekhar Signed-off-by: Lakshmi Narayana Kalavala --- drivers/gpu/drm/msm/sde/sde_encoder.c | 8 +- drivers/gpu/drm/msm/sde/sde_trace.h | 18 +++- drivers/gpu/drm/msm/sde_dbg.c | 123 +++++++++++++++++++++++++- drivers/gpu/drm/msm/sde_dbg.h | 22 ++++- 4 files changed, 167 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/sde/sde_encoder.c b/drivers/gpu/drm/msm/sde/sde_encoder.c index 6e4de62c6957..fa17768d9939 100644 --- a/drivers/gpu/drm/msm/sde/sde_encoder.c +++ b/drivers/gpu/drm/msm/sde/sde_encoder.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014-2017, The Linux Foundation. All rights reserved. + * Copyright (c) 2014-2018, The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark * @@ -602,6 +602,12 @@ static void sde_encoder_underrun_callback(struct drm_encoder *drm_enc, SDE_ATRACE_BEGIN("encoder_underrun_callback"); atomic_inc(&phy_enc->underrun_cnt); SDE_EVT32(DRMID(drm_enc), atomic_read(&phy_enc->underrun_cnt)); + + trace_sde_encoder_underrun(DRMID(drm_enc), + atomic_read(&phy_enc->underrun_cnt)); + SDE_DBG_CTRL("stop_ftrace"); + SDE_DBG_CTRL("panic_underrun"); + SDE_ATRACE_END("encoder_underrun_callback"); } diff --git a/drivers/gpu/drm/msm/sde/sde_trace.h b/drivers/gpu/drm/msm/sde/sde_trace.h index 2a4e6b59a08c..d28562eabccb 100644 --- a/drivers/gpu/drm/msm/sde/sde_trace.h +++ b/drivers/gpu/drm/msm/sde/sde_trace.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2014-2016, The Linux Foundation. All rights reserved. +/* Copyright (c) 2014-2018, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -125,6 +125,22 @@ TRACE_EVENT(sde_cmd_release_bw, TP_printk("crtc:%d", __entry->crtc_id) ); +TRACE_EVENT(sde_encoder_underrun, + TP_PROTO(u32 enc_id, u32 underrun_cnt), + TP_ARGS(enc_id, underrun_cnt), + TP_STRUCT__entry( + __field(u32, enc_id) + __field(u32, underrun_cnt) + ), + TP_fast_assign( + __entry->enc_id = enc_id; + __entry->underrun_cnt = underrun_cnt; + + ), + TP_printk("enc:%d underrun_cnt:%d", __entry->enc_id, + __entry->underrun_cnt) +); + TRACE_EVENT(sde_mark_write, TP_PROTO(int pid, const char *name, bool trace_begin), TP_ARGS(pid, name, trace_begin), diff --git a/drivers/gpu/drm/msm/sde_dbg.c b/drivers/gpu/drm/msm/sde_dbg.c index ea133619cf62..0984ce86b969 100644 --- a/drivers/gpu/drm/msm/sde_dbg.c +++ b/drivers/gpu/drm/msm/sde_dbg.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2009-2017, The Linux Foundation. All rights reserved. +/* Copyright (c) 2009-2018, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -57,6 +57,9 @@ /* print debug ranges in groups of 4 u32s */ #define REG_DUMP_ALIGN 16 +#define DBG_CTRL_STOP_FTRACE BIT(0) +#define DBG_CTRL_PANIC_UNDERRUN BIT(1) +#define DBG_CTRL_MAX BIT(2) /** * struct sde_dbg_reg_offset - tracking for start and end of region @@ -180,6 +183,7 @@ static struct sde_dbg_base { struct sde_dbg_sde_debug_bus dbgbus_sde; struct sde_dbg_vbif_debug_bus dbgbus_vbif_rt; + u32 debugfs_ctrl; } sde_dbg_base; /* sde_dbg_base_evtlog - global pointer to main sde event log for macro use */ @@ -1557,6 +1561,45 @@ void sde_dbg_dump(bool queue_work, const char *name, ...) } } +void sde_dbg_ctrl(const char *name, ...) +{ + int i = 0; + va_list args; + char *blk_name = NULL; + + + /* no debugfs controlled events are enabled, just return */ + if (!sde_dbg_base.debugfs_ctrl) + return; + + va_start(args, name); + + while ((blk_name = va_arg(args, char*))) { + if (i++ >= SDE_EVTLOG_MAX_DATA) { + pr_err("could not parse all dbg arguments\n"); + break; + } + + if (IS_ERR_OR_NULL(blk_name)) + break; + + if (!strcmp(blk_name, "stop_ftrace") && + sde_dbg_base.debugfs_ctrl & + DBG_CTRL_STOP_FTRACE) { + pr_debug("tracing off\n"); + tracing_off(); + } + + if (!strcmp(blk_name, "panic_underrun") && + sde_dbg_base.debugfs_ctrl & + DBG_CTRL_PANIC_UNDERRUN) { + pr_debug("panic underrun\n"); + panic("underrun"); + } + } + +} + /* * sde_dbg_debugfs_open - debugfs open handler for evtlog dump * @inode: debugfs inode @@ -1621,6 +1664,82 @@ static const struct file_operations sde_evtlog_fops = { .write = sde_evtlog_dump_write, }; +/** + * sde_dbg_ctrl_read - debugfs read handler for debug ctrl read + * @file: file handler + * @buff: user buffer content for debugfs + * @count: size of user buffer + * @ppos: position offset of user buffer + */ +static ssize_t sde_dbg_ctrl_read(struct file *file, char __user *buff, + size_t count, loff_t *ppos) +{ + ssize_t len = 0; + char buf[24] = {'\0'}; + + if (!buff || !ppos) + return -EINVAL; + + if (*ppos) + return 0; /* the end */ + + len = snprintf(buf, sizeof(buf), "0x%x\n", sde_dbg_base.debugfs_ctrl); + pr_debug("%s: ctrl:0x%x len:0x%zx\n", + __func__, sde_dbg_base.debugfs_ctrl, len); + + if ((count < sizeof(buf)) || copy_to_user(buff, buf, len)) { + pr_err("error copying the buffer! count:0x%zx\n", count); + return -EFAULT; + } + + *ppos += len; /* increase offset */ + return len; +} + +/** + * sde_dbg_ctrl_write - debugfs read handler for debug ctrl write + * @file: file handler + * @user_buf: user buffer content from debugfs + * @count: size of user buffer + * @ppos: position offset of user buffer + */ +static ssize_t sde_dbg_ctrl_write(struct file *file, + const char __user *user_buf, size_t count, loff_t *ppos) +{ + u32 dbg_ctrl = 0; + char buf[24]; + + if (!file) { + pr_err("DbgDbg: %s: error no file --\n", __func__); + return -EINVAL; + } + + if (count >= sizeof(buf)) + return -EFAULT; + + + if (copy_from_user(buf, user_buf, count)) + return -EFAULT; + + buf[count] = 0; /* end of string */ + + if (kstrtouint(buf, 0, &dbg_ctrl)) { + pr_err("%s: error in the number of bytes\n", __func__); + return -EFAULT; + } + + pr_debug("dbg_ctrl_read:0x%x\n", dbg_ctrl); + sde_dbg_base.debugfs_ctrl = dbg_ctrl; + + return count; +} + +static const struct file_operations sde_dbg_ctrl_fops = { + .open = sde_dbg_debugfs_open, + .read = sde_dbg_ctrl_read, + .write = sde_dbg_ctrl_write, +}; + void sde_dbg_init_dbg_buses(u32 hwversion) { static struct sde_dbg_base *dbg = &sde_dbg_base; @@ -1695,6 +1814,8 @@ int sde_dbg_init(struct dentry *debugfs_root, struct device *dev, for (i = 0; i < SDE_EVTLOG_ENTRY; i++) sde_dbg_base.evtlog->logs[i].counter = i; + debugfs_create_file("dbg_ctrl", 0600, sde_dbg_base.root, NULL, + &sde_dbg_ctrl_fops); debugfs_create_file("dump", 0600, sde_dbg_base.root, NULL, &sde_evtlog_fops); debugfs_create_u32("enable", 0600, sde_dbg_base.root, diff --git a/drivers/gpu/drm/msm/sde_dbg.h b/drivers/gpu/drm/msm/sde_dbg.h index 74fd4c94b490..95528eff6afa 100644 --- a/drivers/gpu/drm/msm/sde_dbg.h +++ b/drivers/gpu/drm/msm/sde_dbg.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2016-2017, The Linux Foundation. All rights reserved. +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -123,6 +123,13 @@ extern struct sde_dbg_evtlog *sde_dbg_base_evtlog; #define SDE_DBG_DUMP_WQ(...) sde_dbg_dump(true, __func__, ##__VA_ARGS__, \ SDE_DBG_DUMP_DATA_LIMITER) +/** + * SDE_DBG_EVT_CTRL - trigger a different driver events + * event: event that trigger different behavior in the driver + */ +#define SDE_DBG_CTRL(...) sde_dbg_ctrl(__func__, ##__VA_ARGS__, \ + SDE_DBG_DUMP_DATA_LIMITER) + #if defined(CONFIG_DEBUG_FS) /** @@ -213,6 +220,15 @@ void sde_dbg_destroy(void); */ void sde_dbg_dump(bool queue_work, const char *name, ...); +/** + * sde_dbg_ctrl - trigger specific actions for the driver with debugging + * purposes. Those actions need to be enabled by the debugfs entry + * so the driver executes those actions in the corresponding calls. + * @va_args: list of actions to trigger + * Returns: none + */ +void sde_dbg_ctrl(const char *name, ...); + /** * sde_dbg_reg_register_base - register a hw register address section for later * dumping. call this before calling sde_dbg_reg_register_dump_range @@ -295,6 +311,10 @@ static inline void sde_dbg_dump(bool queue_work, const char *name, ...) { } +static inline void sde_dbg_ctrl(const char *name, ...) +{ +} + static inline int sde_dbg_reg_register_base(const char *name, void __iomem *base, size_t max_offset) { From 89ddcc12a182913177718228e0dc9eef2161998f Mon Sep 17 00:00:00 2001 From: Lakshmi Narayana Kalavala Date: Thu, 22 Feb 2018 11:42:48 -0800 Subject: [PATCH 2/4] drm/msm: add input sanitization on debug dump debugfs Add checks to debugfs input parameters in sde debug dump debugfs entries. Change-Id: Iea170b75c1eb9aa46366662d36e677cb3251830b Signed-off-by: Lloyd Atkinson Signed-off-by: Lakshmi Narayana Kalavala --- drivers/gpu/drm/msm/sde_dbg.c | 53 +++++++++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/sde_dbg.c b/drivers/gpu/drm/msm/sde_dbg.c index 0984ce86b969..66c9844fdf26 100644 --- a/drivers/gpu/drm/msm/sde_dbg.c +++ b/drivers/gpu/drm/msm/sde_dbg.c @@ -1607,6 +1607,9 @@ void sde_dbg_ctrl(const char *name, ...) */ static int sde_dbg_debugfs_open(struct inode *inode, struct file *file) { + if (!inode || !file) + return -EINVAL; + /* non-seekable */ file->f_mode &= ~(FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE); file->private_data = inode->i_private; @@ -1626,6 +1629,9 @@ static ssize_t sde_evtlog_dump_read(struct file *file, char __user *buff, ssize_t len = 0; char evtlog_buf[SDE_EVTLOG_BUF_MAX]; + if (!buff || !ppos) + return -EINVAL; + len = sde_evtlog_dump_to_buffer(sde_dbg_base.evtlog, evtlog_buf, SDE_EVTLOG_BUF_MAX); if (copy_to_user(buff, evtlog_buf, len)) @@ -1857,7 +1863,14 @@ void sde_dbg_destroy(void) */ static int sde_dbg_reg_base_release(struct inode *inode, struct file *file) { - struct sde_dbg_reg_base *dbg = file->private_data; + struct sde_dbg_reg_base *dbg; + + if (!file) + return -EINVAL; + + dbg = file->private_data; + if (!dbg) + return -ENODEV; mutex_lock(&sde_dbg_base.mutex); if (dbg && dbg->buf) { @@ -1881,12 +1894,16 @@ static int sde_dbg_reg_base_release(struct inode *inode, struct file *file) static ssize_t sde_dbg_reg_base_offset_write(struct file *file, const char __user *user_buf, size_t count, loff_t *ppos) { - struct sde_dbg_reg_base *dbg = file->private_data; + struct sde_dbg_reg_base *dbg; u32 off = 0; u32 cnt = DEFAULT_BASE_REG_CNT; char buf[24]; ssize_t rc = count; + if (!file) + return -EINVAL; + + dbg = file->private_data; if (!dbg) return -ENODEV; @@ -1920,6 +1937,9 @@ static ssize_t sde_dbg_reg_base_offset_write(struct file *file, goto exit; } + if (cnt == 0) + return -EINVAL; + dbg->off = off; dbg->cnt = cnt; @@ -1940,17 +1960,29 @@ exit: static ssize_t sde_dbg_reg_base_offset_read(struct file *file, char __user *buff, size_t count, loff_t *ppos) { - struct sde_dbg_reg_base *dbg = file->private_data; + struct sde_dbg_reg_base *dbg; int len = 0; char buf[24] = {'\0'}; + if (!file) + return -EINVAL; + + dbg = file->private_data; if (!dbg) return -ENODEV; + if (!ppos) + return -EINVAL; + if (*ppos) return 0; /* the end */ mutex_lock(&sde_dbg_base.mutex); + if (dbg->off % sizeof(u32)) { + mutex_unlock(&sde_dbg_base.mutex); + return -EFAULT; + } + len = snprintf(buf, sizeof(buf), "0x%08zx %zx\n", dbg->off, dbg->cnt); if (len < 0 || len >= sizeof(buf)) { mutex_unlock(&sde_dbg_base.mutex); @@ -1978,11 +2010,15 @@ static ssize_t sde_dbg_reg_base_offset_read(struct file *file, static ssize_t sde_dbg_reg_base_reg_write(struct file *file, const char __user *user_buf, size_t count, loff_t *ppos) { - struct sde_dbg_reg_base *dbg = file->private_data; + struct sde_dbg_reg_base *dbg; size_t off; u32 data, cnt; char buf[24]; + if (!file) + return -EINVAL; + + dbg = file->private_data; if (!dbg) return -ENODEV; @@ -2028,14 +2064,21 @@ static ssize_t sde_dbg_reg_base_reg_write(struct file *file, static ssize_t sde_dbg_reg_base_reg_read(struct file *file, char __user *user_buf, size_t count, loff_t *ppos) { - struct sde_dbg_reg_base *dbg = file->private_data; + struct sde_dbg_reg_base *dbg; size_t len; + if (!file) + return -EINVAL; + + dbg = file->private_data; if (!dbg) { pr_err("invalid handle\n"); return -ENODEV; } + if (!ppos) + return -EINVAL; + mutex_lock(&sde_dbg_base.mutex); if (!dbg->buf) { char *hwbuf; From 1f9861ad386a36c435cf664c5a99e0e15de54c00 Mon Sep 17 00:00:00 2001 From: Lakshmi Narayana Kalavala Date: Mon, 26 Feb 2018 15:06:22 -0800 Subject: [PATCH 3/4] drm/msm: limit sde_dbg_dump output to current entries Update the sde_dbg_dump function to only dump event log entries up to the time the dump is initiated, and increase the number of recorded event log entries. This places a limit on the total number of entries that are output to the dmesg log even if more entries would have been added while the dump is executing (e.g., interrupt events), while still allowing the extra events to be recorded properly. Change-Id: I66f850d21a2d0217f9049facffce074831b7e17d Signed-off-by: Clarence Ip Signed-off-by: Lakshmi Narayana Kalavala --- drivers/gpu/drm/msm/sde_dbg.c | 19 ++++++++++---- drivers/gpu/drm/msm/sde_dbg.h | 13 +++++++--- drivers/gpu/drm/msm/sde_dbg_evtlog.c | 37 ++++++++++++++++++---------- 3 files changed, 47 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/msm/sde_dbg.c b/drivers/gpu/drm/msm/sde_dbg.c index 66c9844fdf26..c886950e5212 100644 --- a/drivers/gpu/drm/msm/sde_dbg.c +++ b/drivers/gpu/drm/msm/sde_dbg.c @@ -165,6 +165,7 @@ struct sde_dbg_vbif_debug_bus { * @enable_reg_dump: whether to dump registers into memory, kernel log, or both * @dbgbus_sde: debug bus structure for the sde * @dbgbus_vbif_rt: debug bus structure for the realtime vbif + * @dump_all: dump all entries in register dump */ static struct sde_dbg_base { struct sde_dbg_evtlog *evtlog; @@ -183,6 +184,7 @@ static struct sde_dbg_base { struct sde_dbg_sde_debug_bus dbgbus_sde; struct sde_dbg_vbif_debug_bus dbgbus_vbif_rt; + bool dump_all; u32 debugfs_ctrl; } sde_dbg_base; @@ -1452,7 +1454,7 @@ static void _sde_dbg_dump_vbif_dbg_bus(struct sde_dbg_vbif_debug_bus *bus) */ static void _sde_dump_array(struct sde_dbg_reg_base *blk_arr[], u32 len, bool do_panic, const char *name, bool dump_dbgbus_sde, - bool dump_dbgbus_vbif_rt) + bool dump_dbgbus_vbif_rt, bool dump_all) { int i; @@ -1464,7 +1466,8 @@ static void _sde_dump_array(struct sde_dbg_reg_base *blk_arr[], sde_dbg_base.enable_reg_dump); } - sde_evtlog_dump_all(sde_dbg_base.evtlog); + if (dump_all) + sde_evtlog_dump_all(sde_dbg_base.evtlog); if (dump_dbgbus_sde) _sde_dbg_dump_sde_dbg_bus(&sde_dbg_base.dbgbus_sde); @@ -1488,7 +1491,8 @@ static void _sde_dump_work(struct work_struct *work) ARRAY_SIZE(sde_dbg_base.req_dump_blks), sde_dbg_base.work_panic, "evtlog_workitem", sde_dbg_base.dbgbus_sde.cmn.include_in_deferred_work, - sde_dbg_base.dbgbus_vbif_rt.cmn.include_in_deferred_work); + sde_dbg_base.dbgbus_vbif_rt.cmn.include_in_deferred_work, + sde_dbg_base.dump_all); } void sde_dbg_dump(bool queue_work, const char *name, ...) @@ -1497,6 +1501,7 @@ void sde_dbg_dump(bool queue_work, const char *name, ...) bool do_panic = false; bool dump_dbgbus_sde = false; bool dump_dbgbus_vbif_rt = false; + bool dump_all = false; va_list args; char *blk_name = NULL; struct sde_dbg_reg_base *blk_base = NULL; @@ -1514,6 +1519,7 @@ void sde_dbg_dump(bool queue_work, const char *name, ...) memset(sde_dbg_base.req_dump_blks, 0, sizeof(sde_dbg_base.req_dump_blks)); + sde_dbg_base.dump_all = false; va_start(args, name); i = 0; @@ -1535,6 +1541,8 @@ void sde_dbg_dump(bool queue_work, const char *name, ...) blk_name); } } + if (!strcmp(blk_name, "all")) + dump_all = true; if (!strcmp(blk_name, "dbg_bus")) dump_dbgbus_sde = true; @@ -1554,10 +1562,11 @@ void sde_dbg_dump(bool queue_work, const char *name, ...) dump_dbgbus_sde; sde_dbg_base.dbgbus_vbif_rt.cmn.include_in_deferred_work = dump_dbgbus_vbif_rt; + sde_dbg_base.dump_all = dump_all; schedule_work(&sde_dbg_base.dump_work); } else { _sde_dump_array(blk_arr, blk_len, do_panic, name, - dump_dbgbus_sde, dump_dbgbus_vbif_rt); + dump_dbgbus_sde, dump_dbgbus_vbif_rt, dump_all); } } @@ -1633,7 +1642,7 @@ static ssize_t sde_evtlog_dump_read(struct file *file, char __user *buff, return -EINVAL; len = sde_evtlog_dump_to_buffer(sde_dbg_base.evtlog, evtlog_buf, - SDE_EVTLOG_BUF_MAX); + SDE_EVTLOG_BUF_MAX, true); if (copy_to_user(buff, evtlog_buf, len)) return -EFAULT; *ppos += len; diff --git a/drivers/gpu/drm/msm/sde_dbg.h b/drivers/gpu/drm/msm/sde_dbg.h index 95528eff6afa..ce36cba08039 100644 --- a/drivers/gpu/drm/msm/sde_dbg.h +++ b/drivers/gpu/drm/msm/sde_dbg.h @@ -17,9 +17,10 @@ #include #include -#define SDE_EVTLOG_DATA_LIMITER (-1) +#define SDE_EVTLOG_DATA_LIMITER (0xC0DEBEEF) #define SDE_EVTLOG_FUNC_ENTRY 0x1111 #define SDE_EVTLOG_FUNC_EXIT 0x2222 +#define SDE_EVTLOG_ERROR 0xebad #define SDE_DBG_DUMP_DATA_LIMITER (NULL) @@ -52,7 +53,7 @@ enum sde_dbg_dump_flag { * number must be greater than print entry to prevent out of bound evtlog * entry array access. */ -#define SDE_EVTLOG_ENTRY (SDE_EVTLOG_PRINT_ENTRY * 4) +#define SDE_EVTLOG_ENTRY (SDE_EVTLOG_PRINT_ENTRY * 8) #define SDE_EVTLOG_MAX_DATA 15 #define SDE_EVTLOG_BUF_MAX 512 #define SDE_EVTLOG_BUF_ALIGN 32 @@ -77,6 +78,7 @@ struct sde_dbg_evtlog { struct sde_dbg_evtlog_log logs[SDE_EVTLOG_ENTRY]; u32 first; u32 last; + u32 last_dump; u32 curr; u32 next; u32 enable; @@ -179,10 +181,12 @@ bool sde_evtlog_is_enabled(struct sde_dbg_evtlog *evtlog, u32 flag); * @evtlog: pointer to evtlog * @evtlog_buf: target buffer to print into * @evtlog_buf_size: size of target buffer + * @update_last_entry:ยป whether or not to stop at most recent entry * Returns: number of bytes written to buffer */ ssize_t sde_evtlog_dump_to_buffer(struct sde_dbg_evtlog *evtlog, - char *evtlog_buf, ssize_t evtlog_buf_size); + char *evtlog_buf, ssize_t evtlog_buf_size, + bool update_last_entry); /** * sde_dbg_init_dbg_buses - initialize debug bus dumping support for the chipset @@ -288,7 +292,8 @@ static inline bool sde_evtlog_is_enabled(struct sde_dbg_evtlog *evtlog, } static inline ssize_t sde_evtlog_dump_to_buffer(struct sde_dbg_evtlog *evtlog, - char *evtlog_buf, ssize_t evtlog_buf_size) + char *evtlog_buf, ssize_t evtlog_buf_size, + bool update_last_entry) { return 0; } diff --git a/drivers/gpu/drm/msm/sde_dbg_evtlog.c b/drivers/gpu/drm/msm/sde_dbg_evtlog.c index 759bdab48840..70ba127ceb08 100644 --- a/drivers/gpu/drm/msm/sde_dbg_evtlog.c +++ b/drivers/gpu/drm/msm/sde_dbg_evtlog.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2016-2017, The Linux Foundation. All rights reserved. +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -75,7 +75,8 @@ void sde_evtlog_log(struct sde_dbg_evtlog *evtlog, const char *name, int line, } /* always dump the last entries which are not dumped yet */ -static bool _sde_evtlog_dump_calc_range(struct sde_dbg_evtlog *evtlog) +static bool _sde_evtlog_dump_calc_range(struct sde_dbg_evtlog *evtlog, + bool update_last_entry) { bool need_dump = true; unsigned long flags; @@ -87,21 +88,26 @@ static bool _sde_evtlog_dump_calc_range(struct sde_dbg_evtlog *evtlog) evtlog->first = evtlog->next; - if (evtlog->last == evtlog->first) { + if (update_last_entry) + evtlog->last_dump = evtlog->last; + + if (evtlog->last_dump == evtlog->first) { need_dump = false; goto dump_exit; } - if (evtlog->last < evtlog->first) { + if (evtlog->last_dump < evtlog->first) { evtlog->first %= SDE_EVTLOG_ENTRY; - if (evtlog->last < evtlog->first) - evtlog->last += SDE_EVTLOG_ENTRY; + if (evtlog->last_dump < evtlog->first) + evtlog->last_dump += SDE_EVTLOG_ENTRY; } - if ((evtlog->last - evtlog->first) > SDE_EVTLOG_PRINT_ENTRY) { - pr_warn("evtlog buffer overflow before dump: %d\n", - evtlog->last - evtlog->first); - evtlog->first = evtlog->last - SDE_EVTLOG_PRINT_ENTRY; + if ((evtlog->last_dump - evtlog->first) > SDE_EVTLOG_PRINT_ENTRY) { + pr_info("evtlog skipping %d entries, last=%d\n", + evtlog->last_dump - evtlog->first - + SDE_EVTLOG_PRINT_ENTRY, + evtlog->last_dump - 1); + evtlog->first = evtlog->last_dump - SDE_EVTLOG_PRINT_ENTRY; } evtlog->next = evtlog->first + 1; @@ -112,7 +118,8 @@ dump_exit: } ssize_t sde_evtlog_dump_to_buffer(struct sde_dbg_evtlog *evtlog, - char *evtlog_buf, ssize_t evtlog_buf_size) + char *evtlog_buf, ssize_t evtlog_buf_size, + bool update_last_entry) { int i; ssize_t off = 0; @@ -123,7 +130,7 @@ ssize_t sde_evtlog_dump_to_buffer(struct sde_dbg_evtlog *evtlog, return 0; /* update markers, exit if nothing to print */ - if (!_sde_evtlog_dump_calc_range(evtlog)) + if (!_sde_evtlog_dump_calc_range(evtlog, update_last_entry)) return 0; spin_lock_irqsave(&evtlog->spin_lock, flags); @@ -159,12 +166,16 @@ ssize_t sde_evtlog_dump_to_buffer(struct sde_dbg_evtlog *evtlog, void sde_evtlog_dump_all(struct sde_dbg_evtlog *evtlog) { char buf[SDE_EVTLOG_BUF_MAX]; + bool update_last_entry = true; if (!evtlog) return; - while (sde_evtlog_dump_to_buffer(evtlog, buf, sizeof(buf))) + while (sde_evtlog_dump_to_buffer(evtlog, buf, sizeof(buf), + update_last_entry)) { pr_info("%s", buf); + update_last_entry = false; + } } struct sde_dbg_evtlog *sde_evtlog_init(void) From 3c6964a08c97236bf9a4938cf67becae4a0c5bf5 Mon Sep 17 00:00:00 2001 From: Lloyd Atkinson Date: Mon, 18 Sep 2017 16:51:24 -0400 Subject: [PATCH 4/4] drm/msm/sde: take irq callback lock before reading cb list Take the callback spinlock before checking whether the callback table list is empty. This resolves a race condition where the callback list could be empty during the servicing of an interrupt. Change-Id: I8d59c0211526173ce98c8ca2dac36ec4743dc8f8 Signed-off-by: Lloyd Atkinson Signed-off-by: Lakshmi Narayana Kalavala --- drivers/gpu/drm/msm/sde/sde_core_irq.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/sde/sde_core_irq.c b/drivers/gpu/drm/msm/sde/sde_core_irq.c index 83c8982b2e00..4f7e688650de 100644 --- a/drivers/gpu/drm/msm/sde/sde_core_irq.c +++ b/drivers/gpu/drm/msm/sde/sde_core_irq.c @@ -31,23 +31,35 @@ static void sde_core_irq_callback_handler(void *arg, int irq_idx) struct sde_irq *irq_obj = &sde_kms->irq_obj; struct sde_irq_callback *cb; unsigned long irq_flags; + bool cb_tbl_error = false; + int enable_counts = 0; pr_debug("irq_idx=%d\n", irq_idx); - if (list_empty(&irq_obj->irq_cb_tbl[irq_idx])) - SDE_ERROR("irq_idx=%d has no registered callback\n", irq_idx); + spin_lock_irqsave(&sde_kms->irq_obj.cb_lock, irq_flags); + if (list_empty(&irq_obj->irq_cb_tbl[irq_idx])) { + /* print error outside lock */ + cb_tbl_error = true; + enable_counts = atomic_read( + &sde_kms->irq_obj.enable_counts[irq_idx]); + } atomic_inc(&irq_obj->irq_counts[irq_idx]); /* * Perform registered function callback */ - spin_lock_irqsave(&sde_kms->irq_obj.cb_lock, irq_flags); list_for_each_entry(cb, &irq_obj->irq_cb_tbl[irq_idx], list) if (cb->func) cb->func(cb->arg, irq_idx); spin_unlock_irqrestore(&sde_kms->irq_obj.cb_lock, irq_flags); + if (cb_tbl_error) { + SDE_ERROR("irq has no registered callback, idx %d enables %d\n", + irq_idx, enable_counts); + SDE_EVT32_IRQ(irq_idx, enable_counts, SDE_EVTLOG_ERROR); + } + /* * Clear pending interrupt status in HW. * NOTE: sde_core_irq_callback_handler is protected by top-level