On Mon, Oct 28, 2019 at 01:00:31PM +0200, Imre Deak wrote:
> For the HPD interrupt functionality the HW depends on power wells in the
> display core domain to be on. Accordingly when enabling these power
> wells the HPD polling logic will force an HPD detection cycle to account
> for hotplug events that may have happened when such a power well was
> off.
>
> Thus a detect cycle started by polling could start a new detect cycle if
> a power well in the display core domain gets enabled during detect and
> stays enabled after detect completes. That in turn can lead to a
> detection cycle runaway.
>
> To prevent re-triggering a poll-detect cycle make sure we drop all power
> references we acquired during detect synchronously by the end of detect.
> This will let the poll-detect logic continue with polling (matching the
> off state of the corresponding power wells) instead of scheduling a new
> detection cycle.
>
> Fixes: 6cfe7ec02e85 ("drm/i915: Remove the unneeded AUX power ref from intel_dp_detect()")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112125
> Reported-by: Val Kulkov <val.kulkov@xxxxxxxxx>
> Reported-and-tested-by: wangqr < wqr.prg@xxxxxxxxx>
> Cc: Val Kulkov <val.kulkov@xxxxxxxxx>
> Cc: wangqr < wqr.prg@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_crt.c | 7 +++++++
> drivers/gpu/drm/i915/display/intel_dp.c | 24 ++++++++++++++---------
> drivers/gpu/drm/i915/display/intel_hdmi.c | 6 ++++++
> 3 files changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> index ff6126ea793c..834bf1d43bb8 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -864,6 +864,13 @@ intel_crt_detect(struct drm_connector *connector,
>
> out:
> intel_display_power_put(dev_priv, intel_encoder->power_domain, wakeref);
> +
> + /*
> + * Make sure the refs for power wells enabled during detect are
> + * dropped to avoid a new detect cycle triggered by HPD polling.
> + */
> + intel_display_power_flush_work(dev_priv);
> +
> return status;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 86989ec25bc6..f4e0ec05d7c9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5600,6 +5600,7 @@ intel_dp_detect(struct drm_connector *connector,
> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> struct intel_encoder *encoder = &dig_port->base;
> enum drm_connector_status status;
> + int err = 0;
>
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> connector->base.id, connector->name);
> @@ -5626,7 +5627,7 @@ intel_dp_detect(struct drm_connector *connector,
> intel_dp->is_mst);
> }
>
> - goto out;
> + goto out_update_edid;
> }
>
> if (intel_dp->reset_link_params) {
> @@ -5654,7 +5655,7 @@ intel_dp_detect(struct drm_connector *connector,
> * with EDID on it
> */
> status = connector_status_disconnected;
> - goto out;
> + goto out_update_edid;
> }
>
> /*
> @@ -5662,11 +5663,9 @@ intel_dp_detect(struct drm_connector *connector,
> * with an IRQ_HPD, so force a link status check.
> */
> if (!intel_dp_is_edp(intel_dp)) {
> - int ret;
> -
> - ret = intel_dp_retrain_link(encoder, ctx);
> - if (ret)
> - return ret;
> + err = intel_dp_retrain_link(encoder, ctx);
> + if (err)
This should probably read
if (err == -EDEADLK)
Also I don't think we need to change this to a goto since it just
means we're going to retry the whole thing again, so the straight
return should be fine.
> + goto out_sync_power;
> }
>
> /*
> @@ -5684,11 +5683,18 @@ intel_dp_detect(struct drm_connector *connector,
>
> intel_dp_check_service_irq(intel_dp);
>
> -out:
> +out_update_edid:
> if (status != connector_status_connected && !intel_dp->is_mst)
> intel_dp_unset_edid(intel_dp);
>
> - return status;
> +out_sync_power:
> + /*
> + * Make sure the refs for power wells enabled during detect are
> + * dropped to avoid a new detect cycle triggered by HPD polling.
> + */
> + intel_display_power_flush_work(dev_priv);
> +
> + return err ? err : status;
> }
>
> static void
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index b54ccbb5aad5..ff71a4da3d00 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2626,6 +2626,12 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> if (status != connector_status_connected)
> cec_notifier_phys_addr_invalidate(intel_hdmi->cec_notifier);
>
> + /*
> + * Make sure the refs for power wells enabled during detect are
> + * dropped to avoid a new detect cycle triggered by HPD polling.
> + */
> + intel_display_power_flush_work(dev_priv);
> +
> return status;
> }
>
> --
> 2.17.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
