| Message ID | 20240819141017.502-1-shenghao-ding@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] ASoc: tas2781: fixed the issue that the chip do not shutdown immediatly after aplay stopped | expand |
On Mon, Aug 19, 2024 at 10:10:12PM +0800, Shenghao Ding wrote: > Issue reported from customer that the chip do not shutdown after aplay > stopped until 6 mins later. Drop tasdevice_dapm_event and implement > .stream_mute in the tasdevice_dai_ops. Six minutes sounds like a usersrpace issue with userspace sitting playing silence for a long time rather than a driver issue. By default DAPM does defer powerdown, but only by seconds not minutes. > - /* Codec Lock Release*/ > - mutex_unlock(&tas_priv->codec_lock); > + /* Codec Lock/UnLock */ > + guard(mutex)(&tas_priv->codec_lock); > + tasdevice_tuning_switch(tas_priv, mute); This is a much heavier weight operation than we're expecting for a mute, it should usually just be literally muting - one or two register writes, not a power up/down sequence
Hi Broonie > -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: Tuesday, August 20, 2024 12:07 AM > To: Ding, Shenghao <shenghao-ding@ti.com> > Cc: andriy.shevchenko@linux.intel.com; lgirdwood@gmail.com; > perex@perex.cz; pierre-louis.bossart@linux.intel.com; > 13916275206@139.com; zhourui@huaqin.com; alsa-devel@alsa-project.org; > Salazar, Ivan <i-salazar@ti.com>; liam.r.girdwood@intel.com; Yue, Jaden > <jaden-yue@ti.com>; yung-chuan.liao@linux.intel.com; Rao, Dipa > <dipa@ti.com>; yuhsuan@google.com; Lo, Henry <henry.lo@ti.com>; > tiwai@suse.de; Xu, Baojun <baojun.xu@ti.com>; Baojun.Xu@fpt.com; Chadha, > Jasjot Singh <j-chadha@ti.com>; judyhsiao@google.com; Navada Kanyana, > Mukund <navada@ti.com>; cujomalainey@google.com; Kutty, Aanya > <aanya@ti.com>; Mahmud, Nayeem <nayeem.mahmud@ti.com>; > savyasanchi.shukla@netradyne.com; flaviopr@microsoft.com; Ji, Jesse <jesse- > ji@ti.com>; darren.ye@mediatek.com; antheas.dk@gmail.com; > Jerry2.Huang@lcfuturecenter.com; jim.shil@goertek.com > Subject: [EXTERNAL] Re: [PATCH v1] ASoc: tas2781: fixed the issue that the > chip do not shutdown immediatly after aplay stopped > > On Mon, Aug 19, 2024 at 10:10:12PM +0800, Shenghao Ding wrote: > > Issue reported from customer that the chip do not shutdown after aplay > > stopped until 6 mins later. Drop tasdevice_dapm_event and implement > > .stream_mute in the tasdevice_dai_ops. > > Six minutes sounds like a usersrpace issue with userspace sitting playing silence > for a long time rather than a driver issue. By default DAPM does defer > powerdown, but only by seconds not minutes. > > > - /* Codec Lock Release*/ > > - mutex_unlock(&tas_priv->codec_lock); > > + /* Codec Lock/UnLock */ > > + guard(mutex)(&tas_priv->codec_lock); > > + tasdevice_tuning_switch(tas_priv, mute); > > This is a much heavier weight operation than we're expecting for a mute, it > should usually just be literally muting - one or two register writes, not a power > up/down sequence Can the power on is still kept in DAMP, and move the power off(Only set the register 0x2 to 0xe) into the .mute_stream?
On Tue, Aug 20, 2024 at 04:34:00AM +0000, Ding, Shenghao wrote: > > This is a much heavier weight operation than we're expecting for a mute, it > > should usually just be literally muting - one or two register writes, not a power > > up/down sequence > Can the power on is still kept in DAMP, and move the power off(Only set the register 0x2 to 0xe) > into the .mute_stream? Wouldn't that break when a second stream is played, unless whatever is holding the output open is fixed DAPM will still have the device active and therefore not redo the power up sequence?
diff --git a/sound/soc/codecs/tas2781-i2c.c b/sound/soc/codecs/tas2781-i2c.c index 9a32e0504857..145c45349399 100644 --- a/sound/soc/codecs/tas2781-i2c.c +++ b/sound/soc/codecs/tas2781-i2c.c @@ -557,30 +557,22 @@ static void tasdevice_fw_ready(const struct firmware *fmw, release_firmware(fmw); } -static int tasdevice_dapm_event(struct snd_soc_dapm_widget *w, - struct snd_kcontrol *kcontrol, int event) +static int tasdevice_mute(struct snd_soc_dai *dai, int mute, int stream) { - struct snd_soc_component *codec = snd_soc_dapm_to_component(w->dapm); + struct snd_soc_component *codec = dai->component; struct tasdevice_priv *tas_priv = snd_soc_component_get_drvdata(codec); - int state = 0; - /* Codec Lock Hold */ - mutex_lock(&tas_priv->codec_lock); - if (event == SND_SOC_DAPM_PRE_PMD) - state = 1; - tasdevice_tuning_switch(tas_priv, state); - /* Codec Lock Release*/ - mutex_unlock(&tas_priv->codec_lock); + /* Codec Lock/UnLock */ + guard(mutex)(&tas_priv->codec_lock); + tasdevice_tuning_switch(tas_priv, mute); return 0; } static const struct snd_soc_dapm_widget tasdevice_dapm_widgets[] = { SND_SOC_DAPM_AIF_IN("ASI", "ASI Playback", 0, SND_SOC_NOPM, 0, 0), - SND_SOC_DAPM_AIF_OUT_E("ASI OUT", "ASI Capture", 0, SND_SOC_NOPM, - 0, 0, tasdevice_dapm_event, - SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD), - SND_SOC_DAPM_SPK("SPK", tasdevice_dapm_event), + SND_SOC_DAPM_AIF_OUT("ASI OUT", "ASI Capture", 0, SND_SOC_NOPM, 0, 0), + SND_SOC_DAPM_SPK("SPK", NULL), SND_SOC_DAPM_OUTPUT("OUT"), SND_SOC_DAPM_INPUT("DMIC") }; @@ -667,6 +659,7 @@ static const struct snd_soc_dai_ops tasdevice_dai_ops = { .startup = tasdevice_startup, .hw_params = tasdevice_hw_params, .set_sysclk = tasdevice_set_dai_sysclk, + .mute_stream = tasdevice_mute, }; static struct snd_soc_dai_driver tasdevice_dai_driver[] = {
Issue reported from customer that the chip do not shutdown after aplay stopped until 6 mins later. Drop tasdevice_dapm_event and implement .stream_mute in the tasdevice_dai_ops. Signed-off-by: Shenghao Ding <shenghao-ding@ti.com> --- v1: - Drop tasdevice_dapm_event and implement .stream_mute in the tasdevice_dai_ops - Remove unnecessary line feed for tasdevice_codec_remove --- sound/soc/codecs/tas2781-i2c.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-)