SSP BLCK and MCLK early start - take3 #2965
Merged
Conversation
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
plbossart
requested review from
RanderWang,
bardliao,
dbaluta,
juimonen,
keyonjie,
libinyang,
ranj063 and
singalsu
as code owners
plbossart
added
ABI
ranj063
previously approved these changes
Jun 3, 2021
Collaborator
ranj063
left a comment
ranj063
left a comment
There was a problem hiding this comment.
looks good @plbossart. you have a typo on "botht" in the second patch's commit message
plbossart
force-pushed
the
fix/ssp-dai-config
branch
from
e240691 to
e04879c
Compare
Member
Author
thanks, fixed now |
kv2019i
dismissed
their stale review
I don't want my minor comment to block the PR
plbossart
force-pushed
the
fix/ssp-dai-config
branch
from
1aaa70d to
c4a3cf2
Compare
Member
Author
Collaborator
I'm good with the change @plbossart |
lyakh
requested changes
Jun 9, 2021
This was added in ABI 3.17 but never added to the kernel tree. The group_id is not currently used but this patch is required before additional changes. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
If a system suspend happens (e.g. with rtcwake in tests) while playback/capture is on-going, the hw_params step is not invoked on resume. Other dais such as HDA and ALH/SoundWire use both the .hw_params and the .prepare stage, but in the latter case use a state variable to detect the difference between underflows and resume operations due to restrictions on programming sequences. For the SSP it's fine to only do the configuration in the .prepare step. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Mirror changes done in SOF tree. The changes do not rely on BIT/GENMASK on purpose to keep the structure and flags common with the firmware tree. The DAI_CONFIG IPC is currently used in multiple ways. It is sent to the DSP firmware when enabling static or dynamic pipelines, in hw_params or prepare callbacks for Intel SSP, HDaudio and ALH, on trigger_stop and hw_free. This IPC has been abused a bit in the past, i.e. the values used for some of the DAI-specific fields are used to either allocate or free resources. Two typical examples are Intel HDaudio and SoundWire/ALH DAIs, where using a zero DMA channel number or stream tag signals to the firmware the DMA channels or tags allocated earlier can be freed. Rather than add a new IPC for 'hw_params' and 'hw_free', this patch suggests supporting a 2-bit value conveying the 'stage' information in an existing IPC structure. Only 3 possible values are used. The mapping between HW_PARAMS and HW_FREE flags and ALSA definitions is not strictly 1:1, e.g. in some cases the HW_PARAM flag might be set during the .prepare callback, while the HW_FREE might be sent during the ALSA .trigger for stop/suspend. The semantics of the flags is to reserve and start/stop all needed resources, typically hardware related such as DMAs or clocks, when the HW_PARAMS is set, while the HW_FREE flag allows the firmware to release the resources allocated. The data transfers are still controlled within the firmware through the propagation of the trigger command. The driver can then pass information that the DAI_CONFIG was invoked in e.g. a pipeline/DAI setup, hw_params or hw_free stage without having to use a special DAI-specific encoding. Unfortunately we can't remove old encodings due to backwards-compatibility requirements but for new cases, such as the SSP in follow-up patches, we can make the IPC less cryptic. This change is tagged as ABI 3.19 and is completely backwards compatible. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The DAI_CONFIG is used for both hw_params and hw_free. Use flags to specify what stage the configuration applies to. the DAI_CONFIG IPC may be sent also during the widget setup so each flag is cleared after the IPC to restore the state. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Add two clks_control bits. MCLK and/or BCLK will start during hw_params and stop during hw_free if the corresponding bit is set. While the kernel does not do anything with these bitfields, this is also tagged as part of the ABI 3.19 changes. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
plbossart
force-pushed
the
fix/ssp-dai-config
branch
from
c4a3cf2 to
a518c19
Compare
Member
Author
plbossart
requested review from
RanderWang,
bardliao,
brentlu,
cujomalainey and
kv2019i
bardliao
approved these changes
Jun 15, 2021
lyakh
approved these changes
Jun 15, 2021
brentlu
approved these changes
Jun 15, 2021
Member
Author
|
Merging, there are only minor fixes needed for the firmware side. |
|
@plbossart @bardliao @brentlu looks the merging of this introduces issue at SSP ports and the xrun issue (thesofproject/sof#3953) happen again, is this expected without the FW PR and theoretically will the issue be fixed once the FW PR get merged? |
|
there are two changes to SSP DAIs only:
Is there FW log or dmesg log captured? |
Collaborator
No, I didn't expect the PR (without the FW PR merged) will lead to the xrun issue. On the other hand, I don't expect the corresponding FW PR can fix the xrun issue. |
Member
Author
|
What issue are you referring to @keyonjie? TGL_NOCODEC has an IPC issue, not an xrun. test 4645 TGLU_RVP_NOCODEC check-playback-50rounds |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 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 PR builds on previous contributions from @bardliao (PR #2936) and @brentlu (PR ##2951)
We also build on previous work from @juimonen for the multi-ssp config where we send a DAI_CONFIG IPC in the hw_params and hw_free stages. Currently we send the same information in the two cases, so the firmware cannot take any action wrt. early clock enablement. Adding a flag is enough to provide the firmware with the information missing.
This is tagged as an ABI 3.18 change.