From patchwork Tue Aug 11 14:06:45 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 40681 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n7BE7OGV022405 for ; Tue, 11 Aug 2009 14:07:24 GMT Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 76B6E9EFC3; Tue, 11 Aug 2009 07:07:24 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail.ffwll.ch (cable-static-49-187.intergga.ch [157.161.49.187]) by gabe.freedesktop.org (Postfix) with ESMTP id E96C79EFCE for ; Tue, 11 Aug 2009 07:07:17 -0700 (PDT) Received: by mail.ffwll.ch (Postfix, from userid 1000) id 64A8520C22A; Wed, 12 Aug 2009 00:00:46 +0200 (CEST) X-Spam-ASN: X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on orange.ffwll.ch X-Spam-Level: X-Spam-Hammy: 0.000-+--struct, 0.000-+--100644, 0.000-+--insertions X-Spam-Status: No, score=-4.4 required=6.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Spammy: Received: from biene (unknown [192.168.23.129]) by mail.ffwll.ch (Postfix) with ESMTP id 8F64F20C22F; Wed, 12 Aug 2009 00:00:08 +0200 (CEST) Received: from daniel by biene with local (Exim 4.69) (envelope-from ) id 1Mas0B-0003eq-SW; Tue, 11 Aug 2009 16:06:59 +0200 From: Daniel Vetter To: intel-gfx@lists.freedesktop.org Date: Tue, 11 Aug 2009 16:06:45 +0200 Message-Id: <3f8b26f517d6bea6dc326136b6e9ea9d210e9a5a.1249999028.git.daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.6.3.3 In-Reply-To: <06188abbed359c266f2033d8b3b28aa0aaf8aada.1249999028.git.daniel.vetter@ffwll.ch> References: <0328734a90544a6cd72d9eaf64015db9d3462921.1249999028.git.daniel.vetter@ffwll.ch> <10614c7dd4ecbb1b4d3dd6a15b949cb389053f1f.1249999028.git.daniel.vetter@ffwll.ch> <891b387c6b31972a3e339508e57bd660b2991a17.1249999028.git.daniel.vetter@ffwll.ch> <2096013512e0099bfbb89439943c1b70cccabc92.1249999028.git.daniel.vetter@ffwll.ch> <8a3ddc0d78dcb40a14f8037b81cf202eaa40c301.1249999028.git.daniel.vetter@ffwll.ch> <3e0435569d2d7f58d58eb2f7c8a6952cc29b6934.1249999028.git.daniel.vetter@ffwll.ch> <52250eeaf73398c9c1f91ca7b2317f785f8eba56.1249999028.git.daniel.vetter@ffwll.ch> <3a4e39fa527e6f1bec9306a0c9d7b69b222532a6.1249999028.git.daniel.vetter@ffwll.ch> <06188abbed359c266f2033d8b3b28aa0aaf8aada.1249999028.git.daniel.vetter@ffwll.ch> In-Reply-To: References: Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH 15/18] Xv: rework overlay buffer management X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.9 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org The basic idea is to only pin the buffer into the gtt when the overlay hw is actually using it. This results in a few changes: - Unify data copied/buffer handling with textured video. Now offsets are always buffer relative and we just use drm_bo_map to access a buffer. - Implement double buffering using two bo's. This is necessary because we can't pin the same buffer to the gtt and map it as normal memory. - Kill XV_DOUBLE_BUFFER. With the above changes, overlay video is always doubel buffered. There is still the XvMC passthrough case, which makes the code slightly ugly. Unfortunately we can't get at the bo behind this buffer. Changes since the last review-round: - Don't overallocate by a factor of 2. - Prevent possible use-after-free issue. --- src/i830_video.c | 167 ++++++++++++++++++++++++------------------------------ src/i830_video.h | 8 +- 2 files changed, 78 insertions(+), 97 deletions(-) diff --git a/src/i830_video.c b/src/i830_video.c index 8592750..15ada0f 100644 --- a/src/i830_video.c +++ b/src/i830_video.c @@ -103,7 +103,7 @@ static int I830QueryImageAttributes(ScrnInfoPtr, int, unsigned short *, #define MAKE_ATOM(a) MakeAtom(a, sizeof(a) - 1, TRUE) -static Atom xvBrightness, xvContrast, xvSaturation, xvColorKey, xvPipe, xvDoubleBuffer; +static Atom xvBrightness, xvContrast, xvSaturation, xvColorKey, xvPipe; static Atom xvGamma0, xvGamma1, xvGamma2, xvGamma3, xvGamma4, xvGamma5; static Atom xvSyncToVblank; @@ -214,13 +214,12 @@ static XF86AttributeRec CloneAttributes[CLONE_ATTRIBUTES] = { {XvSettable | XvGettable, -1, 1, "XV_PIPE"} }; -#define NUM_ATTRIBUTES 5 +#define NUM_ATTRIBUTES 4 static XF86AttributeRec Attributes[NUM_ATTRIBUTES] = { {XvSettable | XvGettable, 0, (1 << 24) - 1, "XV_COLORKEY"}, {XvSettable | XvGettable, -128, 127, "XV_BRIGHTNESS"}, {XvSettable | XvGettable, 0, 255, "XV_CONTRAST"}, - {XvSettable | XvGettable, 0, 1023, "XV_SATURATION"}, - {XvSettable | XvGettable, 0, 1, "XV_DOUBLE_BUFFER"} + {XvSettable | XvGettable, 0, 1023, "XV_SATURATION"} }; #define NUM_TEXTURED_ATTRIBUTES 3 @@ -487,6 +486,8 @@ i830_overlay_continue(ScrnInfoPtr pScrn, Bool update_filter) OUT_BATCH(flip_addr); ADVANCE_BATCH(); OVERLAY_DEBUG("overlay_continue\n"); + + I830Sync(pScrn); } static void @@ -895,14 +896,14 @@ I830SetupImageVideoOverlay(ScreenPtr pScreen) pPriv->current_crtc = NULL; pPriv->desired_crtc = NULL; pPriv->buf = NULL; - pPriv->currentBuf = 0; + pPriv->oldBuf = NULL; + pPriv->oldBuf_pinned = FALSE; pPriv->gamma5 = 0xc0c0c0; pPriv->gamma4 = 0x808080; pPriv->gamma3 = 0x404040; pPriv->gamma2 = 0x202020; pPriv->gamma1 = 0x101010; pPriv->gamma0 = 0x080808; - pPriv->doubleBuffer = 1; pPriv->rotation = RR_Rotate_0; @@ -928,7 +929,6 @@ I830SetupImageVideoOverlay(ScreenPtr pScreen) xvBrightness = MAKE_ATOM("XV_BRIGHTNESS"); xvContrast = MAKE_ATOM("XV_CONTRAST"); xvSaturation = MAKE_ATOM("XV_SATURATION"); - xvDoubleBuffer = MAKE_ATOM("XV_DOUBLE_BUFFER"); /* Allow the pipe to be switched from pipe A to B when in clone mode */ xvPipe = MAKE_ATOM("XV_PIPE"); @@ -1008,8 +1008,8 @@ I830SetupImageVideoTextured(ScreenPtr pScreen) pPriv->textured = TRUE; pPriv->videoStatus = 0; pPriv->buf = NULL; - pPriv->currentBuf = 0; - pPriv->doubleBuffer = 0; + pPriv->oldBuf = NULL; + pPriv->oldBuf_pinned = FALSE; pPriv->rotation = RR_Rotate_0; pPriv->SyncToVblank = 1; @@ -1026,6 +1026,23 @@ I830SetupImageVideoTextured(ScreenPtr pScreen) } static void +i830_free_video_buffers(I830PortPrivPtr pPriv) +{ + if (pPriv->buf) { + drm_intel_bo_unreference(pPriv->buf); + pPriv->buf = NULL; + } + + if (pPriv->oldBuf) { + if (pPriv->oldBuf_pinned) + drm_intel_bo_unpin(pPriv->oldBuf); + drm_intel_bo_unreference(pPriv->oldBuf); + pPriv->oldBuf = NULL; + pPriv->oldBuf_pinned = FALSE; + } +} + +static void I830StopVideo(ScrnInfoPtr pScrn, pointer data, Bool shutdown) { I830PortPrivPtr pPriv = (I830PortPrivPtr) data; @@ -1042,12 +1059,8 @@ I830StopVideo(ScrnInfoPtr pScrn, pointer data, Bool shutdown) i830_overlay_off(pScrn); } - if (pPriv->buf) { - drm_intel_bo_unpin(pPriv->buf); - drm_intel_bo_unreference(pPriv->buf); - pPriv->buf = NULL; - pPriv->videoStatus = 0; - } + i830_free_video_buffers(pPriv); + pPriv->videoStatus = 0; } else { if (pPriv->videoStatus & CLIENT_VIDEO_ON) { pPriv->videoStatus |= OFF_TIMER; @@ -1153,12 +1166,6 @@ I830SetPortAttributeOverlay(ScrnInfoPtr pScrn, OVERLAY_DEBUG("COLORKEY\n"); i830_overlay_continue (pScrn, FALSE); REGION_EMPTY(pScrn->pScreen, &pPriv->clip); - } else if(attribute == xvDoubleBuffer) { - if ((value < 0) || (value > 1)) - return BadValue; - /* Do not allow buffer change while playing video */ - if(!pI830->overlayOn) - pPriv->doubleBuffer = value; } else return BadMatch; @@ -1212,8 +1219,6 @@ I830GetPortAttribute(ScrnInfoPtr pScrn, *value = pPriv->gamma5; } else if (attribute == xvColorKey) { *value = pPriv->colorKey; - } else if (attribute == xvDoubleBuffer) { - *value = pPriv->doubleBuffer; } else if (attribute == xvSyncToVblank) { *value = pPriv->SyncToVblank; } else @@ -1239,12 +1244,11 @@ I830QueryBestSize(ScrnInfoPtr pScrn, } static void -I830CopyPackedData(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, +I830CopyPackedData(I830PortPrivPtr pPriv, unsigned char *buf, int srcPitch, int dstPitch, int top, int left, int h, int w) { - I830Ptr pI830 = I830PTR(pScrn); unsigned char *src, *dst, *dst_base; int i,j; unsigned char *s; @@ -1257,13 +1261,8 @@ I830CopyPackedData(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, src = buf + (top * srcPitch) + (left << 1); - if (pPriv->textured) { - drm_intel_bo_map(pPriv->buf, TRUE); - dst_base = pPriv->buf->virtual; - } else { - drm_intel_gem_bo_start_gtt_access(pPriv->buf, TRUE); - dst_base = pI830->FbBase; - } + drm_intel_bo_map(pPriv->buf, TRUE); + dst_base = pPriv->buf->virtual; dst = dst_base + pPriv->YBufOffset; @@ -1339,8 +1338,7 @@ I830CopyPackedData(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, break; } - if (pPriv->textured) - drm_intel_bo_unmap(pPriv->buf); + drm_intel_bo_unmap(pPriv->buf); } static void i830_memcpy_plane(unsigned char *dst, unsigned char *src, @@ -1393,12 +1391,11 @@ static void i830_memcpy_plane(unsigned char *dst, unsigned char *src, } static void -I830CopyPlanarData(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, +I830CopyPlanarData(I830PortPrivPtr pPriv, unsigned char *buf, int srcPitch, int srcPitch2, int dstPitch, int srcH, int top, int left, int h, int w, int id) { - I830Ptr pI830 = I830PTR(pScrn); unsigned char *src1, *src2, *src3, *dst_base, *dst1, *dst2, *dst3; int dstPitch2 = dstPitch << 1; @@ -1416,13 +1413,8 @@ I830CopyPlanarData(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, (unsigned long)src1 - (unsigned long)buf); #endif - if (pPriv->textured) { - drm_intel_bo_map(pPriv->buf, TRUE); - dst_base = pPriv->buf->virtual; - } else { - drm_intel_gem_bo_start_gtt_access(pPriv->buf, TRUE); - dst_base = pI830->FbBase; - } + drm_intel_bo_map(pPriv->buf, TRUE); + dst_base = pPriv->buf->virtual; dst1 = dst_base + pPriv->YBufOffset; @@ -1464,8 +1456,7 @@ I830CopyPlanarData(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, i830_memcpy_plane(dst3, src3, h/2, w/2, dstPitch, srcPitch2, pPriv->rotation); - if (pPriv->textured) - drm_intel_bo_unmap(pPriv->buf); + drm_intel_bo_unmap(pPriv->buf); } typedef struct { @@ -1972,7 +1963,7 @@ xvmc_passthrough(int id, Rotation rotation) #endif } -static void +static Bool i830_display_overlay(ScrnInfoPtr pScrn, xf86CrtcPtr crtc, int id, short width, short height, int dstPitch, int x1, int y1, int x2, int y2, BoxPtr dstBox, @@ -1985,6 +1976,7 @@ i830_display_overlay(ScrnInfoPtr pScrn, xf86CrtcPtr crtc, uint32_t swidth, swidthsw, sheigth; int tmp; Bool scaleChanged; + drm_intel_bo *tmp_buf; OVERLAY_DEBUG("I830DisplayVideo: %dx%d (pitch %d)\n", width, height, dstPitch); @@ -2000,7 +1992,7 @@ i830_display_overlay(ScrnInfoPtr pScrn, xf86CrtcPtr crtc, { pPriv->current_crtc = NULL; i830_overlay_off (pScrn); - return; + return TRUE; } if (crtc != pPriv->current_crtc) @@ -2013,7 +2005,7 @@ i830_display_overlay(ScrnInfoPtr pScrn, xf86CrtcPtr crtc, } if (!pPriv->overlayOK) - return; + return TRUE; i830_update_dst_box_to_crtc_coords(pScrn, crtc, dstBox); @@ -2056,9 +2048,17 @@ i830_display_overlay(ScrnInfoPtr pScrn, xf86CrtcPtr crtc, dstBox->x1, dstBox->y1, dstBox->x2, dstBox->y2); /* buffer locations */ - overlay->OBUF_0Y = pPriv->YBufOffset; - overlay->OBUF_0U = pPriv->UBufOffset; - overlay->OBUF_0V = pPriv->VBufOffset; + if (xvmc_passthrough(id, pPriv->rotation)) { + overlay->OBUF_0Y = pPriv->YBufOffset; + overlay->OBUF_0U = pPriv->UBufOffset; + overlay->OBUF_0V = pPriv->VBufOffset; + } else { + if (drm_intel_bo_pin(pPriv->buf, GTT_PAGE_SIZE) != 0) + return FALSE; + overlay->OBUF_0Y = pPriv->YBufOffset + pPriv->buf->offset; + overlay->OBUF_0U = pPriv->UBufOffset + pPriv->buf->offset; + overlay->OBUF_0V = pPriv->VBufOffset + pPriv->buf->offset; + } OVERLAY_DEBUG("pos: 0x%x, size: 0x%x\n", overlay->DWINPOS, overlay->DWINSZ); @@ -2084,6 +2084,20 @@ i830_display_overlay(ScrnInfoPtr pScrn, xf86CrtcPtr crtc, i830_overlay_on (pScrn); /* and show this frame */ i830_overlay_continue (pScrn, scaleChanged); + + if (xvmc_passthrough(id, pPriv->rotation)) + return TRUE; + + /* unpin the old buffer and exchange buffers */ + if (pPriv->oldBuf_pinned) { + drm_intel_bo_unpin(pPriv->oldBuf); + } + tmp_buf = pPriv->buf; + pPriv->buf = pPriv->oldBuf; + pPriv->oldBuf = tmp_buf; + pPriv->oldBuf_pinned = TRUE; + + return TRUE; } static Bool @@ -2209,31 +2223,18 @@ i830_setup_video_buffer(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, I830Ptr pI830 = I830PTR(pScrn); /* Free the current buffer if we're going to have to reallocate */ if (pPriv->buf && pPriv->buf->size < alloc_size) { - if (!pPriv->textured) - drm_intel_bo_unpin(pPriv->buf); drm_intel_bo_unreference(pPriv->buf); pPriv->buf = NULL; } if (xvmc_passthrough(id, pPriv->rotation)) { - if (pPriv->buf) { - assert(pPriv->textured); - drm_intel_bo_unreference(pPriv->buf); - pPriv->buf = NULL; - } + i830_free_video_buffers(pPriv); } else { if (pPriv->buf == NULL) { pPriv->buf = drm_intel_bo_alloc(pI830->bufmgr, "xv buffer", alloc_size, 4096); if (pPriv->buf == NULL) return FALSE; - if (!pPriv->textured && drm_intel_bo_pin(pPriv->buf, 4096) != 0) { - drm_intel_bo_unreference(pPriv->buf); - pPriv->buf = NULL; - xf86DrvMsg(pScrn->scrnIndex, X_ERROR, - "Failed to pin xv buffer\n"); - return FALSE; - } } } @@ -2315,7 +2316,6 @@ i830_copy_video_data(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, I830Ptr pI830 = I830PTR(pScrn); int srcPitch = 0, srcPitch2 = 0; int top, left, npixels, nlines, size; - int alloc_size; if (is_planar_fourcc(id)) { srcPitch = (width + 0x3) & ~0x3; @@ -2327,11 +2327,7 @@ i830_copy_video_data(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, i830_dst_pitch_and_size(pScrn, pPriv, width, height, dstPitch, dstPitch2, &size, id); - alloc_size = size; - if (pPriv->doubleBuffer) - alloc_size *= 2; - - if (!i830_setup_video_buffer(pScrn, pPriv, alloc_size, id)) + if (!i830_setup_video_buffer(pScrn, pPriv, size, id)) return FALSE; /* fixup pointers */ @@ -2342,20 +2338,7 @@ i830_copy_video_data(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, pPriv->UBufOffset = pPriv->VBufOffset + (*dstPitch * height / 2); } else { #endif - if (pPriv->textured) - pPriv->YBufOffset = 0; - else - pPriv->YBufOffset = pPriv->buf->offset; - - /* switch buffers if double buffered */ - if (!pPriv->textured && pPriv->doubleBuffer) { - if (pPriv->currentBuf == 0) - pPriv->currentBuf = 1; - else - pPriv->currentBuf = 0; - - pPriv->YBufOffset += size*pPriv->currentBuf; - } + pPriv->YBufOffset = 0; if (pPriv->rotation & (RR_Rotate_90 | RR_Rotate_270)) { pPriv->UBufOffset = pPriv->YBufOffset + (*dstPitch * 2 * width); @@ -2377,12 +2360,12 @@ i830_copy_video_data(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, if (!xvmc_passthrough(id, pPriv->rotation)) { top &= ~1; nlines = ((((y2 + 0xffff) >> 16) + 1) & ~1) - top; - I830CopyPlanarData(pScrn, pPriv, buf, srcPitch, srcPitch2, *dstPitch, + I830CopyPlanarData(pPriv, buf, srcPitch, srcPitch2, *dstPitch, height, top, left, nlines, npixels, id); } } else { nlines = ((y2 + 0xffff) >> 16) - top; - I830CopyPackedData(pScrn, pPriv, buf, srcPitch, *dstPitch, top, left, + I830CopyPackedData(pPriv, buf, srcPitch, *dstPitch, top, left, nlines, npixels); } @@ -2475,9 +2458,10 @@ I830PutImage(ScrnInfoPtr pScrn, return BadAlloc; if (!pPriv->textured) { - i830_display_overlay(pScrn, crtc, id, width, height, dstPitch, + if (!i830_display_overlay(pScrn, crtc, id, width, height, dstPitch, x1, y1, x2, y2, &dstBox, src_w, src_h, - drw_w, drw_h); + drw_w, drw_h)) + return BadAlloc; /* update cliplist */ if (!REGION_EQUAL(pScrn->pScreen, &pPriv->clip, clipBoxes)) { @@ -2629,10 +2613,7 @@ I830VideoBlockHandler(int i, pointer blockData, pointer pTimeout, } } else { /* FREE_TIMER */ if (pPriv->freeTime < now) { - if (!pPriv->textured) - drm_intel_bo_unpin(pPriv->buf); - drm_intel_bo_unreference(pPriv->buf); - pPriv->buf = NULL; + i830_free_video_buffers(pPriv); pPriv->videoStatus = 0; } } diff --git a/src/i830_video.h b/src/i830_video.h index 9093d5d..cdccd16 100644 --- a/src/i830_video.h +++ b/src/i830_video.h @@ -32,14 +32,11 @@ typedef struct { uint32_t UBufOffset; uint32_t VBufOffset; - unsigned char currentBuf; - int brightness; int contrast; int saturation; xf86CrtcPtr current_crtc; xf86CrtcPtr desired_crtc; - int doubleBuffer; RegionRec clip; uint32_t colorKey; @@ -54,7 +51,10 @@ typedef struct { uint32_t videoStatus; Time offTime; Time freeTime; - drm_intel_bo *buf; /** YUV data buffer */ + /** YUV data buffers */ + drm_intel_bo *buf; /* current buffer to draw into */ + drm_intel_bo *oldBuf; /* old buffer, may be in use by the overlay hw */ + Bool oldBuf_pinned; /* only actually pinned when in use by the overlay hw */ Bool overlayOK; int oneLineMode;