[PATCH v6] Quirk for IVB graphics FLR errata

[PATCH v6] Quirk for IVB graphics FLR errata

[Hao, Xudong]

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 <xudong.hao@intel.com>
Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 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 <linux/sched.h>
 #include <linux/ktime.h>
 #include <asm/dma.h>	/* isa_dma_bridge_buggy */
+#include <asm/io.h>
 #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

Re: [PATCH v6] Quirk for IVB graphics FLR errata

[Bjorn Helgaas]

On Tue, May 1, 2012 at 6:37 PM, Hao, Xudong <xudong.hao@intel.com> wrote:
>> -----Original Message-----
>> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
>> Sent: Saturday, April 28, 2012 1:42 AM
>> To: Hao, Xudong
>> Cc: linux-pci@vger.kernel.org; Don Dutile; Matthew Wilcox; Zhang, Xiantao
>> Subject: Re: [PATCH v6] Quirk for IVB graphics FLR errata
>>
>> On Fri, Apr 27, 2012 at 01:26:18AM +0000, Hao, Xudong wrote:
>> > Maybe something configuration wrong on my mail client, I attach the patch
>> as an attachment, can you open it in your side?
>>
>> Yes, I could open the attachment.  But it makes it easier for everybody to
>> read & review your patches if you can figure out how to post patches
>> directly in the message rather than as an attachment.
>>
>
> Sure, I'm trying configure my email, make it better to work.
>
>> Here's an updated version of your patch.  I changed the following:
>>
>>     - Wrapped changelog text so it fits nicely for "git log".
>>     - Added #define for 0xd0100 offset (please supply a more useful name).
>>     - Used pci_iomap() and ioread32()/iowrite32().
>>     - Used msleep() rather than spinning (Matthew suggested this earlier,
>>       but you apparently missed it).  Note that I went back to your
>>       original "do {} while ()" structure to make sure we read PCH_PP_STATUS
>>       at least once.
>>     - Added message if reset times out.
>>
>> This is still x86-specific code that clutters all other architectures.  We
>> might fix this someday by adding a DECLARE_PCI_FIXUP_RESET(), so the IVB
>> code could live in arch/x86, and the linker could still collect all the
>> device-specific reset methods.  But I haven't done that yet.
>>
>> Please test and comment on this (and supply a name for 0xd0100):
>>
>
> We can named 0xd0100 "NSDE_PWR_STATE" instead of "XXXX" in code.
>
> Testing done, patch works.
>
> I do not send patch again, can you replace XXXX to NSDE_PWR_STATE and apply this patch?

Fixed up and applied to my "next" branch, thanks.

Bjorn

RE: [PATCH v6] Quirk for IVB graphics FLR errata

[Hao, Xudong]

> -----Original Message-----
> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
> Sent: Saturday, April 28, 2012 1:42 AM
> To: Hao, Xudong
> Cc: linux-pci@vger.kernel.org; Don Dutile; Matthew Wilcox; Zhang, Xiantao
> Subject: Re: [PATCH v6] Quirk for IVB graphics FLR errata
> 
> On Fri, Apr 27, 2012 at 01:26:18AM +0000, Hao, Xudong wrote:
> > Maybe something configuration wrong on my mail client, I attach the patch
> as an attachment, can you open it in your side?
> 
> Yes, I could open the attachment.  But it makes it easier for everybody to
> read & review your patches if you can figure out how to post patches
> directly in the message rather than as an attachment.
> 

Sure, I'm trying configure my email, make it better to work.

> Here's an updated version of your patch.  I changed the following:
> 
>     - Wrapped changelog text so it fits nicely for "git log".
>     - Added #define for 0xd0100 offset (please supply a more useful name).
>     - Used pci_iomap() and ioread32()/iowrite32().
>     - Used msleep() rather than spinning (Matthew suggested this earlier,
>       but you apparently missed it).  Note that I went back to your
>       original "do {} while ()" structure to make sure we read PCH_PP_STATUS
>       at least once.
>     - Added message if reset times out.
> 
> This is still x86-specific code that clutters all other architectures.  We
> might fix this someday by adding a DECLARE_PCI_FIXUP_RESET(), so the IVB
> code could live in arch/x86, and the linker could still collect all the
> device-specific reset methods.  But I haven't done that yet.
> 
> Please test and comment on this (and supply a name for 0xd0100):
> 

We can named 0xd0100 "NSDE_PWR_STATE" instead of "XXXX" in code.

Testing done, patch works. 

I do not send patch again, can you replace XXXX to NSDE_PWR_STATE and apply this patch?


> commit 8566df6d83bf89c8cad3810d5d333a31a95b133e
> Author: Xudong Hao <xudong.hao@intel.com>
> Date:   Fri Apr 27 09:16:46 2012 -0600
> 
>     PCI: work around IvyBridge internal graphics FLR erratum
> 
>     For IvyBridge Mobile platform, a system hang may occur if a FLR (Function
>     Level Reset) is asserted to internal graphics.
> 
>     This quirk is a 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@intel.com>
>     Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
>     Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 4bf7102..d5646de 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3085,16 +3085,74 @@ 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 XXXX			0xd0100
> +#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 = pci_iomap(dev, 0, 0);
> +	if (!mmio_base)
> +		return -ENOMEM;
> +
> +	iowrite32(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.
> +	 */
> +	iowrite32(0x00000005, mmio_base + SOUTH_CHICKEN2);
> +
> +	val = ioread32(mmio_base + PCH_PP_CONTROL) & 0xfffffffe;
> +	iowrite32(val, mmio_base + PCH_PP_CONTROL);
> +
> +	timeout = jiffies + msecs_to_jiffies(IGD_OPERATION_TIMEOUT);
> +	do {
> +		val = ioread32(mmio_base + PCH_PP_STATUS);
> +		if ((val & 0xB0000000) == 0)
> +			goto reset_complete;
> +		msleep(10);
> +	} while (time_before(jiffies, timeout));
> +	dev_warn(&dev->dev, "timeout during reset\n");
> +
> +reset_complete:
> +	iowrite32(0x00000002, mmio_base + XXXX);
> +
> +	pci_iounmap(dev, mmio_base);
> +	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 }
>  };
> 
> +/*
> + * These device-specific reset methods are here rather than in a driver
> + * because when a host assigns a device to a guest VM, the host may need
> + * to reset the device but probably doesn't have a driver for it.
> + */
>  int pci_dev_specific_reset(struct pci_dev *dev, int probe)
>  {
>  	const struct pci_dev_reset_methods *i;

Re: [PATCH v6] Quirk for IVB graphics FLR errata

[Bjorn Helgaas]

On Fri, Apr 27, 2012 at 01:26:18AM +0000, Hao, Xudong wrote:
> Maybe something configuration wrong on my mail client, I attach the patch as an attachment, can you open it in your side?

Yes, I could open the attachment.  But it makes it easier for everybody to
read & review your patches if you can figure out how to post patches
directly in the message rather than as an attachment.

Here's an updated version of your patch.  I changed the following:

    - Wrapped changelog text so it fits nicely for "git log".
    - Added #define for 0xd0100 offset (please supply a more useful name).
    - Used pci_iomap() and ioread32()/iowrite32().
    - Used msleep() rather than spinning (Matthew suggested this earlier,
      but you apparently missed it).  Note that I went back to your
      original "do {} while ()" structure to make sure we read PCH_PP_STATUS
      at least once.
    - Added message if reset times out.

This is still x86-specific code that clutters all other architectures.  We
might fix this someday by adding a DECLARE_PCI_FIXUP_RESET(), so the IVB
code could live in arch/x86, and the linker could still collect all the
device-specific reset methods.  But I haven't done that yet.

Please test and comment on this (and supply a name for 0xd0100):

commit 8566df6d83bf89c8cad3810d5d333a31a95b133e
Author: Xudong Hao <xudong.hao@intel.com>
Date:   Fri Apr 27 09:16:46 2012 -0600

    PCI: work around IvyBridge internal graphics FLR erratum
    
    For IvyBridge Mobile platform, a system hang may occur if a FLR (Function
    Level Reset) is asserted to internal graphics.
    
    This quirk is a 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@intel.com>
    Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
    Signed-off-by: Matthew Wilcox <willy@linux.intel.com>

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 4bf7102..d5646de 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3085,16 +3085,74 @@ 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 XXXX			0xd0100
+#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 = pci_iomap(dev, 0, 0);
+	if (!mmio_base)
+		return -ENOMEM;
+
+	iowrite32(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.
+	 */
+	iowrite32(0x00000005, mmio_base + SOUTH_CHICKEN2);
+
+	val = ioread32(mmio_base + PCH_PP_CONTROL) & 0xfffffffe;
+	iowrite32(val, mmio_base + PCH_PP_CONTROL);
+
+	timeout = jiffies + msecs_to_jiffies(IGD_OPERATION_TIMEOUT);
+	do {
+		val = ioread32(mmio_base + PCH_PP_STATUS);
+		if ((val & 0xB0000000) == 0)
+			goto reset_complete;
+		msleep(10);
+	} while (time_before(jiffies, timeout));
+	dev_warn(&dev->dev, "timeout during reset\n");
+
+reset_complete:
+	iowrite32(0x00000002, mmio_base + XXXX);
+
+	pci_iounmap(dev, mmio_base);
+	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 }
 };
 
