[v2,16/22] drm: Create new fb and replace default 1024x768 fb on hotplug event

Commit Message

Lukas Wunner May 25, 2015, 1:15 p.m. UTC

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 <[e-mail]>
    [MBP  6,2 2010  intel ILK + nvidia GT216  pre-retina]
Tested-by: William Brown <[e-mail]>
    [MBP  8,2 2011  intel SNB + amd turks     pre-retina]
Tested-by: Lukas Wunner <[e-mail]>
    [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina]
Tested-by: Bruno Bierbaumer <[e-mail]>
    [MBP 11,3 2013  intel HSW + nvidia GK107  retina -- work in progress]

Signed-off-by: Lukas Wunner <[e-mail]>
---
 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(-)

Patch

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;
 }