On 6/14/07, Keith Whitwell <[EMAIL PROTECTED]> wrote:
Kristian Høgsberg wrote:
> Hi,
>
> I've finished the changes to the DRI interface that I've been talking
> about for a while (see #5714). Ian had a look at the DRI driver side
> of things, and ACK'ed those changes.  I've done the X server changes
> now plus a couple of GLX module cleanups, and I think it's all ready
> to push:
>
>   http://gitweb.freedesktop.org/?p=users/krh/mesa.git;a=shortlog;h=dri2 and
>   http://gitweb.freedesktop.org/?p=users/krh/xserver.git;a=shortlog;h=dri2
>
> One thing that's still missing is Alan H's changes to how DRI/DDX maps
> the front buffer.  While the changes above break the DRI interface,
> they only require an X server and a Mesa update. Alans patches change
> the device private shared between the DDX and DRI driver and thus
> requires updating every DRI capable DDX driver in a non-compatible
> way.

Kristian,

Just letting you know Alan's on holidays this week, back on Monday.

Ah, thanks.  I was talking to Dave about it in IRC and it sounds like
we can let the DDX driver add the front buffer map and let the DRI
driver set it up without breaking the shared private struct.  I've
attached three patches (on top of my dri2 work) to illustrate the
idea.  What's missing is the i810 specific setup in mesa and a
mechanism to indicate whether or not dri_util.c should map the buffer
or if the DRI driver will do that.  But that's all behind the DRI
interface, so it's not a big deal.

I'm not sure why we're doing this though.  It looks like things are
mostly working as it is, it's just not very elegant.  And if we're
about to shake things up with DRM memory manager enabled DDX and DRI
drivers, do we want to add a hack like this?

Kristian
commit 42ed8fb718bd552a8b6a03efa0e739cc9ccf9818
Author: Kristian Høgsberg <[EMAIL PROTECTED]>
Date:   Fri Jun 15 18:52:54 2007 -0400

    Tell libdri not to map the front buffer if it's at least 5.4.0.

diff --git a/src/i810_dri.c b/src/i810_dri.c
index 72718d3..fc081ca 100644
--- a/src/i810_dri.c
+++ b/src/i810_dri.c
@@ -295,6 +295,7 @@ I810DRIScreenInit(ScreenPtr pScreen)
    unsigned long tom;
    drm_handle_t agpHandle;
    drm_handle_t dcacheHandle;
+   int major, minor, patch;
    int sysmem_size = 0;
    int back_size = 0;
    unsigned int pitch_idx = 0;
@@ -320,18 +321,14 @@ I810DRIScreenInit(ScreenPtr pScreen)
    }
 
    /* Check the DRI version */
