From: Imre Deak <imre.deak@intel.com> To: "Ville Syrjälä" <ville.syrjala@linux.intel.com> Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Date: Thu, 4 Jun 2020 18:09:42 +0300 [thread overview] Message-ID: <20200604150942.GC15427@ideak-desk.fi.intel.com> (raw) In-Reply-To: <20200604145530.GS6112@intel.com> On Thu, Jun 04, 2020 at 05:55:30PM +0300, Ville Syrjälä wrote: > 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@intel.com> > > --- > > 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. Ok, makes sense. > > > + } else if (!intel_dp_short_pulse(intel_dp)) { > > + return IRQ_NONE; > > } > > > > return IRQ_HANDLED; > > -- > > 2.23.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Imre Deak <imre.deak@intel.com> To: "Ville Syrjälä" <ville.syrjala@linux.intel.com> Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Date: Thu, 4 Jun 2020 18:09:42 +0300 [thread overview] Message-ID: <20200604150942.GC15427@ideak-desk.fi.intel.com> (raw) In-Reply-To: <20200604145530.GS6112@intel.com> On Thu, Jun 04, 2020 at 05:55:30PM +0300, Ville Syrjälä wrote: > 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@intel.com> > > --- > > 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. Ok, makes sense. > > > + } else if (!intel_dp_short_pulse(intel_dp)) { > > + return IRQ_NONE; > > } > > > > return IRQ_HANDLED; > > -- > > 2.23.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-06-04 15:09 UTC|newest] Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-03 21:10 [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Imre Deak 2020-06-03 21:10 ` [Intel-gfx] " Imre Deak 2020-06-03 21:10 ` [PATCH 2/3] drm/dp_mst: Sanitize mgr->qlock locking in drm_dp_mst_wait_tx_reply() Imre Deak 2020-06-03 21:10 ` [Intel-gfx] " Imre Deak 2020-06-03 21:27 ` Souza, Jose 2020-06-03 21:27 ` Souza, Jose 2020-06-03 21:10 ` [PATCH 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses Imre Deak 2020-06-03 21:10 ` [Intel-gfx] " Imre Deak 2020-06-03 22:18 ` [PATCH v2 " Imre Deak 2020-06-03 22:18 ` [Intel-gfx] " Imre Deak 2020-06-04 15:12 ` Ville Syrjälä 2020-06-04 15:12 ` Ville Syrjälä 2020-06-04 15:41 ` Imre Deak 2020-06-04 15:41 ` Imre Deak 2020-06-04 18:45 ` [PATCH v3 " Imre Deak 2020-06-04 18:45 ` [Intel-gfx] " Imre Deak 2020-06-04 18:54 ` Ville Syrjälä 2020-06-04 18:54 ` [Intel-gfx] " Ville Syrjälä 2020-06-09 12:15 ` Imre Deak 2020-06-09 12:15 ` Imre Deak 2020-06-09 15:58 ` Lyude Paul 2020-06-09 15:58 ` Lyude Paul 2020-06-09 18:03 ` Imre Deak 2020-06-09 18:03 ` Imre Deak 2020-06-03 21:27 ` [Intel-gfx] [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Souza, Jose 2020-06-03 21:27 ` Souza, Jose 2020-06-03 21:34 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] " Patchwork 2020-06-03 21:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2020-06-03 22:56 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/dp_mst: Fix disabling MST on a port (rev2) Patchwork 2020-06-03 23:18 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2020-06-04 8:26 ` [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915/dp_mst: Fix disabling MST on a port Patchwork 2020-06-04 8:47 ` [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915/dp_mst: Fix disabling MST on a port (rev2) Patchwork 2020-06-04 14:55 ` [Intel-gfx] [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Ville Syrjälä 2020-06-04 14:55 ` Ville Syrjälä 2020-06-04 15:09 ` Imre Deak [this message] 2020-06-04 15:09 ` Imre Deak 2020-06-04 18:44 ` [PATCH v2 " Imre Deak 2020-06-04 18:44 ` [Intel-gfx] " Imre Deak 2020-06-05 9:16 ` [Intel-gfx] [PATCH v3 " Imre Deak 2020-06-05 9:48 ` [Intel-gfx] [PATCH RESEND " Imre Deak 2020-06-07 22:11 ` Souza, Jose 2020-06-07 23:15 ` Imre Deak 2020-06-04 19:36 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915/dp_mst: Fix disabling MST on a port (rev4) Patchwork 2020-06-04 23:55 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork 2020-06-05 9:27 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [v3,1/3] drm/i915/dp_mst: Fix disabling MST on a port (rev5) Patchwork 2020-06-05 10:24 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [RESEND,v3,1/3] drm/i915/dp_mst: Fix disabling MST on a port (rev6) Patchwork 2020-06-05 11:26 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork 2020-06-05 11:50 ` Imre Deak 2020-06-05 15:03 ` Vudum, Lakshminarayana 2020-06-05 13:53 ` [Intel-gfx] ✓ Fi.CI.IGT: success " Patchwork 2020-06-11 12:47 ` Imre Deak 2020-06-11 12:47 ` [Intel-gfx] " Imre Deak
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200604150942.GC15427@ideak-desk.fi.intel.com \ --to=imre.deak@intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=ville.syrjala@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.