Re: [acpi] patch to run _INI methods before table devices attach
On Sat, Oct 22, 2022 at 01:41:23PM +0200, Mark Kettenis wrote: > Identifying which of the "table-driven" device attachments is the > culprit here might help. I have following tables: acpi0: tables DSDT FACP UEFI SSDT SSDT SSDT SSDT SSDT SSDT TPM2 MSDM NHLT SSDT LPIT WSMT SSDT SSDT DBGP DBG2 ECDT HPET APIC MCFG SSDT DMAR SSDT SSDT FPDT PTDT BGRT if I run aml_find_node(sc->sc_root, "_INI", acpi_inidev, sc); here: DSDT FACP UEFI SSDT SSDT SSDT SSDT SSDT SSDT TPM2 MSDM NHLT SSDT LPIT WSMT SSDT SSDT DBGP DBG2 ECDT HPET APIC MCFG SSDT DMAR SSDT SSDT FPDT PTDT BGRT ^ brightness keys works, if here (right before ECDT): DSDT FACP UEFI SSDT SSDT SSDT SSDT SSDT SSDT TPM2 MSDM NHLT SSDT LPIT WSMT SSDT SSDT DBGP DBG2 ECDT HPET APIC MCFG SSDT DMAR SSDT SSDT FPDT PTDT BGRT ^ still works, but if I run it here (after ECDT): DSDT FACP UEFI SSDT SSDT SSDT SSDT SSDT SSDT TPM2 MSDM NHLT SSDT LPIT WSMT SSDT SSDT DBGP DBG2 ECDT HPET APIC MCFG SSDT DMAR SSDT SSDT FPDT PTDT BGRT ^ it doesn't work.
Re: [acpi] patch to run _INI methods before table devices attach
On Sun, Oct 23, 2022 at 05:51:41PM +0300, Mikhail wrote:
> On Sun, Oct 23, 2022 at 08:32:47AM -0600, Chris Waddey wrote:
> > So I've been following the work on this because I have a lenovo laptop
> > as well and the first patch you sent in I applied and have had
> > everything working that I expect to (battery status/apm all works
> > great, as well as brightness keys and -- surprise -- volume
> > keys). Mark's patch that he suggested instead of this one did not
> > produce any meaningful difference from -current.
> >
> > I'll try the newer patches and let you know how it goes. I also have
> > an HP desktop that I can try it on at home to see if it breaks anything.
>
> > acpiec0 at acpi0: not present
> [...]
> > acpiec1 at acpi0
>
> Based on your dmesg I think you have the same problem as I do - ECDT has
> wrong device name in its EC_ID.
>
> To check this you can acpidump(8) your tables, then decompile AML with
> 'iasl -d DSDT.xxx' and 'iasl -d ECDT.xxx' (iasl is in 'acpica' package),
> after this take a look in ECDT.dsl, you need 'Namepath' field, for me
> it's "\_SB.PC00.LPCB.H_EC", but actual device is "\_SB.PC00.LPCB.EC0" -
> this one you can lookup in DSDT.dsl, try searching for something like
> "Device (EC0)".
>
> If this mismatch of ECDT and DSDT is your case - you can report it to
> Lenovo, as I did, maybe they will fix the BIOS.
It does appear that I have the same mismatch. I have exactly
"\_SB.PC00.LPCB.H_EC" in the namepath field of my ECDT.dsl, but no
mentions of the same in DSDT.dsl. There are, however, many mentions of
"\_SB.PC00.LPCB.EC0" in DSDT.dsl, especially under the "Device (ECO)"
section.
> Also, I'd be appreciated if you come up with the results to tech@, so
> Mark and others can see that it's not only my problem.
CC'ing tech@.
I've applied the following two patches and apm give this output:
Battery state: charging, 57% remaining, 35 minutes recharge time estimate
AC adapter state: connected
Performance adjustment mode: auto (2401 MHz)
or
Battery state: high, 57% remaining, 190 minutes life estimate
AC adapter state: not connected
Performance adjustment mode: auto (400 MHz)
depending on if I'm charging or not. I also have working brightness
and volume keys still.
diff /usr/src
commit - 5cb1d9dce18f152bf9f5d7d4251dafe18a5c0c82
path + /usr/src
blob - 5ef24d5179de52d5321e578b3b73dd9524e7c1de
file + sys/dev/acpi/acpiec.c
--- sys/dev/acpi/acpiec.c
+++ sys/dev/acpi/acpiec.c
@@ -275,14 +275,13 @@ acpiec_attach(struct device *parent, struct device *se
struct acpiec_softc *sc = (struct acpiec_softc *)self;
struct acpi_attach_args *aa = aux;
struct aml_value res;
- int64_t st;
+ int64_t st = 0;
sc->sc_acpi = (struct acpi_softc *)parent;
sc->sc_devnode = aa->aaa_node;
sc->sc_cantburst = 0;
- if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "_STA", 0, NULL, ))
- st = STA_PRESENT | STA_ENABLED | STA_DEV_OK;
+ aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "_STA", 0, NULL, );
if ((st & STA_PRESENT) == 0) {
printf(": not present\n");
return;
diff /usr/src
commit - 3fb2197480c345a19f2098d1b787c28b9c717dcb
path + /usr/src
blob - fc3c1d160ee0ccc7217d77be184c253b29422983
file + sys/dev/acpi/acpi.c
--- sys/dev/acpi/acpi.c
+++ sys/dev/acpi/acpi.c
@@ -1203,6 +1203,9 @@ acpi_attach_common(struct acpi_softc *sc, paddr_t base
}
#endif /* SMALL_KERNEL */
+ /* initialize runtime environment */
+ aml_find_node(sc->sc_root, "_INI", acpi_inidev, sc);
+
/*
* Attach table-defined devices
*/
@@ -1217,9 +1220,6 @@ acpi_attach_common(struct acpi_softc *sc, paddr_t base
config_found_sm(>sc_dev, , acpi_print, acpi_submatch);
}
- /* initialize runtime environment */
- aml_find_node(sc->sc_root, "_INI", acpi_inidev, sc);
-
/* Get PCI mapping */
aml_walknodes(sc->sc_root, AML_WALK_PRE, acpi_getpci, sc);
dmesg:
OpenBSD 7.2-current (GENERIC.MP) #9: Sun Oct 23 09:16:32 MDT 2022
me@thief.private:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 8346333184 (7959MB)
avail mem = 8075952128 (7701MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.3 @ 0x439de000 (69 entries)
bios0: vendor LENOVO version "FHCN54WW" date 07/05/2021
bios0: LENOVO 82FG
efi0 at bios0: UEFI 2.7
efi0: INSYDE Corp. rev 0x59474054
acpi0 at bios0: ACPI 6.1Undefined scope: \\_SB_.PCI0
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP UEFI SSDT SSDT SSDT SSDT SSDT MSDM NHLT SSDT LPIT WSMT
SSDT SSDT DBGP DBG2 ECDT HPET APIC MCFG SSDT DMAR SSDT SSDT SSDT FPDT PTDT BGRT
acpi0: wakeup devices PEG0(S4) PEGP(S4) PEGA(S4) PEGP(S4) PEGP(S4) XHCI(S4)
XDCI(S4) HDAS(S4) RP01(S4) PXSX(S4) RP02(S4) PXSX(S4) RP03(S4) PXSX(S4)
RP04(S4) PXSX(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpiec0 at acpi0: not present
acpihpet0
Re: [acpi] patch to run _INI methods before table devices attach
On Sat, Oct 22, 2022 at 01:41:23PM +0200, Mark Kettenis wrote:
> > I continue my fight with ACPI on Lenovo IdeaPad 3 14ITL05, now, when
> > battery status problem has been resolved[1] next target is backlight
> > brightness keys.
>
> Well it hasn't been resolved yet. The firmware is clearly broken, and
> we still have to find a workaround that doesn't break other
> machines...
If I replace
sc->sc_devnode = aml_searchname(sc->sc_acpi->sc_root, ecdt->ec_id);
with
sc->sc_devnode = aml_searchname(sc->sc_acpi->sc_root, "\\_SB.PC00.LPCB.EC0");
(Firmware has "\_SB.PC00.LPCB.H_EC", and that's what I meant by
"explicitly specified correct EC_ID")
in this code:
430 /* Check if this is ECDT initialization */
431 if (ecdt) {
432 /* Get GPE, Data and Control segments */
433 sc->sc_gpe = ecdt->gpe_bit;
434
435 if (ecdt->ec_control.address_space_id == GAS_SYSTEM_IOSPACE)
436 sc->sc_cmd_bt = sc->sc_acpi->sc_iot;
437 else
438 sc->sc_cmd_bt = sc->sc_acpi->sc_memt;
439 sc->sc_ec_sc = ecdt->ec_control.address;
440
441 if (ecdt->ec_data.address_space_id == GAS_SYSTEM_IOSPACE)
442 sc->sc_data_bt = sc->sc_acpi->sc_iot;
443 else
444 sc->sc_data_bt = sc->sc_acpi->sc_memt;
445 sc->sc_ec_data = ecdt->ec_data.address;
446
447 /* Get devnode from header */
448 sc->sc_devnode = aml_searchname(sc->sc_acpi->sc_root,
449 ecdt->ec_id);
450
451 goto ecdtdone;
452 }
I can see battery status, so only firmware has to be patched, no other
modifications to OpenBSD are necessary, ECDT attach will work,
brightness problem looks like a different one.
Lenovo guy told that he will "inform needed departments" regarding wrong
EC_ID.
> > With the inlined patch and explicitly specified correct EC_ID in the
> > code I'm able to control brightness with functional keys. The patch
> > moves _INI methods evaluation "immediately before" table-defined device
> > attach, before the patch it was "immediately after".
> >
> > Only reference which I found in the spec is [2], and it doesn't make it
> > clear when _INI should be eval'ed: "This control method is located under
> > a device object and is run only when OSPM loads a description table."
> > Another reference in the same chapter: "_INI - Device initialization
> > method that is run shortly after ACPI has been enabled." Term "shortly"
> > isn't very precise description of when to run _INI either.
>
> Welcome to ACPI. More than 1000 pages of documentation and still
> woefully underspecified.
>
> Your interpretation doesn't make a lot of sense to me. The whole
> purpose of the tables is to have certain things (such as access to PCI
> config space or accessing the Embedded Controller) work before running
> any of the AML methods.
>
> > I understand that this isn't very grown up engineering attitude "I do
> > this and everything works", but currently I lack explanation why this
> > approach helps. If anyone has ideas how to investigate this further - I
> > will be appreciated.
>
> Identifying which of the "table-driven" device attachments is the
> culprit here might help.
Thank you, I'll give it a shot on a workweek.
Re: [acpi] patch to run _INI methods before table devices attach
> Date: Sat, 22 Oct 2022 11:26:31 +0300 > From: Mikhail > > I continue my fight with ACPI on Lenovo IdeaPad 3 14ITL05, now, when > battery status problem has been resolved[1] next target is backlight > brightness keys. Well it hasn't been resolved yet. The firmware is clearly broken, and we still have to find a workaround that doesn't break other machines... > With the inlined patch and explicitly specified correct EC_ID in the > code I'm able to control brightness with functional keys. The patch > moves _INI methods evaluation "immediately before" table-defined device > attach, before the patch it was "immediately after". > > Only reference which I found in the spec is [2], and it doesn't make it > clear when _INI should be eval'ed: "This control method is located under > a device object and is run only when OSPM loads a description table." > Another reference in the same chapter: "_INI - Device initialization > method that is run shortly after ACPI has been enabled." Term "shortly" > isn't very precise description of when to run _INI either. Welcome to ACPI. More than 1000 pages of documentation and still woefully underspecified. Your interpretation doesn't make a lot of sense to me. The whole purpose of the tables is to have certain things (such as access to PCI config space or accessing the Embedded Controller) work before running any of the AML methods. > I understand that this isn't very grown up engineering attitude "I do > this and everything works", but currently I lack explanation why this > approach helps. If anyone has ideas how to investigate this further - I > will be appreciated. Identifying which of the "table-driven" device attachments is the culprit here might help. > If your brightness keys or other ACPI stuff doesn't work, you can give > this patch a try. > > Will be grateful for testing and reviews. > > [1] - https://marc.info/?l=openbsd-tech=166600434429788=2 > [2] - https://uefi.org/specs/ACPI/6.5/06_Device_Configuration.html#ini-init > > diff /usr/src > commit - 3fb2197480c345a19f2098d1b787c28b9c717dcb > path + /usr/src > blob - fc3c1d160ee0ccc7217d77be184c253b29422983 > file + sys/dev/acpi/acpi.c > --- sys/dev/acpi/acpi.c > +++ sys/dev/acpi/acpi.c > @@ -1203,6 +1203,9 @@ acpi_attach_common(struct acpi_softc *sc, paddr_t base > } > #endif /* SMALL_KERNEL */ > > + /* initialize runtime environment */ > + aml_find_node(sc->sc_root, "_INI", acpi_inidev, sc); > + > /* >* Attach table-defined devices >*/ > @@ -1217,9 +1220,6 @@ acpi_attach_common(struct acpi_softc *sc, paddr_t base > config_found_sm(>sc_dev, , acpi_print, acpi_submatch); > } > > - /* initialize runtime environment */ > - aml_find_node(sc->sc_root, "_INI", acpi_inidev, sc); > - > /* Get PCI mapping */ > aml_walknodes(sc->sc_root, AML_WALK_PRE, acpi_getpci, sc); > > >
[acpi] patch to run _INI methods before table devices attach
I continue my fight with ACPI on Lenovo IdeaPad 3 14ITL05, now, when battery status problem has been resolved[1] next target is backlight brightness keys. With the inlined patch and explicitly specified correct EC_ID in the code I'm able to control brightness with functional keys. The patch moves _INI methods evaluation "immediately before" table-defined device attach, before the patch it was "immediately after". Only reference which I found in the spec is [2], and it doesn't make it clear when _INI should be eval'ed: "This control method is located under a device object and is run only when OSPM loads a description table." Another reference in the same chapter: "_INI - Device initialization method that is run shortly after ACPI has been enabled." Term "shortly" isn't very precise description of when to run _INI either. I understand that this isn't very grown up engineering attitude "I do this and everything works", but currently I lack explanation why this approach helps. If anyone has ideas how to investigate this further - I will be appreciated. If your brightness keys or other ACPI stuff doesn't work, you can give this patch a try. Will be grateful for testing and reviews. [1] - https://marc.info/?l=openbsd-tech=166600434429788=2 [2] - https://uefi.org/specs/ACPI/6.5/06_Device_Configuration.html#ini-init diff /usr/src commit - 3fb2197480c345a19f2098d1b787c28b9c717dcb path + /usr/src blob - fc3c1d160ee0ccc7217d77be184c253b29422983 file + sys/dev/acpi/acpi.c --- sys/dev/acpi/acpi.c +++ sys/dev/acpi/acpi.c @@ -1203,6 +1203,9 @@ acpi_attach_common(struct acpi_softc *sc, paddr_t base } #endif /* SMALL_KERNEL */ + /* initialize runtime environment */ + aml_find_node(sc->sc_root, "_INI", acpi_inidev, sc); + /* * Attach table-defined devices */ @@ -1217,9 +1220,6 @@ acpi_attach_common(struct acpi_softc *sc, paddr_t base config_found_sm(>sc_dev, , acpi_print, acpi_submatch); } - /* initialize runtime environment */ - aml_find_node(sc->sc_root, "_INI", acpi_inidev, sc); - /* Get PCI mapping */ aml_walknodes(sc->sc_root, AML_WALK_PRE, acpi_getpci, sc);
