[v4,0/5] ALSA/HDA: abort probe when DMICs are detected
mbox series

Message ID 20190729155151.14055-1-pierre-louis.bossart@linux.intel.com
Headers show
Series
  • ALSA/HDA: abort probe when DMICs are detected
Related show

Message

Pierre-Louis Bossart July 29, 2019, 3:51 p.m. UTC
This is the second take on same problem of detecting when the HDaudio
legacy driver can be used and when the SST or SOF drivers are
required.

The previous attempt based on a the PCI Class information was a
resounding failure and broke audio on Linus' laptop, so we need
additional information to avoid enabling a DSP-based driver without a
good reason to do so.

This patchset suggests the use of the NHLT information which *in
theory* exposes DMIC endpoints. The legacy HDaudio driver cannot
handle DMICs and will not provide any capture capabilities. Since it's
typically the first one to probe due to the Makefile order, aborting
the probe will let the PCI subsystem look for the next driver which
hopefully will support this capability.

I tested this patch on 5 devices (SKL, KBL, APL, GLK, WHL), three
without DMICs and two with, and the detection seems to work as
planned. Additional testing by Canonical and Endless folks did not
expose any issues.

Changes since v3 (Feedback from Cezary)
Code cleanups (spaces and unnecessary inits)
Flipped test statement to return on errors and reduce indentation
Removed ACPI leak by adding missing ACPI_FREE()

Changes since v2 (Feedback from Takahi and Cezary)
Added comment in Kconfig to alert the user to the dependency on ACPI

Changes since v1 (Feedback from Takashi):
Squashed patch3 in patch2
Changed log to dbg_info
Fixed module parameter handling

Changes since RFC:
Cosmetic code improvements
Moved intel-nhlt.h to include/sound
Moved intel-nhlt.c to sound/hda
Removed SOC prefixes
Added full-support for vendor-defined geometries
Added Kconfig and kernel module parameter to opt-in

Pierre-Louis Bossart (5):
  ASoC: Intel: Skylake: move NHLT header to common directory
  ALSA: hda: move parts of NHLT code to new module
  ALSA: hda: intel-nhlt: handle NHLT VENDOR_DEFINED DMIC geometry
  ASoC: Intel: Skylake: use common NHLT module
  ALSA: hda/intel: stop probe if DMICS are detected on Skylake+
    platforms

 .../skl-nhlt.h => include/sound/intel-nhlt.h  |  51 +++++++--
 sound/hda/Kconfig                             |   5 +
 sound/hda/Makefile                            |   3 +
 sound/hda/intel-nhlt.c                        | 107 ++++++++++++++++++
 sound/pci/hda/Kconfig                         |  10 ++
 sound/pci/hda/hda_intel.c                     |  34 ++++++
 sound/soc/intel/Kconfig                       |   1 +
 sound/soc/intel/skylake/skl-nhlt.c            |  91 +--------------
 sound/soc/intel/skylake/skl-ssp-clk.c         |   1 +
 sound/soc/intel/skylake/skl-topology.c        |   1 +
 sound/soc/intel/skylake/skl.c                 |  12 +-
 sound/soc/intel/skylake/skl.h                 |   4 -
 12 files changed, 212 insertions(+), 108 deletions(-)
 rename sound/soc/intel/skylake/skl-nhlt.h => include/sound/intel-nhlt.h (65%)
 create mode 100644 sound/hda/intel-nhlt.c

Comments

