From: Jeremie Courreges-Anglas <jca@wxcvbn.org>
Subject: Re: diff: trigger acpiac_refresh when acpibat notification
To: YASUOKA Masahiko <yasuoka@openbsd.org>
Cc: jcs@jcs.org, tech@openbsd.org
Date: Fri, 29 Nov 2024 18:26:32 +0100

On Fri, Nov 29, 2024 at 12:19:22AM +0900, YASUOKA Masahiko wrote:
> Let me ping this diff.
> The diff becomes more important when we have hw.perfpolicy for me.
> 
> ok? comments?

Seems like a pragmatic approach, but please see comments inline.

> On Thu, 17 Aug 2023 16:12:07 +0900 (JST)
> YASUOKA Masahiko <yasuoka@openbsd.org> wrote:
> > Hi,
> > 
> > Update the AC status when the battery notification is happened.
> > Because the AC status notification doesn't happen on some machines.
> > My vaio actually has this problem.
> > 
> > Also Linux is doing the same thing
> > 
> > https://github.com/torvalds/linux/blob/v6.4/drivers/acpi/ac.c#L165-L183
> 
> Index: sys/dev/acpi/acpiac.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/acpiac.c,v
> diff -u -p -r1.36 acpiac.c
> --- sys/dev/acpi/acpiac.c	6 Apr 2022 18:59:27 -0000	1.36
> +++ sys/dev/acpi/acpiac.c	28 Nov 2024 15:17:44 -0000
> @@ -140,6 +140,8 @@ acpiac_getpsr(struct acpiac_softc *sc)
>  	return (0);
>  }
>  
> +static int acpiac_notify_triggered = 0;
> +
>  int
>  acpiac_notify(struct aml_node *node, int notify_type, void *arg)
>  {
> @@ -148,6 +150,8 @@ acpiac_notify(struct aml_node *node, int
>  	dnprintf(10, "acpiac_notify: %.2x %s\n", notify_type,
>  	    DEVNAME(sc));
>  
> +	acpiac_notify_triggered = 1;
> +
>  	switch (notify_type) {
>  	case 0x00:
>  	case 0x01:

Here's the full function body,
---
int
acpiac_notify(struct aml_node *node, int notify_type, void *arg)
{
        struct acpiac_softc *sc = arg;

        dnprintf(10, "acpiac_notify: %.2x %s\n", notify_type,
            DEVNAME(sc));

        switch (notify_type) {
        case 0x00:
        case 0x01:
        case 0x81:
                /*
                 * XXX some sony vaio's use the wrong notify type
                 * work around it by honoring it as a 0x80
                 */
                /* FALLTHROUGH */
        case 0x80:
                acpiac_refresh(sc);
                acpi_record_event(sc->sc_acpi, APM_POWER_CHANGE);
                dnprintf(10, "A/C status: %d\n", sc->sc_ac_stat);
                break;
        }
        return (0);
}
---

Maybe your Vaio uses yet another notification type?  Did you try a
kernel built with ACPI_DEBUG?

> @@ -164,4 +168,22 @@ acpiac_notify(struct aml_node *node, int
>  		break;
>  	}
>  	return (0);
> +}
> +
> +void
> +acpiac_battery_notify(void)
> +{
> +	struct acpi_softc *sc = acpi_softc;
> +	struct acpi_ac *ac;
> +
> +	if (acpiac_notify_triggered)
> +		return;
> +	/*
> +	 * On some machines (vaio VJPK23 at least) AC status notifications
> +	 * are not triggered.  Update the AC status when battery notifications.

s/when/on/ ?

> +	 */
> +	SLIST_FOREACH(ac, &sc->sc_ac, aac_link) {
> +		acpiac_refresh(ac->aac_softc);
> +		acpi_record_event(sc, APM_POWER_CHANGE);
> +	}
>  }
> Index: sys/dev/acpi/acpibat.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/acpibat.c,v
> diff -u -p -r1.72 acpibat.c
> --- sys/dev/acpi/acpibat.c	5 Aug 2024 18:37:29 -0000	1.72
> +++ sys/dev/acpi/acpibat.c	28 Nov 2024 15:17:44 -0000
> @@ -540,5 +540,7 @@ acpibat_notify(struct aml_node *node, in
>  	acpibat_refresh(sc);
>  	acpi_record_event(sc->sc_acpi, APM_POWER_CHANGE);
>  
> +	acpiac_battery_notify();

I'd suggest moving the acpiac_battery_notify() call between
acpibat_refresh() and acpi_record_event().  Thay way you don't need to
send an extra APM_POWER_CHANGE event from the acpiac_battery_notify()
function.

> +
>  	return (0);
>  }
> Index: sys/dev/acpi/acpidev.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/acpidev.h,v
> diff -u -p -r1.45 acpidev.h
> --- sys/dev/acpi/acpidev.h	6 Aug 2024 17:38:56 -0000	1.45
> +++ sys/dev/acpi/acpidev.h	28 Nov 2024 15:17:44 -0000
> @@ -307,6 +307,8 @@ struct acpiac_softc {
>  	struct ksensordev	sc_sensdev;
>  };
>  
> +void acpiac_battery_notify(void);
> +
>  struct acpibat_softc {
>  	struct device		sc_dev;
>  
> 

-- 
jca