| Message ID | aCXswg1gr6cufyzp@ddadap-lakeline.nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ALSA: hda - Add new driver for HDA controllers listed via ACPI | expand |
On Thu, 15 May 2025 15:31:46 +0200, Daniel Dadap wrote: > > Some systems expose HD-Audio controllers via objects in the ACPI tables > which encapsulate the controller's interrupt and the base address for the > HDA registers in an ACPI _CRS object, for example, as listed in this ACPI > table dump excerpt: > > Device (HDA0) > { > Name (_HID, "NVDA2014") // _HID: Hardware ID > ... > Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings > { > Memory32Fixed (ReadWrite, > 0x36078000, // Address Base > 0x00008000, // Address Length > ) > Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, ) > { > 0x0000021E, > } > }) > } > > Add support for HDA controllers discovered through ACPI, including support > for some platforms which expose such HDA controllers on NVIDIA SoCs. This > is done with a new driver which uses existing infrastructure for extracting > resource information from _CRS objects and plumbs the parsed resource > information through to the existing HDA infrastructure to enable HD-Audio > functionality on such devices. > > Although this driver is in the sound/pci/hda/ directory, it targets devices > which are not actually enumerated on the PCI bus. This is because it depends > upon the Intel "Azalia" infrastructure which has traditionally been used for > PCI-based devices. > > Signed-off-by: Daniel Dadap <ddadap@nvidia.com> Thanks for the patch. The code looks fine. Just a nitpicking: > +static int __maybe_unused hda_acpi_suspend(struct device *dev) .... > +static int __maybe_unused hda_acpi_resume(struct device *dev) The __maybe_unused is superfluous when you set up SYSTEM_SLEEP_PM_OPS() macro instead in the below: > +static const struct dev_pm_ops hda_acpi_pm = { > + SET_SYSTEM_SLEEP_PM_OPS(hda_acpi_suspend, hda_acpi_resume) thanks, Takashi
On Thu, May 15, 2025 at 04:45:52PM +0200, Takashi Iwai wrote: > On Thu, 15 May 2025 15:31:46 +0200, > Daniel Dadap wrote: > > > > Some systems expose HD-Audio controllers via objects in the ACPI tables > > which encapsulate the controller's interrupt and the base address for the > > HDA registers in an ACPI _CRS object, for example, as listed in this ACPI > > table dump excerpt: > > > > Device (HDA0) > > { > > Name (_HID, "NVDA2014") // _HID: Hardware ID > > ... > > Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings > > { > > Memory32Fixed (ReadWrite, > > 0x36078000, // Address Base > > 0x00008000, // Address Length > > ) > > Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, ) > > { > > 0x0000021E, > > } > > }) > > } > > > > Add support for HDA controllers discovered through ACPI, including support > > for some platforms which expose such HDA controllers on NVIDIA SoCs. This > > is done with a new driver which uses existing infrastructure for extracting > > resource information from _CRS objects and plumbs the parsed resource > > information through to the existing HDA infrastructure to enable HD-Audio > > functionality on such devices. > > > > Although this driver is in the sound/pci/hda/ directory, it targets devices > > which are not actually enumerated on the PCI bus. This is because it depends > > upon the Intel "Azalia" infrastructure which has traditionally been used for > > PCI-based devices. > > > > Signed-off-by: Daniel Dadap <ddadap@nvidia.com> > > Thanks for the patch. > The code looks fine. Just a nitpicking: > > > +static int __maybe_unused hda_acpi_suspend(struct device *dev) > .... > > +static int __maybe_unused hda_acpi_resume(struct device *dev) > > The __maybe_unused is superfluous when you set up > SYSTEM_SLEEP_PM_OPS() macro instead in the below: > > > +static const struct dev_pm_ops hda_acpi_pm = { > > + SET_SYSTEM_SLEEP_PM_OPS(hda_acpi_suspend, hda_acpi_resume) > Thanks, I'll send an updated patch. The hda_tegra driver has these as well so I presume it can be cleaned up from that driver as well? > > thanks, > > Takashi
On Thu, May 15, 2025 at 04:54:25PM +0200, Takashi Iwai wrote: > On Thu, 15 May 2025 16:45:52 +0200, > Takashi Iwai wrote: > > > > On Thu, 15 May 2025 15:31:46 +0200, > > Daniel Dadap wrote: > > > > > > Some systems expose HD-Audio controllers via objects in the ACPI tables > > > which encapsulate the controller's interrupt and the base address for the > > > HDA registers in an ACPI _CRS object, for example, as listed in this ACPI > > > table dump excerpt: > > > > > > Device (HDA0) > > > { > > > Name (_HID, "NVDA2014") // _HID: Hardware ID > > > ... > > > Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings > > > { > > > Memory32Fixed (ReadWrite, > > > 0x36078000, // Address Base > > > 0x00008000, // Address Length > > > ) > > > Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, ) > > > { > > > 0x0000021E, > > > } > > > }) > > > } > > > > > > Add support for HDA controllers discovered through ACPI, including support > > > for some platforms which expose such HDA controllers on NVIDIA SoCs. This > > > is done with a new driver which uses existing infrastructure for extracting > > > resource information from _CRS objects and plumbs the parsed resource > > > information through to the existing HDA infrastructure to enable HD-Audio > > > functionality on such devices. > > > > > > Although this driver is in the sound/pci/hda/ directory, it targets devices > > > which are not actually enumerated on the PCI bus. This is because it depends > > > upon the Intel "Azalia" infrastructure which has traditionally been used for > > > PCI-based devices. > > > > > > Signed-off-by: Daniel Dadap <ddadap@nvidia.com> > > > > Thanks for the patch. > > The code looks fine. Just a nitpicking: > > > > > +static int __maybe_unused hda_acpi_suspend(struct device *dev) > > .... > > > +static int __maybe_unused hda_acpi_resume(struct device *dev) > > > > The __maybe_unused is superfluous when you set up > > SYSTEM_SLEEP_PM_OPS() macro instead in the below: > > > > > +static const struct dev_pm_ops hda_acpi_pm = { > > > + SET_SYSTEM_SLEEP_PM_OPS(hda_acpi_suspend, hda_acpi_resume) > > Also, at the next time, please put the maintainer (me) to Cc. > Sure, already sent it with you on To: as a reply to the last message. I was wrong about hda_tegra having __maybe_unused, you already cleaned that up. > > thanks, > > Takashi
On Thu, 15 May 2025 17:45:08 +0200, Daniel Dadap wrote: > > On Thu, May 15, 2025 at 04:45:52PM +0200, Takashi Iwai wrote: > > On Thu, 15 May 2025 15:31:46 +0200, > > Daniel Dadap wrote: > > > > > > Some systems expose HD-Audio controllers via objects in the ACPI tables > > > which encapsulate the controller's interrupt and the base address for the > > > HDA registers in an ACPI _CRS object, for example, as listed in this ACPI > > > table dump excerpt: > > > > > > Device (HDA0) > > > { > > > Name (_HID, "NVDA2014") // _HID: Hardware ID > > > ... > > > Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings > > > { > > > Memory32Fixed (ReadWrite, > > > 0x36078000, // Address Base > > > 0x00008000, // Address Length > > > ) > > > Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, ) > > > { > > > 0x0000021E, > > > } > > > }) > > > } > > > > > > Add support for HDA controllers discovered through ACPI, including support > > > for some platforms which expose such HDA controllers on NVIDIA SoCs. This > > > is done with a new driver which uses existing infrastructure for extracting > > > resource information from _CRS objects and plumbs the parsed resource > > > information through to the existing HDA infrastructure to enable HD-Audio > > > functionality on such devices. > > > > > > Although this driver is in the sound/pci/hda/ directory, it targets devices > > > which are not actually enumerated on the PCI bus. This is because it depends > > > upon the Intel "Azalia" infrastructure which has traditionally been used for > > > PCI-based devices. > > > > > > Signed-off-by: Daniel Dadap <ddadap@nvidia.com> > > > > Thanks for the patch. > > The code looks fine. Just a nitpicking: > > > > > +static int __maybe_unused hda_acpi_suspend(struct device *dev) > > .... > > > +static int __maybe_unused hda_acpi_resume(struct device *dev) > > > > The __maybe_unused is superfluous when you set up > > SYSTEM_SLEEP_PM_OPS() macro instead in the below: > > > > > +static const struct dev_pm_ops hda_acpi_pm = { > > > + SET_SYSTEM_SLEEP_PM_OPS(hda_acpi_suspend, hda_acpi_resume) > > > > Thanks, I'll send an updated patch. The hda_tegra driver has these as well > so I presume it can be cleaned up from that driver as well? It's been already cleaned up on 6.15-rc1 :) Takashi
On Thu, May 15, 2025 at 10:49:13AM -0500, Daniel Dadap wrote: > On Thu, May 15, 2025 at 04:54:25PM +0200, Takashi Iwai wrote: > > On Thu, 15 May 2025 16:45:52 +0200, > > Takashi Iwai wrote: > > > > > > On Thu, 15 May 2025 15:31:46 +0200, > > > Daniel Dadap wrote: > > > > > > > > Some systems expose HD-Audio controllers via objects in the ACPI tables > > > > which encapsulate the controller's interrupt and the base address for the > > > > HDA registers in an ACPI _CRS object, for example, as listed in this ACPI > > > > table dump excerpt: > > > > > > > > Device (HDA0) > > > > { > > > > Name (_HID, "NVDA2014") // _HID: Hardware ID > > > > ... > > > > Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings > > > > { > > > > Memory32Fixed (ReadWrite, > > > > 0x36078000, // Address Base > > > > 0x00008000, // Address Length > > > > ) > > > > Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, ) > > > > { > > > > 0x0000021E, > > > > } > > > > }) > > > > } > > > > > > > > Add support for HDA controllers discovered through ACPI, including support > > > > for some platforms which expose such HDA controllers on NVIDIA SoCs. This > > > > is done with a new driver which uses existing infrastructure for extracting > > > > resource information from _CRS objects and plumbs the parsed resource > > > > information through to the existing HDA infrastructure to enable HD-Audio > > > > functionality on such devices. > > > > > > > > Although this driver is in the sound/pci/hda/ directory, it targets devices > > > > which are not actually enumerated on the PCI bus. This is because it depends > > > > upon the Intel "Azalia" infrastructure which has traditionally been used for > > > > PCI-based devices. > > > > > > > > Signed-off-by: Daniel Dadap <ddadap@nvidia.com> > > > > > > Thanks for the patch. > > > The code looks fine. Just a nitpicking: > > > > > > > +static int __maybe_unused hda_acpi_suspend(struct device *dev) > > > .... > > > > +static int __maybe_unused hda_acpi_resume(struct device *dev) > > > > > > The __maybe_unused is superfluous when you set up > > > SYSTEM_SLEEP_PM_OPS() macro instead in the below: > > > > > > > +static const struct dev_pm_ops hda_acpi_pm = { > > > > + SET_SYSTEM_SLEEP_PM_OPS(hda_acpi_suspend, hda_acpi_resume) > > > > Also, at the next time, please put the maintainer (me) to Cc. > > > > Sure, already sent it with you on To: as a reply to the last message. I was > wrong about hda_tegra having __maybe_unused, you already cleaned that up. Oops, sorry for the churn, I did not notice the difference between the SYSTEM_SLEEP_PM_OPS() and SET_SYSTEM_SLEEP_PM_OPS() macros. I think I got it right, now. > > > > > thanks, > > > > Takashi
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 9c427270ff4f..436dfd182a09 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -42,6 +42,17 @@ config SND_HDA_TEGRA To compile this driver as a module, choose M here: the module will be called snd-hda-tegra. +config SND_HDA_ACPI + tristate "HD Audio ACPI" + depends on ACPI + select SND_HDA + help + Say Y here to include support for Azalia-compatible HDA controllers + which are advertised via ACPI objects. + + To compile this driver as a module, choose M here: the module + will be called snd-hda-acpi. + if SND_HDA config SND_HDA_HWDEP diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile index 210c406dfbc5..7a7c16d705dd 100644 --- a/sound/pci/hda/Makefile +++ b/sound/pci/hda/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 snd-hda-intel-y := hda_intel.o snd-hda-tegra-y := hda_tegra.o +snd-hda-acpi-y := hda_acpi.o snd-hda-codec-y := hda_bind.o hda_codec.o hda_jack.o hda_auto_parser.o hda_sysfs.o snd-hda-codec-y += hda_controller.o @@ -80,3 +81,4 @@ obj-$(CONFIG_SND_HDA_SCODEC_TAS2781_SPI) += snd-hda-scodec-tas2781-spi.o # when built in kernel obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-intel.o obj-$(CONFIG_SND_HDA_TEGRA) += snd-hda-tegra.o +obj-$(CONFIG_SND_HDA_ACPI) += snd-hda-acpi.o diff --git a/sound/pci/hda/hda_acpi.c b/sound/pci/hda/hda_acpi.c new file mode 100644 index 000000000000..736eac59eaa0 --- /dev/null +++ b/sound/pci/hda/hda_acpi.c @@ -0,0 +1,316 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * ALSA driver for ACPI-based HDA Controllers. + */ + +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/acpi.h> + +#include <sound/hda_codec.h> + +#include "hda_controller.h" + +struct hda_acpi { + struct azx azx; + struct snd_card *card; + struct platform_device *pdev; + void __iomem *regs; + struct work_struct probe_work; + const struct hda_data *data; +}; + +/** + * struct hda_data - Optional device-specific data + * @short_name: Used for the ALSA card name; defaults to KBUILD_MODNAME + * @long_name: Used for longer description; defaults to short_name + * @flags: Passed to &azx->driver_caps + * + * A pointer to a record of this type may be stored in the + * &acpi_device_id->driver_data field of an ACPI match table entry in order to + * customize the naming and behavior of a particular device. All fields are + * optional and sensible defaults will be selected in their absence. + */ +struct hda_data { + const char *short_name; + const char *long_name; + unsigned long flags; +}; + +static int hda_acpi_dev_disconnect(struct snd_device *device) +{ + struct azx *chip = device->device_data; + + chip->bus.shutdown = 1; + return 0; +} + +static int hda_acpi_dev_free(struct snd_device *device) +{ + struct azx *azx = device->device_data; + struct hda_acpi *hda = container_of(azx, struct hda_acpi, azx); + + cancel_work_sync(&hda->probe_work); + if (azx_bus(azx)->chip_init) { + azx_stop_all_streams(azx); + azx_stop_chip(azx); + } + + azx_free_stream_pages(azx); + azx_free_streams(azx); + snd_hdac_bus_exit(azx_bus(azx)); + + return 0; +} + +static int hda_acpi_init(struct hda_acpi *hda) +{ + struct hdac_bus *bus = azx_bus(&hda->azx); + struct snd_card *card = hda->azx.card; + struct device *dev = &hda->pdev->dev; + struct azx *azx = &hda->azx; + struct resource *res; + unsigned short gcap; + const char *sname, *lname; + int err, irq; + + /* The base address for the HDA registers and the interrupt are wrapped + * in an ACPI _CRS object which can be parsed by platform_get_irq() and + * devm_platform_get_and_ioremap_resource() */ + + irq = platform_get_irq(hda->pdev, 0); + if (irq < 0) + return irq; + + hda->regs = devm_platform_get_and_ioremap_resource(hda->pdev, 0, &res); + if (IS_ERR(hda->regs)) + return PTR_ERR(hda->regs); + + bus->remap_addr = hda->regs; + bus->addr = res->start; + + err = devm_request_irq(dev, irq, azx_interrupt, + IRQF_SHARED, KBUILD_MODNAME, azx); + if (err) { + dev_err(dev, "unable to request IRQ %d, disabling device\n", irq); + return err; + } + bus->irq = irq; + bus->dma_stop_delay = 100; + card->sync_irq = bus->irq; + + gcap = azx_readw(azx, GCAP); + dev_dbg(dev, "chipset global capabilities = 0x%x\n", gcap); + + azx->align_buffer_size = 1; + + azx->capture_streams = (gcap >> 8) & 0x0f; + azx->playback_streams = (gcap >> 12) & 0x0f; + + azx->capture_index_offset = 0; + azx->playback_index_offset = azx->capture_streams; + azx->num_streams = azx->playback_streams + azx->capture_streams; + + err = azx_init_streams(azx); + if (err < 0) { + dev_err(dev, "failed to initialize streams: %d\n", err); + return err; + } + + err = azx_alloc_stream_pages(azx); + if (err < 0) { + dev_err(dev, "failed to allocate stream pages: %d\n", err); + return err; + } + + azx_init_chip(azx, 1); + + if (!bus->codec_mask) { + dev_err(dev, "no codecs found!\n"); + return -ENODEV; + } + + strscpy(card->driver, "hda-acpi", sizeof(card->driver)); + + sname = hda->data->short_name ? hda->data->short_name : KBUILD_MODNAME; + + if (strlen(sname) > sizeof(card->shortname)) + dev_info(dev, "truncating shortname for card %s\n", sname); + strscpy(card->shortname, sname, sizeof(card->shortname)); + + lname = hda->data->long_name ? hda->data->long_name : sname; + + snprintf(card->longname, sizeof(card->longname), + "%s at 0x%lx irq %i", lname, bus->addr, bus->irq); + + return 0; +} + +static void hda_acpi_probe_work(struct work_struct *work) +{ + struct hda_acpi *hda = container_of(work, struct hda_acpi, probe_work); + struct azx *chip = &hda->azx; + int err; + + err = hda_acpi_init(hda); + if (err < 0) + return; + + err = azx_probe_codecs(chip, 8); + if (err < 0) + return; + + err = azx_codec_configure(chip); + if (err < 0) + return; + + err = snd_card_register(chip->card); + if (err < 0) + return; + + chip->running = 1; +} + +static int hda_acpi_create(struct hda_acpi *hda) +{ + static const struct snd_device_ops ops = { + .dev_disconnect = hda_acpi_dev_disconnect, + .dev_free = hda_acpi_dev_free, + }; + static const struct hda_controller_ops null_ops; + struct azx *azx = &hda->azx; + int err; + + mutex_init(&azx->open_mutex); + azx->card = hda->card; + INIT_LIST_HEAD(&azx->pcm_list); + + azx->ops = &null_ops; + azx->driver_caps = hda->data->flags; + azx->driver_type = hda->data->flags & 0xff; + azx->codec_probe_mask = -1; + + err = azx_bus_init(azx, NULL); + if (err < 0) + return err; + + err = snd_device_new(hda->card, SNDRV_DEV_LOWLEVEL, &hda->azx, &ops); + if (err < 0) { + dev_err(&hda->pdev->dev, "Error creating device\n"); + return err; + } + + return 0; +} + +static int hda_acpi_probe(struct platform_device *pdev) +{ + struct hda_acpi *hda; + int err; + + hda = devm_kzalloc(&pdev->dev, sizeof(*hda), GFP_KERNEL); + if (!hda) + return -ENOMEM; + + hda->pdev = pdev; + hda->data = acpi_device_get_match_data(&pdev->dev); + + err = snd_card_new(&pdev->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1, + THIS_MODULE, 0, &hda->card); + if (err < 0) { + dev_err(&pdev->dev, "Error creating card!\n"); + return err; + } + + INIT_WORK(&hda->probe_work, hda_acpi_probe_work); + + err = hda_acpi_create(hda); + if (err < 0) + goto out_free; + hda->card->private_data = &hda->azx; + + dev_set_drvdata(&pdev->dev, hda->card); + + schedule_work(&hda->probe_work); + + return 0; + +out_free: + snd_card_free(hda->card); + return err; +} + +static void hda_acpi_remove(struct platform_device *pdev) +{ + snd_card_free(dev_get_drvdata(&pdev->dev)); +} + +static void hda_acpi_shutdown(struct platform_device *pdev) +{ + struct snd_card *card = dev_get_drvdata(&pdev->dev); + struct azx *chip; + + if (!card) + return; + chip = card->private_data; + if (chip && chip->running) + azx_stop_chip(chip); +} + +static int __maybe_unused hda_acpi_suspend(struct device *dev) +{ + struct snd_card *card = dev_get_drvdata(dev); + int rc; + + rc = pm_runtime_force_suspend(dev); + if (rc < 0) + return rc; + snd_power_change_state(card, SNDRV_CTL_POWER_D3hot); + + return 0; +} + +static int __maybe_unused hda_acpi_resume(struct device *dev) +{ + struct snd_card *card = dev_get_drvdata(dev); + int rc; + + rc = pm_runtime_force_resume(dev); + if (rc < 0) + return rc; + snd_power_change_state(card, SNDRV_CTL_POWER_D0); + + return 0; +} + +static const struct dev_pm_ops hda_acpi_pm = { + SET_SYSTEM_SLEEP_PM_OPS(hda_acpi_suspend, hda_acpi_resume) +}; + +struct hda_data nvidia_hda_data = { + .short_name = "NVIDIA", + .long_name = "NVIDIA HDA Controller", + .flags = AZX_DCAPS_CORBRP_SELF_CLEAR, +}; + +static const struct acpi_device_id hda_acpi_match[] = { + { .id = "NVDA2014", .driver_data = (uintptr_t) &nvidia_hda_data }, + { .id = "NVDA2015", .driver_data = (uintptr_t) &nvidia_hda_data }, + {}, +}; +MODULE_DEVICE_TABLE(acpi, hda_acpi_match); + +static struct platform_driver hda_acpi_platform_driver = { + .driver = { + .name = KBUILD_MODNAME, + .pm = &hda_acpi_pm, + .acpi_match_table = hda_acpi_match, + }, + .probe = hda_acpi_probe, + .remove = hda_acpi_remove, + .shutdown = hda_acpi_shutdown, +}; +module_platform_driver(hda_acpi_platform_driver); + +MODULE_DESCRIPTION("Driver for ACPI-based HDA Controllers"); +MODULE_LICENSE("GPL v2");
Some systems expose HD-Audio controllers via objects in the ACPI tables which encapsulate the controller's interrupt and the base address for the HDA registers in an ACPI _CRS object, for example, as listed in this ACPI table dump excerpt: Device (HDA0) { Name (_HID, "NVDA2014") // _HID: Hardware ID ... Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings { Memory32Fixed (ReadWrite, 0x36078000, // Address Base 0x00008000, // Address Length ) Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, ) { 0x0000021E, } }) } Add support for HDA controllers discovered through ACPI, including support for some platforms which expose such HDA controllers on NVIDIA SoCs. This is done with a new driver which uses existing infrastructure for extracting resource information from _CRS objects and plumbs the parsed resource information through to the existing HDA infrastructure to enable HD-Audio functionality on such devices. Although this driver is in the sound/pci/hda/ directory, it targets devices which are not actually enumerated on the PCI bus. This is because it depends upon the Intel "Azalia" infrastructure which has traditionally been used for PCI-based devices. Signed-off-by: Daniel Dadap <ddadap@nvidia.com> --- sound/pci/hda/Kconfig | 11 ++ sound/pci/hda/Makefile | 2 + sound/pci/hda/hda_acpi.c | 316 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 329 insertions(+) create mode 100644 sound/pci/hda/hda_acpi.c