[alsa-devel] [PATCH] aplay: support no period wakeup option in argument

[S.j. Wang]

Hi

> -----Original Message-----
> From: Takashi Sakamoto <o-takashi at sakamocchi.jp>
> Sent: Wednesday, December 26, 2018 9:35 PM
> 
> Hi,
> 
> On Wed, Dec 26, 2018 at 11:28:11AM +0000, S.j. Wang wrote:
> > 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 at 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
> 
> As of v4.21-rc1, runtime of PCM substream can't run no_period_wakeup
> mode unless two conditions are satisfied[1]:
>  - driver supports SNDRV_PCM_INFO_NO_PERIOD_WAKEUP
>  - PCM applications set SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP
> flag to
>    hardware parameter structure.
> 
> Neither alsa-lib nor the most of existent userspace applications set
> SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP without explicit call of
> helper API. In this point, I can get your intention for this patch. If a driver
> just supports no_period_wakeup mode, such driver can't work well.
> 
> However, such driver is problematic because apparently it can not run with
> many existent userspace applications and alsa-lib. Before applying this
> patch, we have enough discussion to prevent problems to introduce such
> problematic drivers into Linux sound subsystem, in my opinion.

What is helper API?  I found that you have a similar function in axfer/xfer-libasound.c,
Which is disable_period_wakeup().  Do you think I need to porting this function to
Aplay? 

Best regards
Wang shengjiu
> 
> [1]
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftiwai%2Fsound.git%2
> Ftree%2Fsound%2Fcore%2Fpcm_native.c%3Fh%3Dsound-4.21-
> rc1%23n727&data=02%7C01%7Cshengjiu.wang%40nxp.com%7Cdaa8cb
> 93830245f7b27108d66b36fe91%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
> 0%7C0%7C636814281289991663&sdata=Qv2Jfrd2qXc91gpwJhu28bhMC
> SJ5%2FgZJWDzP0gDT0Ws%3D&reserved=0
> 
> 
> Regards
> 
> Takashi Sakamoto