From patchwork Mon Jun 26 20:40:08 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lyude Paul X-Patchwork-Id: 780865 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3wxLXf1h27z9s81 for ; Tue, 27 Jun 2017 06:40:18 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751439AbdFZUkQ (ORCPT ); Mon, 26 Jun 2017 16:40:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36424 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751432AbdFZUkP (ORCPT ); Mon, 26 Jun 2017 16:40:15 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D0CFD61B82; Mon, 26 Jun 2017 20:40:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D0CFD61B82 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lyude@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com D0CFD61B82 Received: from malachite.bss.redhat.com (unknown [10.20.4.238]) by smtp.corp.redhat.com (Postfix) with ESMTP id BAE6C79826; Mon, 26 Jun 2017 20:40:09 +0000 (UTC) From: Lyude To: intel-gfx@lists.freedesktop.org Cc: Lyude , "Rafael J . Wysocki" , Benjamin Tissoires , Mika Westerberg , Jean Delvare , Wolfram Sang , Jean Delvare , linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] i2c: i801: Allow ACPI SystemIO OpRegion to conflict harder Date: Mon, 26 Jun 2017 16:40:08 -0400 Message-Id: <20170626204009.32607-1-lyude@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 26 Jun 2017 20:40:15 +0000 (UTC) Sender: linux-i2c-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org There's quite a number of machines on the market, mainly Lenovo ThinkPads, that make the horrible mistake in their firmware of reusing the PCIBAR space reserved for the SMBus for things that are completely unrelated to the SMBus controller, such as the OpRegion used for i915. This is very bad and entirely evil, but with Lenovo's historically poor track record of fixing their firmware, it is extremely unlikely this is ever going to be properly fixed. So, while it would be nice if we could just cut off the SMBus controller and call it a day this unfortunately breaks RMI4 mode completely for most of the ThinkPads. Even though this behavior is extremely wrong, for whatever reason sharing the PCIBAR between the OpRegion and SMBus seems to be just fine. Regardless however, I think it's safe to assume that when the BIOS accesses the PCIBAR space of the SMBus controller like this that it's trying to get to something else that we mapped the SMBus controller over. On my X1 Carbon, this assumption appears to be correct. I've traced down the firmware accesses to being caused by the firmware mistakenly placing the SWSCI mailbox for i915 on top of the SMBus host controller region. And indeed, blacklisting i915 causes the firmware to never attempt to touch this region. So, in order to try to workaround this and not break either SMBus or i915, we temporarily unmap the PCI device for the SMBus controller, do the thing that the firmware wanted to do, then remap the device and report a firmware bug. Signed-off-by: Lyude Cc: Rafael J. Wysocki Cc: Benjamin Tissoires Cc: Mika Westerberg Cc: Jean Delvare Cc: Wolfram Sang Cc: intel-gfx@lists.freedesktop.org --- So: unfortunately a7ae81952cda (i2c: i801: Allow ACPI SystemIO OpRegion to conflict with PCI BAR) Seems to prevent the ThinkPad X1 Carbon 4th gen and the T460s from actually using their SMBus controllers at all. As mentioned above, I've traced the issue down to the firmware responding to the SWSCI by sticking data in places it shouldn't, e.g. the SMBus registers. I'm entirely unsure if this patch is the correct fix for this, and wouldn't be at all surprised if it's just as bad of a patch as I think it is ;P. So I figured I'd send it to intel-gfx and the authors of the original version of this patch to get their take on it and see if there might be something less hacky we can do to fix this. drivers/i2c/busses/i2c-i801.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 6484fa6..bfbe0f9 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -1406,33 +1406,42 @@ i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits, { struct i801_priv *priv = handler_context; struct pci_dev *pdev = priv->pci_dev; + struct device *dev = &pdev->dev; acpi_status status; + int err; - /* - * Once BIOS AML code touches the OpRegion we warn and inhibit any - * further access from the driver itself. This device is now owned - * by the system firmware. - */ mutex_lock(&priv->acpi_lock); - if (!priv->acpi_reserved) { - priv->acpi_reserved = true; + /* + * BIOS AML code should never actually touch the SMBus registers, + * however crappy firmware (mainly Lenovo's) can make the mistake of + * mapping things over the SMBus region that should definitely not be + * there (such as the OpRegion for Intel GPUs). + * This is extremely bad firmware behavior, but it is unlikely this will + * ever get fixed by Lenovo. + */ + dev_warn_once(dev, + FW_BUG "OpRegion overlaps with SMBus registers, working around\n"); - dev_warn(&pdev->dev, "BIOS is accessing SMBus registers\n"); - dev_warn(&pdev->dev, "Driver SMBus register access inhibited\n"); - - /* - * BIOS is accessing the host controller so prevent it from - * suspending automatically from now on. - */ - pm_runtime_get_sync(&pdev->dev); - } + pm_runtime_get_sync(dev); + pcim_iounmap_regions(pdev, 1 << SMBBAR); if ((function & ACPI_IO_MASK) == ACPI_READ) status = acpi_os_read_port(address, (u32 *)value, bits); else status = acpi_os_write_port(address, (u32)*value, bits); + err = pcim_iomap_regions(pdev, 1 << SMBBAR, + dev_driver_string(&pdev->dev)); + if (err) { + dev_err(&pdev->dev, + FW_BUG "Failed to restore SMBus region 0x%lx-0x%Lx. SMBus is now broken.\n", + priv->smba, + (unsigned long long)pci_resource_end(pdev, SMBBAR)); + priv->acpi_reserved = true; + } + + pm_runtime_put(dev); mutex_unlock(&priv->acpi_lock); return status;