[Intel-gfx] [PATCH] i830: use GTT maps for fallbacks & fence setup

[Eric Anholt]

On Thu, 2009-03-26 at 18:08 -0700, Jesse Barnes wrote:
> On Thu, 26 Mar 2009 16:45:58 -0700
> Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> 
> > On Thu, 26 Mar 2009 16:41:02 -0700
> > Eric Anholt <eric at anholt.net> wrote:
> > 
> > > On Thu, 2009-03-26 at 12:10 -0700, Jesse Barnes wrote:
> > > > Corresponding 2D patch to use GTT mapping where necessary for
> > > > tiling and fallback support.
> > > 
> > > Err, so if you don't have GTT mapping support (2.6.29), you just
> > > lose?
> > 
> > Any suggestions on how to handle it?  I guess I could key off of
> > kernel_exec_fencing and fallback to the pin +
> > drm_intel_gem_bo_start_gtt_access stuff if it's not there...
> 
> Ok how about this.  Should cover EXA, UXA, XAA, KMS and no-KMS, kernel
> fence reg management and not (just testing the last bit now).
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center
> 
> diff --git a/src/i830_driver.c b/src/i830_driver.c
> index 7502d30..2905f76 100644
> --- a/src/i830_driver.c
> +++ b/src/i830_driver.c
> @@ -833,10 +833,23 @@ i830_update_front_offset(ScrnInfoPtr pScrn)
>      * yet.  We'll fix it up at CreateScreenResources.
>      */
>     if (!pI830->starting && pI830->accel != ACCEL_UXA) {
> +      dri_bo *bo = pI830->front_buffer->bo;
> +      pointer data = pI830->FbBase + pScrn->fbOffset; /* default to legacy */
> +
> +      if (bo) {
> +	  if (pI830->kernel_exec_fencing) {
> +	      if (drm_intel_gem_bo_map_gtt(bo))
> +		  xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "%s: bo map failed\n",
> +			     __FUNCTION__);
> +	  } else {
> +	      /* Will already be pinned by bind_all_memory in this case */
> +	      drm_intel_gem_bo_start_gtt_access(bo, 1);
> +	  }
> +	  data = bo->virtual;
> +      }
>        if (!pScreen->ModifyPixmapHeader(pScreen->GetScreenPixmap(pScreen),
>  				       pScrn->virtualX, pScrn->virtualY, -1, -1,
> -				       pitch, (pointer)(pI830->FbBase +
> -							pScrn->fbOffset)))
> +				       pitch, data))
>         FatalError("Couldn't adjust screen pixmap\n");
>     }
>  }

If something actually dereferences that pointer, it's not going to get
the sort of results it's hoping for.  What happens if we set it to NULL,
anyone know?  EXA/UXA should be cool with that, I think.

I'm uncomfortable with the mismatched map/unmap here.

> diff --git a/src/i830_exa.c b/src/i830_exa.c
> index 0a22486..1710e24 100644
> --- a/src/i830_exa.c
> +++ b/src/i830_exa.c
> @@ -37,6 +37,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>  #include "i810_reg.h"
>  #include "i915_drm.h"
>  #include <string.h>
> +#include <sys/mman.h>
>  
>  #define ALWAYS_SYNC		0
>  #define ALWAYS_FLUSH		0
> @@ -837,8 +838,6 @@ i830_uxa_set_pixmap_bo (PixmapPtr pixmap, dri_bo *bo)
>  static Bool
>  i830_uxa_prepare_access (PixmapPtr pixmap, uxa_access_t access)
>  {
> -    ScrnInfoPtr pScrn = xf86Screens[pixmap->drawable.pScreen->myNum];
> -    I830Ptr pI830 = I830PTR(pScrn);
>      dri_bo *bo = i830_get_pixmap_bo (pixmap);
>  
>      if (bo) {
> @@ -853,16 +852,22 @@ i830_uxa_prepare_access (PixmapPtr pixmap, uxa_access_t access)
>  	    i830->need_sync = FALSE;
>  	}
>  
> -	if (pScrn->vtSema && !pI830->use_drm_mode && pI830->memory_manager) {
> +	if (!scrn->vtSema)
> +	    return TRUE;

Err, if !scrn->vtSema we'll now dereference NULL for software rendering,
I think.

> +
> +	/* Kernel manages fences at GTT map/fault time */
> +	if (i830->kernel_exec_fencing) {
> +	    if (drm_intel_gem_bo_map_gtt(bo)) {
> +		xf86DrvMsg(scrn->scrnIndex, X_WARNING, "%s: bo map failed\n",
> +			   __FUNCTION__);
> +		return FALSE;
> +	    }
> +	} else { /* or not... */
>  	    if (drm_intel_bo_pin(bo, 4096) != 0)
>  		return FALSE;
>  	    drm_intel_gem_bo_start_gtt_access(bo, access == UXA_ACCESS_RW);
> -	    pixmap->devPrivate.ptr = pI830->FbBase + bo->offset;
> -	} else {
> -	    if (dri_bo_map(bo, access == UXA_ACCESS_RW) != 0)
> -		return FALSE;
> -	    pixmap->devPrivate.ptr = bo->virtual;
>  	}
> +	pixmap->devPrivate.ptr = bo->virtual;
>      }
>      return TRUE;
>  }

non-GEM UXA will now completely fail, I think, since drm_intel_bo_pin
won't work.  It was so slow it couldn't really be called working, so I'm
not too worried (and if it were to be made decent, it would probably
involve implementing bo_pin).

> @@ -870,8 +875,6 @@ i830_uxa_prepare_access (PixmapPtr pixmap, uxa_access_t access)
>  static void
>  i830_uxa_finish_access (PixmapPtr pixmap)
>  {
> -    ScrnInfoPtr pScrn = xf86Screens[pixmap->drawable.pScreen->myNum];
> -    I830Ptr pI830 = I830PTR(pScrn);
>      dri_bo *bo = i830_get_pixmap_bo (pixmap);
>  
>      if (bo) {
> @@ -879,11 +882,13 @@ i830_uxa_finish_access (PixmapPtr pixmap)
>  	ScrnInfoPtr scrn = xf86Screens[screen->myNum];
>  	I830Ptr i830 = I830PTR(scrn);
>  
> -	if (pScrn->vtSema && !pI830->use_drm_mode && pI830->memory_manager)
> -	    drm_intel_bo_unpin(bo);
> -	else
> -	    dri_bo_unmap(bo);
> +	if (!scrn->vtSema)
> +	    return;
>  
> +	if (i830->kernel_exec_fencing)
> +	    drm_intel_gem_bo_unmap_gtt(bo);
> +	else
> +	    drm_intel_bo_unpin(bo);
>  	pixmap->devPrivate.ptr = NULL;
>  	if (bo == i830->front_buffer->bo)
>  	    i830->need_flush = TRUE;
-- 
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/20090326/ccf0a34e/attachment.sig>