If no connectors with modes are found, drm_fb_helper_single_fb_probe() will allocate a default 1024x768 fb. This becomes a problem if one of the connectors subsequently changes to connected status: We're stuck with the default fb even if the display allows a larger resolution (or doesn't support 1024x768 at all) since modes larger than the existing fb will be deemed ineligible by drm_fb_helper_hotplug_event().
One relevant use case are dual gpu laptops such as the MacBook Pro which require either a handler to switch DDC lines, or the active gpu to proxy the DDC/AUX communication: Both the handler and the active gpu may register late, and consequently, the inactive gpu's eDP and LVDS connectors may seem disconnected at startup but may later on turn out to be connected.
Although primarily aimed at vga_switcheroo setups, the patch may come in handy in other use cases: One example is a desktop machine that is (inadvertantly or otherwise) booted headless, and the user later on plugs in a display. Another example is unplugging of the display by the user and replacement with a larger one.
Solve this by checking if there aren't any modes at all. If so, don't constrain probed modes to the existing fb size but rather allow them to occupy the maximum surface size. After probing, check if we found any modes. If we did, discard the existing fb and recreate it from scratch by invoking the driver's ->fb_probe callback by way of drm_fb_helper_single_fb_probe().
That function is normally called only once on driver initialization. The fb_helper is kept so ensure that it's not added once more to the panic handler list.
Because multiple hotplug events may fire simultaneously, protect connector probing and fb recreation with drm_modeset_lock_all(). In particular, drm_fb_helper_hotplug_event() may itself fire another hotplug event by way of drm_fb_helper_probe_connector_modes(), which invokes the ->fill_modes callback, which for most drivers is drm_helper_probe_single_connector_modes(), which schedules output_poll_execute().
Amend the ->fb_probe callback in the i915, nouveau and radeon drivers to discard an existing fb. Because the _destroy() function is now used further up in intel_fbdev.c, nouveau_fbcon.c and radeon_fb.c, add a prototype for it. This is intended to ease reviewing, the prototype shall be replaced by the body of the function before this gets merged.
Drivers other than these three will just create an additional fb under the limited circumstances described above, which wastes memory but is otherwise "mostly harmless".
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]
Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/gpu/drm/drm_fb_helper.c | 41 ++++++++++++++++++++++++++------- drivers/gpu/drm/i915/intel_fbdev.c | 26 ++++++++++----------- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 10 +++++++- drivers/gpu/drm/radeon/radeon_fb.c | 11 ++++++--- 4 files changed, 63 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 73f90f7..6df0ee8 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1097,7 +1097,8 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op); }
- list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list); + if (list_empty(&fb_helper->kernel_fb_list)) + list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
return 0; } @@ -1770,23 +1771,47 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) { struct drm_device *dev = fb_helper->dev; u32 max_width, max_height; + bool modes_before_probe = false; + bool modes_after_probe = false; + int i;
- mutex_lock(&fb_helper->dev->mode_config.mutex); + drm_modeset_lock_all(dev); if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) { fb_helper->delayed_hotplug = true; - mutex_unlock(&fb_helper->dev->mode_config.mutex); + drm_modeset_unlock_all(dev); return 0; } DRM_DEBUG_KMS("\n");
- max_width = fb_helper->fb->width; - max_height = fb_helper->fb->height; + /* If so far we have no modes and thus only the default 1024x768 fb, + * allow probed modes to occupy the maximum surface size */ + for (i = 0; i < fb_helper->crtc_count; i++) + if (fb_helper->crtc_info[i].desired_mode) { + modes_before_probe = true; + break; + } + if (!modes_before_probe) { + max_width = dev->mode_config.max_width; + max_height = dev->mode_config.max_width; + } else { + max_width = fb_helper->fb->width; + max_height = fb_helper->fb->height; + }
drm_fb_helper_probe_connector_modes(fb_helper, max_width, max_height); - mutex_unlock(&fb_helper->dev->mode_config.mutex); - - drm_modeset_lock_all(dev); drm_setup_crtcs(fb_helper); + + /* If previously there were no connectors with modes and now we + * found some, create new fb and replace default 1024x768 fb */ + for (i = 0; i < fb_helper->crtc_count; i++) + if (fb_helper->crtc_info[i].desired_mode) { + modes_after_probe = true; + break; + } + if (!modes_before_probe && modes_after_probe) + drm_fb_helper_single_fb_probe(fb_helper, + fb_helper->fb->bits_per_pixel); + drm_modeset_unlock_all(dev); drm_fb_helper_set_par(fb_helper->fbdev);
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index dd138aa..637146f 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -199,6 +199,8 @@ out: return ret; }
+static void intel_fbdev_destroy(struct fb_info *info); + static int intelfb_create(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes) { @@ -220,6 +222,7 @@ static int intelfb_create(struct drm_fb_helper *helper, " releasing it\n", intel_fb->base.width, intel_fb->base.height, sizes->fb_width, sizes->fb_height); + intel_fbdev_destroy(ifbdev->helper.fbdev); drm_framebuffer_unreference(&intel_fb->base); intel_fb = ifbdev->fb = NULL; } @@ -545,12 +548,9 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { .fb_probe = intelfb_create, };
-static void intel_fbdev_destroy(struct drm_device *dev, - struct intel_fbdev *ifbdev) +static void intel_fbdev_destroy(struct fb_info *info) { - if (ifbdev->helper.fbdev) { - struct fb_info *info = ifbdev->helper.fbdev; - + if (info) { unregister_framebuffer(info); iounmap(info->screen_base); if (info->cmap.len) @@ -558,11 +558,6 @@ static void intel_fbdev_destroy(struct drm_device *dev,
framebuffer_release(info); } - - drm_fb_helper_fini(&ifbdev->helper); - - drm_framebuffer_unregister_private(&ifbdev->fb->base); - drm_framebuffer_remove(&ifbdev->fb->base); }
/* @@ -750,13 +745,18 @@ void intel_fbdev_initial_config(void *data, async_cookie_t cookie) void intel_fbdev_fini(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - if (!dev_priv->fbdev) + struct intel_fbdev *ifbdev = dev_priv->fbdev; + + if (!ifbdev) return;
flush_work(&dev_priv->fbdev_suspend_work); - async_synchronize_full(); - intel_fbdev_destroy(dev, dev_priv->fbdev); + + intel_fbdev_destroy(ifbdev->helper.fbdev); + drm_fb_helper_fini(&ifbdev->helper); + drm_framebuffer_unregister_private(&ifbdev->fb->base); + drm_framebuffer_remove(&ifbdev->fb->base); kfree(dev_priv->fbdev); dev_priv->fbdev = NULL; } diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index 6751553..7b7b112 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -305,6 +305,9 @@ nouveau_fbcon_zfill(struct drm_device *dev, struct nouveau_fbdev *fbcon) }
static int +nouveau_fbcon_destroy(struct drm_device *dev, struct nouveau_fbdev *fbcon); + +static int nouveau_fbcon_create(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes) { @@ -322,6 +325,11 @@ nouveau_fbcon_create(struct drm_fb_helper *helper, struct pci_dev *pdev = dev->pdev; int size, ret;
+ if (helper->fbdev) { + nouveau_fbcon_accel_fini(dev); + nouveau_fbcon_destroy(dev, fbcon); + } + mode_cmd.width = sizes->surface_width; mode_cmd.height = sizes->surface_height;
@@ -467,7 +475,6 @@ nouveau_fbcon_destroy(struct drm_device *dev, struct nouveau_fbdev *fbcon) drm_gem_object_unreference_unlocked(&nouveau_fb->nvbo->gem); nouveau_fb->nvbo = NULL; } - drm_fb_helper_fini(&fbcon->helper); drm_framebuffer_unregister_private(&nouveau_fb->base); drm_framebuffer_cleanup(&nouveau_fb->base); return 0; @@ -567,6 +574,7 @@ nouveau_fbcon_fini(struct drm_device *dev)
nouveau_fbcon_accel_fini(dev); nouveau_fbcon_destroy(dev, drm->fbcon); + drm_fb_helper_fini(&drm->fbcon->helper); kfree(drm->fbcon); drm->fbcon = NULL; } diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index aeb6767..88b9f32 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -216,6 +216,8 @@ out_unref: return ret; }
+static int radeon_fbdev_destroy(struct radeon_fbdev *rfbdev); + static int radeonfb_create(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes) { @@ -231,6 +233,9 @@ static int radeonfb_create(struct drm_fb_helper *helper, int ret; unsigned long tmp;
+ if (helper->fbdev) + radeon_fbdev_destroy(rfbdev); + mode_cmd.width = sizes->surface_width; mode_cmd.height = sizes->surface_height;
@@ -337,7 +342,7 @@ void radeon_fb_output_poll_changed(struct radeon_device *rdev) drm_fb_helper_hotplug_event(&rdev->mode_info.rfbdev->helper); }
-static int radeon_fbdev_destroy(struct drm_device *dev, struct radeon_fbdev *rfbdev) +static int radeon_fbdev_destroy(struct radeon_fbdev *rfbdev) { struct fb_info *info; struct radeon_framebuffer *rfb = &rfbdev->rfb; @@ -355,7 +360,6 @@ static int radeon_fbdev_destroy(struct drm_device *dev, struct radeon_fbdev *rfb radeonfb_destroy_pinned_object(rfb->obj); rfb->obj = NULL; } - drm_fb_helper_fini(&rfbdev->helper); drm_framebuffer_unregister_private(&rfb->base); drm_framebuffer_cleanup(&rfb->base);
@@ -419,7 +423,8 @@ void radeon_fbdev_fini(struct radeon_device *rdev) if (!rdev->mode_info.rfbdev) return;
- radeon_fbdev_destroy(rdev->ddev, rdev->mode_info.rfbdev); + radeon_fbdev_destroy(rdev->mode_info.rfbdev); + drm_fb_helper_fini(&rdev->mode_info.rfbdev->helper); kfree(rdev->mode_info.rfbdev); rdev->mode_info.rfbdev = NULL; }