> Date: Fri, 10 Jan 2020 13:37:38 -0500
> From: Bryan Steele <[email protected]>
> 
> On Fri, Jan 10, 2020 at 02:59:10AM -0500, James Hastings wrote:
> > > On Sat, Jan 04, 2020 at 06:23:44PM +0100, Mark Kettenis wrote:
> > > > Date: Sat, 4 Jan 2020 12:03:14 -0500
> > > > From: Bryan Steele <[email protected]>
> > > > 
> > > > On Sat, Jan 04, 2020 at 11:30:56AM -0500, Bryan Steele wrote:
> > > > > On Sat, Jan 04, 2020 at 05:07:04PM +0100, Mark Kettenis wrote:
> > > > > > > Date: Sat, 4 Jan 2020 10:52:24 -0500
> > > > > > > From: Bryan Steele <[email protected]>
> > > > > > > 
> > > > > > > I noticed an unusually high interrupt rate for amdgpio0 on my 
> > > > > > > Huawei
> > > > > > > Matebook D laptop. I'm suspecting this may be partially why it 
> > > > > > > apmd -A
> > > > > > > has been struggling, as the CPU is constantly busy so it never 
> > > > > > > has a
> > > > > > > chance to scale down.
> > > > > > > 
> > > > > > > Any ideas?
> > > > > > 
> > > > > > Please send acpidump output (all files in /var/db/acpi).
> > > > > > 
> > > > > > Try to figure out which GPIO pin is causing the interrupt.  That may
> > > > > > be tricky since the interrupt fires again and again, so if you add a
> > > > > > printf in amdgpio_intr() your machine will become unusable.  Maybe
> > > > > > just print the pin every 10000 times:
> > > > > > 
> > > > > >     static count = 0;
> > > > > > 
> > > > > >     ....
> > > > > > 
> > > > > >     if ((count++ % 10000) == 0)
> > > > > >         printf("st %llx\n", status)
> > > > > > 
> > > > > > Cheers,
> > > > > > 
> > > > > > Mark
> > > > > 
> > > > > Thanks, for some reason it vanished with on a cold boot. I'm not sure
> > > > > what it was that caused it. In case it helps, sending the acpidump
> > > > > anyway.
> > > > > 
> > > > > If I can figure out what caused it, I'll try your other suggestion.
> > > > > 
> > > > > Sigh.. :-)
> > > > 
> > > > Aha! I must of accidentally bumped the Touchscreen at some point, doing
> > > > that causes the amdgpio0 rate to spike.
> > > > 
> > > > I had sent a diff to add AMD controller support to dwiic(4) months ago,
> > > > I could never get interrupts to work, only polling mode. Perhaps this
> > > > issue explains some of that. I don't have this diff in my tree at the
> > > > moment, had to restore from backup.
> > > > 
> > > > Managed to login and type shutdown, lol.
> > > > 
> > > > ..
> > > > st 1f00000000000008
> > > 
> > > Very likely.  The AML defines an I2C device with:
> > > 
> > >            Name (_DDN, "Raydium Touchscreen")  // _DDN: DOS Device Name
> > > 
> > > that uses a GPIO interrupt that matches the lowest bit set in the
> > > status register.
> > >
> > > This suggest we may need to be a little bit more careful and mask
> > > interrupts for which we don't have an interrupt handler.
> > That is my fault. Does this diff stop your interrupt storm?
> 
> Yes. This works, thanks!
> 
> ok brynet@ (if Mark agrees)

I fear that this isn't the right approach.  Some of the GPIO pins
might be used for SMIs.  And it isn't clear to me whether disabling
interrupts will also stop SMIs from being generated.

A better strategy would be to have the interrupt handler disable pins
for which it sees an interrupt pending when there is no interrupt
handler registered.  I believe that is what Linux does.

James, is that something you'd like to work on?

Also, I don't think that this will fix the issue that claudo@ and I
are seeing on the m715q.  There I'm starting to suspect that the
problem is that the interrupt is shared with a quirky PCI device.
There a BIOS update (which stops amdgpio(4) from attaching) may
actually be the only reasonable fix.

Cheers,

Mark

> > Index: dev/acpi/amdgpio.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/acpi/amdgpio.c,v
> > retrieving revision 1.1
> > diff -u -p -r1.1 amdgpio.c
> > --- dev/acpi/amdgpio.c      23 Dec 2019 08:05:42 -0000      1.1
> > +++ dev/acpi/amdgpio.c      10 Jan 2020 07:30:32 -0000
> > @@ -108,6 +108,8 @@ amdgpio_attach(struct device *parent, st
> >     struct amdgpio_softc *sc = (struct amdgpio_softc *)self;
> >     struct aml_value res;
> >     int64_t uid;
> > +   uint32_t reg;
> > +   int i;
> >  
> >     sc->sc_acpi = (struct acpi_softc *)parent;
> >     sc->sc_node = aaa->aaa_node;
> > @@ -156,6 +158,16 @@ amdgpio_attach(struct device *parent, st
> >         &sc->sc_memh)) {
> >             printf(", can't map registers\n");
> >             goto free;
> > +   }
> > +
> > +   /* Mask and clear all interrupts */
> > +   for (i = 0; i < sc->sc_npins; i++) {
> > +           if (i == 63)
> > +                   continue;
> > +           reg = bus_space_read_4(sc->sc_memt, sc->sc_memh, i * 4);
> > +           reg &= ~(AMDGPIO_CONF_INT_MASK | AMDGPIO_CONF_INT_EN);
> > +           reg |= AMDGPIO_CONF_INT_STS;
> > +           bus_space_write_4(sc->sc_memt, sc->sc_memh, i * 4, reg);
> >     }
> >  
> >     sc->sc_ih = acpi_intr_establish(sc->sc_irq, sc->sc_irq_flags, IPL_BIO,
> > 
> > 
> 
> 

Reply via email to