Skip to content
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.

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

[RFT]dai: set config only if the config dai_index matches comp dai_index #158

Merged
merged 1 commit into from Jul 30, 2018

Conversation

ranj063
Copy link
Collaborator

@ranj063 ranj063 commented Jul 26, 2018

Set config only for the comp who's dai_index matches the dai_index
in the ipc config.

Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com

@ranj063
Copy link
Collaborator Author

ranj063 commented Jul 26, 2018

@singalsu could you please test this to see if it fixes your issue?

@ranj063 ranj063 requested a review from singalsu July 26, 2018 18:39
src/audio/dai.c Outdated

/* return if config is not for this comp */
if (dai->dai_index != config->dai_index)
return 0;
Copy link
Member

Choose a reason for hiding this comment

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

Add error trace and return -EINVAL?
This looks like something we'd need to do for all DAIs btw.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@plbossart this is not an error. Basically when the dai_config ipc arrives, the ipc handler calls the dai_config() callback for all comps. So we should just ignore the request if the dai_config is not address for it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So either we ignore the request here or maybe fix the ipc handler to send the config only to the relevant comps. I think the latter makes more sense. I will update the PR.

@ranj063
Copy link
Collaborator Author

ranj063 commented Jul 26, 2018

@plbossart I have update this pull request to move the dai index check to the ipc handler and tested it on the up squared board. But we should wait until after @singalsu can also confirm.

@singalsu
Copy link
Collaborator

singalsu commented Jul 27, 2018

This patch reduced the number of dai_config() calls for config->type SOF_DAI_INTEL_DMIC from 13 to 3. I use sof-apl-nocodec topology. I wonder why two extra times?

It didn't help me to get DMA configured to 32 bits so there must be some other issue in addition.

Copy link
Collaborator

@singalsu singalsu left a comment

Choose a reason for hiding this comment

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

This is improvement but maybe something is still missing due to seeing still more than one successive dai_config() for one DMIC DAI.

Set config only for the comp who's dai_index matches the dai_index
in the ipc config.

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

I tested the updated patch and now there's single dai_config() happening for DMIC. I recommend to merge this patch. Thanks Ranjani!

@lgirdwood lgirdwood merged commit 0a1587b into thesofproject:master Jul 30, 2018
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.

None yet

4 participants