On 04/29/2013 05:02 PM, Jesse Barnes wrote:
On Sat, 27 Apr 2013 13:35:29 +0200
Daniel Vetter <dan...@ffwll.ch> wrote:
On Sat, Apr 27, 2013 at 09:20:39AM +0000, Wang, Xingchao wrote:
Let me throw a basic proposal on Audio driver side, please give your comments
freely.
it contains the power well control usage points:
#1: audio request power well at boot up.
I915 may shut down power well after bootup initialization, as there's no
monitor connected outside or only eDP on pipe A.
#2: audio request power on resume
After exit from D3 mode, audio driver quest power on. This may happen at normal
resume or runtime resume.
#3: audio release power well control at suspend
Audio driver will let i915 know it doensot need power well anymore as it's
going to suspend. This may happened at normal suspend or runtime suspend point.
#4: audio release power well when module unload
Audio release power well at remove callback to let i915 know.
I miss the power well grab/dropping at runtime from the audio side. If the
audio driver forces the power well to be on the entire time it's loaded,
that's not good, since the power well stuff is very much for runtime PM.
We _must_ be able to switch off the power well whenever possible.
Xingchao, I'm not an audio developer so I'm probably way off.
But what we really need is a very small and targeted set of calls into
the i915 driver from say the HDMI driver in HDA. It looks like the
prepare/cleanup pair in the pcm_ops structure might be the right place
to put things? If that's too fine grained, you could do it at
open/close time I guess, but the danger there is that some app will
keep the device open even while it's not playing.
If that won't work, maybe calling i915 from hda_power_work in the
higher level code would be better?
For detecting whether to call i915 at all, you can filter on the PCI
IDs (just look for an Intel graphics device and if present, try to get
the i915 symbols for the power functions).
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -3860,6 +3860,8 @@ static unsigned int hda_call_codec_suspend(struct hda_code
codec->patch_ops.suspend(codec);
hda_cleanup_all_streams(codec);
state = hda_set_power_state(codec, AC_PWRST_D3);
+ if (i915_shared_power_well)
+ i915_put_power_well(codec->i915_data);
/* Cancel delayed work if we aren't currently running from it. */
if (!in_wq)
cancel_delayed_work_sync(&codec->power_work);
@@ -4807,6 +4809,9 @@ static void __snd_hda_power_up(struct hda_codec *codec, bo
return;
spin_unlock(&codec->power_lock);
+ if (i915_shared_power_well)
+ i915_get_power_well(codec->i915_data);
Is it wise that a _get function actually has side effects? Perhaps _push
and _pop or something else would be better semantics.
+
cancel_delayed_work_sync(&codec->power_work);
spin_lock(&codec->power_lock);
With some code at init time to get the i915 symbols you need to call
and whether or not the shared power well is present...
Takashi, any other ideas?
The high level goal here should be for the audio driver to call into
i915 with get/put power well around the sequences where it needs the
power to be up (reading/writing registers, playing audio), but not
across the whole time the driver is loaded, just like you already do
with the powersave work functions, e.g. hda_call_codec_suspend.
I think this sounds about right. The question is how to avoid a
dependency on the i915 driver when it's not necessary, such as when the
HDMI codec is AMD or Nvidia.
The most obvious way to me seems to be to create a new
snd-hda-codec-hdmi-haswell module (that depends on both i915 and
snd-hda-codec-hdmi), and then let that call into i915 and codec-hdmi
drivers as necessary, e g using the set_power_state callback for the
i915 stuff.
But maybe there's something smarter to do here, as I'm not experienced
in mending kernel pieces together :-)
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
