AudioDxe:

 - Fix broken in-OS sound in Windows after AudioDxe with some firmware (closes https://github.com/acidanthera/bugtracker/issues/1909)
 - Add Pcds for some AudioDxe code
 - Other minor updates
OcDebugLogLib:
 - Add DebugPrintDevicePathForHandle

Changelog.md

@@ -9,10 +9,11 @@ OpenCore Changelog
 - Resolved two possible crashes in QEMU in AudioDxe
 - Added AudioDxe settings caching (avoids non-needed setup delays)
 - Added DisconnectHda quirk to allow UEFI sound on Apple hardware (and others)
-- Added autodetected Cirrus Logic GPIO enable to allow UEFI sound on Apple hardware
+- Added autodetected Cirrus Logic GPIO quirk to enable UEFI audio on some Apple hardware
 - Added workarounds for bugs in QEMU intel-hda driver to allow UEFI sound in QEMU
-- Implemented multi-channel (e.g. bass+main speaker; speakers+headphones) UEFI sound configured with `AudioOutMask`
+- Implemented multi-channel (e.g. bass+main speaker; speakers+headphones) UEFI sound with `AudioOutMask`
 - Fixed AudioDxe startup stalls when Nvidia HDA audio present
+- Resolved AudioDxe disabling sound in Windows on some firmware
 
 #### v0.7.6
 - Fixed stack canary support when compiling with GCC

Docs/Configuration.md5

-61ef35d139ecdfed1d7a2bec17ac1da4
+cc1099e4a1a2904f4902fa13895e4c19

Docs/Configuration.tex

@@ -6935,9 +6935,10 @@ with the boot menu.
   headphones plus speakers) as long as all the chosen outputs support the sound file format in use; if any do not then no
   sound will play and a warning will be logged.
 
-  When all available output channels on the codec support the available sound file format, then a value
-  of \texttt{-1} will play sound to all channels simultaneously. If this does not work, it will usually be quickest
-  to try each available output channel one by one, in order to work out which channel(s) to use.
+  When all available output channels on the codec support the available sound file format then a value
+  of \texttt{-1} will play sound to all channels simultaneously. If this does not work it will usually be quickest
+  to try each available output channel one by one, by setting \texttt{AudioOutMask} to \texttt{1}, \texttt{2},
+  \texttt{4}, etc., up to 2 \texttt{\^{}} \texttt{N - 1}, in order to work out which channel(s) produce sound.
 
   \item
   \texttt{AudioSupport}\\

Docs/Differences/Differences.tex

 \documentclass[]{article}
 %DIF LATEXDIFF DIFFERENCE FILE
-%DIF DEL PreviousConfiguration.tex   Sun Jan  2 00:29:23 2022
-%DIF ADD ../Configuration.tex        Sun Jan  2 00:33:00 2022
+%DIF DEL PreviousConfiguration.tex   Sun Dec 12 09:04:19 2021
+%DIF ADD ../Configuration.tex        Sun Jan  2 08:49:19 2022
 
 \usepackage{lmodern}
 \usepackage{amssymb,amsmath}
@@ -7014,12 +7014,11 @@ with the boot menu.
   sound will play and a warning will be logged.
 }
 
-  \DIFadd{When all available output channels on the codec support the available sound file format, then a value
+  \DIFadd{When all available output channels on the codec support the available sound file format then a value
   of }\DIFaddend \texttt{\DIFdelbegin \DIFdel{0}%DIFDELCMD < \MBLOCKRIGHTBRACE %%%
-\DIFdel{to }\texttt{\DIFdel{N - 1}}%DIFAUXCMD
-\DIFdel{. }\DIFdelend \DIFaddbegin \DIFadd{-1}} \DIFadd{will play sound to all channels simultaneously. If this does not work, it will usually be quickest
-  to try each available output channel one by one, in order to work out which channel(s) to use.
-}\DIFaddend 
+\DIFdel{to }\DIFdelend \DIFaddbegin \DIFadd{-1}} \DIFadd{will play sound to all channels simultaneously. If this does not work it will usually be quickest
+  to try each available output channel one by one, by setting }\texttt{\DIFadd{AudioOutMask}} \DIFadd{to }\texttt{\DIFadd{1}}\DIFadd{, }\texttt{\DIFadd{2}}\DIFadd{,
+  }\texttt{\DIFadd{4}}\DIFadd{, etc., up to 2 }\texttt{\DIFadd{\^{}}} \DIFaddend \texttt{N - 1}\DIFaddbegin \DIFadd{, in order to work out which channel(s) produce sound}\DIFaddend .
 
   \item
   \texttt{AudioSupport}\\

