ac97: remove duplicated code

Crosvm's AC97 device had code that was duplicated between playback and
capture stream creation. Abstract that code out so it can be shared.

BUG=chromium:968724
TEST=aplay /dev/urandom within container

Change-Id: If2fb50a0655656726dd9c6255bc84493e91c04e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1749948
Tested-by: Fletcher Woodruff <fletcherw@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Chih-Yang Hsia <paulhsia@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Commit-Queue: Fletcher Woodruff <fletcherw@chromium.org>
This commit is contained in:
Fletcher Woodruff 2019-08-07 12:45:10 -06:00 committed by Commit Bot
parent 47becf5b04
commit 99c9c685b5
2 changed files with 60 additions and 67 deletions

View file

@ -160,6 +160,23 @@ impl Display for CaptureError {
type CaptureResult<T> = std::result::Result<T, CaptureError>; type CaptureResult<T> = std::result::Result<T, CaptureError>;
// Audio thread book-keeping data
struct AudioThreadInfo {
thread: Option<thread::JoinHandle<()>>,
thread_run: Arc<AtomicBool>,
stream_control: Option<Box<dyn StreamControl>>,
}
impl AudioThreadInfo {
fn new() -> Self {
Self {
thread: None,
thread_run: Arc::new(AtomicBool::new(false)),
stream_control: None,
}
}
}
/// `Ac97BusMaster` emulates the bus master portion of AC97. It exposes a register read/write /// `Ac97BusMaster` emulates the bus master portion of AC97. It exposes a register read/write
/// interface compliant with the ICH bus master. /// interface compliant with the ICH bus master.
pub struct Ac97BusMaster { pub struct Ac97BusMaster {
@ -168,15 +185,9 @@ pub struct Ac97BusMaster {
regs: Arc<Mutex<Ac97BusMasterRegs>>, regs: Arc<Mutex<Ac97BusMasterRegs>>,
acc_sema: u8, acc_sema: u8,
// Audio thread for capture stream. // Bookkeeping info for playback and capture stream.
audio_thread_pi: Option<thread::JoinHandle<()>>, po_info: AudioThreadInfo,
audio_thread_pi_run: Arc<AtomicBool>, pi_info: AudioThreadInfo,
pi_stream_control: Option<Box<dyn StreamControl>>,
// Audio thread book keeping.
audio_thread_po: Option<thread::JoinHandle<()>>,
audio_thread_po_run: Arc<AtomicBool>,
po_stream_control: Option<Box<dyn StreamControl>>,
// Audio server used to create playback or capture streams. // Audio server used to create playback or capture streams.
audio_server: Box<dyn StreamSource>, audio_server: Box<dyn StreamSource>,
@ -194,13 +205,8 @@ impl Ac97BusMaster {
regs: Arc::new(Mutex::new(Ac97BusMasterRegs::new())), regs: Arc::new(Mutex::new(Ac97BusMasterRegs::new())),
acc_sema: 0, acc_sema: 0,
audio_thread_pi: None, po_info: AudioThreadInfo::new(),
audio_thread_pi_run: Arc::new(AtomicBool::new(false)), pi_info: AudioThreadInfo::new(),
pi_stream_control: None,
audio_thread_po: None,
audio_thread_po_run: Arc::new(AtomicBool::new(false)),
po_stream_control: None,
audio_server, audio_server,
@ -257,7 +263,7 @@ impl Ac97BusMaster {
/// Called when `mixer` has been changed and the new values should be applied to currently /// Called when `mixer` has been changed and the new values should be applied to currently
/// active streams. /// active streams.
pub fn update_mixer_settings(&mut self, mixer: &Ac97Mixer) { pub fn update_mixer_settings(&mut self, mixer: &Ac97Mixer) {
if let Some(control) = self.po_stream_control.as_mut() { if let Some(control) = self.po_info.stream_control.as_mut() {
// The audio server only supports one volume, not separate left and right. // The audio server only supports one volume, not separate left and right.
let (muted, left_volume, _right_volume) = mixer.get_master_volume(); let (muted, left_volume, _right_volume) = mixer.get_master_volume();
control.set_volume(left_volume); control.set_volume(left_volume);
@ -302,7 +308,7 @@ impl Ac97BusMaster {
PO_SR_16 => regs.po_regs.sr, PO_SR_16 => regs.po_regs.sr,
PO_PICB_18 => { PO_PICB_18 => {
// PO PICB // PO PICB
if !self.audio_thread_po_run.load(Ordering::Relaxed) { if !self.po_info.thread_run.load(Ordering::Relaxed) {
// Not running, no need to estimate what has been consumed. // Not running, no need to estimate what has been consumed.
regs.po_regs.picb regs.po_regs.picb
} else { } else {
@ -457,8 +463,8 @@ impl Ac97BusMaster {
func_regs.civ = 0; func_regs.civ = 0;
func_regs.sr &= !SR_DCH; func_regs.sr &= !SR_DCH;
} }
if self.start_audio(func, mixer).is_err() { if let Err(e) = self.start_audio(func, mixer) {
warn!("Failed to start audio"); warn!("Failed to start audio: {}", e);
} }
} }
let mut regs = self.regs.lock(); let mut regs = self.regs.lock();
@ -479,7 +485,7 @@ impl Ac97BusMaster {
if new_glob_cnt & GLOB_CNT_WARM_RESET != 0 { if new_glob_cnt & GLOB_CNT_WARM_RESET != 0 {
// Check if running and if so, ignore. Warm reset is specified to no-op when the device // Check if running and if so, ignore. Warm reset is specified to no-op when the device
// is playing or recording audio. // is playing or recording audio.
if !self.audio_thread_po_run.load(Ordering::Relaxed) { if !self.po_info.thread_run.load(Ordering::Relaxed) {
self.stop_all_audio(); self.stop_all_audio();
let mut regs = self.regs.lock(); let mut regs = self.regs.lock();
regs.glob_cnt = new_glob_cnt & !GLOB_CNT_WARM_RESET; // Auto-cleared reset bit. regs.glob_cnt = new_glob_cnt & !GLOB_CNT_WARM_RESET; // Auto-cleared reset bit.
@ -492,26 +498,31 @@ impl Ac97BusMaster {
fn start_audio(&mut self, func: Ac97Function, mixer: &Ac97Mixer) -> Result<(), Box<dyn Error>> { fn start_audio(&mut self, func: Ac97Function, mixer: &Ac97Mixer) -> Result<(), Box<dyn Error>> {
const AUDIO_THREAD_RTPRIO: u16 = 10; // Matches other cros audio clients. const AUDIO_THREAD_RTPRIO: u16 = 10; // Matches other cros audio clients.
let thread_info = match func {
Ac97Function::Microphone => return Ok(()),
Ac97Function::Input => &mut self.pi_info,
Ac97Function::Output => &mut self.po_info,
};
let num_channels = 2;
let buffer_samples = current_buffer_size(self.regs.lock().func_regs(func), &self.mem)?;
let buffer_frames = buffer_samples / num_channels;
thread_info.thread_run.store(true, Ordering::Relaxed);
let thread_run = thread_info.thread_run.clone();
let thread_mem = self.mem.clone();
let thread_regs = self.regs.clone();
match func { match func {
Ac97Function::Input => { Ac97Function::Input => {
let num_channels = 2;
let buffer_samples =
current_buffer_size(self.regs.lock().func_regs(func), &self.mem)?;
let buffer_frames = buffer_samples / num_channels;
let (stream_control, input_stream) = self.audio_server.new_capture_stream( let (stream_control, input_stream) = self.audio_server.new_capture_stream(
num_channels, num_channels,
DEVICE_SAMPLE_RATE, DEVICE_SAMPLE_RATE,
buffer_frames, buffer_frames,
)?; )?;
self.pi_stream_control = Some(stream_control); self.pi_info.stream_control = Some(stream_control);
self.update_mixer_settings(mixer); self.update_mixer_settings(mixer);
self.audio_thread_pi_run.store(true, Ordering::Relaxed); self.pi_info.thread = Some(thread::spawn(move || {
let thread_run = self.audio_thread_pi_run.clone();
let thread_mem = self.mem.clone();
let thread_regs = self.regs.clone();
self.audio_thread_pi = Some(thread::spawn(move || {
if set_rt_prio_limit(u64::from(AUDIO_THREAD_RTPRIO)).is_err() if set_rt_prio_limit(u64::from(AUDIO_THREAD_RTPRIO)).is_err()
|| set_rt_round_robin(i32::from(AUDIO_THREAD_RTPRIO)).is_err() || set_rt_round_robin(i32::from(AUDIO_THREAD_RTPRIO)).is_err()
{ {
@ -526,27 +537,15 @@ impl Ac97BusMaster {
})); }));
} }
Ac97Function::Output => { Ac97Function::Output => {
let num_channels = 2;
let buffer_samples =
current_buffer_size(self.regs.lock().func_regs(func), &self.mem)?;
let buffer_frames = buffer_samples / num_channels;
let (stream_control, output_stream) = self.audio_server.new_playback_stream( let (stream_control, output_stream) = self.audio_server.new_playback_stream(
num_channels, num_channels,
DEVICE_SAMPLE_RATE, DEVICE_SAMPLE_RATE,
buffer_frames, buffer_frames,
)?; )?;
self.po_stream_control = Some(stream_control); self.po_info.stream_control = Some(stream_control);
self.update_mixer_settings(mixer); self.update_mixer_settings(mixer);
self.audio_thread_po_run.store(true, Ordering::Relaxed); self.po_info.thread = Some(thread::spawn(move || {
let thread_run = self.audio_thread_po_run.clone();
let thread_mem = self.mem.clone();
let thread_regs = self.regs.clone();
self.audio_thread_po = Some(thread::spawn(move || {
if set_rt_prio_limit(u64::from(AUDIO_THREAD_RTPRIO)).is_err() if set_rt_prio_limit(u64::from(AUDIO_THREAD_RTPRIO)).is_err()
|| set_rt_round_robin(i32::from(AUDIO_THREAD_RTPRIO)).is_err() || set_rt_round_robin(i32::from(AUDIO_THREAD_RTPRIO)).is_err()
{ {
@ -561,30 +560,22 @@ impl Ac97BusMaster {
})); }));
} }
Ac97Function::Microphone => (), Ac97Function::Microphone => (),
} };
Ok(()) Ok(())
} }
fn stop_audio(&mut self, func: Ac97Function) { fn stop_audio(&mut self, func: Ac97Function) {
match func { let thread_info = match func {
Ac97Function::Input => { Ac97Function::Microphone => return,
self.audio_thread_pi_run.store(false, Ordering::Relaxed); Ac97Function::Input => &mut self.pi_info,
if let Some(thread) = self.audio_thread_pi.take() { Ac97Function::Output => &mut self.po_info,
if let Err(e) = thread.join() {
error!("Failed to join the capture thread: {:?}.", e);
}
}
}
Ac97Function::Output => {
self.audio_thread_po_run.store(false, Ordering::Relaxed);
if let Some(thread) = self.audio_thread_po.take() {
if let Err(e) = thread.join() {
error!("Failed to join the playback thread: {:?}.", e);
}
}
}
Ac97Function::Microphone => (),
}; };
thread_info.thread_run.store(false, Ordering::Relaxed);
if let Some(thread) = thread_info.thread.take() {
if let Err(e) = thread.join() {
error!("Failed to join {:?} thread: {:?}.", func, e);
}
}
} }
fn stop_all_audio(&mut self) { fn stop_all_audio(&mut self) {
@ -694,7 +685,9 @@ fn buffer_completed(
update_sr(regs, func, new_sr); update_sr(regs, func, new_sr);
} }
regs.po_pointer_update_time = Instant::now(); if func == Ac97Function::Output {
regs.po_pointer_update_time = Instant::now();
}
Ok(()) Ok(())
} }

View file

@ -183,7 +183,7 @@ pub const DESCRIPTOR_LENGTH: usize = 8;
pub const BD_IOC: u32 = 1 << 31; pub const BD_IOC: u32 = 1 << 31;
/// The functions that are supported by the Ac97 subsystem. /// The functions that are supported by the Ac97 subsystem.
#[derive(Copy, Clone)] #[derive(Debug, Copy, Clone, PartialEq)]
pub enum Ac97Function { pub enum Ac97Function {
Input, Input,
Output, Output,