[PATCH v6 4/4] drm/i915: Fix error handling in intelfb_create

[Lukas Wunner <lukas@wunner.de>]

intelfb_create() is called once on driver initialization. It either
uses the fb inherited from BIOS or allocates a new one by calling
intelfb_alloc(). Afterwards, it calls two functions which can fail:

- drm_fb_helper_alloc_fbi() can fail with -ENOMEM.
- ioremap_wc() can fail on alignment issues etc.

In the case where we allocated the fb with intelfb_alloc(), if either
of the two functions fail we currently unpin and unref the bo,
but leak the fb. We need to unref the fb instead of the bo here.

In the case where the fb was inherited from BIOS, the fb struct was
allocated by dev_priv->display.get_initial_plane_config() and is in
active use by a crtc, so it seems wrong to unpin and unref its bo.
We could treat failure of the above two functions as a fatal error
and call i915_driver_unload(), or try to limp on (fbcon won't work,
but X11 might). Let's do the latter.

However we should at least log an error message. Currently a failure
of the above two functions is not reported at all: The negative return
value is passed up the call stack to intel_fbdev_initial_config()
which ignores it.

To be fair, this is a corner case which few users probably ever
experience, nevertheless we should try to get it right.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/i915/intel_fbdev.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 12597b5..b8c11a1 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -227,6 +227,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 
 	info = drm_fb_helper_alloc_fbi(helper);
 	if (IS_ERR(info)) {
+		DRM_ERROR("Failed to allocate fb_info\n");
 		ret = PTR_ERR(info);
 		goto out_unpin;
 	}
@@ -253,6 +254,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 		ioremap_wc(dev_priv->gtt.mappable_base + i915_gem_obj_ggtt_offset(obj),
 			   size);
 	if (!info->screen_base) {
+		DRM_ERROR("Failed to remap framebuffer into virtual memory\n");
 		ret = -ENOSPC;
 		goto out_destroy_fbi;
 	}
@@ -284,9 +286,14 @@ static int intelfb_create(struct drm_fb_helper *helper,
 out_destroy_fbi:
 	drm_fb_helper_release_fbi(helper);
 out_unpin:
-	i915_gem_object_ggtt_unpin(obj);
-	drm_gem_object_unreference(&obj->base);
-	mutex_unlock(&dev->struct_mutex);
+	/* If fb was preallocated by BIOS, try to limp on. Else free it. */
+	if (prealloc)
+		mutex_unlock(&dev->struct_mutex);
+	else {
+		i915_gem_object_ggtt_unpin(obj);
+		mutex_unlock(&dev->struct_mutex);
+		drm_framebuffer_unreference(&intel_fb->base);
+	}
 	return ret;
 }
 
-- 
1.8.5.2 (Apple Git-48)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx