All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: "Hans Hu(SH-RD)" <HansHu@zhaoxin.com>
Cc: "Tony W. Wang(BJ-RD)" <TonyWWang@zhaoxin.com>,
	"'alsa-devel@alsa-project.org'" <alsa-devel@alsa-project.org>,
	"Tim Guo(BJ-RD)" <TimGuo@zhaoxin.com>,
	"Annie Liu(BJ-RD)" <AnnieLiu@zhaoxin.com>
Subject: Re: 答复: 答复: 答复:  A bug about cache inconsistency report
Date: Mon, 06 Aug 2018 07:36:57 +0200	[thread overview]
Message-ID: <s5hlg9krr7q.wl-tiwai@suse.de> (raw)
In-Reply-To: <6da6ea670ec84b438f59d1d91070903c@zhaoxin.com>

On Mon, 06 Aug 2018 06:01:34 +0200,
Hans Hu(SH-RD) wrote:
> 
> 
> 
> -----邮件原件-----
> 发件人: Takashi Iwai [mailto:tiwai@suse.de]
> 发送时间: 02 August 2018 22:36
> 收件人: Hans Hu(SH-RD) <HansHu@zhaoxin.com>
> 抄送: 'alsa-devel@alsa-project.org' <alsa-devel@alsa-project.org>
> 主题: Re: 答复: 答复: [alsa-devel] A bug about cache inconsistency report
> 
> On Thu, 02 Aug 2018 15:59:31 +0200,
> Hans Hu(SH-RD) wrote:
> >
> > > 1. noise relate test:
> > > When I annotate the return after set_pages_array_wc, then
> > > set_memory_wc works, noise disappear static void
> > > __mark_pages_wc(struct azx *chip, struct snd_dma_buffer *dmab, bool
> > > on) { ...
> > > set_pages_array_wb(sgbuf->page_table, sgbuf->pages); //return; }
> > > #endif
> >
> > > set_memory_uc() performs __pa() for the given address, and in this case, it's vmapped address.  Something bogus in your test, I'm afraid.
> >
> > [Hans:] I know __pa() handle the vmapped address is wrong, that why I don't know how to fix the problem, I try this because I see kernel older than 3.7.6. [bogus? pretending to be real or genuine?] no no no, but the noise problem machine(8086:1c20) in another city, I will make sure it again.
> 
> >I meant about the reliability of the test result.  Or whether we're heading to a wrong fish.
> >In anyway, which kernel version are you testing?  And exactly what is your problem?  Which machine, which hardware setup (codec, etc) and which output, and how is the sound backend (PulseAudio or direct ALSA write)?
> 
> [Hans:] OS: Ubuntu 17.10 + 4.13.0-21-generic (or any one after 3.7.6).
> HW: mother board is Dell 042P49, HDA controller is 8086:1c20, codec is cx20641.
> Output and sound backend: Except the command below, anything other is default setting.
> 
> Problem(what I have done and found): When system setup, I add "modprobe snd-had-intel.snoop=0" in grub, after setup, execute command "aplay -D plughw:0,0 44.1k_16b_2c.wav", then I hear music with noise. The reason cause this is the buffer address should been mark as WC type and the __mark_pages_wc was patched for this, but if define CONFIG_SND_DMA_SGBUF, the actual marked address is the address before vmapped(0xffff88*...*), however the address used in snd_pcm_lib_write_transfer() is the address after vmapped(0xffffc9*...*), so the mark didn't work and cache inconsistency occur(noise).

Well, basically the vmapped address isn't used *at all* on the
HD-audio controller hardware.  The SG buffer addresses are passed on
BDL of the chip, but the vmapped address is used only in PCM core for
some data manipulation.  And this is purely software stuff, hence it
shouldn't matter there whether it's in snoop mode or not.  The snoop
mode difference appears mainly when accessed via mmap from user-space
directly to kernel-space.

That said, I guess your suspect heading to a wrong direction.

> > > And moreover, all Intel chipsets do work with snooping well, AFAIK.
> > [Hans:] yes, all Intel chipsets do work with snooping well here too. And the problem case here is "non-snoop"+"aplay -D plughw:*.*"+"44.1k 16bit"+"8086:1c20", I will see if I can find more machine do the test.
> >Ah, so you explicitly test with non-snoop mode for this chipset?
> >For what purpose...?
> 
> [Hans:] Yes, I've been testing a HDA controller recently(predecessor is VIA), it has the same problem. I want to figure out if it is hardware's problem or not.
> 
> [Hans:] I did the same test on another platform(8086:A170), it do work with non-snoop well.  In addition, I did such a change, oneliner, the purpose is to mark nothing, and it still do work with non-snoop well, this  supposed to be right?
> 
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> #ifdef CONFIG_X86
> static void __mark_pages_wc(struct azx *chip, struct snd_dma_buffer *dmab, bool on)
> {
> int pages;
> +return;

This is essentially other way round; it skips the non-cache setup.


> > > 2. change *_SG relate test:
> > > Just followed your proposal
> > > -snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV_SG,
> > > +snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV,
> >
> > >OK, you need one more oneliner:
> >
> > >diff --git a/sound/pci/hda/hda_controller.c
> > >b/sound/pci/hda/hda_controller.c index a12e594d4e3b..2eaed4956e7f
> > >100644
> > >--- a/sound/pci/hda/hda_controller.c
> > >+++ b/sound/pci/hda/hda_controller.c
> > >@@ -707,7 +707,7 @@ static const struct snd_pcm_ops azx_pcm_ops = {
> >  >.pointer = azx_pcm_pointer,
> >  >.get_time_info =  azx_get_time_info,  >.mmap = azx_pcm_mmap,
> > >-.page = snd_pcm_sgbuf_ops_page,
> > >+// .page = snd_pcm_sgbuf_ops_page,
> > > };
> >
> > > static void azx_pcm_free(struct snd_pcm *pcm)
> > [Hans :] I did this, and the Oops report as the same.
> 
> >My bad, there are other SG-buffer codes in sound/hda/*, too.
> 
> >So scratch these two oneliners.  The only thing you'd need is the big-hammer change (again oneliner :)
> 
> >--- a/sound/core/Kconfig
> >+++ b/sound/core/Kconfig
> >@@ -181,7 +181,7 @@ config SND_VMASTER
>  >bool
> >
> > config SND_DMA_SGBUF
> >-def_bool y
> >+def_bool n
> > depends on X86
> 
> > source "sound/core/seq/Kconfig"
> 
> [Hans :] If this, All the platform are all right.

Then the next step would be to fake sg-buffer from this straight
buffer.  Revert the above, and modify sgbuf.c to the following:
- Allocate a large continuous buffer
- Assign each page in this large buffer

If this still works, it's not about vmap, but it just means that the
physically ordered pages do matter -- implicitly showing that the
snooping behavior isn't properly turned on / off on the controller.


Takashi

> Thank you,
> 
> Hans
> 
> 
> 保密声明:
> 本邮件含有保密或专有信息,仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
> CONFIDENTIAL NOTE:
> This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

  reply	other threads:[~2018-08-06  8:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-02  8:22 A bug about cache inconsistency report Hans Hu(SH-RD)
2018-08-02  8:50 ` Takashi Iwai
2018-08-02 10:56   ` 答复: " Hans Hu(SH-RD)
2018-08-02 11:52     ` Takashi Iwai
     [not found]       ` <fd1d78d3a46845f2867fbc79deabe066@zhaoxin.com>
2018-08-02 14:35         ` 答复: " Takashi Iwai
2018-08-06  4:01           ` 答复: " Hans Hu(SH-RD)
2018-08-06  5:36             ` Takashi Iwai [this message]
2018-08-07  9:00 Hans Hu(SH-RD)
2018-08-07  9:25 ` Takashi Iwai
2018-08-07 10:59   ` 答复: " Hans Hu(SH-RD)
2018-08-07 12:00     ` Takashi Iwai
2018-08-08  9:54       ` 答复: " Hans Hu(SH-RD)
2018-08-08 10:04         ` Takashi Iwai
2018-08-08 10:44           ` 答复: " Hans Hu(SH-RD)
2018-08-08 11:02             ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hlg9krr7q.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=AnnieLiu@zhaoxin.com \
    --cc=HansHu@zhaoxin.com \
    --cc=TimGuo@zhaoxin.com \
    --cc=TonyWWang@zhaoxin.com \
    --cc=alsa-devel@alsa-project.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.