Fallback to std DRI2CopyRegion when DRI2UpdatePrime fails

Commit Message

Chris Wilson Oct. 5, 2014, 7:06 a.m. UTC

I was looking at a bug report today of intel/ati prime and noticed a
number of sna_share_pixmap_backing() failures (called from
DRI2UpdatePrime). These were failing as the request was for the scanout
buffer (which is tiled and so we refuse to share it, and since it is
already on the scanout we refuse to change tiling).

But looking at radeon_dri2_copy_region2(), if DRI2UpdatePrime() fails,
the copy is aborted and the update lost. If the copy is made to the
normal window drawable is that enough for it to be propagated back
through damage tracking?
---
 src/radeon_dri2.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Comments

Michel Dänzer Oct. 6, 2014, 2:04 a.m. UTC | #1

On 05.10.2014 16:06, Chris Wilson wrote:
> I was looking at a bug report today of intel/ati prime and noticed a
> number of sna_share_pixmap_backing() failures (called from
> DRI2UpdatePrime). These were failing as the request was for the scanout
> buffer (which is tiled and so we refuse to share it, and since it is
> already on the scanout we refuse to change tiling).
>
> But looking at radeon_dri2_copy_region2(), if DRI2UpdatePrime() fails,
> the copy is aborted and the update lost. If the copy is made to the
> normal window drawable is that enough for it to be propagated back
> through damage tracking?

Have you asked the reporter of that bug to test your patch?

Chris Wilson Oct. 6, 2014, 6:39 a.m. UTC | #2

On Mon, Oct 06, 2014 at 11:04:51AM +0900, Michel Dänzer wrote:
> On 05.10.2014 16:06, Chris Wilson wrote:
> >I was looking at a bug report today of intel/ati prime and noticed a
> >number of sna_share_pixmap_backing() failures (called from
> >DRI2UpdatePrime). These were failing as the request was for the scanout
> >buffer (which is tiled and so we refuse to share it, and since it is
> >already on the scanout we refuse to change tiling).
> >
> >But looking at radeon_dri2_copy_region2(), if DRI2UpdatePrime() fails,
> >the copy is aborted and the update lost. If the copy is made to the
> >normal window drawable is that enough for it to be propagated back
> >through damage tracking?
> 
> Have you asked the reporter of that bug to test your patch?

They didn't notice the issue, presumably because it only happens quite
early in the DE startup. However, I do remember Dave mentioning what
seemed to be a similar issue: a peristent blank window. Hence the
inquisitive nature of the patch.
-Chris

Mike Lothian Oct. 6, 2014, 7:37 a.m. UTC | #3

Is this the issue in KDE that when I start a game I have to toggle
compositing a couple of times to get it rendering?
On 6 Oct 2014 07:39, "Chris Wilson" <[e-mail]> wrote:

> On Mon, Oct 06, 2014 at 11:04:51AM +0900, Michel Dänzer wrote:
> > On 05.10.2014 16:06, Chris Wilson wrote:
> > >I was looking at a bug report today of intel/ati prime and noticed a
> > >number of sna_share_pixmap_backing() failures (called from
> > >DRI2UpdatePrime). These were failing as the request was for the scanout
> > >buffer (which is tiled and so we refuse to share it, and since it is
> > >already on the scanout we refuse to change tiling).
> > >
> > >But looking at radeon_dri2_copy_region2(), if DRI2UpdatePrime() fails,
> > >the copy is aborted and the update lost. If the copy is made to the
> > >normal window drawable is that enough for it to be propagated back
> > >through damage tracking?
> >
> > Have you asked the reporter of that bug to test your patch?
>
> They didn't notice the issue, presumably because it only happens quite
> early in the DE startup. However, I do remember Dave mentioning what
> seemed to be a similar issue: a peristent blank window. Hence the
> inquisitive nature of the patch.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> dri-devel mailing list
> [e-mail]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

Patch

diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c
index 9a9918b..0d113b9 100644
--- a/src/radeon_dri2.c
+++ b/src/radeon_dri2.c
@@ -409,26 +409,27 @@  radeon_dri2_copy_region2(ScreenPtr pScreen,
     dst_drawable = &dst_private->pixmap->drawable;
 
     if (src_private->attachment == DRI2BufferFrontLeft) {
+	src_drawable = NULL;
 #ifdef USE_DRI2_PRIME
-	if (drawable->pScreen != pScreen) {
+	if (drawable->pScreen != pScreen)
 	    src_drawable = DRI2UpdatePrime(drawable, src_buffer);
-	    if (!src_drawable)
-		return;
-	} else
 #endif
+	if (src_drawable == NULL)
 	    src_drawable = drawable;
     }
     if (dst_private->attachment == DRI2BufferFrontLeft) {
+	dst_drawable = NULL;
 #ifdef USE_DRI2_PRIME
 	if (drawable->pScreen != pScreen) {
 	    dst_drawable = DRI2UpdatePrime(drawable, dest_buffer);
-	    if (!dst_drawable)
-		return;
-	    dst_ppix = (PixmapPtr)dst_drawable;
-	    if (dst_drawable != drawable)
-		translate = TRUE;
-	} else
+	    if (dst_drawable) {
+		dst_ppix = (PixmapPtr)dst_drawable;
+		if (dst_drawable != drawable)
+		    translate = TRUE;
+	    }
+	}
 #endif
+	if (dst_drawable == NULL)
 	    dst_drawable = drawable;
     }