[PATCH] modesetting: code refactor for PRIME sync

Jim Qu Jim.Qu at amd.com
Fri Aug 24 03:30:58 UTC 2018


The PRIME sync on modesetting assume that all two GPUs are used
modesetting driver on the hybrid system. The X will be crashed
on the system with other DDX driver, such as amdgpu.

show the log like:

randr: falling back to unsynchronized pixmap sharing
(EE)
(EE) Backtrace:
(EE) 0: /usr/lib/xorg/Xorg (xorg_backtrace+0x4e)
(EE) 1: /usr/lib/xorg/Xorg (0x55cb0151a000+0x1b5ce9)
(EE) 2: /lib/x86_64-linux-gnu/libpthread.so.0 (0x7f1587a1d000+0x11390)
(EE)
(EE) Segmentation fault at address 0x0
(EE)

The issue is that modesetting as the master, and amdgpu as the slave.
Thus, when the master attempts to access pSlavePixPriv in ms_dirty_update(),
problems result due to the fact that it's accessing AMD's 'ppriv' using the
modesetting structure definition.

Apart from fixing crash issue, the patch also try to resolve the dependence
between master interface and slave interface, that say driver can not assume
that the other also use modesetting.

To make the logic cleanly, the slave always input master pixmap to master
interfaces, master can refer the slave pixmap in master driver if it need.
there is an exception, master->StartPixmapTracking()/StopPixmapTracking,
the backend PixmapStartDirtyTracking()/PixmapStopDirtyTracking() are used by
other vendors, so it can be refined in next step.

Signed-off-by: Jim Qu <Jim.Qu at amd.com>

Change-Id: I4398ff85bb69848cacda1d20db960283bcc526b5
---
 dix/pixmap.c                                     |  1 +
 fb/fbpixmap.c                                    |  1 +
 hw/xfree86/drivers/modesetting/driver.c          | 54 ++++++++++++------------
 hw/xfree86/drivers/modesetting/drmmode_display.c |  4 +-
 include/pixmapstr.h                              |  1 +
 randr/rrcrtc.c                                   | 10 ++---
 6 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/dix/pixmap.c b/dix/pixmap.c
index 81ac5e2..ee5ad73 100644
--- a/dix/pixmap.c
+++ b/dix/pixmap.c
@@ -162,6 +162,7 @@ PixmapPtr PixmapShareToSlave(PixmapPtr pixmap, ScreenPtr slave)
     pixmap->refcnt++;
 
     spix->master_pixmap = pixmap;
