Message ID | 20220308192610.392950-1-pierre-louis.bossart@linux.intel.com |
---|---|
Headers | show |
Series | ALSA/ASoC/SOF/Intel: improve support for ES8336-based platforms | expand |
On Tue, 08 Mar 2022 20:25:54 +0100, Pierre-Louis Bossart wrote: > > The NHLT information can be used to figure out which SSPs are enabled > in a platform. > > The 'SSP' link type is too broad for machine drivers, since it can > cover the Bluetooth sideband and the analog audio codec connections, > so this helper exposes a parameter to filter with the device > type (DEVICE_I2S refers to analog audio codec in NHLT parlance). > > The helper returns a mask, since more than one SSP may be used for > analog audio, e.g. the NHLT spec describes the use of SSP0 for > amplifiers and SSP1 for headset codec. Note that if more than one bit > is set, it's impossible to determine which SSP is connected to what > external component. Additional platform-specific information based on > e.g. DMI quirks would still be required in the machine driver to > configure the relevant dailinks. > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com> > Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com> Acked-by: Takashi Iwai <tiwai@suse.de> thanks, Takashi
On 2022-03-08 8:25 PM, Pierre-Louis Bossart wrote: > The NHLT information can be used to figure out which SSPs are enabled > in a platform. > > The 'SSP' link type is too broad for machine drivers, since it can > cover the Bluetooth sideband and the analog audio codec connections, > so this helper exposes a parameter to filter with the device > type (DEVICE_I2S refers to analog audio codec in NHLT parlance). > > The helper returns a mask, since more than one SSP may be used for > analog audio, e.g. the NHLT spec describes the use of SSP0 for > amplifiers and SSP1 for headset codec. Note that if more than one bit > is set, it's impossible to determine which SSP is connected to what > external component. Additional platform-specific information based on > e.g. DMI quirks would still be required in the machine driver to > configure the relevant dailinks. ... > diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c > index 128476aa7c61..4063da378283 100644 > --- a/sound/hda/intel-nhlt.c > +++ b/sound/hda/intel-nhlt.c > @@ -130,6 +130,28 @@ bool intel_nhlt_has_endpoint_type(struct nhlt_acpi_table *nhlt, u8 link_type) > } > EXPORT_SYMBOL(intel_nhlt_has_endpoint_type); > > +int intel_nhlt_ssp_endpoint_mask(struct nhlt_acpi_table *nhlt, u8 device_type) > +{ > + struct nhlt_endpoint *epnt; > + int ssp_mask = 0; > + int i; > + > + if (!nhlt || (device_type != NHLT_DEVICE_BT && device_type != NHLT_DEVICE_I2S)) The '!nhlt' safety is superfluous in my opinion. Kernel core API e.g.: device one assumes caller is sane in basically all cases. > + return 0; > + > + epnt = (struct nhlt_endpoint *)nhlt->desc; > + for (i = 0; i < nhlt->endpoint_count; i++) { > + if (epnt->linktype == NHLT_LINK_SSP && epnt->device_type == device_type) { > + /* for SSP the virtual bus id is the SSP port */ > + ssp_mask |= BIT(epnt->virtual_bus_id); > + } > + epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length); > + } > + > + return ssp_mask; > +} > +EXPORT_SYMBOL(intel_nhlt_ssp_endpoint_mask); Since this is a *public* API - not direct part of any driver, really - providing kernel-doc is recommended. Regards, Czarek
>> diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c >> index 128476aa7c61..4063da378283 100644 >> --- a/sound/hda/intel-nhlt.c >> +++ b/sound/hda/intel-nhlt.c >> @@ -130,6 +130,28 @@ bool intel_nhlt_has_endpoint_type(struct >> nhlt_acpi_table *nhlt, u8 link_type) >> } >> EXPORT_SYMBOL(intel_nhlt_has_endpoint_type); >> +int intel_nhlt_ssp_endpoint_mask(struct nhlt_acpi_table *nhlt, u8 >> device_type) >> +{ >> + struct nhlt_endpoint *epnt; >> + int ssp_mask = 0; >> + int i; >> + >> + if (!nhlt || (device_type != NHLT_DEVICE_BT && device_type != >> NHLT_DEVICE_I2S)) > > The '!nhlt' safety is superfluous in my opinion. Kernel core API e.g.: > device one assumes caller is sane in basically all cases. Agree. I will remove this test in a follow-up optimization patch. the same pattern is used for existing dmic stuff so it's better to remove it in one shot. >> + return 0; >> + >> + epnt = (struct nhlt_endpoint *)nhlt->desc; >> + for (i = 0; i < nhlt->endpoint_count; i++) { >> + if (epnt->linktype == NHLT_LINK_SSP && epnt->device_type == >> device_type) { >> + /* for SSP the virtual bus id is the SSP port */ >> + ssp_mask |= BIT(epnt->virtual_bus_id); >> + } >> + epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length); >> + } >> + >> + return ssp_mask; >> +} >> +EXPORT_SYMBOL(intel_nhlt_ssp_endpoint_mask); > > Since this is a *public* API - not direct part of any driver, really - > providing kernel-doc is recommended. there isn't a single line of kernel-doc for the entire NHLT stuff. and ahem, that includes recent additions from your team ;-) bool intel_nhlt_has_endpoint_type(struct nhlt_acpi_table *nhlt, u8 link_type); So agree, but let's do this in a follow-up patchset. the goal of this patchset is to help community users that don't see an audio card created, not to make NHLT support super shiny.
On Tue, 8 Mar 2022 13:25:50 -0600, Pierre-Louis Bossart wrote: > This patchset adds a number of improvements for ES8336-based Intel > platforms, which are not well supported at all in Linux. Since > Christmas 2021, we've seen dozens of reports of broken audio [1]. > > The fundamental problem is that those platforms were built for Windows > but using an I2S codec - instead of the HDaudio traditional > solution. As a result, we are missing all the usual information needed > to configure the audio card (which I2S, what configuration, DMICs or > not, etc). The situation is similar to Baytrail with all possible > permutations enabled. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [01/20] ASoC: soc-acpi: fix kernel-doc descriptor commit: 1174442b82b6cf13328a5f9574fac3655c58ae06 [02/20] ASoC: soc-acpi: add information on I2S/TDM link mask commit: 679aa83a0fb70dcbf9e97cbdfd573e6fc8bf9b1a [03/20] ASoC: SOF: Intel: hda: retrieve DMIC number for I2S boards commit: 92c1b7c0f780f0084f7b114be316ae4e182676e5 [04/20] ALSA: intel-nhlt: add helper to detect SSP link mask commit: 0c470db0399e17310ed2ba54dd1c25cfa16ce0d3 [05/20] ASoC: SOF: Intel: hda: report SSP link mask to machine driver commit: bd015f633b05a3d4f88a3d7099746b2819a523f5 [06/20] ASoC: Intel: soc-acpi: quirk topology filename dynamically commit: 4694b8382d6b79bcf95995757419d279a3ab375b [07/20] ALSA: intel-dsp-config: add more ACPI HIDs for ES83x6 devices commit: de24d97fb845ffd2229811ee256438e42b5a8d12 [08/20] ASoC: Intel: soc-acpi: add more ACPI HIDs for ES83x6 devices commit: 1cedb6eabf0f2dd8285d3bb0ce1abd2369529084 [09/20] ALSA: intel-dspconfig: add ES8336 support for CNL commit: cded07a2dccd5493696a3adce175f01e413423c6 [10/20] ASoC: Intel: soc-acpi: add ESSX8336 support on Cannon Lake machines commit: b3d6a07236ebf6e0111adb957d69bebccf4f0a19 [11/20] ASoC: Intel: sof_es8336: make gpio optional commit: 5a6cfba5553b4f315b3d12b423e56a7fb9e8e0be [12/20] ASoC: Intel: sof_es8336: get codec device with ACPI instead of bus search commit: 42302b205f03c74c0226bbc79dca48448dd11d48 [13/20] ASoC: Intel: Revert "ASoC: Intel: sof_es8336: add quirk for Huawei D15 2021" commit: 1b5283483a782f6560999d8d5965b1874d104812 [14/20] ASoC: Intel: sof_es8336: use NHLT information to set dmic and SSP commit: 651c304df7f6e3fbb4779527efa3eb128ef91329 [15/20] ASoC: Intel: sof_es8336: log all quirks commit: 9c818d849192491a8799b1cb14ca0f7aead4fb09 [16/20] ASoC: Intel: sof_es8336: move comment to the right place commit: d94c11a9b0e8620d7cf0e6d8e60d685cdb24c475 [17/20] ASoC: Intel: sof_es8336: add support for JD inverted quirk commit: 8e5db49182415b8bfce3b5843fc87e49c83c02aa [18/20] ASoC: Intel: sof_es8336: extend machine driver to support ES8326 codec commit: 70b519e5cade92bddf837bd3941f905b44232b05 [19/20] ASoC: Intel: sof_es8336: add cfg-dmics component for UCM support commit: 6e13567d2fdf54ce00212cac7ecd5418648da749 [20/20] ASoC: Intel: bytcht_es8316: move comment to the right place commit: fe0596a006081bc963874d4f3d38cd0b1b5e46d4 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark