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

Thu Jul 18 11:34:32 CEST 2013

At Thu, 18 Jul 2013 09:23:57 +0000,
Wang, Xingchao wrote:
> 
> 
> 
> > -----Original Message-----
> > From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
> > Sent: Thursday, July 18, 2013 2:44 PM
> > To: Wang, Xingchao
> > Cc: Paulo Zanoni; daniel.vetter at ffwll.ch; alsa-devel at alsa-project.org; Daniel
> > Vetter; intel-gfx at lists.freedesktop.org; Wang xingchao; Girdwood, Liam R; Jin,
> > Gordon; Takashi Iwai; David Henningsson
> > Subject: Re: [alsa-devel] [Intel-gfx] [PATCH 0/4 V7] Power-well API
> > implementation for Haswell
> > 
> > On Thu, Jul 18, 2013 at 01:00:07AM +0000, Wang, Xingchao wrote:
> > > Hi Paulo/Daniel,
> > >
> > > Do you agree to export an API in gfx side for eDP case?
> > > Basically the api will let audio drver know which pipe in use. i.e. in
> > > the eDP only caes, audio driver Will know gfx is not using HDMI/DP and would
> > like to let power-well off.
> > > As there's the conflict when user expect display audio driver always active but
> > gfx need audio driver off.
> > > Audio driver could make decision to release power-well if it knows the eDP
> > only case through the API.
> > >
> > > OTOH, I think audio driver could also export an API for gfx side, if
> > > gfx driver need audio driver release power-well but it's in usage, It will call
> > this API and audio drvier will release power-well accordingly.
> > >
> > > This change make HDMI/DP hotplug handling complicated in audio driver side,
> > if audio driver release power-well, it would enter suspend mode.
> > > Meanwhile the user may expect it's in active mode, this may cause some
> > confuse.
> > 
> > Afaik (and I know very little about audio) the audio side already knows which
> > pipes have audio enabled, since we set the respective bit only when it's needed.
> > And audio will receive the unsolicited even interrupt (or whatever it's called)
> > when this happens.
> > 
> For haswell, Audio driver can get DDI port/Pin usage information according to the unsolicited event, not Pipe info.
> However at this stage, seems only that is enough: if no pin has valid ELD, audio driver can think about that no monitor connected with DDI ports.
> In this case, Audio driver could release power-well and enter suspend mode automatically, this avoid blocking eDP feature enabling. And once gfx dirver
> Detect external monitor connected, it will also wake up audio driver.
> 
> Takashi, do you think this solution acceptable?

It's the current situation, isn't it?  So the question is only whether
this works _as expected_.

Basically system/user needs to set up two parameters for the audio
power-saving.  If both are set well, but it still doesn't work, we
need to debug.

Of course, we can improve things, for example, the default runtime PM
setup.  (Note that this is about the default value, not the value
force to set.)

Takashi

> 
> Thanks
> --xingchao
>  
> > So I think we already have the means (albeit with that quirky hw interface, but
> > it seems to have been good enough for a long time already) to do that. Or do I
> > miss something?
> > -Daniel
> > 
> > >
> > > Thanks
> > > --xingchao
> > >
> > > > -----Original Message-----
> > > > From: Wang, Xingchao
> > > > Sent: Thursday, July 18, 2013 7:18 AM
> > > > To: 'Takashi Iwai'; David Henningsson; Paulo Zanoni
> > > > Cc: 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
> > > >
> > > >
> > > >
> > > > > -----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
> > 
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 