Improved error handling in ARMSOCScreenInit

Ensures that resources are released when ARMSOCScreenInit() fails

Change-Id: I7525265b260fd063d021ebed3485eeab0943461b

3 files changed, 98 insertions, 39 deletions

diff --git a/src/armsoc_driver.c b/src/armsoc_driver.c
index 9cdd44f..e932073 100644
--- a/src/armsoc_driver.c
+++ b/src/armsoc_driver.c
@@ -583,8 +583,9 @@ ARMSOCAccelInit(ScreenPtr pScreen)
 }
 
 /**
- * The driver's ScreenInit() function.  Fill in pScreen, map the frame buffer,
- * save state, initialize the mode, etc.
+ * The driver's ScreenInit() function, called at the start of each server
+ * generation. Fill in pScreen, map the frame buffer, save state, 
+ * initialize the mode, etc.
  */
 static Bool
 ARMSOCScreenInit(SCREEN_INIT_ARGS_DECL)
@@ -600,22 +601,20 @@ ARMSOCScreenInit(SCREEN_INIT_ARGS_DECL)
 	/* set drm master before allocating scanout buffer */
 	if(ARMSOCSetDRMMaster()) {
 		ERROR_MSG("Cannot get DRM master: %s", strerror(errno));
-		return FALSE;
+		goto fail;
 	}
 	/* Allocate initial scanout buffer */
 	DEBUG_MSG("allocating new scanout buffer: %dx%d",
 			pScrn->virtualX, pScrn->virtualY);
-
 	assert(!pARMSOC->scanout);
 	pARMSOC->scanout = armsoc_bo_new_with_dim(pARMSOC->dev, pScrn->virtualX,
 			pScrn->virtualY, pScrn->depth, pScrn->bitsPerPixel,
 			ARMSOC_BO_SCANOUT );
 	if (!pARMSOC->scanout) {
-		ERROR_MSG("Error allocating scanout buffer\n");
-		return FALSE;
+		ERROR_MSG("Cannot allocate scanout buffer\n");
+		goto fail1;
 	}
 	pScrn->displayWidth = armsoc_bo_pitch(pARMSOC->scanout) / (pScrn->bitsPerPixel / 8);
-
 	xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
 
 	/* need to point to new screen on server regeneration */
@@ -636,16 +635,13 @@ ARMSOCScreenInit(SCREEN_INIT_ARGS_DECL)
 	 * are not appropriate.  In this driver, we fixup the visuals after.
 	 */
 
-	/*
-	 * Reset the visual list.
-	 */
-
+	/* Reset the visual list. */
 	miClearVisualTypes();
 	if (!miSetVisualTypes(pScrn->bitsPerPixel, miGetDefaultVisualMask(pScrn->depth),
 			pScrn->rgbBits, pScrn->defaultVisual)) {
 		ERROR_MSG("Cannot initialize the visual type for %d bits per pixel!",
 				pScrn->bitsPerPixel);
-		goto fail;
+		goto fail2;
 	}
 
 	if(pScrn->bitsPerPixel == 32 && pScrn->depth == 24) {
@@ -660,7 +656,7 @@ ARMSOCScreenInit(SCREEN_INIT_ARGS_DECL)
 
 	if (!miSetPixmapDepths()) {
 		ERROR_MSG("Cannot initialize the pixmap depth!");
-		goto fail;
+		goto fail3;
 	}
 
 	/* Initialize some generic 2D drawing functions: */
@@ -669,7 +665,7 @@ ARMSOCScreenInit(SCREEN_INIT_ARGS_DECL)
 			pScrn->xDpi, pScrn->yDpi, pScrn->displayWidth,
 			pScrn->bitsPerPixel)) {
 		ERROR_MSG("fbScreenInit() failed!");
-		goto fail;
+		goto fail3;
 	}
 
 	/* Fixup RGB ordering: */
@@ -692,7 +688,7 @@ ARMSOCScreenInit(SCREEN_INIT_ARGS_DECL)
 	 */
 	if (!fbPictureInit(pScreen, NULL, 0)) {
 		ERROR_MSG("fbPictureInit() failed!");
-		goto fail;
+		goto fail4;
 	}
 
 	/* Set the initial black & white colormap indices: */
@@ -711,13 +707,16 @@ ARMSOCScreenInit(SCREEN_INIT_ARGS_DECL)
 	xf86SetSilkenMouse(pScreen);
 
 	/* Initialize the cursor: */
