Bug 229791 - ig4 autoload breaks S3 (ig4iic_pci0 controller error during attach-2)
Summary: ig4 autoload breaks S3 (ig4iic_pci0 controller error during attach-2)
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Oleksandr Tymoshenko
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-07-15 18:02 UTC by Ali Abdallah
Modified: 2019-05-10 12:27 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ali Abdallah 2018-07-15 18:02:35 UTC
In reference to the commit r336141, which adds PNP table for the ig4 module, when the module gets loaded on my Thinkpad T480, it drops the following messages: 

ig4iic_pci0: <Intel Sunrise Point-LP I2C Controller-0> mem 0xec154000-0xec154fff at device 21.0 on pci0
ig4iic_pci0: Using MSI
ig4iic_pci0: controller error during attach-2
iicbus0: <Philips I2C bus> on ig4iic_pci0

Now with the module loaded, S3 suspend (which works perfectly without the module being loaded) fails, last message I see is

[drm:wait_for_engines] Failed to idle engines, declaring wedged!

and then the system hangs.
Comment 1 commit-hook freebsd_committer 2018-07-16 01:34:54 UTC
A commit references this bug:

Author: gonzo
Date: Mon Jul 16 01:34:46 UTC 2018
New revision: 336326
URL: https://svnweb.freebsd.org/changeset/base/336326

Log:
  Remove MODULE_PNP_INFO for ig4(4) driver

  ig4(4) does not support suspend/resume but present on the hardware where
  such functionality is critical, like laptops. Remove PNP info to avoid
  breaking suspend/resume on the systems where ig4(4) load is not explicitly
  requested by the user.

  PR:             229791
  Reported by:    Ali Abdallah

Changes:
  head/sys/dev/ichiic/ig4_pci.c
Comment 2 Ali Abdallah 2018-07-16 06:58:49 UTC
Thanks for the quick reaction on this. 

Is ig4 going to get suspend support in the near future (and eventually its PNP data will be re-added) ?
Comment 3 Daniel Zeisig 2018-08-10 14:24:42 UTC
I can see the same error message on 11.2 Release and also experience the suspend/resume issue described by Ali Abdallah.
The Laptop is Huawei Matebook X Pro. With an i7 Kabylake.

pci0: <dasp> at device 20.2 (no driver attached)
ig4iic_pci0: <Intel Sunrise Point-LP I2C Controller-0> mem 0x2ff3021000-0x2ff3021fff at device 21.0 on pci0
ig4iic_pci0: Using MSI
pcib0: matched entry for 0.21.INTA
pcib0: slot 21 INTA hardwired to IRQ 16
ig4iic_pci0: set_controller: error 0
ig4iic_pci0: controller error during attach-2

It seems to fail in ig4_iic.c while trying to enable the controller: "set_controller(ig4iic_softc_t *sc, uint32_t ctl)"
I have added 1 line of debug code to confirm it, pls. see below.

if (((v ^ ctl) & IG4_I2C_ENABLE) == 0) {
             error = 0;
             device_printf(sc->dev,
                       "set_controller: error %d\n", error);
             break;
         }

