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

[Manasi Navare]

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 at linux.intel.com>
> ---
> 
> WARNING: The patches have not been merged in the kernel yet, so this patch is
> merely an RFC.
> 
> 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

The kernel does not prune the mode before sending a hotplug event. This happens
when userspace tries to do a mdoeset on the current mode after which kernel will
validate it agianst the new degraded link parameters and prune it if its not
supported anymore in which case the userspace driver will need to request a modeset
on the next lower mode.

Manasi


> +     * 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);
> +            }

What happens incase this mdoe is pruned? In this case the kernel will fail
the atomic_check() and return an error early on.
How does the driver request a new list of supported modes and request
a lower resolution mode?

Regards
Manasi


> +            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
>