On Fri, Apr 26, 2013 at 05:45:15PM +0200, Takashi Iwai wrote: > At Fri, 26 Apr 2013 17:42:07 +0200, > Daniel Vetter wrote: > > > > On Fri, Apr 26, 2013 at 05:12:34PM +0200, Takashi Iwai wrote: > > > At Fri, 26 Apr 2013 16:57:08 +0200, > > > Daniel Vetter wrote: > > > > > > > > On Fri, Apr 26, 2013 at 07:53:41AM +0000, Li, Jocelyn wrote: > > > > > -----Original Message----- > > > > > From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] > > > > > Sent: Friday, April 26, 2013 3:25 PM > > > > > To: Li, Jocelyn > > > > > Cc: Wang, Xingchao; Zanoni, Paulo R; ville.syrjala at linux.intel.com; Lin, Mengdong; Girdwood, Liam R; intel-gfx at lists.freedesktop.org; alsa-devel at alsa-project.org; Wang Xingchao; Takashi Iwai; Barnes, Jesse; Wysocki, Rafael J; Hindman, Gavin > > > > > Subject: Re: [PATCH] drm/i915: Add private api for power well usage -- alignment between graphic team and audio team > > > > > Once we've figured out what needs to be changed in the audio driver we > > > > > can look at the entire patch series and the interface to i915.ko. > > > > > That's also why I didn't comment on Xingchao's patch right away, but > > > > > only once he specifically asked for feedback, since doing a real review > > > > > of the interface doesn't make sense until we have all the pieces (and a > > > > > working audio driver). > > > > > > > > > > [Jocelyn] I think you have made constructive comments on Xingchao's > > > > > patch yesterday. Next, shall we have Xingchao improve his patch? Or we > > > > > just have Xingchao wait till you have completed your pieces. Sorry, I am > > > > > a little confused :) > > > > > > > > I think the next step is to use Xinchao's patch as-is and get the audio > > > > side going. Once we have that fixed up, we can revisit the interface and > > > > make it solid. But for now trying to polish this relatively simple part > > > > seems like wasted time. > > > > > > > > Also, reviewing an interface is much easier if we have both the i915 > > > > pieces (already here) and the audio pieces (which I haven't seen yet) > > > > avaialble. > > > > > > I haven't checked the patch properly yet, but the patch pasted in the > > > post looks like i915 driver exports the functions to control power > > > well, and let audio driver calling them. If so, the big mess in such > > > a case is the dependency between driver modules. > > > > > > A simple workaround would be to split this function and the relevant > > > instance out of i915 and snd-hda-intel and put into an individual > > > module (e.g. i915-powerwell-ctl). > > > > Yeah, the current patch doesn't work for loading i915/snd-hda-intel in any > > order. But it should be good enough to fix the hw interactions. > > Well, my current concern is that the call is built into snd-hda-intel > module and this will lead to load i915 module no matter whether it's > used or not. snd-hda-intel is used for any HD-audio controllers > (despite its name). Loading i915 on a pure AMD system will annoy some > non-Intel people :) > > Even if we built the code via ifdef CONFIG_DRM_I915 or such, it's > still hardcoded, thus the problem above will occur with distro > kernels. Sure, the current patch won't cut it for upstreaming. But it should be good enough to figure out what exactly we need (which to still be a bit unclear). Or at least where exactly we need it and what kind of other changes in snd-hda-intel are required to get things going. > > > Also, it would be feasible to implement some PM governor over both > > > graphics and audio, that is, it gives the proper serialization of > > > power up for audio controller, for example. > > > > Yeah, I think once we have the hw interaction/sequence and stuff all > > figured out we need to figure out what kind of interface would suit best > > here. One of the options we've talked about a bit is a full runtime PM > > governor on a special device. Then only the device needs to be > > instantiated first, the audio driver could just grab runtime pm references > > and i915 could implement the PM backend. > > > > But like I've said, those considerations should be a second step once we > > have something working with a quickly hacked-together interface. > > Yep, I agree that we'll need a quick fix at this stage. power well stuff is disabled by default, 3.10 is kinda done already and we still have plenty of time to hit anything for 3.11. So I don't think we should rush the solution for upstream before it's clear what snd-hda-intel precisely needs. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
