[Intel-gfx] [PATCH] drm/i915: Add private api for power well usage

[Wang, Xingchao]

Hi Daniel,

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch]
> Sent: Thursday, April 25, 2013 3:52 PM
> To: Wang, Xingchao
> Cc: Zanoni, Paulo R; ville.syrjala at linux.intel.com; Lin, Mengdong; Girdwood,
> Liam R; Li, Jocelyn; intel-gfx at lists.freedesktop.org;
> alsa-devel at alsa-project.org; Wang Xingchao; Takashi Iwai; Barnes, Jesse;
> Wysocki, Rafael J
> Subject: Re: [PATCH] drm/i915: Add private api for power well usage
> 
> On Thu, Apr 25, 2013 at 4:36 AM, Wang, Xingchao <xingchao.wang at intel.com>
> wrote:
> > Hi Daniel/Paulo,
> >
> > Any comments on this?
> > Add Jesse and Rafael in loop.
> 
> Well I've figured we want to get the power well integration working before we
> waste time reviewing an interface which might or might not quite work. 

Well it's good news you have plan to add power well API support in graphic side, then
it's easy for audio driver to use. I donot know much background about your idea and current status
of the power well integration status there so it's better to avoid duplicated work.


> But if
> you want, I can drop two quick comments:
> - I think the functions needs some prefix like i915_
> - Imo the power well should just be reference counted (the current interface
> name suggests it's generally useable, but actually can only cope with one
> request from the audio driver).

Indeed it was my initial idea to use reference counter to track "power well" users. A second thinking is
It become complex as i915 did not call intel_set_power_well(enable/disable) symmetrically. It may enable
Power well several times but disable it only once. That's why I just use a basic logic to track the "power well" usage.

> - Is the spin_lock (irq-disabling even) really required for the audio driver to work?
> power well enabling can take up to 20ms, so this won't work as-is. Can't we
> just use a mutex?
> - I'd need to see the audio driver side of this to check how this works for coping
> with arbitrary module load ordering.

Thanks your review, i will change it in next versioin patch.

Thanks
--xingchao