[Intel-gfx] [alsa-devel] [PATCH 0/4 V7] Power-well API implementation for Haswell

[Takashi Iwai]

At Wed, 17 Jul 2013 16:05:43 +0200,
David Henningsson wrote:
> 
> On 07/17/2013 04:00 PM, Takashi Iwai wrote:
> > At Wed, 17 Jul 2013 10:31:26 -0300,
> > Paulo Zanoni wrote:
> >>
> >> 2013/7/17 Wang, Xingchao <xingchao.wang at intel.com>:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Takashi Iwai [mailto:tiwai at suse.de]
> >>>> Sent: Wednesday, July 17, 2013 4:18 PM
> >>>> To: Wang, Xingchao
> >>>> Cc: Paulo Zanoni; alsa-devel at alsa-project.org; Daniel Vetter;
> >>>> daniel.vetter at ffwll.ch; intel-gfx at lists.freedesktop.org; Wang xingchao;
> >>>> Girdwood, Liam R; david.henningsson at canonical.com
> >>>> Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
> >>>> implementation for Haswell
> >>>>
> >>>> At Wed, 17 Jul 2013 08:03:38 +0000,
> >>>> Wang, Xingchao wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Takashi Iwai [mailto:tiwai at suse.de]
> >>>>>> Sent: Wednesday, July 17, 2013 3:34 PM
> >>>>>> To: Wang, Xingchao
> >>>>>> Cc: Paulo Zanoni; alsa-devel at alsa-project.org; Daniel Vetter;
> >>>>>> daniel.vetter at ffwll.ch; intel-gfx at lists.freedesktop.org; Wang
> >>>>>> xingchao; Girdwood, Liam R; david.henningsson at canonical.com
> >>>>>> Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
> >>>>>> implementation for Haswell
> >>>>>>
> >>>>>> At Wed, 17 Jul 2013 02:52:41 +0000,
> >>>>>> Wang, Xingchao wrote:
> >>>>>>>
> >>>>>>> Hi Takashi/Paulo,
> >>>>>>>
> >>>>>>>>>> would you change it to "auto" and test again.
> >>>>>>>>>> Runtime power save should be enabled with "auto".
> >>>>>>>>>
> >>>>>>>>> Doesn't solve the problem. Should I open a bug report somewhere?
> >>>>>>>>> Having the power well enabled prevents some power saving
> >>>>>>>>> features from the graphics driver.
> >>>>>>>>
> >>>>>>>> Is the HD-audio power-saving itself working?  You can check it
> >>>>>>>> via watching /sys/class/hwC*/power_{on|off}_acct files.
> >>>>>>>>
> >>>>>>>> power_save option has to be adjusted appropriately.  Note that
> >>>>>>>> many DEs change this value dynamically per AC-cable plug/unplug
> >>>>>>>> depending on the configuration, and often it's set to 0 (= no
> >>>>>>>> power save) when AC-cable is plugged.
> >>>>>>>>
> >>>>>>> [Wang, Xingchao] Paulo used a new Ultrabook board with charger
> >>>>>>> connected,
> >>>>>> and see the default parameter "auto=on".
> >>>>>>> In such scenario, power-well is always occupied by Display audio
> >>>>>>> controller. Moreover, in this board, if no external monitors
> >>>>>>> connected, It's
> >>>>>> using internal eDP and totally no audio support. Power-well usage in
> >>>>>> such case also blocks some eDP features as Paulo told me.
> >>>>>>>
> >>>>>>> So I think it's not a good idea to break the rule of power policy
> >>>>>>> when charger
> >>>>>> connected but it's necessary to add support in this particular case.
> >>>>>>> Takashi, do you think it's acceptable to let Display audio
> >>>>>>> controller/codec
> >>>>>> suspend in such case?
> >>>>>>
> >>>>>> Do you mean the driver enables the powersave forcibly?
> >>>>>
> >>>>> Yes. I mean call pm_runtime_allow() for the power-well HD-A controller.
> >>>>>
> >>>>>> Then, no, not in general.
> >>>>>>
> >>>>>> If the default parameter of autopm is the problem, this should be
> >>>>>> changed, instead of forcing the policy in the driver.
> >>>>>>
> >>>>>> OTOH, the audio codec's powersave policy is governed by the
> >>>>>> power_save option and it's set up dynamically by the desktop system.
> >>>>>> We shouldn't override it in the driver.
> >>>>>>
> >>>>>> If the power well *must* be off when only an eDP is used (e.g.
> >>>>>> otherwise the hardware doesn't work properly), then it's a
> >>>>>> different story.  Is it the case?   And what exactly would be the
> >>>>>> problem?
> >>>>> In the eDP only case, no audio is needed for the HD-A controller, so it's
> >>>> wasting power in current design.
> >>>>> I think Paulo or Daniel could explain more details on the impact.
> >>>>
> >>>> Consuming more power is the expected behavior.  What else?
> >>>>
> >>>>
> >>>>>> If it's the case, controlling the power well based on the runtime PM
> >>>>>> is likely a bad design, as it relies on the parameter user sets.
> >>>>>> (And remember that the power-saving of the audio can be disabled
> >>>>>> completely via Kconfig, too.)
> >>>>>  From audio controller's point of view, if it's asked be active, it needs power
> >>>> and should request power-well from gfx side.
> >>>>> In above case, audio controller should not be active but user set it be
> >>>> "active".
> >>>>
> >>>> By setting the autopm "on", user expects that no runtime PM happens.
> >>>> In other words, the audio controller must be kept active as long as this
> >>>> parameter is set.  And this is the parameter user controls, and not what the
> >>>> driver forcibly sets.
> >>>
> >>> Okay, become clear now. :)
> >>> So I think the conflict for Paulo becomes, in eDP caes, if audio is active and requested power-well, some eDP feature was under impact?
> >>> Paulo, would you clarify this in more details?
> >>
> >> On our driver we try to disable the power well whenever possible, as
> >> soon as possible. We don't change our behavior based on power AC or
> >> other user-space modifiable behavior (except the
> >> i915.disable_power_well Kernel option). If the power well is not
> >> disabled we can't enable some features, like PSR (panel self refresh,
> >> and eDP feature) or PC8, which is another power-saving feature. This
> >> will also make our QA procedures a lot more complex since when we want
> >> to test specific features (e.g., PSR, PC8) we'll have to disconnect
> >> the AC adapter or run scripts. So the behavior/predictability of our
> >> driver will be based on the Audio driver power management policies.
> >
> > So all missing feature are about the power saving?
> >
> >> I am not so experienced with general Linux Power Management code, so
> >> maybe the way the Audio driver is behaving is just the usual way, but
> >> I have to admit I was expecting the audio driver would only require
> >> the power well when it is actually needed, and release it as soon as
> >> possible.
> >
> > It would behave so, if all setups are for power-saving.
> >
> > But, in your case, the runtime PM control attribute shows "on"; it
> > implies that the runtime PM is effectively disabled, thus disabling
> > power well is also impossible (because it would require turning off
> > the audio controller, too).
> 
> So, if the machine only has an eDP (which has no audio function in 
> itself, right?) and never HDMI, DP output because there are no such 
> physical ports, the audio controller has no function.
> Maybe we can, before doing anything else, ask the video driver first if 
> this is the case, and if so, never create the sound card at all, and 
> just leave things the way the video driver wants?

Well, doesn't BIOS mark HDMI/DP audio pins as unused?  Then the audio
driver won't create any instances.  Of course, we can optimize such a
case, indeed.


Takashi