From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 3/4] drm/i915: make sure VDD is turned off during system suspend Date: Tue, 12 Aug 2014 15:58:30 +0300 Message-ID: <1407848310.11068.14.camel@intelbox> References: <1407783891-9406-1-git-send-email-imre.deak@intel.com> <20140812123447.GY4193@intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0194111019==" Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 3BEFC6E05A for ; Tue, 12 Aug 2014 05:58:33 -0700 (PDT) In-Reply-To: <20140812123447.GY4193@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ville =?ISO-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0194111019== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-4SERTeX6WUStCGjrCKfn" --=-4SERTeX6WUStCGjrCKfn Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2014-08-12 at 15:34 +0300, Ville Syrj=C3=A4l=C3=A4 wrote: > On Mon, Aug 11, 2014 at 10:04:50PM +0300, Imre Deak wrote: > > Atm we may leave eDP VDD enabled during system suspend after the CRTCs > > are disabled through an HPD->DPCD read event. So disable VDD during > > suspend at a point when no HPDs can occur. >=20 > And vdd itself holds a runtime pm ref so this problem can't occur there. Yep, I'll update the commit message. > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++++++++ > > drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++++-- > > drivers/gpu/drm/i915/intel_drv.h | 6 ++++++ > > 3 files changed, 32 insertions(+), 2 deletions(-) > >=20 > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i91= 5_drv.c > > index 0653761..1c7979e 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -501,6 +501,19 @@ void intel_hpd_cancel_work(struct drm_i915_private= *dev_priv) > > cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work); > > } > > =20 > > +static void intel_suspend_encoders(struct drm_i915_private *dev_priv) > > +{ > > + struct drm_device *dev =3D dev_priv->dev; > > + struct intel_encoder *intel_encoder; > > + > > + drm_modeset_lock_all(dev); > > + for_each_intel_encoder(dev, intel_encoder) { > > + if (intel_encoder->suspend) > > + intel_encoder->suspend(intel_encoder); > > + } > > + drm_modeset_unlock_all(dev); > > +} > > + > > static int i915_drm_freeze(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv =3D dev->dev_private; > > @@ -547,6 +560,8 @@ static int i915_drm_freeze(struct drm_device *dev) > > intel_runtime_pm_disable_interrupts(dev); > > intel_hpd_cancel_work(dev_priv); > > =20 > > + intel_suspend_encoders(dev_priv); > > + > > intel_suspend_gt_powersave(dev); > > =20 > > intel_modeset_suspend_hw(dev); > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/int= el_dp.c > > index 34e3c47..d7f5d0a 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -1291,8 +1291,6 @@ static void edp_panel_vdd_off(struct intel_dp *in= tel_dp, bool sync) > > if (!is_edp(intel_dp)) > > return; > > =20 > > - WARN(!intel_dp->want_panel_vdd, "eDP VDD not forced on"); > > - >=20 > Should we perhaps keep this warn here and call the _sync() version direct= ly > from ->suspend()? I think we already do it that way when tearing down > the encoder. Ok. > > intel_dp->want_panel_vdd =3D false; > > =20 > > if (sync) > > @@ -4003,6 +4001,16 @@ void intel_dp_encoder_destroy(struct drm_encoder= *encoder) > > kfree(intel_dig_port); > > } > > =20 > > +void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder) > > +{ > > + struct intel_dp *intel_dp =3D enc_to_intel_dp(&intel_encoder->base); > > + > > + if (!is_edp(intel_dp)) > > + return; >=20 > edp_panel_vdd_off() already has the same check, but if you convert it > to use the _sync() directly then the check is needed. >=20 > > + > > + edp_panel_vdd_off(intel_dp, true); > > +} > > + > > static void intel_dp_encoder_reset(struct drm_encoder *encoder) > > { > > intel_edp_panel_vdd_sanitize(to_intel_encoder(encoder)); > > @@ -4722,6 +4730,7 @@ intel_dp_init(struct drm_device *dev, int output_= reg, enum port port) > > intel_encoder->disable =3D intel_disable_dp; > > intel_encoder->get_hw_state =3D intel_dp_get_hw_state; > > intel_encoder->get_config =3D intel_dp_get_config; > > + intel_encoder->suspend =3D intel_dp_encoder_suspend; > > if (IS_CHERRYVIEW(dev)) { > > intel_encoder->pre_pll_enable =3D chv_dp_pre_pll_enable; > > intel_encoder->pre_enable =3D chv_pre_enable_dp; > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/in= tel_drv.h > > index 1b3d1d7..cefd337 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -153,6 +153,12 @@ struct intel_encoder { > > * be set correctly before calling this function. */ > > void (*get_config)(struct intel_encoder *, > > struct intel_crtc_config *pipe_config); > > + /* > > + * Called during system suspend after all pending requests for the > > + * encoder are flushed (for example for DP AUX transactions) and > > + * device interrupts are disabled. > > + */ > > + void (*suspend)(struct intel_encoder *); > > int crtc_mask; > > enum hpd_pin hpd_pin; > > }; > > --=20 > > 1.8.4 > >=20 > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >=20 --=-4SERTeX6WUStCGjrCKfn Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) iQEcBAABAgAGBQJT6g92AAoJEORIIAnNuWDFT3AIANTCRGbk0JNHJ0SSoymS52RO NxXQh4u01NwZ8c3dOk+gmK6MzAow2VcEjlLAN9fh6NSfzZ1OkK6VPe6ug5vquN9G Wq9finFVk/HNO0sTvYGLgyG4bBsrlJ2UTBMV4vxo3AV+ITL/BurCoIfBvZU88eZK tVFary3QPrnt0eOCClvSHDsWuQety96AJ0EzZtvnKBwhveLTghpMl0+iU+teovn5 AeunYLGYhBa9XrpyRiPjAoCP4uE+W2Er+ItAfb7ARvvZuj0tnh0fAO8g9S6xSpEO hnvCw+5PisMYyEcTSmFYhQAsxHoN8DVPqub236Ca+0DBUvizwrCguJHDl/GO1K4= =L7J4 -----END PGP SIGNATURE----- --=-4SERTeX6WUStCGjrCKfn-- --===============0194111019== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0194111019==--