Skip to content

Commit 27a153e

Browse files
brooniebroonie
committed
ASoC: Intel: avs: Refactor IRQ handling
Merge series from Cezary Rojewski <cezary.rojewski@intel.com>: The existing code can be both improved and simplified. To make this change easier to manage, first add new implementation and then remove deadcode in a separate patch. Simplification achieved with: - reduce the amount of resources requested by the driver i.e.: IPC and CLDMA request_irq() merged into one - reduce the number of DSP ops from 2 to 1: irq_handler/thread() vs dsp_interrupt() - drop ambiguity around CLDMA interrupt, let skl.c handle that explicitly as it is the only user With that done, switch to the new implementation and remove unused members. While the change is non-trivial, from functional perspective status quo is achieved.
2 parents ed37d24 + 84049e2 commit 27a153e

10 files changed

Lines changed: 186 additions & 188 deletions

File tree

‎sound/soc/intel/avs/apl.c‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,28 @@
88

99
#include <linux/devcoredump.h>
1010
#include <linux/slab.h>
11+
#include <sound/hdaudio_ext.h>
1112
#include "avs.h"
1213
#include "messages.h"
1314
#include "path.h"
1415
#include "topology.h"
1516

17+
static irqreturn_t avs_apl_dsp_interrupt(struct avs_dev *adev)
18+
{
19+
u32 adspis = snd_hdac_adsp_readl(adev, AVS_ADSP_REG_ADSPIS);
20+
irqreturn_t ret = IRQ_NONE;
21+
22+
if (adspis == UINT_MAX)
23+
return ret;
24+
25+
if (adspis & AVS_ADSP_ADSPIS_IPC) {
26+
avs_skl_ipc_interrupt(adev);
27+
ret = IRQ_HANDLED;
28+
}
29+
30+
return ret;
31+
}
32+
1633
#ifdef CONFIG_DEBUG_FS
1734
int avs_apl_enable_logs(struct avs_dev *adev, enum avs_log_enable enable, u32 aging_period,
1835
u32 fifo_full_period, unsigned long resource_mask, u32 *priorities)
@@ -237,8 +254,7 @@ const struct avs_dsp_ops avs_apl_dsp_ops = {
237254
.power = avs_dsp_core_power,
238255
.reset = avs_dsp_core_reset,
239256
.stall = avs_dsp_core_stall,
240-
.irq_handler = avs_irq_handler,
241-
.irq_thread = avs_skl_irq_thread,
257+
.dsp_interrupt = avs_apl_dsp_interrupt,
242258
.int_control = avs_dsp_interrupt_control,
243259
.load_basefw = avs_hda_load_basefw,
244260
.load_lib = avs_hda_load_library,

‎sound/soc/intel/avs/avs.h‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ struct avs_dsp_ops {
4646
int (* const power)(struct avs_dev *, u32, bool);
4747
int (* const reset)(struct avs_dev *, u32, bool);
4848
int (* const stall)(struct avs_dev *, u32, bool);
49-
irqreturn_t (* const irq_handler)(struct avs_dev *);
50-
irqreturn_t (* const irq_thread)(struct avs_dev *);
49+
irqreturn_t (* const dsp_interrupt)(struct avs_dev *);
5150
void (* const int_control)(struct avs_dev *, bool);
5251
int (* const load_basefw)(struct avs_dev *, struct firmware *);
5352
int (* const load_lib)(struct avs_dev *, struct firmware *, u32);
@@ -245,7 +244,6 @@ struct avs_ipc {
245244
#define AVS_IPC_RET(ret) \
246245
(((ret) <= 0) ? (ret) : -AVS_EIPC)
247246

248-
irqreturn_t avs_irq_handler(struct avs_dev *adev);
249247
void avs_dsp_process_response(struct avs_dev *adev, u64 header);
250248
int avs_dsp_send_msg_timeout(struct avs_dev *adev, struct avs_ipc_msg *request,
251249
struct avs_ipc_msg *reply, int timeout, const char *name);
@@ -267,8 +265,8 @@ void avs_ipc_block(struct avs_ipc *ipc);
267265
int avs_dsp_disable_d0ix(struct avs_dev *adev);
268266
int avs_dsp_enable_d0ix(struct avs_dev *adev);
269267

270-
irqreturn_t avs_skl_irq_thread(struct avs_dev *adev);
271-
irqreturn_t avs_cnl_irq_thread(struct avs_dev *adev);
268+
void avs_skl_ipc_interrupt(struct avs_dev *adev);
269+
irqreturn_t avs_cnl_dsp_interrupt(struct avs_dev *adev);
272270
int avs_apl_enable_logs(struct avs_dev *adev, enum avs_log_enable enable, u32 aging_period,
273271
u32 fifo_full_period, unsigned long resource_mask, u32 *priorities);
274272
int avs_icl_enable_logs(struct avs_dev *adev, enum avs_log_enable enable, u32 aging_period,

‎sound/soc/intel/avs/cldma.c‎

Lines changed: 8 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -248,69 +248,43 @@ void hda_cldma_setup(struct hda_cldma *cl)
248248
snd_hdac_stream_writel(cl, CL_SPBFCTL, 1);
249249
}
250250

251-
static irqreturn_t cldma_irq_handler(int irq, void *dev_id)
251+
void hda_cldma_interrupt(struct hda_cldma *cl)
252252
{
253-
struct hda_cldma *cl = dev_id;
254-
u32 adspis;
255-
256-
adspis = snd_hdac_adsp_readl(cl, AVS_ADSP_REG_ADSPIS);
257-
if (adspis == UINT_MAX)
258-
return IRQ_NONE;
259-
if (!(adspis & AVS_ADSP_ADSPIS_CLDMA))
260-
return IRQ_NONE;
261-
262-
cl->sd_status = snd_hdac_stream_readb(cl, SD_STS);
263-
dev_warn(cl->dev, "%s sd_status: 0x%08x\n", __func__, cl->sd_status);
264-
265253
/* disable CLDMA interrupt */
266254
snd_hdac_adsp_updatel(cl, AVS_ADSP_REG_ADSPIC, AVS_ADSP_ADSPIC_CLDMA, 0);
267255

268-
complete(&cl->completion);
256+
cl->sd_status = snd_hdac_stream_readb(cl, SD_STS);
257+
dev_dbg(cl->dev, "%s sd_status: 0x%08x\n", __func__, cl->sd_status);
269258

270-
return IRQ_HANDLED;
259+
complete(&cl->completion);
271260
}
272261

273262
int hda_cldma_init(struct hda_cldma *cl, struct hdac_bus *bus, void __iomem *dsp_ba,
274263
unsigned int buffer_size)
275264
{
276-
struct pci_dev *pci = to_pci_dev(bus->dev);
277265
int ret;
278266

279267
ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, bus->dev, buffer_size, &cl->dmab_data);
280268
if (ret < 0)
281269
return ret;
282270

283271
ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, bus->dev, BDL_SIZE, &cl->dmab_bdl);
284-
if (ret < 0)
285-
goto alloc_err;
272+
if (ret < 0) {
273+
snd_dma_free_pages(&cl->dmab_data);
274+
return ret;
275+
}
286276

287277
cl->dev = bus->dev;
288278
cl->bus = bus;
289279
cl->dsp_ba = dsp_ba;
290280
cl->buffer_size = buffer_size;
291281
cl->sd_addr = dsp_ba + AZX_CL_SD_BASE;
292282

293-
ret = pci_request_irq(pci, 0, cldma_irq_handler, NULL, cl, "CLDMA");
294-
if (ret < 0) {
295-
dev_err(cl->dev, "Failed to request CLDMA IRQ handler: %d\n", ret);
296-
goto req_err;
297-
}
298-
299283
return 0;
300-
301-
req_err:
302-
snd_dma_free_pages(&cl->dmab_bdl);
303-
alloc_err:
304-
snd_dma_free_pages(&cl->dmab_data);
305-
306-
return ret;
307284
}
308285

309286
void hda_cldma_free(struct hda_cldma *cl)
310287
{
311-
struct pci_dev *pci = to_pci_dev(cl->dev);
312-
313-
pci_free_irq(pci, 0, cl);
314288
snd_dma_free_pages(&cl->dmab_data);
315289
snd_dma_free_pages(&cl->dmab_bdl);
316290
}

‎sound/soc/intel/avs/cldma.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ int hda_cldma_reset(struct hda_cldma *cl);
2424

2525
void hda_cldma_set_data(struct hda_cldma *cl, void *data, unsigned int size);
2626
void hda_cldma_setup(struct hda_cldma *cl);
27+
void hda_cldma_interrupt(struct hda_cldma *cl);
2728
int hda_cldma_init(struct hda_cldma *cl, struct hdac_bus *bus, void __iomem *dsp_ba,
2829
unsigned int buffer_size);
2930
void hda_cldma_free(struct hda_cldma *cl);

‎sound/soc/intel/avs/cnl.c‎

Lines changed: 60 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,44 +10,73 @@
1010
#include "avs.h"
1111
#include "messages.h"
1212

13-
irqreturn_t avs_cnl_irq_thread(struct avs_dev *adev)
13+
static void avs_cnl_ipc_interrupt(struct avs_dev *adev)
1414
{
15-
union avs_reply_msg msg;
16-
u32 hipctdr, hipctdd, hipctda;
17-
18-
hipctdr = snd_hdac_adsp_readl(adev, CNL_ADSP_REG_HIPCTDR);
19-
hipctdd = snd_hdac_adsp_readl(adev, CNL_ADSP_REG_HIPCTDD);
20-
21-
/* Ensure DSP sent new response to process. */
22-
if (!(hipctdr & CNL_ADSP_HIPCTDR_BUSY))
23-
return IRQ_NONE;
24-
25-
msg.primary = hipctdr;
26-
msg.ext.val = hipctdd;
27-
avs_dsp_process_response(adev, msg.val);
28-
29-
/* Tell DSP we accepted its message. */
30-
snd_hdac_adsp_updatel(adev, CNL_ADSP_REG_HIPCTDR,
31-
CNL_ADSP_HIPCTDR_BUSY, CNL_ADSP_HIPCTDR_BUSY);
32-
/* Ack this response. */
33-
snd_hdac_adsp_updatel(adev, CNL_ADSP_REG_HIPCTDA,
34-
CNL_ADSP_HIPCTDA_DONE, CNL_ADSP_HIPCTDA_DONE);
35-
/* HW might have been clock gated, give some time for change to propagate. */
36-
snd_hdac_adsp_readl_poll(adev, CNL_ADSP_REG_HIPCTDA, hipctda,
37-
!(hipctda & CNL_ADSP_HIPCTDA_DONE), 10, 1000);
38-
/* Unmask busy interrupt. */
39-
snd_hdac_adsp_updatel(adev, CNL_ADSP_REG_HIPCCTL,
40-
AVS_ADSP_HIPCCTL_BUSY, AVS_ADSP_HIPCCTL_BUSY);
41-
42-
return IRQ_HANDLED;
15+
const struct avs_spec *spec = adev->spec;
16+
u32 hipc_ack, hipc_rsp;
17+
18+
snd_hdac_adsp_updatel(adev, spec->hipc->ctl_offset,
19+
AVS_ADSP_HIPCCTL_DONE | AVS_ADSP_HIPCCTL_BUSY, 0);
20+
21+
hipc_ack = snd_hdac_adsp_readl(adev, spec->hipc->ack_offset);
22+
hipc_rsp = snd_hdac_adsp_readl(adev, spec->hipc->rsp_offset);
23+
24+
/* DSP acked host's request. */
25+
if (hipc_ack & spec->hipc->ack_done_mask) {
26+
complete(&adev->ipc->done_completion);
27+
28+
/* Tell DSP it has our attention. */
29+
snd_hdac_adsp_updatel(adev, spec->hipc->ack_offset, spec->hipc->ack_done_mask,
30+
spec->hipc->ack_done_mask);
31+
}
32+
33+
/* DSP sent new response to process. */
34+
if (hipc_rsp & spec->hipc->rsp_busy_mask) {
35+
union avs_reply_msg msg;
36+
u32 hipctda;
37+
38+
msg.primary = snd_hdac_adsp_readl(adev, CNL_ADSP_REG_HIPCTDR);
39+
msg.ext.val = snd_hdac_adsp_readl(adev, CNL_ADSP_REG_HIPCTDD);
40+
41+
avs_dsp_process_response(adev, msg.val);
42+
43+
/* Tell DSP we accepted its message. */
44+
snd_hdac_adsp_updatel(adev, CNL_ADSP_REG_HIPCTDR,
45+
CNL_ADSP_HIPCTDR_BUSY, CNL_ADSP_HIPCTDR_BUSY);
46+
/* Ack this response. */
47+
snd_hdac_adsp_updatel(adev, CNL_ADSP_REG_HIPCTDA,
48+
CNL_ADSP_HIPCTDA_DONE, CNL_ADSP_HIPCTDA_DONE);
49+
/* HW might have been clock gated, give some time for change to propagate. */
50+
snd_hdac_adsp_readl_poll(adev, CNL_ADSP_REG_HIPCTDA, hipctda,
51+
!(hipctda & CNL_ADSP_HIPCTDA_DONE), 10, 1000);
52+
}
53+
54+
snd_hdac_adsp_updatel(adev, spec->hipc->ctl_offset,
55+
AVS_ADSP_HIPCCTL_DONE | AVS_ADSP_HIPCCTL_BUSY,
56+
AVS_ADSP_HIPCCTL_DONE | AVS_ADSP_HIPCCTL_BUSY);
57+
}
58+
59+
irqreturn_t avs_cnl_dsp_interrupt(struct avs_dev *adev)
60+
{
61+
u32 adspis = snd_hdac_adsp_readl(adev, AVS_ADSP_REG_ADSPIS);
62+
irqreturn_t ret = IRQ_NONE;
63+
64+
if (adspis == UINT_MAX)
65+
return ret;
66+
67+
if (adspis & AVS_ADSP_ADSPIS_IPC) {
68+
avs_cnl_ipc_interrupt(adev);
69+
ret = IRQ_HANDLED;
70+
}
71+
72+
return ret;
4373
}
4474

4575
const struct avs_dsp_ops avs_cnl_dsp_ops = {
4676
.power = avs_dsp_core_power,
4777
.reset = avs_dsp_core_reset,
4878
.stall = avs_dsp_core_stall,
49-
.irq_handler = avs_irq_handler,
50-
.irq_thread = avs_cnl_irq_thread,
79+
.dsp_interrupt = avs_cnl_dsp_interrupt,
5180
.int_control = avs_dsp_interrupt_control,
5281
.load_basefw = avs_hda_load_basefw,
5382
.load_lib = avs_hda_load_library,

‎sound/soc/intel/avs/core.c‎

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -257,67 +257,55 @@ static void hdac_update_stream(struct hdac_bus *bus, struct hdac_stream *stream)
257257
}
258258
}
259259

