Re: [PATCH 3/7] drm/i915: add intel_using_power_well

[Daniel Vetter <daniel@ffwll.ch>]

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@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@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