[V2] modesetting: code refactor for PRIME sync

[jimqu]

On 2018年08月29日 09:20, Alex Goins wrote:
> I'm still having issues with my local setup for testing it with
> modesetting<->modesetting PRIME, but I think it looks good by inspection, and
> would expect it to work. It does work with NVIDIA<->modesetting PRIME.
>
> It would be a good idea to test modesetting<->modesetting PRIME Sync with this
> patch before merging it, but I don't have any further objections.
>
> Reviewed-by: Alex Goins <agoins at nvidia.com>
>
> Thanks!
> Alex

Thanks very much! Alex.

I tested it on Intel(modesetting) + AMD(modesetting/amdgpu) and 
Intel(modesetting)+NV(modesetting/nouveau), PRIME, reverse PRIME and 
also PRIME offloading cases.
For the NV card, I don't know what issue you encountered,  In my side , 
at first, I found a GTX690, but PRIME can not work duo to there were two 
DRM device nodes from NV card, so there are totally three screens(one 
master and two GPUs) on the system. then I found a GTX500, it had only 
one device node.

Did you encounter the same issue?

BTW, could you do me a favor to help me push the patch to the master or 
other proper branch? I do not familiar with the processes that push the 
patch to Xorg.

Thanks
JimQu
>> Hi AlexG,
>>
>> What's about new patch?
>>
>> Thanks
>> JimQu
>>
>>
>> 在 2018/8/27 13:37, Jim Qu 写道:
>>> 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 fix other issue in master interface
>>> in which driver should refer to master pixmap.
>>>
>>> Change-Id: I274633c9657c3621c95efd7632355662b9f5052c
>>> Signed-off-by: Jim Qu <Jim.Qu at amd.com>
>>> ---
>>>    hw/xfree86/drivers/modesetting/driver.c | 36
>>> +++++++++++++++++----------------
>>>    1 file changed, 19 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/hw/xfree86/drivers/modesetting/driver.c
>>> b/hw/xfree86/drivers/modesetting/driver.c
>>> index 9362370..792dfb6 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);
>>> @@ -1251,8 +1253,8 @@ msStartFlippingPixmapTracking(RRCrtcPtr crtc,
>>> DrawablePtr src,
>>>        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,
>>> slave_dst1->master_pixmap),
>>> +                    ppriv2 = msGetPixmapPriv(&ms->drmmode,
>>> slave_dst2->master_pixmap);
>>>          if (!PixmapStartDirtyTracking(src, slave_dst1, x, y,
>>>                                      dst_x, dst_y, rotation)) {
>>> @@ -1280,10 +1282,10 @@ msStartFlippingPixmapTracking(RRCrtcPtr crtc,
>>> DrawablePtr src,
>>>    static Bool
>>>    msPresentSharedPixmap(PixmapPtr slave_dst)
>>>    {
>>> -    ScreenPtr pScreen = slave_dst->drawable.pScreen;
>>> +    ScreenPtr pScreen = slave_dst->master_pixmap->drawable.pScreen;
>>>        modesettingPtr ms = modesettingPTR(xf86ScreenToScrn(pScreen));
>>>    -    msPixmapPrivPtr ppriv = msGetPixmapPriv(&ms->drmmode, slave_dst);
>>> +    msPixmapPrivPtr ppriv = msGetPixmapPriv(&ms->drmmode,
>>> slave_dst->master_pixmap);
>>>          RegionPtr region = DamageRegion(ppriv->dirty->damage);
>>>    @@ -1304,8 +1306,8 @@ msStopFlippingPixmapTracking(DrawablePtr src,
>>>        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,
>>> slave_dst1->master_pixmap),
>>> +                    ppriv2 = msGetPixmapPriv(&ms->drmmode,
>>> slave_dst2->master_pixmap);
>>>          Bool ret = TRUE;
>>>    @@ -1471,7 +1473,7 @@ msRequestSharedPixmapNotifyDamage(PixmapPtr ppix)
>>>        ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
>>>        modesettingPtr ms = modesettingPTR(scrn);
>>>    -    msPixmapPrivPtr ppriv = msGetPixmapPriv(&ms->drmmode, ppix);
>>> +    msPixmapPrivPtr ppriv = msGetPixmapPriv(&ms->drmmode,
>>> ppix->master_pixmap);
>>>          ppriv->notify_on_damage = TRUE;
>>>