[Intel-gfx] [PATCH 1/2] drm/i915: check for power well in redisable VGA with generic fn

[Jesse Barnes]

On Fri, 11 Oct 2013 17:16:55 -0300
Paulo Zanoni <przanoni at gmail.com> wrote:

> 2013/10/11 Jesse Barnes <jbarnes at virtuousgeek.org>:
> > Rather than using a HSW specific check.
> >
> > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 53c4ea8..5452b34 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10667,8 +10667,7 @@ void i915_redisable_vga(struct drm_device *dev)
> >          * level, just check if the power well is enabled instead of trying to
> >          * follow the "don't touch the power well if we don't need it" policy
> >          * the rest of the driver uses. */
> > -       if (HAS_POWER_WELL(dev) &&
> > -           (I915_READ(HSW_PWR_WELL_DRIVER) & HSW_PWR_WELL_STATE_ENABLED) == 0)
> > +       if (!intel_display_power_enabled(dev, POWER_DOMAIN_VGA))
> 
> See the big comment. With your patch we're not only going to return in
> case the power well is disabled, we're going to return in case it's
> disabled _or_ we promised to not touch it (even if it's enabled). I
> originally made the same mistake, then Ville spotted it on the code
> review. This function should assume the BIOS is being an idiot and is
> trying to mess with us.
> 
> I admit I don't like it, suggestions to further improve this code are
> welcome. But it seems no one really knows what's the best and safest
> thing to do here :(

Hm I'm not seeing it.  For a simple status check we don't depend on the
power_well struct at all, we just look at the bits.

In this case do we need to explicitly ignore the
HSW_PWR_WELL_ENABLE_REQUEST bit or something?

-- 
Jesse Barnes, Intel Open Source Technology Center