From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: HSW: allow PCH clock gating for suspend Date: Tue, 16 Apr 2013 14:35:13 +0300 Message-ID: <20130416113513.GH4469@intel.com> References: <1366111516-27448-1-git-send-email-imre.deak@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id 9C0A6E6407 for ; Tue, 16 Apr 2013 04:35:17 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1366111516-27448-1-git-send-email-imre.deak@intel.com> 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: Imre Deak Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Apr 16, 2013 at 02:25:16PM +0300, Imre Deak wrote: > For the device to enter D3 we should enable PCH clock gating. > = > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_drv.c | 2 ++ > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_display.c | 5 +++++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_pm.c | 18 ++++++++++++++++++ > 5 files changed, 27 insertions(+) > = > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_= drv.c > index bddb9a5..e9a82f1 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -521,6 +521,8 @@ static int i915_drm_freeze(struct drm_device *dev) > */ > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) > dev_priv->display.crtc_disable(crtc); > + > + intel_modeset_suspend_hw(dev); > } > = > i915_save_state(dev); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_= drv.h > index b5a495a..e549e6c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1831,6 +1831,7 @@ static inline void intel_unregister_dsm_handler(voi= d) { return; } > = > /* modesetting */ > extern void intel_modeset_init_hw(struct drm_device *dev); > +extern void intel_modeset_suspend_hw(struct drm_device *dev); > extern void intel_modeset_init(struct drm_device *dev); > extern void intel_modeset_gem_init(struct drm_device *dev); > extern void intel_modeset_cleanup(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index 457a0a0..e9192bf 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8984,6 +8984,11 @@ void intel_modeset_init_hw(struct drm_device *dev) > mutex_unlock(&dev->struct_mutex); > } > = > +void intel_modeset_suspend_hw(struct drm_device *dev) > +{ > + intel_suspend_hw(dev); > +} > + Why this extra level of indirection? > void intel_modeset_init(struct drm_device *dev) > { > struct drm_i915_private *dev_priv =3D dev->dev_private; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/inte= l_drv.h > index d7bd031..8b29897 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -652,6 +652,7 @@ extern void assert_pipe(struct drm_i915_private *dev_= priv, enum pipe pipe, > #define assert_pipe_disabled(d, p) assert_pipe(d, p, false) > = > extern void intel_init_clock_gating(struct drm_device *dev); > +extern void intel_suspend_hw(struct drm_device *dev); > extern void intel_write_eld(struct drm_encoder *encoder, > struct drm_display_mode *mode); > extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel= _pm.c > index baea4fc..3567881 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3735,6 +3735,18 @@ static void lpt_init_clock_gating(struct drm_devic= e *dev) > PCH_LP_PARTITION_LEVEL_DISABLE); > } > = > +static void lpt_allow_clock_gating(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + > + if (dev_priv->pch_id =3D=3D INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) { > + uint32_t val =3D I915_READ(SOUTH_DSPCLK_GATE_D); > + > + val &=3D ~PCH_LP_PARTITION_LEVEL_DISABLE; > + I915_WRITE(SOUTH_DSPCLK_GATE_D, val); > + } So who sets it back when we resume? > +} > + > static void haswell_init_clock_gating(struct drm_device *dev) > { > struct drm_i915_private *dev_priv =3D dev->dev_private; > @@ -4085,6 +4097,12 @@ void intel_init_clock_gating(struct drm_device *de= v) > dev_priv->display.init_clock_gating(dev); > } > = > +void intel_suspend_hw(struct drm_device *dev) > +{ > + if (IS_HASWELL(dev)) > + lpt_allow_clock_gating(dev); Do you need the HSW check? Isn't the LPT LP check enough? > +} > + > void intel_set_power_well(struct drm_device *dev, bool enable) > { > struct drm_i915_private *dev_priv =3D dev->dev_private; > -- = > 1.7.10.4 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC