Re: [PATCH 3/4] drm/i915/ddi: Fix eDP VDD handling during booting and suspend/resume

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Apr 18, 2016 at 10:04:21AM +0300, Imre Deak wrote:
> The driver's VDD on/off logic assumes that whenever the VDD is on we
> also hold an AUX power domain reference. Since BIOS can leave the VDD on
> during booting and resuming and on DDI platforms we won't take a
> corresponding power reference, the above assumption won't hold on those
> platforms and an eventual delayed VDD off work will do an extraneous AUX
> power domain put resulting in a refcount underflow. Fix this the same
> way we did this for non-DDI DP encoders:
> 
> 6d93c0c41760c0 ("drm/i915: fix VDD state tracking after system resume")
> 
> At the same time call the DP encoder suspend handler the same way as the
> non-DDI DP encoders do to flush any pending VDD off work. Leaving the
> work running may cause a HW access where we don't expect this (at a point
> where power domains are suspended already).
> 
> While at it remove an unnecessary function call indirection.
> 
> This fixed for me AUX refcount underflow problems on BXT during
> suspend/resume.
> 
> CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
> CC: stable@vger.kernel.org
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 10 +++-------
>  drivers/gpu/drm/i915/intel_dp.c  |  4 ++--
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  3 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 96234c5..c2348fb 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2206,12 +2206,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  	intel_ddi_clock_get(encoder, pipe_config);
>  }
>  
> -static void intel_ddi_destroy(struct drm_encoder *encoder)
> -{
> -	/* HDMI has nothing special to destroy, so we can go with this. */
> -	intel_dp_encoder_destroy(encoder);
> -}
> -
>  static bool intel_ddi_compute_config(struct intel_encoder *encoder,
>  				     struct intel_crtc_state *pipe_config)
>  {
> @@ -2230,7 +2224,8 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
>  }
>  
>  static const struct drm_encoder_funcs intel_ddi_funcs = {
> -	.destroy = intel_ddi_destroy,
> +	.reset = intel_dp_encoder_reset,
> +	.destroy = intel_dp_encoder_destroy,
>  };
>  
>  static struct intel_connector *
> @@ -2329,6 +2324,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  	intel_encoder->post_disable = intel_ddi_post_disable;
>  	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
>  	intel_encoder->get_config = intel_ddi_get_config;
> +	intel_encoder->suspend = intel_dp_encoder_suspend;
>  
>  	intel_dig_port->port = port;
>  	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 61ee226..c6af3d0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4889,7 +4889,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
>  	kfree(intel_dig_port);
>  }
>  
> -static void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
> +void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
>  
> @@ -4931,7 +4931,7 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
>  	edp_panel_vdd_schedule_off(intel_dp);
>  }
>  
> -static void intel_dp_encoder_reset(struct drm_encoder *encoder)
> +void intel_dp_encoder_reset(struct drm_encoder *encoder)
>  {
>  	struct intel_dp *intel_dp;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e13ce22..10dfe72 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1285,6 +1285,8 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
>  void intel_dp_start_link_train(struct intel_dp *intel_dp);
>  void intel_dp_stop_link_train(struct intel_dp *intel_dp);
>  void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
> +void intel_dp_encoder_reset(struct drm_encoder *encoder);
> +void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
>  void intel_dp_encoder_destroy(struct drm_encoder *encoder);
>  int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
>  bool intel_dp_compute_config(struct intel_encoder *encoder,
> -- 
> 2.5.0

-- 
Ville Syrjälä
Intel OTC