July 2019 - Patch - mailman.alsa-project.org

[PATCH] ALSA: pcm: fix lost wakeup event scenarios in snd_pcm_drain
by Adam Miartus 29 Jul '19

29 Jul '19

From: Yuki Tsunashima <ytsunashima(a)jp.adit-jv.com> lost wakeup can occur after enabling irq, therefore put task into interruptible before enabling interrupts, without this change, task can be put to sleep and snd_pcm_drain will delay Signed-off-by: Yuki Tsunashima <ytsunashima(a)jp.adit-jv.com> Signed-off-by: Suresh Udipi <sudipi(a)jp.adit-jv.com> [ported from 4.9] Signed-off-by: Adam Miartus <amiartus(a)de.adit-jv.com> --- sound/core/pcm_native.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 860543a..fe333c8 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1874,6 +1874,7 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream, if (!to_check) break; /* all drained */ init_waitqueue_entry(&wait, current); + set_current_state(TASK_INTERRUPTIBLE); add_wait_queue(&to_check->sleep, &wait); snd_pcm_stream_unlock_irq(substream); if (runtime->no_period_wakeup) @@ -1886,7 +1887,7 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream, } tout = msecs_to_jiffies(tout * 1000); } - tout = schedule_timeout_interruptible(tout); + tout = schedule_timeout(tout); snd_pcm_stream_lock_irq(substream); group = snd_pcm_stream_group_ref(substream); -- 2.7.4

1 0

[PATCH 0/2 v2] Open Alsa dshare without channel binding
by Adam Miartus 24 Jul '19

24 Jul '19

The general idea of this patch is to be able to open the device without defined bindings. As a result, it allows starting the audio clock of underlying device without blocking a channel. This might be useful for creating a dummy dsnoop pcm for hardware that requires running clock early at system start. This clock is then present even without application streaming audio data. Patch n.1 is intended to check for null pointer of pcm callbacks, it is not intended to alter any current functionality, properly defined plugins should not suffer from this change Patch n.2 is not intended to alter any current functionality, only extend it V2 of the patch, changes from V1: - instead of creating new "pcm_unsupported.c" with ops and fast_ops callbacks returning -ENOSYS, callback == NULL is checked directly in core implementation and error is returned when pcm has no callback defined Adam Miartus (2): alsa: pcm: return -ENOSYS when ops or fast_ops callback is NULL alsa: dshare: allow missing bindings src/pcm/pcm.c | 134 +++++++++++++++++++++++++++++++++++++++++---------- src/pcm/pcm_dshare.c | 29 ++++++----- src/pcm/pcm_local.h | 20 ++++++++ src/pcm/pcm_mmap.c | 15 ++++-- src/pcm/pcm_params.c | 10 +++- 5 files changed, 166 insertions(+), 42 deletions(-) -- 2.7.4

1 2

[PATCH v2] ASoC: Intel: Skylake: Recover BXT FW on DSP boot timeout error
by Pawel Harlozinski 08 Jul '19

08 Jul '19

When DSP boots with timeout error try to reload firmware to keep best user expeirience. Signed-off-by: Szymon Mielczarek <szymonx.mielczarek(a)intel.com> Signed-off-by: Pawel Harlozinski <pawel.harlozinski(a)linux.intel.com> --- sound/soc/intel/skylake/bxt-sst.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c index 440bca7afbf1..dd0260557530 100644 --- a/sound/soc/intel/skylake/bxt-sst.c +++ b/sound/soc/intel/skylake/bxt-sst.c @@ -455,13 +455,19 @@ static int bxt_set_dsp_D0(struct sst_dsp *ctx, unsigned int core_id) /* If core 1 was turned on for booting core 0, turn it off */ skl_dsp_core_power_down(ctx, SKL_DSP_CORE_MASK(1)); if (ret == 0) { - dev_err(ctx->dev, "%s: DSP boot timeout\n", __func__); - dev_err(ctx->dev, "Error code=0x%x: FW status=0x%x\n", + dev_warn(ctx->dev, + "DSP boot timeout: Error code=0x%x: FW status=0x%x\n", sst_dsp_shim_read(ctx, BXT_ADSP_ERROR_CODE), sst_dsp_shim_read(ctx, BXT_ADSP_FW_STATUS)); - dev_err(ctx->dev, "Failed to set core0 to D0 state\n"); - ret = -EIO; - goto err; + dev_warn(ctx->dev,"Reloading FW\n"); + + ret = bxt_sst_init_fw(skl->dev, skl); + if (ret < 0) { + dev_err(ctx->dev, "Reload fw failed: %d\n", ret); + dev_err(ctx->dev, "Failed to set core0 to D0 state\n"); + ret = -EIO; + goto err; + } } } @@ -486,7 +492,6 @@ static int bxt_set_dsp_D0(struct sst_dsp *ctx, unsigned int core_id) if (core_id == SKL_DSP_CORE0_ID) core_mask |= SKL_DSP_CORE_MASK(1); skl_dsp_disable_core(ctx, core_mask); - return ret; } -- 2.17.1

