| Message ID | 1485560145-12908-1-git-send-email-ce3a@gmx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Jan 28, 2017 at 12:35:45AM +0100, Sergej Sawazki wrote: > Do not apply rate constraints in the startup() callback. The machine driver > can change the sysclk and hence the supported frame rates in its hw_params(). > This callback is unneeded since commit e369bd006fd6 ("ASoC: wm8741: Allow > master clock switching"). > > Signed-off-by: Sergej Sawazki <ce3a@gmx.de> > --- > sound/soc/codecs/wm8741.c | 15 --------------- > 1 file changed, 15 deletions(-) > > diff --git a/sound/soc/codecs/wm8741.c b/sound/soc/codecs/wm8741.c > index b8c1940..d6e540a 100644 > --- a/sound/soc/codecs/wm8741.c > +++ b/sound/soc/codecs/wm8741.c > @@ -176,20 +176,6 @@ static const struct snd_pcm_hw_constraint_list constraints_36864 = { > .list = rates_36864, > }; > > -static int wm8741_startup(struct snd_pcm_substream *substream, > - struct snd_soc_dai *dai) > -{ > - struct snd_soc_codec *codec = dai->codec; > - struct wm8741_priv *wm8741 = snd_soc_codec_get_drvdata(codec); > - > - if (wm8741->sysclk) > - snd_pcm_hw_constraint_list(substream->runtime, 0, > - SNDRV_PCM_HW_PARAM_RATE, > - wm8741->sysclk_constraints); > - > - return 0; > -} > - This function should not be removed it is performing a useful function. If a sysclk has already been configured by the machine driver then we should inform user-space of the rates we can support from that clock. Without this that information is not available to user-space, so instead of user-space being able to resample appropriately it would just error out from hw_params. Are you perhaps missing a call to clear the sysclk from your machine driver? Usually a dai_set_sysclk call with a rate of zero, also I find if it is a machine driver supporting multiple rates you are best to use ignore_pmdown_time on the DAI link, assuming the devices don't have long bring up times. Thanks, Charles
On Mon, 30 Jan 2017 09:22:59 +0000, Charles Keepax wrote: > On Sat, Jan 28, 2017 at 12:35:45AM +0100, Sergej Sawazki wrote: >> Do not apply rate constraints in the startup() callback. The machine driver >> can change the sysclk and hence the supported frame rates in its hw_params(). >> This callback is unneeded since commit e369bd006fd6 ("ASoC: wm8741: Allow >> master clock switching"). > This function should not be removed it is performing a useful > function. If a sysclk has already been configured by the machine > driver then we should inform user-space of the rates we can > support from that clock. Without this that information is not > available to user-space, so instead of user-space being able to > resample appropriately it would just error out from hw_params. > > Are you perhaps missing a call to clear the sysclk from your > machine driver? Usually a dai_set_sysclk call with a rate of > zero, also I find if it is a machine driver supporting multiple > rates you are best to use ignore_pmdown_time on the DAI link, > assuming the devices don't have long bring up times. After clearing the sysclk, the codec should 'pretend' to support all rates, right? (no sysclk -> no rate constrains) After applying rate constraints using snd_pcm_hw_constraint_list(...), what would be a clean way to remove the constraints from the rates parameter? Thanks, Sergej
On Mon, Jan 30, 2017 at 11:56:52PM +0100, Sergej Sawazki wrote: > On Mon, 30 Jan 2017 09:22:59 +0000, Charles Keepax wrote: > > On Sat, Jan 28, 2017 at 12:35:45AM +0100, Sergej Sawazki wrote: > >> Do not apply rate constraints in the startup() callback. The machine driver > >> can change the sysclk and hence the supported frame rates in its hw_params(). > >> This callback is unneeded since commit e369bd006fd6 ("ASoC: wm8741: Allow > >> master clock switching"). > > > This function should not be removed it is performing a useful > > function. If a sysclk has already been configured by the machine > > driver then we should inform user-space of the rates we can > > support from that clock. Without this that information is not > > available to user-space, so instead of user-space being able to > > resample appropriately it would just error out from hw_params. > > > > Are you perhaps missing a call to clear the sysclk from your > > machine driver? Usually a dai_set_sysclk call with a rate of > > zero, also I find if it is a machine driver supporting multiple > > rates you are best to use ignore_pmdown_time on the DAI link, > > assuming the devices don't have long bring up times. > > After clearing the sysclk, the codec should 'pretend' to support all > rates, right? (no sysclk -> no rate constrains) > Indeed yes, so a normal pattern would often be setting a sysclk rate in set_bias_level(on the way up)/hw_params and clearing it in set_bias_level(on the way down). That way whilst the device is powered up it is fixed at a certain rate so as not to disrupt any running audio but whilst powered down you are free to start it up at any rate. > After applying rate constraints using snd_pcm_hw_constraint_list(...), > what would be a clean way to remove the constraints from the rates > parameter? > I am not sure I fully follow here, normally one would close the stream and open a new one at the new rate. Hence no need to clear the constraits as its a new stream. Are you intending to keep the stream open but reconfigure it for new rates? Thanks, Charles
On Tue, 31 Jan 2017 09:49:31 +0000, Charles Keepax wrote: > On Mon, Jan 30, 2017 at 11:56:52PM +0100, Sergej Sawazki wrote: >> After clearing the sysclk, the codec should 'pretend' to support all >> rates, right? (no sysclk -> no rate constrains) > Indeed yes, so a normal pattern would often be setting a sysclk > rate in set_bias_level(on the way up)/hw_params and clearing it in > set_bias_level(on the way down). That way whilst the device is > powered up it is fixed at a certain rate so as not to disrupt any > running audio but whilst powered down you are free to start it up > at any rate. Thanks Charles, it is clear now, sorry for the noise. Sergej
diff --git a/sound/soc/codecs/wm8741.c b/sound/soc/codecs/wm8741.c index b8c1940..d6e540a 100644 --- a/sound/soc/codecs/wm8741.c +++ b/sound/soc/codecs/wm8741.c @@ -176,20 +176,6 @@ static const struct snd_pcm_hw_constraint_list constraints_36864 = { .list = rates_36864, }; -static int wm8741_startup(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct snd_soc_codec *codec = dai->codec; - struct wm8741_priv *wm8741 = snd_soc_codec_get_drvdata(codec); - - if (wm8741->sysclk) - snd_pcm_hw_constraint_list(substream->runtime, 0, - SNDRV_PCM_HW_PARAM_RATE, - wm8741->sysclk_constraints); - - return 0; -} - static int wm8741_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) @@ -360,7 +346,6 @@ static int wm8741_set_dai_fmt(struct snd_soc_dai *codec_dai, SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE) static const struct snd_soc_dai_ops wm8741_dai_ops = { - .startup = wm8741_startup, .hw_params = wm8741_hw_params, .set_sysclk = wm8741_set_dai_sysclk, .set_fmt = wm8741_set_dai_fmt,
Do not apply rate constraints in the startup() callback. The machine driver can change the sysclk and hence the supported frame rates in its hw_params(). This callback is unneeded since commit e369bd006fd6 ("ASoC: wm8741: Allow master clock switching"). Signed-off-by: Sergej Sawazki <ce3a@gmx.de> --- sound/soc/codecs/wm8741.c | 15 --------------- 1 file changed, 15 deletions(-)