From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: VGA also requires the power well Date: Thu, 6 Jun 2013 17:47:20 +0300 Message-ID: <20130606144720.GN5004@intel.com> References: <1370466351-3683-1-git-send-email-przanoni@gmail.com> <20130606083804.GM5004@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id CCD63E6003 for ; Thu, 6 Jun 2013 07:47:23 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Paulo Zanoni Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Thu, Jun 06, 2013 at 11:35:15AM -0300, Paulo Zanoni wrote: > 2013/6/6 Ville Syrj=E4l=E4 : > > On Wed, Jun 05, 2013 at 06:05:51PM -0300, Paulo Zanoni wrote: > >> From: Paulo Zanoni > >> > >> 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 > >> --- > >> 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/i9= 15_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/i9= 15/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 =3D dev->dev_private; > >> u32 vga_reg =3D 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. > = > It's not a nop for HSW, it's only a nop if the power well is disabled, > which means VGA is disabled, so it's a nop if VGA is disabled. But if > you look at the current function it's also a nop if VGA is disabled. > So we're keeping the same behavior, but checking the power well before > checking vga_reg. > = > VGA mode requires the power well to be enabled, we can be sure that if > the power well is disabled, then VGA is disabled, so you don't need to > do the check for VGA_DISP_DISABLE. Rigth, but intel_display_power_enabled() only checks the driver power well register. If BIOS can leave VGA enabled, then I guess it could've left the power well on too. So I'm thinking we should check for that rather than the what the driver requested. -- = Ville Syrj=E4l=E4 Intel OTC