Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Keyon Jie <yang.jie@linux.intel.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	"Rajwa, Marcin" <marcin.rajwa@linux.intel.com>
Subject: Re: [alsa-devel] [PATCH] ALSA: pcm: fix buffer_bytes max constrained by preallocated bytes issue
Date: Sun, 19 Jan 2020 11:43:10 +0100
Message-ID: <s5h7e1nohfl.wl-tiwai@suse.de> (raw)
In-Reply-To: <ab37a499-305f-2ce8-9b29-1483ea473a57@linux.intel.com>

On Sun, 19 Jan 2020 11:14:56 +0100,
Keyon Jie wrote:
> 
> On 2020/1/19 下午5:04, Takashi Iwai wrote:
> > On Sun, 19 Jan 2020 09:11:17 +0100,
> > Keyon Jie wrote:
> >> On 2020/1/19 下午3:09, Takashi Iwai wrote:
> >> It varies for each stream, most of them are 65536 Bytes only, whereas
> >> one for Wake-On-Voice might need a > 4 Seconds buffer could be up to
> >> about 1~2MBytes, and another one for deep-buffer playback can be up to
> >> about 8MBytes.
> > Hm, so this varies so much depending on the use case?
> > I thought it comes from the topology file and it's essentially
> > consistent over various purposes.
> 
> Yes, we add different buffer_bytes_max limitation to each stream
> depending on its use case, basically we set it to the maximum value we
> claim to support only, we don't want to waste any of the system
> memory.
> 
> >
> >>> I think we can go for passing zero as default, which means skipping
> >>> preallocation.  In addition, we may add an upper limit of the total
> >> Just did an experiment and this works for me, I believe we still need
> >> to call snd_pcm_set_managed_buffer() though the preallocation is
> >> skipped in this, right?
> > No, snd_pcm_set_managed_buffer() is the new PCM preallocation API.
> > The old snd_pcm_lib_preallocate*() is almost gone.
> 
> What I asked is actually that since the preallocation will be
> skipped(with passing size=0), can we just not calling
> snd_pcm_set_managed_buffer() or snd_pcm_lib_preallocate*() in our SOF
> PCM driver? I believe no(we still need the invoking to do
> initialization except buffer allocating)?

You still need to call it.  Otherwise the PCM core doesn't know what
kind of buffer type has to be allocated.

Basically snd_pcm_set_managed_buffer() or snd_pcm_lib_preallocate*()
does two things: set the buffer type and its preallocation (default
and max size).  The latter default size can be 0, meaning that no
default preallocation is performed.  Also the max can be 0, i.e. no
preallocation is needed at all for the buffers (e.g. vmalloc
buffers).  Meanwhile the buffer type and its device pointer are
mandatory and can't be skipped.

> >
> >>> amount of allocation per card, controlled in pcm_memory.c, for
> >>> example.  This logic can be applied to the legacy HDA, too.
> >>>
> >>> This should be relatively easy, and I'll provide the patch in the next
> >>> week.
> >> OK, that's fine for me also, thank you.
> > Below is a quick hack for HDA.  We still need the certain amount of
> > preallocation for non-x86 systems that don't support SG-buffers, so
> > a bit of trick is applied to Kconfig.
> >
> > Totally untested, as usual.
> 
> Did a quick test(plus passing 0 size for preallocate in SOF PCM
> driver) and it works for my use case(no regression comparing that
> without applying this patch), Thank you.

OK, will tidy up and submit later.


Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

  reply index

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-16  4:53 Keyon Jie
2020-01-16  7:15 ` Takashi Iwai
2020-01-16  9:50   ` Keyon Jie
2020-01-16 10:27     ` Takashi Iwai
2020-01-16 11:25       ` Keyon Jie
2020-01-16 11:50         ` Takashi Iwai
2020-01-16 14:14           ` Jie, Yang
2020-01-16 15:31             ` Jie, Yang
2020-01-16 16:07               ` Takashi Iwai
2020-01-16 16:39               ` Pierre-Louis Bossart
2020-01-16 17:25                 ` Rajwa, Marcin
2020-01-16 17:40                   ` Pierre-Louis Bossart
2020-01-16 20:37                     ` Takashi Iwai
2020-01-17  5:30                       ` Keyon Jie
2020-01-17  7:57                         ` Takashi Iwai
2020-01-17 10:13                           ` Keyon Jie
2020-01-17 10:30                             ` Takashi Iwai
2020-01-17 10:56                               ` Keyon Jie
2020-01-17 11:15                                 ` Takashi Iwai
2020-01-17  5:37                     ` Keyon Jie
2020-01-17  8:00                       ` Takashi Iwai
2020-01-17 10:43                         ` Keyon Jie
2020-01-17 11:12                           ` Takashi Iwai
2020-01-19  3:52                             ` Keyon Jie
2020-01-19  7:09                               ` Takashi Iwai
2020-01-19  8:11                                 ` Keyon Jie
2020-01-19  9:04                                   ` Takashi Iwai
2020-01-19 10:14                                     ` Keyon Jie
2020-01-19 10:43                                       ` Takashi Iwai [this message]
2020-01-20  2:23                                         ` Keyon Jie
2020-01-16 15:45             ` Takashi Iwai

Reply instructions:

You may reply publically 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=s5h7e1nohfl.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=marcin.rajwa@linux.intel.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=yang.jie@linux.intel.com \
    /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

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git