ac97: bus_master: CR: Clean up reset registers opt

In bus_master control register (CR), reset registers (RR) operation is
refactored by:
- Don't call stop_audio(), since the bus master should always be in stop
  state while getting this control. (From AC'97 spec: Setting it when the
  Run bit is set will cause undefined consequences."). And the driver
  will always disable the audio first by setting 0 to CR_RPBM bit.
- While doing the registers reset, clean up sr by using update_sr since
  assigning 0 to sr directly won't unset the interrupt bit in global
  status register and the driver might go into snd_intel8x0_update()
  with a stopped substream.
- Introduce helper function - reset_func_regs()

Add steps in unit tests which
- Start the bus masters with "Interrupt on Completion Enable" (CR_IOCE) bit.
- Verify if the interrupt bit in global status register is set / unset.

BUG=chromium:1026538
TEST=Unit tests

Change-Id: Ie90ca4c82cc3c867992ecaeb61ef4b3e9dd0d079
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1925916
Reviewed-by: Chih-Yang Hsia <paulhsia@chromium.org>
Tested-by: Chih-Yang Hsia <paulhsia@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Dylan Reid <dgreid@chromium.org>
This commit is contained in:
paulhsia 2019-11-20 16:35:13 +08:00 committed by Commit Bot
parent b0ac00745a
commit c61d4d05ec
2 changed files with 31 additions and 9 deletions

View file

@ -436,9 +436,8 @@ impl Ac97BusMaster {
fn set_cr(&mut self, func: Ac97Function, val: u8, mixer: &Ac97Mixer) { fn set_cr(&mut self, func: Ac97Function, val: u8, mixer: &Ac97Mixer) {
if val & CR_RR != 0 { if val & CR_RR != 0 {
self.stop_audio(func);
let mut regs = self.regs.lock(); let mut regs = self.regs.lock();
regs.func_regs_mut(func).do_reset(); Self::reset_func_regs(&mut regs, func);
} else { } else {
let cr = self.regs.lock().func_regs(func).cr; let cr = self.regs.lock().func_regs(func).cr;
if val & CR_RPBM == 0 { if val & CR_RPBM == 0 {
@ -579,12 +578,18 @@ impl Ac97BusMaster {
self.stop_audio(Ac97Function::Microphone); self.stop_audio(Ac97Function::Microphone);
} }
// Helper function for resetting function registers.
fn reset_func_regs(regs: &mut Ac97BusMasterRegs, func: Ac97Function) {
regs.func_regs_mut(func).do_reset();
update_sr(regs, func, SR_DCH);
}
fn reset_audio_regs(&mut self) { fn reset_audio_regs(&mut self) {
self.stop_all_audio(); self.stop_all_audio();
let mut regs = self.regs.lock(); let mut regs = self.regs.lock();
regs.pi_regs.do_reset(); Self::reset_func_regs(&mut regs, Ac97Function::Input);
regs.po_regs.do_reset(); Self::reset_func_regs(&mut regs, Ac97Function::Output);
regs.mc_regs.do_reset(); Self::reset_func_regs(&mut regs, Ac97Function::Microphone);
} }
} }
@ -899,7 +904,7 @@ mod test {
bm.writeb(PO_LVI_15, LVI_MASK, &mixer); bm.writeb(PO_LVI_15, LVI_MASK, &mixer);
// Start. // Start.
bm.writeb(PO_CR_1B, CR_RPBM, &mixer); bm.writeb(PO_CR_1B, CR_IOCE | CR_RPBM, &mixer);
std::thread::sleep(time::Duration::from_millis(50)); std::thread::sleep(time::Duration::from_millis(50));
let picb = bm.readw(PO_PICB_18); let picb = bm.readw(PO_PICB_18);
@ -937,6 +942,10 @@ mod test {
assert!(bm.readw(PO_SR_16) & SR_LVBCI != 0); // Hit last buffer assert!(bm.readw(PO_SR_16) & SR_LVBCI != 0); // Hit last buffer
assert!(bm.readw(PO_SR_16) & SR_DCH == SR_DCH); // DMA stopped because of lack of buffers. assert!(bm.readw(PO_SR_16) & SR_DCH == SR_DCH); // DMA stopped because of lack of buffers.
assert_eq!(bm.readb(PO_LVI_15), bm.readb(PO_CIV_14)); assert_eq!(bm.readb(PO_LVI_15), bm.readb(PO_CIV_14));
assert!(
bm.readl(GLOB_STA_30) & GS_POINT != 0,
"POINT bit should be set."
);
// Clear the LVB bit // Clear the LVB bit
bm.writeb(PO_SR_16, SR_LVBCI as u8, &mixer); bm.writeb(PO_SR_16, SR_LVBCI as u8, &mixer);
assert!(bm.readw(PO_SR_16) & SR_LVBCI == 0); assert!(bm.readw(PO_SR_16) & SR_LVBCI == 0);
@ -951,6 +960,11 @@ mod test {
// Stop. // Stop.
bm.writeb(PO_CR_1B, 0, &mixer); bm.writeb(PO_CR_1B, 0, &mixer);
assert!(bm.readw(PO_SR_16) & 0x01 != 0); // DMA is not running. assert!(bm.readw(PO_SR_16) & 0x01 != 0); // DMA is not running.
bm.writeb(PO_CR_1B, CR_RR, &mixer);
assert!(
bm.readl(GLOB_STA_30) & GS_POINT == 0,
"POINT bit should be disabled."
);
} }
#[test] #[test]
@ -984,7 +998,7 @@ mod test {
bm.writeb(PI_LVI_05, LVI_MASK, &mixer); bm.writeb(PI_LVI_05, LVI_MASK, &mixer);
// Start. // Start.
bm.writeb(PI_CR_0B, CR_RPBM, &mixer); bm.writeb(PI_CR_0B, CR_IOCE | CR_RPBM, &mixer);
assert_eq!(bm.readw(PI_PICB_08), 0); assert_eq!(bm.readw(PI_PICB_08), 0);
std::thread::sleep(time::Duration::from_millis(50)); std::thread::sleep(time::Duration::from_millis(50));
@ -1009,6 +1023,10 @@ mod test {
assert_ne!(bm.readw(PI_SR_06) & SR_LVBCI, 0); // Hit last buffer assert_ne!(bm.readw(PI_SR_06) & SR_LVBCI, 0); // Hit last buffer
assert_eq!(bm.readw(PI_SR_06) & SR_DCH, SR_DCH); // DMA stopped because of lack of buffers. assert_eq!(bm.readw(PI_SR_06) & SR_DCH, SR_DCH); // DMA stopped because of lack of buffers.
assert_eq!(bm.readb(PI_LVI_05), bm.readb(PI_CIV_04)); assert_eq!(bm.readb(PI_LVI_05), bm.readb(PI_CIV_04));
assert!(
bm.readl(GLOB_STA_30) & GS_PIINT != 0,
"PIINT bit should be set."
);
// Clear the LVB bit // Clear the LVB bit
bm.writeb(PI_SR_06, SR_LVBCI as u8, &mixer); bm.writeb(PI_SR_06, SR_LVBCI as u8, &mixer);
@ -1024,5 +1042,10 @@ mod test {
// Stop. // Stop.
bm.writeb(PI_CR_0B, 0, &mixer); bm.writeb(PI_CR_0B, 0, &mixer);
assert!(bm.readw(PI_SR_06) & 0x01 != 0); // DMA is not running. assert!(bm.readw(PI_SR_06) & 0x01 != 0); // DMA is not running.
bm.writeb(PI_CR_0B, CR_RR, &mixer);
assert!(
bm.readl(GLOB_STA_30) & GS_PIINT == 0,
"PIINT bit should be disabled."
);
} }
} }

View file

@ -215,12 +215,11 @@ impl Ac97FunctionRegs {
regs regs
} }
/// Reset all the registers to the PoR defaults. /// Reset all the registers to the PoR defaults. `sr` should be updated by `update_sr`.
pub fn do_reset(&mut self) { pub fn do_reset(&mut self) {
self.bdbar = 0; self.bdbar = 0;
self.civ = 0; self.civ = 0;
self.lvi = 0; self.lvi = 0;
self.sr = SR_DCH;
self.picb = 0; self.picb = 0;
self.piv = 0; self.piv = 0;
self.cr &= CR_DONT_CLEAR_MASK; self.cr &= CR_DONT_CLEAR_MASK;