[Intel-gfx] [PATCH 09/13] Xv: split up I830PutImage

Simon Farnsworth simon.farnsworth at onelan.com
Tue Jun 30 18:06:02 CEST 2009


Daniel Vetter wrote:
> This was the last monster-function. Also so small cleanups in
> I830PutImage.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  src/i830_video.c |  394 +++++++++++++++++++++++++++++-------------------------
>  1 files changed, 212 insertions(+), 182 deletions(-)
> 
> diff --git a/src/i830_video.c b/src/i830_video.c
> index 305530a..ff227ab 100644
> --- a/src/i830_video.c
> +++ b/src/i830_video.c
> @@ -2158,101 +2158,91 @@ i830_fill_colorkey (ScreenPtr pScreen, uint32_t key, RegionPtr clipboxes)
>     FreeScratchGC (gc);
>  }
>  
> -/*
> - * The source rectangle of the video is defined by (src_x, src_y, src_w, src_h).
> - * The dest rectangle of the video is defined by (drw_x, drw_y, drw_w, drw_h).
> - * id is a fourcc code for the format of the video.
> - * buf is the pointer to the source data in system memory.
> - * width and height are the w/h of the source data.
> - * If "sync" is TRUE, then we must be finished with *buf at the point of return
> - * (which we always are).
> - * clipBoxes is the clipping region in screen space.
> - * data is a pointer to our port private.
> - * pDraw is a Drawable, which might not be the screen in the case of
> - * compositing.  It's a new argument to the function in the 1.1 server.
> - */
> -static int
> -I830PutImage(ScrnInfoPtr pScrn,
> -	     short src_x, short src_y,
> -	     short drw_x, short drw_y,
> -	     short src_w, short src_h,
> -	     short drw_w, short drw_h,
> -	     int id, unsigned char *buf,
> -	     short width, short height,
> -	     Bool sync, RegionPtr clipBoxes, pointer data,
> -	     DrawablePtr pDraw)
> +static void
> +i830_wait_for_scanline(ScrnInfoPtr pScrn, PixmapPtr pPixmap,
> +	xf86CrtcPtr crtc, RegionPtr clipBoxes)
>  {
>      I830Ptr pI830 = I830PTR(pScrn);
> -    I830PortPrivPtr pPriv = (I830PortPrivPtr) data;
> -    ScreenPtr pScreen = screenInfo.screens[pScrn->scrnIndex];
> -    I830OverlayRegPtr overlay;
> -    PixmapPtr pPixmap = get_drawable_pixmap(pDraw);;
> -    INT32 x1, x2, y1, y2;
> -    int srcPitch = 0, srcPitch2 = 0, dstPitch;
> -    int dstPitch2 = 0;
> -    int top, left, npixels, nlines, size;
> -    BoxRec dstBox;
> -    int pitchAlignMask;
> -    int alloc_size;
> -    xf86CrtcPtr	crtc;
> +    BoxPtr box;
> +    int y1, y2;
> +    int pipe = -1, event, load_scan_lines_pipe;
>  
> -    if (pPriv->textured)
> -	overlay = NULL;
> -    else
> -	overlay = I830OVERLAYREG(pI830);
> +    if (pixmap_is_scanout(pPixmap)) 
> +	pipe = i830_crtc_to_pipe(crtc);
>  
> -#if 0
> -    ErrorF("I830PutImage: src: (%d,%d)(%d,%d), dst: (%d,%d)(%d,%d)\n"
> -	   "width %d, height %d\n", src_x, src_y, src_w, src_h, drw_x, drw_y,
> -	   drw_w, drw_h, width, height);
> -#endif
> -
> -    if (!pPriv->textured) {
> -        /* If dst width and height are less than 1/8th the src size, the
> -         * src/dst scale factor becomes larger than 8 and doesn't fit in
> -         * the scale register. */
> -        if(src_w >= (drw_w * 8))
> -            drw_w = src_w/7;
> +    if (pipe >= 0) {
> +	if (pipe == 0) {
> +	    event = MI_WAIT_FOR_PIPEA_SCAN_LINE_WINDOW;
> +	    load_scan_lines_pipe = MI_LOAD_SCAN_LINES_DISPLAY_PIPEA;
> +	} else {
> +	    event = MI_WAIT_FOR_PIPEB_SCAN_LINE_WINDOW;
> +	    load_scan_lines_pipe = MI_LOAD_SCAN_LINES_DISPLAY_PIPEB;
> +	}
>  
> -        if(src_h >= (drw_h * 8))
> -            drw_h = src_h/7;
> +	box = REGION_EXTENTS(unused, clipBoxes);
> +	y1 = box->y1 - crtc->y;
> +	y2 = box->y2 - crtc->y;
> +

You're missing another of Barry's patches :)
Subject is [Intel-gfx] [patch] Fix XV scan line calculation when rotated

It rotates the draw area *before* trying to work out which scan lines to
wait for - without it, the driver hangs when you use XVideo on a
portrait display. For example, playing a 768x576 video on a 800x600
display that's rotated by 90 degrees will hang.

> +	BEGIN_BATCH(5);
> +	/* The documentation says that the LOAD_SCAN_LINES command
> +	 * always comes in pairs. Don't ask me why. */
> +	OUT_BATCH(MI_LOAD_SCAN_LINES_INCL | load_scan_lines_pipe);
> +	OUT_BATCH((y1 << 16) | y2);
> +	OUT_BATCH(MI_LOAD_SCAN_LINES_INCL | load_scan_lines_pipe);
> +	OUT_BATCH((y1 << 16) | y2);
> +	OUT_BATCH(MI_WAIT_FOR_EVENT | event);
> +	ADVANCE_BATCH();
>      }
> +}
>  
> -    /* Clip */
> -    x1 = src_x;
> -    x2 = src_x + src_w;
> -    y1 = src_y;
> -    y2 = src_y + src_h;
> -
> -    dstBox.x1 = drw_x;
> -    dstBox.x2 = drw_x + drw_w;
> -    dstBox.y1 = drw_y;
> -    dstBox.y2 = drw_y + drw_h;
> -
> -    if (!i830_clip_video_helper(pScrn, 
> -				pPriv,
> -				&crtc,
> -				&dstBox, &x1, &x2, &y1, &y2, clipBoxes,
> -				width, height))
> -	return Success;
> -
> -     if (!pPriv->textured) {
> -	 /* texture video handles rotation differently. */
> -	if (crtc)
> -	    pPriv->rotation = crtc->rotation;
> -	else {
> -	    xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
> -		    "Fail to clip video to any crtc!\n");
> -	    return Success;
> -	}
> -     }
> +static Bool
> +i830_setup_video_buffer(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv,
> +	int alloc_size, int id)
> +{
> +    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 (is_planar_fourcc(id)) {
> -	srcPitch = (width + 0x3) & ~0x3;
> -	srcPitch2 = ((width >> 1) + 0x3) & ~0x3;
> +#ifdef INTEL_XVMC
> +    if (id == FOURCC_XVMC && 
> +        pPriv->rotation == RR_Rotate_0) {
> +        if (pPriv->buf) {
> +            assert(pPriv->textured);
> +            drm_intel_bo_unreference(pPriv->buf);
> +            pPriv->buf = NULL;
> +        }
>      } else {
> -	srcPitch = width << 1;
> +#endif
> +        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;
> +            }
> +        }
> +#ifdef INTEL_XVMC
>      }
> +#endif
> +    return TRUE;
> +}
> +
> +static void
> +i830_dst_pitch_and_size(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, short width,
> +	short height, int *dstPitch, int *dstPitch2, int *size, int id)
> +{
> +    I830Ptr pI830 = I830PTR(pScrn);
> +    int pitchAlignMask;
>  
>      /* Only needs to be DWORD-aligned for textured on i915, but overlay has
>       * stricter requirements.
> @@ -2278,85 +2268,75 @@ I830PutImage(ScrnInfoPtr pScrn,
>      case FOURCC_YV12:
>      case FOURCC_I420:
>  	if (pPriv->rotation & (RR_Rotate_90 | RR_Rotate_270)) {
> -	    dstPitch = ((height / 2) + pitchAlignMask) & ~pitchAlignMask;
> -	    size = dstPitch * width * 3;
> +	    *dstPitch = ((height / 2) + pitchAlignMask) & ~pitchAlignMask;
> +	    *size = *dstPitch * width * 3;
>  	} else {
> -	    dstPitch = ((width / 2) + pitchAlignMask) & ~pitchAlignMask;
> -	    size = dstPitch * height * 3;
> +	    *dstPitch = ((width / 2) + pitchAlignMask) & ~pitchAlignMask;
> +	    *size = *dstPitch * height * 3;
>  	}
>  	break;
>      case FOURCC_UYVY:
>      case FOURCC_YUY2:
>  
>  	if (pPriv->rotation & (RR_Rotate_90 | RR_Rotate_270)) {
> -	    dstPitch = ((height << 1) + pitchAlignMask) & ~pitchAlignMask;
> -	    size = dstPitch * width;
> +	    *dstPitch = ((height << 1) + pitchAlignMask) & ~pitchAlignMask;
> +	    *size = *dstPitch * width;
>  	} else {
> -	    dstPitch = ((width << 1) + pitchAlignMask) & ~pitchAlignMask;
> -	    size = dstPitch * height;
> +	    *dstPitch = ((width << 1) + pitchAlignMask) & ~pitchAlignMask;
> +	    *size = *dstPitch * height;
>  	}
>  	break;
>  #ifdef INTEL_XVMC
>      case FOURCC_XVMC:
> -	dstPitch = ((width / 2) + pitchAlignMask ) & ~pitchAlignMask;
> -	dstPitch2 = (width + pitchAlignMask ) & ~pitchAlignMask;
> -	size = 0;
> +	*dstPitch = ((width / 2) + pitchAlignMask ) & ~pitchAlignMask;
> +	*dstPitch2 = (width + pitchAlignMask ) & ~pitchAlignMask;
> +	*size = 0;
>  	break;
>  #endif
>      default:
> -	dstPitch = 0;
> -	size = 0;
> +	*dstPitch = 0;
> +	*size = 0;
>  	break;
>      }
>  #if 0
> -    ErrorF("srcPitch: %d, dstPitch: %d, size: %d\n", srcPitch, dstPitch, size);
> +    ErrorF("srcPitch: %d, dstPitch: %d, size: %d\n", srcPitch, *dstPitch, size);
>  #endif
> +}
> +
> +static Bool
> +i830_copy_video_data(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv, 
> +	     short width, short height, int *dstPitch, int *dstPitch2,
> +	     INT32 x1, INT32 y1, INT32 x2, INT32 y2,
> +	     int id, unsigned char *buf)
> +{
> +    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;
> +	srcPitch2 = ((width >> 1) + 0x3) & ~0x3;
> +    } else {
> +	srcPitch = width << 1;
> +    }
> +
> +    i830_dst_pitch_and_size(pScrn, pPriv, width, height, dstPitch, dstPitch2,
> +	    &size, id);
>  
>      alloc_size = size;
>      if (pPriv->doubleBuffer)
>  	alloc_size *= 2;
>  
> -    /* 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;
> -    }
> -
> -#ifdef INTEL_XVMC
> -    if (id == FOURCC_XVMC && 
> -        pPriv->rotation == RR_Rotate_0) {
> -        if (pPriv->buf) {
> -            assert(pPriv->textured);
> -            drm_intel_bo_unreference(pPriv->buf);
> -            pPriv->buf = NULL;
> -        }
> -    } else {
> -#endif
> -        if (pPriv->buf == NULL) {
> -            pPriv->buf = drm_intel_bo_alloc(pI830->bufmgr,
> -                                         "xv buffer", alloc_size, 4096);
> -            if (pPriv->buf == NULL)
> -                return BadAlloc;
> -            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 BadAlloc;
> -            }
> -        }
> -#ifdef INTEL_XVMC
> -    }
> -#endif
> +    if (!i830_setup_video_buffer(pScrn, pPriv, alloc_size, id))
> +	return FALSE;
>  
>      /* fixup pointers */
>  #ifdef INTEL_XVMC
>      if (id == FOURCC_XVMC && IS_I915(pI830)) {
>  	pPriv->YBufOffset = (uint32_t)((uintptr_t)buf);
> -	pPriv->VBufOffset = pPriv->YBufOffset + (dstPitch2 * height);
> -	pPriv->UBufOffset = pPriv->VBufOffset + (dstPitch * height / 2);
> +	pPriv->VBufOffset = pPriv->YBufOffset + (*dstPitch2 * height);
> +	pPriv->UBufOffset = pPriv->VBufOffset + (*dstPitch * height / 2);
>      } else {
>  #endif
>  	if (pPriv->textured)
> @@ -2375,11 +2355,11 @@ I830PutImage(ScrnInfoPtr pScrn,
>  	}
>  
>  	if (pPriv->rotation & (RR_Rotate_90 | RR_Rotate_270)) {
> -	    pPriv->UBufOffset = pPriv->YBufOffset + (dstPitch * 2 * width);
> -	    pPriv->VBufOffset = pPriv->UBufOffset + (dstPitch * width / 2);
> +	    pPriv->UBufOffset = pPriv->YBufOffset + (*dstPitch * 2 * width);
> +	    pPriv->VBufOffset = pPriv->UBufOffset + (*dstPitch * width / 2);
>  	} else {
> -	    pPriv->UBufOffset = pPriv->YBufOffset + (dstPitch * 2 * height);
> -	    pPriv->VBufOffset = pPriv->UBufOffset + (dstPitch * height / 2);
> +	    pPriv->UBufOffset = pPriv->YBufOffset + (*dstPitch * 2 * height);
> +	    pPriv->VBufOffset = pPriv->UBufOffset + (*dstPitch * height / 2);
>  	}
>  #ifdef INTEL_XVMC
>      }
> @@ -2395,15 +2375,103 @@ I830PutImage(ScrnInfoPtr pScrn,
>  		|| pPriv->rotation != RR_Rotate_0) {
>  	    top &= ~1;
>  	    nlines = ((((y2 + 0xffff) >> 16) + 1) & ~1) - top;
> -	    I830CopyPlanarData(pScrn, pPriv, buf, srcPitch, srcPitch2, dstPitch,
> +	    I830CopyPlanarData(pScrn, 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(pScrn, pPriv, buf, srcPitch, *dstPitch, top, left,
>  			   nlines, npixels);
>      }
>  
> +    return TRUE;
> +}
> +
> +/*
> + * The source rectangle of the video is defined by (src_x, src_y, src_w, src_h).
> + * The dest rectangle of the video is defined by (drw_x, drw_y, drw_w, drw_h).
> + * id is a fourcc code for the format of the video.
> + * buf is the pointer to the source data in system memory.
> + * width and height are the w/h of the source data.
> + * If "sync" is TRUE, then we must be finished with *buf at the point of return
> + * (which we always are).
> + * clipBoxes is the clipping region in screen space.
> + * data is a pointer to our port private.
> + * pDraw is a Drawable, which might not be the screen in the case of
> + * compositing.  It's a new argument to the function in the 1.1 server.
> + */
> +static int
> +I830PutImage(ScrnInfoPtr pScrn,
> +	     short src_x, short src_y,
> +	     short drw_x, short drw_y,
> +	     short src_w, short src_h,
> +	     short drw_w, short drw_h,
> +	     int id, unsigned char *buf,
> +	     short width, short height,
> +	     Bool sync, RegionPtr clipBoxes, pointer data,
> +	     DrawablePtr pDraw)
> +{
> +    I830Ptr pI830 = I830PTR(pScrn);
> +    I830PortPrivPtr pPriv = (I830PortPrivPtr) data;
> +    ScreenPtr pScreen = screenInfo.screens[pScrn->scrnIndex];
> +    PixmapPtr pPixmap = get_drawable_pixmap(pDraw);;
> +    INT32 x1, x2, y1, y2;
> +    int dstPitch;
> +    int dstPitch2 = 0;
> +    BoxRec dstBox;
> +    xf86CrtcPtr	crtc;
> +
> +#if 0
> +    ErrorF("I830PutImage: src: (%d,%d)(%d,%d), dst: (%d,%d)(%d,%d)\n"
> +	   "width %d, height %d\n", src_x, src_y, src_w, src_h, drw_x, drw_y,
> +	   drw_w, drw_h, width, height);
> +#endif
> +
> +    if (!pPriv->textured) {
> +        /* If dst width and height are less than 1/8th the src size, the
> +         * src/dst scale factor becomes larger than 8 and doesn't fit in
> +         * the scale register. */
> +        if(src_w >= (drw_w * 8))
> +            drw_w = src_w/7;
> +
> +        if(src_h >= (drw_h * 8))
> +            drw_h = src_h/7;
> +    }
> +
> +    /* Clip */
> +    x1 = src_x;
> +    x2 = src_x + src_w;
> +    y1 = src_y;
> +    y2 = src_y + src_h;
> +
> +    dstBox.x1 = drw_x;
> +    dstBox.x2 = drw_x + drw_w;
> +    dstBox.y1 = drw_y;
> +    dstBox.y2 = drw_y + drw_h;
> +
> +    if (!i830_clip_video_helper(pScrn, 
> +				pPriv,
> +				&crtc,
> +				&dstBox, &x1, &x2, &y1, &y2, clipBoxes,
> +				width, height))
> +	return Success;
> +
> +    if (!pPriv->textured) {
> +	 /* texture video handles rotation differently. */
> +	if (crtc)
> +	    pPriv->rotation = crtc->rotation;
> +	else {
> +	    xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
> +		    "Fail to clip video to any crtc!\n");
> +	    return Success;
> +	}
> +    }
> +
> +    if (!i830_copy_video_data(pScrn, pPriv, width, height,
> +		&dstPitch, &dstPitch2,
> +		x1, y1, x2, y2, id, buf))
> +	return BadAlloc;
> +
>      if (!pPriv->textured) {
>  	i830_display_overlay(pScrn, crtc, id, width, height, dstPitch,
>  			   x1, y1, x2, y2, &dstBox, src_w, src_h,
> @@ -2415,45 +2483,8 @@ I830PutImage(ScrnInfoPtr pScrn,
>  	    i830_fill_colorkey (pScreen, pPriv->colorKey, clipBoxes);
>  	}
>      } else {
> -        Bool sync = TRUE;
> -        
> -        if (crtc == NULL) {
> -            sync = FALSE;
> -        } else if (pPriv->SyncToVblank == 0) {
> -            sync = FALSE;
> -        }
> -
> -        if (sync) {
> -	    BoxPtr box;
> -	    int y1, y2;
> -	    int pipe = -1, event, load_scan_lines_pipe;
> -
> -	    if (pixmap_is_scanout(pPixmap))
> -		pipe = i830_crtc_to_pipe(crtc);
> -
> -	    if (pipe >= 0) {
> -		if (pipe == 0) {
> -		    event = MI_WAIT_FOR_PIPEA_SCAN_LINE_WINDOW;
> -		    load_scan_lines_pipe = MI_LOAD_SCAN_LINES_DISPLAY_PIPEA;
> -		} else {
> -		    event = MI_WAIT_FOR_PIPEB_SCAN_LINE_WINDOW;
> -		    load_scan_lines_pipe = MI_LOAD_SCAN_LINES_DISPLAY_PIPEB;
> -		}
> -
> -		box = REGION_EXTENTS(unused, clipBoxes);
> -		y1 = box->y1 - crtc->y;
> -		y2 = box->y2 - crtc->y;
> -
> -		BEGIN_BATCH(5);
> -		/* The documentation says that the LOAD_SCAN_LINES command
> -		 * always comes in pairs. Don't ask me why. */
> -		OUT_BATCH(MI_LOAD_SCAN_LINES_INCL | load_scan_lines_pipe);
> -		OUT_BATCH((y1 << 16) | y2);
> -		OUT_BATCH(MI_LOAD_SCAN_LINES_INCL | load_scan_lines_pipe);
> -		OUT_BATCH((y1 << 16) | y2);
> -		OUT_BATCH(MI_WAIT_FOR_EVENT | event);
> -		ADVANCE_BATCH();
> -	    }
> +        if (!crtc || pPriv->SyncToVblank != 0) {
> +	    i830_wait_for_scanline(pScrn, pPixmap, crtc, clipBoxes);
>          }
>  
>          if (IS_I965G(pI830)) {
> @@ -2472,8 +2503,7 @@ I830PutImage(ScrnInfoPtr pScrn,
>                                       dstPitch, dstPitch2, x1, y1, x2, y2,
>                                       src_w, src_h, drw_w, drw_h, pPixmap);
>          }
> -    }
> -    if (pPriv->textured) {
> +
>  	DamageDamageRegion(pDraw, clipBoxes);
>      }
>  


-- 
Simon Farnsworth




More information about the Intel-gfx mailing list