[Intel-gfx] [PATCH] drm/i915: VGA also requires the power well

[Zanoni, Paulo R]

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Sunday, August 04, 2013 4:50 PM
> To: Paulo Zanoni
> Cc: Ville Syrjälä; intel-gfx at lists.freedesktop.org; Zanoni, Paulo R
> Subject: Re: [Intel-gfx] [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 at linux.intel.com>:
> > > On Wed, Jun 05, 2013 at 06:05:51PM -0300, Paulo Zanoni wrote:
> > >> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > >>
> > >> 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 at intel.com>
> > >> ---
> > >>  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. 


> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch