Uh oh!
There was an error while loading. Please reload this page.
[RFC][DO NOT MERGE][TESTS NEEDED] MSI/legacy PCI IPC improvements - #1109
Closed
plbossart wants to merge 2 commits into
Closed
[RFC][DO NOT MERGE][TESTS NEEDED] MSI/legacy PCI IPC improvements#1109plbossart wants to merge 2 commits into
plbossart wants to merge 2 commits into
Conversation
The addition of a kernel module parameter to optionally disable MSI had the side effect of permanently disabling it. The return value of pci_alloc_irq_vectors() is the number of allocated vectors or a negative number on error, so testing with the ! operator is not quite right. It was one optimization too far. Restore previous behavior to use MSI by default, unless the user selects not to do so or the allocation of irq_vectors fails. Fixes: 672ff5e ('ASoC: SOF: Intel: hda: add a parameter to disable MSI') Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
…ts in MSI mode This patch is a POC to test Guennadi's comments in PR 1042. The solution suggested in this patch keeps the existing code flow for the PCI legacy mode, and avoids losing interrupts for the MSI mode. MSI interrupt handling is currently conceptually broken. By re-enabling the interrupt at the end of the thread, we may lost some interrupts. We could modify the flow to re-enable the interrupt before reading status registers, however this would be sub-optimial if we used the PCI legacy mode since it would cause two calls to both the IRQ handler and thread. Another simpler solution would be to keep the interrupt enabled. This would not be a problem for MSI, but would create an interrupt rain in legacy mode. Having a conditional disabling/re-enabling of the interrupt in the legacy mode solves that problem. Let's test and see how this works. Suggested-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart
requested review from
RanderWang, bardliao, keyonjie, kv2019i, lgirdwood, lyakh, ranj063 and xiulipanAugust 2, 2019 21:01
keyonjie
reviewed
Aug 5, 2019
keyonjie
left a comment
There was a problem hiding this comment.
Change the first commit to:
if (hda_use_msi && !pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_MSI)) {
For the second commit, it looks good if we are aligned that we should disable/re-enable IPC interrupts for legacy IRQs and keep IPC interrupts on for MSI IRQs.
plbossart
commented
Aug 5, 2019
MemberAuthor
@keyonjie Can you take a look at the errors flagged by CI on Up2? this is the reported warning: voidsnd_pcm_period_elapsed(structsnd_pcm_substream*substream)
{
structsnd_pcm_runtime*runtime;
unsigned longflags;
if (PCM_RUNTIME_CHECK(substream))
return;And the firmware has a lot of errors as well: |
plbossart
commented
Aug 15, 2019
MemberAuthor
no one seems to care so closing. If IPC bugs show up again don't blame me. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a proof-of-concept compile-tested only suggestion, based on the slew of comments in
#1042 (stuck for 6+ weeks).
This is my interpretation of the discussion, only provided here to try and make progress. I may perfectly be completely wrong, I am way beyond my comfort zone here...
Comments and tests welcome, this is an important topic.