Message ID | 20170418100754.14621-2-michel@daenzer.net (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 18/04/17 07:07 PM, Michel Dänzer wrote: > From: Michel Dänzer <michel.daenzer@amd.com> > > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> > --- > > Chris / Ilia / Ben, this should be manageable for the intel/nouveau > drivers, right? Any feedback, guys? I want to push the xserver change soon-ish. I could probably come up with a corresponding patch for nouveau in the worst case, but I'm afraid not for intel.
On Thu, Jul 13, 2017 at 4:27 AM, Michel Dänzer <michel@daenzer.net> wrote: > On 18/04/17 07:07 PM, Michel Dänzer wrote: >> From: Michel Dänzer <michel.daenzer@amd.com> >> >> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> >> --- >> >> Chris / Ilia / Ben, this should be manageable for the intel/nouveau >> drivers, right? > > Any feedback, guys? > > I want to push the xserver change soon-ish. I could probably come up > with a corresponding patch for nouveau in the worst case, but I'm afraid > not for intel. Sorry, I'm not 1000% clear on what this patch is doing. Is it literally just a type change from A to B and so code has to be fixed up for the new API? Or are there further implications like having to do/support new things? Thanks, -ilia
On 13/07/17 09:31 PM, Ilia Mirkin wrote: > On Thu, Jul 13, 2017 at 4:27 AM, Michel Dänzer <michel@daenzer.net> wrote: >> On 18/04/17 07:07 PM, Michel Dänzer wrote: >>> From: Michel Dänzer <michel.daenzer@amd.com> >>> >>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> >>> --- >>> >>> Chris / Ilia / Ben, this should be manageable for the intel/nouveau >>> drivers, right? >> >> Any feedback, guys? >> >> I want to push the xserver change soon-ish. I could probably come up >> with a corresponding patch for nouveau in the worst case, but I'm afraid >> not for intel. > > Sorry, I'm not 1000% clear on what this patch is doing. 100% should suffice. ;) > Is it literally just a type change from A to B and so code has to be fixed > up for the new API? That's basically it, corresponding to https://patchwork.freedesktop.org/patch/150938/ .
On Thu, Jul 13, 2017 at 10:14 PM, Michel Dänzer <michel@daenzer.net> wrote: > On 13/07/17 09:31 PM, Ilia Mirkin wrote: >> On Thu, Jul 13, 2017 at 4:27 AM, Michel Dänzer <michel@daenzer.net> wrote: >>> On 18/04/17 07:07 PM, Michel Dänzer wrote: >>>> From: Michel Dänzer <michel.daenzer@amd.com> >>>> >>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> >>>> --- >>>> >>>> Chris / Ilia / Ben, this should be manageable for the intel/nouveau >>>> drivers, right? >>> >>> Any feedback, guys? >>> >>> I want to push the xserver change soon-ish. I could probably come up >>> with a corresponding patch for nouveau in the worst case, but I'm afraid >>> not for intel. >> >> Sorry, I'm not 1000% clear on what this patch is doing. > > 100% should suffice. ;) > >> Is it literally just a type change from A to B and so code has to be fixed >> up for the new API? > > That's basically it, corresponding to > https://patchwork.freedesktop.org/patch/150938/ . From a brief glance at nouveau, it never uses PixmapDirtyUpdate, except in src/nv_driver.c where I don't think the code would need to be updated. https://cgit.freedesktop.org/nouveau/xf86-video-nouveau/tree/src/nv_driver.c#n552 Let me know if you think I'm off. -ilia
On 14/07/17 11:24 AM, Ilia Mirkin wrote: > On Thu, Jul 13, 2017 at 10:14 PM, Michel Dänzer <michel@daenzer.net> wrote: >> On 13/07/17 09:31 PM, Ilia Mirkin wrote: >>> On Thu, Jul 13, 2017 at 4:27 AM, Michel Dänzer <michel@daenzer.net> wrote: >>>> On 18/04/17 07:07 PM, Michel Dänzer wrote: >>>>> From: Michel Dänzer <michel.daenzer@amd.com> >>>>> >>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> >>>>> --- >>>>> >>>>> Chris / Ilia / Ben, this should be manageable for the intel/nouveau >>>>> drivers, right? >>>> >>>> Any feedback, guys? >>>> >>>> I want to push the xserver change soon-ish. I could probably come up >>>> with a corresponding patch for nouveau in the worst case, but I'm afraid >>>> not for intel. >>> >>> Sorry, I'm not 1000% clear on what this patch is doing. >> >> 100% should suffice. ;) >> >>> Is it literally just a type change from A to B and so code has to be fixed >>> up for the new API? >> >> That's basically it, corresponding to >> https://patchwork.freedesktop.org/patch/150938/ . > > From a brief glance at nouveau, it never uses PixmapDirtyUpdate, > except in src/nv_driver.c where I don't think the code would need to > be updated. > > https://cgit.freedesktop.org/nouveau/xf86-video-nouveau/tree/src/nv_driver.c#n552 There's also PixmapStart/StopDirtyTracking, though it looks like those might happen to work as well at runtime, if you don't mind the compiler warnings.
On 13/07/17 05:27 PM, Michel Dänzer wrote: > On 18/04/17 07:07 PM, Michel Dänzer wrote: >> From: Michel Dänzer <michel.daenzer@amd.com> >> >> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> >> --- >> >> Chris / Ilia / Ben, this should be manageable for the intel/nouveau >> drivers, right? > > Any feedback, guys? > > I want to push the xserver change soon-ish. I could probably come up > with a corresponding patch for nouveau in the worst case, but I'm afraid > not for intel. Chris, in response to your bugzilla comment https://bugs.freedesktop.org/show_bug.cgi?id=100086#c9 and patch https://patchwork.freedesktop.org/patch/143180/, I drafted how the ABI could be fixed to reflect current usage in the first comment to your patch. There was no response in a month, so I went ahead and wrote https://patchwork.freedesktop.org/patch/150938/ . After three months with no feedback, I followed up with the ping above. After almost another month without feedback from you, I pinged you about it on IRC, again no response. I'm afraid my patience has run out, I'll push the xserver patch next week. Until the xf86-video-intel SNA driver is fixed up, it'll continue being broken as the master screen driver with RandR 1.4 slave scanout.
diff --git a/src/amdgpu_drv.h b/src/amdgpu_drv.h index e5c44dc36..9e088e71a 100644 --- a/src/amdgpu_drv.h +++ b/src/amdgpu_drv.h @@ -170,6 +170,32 @@ typedef enum { #define AMDGPU_PIXMAP_SHARING 1 #define amdgpu_is_gpu_screen(screen) (screen)->isGPU #define amdgpu_is_gpu_scrn(scrn) (scrn)->is_gpu + +static inline ScreenPtr +amdgpu_dirty_master(PixmapDirtyUpdatePtr dirty) +{ +#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC + ScreenPtr screen = dirty->src->pScreen; +#else + ScreenPtr screen = dirty->src->drawable.pScreen; +#endif + + if (screen->current_master) + return screen->current_master; + + return screen; +} + +static inline Bool +amdgpu_dirty_src_equals(PixmapDirtyUpdatePtr dirty, PixmapPtr pixmap) +{ +#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC + return dirty->src == &pixmap->drawable; +#else + return dirty->src == pixmap; +#endif +} + #else #define amdgpu_is_gpu_screen(screen) 0 #define amdgpu_is_gpu_scrn(scrn) 0 diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c index 0182cbe0a..29d3d076f 100644 --- a/src/amdgpu_kms.c +++ b/src/amdgpu_kms.c @@ -448,7 +448,7 @@ dirty_region(PixmapDirtyUpdatePtr dirty) static void redisplay_dirty(PixmapDirtyUpdatePtr dirty, RegionPtr region) { - ScrnInfoPtr scrn = xf86ScreenToScrn(dirty->src->drawable.pScreen); + ScrnInfoPtr scrn = xf86ScreenToScrn(dirty->slave_dst->drawable.pScreen); if (RegionNil(region)) goto out; @@ -481,12 +481,12 @@ amdgpu_prime_scanout_update_abort(xf86CrtcPtr crtc, void *event_data) void amdgpu_sync_shared_pixmap(PixmapDirtyUpdatePtr dirty) { - ScreenPtr master_screen = dirty->src->master_pixmap->drawable.pScreen; + ScreenPtr master_screen = amdgpu_dirty_master(dirty); PixmapDirtyUpdatePtr ent; RegionPtr region; xorg_list_for_each_entry(ent, &master_screen->pixmap_dirty_list, ent) { - if (ent->slave_dst != dirty->src) + if (!amdgpu_dirty_src_equals(dirty, ent->slave_dst)) continue; region = dirty_region(ent); @@ -501,7 +501,7 @@ amdgpu_sync_shared_pixmap(PixmapDirtyUpdatePtr dirty) static Bool master_has_sync_shared_pixmap(ScrnInfoPtr scrn, PixmapDirtyUpdatePtr dirty) { - ScreenPtr master_screen = dirty->src->master_pixmap->drawable.pScreen; + ScreenPtr master_screen = amdgpu_dirty_master(dirty); return master_screen->SyncSharedPixmap != NULL; } @@ -517,7 +517,7 @@ slave_has_sync_shared_pixmap(ScrnInfoPtr scrn, PixmapDirtyUpdatePtr dirty) static void call_sync_shared_pixmap(PixmapDirtyUpdatePtr dirty) { - ScreenPtr master_screen = dirty->src->master_pixmap->drawable.pScreen; + ScreenPtr master_screen = amdgpu_dirty_master(dirty); master_screen->SyncSharedPixmap(dirty); } @@ -527,7 +527,7 @@ call_sync_shared_pixmap(PixmapDirtyUpdatePtr dirty) static Bool master_has_sync_shared_pixmap(ScrnInfoPtr scrn, PixmapDirtyUpdatePtr dirty) { - ScrnInfoPtr master_scrn = xf86ScreenToScrn(dirty->src->master_pixmap->drawable.pScreen); + ScrnInfoPtr master_scrn = xf86ScreenToScrn(amdgpu_dirty_master(dirty)); return master_scrn->driverName == scrn->driverName; } @@ -581,7 +581,7 @@ amdgpu_prime_scanout_do_update(xf86CrtcPtr crtc, unsigned scanout_id) Bool ret = FALSE; xorg_list_for_each_entry(dirty, &screen->pixmap_dirty_list, ent) { - if (dirty->src == scanoutpix && dirty->slave_dst == + if (amdgpu_dirty_src_equals(dirty, scanoutpix) && dirty->slave_dst == drmmode_crtc->scanout[scanout_id ^ drmmode_crtc->tear_free].pixmap) { RegionPtr region; @@ -738,10 +738,10 @@ amdgpu_dirty_update(ScrnInfoPtr scrn) PixmapDirtyUpdatePtr region_ent = ent; if (master_has_sync_shared_pixmap(scrn, ent)) { - ScreenPtr master_screen = ent->src->master_pixmap->drawable.pScreen; + ScreenPtr master_screen = amdgpu_dirty_master(ent); xorg_list_for_each_entry(region_ent, &master_screen->pixmap_dirty_list, ent) { - if (region_ent->slave_dst == ent->src) + if (amdgpu_dirty_src_equals(ent, region_ent->slave_dst)) break; } } diff --git a/src/drmmode_display.c b/src/drmmode_display.c index 5e0c4133b..06103ed56 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -681,7 +681,7 @@ drmmode_crtc_prime_scanout_update(xf86CrtcPtr crtc, DisplayModePtr mode, xorg_list_for_each_entry(dirty, &screen->pixmap_dirty_list, ent) { - if (dirty->src == crtc->randr_crtc->scanout_pixmap && + if (amdgpu_dirty_src_equals(dirty, crtc->randr_crtc->scanout_pixmap) && dirty->slave_dst == drmmode_crtc->scanout[drmmode_crtc->scanout_id].pixmap) { dirty->slave_dst = @@ -1216,7 +1216,11 @@ static Bool drmmode_set_scanout_pixmap(xf86CrtcPtr crtc, PixmapPtr ppix) return FALSE; } -#ifdef HAS_DIRTYTRACKING_ROTATION +#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC + PixmapStartDirtyTracking(&ppix->drawable, + drmmode_crtc->scanout[scanout_id].pixmap, + 0, 0, 0, 0, RR_Rotate_0); +#elif defined(HAS_DIRTYTRACKING_ROTATION) PixmapStartDirtyTracking(ppix, drmmode_crtc->scanout[scanout_id].pixmap, 0, 0, 0, 0, RR_Rotate_0); #elif defined(HAS_DIRTYTRACKING2)