<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;background-color:#FFFFFF;font-family:Calibri,Arial,Helvetica,sans-serif;">
<p>Sorry, disregard.  The pointer doesn't point inside the struct.  That part of the patch is fine.</p>
<p><br>
</p>
<p>Tom</p>
<br>
<br>
<div style="color: rgb(0, 0, 0);">
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of StDenis, Tom <Tom.StDenis@amd.com><br>
<b>Sent:</b> Friday, August 19, 2016 07:30<br>
<b>To:</b> Yu, Qiang; xorg-devel@lists.x.org<br>
<b>Cc:</b> michel@daenzer.net; emil.l.velikov@gmail.com; amd-gfx@lists.freedesktop.org<br>
<b>Subject:</b> Re: [PATCH v2 xserver 3/5] modesetting: move common page flip handle to pageflip.c</font>
<div> </div>
</div>
<div>
<div id="divtagdefaultwrapper" style="font-size:12pt; color:#000000; background-color:#FFFFFF; font-family:Calibri,Arial,Helvetica,sans-serif">
<p></p>
<div>In ms_pageflip_free() you cannot free the parent structure before freeing things it points to.  That's undefined behaviour. </div>
<div><br>
</div>
<div>Tom</div>
<p></p>
<div style="color:rgb(0,0,0)">
<div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Qiang Yu <Qiang.Yu@amd.com><br>
<b>Sent:</b> Friday, August 19, 2016 08:50<br>
<b>To:</b> xorg-devel@lists.x.org<br>
<b>Cc:</b> Yu, Qiang; michel@daenzer.net; emil.l.velikov@gmail.com; amd-gfx@lists.freedesktop.org<br>
<b>Subject:</b> [PATCH v2 xserver 3/5] modesetting: move common page flip handle to pageflip.c</font>
<div> </div>
</div>
</div>
<font size="2"><span style="font-size:10pt">
<div class="PlainText">The common page flip handle framework can be shared with DRI2<br>
page flip.<br>
<br>
Signed-off-by: Qiang Yu <Qiang.Yu@amd.com><br>
---<br>
 hw/xfree86/drivers/modesetting/driver.h   |  28 --------<br>
 hw/xfree86/drivers/modesetting/pageflip.c | 102 ++++++++++++++++++++++++++++--<br>
 hw/xfree86/drivers/modesetting/present.c  |  63 +++---------------<br>
 3 files changed, 104 insertions(+), 89 deletions(-)<br>
<br>
diff --git a/hw/xfree86/drivers/modesetting/driver.h b/hw/xfree86/drivers/modesetting/driver.h<br>
index c0d80a8..38f0ec0 100644<br>
--- a/hw/xfree86/drivers/modesetting/driver.h<br>
+++ b/hw/xfree86/drivers/modesetting/driver.h<br>
@@ -156,34 +156,6 @@ Bool ms_present_screen_init(ScreenPtr screen);<br>
 <br>
 #ifdef GLAMOR<br>
 <br>
-/*<br>
- * Event data for an in progress flip.<br>
- * This contains a pointer to the vblank event,<br>
- * and information about the flip in progress.<br>
- * a reference to this is stored in the per-crtc<br>
- * flips.<br>
- */<br>
-struct ms_flipdata {<br>
-    ScreenPtr screen;<br>
-    void *event;<br>
-    /* number of CRTC events referencing this */<br>
-    int flip_count;<br>
-    uint64_t fe_msc;<br>
-    uint64_t fe_usec;<br>
-    uint32_t old_fb_id;<br>
-};<br>
-<br>
-/*<br>
- * Per crtc pageflipping infomation,<br>
- * These are submitted to the queuing code<br>
- * one of them per crtc per flip.<br>
- */<br>
-struct ms_crtc_pageflip {<br>
-    Bool on_reference_crtc;<br>
-    /* reference to the ms_flipdata */<br>
-    struct ms_flipdata *flipdata;<br>
-};<br>
-<br>
 typedef void (*ms_pageflip_handler_proc)(uint64_t frame,<br>
                                          uint64_t usec,<br>
                                          void *data);<br>