Takashi Iwai July 31, 2019, 1:52 p.m. UTC | #1
On Mon, 29 Jul 2019 17:51:46 +0200,
Pierre-Louis Bossart wrote:
> 
> This is the second take on same problem of detecting when the HDaudio
> legacy driver can be used and when the SST or SOF drivers are
> required.
> 
> The previous attempt based on a the PCI Class information was a
> resounding failure and broke audio on Linus' laptop, so we need
> additional information to avoid enabling a DSP-based driver without a
> good reason to do so.
> 
> This patchset suggests the use of the NHLT information which *in
> theory* exposes DMIC endpoints. The legacy HDaudio driver cannot
> handle DMICs and will not provide any capture capabilities. Since it's
> typically the first one to probe due to the Makefile order, aborting
> the probe will let the PCI subsystem look for the next driver which
> hopefully will support this capability.
> 
> I tested this patch on 5 devices (SKL, KBL, APL, GLK, WHL), three
> without DMICs and two with, and the detection seems to work as
> planned. Additional testing by Canonical and Endless folks did not
> expose any issues.
> 
> Changes since v3 (Feedback from Cezary)
> Code cleanups (spaces and unnecessary inits)
> Flipped test statement to return on errors and reduce indentation
> Removed ACPI leak by adding missing ACPI_FREE()
> 
> Changes since v2 (Feedback from Takahi and Cezary)
> Added comment in Kconfig to alert the user to the dependency on ACPI
> 
> Changes since v1 (Feedback from Takashi):
> Squashed patch3 in patch2
> Changed log to dbg_info
> Fixed module parameter handling
> 
> Changes since RFC:
> Cosmetic code improvements
> Moved intel-nhlt.h to include/sound
> Moved intel-nhlt.c to sound/hda
> Removed SOC prefixes
> Added full-support for vendor-defined geometries
> Added Kconfig and kernel module parameter to opt-in
> 
> Pierre-Louis Bossart (5):
>   ASoC: Intel: Skylake: move NHLT header to common directory
>   ALSA: hda: move parts of NHLT code to new module
>   ALSA: hda: intel-nhlt: handle NHLT VENDOR_DEFINED DMIC geometry
>   ASoC: Intel: Skylake: use common NHLT module
>   ALSA: hda/intel: stop probe if DMICS are detected on Skylake+
>     platforms

Applied all five patches now.  Thanks.

Mark, the patches are found in topic/hda-dmic branch of sound git
tree, which are based on 5.3-rc1.  If ASoC tree needs these changes,
feel free to pull the branch into yours.


Takashi
Pierre-Louis Bossart July 31, 2019, 3:33 p.m. UTC | #2
On 7/31/19 8:52 AM, Takashi Iwai wrote:
> On Mon, 29 Jul 2019 17:51:46 +0200,
> Pierre-Louis Bossart wrote:
>>
>> This is the second take on same problem of detecting when the HDaudio
>> legacy driver can be used and when the SST or SOF drivers are
>> required.
>>
>> The previous attempt based on a the PCI Class information was a
>> resounding failure and broke audio on Linus' laptop, so we need
>> additional information to avoid enabling a DSP-based driver without a
>> good reason to do so.
>>
>> This patchset suggests the use of the NHLT information which *in
>> theory* exposes DMIC endpoints. The legacy HDaudio driver cannot
>> handle DMICs and will not provide any capture capabilities. Since it's
>> typically the first one to probe due to the Makefile order, aborting
>> the probe will let the PCI subsystem look for the next driver which
>> hopefully will support this capability.
>>
>> I tested this patch on 5 devices (SKL, KBL, APL, GLK, WHL), three
>> without DMICs and two with, and the detection seems to work as
>> planned. Additional testing by Canonical and Endless folks did not
>> expose any issues.
>>
>> Changes since v3 (Feedback from Cezary)
>> Code cleanups (spaces and unnecessary inits)
>> Flipped test statement to return on errors and reduce indentation
>> Removed ACPI leak by adding missing ACPI_FREE()
>>
>> Changes since v2 (Feedback from Takahi and Cezary)
>> Added comment in Kconfig to alert the user to the dependency on ACPI
>>
>> Changes since v1 (Feedback from Takashi):
>> Squashed patch3 in patch2
>> Changed log to dbg_info
>> Fixed module parameter handling
>>
>> Changes since RFC:
>> Cosmetic code improvements
>> Moved intel-nhlt.h to include/sound
>> Moved intel-nhlt.c to sound/hda
>> Removed SOC prefixes
>> Added full-support for vendor-defined geometries
>> Added Kconfig and kernel module parameter to opt-in
>>
>> Pierre-Louis Bossart (5):
>>    ASoC: Intel: Skylake: move NHLT header to common directory
>>    ALSA: hda: move parts of NHLT code to new module
>>    ALSA: hda: intel-nhlt: handle NHLT VENDOR_DEFINED DMIC geometry
>>    ASoC: Intel: Skylake: use common NHLT module
>>    ALSA: hda/intel: stop probe if DMICS are detected on Skylake+
>>      platforms
> 
> Applied all five patches now.  Thanks.
> 
> Mark, the patches are found in topic/hda-dmic branch of sound git
> tree, which are based on 5.3-rc1.  If ASoC tree needs these changes,
> feel free to pull the branch into yours.

Thank you Takashi.

I will need to use these patches for SOF to dynamically change the 
topology file name depending on how many DMICs are detected for HDaudio 
platforms, if at all. The current solution requires the user to rename 
files and it's just not scalable.