Re: [RFC PATCH xserver] modesetting: re-set the crtc's mode when link-status goes BAD

[Manasi Navare <manasi.d.navare-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

On Thu, Jan 26, 2017 at 06:21:20PM +0100, Daniel Vetter wrote:
> On Thu, Jan 26, 2017 at 02:37:28PM +0200, Martin Peres wrote:
> > Despite all the careful planing of the kernel, a link may become
> > insufficient to handle the currently-set mode. At this point, the
> > kernel should mark this particular configuration as being broken
> > and potentially prune the mode before setting the offending connector's
> > link-status to BAD and send the userspace a hotplug event. This may
> > happen right after a modeset or later on.
> > 
> > When available, we should use the link-status information to reset
> > the wanted mode.
> > 
> > Signed-off-by: Martin Peres <martin.peres@linux.intel.com>
> > ---
> > 
> > WARNING: The patches have not been merged in the kernel yet, so this patch is
> > merely an RFC.
> 
> That's how it's supposed to happen, before we can merge the kernel side,
> we need acceptance (=reviewed) by the userspace side. Which is why this
> patch here.
> -Daniel
>

This has been validated with a compliance device inducing a forced link
failure during modeset, after which the kernel sets the link-status to BAD
and sends a hotplug to userspace. The -modesetting driver then resets the
configuration by forcing another mdoeset and retraining the link at lower
link rate/lane count. This has been proven to pass DP compliance.
It has been also verified that this avoids the black screens in case of such
mode failures due to link training failure.

Regards
Manasi

 
> > 
> > This patch is the result of discussions happening mostly in DRI-devel and
> > Intel-GFX on how to handle link training failures. I would advise reading the
> > thread [0] first and then this thread [1] which explain in great length why this
> > is needed and why the selected approach seems to be the best.
> > 
> > The relevant kernel patches + this patch are enough to pass the relevant
> > DisplayPort compliance tests, provided that the Desktop Environment or another
> > program ([2]?) provides the initial modesetting on hotplug.
> > 
> > Would this patch be acceptable to you? Any comments or suggestions?
> > 
> > [0] https://lists.freedesktop.org/archives/dri-devel/2016-November/123366.html
> > [1] https://lists.freedesktop.org/archives/dri-devel/2016-November/124736.html
> > [2] https://cgit.freedesktop.org/~mperes/auto-randr/
> > 
> > 
> >  hw/xfree86/drivers/modesetting/drmmode_display.c | 51 ++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> > 
> > diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
> > index 6e755e9482..3144620c67 100644
> > --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> > +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> > @@ -2262,6 +2262,10 @@ drmmode_setup_colormap(ScreenPtr pScreen, ScrnInfoPtr pScrn)
> >  }
> >  
> >  #ifdef CONFIG_UDEV_KMS
> > +
> > +#define DRM_MODE_LINK_STATUS_GOOD       0
> > +#define DRM_MODE_LINK_STATUS_BAD        1
> > +
> >  static void
> >  drmmode_handle_uevents(int fd, void *closure)
> >  {
> > @@ -2281,6 +2285,49 @@ drmmode_handle_uevents(int fd, void *closure)
> >      if (!found)
> >          return;
> >  
> > +    /* Try to re-set the mode on all the connectors with a BAD link-state:
> > +     * This may happen if a link degrades and a new modeset is necessary, using
> > +     * different link-training parameters. If the kernel found that the current
> > +     * mode is not achievable anymore, it should have pruned the mode before
> > +     * sending the hotplug event. Try to re-set the currently-set mode to keep
> > +     * the display alive, this will fail if the mode has been pruned.
> > +     * In any case, we will send randr events for the Desktop Environment to
> > +     * deal with it, if it wants to.
> > +     */
> > +    for (i = 0; i < config->num_output; i++) {
> > +        xf86OutputPtr output = config->output[i];
> > +        drmmode_output_private_ptr drmmode_output = output->driver_private;
> > +        uint32_t con_id = drmmode_output->mode_output->connector_id;
> > +        drmModeConnectorPtr koutput;
> > +
> > +        /* Get an updated view of the properties for the current connector and
> > +         * look for the link-status property
> > +         */
> > +        koutput = drmModeGetConnectorCurrent(drmmode->fd, con_id);
> > +        for (j = 0; koutput && j < koutput->count_props; j++) {
> > +            drmModePropertyPtr props;
> > +            props = drmModeGetProperty(drmmode->fd, koutput->props[j]);
> > +            if (props && props->flags & DRM_MODE_PROP_ENUM &&
> > +                !strcmp(props->name, "link-status") &&
> > +                koutput->prop_values[j] == DRM_MODE_LINK_STATUS_BAD) {
> > +                xf86CrtcPtr crtc = output->crtc;
> > +                if (!crtc)
> > +                    continue;
> > +
> > +                /* the connector got a link failure, re-set the current mode */
> > +                drmmode_set_mode_major(crtc, &crtc->mode, crtc->rotation,
> > +                                       crtc->x, crtc->y);
> > +
> > +                xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> > +                           "hotplug event: connector %u's link-state is BAD, "
> > +                           "tried resetting the current mode. You may be left"
> > +                           "with a black screen if this fails...\n", con_id);
> > +            }
> > +            drmModeFreeProperty(props);
> > +        }
> > +        drmModeFreeConnector(koutput);
> > +    }
> > +
> >      mode_res = drmModeGetResources(drmmode->fd);
> >      if (!mode_res)
> >          goto out;
> > @@ -2345,6 +2392,10 @@ out_free_res:
> >  out:
> >      RRGetInfo(xf86ScrnToScreen(scrn), TRUE);
> >  }
> > +
> > +#undef DRM_MODE_LINK_STATUS_BAD
> > +#undef DRM_MODE_LINK_STATUS_GOOD
> > +
> >  #endif
> >  
> >  void
> > -- 
> > 2.11.0
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel