On Wed, Apr 17, 2013 at 11:04:23AM +0200, Daniel Vetter wrote: > Hi Paulo, > > On Fri, Mar 22, 2013 at 02:14:13PM -0300, Paulo Zanoni wrote: > > From: Paulo Zanoni <paulo.r.zanoni at intel.com> > > > > It returns true if we've requested to turn the power well on and it's > > really on. It also returns true for all the previous gens. > > > > For now there's just one caller, but I'm going to add more. > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com> > > Yeah, I've merged this but just stumbled over it again while rebasing the > -internal tree. And I'm still unhappy with the name a bit, since > power_well is a bit generic. I know it's what bspec uses, but still I'd > like to have some notion in it that this is about display stuff > > The other thing which always irked me is that sprinkling power wells > checks all over the place just feels ugly. What we actually want to check > is whether the display hw is powered on, which feels much less > platform-specific. > > So what about a s/intel_using_power_well/intel_display_power_enabled? It's > not perfect since the actual piece of hw we care about is still platform > specific, so I'd suggest to throw an enum on top: > > enum intel_display_power_domains { > POWER_DOMAIN_EDP, > POWER_DOMAIN_EDP_PFIT, > POWER_DOMAIN_OTHER > }; > > bool intel_display_power_enabled(struct drm_device *dev, > enum intel_display_power_domain domain); > > We could easily add new domains for e.g. the pc8 stuff with a > POWER_DOMAIN_CONNECTOR_AUX or so if we need to work around more unclaimed > register warnings. > > With that piece of infrastructure I think I'll stop being grumpy about > power wells checks and unclaimed register fixup patches and just merge > them all. Also, that function should probably use HAS_POWER_WELL instead of the manual IS_HASWELL check. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch