> -----Original Message-----
> From: Jesse Barnes [mailto:jbar...@virtuousgeek.org]
> Sent: Thursday, March 01, 2012 2:32 AM
> To: Hao, Xudong
> Cc: linux-...@vger.kernel.org; linux-ker...@vger.kernel.org;
> kvm@vger.kernel.org; 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....@intel.com> 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....@intel.com>
> > Signed-off-by: Kay, Allen M <allen.m....@intel.com>
> > ---
> > 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
