AudioDxe: Improve documentation and logging of AudioOutMask, and change...

AudioDxe: Improve documentation and logging of AudioOutMask, and change default value to 'output all'

Docs/Configuration.md5

-8e6f0670571e5f4b32c0b5f8d4afe0e0
+13177b5c9a453cbce5de81c48baf006b

Docs/Configuration.tex

@@ -6905,34 +6905,39 @@ with the boot menu.
 \item
   \texttt{AudioOutMask}\\
   \textbf{Type}: \texttt{plist\ integer}\\
-  \textbf{Failsafe}: \texttt{0}\\
+  \textbf{Failsafe}: \texttt{-1}\\
   \textbf{Description}: Bit field indicating which output channels to use for UEFI sound.
 
-  This should typically contain a single bit corresponding to the green out of the builtin analog audio
-  controller (\texttt{HDEF}).
-  The number of available output nodes (\texttt{N}) for each HDA codec is shown in the debug log (marked in bold-italic):
+  Audio mask is 1 << audio output (equivalently 2 \texttt{\^{}} audio output). E.g. for audio output \texttt{0} the bitmask is
+  \texttt{1}, for output \texttt{3} it is \texttt{8}, and for outputs \texttt{0} and \texttt{3} it is \texttt{9}.
+
+  The number of available output nodes (\texttt{N}) for each HDA codec is shown in the debug log (marked in bold-italic),
+  audio outputs \texttt{0} to \texttt{N - 1} may be selected:
 
   \texttt{OCAU: 1/3 PciRoot(0x0)/Pci(0x1,0x0)/Pci(0x0,0x1)/VenMsg(<redacted>,00000000) (\textit{\textbf{4 outputs}})}\\
   \texttt{OCAU: 2/3 PciRoot(0x0)/Pci(0x3,0x0)/VenMsg(<redacted>,00000000) (\textit{\textbf{1 outputs}})}\\
   \texttt{OCAU: 3/3 PciRoot(0x0)/Pci(0x1B,0x0)/VenMsg(<redacted>,02000000) (\textit{\textbf{7 outputs}})}
 
-  The first available output node is bit 0 (value \texttt{1}), the second node is bit 1 (value \texttt{2}), etc.
+  When \texttt{AudioDxe} is used then additional information about each output channel is logged during driver binding,
+  including the bitmask for each output. The bitmask values for the desired outputs should be added together to
+  obtain the \texttt{AudioOutMask} value:
 