+    pixmap->slave_pixmap = spix;
 
     ret = slave->SetSharedPixmapBacking(spix, handle);
     if (ret == FALSE) {
diff --git a/fb/fbpixmap.c b/fb/fbpixmap.c
index a524200..982af3f 100644
--- a/fb/fbpixmap.c
+++ b/fb/fbpixmap.c
@@ -69,6 +69,7 @@ fbCreatePixmap(ScreenPtr pScreen, int width, int height, int depth,
     pPixmap->refcnt = 1;
     pPixmap->devPrivate.ptr = (void *) ((char *) pPixmap + base + adjust);
     pPixmap->master_pixmap = NULL;
+    pPixmap->slave_pixmap = NULL;
 
 #ifdef FB_DEBUG
     pPixmap->devPrivate.ptr =
diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c
index 9362370..bccdb20 100644
--- a/hw/xfree86/drivers/modesetting/driver.c
+++ b/hw/xfree86/drivers/modesetting/driver.c
@@ -640,19 +640,21 @@ ms_dirty_update(ScreenPtr screen, int *timeout)
     xorg_list_for_each_entry(ent, &screen->pixmap_dirty_list, ent) {
         region = DamageRegion(ent->damage);
         if (RegionNotEmpty(region)) {
-            msPixmapPrivPtr ppriv =
-                msGetPixmapPriv(&ms->drmmode, ent->slave_dst);
+            if (!screen->isGPU) {
+                msPixmapPrivPtr ppriv =
+                    msGetPixmapPriv(&ms->drmmode, ent->slave_dst->master_pixmap);
 
-            if (ppriv->notify_on_damage) {
-                ppriv->notify_on_damage = FALSE;
+                if (ppriv->notify_on_damage) {
+                    ppriv->notify_on_damage = FALSE;
 
-                ent->slave_dst->drawable.pScreen->
-                    SharedPixmapNotifyDamage(ent->slave_dst);
-            }
+                    ent->slave_dst->drawable.pScreen->
+                        SharedPixmapNotifyDamage(ent->slave_dst);
+                }
 
-            /* Requested manual updating */
-            if (ppriv->defer_dirty_update)
-                continue;
+                /* Requested manual updating */
+                if (ppriv->defer_dirty_update)
+                    continue;
+            }
 
             redisplay_dirty(screen, ent, timeout);
             DamageEmpty(ent->damage);
@@ -1244,32 +1246,32 @@ msDisableSharedPixmapFlipping(RRCrtcPtr crtc)
 
 static Bool
 msStartFlippingPixmapTracking(RRCrtcPtr crtc, DrawablePtr src,
-                              PixmapPtr slave_dst1, PixmapPtr slave_dst2,
+                              PixmapPtr master1, PixmapPtr master2,
                               int x, int y, int dst_x, int dst_y,
                               Rotation rotation)
 {
     ScreenPtr pScreen = src->pScreen;
     modesettingPtr ms = modesettingPTR(xf86ScreenToScrn(pScreen));
 
-    msPixmapPrivPtr ppriv1 = msGetPixmapPriv(&ms->drmmode, slave_dst1),
-                    ppriv2 = msGetPixmapPriv(&ms->drmmode, slave_dst2);
+    msPixmapPrivPtr ppriv1 = msGetPixmapPriv(&ms->drmmode, master1),
+                    ppriv2 = msGetPixmapPriv(&ms->drmmode, master2);
 
-    if (!PixmapStartDirtyTracking(src, slave_dst1, x, y,
+    if (!PixmapStartDirtyTracking(src, master1->slave_pixmap, x, y,
                                   dst_x, dst_y, rotation)) {
         return FALSE;
     }
 
-    if (!PixmapStartDirtyTracking(src, slave_dst2, x, y,
+    if (!PixmapStartDirtyTracking(src, master2->slave_pixmap, x, y,
                                   dst_x, dst_y, rotation)) {
-        PixmapStopDirtyTracking(src, slave_dst1);
+        PixmapStopDirtyTracking(src, master1->slave_pixmap);
         return FALSE;
     }
 
     ppriv1->slave_src = src;
     ppriv2->slave_src = src;
 
-    ppriv1->dirty = ms_dirty_get_ent(pScreen, slave_dst1);
-    ppriv2->dirty = ms_dirty_get_ent(pScreen, slave_dst2);
+    ppriv1->dirty = ms_dirty_get_ent(pScreen, master1->slave_pixmap);
+    ppriv2->dirty = ms_dirty_get_ent(pScreen, master2->slave_pixmap);
 
     ppriv1->defer_dirty_update = TRUE;
     ppriv2->defer_dirty_update = TRUE;
@@ -1278,12 +1280,12 @@ msStartFlippingPixmapTracking(RRCrtcPtr crtc, DrawablePtr src,
 }
 
 static Bool
-msPresentSharedPixmap(PixmapPtr slave_dst)
+msPresentSharedPixmap(PixmapPtr master)
 {
-    ScreenPtr pScreen = slave_dst->drawable.pScreen;
+    ScreenPtr pScreen = master->drawable.pScreen;
     modesettingPtr ms = modesettingPTR(xf86ScreenToScrn(pScreen));
 
-    msPixmapPrivPtr ppriv = msGetPixmapPriv(&ms->drmmode, slave_dst);
+    msPixmapPrivPtr ppriv = msGetPixmapPriv(&ms->drmmode, master);
 
     RegionPtr region = DamageRegion(ppriv->dirty->damage);
 
@@ -1299,18 +1301,18 @@ msPresentSharedPixmap(PixmapPtr slave_dst)
 
 static Bool
 msStopFlippingPixmapTracking(DrawablePtr src,
-                             PixmapPtr slave_dst1, PixmapPtr slave_dst2)
+                             PixmapPtr master1, PixmapPtr master2)
 {
     ScreenPtr pScreen = src->pScreen;
     modesettingPtr ms = modesettingPTR(xf86ScreenToScrn(pScreen));
 
-    msPixmapPrivPtr ppriv1 = msGetPixmapPriv(&ms->drmmode, slave_dst1),
-                    ppriv2 = msGetPixmapPriv(&ms->drmmode, slave_dst2);
+    msPixmapPrivPtr ppriv1 = msGetPixmapPriv(&ms->drmmode, master1),
+                    ppriv2 = msGetPixmapPriv(&ms->drmmode, master2);
 
     Bool ret = TRUE;
 
-    ret &= PixmapStopDirtyTracking(src, slave_dst1);
-    ret &= PixmapStopDirtyTracking(src, slave_dst2);
+    ret &= PixmapStopDirtyTracking(src, master1->slave_pixmap);
+    ret &= PixmapStopDirtyTracking(src, master2->slave_pixmap);
 
     if (ret) {
         ppriv1->slave_src = NULL;
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
index f6f2e9f..5525383 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -1073,7 +1073,7 @@ drmmode_SharedPixmapPresent(PixmapPtr ppix, xf86CrtcPtr crtc,
 {
     ScreenPtr master = crtc->randr_crtc->pScreen->current_master;
 
-    if (master->PresentSharedPixmap(ppix)) {
+    if (master->PresentSharedPixmap(ppix->master_pixmap)) {
         /* Success, queue flip to back target */
         if (drmmode_SharedPixmapFlip(ppix, crtc, drmmode))
             return TRUE;
@@ -1091,7 +1091,7 @@ drmmode_SharedPixmapPresent(PixmapPtr ppix, xf86CrtcPtr crtc,
         /* Set flag first in case we are immediately notified */
         ppriv->wait_for_damage = TRUE;
 
-        if (master->RequestSharedPixmapNotifyDamage(ppix))
+        if (master->RequestSharedPixmapNotifyDamage(ppix->master_pixmap))
             return TRUE;
         else
             ppriv->wait_for_damage = FALSE;
diff --git a/include/pixmapstr.h b/include/pixmapstr.h
index de50101..d572ae3 100644
--- a/include/pixmapstr.h
+++ b/include/pixmapstr.h
@@ -85,6 +85,7 @@ typedef struct _Pixmap {
     unsigned usage_hint;        /* see CREATE_PIXMAP_USAGE_* */
 
     PixmapPtr master_pixmap;    /* pointer to master copy of pixmap for pixmap sharing */
+    PixmapPtr slave_pixmap;  /* pointer to slave copy of pixmap for pixmap sharing */
 } PixmapRec;
 
 typedef struct _PixmapDirtyUpdate {
diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
index 5d90262..f7463ff 100644
--- a/randr/rrcrtc.c
+++ b/randr/rrcrtc.c
@@ -402,8 +402,8 @@ RRCrtcDetachScanoutPixmap(RRCrtcPtr crtc)
             pScrPriv->rrDisableSharedPixmapFlipping(crtc);
 
             master->StopFlippingPixmapTracking(mrootdraw,
-                                               crtc->scanout_pixmap,
-                                               crtc->scanout_pixmap_back);
+                                               crtc->scanout_pixmap->master_pixmap,
+                                               crtc->scanout_pixmap_back->master_pixmap);
 
             rrDestroySharedPixmap(crtc, crtc->scanout_pixmap_back);
             crtc->scanout_pixmap_back = NULL;
@@ -561,15 +561,15 @@ rrSetupPixmapSharing(RRCrtcPtr crtc, int width, int height,
 
         if (!pMasterScrPriv->rrStartFlippingPixmapTracking(crtc,
                                                            mrootdraw,
-                                                           spix_front,
-                                                           spix_back,
+                                                           spix_front->master_pixmap,
+                                                           spix_back->master_pixmap,
                                                            x, y, 0, 0,
                                                            rotation)) {
             pSlaveScrPriv->rrDisableSharedPixmapFlipping(crtc);
             goto fail;
         }
 
-        master->PresentSharedPixmap(spix_front);
+        master->PresentSharedPixmap(spix_front->master_pixmap);
 
         return TRUE;
 
-- 
2.7.4



More information about the xorg-devel mailing list