+/*
+ * These device-specific reset methods are here rather than in a driver
+ * because when a host assigns a device to a guest VM, the host may need
+ * to reset the device but probably doesn't have a driver for it.
+ */
 int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 {
 	const struct pci_dev_reset_methods *i;

RE: [PATCH v6] Quirk for IVB graphics FLR errata

[Hao, Xudong]

[-- Attachment #1: Type: text/plain, Size: 4870 bytes --]

Bjorn, 

Maybe something configuration wrong on my mail client, I attach the patch as an attachment, can you open it in your side?

Thanks,
-Xudong

> -----Original Message-----
> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
> Sent: Friday, April 27, 2012 7:15 AM
> To: Hao, Xudong
> Cc: linux-pci@vger.kernel.org; Don Dutile; Matthew Wilcox; Zhang, Xiantao
> Subject: Re: [PATCH v6] Quirk for IVB graphics FLR errata
> 
> On Wed, Apr 25, 2012 at 9:19 PM, Hao, Xudong <xudong.hao@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.hao@intel.com>
> > Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
> > Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> > ---
> >  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 <linux/sched.h>
> >  #include <linux/ktime.h>
> >  #include <asm/dma.h>   /* isa_dma_bridge_buggy */
> > +#include <asm/io.h>
> >  #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
> >
> >
> >> If we're doing FLR in a pass-through situation, where the host does
> >> the FLR but has no driver for the device, doing this in a quirk makes
> >> sense to me.
> >>
> >> I tried to apply this patch, but it's whitespace-damaged.  Can you repost
> it?
> >>
> >
> >
> > Thanks Bjorn, I repost it and can you try again?
> 
> Did you change anything, or did you just repost it the same way you
> did before?  The patch I see is still corrupted.  Here's what I see:
> 
> 
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -29,6 +29,7 @@
>  #include <linux/sched.h>
>  #include <linux/ktime.h>
>  #include <asm/dma.h>   /* isa_dma_bridge_buggy */
> +#include <asm/io.h>
>  #include "pci.h"
> =20
> ...
> 
> The "=20" is bogus.

[-- Attachment #2: ivb_gfx_quirk_v6.patch --]
[-- Type: application/octet-stream, Size: 2802 bytes --]

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@intel.com>
Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 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 <linux/sched.h>
 #include <linux/ktime.h>
 #include <asm/dma.h>	/* isa_dma_bridge_buggy */
+#include <asm/io.h>
 #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

Re: [PATCH v6] Quirk for IVB graphics FLR errata

[Bjorn Helgaas]

On Wed, Apr 25, 2012 at 9:19 PM, Hao, Xudong <xudong.hao@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.hao@intel.com>
> Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> ---
>  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 <linux/sched.h>
>  #include <linux/ktime.h>
>  #include <asm/dma.h>   /* isa_dma_bridge_buggy */
> +#include <asm/io.h>
>  #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
>
>
>> If we're doing FLR in a pass-through situation, where the host does
>> the FLR but has no driver for the device, doing this in a quirk makes
>> sense to me.
>>
>> I tried to apply this patch, but it's whitespace-damaged.  Can you repost it?
>>
>
>
> Thanks Bjorn, I repost it and can you try again?

Did you change anything, or did you just repost it the same way you
did before?  The patch I see is still corrupted.  Here's what I see:


--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -29,6 +29,7 @@
 #include <linux/sched.h>
 #include <linux/ktime.h>
 #include <asm/dma.h>   /* isa_dma_bridge_buggy */
+#include <asm/io.h>
 #include "pci.h"
=20
...

The "=20" is bogus.

RE: [PATCH v6] Quirk for IVB graphics FLR errata

[Hao, Xudong]

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@intel.com>
Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 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 <linux/sched.h>
 #include <linux/ktime.h>
 #include <asm/dma.h>	/* isa_dma_bridge_buggy */
+#include <asm/io.h>
 #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


> If we're doing FLR in a pass-through situation, where the host does
> the FLR but has no driver for the device, doing this in a quirk makes
> sense to me.
> 
> I tried to apply this patch, but it's whitespace-damaged.  Can you repost it?
> 


Thanks Bjorn, I repost it and can you try again?

> Bjorn

Re: [PATCH v6] Quirk for IVB graphics FLR errata

[Bjorn Helgaas]

On Mon, Apr 23, 2012 at 9:06 PM, Hao, Xudong <xudong.hao@intel.com> wrote:
>
>> -----Original Message-----
>> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
>> Sent: Tuesday, April 24, 2012 5:07 AM
>> To: Hao, Xudong
>> Cc: linux-pci@vger.kernel.org; Don Dutile; Matthew Wilcox; Zhang, Xiantao
>> Subject: Re: [PATCH v6] Quirk for IVB graphics FLR errata
>>
>> On Fri, Apr 20, 2012 at 1:08 AM, Hao, Xudong <xudong.hao@intel.com> 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.
>>
>
> The FLR happened on bus but not for device driver, considering one case that we just want to assign device to guest, but host do not have the device driver, how will a quirk be executed?
>
> Thanks,
> -Xudong
>
>> Bjorn
>>
>> >> -----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 <xudong.hao@intel.com>
>> >> Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
>> >> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
>> >> ---
>> >>  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 <linux/sched.h>
>> >>  #include <linux/ktime.h>
>> >>  #include <asm/dma.h> /* isa_dma_bridge_buggy */
>> >> +#include <asm/io.h>
>> >>  #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 }

If we're doing FLR in a pass-through situation, where the host does
the FLR but has no driver for the device, doing this in a quirk makes
sense to me.

I tried to apply this patch, but it's whitespace-damaged.  Can you repost it?

Bjorn

RE: [PATCH v6] Quirk for IVB graphics FLR errata

[Hao, Xudong]

> -----Original Message-----
> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
> Sent: Tuesday, April 24, 2012 5:07 AM
> To: Hao, Xudong
> Cc: linux-pci@vger.kernel.org; Don Dutile; Matthew Wilcox; Zhang, Xiantao
> Subject: Re: [PATCH v6] Quirk for IVB graphics FLR errata
> 
> On Fri, Apr 20, 2012 at 1:08 AM, Hao, Xudong <xudong.hao@intel.com> 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.
> 

The FLR happened on bus but not for device driver, considering one case that we just want to assign device to guest, but host do not have the device driver, how will a quirk be executed?

Thanks,
-Xudong

> Bjorn
> 
> >> -----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 <xudong.hao@intel.com>
> >> Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
> >> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> >> ---
> >>  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 <linux/sched.h>
> >>  #include <linux/ktime.h>
> >>  #include <asm/dma.h> /* isa_dma_bridge_buggy */
> >> +#include <asm/io.h>
> >>  #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
> >

Re: [PATCH v6] Quirk for IVB graphics FLR errata

[Don Dutile]

On 04/23/2012 05:07 PM, Bjorn Helgaas wrote:
> On Fri, Apr 20, 2012 at 1:08 AM, Hao, Xudong<xudong.hao@intel.com>  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<xudong.hao@intel.com>
>>> Signed-off-by: Kay, Allen M<allen.m.kay@intel.com>
>>> Signed-off-by: Matthew Wilcox<willy@linux.intel.com>
>>> ---
>>>   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<linux/sched.h>
>>>   #include<linux/ktime.h>
>>>   #include<asm/dma.h>  /* isa_dma_bridge_buggy */
>>> +#include<asm/io.h>
>>>   #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
>>

Re: [PATCH v6] Quirk for IVB graphics FLR errata

[Bjorn Helgaas]

On Fri, Apr 20, 2012 at 1:08 AM, Hao, Xudong <xudong.hao@intel.com> 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

>> -----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 <xudong.hao@intel.com>
>> Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
>> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
>> ---
>>  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 <linux/sched.h>
>>  #include <linux/ktime.h>
>>  #include <asm/dma.h> /* isa_dma_bridge_buggy */
>> +#include <asm/io.h>
>>  #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
>

RE: [PATCH v6] Quirk for IVB graphics FLR errata

[Hao, Xudong]

Hi, Bjorn

Do you have any other comments for this patch?

Thanks,
-Xudong

> -----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 <xudong.hao@intel.com>
> Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> ---
>  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 <linux/sched.h>
>  #include <linux/ktime.h>
>  #include <asm/dma.h>	/* isa_dma_bridge_buggy */
> +#include <asm/io.h>
>  #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