On Thu, Mar 31, 2016 at 4:40 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. > > Signed-off-by: Michel Dänzer <[email protected]>
Reviewed-by: Alex Deucher <[email protected]> > --- > src/drmmode_display.c | 20 ++++++++++---------- > src/radeon_dri2.c | 47 ++++++++++++++++++++++++----------------------- > src/radeon_drm_queue.c | 24 ++++++++++++++++++------ > src/radeon_drm_queue.h | 12 +++++------- > src/radeon_kms.c | 36 ++++++++++++++++++------------------ > src/radeon_present.c | 17 +++++++++-------- > 6 files changed, 84 insertions(+), 72 deletions(-) > > diff --git a/src/drmmode_display.c b/src/drmmode_display.c > index 7331015..a368a41 100644 > --- a/src/drmmode_display.c > +++ b/src/drmmode_display.c > @@ -2660,7 +2660,7 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, ClientPtr > client, > int i; > uint32_t tiling_flags = 0; > drmmode_flipdata_ptr flipdata; > - struct radeon_drm_queue_entry *drm_queue = NULL; > + uintptr_t drm_queue_seq = 0; > > if (info->allowColorTiling) { > if (info->ChipFamily >= CHIP_FAMILY_R600) > @@ -2720,11 +2720,11 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, ClientPtr > client, > if (drmmode_crtc->hw_id == ref_crtc_hw_id) > flipdata->fe_crtc = crtc; > > - drm_queue = radeon_drm_queue_alloc(crtc, client, id, > - flipdata, > - drmmode_flip_handler, > - drmmode_flip_abort); > - if (!drm_queue) { > + drm_queue_seq = radeon_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; > @@ -2732,13 +2732,13 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, ClientPtr > client, > > if (drmModePageFlip(drmmode->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) > @@ -2750,8 +2750,8 @@ error: > drmmode->fb_id = flipdata->old_fb_id; > } > > - if (drm_queue) > - radeon_drm_abort_entry(drm_queue); > + if (drm_queue_seq) > + radeon_drm_abort_entry(drm_queue_seq); > else if (crtc) > drmmode_flip_abort(crtc, flipdata); > else if (flipdata && flipdata->flip_count <= 1) > diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c > index 73e09d0..84cd031 100644 > --- a/src/radeon_dri2.c > +++ b/src/radeon_dri2.c > @@ -516,7 +516,7 @@ typedef struct _DRI2FrameEvent { > unsigned frame; > xf86CrtcPtr crtc; > OsTimerPtr timer; > - struct radeon_drm_queue_entry *drm_queue; > + uintptr_t drm_queue_seq; > > /* for swaps & flips only */ > DRI2SwapEventPtr event_complete; > @@ -1079,8 +1079,8 @@ CARD32 radeon_dri2_deferred_event(OsTimerPtr timer, > CARD32 now, pointer data) > */ > if (!event_info->crtc) { > ErrorF("%s no crtc\n", __func__); > - if (event_info->drm_queue) > - radeon_drm_abort_entry(event_info->drm_queue); > + if (event_info->drm_queue_seq) > + radeon_drm_abort_entry(event_info->drm_queue_seq); > else > radeon_dri2_frame_event_abort(NULL, data); > return 0; > @@ -1092,9 +1092,9 @@ CARD32 radeon_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) > radeon_drm_queue_handler(info->dri2.drm_fd, 0, 0, 0, > - event_info->drm_queue); > + (void*)event_info->drm_queue_seq); > else > radeon_dri2_frame_event_handler(crtc, 0, 0, data); > return 0; > @@ -1108,9 +1108,10 @@ CARD32 radeon_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) > radeon_drm_queue_handler(info->dri2.drm_fd, frame, drm_now / 1000000, > - drm_now % 1000000, event_info->drm_queue); > + drm_now % 1000000, > + (void*)event_info->drm_queue_seq); > else > radeon_dri2_frame_event_handler(crtc, frame, drm_now, data); > return 0; > @@ -1141,7 +1142,7 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr > client, DrawablePtr draw, > ScrnInfoPtr scrn = xf86ScreenToScrn(screen); > RADEONInfoPtr info = RADEONPTR(scrn); > DRI2FrameEventPtr wait_info = NULL; > - struct radeon_drm_queue_entry *wait = NULL; > + uintptr_t drm_queue_seq = 0; > xf86CrtcPtr crtc = radeon_dri2_drawable_crtc(draw, TRUE); > uint32_t msc_delta; > drmVBlank vbl; > @@ -1197,15 +1198,15 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr > client, DrawablePtr draw, > current_msc = vbl.reply.sequence + msc_delta; > current_msc &= 0xffffffff; > > - wait = radeon_drm_queue_alloc(crtc, client, RADEON_DRM_QUEUE_ID_DEFAULT, > - wait_info, radeon_dri2_frame_event_handler, > - radeon_dri2_frame_event_abort); > - if (!wait) { > + drm_queue_seq = radeon_drm_queue_alloc(crtc, client, > RADEON_DRM_QUEUE_ID_DEFAULT, > + wait_info, > radeon_dri2_frame_event_handler, > + radeon_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, > @@ -1224,7 +1225,7 @@ static int radeon_dri2_schedule_wait_msc(ClientPtr > client, DrawablePtr draw, > vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT; > vbl.request.type |= radeon_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(info->dri2.drm_fd, &vbl); > if (ret) { > xf86DrvMsg(scrn->scrnIndex, X_WARNING, > @@ -1255,7 +1256,7 @@ static int radeon_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(info->dri2.drm_fd, &vbl); > if (ret) { > xf86DrvMsg(scrn->scrnIndex, X_WARNING, > @@ -1307,7 +1308,7 @@ static int radeon_dri2_schedule_swap(ClientPtr client, > DrawablePtr draw, > drmVBlank vbl; > int ret, flip = 0; > DRI2FrameEventPtr swap_info = NULL; > - struct radeon_drm_queue_entry *swap; > + uintptr_t drm_queue_seq; > CARD64 current_msc; > BoxRec box; > RegionRec region; > @@ -1344,15 +1345,15 @@ static int radeon_dri2_schedule_swap(ClientPtr > client, DrawablePtr draw, > swap_info->back = back; > swap_info->crtc = crtc; > > - swap = radeon_drm_queue_alloc(crtc, client, RADEON_DRM_QUEUE_ID_DEFAULT, > - swap_info, radeon_dri2_frame_event_handler, > - radeon_dri2_frame_event_abort); > - if (!swap) { > + drm_queue_seq = radeon_drm_queue_alloc(crtc, client, > RADEON_DRM_QUEUE_ID_DEFAULT, > + swap_info, > radeon_dri2_frame_event_handler, > + radeon_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 > @@ -1421,7 +1422,7 @@ static int radeon_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(info->dri2.drm_fd, &vbl); > if (ret) { > xf86DrvMsg(scrn->scrnIndex, X_WARNING, > @@ -1466,7 +1467,7 @@ static int radeon_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(info->dri2.drm_fd, &vbl); > if (ret) { > xf86DrvMsg(scrn->scrnIndex, X_WARNING, > diff --git a/src/radeon_drm_queue.c b/src/radeon_drm_queue.c > index 25fb183..0d999dd 100644 > --- a/src/radeon_drm_queue.c > +++ b/src/radeon_drm_queue.c > @@ -40,6 +40,7 @@ > struct radeon_drm_queue_entry { > struct xorg_list list; > uint64_t id; > + uintptr_t seq; > void *data; > ClientPtr client; > xf86CrtcPtr crtc; > @@ -49,6 +50,7 @@ struct radeon_drm_queue_entry { > > static int radeon_drm_queue_refcnt; > static struct xorg_list radeon_drm_queue; > +static uintptr_t radeon_drm_queue_seq; > > > /* > @@ -58,11 +60,11 @@ void > radeon_drm_queue_handler(int fd, unsigned int frame, unsigned int sec, > unsigned int usec, void *user_ptr) > { > - struct radeon_drm_queue_entry *user_data = user_ptr; > + uintptr_t seq = (uintptr_t)user_ptr; > struct radeon_drm_queue_entry *e, *tmp; > > xorg_list_for_each_entry_safe(e, tmp, &radeon_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 @@ radeon_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 radeon_drm_queue_entry * > +uintptr_t > radeon_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client, > uint64_t id, void *data, > radeon_drm_handler_proc handler, > @@ -92,6 +94,9 @@ radeon_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client, > if (!e) > return NULL; > > + if (!radeon_drm_queue_seq) > + radeon_drm_queue_seq = 1; > + e->seq = radeon_drm_queue_seq++; > e->client = client; > e->crtc = crtc; > e->id = id; > @@ -101,7 +106,7 @@ radeon_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client, > > xorg_list_add(&e->list, &radeon_drm_queue); > > - return e; > + return e->seq; > } > > /* > @@ -139,9 +144,16 @@ radeon_drm_abort_client(ClientPtr client) > * Abort specific drm queue entry > */ > void > -radeon_drm_abort_entry(struct radeon_drm_queue_entry *entry) > +radeon_drm_abort_entry(uintptr_t seq) > { > - radeon_drm_abort_one(entry); > + struct radeon_drm_queue_entry *e, *tmp; > + > + xorg_list_for_each_entry_safe(e, tmp, &radeon_drm_queue, list) { > + if (e->seq == seq) { > + radeon_drm_abort_one(e); > + break; > + } > + } > } > > /* > diff --git a/src/radeon_drm_queue.h b/src/radeon_drm_queue.h > index 5d8dedf..0d9d278 100644 > --- a/src/radeon_drm_queue.h > +++ b/src/radeon_drm_queue.h > @@ -41,14 +41,12 @@ typedef void (*radeon_drm_abort_proc)(xf86CrtcPtr crtc, > void *data); > void radeon_drm_queue_handler(int fd, unsigned int frame, > unsigned int tv_sec, unsigned int tv_usec, > void *user_ptr); > -struct radeon_drm_queue_entry *radeon_drm_queue_alloc(xf86CrtcPtr crtc, > - ClientPtr client, > - uint64_t id, > - void *data, > - radeon_drm_handler_proc > handler, > - radeon_drm_abort_proc > abort); > +uintptr_t radeon_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client, > + uint64_t id, void *data, > + radeon_drm_handler_proc handler, > + radeon_drm_abort_proc abort); > void radeon_drm_abort_client(ClientPtr client); > -void radeon_drm_abort_entry(struct radeon_drm_queue_entry *entry); > +void radeon_drm_abort_entry(uintptr_t seq); > void radeon_drm_abort_id(uint64_t id); > void radeon_drm_queue_init(); > void radeon_drm_queue_close(ScrnInfoPtr scrn); > diff --git a/src/radeon_kms.c b/src/radeon_kms.c > index 555d736..c5310ea 100644 > --- a/src/radeon_kms.c > +++ b/src/radeon_kms.c > @@ -491,7 +491,7 @@ static void > radeon_scanout_update(xf86CrtcPtr xf86_crtc) > { > drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private; > - struct radeon_drm_queue_entry *drm_queue_entry; > + uintptr_t drm_queue_seq; > ScrnInfoPtr scrn; > drmVBlank vbl; > DamagePtr pDamage; > @@ -520,13 +520,13 @@ radeon_scanout_update(xf86CrtcPtr xf86_crtc) > return; > > scrn = xf86_crtc->scrn; > - drm_queue_entry = radeon_drm_queue_alloc(xf86_crtc, > - RADEON_DRM_QUEUE_CLIENT_DEFAULT, > - RADEON_DRM_QUEUE_ID_DEFAULT, > - drmmode_crtc, > - radeon_scanout_update_handler, > - radeon_scanout_update_abort); > - if (!drm_queue_entry) { > + drm_queue_seq = radeon_drm_queue_alloc(xf86_crtc, > + RADEON_DRM_QUEUE_CLIENT_DEFAULT, > + RADEON_DRM_QUEUE_ID_DEFAULT, > + drmmode_crtc, > + radeon_scanout_update_handler, > + radeon_scanout_update_abort); > + if (!drm_queue_seq) { > xf86DrvMsg(scrn->scrnIndex, X_WARNING, > "radeon_drm_queue_alloc failed for scanout update\n"); > return; > @@ -535,12 +535,12 @@ radeon_scanout_update(xf86CrtcPtr xf86_crtc) > vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT; > vbl.request.type |= radeon_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(RADEONPTR(scrn)->dri2.drm_fd, &vbl)) { > xf86DrvMsg(scrn->scrnIndex, X_WARNING, > "drmWaitVBlank failed for scanout update: %s\n", > strerror(errno)); > - radeon_drm_abort_entry(drm_queue_entry); > + radeon_drm_abort_entry(drm_queue_seq); > return; > } > > @@ -562,7 +562,7 @@ radeon_scanout_flip(ScreenPtr pScreen, RADEONInfoPtr info, > { > drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private; > ScrnInfoPtr scrn; > - struct radeon_drm_queue_entry *drm_queue_entry; > + uintptr_t drm_queue_seq; > unsigned scanout_id; > > if (drmmode_crtc->scanout_update_pending) > @@ -573,12 +573,12 @@ radeon_scanout_flip(ScreenPtr pScreen, RADEONInfoPtr > info, > return; > > scrn = xf86_crtc->scrn; > - drm_queue_entry = radeon_drm_queue_alloc(xf86_crtc, > - RADEON_DRM_QUEUE_CLIENT_DEFAULT, > - RADEON_DRM_QUEUE_ID_DEFAULT, > - drmmode_crtc, NULL, > - radeon_scanout_flip_abort); > - if (!drm_queue_entry) { > + drm_queue_seq = radeon_drm_queue_alloc(xf86_crtc, > + RADEON_DRM_QUEUE_CLIENT_DEFAULT, > + RADEON_DRM_QUEUE_ID_DEFAULT, > + drmmode_crtc, NULL, > + radeon_scanout_flip_abort); > + if (!drm_queue_seq) { > xf86DrvMsg(scrn->scrnIndex, X_WARNING, > "Allocating DRM event queue entry failed.\n"); > return; > @@ -586,7 +586,7 @@ radeon_scanout_flip(ScreenPtr pScreen, RADEONInfoPtr info, > > if (drmModePageFlip(drmmode_crtc->drmmode->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/radeon_present.c b/src/radeon_present.c > index 3be3360..8988fc6 100644 > --- a/src/radeon_present.c > +++ b/src/radeon_present.c > @@ -156,7 +156,7 @@ radeon_present_queue_vblank(RRCrtcPtr crtc, uint64_t > event_id, uint64_t msc) > RADEONInfoPtr info = RADEONPTR(scrn); > int crtc_id = drmmode_get_crtc_id(xf86_crtc); > struct radeon_present_vblank_event *event; > - struct radeon_drm_queue_entry *queue; > + uintptr_t drm_queue_seq; > drmVBlank vbl; > int ret; > > @@ -164,24 +164,25 @@ radeon_present_queue_vblank(RRCrtcPtr crtc, uint64_t > event_id, uint64_t msc) > if (!event) > return BadAlloc; > event->event_id = event_id; > - queue = radeon_drm_queue_alloc(xf86_crtc, > RADEON_DRM_QUEUE_CLIENT_DEFAULT, > - event_id, event, > - radeon_present_vblank_handler, > - radeon_present_vblank_abort); > - if (!queue) { > + drm_queue_seq = radeon_drm_queue_alloc(xf86_crtc, > + RADEON_DRM_QUEUE_CLIENT_DEFAULT, > + event_id, event, > + radeon_present_vblank_handler, > + radeon_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(info->dri2.drm_fd, &vbl); > if (!ret) > break; > if (errno != EBUSY || !radeon_present_flush_drm_events(screen)) { > - radeon_drm_abort_entry(queue); > + radeon_drm_abort_entry(drm_queue_seq); > return BadAlloc; > } > } > -- > 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
