[Intel-gfx] [PATCH] Xv: free tearing on textured video

Xiang, Haihao haihao.xiang at intel.com
Thu Feb 26 09:30:40 CET 2009


On Thu, 2009-02-26 at 09:07 +0800, Zhenyu Wang wrote:
> On 2009.02.26 00:30:19 +0800, Keith Packard wrote:
> > On Wed, 2009-02-25 at 15:56 +0800, Zou, Nanhai wrote:
> > 
> > > I suggest turn this option default to be true or not use an option at all.
> > > We already have too many options to handle.
> > 
> > Ideally, this code would do the right thing automatically, and be
> > configured when it 'automatic' wasn't working right. Nanhai's suggestion
> > of turning it on by default is a good one, as is Dave's suggestion of
> > using an Xv attribute instead of a config file option. 
> > 
> > I'd suggest one further change -- only sync when the video is 'large',
> > so that videos which are only a small part of the screen don't impact
> > other applications running at the same time. 
> > 
> > How about an Xv attribute which has three values 'on' 'off' and 'auto'?
> > 'on' means always sync, no matter what size, 'off' means never sync and
> > 'auto' means sync if the Xv image is more than half(?) of the pixels on
> > the screen. Then we set the default to 'auto'. Users can adjust the
> > behaviour of the driver using 'xvattr'.
> > 
> > One problem with this plan is that 'xvattr' adjusts the attributes on a
> > single port, and as we offer 16 textured video ports (along with 1
> > overlay port on hardware with an overlay), the user would need to call
> > xvattr up to 17 times to effect the change uniformly. I'd say we should
> > just fix 'xvattr' to add a '-p all' option to adjust a parameter for all
> > ports?
> > 
> 
> Looks a good plan. I just checked spec for MI_WAIT_EVENT, it looks for newer
> G45s some definition might have been changed. Haihao may try it and possibly
> update the patch.

Here is an update. I just test it on GM965 and G45 and MI_WAIT_EVENT works 
for GM965 and G45. 

Add an Xv attribute XV_VSYNC which has three values -1 (off), 0 (auto)
and 1 (on) to control whether textured adapter synchronizes the screen
update to the vblank. The default value is 0.
---
 src/i810_reg.h   |    4 ++
 src/i830_video.c |   97 ++++++++++++++++++++++++++++++++++++++++++++----------
 src/i830_video.h |    2 +
 3 files changed, 85 insertions(+), 18 deletions(-)

diff --git a/src/i810_reg.h b/src/i810_reg.h
index e2ffba1..cd7a634 100644
--- a/src/i810_reg.h
+++ b/src/i810_reg.h
@@ -2432,7 +2432,11 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 /* Wait for Events */
 #define MI_WAIT_FOR_EVENT		(0x03<<23)
+#define MI_WAIT_FOR_PIPEB_SVBLANK	(1<<18)
+#define MI_WAIT_FOR_PIPEA_SVBLANK	(1<<17)
 #define MI_WAIT_FOR_OVERLAY_FLIP	(1<<16)
+#define MI_WAIT_FOR_PIPEB_VBLANK	(1<<7)
+#define MI_WAIT_FOR_PIPEA_VBLANK	(1<<3)
 
 /* Flush */
 #define MI_FLUSH			(0x04<<23)
