Re: [PATCH 6/9] ALSA: emu10k1: fix PCM playback buffer size constraints

On Wed, 17 May 2023 19:42:53 +0200, Oswald Buddenhagen wrote:

The period_bytes_min parameter made no sense at all, as it didn't correlate with any hardware limitation.

Does the device really work with less than 64 bytes period size? I meant not in theory but in practice. Without any value set, dumb applications may try to set 4 bytes for period size, so setting some practical limit still makes sense.

Takashi

The same can be said of the buffer_bytes minimum constraint. Instead, apply a buffer_size constraint, which works on frames.

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

Signed-off-by: Oswald Buddenhagen oswald.buddenhagen@gmx.de

sound/pci/emu10k1/emupcm.c | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/sound/pci/emu10k1/emupcm.c b/sound/pci/emu10k1/emupcm.c index feb575922738..5226f0978408 100644 --- a/sound/pci/emu10k1/emupcm.c +++ b/sound/pci/emu10k1/emupcm.c @@ -457,9 +457,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,

@@ -851,7 +850,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,

@@ -956,25 +954,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_BUFFER_SIZE, 256, 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++)

@@ -1005,12 +1024,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;

-- 2.40.0.152.g15d061e6df