diff mbox series

Can anyone test with AMD onboard or HDMI/DP?

Message ID 87bkur1nil.wl-tiwai@suse.de (mailing list archive)
State New, archived
Headers show
Series Can anyone test with AMD onboard or HDMI/DP? | expand

Commit Message

Takashi Iwai June 17, 2022, 12:29 p.m. UTC 
Hi,

can anyone have an AMD onboard audio device and/or AMD HDMI/DP output
for testing a patch below with 5.18.x or 5.19-rc kernels?  It's a
pending fix (for 5.18+), but currently it can't be verified whether it
causes a regression on the actual audio I/O (while it fixes the kernel
crash).

If the generic allocator still doesn't work as expected here, it
should show some audio stuttering or such effect.


Thanks!

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: memalloc: Drop x86-specific hack for WC allocations

The recent report for a crash on Haswell machines implied that the
x86-specific (rather hackish) implementation for write-cache memory
buffer allocation in ALSA core is buggy with the recent kernel in some
corner cases.  This patch drops the x86-specific implementation and
uses the standard dma_alloc_wc() & co generically for avoiding the bug
and also for simplification.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=216112
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/memalloc.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

Comments

Peter Ujfalusi June 17, 2022, 12:40 p.m. UTC | #1 
On 17/06/2022 15:29, Takashi Iwai wrote:
> Hi,
> 
> can anyone have an AMD onboard audio device and/or AMD HDMI/DP output
> for testing a patch below with 5.18.x or 5.19-rc kernels?  It's a
> pending fix (for 5.18+), but currently it can't be verified whether it
> causes a regression on the actual audio I/O (while it fixes the kernel
> crash).
> 
> If the generic allocator still doesn't work as expected here, it
> should show some audio stuttering or such effect.

But the fallback was needed for some machines using SOF to be able to
load the firmware...
like this:
https://github.com/thesofproject/linux/issues/3609

