|
|
I'll revert that commit. This isn't right and needs to be done
differently.
On Mon, Jul 14, 2008 at 12:25:11AM +0200, Stefan Sperling wrote:
> Hi,
>
> since revision 1.50 of acpibat.c was committed ("Properly handle
> battery insertion/removal"), the battery meter in my laptop has
> been jerky.
>
> Note how the capacity reading drops to 0 and the battery is
> considered absent briefly in the following output:
>
> $ while true; do apm; sleep 1; done
> Battery state: CRITICAL, 21% remaining, 44 minutes life estimate
> A/C adapter state: not connected
> Performance adjustment mode: auto (1000 MHz)
> Battery state: CRITICAL, 21% remaining, 44 minutes life estimate
> A/C adapter state: not connected
> Performance adjustment mode: auto (1000 MHz)
> Battery state: CRITICAL, 21% remaining, 44 minutes life estimate
> A/C adapter state: not connected
> Performance adjustment mode: auto (1000 MHz)
> Battery state: absent, 0% remaining, unknown life estimate
> A/C adapter state: not connected
> Performance adjustment mode: auto (1000 MHz)
> Battery state: absent, 0% remaining, unknown life estimate
> A/C adapter state: not connected
> Performance adjustment mode: auto (1000 MHz)
> Battery state: CRITICAL, 21% remaining, 44 minutes life estimate
> A/C adapter state: not connected
> Performance adjustment mode: auto (1000 MHz)
> Battery state: CRITICAL, 21% remaining, 44 minutes life estimate
> A/C adapter state: not connected
> Performance adjustment mode: auto (1000 MHz)
>
> The same happens when the AC adapter is connected:
>
> $ while true; do apm; sleep 1; done
> Battery state: absent, 0% remaining, unknown life estimate
> A/C adapter state: connected
> Performance adjustment mode: auto (1000 MHz)
> Battery state: CRITICAL, 20% remaining, unknown life estimate
> A/C adapter state: connected
> Performance adjustment mode: auto (1000 MHz)
> Battery state: CRITICAL, 20% remaining, unknown life estimate
> A/C adapter state: connected
> Performance adjustment mode: auto (1000 MHz)
> Battery state: absent, 0% remaining, unknown life estimate
> A/C adapter state: connected
> Performance adjustment mode: auto (1000 MHz)
> Battery state: CRITICAL, 20% remaining, unknown life estimate
> A/C adapter state: connected
> Performance adjustment mode: auto (1000 MHz)
> Battery state: CRITICAL, 20% remaining, unknown life estimate
> A/C adapter state: connected
> Performance adjustment mode: auto (1000 MHz)
> Battery state: CRITICAL, 20% remaining, unknown life estimate
> A/C adapter state: connected
> Performance adjustment mode: auto (1000 MHz)
>
> I've analysed the problem using printfs. The 0 reading occurs
> because the second of the following if-statements (the one testing
> the capacity) in acpiioctl evaluates to true every now and then:
>
> SLIST_FOREACH(bat, &sc->sc_bat, aba_link) {
> if (bat->aba_softc->sc_bat_present == 0)
> continue;
>
> if (bat->aba_softc->sc_bif.bif_last_capacity == 0)
> continue;
>
> bats++;
>
> Because the capacity for some reason is sometimes zero, bats is
> sometimes not incremented. Later on, this causes us to consider
> the battery absent:
>
> if (bats == 0) {
> pi->battery_state = APM_BATTERY_ABSENT;
> pi->battery_life = 0;
> pi->minutes_left = (unsigned int)-1;
> break;
> }
>
> Reverting acpibat.c to rev 1.49 made the issue go away.
>
> Looking at the diff between 1.49 and 1.50, some memset calls were
> moved around in acpibat_getbif and acpibat_getbst.
> The first thing these functions have been doing since rev 1.50 is
> zeroing out the data structure that acpiioctl is using to determine
> battery presence/capacity:
>
> int
> acpibat_getbif(struct acpibat_softc *sc)
> {
> struct aml_value res;
> int rv = EINVAL;
>
> memset(&sc->sc_bif, 0, sizeof(sc->sc_bif))
>
> int
> acpibat_getbst(struct acpibat_softc *sc)
> {
> struct aml_value res;
> int rv = EINVAL;
>
> memset(&sc->sc_bst, 0, sizeof(sc->sc_bst));
>
> With properly placed printfs, I could see that acpiioctl was
> sometimes reading the capacity after acpibat_getbif zeroed out
> sc->sc_bif, but before acpibat_getbif had a chance to populate
> the structure again with fresh readings from the BIOS.
> I guess this has to do with interrupt timings. Also note that
> I run an SMP kernel which may or may not exacerbate the issue,
> I don't know.
>
> The following patch fixes the issue for me. APM is reporting
> consistent values again if run in a loop, and also the xfce
> battery meter does not constantly jump to zero and back up again.
>
> I guess zeroing out these structures only in case the battery is
> absent is sufficient.
>
> Index: sys/dev/acpi/acpibat.c
> ===================================================================
> RCS file: /usr/OpenBSD-CVS/src/sys/dev/acpi/acpibat.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 acpibat.c
> --- sys/dev/acpi/acpibat.c 13 Jun 2008 05:50:21 -0000 1.50
> +++ sys/dev/acpi/acpibat.c 13 Jul 2008 21:37:02 -0000
> @@ -292,9 +292,10 @@ acpibat_getbif(struct acpibat_softc *sc)
> struct aml_value res;
> int rv = EINVAL;
>
> - memset(&sc->sc_bif, 0, sizeof(sc->sc_bif));
> - if (!sc->sc_bat_present)
> + if (!sc->sc_bat_present) {
> + memset(&sc->sc_bif, 0, sizeof(sc->sc_bif));
> return 0;
> + }
>
> if (aml_evalname(sc->sc_acpi, sc->sc_devnode, "_BIF", 0, NULL, &res)) {
> dnprintf(10, "%s: no _BIF\n", DEVNAME(sc));
> @@ -355,9 +356,10 @@ acpibat_getbst(struct acpibat_softc *sc)
> struct aml_value res;
> int rv = EINVAL;
>
> - memset(&sc->sc_bst, 0, sizeof(sc->sc_bst));
> - if (!sc->sc_bat_present)
> + if (!sc->sc_bat_present) {
> + memset(&sc->sc_bst, 0, sizeof(sc->sc_bst));
> return 0;
> + }
>
> if (aml_evalname(sc->sc_acpi, sc->sc_devnode, "_BST", 0, NULL, &res)) {
> dnprintf(10, "%s: no _BST\n", DEVNAME(sc));
>
>
>
> Stefan
>
> Here's my dmesg for reference:
>
> OpenBSD 4.4-beta (GENERIC.MP) #11: Sun Jul 13 23:50:00 CEST 2008
> stsp@xxxxxxxxxxxxxx:/usr/src/sys/arch/i386/compile/GENERIC.MP
> cpu0: Genuine Intel(R) CPU L2400 @ 1.66GHz ("GenuineIntel" 686-class) 1.67
> GHz
> cpu0:
> FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,CFLUSH,DS,A
> CPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,MWAIT,VMX,EST,TM2,xTPR
> real mem = 1063677952 (1014MB)
> avail mem = 1020248064 (972MB)
> mainbus0 at root
> bios0 at mainbus0: AT/286+ BIOS, date 03/31/08, BIOS32 rev. 0 @ 0xfd690,
> SMBIOS rev. 2.4 @ 0xe0010 (67 entries)
> bios0: vendor LENOVO version "7BETD5WW (2.16 )" date 03/31/2008
> bios0: LENOVO 170255G
> acpi0 at bios0: rev 2
> acpi0: tables DSDT FACP SSDT ECDT TCPA APIC MCFG HPET BOOT SSDT SSDT SSDT
> SSDT
> acpi0: wakeup devices LID_(S3) SLPB(S3) DURT(S3) EXP0(S4) EXP1(S4) EXP2(S4)
> EXP3(S4) PCI1(S4) USB0(S3) USB1(S3) USB2(S3) USB7(S3) HDEF(S4)
> acpitimer0 at acpi0: 3579545 Hz, 24 bits
> acpimadt0 at acpi0 addr 0xfee00000: PC-AT compat
> cpu0 at mainbus0: apid 0 (boot processor)
> cpu0: apic clock running at 166MHz
> cpu1 at mainbus0: apid 1 (application processor)
> cpu1: Genuine Intel(R) CPU L2400 @ 1.66GHz ("GenuineIntel" 686-class) 1.67
> GHz
> cpu1:
> FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,CFLUSH,DS,A
> CPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,MWAIT,VMX,EST,TM2,xTPR
> ioapic0 at mainbus0: apid 1 pa 0xfec00000, version 20, 24 pins
> ioapic0: duplicate apic id, remapped to apid 2
> acpihpet0 at acpi0: 14318179 Hz
> acpiprt0 at acpi0: bus 0 (PCI0)
> acpiprt1 at acpi0: bus -1 (AGP_)
> acpiprt2 at acpi0: bus 2 (EXP0)
> acpiprt3 at acpi0: bus 3 (EXP1)
> acpiprt4 at acpi0: bus 4 (EXP2)
> acpiprt5 at acpi0: bus 12 (EXP3)
> acpiprt6 at acpi0: bus 21 (PCI1)
> acpiec0 at acpi0
> acpicpu0 at acpi0: C3, C2
> acpicpu1 at acpi0: C3, C2
> acpitz0 at acpi0: critical temperature 127 degC
> acpitz1 at acpi0: critical temperature 97 degC
> acpibtn0 at acpi0: LID_
> acpibtn1 at acpi0: SLPB
> acpibat0 at acpi0: BAT0 model "92P1163" serial 1585 type LION oem "SANYO"
> acpibat1 at acpi0: BAT1 not present
> acpibat2 at acpi0: BAT2 not present
> acpiac0 at acpi0: AC unit offline
> acpithinkpad0 at acpi0
> acpidock at acpi0 not configured
> acpivideo at acpi0 not configured
> acpivideo at acpi0 not configured
> bios0: ROM list: 0xc0000/0xea00! 0xcf000/0x1000 0xd0000/0x1000 0xdc000/0x4000!
> 0xe0000/0x10000!
> cpu0: unknown Enhanced SpeedStep CPU, msr 0x06130a2206000613
> cpu0: using only highest and lowest power states
> cpu0: Enhanced SpeedStep 1000 MHz (1004 mV): speeds: 1667, 1000 MHz
> pci0 at mainbus0 bus 0: configuration mode 1 (no bios)
> pchb0 at pci0 dev 0 function 0 "Intel 82945GM Host" rev 0x03
> vga1 at pci0 dev 2 function 0 "Intel 82945GM Video" rev 0x03
> wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
> wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
> agp0 at vga1: aperture at 0xd0000000, size 0x10000000
> "Intel 82945GM Video" rev 0x03 at pci0 dev 2 function 1 not configured
> azalia0 at pci0 dev 27 function 0 "Intel 82801GB HD Audio" rev 0x02: apic 2
> int 17 (irq 11)
> azalia0: codec[s]: Analog Devices/0x1981, Conexant/0x2bfa, using Analog
> Devices/0x1981
> audio0 at azalia0
> ppb0 at pci0 dev 28 function 0 "Intel 82801GB PCIE" rev 0x02: apic 2 int 20
> (irq 11)
> pci1 at ppb0 bus 2
> em0 at pci1 dev 0 function 0 "Intel PRO/1000MT (82573L)" rev 0x00: apic 2 int
> 16 (irq 11), address 00:0a:e4:3e:f1:4e
> ppb1 at pci0 dev 28 function 1 "Intel 82801GB PCIE" rev 0x02: apic 2 int 21
> (irq 11)
> pci2 at ppb1 bus 3
> wpi0 at pci2 dev 0 function 0 "Intel PRO/Wireless 3945ABG" rev 0x02: apic 2
> int 17 (irq 11), MoW2, address 00:13:02:03:a5:e7
> ppb2 at pci0 dev 28 function 2 "Intel 82801GB PCIE" rev 0x02: apic 2 int 22
> (irq 11)
> pci3 at ppb2 bus 4
> ppb3 at pci0 dev 28 function 3 "Intel 82801GB PCIE" rev 0x02: apic 2 int 23
> (irq 11)
> pci4 at ppb3 bus 12
> uhci0 at pci0 dev 29 function 0 "Intel 82801GB USB" rev 0x02: apic 2 int 16
> (irq 11)
> uhci1 at pci0 dev 29 function 1 "Intel 82801GB USB" rev 0x02: apic 2 int 17
> (irq 11)
> uhci2 at pci0 dev 29 function 2 "Intel 82801GB USB" rev 0x02: apic 2 int 18
> (irq 11)
> uhci3 at pci0 dev 29 function 3 "Intel 82801GB USB" rev 0x02: apic 2 int 19
> (irq 11)
> ehci0 at pci0 dev 29 function 7 "Intel 82801GB USB" rev 0x02: apic 2 int 19
> (irq 11)
> usb0 at ehci0: USB revision 2.0
> uhub0 at usb0 "Intel EHCI root hub" rev 2.00/1.00 addr 1
> ppb4 at pci0 dev 30 function 0 "Intel 82801BAM Hub-to-PCI" rev 0xe2
> pci5 at ppb4 bus 21
> cbb0 at pci5 dev 0 function 0 "Ricoh 5C476 CardBus" rev 0xb4: apic 2 int 16
> (irq 11)
> "Ricoh 5C552 Firewire" rev 0x09 at pci5 dev 0 function 1 not configured
> sdhc0 at pci5 dev 0 function 2 "Ricoh 5C822 SD/MMC" rev 0x18: apic 2 int 18
> (irq 11)
> sdmmc0 at sdhc0
> cardslot0 at cbb0 slot 0 flags 0
> cardbus0 at cardslot0: bus 22 device 0 cacheline 0x0, lattimer 0xb0
> pcmcia0 at cardslot0
> ichpcib0 at pci0 dev 31 function 0 "Intel 82801GBM LPC" rev 0x02: PM disabled
> pciide0 at pci0 dev 31 function 1 "Intel 82801GB IDE" rev 0x02: DMA, channel 0
> configured to compatibility, channel 1 configured to compatibility
> pciide0: channel 0 disabled (no drives)
> pciide0: channel 1 ignored (disabled)
> ahci0 at pci0 dev 31 function 2 "Intel 82801GBM AHCI" rev 0x02: apic 2 int 16
> (irq 11), AHCI 1.1
> scsibus0 at ahci0: 32 targets, initiator 32
> sd0 at scsibus0 targ 0 lun 0: <ATA, FUJITSU MHV2080B, 0084> SCSI3 0/direct
> fixed
> sd0: 76319MB, 9729 cyl, 255 head, 63 sec, 512 bytes/sec, 156301488 sec total
> ichiic0 at pci0 dev 31 function 3 "Intel 82801GB SMBus" rev 0x02: apic 2 int
> 23 (irq 11)
> iic0 at ichiic0
> usb1 at uhci0: USB revision 1.0
> uhub1 at usb1 "Intel UHCI root hub" rev 1.00/1.00 addr 1
> usb2 at uhci1: USB revision 1.0
> uhub2 at usb2 "Intel UHCI root hub" rev 1.00/1.00 addr 1
> usb3 at uhci2: USB revision 1.0
> uhub3 at usb3 "Intel UHCI root hub" rev 1.00/1.00 addr 1
> usb4 at uhci3: USB revision 1.0
> uhub4 at usb4 "Intel UHCI root hub" rev 1.00/1.00 addr 1
> isa0 at ichpcib0
> isadma0 at isa0
> com1 at isa0 port 0x2f8/8 irq 3: ns16550a, 16 byte fifo
> pckbc0 at isa0 port 0x60/5
> pckbd0 at pckbc0 (kbd slot)
> pckbc0: using irq 1 for kbd slot
> wskbd0 at pckbd0: console keyboard, using wsdisplay0
> pms0 at pckbc0 (aux slot)
> pckbc0: using irq 12 for aux slot
> wsmouse0 at pms0 mux 0
> pcppi0 at isa0 port 0x61
> midi0 at pcppi0: <PC speaker>
> spkr0 at pcppi0
> aps0 at isa0 port 0x1600/31
> npx0 at isa0 port 0xf0/16: reported by CPUID; using exception 16
> mtrr: Pentium Pro MTRR support
> ral0 at cardbus0 dev 0 function 0 "Ralink RT2560" rev 0x01: irq 268570635,
> address 00:0e:2e:5c:55:4f
> ral0: MAC/BBP RT2560 (rev 0x04), RF RT2525
> ugen0 at uhub4 port 2 "STMicroelectronics Biometric Coprocessor" rev 1.00/0.01
> addr 2
> softraid0 at root
> root on sd0a swap on sd0b dump on sd0b
>
> [demime 1.01d removed an attachment of type application/pgp-signature]
|
|