-
Notifications
You must be signed in to change notification settings - Fork 130
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
Remove CODEC DMIC for specific SKUs #5186
Conversation
.group_position = 0, | ||
.group_id = 0, | ||
}, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alternatively reshuffle the endpoint order and put DMICs last, then play with ARRAY_SIZE - 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or use a dai quick. Like 47bae32
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I did kinda want to use a quirk, but it was a little awkward, since the polarity of the quirks is such that they are saying include this DAI. Which means we would need to add the quirk to every laptop except this one and given generally I expect the vast majority of SKUs to use the cs42l43 DMICs that feels suboptimal. I will have another look today see if I can work out a better way to invert the quirk polarity.
bdd32a5
c226595
to
bdd32a5
Compare
Added an update to switch to adding an exclusion quirk mechanism, based on Bard's suggestion. I think I prefer this approach. I need to actually add the quirk, but will do so once I have confirmed with the customer that the SSID can be pushed out. |
if (dai_info->quirk && !(dai_info->quirk & ctx->mc_quirk)) | ||
continue; | ||
if (dai_info->quirk && | ||
!(dai_info->quirk_exclude ^ (dai_info->quirk & ctx->mc_quirk))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use !dai_info->quirk_exclude * (dai_info->quirk & ctx->mc_quirk))
? It seems explain the quirk_exclude
flag more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure that expression results in the correct results, you want the expression to be true for any DAI that should be dropped for which there are two situations:
A DAI that has an include quirk, which is not specified in the machine quirks:
dai_info->quirk_exclude = 0, dai_info->quirk = 1, ctx->mc_quirk = 0
For this your expression would be: !0 * (1 & 0) = 0
A DAI that has an exclude quirk, that is specified in the machine quirks:
dai_info->quirk_exclude = 1, dai_info->quirk = 1, ctx->mc_quirk = 1
For this your expression would be: !1 * (1 & 1) = 0
I think the XOR is probably the best way to convey the intention. One could potentially be more explicit by expanding it out to something like:
if (dai_info->quirk &&
((!dai_info->quirk_exclude && !(dai_info->quirk & ctx->mc_quirk)) ||
(dai_info->quirk_exclude && (dai_info->quirk & ctx->mc_quirk)))) {
But I think I prefer the shortened version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes, thanks for the explanation. I thought we just need to revert the current result when dai_info->quirk_exclude = 1.
include/sound/soc_sdw_utils.h
Outdated
@@ -28,6 +28,7 @@ | |||
* - SOC_SDW_CODEC_SPKR | SOF_SIDECAR_AMPS - Not currently supported | |||
*/ | |||
#define SOC_SDW_SIDECAR_AMPS BIT(16) | |||
#define SOC_SDW_CODEC_NO_MIC BIT(17) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just SOC_SDW_CODEC_MIC
without NO
? We have quirk_exclude
to represent default value with or without the device. So we don't need to represent it in the quirk name, and save some quirk bits. I.e. Other codecs can use the SOC_SDW_CODEC_MIC
quirk to don't use codec mic by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, can we split it to 2 commits? One to add the quirk_exclude
flag and the other to make the cs42l43 MIC optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that makes sense will do that.
bdd32a5
to
e37bf7f
Compare
New version with the pieces split out, the define changes and the actual quirk added, now I have permission. |
e37bf7f
to
7330370
Compare
@@ -487,6 +487,8 @@ struct asoc_sdw_codec_info codec_info_list[] = { | |||
.rtd_init = asoc_sdw_cs42l43_dmic_rtd_init, | |||
.widgets = generic_dmic_widgets, | |||
.num_widgets = ARRAY_SIZE(generic_dmic_widgets), | |||
.quirk = SOC_SDW_CODEC_MIC, | |||
.quirk_exclude = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo in commit message: micrphone
7330370
to
94628a3
Compare
2363b7e
94628a3
to
2363b7e
Compare
The system contains a mechanism for certain DAI links to be included based on a quirk. Add support for certain DAI links to excluded based on a quirk, this is useful in situations where the vast majority of SKUs utilise a feature so it is easier to quirk on those that don't. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
To support some systems using host microphones add a quirk to allow the cs42l43 microphone DAI link to be ignored. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Add quirk to inform the machine driver to not bind in the cs42l43 microphone DAI link. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Add some more sidecar amplifier quirks. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
2363b7e
to
94771a2
Compare
Sorry Bard timing was bad on that with respect to your approval there, was just fxing up the last checkpatch thing (one of those no commit message ones) and adding a new patch to add a couple of other quirks since we might as well get them all merged together. |
Are we ok to get this one merged? The customer has been chasing us on it. |
Some SKUs are using host DMICs rather than the cs42l43 DMICs, update the endpoint structure for these SKUs to remove the DMIC endpoint. Longer term a more automated solution will be sensible, however for now this is limited to SKUs with a particular bus layout so this fix is easy to deploy whilst we work on a more complete solution.