Commit 366e39b4 authored by Lukas Wunner's avatar Lukas Wunner Committed by Daniel Vetter

drm/i915: Tear down fbdev if initialization fails

Currently if intelfb_create() errors out, it unrefs the bo even though
the fb now owns that reference. (Spotted by Ville Syrjälä.) We should
unref the fb instead of the bo.

However the fb was not necessarily allocated by intelfb_create(),
it could be inherited from BIOS (the fb struct was then allocated by
dev_priv->display.get_initial_plane_config()) and be in active use by
a crtc. In this case we should call drm_framebuffer_remove() instead
of _unreference() to also disable the crtc.

Daniel Vetter suggested that "fbdev teardown code will take care of it.
The correct approach is probably to not unref anything at all".

But if fbdev initialization fails, the fbdev isn't torn down and
occupies memory even though it's unusable. Therefore clobber it in
intel_fbdev_initial_config(). (Currently we ignore a negative return
value there.) The idea is that if fbdev initialization fails, the driver
behaves as if CONFIG_DRM_FBDEV_EMULATION wasn't set. Should X11 manage
to start up without errors, it will at least be able to use the memory
that would otherwise be hogged by the unusable fbdev.

Also, log errors in intelfb_create().

Don't call async_synchronize_full() in intel_fbdev_fini() when called
from intel_fbdev_initial_config() to avoid deadlock.

v2: Instead of calling drm_framebuffer_unreference() (if fb was not
    inherited from BIOS), call intel_fbdev_fini().

v3: Rebase on e00bf696 (drm/i915: Move the fbdev async_schedule()
    into intel_fbdev.c), call async_synchronize_full() conditionally
    instead of moving it into i915_driver_unload().

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: default avatarLukas Wunner <lukas@wunner.de>
Link: http://patchwork.freedesktop.org/patch/msgid/49ce5f0daead24b7598ec78591731046c333c18d.1447938059.git.lukas@wunner.deSigned-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 581da2ca
...@@ -227,6 +227,7 @@ static int intelfb_create(struct drm_fb_helper *helper, ...@@ -227,6 +227,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
info = drm_fb_helper_alloc_fbi(helper); info = drm_fb_helper_alloc_fbi(helper);
if (IS_ERR(info)) { if (IS_ERR(info)) {
DRM_ERROR("Failed to allocate fb_info\n");
ret = PTR_ERR(info); ret = PTR_ERR(info);
goto out_unpin; goto out_unpin;
} }
...@@ -253,6 +254,7 @@ static int intelfb_create(struct drm_fb_helper *helper, ...@@ -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), ioremap_wc(dev_priv->gtt.mappable_base + i915_gem_obj_ggtt_offset(obj),
size); size);
if (!info->screen_base) { if (!info->screen_base) {
DRM_ERROR("Failed to remap framebuffer into virtual memory\n");
ret = -ENOSPC; ret = -ENOSPC;
goto out_destroy_fbi; goto out_destroy_fbi;
} }
...@@ -285,7 +287,6 @@ static int intelfb_create(struct drm_fb_helper *helper, ...@@ -285,7 +287,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
drm_fb_helper_release_fbi(helper); drm_fb_helper_release_fbi(helper);
out_unpin: out_unpin:
i915_gem_object_ggtt_unpin(obj); i915_gem_object_ggtt_unpin(obj);
drm_gem_object_unreference(&obj->base);
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
return ret; return ret;
} }
...@@ -713,7 +714,9 @@ static void intel_fbdev_initial_config(void *data, async_cookie_t cookie) ...@@ -713,7 +714,9 @@ static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
struct intel_fbdev *ifbdev = dev_priv->fbdev; struct intel_fbdev *ifbdev = dev_priv->fbdev;
/* Due to peculiar init order wrt to hpd handling this is separate. */ /* Due to peculiar init order wrt to hpd handling this is separate. */
drm_fb_helper_initial_config(&ifbdev->helper, ifbdev->preferred_bpp); if (drm_fb_helper_initial_config(&ifbdev->helper,
ifbdev->preferred_bpp))
intel_fbdev_fini(dev_priv->dev);
} }
void intel_fbdev_initial_config_async(struct drm_device *dev) void intel_fbdev_initial_config_async(struct drm_device *dev)
...@@ -729,6 +732,7 @@ void intel_fbdev_fini(struct drm_device *dev) ...@@ -729,6 +732,7 @@ void intel_fbdev_fini(struct drm_device *dev)
flush_work(&dev_priv->fbdev_suspend_work); flush_work(&dev_priv->fbdev_suspend_work);
if (!current_is_async())
async_synchronize_full(); async_synchronize_full();
intel_fbdev_destroy(dev, dev_priv->fbdev); intel_fbdev_destroy(dev, dev_priv->fbdev);
kfree(dev_priv->fbdev); kfree(dev_priv->fbdev);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment