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...@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,
This is an ABI break.
{
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)
{
Same here. This interface is part of the ABI.
- 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)
Also an ABI break.
{
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))
ABI break.
These might make more logical sense, but changing them will require bumping the
ABI. Moreover, they follow the precedent of the non-sync
{Start/Stop}PixmapTracking, so if we change these, we should probably change
those too.
Thanks,
Alex
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
