ASoC: rt712-sdca: add the function for version B #4912
Conversation
plbossart
left a comment
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.
|
@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. |
@plbossart Regarding RT712VA, we might check ACPI _ADR that the part id 171X exists or not. |
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. |
|
@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... |
@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
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....
|
@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. |
@plbossart As far as I know, RT712VB has a newer SDCA version than RT722. |
plbossart
left a comment
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.
|
The patches were upstreamed already. |
3 participants
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.