On Mon, Aug 12, 2013 at 06:06:48PM +0000, Zanoni, Paulo R wrote: > > -----Original Message----- > > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel > > Vetter > > Sent: Sunday, August 04, 2013 4:50 PM > > To: Paulo Zanoni > > Cc: Ville Syrjälä; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Zanoni, Paulo R > > Subject: Re: [PATCH] drm/i915: VGA also requires the power well > > > > On Fri, Aug 02, 2013 at 02:17:53PM -0300, Paulo Zanoni wrote: > > > 2013/6/6 Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>: > > > > On Wed, Jun 05, 2013 at 06:05:51PM -0300, Paulo Zanoni wrote: > > > >> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > > >> > > > >> So add a power domain and check for it before we try to read > > > >> VGA_CONTROL. > > > >> > > > >> This fixes unclaimed register messages that happen on > > suspend/resume. > > > >> > > > >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > > >> --- > > > >> drivers/gpu/drm/i915/i915_drv.h | 1 + > > > >> drivers/gpu/drm/i915/intel_display.c | 3 +++ > > > >> drivers/gpu/drm/i915/intel_pm.c | 1 + > > > >> 3 files changed, 5 insertions(+) > > > >> > > > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > > >> index 46b1f70..d51ce13 100644 > > > >> --- a/drivers/gpu/drm/i915/i915_drv.h > > > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > > > >> @@ -89,6 +89,7 @@ enum port { > > > >> #define port_name(p) ((p) + 'A') > > > >> > > > >> enum intel_display_power_domain { > > > >> + POWER_DOMAIN_VGA, > > > >> POWER_DOMAIN_PIPE_A, > > > >> POWER_DOMAIN_PIPE_B, > > > >> POWER_DOMAIN_PIPE_C, > > > >> diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > > >> index 4c8fcec..3719d99 100644 > > > >> --- a/drivers/gpu/drm/i915/intel_display.c > > > >> +++ b/drivers/gpu/drm/i915/intel_display.c > > > >> @@ -9950,6 +9950,9 @@ void i915_redisable_vga(struct drm_device > > *dev) > > > >> struct drm_i915_private *dev_priv = dev->dev_private; > > > >> u32 vga_reg = i915_vgacntrl_reg(dev); > > > >> > > > >> + if (!intel_display_power_enabled(dev, POWER_DOMAIN_VGA)) > > > >> + return; > > > >> + > > > > > > > > So it looks like you're essentially making intel_redisable_vga() a nop > > > > for HSW. Shouldn't we instead enable the power well during resume? > > > > > > We enable the power well during resume, but it's only after this function... > > > > Hm, so better move the (temporary) power well enabling in the resume > > code > > up a bit to cover this? > > I don't think so. If you look at i915_redisable_vga and commit > 0fde901f1ddd2ce0e380a6444f1fb7ca555859e9, you will start thinking that > just closing/opening the lid could make the BIOS somehow enable the > power well and then reenable VGA, so moving the check to "earlier in the > resume sequence" won't solve any problems, as VGA could be reenabled > later. Well that's only for machines without opregion afaik. But we lack the check for that, which I didn't really realize. To avoid blocking this even longer I've just merged this patch here with a big note added to the commit message. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
