From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH v2 15/14] drm/i915: Add comments explaining the vdd on/off functions Date: Thu, 4 Sep 2014 15:02:09 +0200 Message-ID: <20140904130209.GK15520@phenom.ffwll.local> References: <1409745161.15662.35.camel@intelbox> <1409831731-29341-1-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wi0-f175.google.com (mail-wi0-f175.google.com [209.85.212.175]) by gabe.freedesktop.org (Postfix) with ESMTP id B682B89B12 for ; Thu, 4 Sep 2014 06:01:47 -0700 (PDT) Received: by mail-wi0-f175.google.com with SMTP id ho1so1033442wib.14 for ; Thu, 04 Sep 2014 06:01:46 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1409831731-29341-1-git-send-email-ville.syrjala@linux.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.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Sep 04, 2014 at 02:55:31PM +0300, ville.syrjala@linux.intel.com wro= te: > From: Ville Syrj=E4l=E4 > = > Jani wanted some comments to explain why we call certain vdd on/off > functions in certain places. > = > v2: Make the comments more thorough (Imre) > = > Reviewed-by: Imre Deak > Signed-off-by: Ville Syrj=E4l=E4 Ok, pulled them all, with two patches slightly frobbed to appease checkpatch a bit. Thanks, Daniel > --- > drivers/gpu/drm/i915/intel_dp.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > = > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel= _dp.c > index e28610f..d8e868a 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1349,6 +1349,11 @@ static u32 ironlake_get_pp_control(struct intel_d= p *intel_dp) > return control; > } > = > +/* > + * Must be paired with edp_panel_vdd_off(). > + * Must hold pps_mutex around the whole on/off sequence. > + * Can be nested with intel_edp_panel_vdd_{on,off}() calls. > + */ > static bool edp_panel_vdd_on(struct intel_dp *intel_dp) > { > struct drm_device *dev =3D intel_dp_to_dev(intel_dp); > @@ -1399,6 +1404,13 @@ static bool edp_panel_vdd_on(struct intel_dp *inte= l_dp) > return need_to_disable; > } > = > +/* > + * Must be paired with intel_edp_panel_vdd_off() or > + * intel_edp_panel_off(). > + * Nested calls to these functions are not allowed since > + * we drop the lock. Caller must use some higher level > + * locking to prevent nested calls from other threads. > + */ > void intel_edp_panel_vdd_on(struct intel_dp *intel_dp) > { > bool vdd; > @@ -1477,6 +1489,11 @@ static void edp_panel_vdd_schedule_off(struct inte= l_dp *intel_dp) > schedule_delayed_work(&intel_dp->panel_vdd_work, delay); > } > = > +/* > + * Must be paired with edp_panel_vdd_on(). > + * Must hold pps_mutex around the whole on/off sequence. > + * Can be nested with intel_edp_panel_vdd_{on,off}() calls. > + */ > static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync) > { > struct drm_i915_private *dev_priv =3D > @@ -1497,6 +1514,12 @@ static void edp_panel_vdd_off(struct intel_dp *int= el_dp, bool sync) > edp_panel_vdd_schedule_off(intel_dp); > } > = > +/* > + * Must be paired with intel_edp_panel_vdd_on(). > + * Nested calls to these functions are not allowed since > + * we drop the lock. Caller must use some higher level > + * locking to prevent nested calls from other threads. > + */ > static void intel_edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync) > { > if (!is_edp(intel_dp)) > @@ -4390,6 +4413,10 @@ void intel_dp_encoder_destroy(struct drm_encoder *= encoder) > drm_encoder_cleanup(encoder); > if (is_edp(intel_dp)) { > cancel_delayed_work_sync(&intel_dp->panel_vdd_work); > + /* > + * vdd might still be enabled do to the delayed vdd off. > + * Make sure vdd is actually turned off here. > + */ > pps_lock(intel_dp); > edp_panel_vdd_off_sync(intel_dp); > pps_unlock(intel_dp); > @@ -4409,6 +4436,10 @@ static void intel_dp_encoder_suspend(struct intel_= encoder *intel_encoder) > if (!is_edp(intel_dp)) > return; > = > + /* > + * vdd might still be enabled do to the delayed vdd off. > + * Make sure vdd is actually turned off here. > + */ > pps_lock(intel_dp); > edp_panel_vdd_off_sync(intel_dp); > pps_unlock(intel_dp); > @@ -5116,6 +5147,10 @@ intel_dp_init_connector(struct intel_digital_port = *intel_dig_port, > drm_dp_aux_unregister(&intel_dp->aux); > if (is_edp(intel_dp)) { > cancel_delayed_work_sync(&intel_dp->panel_vdd_work); > + /* > + * vdd might still be enabled do to the delayed vdd off. > + * Make sure vdd is actually turned off here. > + */ > pps_lock(intel_dp); > edp_panel_vdd_off_sync(intel_dp); > pps_unlock(intel_dp); > -- = > 1.8.5.5 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch