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

Dave Airlie airlied at gmail.com
Wed Feb 25 11:16:22 CET 2009


On Wed, Feb 25, 2009 at 6:18 PM, Xiang, Haihao <haihao.xiang at intel.com> wrote:
> On Wed, 2009-02-25 at 15:56 +0800, Zou, Nanhai wrote:
>> >-----Original Message-----
>> >From: intel-gfx-bounces at lists.freedesktop.org
>> >[mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of Xiang, Haihao
>> >Sent: 2009年2月25日 15:35
>> >To: intel-gfx at lists.freedesktop.org
>> >Subject: [Intel-gfx] [PATCH] Xv: free tearing on textured video
>> >
>> >Synchronize the screen update to vblank and add an option
>> >XvTexturedVsync to control it.
>> >---
>> > man/intel.man     |    5 +++++
>> > src/i810_reg.h    |    4 ++++
>> > src/i830.h        |    1 +
>> > src/i830_driver.c |    3 +++
>> > src/i830_video.c  |   53
>> >++++++++++++++++++++++++++++++++++++++---------------
>> > 5 files changed, 51 insertions(+), 15 deletions(-)
>> >
>> >diff --git a/man/intel.man b/man/intel.man
>> >index c7a3c61..2738ef6 100644
>> >--- a/man/intel.man
>> >+++ b/man/intel.man
>> >@@ -202,6 +202,11 @@ information.
>> > Enable XvMC driver. Current support MPEG2 MC on 915/945 and G33 series.
>> > User should provide absolute path to libIntelXvMC.so in XvMCConfig file.
>> > Default: Disabled.
>> >+.TP
>> >+.BI "Option \*qXvTexturedVsync\*q \*q" boolean \*q
>> >+This option controls whether textured adapter synchronizes the screen update
>> >to
>> >+the vblank.
>> >+Default: disabled.
>> >
>> > .SH OUTPUT CONFIGURATION
>> > On 830M and better chipsets, the driver supports runtime configuration of
>> >diff --git a/src/i810_reg.h b/src/i810_reg.h
>> >index e2ffba1..b6aa769 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.h b/src/i830.h
>> >index 7904b9f..735b0c1 100644
>> >--- a/src/i830.h
>> >+++ b/src/i830.h
>> >@@ -555,6 +555,7 @@ typedef struct _I830Rec {
>> >    Bool XvDisabled;                 /* Xv disabled in PreInit. */
>> >    Bool XvEnabled;                  /* Xv enabled for this generation. */
>> >    Bool XvPreferOverlay;
>> >+   Bool XvTexturedVsync;
>> >
>> > #ifdef I830_XV
>> >    int colorKey;
>> >diff --git a/src/i830_driver.c b/src/i830_driver.c
>> >index 0a8a9c6..5d7285c 100644
>> >--- a/src/i830_driver.c
>> >+++ b/src/i830_driver.c
>> >@@ -318,6 +318,7 @@ typedef enum {
>> >    OPTION_XVMC,
>> > #endif
>> >    OPTION_PREFER_OVERLAY,
>> >+   OPTION_TEXTURED_VSYNC,
>> > } I830Opts;
>> >
>> > static OptionInfoRec I830Options[] = {
>> >@@ -343,6 +344,7 @@ static OptionInfoRec I830Options[] = {
>> >    {OPTION_XVMC,    "XvMC",         OPTV_BOOLEAN,   {0},    TRUE},
>> > #endif
>> >    {OPTION_PREFER_OVERLAY, "XvPreferOverlay", OPTV_BOOLEAN, {0}, FALSE},
>> >+   {OPTION_TEXTURED_VSYNC, "XvTexturedVsync", OPTV_BOOLEAN, {0}, FALSE},
>> >    {-1,                     NULL,           OPTV_NONE,      {0},    FALSE}
>> > };
>> > /* *INDENT-ON* */
>> >@@ -1777,6 +1779,7 @@ I830XvInit(ScrnInfoPtr pScrn)
>> >     !xf86ReturnOptValBool(pI830->Options, OPTION_XVIDEO, TRUE);
>> >
>> >    pI830->XvPreferOverlay = xf86ReturnOptValBool(pI830->Options,
>> >OPTION_PREFER_OVERLAY, FALSE);
>> >+   pI830->XvTexturedVsync = xf86ReturnOptValBool(pI830->Options,
>> >OPTION_TEXTURED_VSYNC, FALSE);
>> >
>> > #ifdef I830_XV
>> >     if (xf86GetOptValInteger(pI830->Options, OPTION_VIDEO_KEY,
>> >diff --git a/src/i830_video.c b/src/i830_video.c
>> >index c9a0181..eea6a5b 100644
>> >--- a/src/i830_video.c
>> >+++ b/src/i830_video.c
>> >@@ -2303,7 +2303,7 @@ I830PutImage(ScrnInfoPtr pScrn,
>> >     dstBox.y2 = drw_y + drw_h;
>> >
>> >     if (!i830_clip_video_helper(pScrn,
>> >-                            pPriv->textured ? NULL : &crtc,
>> >+                            &crtc,
>> >                             &dstBox, &x1, &x2, &y1, &y2, clipBoxes,
>> >                             width, height))
>> >     return Success;
>> >@@ -2528,22 +2528,45 @@ I830PutImage(ScrnInfoPtr pScrn,
>> >         REGION_COPY(pScrn->pScreen, &pPriv->clip, clipBoxes);
>> >         i830_fill_colorkey (pScreen, pPriv->colorKey, clipBoxes);
>> >     }
>> >-    } else if (IS_I965G(pI830)) {
>> >-
>> >+    } else {
>> >+        if (pI830->XvTexturedVsync) {
>> >+            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);
>> >--
>> >1.5.6.3
>> >
>>
>> I suggest turn this option default to be true or not use an option at all.
>> We already have too many options to handle.
> Users maybe care about X performance and they need to control it and
> there is not any dependency of this option.

I'd argue that if I'm playing a video I don't want to see it tear, no
matter what,
you could use an xv attribute like radeon does.

Dave.
>
> Haihao
>>
>> Thanks
>> Zou Nanhai
>> >_______________________________________________
>> >Intel-gfx mailing list
>> >Intel-gfx at lists.freedesktop.org
>> >http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>



More information about the Intel-gfx mailing list