intel: fix fullscreen damage posting on pageflip

Commit Message

Dave Airlie Oct. 11, 2012, 4:30 a.m. UTC

From: Dave Airlie <[e-mail]>

While playing with MPX and sw cursor I noticed page flips won't
end up misrendering some bits, so the sw cursor was replacing the
bits on the wrong pixmap.

Fix the damage handling to be correct and append damage before swapping
the pointers and process damage after.

This fixes misrendering with MPX cursors under a fullscreen compositor,
that pageflips.

Signed-off-by: Dave Airlie <[e-mail]>
---
 src/intel_dri.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

Comments

Chris Wilson Oct. 11, 2012, 7:53 a.m. UTC | #1

On Thu, 11 Oct 2012 14:30:29 +1000, Dave Airlie <[e-mail]> wrote:
> From: Dave Airlie <[e-mail]>
> 
> While playing with MPX and sw cursor I noticed page flips won't
> end up misrendering some bits, so the sw cursor was replacing the
> bits on the wrong pixmap.
> 
> Fix the damage handling to be correct and append damage before swapping
> the pointers and process damage after.
> 
> This fixes misrendering with MPX cursors under a fullscreen compositor,
> that pageflips.

Wow, what magic is this? The created region is the same and
front->drawable left unmodified by the exchange. The exchange only
manipulates the devPrivates and has not other side effects. The only
uncertain quantity there is the intel_glamor_exchange_buffers()...

So the change is that we append the same damage to the same drawable
earlier, yet still process that damage at the same time, with no
apparent side-effects between the two operations. I'm just not
understanding why this has any effect.
-Chris

Dave Airlie Oct. 11, 2012, 8:12 a.m. UTC | #2

On Thu, Oct 11, 2012 at 5:53 PM, Chris Wilson <[e-mail]> wrote:
> On Thu, 11 Oct 2012 14:30:29 +1000, Dave Airlie <[e-mail]> wrote:
>> From: Dave Airlie <[e-mail]>
>>
>> While playing with MPX and sw cursor I noticed page flips won't
>> end up misrendering some bits, so the sw cursor was replacing the
>> bits on the wrong pixmap.
>>
>> Fix the damage handling to be correct and append damage before swapping
>> the pointers and process damage after.
>>
>> This fixes misrendering with MPX cursors under a fullscreen compositor,
>> that pageflips.
>
> Wow, what magic is this? The created region is the same and
> front->drawable left unmodified by the exchange. The exchange only
> manipulates the devPrivates and has not other side effects. The only
> uncertain quantity there is the intel_glamor_exchange_buffers()...
>
> So the change is that we append the same damage to the same drawable
> earlier, yet still process that damage at the same time, with no
> apparent side-effects between the two operations. I'm just not
> understanding why this has any effect.

Hence why I didn't do it right in the first place, I thought I
understood how damage worked :-)

We have pre-rendering damage reporting and post-rendering damage
reporting!, so sw cursors really wants it before the fact so it can
copy the area out underneath itself,

damage append will process the pre-damage stuff, and pending will
process post-damage hooks.

Dave.

Chris Wilson Oct. 11, 2012, 8:26 a.m. UTC | #3

On Thu, 11 Oct 2012 18:12:18 +1000, Dave Airlie <[e-mail]> wrote:
> On Thu, Oct 11, 2012 at 5:53 PM, Chris Wilson <[e-mail]> wrote:
> > On Thu, 11 Oct 2012 14:30:29 +1000, Dave Airlie <[e-mail]> wrote:
> >> From: Dave Airlie <[e-mail]>
> >>
> >> While playing with MPX and sw cursor I noticed page flips won't
> >> end up misrendering some bits, so the sw cursor was replacing the
> >> bits on the wrong pixmap.
> >>
> >> Fix the damage handling to be correct and append damage before swapping
> >> the pointers and process damage after.
> >>
> >> This fixes misrendering with MPX cursors under a fullscreen compositor,
> >> that pageflips.
> >
> > Wow, what magic is this? The created region is the same and
> > front->drawable left unmodified by the exchange. The exchange only
> > manipulates the devPrivates and has not other side effects. The only
> > uncertain quantity there is the intel_glamor_exchange_buffers()...
> >
> > So the change is that we append the same damage to the same drawable
> > earlier, yet still process that damage at the same time, with no
> > apparent side-effects between the two operations. I'm just not
> > understanding why this has any effect.
> 
> Hence why I didn't do it right in the first place, I thought I
> understood how damage worked :-)
> 
> We have pre-rendering damage reporting and post-rendering damage
> reporting!, so sw cursors really wants it before the fact so it can
> copy the area out underneath itself,
> 
> damage append will process the pre-damage stuff, and pending will
> process post-damage hooks.

Thanks for the explanation, I had forgotten that not everything was
PostOp reported!

commit dadffd0061e2b14db397608e3cedfe9c76e546c8
Author: Dave Airlie <[e-mail]>
Date:   Thu Oct 11 14:30:29 2012 +1000

    intel: fix fullscreen damage posting on pageflip
-Chris

Patch

diff --git a/src/intel_dri.c b/src/intel_dri.c
index 64cb567..867a465 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -725,6 +725,17 @@  static struct intel_pixmap *
 intel_exchange_pixmap_buffers(struct intel_screen_private *intel, PixmapPtr front, PixmapPtr back)
 {
 	struct intel_pixmap *new_front, *new_back;
+	RegionRec region;
+
+	/* Post damage on the front buffer so that listeners, such
+	 * as DisplayLink know take a copy and shove it over the USB.
+	 * also for sw cursors.
+	 */
+	region.extents.x1 = region.extents.y1 = 0;
+	region.extents.x2 = front->drawable.width;
+	region.extents.y2 = front->drawable.height;
+	region.data = NULL;
+	DamageRegionAppend(&front->drawable, &region);
 
 	new_front = intel_get_pixmap_private(back);
 	new_back = intel_get_pixmap_private(front);
@@ -735,19 +746,7 @@  intel_exchange_pixmap_buffers(struct intel_screen_private *intel, PixmapPtr fron
 
 	intel_glamor_exchange_buffers(intel, front, back);
 
-	/* Post damage on the new front buffer so that listeners, such
-	 * as DisplayLink know take a copy and shove it over the USB.
-	 */
-	{
-		RegionRec region;
-
-		region.extents.x1 = region.extents.y1 = 0;
-		region.extents.x2 = front->drawable.width;
-		region.extents.y2 = front->drawable.height;
-		region.data = NULL;
-		DamageRegionAppend(&front->drawable, &region);
-		DamageRegionProcessPending(&front->drawable);
-	}
+	DamageRegionProcessPending(&front->drawable);
 
 	return new_front;
 }