diff mbox series

[v2,9/9] ALSA: emu10k1: fix PCM playback buffer size constraints

Message ID 20230518092224.3696958-9-oswald.buddenhagen@gmx.de
State New
Headers show
Series [v2,1/9] ALSA: emu10k1: pass frame instead of byte addresses | expand

Commit Message

Oswald Buddenhagen May 18, 2023, 9:22 a.m. UTC
The period_bytes_min parameter and the buffer_bytes minimum constraint
made no sense at all, as they didn't reflect any hardware limitation.
Instead, apply a frame-based period_size minimum constraint, which is
derived from the cache size (it would be actually possible to go below
that, but it would require special handling, and it would be practically
impossible to keep up with the IRQ rate anyway).

Sync up the constraints of the EFX playback with those of the regular
playback, as there is no reason for them to diverge.

N.b., the maximum buffer size is actually arbitrary - the hardware could
go waay beyond 128 KiB.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
v2:
- constrain period size instead of buffer size, as otherwise a stupid
  app could set a small buffer with many periods, thus making the period
  size too small. takashi was actually right! :-D
---
 sound/pci/emu10k1/emupcm.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

Comments

Takashi Iwai May 18, 2023, 11:04 a.m. UTC | #1
On Thu, 18 May 2023 11:29:00 +0200,
Oswald Buddenhagen wrote:
> 
> sorry for the "flood", i forgot to adjust the declaration of the
> series in my tool. you can ignore the other patches; they didn't
> change from what you already applied.

Don't worry, now I applied this one.


Thanks!

Takashi
diff mbox series

Patch

diff --git a/sound/pci/emu10k1/emupcm.c b/sound/pci/emu10k1/emupcm.c
index 89f7e85034b6..9045359bb461 100644
--- a/sound/pci/emu10k1/emupcm.c
+++ b/sound/pci/emu10k1/emupcm.c
@@ -444,9 +444,8 @@  static const struct snd_pcm_hardware snd_emu10k1_efx_playback =
 	.rate_max =		48000,
 	.channels_min =		NUM_EFX_PLAYBACK,
 	.channels_max =		NUM_EFX_PLAYBACK,
-	.buffer_bytes_max =	(64*1024),
-	.period_bytes_min =	64,
-	.period_bytes_max =	(64*1024),
+	.buffer_bytes_max =	(128*1024),
+	.period_bytes_max =	(128*1024),
 	.periods_min =		2,
 	.periods_max =		2,
 	.fifo_size =		0,
@@ -877,7 +876,6 @@  static const struct snd_pcm_hardware snd_emu10k1_playback =
 	.channels_min =		1,
 	.channels_max =		2,
 	.buffer_bytes_max =	(128*1024),
-	.period_bytes_min =	64,
 	.period_bytes_max =	(128*1024),
 	.periods_min =		1,
 	.periods_max =		1024,
@@ -982,25 +980,46 @@  static int snd_emu10k1_efx_playback_close(struct snd_pcm_substream *substream)
 	return 0;
 }
 
+static int snd_emu10k1_playback_set_constraints(struct snd_pcm_runtime *runtime)
+{
+	int err;
+
+	// The buffer size must be a multiple of the period size, to avoid a
+	// mismatch between the extra voice and the regular voices.
+	err = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
+	if (err < 0)
+		return err;
+	// The hardware is typically the cache's size of 64 frames ahead.
+	// Leave enough time for actually filling up the buffer.
+	err = snd_pcm_hw_constraint_minmax(
+			runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 128, UINT_MAX);
+	return err;
+}
+
 static int snd_emu10k1_efx_playback_open(struct snd_pcm_substream *substream)
 {
 	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
 	struct snd_emu10k1_pcm *epcm;
 	struct snd_emu10k1_pcm_mixer *mix;
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	int i, j;
+	int i, j, err;
 
 	epcm = kzalloc(sizeof(*epcm), GFP_KERNEL);
 	if (epcm == NULL)
 		return -ENOMEM;
 	epcm->emu = emu;
 	epcm->type = PLAYBACK_EFX;
 	epcm->substream = substream;
 	
 	runtime->private_data = epcm;
 	runtime->private_free = snd_emu10k1_pcm_free_substream;
 	runtime->hw = snd_emu10k1_efx_playback;
-	
+	err = snd_emu10k1_playback_set_constraints(runtime);
+	if (err < 0) {
+		kfree(epcm);
+		return err;
+	}
+
 	for (i = 0; i < NUM_EFX_PLAYBACK; i++) {
 		mix = &emu->efx_pcm_mixer[i];
 		for (j = 0; j < 8; j++)
@@ -1031,12 +1050,7 @@  static int snd_emu10k1_playback_open(struct snd_pcm_substream *substream)
 	runtime->private_data = epcm;
 	runtime->private_free = snd_emu10k1_pcm_free_substream;
 	runtime->hw = snd_emu10k1_playback;
-	err = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
-	if (err < 0) {
-		kfree(epcm);
-		return err;
-	}
-	err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 256, UINT_MAX);
+	err = snd_emu10k1_playback_set_constraints(runtime);
 	if (err < 0) {
 		kfree(epcm);
 		return err;