-   {
-      int major, minor, patch;
-
-      DRIQueryVersion(&major, &minor, &patch);
-      if (major != DRIINFO_MAJOR_VERSION || minor < DRIINFO_MINOR_VERSION) {
-	 xf86DrvMsg(pScreen->myNum, X_ERROR,
-		    "[dri] I810DRIScreenInit failed because of a version mismatch.\n"
-		    "[dri] libdri version is %d.%d.%d bug version %d.%d.x is needed.\n"
-		    "[dri] Disabling DRI.\n", major, minor, patch,
-                    DRIINFO_MAJOR_VERSION, DRIINFO_MINOR_VERSION);
-	 return FALSE;
-      }
+   DRIQueryVersion(&major, &minor, &patch);
+   if (major != DRIINFO_MAJOR_VERSION || minor < DRIINFO_MINOR_VERSION) {
+       xf86DrvMsg(pScreen->myNum, X_ERROR,
+		  "[dri] I810DRIScreenInit failed because of a version mismatch.\n"
+		  "[dri] libdri version is %d.%d.%d bug version %d.%d.x is needed.\n"
+		  "[dri] Disabling DRI.\n", major, minor, patch,
+		  DRIINFO_MAJOR_VERSION, DRIINFO_MINOR_VERSION);
+       return FALSE;
    }
 
    pDRIInfo = DRICreateInfoRec();
@@ -361,12 +358,17 @@ I810DRIScreenInit(ScreenPtr pScreen)
    pDRIInfo->ddxDriverMajorVersion = I810_MAJOR_VERSION;
    pDRIInfo->ddxDriverMinorVersion = I810_MINOR_VERSION;
    pDRIInfo->ddxDriverPatchVersion = I810_PATCHLEVEL;
-   pDRIInfo->frameBufferPhysicalAddress = (pointer) pI810->LinearAddr;
-   pDRIInfo->frameBufferSize = (((pScrn->displayWidth *
-				  pScrn->virtualY * pI810->cpp) +
-				 4096 - 1) / 4096) * 4096;
 
+   pDRIInfo->frameBufferPhysicalAddress = (pointer) pI810->LinearAddr;
+   pDRIInfo->frameBufferSize =
+       ROUND_TO_PAGE(pScrn->displayWidth * pScrn->virtualY * pI810->cpp);
    pDRIInfo->frameBufferStride = pScrn->displayWidth * pI810->cpp;
+
+#if DRIINFO_MAJOR_VERSION == 5 && DRIINFO_MINOR_VERSION >= 4
+   if (major == 5 || minor >= 4)
+       pDRIInfo->dontMapFrameBuffer = TRUE;
+#endif
+
    pDRIInfo->ddxDrawableTableEntry = I810_MAX_DRAWABLES;
 
    if (SAREA_MAX_DRAWABLES < I810_MAX_DRAWABLES)
@@ -407,8 +409,9 @@ I810DRIScreenInit(ScreenPtr pScreen)
    pDRIInfo->createDummyCtx = TRUE;
    pDRIInfo->createDummyCtxPriv = FALSE;
 
-   /* This adds the framebuffer as a drm map *before* we have asked agp
-    * to allocate it.  Scary stuff, hold on...
+   /* For pre 5.4.0 libdri this adds the framebuffer as a drm map
+    * *before* we have asked agp to allocate it.  Scary stuff, hold
+    * on...
     */
    if (!DRIScreenInit(pScreen, pDRIInfo, &pI810->drmSubFD)) {
       xf86DrvMsg(pScreen->myNum, X_ERROR,
@@ -860,8 +863,21 @@ I810DRIScreenInit(ScreenPtr pScreen)
    pI810->auxPitch = i810_pitches[pitch_idx];
    pI810->auxPitchBits = i810_pitch_flags[pitch_idx];
    pI810->SavedDcacheMem = pI810->DcacheMem;
-   pI810DRI->backbufferSize = pI810->BackBuffer.Size;
 
+#if DRIINFO_MAJOR_VERSION == 5 && DRIINFO_MINOR_VERSION >= 4
+   if (major == 5 && minor >= 4) {
+       if (drmAddMap(pI810->drmSubFD, (drm_handle_t) pI810->LinearAddr,
+		     pDRIInfo->frameBufferSize, DRM_FRAME_BUFFER, 0,
+		     &pDRIInfo->hFrameBuffer) < 0) {
+	   xf86DrvMsg(pScreen->myNum, X_ERROR,
+		      "[drm] drmAddMap(frontbuffer) failed.  Disabling DRI\n");
+	   DRICloseScreen(pScreen);
+	   return FALSE;
+       }
+   }
+#endif
+
+   pI810DRI->backbufferSize = pI810->BackBuffer.Size;
    if (drmAddMap(pI810->drmSubFD, (drm_handle_t) pI810->BackBuffer.Start,
 		 pI810->BackBuffer.Size, DRM_AGP, 0,
 		 (drmAddress) &pI810DRI->backbuffer) < 0) {
diff --git a/src/i830_dri.c b/src/i830_dri.c
index a17770b..9650257 100644
--- a/src/i830_dri.c
+++ b/src/i830_dri.c
@@ -546,15 +546,11 @@ I830DRIScreenInit(ScreenPtr pScreen)
    pDRIInfo->ddxDriverMajorVersion = I830_MAJOR_VERSION;
    pDRIInfo->ddxDriverMinorVersion = I830_MINOR_VERSION;
    pDRIInfo->ddxDriverPatchVersion = I830_PATCHLEVEL;
-#if 1 /* Remove this soon - see bug 5714 */
+
    pDRIInfo->frameBufferPhysicalAddress = (char *) pI830->LinearAddr +
 					  pI830->front_buffer->offset;
    pDRIInfo->frameBufferSize = ROUND_TO_PAGE(pScrn->displayWidth *
 					     pScrn->virtualY * pI830->cpp);
-#else
-   /* For rotation we map a 0 length framebuffer as we remap ourselves later */
-   pDRIInfo->frameBufferSize = 0;
-#endif
    pDRIInfo->frameBufferStride = pScrn->displayWidth * pI830->cpp;
    pDRIInfo->ddxDrawableTableEntry = I830_MAX_DRAWABLES;
 
@@ -597,6 +593,11 @@ I830DRIScreenInit(ScreenPtr pScreen)
 
       DRIQueryVersion(&major, &minor, &patch);
 
+#if DRIINFO_MAJOR_VERSION == 5 && DRIINFO_MINOR_VERSION >= 4
+   if (major == 5 && minor >= 4)
+       pDRIInfo->dontMapFrameBuffer = TRUE;
+#endif
+
 #if DRIINFO_MAJOR_VERSION == 5 && DRIINFO_MINOR_VERSION >= 3
       if (minor >= 3)
 #endif
@@ -637,27 +638,6 @@ I830DRIScreenInit(ScreenPtr pScreen)
       return FALSE;
    }
 
-#if 0 /* disabled now, see frameBufferSize above being set to 0 */
-   /* for this driver, get rid of the front buffer mapping now */
-   if (xf86LoaderCheckSymbol("DRIGetScreenPrivate")) {
-      DRIScreenPrivPtr pDRIPriv 
-         = (DRIScreenPrivPtr) DRIGetScreenPrivate(pScreen);
-
-      if (pDRIPriv && pDRIPriv->drmFD && pDRIPriv->hFrameBuffer) {
-         xf86DrvMsg(pScreen->myNum, X_ERROR,
-                    "[intel] removing original screen mapping\n");
-         drmRmMap(pDRIPriv->drmFD, pDRIPriv->hFrameBuffer);
-         pDRIPriv->hFrameBuffer = 0;
-         xf86DrvMsg(pScreen->myNum, X_ERROR,
-                    "[intel] done removing original screen mapping\n");
-      }
-   }
-   else {
-      xf86DrvMsg(pScreen->myNum, X_ERROR,
-                 "[intel] DRIGetScreenPrivate not found!!!!\n");
-   }      
-#endif
-
    /* Check the i915 DRM versioning */
    {
       drmVersionPtr version;
@@ -775,10 +755,10 @@ I830DRIMapScreenRegions(ScrnInfoPtr pScrn, drmI830Sarea *sarea)
    ScreenPtr pScreen = pScrn->pScreen;
    I830Ptr pI830 = I830PTR(pScrn);
 
-#if 1 /* Remove this soon - see bug 5714 */
+   pI830->pDRIInfo->frameBufferPhysicalAddress = (char *) pI830->LinearAddr +
+					  pI830->front_buffer->offset;
    pI830->pDRIInfo->frameBufferSize = ROUND_TO_PAGE(pScrn->displayWidth *
 					     pScrn->virtualY * pI830->cpp);
-#endif
 
    /* The I965G isn't ready for the front buffer mapping to be moved around,
     * because of issues with rmmap, it seems.
@@ -797,6 +777,18 @@ I830DRIMapScreenRegions(ScrnInfoPtr pScrn, drmI830Sarea *sarea)
 	 DRICloseScreen(pScreen);
 	 return FALSE;
       }
+
+#if DRIINFO_MAJOR_VERSION == 5 && DRIINFO_MINOR_VERSION >= 4
+      {
+	  int major, minor, patch;
+
+	  DRIQueryVersion(&major, &minor, &patch);
+
+	  if (major == 5 && minor >= 4)
+	      pI830->pDRIInfo->hFrameBuffer = sarea->front_handle;
+      }
+#endif
+
       xf86DrvMsg(pScrn->scrnIndex, X_INFO, "[drm] Front Buffer = 0x%08x\n",
 		 (int)sarea->front_handle);
    }
commit 3a08b8f962970f4cb7bdb108c511956944c6d380
Author: Kristian Høgsberg <[EMAIL PROTECTED]>
Date:   Fri Jun 15 15:29:00 2007 -0400

    Don't map the front buffer in libdri if the ddx driver doesn't set the size.
    
    This lets drivers map the front buffer themselves by setting the size
    to 0 before calling DRIScreenInit.

diff --git a/hw/xfree86/dri/dri.c b/hw/xfree86/dri/dri.c
index 1e73539..c563099 100644
--- a/hw/xfree86/dri/dri.c
+++ b/hw/xfree86/dri/dri.c
@@ -415,23 +415,29 @@ DRIScreenInit(ScreenPtr pScreen, DRIInfoPtr pDRIInfo, int *pDRMFD)
     pDRIPriv->hLSAREA = pDRIEntPriv->hLSAREA;
     pDRIPriv->pLSAREA = pDRIEntPriv->pLSAREA;
 
-    if (drmAddMap( pDRIPriv->drmFD,
-		   (drm_handle_t)pDRIPriv->pDriverInfo->frameBufferPhysicalAddress,
-		   pDRIPriv->pDriverInfo->frameBufferSize,
-		   DRM_FRAME_BUFFER,
-		   0,
-		   &pDRIPriv->hFrameBuffer) < 0)
+    if (pDRIPriv->pDriverInfo->frameBufferSize > 0)
     {
-	pDRIPriv->directRenderingSupport = FALSE;
-	pScreen->devPrivates[DRIScreenPrivIndex].ptr = NULL;
-	drmUnmap(pDRIPriv->pSAREA, pDRIPriv->pDriverInfo->SAREASize);
-	drmClose(pDRIPriv->drmFD);
-        DRIDrvMsg(pScreen->myNum, X_INFO,
-                  "[drm] drmAddMap failed\n");
-	return FALSE;
+	if (drmAddMap( pDRIPriv->drmFD,
+		       (drm_handle_t)pDRIPriv->pDriverInfo->frameBufferPhysicalAddress,
+		       pDRIPriv->pDriverInfo->frameBufferSize,
+		       DRM_FRAME_BUFFER,
+		       0,
+		       &pDRIPriv->pDriverInfo->hFrameBuffer) < 0)
+	    {
+		pDRIPriv->directRenderingSupport = FALSE;
+		pScreen->devPrivates[DRIScreenPrivIndex].ptr = NULL;
+		drmUnmap(pDRIPriv->pSAREA, pDRIPriv->pDriverInfo->SAREASize);
+		drmClose(pDRIPriv->drmFD);
+		DRIDrvMsg(pScreen->myNum, X_INFO,
+			  "[drm] drmAddMap failed\n");
+		return FALSE;
+	    }
+	DRIDrvMsg(pScreen->myNum, X_INFO, "[drm] framebuffer handle = %p\n",
+		  pDRIPriv->pDriverInfo->hFrameBuffer);
+    } else {
+	DRIDrvMsg(pScreen->myNum, X_INFO,
+		  "[drm] framebuffer mapped by ddx driver\n");
     }
-    DRIDrvMsg(pScreen->myNum, X_INFO, "[drm] framebuffer handle = %p\n",
-	      pDRIPriv->hFrameBuffer);
 
     if (pDRIEntPriv->resOwner == NULL) {
 	pDRIEntPriv->resOwner = pScreen;
@@ -1555,7 +1561,7 @@ DRIGetDeviceInfo(ScreenPtr pScreen,
 {
     DRIScreenPrivPtr pDRIPriv = DRI_SCREEN_PRIV(pScreen);
 
-    *hFrameBuffer = pDRIPriv->hFrameBuffer;
+    *hFrameBuffer = pDRIPriv->pDriverInfo->hFrameBuffer;
     *fbOrigin = 0;
     *fbSize = pDRIPriv->pDriverInfo->frameBufferSize;
     *fbStride = pDRIPriv->pDriverInfo->frameBufferStride;
diff --git a/hw/xfree86/dri/dri.h b/hw/xfree86/dri/dri.h
index e49bb6f..4a509ac 100644
--- a/hw/xfree86/dri/dri.h
+++ b/hw/xfree86/dri/dri.h
@@ -107,7 +107,7 @@ typedef struct {
  */
 
 #define DRIINFO_MAJOR_VERSION   5
-#define DRIINFO_MINOR_VERSION   3
+#define DRIINFO_MINOR_VERSION   4
 #define DRIINFO_PATCH_VERSION   0
 
 typedef unsigned long long (*DRITexOffsetStartProcPtr)(PixmapPtr pPix);
@@ -187,6 +187,11 @@ typedef struct {
     /* New with DRI version 5.3.0 */
     DRITexOffsetStartProcPtr  texOffsetStart;
     DRITexOffsetFinishProcPtr texOffsetFinish;
+
+    /* New with DRI version 5.4.0 */
+    drm_handle_t   	hFrameBuffer; /* Handle to framebuffer, either
+				       * mapped by DDX driver or DRI */
+    
 } DRIInfoRec, *DRIInfoPtr;
 
 
diff --git a/hw/xfree86/dri/dristruct.h b/hw/xfree86/dri/dristruct.h
index a3bac85..c3f1ad5 100644
--- a/hw/xfree86/dri/dristruct.h
+++ b/hw/xfree86/dri/dristruct.h
@@ -85,7 +85,6 @@ typedef struct _DRIScreenPrivRec
     int			drmFD;	      /* File descriptor for /dev/video/?   */
     drm_handle_t   	hSAREA;	      /* Handle to SAREA, for mapping       */
     XF86DRISAREAPtr	pSAREA;	      /* Mapped pointer to SAREA            */
-    drm_handle_t   	hFrameBuffer; /* Handle to framebuffer, for mapping */
     drm_context_t          myContext;    /* DDX Driver's context               */
     DRIContextPrivPtr   myContextPriv;/* Pointer to server's private area   */
     DRIContextPrivPtr   lastPartial3DContext;  /* last one partially saved  */
commit 43ee2840162fc99662fe342983422546c3a3662c
Author: Kristian Høgsberg <[EMAIL PROTECTED]>
Date:   Fri Jun 15 18:59:57 2007 -0400

    Don't map the frame buffer in the loader, just pass the drm_handle_t to driver.

diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
index e7fbf8e..f3c0a6e 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -361,12 +361,7 @@ struct __DRIinterfaceMethodsRec {
  * \bug This structure could be better named.
  */
 struct __DRIframebufferRec {
-    unsigned char *base;    /**< Framebuffer base address in the CPU's
-			     * address space.  This value is calculated by
-			     * calling \c drmMap on the framebuffer handle
-			     * returned by \c XF86DRIGetDeviceInfo (or a
-			     * similar function).
-			     */
+    drm_handle_t handle;    /**< Handle to the drm map for the frame buffer. */
     int size;               /**< Framebuffer size, in bytes. */
     int stride;             /**< Number of bytes from one line to the next. */
     int width;              /**< Pixel width of the framebuffer. */
diff --git a/src/glx/x11/glxext.c b/src/glx/x11/glxext.c
index 829a89c..a232122 100644
--- a/src/glx/x11/glxext.c
+++ b/src/glx/x11/glxext.c
@@ -887,7 +887,6 @@ CallCreateNewScreen(Display *dpy, int scrn, __GLXscreenConfigs *psc,
 						   &ddx_version.minor,
 						   &ddx_version.patch,
 						   &driverName)) {
-			drm_handle_t  hFB;
 			int        junk;
 
 			/* No longer needed. */
@@ -902,7 +901,7 @@ CallCreateNewScreen(Display *dpy, int scrn, __GLXscreenConfigs *psc,
 			 */
 			err_msg = "XF86DRIGetDeviceInfo";
 			if (XF86DRIGetDeviceInfo(dpy, scrn,
-						 &hFB,
+						 &framebuffer.handle,
 						 &junk,
 						 &framebuffer.size,
 						 &framebuffer.stride,
@@ -912,46 +911,34 @@ CallCreateNewScreen(Display *dpy, int scrn, __GLXscreenConfigs *psc,
 			    framebuffer.height = DisplayHeight(dpy, scrn);
 
 			    /*
-			     * Map the framebuffer region.
+			     * Map the SAREA region.  Further mmap regions
+			     * may be setup in each DRI driver's
+			     * "createNewScreen" function.
 			     */
-			    status = drmMap(fd, hFB, framebuffer.size, 
-					    (drmAddressPtr)&framebuffer.base);
+			    status = drmMap(fd, hSAREA, SAREA_MAX, &pSAREA);
 
-			    err_msg = "drmMap of framebuffer";
+			    err_msg = "drmMap of sarea";
 			    err_extra = strerror( -status );
 
 			    if ( status == 0 ) {
-				/*
-				 * Map the SAREA region.  Further mmap regions
-				 * may be setup in each DRI driver's
-				 * "createNewScreen" function.
-				 */
-				status = drmMap(fd, hSAREA, SAREA_MAX, 
-						&pSAREA);
-
-				err_msg = "drmMap of sarea";
-				err_extra = strerror( -status );
-
-				if ( status == 0 ) {
-				    __GLcontextModes * driver_modes = NULL;
-
-				    err_msg = "InitDriver";
-				    err_extra = NULL;
-				    psp = (*createNewScreen)(scrn,
-							     &psc->driScreen,
-							     & ddx_version,
-							     & dri_version,
-							     & drm_version,
-							     & framebuffer,
-							     pSAREA,
-							     fd,
-							     api_ver,
-							     & interface_methods,
-							     & driver_modes );
-
-				    filter_modes(&psc->configs, driver_modes);
-				    _gl_context_modes_destroy( driver_modes );
-				}
+				__GLcontextModes * driver_modes = NULL;
+
+				err_msg = "InitDriver";
+				err_extra = NULL;
+				psp = (*createNewScreen)(scrn,
+							 &psc->driScreen,
+							 & ddx_version,
+							 & dri_version,
+							 & drm_version,
+							 & framebuffer,
+							 pSAREA,
+							 fd,
+							 api_ver,
+							 & interface_methods,
+							 & driver_modes );
+
+				filter_modes(&psc->configs, driver_modes);
+				_gl_context_modes_destroy( driver_modes );
 			    }
 			}
 		    }
@@ -965,10 +952,6 @@ CallCreateNewScreen(Display *dpy, int scrn, __GLXscreenConfigs *psc,
 	    (void)drmUnmap(pSAREA, SAREA_MAX);
 	}
 
-	if ( framebuffer.base != MAP_FAILED ) {
-	    (void)drmUnmap((drmAddress)framebuffer.base, framebuffer.size);
-	}
-
 	if ( framebuffer.dev_priv != NULL ) {
 	    Xfree(framebuffer.dev_priv);
 	}
diff --git a/src/mesa/drivers/dri/common/dri_util.c b/src/mesa/drivers/dri/common/dri_util.c
index eaf63a1..a08edd5 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -726,7 +726,11 @@ void * __DRI_CREATE_NEW_SCREEN( int scrn, __DRIscreen *psc,
 
     psp->pSAREA = pSAREA;
 
-    psp->pFB = frame_buffer->base;
+    if (drmMap(fd, frame_buffer->handle, frame_buffer->size, &psp->pFB)) {
+	_mesa_free(psp);
+	return NULL;
+    }
+
     psp->fbSize = frame_buffer->size;
     psp->fbStride = frame_buffer->stride;
     psp->fbWidth = frame_buffer->width;
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
--
_______________________________________________
Dri-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to