Re: [PATCH v2 15/14] drm/i915: Add comments explaining the vdd on/off functions

Daniel Vetter
2014-09-04T13:01:47Z

On Thu, Sep 04, 2014 at 02:55:31PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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 <imre.deak@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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_dp *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 = intel_dp_to_dev(intel_dp);
> @@ -1399,6 +1404,13 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_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 intel_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 =
> @@ -1497,6 +1514,12 @@ static void edp_panel_vdd_off(struct intel_dp *intel_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