diff options
author | Liav A <liavalb@gmail.com> | 2023-04-11 12:45:31 +0300 |
---|---|---|
committer | Linus Groh <mail@linusgroh.de> | 2023-04-14 19:23:12 +0200 |
commit | 4921561687ec5775be1ad7f9210a2c7483fff244 (patch) | |
tree | de6512383124d48abbefee7d67126acb128ec432 | |
parent | 0050358cd331cc750570c53e6d544ced7cd52e41 (diff) | |
download | serenity-4921561687ec5775be1ad7f9210a2c7483fff244.zip |
Kernel/Audio: Simplify initialization sequence for drivers
Instead of enumerating all available controllers and then ask each to
find its audio channels, we change the initialization sequence to match
what happens in the Networking subsystem and Graphics subsystem - we
essentially probe for a matching driver on a PCI device, create a device
instance, and immediately initialize it.
This in fact allows us to immediately find any hardware initialization
issues and report it, and then dropping the created instance, as usually
being done in other initialization paths in the Kernel.
This also opens the opportunity to propagate errors when failed to
initialize an AudioChannel instance, and it will be addressed in a
future commit.
-rw-r--r-- | Kernel/Devices/Audio/AC97.cpp | 23 | ||||
-rw-r--r-- | Kernel/Devices/Audio/AC97.h | 6 | ||||
-rw-r--r-- | Kernel/Devices/Audio/Controller.h | 4 | ||||
-rw-r--r-- | Kernel/Devices/Audio/IntelHDA/Controller.cpp | 33 | ||||
-rw-r--r-- | Kernel/Devices/Audio/IntelHDA/Controller.h | 6 | ||||
-rw-r--r-- | Kernel/Devices/Audio/Management.cpp | 71 | ||||
-rw-r--r-- | Kernel/Devices/Audio/Management.h | 6 |
7 files changed, 77 insertions, 72 deletions
diff --git a/Kernel/Devices/Audio/AC97.cpp b/Kernel/Devices/Audio/AC97.cpp index ab80e4f611..bef42a6f22 100644 --- a/Kernel/Devices/Audio/AC97.cpp +++ b/Kernel/Devices/Audio/AC97.cpp @@ -8,6 +8,7 @@ #include <Kernel/Arch/Delay.h> #include <Kernel/Devices/Audio/AC97.h> #include <Kernel/Devices/DeviceManagement.h> +#include <Kernel/IOWindow.h> #include <Kernel/InterruptDisabler.h> #include <Kernel/Memory/AnonymousVMObject.h> @@ -22,7 +23,7 @@ static constexpr u16 pcm_fixed_sample_rate = 48000; static constexpr u16 pcm_sample_rate_minimum = 8000; static constexpr u16 pcm_sample_rate_maximum = 48000; -UNMAP_AFTER_INIT ErrorOr<NonnullLockRefPtr<AC97>> AC97::try_create(PCI::DeviceIdentifier const& pci_device_identifier) +UNMAP_AFTER_INIT ErrorOr<NonnullRefPtr<AudioController>> AC97::create(PCI::DeviceIdentifier const& pci_device_identifier) { auto mixer_io_window = TRY(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0)); auto bus_io_window = TRY(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR1)); @@ -30,10 +31,13 @@ UNMAP_AFTER_INIT ErrorOr<NonnullLockRefPtr<AC97>> AC97::try_create(PCI::DeviceId auto pcm_out_channel_io_window = TRY(bus_io_window->create_from_io_window_with_offset(NativeAudioBusChannel::PCMOutChannel)); auto pcm_out_channel = TRY(AC97Channel::create_with_parent_pci_device(pci_device_identifier.address(), "PCMOut"sv, move(pcm_out_channel_io_window))); - auto ac97 = adopt_nonnull_lock_ref_or_enomem(new (nothrow) AC97(pci_device_identifier, move(pcm_out_channel), move(mixer_io_window), move(bus_io_window))); - if (!ac97.is_error()) - TRY(ac97.value()->initialize()); - return ac97; + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) AC97(pci_device_identifier, move(pcm_out_channel), move(mixer_io_window), move(bus_io_window)))); +} + +UNMAP_AFTER_INIT ErrorOr<bool> AC97::probe(PCI::DeviceIdentifier const& device_identifier) +{ + VERIFY(device_identifier.class_code().value() == to_underlying(PCI::ClassID::Multimedia)); + return device_identifier.subclass_code().value() == to_underlying(PCI::Multimedia::SubclassID::AudioController); } UNMAP_AFTER_INIT AC97::AC97(PCI::DeviceIdentifier const& pci_device_identifier, NonnullOwnPtr<AC97Channel> pcm_out_channel, NonnullOwnPtr<IOWindow> mixer_io_window, NonnullOwnPtr<IOWindow> bus_io_window) @@ -79,11 +83,13 @@ bool AC97::handle_irq(RegisterState const&) return true; } -UNMAP_AFTER_INIT ErrorOr<void> AC97::initialize() +UNMAP_AFTER_INIT ErrorOr<void> AC97::initialize(Badge<AudioManagement>) { dbgln_if(AC97_DEBUG, "AC97 @ {}: mixer base: {:#04x}", device_identifier().address(), m_mixer_io_window); dbgln_if(AC97_DEBUG, "AC97 @ {}: bus base: {:#04x}", device_identifier().address(), m_bus_io_window); + m_audio_channel = AudioChannel::must_create(*this, 0); + // Read out AC'97 codec revision and vendor auto extended_audio_id = m_mixer_io_window->read16(NativeAudioMixerRegister::ExtendedAudioID); m_codec_revision = static_cast<AC97Revision>(((extended_audio_id & ExtendedAudioMask::Revision) >> 10) & 0b11); @@ -178,11 +184,6 @@ LockRefPtr<AudioChannel> AC97::audio_channel(u32 index) const return {}; } -void AC97::detect_hardware_audio_channels(Badge<AudioManagement>) -{ - m_audio_channel = AudioChannel::must_create(*this, 0); -} - ErrorOr<void> AC97::set_pcm_output_sample_rate(size_t channel_index, u32 samples_per_second_rate) { if (channel_index != 0) diff --git a/Kernel/Devices/Audio/AC97.h b/Kernel/Devices/Audio/AC97.h index bf8178f13b..5964220451 100644 --- a/Kernel/Devices/Audio/AC97.h +++ b/Kernel/Devices/Audio/AC97.h @@ -26,7 +26,8 @@ class AC97 final , public IRQHandler { public: - static ErrorOr<NonnullLockRefPtr<AC97>> try_create(PCI::DeviceIdentifier const&); + static ErrorOr<bool> probe(PCI::DeviceIdentifier const&); + static ErrorOr<NonnullRefPtr<AudioController>> create(PCI::DeviceIdentifier const&); virtual ~AC97() override; @@ -159,16 +160,15 @@ private: // ^IRQHandler virtual bool handle_irq(RegisterState const&) override; - ErrorOr<void> initialize(); void set_master_output_volume(u8, u8, Muted); ErrorOr<void> set_pcm_output_sample_rate(u32); void set_pcm_output_volume(u8, u8, Muted); ErrorOr<void> write_single_buffer(UserOrKernelBuffer const&, size_t, size_t); // ^AudioController + virtual ErrorOr<void> initialize(Badge<AudioManagement>) override; virtual LockRefPtr<AudioChannel> audio_channel(u32 index) const override; virtual ErrorOr<size_t> write(size_t channel_index, UserOrKernelBuffer const& data, size_t length) override; - virtual void detect_hardware_audio_channels(Badge<AudioManagement>) override; virtual ErrorOr<void> set_pcm_output_sample_rate(size_t channel_index, u32 samples_per_second_rate) override; virtual ErrorOr<u32> get_pcm_output_sample_rate(size_t channel_index) override; diff --git a/Kernel/Devices/Audio/Controller.h b/Kernel/Devices/Audio/Controller.h index 64782d138b..b7ba475d11 100644 --- a/Kernel/Devices/Audio/Controller.h +++ b/Kernel/Devices/Audio/Controller.h @@ -33,13 +33,13 @@ public: virtual LockRefPtr<AudioChannel> audio_channel(u32 index) const = 0; virtual ErrorOr<size_t> write(size_t channel_index, UserOrKernelBuffer const& data, size_t length) = 0; - virtual void detect_hardware_audio_channels(Badge<AudioManagement>) = 0; + virtual ErrorOr<void> initialize(Badge<AudioManagement>) = 0; virtual ErrorOr<void> set_pcm_output_sample_rate(size_t channel_index, u32 samples_per_second_rate) = 0; // Note: The return value is rate of samples per second virtual ErrorOr<u32> get_pcm_output_sample_rate(size_t channel_index) = 0; private: - IntrusiveListNode<AudioController, LockRefPtr<AudioController>> m_node; + IntrusiveListNode<AudioController, NonnullRefPtr<AudioController>> m_node; }; } diff --git a/Kernel/Devices/Audio/IntelHDA/Controller.cpp b/Kernel/Devices/Audio/IntelHDA/Controller.cpp index 60787e0e79..767e87210f 100644 --- a/Kernel/Devices/Audio/IntelHDA/Controller.cpp +++ b/Kernel/Devices/Audio/IntelHDA/Controller.cpp @@ -16,13 +16,16 @@ namespace Kernel::Audio::IntelHDA { -UNMAP_AFTER_INIT ErrorOr<NonnullLockRefPtr<Controller>> Controller::create(PCI::DeviceIdentifier const& pci_device_identifier) +UNMAP_AFTER_INIT ErrorOr<bool> Controller::probe(PCI::DeviceIdentifier const& device_identifier) { - auto controller_io_window = TRY(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0)); + VERIFY(device_identifier.class_code().value() == to_underlying(PCI::ClassID::Multimedia)); + return device_identifier.subclass_code().value() == to_underlying(PCI::Multimedia::SubclassID::HDACompatibleController); +} - auto intel_hda = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) Controller(pci_device_identifier, move(controller_io_window)))); - TRY(intel_hda->initialize()); - return intel_hda; +UNMAP_AFTER_INIT ErrorOr<NonnullRefPtr<AudioController>> Controller::create(PCI::DeviceIdentifier const& pci_device_identifier) +{ + auto controller_io_window = TRY(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0)); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) Controller(pci_device_identifier, move(controller_io_window)))); } UNMAP_AFTER_INIT Controller::Controller(PCI::DeviceIdentifier const& pci_device_identifier, NonnullOwnPtr<IOWindow> controller_io_window) @@ -31,7 +34,7 @@ UNMAP_AFTER_INIT Controller::Controller(PCI::DeviceIdentifier const& pci_device_ { } -UNMAP_AFTER_INIT ErrorOr<void> Controller::initialize() +UNMAP_AFTER_INIT ErrorOr<void> Controller::initialize(Badge<AudioManagement>) { // Enable DMA PCI::enable_bus_mastering(device_identifier()); @@ -83,6 +86,13 @@ UNMAP_AFTER_INIT ErrorOr<void> Controller::initialize() } } + auto result = configure_output_route(); + if (result.is_error()) { + dmesgln_pci(*this, "Failed to set up an output audio channel: {}", result.error()); + return result.release_error(); + } + + m_audio_channel = AudioChannel::must_create(*this, fixed_audio_channel_index); return {}; } @@ -294,17 +304,6 @@ ErrorOr<size_t> Controller::write(size_t channel_index, UserOrKernelBuffer const return m_output_path->output_stream().write(data, length); } -UNMAP_AFTER_INIT void Controller::detect_hardware_audio_channels(Badge<AudioManagement>) -{ - auto result = configure_output_route(); - if (result.is_error()) { - dmesgln_pci(*this, "Failed to set up an output audio channel: {}", result.error()); - return; - } - - m_audio_channel = AudioChannel::must_create(*this, fixed_audio_channel_index); -} - ErrorOr<void> Controller::set_pcm_output_sample_rate(size_t channel_index, u32 samples_per_second_rate) { if (channel_index != fixed_audio_channel_index || !m_output_path) diff --git a/Kernel/Devices/Audio/IntelHDA/Controller.h b/Kernel/Devices/Audio/IntelHDA/Controller.h index 6abd20705b..5a04fd279a 100644 --- a/Kernel/Devices/Audio/IntelHDA/Controller.h +++ b/Kernel/Devices/Audio/IntelHDA/Controller.h @@ -28,7 +28,8 @@ class Controller final : public AudioController , public PCI::Device { public: - static ErrorOr<NonnullLockRefPtr<Controller>> create(PCI::DeviceIdentifier const&); + static ErrorOr<bool> probe(PCI::DeviceIdentifier const&); + static ErrorOr<NonnullRefPtr<AudioController>> create(PCI::DeviceIdentifier const&); virtual ~Controller() = default; // ^PCI::Device @@ -59,7 +60,6 @@ private: Controller(PCI::DeviceIdentifier const&, NonnullOwnPtr<IOWindow>); - ErrorOr<void> initialize(); ErrorOr<void> initialize_codec(u8 codec_address); ErrorOr<void> configure_output_route(); ErrorOr<void> reset(); @@ -67,7 +67,7 @@ private: // ^AudioController virtual LockRefPtr<AudioChannel> audio_channel(u32 index) const override; virtual ErrorOr<size_t> write(size_t channel_index, UserOrKernelBuffer const& data, size_t length) override; - virtual void detect_hardware_audio_channels(Badge<AudioManagement>) override; + virtual ErrorOr<void> initialize(Badge<AudioManagement>) override; virtual ErrorOr<void> set_pcm_output_sample_rate(size_t channel_index, u32 samples_per_second_rate) override; virtual ErrorOr<u32> get_pcm_output_sample_rate(size_t channel_index) override; diff --git a/Kernel/Devices/Audio/Management.cpp b/Kernel/Devices/Audio/Management.cpp index eac2834791..076df65d28 100644 --- a/Kernel/Devices/Audio/Management.cpp +++ b/Kernel/Devices/Audio/Management.cpp @@ -37,6 +37,35 @@ UNMAP_AFTER_INIT AudioManagement::AudioManagement() { } +struct PCIAudioDriverInitializer { + ErrorOr<bool> (*probe)(PCI::DeviceIdentifier const&) = nullptr; + ErrorOr<NonnullRefPtr<AudioController>> (*create)(PCI::DeviceIdentifier const&) = nullptr; +}; + +static constexpr PCIAudioDriverInitializer s_initializers[] = { + { AC97::probe, AC97::create }, + { Audio::IntelHDA::Controller::probe, Audio::IntelHDA::Controller::create }, +}; + +UNMAP_AFTER_INIT ErrorOr<NonnullRefPtr<AudioController>> AudioManagement::determine_audio_device(PCI::DeviceIdentifier const& device_identifier) const +{ + for (auto& initializer : s_initializers) { + auto initializer_probe_found_driver_match_or_error = initializer.probe(device_identifier); + if (initializer_probe_found_driver_match_or_error.is_error()) { + dmesgln("AudioManagement: Failed to probe device {}, due to {}", device_identifier.address(), initializer_probe_found_driver_match_or_error.error()); + continue; + } + auto initializer_probe_found_driver_match = initializer_probe_found_driver_match_or_error.release_value(); + if (initializer_probe_found_driver_match) { + auto device = TRY(initializer.create(device_identifier)); + TRY(device->initialize({})); + return device; + } + } + dmesgln("AudioManagement: Failed to initialize device {}, unsupported audio device", device_identifier.address()); + return Error::from_errno(ENODEV); +} + UNMAP_AFTER_INIT void AudioManagement::enumerate_hardware_controllers() { if (PCI::Access::is_disabled()) @@ -46,48 +75,24 @@ UNMAP_AFTER_INIT void AudioManagement::enumerate_hardware_controllers() if (device_identifier.class_code().value() != to_underlying(PCI::ClassID::Multimedia)) return; - auto create_audio_controller = [](PCI::DeviceIdentifier const& device_identifier) -> ErrorOr<NonnullLockRefPtr<AudioController>> { - switch (static_cast<PCI::Multimedia::SubclassID>(device_identifier.subclass_code().value())) { - case PCI::Multimedia::SubclassID::AudioController: - return AC97::try_create(device_identifier); - case PCI::Multimedia::SubclassID::HDACompatibleController: - return Audio::IntelHDA::Controller::create(device_identifier); - default: - return ENOTSUP; - } - }; - - dbgln("AudioManagement: found audio controller {} at {}", device_identifier.hardware_id(), device_identifier.address()); - auto audio_controller_device = create_audio_controller(device_identifier); - if (audio_controller_device.is_error()) { - dbgln("AudioManagement: failed to initialize audio controller: {}", audio_controller_device.error()); + auto result = determine_audio_device(device_identifier); + if (result.is_error()) { + dmesgln("Failed to initialize audio device ({} {}): {}", device_identifier.address(), device_identifier.hardware_id(), result.error()); return; } - m_controllers_list.append(audio_controller_device.release_value()); + m_controllers_list.with([&](auto& list) { list.append(result.release_value()); }); })); } -UNMAP_AFTER_INIT void AudioManagement::enumerate_hardware_audio_channels() -{ - for (auto& controller : m_controllers_list) - controller.detect_hardware_audio_channels({}); -} - UNMAP_AFTER_INIT bool AudioManagement::initialize() { - - /* Explanation on the flow: - * 1. Enumerate the PCI bus and try to find audio controllers - * 2. Ask each controller to detect the audio channels and instantiate AudioChannel objects. - */ enumerate_hardware_controllers(); - enumerate_hardware_audio_channels(); - - if (m_controllers_list.is_empty()) { + auto list_empty = m_controllers_list.with([&](auto& list) -> bool { + return list.is_empty(); + }); + if (list_empty) dbgln("AudioManagement: no audio controller was initialized."); - return false; - } - return true; + return !list_empty; } } diff --git a/Kernel/Devices/Audio/Management.h b/Kernel/Devices/Audio/Management.h index 9647938bdb..d5b772d8dc 100644 --- a/Kernel/Devices/Audio/Management.h +++ b/Kernel/Devices/Audio/Management.h @@ -28,10 +28,10 @@ public: bool initialize(); private: - void enumerate_hardware_controllers(); - void enumerate_hardware_audio_channels(); + ErrorOr<NonnullRefPtr<AudioController>> determine_audio_device(PCI::DeviceIdentifier const& device_identifier) const; - IntrusiveList<&AudioController::m_node> m_controllers_list; + void enumerate_hardware_controllers(); + SpinlockProtected<IntrusiveList<&AudioController::m_node>, LockRank::None> m_controllers_list; }; } |