On Thu, Jun 04, 2020 at 12:10:38AM +0300, Imre Deak wrote:
> Currently MST on a port can get enabled/disabled from the hotplug work
> and get disabled from the short pulse work in a racy way. Fix this by
> relying on the MST state checking in the hotplug work and just schedule
> a hotplug work from the short pulse handler if some problem happened
> during the MST interrupt handling.
>
> This removes the explicit MST disabling in case of an AUX failure, but
> if AUX fails, then probably the detection will also fail during the
> scheduled hotplug work and it's not guaranteed that we'll see
> intermittent errors anyway.
>
> While at it also simplify the error checking of the MST interrupt
> handler.
>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 33 +++----------------------
> 1 file changed, 4 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 55fda074c0ad..befbcacddaa1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5604,7 +5604,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
> }
> }
>
> - return need_retrain;
> + return need_retrain ? -EINVAL : 0;
> }
>
> static bool
> @@ -7255,35 +7255,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> }
>
> if (intel_dp->is_mst) {
> - switch (intel_dp_check_mst_status(intel_dp)) {
> - case -EINVAL:
> - /*
> - * If we were in MST mode, and device is not
> - * there, get out of MST mode
> - */
> - drm_dbg_kms(&i915->drm,
> - "MST device may have disappeared %d vs %d\n",
> - intel_dp->is_mst,
> - intel_dp->mst_mgr.mst_state);
> - intel_dp->is_mst = false;
> - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> - intel_dp->is_mst);
> -
> - return IRQ_NONE;
> - case 1:
> - return IRQ_NONE;
> - default:
> - break;
> - }
> - }
> -
> - if (!intel_dp->is_mst) {
> - bool handled;
> -
> - handled = intel_dp_short_pulse(intel_dp);
> -
> - if (!handled)
> + if (intel_dp_check_mst_status(intel_dp) < 0)
> return IRQ_NONE;
Since we no longer need the tristate return, can you follow up
with a conversion to bool return? I'd vote to make it match the
semantics of intel_dp_short_pulse() so we get one step
closer to unifying the hpd_irq handling across the board.
> + } else if (!intel_dp_short_pulse(intel_dp)) {
> + return IRQ_NONE;
> }
>
> return IRQ_HANDLED;
> --
> 2.23.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
