On Fri, 11 Oct 2013 17:16:55 -0300 Paulo Zanoni <przanoni@xxxxxxxxx> wrote: > 2013/10/11 Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>: > > Rather than using a HSW specific check. > > > > Signed-off-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> > > --- > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx