[PATCH v2 2/4] gpu/radeon: Move 64-bit MSI quirk from arch to driver

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 <***@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(-)

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;

Stephen Rothwell

2014-10-02 01:52:02 UTC

Hi Ben,

Post by Benjamin Herrenschmidt
--- 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 ...

--
Cheers,
Stephen Rothwell ***@canb.auug.org.au

Bjorn Helgaas

2014-10-02 15:34:02 UTC

I'm sorta confused because I got two "v2 2/4" emails a minute or so apart.
I assume they're the same.

Post by Benjamin Herrenschmidt
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?

Post by Benjamin Herrenschmidt
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;

Benjamin Herrenschmidt

2014-10-02 21:02:25 UTC

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 :(

Why do we keep the 0xaa68 quirk? Shouldn't that be made generic, too?

Bjorn Helgaas

2014-10-02 21:44:57 UTC

On Thu, Oct 2, 2014 at 3:02 PM, Benjamin Herrenschmidt

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 :)

Why do we keep the 0xaa68 quirk? Shouldn't that be made generic, too?

Benjamin Herrenschmidt

2014-10-02 22:23:13 UTC

Post by Bjorn Helgaas
On Thu, Oct 2, 2014 at 3:02 PM, Benjamin Herrenschmidt

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 :-)

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.

Alex Deucher

2014-10-02 02:18:07 UTC