[Intel-gfx] [PATCH] support tiled rendering in 2D driver

Eric Anholt eric at anholt.net
Sat Jan 24 01:24:05 CET 2009


On Fri, 2009-01-23 at 14:15 -0800, Jesse Barnes wrote:
> Set alignments, tile settings and flags correctly in the 2D driver to support
> tiled rendering.  UXA's create pixmap function currently assumes the worst
> about the alignment constraints; that should probably be fixed.  Some of the
> 1M alignment fixes could probably be done more cleanly as well.
> 
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center
> 
> diff --git a/src/i830.h b/src/i830.h
> index 4794169..f9444b1 100644
> --- a/src/i830.h
> +++ b/src/i830.h
> @@ -969,6 +969,7 @@ i830_get_transformed_coordinates_3d(int x, int y, PictTransformPtr transform,
>  				    float *x_out, float *y_out, float *z_out);
>  
>  void i830_enter_render(ScrnInfoPtr);
> +unsigned long I830GetFencePitch(I830Ptr pI830, unsigned long pitch, int format);
>  
>  static inline void
>  i830_wait_ring_idle(ScrnInfoPtr pScrn)
> diff --git a/src/i830_dri.c b/src/i830_dri.c
> index d6698da..72c8a38 100644
> --- a/src/i830_dri.c
> +++ b/src/i830_dri.c
> @@ -1525,6 +1525,34 @@ I830DRIUnlock(ScrnInfoPtr pScrn)
>  
>  #ifdef DRI2
>  
> +/**
> + * On some chips, pitch width has to be a power of two tile width, so
> + * calculate that here.
> + */
> +unsigned long
> +I830GetFencePitch(I830Ptr pI830, unsigned long pitch, int format)

DeathToStudlyCaps()

> +{
> +    unsigned long i;
> +    unsigned long tile_width = 512;
> +
> +    if (IS_I965G(pI830)) {
> +	/* 965 is flexible */
> +	if (format == I915_TILING_Y)
> +	    tile_width = 128;
> +
> +	return ROUND_TO(pitch, tile_width);
> +    }
> +
> +    if (IS_I945GM(pI830) && format == I915_TILING_Y)
> +	tile_width = 128;

I'd skip the chipset-based conditionals here.  If they asked for
TILING_Y, give them the TILING_Y-aligned width.  You missed
IS_I945G(pI830) here.

> +    /* Pre-965 needs power of two tile width */
> +    for (i = tile_width; i < pitch; i <<= 1)
> +	;
> +
> +    return ROUND_TO(pitch, i);

this is just return i, right?

> +}
> +
>  typedef struct {
>      PixmapPtr pPixmap;
>  } I830DRI2BufferPrivateRec, *I830DRI2BufferPrivatePtr;
> @@ -1570,7 +1598,7 @@ I830DRI2CreateBuffers(DrawablePtr pDraw, unsigned int *attachments, int count)
>  					       pDraw->depth, 0);
>  	    switch (attachments[i]) {
>  	    case DRI2BufferDepth:
> -		if (IS_I965G(pI830))
> +		if (IS_I965G(pI830) || IS_I945GM(pI830))
>  		    tiling = I915_TILING_Y;
>  		else
>  		    tiling = I915_TILING_X;

Interesting.  This is the first time we've done TILING_Y on pre-965.  I
think Mesa is prepared for it.  Oh, and 945G probably wants it too in
that case.

> @@ -1583,19 +1611,15 @@ I830DRI2CreateBuffers(DrawablePtr pDraw, unsigned int *attachments, int count)
>  		break;
>  	    }
>  
> -	    /* Disable tiling on 915-class 3D for now.  Because the 2D blitter
> -	     * requires fence regs to operate, and they're not being managed
> -	     * by the kernel yet, we don't want to expose tiled buffers to the
> -	     * 3D client as it'll just render incorrectly if it pays attention
> -	     * to our tiling bits at all.
> -	     */
> -	    if (!IS_I965G(pI830))
> +	    if (!pI830->tiling)
>  		tiling = I915_TILING_NONE;
>  

Since we've got released kernels without the support, we should probably
only enable this if we got success from the ioctl for setting the number
of fences or from getting available fences.