diff --git a/src/i830_video.c b/src/i830_video.c
index cdb1072..bf359bc 100644
--- a/src/i830_video.c
+++ b/src/i830_video.c
@@ -117,6 +117,7 @@ static int I830QueryImageAttributesTextured(ScrnInfoPtr, int, unsigned short *,
 
 static Atom xvBrightness, xvContrast, xvSaturation, xvColorKey, xvPipe, xvDoubleBuffer;
 static Atom xvGamma0, xvGamma1, xvGamma2, xvGamma3, xvGamma4, xvGamma5;
+static Atom xvVsync;
 
 /* Limits for the overlay/textured video source sizes.  The documented hardware
  * limits are 2048x2048 or better for overlay and both of our textured video
@@ -247,10 +248,11 @@ static XF86AttributeRec Attributes[NUM_ATTRIBUTES] = {
     {XvSettable | XvGettable, 0, 1, "XV_DOUBLE_BUFFER"}
 };
 
-#define NUM_TEXTURED_ATTRIBUTES 2
+#define NUM_TEXTURED_ATTRIBUTES 3
 static XF86AttributeRec TexturedAttributes[NUM_ATTRIBUTES] = {
     {XvSettable | XvGettable, -128, 127, "XV_BRIGHTNESS"},
     {XvSettable | XvGettable, 0, 255, "XV_CONTRAST"},
+    {XvSettable | XvGettable, -1, 1, "XV_VSYNC"},
 };
 
 #define GAMMA_ATTRIBUTES 6
@@ -914,6 +916,7 @@ I830SetupImageVideoOverlay(ScreenPtr pScreen)
     pPriv->doubleBuffer = 1;
 
     pPriv->rotation = RR_Rotate_0;
+    pPriv->xvsync = 0;
 
     /* gotta uninit this someplace */
     REGION_NULL(pScreen, &pPriv->clip);
@@ -1021,6 +1024,7 @@ I830SetupImageVideoTextured(ScreenPtr pScreen)
 	pPriv->doubleBuffer = 0;
 
 	pPriv->rotation = RR_Rotate_0;
+	pPriv->xvsync = 0;
 
 	/* gotta uninit this someplace, XXX: shouldn't be necessary for textured */
 	REGION_NULL(pScreen, &pPriv->clip);
@@ -1028,6 +1032,8 @@ I830SetupImageVideoTextured(ScreenPtr pScreen)
 	adapt->pPortPrivates[i].ptr = (pointer) (pPriv);
     }
 
+    xvVsync = MAKE_ATOM("XV_VSYNC");
+
     return adapt;
 }
 
@@ -1108,6 +1114,12 @@ I830SetPortAttributeTextured(ScrnInfoPtr pScrn,
 	    return BadValue;
 	pPriv->contrast = value;
 	return Success;
+    } else if (attribute == xvVsync) {
+        if ((value < -1) || (value > 1))
+            return BadValue;
+        
+        pPriv->xvsync = value;
+        return Success;
     } else {
 	return BadMatch;
     }
@@ -1243,7 +1255,9 @@ I830GetPortAttribute(ScrnInfoPtr pScrn,
 	*value = pPriv->colorKey;
     } else if (attribute == xvDoubleBuffer) {
 	*value = pPriv->doubleBuffer;
-    } else 
+    } else if (attribute == xvVsync) {
+        *value = pPriv->xvsync;
+    } else
 	return BadMatch;
 
     return Success;
@@ -2139,6 +2153,7 @@ i830_display_video(ScrnInfoPtr pScrn, xf86CrtcPtr crtc,
 
 static Bool
 i830_clip_video_helper (ScrnInfoPtr pScrn,
+			I830PortPrivPtr pPriv,
 			xf86CrtcPtr *crtc_ret,
 			BoxPtr	    dst,
 			INT32	    *xa,
@@ -2160,7 +2175,6 @@ i830_clip_video_helper (ScrnInfoPtr pScrn,
     if (crtc_ret)
     {
 	I830Ptr		pI830 = I830PTR(pScrn);
-	I830PortPrivPtr pPriv = pI830->adaptor->pPortPrivates[0].ptr;
 	BoxRec		crtc_box;
 	xf86CrtcPtr	crtc = i830_covering_crtc (pScrn, dst,
 						   pPriv->desired_crtc,
@@ -2303,7 +2317,8 @@ I830PutImage(ScrnInfoPtr pScrn,
     dstBox.y2 = drw_y + drw_h;
 
     if (!i830_clip_video_helper(pScrn, 
-				pPriv->textured ? NULL : &crtc,
+				pPriv,
+				&crtc,
 				&dstBox, &x1, &x2, &y1, &y2, clipBoxes,
 				width, height))
 	return Success;
@@ -2541,22 +2556,68 @@ I830PutImage(ScrnInfoPtr pScrn,
 	    REGION_COPY(pScrn->pScreen, &pPriv->clip, clipBoxes);
 	    i830_fill_colorkey (pScreen, pPriv->colorKey, clipBoxes);
 	}
-    } else if (IS_I965G(pI830)) {
+    } else {
+        Bool sync = TRUE;
+        
+        if (pPriv->xvsync == -1) {
+            sync = FALSE;
+        } else if (pPriv->xvsync == 0) {
+            BoxRec crtc_box;
+            BoxPtr pbox;
+            int nbox, crtc_area, coverage = 0;
+
+            i830_crtc_box(crtc, &crtc_box);
+            crtc_area = i830_box_area(&crtc_box);
+            pbox = REGION_RECTS(clipBoxes);
+            nbox = REGION_NUM_RECTS(clipBoxes);
+            
+            while (nbox--) {
+                coverage += i830_box_area(pbox);
+                pbox++;
+            }
+
+            if ((coverage << 1) < crtc_area)
+                sync = FALSE;
+        }
+
+        if (sync) {
+            I830CrtcPrivatePtr intel_crtc = crtc->driver_private;
+            int event;
+
+            if (IS_I965G(pI830)) {
+                if (intel_crtc->pipe == 0)
+                    event = MI_WAIT_FOR_PIPEA_SVBLANK;
+                else
+                    event = MI_WAIT_FOR_PIPEB_SVBLANK;
+            } else {
+                if (intel_crtc->pipe == 0)
+                    event = MI_WAIT_FOR_PIPEA_VBLANK;
+                else
+                    event = MI_WAIT_FOR_PIPEB_VBLANK;
+            }
+
+            BEGIN_BATCH(2);
+            OUT_BATCH(MI_WAIT_FOR_EVENT | event);
+            OUT_BATCH(MI_NOOP);
+            ADVANCE_BATCH();
+        }
 
+        if (IS_I965G(pI830)) {
 #ifdef INTEL_XVMC
-	if (id == FOURCC_XVMC && pPriv->rotation == RR_Rotate_0) {
-	    pPriv->YBuf0offset = buf -  pI830->FbBase;
-	    pPriv->UBuf0offset = pPriv->YBuf0offset + height*width; 
-	    pPriv->VBuf0offset = pPriv->UBuf0offset + height*width/4; 
-	}
+            if (id == FOURCC_XVMC && pPriv->rotation == RR_Rotate_0) {
+                pPriv->YBuf0offset = buf -  pI830->FbBase;
+                pPriv->UBuf0offset = pPriv->YBuf0offset + height*width; 
+                pPriv->VBuf0offset = pPriv->UBuf0offset + height*width/4; 
+            }
 #endif
-	I965DisplayVideoTextured(pScrn, pPriv, destId, clipBoxes, width, height,
-				 dstPitch, x1, y1, x2, y2,
-				 src_w, src_h, drw_w, drw_h, pPixmap);
-    } else {
-	I915DisplayVideoTextured(pScrn, pPriv, destId, clipBoxes, width, height,
-				 dstPitch, dstPitch2, x1, y1, x2, y2,
-				 src_w, src_h, drw_w, drw_h, pPixmap);
+            I965DisplayVideoTextured(pScrn, pPriv, destId, clipBoxes, width, height,
+                                     dstPitch, x1, y1, x2, y2,
+                                     src_w, src_h, drw_w, drw_h, pPixmap);
+        } else {
+            I915DisplayVideoTextured(pScrn, pPriv, destId, clipBoxes, width, height,
+                                     dstPitch, dstPitch2, x1, y1, x2, y2,
+                                     src_w, src_h, drw_w, drw_h, pPixmap);
+        }
     }
     if (pPriv->textured) {
 	DamageDamageRegion(pDraw, clipBoxes);
@@ -2867,7 +2928,7 @@ I830DisplaySurface(XF86SurfacePtr surface,
     dstBox.y1 = drw_y;
     dstBox.y2 = drw_y + drw_h;
 
-    if (!i830_clip_video_helper (pScrn, &crtc, &dstBox,
+    if (!i830_clip_video_helper (pScrn, pI830Priv, &crtc, &dstBox,
 				 &x1, &x2, &y1, &y2, clipBoxes,
 				 surface->width, surface->height))
 	return Success;
diff --git a/src/i830_video.h b/src/i830_video.h
index 254ee32..b47ac90 100644
--- a/src/i830_video.h
+++ b/src/i830_video.h
@@ -65,6 +65,8 @@ typedef struct {
    int scaleRatio;
    Bool textured;
    Rotation rotation; /* should remove I830->rotation later*/
+
+   int xvsync; /* -1: off, 0: auto, 1: on */
 } I830PortPrivRec, *I830PortPrivPtr;
 
 #define GET_PORT_PRIVATE(pScrn) \
-- 
1.5.6.3


> 




More information about the Intel-gfx mailing list