From: Michel Dänzer <[email protected]> If the memory for an entry was allocated at the same address as that for a previously cancelled entry, the handler could theoretically be called prematurely, triggered by the DRM event which was submitted for the cancelled entry.
(Ported from radeon commit 4693b1bd5b5c381e8b7b68a6f7f0c6696d6a68df) Signed-off-by: Michel Dänzer <[email protected]> --- src/amdgpu_dri2.c | 47 ++++++++++++++++++++++++----------------------- src/amdgpu_drm_queue.c | 24 ++++++++++++++++++------ src/amdgpu_drm_queue.h | 12 +++++------- src/amdgpu_kms.c | 36 ++++++++++++++++++------------------ src/amdgpu_present.c | 17 +++++++++-------- src/drmmode_display.c | 20 ++++++++++---------- 6 files changed, 84 insertions(+), 72 deletions(-) diff --git a/src/amdgpu_dri2.c b/src/amdgpu_dri2.c index 4478b16..29f60ba 100644 --- a/src/amdgpu_dri2.c +++ b/src/amdgpu_dri2.c @@ -395,7 +395,7 @@ typedef struct _DRI2FrameEvent { unsigned frame; xf86CrtcPtr crtc; OsTimerPtr timer; - struct amdgpu_drm_queue_entry *drm_queue; + uintptr_t drm_queue_seq; /* for swaps & flips only */ DRI2SwapEventPtr event_complete; @@ -961,8 +961,8 @@ CARD32 amdgpu_dri2_deferred_event(OsTimerPtr timer, CARD32 now, pointer data) */ if (!event_info->crtc) { ErrorF("%s no crtc\n", __func__); - if (event_info->drm_queue) - amdgpu_drm_abort_entry(event_info->drm_queue); + if (event_info->drm_queue_seq) + amdgpu_drm_abort_entry(event_info->drm_queue_seq); else amdgpu_dri2_frame_event_abort(NULL, data); return 0; @@ -974,9 +974,9 @@ CARD32 amdgpu_dri2_deferred_event(OsTimerPtr timer, CARD32 now, pointer data) if (ret) { xf86DrvMsg(scrn->scrnIndex, X_ERROR, "%s cannot get current time\n", __func__); - if (event_info->drm_queue) + if (event_info->drm_queue_seq) amdgpu_drm_queue_handler(pAMDGPUEnt->fd, 0, 0, 0, - event_info->drm_queue); + (void*)event_info->drm_queue_seq); else amdgpu_dri2_frame_event_handler(crtc, 0, 0, data); return 0; @@ -990,9 +990,10 @@ CARD32 amdgpu_dri2_deferred_event(OsTimerPtr timer, CARD32 now, pointer data) delta_seq = delta_t * drmmode_crtc->dpms_last_fps; delta_seq /= 1000000; frame = (CARD64) drmmode_crtc->dpms_last_seq + delta_seq; - if (event_info->drm_queue) + if (event_info->drm_queue_seq) amdgpu_drm_queue_handler(pAMDGPUEnt->fd, frame, drm_now / 1000000, - drm_now % 1000000, event_info->drm_queue); + drm_now % 1000000, + (void*)event_info->drm_queue_seq); else amdgpu_dri2_frame_event_handler(crtc, frame, drm_now, data); return 0; @@ -1023,7 +1024,7 @@ static int amdgpu_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw, ScrnInfoPtr scrn = xf86ScreenToScrn(screen); AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn); DRI2FrameEventPtr wait_info = NULL; - struct amdgpu_drm_queue_entry *wait = NULL; + uintptr_t drm_queue_seq = 0; xf86CrtcPtr crtc = amdgpu_dri2_drawable_crtc(draw, TRUE); uint32_t msc_delta; drmVBlank vbl; @@ -1079,15 +1080,15 @@ static int amdgpu_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw, current_msc = vbl.reply.sequence + msc_delta; current_msc &= 0xffffffff; - wait = amdgpu_drm_queue_alloc(crtc, client, AMDGPU_DRM_QUEUE_ID_DEFAULT, - wait_info, amdgpu_dri2_frame_event_handler, - amdgpu_dri2_frame_event_abort); - if (!wait) { + drm_queue_seq = amdgpu_drm_queue_alloc(crtc, client, AMDGPU_DRM_QUEUE_ID_DEFAULT, + wait_info, amdgpu_dri2_frame_event_handler, + amdgpu_dri2_frame_event_abort); + if (!drm_queue_seq) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, "Allocating DRM queue event entry failed.\n"); goto out_complete; } - wait_info->drm_queue = wait; + wait_info->drm_queue_seq = drm_queue_seq; /* * If divisor is zero, or current_msc is smaller than target_msc, @@ -1106,7 +1107,7 @@ static int amdgpu_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw, vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT; vbl.request.type |= amdgpu_populate_vbl_request_type(crtc); vbl.request.sequence = target_msc - msc_delta; - vbl.request.signal = (unsigned long)wait; + vbl.request.signal = drm_queue_seq; ret = drmWaitVBlank(pAMDGPUEnt->fd, &vbl); if (ret) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, @@ -1138,7 +1139,7 @@ static int amdgpu_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw, if ((current_msc % divisor) >= remainder) vbl.request.sequence += divisor; - vbl.request.signal = (unsigned long)wait; + vbl.request.signal = drm_queue_seq; ret = drmWaitVBlank(pAMDGPUEnt->fd, &vbl); if (ret) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, @@ -1190,7 +1191,7 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw, drmVBlank vbl; int ret, flip = 0; DRI2FrameEventPtr swap_info = NULL; - struct amdgpu_drm_queue_entry *swap; + uintptr_t drm_queue_seq; CARD64 current_msc; BoxRec box; RegionRec region; @@ -1227,15 +1228,15 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw, swap_info->back = back; swap_info->crtc = crtc; - swap = amdgpu_drm_queue_alloc(crtc, client, AMDGPU_DRM_QUEUE_ID_DEFAULT, - swap_info, amdgpu_dri2_frame_event_handler, - amdgpu_dri2_frame_event_abort); - if (!swap) { + drm_queue_seq = amdgpu_drm_queue_alloc(crtc, client, AMDGPU_DRM_QUEUE_ID_DEFAULT, + swap_info, amdgpu_dri2_frame_event_handler, + amdgpu_dri2_frame_event_abort); + if (!drm_queue_seq) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, "Allocating DRM queue entry failed.\n"); goto blit_fallback; } - swap_info->drm_queue = swap; + swap_info->drm_queue_seq = drm_queue_seq; /* * CRTC is in DPMS off state, fallback to blit, but calculate @@ -1304,7 +1305,7 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw, *target_msc = current_msc; vbl.request.sequence = *target_msc - msc_delta; - vbl.request.signal = (unsigned long)swap; + vbl.request.signal = drm_queue_seq; ret = drmWaitVBlank(pAMDGPUEnt->fd, &vbl); if (ret) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, @@ -1350,7 +1351,7 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw, /* Account for 1 frame extra pageflip delay if flip > 0 */ vbl.request.sequence -= flip; - vbl.request.signal = (unsigned long)swap; + vbl.request.signal = drm_queue_seq; ret = drmWaitVBlank(pAMDGPUEnt->fd, &vbl); if (ret) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c index 11b74a0..25b7114 100644 --- a/src/amdgpu_drm_queue.c +++ b/src/amdgpu_drm_queue.c @@ -40,6 +40,7 @@ struct amdgpu_drm_queue_entry { struct xorg_list list; uint64_t id; + uintptr_t seq; void *data; ClientPtr client; xf86CrtcPtr crtc; @@ -49,6 +50,7 @@ struct amdgpu_drm_queue_entry { static int amdgpu_drm_queue_refcnt; static struct xorg_list amdgpu_drm_queue; +static uintptr_t amdgpu_drm_queue_seq; /* @@ -58,11 +60,11 @@ void amdgpu_drm_queue_handler(int fd, unsigned int frame, unsigned int sec, unsigned int usec, void *user_ptr) { - struct amdgpu_drm_queue_entry *user_data = user_ptr; + uintptr_t seq = (uintptr_t)user_ptr; struct amdgpu_drm_queue_entry *e, *tmp; xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_queue, list) { - if (e == user_data) { + if (e->seq == seq) { xorg_list_del(&e->list); if (e->handler) e->handler(e->crtc, frame, @@ -80,7 +82,7 @@ amdgpu_drm_queue_handler(int fd, unsigned int frame, unsigned int sec, * Enqueue a potential drm response; when the associated response * appears, we've got data to pass to the handler from here */ -struct amdgpu_drm_queue_entry * +uintptr_t amdgpu_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client, uint64_t id, void *data, amdgpu_drm_handler_proc handler, @@ -92,6 +94,9 @@ amdgpu_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client, if (!e) return NULL; + if (!amdgpu_drm_queue_seq) + amdgpu_drm_queue_seq = 1; + e->seq = amdgpu_drm_queue_seq++; e->client = client; e->crtc = crtc; e->id = id; @@ -101,7 +106,7 @@ amdgpu_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client, xorg_list_add(&e->list, &amdgpu_drm_queue); - return e; + return e->seq; } /* @@ -139,9 +144,16 @@ amdgpu_drm_abort_client(ClientPtr client) * Abort specific drm queue entry */ void -amdgpu_drm_abort_entry(struct amdgpu_drm_queue_entry *entry) +amdgpu_drm_abort_entry(uintptr_t seq) { - amdgpu_drm_abort_one(entry); + struct amdgpu_drm_queue_entry *e, *tmp; + + xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_queue, list) { + if (e->seq == seq) { + amdgpu_drm_abort_one(e); + break; + } + } } /* diff --git a/src/amdgpu_drm_queue.h b/src/amdgpu_drm_queue.h index 892ba13..b4d4009 100644 --- a/src/amdgpu_drm_queue.h +++ b/src/amdgpu_drm_queue.h @@ -43,14 +43,12 @@ typedef void (*amdgpu_drm_abort_proc)(xf86CrtcPtr crtc, void *data); void amdgpu_drm_queue_handler(int fd, unsigned int frame, unsigned int tv_sec, unsigned int tv_usec, void *user_ptr); -struct amdgpu_drm_queue_entry *amdgpu_drm_queue_alloc(xf86CrtcPtr crtc, - ClientPtr client, - uint64_t id, - void *data, - amdgpu_drm_handler_proc handler, - amdgpu_drm_abort_proc abort); +uintptr_t amdgpu_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client, + uint64_t id, void *data, + amdgpu_drm_handler_proc handler, + amdgpu_drm_abort_proc abort); void amdgpu_drm_abort_client(ClientPtr client); -void amdgpu_drm_abort_entry(struct amdgpu_drm_queue_entry *entry); +void amdgpu_drm_abort_entry(uintptr_t seq); void amdgpu_drm_abort_id(uint64_t id); void amdgpu_drm_queue_init(); void amdgpu_drm_queue_close(ScrnInfoPtr scrn); diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c index 6dcec69..ae98cf1 100644 --- a/src/amdgpu_kms.c +++ b/src/amdgpu_kms.c @@ -397,7 +397,7 @@ static void amdgpu_scanout_update(xf86CrtcPtr xf86_crtc) { drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private; - struct amdgpu_drm_queue_entry *drm_queue_entry; + uintptr_t drm_queue_seq; ScrnInfoPtr scrn; AMDGPUEntPtr pAMDGPUEnt; drmVBlank vbl; @@ -427,13 +427,13 @@ amdgpu_scanout_update(xf86CrtcPtr xf86_crtc) return; scrn = xf86_crtc->scrn; - drm_queue_entry = amdgpu_drm_queue_alloc(xf86_crtc, - AMDGPU_DRM_QUEUE_CLIENT_DEFAULT, - AMDGPU_DRM_QUEUE_ID_DEFAULT, - drmmode_crtc, - amdgpu_scanout_update_handler, - amdgpu_scanout_update_abort); - if (!drm_queue_entry) { + drm_queue_seq = amdgpu_drm_queue_alloc(xf86_crtc, + AMDGPU_DRM_QUEUE_CLIENT_DEFAULT, + AMDGPU_DRM_QUEUE_ID_DEFAULT, + drmmode_crtc, + amdgpu_scanout_update_handler, + amdgpu_scanout_update_abort); + if (!drm_queue_seq) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, "amdgpu_drm_queue_alloc failed for scanout update\n"); return; @@ -443,12 +443,12 @@ amdgpu_scanout_update(xf86CrtcPtr xf86_crtc) vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT; vbl.request.type |= amdgpu_populate_vbl_request_type(xf86_crtc); vbl.request.sequence = 1; - vbl.request.signal = (unsigned long)drm_queue_entry; + vbl.request.signal = drm_queue_seq; if (drmWaitVBlank(pAMDGPUEnt->fd, &vbl)) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, "drmWaitVBlank failed for scanout update: %s\n", strerror(errno)); - amdgpu_drm_abort_entry(drm_queue_entry); + amdgpu_drm_abort_entry(drm_queue_seq); return; } @@ -471,7 +471,7 @@ amdgpu_scanout_flip(ScreenPtr pScreen, AMDGPUInfoPtr info, drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private; ScrnInfoPtr scrn; AMDGPUEntPtr pAMDGPUEnt; - struct amdgpu_drm_queue_entry *drm_queue_entry; + uintptr_t drm_queue_seq; unsigned scanout_id; if (drmmode_crtc->scanout_update_pending) @@ -482,12 +482,12 @@ amdgpu_scanout_flip(ScreenPtr pScreen, AMDGPUInfoPtr info, return; scrn = xf86_crtc->scrn; - drm_queue_entry = amdgpu_drm_queue_alloc(xf86_crtc, - AMDGPU_DRM_QUEUE_CLIENT_DEFAULT, - AMDGPU_DRM_QUEUE_ID_DEFAULT, - drmmode_crtc, NULL, - amdgpu_scanout_flip_abort); - if (!drm_queue_entry) { + drm_queue_seq = amdgpu_drm_queue_alloc(xf86_crtc, + AMDGPU_DRM_QUEUE_CLIENT_DEFAULT, + AMDGPU_DRM_QUEUE_ID_DEFAULT, + drmmode_crtc, NULL, + amdgpu_scanout_flip_abort); + if (!drm_queue_seq) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, "Allocating DRM event queue entry failed.\n"); return; @@ -496,7 +496,7 @@ amdgpu_scanout_flip(ScreenPtr pScreen, AMDGPUInfoPtr info, pAMDGPUEnt = AMDGPUEntPriv(scrn); if (drmModePageFlip(pAMDGPUEnt->fd, drmmode_crtc->mode_crtc->crtc_id, drmmode_crtc->scanout[scanout_id].fb_id, - DRM_MODE_PAGE_FLIP_EVENT, drm_queue_entry)) { + DRM_MODE_PAGE_FLIP_EVENT, (void*)drm_queue_seq)) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, "flip queue failed in %s: %s\n", __func__, strerror(errno)); return; diff --git a/src/amdgpu_present.c b/src/amdgpu_present.c index 4b33ce2..4aa0708 100644 --- a/src/amdgpu_present.c +++ b/src/amdgpu_present.c @@ -157,7 +157,7 @@ amdgpu_present_queue_vblank(RRCrtcPtr crtc, uint64_t event_id, uint64_t msc) AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn); int crtc_id = drmmode_get_crtc_id(xf86_crtc); struct amdgpu_present_vblank_event *event; - struct amdgpu_drm_queue_entry *queue; + uintptr_t drm_queue_seq; drmVBlank vbl; int ret; @@ -165,24 +165,25 @@ amdgpu_present_queue_vblank(RRCrtcPtr crtc, uint64_t event_id, uint64_t msc) if (!event) return BadAlloc; event->event_id = event_id; - queue = amdgpu_drm_queue_alloc(xf86_crtc, AMDGPU_DRM_QUEUE_CLIENT_DEFAULT, - event_id, event, - amdgpu_present_vblank_handler, - amdgpu_present_vblank_abort); - if (!queue) { + drm_queue_seq = amdgpu_drm_queue_alloc(xf86_crtc, + AMDGPU_DRM_QUEUE_CLIENT_DEFAULT, + event_id, event, + amdgpu_present_vblank_handler, + amdgpu_present_vblank_abort); + if (!drm_queue_seq) { free(event); return BadAlloc; } vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT | crtc_select(crtc_id); vbl.request.sequence = msc; - vbl.request.signal = (unsigned long)queue; + vbl.request.signal = drm_queue_seq; for (;;) { ret = drmWaitVBlank(pAMDGPUEnt->fd, &vbl); if (!ret) break; if (errno != EBUSY || !amdgpu_present_flush_drm_events(screen)) { - amdgpu_drm_abort_entry(queue); + amdgpu_drm_abort_entry(drm_queue_seq); return BadAlloc; } } diff --git a/src/drmmode_display.c b/src/drmmode_display.c index 2aea542..07ae9b2 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -2504,7 +2504,7 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client, drmmode_ptr drmmode = drmmode_crtc->drmmode; int i; drmmode_flipdata_ptr flipdata; - struct amdgpu_drm_queue_entry *drm_queue = NULL; + uintptr_t drm_queue_seq = 0; uint32_t new_front_handle; if (!amdgpu_pixmap_get_handle(new_front, &new_front_handle)) { @@ -2559,11 +2559,11 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client, if (drmmode_crtc->hw_id == ref_crtc_hw_id) flipdata->fe_crtc = crtc; - drm_queue = amdgpu_drm_queue_alloc(crtc, client, id, - flipdata, - drmmode_flip_handler, - drmmode_flip_abort); - if (!drm_queue) { + drm_queue_seq = amdgpu_drm_queue_alloc(crtc, client, id, + flipdata, + drmmode_flip_handler, + drmmode_flip_abort); + if (!drm_queue_seq) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, "Allocating DRM queue event entry failed.\n"); goto error; @@ -2571,13 +2571,13 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client, if (drmModePageFlip(pAMDGPUEnt->fd, drmmode_crtc->mode_crtc->crtc_id, drmmode->fb_id, DRM_MODE_PAGE_FLIP_EVENT, - drm_queue)) { + (void*)drm_queue_seq)) { xf86DrvMsg(scrn->scrnIndex, X_WARNING, "flip queue failed: %s\n", strerror(errno)); goto error; } drmmode_crtc->flip_pending = TRUE; - drm_queue = NULL; + drm_queue_seq = 0; } if (flipdata->flip_count > 0) @@ -2589,8 +2589,8 @@ error: drmmode->fb_id = flipdata->old_fb_id; } - if (drm_queue) - amdgpu_drm_abort_entry(drm_queue); + if (drm_queue_seq) + amdgpu_drm_abort_entry(drm_queue_seq); else if (crtc) drmmode_flip_abort(crtc, flipdata); else if (flipdata && flipdata->flip_count <= 1) -- 2.8.0.rc3 _______________________________________________ xorg-driver-ati mailing list [email protected] https://lists.x.org/mailman/listinfo/xorg-driver-ati
