Opened 10 years ago

Closed 9 years ago

#5838 closed bug (fixed)

Enabled ACPI , but cant install Battery Bar Replicant in tray.

Reported by: streak Owned by: czeidler
Priority: normal Milestone: R1/beta1
Component: Drivers/ACPI Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

On hrev36511 i Enabled ACPI , but cant install Battery Bar Replicant in tray.

My Laptop -> MSI Wind.

Attachments (2)

syslog.txt (487.9 KB ) - added by streak 10 years ago.
syslog (413.4 KB ) - added by streak 10 years ago.

Download all attachments as: .zip

Change History (27)

comment:1 by streak, 10 years ago

Component: - GeneralDrivers/ACPI

comment:2 by streak, 10 years ago

Milestone: R1R1/alpha2
Version: R1/alpha1

comment:3 by anevilyak, 10 years ago

Can you provide a syslog?

comment:4 by anevilyak, 10 years ago

Version: R1/Development

comment:5 by streak, 10 years ago

Ill add a syslog soon.

btw. Running CPU Freq gives me trip to KDL without keyboard debugging possibility ..

comment:6 by anevilyak, 10 years ago

Owner: changed from nobody to tqh
Status: newassigned

by streak, 10 years ago

Attachment: syslog.txt added

comment:7 by tqh, 10 years ago

Can you please update your tree and rebuild Haiku and try again?

To me it looks like you are using something before hrev36511 or that the build isn't correct. Also check that 'svn status' don't mention anything with acp_embedded_controller.

comment:8 by streak, 10 years ago

The hrev36511 build was taken from haiku-files official site..

by streak, 10 years ago

Attachment: syslog added

comment:9 by streak, 10 years ago

New syslog from hrev36601 , still the same. Battery bar not installing info tray, acpi not work..

comment:10 by tqh, 10 years ago

Does it power off on shutdown?

Could you also attach the output of 'more -f /dev/acpi/namespace' if it works?

comment:11 by tqh, 10 years ago

If CPU freq gives you kdl, could try grab a photo of what is there?

comment:12 by streak, 10 years ago

Yes. Im getting a KDL after entering into CPU Freq: here's the image. I didnt add it as an attachment because it was too big > 1.5mb

link to imageshack -> http://img16.imageshack.us/img16/4811/cpufreqerror.jpg

"Does it power off on shutdown?" Yes it does.

comment:13 by tqh, 10 years ago

Owner: changed from tqh to czeidler

This seems to be a problem with CPU Frequency driver. It crashes here: source:/haiku/trunk/src/add-ons/kernel/drivers/power/enhanced_speedstep/frequency.cpp#L34

comment:14 by phoudoin, 10 years ago

Currently the driver supports only EIST control & status registers addresses and freq/states values hardcoded.

But here it sounds like EIST set and get state MSR registers are not located/setup by MSI bios at default addresses 0x198 and 0x199. These locations should probably be retrieved from _PCT ACPI object instead, as the supported freq/states pairs, in _PSS ACPI object. In particular when ACPI 2.0 is in used, like with this MSI Wind.

in reply to:  14 ; comment:15 by bonefish, 10 years ago

Replying to phoudoin:

Currently the driver supports only EIST control & status registers addresses and freq/states values hardcoded.

But here it sounds like EIST set and get state MSR registers are not located/setup by MSI bios at default addresses 0x198 and 0x199. These locations should probably be retrieved from _PCT ACPI object instead, as the supported freq/states pairs, in _PSS ACPI object. In particular when ACPI 2.0 is in used, like with this MSI Wind.

Not sure what you're talking about here. My knowledge regarding ACPI is close to non-existent. I do, however, have the "Intel® 64 and IA-32 Architectures, Software Developer’s Manual, Volume 3A: System Programming Guide, Part 1, June 2009" at hand, which in chapter 14 details on Power and Thermal Management. To my knowledge MSRs always have fixed indexes. For the three concerned ones the following table gives the names of the registers as per spec, the corresponding macro names in the driver (unfortunately not matching the spec name), the register indexes, and specification sections of definition for the indexes:

Name (Spec)Name (Driver)Register IndexDefined in Spec Section
IA32_PERF_STATUSMSR_GET_FREQ_STATE0x19814.3.2.2
IA32_PERF_CTLMSR_SET_FREQ_STATE0x19914.3.2.2
IA32_MISC_ENABLEMSR_MISC0x1a08.7.8

After a quick look at the driver I see at least two issues:

  • est_open() enables SpeedStep only on the current logical CPU, not on all CPUs. At least after a quick scan of the specs I find no indication that this has not to be done for all CPUs. That would at least explain an exception (probably #GP; the panic message is unfortunately missing in the screen shot) -- when writing to IA32_PERF_CTL on a CPU that doesn't have the feature enabled.
  • The driver seems to be ignorant where multi-threading is concerned. I see no locking at all, and I have the feeling that e.g. est_set_id16() should really not be executed by two threads at the same time. Since interrupts are enabled a thread might even change the CPU while executing the function, which definitely is a problem, if the MSRs are really per-CPU.

comment:16 by czeidler, 10 years ago

Yes only one cpu is assumed in the driver. I think also the threading point is an issue. The problem is that I don't have the cpu any more, I have an AMD now. When I checked some points on my todo list, like make ACPI working on my machine, I may start to take a look at an ACPI implementation of this driver... So feel free to step into it!

comment:17 by tqh, 10 years ago

Have you checked if it starts booting with ACPI now. I've fixed some issues that prevented booting on some machines (i.e don't use condvars and sleep that early in the boot process).

comment:18 by czeidler, 10 years ago

Yes tried it but it still hangs at the disk icon. Can't even enter KDL. Maybe its some hardware quirk at least mmlr assumed it with regard to my also buggy usb. Will debug it after I implement the radeon_hd mode setting correctly...

in reply to:  15 comment:19 by phoudoin, 10 years ago

Not sure what you're talking about here.

Me neither, anymore :-)

To my knowledge MSRs always have fixed indexes.

True. But on multi-CPUs, the driver should read & write the good registers, hence why ACPI 2.0 introduced the possibility to publish via _PCT object table memory mapped or IO addresses for these performance control & status registers, which allow a driver to modify them without having to run on the CPU. FreeBSD's acpi_perf.c and Linux's processor_perflib.c uses them when they're available.

I suspect the enhanced_speedstep driver, assuming only one CPU, read or write perf MSRs on the wrong CPU, while only the one who ran est_open() have EST enabled, without any warranty they're the same one...

comment:20 by axeld, 10 years ago

What about disabling the driver when it runs on an SMP system for now?

in reply to:  20 comment:21 by phoudoin, 10 years ago

Replying to axeld:

What about disabling the driver when it runs on an SMP system for now?

+1

Except if it could be fixed for r1a2, aka... yesterday :-)

in reply to:  20 comment:22 by bonefish, 10 years ago

Replying to axeld:

What about disabling the driver when it runs on an SMP system for now?

Done in hrev36726 (trunk).

comment:23 by scottmc, 9 years ago

Milestone: R1/alpha3R1/beta1

Any updates on this one?

comment:24 by scottmc, 9 years ago

Is this related to #1492?

comment:25 by czeidler, 9 years ago

Resolution: fixed
Status: assignedclosed

Does the main battery problem still exist? will close it for now. Hopefully fixed with the latest acpi updates...

Note: See TracTickets for help on using tickets.