On 2018-08-03 03:27 PM, Jim Qu wrote: > On some Intel iGPU + AMD dGPU platform, when connect extern display > from dGPU, X will crash, 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) > > There is NULL pointer accessing on ent->slave_dst->drawable.pScreen-> > SharedPixmapNotifyDamage. > > On the platform, since the dGPU is GPU device, so that the iGPU is > output master device. SharedPixmapNotifyDamage() should be called when > current device is output slave. > > Change-Id: Id633e29c126670ee64ff1f5f79d489e5068cd439 > Signed-off-by: Jim Qu <jim...@amd.com> > --- > hw/xfree86/drivers/modesetting/driver.c | 23 ++++++++++++----------- > 1 file changed, 12 insertions(+), 11 deletions(-) > > diff --git a/hw/xfree86/drivers/modesetting/driver.c > b/hw/xfree86/drivers/modesetting/driver.c > index 9362370..6022315 100644 > --- a/hw/xfree86/drivers/modesetting/driver.c > +++ b/hw/xfree86/drivers/modesetting/driver.c > @@ -640,20 +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); > > - 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); > - } > - > - /* Requested manual updating */ > - if (ppriv->defer_dirty_update) > - continue; > + ent->slave_dst->drawable.pScreen-> > + SharedPixmapNotifyDamage(ent->slave_dst); > + } > > + /* Requested manual updating */ > + if (ppriv->defer_dirty_update) > + continue; > + }
I don't think this is right. E.g. why would the slave driver call its own SharedPixmapNotifyDamage hook? Also, ppriv->defer_dirty_update is only set to TRUE by hooks which are only called for the master screen. So, I think the start of the new block needs to be something like: if (!screen->isGPU) { msPixmapPrivPtr ppriv = msGetPixmapPriv(&ms->drmmode, ent->slave_dst->master_pixmap); and msStartFlippingPixmapTracking and msStopFlippingPixmapTracking also need to use ->master_pixmap instead of the slave pixmaps. Not sure about the defer_dirty_update related code, that might only make sense in the slave. The more I look at this PRIME synchronization related code, the more I wonder if it was ever tested with master and slave screens using different drivers... -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel