On Thu, 9 May 2013 19:17:36 +0200 Takashi Iwai <tiwai at suse.de> wrote: > At Thu, 9 May 2013 11:23:21 +0200, > Daniel Vetter wrote: > > > > 2. On a quick read of the hda driver stuff I don't think the power > > well enable/disable places are at the right spot: We force the power > > well on whenever the hda codec is used, but we must only do that when > > we want to output audio to external screens. And since that state > > transition can only really happen due to a modeset change on the gfx > > side I don't think we need any autosuspend delays either. > > > > The use-case I'm thinking of is media playback with just the panel > > enabled: In that case we can switch off the power well on the display > > side, and I don't think the power well is required for audio play back > > on the integrated speakerrs/headphone-jack either. > > Well, in the case of Haswell, the audio controller/codec is dedicated > to only HDMI audio, and in the audio driver level, the power saving of > each codec/controller chip is managed individually. So, it's not that > bad to handle power well on/off at that point. A sane power-conscious > OS would open/close the audio device file in a fine grained way. So hda_intel and the azx_suspend/resume functions will only be called for HDMI audio on HSW? If so, then I guess this patch has the calls in the right places. If it's going to force the power well on for non-HDMI audio, then I think we'll need some additional changes. > > > 3. The moduel depencies seem to still not work out dynamically, at > > least I think we still have a hard chain between hda-intel and i915.ko > > (just with one thing in between now). > > True. > > The question is in which level we need power_well_*() controls. > If the initialization of HD-audio controller (e.g. PCI registers) > requires the power well on, hda_intel.c requires the calls, as this > patch does. OTOH, if it's only about the HD-audio verbs, basically we > can push the power well calls into the codec driver, > i.e. patch_hdmi.c. In the latter case, as David once suggested, we > can split the Haswell codec driver from patch_hdmi.c so that only the > new driver depends on i915. I think it's needed for the HDMI related MMIO regs as well, but I don't have a platform to test and make sure. Jesse