Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

soundwire: intel/cadence: merge Soundwire interrupt handlers/threads

The existing code uses one pair of interrupt handler/thread per link
but at the hardware level the interrupt is shared. This works fine for
legacy PCI interrupts, but leads to timeouts in MSI (Message-Signaled
Interrupt) mode, likely due to edges being lost.

This patch unifies interrupt handling for all links. The dedicated
handler is removed since we use a common one for all shared interrupt
sources, and the thread function takes care of dealing with interrupt
sources. This partition follows the model used for the SOF IPC on
HDaudio platforms, where similar timeout issues were noticed and doing
all the interrupt handling/clearing in the thread improved
reliability/stability.

Validation results with 4 links active in parallel show a night-and-day
improvement with no timeouts noticed even during stress tests. Latency
and quality of service are not affected by the change - mostly because
events on a SoundWire link are throttled by the bus frame rate
(typically 8..48kHz).

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200716150947.22119-8-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>

authored by

Bard Liao and committed by
Vinod Koul
4a98a6b2 8459cea7

+37 -23
+10 -8
drivers/soundwire/cadence_master.c
··· 17 17 #include <linux/soundwire/sdw.h> 18 18 #include <sound/pcm_params.h> 19 19 #include <sound/soc.h> 20 + #include <linux/workqueue.h> 20 21 #include "bus.h" 21 22 #include "cadence_master.h" 22 23 ··· 791 790 CDNS_MCP_INT_SLAVE_MASK, 0); 792 791 793 792 int_status &= ~CDNS_MCP_INT_SLAVE_MASK; 794 - ret = IRQ_WAKE_THREAD; 793 + schedule_work(&cdns->work); 795 794 } 796 795 797 796 cdns_writel(cdns, CDNS_MCP_INTSTAT, int_status); ··· 800 799 EXPORT_SYMBOL(sdw_cdns_irq); 801 800 802 801 /** 803 - * sdw_cdns_thread() - Cadence irq thread handler 804 - * @irq: irq number 805 - * @dev_id: irq context 802 + * To update slave status in a work since we will need to handle 803 + * other interrupts eg. CDNS_MCP_INT_RX_WL during the update slave 804 + * process. 805 + * @work: cdns worker thread 806 806 */ 807 - irqreturn_t sdw_cdns_thread(int irq, void *dev_id) 807 + static void cdns_update_slave_status_work(struct work_struct *work) 808 808 { 809 - struct sdw_cdns *cdns = dev_id; 809 + struct sdw_cdns *cdns = 810 + container_of(work, struct sdw_cdns, work); 810 811 u32 slave0, slave1; 811 812 812 813 dev_dbg_ratelimited(cdns->dev, "Slave status change\n"); ··· 825 822 cdns_updatel(cdns, CDNS_MCP_INTMASK, 826 823 CDNS_MCP_INT_SLAVE_MASK, CDNS_MCP_INT_SLAVE_MASK); 827 824 828 - return IRQ_HANDLED; 829 825 } 830 - EXPORT_SYMBOL(sdw_cdns_thread); 831 826 832 827 /* 833 828 * init routines ··· 1428 1427 init_completion(&cdns->tx_complete); 1429 1428 cdns->bus.port_ops = &cdns_port_ops; 1430 1429 1430 + INIT_WORK(&cdns->work, cdns_update_slave_status_work); 1431 1431 return 0; 1432 1432 } 1433 1433 EXPORT_SYMBOL(sdw_cdns_probe);
+4
drivers/soundwire/cadence_master.h
··· 129 129 130 130 bool link_up; 131 131 unsigned int msg_count; 132 + 133 + struct work_struct work; 134 + 135 + struct list_head list; 132 136 }; 133 137 134 138 #define bus_to_cdns(_bus) container_of(_bus, struct sdw_cdns, bus)
-15
drivers/soundwire/intel.c
··· 1258 1258 "SoundWire master %d is disabled, will be ignored\n", 1259 1259 bus->link_id); 1260 1260 1261 - /* Acquire IRQ */ 1262 - ret = request_threaded_irq(sdw->link_res->irq, 1263 - sdw_cdns_irq, sdw_cdns_thread, 1264 - IRQF_SHARED, KBUILD_MODNAME, cdns); 1265 - if (ret < 0) { 1266 - dev_err(dev, "unable to grab IRQ %d, disabling device\n", 1267 - sdw->link_res->irq); 1268 - goto err_init; 1269 - } 1270 - 1271 1261 return 0; 1272 - 1273 - err_init: 1274 - sdw_bus_master_delete(bus); 1275 - return ret; 1276 1262 } 1277 1263 1278 1264 int intel_master_startup(struct platform_device *pdev) ··· 1330 1344 if (!bus->prop.hw_disabled) { 1331 1345 intel_debugfs_exit(sdw); 1332 1346 sdw_cdns_enable_interrupt(cdns, false); 1333 - free_irq(sdw->link_res->irq, sdw); 1334 1347 snd_soc_unregister_component(dev); 1335 1348 } 1336 1349 sdw_bus_master_delete(bus);
+4
drivers/soundwire/intel.h
··· 17 17 * @dev: device implementing hw_params and free callbacks 18 18 * @shim_lock: mutex to handle access to shared SHIM registers 19 19 * @shim_mask: global pointer to check SHIM register initialization 20 + * @cdns: Cadence master descriptor 21 + * @list: used to walk-through all masters exposed by the same controller 20 22 */ 21 23 struct sdw_intel_link_res { 22 24 struct platform_device *pdev; ··· 31 29 struct device *dev; 32 30 struct mutex *shim_lock; /* protect shared registers */ 33 31 u32 *shim_mask; 32 + struct sdw_cdns *cdns; 33 + struct list_head list; 34 34 }; 35 35 36 36 struct sdw_intel {
+19
drivers/soundwire/intel_init.c
··· 9 9 10 10 #include <linux/acpi.h> 11 11 #include <linux/export.h> 12 + #include <linux/interrupt.h> 12 13 #include <linux/io.h> 13 14 #include <linux/module.h> 14 15 #include <linux/platform_device.h> ··· 167 166 } 168 167 EXPORT_SYMBOL_NS(sdw_intel_enable_irq, SOUNDWIRE_INTEL_INIT); 169 168 169 + irqreturn_t sdw_intel_thread(int irq, void *dev_id) 170 + { 171 + struct sdw_intel_ctx *ctx = dev_id; 172 + struct sdw_intel_link_res *link; 173 + 174 + list_for_each_entry(link, &ctx->link_list, list) 175 + sdw_cdns_irq(irq, link->cdns); 176 + 177 + sdw_intel_enable_irq(ctx->mmio_base, true); 178 + return IRQ_HANDLED; 179 + } 180 + EXPORT_SYMBOL_NS(sdw_intel_thread, SOUNDWIRE_INTEL_INIT); 181 + 170 182 static struct sdw_intel_ctx 171 183 *sdw_intel_probe_controller(struct sdw_intel_res *res) 172 184 { ··· 223 209 link = ctx->links; 224 210 link_mask = ctx->link_mask; 225 211 212 + INIT_LIST_HEAD(&ctx->link_list); 213 + 226 214 /* Create SDW Master devices */ 227 215 for (i = 0; i < count; i++, link++) { 228 216 if (!(link_mask & BIT(i))) { ··· 262 246 goto err; 263 247 } 264 248 link->pdev = pdev; 249 + link->cdns = platform_get_drvdata(pdev); 250 + 251 + list_add_tail(&link->list, &ctx->link_list); 265 252 } 266 253 267 254 return ctx;