ASoC: SOF: Intel: Add topology overwrite for Felwinter #3271
Conversation
|
sof PR: thesofproject/sof#4979 |
sound/soc/sof/sof-pci-dev.c
Outdated
| { | ||
| .callback = sof_tplg_cb, | ||
| .matches = { | ||
| DMI_MATCH(DMI_PRODUCT_NAME, "Felwinter"), |
There was a problem hiding this comment.
Humm, I am not sure I get this. We discussed with Google that they would use the DMI_OEM_STRING.
It'd be a bit odd to use a PRODUCT_NAME only
@cujomalainey @bzhg need your help here.
There was a problem hiding this comment.
Yeap I saw some dmi quirks are using oem string. Is there coding convention or naming rule?
There was a problem hiding this comment.
Even then some OEMs are not happy that we are using OEM string. @plbossart Would it be worth allowing the AP FW to express a custom topology file name?
There was a problem hiding this comment.
well it's a bit late @cujomalainey since we've already blown the DMI_OEM_STRING to be an identifier that is translated to a topology file by the kernel. If you change the semantic of that string to be a topology file that would be problematic...
I am not sure what other strings are left for the platform firmware to populate?
There was a problem hiding this comment.
I was thinking more like passing it through platform data if possible to the SOF driver. That being said I am not an expert in coreboot, so I have no idea if that is possible
There was a problem hiding this comment.
Documentation though, yes i agree that would be needed
There was a problem hiding this comment.
I am not sure how this would be abused? I thought coreboot was a trusted source of information
Abused by others...
There was a problem hiding this comment.
Can you explain a little more how you think this is open to abuse? If somebody modifies their firmware to produce the wrong topology (and of course, if you're modifying your firmware, all bets are off if you don't know what you're doing), then of course you can't expect things to still work.
Please allow me to quote from the ACPI spec 6.4 here from section 9.1.1 about _DSM:
Successive revisions of Function Arguments must be backward compatible with earlier revisions. New UUIDs may
also be created by OEMs and IHVs for custom devices and other interface or device governing bodies (e.g. the PCI
SIG), as long as the UUID is different from other published UUIDs. Only the issuer of a UUID can authorize a new
Function Index, Revision ID or Function Argument for that UUID.
Sometimes _DSM is the best way to inform the kernel of something before there is a standard way to do things in ACPI.
I can think of a few different ways to solve this off the top of my head:
- Create an ACPI hierarchy of devices (that don't exist in the spec yet) and use properties on these to determine what the audio topology looks like
- Use a topology string that the kernel can parse in a standard format that can be used across generations
This is to avoid having to reinvent the wheel everytime when the ACPI spec is slower to move than real hardware
There was a problem hiding this comment.
If somebody modifies their firmware to point to a non-existent topology file on the file system, then we will get a bug report that 'audio is broken' and we will have to add a quirk in the kernel. Been there, done that.
It's not about the mechanism to retrieve the information, it's about making sure that the bar is high-enough for compliance to a minimal documentation.
There was a problem hiding this comment.
Isn't this an example of a device property being pulled for a PCI device? We could add a string field for the topology name
Seems the code is for pci root port, it's enumerated from acpi but the pci device should be enumerated by pci itself. It's my understanding thought I'm not pci expert.
|
@brentlu rebase needed, thanks! |
The Felwinter uses four max98360a amplifiers on corresponding CH0~CH3. There are four amps on the board connecting to headphone to SSP0 port, amp to SSP1,and the DAI format would be DSP_A,8-slots, 32 bit slot-width. CH0: L(Woofer), CH1:R(Woofer), CH2:L(Tweeter), CH3:R(Tweeter) Signed-off-by: Ajye Huang <ajye_huang@compal.corp-partner.google.com> Signed-off-by: Brent Lu <brent.lu@intel.com>
|
rebase and update the OEM string discussed with ODM and Google. |
|
Next TSC we need to discuss how to properly expose this information because this is duct tape at best :/ |
|
Anyone could merge this PR? thanks. |
thesofproject/linux#3271 Felwinter will use the OEM string for SOF tplg loading. Update the name that match to the kernel driver. BUG=b:210061842 TEST=dmidecode can show AUDIO_AMP-MAX98360_ALC5682VS_I2S_2WAY. Signed-off-by: Eric Lai <ericr_lai@compal.corp-partner.google.com> Change-Id: Ib6114d047762ba26071c9cdc6c43d80f933c1eb9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61070 Reviewed-by: Subrata Banik <subratabanik@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The Felwinter uses four max98360a amplifiers on corresponding CH0~CH3.
There are four amps on the board connecting to headphone to SSP0 port,
amp to SSP1,and the DAI format would be DSP_A,8-slots, 32 bit slot-width.
CH0: L(Woofer), CH1:R(Woofer), CH2:L(Tweeter), CH3:R(Tweeter)
Signed-off-by: Ajye Huang ajye_huang@compal.corp-partner.google.com
Signed-off-by: Brent Lu brent.lu@intel.com