[PATCH xf86-video-amdgpu] Use reference counting for tracking KMS framebuffer lifetimes
Deucher, Alexander
Alexander.Deucher at amd.com
Tue May 30 16:08:51 UTC 2017
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Michel Dänzer
> Sent: Monday, May 29, 2017 4:08 AM
> To: amd-gfx at lists.freedesktop.org
> Subject: [PATCH xf86-video-amdgpu] Use reference counting for tracking
> KMS framebuffer lifetimes
>
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> 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 <michel.daenzer at amd.com>
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> ---
> 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
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list