From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH] drm/i915: fix up ilk rc6 disabling confusion Date: Sat, 30 Jun 2012 20:04:44 -0700 Message-ID: <20120630200444.71e4b92d@bwidawsk.net> References: <1341005536-5324-1-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id 19E529E754 for ; Sat, 30 Jun 2012 20:06:41 -0700 (PDT) In-Reply-To: <1341005536-5324-1-git-send-email-daniel.vetter@ffwll.ch> 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: Daniel Vetter Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Fri, 29 Jun 2012 23:32:16 +0200 Daniel Vetter wrote: > While creating the new enable/disable_gt_powersave functions in > > commit 8090c6b9daa04dda649ac0a2209601042abfb0a4 > Author: Daniel Vetter > Date: Sun Jun 24 16:42:32 2012 +0200 > > drm/i915: wrap up gt powersave enabling functions > > I've botched up the handling of ironlake_disable_rc6. Fix this up by > calling it at the right place. Note though that ironlake_disable_rc6 > does a bit more than just disabling rc6 - it also tears down all the > allocated context objects. I can only hope that one day you will appreciate the API I set up in the context code to handle this. > > Hence we need to move intel_teardown_rc6 out and directly call it from > intel_modeset_cleanup. I don't mind that you've done this, however could you explain/modify the comment when we want to disable without destroying the backing context object? > > Also properly mark ironlake_enable_rc6 as static and kill the un-used > declaration in i915_drv.h. > > Cc: Ben Widawsky > Signed-Off-by: Daniel Vetter Reviewed-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_drv.h | 1 - > drivers/gpu/drm/i915/intel_display.c | 2 ++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_pm.c | 13 +++++++------ > 4 files changed, 10 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a0c15ab..6c077e9 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1510,7 +1510,6 @@ extern bool intel_fbc_enabled(struct drm_device *dev); > extern void intel_disable_fbc(struct drm_device *dev); > extern bool ironlake_set_drps(struct drm_device *dev, u8 val); > extern void ironlake_init_pch_refclk(struct drm_device *dev); > -extern void ironlake_enable_rc6(struct drm_device *dev); > extern void gen6_set_rps(struct drm_device *dev, u8 val); > extern void intel_detect_pch(struct drm_device *dev); > extern int intel_trans_dp_port_sel(struct drm_crtc *crtc); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 3fbc802..717b553 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -7270,6 +7270,8 @@ void intel_modeset_cleanup(struct drm_device *dev) > > intel_disable_gt_powersave(dev); > > + ironlake_teardown_rc6(dev); > + > if (IS_VALLEYVIEW(dev)) > vlv_init_dpio(dev); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index cc1573b..db226c5 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -493,6 +493,7 @@ extern void intel_gpu_ips_teardown(void); > > extern void intel_enable_gt_powersave(struct drm_device *dev); > extern void intel_disable_gt_powersave(struct drm_device *dev); > +extern void ironlake_teardown_rc6(struct drm_device *dev); > > extern void intel_ddi_dpms(struct drm_encoder *encoder, int mode); > extern void intel_ddi_mode_set(struct drm_encoder *encoder, > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 99bc1f3..ec25eca 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2547,7 +2547,7 @@ static void gen6_update_ring_freq(struct drm_device *dev) > } > } > > -static void ironlake_teardown_rc6(struct drm_device *dev) > +void ironlake_teardown_rc6(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > @@ -2564,7 +2564,7 @@ static void ironlake_teardown_rc6(struct drm_device *dev) > } > } > > -void ironlake_disable_rc6(struct drm_device *dev) > +static void ironlake_disable_rc6(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > @@ -2580,8 +2580,6 @@ void ironlake_disable_rc6(struct drm_device *dev) > I915_WRITE(RSTDBYCTL, I915_READ(RSTDBYCTL) & ~RCX_SW_EXIT); > POSTING_READ(RSTDBYCTL); > } > - > - ironlake_teardown_rc6(dev); > } > > static int ironlake_setup_rc6(struct drm_device *dev) > @@ -2603,7 +2601,7 @@ static int ironlake_setup_rc6(struct drm_device *dev) > return 0; > } > > -void ironlake_enable_rc6(struct drm_device *dev) > +static void ironlake_enable_rc6(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_ring_buffer *ring = &dev_priv->ring[RCS]; > @@ -3225,8 +3223,11 @@ static void intel_init_emon(struct drm_device *dev) > > void intel_disable_gt_powersave(struct drm_device *dev) > { > - if (IS_IRONLAKE_M(dev)) > + if (IS_IRONLAKE_M(dev)) { > ironlake_disable_drps(dev); > + ironlake_disable_rc6(dev); > + } > + > if (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev)) > gen6_disable_rps(dev); > } -- Ben Widawsky, Intel Open Source Technology Center