I could live with the suspend/resume not working for now but I really need the controller to work. This Laptop has the trackpad and touchpad connected via i2c and I'm trying to see if I can get HID over i2c to work.
Comment 4 Oleksandr Tymoshenko freebsd_committer freebsd_triage 2018-08-13 19:02:05 UTC
(In reply to Ali Abdallah from comment #2)

I've just committed a fix for the attach error in base r229791. Could you try to update to the latest HEAD and try sleep/wakeup cycle?
Comment 5 Ali Abdallah 2018-08-14 08:09:03 UTC
(In reply to Oleksandr Tymoshenko from comment #4)

Did you mean base r337719?
Comment 6 Oleksandr Tymoshenko freebsd_committer freebsd_triage 2018-08-14 18:40:30 UTC
(In reply to Ali Abdallah from comment #5)

Yes, base r337719, Sorry for the confusion.
Comment 7 Ali Abdallah 2018-08-14 20:18:25 UTC
Tested. It loads fine, suspend/resume works as well. However, I've noticed that my T480 takes more time to resume from S3 when the module is loaded, and more time to be fully responsive under X11.
Comment 8 Daniel Zeisig 2018-08-22 03:31:53 UTC
I can confirm that the patch also works for me in regards of correctly initialize my IC2 controller.

So "ig4iic_pci0: controller error during attach-2" is gone.

Suspend resume however still fails if ig4 is loaded. The Laptop will hang while resuming. Only a reboot brings me out of this state.
Comment 9 commit-hook freebsd_committer 2018-09-30 23:14:15 UTC
A commit references this bug:

Author: gonzo
Date: Sun Sep 30 23:14:08 UTC 2018
New revision: 339029
URL: https://svnweb.freebsd.org/changeset/base/339029

Log:
  MFC r336050-r336051, r336142, r336326, r337719

  r336050:
  ig4(4): add support for Apollo Lake I2C controllers

  Add PCI ids for I2C controllers on Apollo Lake platform. Also convert
  switch/case probe logic into a table.

  Reviewed by:	avg
  Differential Revision:	https://reviews.freebsd.org/D16120

  r336051:
  ig4(4): Fix Apollo lake entries platform identifier

  Identify Apollo Lake controllers as IG4_APL and not as a IG4_SKYLAKE

  Reported by:	rpokala@

  r336142:
  ig4(4): add devmatch(8) PNP info

  Now that we have all devices ids in a table add MODULE_PNP_INFO macro
  to let devmatch autoload module

  r336326:
  Remove MODULE_PNP_INFO for ig4(4) driver

  ig4(4) does not support suspend/resume but present on the hardware where
  such functionality is critical, like laptops. Remove PNP info to avoid
  breaking suspend/resume on the systems where ig4(4) load is not explicitly
  requested by the user.

  PR:             229791
  Reported by:    Ali Abdallah

  r337719:
  [ig4] Fix initialization sequence for newer ig4 chips

  Newer chips may require assert/deassert after power down for proper
  startup. Check respective flag in DEVIDLE_CTRL and perform operation
  if neccesssary.

  PR:		221777
  Submitted by:	marc.priggemeyer@gmail.com
  Obtained from:	DragonFly BSD
  Tested on:	Thinkpad T470

Changes:
_U  stable/11/
  stable/11/sys/dev/ichiic/ig4_iic.c
  stable/11/sys/dev/ichiic/ig4_pci.c
  stable/11/sys/dev/ichiic/ig4_reg.h
  stable/11/sys/dev/ichiic/ig4_var.h
Comment 10 Andriy Gapon freebsd_committer 2018-12-17 21:22:53 UTC
I think that we can try to reinstate base r336142 now that base r342170 has been committed.  I believe that it was a root cause of the hang on resume.
Comment 11 Mark Johnston freebsd_committer 2018-12-17 21:46:43 UTC
(In reply to Andriy Gapon from comment #10)
Oops, I wasn't aware of this PR.  I commited r342178 (which requires a follow-up: r342180) for this.
Comment 12 Oleksandr Tymoshenko freebsd_committer freebsd_triage 2019-01-14 03:20:26 UTC
Closing PR since Mark committed the fix for autoloading ig4 using PNP info.
Comment 13 Nikola Lečić 2019-05-10 12:27:09 UTC
Hi all,

I'm trying to get HID over IIC working on Asus Zenbook 14 UX410UFR, so I'm testing the code from this thread: https://reviews.freebsd.org/D16698%C2%A0

I'm on 12.0-RELEASE/amd64 and /boot/modules/i915kms.ko.

acpi_iichid needs ig4. They together break suspend (which works just fine without them). After 'acpiconf -s3' laptop hangs on ttyv1 with quickly blinking mouse cursor and these lines:

...
<6>[drm] GPU HANG: ecode 9:1:0xfffffffe, reason: Hang on bcs0, action: reset 
drmn0: Failed to idle engines, declaring wedged!

Do you think this problem might be related to this bug?

Anyway, I can't compile new ichiic on 12.0-RELEASE. Can someone experienced tell me what else I should pick up from -CURRENT to get it compiled?

/usr/src/sys/dev/ichiic/ig4_acpi.c:78:62: error: too many arguments to function call, expected 3, have 4
        rv = ACPI_ID_PROBE(device_get_parent(dev), dev, ig4iic_ids, &hid);
             ~~~~~~~~~~~~~                                          ^~~~
./acpi_if.h:29:1: note: 'ACPI_ID_PROBE' declared here
static __inline char * ACPI_ID_PROBE(device_t bus, device_t dev, char **ids)
^
1 error generated.
*** Error code 1

Stop.