Skip to content

Conversation

@ranj063
Copy link
Collaborator

@ranj063 ranj063 commented Jul 7, 2021

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. Avoid this by checking
if the current state is > COMP_STATE_READY before configuring the
SSP.

Fixes #4457

@plbossart
Copy link
Member

@ranj063 I am not completely on-board with the explanations and the fix.

#define COMP_STATE_INIT		0	/**< Component being initialised */
#define COMP_STATE_READY	1	/**< Component inactive, but ready */
#define COMP_STATE_SUSPEND	2	/**< Component suspended */
#define COMP_STATE_PREPARE	3	/**< Component prepared */
#define COMP_STATE_PAUSED	4	/**< Component paused */
#define COMP_STATE_ACTIVE	5	/**< Component active */

At the end of ssp_set_config() we do

	ssp->state[DAI_DIR_PLAYBACK] = COMP_STATE_PREPARE;
	ssp->state[DAI_DIR_CAPTURE] = COMP_STATE_PREPARE;

So you should probably clarify that we don't do the configuration twice.

The other open I have is that in theory we said that this configuration can be done multiple times. I think what happened is that we do a set of register writes, and later some register updates. We should probably only do a write once per register so that in the event where the configuration is run twice, there is no change to the register settings.

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.

Suggested edit to the commit message:

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 disables DMA transfers and SSP clocks. This is not desirable at all...

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.

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.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
Copy link
Collaborator Author

ranj063 commented Jul 8, 2021

Suggested edit to the commit message:

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 disables DMA transfers and SSP clocks. This is not desirable at all...

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.

Thanks @plbossart. Updated now

@ranj063 ranj063 requested a review from plbossart July 8, 2021 15:04
@plbossart
Copy link
Member

I am going to take a calculated risk and merge this to see if this solves our daily test issue before the end of the week. If not we can always revert/improve tomorrow

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.

[BUG]IO error when simultaneous playback -capture on TGLU_RVP_NOCODEC , JSL_RVP_NOCODEC, CML_RVP_NOCODEC

2 participants