> -----Original Message-----
> From: amd-gfx [mailto:[email protected]] On Behalf
> Of Michel Dänzer
> Sent: Monday, May 29, 2017 4:08 AM
> To: [email protected]
> Subject: [PATCH xf86-video-amdgpu] Use reference counting for tracking
> KMS framebuffer lifetimes
> 
> From: Michel Dänzer <[email protected]>
> 
> References are held by the pixmaps corresponding to the FBs (so
> the same KMS FB can be reused as long as the pixmap exists) and by the
> CRTCs scanning out from them (so a KMS FB is only destroyed once it's
> not being scanned out anymore, preventing intermittent black screens and
> worse issues due to a CRTC turning off when it should be on).
> 
> v2:
> * Only increase reference count in drmmode_fb_reference if it was sane
>   before
> * Make drmmode_fb_reference's indentation match the rest of
>   drmmode_display.h
> 
> (Ported from radeon commit 55e513b978b2afc52b7cafc5bfcb0d1dc78d75f6)
> 
> Signed-off-by: Michel Dänzer <[email protected]>

Reviewed-by: Alex Deucher <[email protected]>

> ---
>  src/amdgpu_kms.c      |  70 +++++++++++++++++++---
>  src/amdgpu_pixmap.h   |  58 +++++++++++++++++++
>  src/amdgpu_present.c  |   9 ---
>  src/drmmode_display.c | 157 ++++++++++++++++++++-------------------------
> -----
>  src/drmmode_display.h |  42 ++++++++++++--
>  5 files changed, 219 insertions(+), 117 deletions(-)
> 
> diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
> index a418cf9d3..69d61943d 100644
> --- a/src/amdgpu_kms.c
> +++ b/src/amdgpu_kms.c
> @@ -675,6 +675,18 @@ amdgpu_prime_scanout_flip_abort(xf86CrtcPtr crtc,
> void *event_data)
>  }
> 
>  static void
> +amdgpu_prime_scanout_flip_handler(xf86CrtcPtr crtc, uint32_t msc,
> uint64_t usec,
> +                               void *event_data)
> +{
> +     AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
> +     drmmode_crtc_private_ptr drmmode_crtc = event_data;
> +
> +     drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb,
> +                          drmmode_crtc->flip_pending);
> +     amdgpu_prime_scanout_flip_abort(crtc, event_data);
> +}
> +
> +static void
>  amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
>  {
>       ScreenPtr screen = ent->slave_dst->drawable.pScreen;
> @@ -701,7 +713,8 @@ amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr
> ent)
>       drm_queue_seq = amdgpu_drm_queue_alloc(crtc,
> 
> AMDGPU_DRM_QUEUE_CLIENT_DEFAULT,
> 
> AMDGPU_DRM_QUEUE_ID_DEFAULT,
> -                                            drmmode_crtc, NULL,
> +                                            drmmode_crtc,
> +
> amdgpu_prime_scanout_flip_handler,
> 
> amdgpu_prime_scanout_flip_abort);
>       if (drm_queue_seq == AMDGPU_DRM_QUEUE_ERROR) {
>               xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> @@ -709,8 +722,17 @@ amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr
> ent)
>               return;
>       }
> 
> +     drmmode_crtc->flip_pending =
> +             amdgpu_pixmap_get_fb(drmmode_crtc-
> >scanout[scanout_id].pixmap);
> +     if (!drmmode_crtc->flip_pending) {
> +             xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> +                        "Failed to get FB for PRIME flip.\n");
> +             amdgpu_drm_abort_entry(drm_queue_seq);
> +             return;
> +     }
> +
>       if (drmmode_page_flip_target_relative(pAMDGPUEnt,
> drmmode_crtc,
> -                                           drmmode_crtc-
> >scanout[scanout_id].fb_id,
> +                                           drmmode_crtc->flip_pending-
> >handle,
>                                             0, drm_queue_seq, 0) != 0) {
>               xf86DrvMsg(scrn->scrnIndex, X_WARNING, "flip queue
> failed in %s: %s\n",
>                          __func__, strerror(errno));
> @@ -720,7 +742,6 @@ amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr
> ent)
> 
>       drmmode_crtc->scanout_id = scanout_id;
>       drmmode_crtc->scanout_update_pending = TRUE;
> -     drmmode_crtc->flip_pending = TRUE;
>  }
> 
>  static void
> @@ -950,10 +971,14 @@ amdgpu_scanout_update(xf86CrtcPtr xf86_crtc)
>  static void
>  amdgpu_scanout_flip_abort(xf86CrtcPtr crtc, void *event_data)
>  {
> -     drmmode_crtc_private_ptr drmmode_crtc = event_data;
> +     amdgpu_prime_scanout_flip_abort(crtc, event_data);
> +}
> 
> -     drmmode_crtc->scanout_update_pending = FALSE;
> -     drmmode_clear_pending_flip(crtc);
> +static void
> +amdgpu_scanout_flip_handler(xf86CrtcPtr crtc, uint32_t msc, uint64_t
> usec,
> +                         void *event_data)
> +{
> +     amdgpu_prime_scanout_flip_handler(crtc, msc, usec, event_data);
>  }
> 
>  static void
> @@ -977,7 +1002,8 @@ amdgpu_scanout_flip(ScreenPtr pScreen,
> AMDGPUInfoPtr info,
>       drm_queue_seq = amdgpu_drm_queue_alloc(xf86_crtc,
> 
> AMDGPU_DRM_QUEUE_CLIENT_DEFAULT,
> 
> AMDGPU_DRM_QUEUE_ID_DEFAULT,
> -                                            drmmode_crtc, NULL,
> +                                            drmmode_crtc,
> +                                            amdgpu_scanout_flip_handler,
>                                              amdgpu_scanout_flip_abort);
>       if (drm_queue_seq == AMDGPU_DRM_QUEUE_ERROR) {
>               xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> @@ -985,8 +1011,17 @@ amdgpu_scanout_flip(ScreenPtr pScreen,
> AMDGPUInfoPtr info,
>               return;
>       }
> 
> +     drmmode_crtc->flip_pending =
> +             amdgpu_pixmap_get_fb(drmmode_crtc-
> >scanout[scanout_id].pixmap);
> +     if (!drmmode_crtc->flip_pending) {
> +             xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> +                        "Failed to get FB for scanout flip.\n");
> +             amdgpu_drm_abort_entry(drm_queue_seq);
> +             return;
> +     }
> +
>       if (drmmode_page_flip_target_relative(pAMDGPUEnt,
> drmmode_crtc,
> -                                           drmmode_crtc-
> >scanout[scanout_id].fb_id,
> +                                           drmmode_crtc->flip_pending-
> >handle,
>                                             0, drm_queue_seq, 0) != 0) {
>               xf86DrvMsg(scrn->scrnIndex, X_WARNING, "flip queue
> failed in %s: %s\n",
>                          __func__, strerror(errno));
> @@ -996,13 +1031,13 @@ amdgpu_scanout_flip(ScreenPtr pScreen,
> AMDGPUInfoPtr info,
> 
>       drmmode_crtc->scanout_id = scanout_id;
>       drmmode_crtc->scanout_update_pending = TRUE;
> -     drmmode_crtc->flip_pending = TRUE;
>  }
> 
>  static void AMDGPUBlockHandler_KMS(BLOCKHANDLER_ARGS_DECL)
>  {
>       SCREEN_PTR(arg);
>       ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
> +     AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
>       AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
>       xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
>       int c;
> @@ -1011,6 +1046,23 @@ static void
> AMDGPUBlockHandler_KMS(BLOCKHANDLER_ARGS_DECL)
>       (*pScreen->BlockHandler) (BLOCKHANDLER_ARGS);
>       pScreen->BlockHandler = AMDGPUBlockHandler_KMS;
> 
> +     if (!pScrn->vtSema) {
> +#if XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(1,19,0,0,0)
> +             if (info->use_glamor)
> +                     amdgpu_glamor_flush(pScrn);
> +#endif
> +
> +             for (c = 0; c < xf86_config->num_crtc; c++) {
> +                     drmmode_crtc_private_ptr drmmode_crtc =
> +                             xf86_config->crtc[c]->driver_private;
> +
> +                     drmmode_fb_reference(pAMDGPUEnt->fd,
> &drmmode_crtc->fb,
> +                                          NULL);
> +             }
> +
> +             return;
> +     }
> +
>       if (!amdgpu_is_gpu_screen(pScreen))
>       {
>               for (c = 0; c < xf86_config->num_crtc; c++) {
> diff --git a/src/amdgpu_pixmap.h b/src/amdgpu_pixmap.h
> index a8de26a93..00fb5bf05 100644
> --- a/src/amdgpu_pixmap.h
> +++ b/src/amdgpu_pixmap.h
> @@ -35,6 +35,7 @@ struct amdgpu_pixmap {
>       uint64_t tiling_info;
> 
>       struct amdgpu_buffer *bo;
> +     struct drmmode_fb *fb;
> 
>       /* GEM handle for pixmaps shared via DRI2/3 */
>       Bool handle_valid;
> @@ -56,6 +57,8 @@ static inline void
> amdgpu_set_pixmap_private(PixmapPtr pixmap,
> 
>  static inline Bool amdgpu_set_pixmap_bo(PixmapPtr pPix, struct
> amdgpu_buffer *bo)
>  {
> +     ScrnInfoPtr scrn = xf86ScreenToScrn(pPix->drawable.pScreen);
> +     AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
>       struct amdgpu_pixmap *priv;
> 
>       priv = amdgpu_get_pixmap_private(pPix);
> @@ -71,6 +74,8 @@ static inline Bool amdgpu_set_pixmap_bo(PixmapPtr
> pPix, struct amdgpu_buffer *bo
>                       priv->handle_valid = FALSE;
>               }
> 
> +             drmmode_fb_reference(pAMDGPUEnt->fd, &priv->fb,
> NULL);
> +
>               if (!bo) {
>                       free(priv);
>                       priv = NULL;
> @@ -98,6 +103,59 @@ static inline struct amdgpu_buffer
> *amdgpu_get_pixmap_bo(PixmapPtr pPix)
>       return priv ? priv->bo : NULL;
>  }
> 
> +static inline struct drmmode_fb*
> +amdgpu_fb_create(int drm_fd, uint32_t width, uint32_t height, uint8_t
> depth,
> +              uint8_t bpp, uint32_t pitch, uint32_t handle)
> +{
> +     struct drmmode_fb *fb  = malloc(sizeof(*fb));
> +
> +     if (!fb)
> +             return NULL;
> +
> +     fb->refcnt = 1;
> +     if (drmModeAddFB(drm_fd, width, height, depth, bpp, pitch, handle,
> +                      &fb->handle) == 0)
> +             return fb;
> +
> +     free(fb);
> +     return NULL;
> +}
> +
> +static inline struct drmmode_fb*
> +amdgpu_pixmap_create_fb(int drm_fd, PixmapPtr pix)
> +{
> +     uint32_t handle;
> +
> +     if (!amdgpu_pixmap_get_handle(pix, &handle))
> +             return NULL;
> +
> +     return amdgpu_fb_create(drm_fd, pix->drawable.width, pix-
> >drawable.height,
> +                             pix->drawable.depth, pix-
> >drawable.bitsPerPixel,
> +                             pix->devKind, handle);
> +}
> +
> +static inline struct drmmode_fb*
> +amdgpu_pixmap_get_fb(PixmapPtr pix)
> +{
> +     ScrnInfoPtr scrn = xf86ScreenToScrn(pix->drawable.pScreen);
> +     AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
> +     AMDGPUInfoPtr info = AMDGPUPTR(scrn);
> +
> +     if (info->use_glamor) {
> +             struct amdgpu_pixmap *priv =
> amdgpu_get_pixmap_private(pix);
> +
> +             if (!priv)
> +                     return NULL;
> +
> +             if (!priv->fb)
> +                     priv->fb =
> amdgpu_pixmap_create_fb(pAMDGPUEnt->fd, pix);
> +
> +             return priv->fb;
> +     }
> +
> +     return NULL;
> +}
> +
>  enum {
>       AMDGPU_CREATE_PIXMAP_DRI2    = 0x08000000,
>       AMDGPU_CREATE_PIXMAP_LINEAR  = 0x04000000,
> diff --git a/src/amdgpu_present.c b/src/amdgpu_present.c
> index 3c4bc5259..86b095cd1 100644
> --- a/src/amdgpu_present.c
> +++ b/src/amdgpu_present.c
> @@ -350,7 +350,6 @@ static void
>  amdgpu_present_unflip(ScreenPtr screen, uint64_t event_id)
>  {
>       ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
> -     AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
>       AMDGPUInfoPtr info = AMDGPUPTR(scrn);
>       xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
>       struct amdgpu_present_vblank_event *event;
> @@ -358,7 +357,6 @@ amdgpu_present_unflip(ScreenPtr screen, uint64_t
> event_id)
>       enum drmmode_flip_sync flip_sync =
>               (amdgpu_present_screen_info.capabilities &
> PresentCapabilityAsync) ?
>               FLIP_ASYNC : FLIP_VSYNC;
> -     int old_fb_id;
>       int i;
> 
>       if (!amdgpu_present_check_unflip(scrn))
> @@ -380,12 +378,6 @@ amdgpu_present_unflip(ScreenPtr screen, uint64_t
> event_id)
>               return;
> 
>  modeset:
> -     /* info->drmmode.fb_id still points to the FB for the last flipped BO.
> -      * Clear it, drmmode_set_mode_major will re-create it
> -      */
> -     old_fb_id = info->drmmode.fb_id;
> -     info->drmmode.fb_id = 0;
> -
>       amdgpu_glamor_finish(scrn);
>       for (i = 0; i < config->num_crtc; i++) {
>               xf86CrtcPtr crtc = config->crtc[i];
> @@ -401,7 +393,6 @@ modeset:
>                       drmmode_crtc->need_modeset = TRUE;
>       }
> 
> -     drmModeRmFB(pAMDGPUEnt->fd, old_fb_id);
>       present_event_notify(event_id, 0, 0);
>       info->drmmode.present_flipping = FALSE;
>  }
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index add8287a0..f4bea0ce2 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -322,6 +322,7 @@ drmmode_crtc_dpms(xf86CrtcPtr crtc, int mode)
> 
>               drmModeSetCrtc(pAMDGPUEnt->fd, drmmode_crtc-
> >mode_crtc->crtc_id,
>                              0, 0, 0, NULL, 0, NULL);
> +             drmmode_fb_reference(pAMDGPUEnt->fd,
> &drmmode_crtc->fb, NULL);
>       } else if (drmmode_crtc->dpms_mode != DPMSModeOn)
>               crtc->funcs->set_mode_major(crtc, &crtc->mode, crtc-
> >rotation,
>                                           crtc->x, crtc->y);
> @@ -390,8 +391,9 @@ void drmmode_copy_fb(ScrnInfoPtr pScrn,
> drmmode_ptr drmmode)
>  {
>       xf86CrtcConfigPtr   xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
>       AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
> -     PixmapPtr src, dst;
>       ScreenPtr pScreen = pScrn->pScreen;
> +     PixmapPtr src, dst = pScreen->GetScreenPixmap(pScreen);
> +     struct drmmode_fb *fb = amdgpu_pixmap_get_fb(dst);
>       int fbcon_id = 0;
>       GCPtr gc;
>       int i;
> @@ -406,7 +408,7 @@ void drmmode_copy_fb(ScrnInfoPtr pScrn,
> drmmode_ptr drmmode)
>       if (!fbcon_id)
>               return;
> 
> -     if (fbcon_id == drmmode->fb_id) {
> +     if (fbcon_id == fb->handle) {
>               /* in some rare case there might be no fbcon and we might
> already
>                * be the one with the current fb to avoid a false deadlck in
>                * kernel ttm code just do nothing as anyway there is nothing
> @@ -419,8 +421,6 @@ void drmmode_copy_fb(ScrnInfoPtr pScrn,
> drmmode_ptr drmmode)
>       if (!src)
>               return;
> 
> -     dst = pScreen->GetScreenPixmap(pScreen);
> -
>       gc = GetScratchGC(pScrn->depth, pScreen);
>       ValidateGC(&dst->drawable, gc);
> 
> @@ -449,10 +449,6 @@ drmmode_crtc_scanout_destroy(drmmode_ptr
> drmmode,
>       }
> 
>       if (scanout->bo) {
> -             AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(drmmode-
> >scrn);
> -
> -             drmModeRmFB(pAMDGPUEnt->fd, scanout->fb_id);
> -             scanout->fb_id = 0;
>               amdgpu_bo_unref(&scanout->bo);
>               scanout->bo = NULL;
>       }
> @@ -497,10 +493,8 @@ drmmode_crtc_scanout_create(xf86CrtcPtr crtc,
> struct drmmode_scanout *scanout,
>                           int width, int height)
>  {
>       ScrnInfoPtr pScrn = crtc->scrn;
> -     AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
>       drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
>       drmmode_ptr drmmode = drmmode_crtc->drmmode;
> -     union gbm_bo_handle bo_handle;
>       int pitch;
> 
>       if (scanout->pixmap) {
> @@ -516,15 +510,7 @@ drmmode_crtc_scanout_create(xf86CrtcPtr crtc,
> struct drmmode_scanout *scanout,
>       if (!scanout->bo) {
>               xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
>                          "Failed to allocate scanout buffer memory\n");
> -             goto error;
> -     }
> -
> -     bo_handle = gbm_bo_get_handle(scanout->bo->bo.gbm);
> -     if (drmModeAddFB(pAMDGPUEnt->fd, width, height, pScrn->depth,
> -                      pScrn->bitsPerPixel, pitch,
> -                      bo_handle.u32, &scanout->fb_id) != 0) {
> -             ErrorF("failed to add scanout fb\n");
> -             goto error;
> +             return NULL;
>       }
> 
>       scanout->pixmap = drmmode_create_bo_pixmap(pScrn,
> @@ -532,13 +518,17 @@ drmmode_crtc_scanout_create(xf86CrtcPtr crtc,
> struct drmmode_scanout *scanout,
>                                                pScrn->depth,
>                                                pScrn->bitsPerPixel,
>                                                pitch, scanout->bo);
> -     if (scanout->pixmap) {
> +     if (!scanout->pixmap) {
> +             ErrorF("failed to create CRTC scanout pixmap\n");
> +             goto error;
> +     }
> +
> +     if (amdgpu_pixmap_get_fb(scanout->pixmap)) {
>               scanout->width = width;
>               scanout->height = height;
>       } else {
> -             xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
> -                        "Couldn't allocate scanout pixmap for CRTC\n");
> -error:
> +             ErrorF("failed to create CRTC scanout FB\n");
> +error:
>               drmmode_crtc_scanout_destroy(drmmode, scanout);
>       }
> 
> @@ -651,8 +641,8 @@ drmmode_handle_transform(xf86CrtcPtr crtc)
> 
>  static void
>  drmmode_crtc_prime_scanout_update(xf86CrtcPtr crtc, DisplayModePtr
> mode,
> -                               unsigned scanout_id, int *fb_id, int *x,
> -                               int *y)
> +                               unsigned scanout_id, struct drmmode_fb
> **fb,
> +                               int *x, int *y)
>  {
>       ScrnInfoPtr scrn = crtc->scrn;
>       ScreenPtr screen = scrn->pScreen;
> @@ -701,7 +691,7 @@ drmmode_crtc_prime_scanout_update(xf86CrtcPtr
> crtc, DisplayModePtr mode,
>               }
>       }
> 
> -     *fb_id = drmmode_crtc->scanout[scanout_id].fb_id;
> +     *fb = amdgpu_pixmap_get_fb(drmmode_crtc-
> >scanout[scanout_id].pixmap);
>       *x = *y = 0;
>       drmmode_crtc->scanout_id = scanout_id;
>  }
> @@ -710,7 +700,8 @@ drmmode_crtc_prime_scanout_update(xf86CrtcPtr
> crtc, DisplayModePtr mode,
> 
>  static void
>  drmmode_crtc_scanout_update(xf86CrtcPtr crtc, DisplayModePtr mode,
> -                         unsigned scanout_id, int *fb_id, int *x, int *y)
> +                         unsigned scanout_id, struct drmmode_fb **fb, int
> *x,
> +                         int *y)
>  {
>       ScrnInfoPtr scrn = crtc->scrn;
>       ScreenPtr screen = scrn->pScreen;
> @@ -746,7 +737,7 @@ drmmode_crtc_scanout_update(xf86CrtcPtr crtc,
> DisplayModePtr mode,
>               box->x2 = max(box->x2, scrn->virtualX);
>               box->y2 = max(box->y2, scrn->virtualY);
> 
> -             *fb_id = drmmode_crtc->scanout[scanout_id].fb_id;
> +             *fb = amdgpu_pixmap_get_fb(drmmode_crtc-
> >scanout[scanout_id].pixmap);
>               *x = *y = 0;
> 
>               amdgpu_scanout_do_update(crtc, scanout_id);
> @@ -784,9 +775,8 @@ drmmode_set_mode_major(xf86CrtcPtr crtc,
> DisplayModePtr mode,
>       int output_count = 0;
>       Bool ret = FALSE;
>       int i;
> -     int fb_id;
> +     struct drmmode_fb *fb = NULL;
>       drmModeModeInfo kmode;
> -     uint32_t bo_handle;
> 
>       /* The root window contents may be undefined before the
> WindowExposures
>        * hook is called for it, so bail if we get here before that
> @@ -834,15 +824,14 @@ drmmode_set_mode_major(xf86CrtcPtr crtc,
> DisplayModePtr mode,
> 
>               drmmode_ConvertToKMode(crtc->scrn, &kmode, mode);
> 
> -             fb_id = drmmode->fb_id;
>  #ifdef AMDGPU_PIXMAP_SHARING
>               if (drmmode_crtc->prime_scanout_pixmap) {
>                       drmmode_crtc_prime_scanout_update(crtc, mode,
> scanout_id,
> -                                                       &fb_id, &x, &y);
> +                                                       &fb, &x, &y);
>               } else
>  #endif
> -             if (drmmode_crtc->rotate.fb_id) {
> -                     fb_id = drmmode_crtc->rotate.fb_id;
> +             if (drmmode_crtc->rotate.pixmap) {
> +                     fb = amdgpu_pixmap_get_fb(drmmode_crtc-
> >rotate.pixmap);
>                       x = y = 0;
> 
>               } else if (!amdgpu_is_gpu_screen(pScreen) &&
> @@ -852,26 +841,27 @@ drmmode_set_mode_major(xf86CrtcPtr crtc,
> DisplayModePtr mode,
>  #endif
>                           info->shadow_primary)) {
>                       drmmode_crtc_scanout_update(crtc, mode,
> scanout_id,
> -                                                 &fb_id, &x, &y);
> +                                                 &fb, &x, &y);
>               }
> 
> -             if (fb_id == 0) {
> -                     if (!amdgpu_bo_get_handle(info->front_buffer,
> &bo_handle)) {
> -                             ErrorF("failed to get BO handle for FB\n");
> -                             goto done;
> -                     }
> -
> -                     if (drmModeAddFB(pAMDGPUEnt->fd,
> -                                pScrn->virtualX,
> -                                pScrn->virtualY,
> -                                pScrn->depth, pScrn->bitsPerPixel,
> -                                pScrn->displayWidth * info->pixel_bytes,
> -                                bo_handle, &drmmode->fb_id) < 0) {
> -                             ErrorF("failed to add fb\n");
> -                             goto done;
> -                     }
> -
> -                     fb_id = drmmode->fb_id;
> +             if (!fb)
> +                     fb = amdgpu_pixmap_get_fb(pScreen-
> >GetWindowPixmap(pScreen->root));
> +             if (!fb) {
> +                     union gbm_bo_handle bo_handle;
> +
> +                     bo_handle = gbm_bo_get_handle(info-
> >front_buffer->bo.gbm);
> +                     fb = amdgpu_fb_create(pAMDGPUEnt->fd, pScrn-
> >virtualX,
> +                                           pScrn->virtualY, pScrn->depth,
> +                                           pScrn->bitsPerPixel,
> +                                           pScrn->displayWidth * info-
> >pixel_bytes,
> +                                           bo_handle.u32);
> +                     /* Prevent refcnt of ad-hoc FBs from reaching 2 */
> +                     drmmode_fb_reference(pAMDGPUEnt->fd,
> &drmmode_crtc->fb, NULL);
> +                     drmmode_crtc->fb = fb;
> +             }
> +             if (!fb) {
> +                     ErrorF("failed to add FB for modeset\n");
> +                     goto done;
>               }
> 
>               /* Wait for any pending flip to finish */
> @@ -881,13 +871,15 @@ drmmode_set_mode_major(xf86CrtcPtr crtc,
> DisplayModePtr mode,
> 
>               if (drmModeSetCrtc(pAMDGPUEnt->fd,
>                                  drmmode_crtc->mode_crtc->crtc_id,
> -                                fb_id, x, y, output_ids,
> +                                fb->handle, x, y, output_ids,
>                                  output_count, &kmode) != 0) {
>                       xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
>                                  "failed to set mode: %s\n", strerror(errno));
>                       goto done;
> -             } else
> +             } else {
>                       ret = TRUE;
> +                     drmmode_fb_reference(pAMDGPUEnt->fd,
> &drmmode_crtc->fb, fb);
> +             }
> 
>               if (pScreen)
>                       xf86CrtcSetScreenSubpixelOrder(pScreen);
> @@ -933,7 +925,9 @@ done:
>       } else {
>               crtc->active = TRUE;
> 
> -             if (fb_id != drmmode_crtc->scanout[scanout_id].fb_id)
> +             if (drmmode_crtc->scanout[scanout_id].pixmap &&
> +                 fb != amdgpu_pixmap_get_fb(drmmode_crtc->
> +                                            scanout[scanout_id].pixmap))
>                       drmmode_crtc_scanout_free(drmmode_crtc);
>               else if (!drmmode_crtc->tear_free) {
>                       drmmode_crtc_scanout_destroy(drmmode,
> @@ -2079,14 +2073,9 @@ int drmmode_get_pitch_align(ScrnInfoPtr scrn, int
> bpe)
>  static Bool drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
>  {
>       xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
> -     drmmode_crtc_private_ptr
> -         drmmode_crtc = xf86_config->crtc[0]->driver_private;
> -     drmmode_ptr drmmode = drmmode_crtc->drmmode;
>       AMDGPUInfoPtr info = AMDGPUPTR(scrn);
> -     AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
>       struct amdgpu_buffer *old_front = NULL;
>       ScreenPtr screen = xf86ScrnToScreen(scrn);
> -     uint32_t old_fb_id;
>       int i, pitch, old_width, old_height, old_pitch;
>       int cpp = info->pixel_bytes;
>       PixmapPtr ppix = screen->GetScreenPixmap(screen);
> @@ -2109,8 +2098,6 @@ static Bool drmmode_xf86crtc_resize(ScrnInfoPtr
> scrn, int width, int height)
>       old_width = scrn->virtualX;
>       old_height = scrn->virtualY;
>       old_pitch = scrn->displayWidth;
> -     old_fb_id = drmmode->fb_id;
> -     drmmode->fb_id = 0;
>       old_front = info->front_buffer;
> 
>       scrn->virtualX = width;
> @@ -2182,8 +2169,6 @@ static Bool drmmode_xf86crtc_resize(ScrnInfoPtr
> scrn, int width, int height)
>                                      crtc->rotation, crtc->x, crtc->y);
>       }
> 
> -     if (old_fb_id)
> -             drmModeRmFB(pAMDGPUEnt->fd, old_fb_id);
>       if (old_front) {
>               amdgpu_bo_unref(&old_front);
>       }
> @@ -2198,7 +2183,6 @@ fail:
>       scrn->virtualX = old_width;
>       scrn->virtualY = old_height;
>       scrn->displayWidth = old_pitch;
> -     drmmode->fb_id = old_fb_id;
> 
>       return FALSE;
>  }
> @@ -2212,7 +2196,7 @@ drmmode_clear_pending_flip(xf86CrtcPtr crtc)
>  {
>       drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> 
> -     drmmode_crtc->flip_pending = FALSE;
> +     drmmode_crtc->flip_pending = NULL;
> 
>       if (!crtc->enabled ||
>           (drmmode_crtc->pending_dpms_mode != DPMSModeOn &&
> @@ -2257,6 +2241,7 @@ static void
>  drmmode_flip_handler(xf86CrtcPtr crtc, uint32_t frame, uint64_t usec, void
> *event_data)
>  {
>       AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
> +     drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
>       drmmode_flipdata_ptr flipdata = event_data;
> 
>       /* Is this the event whose info shall be delivered to higher level? */
> @@ -2276,12 +2261,11 @@ drmmode_flip_handler(xf86CrtcPtr crtc, uint32_t
> frame, uint64_t usec, void *even
>               else
>                       flipdata->handler(crtc, frame, usec, flipdata-
> >event_data);
> 
> -             /* Release framebuffer */
> -             drmModeRmFB(pAMDGPUEnt->fd, flipdata->old_fb_id);
> -
>               free(flipdata);
>       }
> 
> +     drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb,
> +                          drmmode_crtc->flip_pending);
>       drmmode_clear_pending_flip(crtc);
>  }
> 
> @@ -2514,6 +2498,8 @@ Bool drmmode_set_desired_modes(ScrnInfoPtr
> pScrn, drmmode_ptr drmmode,
>                               drmModeSetCrtc(pAMDGPUEnt->fd,
>                                              drmmode_crtc->mode_crtc-
> >crtc_id,
>                                              0, 0, 0, NULL, 0, NULL);
> +                             drmmode_fb_reference(pAMDGPUEnt->fd,
> +                                                  &drmmode_crtc->fb,
> NULL);
>                       }
>                       continue;
>               }
> @@ -2773,18 +2759,11 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn,
> ClientPtr client,
>       xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
>       xf86CrtcPtr crtc = NULL;
>       drmmode_crtc_private_ptr drmmode_crtc = config->crtc[0]-
> >driver_private;
> -     drmmode_ptr drmmode = drmmode_crtc->drmmode;
>       int i;
>       uint32_t flip_flags = flip_sync == FLIP_ASYNC ?
> DRM_MODE_PAGE_FLIP_ASYNC : 0;
>       drmmode_flipdata_ptr flipdata;
>       uintptr_t drm_queue_seq = 0;
> -     uint32_t new_front_handle;
> -
> -     if (!amdgpu_pixmap_get_handle(new_front, &new_front_handle))
> {
> -             xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> -                        "flip queue: data alloc failed.\n");
> -             return FALSE;
> -     }
> +     struct drmmode_fb *fb;
> 
>       flipdata = calloc(1, sizeof(drmmode_flipdata_rec));
>       if (!flipdata) {
> @@ -2793,15 +2772,11 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn,
> ClientPtr client,
>               goto error;
>       }
> 
> -     /*
> -      * Create a new handle for the back buffer
> -      */
> -     flipdata->old_fb_id = drmmode->fb_id;
> -     if (drmModeAddFB(pAMDGPUEnt->fd, new_front->drawable.width,
> -                      new_front->drawable.height, scrn->depth,
> -                      scrn->bitsPerPixel, new_front->devKind,
> -                      new_front_handle, &drmmode->fb_id))
> +     fb = amdgpu_pixmap_get_fb(new_front);
> +     if (!fb) {
> +             ErrorF("Failed to get FB for flip\n");
>               goto error;
> +     }
> 
>       /*
>        * Queue flips on all enabled CRTCs
> @@ -2845,7 +2820,7 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn,
> ClientPtr client,
>               if (drmmode_crtc->hw_id == ref_crtc_hw_id) {
>                       if
> (drmmode_page_flip_target_absolute(pAMDGPUEnt,
>                                                             drmmode_crtc,
> -                                                           drmmode->fb_id,
> +                                                           fb->handle,
>                                                             flip_flags,
>                                                             drm_queue_seq,
>                                                             target_msc) != 0)
> @@ -2853,13 +2828,13 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn,
> ClientPtr client,
>               } else {
>                       if
> (drmmode_page_flip_target_relative(pAMDGPUEnt,
>                                                             drmmode_crtc,
> -                                                           drmmode->fb_id,
> +                                                           fb->handle,
>                                                             flip_flags,
>                                                             drm_queue_seq,
> 0) != 0)
>                               goto flip_error;
>               }
> 
> -             drmmode_crtc->flip_pending = TRUE;
> +             drmmode_crtc->flip_pending = fb;
>               drm_queue_seq = 0;
>       }
> 
> @@ -2871,12 +2846,6 @@ flip_error:
>                  strerror(errno));
> 
>  error:
> -     if (flipdata && flipdata->flip_count <= 1 &&
> -         drmmode->fb_id != flipdata->old_fb_id) {
> -             drmModeRmFB(pAMDGPUEnt->fd, drmmode->fb_id);
> -             drmmode->fb_id = flipdata->old_fb_id;
> -     }
> -
>       if (drm_queue_seq)
>               amdgpu_drm_abort_entry(drm_queue_seq);
>       else if (crtc)
> diff --git a/src/drmmode_display.h b/src/drmmode_display.h
> index 6a57fd23b..b64a938cd 100644
> --- a/src/drmmode_display.h
> +++ b/src/drmmode_display.h
> @@ -37,7 +37,6 @@
>  #include "amdgpu.h"
> 
>  typedef struct {
> -     unsigned fb_id;
>       ScrnInfoPtr scrn;
>  #ifdef HAVE_LIBUDEV
>       struct udev_monitor *uevent_monitor;
> @@ -53,7 +52,6 @@ typedef struct {
>  } drmmode_rec, *drmmode_ptr;
> 
>  typedef struct {
> -     unsigned old_fb_id;
>       int flip_count;
>       void *event_data;
>       unsigned int fe_frame;
> @@ -63,10 +61,14 @@ typedef struct {
>       amdgpu_drm_abort_proc abort;
>  } drmmode_flipdata_rec, *drmmode_flipdata_ptr;
> 
> +struct drmmode_fb {
> +     int refcnt;
> +     uint32_t handle;
> +};
> +
>  struct drmmode_scanout {
>       struct amdgpu_buffer *bo;
>       PixmapPtr pixmap;
> -     unsigned fb_id;
>       int width, height;
>  };
> 
> @@ -96,8 +98,10 @@ typedef struct {
> 
>       /* Modeset needed for DPMS on */
>       Bool need_modeset;
> -     /* A flip is pending for this CRTC */
> -     Bool flip_pending;
> +     /* A flip to this FB is pending for this CRTC */
> +     struct drmmode_fb *flip_pending;
> +     /* The FB currently being scanned out by this CRTC, if any */
> +     struct drmmode_fb *fb;
>  } drmmode_crtc_private_rec, *drmmode_crtc_private_ptr;
> 
>  typedef struct {
> @@ -128,6 +132,34 @@ enum drmmode_flip_sync {
>  };
> 
> 
> +static inline void
> +drmmode_fb_reference(int drm_fd, struct drmmode_fb **old, struct
> drmmode_fb *new)
> +{
> +     if (new) {
> +             if (new->refcnt <= 0) {
> +                     ErrorF("New FB's refcnt was %d in %s\n", new-
> >refcnt,
> +                            __func__);
> +             } else {
> +                     new->refcnt++;
> +             }
> +     }
> +
> +     if (*old) {
> +             if ((*old)->refcnt <= 0) {
> +                     ErrorF("Old FB's refcnt was %d in %s\n",
> +                            (*old)->refcnt, __func__);
> +             } else {
> +                     if (--(*old)->refcnt == 0) {
> +                             drmModeRmFB(drm_fd, (*old)->handle);
> +                             free(*old);
> +                     }
> +             }
> +     }
> +
> +     *old = new;
> +}
> +
> +
>  extern int drmmode_page_flip_target_absolute(AMDGPUEntPtr
> pAMDGPUEnt,
>                                            drmmode_crtc_private_ptr
> drmmode_crtc,
>                                            int fb_id, uint32_t flags,
> --
> 2.11.0
> 
> _______________________________________________
> amd-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to