In case of link training failure and requiring user space to set a lower mode, would full mode set address it? How do we make user mode select lower resolution? For example 4k@60Hz monitor, and link training at 4 lane HBR2 fails and fallback to 4 lanes HBR, 4k@60 is no longer doable. I would think preventing user mode from seeing 4K@60Hz from the start is a easier and more robust solution and requiring user mode to have logic to decide how to fallback. -----Original Message----- From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] Sent: Friday, November 11, 2016 11:44 AM To: Cheng, Tony <Tony.Cheng@xxxxxxx> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; 'Jani Nikula' <jani.nikula@xxxxxxxxxxxxxxx>; Manasi Navare <manasi.d.navare@xxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Wentland, Harry <Harry.Wentland@xxxxxxx>; Peres, Martin <martin.peres@xxxxxxxxx> Subject: Re: [PATCH 0/5] Handle Link Training Failure during modeset On Fri, Nov 11, 2016 at 04:21:58PM +0000, Cheng, Tony wrote: > For HDMI, you can yank the cable, plug back in, HDMI will light up without user mode or kernel mode doing anything. > > For DP this is not possible, someone will have to retrain the link when plugging back in or DP will not light up. We see that on Ubuntu if someone unplug display and plug it back into same connector, we do not get KMS so in this case. It appears there is some optimizations somewhere in user mode stack which I am not familiar with yet. dal added workaround to retrain the link to light it back up (after the test training at end of detection). The link should get retrained as the driver should check the link state on HPD and retrain if it's not good. At least that's what we do in i915. Of course if it's not the same display that got plugged back, the retraining might fail. The new property can help with that case as userspace would then attempt a full modeset after the failed link training. > > >From user mode perspective I think it make sense to keep CRTC running, so vblank is still going so UMD isn't impacted. As regard to connector and encoder does it matter if kernel mode change state without user mode knowing? It seems to me those are more informational to UMD as UMD doesn't act on them. > > windows does not know link state and all link management is hidden behind kernel driver. If the user visible state doesn't change in any way, then the kernel could try to manage it all internally. > > -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > Sent: Friday, November 11, 2016 9:05 AM > To: Cheng, Tony <Tony.Cheng@xxxxxxx> > Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; 'Jani Nikula' > <jani.nikula@xxxxxxxxxxxxxxx>; Manasi Navare > <manasi.d.navare@xxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx; > intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Wentland, Harry > <Harry.Wentland@xxxxxxx>; Peres, Martin <martin.peres@xxxxxxxxx> > Subject: Re: [PATCH 0/5] Handle Link Training Failure > during modeset > > On Thu, Nov 10, 2016 at 06:51:40PM +0000, Cheng, Tony wrote: > > Amdgpu dal implementation will do a test link training at end of detection to verify we can achieve the capability reported in DPCD. We then report mode base on result of test training. > > > > AMD hardware (at least the generations supported by amdgpu) is able to link train without timing being setup (DP encoder and CRTC is decoupled). Do we have limitation from other vendors where you need timing to be there before you can link train? > > I can't recall the specifics for all of our supported platforms, but at least I have the recollection that it would be the case yes. > > The other problem wiyh this apporach is that even if you don't need the crtc, you still need the link itself. What happens if the link is still active since userspace just didn't bother to shut it down when the cable was yanked? Can you keep the crtc going but stop it from feeding the link in a way that userspace won't be able to notice? The kms design has always been pretty much that policy is in userspace, and thus the kernel shouldn't shut down crtcs unless explicitly asked to do so. > > > > > We can also past DP1.2 link layer compliance with this approach. > > > > -----Original Message----- > > From: Deucher, Alexander > > Sent: Thursday, November 10, 2016 1:43 PM > > To: 'Jani Nikula' <jani.nikula@xxxxxxxxxxxxxxx>; Manasi Navare > > <manasi.d.navare@xxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx; > > intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Wentland, Harry > > <Harry.Wentland@xxxxxxx>; Cheng, Tony <Tony.Cheng@xxxxxxx> > > Cc: Dave Airlie <airlied@xxxxxxxxx>; Peres, Martin > > <martin.peres@xxxxxxxxx> > > Subject: RE: [PATCH 0/5] Handle Link Training Failure > > during modeset > > > > Adding Harry and Tony from our display team to review. > > > > > -----Original Message----- > > > From: Jani Nikula [mailto:jani.nikula@xxxxxxxxxxxxxxx] > > > Sent: Thursday, November 10, 2016 1:20 PM > > > To: Manasi Navare; dri-devel@xxxxxxxxxxxxxxxxxxxxx; intel- > > > gfx@xxxxxxxxxxxxxxxxxxxxx > > > Cc: Dave Airlie; Peres, Martin; Deucher, Alexander > > > Subject: Re: [PATCH 0/5] Handle Link Training Failure > > > during modeset > > > > > > On Thu, 10 Nov 2016, Manasi Navare <manasi.d.navare@xxxxxxxxx> wrote: > > > > Link training failure is handled by lowering the link rate first > > > > until it reaches the minimum and keeping the lane count maximum > > > > and then lowering the lane count until it reaches minimim. These > > > > fallback values are saved and hotplug uevent is sent to the > > > > userspace after setting the connector link status property to BAD. > > > > Userspace should triiger another modeset on a uevent and if link > > > > status property is BAD. This will retrain the link at fallback values. > > > > This is repeated until the link is successfully trained. > > > > > > > > This has been validated to pass DP compliance. > > > > > > This cover letter and the commit messages do a good job of > > > explaining what the patches do. However, you're lacking the > > > crucial information of > > > *why* we need userspace cooperation to handle link training > > > failures on DP mode setting, and *why* a new connector property is > > > a good solution for this. > > > > > > Here goes, including some alternative approaches we've considered > > > (and even tried): > > > > > > At the time userspace does setcrtc, we've already promised the > > > mode would work. The promise is based on the theoretical > > > capabilities of the link, but it's possible we can't reach this in > > > practice. The DP spec describes how the link should be reduced, > > > but we can't reduce the link below the requirements of the mode. Black screen follows. > > > > > > One idea would be to have setcrtc return a failure. However, it is > > > my understanding that it already should not fail as the atomic > > > checks have passed [citation needed]. It would also conflict with > > > the idea of making setcrtc asynchronous in the future, returning > > > before the actual mode setting and link training. > > > > > > Another idea is to train the link "upfront" at hotplug time, > > > before pruning the mode list, so that we can do the pruning based > > > on practical not theoretical capabilities. However, the changes > > > for link training are pretty drastic, all for the sake of error > > > handling and DP compliance, when the most common happy day > > > scenario is the current approach of link training at mode setting > > > time, using the optimal parameters for the mode. It is also not > > > certain all hardware could do this without the pipe on; not even > > > all our hardware can do this. Some of this can be solved, but not trivially. > > > > > > Both of the above ideas also fail to address link degradation > > > *during* operation. > > > > > > So the idea presented in these patches is to do this in a way that > > > a) changes the current happy day scenario as little as possible, > > > to avoid regressions, b) can be implemented the same way by all > > > drm drivers, c) is still opt-in for the drivers and userspace, and > > > opting out doesn't regress the user experience, d) doesn't prevent > > > drivers from implementing better or alternate approaches, possibly > > > without userspace involvement. And, of course, handles all the issues presented. > > > > > > The solution is to add a "link status" connector property. In the > > > usual happy day scenario, this is always "good". If something > > > fails during or after a mode set, the kernel driver can set the > > > link status to "bad", prune the mode list based on new information > > > as necessary, and send a hotplug uevent for userspace to have it > > > re-check the valid modes through getconnector, and try again. If > > > the theoretical capabilities of the link can't be reached, the mode list is trimmed based on that. > > > > > > If the userspace is not aware of the property, the user experience > > > is the same as it currently is. If the userspace is aware of the > > > property, it has a chance to improve user experience. If a drm > > > driver does not modify the property (it stays "good"), the user > > > experience is the same as it currently is. A drm driver can also > > > choose to try to handle more of the failures in kernel, hardware > > > not limiting, or it can choose to involve userspace more. Up to the drivers. > > > > > > The reason for adding the property is to handle link training > > > failures, but it is not limited to DP or link training. For > > > example, if we implement asynchronous setcrtc, we can use this to > > > report any failures in that. > > > > > > Finally, while DP CTS compliance is advertized (which is great, > > > and could be made to work similarly for all drm drivers), this can > > > be used for the more important goal of improving user experience > > > on link training failures, by avoiding black screens. > > > > > > > > > BR, > > > Jani. > > > > > > > > > > > > > > Manasi Navare (5): > > > > drm: Add a new connector property for link status > > > > drm/i915: Set link status property for DP connector > > > > drm/i915: Update CRTC state if connector link status property > > > > changed > > > ^^^^^^^^ > > > > > > This is really drm, not i915. > > > > > > > > > > drm/i915: Find fallback link rate/lane count > > > > drm/i915: Implement Link Rate fallback on Link training > > > > failure > > > > > > > > drivers/gpu/drm/drm_atomic_helper.c | 7 ++ > > > > drivers/gpu/drm/drm_connector.c | 17 ++++ > > > > drivers/gpu/drm/i915/intel_ddi.c | 21 +++- > > > > drivers/gpu/drm/i915/intel_dp.c | 138 > > > +++++++++++++++++++++++++- > > > > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > > > > drivers/gpu/drm/i915/intel_drv.h | 12 ++- > > > > include/drm/drm_connector.h | 7 +- > > > > include/drm/drm_crtc.h | 5 + > > > > include/uapi/drm/drm_mode.h | 4 + > > > > 9 files changed, 214 insertions(+), 9 deletions(-) > > > > > > -- > > > Jani Nikula, Intel Open Source Technology Center > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