diff --git a/hw/xfree86/drivers/modesetting/pageflip.c b/hw/xfree86/drivers/modesetting/pageflip.c<br>
index 4549792..5f35999 100644<br>
--- a/hw/xfree86/drivers/modesetting/pageflip.c<br>
+++ b/hw/xfree86/drivers/modesetting/pageflip.c<br>
@@ -32,6 +32,97 @@<br>
 #ifdef GLAMOR<br>
 <br>
 /*<br>
+ * Event data for an in progress flip.<br>
+ * This contains a pointer to the vblank event,<br>
+ * and information about the flip in progress.<br>
+ * a reference to this is stored in the per-crtc<br>
+ * flips.<br>
+ */<br>
+struct ms_flipdata {<br>
+    ScreenPtr screen;<br>
+    void *event;<br>
+    ms_pageflip_handler_proc event_handler;<br>
+    ms_pageflip_abort_proc abort_handler;<br>
+    /* number of CRTC events referencing this */<br>
+    int flip_count;<br>
+    uint64_t fe_msc;<br>
+    uint64_t fe_usec;<br>
+    uint32_t old_fb_id;<br>
+};<br>
+<br>
+/*<br>
+ * Per crtc pageflipping infomation,<br>
+ * These are submitted to the queuing code<br>
+ * one of them per crtc per flip.<br>
+ */<br>
+struct ms_crtc_pageflip {<br>
+    Bool on_reference_crtc;<br>
+    /* reference to the ms_flipdata */<br>
+    struct ms_flipdata *flipdata;<br>
+};<br>
+<br>
+/**<br>
+ * Free an ms_crtc_pageflip.<br>
+ *<br>
+ * Drops the reference count on the flipdata.<br>
+ */<br>
+static void<br>
+ms_pageflip_free(struct ms_crtc_pageflip *flip)<br>
+{<br>
+    struct ms_flipdata *flipdata = flip->flipdata;<br>
+<br>
+    free(flip);<br>
+    if (--flipdata->flip_count > 0)<br>
+        return;<br>
+    free(flipdata);<br>
+}<br>
+<br>
+/**<br>
+ * Callback for the DRM event queue when a single flip has completed<br>
+ *<br>
+ * Once the flip has been completed on all pipes, notify the<br>
+ * extension code telling it when that happened<br>
+ */<br>
+static void<br>
+ms_pageflip_handler(uint64_t msc, uint64_t ust, void *data)<br>
+{<br>
+    struct ms_crtc_pageflip *flip = data;<br>
+    struct ms_flipdata *flipdata = flip->flipdata;<br>
+    ScreenPtr screen = flipdata->screen;<br>
+    ScrnInfoPtr scrn = xf86ScreenToScrn(screen);<br>
+    modesettingPtr ms = modesettingPTR(scrn);<br>
+<br>
+    if (flip->on_reference_crtc) {<br>
+        flipdata->fe_msc = msc;<br>
+        flipdata->fe_usec = ust;<br>
+    }<br>
+<br>
+    if (flipdata->flip_count == 1) {<br>
+        flipdata->event_handler(flipdata->fe_msc,<br>
+                                flipdata->fe_usec,<br>
+                                flipdata->event);<br>
+<br>
+        drmModeRmFB(ms->fd, flipdata->old_fb_id);<br>
+    }<br>
+    ms_pageflip_free(flip);<br>
+}<br>
+<br>
+/*<br>
+ * Callback for the DRM queue abort code.  A flip has been aborted.<br>
+ */<br>
+static void<br>
+ms_pageflip_abort(void *data)<br>
+{<br>
+    struct ms_crtc_pageflip *flip = data;<br>
+    struct ms_flipdata *flipdata = flip->flipdata;<br>
+<br>
+    if (flipdata->flip_count == 1)<br>
+        flipdata->abort_handler(flipdata->event);<br>
+<br>
+    ms_pageflip_free(flip);<br>
+}<br>
+<br>
+/*<br>
  * Flush the DRM event queue when full; makes space for new events.<br>
  *<br>
  * Returns a negative value on error, 0 if there was nothing to process,<br>
@@ -68,9 +159,7 @@ ms_flush_drm_events(ScreenPtr screen)<br>
 static Bool<br>
 queue_flip_on_crtc(ScreenPtr screen, xf86CrtcPtr crtc,<br>
                    struct ms_flipdata *flipdata,<br>
-                   int ref_crtc_vblank_pipe, uint32_t flags,<br>
-                   ms_pageflip_handler_proc pageflip_handler,<br>
-                   ms_pageflip_abort_proc pageflip_abort)<br>
+                   int ref_crtc_vblank_pipe, uint32_t flags)<br>
 {<br>
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);<br>
     modesettingPtr ms = modesettingPTR(scrn);<br>
@@ -92,7 +181,7 @@ queue_flip_on_crtc(ScreenPtr screen, xf86CrtcPtr crtc,<br>
     flip->on_reference_crtc = (drmmode_crtc->vblank_pipe == ref_crtc_vblank_pipe);<br>
     flip->flipdata = flipdata;<br>
 <br>
-    seq = ms_drm_queue_alloc(crtc, flip, pageflip_handler, pageflip_abort);<br>
+    seq = ms_drm_queue_alloc(crtc, flip, ms_pageflip_handler, ms_pageflip_abort);<br>
     if (!seq) {<br>
         free(flip);<br>
         return FALSE;<br>
@@ -164,6 +253,8 @@ ms_do_pageflip(ScreenPtr screen,<br>
 <br>
     flipdata->event = event;<br>
     flipdata->screen = screen;<br>
+    flipdata->event_handler = pageflip_handler;<br>
+    flipdata->abort_handler = pageflip_abort;<br>
 <br>
     /*<br>
      * Take a local reference on flipdata.<br>
@@ -206,8 +297,7 @@ ms_do_pageflip(ScreenPtr screen,<br>
 <br>
         if (!queue_flip_on_crtc(screen, crtc, flipdata,<br>
                                 ref_crtc_vblank_pipe,<br>
-                                flags, pageflip_handler,<br>
-                                pageflip_abort)) {<br>
+                                flags)) {<br>
             goto error_undo;<br>
         }<br>
     }<br>
diff --git a/hw/xfree86/drivers/modesetting/present.c b/hw/xfree86/drivers/modesetting/present.c<br>
index 18c82cd..deee1f1 100644<br>
--- a/hw/xfree86/drivers/modesetting/present.c<br>
+++ b/hw/xfree86/drivers/modesetting/present.c<br>
@@ -192,77 +192,30 @@ ms_present_flush(WindowPtr window)<br>
 #ifdef GLAMOR<br>
 <br>
 /**<br>
- * Free an ms_crtc_pageflip.<br>
- *<br>
- * Drops the reference count on the flipdata.<br>
- */<br>
-static void<br>
-ms_present_flip_free(struct ms_crtc_pageflip *flip)<br>
-{<br>
-    struct ms_flipdata *flipdata = flip->flipdata;<br>
-<br>
-    free(flip);<br>
-    if (--flipdata->flip_count > 0)<br>
-        return;<br>
-    free(flipdata);<br>
-}<br>
-<br>
-/**<br>
- * Callback for the DRM event queue when a single flip has completed<br>
- *<br>
- * Once the flip has been completed on all pipes, notify the<br>
+ * Callback for the flip has been completed on all pipes, notify the<br>
  * extension code telling it when that happened<br>
  */<br>
 static void<br>
 ms_present_flip_handler(uint64_t msc, uint64_t ust, void *data)<br>
 {<br>
-    struct ms_crtc_pageflip *flip = data;<br>
-    ScreenPtr screen = flip->flipdata->screen;<br>
-    ScrnInfoPtr scrn = xf86ScreenToScrn(screen);<br>
-    modesettingPtr ms = modesettingPTR(scrn);<br>
-    struct ms_flipdata *flipdata = flip->flipdata;<br>
+    struct ms_present_vblank_event *event = data;<br>
 <br>
-    DebugPresent(("\t\tms:fh %lld c %d msc %llu ust %llu\n",<br>
-                  (long long) flipdata->event->event_id,<br>
-                  flipdata->flip_count,<br>
+    DebugPresent(("\t\tms:fc %lld msc %llu ust %llu\n",<br>
+                  (long long) event->event_id,<br>
                   (long long) msc, (long long) ust));<br>
 <br>
-    if (flip->on_reference_crtc) {<br>
-        flipdata->fe_msc = msc;<br>
-        flipdata->fe_usec = ust;<br>
-    }<br>
-<br>
-    if (flipdata->flip_count == 1) {<br>
-        DebugPresent(("\t\tms:fc %lld c %d msc %llu ust %llu\n",<br>
-                      (long long) flipdata->event->event_id,<br>
-                      flipdata->flip_count,<br>
-                      (long long) flipdata->fe_msc, (long long) flipdata->fe_usec));<br>
-<br>
-<br>
-        ms_present_vblank_handler(flipdata->fe_msc,<br>
-                                  flipdata->fe_usec,<br>
-                                  flipdata->event);<br>
-<br>
-        drmModeRmFB(ms->fd, flipdata->old_fb_id);<br>
-    }<br>
-    ms_present_flip_free(flip);<br>
+    ms_present_vblank_handler(msc, ust, event);<br>
 }<br>
 <br>
 /*<br>
- * Callback for the DRM queue abort code.  A flip has been aborted.<br>
+ * Callback for the flip has been aborted.<br>
  */<br>
 static void<br>
 ms_present_flip_abort(void *data)<br>
 {<br>
-    struct ms_crtc_pageflip *flip = data;<br>
-    struct ms_flipdata *flipdata = flip->flipdata;<br>
-<br>
-    DebugPresent(("\t\tms:fa %lld c %d\n", (long long) flipdata->event->event_id, flipdata->flip_count));<br>
-<br>
-    if (flipdata->flip_count == 1)<br>
-        free(flipdata->event);<br>
+    struct ms_present_vblank_event *event = data;<br>
 <br>
-    ms_present_flip_free(flip);<br>
+    free(event);<br>
 }<br>
 <br>
 /*<br>
-- <br>
2.7.4<br>
<br>
_______________________________________________<br>
amd-gfx mailing list<br>
amd-gfx@lists.freedesktop.org<br>
<a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" id="LPlnk437166">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a>
<div id="LPBorder_GT_14716061893940.8420357393682676" style="margin-bottom:20px; overflow:auto; width:100%; text-indent:0px">
<table id="LPContainer_14716061893930.7108288203263247" cellspacing="0" style="width:90%; overflow:auto; padding-top:20px; padding-bottom:20px; margin-top:20px; border-top:1px dotted rgb(200,200,200); border-bottom:1px dotted rgb(200,200,200); background-color:rgb(255,255,255)">
<tbody>
<tr valign="top" style="border-spacing:0px">
<td id="TextCell_14716061893930.9899862651724711" colspan="2" style="vertical-align: top; padding: 0px; display: table-cell; position: relative;">
<div id="LPRemovePreviewContainer_14716061893930.47806653578636316"></div>
<div id="LPTitle_14716061893930.16493228848820385" style="top:0px; color:rgb(59,87,119); font-weight:normal; font-size:21px; font-family:wf_segoe-ui_light,"Segoe UI Light","Segoe WP Light","Segoe UI","Segoe WP",Tahoma,Arial,sans-serif; line-height:21px">
<a id="LPUrlAnchor_14716061893930.6212120889357053" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" target="_blank" style="text-decoration:none">amd-gfx Info Page - lists.freedesktop.org</a></div>
<div id="LPMetadata_14716061893930.2849713450368425" style="margin:10px 0px 16px; color:rgb(102,102,102); font-weight:normal; font-family:wf_segoe-ui_normal,"Segoe UI","Segoe WP",Tahoma,Arial,sans-serif; font-size:14px; line-height:14px">
lists.freedesktop.org</div>
<div id="LPDescription_14716061893940.8733822341303554" style="display:block; color:rgb(102,102,102); font-weight:normal; font-family:wf_segoe-ui_normal,"Segoe UI","Segoe WP",Tahoma,Arial,sans-serif; font-size:14px; line-height:20px; max-height:100px; overflow:hidden">
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 ...</div>
</td>
</tr>
</tbody>
</table>
</div>
<br>
<br>
</div>
</span></font></div>
</div>
</div>
</div>
</div>
</body>
</html>