[PATCH] exa/mixed: avoid unnecessary dest system buffer copy in prepare_access_reg

[Karl Tomlinson]

I came across a situation
(http://ie.microsoft.com/testdrive/Performance/FishIE%20tank/Default.html)
where copying with operator source from a pixmap with
accel_blocked (due to large pitch) to a destination pixmap was
causing unnecessary read-back.

The destination pixmap was created by cairo, which clears the
surface, an operator which can be accelerated.

Then copying from the accel_blocked pixmap is not accelerated and
so causes creation of a system copy of the destination pixmap, and
the copy is initially invalid.  exaPrepareAccessReg_mixed has only
just created the damage and so assumes that everything needs to be
copied to system, even though the operator will overwrite the
copied pixels.

The attached patch avoids the read-back into the system copy.


>From b684137b8e96f231157a48fa199bcaa829362bca Mon Sep 17 00:00:00 2001
From: Karl Tomlinson <kar...@karlt.net>
Date: Thu, 26 Aug 2010 22:29:35 +1200
Subject: [PATCH] exa/mixed: avoid unnecessary dest system buffer copy in 
prepare_access_reg

A region is only provided to prepare_access_reg for destination pixmaps when
the operator will not read from the destination.  A non-NULL region therefore
indicates that no pixels in the destination gpu buffer need to be copied to
the system buffer.

The provided region still serves a purpose, as it indicates the portion of
the system buffer that will be damaged.
---
 exa/exa_migration_mixed.c |   42 ++++++++++++++++++++++++++++--------------
 1 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/exa/exa_migration_mixed.c b/exa/exa_migration_mixed.c
index fb47151..605235e 100644
--- a/exa/exa_migration_mixed.c
+++ b/exa/exa_migration_mixed.c
@@ -196,6 +196,8 @@ exaPrepareAccessReg_mixed(PixmapPtr pPixmap, int index, 
RegionPtr pReg)
     }
 
     if (!success) {
+       BoxRec box;
+       RegionRec region_all;
        ExaMigrationRec pixmaps[1];
 
        /* Do we need to allocate our system buffer? */
@@ -207,15 +209,27 @@ exaPrepareAccessReg_mixed(PixmapPtr pPixmap, int index, 
RegionPtr pReg)
                           pExaPixmap->sys_pitch * pPixmap->drawable.height);
        }
 
+       box.x1 = 0;
+       box.y1 = 0;
+       box.x2 = pPixmap->drawable.width;
+       box.y2 = pPixmap->drawable.height;
+       RegionInit(&region_all, &box, 1);
+
+       pixmaps[0].pPix = pPixmap;
+       pixmaps[0].pReg = pReg;
        if (index == EXA_PREPARE_DEST || index == EXA_PREPARE_AUX_DEST) {
+           /* A pReg for a destination indicates that the operator will not
+            * read from the destination, and so there is no need to copy any
+            * pixels.  The valid regions will still be updated.
+            */
+           if (pReg)
+               pixmaps[0].pReg = &region_all;
            pixmaps[0].as_dst = TRUE;
            pixmaps[0].as_src = FALSE;
        } else {
            pixmaps[0].as_dst = FALSE;
            pixmaps[0].as_src = TRUE;
        }
-       pixmaps[0].pPix = pPixmap;
-       pixmaps[0].pReg = pReg;
 
        if (!pExaPixmap->pDamage &&
                (has_gpu_copy || !exaPixmapIsPinned(pPixmap))) {
@@ -232,27 +246,27 @@ exaPrepareAccessReg_mixed(PixmapPtr pPixmap, int index, 
RegionPtr pReg)
            /* This is used by exa to optimize migration. */
            DamageSetReportAfterOp(pExaPixmap->pDamage, TRUE);
 
-           if (has_gpu_copy) {
+           if (has_gpu_copy)
                exaPixmapDirty(pPixmap, 0, 0, pPixmap->drawable.width,
                               pPixmap->drawable.height);
 
-               /* We don't know which region of the destination will be 
damaged,
-                * have to assume all of it
-                */
-               if (as_dst) {
-                   pixmaps[0].as_dst = FALSE;
-                   pixmaps[0].as_src = TRUE;
-                   pixmaps[0].pReg = NULL;
-               }
+           /* When we don't know which region of the destination will be
+            * damaged, have to assume all of it
+            */
+           if (as_dst)
+               DamageRegionAppend(&pPixmap->drawable,
+                                  pReg ? pReg : &region_all);
+
+           if (has_gpu_copy)
                exaCopyDirtyToSys(pixmaps);
-           }
 
            if (as_dst)
-               exaPixmapDirty(pPixmap, 0, 0, pPixmap->drawable.width,
-                              pPixmap->drawable.height);
+               DamageRegionProcessPending(&pPixmap->drawable);
        } else if (has_gpu_copy)
            exaCopyDirtyToSys(pixmaps);
 
+       RegionUninit(&region_all);
+
        pPixmap->devPrivate.ptr = pExaPixmap->sys_ptr;
        pPixmap->devKind = pExaPixmap->sys_pitch;
        pExaPixmap->use_gpu_copy = FALSE;
-- 
1.7.1


_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel