| Message ID | 1412210062.10667.1.camel@pasglop (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi Ben, On Thu, 02 Oct 2014 10:34:22 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c > @@ -202,6 +202,16 @@ static bool radeon_msi_ok(struct radeon_device *rdev) > if (rdev->flags & RADEON_IS_AGP) > return false; > > + /* > + * Older chips have a HW limitation, they can only generate 40 bits > + * of address for "64-bit" MSIs which breaks on some platforms, notably > + * IBM POWER servers, so we limit them > + */ > + if (rdev->family < CHIP_BONAIRE) { > + dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n"); > + rdev->pdev->no_64bit_msi = true; Again, no_64bit_msi is not a bool ...
On Thu, Oct 02, 2014 at 10:34:22AM +1000, Benjamin Herrenschmidt wrote: > > A number of radeon cards have a HW limitation causing them to be > unable to generate the full 64-bit of address bits for MSIs. This > breaks MSIs on some platforms such as POWER machines. > > We used to have a powerpc specific quirk to address that on a > single card, but this doesn't scale very well, this is better > put under control of the drivers who know precisely what a given > HW revision can do. > > This moves the setting of the quirk flag to the radeon driver > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > > v2: This is just adjusted to the new flag name I'm sorta confused because I got two "v2 2/4" emails a minute or so apart. I assume they're the same. > > arch/powerpc/kernel/pci_64.c | 1 - > drivers/gpu/drm/radeon/radeon_irq_kms.c | 10 ++++++++++ > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c > index d41a831..5330f6d 100644 > --- a/arch/powerpc/kernel/pci_64.c > +++ b/arch/powerpc/kernel/pci_64.c > @@ -271,5 +271,4 @@ static void quirk_radeon_32bit_msi(struct pci_dev *dev) > { > dev->no_64bit_msi = true; > } > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x68f2, quirk_radeon_32bit_msi); > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi); Why do we keep the 0xaa68 quirk? Shouldn't that be made generic, too? > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c > index 16807af..e760671 100644 > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c > @@ -202,6 +202,16 @@ static bool radeon_msi_ok(struct radeon_device *rdev) > if (rdev->flags & RADEON_IS_AGP) > return false; > > + /* > + * Older chips have a HW limitation, they can only generate 40 bits > + * of address for "64-bit" MSIs which breaks on some platforms, notably > + * IBM POWER servers, so we limit them > + */ > + if (rdev->family < CHIP_BONAIRE) { > + dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n"); > + rdev->pdev->no_64bit_msi = true; > + } > + > /* force MSI on */ > if (radeon_msi == 1) > return true; > > >
On Thu, 2014-10-02 at 09:34 -0600, Bjorn Helgaas wrote: > On Thu, Oct 02, 2014 at 10:34:22AM +1000, Benjamin Herrenschmidt wrote: > > > > A number of radeon cards have a HW limitation causing them to be > > unable to generate the full 64-bit of address bits for MSIs. This > > breaks MSIs on some platforms such as POWER machines. > > > > We used to have a powerpc specific quirk to address that on a > > single card, but this doesn't scale very well, this is better > > put under control of the drivers who know precisely what a given > > HW revision can do. > > > > This moves the setting of the quirk flag to the radeon driver > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > --- > > > > v2: This is just adjusted to the new flag name > > I'm sorta confused because I got two "v2 2/4" emails a minute or so apart. > I assume they're the same. Yes, evo blew up while sending the series the first time around :( > > > > arch/powerpc/kernel/pci_64.c | 1 - > > drivers/gpu/drm/radeon/radeon_irq_kms.c | 10 ++++++++++ > > 2 files changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c > > index d41a831..5330f6d 100644 > > --- a/arch/powerpc/kernel/pci_64.c > > +++ b/arch/powerpc/kernel/pci_64.c > > @@ -271,5 +271,4 @@ static void quirk_radeon_32bit_msi(struct pci_dev *dev) > > { > > dev->no_64bit_msi = true; > > } > > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x68f2, quirk_radeon_32bit_msi); > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi); > > Why do we keep the 0xaa68 quirk? Shouldn't that be made generic, too? > > > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c > > index 16807af..e760671 100644 > > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c > > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c > > @@ -202,6 +202,16 @@ static bool radeon_msi_ok(struct radeon_device *rdev) > > if (rdev->flags & RADEON_IS_AGP) > > return false; > > > > + /* > > + * Older chips have a HW limitation, they can only generate 40 bits > > + * of address for "64-bit" MSIs which breaks on some platforms, notably > > + * IBM POWER servers, so we limit them > > + */ > > + if (rdev->family < CHIP_BONAIRE) { > > + dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n"); > > + rdev->pdev->no_64bit_msi = true; > > + } > > + > > /* force MSI on */ > > if (radeon_msi == 1) > > return true; > > > > > >
On Thu, Oct 2, 2014 at 3:02 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Thu, 2014-10-02 at 09:34 -0600, Bjorn Helgaas wrote: >> On Thu, Oct 02, 2014 at 10:34:22AM +1000, Benjamin Herrenschmidt wrote: >> > >> > A number of radeon cards have a HW limitation causing them to be >> > unable to generate the full 64-bit of address bits for MSIs. This >> > breaks MSIs on some platforms such as POWER machines. >> > >> > We used to have a powerpc specific quirk to address that on a >> > single card, but this doesn't scale very well, this is better >> > put under control of the drivers who know precisely what a given >> > HW revision can do. >> > >> > This moves the setting of the quirk flag to the radeon driver >> > >> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> > --- >> > >> > v2: This is just adjusted to the new flag name >> >> I'm sorta confused because I got two "v2 2/4" emails a minute or so apart. >> I assume they're the same. > > Yes, evo blew up while sending the series the first time around :( No problem. My real question is below, but you probably missed it because of my email gripe :) >> > arch/powerpc/kernel/pci_64.c | 1 - >> > drivers/gpu/drm/radeon/radeon_irq_kms.c | 10 ++++++++++ >> > 2 files changed, 10 insertions(+), 1 deletion(-) >> > >> > diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c >> > index d41a831..5330f6d 100644 >> > --- a/arch/powerpc/kernel/pci_64.c >> > +++ b/arch/powerpc/kernel/pci_64.c >> > @@ -271,5 +271,4 @@ static void quirk_radeon_32bit_msi(struct pci_dev *dev) >> > { >> > dev->no_64bit_msi = true; >> > } >> > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x68f2, quirk_radeon_32bit_msi); >> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi); >> >> Why do we keep the 0xaa68 quirk? Shouldn't that be made generic, too? >> >> > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c >> > index 16807af..e760671 100644 >> > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c >> > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c >> > @@ -202,6 +202,16 @@ static bool radeon_msi_ok(struct radeon_device *rdev) >> > if (rdev->flags & RADEON_IS_AGP) >> > return false; >> > >> > + /* >> > + * Older chips have a HW limitation, they can only generate 40 bits >> > + * of address for "64-bit" MSIs which breaks on some platforms, notably >> > + * IBM POWER servers, so we limit them >> > + */ >> > + if (rdev->family < CHIP_BONAIRE) { >> > + dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n"); >> > + rdev->pdev->no_64bit_msi = true; >> > + } >> > + >> > /* force MSI on */ >> > if (radeon_msi == 1) >> > return true; >> > >> > >> > > >
On Thu, 2014-10-02 at 15:44 -0600, Bjorn Helgaas wrote: > On Thu, Oct 2, 2014 at 3:02 PM, Benjamin Herrenschmidt > <benh@kernel.crashing.org> wrote: > > On Thu, 2014-10-02 at 09:34 -0600, Bjorn Helgaas wrote: > >> On Thu, Oct 02, 2014 at 10:34:22AM +1000, Benjamin Herrenschmidt wrote: > >> > > >> > A number of radeon cards have a HW limitation causing them to be > >> > unable to generate the full 64-bit of address bits for MSIs. This > >> > breaks MSIs on some platforms such as POWER machines. > >> > > >> > We used to have a powerpc specific quirk to address that on a > >> > single card, but this doesn't scale very well, this is better > >> > put under control of the drivers who know precisely what a given > >> > HW revision can do. > >> > > >> > This moves the setting of the quirk flag to the radeon driver > >> > > >> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > >> > --- > >> > > >> > v2: This is just adjusted to the new flag name > >> > >> I'm sorta confused because I got two "v2 2/4" emails a minute or so apart. > >> I assume they're the same. > > > > Yes, evo blew up while sending the series the first time around :( > > No problem. My real question is below, but you probably missed it > because of my email gripe :) Heh ok :-) > >> > arch/powerpc/kernel/pci_64.c | 1 - > >> > drivers/gpu/drm/radeon/radeon_irq_kms.c | 10 ++++++++++ > >> > 2 files changed, 10 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c > >> > index d41a831..5330f6d 100644 > >> > --- a/arch/powerpc/kernel/pci_64.c > >> > +++ b/arch/powerpc/kernel/pci_64.c > >> > @@ -271,5 +271,4 @@ static void quirk_radeon_32bit_msi(struct pci_dev *dev) > >> > { > >> > dev->no_64bit_msi = true; > >> > } > >> > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x68f2, quirk_radeon_32bit_msi); > >> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi); > >> > >> Why do we keep the 0xaa68 quirk? Shouldn't that be made generic, too? aa68 is the audio part, it's removed by the audio driver patch. But I can break things down into smaller bits as you suggested. I'll try to get that sorted later today. Cheers, Ben. > >> > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c > >> > index 16807af..e760671 100644 > >> > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c > >> > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c > >> > @@ -202,6 +202,16 @@ static bool radeon_msi_ok(struct radeon_device *rdev) > >> > if (rdev->flags & RADEON_IS_AGP) > >> > return false; > >> > > >> > + /* > >> > + * Older chips have a HW limitation, they can only generate 40 bits > >> > + * of address for "64-bit" MSIs which breaks on some platforms, notably > >> > + * IBM POWER servers, so we limit them > >> > + */ > >> > + if (rdev->family < CHIP_BONAIRE) { > >> > + dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n"); > >> > + rdev->pdev->no_64bit_msi = true; > >> > + } > >> > + > >> > /* force MSI on */ > >> > if (radeon_msi == 1) > >> > return true; > >> > > >> > > >> > > > > >
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c index d41a831..5330f6d 100644 --- a/arch/powerpc/kernel/pci_64.c +++ b/arch/powerpc/kernel/pci_64.c @@ -271,5 +271,4 @@ static void quirk_radeon_32bit_msi(struct pci_dev *dev) { dev->no_64bit_msi = true; } -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x68f2, quirk_radeon_32bit_msi); DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi); diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c index 16807af..e760671 100644 --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c @@ -202,6 +202,16 @@ static bool radeon_msi_ok(struct radeon_device *rdev) if (rdev->flags & RADEON_IS_AGP) return false; + /* + * Older chips have a HW limitation, they can only generate 40 bits + * of address for "64-bit" MSIs which breaks on some platforms, notably + * IBM POWER servers, so we limit them + */ + if (rdev->family < CHIP_BONAIRE) { + dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n"); + rdev->pdev->no_64bit_msi = true; + } + /* force MSI on */ if (radeon_msi == 1) return true;
A number of radeon cards have a HW limitation causing them to be unable to generate the full 64-bit of address bits for MSIs. This breaks MSIs on some platforms such as POWER machines. We used to have a powerpc specific quirk to address that on a single card, but this doesn't scale very well, this is better put under control of the drivers who know precisely what a given HW revision can do. This moves the setting of the quirk flag to the radeon driver Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- v2: This is just adjusted to the new flag name arch/powerpc/kernel/pci_64.c | 1 - drivers/gpu/drm/radeon/radeon_irq_kms.c | 10 ++++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)