>  	    if (tiling != I915_TILING_NONE) {
> +		int pitch = (pDraw->width * pDraw->bitsPerPixel + 7) / 8;
> +		int stride = I830GetFencePitch(pI830, pitch, tiling);
> +
>  		bo = i830_get_pixmap_bo(pPixmap);
> -		drm_intel_bo_set_tiling(bo, &tiling,
> -					pDraw->width * pDraw->bitsPerPixel / 8);
> +		drm_intel_bo_set_tiling(bo, &tiling, stride);

If stride is anything but i830_pixmap_pitch(pPixmap), we'd be in for
some serious trouble.  Let's just use that.

>  	    }
>  	}
>  
> diff --git a/src/i830_exa.c b/src/i830_exa.c
> index 9249074..3e487f9 100644
> --- a/src/i830_exa.c
> +++ b/src/i830_exa.c
> @@ -872,7 +872,7 @@ i830_uxa_create_pixmap (ScreenPtr screen, int w, int h, int depth, unsigned usag
>      ScrnInfoPtr scrn = xf86Screens[screen->myNum];
>      I830Ptr i830 = I830PTR(scrn);
>      dri_bo *bo;
> -    int stride;
> +    int stride, align;
>      PixmapPtr pixmap;
>      
>      if (w > 32767 || h > 32767)
> @@ -884,9 +884,12 @@ i830_uxa_create_pixmap (ScreenPtr screen, int w, int h, int depth, unsigned usag
>      {
>  	stride = ROUND_TO((w * pixmap->drawable.bitsPerPixel + 7) / 8,
>  			  i830->accel_pixmap_pitch_alignment);
> +
> +	stride = I830GetFencePitch(i830, stride, I915_TILING_X);
> +
> +	align = stride * h > 1024*1024 ? stride * h : 1024*1024;

If we're going to have to calculate alignment in userland for the
purpose of tiling, it should be a separate function like it is in the
kernel, and match its behavior (I still think the kernel should just do
it for us).

> -	bo = dri_bo_alloc (i830->bufmgr, "pixmap", stride * h, 
> -			   i830->accel_pixmap_offset_alignment);
> +	bo = dri_bo_alloc (i830->bufmgr, "pixmap", stride * h, align);
>  	if (!bo) {
>  	    fbDestroyPixmap (pixmap);
>  	    return NullPixmap;
> diff --git a/src/i830_memory.c b/src/i830_memory.c
> index 9bfee81..7d55910 100644
> --- a/src/i830_memory.c
> +++ b/src/i830_memory.c
> @@ -388,6 +388,7 @@ i830_allocator_init(ScrnInfoPtr pScrn, unsigned long offset, unsigned long size)
>  #ifdef XF86DRI
>      int dri_major, dri_minor, dri_patch;
>      struct drm_i915_getparam gp;
> +    struct drm_i915_setparam sp;
>      int has_gem;
>      int has_dri;
>  #endif
> @@ -495,6 +496,17 @@ i830_allocator_init(ScrnInfoPtr pScrn, unsigned long offset, unsigned long size)
>  				   TILE_NONE);
>  
>  	if (pI830->memory_manager != NULL) {
> +	    sp.param = I915_SETPARAM_NUM_USED_FENCES;
> +	    if (pI830->use_drm_mode)
> +		sp.value = 0; /* kernel gets them all */
> +	    else if (pI830->directRenderingType == DRI_XF86DRI)
> +		sp.value = 3; /* front/back/depth */
> +	    else
> +		sp.value = 2; /* just front for DRI2 (both old & new though) */
> +	    /* Don't care about failure, we'll just get slow rendering */
> +	    drmCommandWrite(pI830->drmSubFD, DRM_I915_SETPARAM, &sp,
> +			    sizeof(sp));
> +

Well, we'll get wrong rendering as well if we enable tiling despite not
having a new enough kernel.  Any SW fallbacks will use the tiling state
rather than the unset fence registers, and you'll get swizzled.

-- 
Eric Anholt
eric at anholt.net                         eric.anholt at intel.com


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20090123/06440ec2/attachment.sig>


More information about the Intel-gfx mailing list