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. > > 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. thanks, Takashi
