From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:40013 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755285Ab2DWVox (ORCPT ); Mon, 23 Apr 2012 17:44:53 -0400 Message-ID: <4F95CD52.1030409@redhat.com> Date: Mon, 23 Apr 2012 17:44:50 -0400 From: Don Dutile MIME-Version: 1.0 To: Bjorn Helgaas CC: "Hao, Xudong" , "linux-pci@vger.kernel.org" , Matthew Wilcox , "Zhang, Xiantao" Subject: Re: [PATCH v6] Quirk for IVB graphics FLR errata References: <403610A45A2B5242BD291EDAE8B37D300FD24698@SHSMSX102.ccr.corp.intel.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Sender: linux-pci-owner@vger.kernel.org List-ID: On 04/23/2012 05:07 PM, Bjorn Helgaas wrote: > On Fri, Apr 20, 2012 at 1:08 AM, Hao, Xudong wrote: >> Hi, Bjorn >> >> Do you have any other comments for this patch? > > What happened with the idea of adding a .reset hook in struct > pci_driver? It would be better if all the device-related code could > live in the device-specific driver, rather than having to put it in a > quirk. Everybody pays the cost of a quirk, even on systems that don't > include this device. > > Bjorn > Well, its possible to have a bad device that needs an FLR quirk w/o its driver being loaded... e.g., a device assigned to a KVM guest ... no driver in the host, but will FLR the device when removed from the guest. I basically agree with your point, just thought of this configuration/option. >>> -----Original Message----- >>> From: Hao, Xudong >>> Sent: Monday, April 16, 2012 9:40 AM >>> To: 'Bjorn Helgaas' >>> Cc: 'linux-pci@vger.kernel.org'; 'Don Dutile'; 'Matthew Wilcox' >>> Subject: [PATCH v6] Quirk for IVB graphics FLR errata >>> >>> For IvyBridge Mobile platform, a system hang may occur if a FLR(Function Level >>> Reset) is asserted to internal graphics. >>> >>> This quirk patch is workaround for the IVB FLR errata issue. >>> We are disabling the FLR reset handshake between the PCH and CPU display, >>> then manually powering down the panel power sequencing and resetting the >>> PCH display. >>> >>> Changes from v5: >>> - clean up patch with Matthew's comments. >>> >>> Signed-off-by: Xudong Hao >>> Signed-off-by: Kay, Allen M >>> Signed-off-by: Matthew Wilcox >>> --- >>> drivers/pci/quirks.c | 48 >>> ++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 files changed, 48 insertions(+), 0 deletions(-) >>> >>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 4bf7102..213cad9 >>> 100644 >>> --- a/drivers/pci/quirks.c >>> +++ b/drivers/pci/quirks.c >>> @@ -29,6 +29,7 @@ >>> #include >>> #include >>> #include /* isa_dma_bridge_buggy */ >>> +#include >>> #include "pci.h" >>> >>> /* >>> @@ -3085,11 +3086,58 @@ static int reset_intel_82599_sfp_virtfn(struct >>> pci_dev *dev, int probe) >>> return 0; >>> } >>> >>> +#include "../gpu/drm/i915/i915_reg.h" >>> +#define MSG_CTL 0x45010 >>> +#define IGD_OPERATION_TIMEOUT 10000 /* set timeout 10 seconds */ >>> + >>> +static int reset_ivb_igd(struct pci_dev *dev, int probe) { >>> + void __iomem *mmio_base; >>> + unsigned long timeout; >>> + u32 val; >>> + >>> + if (probe) >>> + return 0; >>> + >>> + mmio_base = ioremap_nocache(pci_resource_start(dev, 0), >>> + pci_resource_len(dev, 0)); >>> + if (!mmio_base) >>> + return -ENOMEM; >>> + >>> + /* Work Around */ >>> + writel(0x00000002, mmio_base + MSG_CTL); >>> + /* Clobbering SOUTH_CHICKEN2 register is fine only if the next >>> + * driver loaded sets the right bits. However, this's a reset and >>> + * the bits have been set by i915 previously, so we clobber >>> + * SOUTH_CHICKEN2 register directly here. >>> + */ >>> + writel(0x00000005, mmio_base + SOUTH_CHICKEN2); >>> + val = readl(mmio_base + PCH_PP_CONTROL)& 0xfffffffe; >>> + writel(val, mmio_base + PCH_PP_CONTROL); >>> + timeout = jiffies + msecs_to_jiffies(IGD_OPERATION_TIMEOUT); >>> + while (time_before(jiffies, timeout)) { >>> + val = readl(mmio_base + PCH_PP_STATUS); >>> + if ((val& 0xB0000000) == 0) >>> + break; >>> + cpu_relax(); >>> + } >>> + writel(0x00000002, mmio_base + 0xd0100); >>> + >>> + iounmap(pci_resource_start(dev, 0)); >>> + return 0; >>> +} >>> + >>> #define PCI_DEVICE_ID_INTEL_82599_SFP_VF 0x10ed >>> +#define PCI_DEVICE_ID_INTEL_IVB_M_VGA 0x0156 >>> +#define PCI_DEVICE_ID_INTEL_IVB_M2_VGA 0x0166 >>> >>> static const struct pci_dev_reset_methods pci_dev_reset_methods[] = { >>> { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82599_SFP_VF, >>> reset_intel_82599_sfp_virtfn }, >>> + { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IVB_M_VGA, >>> + reset_ivb_igd }, >>> + { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IVB_M2_VGA, >>> + reset_ivb_igd }, >>> { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, >>> reset_intel_generic_dev }, >>> { 0 } >>> -- >>> 1.6.0.rc1 >>