+Rafael. > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > Sent: Wednesday, July 24, 2013 7:02 PM > To: Wang, Xingchao > Cc: Paulo Zanoni; Daniel Vetter; daniel.vetter@xxxxxxxx; > alsa-devel@xxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Girdwood, Liam > R; Jin, Gordon; David Henningsson > Subject: Re: [alsa-devel] [PATCH 0/4 V7] Power-well API > implementation for Haswell > > At Wed, 24 Jul 2013 10:31:22 +0000, > Wang, Xingchao wrote: > > > > Hi Paulo, > > > > Would you help verify attached patch to fix this issue for you? > > > > The patch is based on Takashi's tree, the last commit is: > > commit 9b298cfe296c0f8e088b9ed9a670783a06005e6b > > I think it should be safe to merge into your tree. :) > > > > I tested the patch on Harris Beach, it would let audio driver release > power-well even with charger connected. > > > > Please note maybe this is not the final solution for this issue as it breaks some > rule from user's point of view. > > Well, this patch is NACK from my side. > Sorry, but this is a wrong approach. > > > > Some background of this issue: > > This patch intended to fix power-well regression on Haswell. > > > > On Harris Beach(Ultrabook with battery), there's only eDP panel connected > by default, no HDMI/DP. > > And gfx driver needs enable some HW feature for eDP, power-well *must* > > be disabled in this scenario. > > - Witout charger connected, power-well feature is perfect > > - with charger connected, audio never release power-well. > > > > Basically, power-well should be release if audio driver doesnot use > > it, that's why we enabled runtime power-save feature. > > > > In second case above, with charger connected, the parameter under > > "/sys/devices/../power/control" become "on" always. > > Why this is set to "on" *always*, even if the device can be actually controlled > via runtime PM? Yes, seems it was changed by App through sysfs. I donot' know much about the user space power manager policy, could Rafael share some information? > > > In audio driver side, power_save was set to "0", which disable > > power_down the codecs and controller, thus never release > power->usage_count. > > Why it is set to zero at all? I will continue looking into this change. > > > And this blocks audio driver release power-well. > > > > In the second case, if audio driver detect hdmi pins are free and no > > Apps opened device, it will eanble runtime power-save feature as an > exception. > > > > I test this patch on Harris beach with charger connected, the > > power-well could be released as expected. > > The primary problem is that these flags are set. Yes, I agree. I'm debugging this issue on Ubuntu, not sure it happens on other distribution too. If it's related to Ubuntu, maybe need check Ubuntu power policy. Does anyone know the Ubuntu power-policy on laptop? i.e. when charger connected, will Ubuntu make decision to disable power-save feature for audio subsystem? > > You're trying to implement an "ignore stop-signs on the street if no other cars > are running around you" feature in a new auto-cruise system. It would work, > but it's not what's accepted in general. > (And it makes things complicated and fragile.) > > The real question is why there are two useless STOP signs there. Thanks for clarify this. I also think it's not a good solution to make changes inside audio dirver. :( Donot worry, I will continue to investigate this. Thanks --xingchao > > > thanks, > > Takashi > > > > Thanks > > --xingchao > > > > > -----Original Message----- > > > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > > > Sent: Thursday, July 18, 2013 5:35 PM > > > To: Wang, Xingchao > > > Cc: Daniel Vetter; Paulo Zanoni; daniel.vetter@xxxxxxxx; > > > alsa-devel@xxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Wang > > > xingchao; Girdwood, Liam R; Jin, Gordon; David Henningsson > > > Subject: Re: [alsa-devel] [PATCH 0/4 V7] Power-well API > > > implementation for Haswell > > > > > > At Thu, 18 Jul 2013 09:23:57 +0000, > > > Wang, Xingchao wrote: > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of > > > > > Daniel Vetter > > > > > Sent: Thursday, July 18, 2013 2:44 PM > > > > > To: Wang, Xingchao > > > > > Cc: Paulo Zanoni; daniel.vetter@xxxxxxxx; > > > > > alsa-devel@xxxxxxxxxxxxxxxx; Daniel Vetter; > > > > > intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Wang xingchao; Girdwood, Liam > > > > > R; Jin, Gordon; Takashi Iwai; David Henningsson > > > > > Subject: Re: [alsa-devel] [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@xxxxxxxxxxxxxxxx; Daniel Vetter; > > > > > > > daniel.vetter@xxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; > > > > > > > Wang xingchao; Girdwood, Liam R; Jin, Gordon > > > > > > > Subject: RE: [alsa-devel] [PATCH 0/4 V7] > > > > > > > Power-well API implementation for Haswell > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > > > > > > > > Sent: Wednesday, July 17, 2013 10:22 PM > > > > > > > > To: David Henningsson > > > > > > > > Cc: Paulo Zanoni; Wang, Xingchao; > > > > > > > > alsa-devel@xxxxxxxxxxxxxxxx; Daniel Vetter; > > > > > > > > daniel.vetter@xxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; > > > > > > > > Wang xingchao; Girdwood, Liam R; Jin, Gordon > > > > > > > > Subject: Re: [alsa-devel] [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@xxxxxxxxx>: > > > > > > > > > >>> > > > > > > > > > >>> > > > > > > > > > >>>> -----Original Message----- > > > > > > > > > >>>> From: Takashi Iwai [mailto:tiwai@xxxxxxx] > > > > > > > > > >>>> Sent: Wednesday, July 17, 2013 4:18 PM > > > > > > > > > >>>> To: Wang, Xingchao > > > > > > > > > >>>> Cc: Paulo Zanoni; alsa-devel@xxxxxxxxxxxxxxxx; > > > > > > > > > >>>> Daniel Vetter; daniel.vetter@xxxxxxxx; > > > > > > > > > >>>> intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Wang xingchao; > > > > > > > > > >>>> Girdwood, Liam R; david.henningsson@xxxxxxxxxxxxx > > > > > > > > > >>>> Subject: Re: [alsa-devel] [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@xxxxxxx] > > > > > > > > > >>>>>> Sent: Wednesday, July 17, 2013 3:34 PM > > > > > > > > > >>>>>> To: Wang, Xingchao > > > > > > > > > >>>>>> Cc: Paulo Zanoni; alsa-devel@xxxxxxxxxxxxxxxx; > > > > > > > > > >>>>>> Daniel Vetter; daniel.vetter@xxxxxxxx; > > > > > > > > > >>>>>> intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Wang xingchao; > > > > > > > > > >>>>>> Girdwood, Liam R; david.henningsson@xxxxxxxxxxxxx > > > > > > > > > >>>>>> Subject: Re: [alsa-devel] [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 > > > > > > [2 0001-ALSA-hda-Change-power-save-policy-for-power-well-reg.patch > > <application/octet-stream (base64)>] > > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
