diff mbox

[xf86-video-amdgpu] Adapt to PixmapDirtyUpdateRec::src being a DrawablePtr

Message ID 20170418100754.14621-2-michel@daenzer.net (mailing list archive)
State New, archived
Headers show

Commit Message

Michel Dänzer April 18, 2017, 10:07 a.m. UTC
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?

 src/amdgpu_drv.h      | 26 ++++++++++++++++++++++++++
 src/amdgpu_kms.c      | 18 +++++++++---------
 src/drmmode_display.c |  8 ++++++--
 3 files changed, 41 insertions(+), 11 deletions(-)

Comments

Michel Dänzer July 13, 2017, 8:27 a.m. UTC | #1
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.
Ilia Mirkin July 13, 2017, 12:31 p.m. UTC | #2
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
Michel Dänzer July 14, 2017, 2:14 a.m. UTC | #3
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/ .
Ilia Mirkin July 14, 2017, 2:24 a.m. UTC | #4
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
Michel Dänzer July 14, 2017, 7:38 a.m. UTC | #5
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.
Michel Dänzer Aug. 9, 2017, 1:28 a.m. UTC | #6
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 mbox

Patch

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)