> 
> Thanks!
> 
> Takashi
> 
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ALSA: memalloc: Drop x86-specific hack for WC allocations
> 
> The recent report for a crash on Haswell machines implied that the
> x86-specific (rather hackish) implementation for write-cache memory
> buffer allocation in ALSA core is buggy with the recent kernel in some
> corner cases.  This patch drops the x86-specific implementation and
> uses the standard dma_alloc_wc() & co generically for avoiding the bug
> and also for simplification.
> 
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=216112
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/core/memalloc.c | 23 +----------------------
>  1 file changed, 1 insertion(+), 22 deletions(-)
> 
> diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c
> index 15dc7160ba34..8cfdaee77905 100644
> --- a/sound/core/memalloc.c
> +++ b/sound/core/memalloc.c
> @@ -431,33 +431,17 @@ static const struct snd_malloc_ops snd_dma_iram_ops = {
>   */
>  static void *snd_dma_dev_alloc(struct snd_dma_buffer *dmab, size_t size)
>  {
> -	void *p;
> -
> -	p = dma_alloc_coherent(dmab->dev.dev, size, &dmab->addr, DEFAULT_GFP);
> -#ifdef CONFIG_X86
> -	if (p && dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC)
> -		set_memory_wc((unsigned long)p, PAGE_ALIGN(size) >> PAGE_SHIFT);
> -#endif
> -	return p;
> +	return dma_alloc_coherent(dmab->dev.dev, size, &dmab->addr, DEFAULT_GFP);
>  }
>  
>  static void snd_dma_dev_free(struct snd_dma_buffer *dmab)
>  {
> -#ifdef CONFIG_X86
> -	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC)
> -		set_memory_wb((unsigned long)dmab->area,
> -			      PAGE_ALIGN(dmab->bytes) >> PAGE_SHIFT);
> -#endif
>  	dma_free_coherent(dmab->dev.dev, dmab->bytes, dmab->area, dmab->addr);
>  }
>  
>  static int snd_dma_dev_mmap(struct snd_dma_buffer *dmab,
>  			    struct vm_area_struct *area)
>  {
> -#ifdef CONFIG_X86
> -	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC)
> -		area->vm_page_prot = pgprot_writecombine(area->vm_page_prot);
> -#endif
>  	return dma_mmap_coherent(dmab->dev.dev, area,
>  				 dmab->area, dmab->addr, dmab->bytes);
>  }
> @@ -471,10 +455,6 @@ static const struct snd_malloc_ops snd_dma_dev_ops = {
>  /*
>   * Write-combined pages
>   */
> -#ifdef CONFIG_X86
> -/* On x86, share the same ops as the standard dev ops */
> -#define snd_dma_wc_ops	snd_dma_dev_ops
> -#else /* CONFIG_X86 */
>  static void *snd_dma_wc_alloc(struct snd_dma_buffer *dmab, size_t size)
>  {
>  	return dma_alloc_wc(dmab->dev.dev, size, &dmab->addr, DEFAULT_GFP);
> @@ -497,7 +477,6 @@ static const struct snd_malloc_ops snd_dma_wc_ops = {
>  	.free = snd_dma_wc_free,
>  	.mmap = snd_dma_wc_mmap,
>  };
> -#endif /* CONFIG_X86 */
>  
>  #ifdef CONFIG_SND_DMA_SGBUF
>  static void *snd_dma_sg_fallback_alloc(struct snd_dma_buffer *dmab, size_t size);
Peter Ujfalusi June 17, 2022, 1:56 p.m. UTC | #2 
Hi Takashi,

On 17/06/2022 15:29, Takashi Iwai wrote:
> Hi,

Try #2, first mail did not made it through, I think.

> can anyone have an AMD onboard audio device and/or AMD HDMI/DP output
> for testing a patch below with 5.18.x or 5.19-rc kernels?  It's a
> pending fix (for 5.18+), but currently it can't be verified whether it
> causes a regression on the actual audio I/O (while it fixes the kernel
> crash).
> 
> If the generic allocator still doesn't work as expected here, it
> should show some audio stuttering or such effect.

But the fallback was needed for some machines using SOF to be able to
load the firmware...
like this:
https://github.com/thesofproject/linux/issues/3609

> 
> 
> Thanks!
> 
> Takashi
> 
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ALSA: memalloc: Drop x86-specific hack for WC allocations
> 
> The recent report for a crash on Haswell machines implied that the
> x86-specific (rather hackish) implementation for write-cache memory
> buffer allocation in ALSA core is buggy with the recent kernel in some
> corner cases.  This patch drops the x86-specific implementation and
> uses the standard dma_alloc_wc() & co generically for avoiding the bug
> and also for simplification.
> 
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=216112
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/core/memalloc.c | 23 +----------------------
>  1 file changed, 1 insertion(+), 22 deletions(-)
> 
> diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c
> index 15dc7160ba34..8cfdaee77905 100644
> --- a/sound/core/memalloc.c
> +++ b/sound/core/memalloc.c
> @@ -431,33 +431,17 @@ static const struct snd_malloc_ops snd_dma_iram_ops = {
>   */
>  static void *snd_dma_dev_alloc(struct snd_dma_buffer *dmab, size_t size)
>  {
> -	void *p;
> -
> -	p = dma_alloc_coherent(dmab->dev.dev, size, &dmab->addr, DEFAULT_GFP);
> -#ifdef CONFIG_X86
> -	if (p && dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC)
> -		set_memory_wc((unsigned long)p, PAGE_ALIGN(size) >> PAGE_SHIFT);
> -#endif
> -	return p;
> +	return dma_alloc_coherent(dmab->dev.dev, size, &dmab->addr, DEFAULT_GFP);
>  }
>  
>  static void snd_dma_dev_free(struct snd_dma_buffer *dmab)
>  {
> -#ifdef CONFIG_X86
> -	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC)
> -		set_memory_wb((unsigned long)dmab->area,
> -			      PAGE_ALIGN(dmab->bytes) >> PAGE_SHIFT);
> -#endif
>  	dma_free_coherent(dmab->dev.dev, dmab->bytes, dmab->area, dmab->addr);
>  }
>  
>  static int snd_dma_dev_mmap(struct snd_dma_buffer *dmab,
>  			    struct vm_area_struct *area)
>  {
> -#ifdef CONFIG_X86
> -	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC)
> -		area->vm_page_prot = pgprot_writecombine(area->vm_page_prot);
> -#endif
>  	return dma_mmap_coherent(dmab->dev.dev, area,
>  				 dmab->area, dmab->addr, dmab->bytes);
>  }
> @@ -471,10 +455,6 @@ static const struct snd_malloc_ops snd_dma_dev_ops = {
>  /*
>   * Write-combined pages
>   */
> -#ifdef CONFIG_X86
> -/* On x86, share the same ops as the standard dev ops */
> -#define snd_dma_wc_ops	snd_dma_dev_ops
> -#else /* CONFIG_X86 */
>  static void *snd_dma_wc_alloc(struct snd_dma_buffer *dmab, size_t size)
>  {
>  	return dma_alloc_wc(dmab->dev.dev, size, &dmab->addr, DEFAULT_GFP);
> @@ -497,7 +477,6 @@ static const struct snd_malloc_ops snd_dma_wc_ops = {
>  	.free = snd_dma_wc_free,
>  	.mmap = snd_dma_wc_mmap,
>  };
> -#endif /* CONFIG_X86 */
>  
>  #ifdef CONFIG_SND_DMA_SGBUF
>  static void *snd_dma_sg_fallback_alloc(struct snd_dma_buffer *dmab, size_t size);
Takashi Iwai June 17, 2022, 2:37 p.m. UTC | #3 
On Fri, 17 Jun 2022 15:56:31 +0200,
Péter Ujfalusi wrote:
> 
> Hi Takashi,
> 
> On 17/06/2022 15:29, Takashi Iwai wrote:
> > Hi,
> 
> Try #2, first mail did not made it through, I think.
> 
> > can anyone have an AMD onboard audio device and/or AMD HDMI/DP output
> > for testing a patch below with 5.18.x or 5.19-rc kernels?  It's a
> > pending fix (for 5.18+), but currently it can't be verified whether it
> > causes a regression on the actual audio I/O (while it fixes the kernel
> > crash).
> > 
> > If the generic allocator still doesn't work as expected here, it
> > should show some audio stuttering or such effect.
> 
> But the fallback was needed for some machines using SOF to be able to
> load the firmware...
> like this:
> https://github.com/thesofproject/linux/issues/3609

The bug isn't about the fallback of SG buffer we've already resolved,
but rather about the continuous WC page allocations that happen for
the CORB/RIRB or the position buffer on some devices like AMD.
The Intel HD-audio doesn't hit the problem.


Takashi

> 
> > 
> > 
> > Thanks!
> > 
> > Takashi
> > 
> > -- 8< --
> > From: Takashi Iwai <tiwai@suse.de>
> > Subject: [PATCH] ALSA: memalloc: Drop x86-specific hack for WC allocations
> > 
> > The recent report for a crash on Haswell machines implied that the
> > x86-specific (rather hackish) implementation for write-cache memory
> > buffer allocation in ALSA core is buggy with the recent kernel in some
> > corner cases.  This patch drops the x86-specific implementation and
> > uses the standard dma_alloc_wc() & co generically for avoiding the bug
> > and also for simplification.
> > 
> > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=216112
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  sound/core/memalloc.c | 23 +----------------------
> >  1 file changed, 1 insertion(+), 22 deletions(-)
> > 
> > diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c
> > index 15dc7160ba34..8cfdaee77905 100644
> > --- a/sound/core/memalloc.c
> > +++ b/sound/core/memalloc.c
> > @@ -431,33 +431,17 @@ static const struct snd_malloc_ops snd_dma_iram_ops = {
> >   */
> >  static void *snd_dma_dev_alloc(struct snd_dma_buffer *dmab, size_t size)
> >  {
> > -	void *p;
> > -
> > -	p = dma_alloc_coherent(dmab->dev.dev, size, &dmab->addr, DEFAULT_GFP);
> > -#ifdef CONFIG_X86
> > -	if (p && dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC)
> > -		set_memory_wc((unsigned long)p, PAGE_ALIGN(size) >> PAGE_SHIFT);
> > -#endif
> > -	return p;
> > +	return dma_alloc_coherent(dmab->dev.dev, size, &dmab->addr, DEFAULT_GFP);
> >  }
> >  
> >  static void snd_dma_dev_free(struct snd_dma_buffer *dmab)
> >  {
> > -#ifdef CONFIG_X86
> > -	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC)
> > -		set_memory_wb((unsigned long)dmab->area,
> > -			      PAGE_ALIGN(dmab->bytes) >> PAGE_SHIFT);
> > -#endif
> >  	dma_free_coherent(dmab->dev.dev, dmab->bytes, dmab->area, dmab->addr);
> >  }
> >  
> >  static int snd_dma_dev_mmap(struct snd_dma_buffer *dmab,
> >  			    struct vm_area_struct *area)
> >  {
> > -#ifdef CONFIG_X86
> > -	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC)
> > -		area->vm_page_prot = pgprot_writecombine(area->vm_page_prot);
> > -#endif
> >  	return dma_mmap_coherent(dmab->dev.dev, area,
> >  				 dmab->area, dmab->addr, dmab->bytes);
> >  }
> > @@ -471,10 +455,6 @@ static const struct snd_malloc_ops snd_dma_dev_ops = {
> >  /*
> >   * Write-combined pages
> >   */
> > -#ifdef CONFIG_X86
> > -/* On x86, share the same ops as the standard dev ops */
> > -#define snd_dma_wc_ops	snd_dma_dev_ops
> > -#else /* CONFIG_X86 */
> >  static void *snd_dma_wc_alloc(struct snd_dma_buffer *dmab, size_t size)
> >  {
> >  	return dma_alloc_wc(dmab->dev.dev, size, &dmab->addr, DEFAULT_GFP);
> > @@ -497,7 +477,6 @@ static const struct snd_malloc_ops snd_dma_wc_ops = {
> >  	.free = snd_dma_wc_free,
> >  	.mmap = snd_dma_wc_mmap,
> >  };
> > -#endif /* CONFIG_X86 */
> >  
> >  #ifdef CONFIG_SND_DMA_SGBUF
> >  static void *snd_dma_sg_fallback_alloc(struct snd_dma_buffer *dmab, size_t size);
> 
> -- 
> Péter
>
diff mbox series

Patch

diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c
index 15dc7160ba34..8cfdaee77905 100644
--- a/sound/core/memalloc.c
+++ b/sound/core/memalloc.c
@@ -431,33 +431,17 @@  static const struct snd_malloc_ops snd_dma_iram_ops = {
  */
 static void *snd_dma_dev_alloc(struct snd_dma_buffer *dmab, size_t size)
 {
-	void *p;
-
-	p = dma_alloc_coherent(dmab->dev.dev, size, &dmab->addr, DEFAULT_GFP);
-#ifdef CONFIG_X86
-	if (p && dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC)
-		set_memory_wc((unsigned long)p, PAGE_ALIGN(size) >> PAGE_SHIFT);
-#endif
-	return p;
+	return dma_alloc_coherent(dmab->dev.dev, size, &dmab->addr, DEFAULT_GFP);
 }
 
 static void snd_dma_dev_free(struct snd_dma_buffer *dmab)
 {
-#ifdef CONFIG_X86
-	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC)
-		set_memory_wb((unsigned long)dmab->area,
-			      PAGE_ALIGN(dmab->bytes) >> PAGE_SHIFT);
-#endif
 	dma_free_coherent(dmab->dev.dev, dmab->bytes, dmab->area, dmab->addr);
 }
 
 static int snd_dma_dev_mmap(struct snd_dma_buffer *dmab,
 			    struct vm_area_struct *area)
 {
-#ifdef CONFIG_X86
-	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC)
-		area->vm_page_prot = pgprot_writecombine(area->vm_page_prot);
-#endif
 	return dma_mmap_coherent(dmab->dev.dev, area,
 				 dmab->area, dmab->addr, dmab->bytes);
 }
@@ -471,10 +455,6 @@  static const struct snd_malloc_ops snd_dma_dev_ops = {
 /*
  * Write-combined pages
  */
-#ifdef CONFIG_X86
-/* On x86, share the same ops as the standard dev ops */
-#define snd_dma_wc_ops	snd_dma_dev_ops
-#else /* CONFIG_X86 */
 static void *snd_dma_wc_alloc(struct snd_dma_buffer *dmab, size_t size)
 {
 	return dma_alloc_wc(dmab->dev.dev, size, &dmab->addr, DEFAULT_GFP);
@@ -497,7 +477,6 @@  static const struct snd_malloc_ops snd_dma_wc_ops = {
 	.free = snd_dma_wc_free,
 	.mmap = snd_dma_wc_mmap,
 };
-#endif /* CONFIG_X86 */
 
 #ifdef CONFIG_SND_DMA_SGBUF
 static void *snd_dma_sg_fallback_alloc(struct snd_dma_buffer *dmab, size_t size);