[PATCH v2 xserver 3/5] modesetting: move common page flip handle to pageflip.c
StDenis, Tom
Tom.StDenis at amd.com
Fri Aug 19 11:32:56 UTC 2016
Sorry, disregard. The pointer doesn't point inside the struct. That part of the patch is fine.
Tom
________________________________
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of StDenis, Tom <Tom.StDenis at amd.com>
Sent: Friday, August 19, 2016 07:30
To: Yu, Qiang; xorg-devel at lists.x.org
Cc: michel at daenzer.net; emil.l.velikov at gmail.com; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH v2 xserver 3/5] modesetting: move common page flip handle to pageflip.c
In ms_pageflip_free() you cannot free the parent structure before freeing things it points to. That's undefined behaviour.
Tom
________________________________
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of Qiang Yu <Qiang.Yu at amd.com>
Sent: Friday, August 19, 2016 08:50
To: xorg-devel at lists.x.org
Cc: Yu, Qiang; michel at daenzer.net; emil.l.velikov at gmail.com; amd-gfx at lists.freedesktop.org
Subject: [PATCH v2 xserver 3/5] modesetting: move common page flip handle to pageflip.c
The common page flip handle framework can be shared with DRI2
page flip.
Signed-off-by: Qiang Yu <Qiang.Yu at amd.com>
---
hw/xfree86/drivers/modesetting/driver.h | 28 --------
hw/xfree86/drivers/modesetting/pageflip.c | 102 ++++++++++++++++++++++++++++--
hw/xfree86/drivers/modesetting/present.c | 63 +++---------------
3 files changed, 104 insertions(+), 89 deletions(-)
diff --git a/hw/xfree86/drivers/modesetting/driver.h b/hw/xfree86/drivers/modesetting/driver.h
index c0d80a8..38f0ec0 100644
--- a/hw/xfree86/drivers/modesetting/driver.h
+++ b/hw/xfree86/drivers/modesetting/driver.h
@@ -156,34 +156,6 @@ Bool ms_present_screen_init(ScreenPtr screen);
#ifdef GLAMOR
-/*
- * Event data for an in progress flip.
- * This contains a pointer to the vblank event,
- * and information about the flip in progress.
- * a reference to this is stored in the per-crtc
- * flips.
- */
-struct ms_flipdata {
- ScreenPtr screen;
- void *event;
- /* number of CRTC events referencing this */
- int flip_count;
- uint64_t fe_msc;
- uint64_t fe_usec;
- uint32_t old_fb_id;
-};
-
-/*
- * Per crtc pageflipping infomation,
- * These are submitted to the queuing code
- * one of them per crtc per flip.
- */
-struct ms_crtc_pageflip {
- Bool on_reference_crtc;
- /* reference to the ms_flipdata */
- struct ms_flipdata *flipdata;
-};
-
typedef void (*ms_pageflip_handler_proc)(uint64_t frame,
uint64_t usec,
void *data);
diff --git a/hw/xfree86/drivers/modesetting/pageflip.c b/hw/xfree86/drivers/modesetting/pageflip.c
index 4549792..5f35999 100644
--- a/hw/xfree86/drivers/modesetting/pageflip.c
+++ b/hw/xfree86/drivers/modesetting/pageflip.c
@@ -32,6 +32,97 @@
#ifdef GLAMOR
/*
+ * Event data for an in progress flip.
+ * This contains a pointer to the vblank event,
+ * and information about the flip in progress.
+ * a reference to this is stored in the per-crtc
+ * flips.
+ */
+struct ms_flipdata {
+ ScreenPtr screen;
+ void *event;
+ ms_pageflip_handler_proc event_handler;
+ ms_pageflip_abort_proc abort_handler;
+ /* number of CRTC events referencing this */
+ int flip_count;
+ uint64_t fe_msc;
+ uint64_t fe_usec;
+ uint32_t old_fb_id;
+};
+
+/*
+ * Per crtc pageflipping infomation,
+ * These are submitted to the queuing code
+ * one of them per crtc per flip.
+ */
+struct ms_crtc_pageflip {
+ Bool on_reference_crtc;
+ /* reference to the ms_flipdata */
+ struct ms_flipdata *flipdata;
+};
+
+/**
+ * Free an ms_crtc_pageflip.
+ *
+ * Drops the reference count on the flipdata.
+ */
+static void
+ms_pageflip_free(struct ms_crtc_pageflip *flip)
+{
+ struct ms_flipdata *flipdata = flip->flipdata;
+
+ free(flip);
+ if (--flipdata->flip_count > 0)
+ return;
+ free(flipdata);
+}
+
+/**
+ * Callback for the DRM event queue when a single flip has completed
+ *
+ * Once the flip has been completed on all pipes, notify the
+ * extension code telling it when that happened
+ */
+static void
+ms_pageflip_handler(uint64_t msc, uint64_t ust, void *data)
+{
+ struct ms_crtc_pageflip *flip = data;
+ struct ms_flipdata *flipdata = flip->flipdata;
+ ScreenPtr screen = flipdata->screen;
+ ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
+ modesettingPtr ms = modesettingPTR(scrn);
+
+ if (flip->on_reference_crtc) {
+ flipdata->fe_msc = msc;
+ flipdata->fe_usec = ust;
+ }
+
+ if (flipdata->flip_count == 1) {
+ flipdata->event_handler(flipdata->fe_msc,
+ flipdata->fe_usec,
+ flipdata->event);
+
+ drmModeRmFB(ms->fd, flipdata->old_fb_id);
+ }
+ ms_pageflip_free(flip);
+}
+
+/*
+ * Callback for the DRM queue abort code. A flip has been aborted.
+ */
+static void
+ms_pageflip_abort(void *data)
+{
+ struct ms_crtc_pageflip *flip = data;
+ struct ms_flipdata *flipdata = flip->flipdata;
+
+ if (flipdata->flip_count == 1)
+ flipdata->abort_handler(flipdata->event);
+
+ ms_pageflip_free(flip);
+}
+
+/*
* Flush the DRM event queue when full; makes space for new events.
*
* Returns a negative value on error, 0 if there was nothing to process,
@@ -68,9 +159,7 @@ ms_flush_drm_events(ScreenPtr screen)
static Bool
queue_flip_on_crtc(ScreenPtr screen, xf86CrtcPtr crtc,
struct ms_flipdata *flipdata,
- int ref_crtc_vblank_pipe, uint32_t flags,
- ms_pageflip_handler_proc pageflip_handler,
- ms_pageflip_abort_proc pageflip_abort)
+ int ref_crtc_vblank_pipe, uint32_t flags)
{
ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
modesettingPtr ms = modesettingPTR(scrn);
@@ -92,7 +181,7 @@ queue_flip_on_crtc(ScreenPtr screen, xf86CrtcPtr crtc,
flip->on_reference_crtc = (drmmode_crtc->vblank_pipe == ref_crtc_vblank_pipe);
flip->flipdata = flipdata;
- seq = ms_drm_queue_alloc(crtc, flip, pageflip_handler, pageflip_abort);
+ seq = ms_drm_queue_alloc(crtc, flip, ms_pageflip_handler, ms_pageflip_abort);
if (!seq) {
free(flip);
return FALSE;
@@ -164,6 +253,8 @@ ms_do_pageflip(ScreenPtr screen,
flipdata->event = event;
flipdata->screen = screen;
+ flipdata->event_handler = pageflip_handler;
+ flipdata->abort_handler = pageflip_abort;
/*
* Take a local reference on flipdata.
@@ -206,8 +297,7 @@ ms_do_pageflip(ScreenPtr screen,
if (!queue_flip_on_crtc(screen, crtc, flipdata,
ref_crtc_vblank_pipe,
- flags, pageflip_handler,
- pageflip_abort)) {
+ flags)) {
goto error_undo;
}
}
diff --git a/hw/xfree86/drivers/modesetting/present.c b/hw/xfree86/drivers/modesetting/present.c
index 18c82cd..deee1f1 100644
--- a/hw/xfree86/drivers/modesetting/present.c
+++ b/hw/xfree86/drivers/modesetting/present.c
@@ -192,77 +192,30 @@ ms_present_flush(WindowPtr window)
#ifdef GLAMOR
/**
- * Free an ms_crtc_pageflip.
- *
- * Drops the reference count on the flipdata.
- */
-static void
-ms_present_flip_free(struct ms_crtc_pageflip *flip)
-{
- struct ms_flipdata *flipdata = flip->flipdata;
-
- free(flip);
- if (--flipdata->flip_count > 0)
- return;
- free(flipdata);
-}
-
-/**
- * Callback for the DRM event queue when a single flip has completed
- *
- * Once the flip has been completed on all pipes, notify the
+ * Callback for the flip has been completed on all pipes, notify the
* extension code telling it when that happened
*/
static void
ms_present_flip_handler(uint64_t msc, uint64_t ust, void *data)
{
- struct ms_crtc_pageflip *flip = data;
- ScreenPtr screen = flip->flipdata->screen;
- ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
- modesettingPtr ms = modesettingPTR(scrn);
- struct ms_flipdata *flipdata = flip->flipdata;
+ struct ms_present_vblank_event *event = data;
- DebugPresent(("\t\tms:fh %lld c %d msc %llu ust %llu\n",
- (long long) flipdata->event->event_id,
- flipdata->flip_count,
+ DebugPresent(("\t\tms:fc %lld msc %llu ust %llu\n",
+ (long long) event->event_id,
(long long) msc, (long long) ust));
- if (flip->on_reference_crtc) {
- flipdata->fe_msc = msc;
- flipdata->fe_usec = ust;
- }
-
- if (flipdata->flip_count == 1) {
- DebugPresent(("\t\tms:fc %lld c %d msc %llu ust %llu\n",
- (long long) flipdata->event->event_id,
- flipdata->flip_count,
- (long long) flipdata->fe_msc, (long long) flipdata->fe_usec));
-
-
- ms_present_vblank_handler(flipdata->fe_msc,
- flipdata->fe_usec,
- flipdata->event);
-
- drmModeRmFB(ms->fd, flipdata->old_fb_id);
- }
- ms_present_flip_free(flip);
+ ms_present_vblank_handler(msc, ust, event);
}
/*
- * Callback for the DRM queue abort code. A flip has been aborted.
+ * Callback for the flip has been aborted.
*/
static void
ms_present_flip_abort(void *data)
{
- struct ms_crtc_pageflip *flip = data;
- struct ms_flipdata *flipdata = flip->flipdata;
-
- DebugPresent(("\t\tms:fa %lld c %d\n", (long long) flipdata->event->event_id, flipdata->flip_count));
-
- if (flipdata->flip_count == 1)
- free(flipdata->event);
+ struct ms_present_vblank_event *event = data;
- ms_present_flip_free(flip);
+ free(event);
}
/*
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - lists.freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org
To see the collection of prior postings to the list, visit the amd-gfx Archives. Using amd-gfx: To post a message to all the list members, send email ...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20160819/62b61612/attachment-0001.html>
More information about the amd-gfx
mailing list