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.
thanks,
Takashi
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@suse.de
sound/x86/intel_hdmi_audio.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 95b07a260d54..506cff306b5c 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,6 +1833,10 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev) pdata->notify_pending = false; spin_unlock_irq(&pdata->lpe_audio_slock);
- /* set a relatively long autosuspend delay (2 seconds) for making
* keep_link feature working reasonably*/- pm_runtime_set_autosuspend_delay(&pdev->dev, 2000); pm_runtime_use_autosuspend(&pdev->dev); pm_runtime_mark_last_busy(&pdev->dev);