Include/Acidanthera/Library/OcDebugLogLib.h

@@ -154,7 +154,21 @@ VOID
 DebugPrintDevicePath (
   IN UINTN                     ErrorLevel,
   IN CONST CHAR8               *Message,
-  IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath
+  IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath  OPTIONAL
+  );
+
+/**
+  Print Device Path corresponding to EFI Handle to log.
+
+  @param[in] ErrorLevel  Debug error level.
+  @param[in] Message     Prefixed message.
+  @param[in] Handle      Handle corresponding to Device path to print.
+**/
+VOID
+DebugPrintDevicePathForHandle (
+  IN UINTN                     ErrorLevel,
+  IN CONST CHAR8               *Message,
+  IN EFI_HANDLE                Handle       OPTIONAL
   );
 
 /**

Library/OcAfterBootCompatLib/ServiceOverrides.c

@@ -927,6 +927,17 @@ OcStartImage (
 /**
   UEFI Boot Services ExitBootServices override.
   Patches kernel entry point with jump to our KernelEntryPatchJumpBack().
+  
+  Notes:
+    - Most OSes attempt to call ExitBootServices more than once if it fails initially
+      (similar to OpenCore ForceExitBootServices)
+      - Therefore, OcExitBootServices may get called more than once
+      - However this should never be relied upon for correct operation
+    - Any logging within this call but before original ExitBootServices is attempted
+      (e.g. within a scheduled handler) may cause ExitBootServices to fail (e.g. it may
+      change the memory map by allocating), and should only be done, if at all, in the
+      case of unexpected errors
+    - Never log after original ExitBootServices has been attempted, not even on error
 **/
 STATIC
 EFI_STATUS

Library/OcDebugLogLib/DebugPrint.c

