* [PATCH 1/2] pcm: pass hw_params flags to slave @ 2010-11-01 22:13 Pierre-Louis Bossart 2010-11-01 22:13 ` [PATCH 2/2] add API to allow disabling period interrupt Pierre-Louis Bossart 2010-11-02 7:27 ` [PATCH 1/2] pcm: pass hw_params flags to slave Takashi Iwai 0 siblings, 2 replies; 14+ messages in thread From: Pierre-Louis Bossart @ 2010-11-01 22:13 UTC (permalink / raw) To: alsa-devel; +Cc: Pierre-Louis Bossart fix required before interrupt disabling routines patch can be applied. Without this fix, the interrupts are only disabled when accessing directly hw devices. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@intel.com> --- src/pcm/pcm_params.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/pcm/pcm_params.c b/src/pcm/pcm_params.c index 3a90bcd..416b081 100644 --- a/src/pcm/pcm_params.c +++ b/src/pcm/pcm_params.c @@ -2126,6 +2126,7 @@ int _snd_pcm_hw_params_refine(snd_pcm_hw_params_t *params, err = changed; } params->info &= src->info; + params->flags = src->flags; /* propagate flags to slave */ return err; } -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] add API to allow disabling period interrupt 2010-11-01 22:13 [PATCH 1/2] pcm: pass hw_params flags to slave Pierre-Louis Bossart @ 2010-11-01 22:13 ` Pierre-Louis Bossart 2010-11-02 7:28 ` Takashi Iwai 2010-11-02 8:23 ` Clemens Ladisch 2010-11-02 7:27 ` [PATCH 1/2] pcm: pass hw_params flags to slave Takashi Iwai 1 sibling, 2 replies; 14+ messages in thread From: Pierre-Louis Bossart @ 2010-11-01 22:13 UTC (permalink / raw) To: alsa-devel; +Cc: Pierre-Louis Bossart cleaned-up version of the patch posted on May 17, 2010 by Clemens Ladisch <clemens@ladisch.de> (No filtering in pcm_multi and pcm_direct info fields) Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@intel.com> --- configure.in | 2 +- include/pcm.h | 3 ++ include/sound/asound.h | 5 +++- src/Versions.in | 7 +++++ src/pcm/pcm.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ src/pcm/pcm_local.h | 3 ++ 6 files changed, 79 insertions(+), 2 deletions(-) diff --git a/configure.in b/configure.in index c353759..0a67de7 100644 --- a/configure.in +++ b/configure.in @@ -12,7 +12,7 @@ dnl add API = c+1:0:a+1 dnl remove API = c+1:0:0 dnl ************************************************* AC_CANONICAL_HOST -AM_INIT_AUTOMAKE(alsa-lib, 1.0.23) +AM_INIT_AUTOMAKE(alsa-lib, 1.0.24) eval LIBTOOL_VERSION_INFO="2:0:0" dnl ************************************************* AM_CONDITIONAL(INSTALL_M4, test -n "${ACLOCAL}") diff --git a/include/pcm.h b/include/pcm.h index f3618c3..cd506ab 100644 --- a/include/pcm.h +++ b/include/pcm.h @@ -531,6 +531,7 @@ int snd_pcm_hw_params_can_resume(const snd_pcm_hw_params_t *params); int snd_pcm_hw_params_is_half_duplex(const snd_pcm_hw_params_t *params); int snd_pcm_hw_params_is_joint_duplex(const snd_pcm_hw_params_t *params); int snd_pcm_hw_params_can_sync_start(const snd_pcm_hw_params_t *params); +int snd_pcm_hw_params_can_disable_period_irq(const snd_pcm_hw_params_t *params); int snd_pcm_hw_params_get_rate_numden(const snd_pcm_hw_params_t *params, unsigned int *rate_num, unsigned int *rate_den); @@ -626,6 +627,8 @@ int snd_pcm_hw_params_set_rate_resample(snd_pcm_t *pcm, snd_pcm_hw_params_t *par int snd_pcm_hw_params_get_rate_resample(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val); int snd_pcm_hw_params_set_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val); int snd_pcm_hw_params_get_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val); +int snd_pcm_hw_params_set_period_irq(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val); +int snd_pcm_hw_params_get_period_irq(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val); int snd_pcm_hw_params_get_period_time(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir); int snd_pcm_hw_params_get_period_time_min(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir); diff --git a/include/sound/asound.h b/include/sound/asound.h index fa88938..fbcfef9 100644 --- a/include/sound/asound.h +++ b/include/sound/asound.h @@ -278,6 +278,8 @@ enum sndrv_pcm_subformat { #define SNDRV_PCM_INFO_HALF_DUPLEX 0x00100000 /* only half duplex */ #define SNDRV_PCM_INFO_JOINT_DUPLEX 0x00200000 /* playback and capture stream are somewhat correlated */ #define SNDRV_PCM_INFO_SYNC_START 0x00400000 /* pcm support some kind of sync go */ +#define SNDRV_PCM_INFO_NO_PERIOD_IRQ 0x00800000 /* period interrupt can be disabled */ +#define SNDRV_PCM_INFO_FIFO_IN_FRAMES 0x80000000 /* internal kernel flag - FIFO size is in frames */ enum sndrv_pcm_state { SNDRV_PCM_STATE_OPEN = 0, /* stream is open */ @@ -346,7 +348,8 @@ enum sndrv_pcm_hw_param { #define SNDRV_PCM_HW_PARAMS_NORESAMPLE (1<<0) /* avoid rate resampling */ #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER (1<<1) /* export buffer */ - +#define SNDRV_PCM_HW_PARAMS_NO_PERIOD_IRQ (1<<2) /* disable period + interrupts */ struct sndrv_interval { unsigned int min, max; unsigned int openmin:1, diff --git a/src/Versions.in b/src/Versions.in index 8d2dd11..112b9ec 100644 --- a/src/Versions.in +++ b/src/Versions.in @@ -129,3 +129,10 @@ ALSA_0.9.7 { @SYMBOL_PREFIX@alsa_lisp_*; } ALSA_0.9.5; +ALSA_1.0.24 { + global: + @SYMBOL_PREFIX@snd_pcm_hw_params_can_disable_period_irq; + @SYMBOL_PREFIX@snd_pcm_hw_params_set_period_irq; + @SYMBOL_PREFIX@snd_pcm_hw_params_get_period_irq; + +} ALSA_0.9.7; \ No newline at end of file diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c index a49b5b9..6eb7223 100644 --- a/src/pcm/pcm.c +++ b/src/pcm/pcm.c @@ -3081,6 +3081,27 @@ int snd_pcm_hw_params_can_sync_start(const snd_pcm_hw_params_t *params) } /** + * \brief Check if hardware can disable period interrupts + * \param params Configuration space + * \return Boolean value + * \retval 0 Hardware cannot disable period interrupts + * \retval 1 Hardware can disable period interrupts + * + * It is not allowed to call this function when given configuration is not exactly one. + * Usually, #snd_pcm_hw_params() function chooses one configuration + * from the configuration space. + */ +int snd_pcm_hw_params_can_disable_period_irq(const snd_pcm_hw_params_t *params) +{ + assert(params); + if (CHECK_SANITY(params->info == ~0U)) { + SNDMSG("invalid PCM info field"); + return 0; /* FIXME: should be a negative error? */ + } + return !!(params->info & SNDRV_PCM_INFO_NO_PERIOD_IRQ); +} + +/** * \brief Get rate exact info from a configuration space * \param params Configuration space * \param rate_num Pointer to returned rate numerator @@ -4200,6 +4221,46 @@ int snd_pcm_hw_params_get_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *par } /** + * \brief Restrict a configuration space to settings without period interrupts + * \param pcm PCM handle + * \param params Configuration space + * \param val 0 = disable, 1 = enable (default) period interrupts + * \return Zero on success, otherwise a negative error code. + * + * This function should be called only on devices where + * #snd_pcm_hw_params_can_disable_period_irq() returns true. (too late, FIXME) + * + * Even with disabled period interrupts, the period size/time/count parameters + * are valid; it is suggested to use #snd_pcm_hw_params_set_period_size_last(). + * + * When period interrupts are disabled, the application must not use poll() or + * any functions that could block on this device. + */ +int snd_pcm_hw_params_set_period_irq(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val) +{ + assert(pcm && params); + if (!val) + params->flags |= SND_PCM_HW_PARAMS_NO_PERIOD_IRQ; + else + params->flags &= ~SND_PCM_HW_PARAMS_NO_PERIOD_IRQ; + return snd_pcm_hw_refine(pcm, params); +} + +/** + * \brief Extract period interrupt mask from a configuration space + * \param pcm PCM handle + * \param params Configuration space + * \param val overwritten with 0 = disabled, 1 = enabled period interrupts + * \return Zero + */ +int snd_pcm_hw_params_get_period_irq(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val) +{ + assert(pcm && params && val); + *val = params->flags & SND_PCM_HW_PARAMS_NO_PERIOD_IRQ ? 0 : 1; + return 0; +} + +/** * \brief Extract period time from a configuration space * \param params Configuration space * \param val Returned approximate period duration in us diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h index 2f6fcd2..768f526 100644 --- a/src/pcm/pcm_local.h +++ b/src/pcm/pcm_local.h @@ -91,9 +91,12 @@ typedef enum sndrv_pcm_hw_param snd_pcm_hw_param_t; #define SND_PCM_INFO_JOINT_DUPLEX SNDRV_PCM_INFO_JOINT_DUPLEX /** device can do a kind of synchronized start */ #define SND_PCM_INFO_SYNC_START SNDRV_PCM_INFO_SYNC_START +/** device can disable period interrupts */ +#define SND_PCM_INFO_NO_PERIOD_IRQ SNDRV_PCM_INFO_NO_PERIOD_IRQ #define SND_PCM_HW_PARAMS_NORESAMPLE SNDRV_PCM_HW_PARAMS_NORESAMPLE #define SND_PCM_HW_PARAMS_EXPORT_BUFFER SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER +#define SND_PCM_HW_PARAMS_NO_PERIOD_IRQ SNDRV_PCM_HW_PARAMS_NO_PERIOD_IRQ #define SND_PCM_INFO_MONOTONIC 0x80000000 -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] add API to allow disabling period interrupt 2010-11-01 22:13 ` [PATCH 2/2] add API to allow disabling period interrupt Pierre-Louis Bossart @ 2010-11-02 7:28 ` Takashi Iwai 2010-11-02 8:23 ` Clemens Ladisch 1 sibling, 0 replies; 14+ messages in thread From: Takashi Iwai @ 2010-11-02 7:28 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: alsa-devel, Pierre-Louis Bossart At Mon, 1 Nov 2010 17:13:34 -0500, Pierre-Louis Bossart wrote: > > cleaned-up version of the patch posted on May 17, 2010 by > Clemens Ladisch <clemens@ladisch.de> > (No filtering in pcm_multi and pcm_direct info fields) > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@intel.com> > --- > configure.in | 2 +- > include/pcm.h | 3 ++ > include/sound/asound.h | 5 +++- > src/Versions.in | 7 +++++ > src/pcm/pcm.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ > src/pcm/pcm_local.h | 3 ++ > 6 files changed, 79 insertions(+), 2 deletions(-) > > diff --git a/configure.in b/configure.in > index c353759..0a67de7 100644 > --- a/configure.in > +++ b/configure.in > @@ -12,7 +12,7 @@ dnl add API = c+1:0:a+1 > dnl remove API = c+1:0:0 > dnl ************************************************* > AC_CANONICAL_HOST > -AM_INIT_AUTOMAKE(alsa-lib, 1.0.23) > +AM_INIT_AUTOMAKE(alsa-lib, 1.0.24) > eval LIBTOOL_VERSION_INFO="2:0:0" > dnl ************************************************* > AM_CONDITIONAL(INSTALL_M4, test -n "${ACLOCAL}") > diff --git a/include/pcm.h b/include/pcm.h > index f3618c3..cd506ab 100644 > --- a/include/pcm.h > +++ b/include/pcm.h > @@ -531,6 +531,7 @@ int snd_pcm_hw_params_can_resume(const snd_pcm_hw_params_t *params); > int snd_pcm_hw_params_is_half_duplex(const snd_pcm_hw_params_t *params); > int snd_pcm_hw_params_is_joint_duplex(const snd_pcm_hw_params_t *params); > int snd_pcm_hw_params_can_sync_start(const snd_pcm_hw_params_t *params); > +int snd_pcm_hw_params_can_disable_period_irq(const snd_pcm_hw_params_t *params); > int snd_pcm_hw_params_get_rate_numden(const snd_pcm_hw_params_t *params, > unsigned int *rate_num, > unsigned int *rate_den); > @@ -626,6 +627,8 @@ int snd_pcm_hw_params_set_rate_resample(snd_pcm_t *pcm, snd_pcm_hw_params_t *par > int snd_pcm_hw_params_get_rate_resample(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val); > int snd_pcm_hw_params_set_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val); > int snd_pcm_hw_params_get_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val); > +int snd_pcm_hw_params_set_period_irq(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val); > +int snd_pcm_hw_params_get_period_irq(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val); > > int snd_pcm_hw_params_get_period_time(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir); > int snd_pcm_hw_params_get_period_time_min(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir); > diff --git a/include/sound/asound.h b/include/sound/asound.h > index fa88938..fbcfef9 100644 > --- a/include/sound/asound.h > +++ b/include/sound/asound.h > @@ -278,6 +278,8 @@ enum sndrv_pcm_subformat { > #define SNDRV_PCM_INFO_HALF_DUPLEX 0x00100000 /* only half duplex */ > #define SNDRV_PCM_INFO_JOINT_DUPLEX 0x00200000 /* playback and capture stream are somewhat correlated */ > #define SNDRV_PCM_INFO_SYNC_START 0x00400000 /* pcm support some kind of sync go */ > +#define SNDRV_PCM_INFO_NO_PERIOD_IRQ 0x00800000 /* period interrupt can be disabled */ > +#define SNDRV_PCM_INFO_FIFO_IN_FRAMES 0x80000000 /* internal kernel flag - FIFO size is in frames */ > > enum sndrv_pcm_state { > SNDRV_PCM_STATE_OPEN = 0, /* stream is open */ > @@ -346,7 +348,8 @@ enum sndrv_pcm_hw_param { > > #define SNDRV_PCM_HW_PARAMS_NORESAMPLE (1<<0) /* avoid rate resampling */ > #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER (1<<1) /* export buffer */ > - > +#define SNDRV_PCM_HW_PARAMS_NO_PERIOD_IRQ (1<<2) /* disable period > + interrupts */ > struct sndrv_interval { > unsigned int min, max; > unsigned int openmin:1, > diff --git a/src/Versions.in b/src/Versions.in > index 8d2dd11..112b9ec 100644 > --- a/src/Versions.in > +++ b/src/Versions.in > @@ -129,3 +129,10 @@ ALSA_0.9.7 { > @SYMBOL_PREFIX@alsa_lisp_*; > } ALSA_0.9.5; > > +ALSA_1.0.24 { > + global: > + @SYMBOL_PREFIX@snd_pcm_hw_params_can_disable_period_irq; > + @SYMBOL_PREFIX@snd_pcm_hw_params_set_period_irq; > + @SYMBOL_PREFIX@snd_pcm_hw_params_get_period_irq; > + > +} ALSA_0.9.7; > \ No newline at end of file In general we don't put this any more. All snd_* are exported unless blacklisted. So, avoid changing the version number in your patch. thanks, Takashi ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] add API to allow disabling period interrupt 2010-11-01 22:13 ` [PATCH 2/2] add API to allow disabling period interrupt Pierre-Louis Bossart 2010-11-02 7:28 ` Takashi Iwai @ 2010-11-02 8:23 ` Clemens Ladisch 2010-11-02 8:34 ` Jaroslav Kysela 2010-11-02 8:38 ` Jaroslav Kysela 1 sibling, 2 replies; 14+ messages in thread From: Clemens Ladisch @ 2010-11-02 8:23 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: alsa-devel, Pierre-Louis Bossart Sorry for not answering sooner. Pierre-Louis Bossart wrote: > (No filtering in pcm_multi and pcm_direct info fields) I'm not sure whether filtering or not filtering would be a good idea for yet unknown flags. > + * It is not allowed to call this function when given configuration is not exactly one. > ... > +int snd_pcm_hw_params_can_disable_period_irq(const snd_pcm_hw_params_t *params) This function is useless because it cannot be called before snd_pcm_hw_params_set_period_irq(). > + * This function should be called only on devices where > + * #snd_pcm_hw_params_can_disable_period_irq() returns true. (too late, FIXME) Therefore, this paragraph needs to go away. I think there should be a flag that can be added to snd_pcm_open, like SND_PCM_NO_AUTO_RESAMPLE. > + * Even with disabled period interrupts, the period size/time/count parameters > + * are valid; it is suggested to use #snd_pcm_hw_params_set_period_size_last(). > + * > + * When period interrupts are disabled, the application must not use poll() or > + * any functions that could block on this device. We should also mention that the application must call some update function often enough. Regards, Clemens ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] add API to allow disabling period interrupt 2010-11-02 8:23 ` Clemens Ladisch @ 2010-11-02 8:34 ` Jaroslav Kysela 2010-11-02 8:38 ` Jaroslav Kysela 1 sibling, 0 replies; 14+ messages in thread From: Jaroslav Kysela @ 2010-11-02 8:34 UTC (permalink / raw) To: Clemens Ladisch; +Cc: Pierre-Louis Bossart, alsa-devel, Pierre-Louis Bossart On Tue, 2 Nov 2010, Clemens Ladisch wrote: >> + * When period interrupts are disabled, the application must not use poll() or >> + * any functions that could block on this device. > > We should also mention that the application must call some update > function often enough. Reading this - the poll() inside the driver may return POLLERR when no period updates are invoked in the driver. It does not make much sense to block the application when we know that the wake up function is not supported. Also all wait functions in alsa-lib should be reviewed and changed to return an error. Also, the blocking behaviour should not be allowed - it means that apps must use SND_PCM_NONBLOCK or snd_pcm_nonblock(pcm, 1) for this mode. Jaroslav ----- Jaroslav Kysela <perex@perex.cz> Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] add API to allow disabling period interrupt 2010-11-02 8:23 ` Clemens Ladisch 2010-11-02 8:34 ` Jaroslav Kysela @ 2010-11-02 8:38 ` Jaroslav Kysela 2010-11-02 9:04 ` Clemens Ladisch 1 sibling, 1 reply; 14+ messages in thread From: Jaroslav Kysela @ 2010-11-02 8:38 UTC (permalink / raw) To: Clemens Ladisch; +Cc: Pierre-Louis Bossart, alsa-devel, Pierre-Louis Bossart On Tue, 2 Nov 2010, Clemens Ladisch wrote: >> + * It is not allowed to call this function when given configuration is not exactly one. >> ... >> +int snd_pcm_hw_params_can_disable_period_irq(const snd_pcm_hw_params_t *params) > > This function is useless because it cannot be called before > snd_pcm_hw_params_set_period_irq(). I don't think so. The info bits from the driver are used in this check which are read-only. >> + * This function should be called only on devices where >> + * #snd_pcm_hw_params_can_disable_period_irq() returns true. (too late, FIXME) > > Therefore, this paragraph needs to go away. > > I think there should be a flag that can be added to snd_pcm_open, like > SND_PCM_NO_AUTO_RESAMPLE. It's not required. It's better to allow checking if no period update feature is available after snd_pcm_open(). Jaroslav ----- Jaroslav Kysela <perex@perex.cz> Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] add API to allow disabling period interrupt 2010-11-02 8:38 ` Jaroslav Kysela @ 2010-11-02 9:04 ` Clemens Ladisch 2010-11-02 9:52 ` Jaroslav Kysela 0 siblings, 1 reply; 14+ messages in thread From: Clemens Ladisch @ 2010-11-02 9:04 UTC (permalink / raw) To: Jaroslav Kysela; +Cc: Pierre-Louis Bossart, alsa-devel, Pierre-Louis Bossart Jaroslav Kysela wrote: > On Tue, 2 Nov 2010, Clemens Ladisch wrote: >>> + * It is not allowed to call this function when given configuration is not exactly one. >>> ... >>> +int snd_pcm_hw_params_can_disable_period_irq(const snd_pcm_hw_params_t *params) >> >> This function is useless because it cannot be called before >> snd_pcm_hw_params_set_period_irq(). > > I don't think so. The info bits from the driver are used in this check > which are read-only. All the snd_pcm_hw_params_can_* functions that read the info bits are documented to require a completed configuration. So they actually don't, and there will never be a flag that depends on the configuration? Regards, Clemens ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] add API to allow disabling period interrupt 2010-11-02 9:04 ` Clemens Ladisch @ 2010-11-02 9:52 ` Jaroslav Kysela 2010-11-02 13:12 ` pl bossart 0 siblings, 1 reply; 14+ messages in thread From: Jaroslav Kysela @ 2010-11-02 9:52 UTC (permalink / raw) To: Clemens Ladisch; +Cc: Pierre-Louis Bossart, alsa-devel, Pierre-Louis Bossart On Tue, 2 Nov 2010, Clemens Ladisch wrote: > Jaroslav Kysela wrote: >> On Tue, 2 Nov 2010, Clemens Ladisch wrote: >>>> + * It is not allowed to call this function when given configuration is not exactly one. >>>> ... >>>> +int snd_pcm_hw_params_can_disable_period_irq(const snd_pcm_hw_params_t *params) >>> >>> This function is useless because it cannot be called before >>> snd_pcm_hw_params_set_period_irq(). >> >> I don't think so. The info bits from the driver are used in this check >> which are read-only. > > All the snd_pcm_hw_params_can_* functions that read the info bits > are documented to require a completed configuration. So they actually > don't, and there will never be a flag that depends on the configuration? You're right. The *_can_* check should be removed. Jaroslav ----- Jaroslav Kysela <perex@perex.cz> Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] add API to allow disabling period interrupt 2010-11-02 9:52 ` Jaroslav Kysela @ 2010-11-02 13:12 ` pl bossart 2010-11-03 7:38 ` Jaroslav Kysela 0 siblings, 1 reply; 14+ messages in thread From: pl bossart @ 2010-11-02 13:12 UTC (permalink / raw) To: Jaroslav Kysela; +Cc: alsa-devel, Clemens Ladisch >>>>> +int snd_pcm_hw_params_can_disable_period_irq(const snd_pcm_hw_params_t >>>>> *params) >>>> >>>> This function is useless because it cannot be called before >>>> snd_pcm_hw_params_set_period_irq(). >>> >>> I don't think so. The info bits from the driver are used in this check >>> which are read-only. >> >> All the snd_pcm_hw_params_can_* functions that read the info bits >> are documented to require a completed configuration. So they actually >> don't, and there will never be a flag that depends on the configuration? > > You're right. The *_can_* check should be removed. If I remove the *_can_* check and add a flag for snd_pcm_open, actually the other set/get routines need to go as well. Everything would happen at the opening stages instead of when the hardware params are set, you would try to disable the interrupts and if the open fails you would fall back to 'normal' mode. The alternative would be to remove the _*can* but keep the set/get routines. In that case the hw_params configuration would be rejected if the hardware doesn't support this feature. Which solution do you guys prefer? Either way, it makes sense to check that the app uses the non-blocking mode as well, otherwise everything would be broken as noted in the remarks. -Pierre ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] add API to allow disabling period interrupt 2010-11-02 13:12 ` pl bossart @ 2010-11-03 7:38 ` Jaroslav Kysela 2010-11-08 20:58 ` pl bossart 0 siblings, 1 reply; 14+ messages in thread From: Jaroslav Kysela @ 2010-11-03 7:38 UTC (permalink / raw) To: pl bossart; +Cc: alsa-devel, Clemens Ladisch [-- Attachment #1: Type: TEXT/PLAIN, Size: 2643 bytes --] On Tue, 2 Nov 2010, pl bossart wrote: >>>>>> +int snd_pcm_hw_params_can_disable_period_irq(const snd_pcm_hw_params_t >>>>>> *params) >>>>> >>>>> This function is useless because it cannot be called before >>>>> snd_pcm_hw_params_set_period_irq(). >>>> >>>> I don't think so. The info bits from the driver are used in this check >>>> which are read-only. >>> >>> All the snd_pcm_hw_params_can_* functions that read the info bits >>> are documented to require a completed configuration. So they actually >>> don't, and there will never be a flag that depends on the configuration? >> >> You're right. The *_can_* check should be removed. > > If I remove the *_can_* check and add a flag for snd_pcm_open, > actually the other set/get routines need to go as well. Everything > would happen at the opening stages instead of when the hardware params > are set, you would try to disable the interrupts and if the open fails > you would fall back to 'normal' mode. I'm not sure if it's good to have another flag for snd_pcm_open. If we accept new open flag, it would be just an optional hint which should be verified by the application if it's really enabled using a function. > The alternative would be to remove the _*can* but keep the set/get > routines. In that case the hw_params configuration would be rejected > if the hardware doesn't support this feature. > Which solution do you guys prefer? Note that there is a big difference between your new flag and the existing ones. The *_can_* functions just reflects the extended read only capabilities: snd_pcm_hw_params_can_mmap_sample_resolution snd_pcm_hw_params_can_overrange snd_pcm_hw_params_can_pause snd_pcm_hw_params_can_resume snd_pcm_hw_params_can_sync_start I see two ways: 1) leave set/get functions with this semantics: - application tries to set the no period update behaviour for hw_params - alsa-lib should check if nonblock flag is set at this moment - snd_pcm_hw_params() call - application reads back the no period update state and if the flag is not set, it will use standard period updates 2) create new optional snd_pcm_open flag - application opens device with nonblock and no period update flags - snd_pcm_hw_params() call - application reads back the no period update state and if the flag is not set, it will use standard period updates Note that we can even implement both methods together like for handling the nonblock behaviour. Jaroslav ----- Jaroslav Kysela <perex@perex.cz> Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc. [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] add API to allow disabling period interrupt 2010-11-03 7:38 ` Jaroslav Kysela @ 2010-11-08 20:58 ` pl bossart 2010-11-08 21:03 ` Jaroslav Kysela 0 siblings, 1 reply; 14+ messages in thread From: pl bossart @ 2010-11-08 20:58 UTC (permalink / raw) To: Jaroslav Kysela; +Cc: alsa-devel, Clemens Ladisch > I see two ways: > > 1) leave set/get functions with this semantics: > - application tries to set the no period update behaviour for hw_params > - alsa-lib should check if nonblock flag is set at this moment > - snd_pcm_hw_params() call > - application reads back the no period update state and if the flag is > not set, it will use standard period updates > 2) create new optional snd_pcm_open flag > - application opens device with nonblock and no period update flags > - snd_pcm_hw_params() call > - application reads back the no period update state and if the flag is > not set, it will use standard period updates > > Note that we can even implement both methods together like for handling the > nonblock behaviour. I re-implemented the first solution and checked if the mode is indeed non-blocking. This solution assumes that the apps cannot go back to blocking mode at a later stage. Is it a valid asumption, or can the blocking mode be enabled/disabled at any time with snd_pcm_nonblock()? -Pierre ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] add API to allow disabling period interrupt 2010-11-08 20:58 ` pl bossart @ 2010-11-08 21:03 ` Jaroslav Kysela 0 siblings, 0 replies; 14+ messages in thread From: Jaroslav Kysela @ 2010-11-08 21:03 UTC (permalink / raw) To: pl bossart; +Cc: alsa-devel, Clemens Ladisch [-- Attachment #1: Type: TEXT/PLAIN, Size: 1329 bytes --] On Mon, 8 Nov 2010, pl bossart wrote: >> I see two ways: >> >> 1) leave set/get functions with this semantics: >> - application tries to set the no period update behaviour for hw_params >> - alsa-lib should check if nonblock flag is set at this moment >> - snd_pcm_hw_params() call >> - application reads back the no period update state and if the flag is >> not set, it will use standard period updates >> 2) create new optional snd_pcm_open flag >> - application opens device with nonblock and no period update flags >> - snd_pcm_hw_params() call >> - application reads back the no period update state and if the flag is >> not set, it will use standard period updates >> >> Note that we can even implement both methods together like for handling the >> nonblock behaviour. > > I re-implemented the first solution and checked if the mode is indeed > non-blocking. This solution assumes that the apps cannot go back to > blocking mode at a later stage. Is it a valid asumption, or can the > blocking mode be enabled/disabled at any time with snd_pcm_nonblock()? Yes, this function must return an error if an app tries to set blocking mode.. Jaroslav ----- Jaroslav Kysela <perex@perex.cz> Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc. [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] pcm: pass hw_params flags to slave 2010-11-01 22:13 [PATCH 1/2] pcm: pass hw_params flags to slave Pierre-Louis Bossart 2010-11-01 22:13 ` [PATCH 2/2] add API to allow disabling period interrupt Pierre-Louis Bossart @ 2010-11-02 7:27 ` Takashi Iwai 2010-11-02 12:52 ` pl bossart 1 sibling, 1 reply; 14+ messages in thread From: Takashi Iwai @ 2010-11-02 7:27 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: alsa-devel, Pierre-Louis Bossart At Mon, 1 Nov 2010 17:13:33 -0500, Pierre-Louis Bossart wrote: > > fix required before interrupt disabling routines patch can be applied. > Without this fix, the interrupts are only disabled when accessing > directly hw devices. > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@intel.com> I'm a bit skeptical whether this is safe to apply. The plugins might not be always transparent. Double-check whether this doesn't break all plugins behaviors. thanks, Takashi > --- > src/pcm/pcm_params.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/src/pcm/pcm_params.c b/src/pcm/pcm_params.c > index 3a90bcd..416b081 100644 > --- a/src/pcm/pcm_params.c > +++ b/src/pcm/pcm_params.c > @@ -2126,6 +2126,7 @@ int _snd_pcm_hw_params_refine(snd_pcm_hw_params_t *params, > err = changed; > } > params->info &= src->info; > + params->flags = src->flags; /* propagate flags to slave */ > return err; > } > > -- > 1.7.2.3 > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] pcm: pass hw_params flags to slave 2010-11-02 7:27 ` [PATCH 1/2] pcm: pass hw_params flags to slave Takashi Iwai @ 2010-11-02 12:52 ` pl bossart 0 siblings, 0 replies; 14+ messages in thread From: pl bossart @ 2010-11-02 12:52 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel Thanks Takashi for the review. > I'm a bit skeptical whether this is safe to apply. > The plugins might not be always transparent. Double-check whether > this doesn't break all plugins behaviors. I am really not sure how I can prove that passing flags does not alter existing behavior, I identified this issue experimentally after several hours and I must admit I don't really understand the alsa-lib code. The question is really why weren't the flags propagated in the initial code? All parameters (info, rate, etc) are passed to the slave, except the flags. Either this was a conscious design decision, in which case my patch would create a problem, or this was a miss and my patch corrects it. >> params->info &= src->info; >> + params->flags = src->flags; /* propagate flags to slave */ ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2010-11-08 21:03 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-11-01 22:13 [PATCH 1/2] pcm: pass hw_params flags to slave Pierre-Louis Bossart 2010-11-01 22:13 ` [PATCH 2/2] add API to allow disabling period interrupt Pierre-Louis Bossart 2010-11-02 7:28 ` Takashi Iwai 2010-11-02 8:23 ` Clemens Ladisch 2010-11-02 8:34 ` Jaroslav Kysela 2010-11-02 8:38 ` Jaroslav Kysela 2010-11-02 9:04 ` Clemens Ladisch 2010-11-02 9:52 ` Jaroslav Kysela 2010-11-02 13:12 ` pl bossart 2010-11-03 7:38 ` Jaroslav Kysela 2010-11-08 20:58 ` pl bossart 2010-11-08 21:03 ` Jaroslav Kysela 2010-11-02 7:27 ` [PATCH 1/2] pcm: pass hw_params flags to slave Takashi Iwai 2010-11-02 12:52 ` pl bossart
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.