| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87a7d6h3b2.wl-kuninori.morimoto.gx@renesas.com>
Date: 22 Jul 2019 17:41:37 +0900
From: Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
To: Jiada Wang <jiada_wang@...tor.com>
Cc: <lgirdwood@...il.com>, <broonie@...nel.org>, <perex@...ex.cz>,
<tiwai@...e.com>, <twischer@...adit-jv.com>,
<alsa-devel@...a-project.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/3] ASoC: rsnd: Allow reconfiguration of clock rate
Hi Jiada
The solution looks very over-kill to me,
especiallyq [3/3] patch is too much to me.
1st, can we start clock at .hw_param, instead of .prepare ?
and stop it at .hw_free ?
2nd, can we keep usrcnt setup as-is ?
I guess we can just avoid rsnd_ssi_master_clk_start() if ssi->rate was not 0 ?
similar for rsnd_ssi_master_clk_stop()
static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
struct rsnd_dai_stream *io)
{
...
if (ssi->rate)
return 0;
...
}
static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
struct rsnd_dai_stream *io)
{
...
- if (ssi->usrcnt > 1)
+ if (ssi->rate == 0)
return;
...
}
> From: Timo Wischer <twischer@...adit-jv.com>
>
> Currently after clock rate is started in .prepare reconfiguration
> of clock rate is not allowed, unless the stream is stopped.
>
> But there is use case in Gstreamer ALSA sink, in case of changed caps
> Gsreatmer reconfigs and it calls snd_pcm_hw_free() before snd_pcm_prepre().
> See gstreamer1.0-plugins-base/
> gst-libs/gst/audio/gstaudiobasesink.c: gst_audio_base_sink_setcaps():
> - gst_audio_ring_buffer_release()
> - gst_audio_sink_ring_buffer_release()
> - gst_alsasink_unprepare()
> - snd_pcm_hw_free()
> is called before
> - gst_audio_ring_buffer_acquire()
> - gst_audio_sink_ring_buffer_acquire()
> - gst_alsasink_prepare()
> - set_hwparams()
> - snd_pcm_hw_params()
> - snd_pcm_prepare()
>
> The issue mentioned in this commit can be reproduced with the following
> aplay patch:
>
> >diff --git a/aplay/aplay.c b/aplay/aplay.c
> >@@ -2760,6 +2760,8 @@ static void playback_go(int fd, size_t loaded,
> > header(rtype, name);
> > set_params();
> >+ hwparams.rate = (hwparams.rate == 48000) ? 44100 : 48000;
> >+ set_params();
> >
> > while (loaded > chunk_bytes && written < count && !in_aborting)
> > {
> > if (pcm_write(audiobuf + written, chunk_size) <= 0)
>
> $ aplay -Dplughw:0,0,0 -c8 -fS24_LE -r48000 /dev/zero
> rcar_sound ec500000.sound: SSI parent/child should use same rate
> rcar_sound ec500000.sound: ssi[3] : prepare error -22
> rcar_sound ec500000.sound: ASoC: cpu DAI prepare error: -22
> rsnd_link0: ASoC: prepare FE rsnd_link0 failed
>
> this patch address the issue by stop clock in .hw_free,
> to allow reconfiguration of clock rate without stop of the stream.
>
> Signed-off-by: Timo Wischer <twischer@...adit-jv.com>
> Signed-off-by: Jiada Wang <jiada_wang@...tor.com>
> ---
> sound/soc/sh/rcar/ssi.c | 53 +++++++++++++++++++++++++++++------------
> 1 file changed, 38 insertions(+), 15 deletions(-)
>
> diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
> index f6a7466622ea..f43937d2c588 100644
> --- a/sound/soc/sh/rcar/ssi.c
> +++ b/sound/soc/sh/rcar/ssi.c
> @@ -286,7 +286,7 @@ static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
> if (rsnd_ssi_is_multi_slave(mod, io))
> return 0;
>
> - if (ssi->usrcnt > 0) {
> + if (ssi->rate) {
> if (ssi->rate != rate) {
> dev_err(dev, "SSI parent/child should use same rate\n");
> return -EINVAL;
> @@ -471,13 +471,9 @@ static int rsnd_ssi_init(struct rsnd_mod *mod,
> struct rsnd_dai_stream *io,
> struct rsnd_priv *priv)
> {
> - struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
> -
> if (!rsnd_ssi_is_run_mods(mod, io))
> return 0;
>
> - ssi->usrcnt++;
> -
> rsnd_mod_power_on(mod);
>
> rsnd_ssi_config_init(mod, io);
> @@ -505,18 +501,8 @@ static int rsnd_ssi_quit(struct rsnd_mod *mod,
> return -EIO;
> }
>
> - rsnd_ssi_master_clk_stop(mod, io);
> -
> rsnd_mod_power_off(mod);
>
> - ssi->usrcnt--;
> -
> - if (!ssi->usrcnt) {
> - ssi->cr_own = 0;
> - ssi->cr_mode = 0;
> - ssi->wsr = 0;
> - }
> -
> return 0;
> }
>
> @@ -525,6 +511,7 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
> struct snd_pcm_substream *substream,
> struct snd_pcm_hw_params *params)
> {
> + struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
> struct rsnd_dai *rdai = rsnd_io_to_rdai(io);
> unsigned int fmt_width = snd_pcm_format_width(params_format(params));
>
> @@ -536,6 +523,11 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
> return -EINVAL;
> }
>
> + if (!rsnd_ssi_is_run_mods(mod, io))
> + return 0;
> +
> + ssi->usrcnt++;
> +
> return 0;
> }
>
> @@ -913,6 +905,35 @@ static int rsnd_ssi_prepare(struct rsnd_mod *mod,
> return rsnd_ssi_master_clk_start(mod, io);
> }
>
> +static int rsnd_ssi_hw_free(struct rsnd_mod *mod, struct rsnd_dai_stream *io,
> + struct snd_pcm_substream *substream)
> +{
> + struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
> +
> + if (!rsnd_ssi_is_run_mods(mod, io))
> + return 0;
> +
> + if (!ssi->usrcnt) {
> + struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
> + struct device *dev = rsnd_priv_to_dev(priv);
> +
> + dev_err(dev, "%s usrcnt error\n", rsnd_mod_name(mod));
> + return -EIO;
> + }
> +
> + rsnd_ssi_master_clk_stop(mod, io);
> +
> + ssi->usrcnt--;
> +
> + if (!ssi->usrcnt) {
> + ssi->cr_own = 0;
> + ssi->cr_mode = 0;
> + ssi->wsr = 0;
> + }
> +
> + return 0;
> +}
> +
> static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
> .name = SSI_NAME,
> .probe = rsnd_ssi_common_probe,
> @@ -926,6 +947,7 @@ static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
> .pcm_new = rsnd_ssi_pcm_new,
> .hw_params = rsnd_ssi_hw_params,
> .prepare = rsnd_ssi_prepare,
> + .hw_free = rsnd_ssi_hw_free,
> .get_status = rsnd_ssi_get_status,
> };
>
> @@ -1012,6 +1034,7 @@ static struct rsnd_mod_ops rsnd_ssi_dma_ops = {
> .pcm_new = rsnd_ssi_pcm_new,
> .fallback = rsnd_ssi_fallback,
> .hw_params = rsnd_ssi_hw_params,
> + .hw_free = rsnd_ssi_hw_free,
> .prepare = rsnd_ssi_prepare,
> .get_status = rsnd_ssi_get_status,
> };
> --
> 2.19.2
>
Powered by blists - more mailing lists