Skip to content

Conversation

@ranj063
Copy link
Collaborator

@ranj063 ranj063 commented Apr 28, 2019

There is a HW limitation that requires the host and link DMA engines
to be coupled whenever link DMA engine needs to be reset. This means
that even in decoupled mode, we always need to reserve link and
host DMA channels with the same ID. Trace utilizes only the host DMA.
Therefore, to prevent the link DMA with the same ID as trace host DMA
from being assigned to another stream, its link_locked attribute
must be set.

fixes thesofproject/sof#1173

There is a HW limitation that requires the host and link DMA engines
to be coupled whenever link DMA engine needs to be reset. This means
that even in decoupled mode, we always need to reserve link and
host DMA channels with the same ID. Trace utilizes only the host DMA.
Therefore, to prevent the link DMA with the same ID as trace host DMA
from being assigned to another stream, its link_locked attribute
must be set.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063 ranj063 requested a review from RanderWang April 28, 2019 04:03
@ranj063
Copy link
Collaborator Author

ranj063 commented Apr 28, 2019

@RanderWang @keqiaozhang I dont have a CML to test this. Could I please request to check if this fixes the issue?

@ranj063 ranj063 requested a review from plbossart April 28, 2019 04:05
@ranj063 ranj063 changed the title [RFC] Ensure link DMA channel with the same ID as trace host DMA is reserved [RFC] [DO NOT MERGER] Ensure link DMA channel with the same ID as trace host DMA is reserved Apr 28, 2019
@ranj063 ranj063 changed the title [RFC] [DO NOT MERGER] Ensure link DMA channel with the same ID as trace host DMA is reserved [RFC] [DO NOT MERGE] Ensure link DMA channel with the same ID as trace host DMA is reserved Apr 28, 2019
@RanderWang
Copy link

RanderWang commented Apr 28, 2019

@RanderWang @keqiaozhang I dont have a CML to test this. Could I please request to check if this fixes the issue?

This PR could fix the failure of FW loading, but it doesn't fix the coupled & decoupled issue in SOF.

@RanderWang RanderWang requested a review from keyonjie April 28, 2019 06:31
@RanderWang
Copy link

RanderWang commented Apr 28, 2019

I share you some cases to your list.
There are two issue in our SOF. one is the coupled-set_format-decoupled sequence in hda_stream_params, which would coupled alink dma channel used by other streams. Another one is restoring coupled mode when stream is closed, and this would also coupled a link dma channel used by other streams. I assume you get these points. It is also discussed in our Email.

Assuming all dma channel zero are used by dma trace.
(1) pcm1 is running with host dma channel two and link dma channel one, and pcm2 is started with host dma channel one and link dma channel two. (this is a bug report by QA) The coupled-set_format-decoupled sequence in hda_stream_params for pcm2 would break pcm1 because it would couple host dma channel one and link dma channel one used by pcm1. And even without this issue, when pcm2 is closed, it would restore link dma one & host dma one to coupled mode, this would also break pcm1

(2) a pcm with one FE and multiple BEs

(3) There are one FE and two BEs for pcm1, one FE & one BE fro pcm2, how to make them not affect each other. The key idea here is how to allocated dma channel

(3) a few pcms mixed by FW to one BEs.

(4) for tone generator, there is no FE only BE

(5) for stream like dma trace, there is only FE without BE

(6) For M streams map to N BEs. The streams are enabled in different time and maybe disabled in different time.

My advice is to refine the allocation algorithm and refine coupled mode setting in hw_free. According to Proborszcz, Filip( please review the Email for coupled &decoupled mode), they also refined the allocation algorithm on windows.

@ranj063
Copy link
Collaborator Author

ranj063 commented Apr 28, 2019

@RanderWang ack and thanks for the explanation. Agree that we need a slightly more sophisticated stream allocation scheme to get around the HW limitation. But this PR is still relevant isnt it?

@RanderWang
Copy link

@RanderWang ack and thanks for the explanation. Agree that we need a slightly more sophisticated stream allocation scheme to get around the HW limitation. But this PR is still relevant isnt it?

yes, it is relevant. It is a special case for allocation algorithm.

Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objection on the change proper, although it's not generic enough to cover all usages we discussed before. However the commit message and comments are misleading.
First we never talk about 'hardware limitation', they are 'recommended programming sequences' and second this is really related to the fact that the format has to be set while the host and link DMA are coupled.

}

/*
* There is a HW limitation that requires the host and link DMA engines
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope. the hw recommended programming sequence is to couple the host and link DMA so set the format, as done in a previous commit. This change has nothing to do with reset.

@ranj063
Copy link
Collaborator Author

ranj063 commented Apr 29, 2019

@plbossart I understand this isnt generic enough. I think this change along with reverting back to assigning the link dma channel during hw_params should take care of most of the cases. The only case that will still not work is 1 FE connected to multiple BE's. I think I'll need to have a chat with you about this.

@ranj063
Copy link
Collaborator Author

ranj063 commented May 1, 2019

Closing this one. Will be replaced by a slightly more generic solution soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[WHL HDA] pulseaudio have no sound

3 participants