1 0

[PATCH 0/2] expand dshare to allow audio clock when not streaming data
by Adam Miartus 08 Jul '19

08 Jul '19

The general idea of these changes is to be able to open the device without defined bindings. This would start the audio clock without blocking a channel. This might be useful for hardware that requires running clock early at system start and to be available even without application streaming audio data. Andreas Pape (2): alsa: pcm: add unsupported OPS alsa: dshare: allow missing bindings src/pcm/Makefile.am | 4 +- src/pcm/pcm_dshare.c | 72 +++++++++++-- src/pcm/pcm_unsupported.c | 265 ++++++++++++++++++++++++++++++++++++++++++++++ src/pcm/pcm_unsupported.h | 167 +++++++++++++++++++++++++++++ 4 files changed, 495 insertions(+), 13 deletions(-) create mode 100644 src/pcm/pcm_unsupported.c create mode 100644 src/pcm/pcm_unsupported.h -- 2.7.4

1 2

[PATCH 2/3 v2] pcm_file: improve error checking in write_wav_header function
by Adam Miartus 05 Jul '19

05 Jul '19

previously errno would be returned even for cases where it may have not been populated, for example one of the write functions failing, or writing only partial buffer, now progress through write operations separately and report errno when appropriate Signed-off-by: Adam Miartus <amiartus(a)de.adit-jv.com> Reviewed-by: Timo Wischer <twischer(a)de.adit-jv.com> --- src/pcm/pcm_file.c | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c index 03c1640..da7f087 100644 --- a/src/pcm/pcm_file.c +++ b/src/pcm/pcm_file.c @@ -327,6 +327,8 @@ static void setup_wav_header(snd_pcm_t *pcm, struct wav_fmt *fmt) static int write_wav_header(snd_pcm_t *pcm) { snd_pcm_file_t *file = pcm->private_data; + ssize_t res; + static const char header[] = { 'R', 'I', 'F', 'F', 0x24, 0, 0, 0, @@ -341,15 +343,35 @@ static int write_wav_header(snd_pcm_t *pcm) setup_wav_header(pcm, &file->wav_header); - if (write(file->fd, header, sizeof(header)) != sizeof(header) || - write(file->fd, &file->wav_header, sizeof(file->wav_header)) != - sizeof(file->wav_header) || - write(file->fd, header2, sizeof(header2)) != sizeof(header2)) { - int err = errno; - SYSERR("%s write header failed, file data may be corrupt", file->fname); - return -err; - } + res = write(file->fd, header, sizeof(header)); + if (res != sizeof(header)) + goto write_error; + + res = write(file->fd, &file->wav_header, sizeof(file->wav_header)); + if (res != sizeof(file->wav_header)) + goto write_error; + + res = write(file->fd, header2, sizeof(header2)); + if (res != sizeof(header2)) + goto write_error; + return 0; + +write_error: + /* + * print real errno if available and return EIO, reason for this is + * to block possible EPIPE in case file->fd is a pipe. EPIPE from + * file->fd conflicts with EPIPE from playback stream which should + * be used to signal XRUN on playback device + */ + if (res < 0) + SYSERR("%s write header failed, file data may be corrupt", file->fname); + else + SNDERR("%s write header incomplete, file data may be corrupt", file->fname); + + memset(&file->wav_header, 0, sizeof(struct wav_fmt)); + + return -EIO; } /* fix up the length fields in WAV header */ -- 2.7.4

1 0

[PATCH 0/3] pcm_file report EIO when fail to write to debug file
by Adam Miartus 01 Jul '19

01 Jul '19

Previously, I introduced a patch that may return EPIPE error when pcm_file plugin fails to write to output file for some reason. The purpose of reporting the error was to evade assert in code to allow API user error handling and recovery, or more clarity in debugging. Failing to write to a file usually means bad file descriptor, running out of free memory, insufficient file permissions or in case file is a pipe there may be other reasons preventing the write operation. All these are not easily recoverable by restaring playback as EPIPE error which is used to signal XRUN might suggest. Therefore, use EIO to signal a different kind of error. write_wav_header function is refactored a bit for more detailed error handling in case of failed write operation. Adam Miartus (3): pcm_file: use EIO instead of EPIPE when failing to write output file pcm_file: improve error checking in write_wav_header function pcm_file: in case of failed write clear file buffer variables src/pcm/pcm_file.c | 54 +++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 13 deletions(-) -- 2.7.4

1 3

GIT: Regarding the issue we are facing in the commit 37728639ae05de702825d96bd1d42e24ae772248
by vanitha.channaiah＠in.bosch.com 01 Jul '19

01 Jul '19

This mail is regarding the fix at below: commit 37728639ae05de702825d96bd1d42e24ae772248 Author: Jaroslav Kysela <perex(a)perex.cz> Date: Sat Feb 7 15:01:31 2004 +00 The commit has following changes : ******************************* code ***************************************** static int snd_pcm_rate_hw_refine_cchange(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, snd_pcm_hw_params_t *sparams) { + buffer_size = snd_pcm_hw_param_get_interval(params, SND_PCM_HW_PARAM_BUFFER_SIZE); + /* + * this condition probably needs more work: + * in case when the buffer_size is known and we are looking + * for best period_size, we should prefer situation when + * (buffer_size / period_size) * period_size == buffer_size + */ + if (snd_interval_single(buffer_size) && buffer_size->integer) { + snd_interval_t *period_size; + period_size = (snd_interval_t *)snd_pcm_hw_param_get_interval(params, SND_PCM_HW_PARAM_PERIOD_SIZE); + if (!snd_interval_checkempty(period_size) && + period_size->openmin && period_size->openmax && + period_size->min + 1 == period_size->max) { + if ((buffer_size->min / period_size->min) * period_size->min == buffer_size->min) { + snd_interval_set_value(period_size, period_size->min); + } else if ((buffer_size->max / period_size->max) * period_size->max == buffer_size->max) { + snd_interval_set_value(period_size, period_size->max); + } + } + } } ******************************* code **************************************** Update rate depending on period_size and period_time. Issue found in Hardware : a. RCAR salvator-xs which supports 2 channel. b. IMX which supports 8 channel. 1. Usecase which is PASSED without the mentioned commit and FAILED with the commit. For the usecase with below command : rate = 11025 channel = 6 $aplay -Dentertainment_main -r11025 -c6 -fS16_LE /dev/urandom a. With the commit: In the calculation of RATE at Rule 7: RATE=11025 dependent parameters are: PERIOD_SIZE=88 PERIOD_TIME=8000 parameters min max open_min open_max interval PERIOD_SIZE 88 88 0 0 0 PERIO_TIME 8000 8000 0 0 0 RATE 11025 11000 0 0 1 RATE values are calculated in snd_interval_refine() Return value: INVALID as rate_min > rate_max This is because, dependent parameter “period_size” is rounded to 88 in “rate plugin” in snd_pcm_rate_hw_refine_cchange() Since, the below condition gets satisfied(buffer_size = 352 aligned to period_size = 88), period_size gets rounded to 88. if ((buffer_size->min / period_size->min) * period_size->min == buffer_size->min) { This commit changes causing the issue to get rate_min > rate_max. The flow of code execution is as follows: - snd_pcm_hw_refine_slave() - Enters snd_pcm_rate_hw_refine_cchange(), rounding of period_size to 88. - snd_pcm_rate_hw_refine_cchange() exit. - snd_pcm_hw_refine_soft() is called, here exists params->rmask to evaluate for SAMPLE_BITS, FRAME_BITS, PERIOD_BYTES, BUFFER_BYTES - execution of RULES calculation. At RULE 7 rate calculation INVALID error is observed. b. Without the commit: In the case if period_size doesn’t get rounded off to 88, the usecase would get PASSED. Rate calculation goes fine with period_size open interval (88, 89) parameters min max open_min open_max interval PERIOD_SIZE 88 89 0 0 0 PERIOD_TIME 8000 8000 0 0 0 RATE 11025 11025 0 0 1 Here, there is no issue of rate_min > rate_max. **************************************************************************************************************************************************************** Now, I just have one scenario where with/without commit, rate calculation goes fine. 2. Usecase which is PASSED with and without above commit. For the usecase with below command : rate =11025 channel=2 $aplay -Dentertainment_main -r11025 -c2 -fS16_LE /dev/urandom a. Including the above commit code: Below is the calculation that goes: The flow of code execution is as follows: - snd_pcm_hw_refine_slave() - Enters snd_pcm_rate_hw_refine_cchange() , rounding of period_size to 88. - snd_pcm_rate_hw_refine_cchange() exit. - snd_pcm_hw_refine_soft() is called, here params->rmask is 0. - RULES calculation doesnt occur. b. Without the commit: In the case if period_size doesn’t get rounded off to 88. Rate calculation goes fine with period_size open interval (88, 89) Is there any dependency for the commit ? If yes, can you please suggest the corner case which fails without the commit ? Can we revert the changes ?

1 0