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

[Wang, Xingchao]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Wednesday, July 17, 2013 10:22 PM
> To: David Henningsson
> Cc: Paulo Zanoni; Wang, Xingchao; alsa-devel at alsa-project.org; Daniel Vetter;
> daniel.vetter at ffwll.ch; intel-gfx at lists.freedesktop.org; Wang xingchao;
> Girdwood, Liam R; Jin, Gordon
> Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
> implementation for Haswell
> 
> 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.

As I know, the eDP only case doesnot mean no HDMI/DP support. User would plug in HDMI/DP monitor at any time.
So diable audio controller totoally is not a good idea. :(.
Paulo, is that correct for you case? 

--xingchao
> 
> 
> Takashi