[PATCH 1/3] Increase robustness against DRM page flip ioctl failures v2

[Alex Deucher]

On Thu, Mar 26, 2015 at 4:30 AM, Michel Dänzer <michel at daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> Centralize cleanup, only clean up things that have been allocated for
> the failed ioctl call.
>
> Fixes double-free after a flip ioctl failure.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89681
>
> v2: Only call drmModeRmFB for flipdata->old_fb_id on receipt of last DRM
>     page flip event. Fixes Black screen on making glxgears fullscreen with
>     DRI3 enabled.
>
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>

For the series:
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

> ---
>  src/drmmode_display.c | 46 +++++++++++++++++++++++++---------------------
>  1 file changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index f719f0c..35bbd9e 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -1836,9 +1836,6 @@ drmmode_flip_free(drmmode_flipevtcarrier_ptr flipcarrier)
>         if (--flipdata->flip_count > 0)
>                 return;
>
> -       /* Release framebuffer */
> -       drmModeRmFB(flipdata->drmmode->fd, flipdata->old_fb_id);
> -
>         free(flipdata);
>  }
>
> @@ -1867,10 +1864,16 @@ drmmode_flip_handler(ScrnInfoPtr scrn, uint32_t frame, uint64_t usec, void *even
>                 flipdata->fe_usec = usec;
>         }
>
> -       /* Deliver cached msc, ust from reference crtc to flip event handler */
> -       if (flipdata->event_data && flipdata->flip_count == 1)
> -               flipcarrier->handler(scrn, flipdata->fe_frame, flipdata->fe_usec,
> -                                    flipdata->event_data);
> +       if (flipdata->flip_count == 1) {
> +               /* Deliver cached msc, ust from reference crtc to flip event handler */
> +               if (flipdata->event_data)
> +                       flipcarrier->handler(scrn, flipdata->fe_frame,
> +                                            flipdata->fe_usec,
> +                                            flipdata->event_data);
> +
> +               /* Release framebuffer */
> +               drmModeRmFB(flipdata->drmmode->fd, flipdata->old_fb_id);
> +       }
>
>         drmmode_flip_free(flipcarrier);
>  }
> @@ -2276,10 +2279,10 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
>         unsigned int pitch;
>         int i, old_fb_id;
>         uint32_t tiling_flags = 0;
> -       int height, emitted = 0;
> +       int height;
>         drmmode_flipdata_ptr flipdata;
>         drmmode_flipevtcarrier_ptr flipcarrier = NULL;
> -       struct radeon_drm_queue_entry *drm_queue = 0;
> +       struct radeon_drm_queue_entry *drm_queue = NULL;
>
>         if (info->allowColorTiling) {
>                 if (info->ChipFamily >= CHIP_FAMILY_R600)
> @@ -2322,6 +2325,7 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
>
>          flipdata->event_data = data;
>          flipdata->drmmode = drmmode;
> +        flipdata->old_fb_id = old_fb_id;
>
>         for (i = 0; i < config->num_crtc; i++) {
>                 if (!config->crtc[i]->enabled)
> @@ -2334,8 +2338,6 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
>                 if (!flipcarrier) {
>                         xf86DrvMsg(scrn->scrnIndex, X_WARNING,
>                                    "flip queue: carrier alloc failed.\n");
> -                       if (emitted == 0)
> -                               free(flipdata);
>                         goto error_undo;
>                 }
>
> @@ -2362,25 +2364,27 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
>                                     drm_queue)) {
>                         xf86DrvMsg(scrn->scrnIndex, X_WARNING,
>                                    "flip queue failed: %s\n", strerror(errno));
> -                       free(flipcarrier);
> -                       if (emitted == 0)
> -                               free(flipdata);
>                         goto error_undo;
>                 }
> -               emitted++;
> +               flipcarrier = NULL;
> +               drm_queue = NULL;
>         }
>
> -       flipdata->old_fb_id = old_fb_id;
> -       return TRUE;
> +       if (flipdata->flip_count > 0)
> +               return TRUE;
>
>  error_undo:
> +       if (!flipdata || flipdata->flip_count <= 1) {
> +               drmModeRmFB(drmmode->fd, drmmode->fb_id);
> +               drmmode->fb_id = old_fb_id;
> +       }
> +
>         if (drm_queue)
>                 radeon_drm_abort_entry(drm_queue);
> -       else
> +       else if (flipcarrier)
>                 drmmode_flip_abort(scrn, flipcarrier);
> -
> -       drmModeRmFB(drmmode->fd, drmmode->fb_id);
> -       drmmode->fb_id = old_fb_id;
> +       else
> +               free(flipdata);
>
>  error_out:
>         xf86DrvMsg(scrn->scrnIndex, X_WARNING, "Page flip failed: %s\n",
> --
> 2.1.4
>
> _______________________________________________
> xorg-driver-ati mailing list
> xorg-driver-ati at lists.x.org
> http://lists.x.org/mailman/listinfo/xorg-driver-ati