Re: вопрос по suspend/resume в goodix

[Hans de Goede <hdegoede@xxxxxxxxxx>]

Hi,

On 20-12-2019 16:40, Dmitry Mastykin wrote:
> Thank you Hans!
>
> Your explanations were extremely useful!
> Manufacturer answered, he had an error in pin numbers.
>
> I've changed table to:
> --------------------
> Name (RBUF, ResourceTemplate ()
> {
> I2cSerialBusV2 (0x0014, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\_SB.PCI0.I2C3",
> 0x00, ResourceConsumer, , Exclusive,
> )
> GpioInt (Edge, ActiveHigh, Shared, PullDefault, 0x0000,
> "\\_SB.GPO0", 0x00, ResourceConsumer, ,
> )
> { // Pin list
> 0x0015
> }
> GpioIo (Shared, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
> "\\_SB.GPO0", 0x00, ResourceConsumer, ,
> )
> { // Pin list
> 0x0019
> }
> })
> Now the I2C address is stably detected. But interrupt doesn't work:
> Goodix-TS i2c-GDIX1002:00: request IRQ failed: -1

Right, because the GPIO pin for the touchscreen irq pin is still
in direct-irq mode (as it was set by the BIOS), so we cannot use it
as GPIO. I've just received another bugreport about a similar problem
on another Apollo Lake device with Goodix touchscreen:

https://bugzilla.redhat.com/show_bug.cgi?id=1786317

This is about the Trekstor S11B and there the ACPI bits describing
the touchscreen has the following methods: INTI INTO and INTS
to respectively put the interrupt pin in input mode, put it in output
mode (and pass a value to output) and to get its current value.
See the Trekstor S11B acpidump attached to the bugzilla link above.

I can see the same methods in the DSDT-s of all Cherry Trail devices
with a goodix touchscreen I have, but those describe the irq as a
GpioInt, so there we can also just use normal gpio accesses instead
of poking the ACPI methods. Still we probably need to add support for
the ACPI methods for the Trekstor S11B case.

The combination of using a direct-irq enabled GPIO for the irq +
listing 2 normal GpioIo resources as your device does is seen on
Bay Trail devices, but is normally not used on anything newer and
on Bay Trail devices the touchscreen just stays awake during suspend
and still works afterwards. I don't really see a clean way to
support that weird combo (without the ACPI methods).

Long story short I don't really see a way to make things work
properly OOTB with your specific device. But I'm working on a

Regards,

Hans











> --------------------
>
> If I use:
> --------------------
> I2cSerialBusV2 (0x0014, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\_SB.PCI0.I2C3",
> 0x00, ResourceConsumer, , Exclusive,
> )
> GpioIo (Shared, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
> "\\_SB.GPO0", 0x00, ResourceConsumer, ,
> )
> { // Pin list
> 0x0015
> }
> GpioIo (Shared, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
> "\\_SB.GPO0", 0x00, ResourceConsumer, ,
> )
> { // Pin list
> 0x0019
> }
> Interrupt (ResourceConsumer, Edge, ActiveHigh, Shared, ,, )
> {
> 0x00000032,
> }
> })
> --------------------
> there is no error, but no interrupt is generated.
>
> I understand that the problem is far from the driver but may be you
> could suggest where the problem is.
> Thank you!
>
> Dmitry
>
> On Fri, Dec 20, 2019 at 1:45 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> > Hi,
> >
> > On 19-12-2019 10:10, Dmitry Mastykin wrote:
> > > Hi,
> > > > > I discovered what was with my device.
> > > > > After suspend/resume I2C address often changes to 0x5d (and ACPI table
> > > > > has 0x14).
> > > > Often but not always?
> > > Yes, I think this is because INT from chipset side is tri-stated.
> > >
> > > I'm attaching the acpidump.
> >
> > Thanks, hmm, so looking at the acpidump, your device is using an Apollo
> > Lake and thus definitely is NOT the fieldbook-k122 as that is using
> > a Cherry Trail SoC and not an Apollo Lake SoC.
> >
> > It is also using a GPIO pin configured in direct-irq mode pin for the IRQ line:
> >
> >          Name (RBUF, ResourceTemplate ()
> >          {
> >              I2cSerialBusV2 (0x0014, ControllerInitiated, 0x00061A80,
> >                  AddressingMode7Bit, "\\_SB.PCI0.I2C3",
> >                  0x00, ResourceConsumer, , Exclusive,
> >                  )
> >              Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, )
> >              {
> >                  0x00000032,
> >              }
> >              GpioIo (Shared, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
> >                  "\\_SB.GPO0", 0x00, ResourceConsumer, ,
> >                  )
> >                  {   // Pin list
> >                      0x0019
> >                  }
> >              GpioIo (Shared, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
> >                  "\\_SB.GPO0", 0x00, ResourceConsumer, ,
> >                  )
> >                  {   // Pin list
> >                      0x0005
> >                  }
> >          })
> >
> > And both GPIOs are listed as output-only which certainly does not match the
> > IRQ pin. So this setup is a bit foobar and the plan I have to allow the touchscreen
> > to properly power-down on suspend on Cherry Trail devices, will not work here :|
> >
> > For reference here are the ACPI resources for a GDIX touchscreen on Cherry Trail:
> >
> >       Name (WBUF, ResourceTemplate ()
> >       {
> >           I2cSerialBusV2 (0x0014, ControllerInitiated, 0x00061A80,
> >               AddressingMode7Bit, "\\_SB.PCI0.I2C6",
> >               0x00, ResourceConsumer, , Exclusive,
> >               )
> >           GpioInt (Edge, ActiveLow, Shared, PullDefault, 0x0000,
> >               "\\_SB.GPO3", 0x00, ResourceConsumer, ,
> >               )
> >               {   // Pin list
> >                   0x004D
> >               }
> >           GpioIo (Shared, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
> >               "\\_SB.GPO1", 0x00, ResourceConsumer, ,
> >               )
> >               {   // Pin list
> >                   0x0014
> >               }
> >       })
> >
> > Notice how the Interrupt is specified as a GpioInt so we should be able
> > to use it as a GPIO during suspend so that we can wakeup the touchscreen on
> > resume...
> >
> > I'm afraid I do not have any easy ideas / fixes for how to fix the issue
> > you are seeing on your device.
> >
> > Regards,
> >
> > Hans
> >
> >
> >
> >
> > > On Wed, Dec 18, 2019 at 6:07 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> > > > Hi,
> > > >
> > > > On 18-12-2019 15:50, Dmitry Mastykin wrote:
> > > > > Hi,
> > > > >
> > > > > I discovered what was with my device.
> > > > > After suspend/resume I2C address often changes to 0x5d (and ACPI table
> > > > > has 0x14).
> > > >
> > > > Often but not always?
> > > >
> > > > > ACPI tables doesn't contain _DSD section, and GPIOs are not mapped to
> > > > > ts->gpiod_int/rst. They are NULL. So reset sequence is not called.
> > > > > That's why rmmod/insmod doesn't help.
> > > > >
> > > > > (As temporary solution i changed client->addr, and will wait for
> > > > > better ACPI tables.
> > > > > I tried also to map gpios by index and seems that RESET works but INT
> > > > > doesn't. I can't change I2C address by reset sequence.
> > > > > I've sent a question to tablet's manufacturer about that.)
> > > >
> > > > Interesting, can you provide an acpidump of the device:
> > > >
> > > > sudo acpidump -o acpidump
> > > >
> > > > And then attach the generated acpidump file to your next email ?
> > > >
> > > > Regards,
> > > >
> > > > Hans
> > > >
> > > >
> > > > > > On most newer devices (such as the GPD) the IRQ pin is actually declared
> > > > > > as a GpioInt in the ACPI tables, so we should be able to use it as a GPIO
> > > > > > with some extra work.
> > > > > I will be glad to test this when it's ready provided I have new ACPI tables.
> > > > >
> > > > > Thank you!
> > > > > Kind regards,
> > > > > Dmitry Mastykin
> > > > >
> > > > > On Wed, Dec 18, 2019 at 9:16 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On 13-12-2019 09:17, Dmitry Mastykin wrote:
> > > > > > > Hi guys!
> > > > > > > Thank you very much for your answers, it's always surprising how fast
> > > > > > > the community reacts!
> > > > > > >
> > > > > > > > We have reports of the touchscreen no longer working after suspend/resume
> > > > > > > > on some GPD pocket devices, but not combined with these errors.
> > > > > > > I copied my error messages precisely from gpd support site, because I
> > > > > > > have the same:
> > > > > > > https://gpdsupport.com/t/touchscreen-issue-resolved/132/84
> > > > > > > But seems that rmmod/insmod helps in their case.
> > > > > > > I noticed that the problem is not repeatable. But happens very often.
> > > > > >
> > > > > > Interesting I did not realize the same errors are showing on the GPD pocket.
> > > > > >
> > > > > > I have the feeling this might have something todo with us not putting the
> > > > > > touchscreen controller to sleep on ACPI devices because we cannot use
> > > > > > the IRQ pin as GPIO which is necessary for wakeup.
> > > > > >
> > > > > > On most newer devices (such as the GPD) the IRQ pin is actually declared
> > > > > > as a GpioInt in the ACPI tables, so we should be able to use it as a GPIO
> > > > > > with some extra work.
> > > > > >
> > > > > > I plan to write a patch to make us do this sometime in the near future
> > > > > > (no idea when I will get around to this though). I will put you
> > > > > > (Dmitry Mastykin) in the Cc of the patch when I get around to writing it.
> > > > > >
> > > > > > Note I do not know if this patch will actually help.
> > > > > >
> > > > > > Regards,
> > > > > >
> > > > > > Hans
> > > > > >
> > > > > >
> > > > > > > > When this happens, are these messages appear constantly, or only in
> > > > > > > > response to actual touch? Can you still interact with the device?
> > > > > > > Constantly with 25Hz frequency.
> > > > > > >
> > > > > > > > The fact that you can talk to the controller like that is interesting.
> > > > > > > > Just to confirm, does it return sane data, or garbage?
> > > > > > > root@or212:/home/u# i2cset -y 6 0x5d 0x81 0x4e 0x00 i
> > > > > > > root@or212:/home/u# i2cset -y 6 0x5d 0x81 0x4e i
> > > > > > > root@or212:/home/u# i2cget -y 6 0x5d
> > > > > > > 0x83
> > > > > > > root@or212:/home/u# i2cset -y 6 0x5d 0x81 0x4e 0x00 i
> > > > > > > root@or212:/home/u# i2cset -y 6 0x5d 0x81 0x4e i
> > > > > > > root@or212:/home/u# i2cget -y 6 0x5d
> > > > > > > 0x81
> > > > > > > First I touch with 3 fingers, then with one.
> > > > > > >
> > > > > > > > What is the model of the tablet?
> > > > > > > Not sure about model, seems that it's this one:
> > > > > > > http://logic-instrument.com/en/fieldbook-k122.php
> > > > > > >
> > > > > > > Dmesg is attached
> > > > > > >
> > > > > > > Thank you!
> > > > > > > Kind regards,
> > > > > > > Dmitry Mastykin
> > > > > > >
> > > > > > > On Thu, Dec 12, 2019 at 11:51 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > On 12-12-2019 20:20, Dmitry Torokhov wrote:
> > > > > > > > > Hi Dmitry,
> > > > > > > > >
> > > > > > > > > On Thu, Dec 12, 2019 at 06:34:46PM +0300, Dmitry Mastykin wrote:
> > > > > > > > > > Здравствуйте, Дмитрий.
> > > > > > > > > > Простите, что отвлекаю, если что - не отвечайте и в лучшем случае
> > > > > > > > > > пошлите в mail-list)
> > > > > > > > > > У меня есть планшет с x86, вариантом Debian с ядром 4.15 и
> > > > > > > > > > контроллером тач-скрина gt928.
> > > > > > > > >
> > > > > > > > > What is the model of the tablet?
> > > > > > > > >
> > > > > > > > > > После suspend/resume не всегда, но очень часто тачскрин перестает
> > > > > > > > > > работать и постоянно выводит:
> > > > > > > > > > [ 3276.729412] Goodix-TS i2c-GDIX1002:00: I2C write end_cmd error
> > > > > > > > > > [ 3276.770153] Goodix-TS i2c-GDIX1002:00: I2C transfer error: -121
> > > > > > > > >
> > > > > > > > > When this happens, are these messages appear constantly, or only in
> > > > > > > > > response to actual touch? Can you still interact with the device?
> > > > > > > > >
> > > > > > > > > Could you please send the full dmesg so we know more about the device.
> > > > > > > > >
> > > > > > > > > Hans, Bastien, have you seen such issues with Goodix occasionally
> > > > > > > > > misbehaving on resume?
> > > > > > > >
> > > > > > > > We have reports of the touchscreen no longer working after suspend/resume
> > > > > > > > on some GPD pocket devices, but not combined with these errors.
> > > > > > > >
> > > > > > > > Regards,
> > > > > > > >
> > > > > > > > Hans
> > > > > > > >
> > > > > > > >
> > > > > > > > > > По i2cget/i2cset контроллер успешно отвечает.
> > > > > > > > >
> > > > > > > > > The fact that you can talk to the controller like that is interesting.
> > > > > > > > > Just to confirm, does it return sane data, or garbage?
> > > > > > > > >
> > > > > > > > > Thanks.