Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BYT: byt_irq_handler() is entered multiple times for a single IPC interrupt #1492

Closed
keyonjie opened this issue Nov 15, 2019 · 1 comment
Closed
Labels
bug Something isn't working

Comments

@keyonjie
Copy link

On BYT, byt_irq_handler() is entered multiple times for a single IPC interrupt, as we don't mask the interrupt in time in the handler.

experiment patch and dmesg attached below:

diff --git a/sound/soc/sof/intel/byt.c b/sound/soc/sof/intel/byt.c
index 2abf80b3eb52..307b4a03f4c5 100644
--- a/sound/soc/sof/intel/byt.c
+++ b/sound/soc/sof/intel/byt.c
@@ -189,6 +189,7 @@ static irqreturn_t byt_irq_handler(int irq, void *context)
 
        /* Interrupt arrived, check src */
        isr = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_ISRX);
+       printk(KERN_DEBUG "Keyon: %s, %d, isr:0x%llx\n", __func__, __LINE__, isr);
        if (isr & (SHIM_ISRX_DONE | SHIM_ISRX_BUSY))
                ret = IRQ_WAKE_THREAD;
 
@@ -203,10 +204,15 @@ static irqreturn_t byt_irq_thread(int irq, void *context)
 
        imrx = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IMRX);
        ipcx = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCX);
+       ipcd = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCD);
+
+       printk(KERN_DEBUG "Keyon: %s, %d, imrx:0x%llx, ipcx:0x%llx, ipcd:0x%llx\n",
+              __func__, __LINE__, imrx, ipcx, ipcd);
 
        /* reply message from DSP */
        if (ipcx & SHIM_BYT_IPCX_DONE &&
            !(imrx & SHIM_IMRX_DONE)) {
+               printk(KERN_DEBUG "Keyon: %s, %d\n", __func__, __LINE__);
                /* Mask Done interrupt before first */
                snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR,
                                                   SHIM_IMRX,
@@ -231,9 +237,9 @@ static irqreturn_t byt_irq_thread(int irq, void *context)
        }
 
        /* new message from DSP */
-       ipcd = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCD);
        if (ipcd & SHIM_BYT_IPCD_BUSY &&
            !(imrx & SHIM_IMRX_BUSY)) {
+               printk(KERN_DEBUG "Keyon: %s, %d\n", __func__, __LINE__);
                /* Mask Busy interrupt before return */
                snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR,
                                                   SHIM_IMRX,

dmesg logs during set volume IPC as below:

[    9.602493] sof-audio-acpi 808622A8:00: ipc tx: 0x50010000
[    9.602545] Keyon: byt_irq_handler, 192, isr:0x1
[    9.602592] Keyon: byt_irq_handler, 192, isr:0x1
[    9.602605] Keyon: byt_irq_thread, 210, imrx:0x0, ipcx:0x4000000000000000, ipcd:0x0
[    9.602609] Keyon: byt_irq_handler, 192, isr:0x1
[    9.602617] Keyon: byt_irq_thread, 215
[    9.602626] Keyon: byt_irq_handler, 192, isr:0x1
[    9.602687] Keyon: byt_irq_thread, 210, imrx:0x0, ipcx:0x0, ipcd:0x0
[    9.602699] sof-audio-acpi 808622A8:00: ipc tx succeeded: 0x50010000
[    9.602773] sof-audio-acpi 808622A8:00: ipc tx: 0x50010000
[    9.602884] Keyon: byt_irq_handler, 192, isr:0x1
[    9.602950] Keyon: byt_irq_handler, 192, isr:0x1
[    9.602971] Keyon: byt_irq_handler, 192, isr:0x1
[    9.602983] Keyon: byt_irq_thread, 210, imrx:0x0, ipcx:0x4000000000000000, ipcd:0x0
[    9.602986] Keyon: byt_irq_thread, 215
[    9.602997] Keyon: byt_irq_handler, 192, isr:0x1
[    9.603054] Keyon: byt_irq_thread, 210, imrx:0x0, ipcx:0x0, ipcd:0x0
@kv2019i kv2019i added the bug Something isn't working label Jan 8, 2020
plbossart added a commit to plbossart/sound that referenced this issue May 22, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

BugLink: thesofproject#1492
Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart added a commit to plbossart/sound that referenced this issue May 22, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

BugLink: thesofproject#1492
Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart added a commit to plbossart/sound that referenced this issue May 22, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

BugLink: thesofproject#1492
Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
youling257 pushed a commit to youling257/android-mainline that referenced this issue May 23, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

BugLink: thesofproject/linux#1492
Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
youling257 pushed a commit to youling257/android-mainline that referenced this issue May 25, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

BugLink: thesofproject/linux#1492
Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart added a commit to plbossart/sound that referenced this issue May 26, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

BugLink: thesofproject#1492
Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart added a commit that referenced this issue May 26, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

BugLink: #1492
Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
fengguang pushed a commit to 0day-ci/linux that referenced this issue May 26, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

BugLink: thesofproject#1492
Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart added a commit that referenced this issue May 26, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

BugLink: #1492
Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@plbossart
Copy link
Member

fixed now

fengguang pushed a commit to 0day-ci/linux that referenced this issue May 27, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
BugLink: thesofproject#1492
Link: https://lore.kernel.org/r/20200526203640.25980-8-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
youling257 pushed a commit to youling257/android-mainline that referenced this issue May 29, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
BugLink: thesofproject/linux#1492
Link: https://lore.kernel.org/r/20200526203640.25980-8-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
youling257 pushed a commit to youling257/android-mainline that referenced this issue May 29, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
BugLink: thesofproject/linux#1492
Link: https://lore.kernel.org/r/20200526203640.25980-8-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
youling257 pushed a commit to youling257/android-mainline that referenced this issue May 31, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
BugLink: thesofproject/linux#1492
Link: https://lore.kernel.org/r/20200526203640.25980-8-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
youling257 pushed a commit to youling257/android-mainline that referenced this issue Jun 1, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
BugLink: thesofproject/linux#1492
Link: https://lore.kernel.org/r/20200526203640.25980-8-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
maurossi pushed a commit to maurossi/linux that referenced this issue Jun 1, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
BugLink: thesofproject/linux#1492
Link: https://lore.kernel.org/r/20200526203640.25980-8-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
kitakar5525 pushed a commit to kitakar5525/linux-kernel that referenced this issue Sep 16, 2020
The DSP may send the same interrupt multiple times before it's handled
in the interrupt thread. Rather than masking it in the thread, mask it
in the handler directly.

This patch also removes useless checks that cannot happen, and masks
that are set don't need to be re-tested.

Suggested-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
BugLink: thesofproject/linux#1492
Link: https://lore.kernel.org/r/20200526203640.25980-8-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
(cherry picked from commit 3d3d1fb)
Signed-off-by: Chintan Patel <chintan.m.patel@intel.com>

Conflicts: Add ret = IRQ_WAKE_THREAD under byt_irq_handler

BUG=b:162008784
TEST=Test audio on volteer.

Signed-off-by: Mike Mason <michael.w.mason@intel.corp-partner.google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants