| Message ID | s5hoatw42f6.wl-tiwai@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Oct 1, 2014 at 4:30 AM, Takashi Iwai <tiwai@suse.de> wrote: > At Wed, 01 Oct 2014 10:09:28 +0200, > Takashi Iwai wrote: >> >> At Wed, 01 Oct 2014 17:41:29 +1000, >> Benjamin Herrenschmidt wrote: >> > >> > On Wed, 2014-10-01 at 09:38 +0200, Takashi Iwai wrote: >> > >> > > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c >> > > > index 3e6d22d..2b679d5 100644 >> > > > --- a/sound/pci/hda/hda_intel.c >> > > > +++ b/sound/pci/hda/hda_intel.c >> > > > @@ -297,7 +297,7 @@ 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_MSI64) >> > > > + AZX_DCAPS_NO_MSI64 | AZX_DCAPS_NO_64BIT) >> > > >> > > The only concern is that this will disable 64bit DMA also on x86 where >> > > it has been working fine. Can we add an ifdef CONFIG_PPC for this? >> > >> > I don't like that approach because technically the chip doesn't do >> > 64-bit DMA ... it does something like 40 or 48 (might actually depend on >> > the chip version) and for all I know it will break on future x86 with >> > more memory or other platforms with similar address encodings as >> > powerpc... >> > >> > The right thing might be to get the exact number of bits and do the >> > appropriate dma_set_mask() like the graphics driver does, but that's a >> > bit tricky unless we add a DMA mask field in that big array of chips in >> > there... >> >> I think setting the dma mask explicitly would be a better approach >> although it results in a bit bigger change. At least, it would impact >> less than forcing 32bit DMA, as most desktop machines have less than >> 40bit address. How about a patch like below? 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. Alex > > Oops, it obviously doesn't work with AMD ID... Fixed in below. > > > Takashi > > -- 8< -- > From: Takashi Iwai <tiwai@suse.de> > Subject: [PATCH v2] ALSA: hda - Limit 40bit DMA for AMD HDMI controllers > > 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> > --- > 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 aa302fb03fc5..99b367bd9b1b 100644 > --- a/sound/pci/hda/hda_intel.c > +++ b/sound/pci/hda/hda_intel.c > @@ -1482,6 +1482,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 */ > @@ -1518,9 +1519,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); > @@ -1550,9 +1556,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)); > } > -- > 2.1.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index aa302fb03fc5..99b367bd9b1b 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -1482,6 +1482,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 */ @@ -1518,9 +1519,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); @@ -1550,9 +1556,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)); }