| Message ID | 1466192674-9334-1-git-send-email-cpaul@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 17, 2016 at 03:44:34PM -0400, Lyude wrote: > From: Lyude Paul <cpaul@redhat.com> > > DRM does not always update the status of each connector during a > hotplug event, and it's generally expected that userspace is supposed to > handle that by reprobing. This happens in a couple situations: > suspend/resume, MST hotplugs, and probably a few others. As a result, > making this assumption actually breaks MST hotplugging. Generally expected? So logind reprobes? We force a probe on wakeup? -Chris
On Fri, 2016-06-17 at 20:57 +0100, Chris Wilson wrote: > On Fri, Jun 17, 2016 at 03:44:34PM -0400, Lyude wrote: > > > > From: Lyude Paul <cpaul@redhat.com> > > > > DRM does not always update the status of each connector during a > > hotplug event, and it's generally expected that userspace is > > supposed to > > handle that by reprobing. This happens in a couple situations: > > suspend/resume, MST hotplugs, and probably a few others. As a > > result, This was meant more to be an example of when DRM connector states aren't consistent. This being said; i915 does do a reprobe on resuming so that is still true. Just to make sure; you saw the IRC convo on this airlied/danvet had right? > > making this assumption actually breaks MST hotplugging. > Generally expected? So logind reprobes? We force a probe on wakeup? > -Chris >
On 18 June 2016 at 05:57, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Fri, Jun 17, 2016 at 03:44:34PM -0400, Lyude wrote: >> From: Lyude Paul <cpaul@redhat.com> >> >> DRM does not always update the status of each connector during a >> hotplug event, and it's generally expected that userspace is supposed to >> handle that by reprobing. This happens in a couple situations: >> suspend/resume, MST hotplugs, and probably a few others. As a result, >> making this assumption actually breaks MST hotplugging. > > Generally expected? So logind reprobes? We force a probe on wakeup? Yes, expected by the API from day one. If you want an API to do something else, write a new API. Don't go shoehorning behaviour into a generic API that is driver specific. So please fix the DDX or I'll push the patch and you can revert it if you don't like but at least then I know you are being painful and I can justify carrying the patch upstream until the Intel DDX is no longer something we care about. Dave.
diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c index 0cf2bdb..79c72cc 100644 --- a/src/sna/sna_display.c +++ b/src/sna/sna_display.c @@ -5114,7 +5114,7 @@ void sna_mode_discover(struct sna *sna, bool tell) ScreenPtr screen = xf86ScrnToScreen(sna->scrn); xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(sna->scrn); struct drm_mode_card_res res; - uint32_t connectors[32], now; + uint32_t connectors[32]; unsigned changed = 0; unsigned serial; int i, j; @@ -5146,7 +5146,6 @@ void sna_mode_discover(struct sna *sna, bool tell) if (serial == 0) serial = ++sna->mode.serial; - now = GetTimeInMillis(); for (i = 0; i < res.count_connectors; i++) { DBG(("%s: connector[%d] = %d\n", __FUNCTION__, i, connectors[i])); for (j = 0; j < sna->mode.num_real_output; j++) { @@ -5172,13 +5171,10 @@ void sna_mode_discover(struct sna *sna, bool tell) continue; if (sna_output->serial == serial) { - if (output_check_status(sna, sna_output)) { - DBG(("%s: output %s (id=%d), retained state\n", - __FUNCTION__, output->name, sna_output->id)); - sna_output->last_detect = now; - } else { - DBG(("%s: output %s (id=%d), changed state, reprobing\n", - __FUNCTION__, output->name, sna_output->id)); + if (!output_check_status(sna, sna_output)) { + DBG(("%s: output %s (id=%d), changed state, reprobing]\n", + __FUNCTION__, output->name, sna_output->id, + sna_output->serial, serial)); sna_output->last_detect = 0; changed |= 4; }