Uh oh!
There was an error while loading. Please reload this page.
ASoC: rt712-sdca: add the function for version B - #4912
Conversation
plbossart
left a comment
There was a problem hiding this comment.
Thanks for sharing @shumingfan, I have rather big concerns on the topology/machine driver support if the VA/VB information is not visible at the ACPI level.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
plbossart
commented
Apr 10, 2024
@shumingfan is the function listed in the DSDT? IIRC all the functions should be a "Device" under the codec device, so in theory we might be able to add a quirk to go read the function and decide which system it is. IOW, we would first start from the ACPI _ADR, but add a quirk to go check if there is a subdevice representing the DMIC function. And in that case we would know for sure what the codec is and select topology. |
shumingfan
commented
Apr 11, 2024
@plbossart Regarding RT712VA, we might check ACPI _ADR that the part id 171X exists or not. |
plbossart
commented
Apr 11, 2024
Yes that's a possibility.
It's probably better to detect the function type, we should see a SmartMic function. I'll start cooking something. |
plbossart
commented
Apr 16, 2024
@shumingfan see #4930 for an example on how we can parse the DSDT information and figure out if there are any SDCA functions in a device. we could use this to check if the device supports the SmartMic function. Of course that assumes ACPI information is valid, which is already not true on the first device I tried... |
shumingfan
commented
Apr 17, 2024
@plbossart Thanks. The PR#4930 looks good to me. The function type of DMIC of RT713 might be a simple mic function. |
The SimpleMic topology is not yet in the SDCA spec, it's still being added for 1.1. So I don't know how if it's really what it is, and how one would infer the topology mask. That's my main fear with this ACPI-based solution, it's a mix of older devices with an older unpublished SDCA/DisCo spec, not sure how we can deal with all the variants. It might be just easier to only support 1.0+... |
This patch doesn't have any change of functionality. Signed-off-by: Shuming Fan <shumingf@realtek.com>
The version B will support the multi-lane function and integrate the DMIC function in one SoundWire interface. Due to some registers having different default values between version A and B, this patch also removes the redundant default registers to avoid confusion. Signed-off-by: Shuming Fan <shumingf@realtek.com>
plbossart
left a comment
There was a problem hiding this comment.
@shumingfan I really don't know how we can support the VB at the machine driver level, this is really really problematic. The opens raised in the initial review have not been solved....
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
plbossart
commented
May 14, 2024
@shumingfan for my education, what's the difference between RT712 VB and RT722? They both seem to support UAJ, SmartAmp and SmartMic functions, along with multi-lane. |
shumingfan
commented
May 15, 2024
@plbossart As far as I know, RT712VB has a newer SDCA version than RT722. |
plbossart
left a comment
There was a problem hiding this comment.
@shumingfan I suggest rebasing on #5010 to make sure ACPI and hardware are aligned.
| if (rt712->version_id == RT712_VA) | ||
| rt712_sdca_va_io_init(rt712); | ||
| else { |
There was a problem hiding this comment.
I would suggest adding a test here
if (!sdca_device_quirk_match(slave, SDCA_QUIRKS_RT712_VB)) {
dev_err(&slave->dev, "%s: RT712 VB detected with no SmartMic Function in ACPI, invalid configuration\n", __func__);
return-EINVAL;
}that way we will not have a case where we trip between hardware and ACPI config.
shumingfan
commented
Jun 28, 2024
The patches were upstreamed already. |


The version B will support the multi-lane function and integrate the DMIC function in one SoundWire interface.
Due to some registers having different default values between version A and B, this patch also removes the redundant default registers to avoid confusion.