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

[Daniel Vetter <daniel@ffwll.ch>]

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