[alsa-devel] [PATCH 4/4] ALSA: x86: Enable keep-link feature

Takashi Iwai tiwai at suse.de
Mon Feb 13 22:26:13 CET 2017


On Mon, 13 Feb 2017 21:05:31 +0100,
Takashi Iwai wrote:
> 
> On Mon, 13 Feb 2017 18:05:37 +0100,
> Pierre-Louis Bossart wrote:
> > 
> > On 2/13/17 8:39 AM, Takashi Iwai wrote:
> > > This patch enables the "keep-link" feature experimentally.  It's a
> > > feature where the device keeps the link and sending the silent output
> > > even after the PCM device is closed.  Then the receiver will be
> > > resumed quickly once when a PCM is opened and a stream is sent again.
> > >
> > > The stream link is turned off when the device goes to the auto
> > > suspend, and it's set to two seconds after the PCM close.  This
> > > timeout value can be changed dynamically in a standard way via sysfs
> > > like other drivers.  For example, to make it 10 seconds, run like:
> > >
> > >   echo 10000 > /sys/bus/platform/devices/hdmi-lpe-audio/power/autosuspend_delay_ms
> > 
> > Keeping the link active has really a limited impact on power since the
> > source provides power to the sink and will also drive the display. For
> > people who rely on HDMI for system sounds/beeps/notifications it'd
> > make more sense to make that value something like 15-30mn
> > (i.e. aligned with display screen saver timeout), otherwise for every
> > sound the receiver will have to spend 1-2 seconds figuring out if the
> > data is PCM or compressed.
> > PulseAudio has a 5s timeout on idle, 2s seems really low to me.
> 
> I have no problem to extend the timeout -- or even to disable the
> autosuspend as default.  The current time was deduced from the past
> experience: keeping the audio link on HSW and onward may cost very
> much in the i915 graphics side because it blocks the power well audio
> domain.  That's why we had to enable HD-audio autosuspend for HDMI
> specifically; there was an explicit request from graphics people.  But
> BYT/CHT has no power well domain, so this might not matter.  (Correct
> me if my assumption is wrong.)
> 
> Basically it's trivial to "fix" it.  We may leave the autosuspend
> timeout untouched (i.e. 0) and don't enable autosuspend as default but
> keep the power on as default.  Two more lines cut.  Good.
> 
> People who care about the power may adjust the standard sysfs entries
> accordingly.

... and below is the revised patch.  Will refresh the branch as well.


Takashi

-- 8< --
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH v2] ALSA: x86: Enable keep-link feature

This patch enables the "keep-link" feature experimentally.  It's a
feature where the device keeps the link and sending the silent output
even after the PCM device is closed.  Then the receiver will be
resumed quickly once when a PCM is opened and a stream is sent again.

The stream link is turned off when the device goes to autosuspend or
manual suspend.

With this commit, the runtime PM is not enabled any longer as
default since there is little gain on BYT/CHT with this audio device
PM.  User who still wants the runtime PM may adjust the corresponding
sysfs files (power/control and power/autosuspend_delay_ms)
appropriately, of course.

This new keep-link feature itself is controlled via a new module
option, keep_link.  You can turn it on/off, again, via sysfs like:

  echo 0 > /sys/module/snd_hdmi_lpe_audio/parameters/keep_link

As default, the feature is turned on.

Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 sound/x86/intel_hdmi_audio.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 95b07a260d54..0805c835bb8b 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -52,6 +52,10 @@ module_param_named(id, hdmi_card_id, charp, 0444);
 MODULE_PARM_DESC(id,
 		"ID string for INTEL Intel HDMI Audio controller.");
 
+static bool keep_link = true;
+module_param(keep_link, bool, 0644);
+MODULE_PARM_DESC(keep_link, "Keep link on after the stream is closed.");
+
 /*
  * ELD SA bits in the CEA Speaker Allocation data block
  */
@@ -217,8 +221,12 @@ static void had_write_register(struct snd_intelhad *ctx, u32 reg, u32 val)
 static void had_enable_audio(struct snd_intelhad *intelhaddata,
 			     bool enable)
 {
+	if (intelhaddata->aud_config.regx.aud_en == enable)
+		return;
+
 	/* update the cached value */
 	intelhaddata->aud_config.regx.aud_en = enable;
+	intelhaddata->aud_config.regx.underrun = keep_link;
 	had_write_register(intelhaddata, AUD_CONFIG,
 			   intelhaddata->aud_config.regval);
 }
@@ -901,6 +909,21 @@ static void had_init_ringbuf(struct snd_pcm_substream *substream,
 	intelhaddata->bd_head = 0; /* reset at head again before starting */
 }
 
+/* Set up the silent output after PCM close */
+static void had_keep_silent(struct snd_intelhad *intelhaddata)
+{
+	int i;
+
+	if (!(keep_link && intelhaddata->connected &&
+	      intelhaddata->aud_config.regval))
+		return;
+
+	for (i = 0; i < HAD_NUM_OF_RING_BUFS; i++)
+		had_invalidate_bd(intelhaddata, i);
+	intelhaddata->need_reset = true; /* reset at next */
+	had_enable_audio(intelhaddata, true);
+}
+
 /* process a bd, advance to the next */
 static void had_advance_ringbuf(struct snd_pcm_substream *substream,
 				struct snd_intelhad *intelhaddata)
@@ -1007,6 +1030,9 @@ static void had_do_reset(struct snd_intelhad *intelhaddata)
 	if (!intelhaddata->need_reset)
 		return;
 
+	/* disable the silent output */
+	had_enable_audio(intelhaddata, false);
+
 	/* Reset buffer pointers */
 	had_reset_audio(intelhaddata);
 	wait_clear_underrun_bit(intelhaddata);
@@ -1101,6 +1127,8 @@ static int had_pcm_close(struct snd_pcm_substream *substream)
 	}
 	spin_unlock_irq(&intelhaddata->had_spinlock);
 
+	had_keep_silent(intelhaddata);
+
 	pm_runtime_mark_last_busy(intelhaddata->dev);
 	pm_runtime_put_autosuspend(intelhaddata->dev);
 	return 0;
@@ -1626,6 +1654,9 @@ static int hdmi_lpe_audio_runtime_suspend(struct device *dev)
 		had_substream_put(ctx);
 	}
 
+	/* disable the silent output */
+	had_enable_audio(ctx, false);
+
 	return 0;
 }
 
@@ -1642,6 +1673,9 @@ static int __maybe_unused hdmi_lpe_audio_suspend(struct device *dev)
 
 static int hdmi_lpe_audio_runtime_resume(struct device *dev)
 {
+	struct snd_intelhad *ctx = dev_get_drvdata(dev);
+
+	had_keep_silent(ctx);
 	pm_runtime_mark_last_busy(dev);
 	return 0;
 }
@@ -1799,11 +1833,13 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 	pdata->notify_pending = false;
 	spin_unlock_irq(&pdata->lpe_audio_slock);
 
+	/* runtime PM isn't enabled as default, since it won't save much on
+	 * BYT/CHT devices; user who want the runtime PM should adjust the
+	 * power/ontrol and power/autosuspend_delay_ms sysfs entries instead
+	 */
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_mark_last_busy(&pdev->dev);
-
 	pm_runtime_set_active(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
 
 	dev_dbg(&pdev->dev, "%s: handle pending notification\n", __func__);
 	schedule_work(&ctx->hdmi_audio_wq);
-- 
2.11.1



More information about the Alsa-devel mailing list