@@ -20,14 +20,17 @@ VOID
 DebugPrintDevicePath (
   IN UINTN                     ErrorLevel,
   IN CONST CHAR8               *Message,
-  IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath
+  IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath  OPTIONAL
   )
 {
   DEBUG_CODE_BEGIN ();
 
   CHAR16 *TextDevicePath;
 
-  TextDevicePath = ConvertDevicePathToText (DevicePath, FALSE, FALSE);
+  TextDevicePath = NULL;
+  if (DevicePath != NULL) {
+    TextDevicePath = ConvertDevicePathToText (DevicePath, FALSE, FALSE);
+  }
   DEBUG ((ErrorLevel, "%a - %s\n", Message, OC_HUMAN_STRING (TextDevicePath)));
   if (TextDevicePath != NULL) {
     FreePool (TextDevicePath);
@@ -36,6 +39,26 @@ DebugPrintDevicePath (
   DEBUG_CODE_END ();
 }
 
+VOID
+DebugPrintDevicePathForHandle (
+  IN UINTN                     ErrorLevel,
+  IN CONST CHAR8               *Message,
+  IN EFI_HANDLE                Handle       OPTIONAL
+  )
+{
+  DEBUG_CODE_BEGIN ();
+  
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
+
+  DevicePath = NULL;
+  if (Handle != NULL) {
+    DevicePath = DevicePathFromHandle (Handle);
+  }
+  DebugPrintDevicePath (ErrorLevel, Message, DevicePath);
+
+  DEBUG_CODE_END ();
+}
+
 VOID
 DebugPrintHexDump (
   IN UINTN                     ErrorLevel,

Library/OcDebugNullLib/OcDebugNullLib.c

@@ -244,7 +244,16 @@ VOID
 DebugPrintDevicePath (
   IN UINTN                     ErrorLevel,
   IN CONST CHAR8               *Message,
-  IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath
+  IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath  OPTIONAL
+  )
+{
+}
+
+VOID
+DebugPrintDevicePathForHandle (
+  IN UINTN                     ErrorLevel,
+  IN CONST CHAR8               *Message,
+  IN EFI_HANDLE                Handle       OPTIONAL
   )
 {
 }

Library/OcDebugSerialLib/OcDebugSerialLib.c

@@ -417,7 +417,16 @@ VOID
 DebugPrintDevicePath (
   IN UINTN                     ErrorLevel,
   IN CONST CHAR8               *Message,
-  IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath
+  IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath  OPTIONAL
+  )
+{
+}
+
+VOID
+DebugPrintDevicePathForHandle (
+  IN UINTN                     ErrorLevel,
+  IN CONST CHAR8               *Message,
+  IN EFI_HANDLE                Handle       OPTIONAL
   )
 {
 }

Library/OcDriverConnectionLib/DriverDisconnection.c

@@ -22,6 +22,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #include <Library/DebugLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/OcDebugLogLib.h>
 #include <Library/OcDeviceMiscLib.h>
 #include <Library/OcDriverConnectionLib.h>
 #include <Library/OcHdaDevicesLib.h>
@@ -243,8 +244,9 @@ OcDisconnectHdaControllers (
         continue;
       }
 
+      DebugPrintDevicePathForHandle (DEBUG_INFO, "OCDC: Disconnecting audio controller", HandleBuffer[Index]);
       Status = gBS->DisconnectController (HandleBuffer[Index], NULL, NULL);
-      DEBUG ((DEBUG_INFO, "OCDC: Disconnected audio controller handle %u - %p result - %r\n", Index, HandleBuffer[Index], Status));
+      DEBUG ((DEBUG_INFO, "OCDC: Disconnected - %r\n", Status));
     }
 
     FreePool (HandleBuffer);

OpenCorePkg.dec

@@ -697,27 +697,44 @@
   ## @Prompt Register a protocol installation notify for Apple KeyMap Database when not found initially.
   gOpenCorePkgTokenSpaceGuid.PcNvramInitDevicePropertyDatabase|FALSE|BOOLEAN|0x00000001
 
-  ## Indicates if USB KeyBoard Driver disconnects itself when the Exit Boot
-  ## Services event is triggered.<BR><BR>
+  ## Indicates if USB KeyBoard Driver disconnects itself when the Exit Boot Services
+  ## event is triggered.<BR><BR>
   ##   TRUE  - USB KeyBoard Driver will disconnect itself.<BR>
   ##   FALSE - USB KeyBoard Driver will not disconnect itself.<BR>
   ## @Prompt Disconnect the USB KeyBoard Driver when exitting Boot Services.
   gOpenCorePkgTokenSpaceGuid.PcdEnableDisconnectOnExitBootServicesInUsbKbDriver|TRUE|BOOLEAN|0x00000002
 
-  ## Indicates if USB KeyBoard Driver registers a Apple KeyMap Database protocol installation notify.
-  ## Services event is triggered.<BR><BR>
+  ## Indicates if USB KeyBoard Driver registers a Apple KeyMap Database protocol installation notify.<BR><BR>
   ##   TRUE  - USB KeyBoard Driver will registers a protocol installation notify.<BR>
   ##   FALSE - USB KeyBoard Driver will registers a protocol installation notify.<BR>
   ## @Prompt Register a protocol installation notify for Apple KeyMap Database when not found initially.
   gOpenCorePkgTokenSpaceGuid.PcdNotifyAppleKeyMapDbInUsbKbDriver|TRUE|BOOLEAN|0x00000003
 
-  ## Indicates if USB KeyBoard Driver disconnects other drivers to take precedence.
-  ## Services event is triggered.<BR><BR>
+  ## Indicates if USB KeyBoard Driver disconnects other drivers to take precedence.<BR><BR>
   ##   TRUE  - USB KeyBoard Driver will disconnect other drivers.<BR>
   ##   FALSE - USB KeyBoard Driver will not disconnect other drivers.<BR>
   ## @Prompt Disconnect other drivers for the USB KeyBoard Driver to take precedence over them.
   gOpenCorePkgTokenSpaceGuid.PcdUsbKbDriverTakePrecedence|TRUE|BOOLEAN|0x00000004
 
+  ## Indicates whether audio codec setup code produces an error when no valid output bits are specified.<BR><BR>
+  ##   TRUE  - Produce an error.<BR>
+  ##   FALSE - No error, just play no sound.<BR>
+  ## @Prompt Audio codec setup code should produce an error when no valid output bits are specified.
+  gOpenCorePkgTokenSpaceGuid.PcdAudioCodecErrorOnNoOutputs|TRUE|BOOLEAN|0x00000005
+
+  ## Indicates if AudioDxe will try to reset the PCI associated with the HDA controller on Exit Boot Services.<BR><BR>
+  ##   TRUE  - Register event to reset the controller on Exit Boot Services.<BR>
+  ##   FALSE - Do not reset the contoller on Exit Boot Services.<BR>
+  ## @Prompt AudioDxe will try to reset the HDA controller on Exit Boot Services.
+  gOpenCorePkgTokenSpaceGuid.PcdAudioControllerResetPciOnExitBootServices|TRUE|BOOLEAN|0x00000006
+
+  ## Indicates if AudioDxe will try EFI_OPEN_PROTOCOL_GET_PROTOCOL if EFI_OPEN_PROTOCOL_BY_DRIVER
+  ## fails with access denied.<BR><BR>
+  ##   TRUE  - Use EFI_OPEN_PROTOCOL_GET_PROTOCOL if EFI_OPEN_PROTOCOL_BY_DRIVER fails.<BR>
+  ##   FALSE - Do not use EFI_OPEN_PROTOCOL_GET_PROTOCOL.<BR>
+  ## @Prompt Try EFI_OPEN_PROTOCOL_GET_PROTOCOL if EFI_OPEN_PROTOCOL_BY_DRIVER fails.
+  gOpenCorePkgTokenSpaceGuid.PcdAudioControllerTryProtocolGetMode|FALSE|BOOLEAN|0x00000007
+
 [PcdsFixedAtBuild]
   ## Defines the Console Control initialization mode set on entry.<BR><BR>
   ##   0 - EfiConsoleControlScreenText<BR>

Staging/AudioDxe/AudioDxe.h

@@ -56,7 +56,7 @@
 #include <Protocol/HdaControllerInfo.h>
 
 // Driver version
-#define AUDIODXE_VERSION        0xA
+#define AUDIODXE_VERSION        0xB
 #define AUDIODXE_PKG_VERSION    1
 
 // Driver Bindings.

Staging/AudioDxe/AudioDxe.inf

@@ -60,6 +60,11 @@
   gEfiAudioDecodeProtocolGuid         # PRODUCES
   gVMwareHdaProtocolGuid              # SOMETIMES_CONSUMES
 
+[FeaturePcd]
+  gOpenCorePkgTokenSpaceGuid.PcdAudioCodecErrorOnNoOutputs                ## CONSUMES
+  gOpenCorePkgTokenSpaceGuid.PcdAudioControllerResetPciOnExitBootServices ## CONSUMES
+  gOpenCorePkgTokenSpaceGuid.PcdAudioControllerTryProtocolGetMode         ## CONSUMES
+
 [Sources]
   HdaCodec/HdaCodecComponentName.h
   HdaCodec/HdaCodecComponentName.c

Staging/AudioDxe/HdaCodec/HdaCodec.h

@@ -27,8 +27,6 @@
 
 #include "AudioDxe.h"
 
-#define HDA_CODEC_ERROR_ON_NO_OUTPUTS
-
 typedef struct _HDA_CODEC_DEV HDA_CODEC_DEV;
 typedef struct _HDA_FUNC_GROUP HDA_FUNC_GROUP;
 typedef struct _HDA_WIDGET_DEV HDA_WIDGET_DEV;

Staging/AudioDxe/HdaCodec/HdaCodecAudioIo.c

@@ -6,6 +6,7 @@
 #include "HdaCodec.h"
 #include <Protocol/AudioIo.h>
 #include <Library/OcMiscLib.h>
+#include <Library/PcdLib.h>
 
 //
 // Cache playback setup.
@@ -260,11 +261,11 @@ HdaCodecAudioIoSetupPlayback(
   DEBUG((DEBUG_VERBOSE, "HdaCodecAudioIoSetupPlayback(): start\n"));
 
   // Basic settings caching.
-  if (mOutputIndexMask == OutputIndexMask &&
-      mVolume == Volume &&
-      mFreq == Freq &&
-      mBits == Bits &&
-      mChannels == Channels) {
+  if (mOutputIndexMask == OutputIndexMask
+      && mVolume == Volume
+      && mFreq == Freq
+      && mBits == Bits
+      && mChannels == Channels) {
     return EFI_SUCCESS;
   }
 
@@ -289,12 +290,10 @@ HdaCodecAudioIoSetupPlayback(
   }
   OutputIndexMask &= ~LShiftU64(MAX_UINT64, HdaCodecDev->OutputPortsCount);
 
-#if defined(HDA_CODEC_ERROR_ON_NO_OUTPUTS)
   // Fail visibily if nothing is requested.
-  if (OutputIndexMask == 0) {
+  if (PcdGetBool (PcdAudioCodecErrorOnNoOutputs) && OutputIndexMask == 0) {
     return EFI_INVALID_PARAMETER;
   }
-#endif
 
   // Avoid Coverity warnings (the bit mask checks actually ensure that these cannot be used uninitialised).
   StreamBits = 0;
@@ -481,12 +480,10 @@ HdaCodecAudioIoSetupPlayback(
   // Save requested outputs.
   AudioIoPrivateData->SelectedOutputIndexMask = OutputIndexMask;
 
-#if !defined(HDA_CODEC_ERROR_ON_NO_OUTPUTS)
   // Nothing to play.
   if (OutputIndexMask == 0) {
     return EFI_SUCCESS;
   }
-#endif
 
   // Calculate stream format and setup stream.
   StreamFmt = HDA_CONVERTER_FORMAT_SET(Channels - 1, StreamBits,

Staging/AudioDxe/HdaController/HdaController.c

@@ -27,9 +27,11 @@
 
 #include <Library/OcGuardLib.h>
 #include <Library/OcDeviceMiscLib.h>
+#include <Library/OcDebugLogLib.h>
 #include <Library/OcHdaDevicesLib.h>
 #include <Library/OcMiscLib.h>
 #include <Library/OcStringLib.h>
+#include <Library/PcdLib.h>
 
 VOID
 EFIAPI
@@ -265,6 +267,10 @@ HdaControllerInitPciHw(
   // If No Snoop is currently enabled, disable it.
   //
   if (HdaDevC & PCI_HDA_DEVC_NOSNOOPEN) {
+    DEBUG ((DEBUG_INFO, "HDA: Controller disable no snoop\n"));
+    HdaControllerDev->OriginalPciDeviceControl = HdaDevC;
+    HdaControllerDev->OriginalPciDeviceControlSaved = TRUE;
+
     HdaDevC &= ~PCI_HDA_DEVC_NOSNOOPEN;
     Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, PCI_HDA_DEVC_OFFSET, 1, &HdaDevC);
     if (EFI_ERROR (Status)) {
@@ -324,10 +330,51 @@ HdaControllerGetName (
   DEBUG ((DEBUG_INFO, "HDA: Controller is %s\n", HdaControllerDev->Name));
 }
 
+STATIC
+EFI_STATUS
+HdaControllerRestoreNoSnoopEn (
+  IN HDA_CONTROLLER_DEV *HdaControllerDev
+  )
+{
+  EFI_PCI_IO_PROTOCOL *PciIo;
+
+  if (!HdaControllerDev->OriginalPciDeviceControlSaved) {
+    return EFI_SUCCESS;
+  }
+
+  PciIo = HdaControllerDev->PciIo;
+
+  DEBUG ((DEBUG_VERBOSE, "HdaControllerCleanup(): restore PCI device control\n"));
+  return PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, PCI_HDA_DEVC_OFFSET, 1, &HdaControllerDev->OriginalPciDeviceControl);
+}
+
+STATIC
+VOID
+EFIAPI
+HdaControllerExitBootServicesHandler (
+  IN EFI_EVENT  Event,
+  IN VOID       *Context
+  )
+{
+  HDA_CONTROLLER_DEV *HdaControllerDev;
+
+  HdaControllerDev = Context;
+
+  //
+  // Restore No Snoop Enable bit at Exit Boot Services to avoid breaking in-OS sound in Windows with some firmware.
+  // Note: Windows sound is fine even without this on many systems where AudioDxe disables No Snoop.
+  // REF: https://github.com/acidanthera/bugtracker/issues/1909
+  // REF: https://github.com/acidanthera/bugtracker/issues/740#issuecomment-998762564
+  // REF: Intel I/O Controller Hub 9 (ICH9) Family Datasheet (DEVC - Device Conrol Register/NSNPEN)
+  //
+  HdaControllerRestoreNoSnoopEn (HdaControllerDev);
+}
+
 EFI_STATUS
 EFIAPI
 HdaControllerReset (
-  IN HDA_CONTROLLER_DEV *HdaControllerDev
+  IN HDA_CONTROLLER_DEV *HdaControllerDev,
+  IN BOOLEAN            Restart
   ) 
 {
   DEBUG ((DEBUG_VERBOSE, "HdaControllerReset(): start\n"));
@@ -364,6 +411,24 @@ HdaControllerReset (
     }
   }
 
+  if (!Restart) {
+    return EFI_SUCCESS;
+  }
+
+  if (PcdGetBool (PcdAudioControllerResetPciOnExitBootServices)
+    && HdaControllerDev->ExitBootServicesEvent == NULL) {
+    Status = gBS->CreateEvent (
+      EVT_SIGNAL_EXIT_BOOT_SERVICES,
+      TPL_CALLBACK,
+      HdaControllerExitBootServicesHandler,
+      HdaControllerDev,
+      &HdaControllerDev->ExitBootServicesEvent
+      );
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+  }
+
   //
   // Set CRST bit to begin the process of coming out of reset.
   //
@@ -734,19 +799,27 @@ HdaControllerInstallProtocols (
 
 VOID
 EFIAPI
-HdaControllerCleanup(
-  IN HDA_CONTROLLER_DEV *HdaControllerDev) {
+HdaControllerCleanup (
+  IN HDA_CONTROLLER_DEV *HdaControllerDev
+  )
+{
+  EFI_STATUS            Status;
+  EFI_PCI_IO_PROTOCOL   *PciIo;
+
   DEBUG((DEBUG_VERBOSE, "HdaControllerCleanup(): start\n"));
 
   // If controller device is already free, we are done.
   if (HdaControllerDev == NULL)
     return;
 
-  // Create variables.
-  EFI_STATUS Status;
-  EFI_PCI_IO_PROTOCOL *PciIo = HdaControllerDev->PciIo;
-  UINT32 HdaGCtl;
+  PciIo = HdaControllerDev->PciIo;
 
+  // Clear ExitBootServices event.
+  if (HdaControllerDev->ExitBootServicesEvent != NULL) {
+    gBS->CloseEvent (HdaControllerDev->ExitBootServicesEvent);
+    HdaControllerDev->ExitBootServicesEvent = NULL;
+  }
+  
   // Clean HDA Controller info protocol.
   if (HdaControllerDev->HdaControllerInfoData != NULL) {
     // Uninstall protocol.
@@ -793,16 +866,26 @@ HdaControllerCleanup(
   HdaControllerCleanupRingBuffer (&HdaControllerDev->Corb, HDA_RING_BUFFER_TYPE_CORB);
   HdaControllerCleanupRingBuffer (&HdaControllerDev->Rirb, HDA_RING_BUFFER_TYPE_RIRB);
 
-  // Get value of CRST bit.
-  Status = PciIo->Mem.Read(PciIo, EfiPciIoWidthUint32, PCI_HDA_BAR, HDA_REG_GCTL, 1, &HdaGCtl);
-
   // Place controller into a reset state to stop it.
-  if (!(EFI_ERROR(Status))) {
-    HdaGCtl &= ~HDA_REG_GCTL_CRST;
-    Status = PciIo->Mem.Write(PciIo, EfiPciIoWidthUint32, PCI_HDA_BAR, HDA_REG_GCTL, 1, &HdaGCtl);
+  HdaControllerReset (HdaControllerDev, FALSE);
+
+  //
+  // Restore PCI device control and attributes if needed.
+  //
+  HdaControllerRestoreNoSnoopEn (HdaControllerDev);
+
+  if (HdaControllerDev->OriginalPciAttributesSaved) {
+    DEBUG((DEBUG_VERBOSE, "HdaControllerCleanup(): restore PCI attributes\n"));
+    PciIo->Attributes (
+      PciIo,
+      EfiPciIoAttributeOperationSet,
+      HdaControllerDev->OriginalPciAttributes,
+      NULL
+      );
   }
 
   // Free controller device.
+  DEBUG((DEBUG_VERBOSE, "HdaControllerCleanup(): free controller device\n"));
   gBS->UninstallProtocolInterface(HdaControllerDev->ControllerHandle,
     &gEfiCallerIdGuid, HdaControllerDev);
   FreePool(HdaControllerDev);
@@ -872,20 +955,11 @@ HdaControllerDriverBindingStart (
   EFI_DEVICE_PATH_PROTOCOL  *HdaControllerDevicePath;
   HDA_CONTROLLER_DEV        *HdaControllerDev;
   UINT32                    OpenMode;
-  CHAR16                    *DevicePath;
 
   //
   // Identify device by the path required to access it in config.plist.
   //
-  DevicePath = ConvertDevicePathToText (
-    DevicePathFromHandle (ControllerHandle),
-    FALSE,
-    FALSE
-    );
-  DEBUG ((DEBUG_INFO, "HDA: Connecting controller %s\n", DevicePath));
-  if (DevicePath != NULL) {
-    FreePool (DevicePath);
-  }
+  DebugPrintDevicePathForHandle (DEBUG_INFO, "HDA: Connecting controller", ControllerHandle);
 
   OpenMode = EFI_OPEN_PROTOCOL_BY_DRIVER;
 
@@ -894,19 +968,33 @@ HdaControllerDriverBindingStart (
   // Access Denied typically means OpenCore DisconnectHda quirk is required
   // to free up the controller, e.g. on Apple hardware or VMware Fusion.
   //
-  Status = gBS->OpenProtocol (
-    ControllerHandle,
-    &gEfiPciIoProtocolGuid,
-    (VOID**) &PciIo,
-    This->DriverBindingHandle,
-    ControllerHandle,
-    OpenMode
-    );
+  do {
+    Status = gBS->OpenProtocol (
+      ControllerHandle,
+      &gEfiPciIoProtocolGuid,
+      (VOID**) &PciIo,
+      This->DriverBindingHandle,
+      ControllerHandle,
+      OpenMode
+      );
 
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_WARN, "HDA: Open PCI IO protocol - %r\n", Status));
-    return Status;
-  }
+    if (EFI_ERROR (Status)) {
+      if (PcdGetBool (PcdAudioControllerTryProtocolGetMode)
+        && Status == EFI_ACCESS_DENIED
+        && OpenMode == EFI_OPEN_PROTOCOL_BY_DRIVER) {
+        //
+        // No longer applied just if protocol gVMwareHdaProtocolGuid is found, since it also
+        // allows sound on other devices where HDA controller is already connected, e.g. Macs.
+        // Now on Pcd because it appears never to be needed if DisconnectHda is applied.
+        //
+        DEBUG ((DEBUG_INFO, "HDA: %r using DRIVER mode, trying GET mode\n", Status));
+        OpenMode = EFI_OPEN_PROTOCOL_GET_PROTOCOL;
+        continue;
+      }
+      DEBUG ((DEBUG_WARN, "HDA: Open PCI I/O protocol (try DisconnectHda quirk?) - %r\n", Status));
+      return Status;
+    }
+  } while (EFI_ERROR (Status));
 
   //
   // Open Device Path protocol.
@@ -962,7 +1050,7 @@ HdaControllerDriverBindingStart (
   //
   // Reset controller.
   //
-  Status = HdaControllerReset (HdaControllerDev);
+  Status = HdaControllerReset (HdaControllerDev, TRUE);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_WARN, "HDA: Controller reset - %r\n", Status));
     goto FREE_CONTROLLER;
@@ -1039,18 +1127,6 @@ HdaControllerDriverBindingStart (
 
 FREE_CONTROLLER:
 
-  //
-  // Restore PCI attributes if needed.
-  //
-  if (HdaControllerDev->OriginalPciAttributesSaved) {
-    PciIo->Attributes (
-      PciIo,
-      EfiPciIoAttributeOperationSet,
-      HdaControllerDev->OriginalPciAttributes,
-      NULL
-      );
-  }
-
   //
   // Free controller device.
   //
@@ -1092,12 +1168,15 @@ HdaControllerDriverBindingStop (
   HDA_CONTROLLER_DEV  *HdaControllerDev;
   UINT32              OpenMode;
 
-  DEBUG ((DEBUG_INFO, "HDA: Stopping for %p\n", ControllerHandle));
+  //
+  // Identify device by the path required to access it in config.plist.
+  //
+  DebugPrintDevicePathForHandle (DEBUG_INFO, "HDA: Disconnecting controller", ControllerHandle);
 
   OpenMode = EFI_OPEN_PROTOCOL_BY_DRIVER;
 
   //
-  // Get codec device.
+  // Get controller device.
   //
   Status = gBS->OpenProtocol (
     ControllerHandle,
@@ -1109,6 +1188,7 @@ HdaControllerDriverBindingStop (
     );
 
   if (!EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_VERBOSE, "HDA: Cleaning up\n"));
     //
     // Gather open mode.
     //
@@ -1121,18 +1201,6 @@ HdaControllerDriverBindingStop (
       return EFI_INVALID_PARAMETER;
     }
 
-    //
-    // Restore PCI attributes if needed.
-    //
-    if (HdaControllerDev->OriginalPciAttributesSaved) {
-      HdaControllerDev->PciIo->Attributes (
-        HdaControllerDev->PciIo,
-        EfiPciIoAttributeOperationSet,
-        HdaControllerDev->OriginalPciAttributes,
-        NULL
-        );
-    }
-
     //
     // Cleanup controller.
     //
@@ -1143,21 +1211,23 @@ HdaControllerDriverBindingStop (
   // Close protocols.
   //
   if (OpenMode == EFI_OPEN_PROTOCOL_BY_DRIVER) {
-    gBS->CloseProtocol (
+    Status = gBS->CloseProtocol (
       ControllerHandle,
       &gEfiDevicePathProtocolGuid,
       This->DriverBindingHandle,
       ControllerHandle
       );
-    gBS->CloseProtocol (
+    DEBUG ((DEBUG_VERBOSE, "HDA: Close device path protocol - %r\n", Status));
+    Status = gBS->CloseProtocol (
       ControllerHandle,
       &gEfiPciIoProtocolGuid,
       This->DriverBindingHandle,
       ControllerHandle
       );
+    DEBUG ((DEBUG_VERBOSE, "HDA: Close PCI I/O protocol - %r\n", Status));
   }
 
-  DEBUG ((DEBUG_INFO, "HDA: Stopping done for %p\n", ControllerHandle));
+  DEBUG ((DEBUG_INFO, "HDA: Disconnected\n"));
 
   return EFI_SUCCESS;
 }

Staging/AudioDxe/HdaController/HdaController.h

@@ -323,6 +323,8 @@ struct _HDA_CONTROLLER_DEV {
   // PCI.
   UINT64 OriginalPciAttributes;
   BOOLEAN OriginalPciAttributesSaved;
+  UINT16 OriginalPciDeviceControl;
+  BOOLEAN OriginalPciDeviceControlSaved;
 
   // Published info protocol.
   HDA_CONTROLLER_INFO_PRIVATE_DATA *HdaControllerInfoData;
@@ -353,7 +355,7 @@ struct _HDA_CONTROLLER_DEV {
 
   // Events.
   EFI_EVENT ResponsePollTimer;
-  EFI_EVENT ExitBootServiceEvent;
+  EFI_EVENT ExitBootServicesEvent;
   SPIN_LOCK SpinLock;
 
   // Required quirks. 
@@ -495,7 +497,8 @@ HdaControllerStreamOutputPollTimerHandler (
 EFI_STATUS
 EFIAPI
 HdaControllerReset (
-  IN HDA_CONTROLLER_DEV *HdaControllerDev
+  IN HDA_CONTROLLER_DEV *HdaControllerDev,
+  IN BOOLEAN            Restart
   );
 
 EFI_STATUS