> -----Original Message----- > From: Jesse Barnes [mailto:jbarnes@xxxxxxxxxxxxxxxx] > Sent: Thursday, March 01, 2012 2:32 AM > To: Hao, Xudong > Cc: linux-pci@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > kvm@xxxxxxxxxxxxxxx; Kay, Allen M; Zhang, Xiantao > Subject: Re: [PATCH V2] Quirk for IVB graphics FLR errata > > On Wed, 29 Feb 2012 03:11:24 +0000 > "Hao, Xudong" <xudong.hao@xxxxxxxxx> wrote: > > > 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. > > > > Signed-off-by: Xudong Hao <xudong.hao@xxxxxxxxx> > > Signed-off-by: Kay, Allen M <allen.m.kay@xxxxxxxxx> > > --- > > drivers/pci/quirks.c | 49 > > +++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, > 49 > > insertions(+), 0 deletions(-) > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index > > 6476547..5223b80 100644 --- a/drivers/pci/quirks.c > > +++ b/drivers/pci/quirks.c > > @@ -29,6 +29,11 @@ > > #include <asm/dma.h> /* isa_dma_bridge_buggy */ > > #include "pci.h" > > > > +#include "../gpu/drm/i915/i915_reg.h" > > Ugg... this is ugly. Should probably go near the definition of the quirk at any > rate. > OK, I'll move it just above the reset_ivb_igd function. > > +#include <asm/tsc.h> > > +/* 10 seconds */ > > +#define IGD_OPERATION_TIMEOUT ((cycles_t) tsc_khz*10*1000) > > + > > Same here, the asm/tsc.h can go at the top, but the timeout definition should > go near the reset function. > > May as well make it a static const cycles_t as well. > Will modify it. > > +#define MSG_CTL 0x45010 > > + > > +static int reset_ivb_igd(struct pci_dev *dev, int probe) { > > + u8 *mmio_base; > > + 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 */ > > + *((u32 *)(mmio_base + MSG_CTL)) = 0x00000002; > > + *((u32 *)(mmio_base + SOUTH_CHICKEN2)) = 0x00000005; > > + val = *((u32 *)(mmio_base + PCH_PP_CONTROL)) & 0xfffffffe; > > + *((u32 *)(mmio_base + PCH_PP_CONTROL)) = val; > > These clobber existing values. Since we're doing a reset, clobbering > PCH_PP_CONTROL should be ok, but clobbering SOUTH_CHICKEN2 is only ok if > the next driver that loads sets the right bits (if i915 was previously using the > device, it would have set a couple of bits). > > But again since it's it a reset that's probably ok, but you should put a comment > indicating as much. > I'll add comment here, thanks Jesse. > > + do { > > + cycles_t start_time = get_cycles(); > > + while (1) { > > + val = *((u32 *)(mmio_base + PCH_PP_STATUS)); > > + if (((val & 0x80000000) == 0) > > + && ((val & 0x30000000) == 0)) > > + break; > > + if (IGD_OPERATION_TIMEOUT < (get_cycles() - > > start_time)) > > + break; > > + cpu_relax(); > > + } > > + } while (0); > > + *((u32 *)(mmio_base + 0xd0100)) = 0x00000002; > > + > > + 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 } > > Looks ok otherwise, thanks. > > Jesse -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
