* Re: [PATCH 4/4] ALSA: hda - Limit 40bit DMA for AMD HDMI controllers
From: Alex Deucher @ 2014-10-02 2:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Dave Airlie, Linux PCI, Anton Blanchard, Brian King,
Yijing Wang, Takashi Iwai, Bjorn Helgaas
In-Reply-To: <1412210092.10667.3.camel@pasglop>
On Wed, Oct 1, 2014 at 8:34 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> AMD/ATI HDMI controller chip models, we already have a filter to lower
> to 32bit DMA, but the rest are supposed to be working with 64bit
> although the hardware doesn't really work with 63bit but only with 40
> or 48bit DMA. In this patch, we take 40bit DMA for safety for the
> AMD/ATI controllers as the graphics drivers does.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: <stable@vger.kernel.org>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>
> Tested, works fine. This patch is actually independent of the rest
> of the series
>
> sound/pci/hda/hda_intel.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index f91ba7f..48d0f30 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -1483,6 +1483,7 @@ static int azx_first_init(struct azx *chip)
> struct snd_card *card = chip->card;
> int err;
> unsigned short gcap;
> + unsigned int dma_bits = 64;
>
> #if BITS_PER_LONG != 64
> /* Fix up base address on ULI M5461 */
> @@ -1524,9 +1525,14 @@ static int azx_first_init(struct azx *chip)
> gcap = azx_readw(chip, GCAP);
> dev_dbg(card->dev, "chipset global capabilities = 0x%x\n", gcap);
>
> + /* AMD devices support 40 or 48bit DMA, take the safe one */
> + if (chip->pci->vendor == PCI_VENDOR_ID_AMD)
> + dma_bits = 40;
> +
> /* disable SB600 64bit support for safety */
> if (chip->pci->vendor == PCI_VENDOR_ID_ATI) {
> struct pci_dev *p_smbus;
> + dma_bits = 40;
> p_smbus = pci_get_device(PCI_VENDOR_ID_ATI,
> PCI_DEVICE_ID_ATI_SBX00_SMBUS,
> NULL);
> @@ -1556,9 +1562,11 @@ static int azx_first_init(struct azx *chip)
> }
>
> /* allow 64bit DMA address if supported by H/W */
> - if ((gcap & AZX_GCAP_64OK) && !pci_set_dma_mask(pci, DMA_BIT_MASK(64)))
> - pci_set_consistent_dma_mask(pci, DMA_BIT_MASK(64));
> - else {
> + if (!(gcap & AZX_GCAP_64OK))
> + dma_bits = 32;
> + if (!pci_set_dma_mask(pci, DMA_BIT_MASK(dma_bits))) {
> + pci_set_consistent_dma_mask(pci, DMA_BIT_MASK(dma_bits));
> + } else {
> pci_set_dma_mask(pci, DMA_BIT_MASK(32));
> pci_set_consistent_dma_mask(pci, DMA_BIT_MASK(32));
> }
>
>
>
>
^ permalink raw reply
* Re: [PATCH v2 3/4] sound/radeon: Move 64-bit MSI quirk from arch to driver
From: Alex Deucher @ 2014-10-02 2:13 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Dave Airlie, Linux PCI, Anton Blanchard, Brian King,
Yijing Wang, Takashi Iwai, Bjorn Helgaas
In-Reply-To: <1412210082.10667.2.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 3340 bytes --]
On Wed, Oct 1, 2014 at 8:34 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> 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 audio driver.
>
> While recent ASICs have that problem fixed, they don't seem to
> be listed in the PCI IDs of the current driver, so let's quirk all
> the ATI HDMI for now. The consequences are nil on x86 anyway.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: <stable@vger.kernel.org>
The attached updated patch only flags the affected asics.
Alex
> ---
> arch/powerpc/kernel/pci_64.c | 6 ------
> sound/pci/hda/hda_intel.c | 10 ++++++++--
> sound/pci/hda/hda_priv.h | 1 +
> 3 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index 5330f6d..b15194e 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -266,9 +266,3 @@ int pcibus_to_node(struct pci_bus *bus)
> }
> EXPORT_SYMBOL(pcibus_to_node);
> #endif
> -
> -static void quirk_radeon_32bit_msi(struct pci_dev *dev)
> -{
> - dev->no_64bit_msi = true;
> -}
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi);
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index aa302fb..f91ba7f 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -296,7 +296,8 @@ enum {
>
> /* quirks for ATI/AMD HDMI */
> #define AZX_DCAPS_PRESET_ATI_HDMI \
> - (AZX_DCAPS_NO_TCSEL | AZX_DCAPS_SYNC_WRITE | AZX_DCAPS_POSFIX_LPIB)
> + (AZX_DCAPS_NO_TCSEL | AZX_DCAPS_SYNC_WRITE | AZX_DCAPS_POSFIX_LPIB|\
> + AZX_DCAPS_NO_MSI64)
>
> /* quirks for Nvidia */
> #define AZX_DCAPS_PRESET_NVIDIA \
> @@ -1505,9 +1506,14 @@ static int azx_first_init(struct azx *chip)
> return -ENXIO;
> }
>
> - if (chip->msi)
> + if (chip->msi) {
> + if (chip->driver_caps & AZX_DCAPS_NO_MSI64) {
> + dev_dbg(card->dev, "Disabling 64bit MSI\n");
> + pci->no_64bit_msi = true;
> + }
> if (pci_enable_msi(pci) < 0)
> chip->msi = 0;
> + }
>
> if (azx_acquire_irq(chip, 0) < 0)
> return -EBUSY;
> diff --git a/sound/pci/hda/hda_priv.h b/sound/pci/hda/hda_priv.h
> index 949cd43..5016014 100644
> --- a/sound/pci/hda/hda_priv.h
> +++ b/sound/pci/hda/hda_priv.h
> @@ -171,6 +171,7 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
> #define AZX_DCAPS_PM_RUNTIME (1 << 26) /* runtime PM support */
> #define AZX_DCAPS_I915_POWERWELL (1 << 27) /* HSW i915 powerwell support */
> #define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28) /* CORBRP clears itself after reset */
> +#define AZX_DCAPS_NO_MSI64 (1 << 29) /* Stick to 32-bit MSIs */
>
> /* HD Audio class code */
> #define PCI_CLASS_MULTIMEDIA_HD_AUDIO 0x0403
>
>
>
>
[-- Attachment #2: 0001-sound-radeon-Move-64-bit-MSI-quirk-from-arch-to-driv.patch --]
[-- Type: text/x-patch, Size: 8660 bytes --]
From 412fce0929558602a43a41feab051c4cf73c7142 Mon Sep 17 00:00:00 2001
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Thu, 2 Oct 2014 10:34:42 +1000
Subject: [PATCH] sound/radeon: Move 64-bit MSI quirk from arch to driver (v2)
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 audio driver.
While recent ASICs have that problem fixed, they don't seem to
be listed in the PCI IDs of the current driver, so let's quirk all
the ATI HDMI for now. The consequences are nil on x86 anyway.
v2: Alex Deucher: only quirk the affected asics
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org>
---
arch/powerpc/kernel/pci_64.c | 6 ---
sound/pci/hda/hda_intel.c | 96 +++++++++++++++++++++++++++++++-------------
sound/pci/hda/hda_priv.h | 1 +
3 files changed, 69 insertions(+), 34 deletions(-)
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 5330f6d..b15194e 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -266,9 +266,3 @@ int pcibus_to_node(struct pci_bus *bus)
}
EXPORT_SYMBOL(pcibus_to_node);
#endif
-
-static void quirk_radeon_32bit_msi(struct pci_dev *dev)
-{
- dev->no_64bit_msi = true;
-}
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi);
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index aa302fb..4896f25 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1505,9 +1505,14 @@ static int azx_first_init(struct azx *chip)
return -ENXIO;
}
- if (chip->msi)
+ if (chip->msi) {
+ if (chip->driver_caps & AZX_DCAPS_NO_MSI64) {
+ dev_dbg(card->dev, "Disabling 64bit MSI\n");
+ pci->no_64bit_msi = true;
+ }
if (pci_enable_msi(pci) < 0)
chip->msi = 0;
+ }
if (azx_acquire_irq(chip, 0) < 0)
return -EBUSY;
@@ -2062,58 +2067,93 @@ static const struct pci_device_id azx_ids[] = {
{ PCI_DEVICE(0x1022, 0x780d),
.driver_data = AZX_DRIVER_GENERIC | AZX_DCAPS_PRESET_ATI_SB },
/* ATI HDMI */
- { PCI_DEVICE(0x1002, 0x793b),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ { PCI_DEVICE(0x1002, 0x1314),
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0x7919),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
+ { PCI_DEVICE(0x1002, 0x7969),
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
+ { PCI_DEVICE(0x1002, 0x793b),
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0x960f),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
+ { PCI_DEVICE(0x1002, 0x9646),
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0x970f),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa00),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa08),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa10),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa18),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa20),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa28),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa30),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa38),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa40),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa48),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa50),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa58),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa60),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa68),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa80),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa88),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa90),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaa98),
- .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0x9902),
- .driver_data = AZX_DRIVER_ATIHDMI_NS | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI_NS | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaaa0),
- .driver_data = AZX_DRIVER_ATIHDMI_NS | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI_NS | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaaa8),
- .driver_data = AZX_DRIVER_ATIHDMI_NS | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI_NS | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
{ PCI_DEVICE(0x1002, 0xaab0),
- .driver_data = AZX_DRIVER_ATIHDMI_NS | AZX_DCAPS_PRESET_ATI_HDMI },
+ .driver_data = AZX_DRIVER_ATIHDMI_NS | AZX_DCAPS_PRESET_ATI_HDMI |
+ AZX_DCAPS_NO_MSI64 },
/* VIA VT8251/VT8237A */
{ PCI_DEVICE(0x1106, 0x3288),
.driver_data = AZX_DRIVER_VIA | AZX_DCAPS_POSFIX_VIA },
diff --git a/sound/pci/hda/hda_priv.h b/sound/pci/hda/hda_priv.h
index 949cd43..5016014 100644
--- a/sound/pci/hda/hda_priv.h
+++ b/sound/pci/hda/hda_priv.h
@@ -171,6 +171,7 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
#define AZX_DCAPS_PM_RUNTIME (1 << 26) /* runtime PM support */
#define AZX_DCAPS_I915_POWERWELL (1 << 27) /* HSW i915 powerwell support */
#define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28) /* CORBRP clears itself after reset */
+#define AZX_DCAPS_NO_MSI64 (1 << 29) /* Stick to 32-bit MSIs */
/* HD Audio class code */
#define PCI_CLASS_MULTIMEDIA_HD_AUDIO 0x0403
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 04/17] powerpc/msi: Improve IRQ bitmap allocator
From: Michael Neuling @ 2014-10-02 2:01 UTC (permalink / raw)
To: Michael Ellerman
Cc: cbe-oss-dev, arnd, Aneesh Kumar K.V, greg, linux-kernel, imunsie,
linuxppc-dev, anton, jk
In-Reply-To: <20141001071331.CBE1B14017C@ozlabs.org>
On Wed, 2014-10-01 at 17:13 +1000, Michael Ellerman wrote:
> On Tue, 2014-30-09 at 10:34:53 UTC, Michael Neuling wrote:
> > From: Ian Munsie <imunsie@au1.ibm.com>
> >=20
> > Currently msi_bitmap_alloc_hwirqs() will round up any IRQ allocation re=
quests
> re=
quest
> > to the nearest power of 2. eg. ask for 5 IRQs and you'll get 8. This =
wastes a
> ^ one space after a period, or die!
>=20
> > lot of IRQs which can be a scarce resource.
> >=20
> > For cxl we can require multiple IRQs for every contexts that is attache=
d to the
> context
> > accelerator. For AFU directed accelerators, there may be 1000s of cont=
exts
>=20
> What is an AFU directed accelerator?
=46rom the documentation in the last patch:
AFU Models
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
There are two programming models supported by the AFU. Dedicated
and AFU directed. AFU may support one or both models.
In dedicated model only one MMU context is supported. In this
model, only one userspace process can use the accelerator at time.
In AFU directed model, up to 16K simultaneous contexts can be
supported. This means up to 16K simultaneous userspace
applications may use the accelerator (although specific AFUs may
support less). In this mode, the AFU sends a 16 bit context ID
with each of its requests. This tells the PSL which context is
associated with this operation. If the PSL can't translate a
request, the ID can also be accessed by the kernel so it can
determine the associated userspace context to service this
translation with.
> =20
> > attached, hence we can easily run out of IRQs, especially if we are nee=
dlessly
> > wasting them.
> >=20
> > This changes the msi_bitmap_alloc_hwirqs() to allocate only the require=
d number
> x
> > of IRQs, hence avoiding this wastage.
>=20
> The crucial detail you failed to mention is that you maintain the behavio=
ur that
> allocations are naturally aligned.
ok, I'll add that.
> Can you add a check in the test code at the bottom of the file to confirm=
that
> please?
Yep
>=20
> > diff --git a/arch/powerpc/sysdev/msi_bitmap.c b/arch/powerpc/sysdev/msi=
_bitmap.c
> > index 2ff6302..961a358 100644
> > --- a/arch/powerpc/sysdev/msi_bitmap.c
> > +++ b/arch/powerpc/sysdev/msi_bitmap.c
> > @@ -20,32 +20,37 @@ int msi_bitmap_alloc_hwirqs(struct msi_bitmap *bmp,=
int num)
> > int offset, order =3D get_count_order(num);
> > =20
> > spin_lock_irqsave(&bmp->lock, flags);
> > - /*
> > - * This is fast, but stricter than we need. We might want to add
> > - * a fallback routine which does a linear search with no alignment.
> > - */
> > - offset =3D bitmap_find_free_region(bmp->bitmap, bmp->irq_count, order=
);
> > +
> > + offset =3D bitmap_find_next_zero_area(bmp->bitmap, bmp->irq_count, 0,
> > + num, (1 << order) - 1);
> > + if (offset > bmp->irq_count)
> > + goto err;
>=20
> Can we get a newline here :)
Ok.
>=20
> > + bitmap_set(bmp->bitmap, offset, num);
> > spin_unlock_irqrestore(&bmp->lock, flags);
> > =20
> > pr_debug("msi_bitmap: allocated 0x%x (2^%d) at offset 0x%x\n",
> > num, order, offset);
>=20
> This print out is a bit confusing now, should probably just drop the orde=
r.
Arrh, yep.
Thanks,
Mikey
^ permalink raw reply
* Re: [PATCH v2 06/17] powerpc/powernv: Split out set MSI IRQ chip code
From: Michael Ellerman @ 2014-10-02 1:57 UTC (permalink / raw)
To: Michael Neuling, greg, arnd, benh
Cc: cbe-oss-dev, mikey, Aneesh Kumar K.V, imunsie, linux-kernel,
linuxppc-dev, jk, anton
In-Reply-To: <1412073306-13812-7-git-send-email-mikey@neuling.org>
On Tue, 2014-30-09 at 10:34:55 UTC, Michael Neuling wrote:
> From: Ian Munsie <imunsie@au1.ibm.com>
>
> Some of the MSI IRQ code in pnv_pci_ioda_msi_setup() is generically useful so
> split it out.
>
> This will be used by some of the cxl PCIe code later.
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index df241b1..329164f 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -1306,14 +1306,36 @@ static void pnv_ioda2_msi_eoi(struct irq_data *d)
> icp_native_eoi(d);
> }
>
> +
> +static void set_msi_irq_chip(struct pnv_phb *phb, unsigned int virq)
> +{
> + struct irq_data *idata;
> + struct irq_chip *ichip;
> +
> + if (phb->type != PNV_PHB_IODA2)
> + return;
> +
> + /*
> + * Change the IRQ chip for the MSI interrupts on PHB3.
> + * The corresponding IRQ chip should be populated for
> + * the first time.
Seeing as you're moving this comment can you clarify the wording.
cheers
^ permalink raw reply
* Re: [PATCH v2 3/4] sound/radeon: Move 64-bit MSI quirk from arch to driver
From: Stephen Rothwell @ 2014-10-02 1:53 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Dave Airlie, linux-pci, Brian King, Anton Blanchard,
Bjorn Helgaas, Yijing Wang, Takashi Iwai, Alex Deucher
In-Reply-To: <1412210082.10667.2.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]
Hi Ben,
On Thu, 02 Oct 2014 10:34:42 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index aa302fb..f91ba7f 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -296,7 +296,8 @@ enum {
>
> /* quirks for ATI/AMD HDMI */
> #define AZX_DCAPS_PRESET_ATI_HDMI \
> - (AZX_DCAPS_NO_TCSEL | AZX_DCAPS_SYNC_WRITE | AZX_DCAPS_POSFIX_LPIB)
> + (AZX_DCAPS_NO_TCSEL | AZX_DCAPS_SYNC_WRITE | AZX_DCAPS_POSFIX_LPIB|\
> + AZX_DCAPS_NO_MSI64)
>
> /* quirks for Nvidia */
> #define AZX_DCAPS_PRESET_NVIDIA \
> @@ -1505,9 +1506,14 @@ static int azx_first_init(struct azx *chip)
> return -ENXIO;
> }
>
> - if (chip->msi)
> + if (chip->msi) {
> + if (chip->driver_caps & AZX_DCAPS_NO_MSI64) {
> + dev_dbg(card->dev, "Disabling 64bit MSI\n");
> + pci->no_64bit_msi = true;
You get the idea :-)
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/4] gpu/radeon: Move 64-bit MSI quirk from arch to driver
From: Stephen Rothwell @ 2014-10-02 1:52 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Dave Airlie, linux-pci, Brian King, Anton Blanchard,
Bjorn Helgaas, Yijing Wang, Takashi Iwai, Alex Deucher
In-Reply-To: <1412210062.10667.1.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 813 bytes --]
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 ...
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/4] pci/msi: Move "no_64bit_msi" flag from powerpc to generic pci_dev
From: Stephen Rothwell @ 2014-10-02 1:48 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Dave Airlie, linux-pci, Brian King, Anton Blanchard,
Bjorn Helgaas, Yijing Wang, Takashi Iwai, Alex Deucher
In-Reply-To: <1412210012.4285.250.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]
Hi Ben,
On Thu, 02 Oct 2014 10:33:32 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -269,10 +269,7 @@ EXPORT_SYMBOL(pcibus_to_node);
>
> static void quirk_radeon_32bit_msi(struct pci_dev *dev)
> {
> - struct pci_dn *pdn = pci_get_pdn(dev);
> -
> - if (pdn)
> - pdn->force_32bit_msi = true;
> + 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);
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -331,6 +331,7 @@ struct pci_dev {
> unsigned int is_added:1;
> unsigned int is_busmaster:1; /* device is busmaster */
> unsigned int no_msi:1; /* device may not use msi */
> + unsigned int no_64bit_msi:1; /* Device has broken 64-bit MSIs */
> unsigned int block_cfg_access:1; /* config space access is blocked */
> unsigned int broken_parity_status:1; /* Device generates false positive parity */
> unsigned int irq_reroute_variant:2; /* device needs IRQ rerouting variant */
You really should not be assigning "true" to a single bit field ...
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4] sounds/hda/radeon: Disable 64-bit DMA on radeon
From: Alex Deucher @ 2014-10-02 1:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Takashi Iwai, Linux PCI, Brian King,
Anton Blanchard, Dave Airlie, Yijing Wang, Bjorn Helgaas
In-Reply-To: <1412201282.4285.232.camel@pasglop>
On Wed, Oct 1, 2014 at 6:08 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2014-10-01 at 13:58 -0400, Alex Deucher wrote:
>> Patch looks good. Audio DMAs are limited to 40 bits, same as the GPU
>> side. I'm still waiting to hear back on the MSIs for audio, but they
>> probably follow the GPU side, so I expect they should be fixed on Sea
>> Islands as well.
>
> In the audio driver we don't have the "family names", just a list of
> PCI IDs, do you happen to know which ones are unaffected ?
The list of unaffected ones are:
0x1308 //kaveri
0x9840 //Kabini
0xaac0 //bonaire
0xaac8 //hawaii
0xaad8 //tonga
Pus all newer asics.
Alternatively, the list of older asics that are affected (some of
these may not even support MSI64 in the first place):
0x1314 //palm
0x7919 //RS690
0x793b //RS600
0x7969 //RS740
0x960f //RS780
0x9646 //sumo
0x970f //RS880
0x9902 //trinity
0xaa00 //R600
0xaa08 //RV630
0xaa10 //RV610
0xaa18 //RV670
0xaa20 //RV635
0xaa28 //RV620
0xaa30 //RV770
0xaa38 //RV730
0xaa40 //RV710
0xaa48 //RV740
0xaa50 //cypress
0xaa58 //juniper
0xaa60 //redwood
0xaa68 //cedar
0xaa80 //cayman
0xaa88 //barts
0xaa90 //turks
0xaa98 //caicos
0xaaa0 //tahiti
0xaab0 //verde, pitcairn, oland
It might be better to only set the msi32 flag on the above affected
asics so we don't have to worry about new ones.
Alex
^ permalink raw reply
* [PATCH v2 1/4] pci/msi: Move "no_64bit_msi" flag from powerpc to generic pci_dev
From: Benjamin Herrenschmidt @ 2014-10-02 0:34 UTC (permalink / raw)
To: Alex Deucher, Bjorn Helgaas
Cc: linuxppc-dev, Dave Airlie, linux-pci, Anton Blanchard,
Yijing Wang, Takashi Iwai, Brian King
In-Reply-To: <1412112324.4285.160.camel@pasglop>
Some devices have broken 64-bit MSI support which only support some
address bits (40 to 48 typically). This doesn't work on some platforms
such as POWER servers, so we need a quirk.
Currently we keep a flag in a powerpc specific data structure which we
have per PCI device. However this is impractical as we really want the
driver to set that flag appropriately (and the driver shouldn't touch
that arch specific data structure).
It's also not unlikely that this limitation will affect other architectures
in the long run.
So this moves the flag to struct pci_dev instead and adjusts the
corresponding arch/powerpc code to look for it there. At this point,
there is no attempt at making other architectures honor it just yet,
though it appears that x86 doesn't care and always generates 32-bit
MSI addresses.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org>
---
v2: Rename flag to "no_64bit_msi" as suggested by Bjorn
arch/powerpc/include/asm/pci-bridge.h | 2 --
arch/powerpc/kernel/pci_64.c | 5 +----
arch/powerpc/platforms/powernv/pci-ioda.c | 3 +--
arch/powerpc/platforms/powernv/pci.c | 3 +--
arch/powerpc/platforms/pseries/msi.c | 2 +-
include/linux/pci.h | 1 +
6 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 4ca90a3..725247b 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -159,8 +159,6 @@ struct pci_dn {
int pci_ext_config_space; /* for pci devices */
- bool force_32bit_msi;
-
struct pci_dev *pcidev; /* back-pointer to the pci device */
#ifdef CONFIG_EEH
struct eeh_dev *edev; /* eeh device */
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 155013d..d41a831 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -269,10 +269,7 @@ EXPORT_SYMBOL(pcibus_to_node);
static void quirk_radeon_32bit_msi(struct pci_dev *dev)
{
- struct pci_dn *pdn = pci_get_pdn(dev);
-
- if (pdn)
- pdn->force_32bit_msi = true;
+ 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/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index df241b1..a188bb8 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1311,7 +1311,6 @@ static int pnv_pci_ioda_msi_setup(struct pnv_phb *phb, struct pci_dev *dev,
unsigned int is_64, struct msi_msg *msg)
{
struct pnv_ioda_pe *pe = pnv_ioda_get_pe(dev);
- struct pci_dn *pdn = pci_get_pdn(dev);
struct irq_data *idata;
struct irq_chip *ichip;
unsigned int xive_num = hwirq - phb->msi_base;
@@ -1327,7 +1326,7 @@ static int pnv_pci_ioda_msi_setup(struct pnv_phb *phb, struct pci_dev *dev,
return -ENXIO;
/* Force 32-bit MSI on some broken devices */
- if (pdn && pdn->force_32bit_msi)
+ if (dev->no_64bit_msi)
is_64 = 0;
/* Assign XIVE to PE */
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index b854b57..89c6608 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -50,9 +50,8 @@ static int pnv_msi_check_device(struct pci_dev* pdev, int nvec, int type)
{
struct pci_controller *hose = pci_bus_to_host(pdev->bus);
struct pnv_phb *phb = hose->private_data;
- struct pci_dn *pdn = pci_get_pdn(pdev);
- if (pdn && pdn->force_32bit_msi && !phb->msi32_support)
+ if (pdev->no_64bit_msi && !phb->msi32_support)
return -ENODEV;
return (phb && phb->msi_bmp.bitmap) ? 0 : -ENODEV;
diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 18ff462..6fd96d8 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -429,7 +429,7 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
*/
again:
if (type == PCI_CAP_ID_MSI) {
- if (pdn->force_32bit_msi) {
+ if (pdev->no_64bit_msi) {
rc = rtas_change_msi(pdn, RTAS_CHANGE_32MSI_FN, nvec);
if (rc < 0) {
/*
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 96453f9..fc938003 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -331,6 +331,7 @@ struct pci_dev {
unsigned int is_added:1;
unsigned int is_busmaster:1; /* device is busmaster */
unsigned int no_msi:1; /* device may not use msi */
+ unsigned int no_64bit_msi:1; /* Device has broken 64-bit MSIs */
unsigned int block_cfg_access:1; /* config space access is blocked */
unsigned int broken_parity_status:1; /* Device generates false positive parity */
unsigned int irq_reroute_variant:2; /* device needs IRQ rerouting variant */
^ permalink raw reply related
* [PATCH v2 2/4] gpu/radeon: Move 64-bit MSI quirk from arch to driver
From: Benjamin Herrenschmidt @ 2014-10-02 0:34 UTC (permalink / raw)
To: Alex Deucher, Bjorn Helgaas
Cc: linuxppc-dev, Dave Airlie, linux-pci, Anton Blanchard,
Yijing Wang, Takashi Iwai, Brian King
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(-)
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;
^ permalink raw reply related
* [PATCH 4/4] ALSA: hda - Limit 40bit DMA for AMD HDMI controllers
From: Benjamin Herrenschmidt @ 2014-10-02 0:34 UTC (permalink / raw)
To: Alex Deucher, Bjorn Helgaas
Cc: linuxppc-dev, Dave Airlie, linux-pci, Anton Blanchard,
Yijing Wang, Takashi Iwai, Brian King
AMD/ATI HDMI controller chip models, we already have a filter to lower
to 32bit DMA, but the rest are supposed to be working with 64bit
although the hardware doesn't really work with 63bit but only with 40
or 48bit DMA. In this patch, we take 40bit DMA for safety for the
AMD/ATI controllers as the graphics drivers does.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org>
---
Tested, works fine. This patch is actually independent of the rest
of the series
sound/pci/hda/hda_intel.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index f91ba7f..48d0f30 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1483,6 +1483,7 @@ static int azx_first_init(struct azx *chip)
struct snd_card *card = chip->card;
int err;
unsigned short gcap;
+ unsigned int dma_bits = 64;
#if BITS_PER_LONG != 64
/* Fix up base address on ULI M5461 */
@@ -1524,9 +1525,14 @@ static int azx_first_init(struct azx *chip)
gcap = azx_readw(chip, GCAP);
dev_dbg(card->dev, "chipset global capabilities = 0x%x\n", gcap);
+ /* AMD devices support 40 or 48bit DMA, take the safe one */
+ if (chip->pci->vendor == PCI_VENDOR_ID_AMD)
+ dma_bits = 40;
+
/* disable SB600 64bit support for safety */
if (chip->pci->vendor == PCI_VENDOR_ID_ATI) {
struct pci_dev *p_smbus;
+ dma_bits = 40;
p_smbus = pci_get_device(PCI_VENDOR_ID_ATI,
PCI_DEVICE_ID_ATI_SBX00_SMBUS,
NULL);
@@ -1556,9 +1562,11 @@ static int azx_first_init(struct azx *chip)
}
/* allow 64bit DMA address if supported by H/W */
- if ((gcap & AZX_GCAP_64OK) && !pci_set_dma_mask(pci, DMA_BIT_MASK(64)))
- pci_set_consistent_dma_mask(pci, DMA_BIT_MASK(64));
- else {
+ if (!(gcap & AZX_GCAP_64OK))
+ dma_bits = 32;
+ if (!pci_set_dma_mask(pci, DMA_BIT_MASK(dma_bits))) {
+ pci_set_consistent_dma_mask(pci, DMA_BIT_MASK(dma_bits));
+ } else {
pci_set_dma_mask(pci, DMA_BIT_MASK(32));
pci_set_consistent_dma_mask(pci, DMA_BIT_MASK(32));
}
^ permalink raw reply related
* [PATCH v2 3/4] sound/radeon: Move 64-bit MSI quirk from arch to driver
From: Benjamin Herrenschmidt @ 2014-10-02 0:34 UTC (permalink / raw)
To: Alex Deucher, Bjorn Helgaas
Cc: linuxppc-dev, Dave Airlie, linux-pci, Anton Blanchard,
Yijing Wang, Takashi Iwai, Brian King
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 audio driver.
While recent ASICs have that problem fixed, they don't seem to
be listed in the PCI IDs of the current driver, so let's quirk all
the ATI HDMI for now. The consequences are nil on x86 anyway.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org>
---
arch/powerpc/kernel/pci_64.c | 6 ------
sound/pci/hda/hda_intel.c | 10 ++++++++--
sound/pci/hda/hda_priv.h | 1 +
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 5330f6d..b15194e 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -266,9 +266,3 @@ int pcibus_to_node(struct pci_bus *bus)
}
EXPORT_SYMBOL(pcibus_to_node);
#endif
-
-static void quirk_radeon_32bit_msi(struct pci_dev *dev)
-{
- dev->no_64bit_msi = true;
-}
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi);
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index aa302fb..f91ba7f 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -296,7 +296,8 @@ enum {
/* quirks for ATI/AMD HDMI */
#define AZX_DCAPS_PRESET_ATI_HDMI \
- (AZX_DCAPS_NO_TCSEL | AZX_DCAPS_SYNC_WRITE | AZX_DCAPS_POSFIX_LPIB)
+ (AZX_DCAPS_NO_TCSEL | AZX_DCAPS_SYNC_WRITE | AZX_DCAPS_POSFIX_LPIB|\
+ AZX_DCAPS_NO_MSI64)
/* quirks for Nvidia */
#define AZX_DCAPS_PRESET_NVIDIA \
@@ -1505,9 +1506,14 @@ static int azx_first_init(struct azx *chip)
return -ENXIO;
}
- if (chip->msi)
+ if (chip->msi) {
+ if (chip->driver_caps & AZX_DCAPS_NO_MSI64) {
+ dev_dbg(card->dev, "Disabling 64bit MSI\n");
+ pci->no_64bit_msi = true;
+ }
if (pci_enable_msi(pci) < 0)
chip->msi = 0;
+ }
if (azx_acquire_irq(chip, 0) < 0)
return -EBUSY;
diff --git a/sound/pci/hda/hda_priv.h b/sound/pci/hda/hda_priv.h
index 949cd43..5016014 100644
--- a/sound/pci/hda/hda_priv.h
+++ b/sound/pci/hda/hda_priv.h
@@ -171,6 +171,7 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
#define AZX_DCAPS_PM_RUNTIME (1 << 26) /* runtime PM support */
#define AZX_DCAPS_I915_POWERWELL (1 << 27) /* HSW i915 powerwell support */
#define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28) /* CORBRP clears itself after reset */
+#define AZX_DCAPS_NO_MSI64 (1 << 29) /* Stick to 32-bit MSIs */
/* HD Audio class code */
#define PCI_CLASS_MULTIMEDIA_HD_AUDIO 0x0403
^ permalink raw reply related
* Re: [PATCH v2 02/17] powerpc/cell: Move data segment faulting code out of cell platform
From: Michael Neuling @ 2014-10-02 0:58 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: cbe-oss-dev, arnd, greg, linux-kernel, linuxppc-dev, anton,
imunsie, jk
In-Reply-To: <87zjdgm7zn.fsf@linux.vnet.ibm.com>
On Wed, 2014-10-01 at 15:23 +0530, Aneesh Kumar K.V wrote:
> Michael Neuling <mikey@neuling.org> writes:
>=20
> > From: Ian Munsie <imunsie@au1.ibm.com>
> >
> > __spu_trap_data_seg() currently contains code to determine the VSID and=
ESID
> > required for a particular EA and mm struct.
> >
> > This code is generically useful for other co-processors. This moves th=
e code
> > of the cell platform so it can be used by other powerpc code. It also =
adds 1TB
> > segment handling which Cell didn't have.
> >
> > Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
> > ---
> > arch/powerpc/include/asm/mmu-hash64.h | 7 ++++-
> > arch/powerpc/mm/copro_fault.c | 48 ++++++++++++++++++++++++++=
++++++++
> > arch/powerpc/mm/slb.c | 3 ---
> > arch/powerpc/platforms/cell/spu_base.c | 41 +++-----------------------=
---
> > 4 files changed, 58 insertions(+), 41 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/mmu-hash64.h b/arch/powerpc/inclu=
de/asm/mmu-hash64.h
> > index d765144..6d0b7a2 100644
> > --- a/arch/powerpc/include/asm/mmu-hash64.h
> > +++ b/arch/powerpc/include/asm/mmu-hash64.h
> > @@ -189,7 +189,12 @@ static inline unsigned int mmu_psize_to_shift(unsi=
gned int mmu_psize)
> > #define LP_MASK(i) ((0xFF >> (i)) << LP_SHIFT)
> > =20
> > #ifndef __ASSEMBLY__
> > -
> > +static inline int slb_vsid_shift(int ssize)
> > +{
> > + if (ssize =3D=3D MMU_SEGSIZE_256M)
> > + return SLB_VSID_SHIFT;
> > + return SLB_VSID_SHIFT_1T;
> > +}
> > static inline int segment_shift(int ssize)
> > {
> > if (ssize =3D=3D MMU_SEGSIZE_256M)
> > diff --git a/arch/powerpc/mm/copro_fault.c b/arch/powerpc/mm/copro_faul=
t.c
> > index ba7df14..b865697 100644
> > --- a/arch/powerpc/mm/copro_fault.c
> > +++ b/arch/powerpc/mm/copro_fault.c
> > @@ -90,3 +90,51 @@ out_unlock:
> > return ret;
> > }
> > EXPORT_SYMBOL_GPL(copro_handle_mm_fault);
> > +
> > +int copro_data_segment(struct mm_struct *mm, u64 ea, u64 *esid, u64 *v=
sid)
> > +{
> > + int psize, ssize;
> > +
> > + *esid =3D (ea & ESID_MASK) | SLB_ESID_V;
> > +
> > + switch (REGION_ID(ea)) {
> > + case USER_REGION_ID:
> > + pr_devel("copro_data_segment: 0x%llx -- USER_REGION_ID\n", ea);
> > +#ifdef CONFIG_PPC_MM_SLICES
> > + psize =3D get_slice_psize(mm, ea);
> > +#else
> > + psize =3D mm->context.user_psize;
> > +#endif
> > + ssize =3D user_segment_size(ea);
> > + *vsid =3D (get_vsid(mm->context.id, ea, ssize)
> > + << slb_vsid_shift(ssize)) | SLB_VSID_USER;
> > + break;
> > + case VMALLOC_REGION_ID:
> > + pr_devel("copro_data_segment: 0x%llx -- VMALLOC_REGION_ID\n", ea);
> > + if (ea < VMALLOC_END)
> > + psize =3D mmu_vmalloc_psize;
> > + else
> > + psize =3D mmu_io_psize;
> > + ssize =3D mmu_kernel_ssize;
> > + *vsid =3D (get_kernel_vsid(ea, mmu_kernel_ssize)
> > + << SLB_VSID_SHIFT) | SLB_VSID_KERNEL;
>=20
> why not
> *vsid =3D (get_kernel_vsid(ea, mmu_kernel_ssize)
> << slb_vsid_shift(ssize)) | SLB_VSID_KERNEL;
>=20
> for vmalloc and kernel region ? We could end up using 1T segments for ker=
nel mapping too.
Yep, but I'm going to do this in patch 10 where the other optimisations
are for this.
Mikey
^ permalink raw reply
* Re: [PATCH v2 02/17] powerpc/cell: Move data segment faulting code out of cell platform
From: Michael Neuling @ 2014-10-02 0:42 UTC (permalink / raw)
To: Michael Ellerman
Cc: cbe-oss-dev, arnd, Aneesh Kumar K.V, greg, linux-kernel, imunsie,
linuxppc-dev, anton, jk
In-Reply-To: <20141001064757.BFF88140174@ozlabs.org>
On Wed, 2014-10-01 at 16:47 +1000, Michael Ellerman wrote:
> On Tue, 2014-30-09 at 10:34:51 UTC, Michael Neuling wrote:
> > From: Ian Munsie <imunsie@au1.ibm.com>
> >=20
> > __spu_trap_data_seg() currently contains code to determine the VSID and=
ESID
> > required for a particular EA and mm struct.
> >=20
> > This code is generically useful for other co-processors. This moves th=
e code
> > of the cell platform so it can be used by other powerpc code. It also =
adds 1TB
> > segment handling which Cell didn't have.
>=20
> I'm not loving this.
>=20
> For starters the name "copro_data_segment()" doesn't contain any verbs, a=
nd it
> doesn't tell me what it does.
Ok.
> If we give it a name that says what it does, we get copro_get_ea_esid_and=
_vsid().
> Or something equally ugly.
Ok
> And then in patch 10 you move the bulk of the logic into calculate_vsid()=
.
That was intentional on my part. I want this patch to be clear that
we're moving this code out of cell. Then I wanted the optimisations to
be in a separate patch. It does mean we touch the code twice in this
series, but I was hoping it would make it easier to review. Alas. :-)
> So instead can we:
> - add a small helper that does the esid calculation, eg. calculate_esid(=
) ?
> - factor out the vsid logic into a helper, calculate_vsid() ?
> - rework the spu code to use those, dropping __spu_trap_data_seg()
> - use the helpers in the cxl code
OK, I think I can do that. I might change the name to something better
in this patch, but I'll leave these cleanups to the later patch 10.
Mikey
^ permalink raw reply
* [PATCH v2 2/4] gpu/radeon: Move 64-bit MSI quirk from arch to driver
From: Benjamin Herrenschmidt @ 2014-10-02 0:33 UTC (permalink / raw)
To: Alex Deucher, Bjorn Helgaas
Cc: linuxppc-dev, Dave Airlie, linux-pci, Anton Blanchard,
Yijing Wang, Takashi Iwai, Brian King
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>
CC: <stable@vger.kernel.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;
^ permalink raw reply related
* [PATCH v2 1/4] pci/msi: Move "no_64bit_msi" flag from powerpc to generic pci_dev
From: Benjamin Herrenschmidt @ 2014-10-02 0:33 UTC (permalink / raw)
To: Alex Deucher, Bjorn Helgaas
Cc: linuxppc-dev, Dave Airlie, linux-pci, Anton Blanchard,
Yijing Wang, Takashi Iwai, Brian King
In-Reply-To: <1412112324.4285.160.camel@pasglop>
Some devices have broken 64-bit MSI support which only support some
address bits (40 to 48 typically). This doesn't work on some platforms
such as POWER servers, so we need a quirk.
Currently we keep a flag in a powerpc specific data structure which we
have per PCI device. However this is impractical as we really want the
driver to set that flag appropriately (and the driver shouldn't touch
that arch specific data structure).
It's also not unlikely that this limitation will affect other architectures
in the long run.
So this moves the flag to struct pci_dev instead and adjusts the
corresponding arch/powerpc code to look for it there. At this point,
there is no attempt at making other architectures honor it just yet,
though it appears that x86 doesn't care and always generates 32-bit
MSI addresses.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org>
---
v2: Rename flag to "no_64bit_msi" as suggested by Bjorn
arch/powerpc/include/asm/pci-bridge.h | 2 --
arch/powerpc/kernel/pci_64.c | 5 +----
arch/powerpc/platforms/powernv/pci-ioda.c | 3 +--
arch/powerpc/platforms/powernv/pci.c | 3 +--
arch/powerpc/platforms/pseries/msi.c | 2 +-
include/linux/pci.h | 1 +
6 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 4ca90a3..725247b 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -159,8 +159,6 @@ struct pci_dn {
int pci_ext_config_space; /* for pci devices */
- bool force_32bit_msi;
-
struct pci_dev *pcidev; /* back-pointer to the pci device */
#ifdef CONFIG_EEH
struct eeh_dev *edev; /* eeh device */
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 155013d..d41a831 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -269,10 +269,7 @@ EXPORT_SYMBOL(pcibus_to_node);
static void quirk_radeon_32bit_msi(struct pci_dev *dev)
{
- struct pci_dn *pdn = pci_get_pdn(dev);
-
- if (pdn)
- pdn->force_32bit_msi = true;
+ 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/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index df241b1..a188bb8 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1311,7 +1311,6 @@ static int pnv_pci_ioda_msi_setup(struct pnv_phb *phb, struct pci_dev *dev,
unsigned int is_64, struct msi_msg *msg)
{
struct pnv_ioda_pe *pe = pnv_ioda_get_pe(dev);
- struct pci_dn *pdn = pci_get_pdn(dev);
struct irq_data *idata;
struct irq_chip *ichip;
unsigned int xive_num = hwirq - phb->msi_base;
@@ -1327,7 +1326,7 @@ static int pnv_pci_ioda_msi_setup(struct pnv_phb *phb, struct pci_dev *dev,
return -ENXIO;
/* Force 32-bit MSI on some broken devices */
- if (pdn && pdn->force_32bit_msi)
+ if (dev->no_64bit_msi)
is_64 = 0;
/* Assign XIVE to PE */
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index b854b57..89c6608 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -50,9 +50,8 @@ static int pnv_msi_check_device(struct pci_dev* pdev, int nvec, int type)
{
struct pci_controller *hose = pci_bus_to_host(pdev->bus);
struct pnv_phb *phb = hose->private_data;
- struct pci_dn *pdn = pci_get_pdn(pdev);
- if (pdn && pdn->force_32bit_msi && !phb->msi32_support)
+ if (pdev->no_64bit_msi && !phb->msi32_support)
return -ENODEV;
return (phb && phb->msi_bmp.bitmap) ? 0 : -ENODEV;
diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 18ff462..6fd96d8 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -429,7 +429,7 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
*/
again:
if (type == PCI_CAP_ID_MSI) {
- if (pdn->force_32bit_msi) {
+ if (pdev->no_64bit_msi) {
rc = rtas_change_msi(pdn, RTAS_CHANGE_32MSI_FN, nvec);
if (rc < 0) {
/*
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 96453f9..fc938003 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -331,6 +331,7 @@ struct pci_dev {
unsigned int is_added:1;
unsigned int is_busmaster:1; /* device is busmaster */
unsigned int no_msi:1; /* device may not use msi */
+ unsigned int no_64bit_msi:1; /* Device has broken 64-bit MSIs */
unsigned int block_cfg_access:1; /* config space access is blocked */
unsigned int broken_parity_status:1; /* Device generates false positive parity */
unsigned int irq_reroute_variant:2; /* device needs IRQ rerouting variant */
^ permalink raw reply related
* Re: [PATCH 4/4] sounds/hda/radeon: Disable 64-bit DMA on radeon
From: Benjamin Herrenschmidt @ 2014-10-02 0:15 UTC (permalink / raw)
To: Alex Deucher
Cc: linuxppc-dev, Takashi Iwai, Linux PCI, Brian King,
Anton Blanchard, Dave Airlie, Yijing Wang, Bjorn Helgaas
In-Reply-To: <1412201282.4285.232.camel@pasglop>
On Thu, 2014-10-02 at 08:08 +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2014-10-01 at 13:58 -0400, Alex Deucher wrote:
> > Patch looks good. Audio DMAs are limited to 40 bits, same as the GPU
> > side. I'm still waiting to hear back on the MSIs for audio, but they
> > probably follow the GPU side, so I expect they should be fixed on Sea
> > Islands as well.
>
> In the audio driver we don't have the "family names", just a list of
> PCI IDs, do you happen to know which ones are unaffected ?
So I tried to be more discriminate but in the end, I had to give up, the
most recent ID we have in the driver seems to be Cap Verde/Pitcairn,
which isn't fixed. So either we are missing IDs or the new cards use the
same ID.
I think the consequences of being over-zealous here are nil on what
matters, ie, x86. AFAIK, x86 always uses 32-bit addresses for MSIs
anyway, so it's unaffected and ARM ... well, I doubt it will be. In
general, I don't think we have a big issue here by just flagging them
all.
I'll send an updated series with just the flag name changed as suggested
by Alex and the DMA bits updated as suggested by Takashi.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off
From: Alexander Graf @ 2014-10-01 23:44 UTC (permalink / raw)
To: Scott Wood
Cc: Arnd Bergmann, Geoff Levand, Alistair Popple, Anatolij Gustschin,
linuxppc-dev, Guenter Roeck
In-Reply-To: <1412206127.13320.362.camel@snotra.buserror.net>
On 02.10.14 01:28, Scott Wood wrote:
> On Thu, 2014-10-02 at 01:21 +0200, Alexander Graf wrote:
>>
>> On 02.10.14 00:39, Scott Wood wrote:
>>> On Wed, 2014-10-01 at 15:27 +0200, Alexander Graf wrote:
>>>> The generic Linux framework to power off the machine is a function pointer
>>>> called pm_power_off. The trick about this pointer is that device drivers can
>>>> potentially implement it rather than board files.
>>>>
>>>> Today on PowerPC we set pm_power_off to invoke our generic full machine power
>>>> off logic which then calls ppc_md.power_off to invoke machine specific power
>>>> off.
>>>>
>>>> However, when we want to add a power off GPIO via the "gpio-poweroff" driver,
>>>> this card house falls apart. That driver only registers itself if pm_power_off
>>>> is NULL to ensure it doesn't override board specific logic. However, since we
>>>> always set pm_power_off to the generic power off logic (which will just not
>>>> power off the machine if no ppc_md.power_off call is implemented), we can't
>>>> implement power off via the generic GPIO power off driver.
>>>>
>>>> To fix this up, let's get rid of the ppc_md.power_off logic and just always use
>>>> pm_power_off as was intended. Then individual drivers such as the GPIO power off
>>>> driver can implement power off logic via that function pointer.
>>>>
>>>> With this patch set applied and a few patches on top of QEMU that implement a
>>>> power off GPIO on the virt e500 machine, I can successfully turn off my virtual
>>>> machine after halt.
>>>
>>> Are there any plans to handle restart similarly?
>>
>> I don't see an immediate need for it, as we do have access to reset via
>> the guts device.
>
> The guts device is problematic from a hardware description perspective,
> as we advertise a bunch of things to the guest that we don't implement.
> E.g. the only reason we don't currently have a problem with Linux trying
> to use guts to sync the timebase across cores is that by chance we
> happen to expose a guts that corresponds to a single-cpu SoC.
True, and it is slightly ugly to expose a specific SoC's guts in our
generic pv machine type.
>
>> I also don't see any generic driver in Linux that would implement reset
>> via a gpio controller device, so apparently it's not incredibly common
>> to implement reset via gpio.
>
> There wasn't a generic gpio-power-off driver either until a couple years
> ago... Regardless of whether it's done via gpio, using ppc_md for it is
> bad for the same reasons as for power-off.
I agree. Today, only ARM has a comparable mechanism for drivers to hook
a reset handler into the machine specific reset path.
I'd say let's wait for Guenter to finish the power off rework and then
tackle a generic reset call chain based approach next.
Alex
^ permalink raw reply
* Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off
From: Scott Wood @ 2014-10-01 23:28 UTC (permalink / raw)
To: Alexander Graf
Cc: Arnd Bergmann, Geoff Levand, Alistair Popple, Anatolij Gustschin,
linuxppc-dev
In-Reply-To: <542C8C69.2010509@suse.de>
On Thu, 2014-10-02 at 01:21 +0200, Alexander Graf wrote:
>
> On 02.10.14 00:39, Scott Wood wrote:
> > On Wed, 2014-10-01 at 15:27 +0200, Alexander Graf wrote:
> >> The generic Linux framework to power off the machine is a function pointer
> >> called pm_power_off. The trick about this pointer is that device drivers can
> >> potentially implement it rather than board files.
> >>
> >> Today on PowerPC we set pm_power_off to invoke our generic full machine power
> >> off logic which then calls ppc_md.power_off to invoke machine specific power
> >> off.
> >>
> >> However, when we want to add a power off GPIO via the "gpio-poweroff" driver,
> >> this card house falls apart. That driver only registers itself if pm_power_off
> >> is NULL to ensure it doesn't override board specific logic. However, since we
> >> always set pm_power_off to the generic power off logic (which will just not
> >> power off the machine if no ppc_md.power_off call is implemented), we can't
> >> implement power off via the generic GPIO power off driver.
> >>
> >> To fix this up, let's get rid of the ppc_md.power_off logic and just always use
> >> pm_power_off as was intended. Then individual drivers such as the GPIO power off
> >> driver can implement power off logic via that function pointer.
> >>
> >> With this patch set applied and a few patches on top of QEMU that implement a
> >> power off GPIO on the virt e500 machine, I can successfully turn off my virtual
> >> machine after halt.
> >
> > Are there any plans to handle restart similarly?
>
> I don't see an immediate need for it, as we do have access to reset via
> the guts device.
The guts device is problematic from a hardware description perspective,
as we advertise a bunch of things to the guest that we don't implement.
E.g. the only reason we don't currently have a problem with Linux trying
to use guts to sync the timebase across cores is that by chance we
happen to expose a guts that corresponds to a single-cpu SoC.
> I also don't see any generic driver in Linux that would implement reset
> via a gpio controller device, so apparently it's not incredibly common
> to implement reset via gpio.
There wasn't a generic gpio-power-off driver either until a couple years
ago... Regardless of whether it's done via gpio, using ppc_md for it is
bad for the same reasons as for power-off.
-Scott
^ permalink raw reply
* Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off
From: Alexander Graf @ 2014-10-01 23:21 UTC (permalink / raw)
To: Scott Wood
Cc: Arnd Bergmann, Geoff Levand, Alistair Popple, Anatolij Gustschin,
linuxppc-dev
In-Reply-To: <1412203153.13320.352.camel@snotra.buserror.net>
On 02.10.14 00:39, Scott Wood wrote:
> On Wed, 2014-10-01 at 15:27 +0200, Alexander Graf wrote:
>> The generic Linux framework to power off the machine is a function pointer
>> called pm_power_off. The trick about this pointer is that device drivers can
>> potentially implement it rather than board files.
>>
>> Today on PowerPC we set pm_power_off to invoke our generic full machine power
>> off logic which then calls ppc_md.power_off to invoke machine specific power
>> off.
>>
>> However, when we want to add a power off GPIO via the "gpio-poweroff" driver,
>> this card house falls apart. That driver only registers itself if pm_power_off
>> is NULL to ensure it doesn't override board specific logic. However, since we
>> always set pm_power_off to the generic power off logic (which will just not
>> power off the machine if no ppc_md.power_off call is implemented), we can't
>> implement power off via the generic GPIO power off driver.
>>
>> To fix this up, let's get rid of the ppc_md.power_off logic and just always use
>> pm_power_off as was intended. Then individual drivers such as the GPIO power off
>> driver can implement power off logic via that function pointer.
>>
>> With this patch set applied and a few patches on top of QEMU that implement a
>> power off GPIO on the virt e500 machine, I can successfully turn off my virtual
>> machine after halt.
>
> Are there any plans to handle restart similarly?
I don't see an immediate need for it, as we do have access to reset via
the guts device.
I also don't see any generic driver in Linux that would implement reset
via a gpio controller device, so apparently it's not incredibly common
to implement reset via gpio.
Alex
^ permalink raw reply
* Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off
From: Scott Wood @ 2014-10-01 22:39 UTC (permalink / raw)
To: Alexander Graf
Cc: Arnd Bergmann, Geoff Levand, Alistair Popple, Anatolij Gustschin,
linuxppc-dev
In-Reply-To: <1412170086-57971-1-git-send-email-agraf@suse.de>
On Wed, 2014-10-01 at 15:27 +0200, Alexander Graf wrote:
> The generic Linux framework to power off the machine is a function pointer
> called pm_power_off. The trick about this pointer is that device drivers can
> potentially implement it rather than board files.
>
> Today on PowerPC we set pm_power_off to invoke our generic full machine power
> off logic which then calls ppc_md.power_off to invoke machine specific power
> off.
>
> However, when we want to add a power off GPIO via the "gpio-poweroff" driver,
> this card house falls apart. That driver only registers itself if pm_power_off
> is NULL to ensure it doesn't override board specific logic. However, since we
> always set pm_power_off to the generic power off logic (which will just not
> power off the machine if no ppc_md.power_off call is implemented), we can't
> implement power off via the generic GPIO power off driver.
>
> To fix this up, let's get rid of the ppc_md.power_off logic and just always use
> pm_power_off as was intended. Then individual drivers such as the GPIO power off
> driver can implement power off logic via that function pointer.
>
> With this patch set applied and a few patches on top of QEMU that implement a
> power off GPIO on the virt e500 machine, I can successfully turn off my virtual
> machine after halt.
Are there any plans to handle restart similarly?
-Scott
^ permalink raw reply
* Re: [PATCH 1/4] pci/msi: Move "force_32bit_msi" flag from powerpc to generic pci_dev
From: Benjamin Herrenschmidt @ 2014-10-01 22:09 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linuxppc-dev, Dave Airlie, linux-pci, Anton Blanchard, Brian King,
Yijing Wang, Takashi Iwai, Alex Deucher
In-Reply-To: <20141001203322.GE4171@google.com>
On Wed, 2014-10-01 at 14:33 -0600, Bjorn Helgaas wrote:
> I like the idea of handling this more generically, e.g., with a bit like
> this in struct pci_dev (I'd probably name it something like "no_64bit_msi"
> along the lines of your driver #defines).
>
> What I don't like is that we haven't done anything to help other
> architectures, because the only code that *looks* at this bit is in
> arch/powerpc. The next arch that tries to use 64-bit MSI addresses for
> these devices will trip over the same problem.
I started looking. From my (limited) understanding of x86, it doesn't
even look at the "size" of MSIs which makes me think it's always 32-bit,
and I plan to look at the others next (though not for stable).
> Can we check in pci_enable_msi_range() and pci_enable_msix_range() whether
> the MSI addresses allocated by the arch are too big, and fail the call if
> they are?
Yes, good idea, I'll add something.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 4/4] sounds/hda/radeon: Disable 64-bit DMA on radeon
From: Benjamin Herrenschmidt @ 2014-10-01 22:08 UTC (permalink / raw)
To: Alex Deucher
Cc: linuxppc-dev, Takashi Iwai, Linux PCI, Brian King,
Anton Blanchard, Dave Airlie, Yijing Wang, Bjorn Helgaas
In-Reply-To: <CADnq5_P1qTkedSuFfgJ=dd5cWji1-OUbmKoUyq+kLQgPGYYaQw@mail.gmail.com>
On Wed, 2014-10-01 at 13:58 -0400, Alex Deucher wrote:
> Patch looks good. Audio DMAs are limited to 40 bits, same as the GPU
> side. I'm still waiting to hear back on the MSIs for audio, but they
> probably follow the GPU side, so I expect they should be fixed on Sea
> Islands as well.
In the audio driver we don't have the "family names", just a list of
PCI IDs, do you happen to know which ones are unaffected ?
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off
From: Alexander Graf @ 2014-10-01 21:25 UTC (permalink / raw)
To: Guenter Roeck
Cc: Arnd Bergmann, Geoff Levand, Alistair Popple, Geert Uytterhoeven,
Scott Wood, Anatolij Gustschin, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141001155434.GA11039@roeck-us.net>
On 01.10.14 17:54, Guenter Roeck wrote:
> On Wed, Oct 01, 2014 at 04:47:23PM +0200, Alexander Graf wrote:
>>
>>
>> On 01.10.14 16:33, Geert Uytterhoeven wrote:
>>> Hi Alex,
>>>
>>> On Wed, Oct 1, 2014 at 3:27 PM, Alexander Graf <agraf@suse.de> wrote:
>>>> The generic Linux framework to power off the machine is a function pointer
>>>> called pm_power_off. The trick about this pointer is that device drivers can
>>>> potentially implement it rather than board files.
>>>>
>>>> Today on PowerPC we set pm_power_off to invoke our generic full machine power
>>>> off logic which then calls ppc_md.power_off to invoke machine specific power
>>>> off.
>>>>
>>>> However, when we want to add a power off GPIO via the "gpio-poweroff" driver,
>>>> this card house falls apart. That driver only registers itself if pm_power_off
>>>> is NULL to ensure it doesn't override board specific logic. However, since we
>>>> always set pm_power_off to the generic power off logic (which will just not
>>>> power off the machine if no ppc_md.power_off call is implemented), we can't
>>>> implement power off via the generic GPIO power off driver.
>>>>
>>>> To fix this up, let's get rid of the ppc_md.power_off logic and just always use
>>>> pm_power_off as was intended. Then individual drivers such as the GPIO power off
>>>> driver can implement power off logic via that function pointer.
>>>>
>>>> With this patch set applied and a few patches on top of QEMU that implement a
>>>> power off GPIO on the virt e500 machine, I can successfully turn off my virtual
>>>> machine after halt.
>>>
>>> This is touching the same area as last night's
>>> "[RFC PATCH 00/16] kernel: Add support for poweroff handler call chain"
>>> https://lkml.org/lkml/2014/9/30/575
>>
>> I agree, and I think your patch set is walking into a reasonable
>> direction. However, I really think it should convert all users of
>> pm_power_off - at which point you'll probably get to the same conclusion
>> that ppc_md.power_off is a bad idea :).
>>
> Yes, that would be the ultimate goal.
>
>> So in a way, this patch set is semantically a prerequisite to the full
>> conversion you'd probably like to do :).
>>
>> Also, in your cover letter you describe that some methods power off the
>> CPU power while others power off the system power. How do you
>> distinguish between them with a call chain? You probably won't get
>> around to trigger the system power off callback after the CPU power off
>> callback ran ;).
>>
> Those are examples. Don't get hung up on it. I may actually replace the
> CPU example with something better in the next version; it is not really
> a good example and may get people stuck on "why on earth would anyone want
> or need a means to turn off the CPU power" instead of focusing on the problem
> the patch set tries to solve.
>
> The basic problem is that there can be different poweroff handlers,
> some of which may not be available on some systems, and some may not
> be as desirable as others for various reasons. The code registering
> those poweroff handlers does not specify the poweroff method, but its
> priority. It would be up to the programmer (hopefully together with
> the board designer) to determine which method should have higher priority.
> Taking the above example, the callback to turn off CPU power would presumably
> be one of last resort, and have a very low priority.
>
> A better example may actually be patch 15/16 of the series. The affected
> driver (drivers/power/reset/restart-poweroff.c) does not really power off
> the system, but restarts it instead. Obviously that would only be a poweroff
> handler of last resort, which should only be executed if no other means
> to power off the system is available.
Sounds like a good plan :). You probably want to have some global list
of priority numbers like "try this first" or "this is a non-optimal, but
working method" and "only ever do this as last resort".
Maybe you could as a first step convert every user of pm_power_off to
this new framework with a global notifier_block, similar to how
pm_power_off is a global today? Then we can at least get rid of
pm_power_off altogether and move to only notifiers, whereas new
notifiers can come before or after the old machine set implementations.
As a nice bonus this automatically converts every user of pm_power_off()
to instead call the notifier chain.
Alex
^ permalink raw reply
* Re: [PATCH 1/4] pci/msi: Move "force_32bit_msi" flag from powerpc to generic pci_dev
From: Bjorn Helgaas @ 2014-10-01 20:33 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Dave Airlie, linux-pci, Anton Blanchard, Brian King,
Yijing Wang, Takashi Iwai, Alex Deucher
In-Reply-To: <1412129363.4285.190.camel@pasglop>
On Wed, Oct 01, 2014 at 12:09:23PM +1000, Benjamin Herrenschmidt wrote:
>
> Some devices have broken 64-bit MSI support which only support some
> address bits (40 to 48 typically). This doesn't work on some platforms
> such as POWER servers, so we need a quirk.
>
> Currently we keep a flag in a powerpc specific data structure which we
> have per PCI device. However this is impractical as we really want the
> driver to set that flag appropriately (and the driver shouldn't touch
> that arch specific data structure).
>
> It's also not unlikely that this limitation will affect other architectures
> in the long run so may as well be prepared for it.
>
> So this moves the flag to struct pci_dev instead and adjusts the
> corresponding arch/powerpc code to look for it there. At this point,
> there is no attempt at making other architectures honor it just yet
> though from what I can tell, x86 seems to always use 32-bit addresses
> for MSIs.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: <stable@vger.kernel.org>
> ---
>
> Note: CC'ing stable as I really want this to hit distros, without that
> (and the three subsequent patches), we crash during boot with a number
> of radeon cards on power machines.
>
> Note2: Alex, we can wait for the response of the HW guys for which revisions
> actually need the quirk in hda_intel but I don't see a big risk or issue in
> just doing it for all AMD/ATI for now and fix that up later
>
> arch/powerpc/include/asm/pci-bridge.h | 2 --
> arch/powerpc/kernel/pci_64.c | 5 +----
> arch/powerpc/platforms/powernv/pci-ioda.c | 3 +--
> arch/powerpc/platforms/powernv/pci.c | 3 +--
> arch/powerpc/platforms/pseries/msi.c | 2 +-
> include/linux/pci.h | 1 +
> 6 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index 4ca90a3..725247b 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -159,8 +159,6 @@ struct pci_dn {
>
> int pci_ext_config_space; /* for pci devices */
>
> - bool force_32bit_msi;
> -
> struct pci_dev *pcidev; /* back-pointer to the pci device */
> #ifdef CONFIG_EEH
> struct eeh_dev *edev; /* eeh device */
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index 155013d..a6ce5fe 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -269,10 +269,7 @@ EXPORT_SYMBOL(pcibus_to_node);
>
> static void quirk_radeon_32bit_msi(struct pci_dev *dev)
> {
> - struct pci_dn *pdn = pci_get_pdn(dev);
> -
> - if (pdn)
> - pdn->force_32bit_msi = true;
> + dev->force_32bit_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/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index df241b1..9d98475 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -1311,7 +1311,6 @@ static int pnv_pci_ioda_msi_setup(struct pnv_phb *phb, struct pci_dev *dev,
> unsigned int is_64, struct msi_msg *msg)
> {
> struct pnv_ioda_pe *pe = pnv_ioda_get_pe(dev);
> - struct pci_dn *pdn = pci_get_pdn(dev);
> struct irq_data *idata;
> struct irq_chip *ichip;
> unsigned int xive_num = hwirq - phb->msi_base;
> @@ -1327,7 +1326,7 @@ static int pnv_pci_ioda_msi_setup(struct pnv_phb *phb, struct pci_dev *dev,
> return -ENXIO;
>
> /* Force 32-bit MSI on some broken devices */
> - if (pdn && pdn->force_32bit_msi)
> + if (dev->force_32bit_msi)
> is_64 = 0;
>
> /* Assign XIVE to PE */
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index b854b57..4e43a6f 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -50,9 +50,8 @@ static int pnv_msi_check_device(struct pci_dev* pdev, int nvec, int type)
> {
> struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> struct pnv_phb *phb = hose->private_data;
> - struct pci_dn *pdn = pci_get_pdn(pdev);
>
> - if (pdn && pdn->force_32bit_msi && !phb->msi32_support)
> + if (pdev->force_32bit_msi && !phb->msi32_support)
> return -ENODEV;
>
> return (phb && phb->msi_bmp.bitmap) ? 0 : -ENODEV;
> diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> index 18ff462..b3f2c1a 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -429,7 +429,7 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
> */
> again:
> if (type == PCI_CAP_ID_MSI) {
> - if (pdn->force_32bit_msi) {
> + if (pdev->force_32bit_msi) {
> rc = rtas_change_msi(pdn, RTAS_CHANGE_32MSI_FN, nvec);
> if (rc < 0) {
> /*
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 96453f9..740cadd 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -331,6 +331,7 @@ struct pci_dev {
> unsigned int is_added:1;
> unsigned int is_busmaster:1; /* device is busmaster */
> unsigned int no_msi:1; /* device may not use msi */
> + unsigned int force_32bit_msi:1; /* Device has broken 64-bit MSIs */
I like the idea of handling this more generically, e.g., with a bit like
this in struct pci_dev (I'd probably name it something like "no_64bit_msi"
along the lines of your driver #defines).
What I don't like is that we haven't done anything to help other
architectures, because the only code that *looks* at this bit is in
arch/powerpc. The next arch that tries to use 64-bit MSI addresses for
these devices will trip over the same problem.
Can we check in pci_enable_msi_range() and pci_enable_msix_range() whether
the MSI addresses allocated by the arch are too big, and fail the call if
they are?
Bjorn
> unsigned int block_cfg_access:1; /* config space access is blocked */
> unsigned int broken_parity_status:1; /* Device generates false positive parity */
> unsigned int irq_reroute_variant:2; /* device needs IRQ rerouting variant */
>
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox