January 2019 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] [PATCH 0/8] alsa-utils: axfer: fulfill help texts and manual sections for each of xfer backend
by GitHub pull_request - opened 02 Jan '19

02 Jan '19

alsa-project/alsa-utils pull request #8 was opened from takaswie: Hi, This patchset fulfills help texts and manual sections for each of xfer backend. All of options are described in this patchset, however I leave a placeholder for scheduling model of libasound. This section will be fulfilled later. (I note that this is a first attempt to explain about `slave` and `snoop` options of libffado.) ``` Takashi Sakamoto (8): axfer: fix no return statement in print_help() function axfer: fix typo in axfer(1) manual axfer: correct description about signal handling axfer: print help for libffado backend axfer: fulfill manual section for libffado backend axfer: print help for libasound backend axfer: fulfill manual section for libasound backend axfer: add text for compatibility loss of sw parameter in libasound backend axfer/axfer-transfer.1 | 383 ++++++++++++++++++++++++++++++++++++++++- axfer/axfer.1 | 2 +- axfer/xfer-libasound.c | 25 ++- axfer/xfer-libffado.c | 11 +- axfer/xfer-options.c | 2 +- 5 files changed, 414 insertions(+), 9 deletions(-) ``` Request URL : https://github.com/alsa-project/alsa-utils/pull/8 Patch URL : https://github.com/alsa-project/alsa-utils/pull/8.patch Repository URL: https://github.com/alsa-project/alsa-utils

1 0

[alsa-devel] [PATCH] conf/ucm: bytcr-rt5651: Document mono speaker wiring
by Hans de Goede 01 Jan '19

01 Jan '19

During my recent work on the bytcht-es8316 UCM profile I realized that the bytcr-rt5651 devices with a single speaker use a differential setup just like the es8316 does. The tell-tale here is the speaker going quiet when playing the exact same sound on both channels when things are configured for stereo speakers. I've run some tests and the rt5651 does have a special mono balanced out mode for its line-out but using this results in the same sound (and the same loudness / volume of the sound) as our current solution, so adding support for this to the kernel buys us nothing. This commit makes no changes, it just documents my findings in a big comment for future reference. Signed-off-by: Hans de Goede <hdegoede(a)redhat.com> --- src/conf/ucm/codecs/rt5651/MonoSpeaker.conf | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/conf/ucm/codecs/rt5651/MonoSpeaker.conf b/src/conf/ucm/codecs/rt5651/MonoSpeaker.conf index 0c3492fb..f4181d10 100644 --- a/src/conf/ucm/codecs/rt5651/MonoSpeaker.conf +++ b/src/conf/ucm/codecs/rt5651/MonoSpeaker.conf @@ -9,7 +9,21 @@ SectionDevice."MonoSpeaker" { EnableSequence [ cdev "hw:bytcrrt5651" - # Map left and right input on left + # On most (all?) devices with a single / mono speaker an + # amplifier with a differential input connected between LOUT + # right and left is used. We deal with this by silencing the + # right output and mixing left + right to the left output. + # + # An alternative approach for devices using the differential + # setup would be to set bit 15 of register 5 in the codec, + # which turns the stereo unbalanced line out into a mono + # balanced line out. + # + # This possibility was discovered after we already shipped the + # current solution and the current solution has the advantage + # of also working on devices which only use LOUT left in + # unbalanced mode. The net result is the same for devices using + # the differential setup, so we just stick with what we have. cset "name='Stereo DAC MIXR DAC R1 Switch' off" cset "name='Stereo DAC MIXL DAC R1 Switch' on" -- 2.20.1

2 1

[alsa-devel] [PATCH 1/4] conf/ucm: bytcr-rt5651: Enable Stereo? ADC MIXL ADC? switches when enabling inputs
by Hans de Goede 01 Jan '19

01 Jan '19

Explicitly enable the right 'Stereo? ADC MIXL ADC?' when selecting an input, rather then just enabling them all in ucm/codecs/rt5651/EnableSeq.conf . This is a preparation patch for adding digital mic support. Signed-off-by: Hans de Goede <hdegoede(a)redhat.com> --- src/conf/ucm/codecs/rt5651/EnableSeq.conf | 16 ++++++++-------- src/conf/ucm/codecs/rt5651/IN1-InternalMic.conf | 4 ++++ src/conf/ucm/codecs/rt5651/IN12-InternalMic.conf | 4 ++++ src/conf/ucm/codecs/rt5651/IN2-InternalMic.conf | 4 ++++ src/conf/ucm/codecs/rt5651/IN3-HeadsetMic.conf | 4 ++++ 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/conf/ucm/codecs/rt5651/EnableSeq.conf b/src/conf/ucm/codecs/rt5651/EnableSeq.conf index 6fb3c1f2..db2fd328 100644 --- a/src/conf/ucm/codecs/rt5651/EnableSeq.conf +++ b/src/conf/ucm/codecs/rt5651/EnableSeq.conf @@ -14,12 +14,14 @@ cset "name='Stereo DAC MIXL DAC L1 Switch' on" cset "name='Stereo DAC MIXL DAC R1 Switch' off" cset "name='Stereo DAC MIXR DAC L1 Switch' off" cset "name='Stereo DAC MIXR DAC R1 Switch' on" -cset "name='Stereo1 ADC MIXR ADC1 Switch' on" -cset "name='Stereo1 ADC MIXR ADC2 Switch' on" -cset "name='Stereo2 ADC MIXL ADC1 Switch' on" -cset "name='Stereo2 ADC MIXL ADC2 Switch' on" -cset "name='Stereo2 ADC MIXR ADC1 Switch' on" -cset "name='Stereo2 ADC MIXR ADC2 Switch' on" +cset "name='Stereo1 ADC MIXL ADC1 Switch' off" +cset "name='Stereo1 ADC MIXR ADC1 Switch' off" +cset "name='Stereo1 ADC MIXL ADC2 Switch' off" +cset "name='Stereo1 ADC MIXR ADC2 Switch' off" +cset "name='Stereo2 ADC MIXL ADC1 Switch' off" +cset "name='Stereo2 ADC MIXR ADC1 Switch' off" +cset "name='Stereo2 ADC MIXL ADC2 Switch' off" +cset "name='Stereo2 ADC MIXR ADC2 Switch' off" cset "name='Headphone Switch' off" cset "name='Speaker Switch' off" @@ -41,8 +43,6 @@ cset "name='IN3 Boost' 1" # input cset "name='Stereo1 ADC L1 Mux' ADC" cset "name='Stereo1 ADC R1 Mux' ADC" -cset "name='Stereo1 ADC MIXL ADC1 Switch' on" -cset "name='Stereo1 ADC MIXR ADC1 Switch' on" cset "name='RECMIXL BST1 Switch' off" cset "name='RECMIXR BST1 Switch' off" cset "name='RECMIXL BST3 Switch' off" diff --git a/src/conf/ucm/codecs/rt5651/IN1-InternalMic.conf b/src/conf/ucm/codecs/rt5651/IN1-InternalMic.conf index 85e25a5c..b2ce17b6 100644 --- a/src/conf/ucm/codecs/rt5651/IN1-InternalMic.conf +++ b/src/conf/ucm/codecs/rt5651/IN1-InternalMic.conf @@ -12,10 +12,14 @@ SectionDevice."InternalMic-IN1" { cset "name='Internal Mic Switch' on" cset "name='RECMIXL BST1 Switch' on" cset "name='RECMIXR BST1 Switch' on" + cset "name='Stereo1 ADC MIXL ADC1 Switch' on" + cset "name='Stereo1 ADC MIXR ADC1 Switch' on" ] DisableSequence [ cdev "hw:bytcrrt5651" + cset "name='Stereo1 ADC MIXL ADC1 Switch' off" + cset "name='Stereo1 ADC MIXR ADC1 Switch' off" cset "name='Internal Mic Switch' off" cset "name='RECMIXL BST1 Switch' off" cset "name='RECMIXR BST1 Switch' off" diff --git a/src/conf/ucm/codecs/rt5651/IN12-InternalMic.conf b/src/conf/ucm/codecs/rt5651/IN12-InternalMic.conf index f1cc17fe..e8041912 100644 --- a/src/conf/ucm/codecs/rt5651/IN12-InternalMic.conf +++ b/src/conf/ucm/codecs/rt5651/IN12-InternalMic.conf @@ -14,10 +14,14 @@ SectionDevice."InternalMic-IN12" { cset "name='RECMIXR BST1 Switch' on" cset "name='RECMIXL BST2 Switch' on" cset "name='RECMIXR BST2 Switch' on" + cset "name='Stereo1 ADC MIXL ADC1 Switch' on" + cset "name='Stereo1 ADC MIXR ADC1 Switch' on" ] DisableSequence [ cdev "hw:bytcrrt5651" + cset "name='Stereo1 ADC MIXL ADC1 Switch' off" + cset "name='Stereo1 ADC MIXR ADC1 Switch' off" cset "name='Internal Mic Switch' off" cset "name='RECMIXL BST1 Switch' off" cset "name='RECMIXR BST1 Switch' off" diff --git a/src/conf/ucm/codecs/rt5651/IN2-InternalMic.conf b/src/conf/ucm/codecs/rt5651/IN2-InternalMic.conf index 18f74e1a..048fcc95 100644 --- a/src/conf/ucm/codecs/rt5651/IN2-InternalMic.conf +++ b/src/conf/ucm/codecs/rt5651/IN2-InternalMic.conf @@ -12,10 +12,14 @@ SectionDevice."InternalMic-IN2" { cset "name='Internal Mic Switch' on" cset "name='RECMIXL BST2 Switch' on" cset "name='RECMIXR BST2 Switch' on" + cset "name='Stereo1 ADC MIXL ADC1 Switch' on" + cset "name='Stereo1 ADC MIXR ADC1 Switch' on" ] DisableSequence [ cdev "hw:bytcrrt5651" + cset "name='Stereo1 ADC MIXL ADC1 Switch' off" + cset "name='Stereo1 ADC MIXR ADC1 Switch' off" cset "name='Internal Mic Switch' off" cset "name='RECMIXL BST2 Switch' off" cset "name='RECMIXR BST2 Switch' off" diff --git a/src/conf/ucm/codecs/rt5651/IN3-HeadsetMic.conf b/src/conf/ucm/codecs/rt5651/IN3-HeadsetMic.conf index bfd0b009..712d14bb 100644 --- a/src/conf/ucm/codecs/rt5651/IN3-HeadsetMic.conf +++ b/src/conf/ucm/codecs/rt5651/IN3-HeadsetMic.conf @@ -12,10 +12,14 @@ SectionDevice."HeadsetMic-IN3" { cset "name='Headset Mic Switch' on" cset "name='RECMIXL BST3 Switch' on" cset "name='RECMIXR BST3 Switch' on" + cset "name='Stereo1 ADC MIXL ADC1 Switch' on" + cset "name='Stereo1 ADC MIXR ADC1 Switch' on" ] DisableSequence [ cdev "hw:bytcrrt5651" + cset "name='Stereo1 ADC MIXL ADC1 Switch' off" + cset "name='Stereo1 ADC MIXR ADC1 Switch' off" cset "name='Headset Mic Switch' off" cset "name='RECMIXL BST3 Switch' off" cset "name='RECMIXR BST3 Switch' off" -- 2.20.1

