On Fri, Apr 1, 2016 at 2:54 AM, Michel Dänzer <[email protected]> wrote: > 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]>
Reviewed-by: Alex Deucher <[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 _______________________________________________ xorg-driver-ati mailing list [email protected] https://lists.x.org/mailman/listinfo/xorg-driver-ati
