| Message ID | e415e26c-8781-6ce1-c0f9-a68249e542d3@redhat.com |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi, On 26-12-16 12:07, Hans de Goede wrote: > Hi, > > On 25-12-16 19:31, Len Brown wrote: >> Is there a simple way to run a test to keep deep C-states >> and instead disable part or all of i2c on this platform, >> to see how much stability separating the two will buy us? > > This should do the trick to completely disable i2c from the kernel > to the pmic, leaving the bus fully free for the punit: > > diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c > index 1590ad0..fe73271 100644 > --- a/drivers/i2c/busses/i2c-designware-baytrail.c > +++ b/drivers/i2c/busses/i2c-designware-baytrail.c > @@ -140,6 +140,7 @@ int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev) > > if (shared_host) { > dev_info(dev->dev, "I2C bus managed by PUNIT\n"); > + return -ENODEV; > dev->acquire_lock = baytrail_i2c_acquire; > dev->release_lock = baytrail_i2c_release; > dev->pm_runtime_disabled = true; > > Note that my patch only disabled deep C-states while the kernel > is accessing the pmic i2c bus, not all the time as most other > workarounds are doing. Ok, some more very interesting input on this, from the bug about the PUNIT semaphore issues on cherrytrail: https://bugzilla.kernel.org/show_bug.cgi?id=155241#c37 "My device is a laptop with no USB charging or OTG. So, I'd tried only SDIO _ADR patch, i2c and axp288_fuel_gauge patches. Everything works well for upto 3 mins after boot, then the device freezes. I hadn't tried any drm patches BTW. Here's the log: [drm:fw_domains_get [i915]] *ERROR* render: timed out waiting for forcewake ack request. [drm:fw_domains_get [i915]] *MEDIA* render: timed out waiting for forcewake ack request. [drm:fw_domains_get [i915]] *ERROR* render: timed out waiting for forcewake ack request. [drm:fw_domains_get [i915]] *MEDIA* render: timed out waiting for forcewake ack request. clocksource: timekeeping watchdog on CPU0: Marking clocksource 'tsc' as unstable because the skew is too large: clocksource: 'refined-jiffies' wd_now: 10002ee30 wd_last: 10002edb8 mask: ffffffff clocksource: 'tsc' cs_now: 16ac2c7744a cs_last: 16a8d9bd8f2 mask: ffffffffffffffff clocksource: Switched to clocksource refined-jiffies usb 1-2: reset high-speed USB device number 2 using xhci_hcd i2c_designware 808622C1:06: punit semaphore timed out, resetting i2c_designware 808622C1:06: PUNIT SEM: 2 i2c_designware 808622C1:06: couldn't acquire bus ownership axp288_fuel_gauge axp288_fuel_gauge: axp288 reg read err:-110 axp288_fuel_gauge axp288_fuel_gauge: PWR STAT read failed:-110 usb 1-2: reset high-speed USB device number 2 using xhci_hcd usb 1-2: reset high-speed USB device number 2 using xhci_hcd usb 1-2: reset high-speed USB device number 2 using xhci_hcd i2c_designware 808622C1:06: punit semaphore timed out, resetting i2c_designware 808622C1:06: PUNIT SEM: 0 i2c_designware 808622C1:06: couldn't acquire bus ownership axp288_fuel_gauge axp288_fuel_gauge: IIO channel read error: fffffffb, 0 power_supply axp288_fuel_gauge: driver failed to report `voltage_now' property: -5 ***SYSTEM FREEZE*** If I blacklist axp288_fuel_gauge, then there were no errors." So it seems that not only bad things happen when the punit tries to change the cpu C-state while we're holding the pmic i2c bus semaphore, but that similar bad things happen when the gpu code tries to acquire / release a forcewake lock on the GPU while we're accessing the pmic i2c bus. This makes sense, the iosf_mbi code which is used by the i2c bus semaphore code has this: arch/x86/platform/intel/iosf_mbi.c: int iosf_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr) { u32 mcr, mcrx; unsigned long flags; int ret; /* Access to the GFX unit is handled by GPU code */ if (port == BT_MBI_UNIT_GFX) { WARN_ON(1); return -EPERM; } ... } So the i915 driver definitely is interacting with the punit through the mailbox interface too... I'll try to write a quick and dirty patch where the i915 code simply calls intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); an extra time on probe, and ask the user who is seeing this to test. Regards, Hans code calls -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 2016-12-31 at 22:29 +0100, Hans de Goede wrote: > This makes sense, the iosf_mbi code which is used by the > i2c bus semaphore code has this: > > arch/x86/platform/intel/iosf_mbi.c: > > int iosf_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr) > { > u32 mcr, mcrx; > unsigned long flags; > int ret; > > /* Access to the GFX unit is handled by GPU code */ > if (port == BT_MBI_UNIT_GFX) { > WARN_ON(1); > return -EPERM; > } > > ... > } > > So the i915 driver definitely is interacting with the punit > through the mailbox interface too... Yes, they have private mailbox support. Once I talked to Ville to make at least definitions common between two: iosf_mbi.h and whatever i915 is using, but have no time to implement that.
diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c index 1590ad0..fe73271 100644 --- a/drivers/i2c/busses/i2c-designware-baytrail.c +++ b/drivers/i2c/busses/i2c-designware-baytrail.c @@ -140,6 +140,7 @@ int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev) if (shared_host) { dev_info(dev->dev, "I2C bus managed by PUNIT\n"); + return -ENODEV; dev->acquire_lock = baytrail_i2c_acquire; dev->release_lock = baytrail_i2c_release; dev->pm_runtime_disabled = true;