Skip to content

ssp: bclk/mclk control is ignored#4650

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.

Sign up for GitHub

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

Merged
lgirdwood merged 2 commits into from
Sep 6, 2021
Merged

ssp: bclk/mclk control is ignored #4650

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8eec304
ssp: bclk/mclk control is ignored
brentlu Aug 3, 2021
282a4d7
ssp: mclk/bclk turned off unexpectedly
brentlu Sep 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/drivers/intel/ssp/ssp.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -273,7 +273,7 @@ static int ssp_set_config_tplg(struct dai *dai, struct ipc_config_dai *common_co
if (ssp->state[DAI_DIR_PLAYBACK] > COMP_STATE_READY ||
ssp->state[DAI_DIR_CAPTURE] > COMP_STATE_READY) {

@keyonjiekeyonjieAug 19, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe change this to "== COMP_STATE_ACTIVE"? in case we want to support multiple times of re-configuration before the SSP is started or during 'PAUSED', or reset configuration after 'stopped'.

@brentlubrentluAug 19, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Ranjani changed it from '==' to '>' to avoid some ssp issue.

commit fc73578
Author: Ranjani Sridharan ranjani.sridharan@linux.intel.com
Date: Wed Jul 7 15:56:20 2021 -0700

drivers: Intel: SSP: ignore config when SSP is already configured
When two streams are started one after the other, 2 DAI_CONFIG
IPC's will be sent before the START trigger. This ends up
configuring the SSP when the first one has already just
configured it and ends up with xruns.
The root cause is that the ssp_set_config() function configures
a set of variables, writes those variables into SSP registers,
and later on does a read-modify-write operation on the TSRE,
RSRE and SSE bits.
If the ssp_set_config is executed more than once, this will
temporarily clear all three bitfields, and temporarily disable
DMA transfers and SSP clocks. Avoid this by checking if the
current state is > COMP_STATE_READY before configuring the
SSP, so that the ssp_set_config() function only modifies SSP
registers once.

@lgirdwoodlgirdwoodAug 28, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like this logic need look at since the alsa-bat failure is still present. @brentlu may need some extra logic e.g.

clk:
switch (config->flags&SOF_DAI_CONFIG_FLAGS_MASK) {
caseSOF_DAI_CONFIG_FLAGS_HW_PARAMS:
if (ssp->params.clks_control&SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES) {
ret=ssp_mclk_prepare_enable(dai);
if (ret<0)
goto out;
ssp->clk_active |= SSP_CLK_MCLK_ES_REQ; <<<<<<<<<<<< WecouldprobablycheckthisfirstandnotsetM/Nifthisstateisalready true.
dai_info(dai, "ssp_set_config(): hw_params stage: enabled MCLK clocks for SSP%d...",
dai->index);
}
if (ssp->params.clks_control&SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES) {
boolenable_sse= false;
if (!(ssp->clk_active&SSP_CLK_BCLK_ACTIVE))
enable_sse= true;
ret=ssp_bclk_prepare_enable(dai);
if (ret<0)
goto out;
ssp->clk_active |= SSP_CLK_BCLK_ES_REQ; <<<<<<<<<<<<<dittoasaboveif (enable_sse) {
/* enable TRSE/RSRE before SSE */ssp_update_bits(dai, SSCR1,
SSCR1_TSRE | SSCR1_RSRE,
SSCR1_TSRE | SSCR1_RSRE);
/* enable port */ssp_update_bits(dai, SSCR0, SSCR0_SSE, SSCR0_SSE);
dai_info(dai, "ssp_set_config(): SSE set for SSP%d", dai->index);
}
dai_info(dai, "ssp_set_config(): hw_params stage: enabled BCLK clocks for SSP%d...",
dai->index);
}
break;

dai_info(dai, "ssp_set_config(): Already configured. Ignore config");
goto out;
goto clk;
Comment thread
plbossart marked this conversation as resolved.
}

dai_info(dai, "ssp_set_config(), config->format = 0x%4x",
Expand DownExpand Up@@ -713,6 +713,7 @@ static int ssp_set_config_tplg(struct dai *dai, struct ipc_config_dai *common_co
ssp->state[DAI_DIR_PLAYBACK] = COMP_STATE_PREPARE;
ssp->state[DAI_DIR_CAPTURE] = COMP_STATE_PREPARE;

clk:
switch (config->flags & SOF_DAI_CONFIG_FLAGS_MASK) {
case SOF_DAI_CONFIG_FLAGS_HW_PARAMS:
if (ssp->params.clks_control & SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES) {
Expand DownExpand Up@@ -756,6 +757,14 @@ static int ssp_set_config_tplg(struct dai *dai, struct ipc_config_dai *common_co
}
break;
case SOF_DAI_CONFIG_FLAGS_HW_FREE:
/* disable SSP port if no users */
if (ssp->state[SOF_IPC_STREAM_CAPTURE] != COMP_STATE_PREPARE ||
ssp->state[SOF_IPC_STREAM_PLAYBACK] != COMP_STATE_PREPARE) {
dai_info(dai, "ssp_set_config(): hw_free stage: ignore since there is still user",
dai->index);

@marc-hbmarc-hbSep 7, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

dai_info() has one parameter but the format string has no specifier. Found by chance by the Zephyr build which has a different logging framework. Fix submitted in #4727

break;
}

if (ssp->params.clks_control & SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES) {
dai_info(dai, "ssp_set_config(): hw_free stage: releasing BCLK clocks for SSP%d...",
dai->index);
Expand Down