From 56f4beffc9a056f70e56f53d01451441106bd4ff Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Mon, 7 Apr 2025 12:17:03 +0200 Subject: [PATCH 1/3] scsi: ufs: core: Drop last_intr_status/ts stats In order to prepare introduction of a threaded interrupt handler, drop last_intr_status & last_intr_ts drop the ufs_stats struct, and the associated debug code. Suggested-by: Bart Van Assche Reviewed-by: Bart Van Assche Signed-off-by: Neil Armstrong Link: https://lore.kernel.org/r/20250407-topic-ufs-use-threaded-irq-v3-1-08bee980f71e@linaro.org Signed-off-by: Martin K. Petersen --- drivers/ufs/core/ufshcd.c | 11 +++-------- include/ufs/ufshcd.h | 5 ----- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 0534390c2a35..5e73ac1e0078 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -643,9 +643,6 @@ static void ufshcd_print_host_state(struct ufs_hba *hba) "last_hibern8_exit_tstamp at %lld us, hibern8_exit_cnt=%d\n", div_u64(hba->ufs_stats.last_hibern8_exit_tstamp, 1000), hba->ufs_stats.hibern8_exit_cnt); - dev_err(hba->dev, "last intr at %lld us, last intr status=0x%x\n", - div_u64(hba->ufs_stats.last_intr_ts, 1000), - hba->ufs_stats.last_intr_status); dev_err(hba->dev, "error handling flags=0x%x, req. abort count=%d\n", hba->eh_flags, hba->req_abort_count); dev_err(hba->dev, "hba->ufs_version=0x%x, Host capabilities=0x%x, caps=0x%x\n", @@ -6984,14 +6981,12 @@ static irqreturn_t ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status) */ static irqreturn_t ufshcd_intr(int irq, void *__hba) { - u32 intr_status, enabled_intr_status = 0; + u32 last_intr_status, intr_status, enabled_intr_status = 0; irqreturn_t retval = IRQ_NONE; struct ufs_hba *hba = __hba; int retries = hba->nutrs; - intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS); - hba->ufs_stats.last_intr_status = intr_status; - hba->ufs_stats.last_intr_ts = local_clock(); + last_intr_status = intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS); /* * There could be max of hba->nutrs reqs in flight and in worst case @@ -7015,7 +7010,7 @@ static irqreturn_t ufshcd_intr(int irq, void *__hba) dev_err(hba->dev, "%s: Unhandled interrupt 0x%08x (0x%08x, 0x%08x)\n", __func__, intr_status, - hba->ufs_stats.last_intr_status, + last_intr_status, enabled_intr_status); ufshcd_dump_regs(hba, 0, UFSHCI_REG_SPACE_SIZE, "host_regs: "); } diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index e3909cc691b2..fffa9cc46543 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -501,8 +501,6 @@ struct ufs_event_hist { /** * struct ufs_stats - keeps usage/err statistics - * @last_intr_status: record the last interrupt status. - * @last_intr_ts: record the last interrupt timestamp. * @hibern8_exit_cnt: Counter to keep track of number of exits, * reset this after link-startup. * @last_hibern8_exit_tstamp: Set time after the hibern8 exit. @@ -510,9 +508,6 @@ struct ufs_event_hist { * @event: array with event history. */ struct ufs_stats { - u32 last_intr_status; - u64 last_intr_ts; - u32 hibern8_exit_cnt; u64 last_hibern8_exit_tstamp; struct ufs_event_hist event[UFS_EVT_CNT]; From 2d6c7bcc6cca63238fce0a95874b053500a1112e Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Mon, 7 Apr 2025 12:17:04 +0200 Subject: [PATCH 2/3] scsi: ufs: core: Track when MCQ ESI is enabled In preparation of adding a threaded interrupt handler, track when the MCQ ESI interrupt handlers were installed so we can optimize the MCQ interrupt handling to avoid walking the threaded handler in the case ESI handlers are enabled. Signed-off-by: Neil Armstrong Link: https://lore.kernel.org/r/20250407-topic-ufs-use-threaded-irq-v3-2-08bee980f71e@linaro.org Reviewed-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/ufs/core/ufshcd.c | 1 + include/ufs/ufshcd.h | 2 ++ 2 files changed, 3 insertions(+) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 5e73ac1e0078..7f256f77b8ba 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -8717,6 +8717,7 @@ static void ufshcd_config_mcq(struct ufs_hba *hba) u32 intrs; ret = ufshcd_mcq_vops_config_esi(hba); + hba->mcq_esi_enabled = !ret; dev_info(hba->dev, "ESI %sconfigured\n", ret ? "is not " : ""); intrs = UFSHCD_ENABLE_MCQ_INTRS; diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index fffa9cc46543..ec999fa67124 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -954,6 +954,7 @@ enum ufshcd_mcq_opr { * ufshcd_resume_complete() * @mcq_sup: is mcq supported by UFSHC * @mcq_enabled: is mcq ready to accept requests + * @mcq_esi_enabled: is mcq ESI configured * @res: array of resource info of MCQ registers * @mcq_base: Multi circular queue registers base address * @uhq: array of supported hardware queues @@ -1122,6 +1123,7 @@ struct ufs_hba { bool mcq_sup; bool lsdb_sup; bool mcq_enabled; + bool mcq_esi_enabled; struct ufshcd_res_info res[RES_MAX]; void __iomem *mcq_base; struct ufs_hw_queue *uhq; From 3c7ac40d732232fec0ba31d0a5e3cc9c112fc2e7 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Mon, 7 Apr 2025 12:17:05 +0200 Subject: [PATCH 3/3] scsi: ufs: core: Delegate the interrupt service routine to a threaded IRQ handler On systems with a large number request slots and unavailable MCQ ESI, the current design of the interrupt handler can delay handling of other subsystems interrupts causing display artifacts, GPU stalls or system firmware requests timeouts. Since the interrupt routine can take quite some time, it's preferable to move it to a threaded handler and leave the hard interrupt handler wake up the threaded interrupt routine, the interrupt line would be masked until the processing is finished in the thread thanks to the IRQS_ONESHOT flag. When MCQ & ESI interrupts are enabled the I/O completions are now directly handled in the "hard" interrupt routine to keep IOPs high since queues handling is done in separate per-queue interrupt routines. This fixes all encountered issued when running FIO tests on the Qualcomm SM8650 platform. Example of errors reported on a loaded system: [drm:dpu_encoder_frame_done_timeout:2706] [dpu error]enc32 frame done timeout msm_dpu ae01000.display-controller: [drm:hangcheck_handler [msm]] *ERROR* 67.5.20.1: hangcheck detected gpu lockup rb 2! msm_dpu ae01000.display-controller: [drm:hangcheck_handler [msm]] *ERROR* 67.5.20.1: completed fence: 74285 msm_dpu ae01000.display-controller: [drm:hangcheck_handler [msm]] *ERROR* 67.5.20.1: submitted fence: 74286 Error sending AMC RPMH requests (-110) Signed-off-by: Neil Armstrong Link: https://lore.kernel.org/r/20250407-topic-ufs-use-threaded-irq-v3-3-08bee980f71e@linaro.org Reviewed-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/ufs/core/ufshcd.c | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 7f256f77b8ba..b40660ca2fa6 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -6971,7 +6971,7 @@ static irqreturn_t ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status) } /** - * ufshcd_intr - Main interrupt service routine + * ufshcd_threaded_intr - Threaded interrupt service routine * @irq: irq number * @__hba: pointer to adapter instance * @@ -6979,7 +6979,7 @@ static irqreturn_t ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status) * IRQ_HANDLED - If interrupt is valid * IRQ_NONE - If invalid interrupt */ -static irqreturn_t ufshcd_intr(int irq, void *__hba) +static irqreturn_t ufshcd_threaded_intr(int irq, void *__hba) { u32 last_intr_status, intr_status, enabled_intr_status = 0; irqreturn_t retval = IRQ_NONE; @@ -7018,6 +7018,29 @@ static irqreturn_t ufshcd_intr(int irq, void *__hba) return retval; } +/** + * ufshcd_intr - Main interrupt service routine + * @irq: irq number + * @__hba: pointer to adapter instance + * + * Return: + * IRQ_HANDLED - If interrupt is valid + * IRQ_WAKE_THREAD - If handling is moved to threaded handled + * IRQ_NONE - If invalid interrupt + */ +static irqreturn_t ufshcd_intr(int irq, void *__hba) +{ + struct ufs_hba *hba = __hba; + + /* Move interrupt handling to thread when MCQ & ESI are not enabled */ + if (!hba->mcq_enabled || !hba->mcq_esi_enabled) + return IRQ_WAKE_THREAD; + + /* Directly handle interrupts since MCQ ESI handlers does the hard job */ + return ufshcd_sl_intr(hba, ufshcd_readl(hba, REG_INTERRUPT_STATUS) & + ufshcd_readl(hba, REG_INTERRUPT_ENABLE)); +} + static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag) { int err = 0; @@ -10577,7 +10600,8 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) ufshcd_readl(hba, REG_INTERRUPT_ENABLE); /* IRQ registration */ - err = devm_request_irq(dev, irq, ufshcd_intr, IRQF_SHARED, UFSHCD, hba); + err = devm_request_threaded_irq(dev, irq, ufshcd_intr, ufshcd_threaded_intr, + IRQF_ONESHOT | IRQF_SHARED, UFSHCD, hba); if (err) { dev_err(hba->dev, "request irq failed\n"); goto out_disable;