-  When the debug version of \texttt{AudioDxe} is used, then additional information on each output channel of each codec
-  is logged during driver binding. Further information on the available output channels may also be found from a Linux
-  codec dump \texttt{cat /proc/asound/card\{n\}/codec\#\{m\}}.
+  \texttt{HDA:  | Port widget @ 0x9 is an output (pin defaults 0x2B4020) (\textit{\textbf{bitmask 1}})}\\
+  \texttt{HDA:  | Port widget @ 0xA is an output (pin defaults 0x90100112) (\textit{\textbf{bitmask 2}})}\\
+  \texttt{HDA:  | Port widget @ 0xB is an output (pin defaults 0x90100110) (\textit{\textbf{bitmask 4}})}\\
+  \texttt{HDA:  | Port widget @ 0x10 is an output (pin defaults 0x4BE030) (\textit{\textbf{bitmask 8}})}
+
+  Further information on the available output channels may be found from a Linux codec dump using the command:
+  
+  \texttt{cat /proc/asound/card\{n\}/codec\#\{m\}}
   
   Using \texttt{AudioOutMask}, it is possible to play sound to more than one channel (e.g. main speaker plus bass speaker;
-  headphones plus speakers). For example, if the main speaker is output 0 and the bass speaker
-  is output 2, then to play to both set \texttt{AudioOutMask} to \texttt{1 << 0 + 1 << 2} i.e. \texttt{5}. This
-  feature is supported when all chosen outputs support the sound file format in use; if any do not then no sound
-  will play and an error will be logged.
-
-  \emph{Note 1}: If all available output channels on the codec support the available sound file format, then a value
-  of \texttt{-1} may be used to play to all channels simultaneously.
+  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.
 
-  \emph{Note 2}: Bits in \texttt{AudioOutMask} do not represent internal codec node ids as found in detailed codec
-  dumps, but rather the available output nodes as shown e.g. in the \texttt{OCAU} log lines above.
+  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.
 
   \item
   \texttt{AudioSupport}\\

Docs/Differences/Differences.tex

 \documentclass[]{article}
 %DIF LATEXDIFF DIFFERENCE FILE
-%DIF DEL PreviousConfiguration.tex   Mon Dec 20 23:08:04 2021
-%DIF ADD ../Configuration.tex        Mon Dec 20 23:08:04 2021
+%DIF DEL PreviousConfiguration.tex   Sun Dec 12 09:04:19 2021
+%DIF ADD ../Configuration.tex        Fri Dec 24 11:17:13 2021
 
 \usepackage{lmodern}
 \usepackage{amssymb,amsmath}
@@ -6976,44 +6976,52 @@ with the boot menu.
 \item
   \texttt{\DIFdelbegin \DIFdel{AudioOut}\DIFdelend \DIFaddbegin \DIFadd{AudioOutMask}\DIFaddend }\\
   \textbf{Type}: \texttt{plist\ integer}\\
-  \textbf{Failsafe}: \texttt{0}\\
+  \textbf{Failsafe}: \texttt{\DIFdelbegin \DIFdel{0}\DIFdelend \DIFaddbegin \DIFadd{-1}\DIFaddend }\\
   \textbf{Description}: \DIFdelbegin \DIFdel{Index of the output port of the specified codec starting from 0.
 }\DIFdelend \DIFaddbegin \DIFadd{Bit field indicating which output channels to use for UEFI sound.
 }\DIFaddend 
 
-  This \DIFdelbegin \DIFdel{typically contains the index of the }\DIFdelend \DIFaddbegin \DIFadd{should typically contain a single bit corresponding to the }\DIFaddend green out of the builtin analog audio
-  controller (\texttt{HDEF}).
-  The number of \DIFaddbegin \DIFadd{available }\DIFaddend output nodes (\texttt{N}) \DIFaddbegin \DIFadd{for each HDA codec is shown }\DIFaddend in the debug log (marked in bold-italic):
+  \DIFdelbegin \DIFdel{This typically contains the index of the green out of the builtin analog audio controller (}\DIFdelend \DIFaddbegin \DIFadd{Audio mask is 1 << audio output (equivalently 2 }\DIFaddend \texttt{\DIFdelbegin \DIFdel{HDEF}%DIFDELCMD < \MBLOCKRIGHTBRACE%%%
+\DIFdel{). }\DIFdelend \DIFaddbegin \DIFadd{\^{}}} \DIFadd{audio output). E.g. for audio output }\texttt{\DIFadd{0}} \DIFadd{the bitmask is
+  }\texttt{\DIFadd{1}}\DIFadd{, for output }\texttt{\DIFadd{3}} \DIFadd{it is }\texttt{\DIFadd{8}}\DIFadd{, and for outputs }\texttt{\DIFadd{0}} \DIFadd{and }\texttt{\DIFadd{3}} \DIFadd{it is }\texttt{\DIFadd{9}}\DIFadd{.
+}
+
+  \DIFaddend The number of \DIFaddbegin \DIFadd{available }\DIFaddend output nodes (\texttt{N}) \DIFaddbegin \DIFadd{for each HDA codec is shown }\DIFaddend in the debug log (marked in bold-italic)\DIFaddbegin \DIFadd{,
+  audio outputs }\texttt{\DIFadd{0}} \DIFadd{to }\texttt{\DIFadd{N - 1}} \DIFadd{may be selected}\DIFaddend :
 
   \texttt{OCAU: 1/3 PciRoot(0x0)/Pci(0x1,0x0)/Pci(0x0,0x1)/VenMsg(<redacted>,00000000) (\textit{\textbf{4 outputs}})}\\
   \texttt{OCAU: 2/3 PciRoot(0x0)/Pci(0x3,0x0)/VenMsg(<redacted>,00000000) (\textit{\textbf{1 outputs}})}\\
   \texttt{OCAU: 3/3 PciRoot(0x0)/Pci(0x1B,0x0)/VenMsg(<redacted>,02000000) (\textit{\textbf{7 outputs}})}
 
-  The \DIFdelbegin \DIFdel{quickest way to find the right port is }\DIFdelend \DIFaddbegin \DIFadd{first available output node is bit 0 (value }\texttt{\DIFadd{1}}\DIFadd{), the second node is bit 1 (value }\texttt{\DIFadd{2}}\DIFadd{), etc.
+  \DIFdelbegin \DIFdel{The quickest way to
+  find the right port is to bruteforce the values from }\DIFdelend \DIFaddbegin \DIFadd{When }\texttt{\DIFadd{AudioDxe}} \DIFadd{is used then additional information about each output channel is logged during driver binding,
+  including the bitmask for each output. The bitmask values for the desired outputs should be added together to
+  obtain the }\texttt{\DIFadd{AudioOutMask}} \DIFadd{value:
 }
 
-  \DIFadd{When the debug version of }\texttt{\DIFadd{AudioDxe}} \DIFadd{is used, then additional information on each output channel of each codec
-  is logged during driver binding. Further information on the available output channels may also be found from a Linux
-  codec dump }\texttt{\DIFadd{cat /proc/asound/card\{n\}/codec\#\{m\}}}\DIFadd{.
+  \texttt{\DIFadd{HDA:  | Port widget @ 0x9 is an output (pin defaults 0x2B4020) (}\textit{\textbf{\DIFadd{bitmask 1}}}\DIFadd{)}}\\
+  \texttt{\DIFadd{HDA:  | Port widget @ 0xA is an output (pin defaults 0x90100112) (}\textit{\textbf{\DIFadd{bitmask 2}}}\DIFadd{)}}\\
+  \texttt{\DIFadd{HDA:  | Port widget @ 0xB is an output (pin defaults 0x90100110) (}\textit{\textbf{\DIFadd{bitmask 4}}}\DIFadd{)}}\\
+  \texttt{\DIFadd{HDA:  | Port widget @ 0x10 is an output (pin defaults 0x4BE030) (}\textit{\textbf{\DIFadd{bitmask 8}}}\DIFadd{)}}
+
+  \DIFadd{Further information on the available output channels may be found from a Linux codec dump using the command:
   }
 
+  \texttt{\DIFadd{cat /proc/asound/card\{n\}/codec\#\{m\}}}
+
   \DIFadd{Using }\texttt{\DIFadd{AudioOutMask}}\DIFadd{, it is possible to play sound to more than one channel (e.g. main speaker plus bass speaker;
-  headphones plus speakers). For example, if the main speaker is output 0 and the bass speaker
-  is output 2, then to play to both set }\texttt{\DIFadd{AudioOutMask}} \DIFaddend to \DIFdelbegin \DIFdel{bruteforce the values from }\DIFdelend \DIFaddbegin \texttt{\DIFadd{1 << 0 + 1 << 2}} \DIFadd{i.e. }\texttt{\DIFadd{5}}\DIFadd{. This
-  feature is supported when all chosen outputs support the sound file format in use; if any do not then no sound
-  will play and an error will be logged.
+  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.
 }
 
-  \emph{\DIFadd{Note 1}}\DIFadd{: If 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
-\DIFdelend \DIFaddbegin \DIFadd{-1}} \DIFadd{may be used to play to all channels simultaneously}\DIFaddend .
-
-  \DIFaddbegin \emph{\DIFadd{Note 2}}\DIFadd{: Bits in }\texttt{\DIFadd{AudioOutMask}} \DIFadd{do not represent internal codec node ids as found in detailed codec
-  dumps, but rather the available output nodes as shown e.g. in the }\texttt{\DIFadd{OCAU}} \DIFadd{log lines above.
-}
+\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 
 
-  \DIFaddend \item
+  \item
   \texttt{AudioSupport}\\
   \textbf{Type}: \texttt{plist\ boolean}\\
   \textbf{Failsafe}: \texttt{false}\\

Include/Acidanthera/Library/OcConfigurationLib.h

@@ -618,7 +618,7 @@ typedef enum {
   _(UINT16                      , VolumeAmplifier    ,     , 0                                 , ()) \
   _(BOOLEAN                     , AudioSupport       ,     , FALSE                             , ()) \
   _(UINT8                       , AudioCodec         ,     , 0                                 , ()) \
-  _(UINT64                      , AudioOutMask       ,     , 0                                 , ()) \
+  _(UINT64                      , AudioOutMask       ,     , -1                                , ()) \
   _(UINT8                       , MinimumVolume      ,     , 0                                 , ()) \
   _(BOOLEAN                     , ResetTrafficClass  ,     , FALSE                             , ()) \
   _(BOOLEAN                     , DisconnectHda      ,     , FALSE                             , ())

Staging/AudioDxe/HdaCodec/HdaCodec.c

@@ -703,22 +703,44 @@ HdaCodecParsePorts(
         continue;
 
       // Determine if port is an output based on the device type.
+      // The types reported here do not correspond particularly well to the real hardware.
       DefaultDeviceType = HDA_VERB_GET_CONFIGURATION_DEFAULT_DEVICE(HdaWidget->DefaultConfiguration);
       if ((DefaultDeviceType == HDA_CONFIG_DEFAULT_DEVICE_LINE_OUT) || (DefaultDeviceType == HDA_CONFIG_DEFAULT_DEVICE_SPEAKER) ||
         (DefaultDeviceType == HDA_CONFIG_DEFAULT_DEVICE_HEADPHONE_OUT) || (DefaultDeviceType == HDA_CONFIG_DEFAULT_DEVICE_SPDIF_OUT) ||
         (DefaultDeviceType == HDA_CONFIG_DEFAULT_DEVICE_OTHER_DIGITAL_OUT)) {
 
         // Try to get upstream output.
-        DEBUG((DEBUG_INFO, "HDA:  | Port widget @ 0x%X is an output (pin defaults 0x%X)\n", HdaWidget->NodeId, HdaWidget->DefaultConfiguration));
         Status = HdaCodecFindUpstreamOutput(HdaWidget, 0);
-        if (EFI_ERROR(Status))
+        if (EFI_ERROR(Status)) {
+          DEBUG((
+            DEBUG_WARN,
+            "HDA: Widget @ 0x%X find upstream output - %r\n",
+            HdaWidget->NodeId,
+            Status
+            ));
           continue;
+        }
 
         // Enable output amp.
         Status = HdaIo->SendCommand(HdaIo, HdaWidget->NodeId, HDA_CODEC_VERB(HDA_VERB_SET_PIN_WIDGET_CONTROL,
           HDA_VERB_SET_PIN_WIDGET_CONTROL_PAYLOAD(0, FALSE, FALSE, TRUE, FALSE)), &Response);
-        if (EFI_ERROR(Status))
+        if (EFI_ERROR(Status)) {
+          DEBUG((
+            DEBUG_WARN,
+            "HDA: Widget @ 0x%X enable output amp - %r\n",
+            HdaWidget->NodeId,
+            Status
+            ));
           continue;
+        }
+
+        // Report output.
+        DEBUG((
+          DEBUG_INFO,
+          "HDA:  | Port widget @ 0x%X is an output (pin defaults 0x%X) (bitmask %u)\n",
+          HdaWidget->NodeId,
+          HdaWidget->DefaultConfiguration, 1 << HdaCodecDev->OutputPortsCount
+          ));
 
         // If EAPD is present, enable.
         if (HdaWidget->PinCapabilities & HDA_PARAMETER_PIN_CAPS_EAPD) {