diff mbox

[4/4] sounds/hda/radeon: Disable 64-bit DMA on radeon

Message ID 1412129393.4285.195.camel@pasglop
State Superseded
Headers show

Commit Message

Benjamin Herrenschmidt Oct. 1, 2014, 2:09 a.m. UTC
The chipset has a limitation in the number of address bits it
can generate. The graphics portion uses a specific mask of
40 or 48 bits depending on the generation. For audio, it's a bit
less of an issue, so just mark them as no-64bit for now.

Without this, it crashes on POWER machines which can use high bits
in the DMA address to distinguish between DMA windows. 

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org>
---
 sound/pci/hda/hda_intel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)



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

Comments

Takashi Iwai Oct. 1, 2014, 7:38 a.m. UTC | #1
At Wed, 01 Oct 2014 12:09:53 +1000,
Benjamin Herrenschmidt wrote:
> 
> The chipset has a limitation in the number of address bits it
> can generate. The graphics portion uses a specific mask of
> 40 or 48 bits depending on the generation. For audio, it's a bit
> less of an issue, so just mark them as no-64bit for now.
> 
> Without this, it crashes on POWER machines which can use high bits
> in the DMA address to distinguish between DMA windows. 
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: <stable@vger.kernel.org>
> ---
>  sound/pci/hda/hda_intel.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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?


Takashi
--
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
Benjamin Herrenschmidt Oct. 1, 2014, 7:41 a.m. UTC | #2
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...

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
Benjamin Herrenschmidt Oct. 1, 2014, 7:48 a.m. UTC | #3
On Wed, 2014-10-01 at 17:41 +1000, Benjamin Herrenschmidt wrote:

> 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...

Note that I made the assumption that in the grand scheme of things, HDMI
audio isn't a very big throughput device and thus the extra iommu
mappings (or swiotlb) shouldn't hurt that much. But I may be wrong...

So by all means, let me know what you want done here, but I don't think
CONFIG_PPC is the right thing to do here, especially since this is not
even something that affects all PPCs, but specifically the POWER server
PCI bridges for which we configure a 64-bit window high up in the
address space.

FYI: Basically we setup PCI DMA addresses 0...2G as a 32-bit IOMMU
remapped window and we also configure a "direct map" window higher up
that bypasses the IOMMU and provides a direct linear mapping of RAM for
64-bit capable devices. The problem is that the HW on those server chips
requires that second window to have address bit 59 set. Then based on
the DMA mask, the arch code provide the device with DMA ops that will
either go through the iommu remap window or through the bypass window
based on the DMA mask set by the driver.

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
diff mbox

Patch

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)
 
 /* quirks for Nvidia */
 #define AZX_DCAPS_PRESET_NVIDIA \