From c61d4d05ec7517749ac3becac6767ed75ba688a5 Mon Sep 17 00:00:00 2001 From: paulhsia Date: Wed, 20 Nov 2019 16:35:13 +0800 Subject: [PATCH] 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 Tested-by: Chih-Yang Hsia Tested-by: kokoro Commit-Queue: Dylan Reid --- devices/src/pci/ac97_bus_master.rs | 37 ++++++++++++++++++++++++------ devices/src/pci/ac97_regs.rs | 3 +-- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/devices/src/pci/ac97_bus_master.rs b/devices/src/pci/ac97_bus_master.rs index 57f0545ef9..a3ad9af8ad 100644 --- a/devices/src/pci/ac97_bus_master.rs +++ b/devices/src/pci/ac97_bus_master.rs @@ -436,9 +436,8 @@ impl Ac97BusMaster { fn set_cr(&mut self, func: Ac97Function, val: u8, mixer: &Ac97Mixer) { if val & CR_RR != 0 { - self.stop_audio(func); let mut regs = self.regs.lock(); - regs.func_regs_mut(func).do_reset(); + Self::reset_func_regs(&mut regs, func); } else { let cr = self.regs.lock().func_regs(func).cr; if val & CR_RPBM == 0 { @@ -579,12 +578,18 @@ impl Ac97BusMaster { 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) { self.stop_all_audio(); let mut regs = self.regs.lock(); - regs.pi_regs.do_reset(); - regs.po_regs.do_reset(); - regs.mc_regs.do_reset(); + Self::reset_func_regs(&mut regs, Ac97Function::Input); + Self::reset_func_regs(&mut regs, Ac97Function::Output); + Self::reset_func_regs(&mut regs, Ac97Function::Microphone); } } @@ -899,7 +904,7 @@ mod test { bm.writeb(PO_LVI_15, LVI_MASK, &mixer); // 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)); 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_DCH == SR_DCH); // DMA stopped because of lack of buffers. 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 bm.writeb(PO_SR_16, SR_LVBCI as u8, &mixer); assert!(bm.readw(PO_SR_16) & SR_LVBCI == 0); @@ -951,6 +960,11 @@ mod test { // Stop. bm.writeb(PO_CR_1B, 0, &mixer); 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] @@ -984,7 +998,7 @@ mod test { bm.writeb(PI_LVI_05, LVI_MASK, &mixer); // 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); 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_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!( + bm.readl(GLOB_STA_30) & GS_PIINT != 0, + "PIINT bit should be set." + ); // Clear the LVB bit bm.writeb(PI_SR_06, SR_LVBCI as u8, &mixer); @@ -1024,5 +1042,10 @@ mod test { // Stop. bm.writeb(PI_CR_0B, 0, &mixer); 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." + ); } } diff --git a/devices/src/pci/ac97_regs.rs b/devices/src/pci/ac97_regs.rs index a8494eb915..5d2ab9e6d7 100644 --- a/devices/src/pci/ac97_regs.rs +++ b/devices/src/pci/ac97_regs.rs @@ -215,12 +215,11 @@ impl Ac97FunctionRegs { 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) { self.bdbar = 0; self.civ = 0; self.lvi = 0; - self.sr = SR_DCH; self.picb = 0; self.piv = 0; self.cr &= CR_DONT_CLEAR_MASK;