2 6

[alsa-devel] [PATCH v2] staging: bcm2835-audio: interpolate audio delay
by Mike Brady 01 Jan '19

01 Jan '19

When the BCM2835 audio output is used, userspace sees a jitter up to 10ms in the audio position, aka "delay" -- the number of frames that must be output before a new frame would be played. Make this a bit nicer for userspace by interpolating the position using the CPU clock. The overhead is small -- an extra ktime_get() every time a GPU message is sent -- and another call and a few calculations whenever the delay is sought from userland. At 48,000 frames per second, i.e. approximately 20 microseconds per frame, it would take a clock inaccuracy of 20 microseconds in 10 milliseconds -- 2,000 parts per million -- to result in an inaccurate estimate, whereas crystal- or resonator-based clocks typically have an inaccuracy of 10s to 100s of parts per million. Signed-off-by: Mike Brady <mikebrady(a)eircom.net> --- Changes in v2 -- remove inappropriate addition of SNDRV_PCM_INFO_BATCH flag .../vc04_services/bcm2835-audio/bcm2835-pcm.c | 20 +++++++++++++++++++ .../vc04_services/bcm2835-audio/bcm2835.h | 1 + 2 files changed, 21 insertions(+) diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c index e66da11af5cf..9053b996cada 100644 --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c @@ -74,6 +74,7 @@ void bcm2835_playback_fifo(struct bcm2835_alsa_stream *alsa_stream, atomic_set(&alsa_stream->pos, pos); alsa_stream->period_offset += bytes; + alsa_stream->interpolate_start = ktime_get(); if (alsa_stream->period_offset >= alsa_stream->period_size) { alsa_stream->period_offset %= alsa_stream->period_size; snd_pcm_period_elapsed(substream); @@ -243,6 +244,7 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream *substream) atomic_set(&alsa_stream->pos, 0); alsa_stream->period_offset = 0; alsa_stream->draining = false; + alsa_stream->interpolate_start = ktime_get(); return 0; } @@ -292,6 +294,24 @@ snd_bcm2835_pcm_pointer(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; struct bcm2835_alsa_stream *alsa_stream = runtime->private_data; + ktime_t now = ktime_get(); + + /* Give userspace better delay reporting by interpolating between GPU + * notifications, assuming audio speed is close enough to the clock + * used for ktime + */ + + if ((ktime_to_ns(alsa_stream->interpolate_start)) && + (ktime_compare(alsa_stream->interpolate_start, now) < 0)) { + u64 interval = + (ktime_to_ns(ktime_sub(now, + alsa_stream->interpolate_start))); + u64 frames_output_in_interval = + div_u64((interval * runtime->rate), 1000000000); + snd_pcm_sframes_t frames_output_in_interval_sized = + -frames_output_in_interval; + runtime->delay = frames_output_in_interval_sized; + } return snd_pcm_indirect_playback_pointer(substream, &alsa_stream->pcm_indirect, diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h index e13435d1c205..595ad584243f 100644 --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h @@ -78,6 +78,7 @@ struct bcm2835_alsa_stream { unsigned int period_offset; unsigned int buffer_size; unsigned int period_size; + ktime_t interpolate_start; struct bcm2835_audio_instance *instance; int idx; -- 2.17.1

4 17

[alsa-devel] [PATCH] aplay: support no period wakeup option in argument
by S.j. Wang 01 Jan '19

01 Jan '19

In the case that alsa driver can't support period wakeup, we need to set the no period wakeup flag Signed-off-by: Shengjiu Wang <shengjiu.wang(a)nxp.com> --- aplay/aplay.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/aplay/aplay.c b/aplay/aplay.c index efc1eb4cae3a..4f562bfe2884 100644 --- a/aplay/aplay.c +++ b/aplay/aplay.c @@ -137,6 +137,7 @@ static int use_strftime = 0; volatile static int recycle_capture_file = 0; static long term_c_lflag = -1; static int dump_hw_params = 0; +static int no_period_wakeup = 0; static int fd = -1; static off64_t pbrec_count = LLONG_MAX, fdcount; @@ -243,6 +244,7 @@ _("Usage: %s [OPTION]... [FILE]...\n" " --use-strftime apply the strftime facility to the output file name\n" " --dump-hw-params dump hw_params of the device\n" " --fatal-errors treat all errors as fatal\n" +" --no-period-wakeup set no period wakeup flag if necessary\n" ) , command); printf(_("Recognized sample formats are:")); @@ -429,6 +431,7 @@ enum { OPT_USE_STRFTIME, OPT_DUMP_HWPARAMS, OPT_FATAL_ERRORS, + OPT_NO_PERIOD_WAKEUP, }; /* @@ -516,6 +519,7 @@ int main(int argc, char *argv[]) {"interactive", 0, 0, 'i'}, {"dump-hw-params", 0, 0, OPT_DUMP_HWPARAMS}, {"fatal-errors", 0, 0, OPT_FATAL_ERRORS}, + {"no-period-wakeup", 0, 0, OPT_NO_PERIOD_WAKEUP}, #ifdef CONFIG_SUPPORT_CHMAP {"chmap", 1, 0, 'm'}, #endif @@ -799,6 +803,9 @@ int main(int argc, char *argv[]) case OPT_FATAL_ERRORS: fatal_errors = 1; break; + case OPT_NO_PERIOD_WAKEUP: + no_period_wakeup = 1; + break; #ifdef CONFIG_SUPPORT_CHMAP case 'm': channel_map = snd_pcm_chmap_parse_string(optarg); @@ -1396,6 +1403,12 @@ static void set_params(void) &buffer_frames); } assert(err >= 0); + + if (no_period_wakeup) { + err = snd_pcm_hw_params_set_period_wakeup(handle, params, 0); + assert(err >= 0); + } + monotonic = snd_pcm_hw_params_is_monotonic(params); can_pause = snd_pcm_hw_params_can_pause(params); err = snd_pcm_hw_params(handle, params); -- 1.9.1

3 6

Re: [alsa-devel] [PATCH] pci: fix a missing check of snd_i2c_sendbytes
by Takashi Iwai 01 Jan '19

01 Jan '19

On Wed, 26 Dec 2018 06:12:04 +0100, Kangjie Lu wrote: > > snd_i2c_sendbytes could fail. The fix checks its return value: if it > fails, issues an error message and returns with its error code. > > Signed-off-by: Kangjie Lu <kjlu(a)umn.edu> > --- > sound/pci/ice1712/ews.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/sound/pci/ice1712/ews.c b/sound/pci/ice1712/ews.c > index b8af747ecb43..b978cdb0fc20 100644 > --- a/sound/pci/ice1712/ews.c > +++ b/sound/pci/ice1712/ews.c > @@ -826,7 +826,10 @@ static int snd_ice1712_6fire_read_pca(struct snd_ice1712 *ice, unsigned char reg > > snd_i2c_lock(ice->i2c); > byte = reg; > - snd_i2c_sendbytes(spec->i2cdevs[EWS_I2C_6FIRE], &byte, 1); > + if (snd_i2c_sendbytes(spec->i2cdevs[EWS_I2C_6FIRE], &byte, 1)) { > + dev_err(ice->card->dev, "cannot send pca\n"); > + return -EIO; You forgot to unlock before returning. thanks, Takashi

1 0

Re: [alsa-devel] [PATCH] line6: add a check for snd_card_register
by Takashi Iwai 01 Jan '19

01 Jan '19

On Wed, 26 Dec 2018 04:39:31 +0100, Kangjie Lu wrote: > > The fix checks if snd_card_register() fails, and if so, issues a > warning. > > Signed-off-by: Kangjie Lu <kjlu(a)umn.edu> Hmm, you submitted multiple patches doing the similar things but the error messages are handled completely differently in both cases. Please make them consistent. ... and above all: > - snd_card_register(line6->card); > + if (snd_card_register(line6->card)) > + pr_warn("Failed to register ALSA card.\n"); This error message is almost useless. Better to use dev_*() variant. thanks, Takashi

1 0

Re: [alsa-devel] [PATCH] sound: sb: fix a missing check of snd_ctl_add
by Takashi Iwai 01 Jan '19

01 Jan '19

On Wed, 26 Dec 2018 02:45:32 +0100, Kangjie Lu wrote: > > snd_ctl_add() could fail, so let's check its return value and return its > error code upstream upon failure. > > Signed-off-by: Kangjie Lu <kjlu(a)umn.edu> > --- > sound/isa/sb/sb16_main.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/sound/isa/sb/sb16_main.c b/sound/isa/sb/sb16_main.c > index 37e6ce7b0b13..c9b25dc1f78e 100644 > --- a/sound/isa/sb/sb16_main.c > +++ b/sound/isa/sb/sb16_main.c > @@ -879,8 +879,14 @@ int snd_sb16dsp_pcm(struct snd_sb *chip, int device) > snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &snd_sb16_playback_ops); > snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &snd_sb16_capture_ops); > > - if (chip->dma16 >= 0 && chip->dma8 != chip->dma16) > - snd_ctl_add(card, snd_ctl_new1(&snd_sb16_dma_control, chip)); > + if (chip->dma16 >= 0 && chip->dma8 != chip->dma16) { > + err = > + snd_ctl_add(card, > + snd_ctl_new1(&snd_sb16_dma_control, > + chip)); > + if (err < 0) > + return err; > + } > else > pcm->info_flags = SNDRV_PCM_INFO_HALF_DUPLEX; The code change looks OK, but could you fix the indentation and avoid unneeded line breaks, as well as the missing braces for else-block? Also, we keep "ALSA:" prefix for the subject line (except for ASoC ones). Please align with it. thanks, Takashi

1 0

Re: [alsa-devel] [PATCH] gus: add a check of the status of snd_ctl_add
by Takashi Iwai 01 Jan '19

01 Jan '19

On Wed, 26 Dec 2018 02:40:51 +0100, Kangjie Lu wrote: > > snd_ctl_add() could fail, so let's check its status and issue an error > message if it indeed fails. > > Signed-off-by: Kangjie Lu <kjlu(a)umn.edu> The function itself prints the error in most of useful cases, so the extra error here would be superfluous. thanks, Takashi

1 0

Re: [alsa-devel] [PATCH] sound:emu10k1:Use kmemdup instead of duplicating its function
by Takashi Iwai 01 Jan '19

01 Jan '19

On Mon, 24 Dec 2018 09:42:48 +0100, huang.zijiang wrote: > > From: "huang.zijiang" <huang.zijiang(a)zte.com.cn> > > kmemdup has implemented the function that kmalloc() and memcpy(). > > Signed-off-by: huang.zijiang <huang.zijiang(a)zte.com.cn> > --- > sound/pci/emu10k1/emufx.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c > index 6ebe817..b4fe4c5 100644 > --- a/sound/pci/emu10k1/emufx.c > +++ b/sound/pci/emu10k1/emufx.c > @@ -671,10 +671,9 @@ static unsigned int *copy_tlv(const unsigned int __user *_tlv, bool in_kernel) > return NULL; > if (data[1] >= MAX_TLV_SIZE) > return NULL; > - tlv = kmalloc(data[1] + sizeof(data), GFP_KERNEL); > + tlv = kmemdup(data, data[1] + sizeof(data), GFP_KERNEL); > if (!tlv) > return NULL; > - memcpy(tlv, data, sizeof(data)); These changes are not equivalent, and rather dangerous, unfortunately. The memcpy() performs only for sizeof(data), and in this case, it's not the same size as the allocation above. thanks, Takashi

1 0