-	miDCInitialize(pScreen, xf86GetPointerScreenFuncs());
-
-	if (!drmmode_cursor_init(pScreen)) {
-		ERROR_MSG("Hardware cursor initialization failed");
+	if(!miDCInitialize(pScreen, xf86GetPointerScreenFuncs())) {
+		ERROR_MSG("miDCInitialize() failed!");
+		goto fail5;
 	}
 
-	/* XXX -- Is this the right place for this?  The Intel i830 driver says:
+	/* ignore failures here as we will fall back to software cursor */
+	(void)drmmode_cursor_init(pScreen);
+
+	/* TODO: MIDEGL-1458: Is this the right place for this?
+	 * The Intel i830 driver says:
 	 * "Must force it before EnterVT, so we are in control of VT..."
 	 */
 	pScrn->vtSema = TRUE;
@@ -727,24 +726,21 @@ ARMSOCScreenInit(SCREEN_INIT_ARGS_DECL)
 	 */
 	if (!ARMSOCEnterVT(VT_FUNC_ARGS(0))) {
 		ERROR_MSG("ARMSOCEnterVT() failed!");
-		goto fail;
+		goto fail6;
 	}
 
-	/* Do some XRandR initialization: */
-	if (!xf86CrtcScreenInit(pScreen)) {
-		ERROR_MSG("xf86CrtcScreenInit() failed!");
-		goto fail;
-	}
+	/* Do some XRandR initialization. Return value is not useful */
+	(void)xf86CrtcScreenInit(pScreen);
 
 	if (!miCreateDefColormap(pScreen)) {
 		ERROR_MSG("Cannot create colormap!");
-		goto fail;
+		goto fail7;
 	}
 
 	if (!xf86HandleColormaps(pScreen, 1 << pScrn->rgbBits, pScrn->rgbBits,
 			ARMSOCLoadPalette, NULL, CMAP_PALETTED_TRUECOLOR)) {
 		ERROR_MSG("xf86HandleColormaps() failed!");
-		goto fail;
+		goto fail8;
 	}
 
 	/* Setup power management: */
@@ -761,6 +757,50 @@ ARMSOCScreenInit(SCREEN_INIT_ARGS_DECL)
 	TRACE_EXIT();
 	return TRUE;
 
+/* cleanup on failures */
+fail8:
+	/* uninstall the default colormap */
+	miUninstallColormap(GetInstalledmiColormap(pScreen));
+
+fail7:
+	ARMSOCLeaveVT(VT_FUNC_ARGS(0));
+	pScrn->vtSema = FALSE;
+
+fail6:
+	drmmode_cursor_fini(pScreen);
+
+fail5:
+	if (pARMSOC->dri) {
+		ARMSOCDRI2CloseScreen(pScreen);
+	}
+	if (pARMSOC->pARMSOCEXA) {
+		if (pARMSOC->pARMSOCEXA->CloseScreen) {
+			pARMSOC->pARMSOCEXA->CloseScreen(pScrn->scrnIndex, pScreen);
+		}
+	}
+fail4:
+	/* Call the CloseScreen functions for fbInitScreen,  miDCInitialize,
+	 * exaDriverInit & xf86CrtcScreenInit as appropriate via their wrapped pointers.
+	 * exaDDXCloseScreen uses the XF86SCRNINFO macro so we must
+	 * set up the key for this before it gets called.
+	 */
+	dixSetPrivate(&pScreen->devPrivates, xf86ScreenKey, pScrn);
+	(*pScreen->CloseScreen)(pScrn->scrnIndex, pScreen);
+
+fail3:
+	/* reset the visual list */
+	miClearVisualTypes();
+
+fail2:
+	/* release the scanout buffer */
+	armsoc_bo_unreference(pARMSOC->scanout);
+	pARMSOC->scanout = NULL;
+	pScrn->displayWidth = 0;
+
+fail1:
+	/* drop drm master */
+	(void)drmDropMaster(pARMSOC->drmFD);
+
 fail:
 	TRACE_EXIT();
 	return FALSE;
@@ -792,17 +832,15 @@ ARMSOCCloseScreen(CLOSE_SCREEN_ARGS_DECL)
 
 	drmmode_screen_fini(pScrn);
 
-
+	if (pARMSOC->dri) {
+		ARMSOCDRI2CloseScreen(pScreen);
+	}
 	if (pARMSOC->pARMSOCEXA) {
 		if (pARMSOC->pARMSOCEXA->CloseScreen) {
 			pARMSOC->pARMSOCEXA->CloseScreen(CLOSE_SCREEN_ARGS);
 		}
 	}
 
-	if (pARMSOC->dri) {
-		ARMSOCDRI2CloseScreen(pScreen);
-	}
-
 	/* release the scanout buffer */
 	armsoc_bo_unreference(pARMSOC->scanout);
 	pARMSOC->scanout = NULL;
diff --git a/src/armsoc_driver.h b/src/armsoc_driver.h
index 39fb6be..00e68dd 100644
--- a/src/armsoc_driver.h
+++ b/src/armsoc_driver.h
@@ -201,6 +201,8 @@ void drmmode_adjust_frame(ScrnInfoPtr pScrn, int x, int y);
 Bool drmmode_page_flip(DrawablePtr draw, uint32_t fb_id, void *priv);
 void drmmode_wait_for_event(ScrnInfoPtr pScrn);
 Bool drmmode_cursor_init(ScreenPtr pScreen);
+void drmmode_cursor_fini(ScreenPtr pScreen);
+
 
 /* 
  * pl111 cursor helper functions..
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 97a1fdc..07403a1 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -475,7 +475,7 @@ drmmode_cursor_init(ScreenPtr pScreen)
 	}
 
 	if(!xf86LoaderCheckSymbol("drmModeGetPlaneResources")) {
-		ERROR_MSG("drmModeGetPlaneResources() not supported (needs libdrm 2.4.30 or higher)");
+		ERROR_MSG("HW cursor not supported (needs libdrm 2.4.30 or higher)");
 		return FALSE;
 	}
 
@@ -485,7 +485,7 @@ drmmode_cursor_init(ScreenPtr pScreen)
 	 */
 	plane_resources = drmModeGetPlaneResources(drmmode->fd);
 	if (!plane_resources) {
-		ERROR_MSG("drmModeGetPlaneResources failed: %s", strerror(errno));
+		ERROR_MSG("HW cursor: drmModeGetPlaneResources failed: %s", strerror(errno));
 		return FALSE;
 	}
 
@@ -497,7 +497,7 @@ drmmode_cursor_init(ScreenPtr pScreen)
 
 	ovr = drmModeGetPlane(drmmode->fd, plane_resources->planes[0]);
 	if (!ovr) {
-		ERROR_MSG("drmModeGetPlane failed: %s", strerror(errno));
+		ERROR_MSG("HW cursor: drmModeGetPlane failed: %s", strerror(errno));
 		drmModeFreePlaneResources(plane_resources);
 		return FALSE;
 	}
@@ -511,7 +511,7 @@ drmmode_cursor_init(ScreenPtr pScreen)
 
 	cursor = calloc(1, sizeof(drmmode_cursor_rec));
 	if (!cursor) {
-		ERROR_MSG("calloc failed");
+		ERROR_MSG("HW cursor: calloc failed");
 		drmModeFreePlane(ovr);
 		drmModeFreePlaneResources(plane_resources);
 		return FALSE;
@@ -521,7 +521,7 @@ drmmode_cursor_init(ScreenPtr pScreen)
 	cursor->bo  = armsoc_bo_new_with_dim(pARMSOC->dev, w, h, 0, 32, ARMSOC_BO_SCANOUT );
 
 	if (!cursor->bo) {
-		ERROR_MSG("error allocating hw cursor buffer");
+		ERROR_MSG("HW cursor: buffer allocation failed");
 		free(cursor);
 		drmModeFreePlane(ovr);
 		drmModeFreePlaneResources(plane_resources);
@@ -534,7 +534,7 @@ drmmode_cursor_init(ScreenPtr pScreen)
 
 	if (drmModeAddFB2(drmmode->fd, w, h, DRM_FORMAT_ARGB8888,
 			handles, pitches, offsets, &cursor->fb_id, 0)) {
-		ERROR_MSG("drmModeAddFB2 failed: %s", strerror(errno));
+		ERROR_MSG("HW cursor: drmModeAddFB2 failed: %s", strerror(errno));
 		armsoc_bo_unreference(cursor->bo);
 		free(cursor);
 		drmModeFreePlane(ovr);
@@ -556,9 +556,28 @@ drmmode_cursor_init(ScreenPtr pScreen)
 
 	INFO_MSG("HW cursor initialized");
 	drmmode->cursor = cursor;
+	drmModeFreePlaneResources(plane_resources);
 	return TRUE;
 }
 
+void drmmode_cursor_fini(ScreenPtr pScreen)
+{
+	ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
+	drmmode_ptr drmmode = drmmode_from_scrn(pScrn);
+	drmmode_cursor_ptr cursor = drmmode->cursor;
+
+	if(!cursor ) {
+		return;
+	}
+	drmmode->cursor = NULL;
+	xf86_cursors_fini(pScreen);
+	drmModeRmFB(drmmode->fd, cursor->fb_id);
+	armsoc_bo_unreference(cursor->bo);
+	drmModeFreePlane(cursor->ovr);
+	free(cursor);
+}
+
+
 #if 1==ARMSOC_SUPPORT_GAMMA
 static void
 drmmode_gamma_set(xf86CrtcPtr crtc, CARD16 *red, CARD16 *green, CARD16 *blue,