260-
static irqreturn_t hdac_bus_irq_handler(int irq, void *context)
260+
static irqreturn_t avs_hda_interrupt(struct hdac_bus *bus)
261261
{
262-
struct hdac_bus *bus = context;
263-
u32 mask, int_enable;
262+
irqreturn_t ret = IRQ_NONE;
264263
u32 status;
265-
int ret = IRQ_NONE;
266-
267-
if (!pm_runtime_active(bus->dev))
268-
return ret;
269-
270-
spin_lock(&bus->reg_lock);
271264

272265
status = snd_hdac_chip_readl(bus, INTSTS);
273-
if (status == 0 || status == UINT_MAX) {
274-
spin_unlock(&bus->reg_lock);
275-
return ret;
276-
}
266+
if (snd_hdac_bus_handle_stream_irq(bus, status, hdac_update_stream))
267+
ret = IRQ_HANDLED;
277268

278-
/* clear rirb int */
269+
spin_lock_irq(&bus->reg_lock);
270+
/* Clear RIRB interrupt. */
279271
status = snd_hdac_chip_readb(bus, RIRBSTS);
280272
if (status & RIRB_INT_MASK) {
281273
if (status & RIRB_INT_RESPONSE)
282274
snd_hdac_bus_update_rirb(bus);
283275
snd_hdac_chip_writeb(bus, RIRBSTS, RIRB_INT_MASK);
284-
}
285-
286-
mask = (0x1 << bus->num_streams) - 1;
287-
288-
status = snd_hdac_chip_readl(bus, INTSTS);
289-
status &= mask;
290-
if (status) {
291-
/* Disable stream interrupts; Re-enable in bottom half */
292-
int_enable = snd_hdac_chip_readl(bus, INTCTL);
293-
snd_hdac_chip_writel(bus, INTCTL, (int_enable & (~mask)));
294-
ret = IRQ_WAKE_THREAD;
295-
} else {
296276
ret = IRQ_HANDLED;
297277
}
298278

299-
spin_unlock(&bus->reg_lock);
279+
spin_unlock_irq(&bus->reg_lock);
300280
return ret;
301281
}
302282

