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

Ville Syrjälä
2016-04-18T11:05:30Z

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