303-
static irqreturn_t hdac_bus_irq_thread(int irq, void *context)
283+
static irqreturn_t avs_hda_irq_handler(int irq, void *dev_id)
284+
{
285+
struct hdac_bus *bus = dev_id;
286+
u32 intsts;
287+
288+
intsts = snd_hdac_chip_readl(bus, INTSTS);
289+
if (intsts == UINT_MAX || !(intsts & AZX_INT_GLOBAL_EN))
290+
return IRQ_NONE;
291+
292+
/* Mask GIE, unmasked in irq_thread(). */
293+
snd_hdac_chip_updatel(bus, INTCTL, AZX_INT_GLOBAL_EN, 0);
294+
295+
return IRQ_WAKE_THREAD;
296+
}
297+
298+
static irqreturn_t avs_hda_irq_thread(int irq, void *dev_id)
304299
{
305-
struct hdac_bus *bus = context;
300+
struct hdac_bus *bus = dev_id;
306301
u32 status;
307-
u32 int_enable;
308-
u32 mask;
309-
unsigned long flags;
310302

311303
status = snd_hdac_chip_readl(bus, INTSTS);
304+
if (status & ~AZX_INT_GLOBAL_EN)
305+
avs_hda_interrupt(bus);
312306

313-
snd_hdac_bus_handle_stream_irq(bus, status, hdac_update_stream);
314-
315-
/* Re-enable stream interrupts */
316-
mask = (0x1 << bus->num_streams) - 1;
317-
spin_lock_irqsave(&bus->reg_lock, flags);
318-
int_enable = snd_hdac_chip_readl(bus, INTCTL);
319-
snd_hdac_chip_writel(bus, INTCTL, (int_enable | mask));
320-
spin_unlock_irqrestore(&bus->reg_lock, flags);
307+
/* Unmask GIE, masked in irq_handler(). */
308+
snd_hdac_chip_updatel(bus, INTCTL, AZX_INT_GLOBAL_EN, AZX_INT_GLOBAL_EN);
321309

322310
return IRQ_HANDLED;
323311
}
@@ -326,14 +314,23 @@ static irqreturn_t avs_dsp_irq_handler(int irq, void *dev_id)
326314
{
327315
struct avs_dev *adev = dev_id;
328316

329-
return avs_dsp_op(adev, irq_handler);
317+
return avs_hda_irq_handler(irq, &adev->base.core);
330318
}
331319

332320
static irqreturn_t avs_dsp_irq_thread(int irq, void *dev_id)
333321
{
334322
struct avs_dev *adev = dev_id;
323+
struct hdac_bus *bus = &adev->base.core;
324+
u32 status;
325+
326+
status = readl(bus->ppcap + AZX_REG_PP_PPSTS);
327+
if (status & AZX_PPCTL_PIE)
328+
avs_dsp_op(adev, dsp_interrupt);
335329

336-
return avs_dsp_op(adev, irq_thread);
330+
/* Unmask GIE, masked in irq_handler(). */
331+
snd_hdac_chip_updatel(bus, INTCTL, AZX_INT_GLOBAL_EN, AZX_INT_GLOBAL_EN);
332+
333+
return IRQ_HANDLED;
337334
}
338335

339336
static int avs_hdac_acquire_irq(struct avs_dev *adev)
@@ -349,7 +346,7 @@ static int avs_hdac_acquire_irq(struct avs_dev *adev)
349346
return ret;
350347
}
351348

352-
ret = pci_request_irq(pci, 0, hdac_bus_irq_handler, hdac_bus_irq_thread, bus,
349+
ret = pci_request_irq(pci, 0, avs_hda_irq_handler, avs_hda_irq_thread, bus,
353350
KBUILD_MODNAME);
354351
if (ret < 0) {
355352
dev_err(adev->dev, "Failed to request stream IRQ handler: %d\n", ret);
@@ -530,8 +527,6 @@ static void avs_pci_shutdown(struct pci_dev *pci)
530527
snd_hdac_bus_stop_chip(bus);
531528
snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, false);
532529

533-
if (avs_platattr_test(adev, CLDMA))
534-
pci_free_irq(pci, 0, &code_loader);
535530
pci_free_irq(pci, 0, adev);
536531
pci_free_irq(pci, 0, bus);
537532
pci_free_irq_vectors(pci);

0 commit comments

Comments
 (0)