All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/12] fbdev helper locking rework and deferred setup
@ 2017-06-21 18:28 Daniel Vetter
  2017-06-21 18:28 ` [PATCH 01/12] drm/fb-helper: Push down modeset lock into FB helpers Daniel Vetter
                   ` (15 more replies)
  0 siblings, 16 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding

Hi all,

This is Thierry's deferred fbdev setup series, with the locking rework almost
entirely redone. The much wider scope is to get rid of drm_modeset_lock_all
calls for atomic drivers and remove users of the fairly nasty
mode_config->acquire_ctx hack, which breaks when doing multiple atomic commits.

Testing&review very much appreciated, especially from people who care about the
various fbdev emulation things and the deferred setup stuff.

Thanks, Daniel

Daniel Vetter (7):
  drm/i915: Drop FBDEV #ifdev in mst code
  drm/fb-helper: Push locking in fb_is_bound
  drm/fb-helper: Drop locking from the vsync wait ioctl code
  drm/fb-helper: Push locking into pan_display_atomic|legacy
  drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy
  drm/fb-helper: Stop using mode_config.mutex for internals
  drm/fb-helper: Split dpms handling into legacy and atomic paths

Thierry Reding (5):
  drm/fb-helper: Push down modeset lock into FB helpers
  drm/fb-helper: Add top-level lock
  drm/fb-helper: Support deferred setup
  drm/exynos: Remove custom FB helper deferred setup
  drm/hisilicon: Remove custom FB helper deferred setup

 drivers/gpu/drm/drm_fb_helper.c                 | 361 ++++++++++++++++++------
 drivers/gpu/drm/drm_vblank.c                    |   2 +-
 drivers/gpu/drm/exynos/exynos_drm_drv.c         |   6 +-
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c       |  26 +-
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  21 +-
 drivers/gpu/drm/i915/intel_dp_mst.c             |  43 +--
 drivers/gpu/drm/radeon/radeon_dp_mst.c          |   7 -
 include/drm/drm_fb_helper.h                     |  42 ++-
 8 files changed, 336 insertions(+), 172 deletions(-)

-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 01/12] drm/fb-helper: Push down modeset lock into FB helpers
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
@ 2017-06-21 18:28 ` Daniel Vetter
  2017-06-21 18:28 ` [PATCH 02/12] drm/i915: Drop FBDEV #ifdev in mst code Daniel Vetter
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding

From: Thierry Reding <treding@nvidia.com>

Move the modeset locking from drivers into FB helpers.

v2: Also handle intel_connector_add_to_fbdev.

Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Thierry Reding <treding@nvidia.com> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fb_helper.c        | 40 +++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_dp_mst.c    |  6 -----
 drivers/gpu/drm/radeon/radeon_dp_mst.c |  7 ------
 3 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 574af01d3ce9..5e73cc9f51e3 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -109,8 +109,8 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
 	for (({ lockdep_assert_held(&(fbh)->dev->mode_config.mutex); }), \
 	     i__ = 0; i__ < (fbh)->connector_count; i__++)
 
-int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
-				    struct drm_connector *connector)
+static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
+					     struct drm_connector *connector)
 {
 	struct drm_fb_helper_connector *fb_conn;
 	struct drm_fb_helper_connector **temp;
@@ -141,8 +141,23 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
 	drm_connector_get(connector);
 	fb_conn->connector = connector;
 	fb_helper->connector_info[fb_helper->connector_count++] = fb_conn;
+
 	return 0;
 }
+
+int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
+				    struct drm_connector *connector)
+{
+	int err;
+
+	mutex_lock(&fb_helper->dev->mode_config.mutex);
+
+	err = __drm_fb_helper_add_one_connector(fb_helper, connector);
+
+	mutex_unlock(&fb_helper->dev->mode_config.mutex);
+
+	return err;
+}
 EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
 
 /**
@@ -172,8 +187,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
 	mutex_lock(&dev->mode_config.mutex);
 	drm_connector_list_iter_begin(dev, &conn_iter);
 	drm_for_each_connector_iter(connector, &conn_iter) {
-		ret = drm_fb_helper_add_one_connector(fb_helper, connector);
-
+		ret = __drm_fb_helper_add_one_connector(fb_helper, connector);
 		if (ret)
 			goto fail;
 	}
@@ -198,8 +212,8 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
 }
 EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors);
 
-int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
-				       struct drm_connector *connector)
+static int __drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
+						struct drm_connector *connector)
 {
 	struct drm_fb_helper_connector *fb_helper_connector;
 	int i, j;
@@ -227,6 +241,20 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 
 	return 0;
 }
+
+int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
+				       struct drm_connector *connector)
+{
+	int err;
+
+	mutex_lock(&fb_helper->dev->mode_config.mutex);
+
+	err = __drm_fb_helper_remove_one_connector(fb_helper, connector);
+
+	mutex_unlock(&fb_helper->dev->mode_config.mutex);
+
+	return err;
+}
 EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
 
 static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct drm_fb_helper *helper)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 2cf046beae0f..5b78165882ce 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -501,11 +501,8 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 static void intel_dp_register_mst_connector(struct drm_connector *connector)
 {
 	struct intel_connector *intel_connector = to_intel_connector(connector);
-	struct drm_device *dev = connector->dev;
 
-	drm_modeset_lock_all(dev);
 	intel_connector_add_to_fbdev(intel_connector);
-	drm_modeset_unlock_all(dev);
 
 	drm_connector_register(&intel_connector->base);
 }
@@ -514,15 +511,12 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 					   struct drm_connector *connector)
 {
 	struct intel_connector *intel_connector = to_intel_connector(connector);
-	struct drm_device *dev = connector->dev;
 
 	drm_connector_unregister(connector);
 
 	/* need to nuke the connector */
-	drm_modeset_lock_all(dev);
 	intel_connector_remove_from_fbdev(intel_connector);
 	intel_connector->mst_port = NULL;
-	drm_modeset_unlock_all(dev);
 
 	drm_connector_unreference(&intel_connector->base);
 	DRM_DEBUG_KMS("\n");
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index 6598306dca9b..ebdf1b859cb6 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -300,9 +300,7 @@ static void radeon_dp_register_mst_connector(struct drm_connector *connector)
 	struct drm_device *dev = connector->dev;
 	struct radeon_device *rdev = dev->dev_private;
 
-	drm_modeset_lock_all(dev);
 	radeon_fb_add_connector(rdev, connector);
-	drm_modeset_unlock_all(dev);
 
 	drm_connector_register(connector);
 }
@@ -315,13 +313,8 @@ static void radeon_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 	struct radeon_device *rdev = dev->dev_private;
 
 	drm_connector_unregister(connector);
-	/* need to nuke the connector */
-	drm_modeset_lock_all(dev);
-	/* dpms off */
 	radeon_fb_remove_connector(rdev, connector);
-
 	drm_connector_cleanup(connector);
-	drm_modeset_unlock_all(dev);
 
 	kfree(connector);
 	DRM_DEBUG_KMS("\n");
-- 
2.11.0

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 02/12] drm/i915: Drop FBDEV #ifdev in mst code
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
  2017-06-21 18:28 ` [PATCH 01/12] drm/fb-helper: Push down modeset lock into FB helpers Daniel Vetter
@ 2017-06-21 18:28 ` Daniel Vetter
  2017-06-21 18:28 ` [PATCH 03/12] drm/fb-helper: Add top-level lock Daniel Vetter
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Daniel Vetter, Intel Graphics Development, Thierry Reding

Since

commit a03fdcb1863297481a4b817c2a759cafcbdfa0ae
Author: Archit Taneja <architt@codeaurora.org>
Date:   Wed Aug 5 12:28:57 2015 +0530

    drm: Add top level Kconfig option for DRM fbdev emulation

this is properly handled using dummy functions. This essentially
undoes

commit 7296c849bf2eca2bd7d34a4686a53e3089150ac1
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Jul 22 20:10:28 2014 +1000

    drm/i915: fix build without fbde

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 37 ++++++++++---------------------------
 1 file changed, 10 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 5b78165882ce..c43ed29995b3 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -443,28 +443,6 @@ static bool intel_dp_mst_get_hw_state(struct intel_connector *connector)
 	return false;
 }
 
-static void intel_connector_add_to_fbdev(struct intel_connector *connector)
-{
-#ifdef CONFIG_DRM_FBDEV_EMULATION
-	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
-
-	if (dev_priv->fbdev)
-		drm_fb_helper_add_one_connector(&dev_priv->fbdev->helper,
-						&connector->base);
-#endif
-}
-
-static void intel_connector_remove_from_fbdev(struct intel_connector *connector)
-{
-#ifdef CONFIG_DRM_FBDEV_EMULATION
-	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
-
-	if (dev_priv->fbdev)
-		drm_fb_helper_remove_one_connector(&dev_priv->fbdev->helper,
-						   &connector->base);
-#endif
-}
-
 static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *pathprop)
 {
 	struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
@@ -500,25 +478,30 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 
 static void intel_dp_register_mst_connector(struct drm_connector *connector)
 {
-	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 
-	intel_connector_add_to_fbdev(intel_connector);
+	if (dev_priv->fbdev)
+		drm_fb_helper_add_one_connector(&dev_priv->fbdev->helper,
+						connector);
 
-	drm_connector_register(&intel_connector->base);
+	drm_connector_register(connector);
 }
 
 static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 					   struct drm_connector *connector)
 {
 	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 
 	drm_connector_unregister(connector);
 
 	/* need to nuke the connector */
-	intel_connector_remove_from_fbdev(intel_connector);
+	if (dev_priv->fbdev)
+		drm_fb_helper_remove_one_connector(&dev_priv->fbdev->helper,
+						   connector);
 	intel_connector->mst_port = NULL;
 
-	drm_connector_unreference(&intel_connector->base);
+	drm_connector_unreference(connector);
 	DRM_DEBUG_KMS("\n");
 }
 
-- 
2.11.0

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 03/12] drm/fb-helper: Add top-level lock
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
  2017-06-21 18:28 ` [PATCH 01/12] drm/fb-helper: Push down modeset lock into FB helpers Daniel Vetter
  2017-06-21 18:28 ` [PATCH 02/12] drm/i915: Drop FBDEV #ifdev in mst code Daniel Vetter
@ 2017-06-21 18:28 ` Daniel Vetter
  2017-06-21 18:28 ` [PATCH 04/12] drm/fb-helper: Push locking in fb_is_bound Daniel Vetter
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding

From: Thierry Reding <treding@nvidia.com>

Introduce a new top-level lock for the FB helper code. This will allow
better locking granularity and avoid the need to abuse modeset locking
for this purpose instead.

This patch just adds the new lock everywhere we currently grab
mode_config->mutex (explicitly, or through drm_modeset_lock_all).
Follow-up patches will push the kms locking down into only the places
that need it.

v2:
- use lockdep_assert_held
- use drm_fb_helper_for_each_connector where possible
- use the new top-level lock consistently, i.e. in all the places
  we're currently acquiring mode_config.mutex.
- small polish to the kerneldoc

Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Thierry Reding <treding@nvidia.com> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fb_helper.c | 47 +++++++++++++++++++++++++++++++++++++----
 include/drm/drm_fb_helper.h     | 19 ++++++++++++++++-
 2 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 5e73cc9f51e3..a0d4603857d8 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -119,7 +119,8 @@ static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
 	if (!drm_fbdev_emulation)
 		return 0;
 
-	WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
+	lockdep_assert_held(&fb_helper->lock);
+	lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
 
 	count = fb_helper->connector_count + 1;
 
@@ -150,11 +151,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
 {
 	int err;
 
+	mutex_lock(&fb_helper->lock);
 	mutex_lock(&fb_helper->dev->mode_config.mutex);
 
 	err = __drm_fb_helper_add_one_connector(fb_helper, connector);
 
 	mutex_unlock(&fb_helper->dev->mode_config.mutex);
+	mutex_unlock(&fb_helper->lock);
 
 	return err;
 }
@@ -184,6 +187,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
 	if (!drm_fbdev_emulation)
 		return 0;
 
+	mutex_lock(&fb_helper->lock);
 	mutex_lock(&dev->mode_config.mutex);
 	drm_connector_list_iter_begin(dev, &conn_iter);
 	drm_for_each_connector_iter(connector, &conn_iter) {
@@ -207,6 +211,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
 out:
 	drm_connector_list_iter_end(&conn_iter);
 	mutex_unlock(&dev->mode_config.mutex);
+	mutex_unlock(&fb_helper->lock);
 
 	return ret;
 }
@@ -221,9 +226,9 @@ static int __drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 	if (!drm_fbdev_emulation)
 		return 0;
 
-	WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
+	lockdep_assert_held(&fb_helper->lock);
 
-	for (i = 0; i < fb_helper->connector_count; i++) {
+	drm_fb_helper_for_each_connector(fb_helper, i) {
 		if (fb_helper->connector_info[i]->connector == connector)
 			break;
 	}
@@ -247,11 +252,13 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 {
 	int err;
 
+	mutex_lock(&fb_helper->lock);
 	mutex_lock(&fb_helper->dev->mode_config.mutex);
 
 	err = __drm_fb_helper_remove_one_connector(fb_helper, connector);
 
 	mutex_unlock(&fb_helper->dev->mode_config.mutex);
+	mutex_unlock(&fb_helper->lock);
 
 	return err;
 }
@@ -517,16 +524,21 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 	if (!drm_fbdev_emulation)
 		return -ENODEV;
 
+	mutex_lock(&fb_helper->lock);
 	drm_modeset_lock_all(dev);
+
 	ret = restore_fbdev_mode(fb_helper);
 
 	do_delayed = fb_helper->delayed_hotplug;
 	if (do_delayed)
 		fb_helper->delayed_hotplug = false;
+
 	drm_modeset_unlock_all(dev);
+	mutex_unlock(&fb_helper->lock);
 
 	if (do_delayed)
 		drm_fb_helper_hotplug_event(fb_helper);
+
 	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
@@ -576,11 +588,13 @@ static bool drm_fb_helper_force_kernel_mode(void)
 		if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 			continue;
 
+		mutex_lock(&helper->lock);
 		drm_modeset_lock_all(dev);
 		ret = restore_fbdev_mode(helper);
 		if (ret)
 			error = true;
 		drm_modeset_unlock_all(dev);
+		mutex_unlock(&helper->lock);
 	}
 	return error;
 }
@@ -620,9 +634,11 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
 	/*
 	 * For each CRTC in this fb, turn the connectors on/off.
 	 */
+	mutex_lock(&fb_helper->lock);
 	drm_modeset_lock_all(dev);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
 		drm_modeset_unlock_all(dev);
+		mutex_unlock(&fb_helper->lock);
 		return;
 	}
 
@@ -641,6 +657,7 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
 		}
 	}
 	drm_modeset_unlock_all(dev);
+	mutex_unlock(&fb_helper->lock);
 }
 
 /**
@@ -762,6 +779,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
 	INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker);
 	INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work);
 	helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
+	mutex_init(&helper->lock);
 	helper->funcs = funcs;
 	helper->dev = dev;
 }
@@ -927,6 +945,7 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
 	}
 	mutex_unlock(&kernel_fb_helper_lock);
 
+	mutex_destroy(&fb_helper->lock);
 	drm_fb_helper_crtc_free(fb_helper);
 
 }
@@ -1257,9 +1276,11 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 	if (oops_in_progress)
 		return -EBUSY;
 
+	mutex_lock(&fb_helper->lock);
 	drm_modeset_lock_all(dev);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
 		drm_modeset_unlock_all(dev);
+		mutex_unlock(&fb_helper->lock);
 		return -EBUSY;
 	}
 
@@ -1292,6 +1313,7 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 	}
  out:
 	drm_modeset_unlock_all(dev);
+	mutex_unlock(&fb_helper->lock);
 	return rc;
 }
 EXPORT_SYMBOL(drm_fb_helper_setcmap);
@@ -1314,6 +1336,7 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 	struct drm_crtc *crtc;
 	int ret = 0;
 
+	mutex_lock(&fb_helper->lock);
 	mutex_lock(&dev->mode_config.mutex);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
 		ret = -EBUSY;
@@ -1360,6 +1383,7 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 
 unlock:
 	mutex_unlock(&dev->mode_config.mutex);
+	mutex_unlock(&fb_helper->lock);
 	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_ioctl);
@@ -1589,9 +1613,11 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 	if (oops_in_progress)
 		return -EBUSY;
 
+	mutex_lock(&fb_helper->lock);
 	drm_modeset_lock_all(dev);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
 		drm_modeset_unlock_all(dev);
+		mutex_unlock(&fb_helper->lock);
 		return -EBUSY;
 	}
 
@@ -1600,6 +1626,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 	else
 		ret = pan_display_legacy(var, info);
 	drm_modeset_unlock_all(dev);
+	mutex_unlock(&fb_helper->lock);
 
 	return ret;
 }
@@ -2266,6 +2293,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
 
 	/* prevent concurrent modification of connector_count by hotplug */
+	lockdep_assert_held(&fb_helper->lock);
 	lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
 
 	crtcs = kcalloc(fb_helper->connector_count,
@@ -2392,12 +2420,14 @@ int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
 	if (!drm_fbdev_emulation)
 		return 0;
 
+	mutex_lock(&fb_helper->lock);
 	mutex_lock(&dev->mode_config.mutex);
 	drm_setup_crtcs(fb_helper,
 			dev->mode_config.max_width,
 			dev->mode_config.max_height);
 	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
 	mutex_unlock(&dev->mode_config.mutex);
+	mutex_unlock(&fb_helper->lock);
 	if (ret)
 		return ret;
 
@@ -2445,25 +2475,34 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
 int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 {
 	struct drm_device *dev = fb_helper->dev;
+	int err = 0;
 
 	if (!drm_fbdev_emulation)
 		return 0;
 
+	mutex_lock(&fb_helper->lock);
 	mutex_lock(&dev->mode_config.mutex);
+
 	if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
 		fb_helper->delayed_hotplug = true;
 		mutex_unlock(&dev->mode_config.mutex);
-		return 0;
+		goto unlock;
 	}
+
 	DRM_DEBUG_KMS("\n");
 
 	drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
 
 	mutex_unlock(&dev->mode_config.mutex);
+	mutex_unlock(&fb_helper->lock);
 
 	drm_fb_helper_set_par(fb_helper->fbdev);
 
 	return 0;
+
+unlock:
+	mutex_unlock(&fb_helper->lock);
+	return err;
 }
 EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
 
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 119e5e4609c7..ea170b96e88d 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -169,7 +169,6 @@ struct drm_fb_helper_connector {
  * @crtc_info: per-CRTC helper state (mode, x/y offset, etc)
  * @connector_count: number of connected connectors
  * @connector_info_alloc_count: size of connector_info
- * @connector_info: array of per-connector information
  * @funcs: driver callbacks for fb helper
  * @fbdev: emulated fbdev device info struct
  * @pseudo_palette: fake palette of 16 colors
@@ -191,6 +190,12 @@ struct drm_fb_helper {
 	struct drm_fb_helper_crtc *crtc_info;
 	int connector_count;
 	int connector_info_alloc_count;
+	/**
+	 * @connector_info:
+	 *
+	 * Array of per-connector information. Do not iterate directly, but use
+	 * drm_fb_helper_for_each_connector.
+	 */
 	struct drm_fb_helper_connector **connector_info;
 	const struct drm_fb_helper_funcs *funcs;
 	struct fb_info *fbdev;
@@ -201,6 +206,18 @@ struct drm_fb_helper {
 	struct work_struct resume_work;
 
 	/**
+	 * @lock:
+	 *
+	 * Top-level FBDEV helper lock. This protects all internal data
+	 * structures and lists, such as @connector_info and @crtc_info.
+	 *
+	 * FIXME: fbdev emulation locking is a mess and long term we want to
+	 * protect all helper internal state with this lock as well as reduce
+	 * core KMS locking as much as possible.
+	 */
+	struct mutex lock;
+
+	/**
 	 * @kernel_fb_list:
 	 *
 	 * Entry on the global kernel_fb_helper_list, used for kgdb entry/exit.
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 04/12] drm/fb-helper: Push locking in fb_is_bound
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (2 preceding siblings ...)
  2017-06-21 18:28 ` [PATCH 03/12] drm/fb-helper: Add top-level lock Daniel Vetter
@ 2017-06-21 18:28 ` Daniel Vetter
  2017-06-21 18:28 ` [PATCH 05/12] drm/fb-helper: Drop locking from the vsync wait ioctl code Daniel Vetter
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding, Daniel Vetter

That function only needs to take the individual crtc locks, not all
the kms locks. Push down the locking and then minimize it.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index a0d4603857d8..14b3f885a01f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -557,10 +557,12 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
 		return false;
 
 	drm_for_each_crtc(crtc, dev) {
+		drm_modeset_lock(&crtc->mutex, NULL);
 		if (crtc->primary->fb)
 			crtcs_bound++;
 		if (crtc->primary->fb == fb_helper->fb)
 			bound++;
+		drm_modeset_unlock(&crtc->mutex);
 	}
 
 	if (bound < crtcs_bound)
@@ -635,13 +637,12 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
 	 * For each CRTC in this fb, turn the connectors on/off.
 	 */
 	mutex_lock(&fb_helper->lock);
-	drm_modeset_lock_all(dev);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
-		drm_modeset_unlock_all(dev);
 		mutex_unlock(&fb_helper->lock);
 		return;
 	}
 
+	drm_modeset_lock_all(dev);
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		crtc = fb_helper->crtc_info[i].mode_set.crtc;
 
@@ -1277,13 +1278,12 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 		return -EBUSY;
 
 	mutex_lock(&fb_helper->lock);
-	drm_modeset_lock_all(dev);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
-		drm_modeset_unlock_all(dev);
 		mutex_unlock(&fb_helper->lock);
 		return -EBUSY;
 	}
 
+	drm_modeset_lock_all(dev);
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		crtc = fb_helper->crtc_info[i].mode_set.crtc;
 		crtc_funcs = crtc->helper_private;
@@ -1337,12 +1337,12 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 	int ret = 0;
 
 	mutex_lock(&fb_helper->lock);
-	mutex_lock(&dev->mode_config.mutex);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
 		ret = -EBUSY;
 		goto unlock;
 	}
 
+	mutex_lock(&dev->mode_config.mutex);
 	switch (cmd) {
 	case FBIO_WAITFORVSYNC:
 		/*
@@ -1614,13 +1614,12 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 		return -EBUSY;
 
 	mutex_lock(&fb_helper->lock);
-	drm_modeset_lock_all(dev);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
-		drm_modeset_unlock_all(dev);
 		mutex_unlock(&fb_helper->lock);
 		return -EBUSY;
 	}
 
+	drm_modeset_lock_all(dev);
 	if (drm_drv_uses_atomic_modeset(dev))
 		ret = pan_display_atomic(var, info);
 	else
@@ -2481,16 +2480,15 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
-	mutex_lock(&dev->mode_config.mutex);
-
 	if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
 		fb_helper->delayed_hotplug = true;
-		mutex_unlock(&dev->mode_config.mutex);
-		goto unlock;
+		mutex_unlock(&fb_helper->lock);
+		return err;
 	}
 
 	DRM_DEBUG_KMS("\n");
 
+	mutex_lock(&dev->mode_config.mutex);
 	drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
 
 	mutex_unlock(&dev->mode_config.mutex);
@@ -2499,10 +2497,6 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 	drm_fb_helper_set_par(fb_helper->fbdev);
 
 	return 0;
-
-unlock:
-	mutex_unlock(&fb_helper->lock);
-	return err;
 }
 EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
 
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 05/12] drm/fb-helper: Drop locking from the vsync wait ioctl code
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (3 preceding siblings ...)
  2017-06-21 18:28 ` [PATCH 04/12] drm/fb-helper: Push locking in fb_is_bound Daniel Vetter
@ 2017-06-21 18:28 ` Daniel Vetter
  2017-06-21 18:28 ` [PATCH 06/12] drm/fb-helper: Push locking into pan_display_atomic|legacy Daniel Vetter
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding,
	John Stultz, Daniel Vetter

Like with the drm-native vblank wait ioctl we can entirely rely on the
spinlocks in drm_vblank.c, no need at all to take expensive mutexes.
The only reason we had to take mode_config.mutex was to protect the
fbdev helper's data-structures, but that's now done by
fb_helper->lock.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 14b3f885a01f..13330c22e6bf 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1342,7 +1342,6 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 		goto unlock;
 	}
 
-	mutex_lock(&dev->mode_config.mutex);
 	switch (cmd) {
 	case FBIO_WAITFORVSYNC:
 		/*
@@ -1382,7 +1381,6 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 	}
 
 unlock:
-	mutex_unlock(&dev->mode_config.mutex);
 	mutex_unlock(&fb_helper->lock);
 	return ret;
 }
-- 
2.11.0

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 06/12] drm/fb-helper: Push locking into pan_display_atomic|legacy
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (4 preceding siblings ...)
  2017-06-21 18:28 ` [PATCH 05/12] drm/fb-helper: Drop locking from the vsync wait ioctl code Daniel Vetter
@ 2017-06-21 18:28 ` Daniel Vetter
  2017-06-21 18:28 ` [PATCH 07/12] drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy Daniel Vetter
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding, Daniel Vetter

For the legacy path we'll keep drm_modeset_lock_all, for the atomic
one we drop the use of the magic implicit context and wire it up
properly.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 13330c22e6bf..5d8bf7dfa618 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1331,7 +1331,6 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 			unsigned long arg)
 {
 	struct drm_fb_helper *fb_helper = info->par;
-	struct drm_device *dev = fb_helper->dev;
 	struct drm_mode_set *mode_set;
 	struct drm_crtc *crtc;
 	int ret = 0;
@@ -1522,12 +1521,17 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
 	struct drm_plane *plane;
 	int i, ret;
 	unsigned int plane_mask;
+	struct drm_modeset_acquire_ctx ctx;
+
+	drm_modeset_acquire_init(&ctx, 0);
 
 	state = drm_atomic_state_alloc(dev);
-	if (!state)
-		return -ENOMEM;
+	if (!state) {
+		ret = -ENOMEM;
+		goto out_ctx;
+	}
 
-	state->acquire_ctx = dev->mode_config.acquire_ctx;
+	state->acquire_ctx = &ctx;
 retry:
 	plane_mask = 0;
 	for (i = 0; i < fb_helper->crtc_count; i++) {
@@ -1540,7 +1544,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
 
 		ret = __drm_atomic_helper_set_config(mode_set, state);
 		if (ret != 0)
-			goto fail;
+			goto out_state;
 
 		plane = mode_set->crtc->primary;
 		plane_mask |= (1 << drm_plane_index(plane));
@@ -1549,23 +1553,27 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
 
 	ret = drm_atomic_commit(state);
 	if (ret != 0)
-		goto fail;
+		goto out_state;
 
 	info->var.xoffset = var->xoffset;
 	info->var.yoffset = var->yoffset;
 
-fail:
+out_state:
 	drm_atomic_clean_old_fb(dev, plane_mask, ret);
 
 	if (ret == -EDEADLK)
 		goto backoff;
 
 	drm_atomic_state_put(state);
+out_ctx:
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
 	return ret;
 
 backoff:
 	drm_atomic_state_clear(state);
-	drm_atomic_legacy_backoff(state);
+	drm_modeset_backoff(&ctx);
 
 	goto retry;
 }
@@ -1578,6 +1586,7 @@ static int pan_display_legacy(struct fb_var_screeninfo *var,
 	int ret = 0;
 	int i;
 
+	drm_modeset_lock_all(fb_helper->dev);
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		modeset = &fb_helper->crtc_info[i].mode_set;
 
@@ -1592,6 +1601,7 @@ static int pan_display_legacy(struct fb_var_screeninfo *var,
 			}
 		}
 	}
+	drm_modeset_unlock_all(fb_helper->dev);
 
 	return ret;
 }
@@ -1617,12 +1627,10 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 		return -EBUSY;
 	}
 
-	drm_modeset_lock_all(dev);
 	if (drm_drv_uses_atomic_modeset(dev))
 		ret = pan_display_atomic(var, info);
 	else
 		ret = pan_display_legacy(var, info);
-	drm_modeset_unlock_all(dev);
 	mutex_unlock(&fb_helper->lock);
 
 	return ret;
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 07/12] drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (5 preceding siblings ...)
  2017-06-21 18:28 ` [PATCH 06/12] drm/fb-helper: Push locking into pan_display_atomic|legacy Daniel Vetter
@ 2017-06-21 18:28 ` Daniel Vetter
  2017-06-21 18:28 ` [PATCH 08/12] drm/fb-helper: Stop using mode_config.mutex for internals Daniel Vetter
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding, Daniel Vetter

Same game as with the panning function, use drm_modeset_lock_all for
legacy paths, and a proper acquire ctx w/w mutex dance for atomic.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 48 +++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 5d8bf7dfa618..400bbb07eff2 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -396,12 +396,17 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
 	struct drm_atomic_state *state;
 	int i, ret;
 	unsigned int plane_mask;
+	struct drm_modeset_acquire_ctx ctx;
+
+	drm_modeset_acquire_init(&ctx, 0);
 
 	state = drm_atomic_state_alloc(dev);
-	if (!state)
-		return -ENOMEM;
+	if (!state) {
+		ret = -ENOMEM;
+		goto out_ctx;
+	}
 
-	state->acquire_ctx = dev->mode_config.acquire_ctx;
+	state->acquire_ctx = &ctx;
 retry:
 	plane_mask = 0;
 	drm_for_each_plane(plane, dev) {
@@ -410,7 +415,7 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
 		plane_state = drm_atomic_get_plane_state(state, plane);
 		if (IS_ERR(plane_state)) {
 			ret = PTR_ERR(plane_state);
-			goto fail;
+			goto out_state;
 		}
 
 		plane_state->rotation = DRM_MODE_ROTATE_0;
@@ -424,7 +429,7 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
 
 		ret = __drm_atomic_helper_disable_plane(plane, plane_state);
 		if (ret != 0)
-			goto fail;
+			goto out_state;
 	}
 
 	for (i = 0; i < fb_helper->crtc_count; i++) {
@@ -432,23 +437,27 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
 
 		ret = __drm_atomic_helper_set_config(mode_set, state);
 		if (ret != 0)
-			goto fail;
+			goto out_state;
 	}
 
 	ret = drm_atomic_commit(state);
 
-fail:
+out_state:
 	drm_atomic_clean_old_fb(dev, plane_mask, ret);
 
 	if (ret == -EDEADLK)
 		goto backoff;
 
 	drm_atomic_state_put(state);
+out_ctx:
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
 	return ret;
 
 backoff:
 	drm_atomic_state_clear(state);
-	drm_atomic_legacy_backoff(state);
+	drm_modeset_backoff(&ctx);
 
 	goto retry;
 }
@@ -457,8 +466,9 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
 {
 	struct drm_device *dev = fb_helper->dev;
 	struct drm_plane *plane;
-	int i;
+	int i, ret = 0;
 
+	drm_modeset_lock_all(fb_helper->dev);
 	drm_for_each_plane(plane, dev) {
 		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
 			drm_plane_force_disable(plane);
@@ -472,32 +482,31 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
 		struct drm_crtc *crtc = mode_set->crtc;
-		int ret;
 
 		if (crtc->funcs->cursor_set2) {
 			ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0);
 			if (ret)
-				return ret;
+				goto out;
 		} else if (crtc->funcs->cursor_set) {
 			ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0);
 			if (ret)
-				return ret;
+				goto out;
 		}
 
 		ret = drm_mode_set_config_internal(mode_set);
 		if (ret)
-			return ret;
+			goto out;
 	}
+out:
+	drm_modeset_unlock_all(fb_helper->dev);
 
-	return 0;
+	return ret;
 }
 
 static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 {
 	struct drm_device *dev = fb_helper->dev;
 
-	drm_warn_on_modeset_not_all_locked(dev);
-
 	if (drm_drv_uses_atomic_modeset(dev))
 		return restore_fbdev_mode_atomic(fb_helper);
 	else
@@ -517,7 +526,6 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
  */
 int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 {
-	struct drm_device *dev = fb_helper->dev;
 	bool do_delayed;
 	int ret;
 
@@ -525,15 +533,11 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 		return -ENODEV;
 
 	mutex_lock(&fb_helper->lock);
-	drm_modeset_lock_all(dev);
-
 	ret = restore_fbdev_mode(fb_helper);
 
 	do_delayed = fb_helper->delayed_hotplug;
 	if (do_delayed)
 		fb_helper->delayed_hotplug = false;
-
-	drm_modeset_unlock_all(dev);
 	mutex_unlock(&fb_helper->lock);
 
 	if (do_delayed)
@@ -591,11 +595,9 @@ static bool drm_fb_helper_force_kernel_mode(void)
 			continue;
 
 		mutex_lock(&helper->lock);
-		drm_modeset_lock_all(dev);
 		ret = restore_fbdev_mode(helper);
 		if (ret)
 			error = true;
-		drm_modeset_unlock_all(dev);
 		mutex_unlock(&helper->lock);
 	}
 	return error;
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 08/12] drm/fb-helper: Stop using mode_config.mutex for internals
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (6 preceding siblings ...)
  2017-06-21 18:28 ` [PATCH 07/12] drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy Daniel Vetter
@ 2017-06-21 18:28 ` Daniel Vetter
  2017-06-21 18:28 ` [PATCH 09/12] drm/fb-helper: Split dpms handling into legacy and atomic paths Daniel Vetter
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding, Daniel Vetter

Those are now all protected using fb_helper->lock.

v2: We still need to hold mode_config.mutex right around calling
connector->fill_modes.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 21 +++------------------
 drivers/gpu/drm/drm_vblank.c    |  2 +-
 2 files changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 400bbb07eff2..7aaac457eeaa 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -106,7 +106,7 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
  */
 
 #define drm_fb_helper_for_each_connector(fbh, i__) \
-	for (({ lockdep_assert_held(&(fbh)->dev->mode_config.mutex); }), \
+	for (({ lockdep_assert_held(&(fbh)->lock); }), \
 	     i__ = 0; i__ < (fbh)->connector_count; i__++)
 
 static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
@@ -120,7 +120,6 @@ static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
 		return 0;
 
 	lockdep_assert_held(&fb_helper->lock);
-	lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
 
 	count = fb_helper->connector_count + 1;
 
@@ -152,11 +151,7 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
 	int err;
 
 	mutex_lock(&fb_helper->lock);
-	mutex_lock(&fb_helper->dev->mode_config.mutex);
-
 	err = __drm_fb_helper_add_one_connector(fb_helper, connector);
-
-	mutex_unlock(&fb_helper->dev->mode_config.mutex);
 	mutex_unlock(&fb_helper->lock);
 
 	return err;
@@ -188,7 +183,6 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
-	mutex_lock(&dev->mode_config.mutex);
 	drm_connector_list_iter_begin(dev, &conn_iter);
 	drm_for_each_connector_iter(connector, &conn_iter) {
 		ret = __drm_fb_helper_add_one_connector(fb_helper, connector);
@@ -210,7 +204,6 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
 	fb_helper->connector_count = 0;
 out:
 	drm_connector_list_iter_end(&conn_iter);
-	mutex_unlock(&dev->mode_config.mutex);
 	mutex_unlock(&fb_helper->lock);
 
 	return ret;
@@ -253,11 +246,7 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 	int err;
 
 	mutex_lock(&fb_helper->lock);
-	mutex_lock(&fb_helper->dev->mode_config.mutex);
-
 	err = __drm_fb_helper_remove_one_connector(fb_helper, connector);
-
-	mutex_unlock(&fb_helper->dev->mode_config.mutex);
 	mutex_unlock(&fb_helper->lock);
 
 	return err;
@@ -1900,10 +1889,12 @@ static int drm_fb_helper_probe_connector_modes(struct drm_fb_helper *fb_helper,
 	int count = 0;
 	int i;
 
+	mutex_lock(&fb_helper->dev->mode_config.mutex);
 	drm_fb_helper_for_each_connector(fb_helper, i) {
 		connector = fb_helper->connector_info[i]->connector;
 		count += connector->funcs->fill_modes(connector, maxX, maxY);
 	}
+	mutex_unlock(&fb_helper->dev->mode_config.mutex);
 
 	return count;
 }
@@ -2301,7 +2292,6 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 
 	/* prevent concurrent modification of connector_count by hotplug */
 	lockdep_assert_held(&fb_helper->lock);
-	lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
 
 	crtcs = kcalloc(fb_helper->connector_count,
 			sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL);
@@ -2428,12 +2418,10 @@ int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
-	mutex_lock(&dev->mode_config.mutex);
 	drm_setup_crtcs(fb_helper,
 			dev->mode_config.max_width,
 			dev->mode_config.max_height);
 	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
-	mutex_unlock(&dev->mode_config.mutex);
 	mutex_unlock(&fb_helper->lock);
 	if (ret)
 		return ret;
@@ -2496,10 +2484,7 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 
 	DRM_DEBUG_KMS("\n");
 
-	mutex_lock(&dev->mode_config.mutex);
 	drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
-
-	mutex_unlock(&dev->mode_config.mutex);
 	mutex_unlock(&fb_helper->lock);
 
 	drm_fb_helper_set_par(fb_helper->fbdev);
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 05d043e9219f..8099574c8a11 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -836,7 +836,7 @@ static void send_vblank_event(struct drm_device *dev,
  * NOTE: Drivers using this to send out the &drm_crtc_state.event as part of an
  * atomic commit must ensure that the next vblank happens at exactly the same
  * time as the atomic commit is committed to the hardware. This function itself
- * does **not** protect again the next vblank interrupt racing with either this
+ * does **not** protect against the next vblank interrupt racing with either this
  * function call or the atomic commit operation. A possible sequence could be:
  *
  * 1. Driver commits new hardware state into vblank-synchronized registers.
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 09/12] drm/fb-helper: Split dpms handling into legacy and atomic paths
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (7 preceding siblings ...)
  2017-06-21 18:28 ` [PATCH 08/12] drm/fb-helper: Stop using mode_config.mutex for internals Daniel Vetter
@ 2017-06-21 18:28 ` Daniel Vetter
  2017-06-22 10:24   ` Peter Rosin
  2017-06-21 18:28 ` [PATCH 10/12] drm/fb-helper: Support deferred setup Daniel Vetter
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, John Stultz,
	Daniel Vetter, Thierry Reding, Peter Rosin

Like with panning and modesetting, and like with those, stick with
simple drm_modeset_locking_all for the legacy path, and the full
atomic dance for atomic drivers.

This means a bit more boilerplate since setting up the atomic state
machinery is rather verbose, but then this is shared code for 30+
drivers or so, so meh.

After this patch there's only the LUT/cmap path which is still using
drm_modeset_lock_all for an atomic driver. But Peter is already
locking into reworking that, so I'll leave that code as-is for now.

Cc: Peter Rosin <peda@axentia.se>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 88 +++++++++++++++++++++++++++++++++++------
 1 file changed, 76 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 7aaac457eeaa..e3d033df068f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -616,23 +616,13 @@ static struct sysrq_key_op sysrq_drm_fb_helper_restore_op = {
 static struct sysrq_key_op sysrq_drm_fb_helper_restore_op = { };
 #endif
 
-static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
+static void dpms_legacy(struct drm_fb_helper *fb_helper, int dpms_mode)
 {
-	struct drm_fb_helper *fb_helper = info->par;
 	struct drm_device *dev = fb_helper->dev;
 	struct drm_crtc *crtc;
 	struct drm_connector *connector;
 	int i, j;
 
-	/*
-	 * For each CRTC in this fb, turn the connectors on/off.
-	 */
-	mutex_lock(&fb_helper->lock);
-	if (!drm_fb_helper_is_bound(fb_helper)) {
-		mutex_unlock(&fb_helper->lock);
-		return;
-	}
-
 	drm_modeset_lock_all(dev);
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		crtc = fb_helper->crtc_info[i].mode_set.crtc;
@@ -649,6 +639,81 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
 		}
 	}
 	drm_modeset_unlock_all(dev);
+}
+
+static void dpms_atomic(struct drm_fb_helper *fb_helper, int dpms_mode)
+{
+	struct drm_device *dev = fb_helper->dev;
+	struct drm_atomic_state *state;
+	int i, ret;
+
+	struct drm_modeset_acquire_ctx ctx;
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	state = drm_atomic_state_alloc(dev);
+	if (!state) {
+		ret = -ENOMEM;
+		goto out_ctx;
+	}
+
+	state->acquire_ctx = &ctx;
+retry:
+	for (i = 0; i < fb_helper->crtc_count; i++) {
+		struct drm_crtc_state *crtc_state;
+		struct drm_crtc *crtc;
+
+		if (!fb_helper->crtc_info[i].mode_set.mode)
+			continue;
+
+		crtc = fb_helper->crtc_info[i].mode_set.crtc;
+
+		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(crtc_state)) {
+			ret = PTR_ERR(crtc_state);
+			goto out_state;
+		}
+
+		crtc_state->active = dpms_mode == DRM_MODE_DPMS_ON;
+	}
+
+	ret = drm_atomic_commit(state);
+out_state:
+	if (ret == -EDEADLK)
+		goto backoff;
+
+	drm_atomic_state_put(state);
+out_ctx:
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	return;
+
+backoff:
+	drm_atomic_state_clear(state);
+	drm_modeset_backoff(&ctx);
+
+	goto retry;
+
+}
+
+static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+
+	/*
+	 * For each CRTC in this fb, turn the connectors on/off.
+	 */
+	mutex_lock(&fb_helper->lock);
+	if (!drm_fb_helper_is_bound(fb_helper)) {
+		mutex_unlock(&fb_helper->lock);
+		return;
+	}
+
+	if (drm_drv_uses_atomic_modeset(fb_helper->dev))
+		dpms_atomic(fb_helper, dpms_mode);
+	else
+		dpms_legacy(fb_helper, dpms_mode);
 	mutex_unlock(&fb_helper->lock);
 }
 
@@ -2469,7 +2534,6 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
  */
 int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 {
-	struct drm_device *dev = fb_helper->dev;
 	int err = 0;
 
 	if (!drm_fbdev_emulation)
-- 
2.11.0

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 10/12] drm/fb-helper: Support deferred setup
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (8 preceding siblings ...)
  2017-06-21 18:28 ` [PATCH 09/12] drm/fb-helper: Split dpms handling into legacy and atomic paths Daniel Vetter
@ 2017-06-21 18:28 ` Daniel Vetter
  2017-06-23 13:31   ` [PATCH] " Daniel Vetter
  2017-06-21 18:28 ` [PATCH 11/12] drm/exynos: Remove custom FB helper " Daniel Vetter
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding, John Stultz

From: Thierry Reding <treding@nvidia.com>

FB helper code falls back to a 1024x768 mode if no outputs are connected
or don't report back any modes upon initialization. This can be annoying
because outputs that are added to FB helper later on can't be used with
FB helper if they don't support a matching mode.

The fallback is in place because VGA connectors can happen to report an
unknown connection status even when they are in fact connected.

Some drivers have custom solutions in place to defer FB helper setup
until at least one output is connected. But the logic behind these
solutions is always the same and there is nothing driver-specific about
it, so a better alterative is to fix the FB helper core and add support
for all drivers automatically.

This patch adds support for deferred FB helper setup. It checks all the
connectors for their connection status, and if all of them report to be
disconnected marks the FB helper as needing deferred setup. Whet setup
is deferred, the FB helper core will automatically retry setup after a
hotplug event, and it will keep trying until it succeeds.

v2: Rebase onto my entirely reworked fbdev helper locking. One big
difference is that this version again drops&reacquires the fbdev lock
(which is now fb_helper->lock, but before this patch series it was
mode_config->mutex), because register_framebuffer must be able to
recurse back into fbdev helper code for the initial screen setup.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Thierry Reding <treding@nvidia.com> (v1)
Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Thierry Reding <treding@nvidia.com> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fb_helper.c | 111 ++++++++++++++++++++++++++++++----------
 include/drm/drm_fb_helper.h     |  23 +++++++++
 2 files changed, 107 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index e3d033df068f..a4cfef9bcfa7 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -521,6 +521,9 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 	if (!drm_fbdev_emulation)
 		return -ENODEV;
 
+	if (READ_ONCE(fb_helper->deferred_setup))
+		return 0;
+
 	mutex_lock(&fb_helper->lock);
 	ret = restore_fbdev_mode(fb_helper);
 
@@ -1693,6 +1696,23 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 }
 EXPORT_SYMBOL(drm_fb_helper_pan_display);
 
+static bool drm_fb_helper_maybe_connected(struct drm_fb_helper *helper)
+{
+	bool connected = false;
+	unsigned int i;
+
+	for (i = 0; i < helper->connector_count; i++) {
+		struct drm_fb_helper_connector *fb = helper->connector_info[i];
+
+		if (fb->connector->status != connector_status_disconnected) {
+			connected = true;
+			break;
+		}
+	}
+
+	return connected;
+}
+
 /*
  * Allocates the backing storage and sets up the fbdev info structure through
  * the ->fb_probe callback and then registers the fbdev and sets up the panic
@@ -2352,8 +2372,6 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 	int i;
 
 	DRM_DEBUG_KMS("\n");
-	if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
-		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
 
 	/* prevent concurrent modification of connector_count by hotplug */
 	lockdep_assert_held(&fb_helper->lock);
@@ -2431,6 +2449,61 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 	kfree(enabled);
 }
 
+/* Note: Drops&reacquires fb_helper->lock */
+static int __drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
+					  int bpp_sel)
+{
+	struct drm_device *dev = fb_helper->dev;
+	struct fb_info *info;
+	unsigned int width, height;
+	int ret;
+
+	width = dev->mode_config.max_width;
+	height = dev->mode_config.max_height;
+
+	if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
+		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
+
+	/*
+	 * If everything's disconnected, there's no use in attempting to set
+	 * up fbdev.
+	 */
+	if (!drm_fb_helper_maybe_connected(fb_helper)) {
+		DRM_INFO("No outputs connected, deferring setup\n");
+		fb_helper->preferred_bpp = bpp_sel;
+		fb_helper->deferred_setup = true;
+		mutex_unlock(&dev->mode_config.mutex);
+		return 0;
+	}
+
+	drm_setup_crtcs(fb_helper, width, height);
+	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
+	if (ret) {
+		mutex_unlock(&fb_helper->lock);
+		return ret;
+	}
+
+	fb_helper->deferred_setup = false;
+	mutex_unlock(&fb_helper->lock);
+
+	info = fb_helper->fbdev;
+	info->var.pixclock = 0;
+	ret = register_framebuffer(info);
+	if (ret < 0)
+		return ret;
+
+	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
+		 info->node, info->fix.id);
+
+	mutex_lock(&kernel_fb_helper_lock);
+	if (list_empty(&kernel_fb_helper_list))
+		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
+
+	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
+
+	return 0;
+}
+
 /**
  * drm_fb_helper_initial_config - setup a sane initial connector configuration
  * @fb_helper: fb_helper device struct
@@ -2475,39 +2548,16 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
  */
 int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
 {
-	struct drm_device *dev = fb_helper->dev;
-	struct fb_info *info;
 	int ret;
 
 	if (!drm_fbdev_emulation)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
-	drm_setup_crtcs(fb_helper,
-			dev->mode_config.max_width,
-			dev->mode_config.max_height);
-	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
-	mutex_unlock(&fb_helper->lock);
-	if (ret)
-		return ret;
-
-	info = fb_helper->fbdev;
-	info->var.pixclock = 0;
-	ret = register_framebuffer(info);
-	if (ret < 0)
-		return ret;
-
-	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
-		 info->node, info->fix.id);
-
-	mutex_lock(&kernel_fb_helper_lock);
-	if (list_empty(&kernel_fb_helper_list))
-		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
-
-	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
+	ret = __drm_fb_helper_initial_config(fb_helper, bpp_sel);
 	mutex_unlock(&kernel_fb_helper_lock);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_initial_config);
 
@@ -2540,6 +2590,13 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
+	if (fb_helper->deferred_setup) {
+		err = __drm_fb_helper_initial_config(fb_helper,
+						     fb_helper->preferred_bpp);
+		mutex_unlock(&fb_helper->lock);
+		return err;
+	}
+
 	if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
 		fb_helper->delayed_hotplug = true;
 		mutex_unlock(&fb_helper->lock);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index ea170b96e88d..a5ea6ffdfecc 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -232,6 +232,29 @@ struct drm_fb_helper {
 	 * needs to be reprobe when fbdev is in control again.
 	 */
 	bool delayed_hotplug;
+
+	/**
+	 * @deferred_setup:
+	 *
+	 * If no outputs are connected (disconnected or unknown) the FB helper
+	 * code will defer setup until at least one of the outputs shows up.
+	 * This field keeps track of the status so that setup can be retried
+	 * at every hotplug event until it succeeds eventually.
+	 *
+	 * Protected by @lock.
+	 */
+	bool deferred_setup;
+
+	/**
+	 * @preferred_bpp:
+	 *
+	 * Temporary storage for the driver's preferred BPP setting passed to
+	 * FB helper initialization. This needs to be tracked so that deferred
+	 * FB helper setup can pass this on.
+	 *
+	 * See also: @deferred_setup
+	 */
+	int preferred_bpp;
 };
 
 /**
-- 
2.11.0

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 11/12] drm/exynos: Remove custom FB helper deferred setup
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (9 preceding siblings ...)
  2017-06-21 18:28 ` [PATCH 10/12] drm/fb-helper: Support deferred setup Daniel Vetter
@ 2017-06-21 18:28 ` Daniel Vetter
  2017-06-21 18:28 ` [PATCH 12/12] drm/hisilicon: " Daniel Vetter
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development
  Cc: Joonyoung Shim, Daniel Vetter, Intel Graphics Development,
	Seung-Woo Kim, Inki Dae, Kyungmin Park, Thierry Reding

From: Thierry Reding <treding@nvidia.com>

The FB helper core now supports deferred setup, so the driver's custom
implementation can be removed.

v2: Drop NULL check, drm_fb_helper_hotplug_event handles that already.

Cc: Inki Dae <inki.dae@samsung.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Thierry Reding <treding@nvidia.com> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c   |  6 ++++--
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 26 +-------------------------
 2 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 35a8dfc93836..cab9e12d7846 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -395,8 +395,9 @@ static int exynos_drm_bind(struct device *dev)
 	/* init kms poll for handling hpd */
 	drm_kms_helper_poll_init(drm);
 
-	/* force connectors detection */
-	drm_helper_hpd_irq_event(drm);
+	ret = exynos_drm_fbdev_init(drm);
+	if (ret)
+		goto err_cleanup_poll;
 
 	/* register the DRM device */
 	ret = drm_dev_register(drm, 0);
@@ -407,6 +408,7 @@ static int exynos_drm_bind(struct device *dev)
 
 err_cleanup_fbdev:
 	exynos_drm_fbdev_fini(drm);
+err_cleanup_poll:
 	drm_kms_helper_poll_fini(drm);
 	exynos_drm_device_subdrv_remove(drm);
 err_unbind_all:
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 641531243e04..c3a068409b48 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -183,24 +183,6 @@ static const struct drm_fb_helper_funcs exynos_drm_fb_helper_funcs = {
 	.fb_probe =	exynos_drm_fbdev_create,
 };
 
-static bool exynos_drm_fbdev_is_anything_connected(struct drm_device *dev)
-{
-	struct drm_connector *connector;
-	bool ret = false;
-
-	mutex_lock(&dev->mode_config.mutex);
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
-		if (connector->status != connector_status_connected)
-			continue;
-
-		ret = true;
-		break;
-	}
-	mutex_unlock(&dev->mode_config.mutex);
-
-	return ret;
-}
-
 int exynos_drm_fbdev_init(struct drm_device *dev)
 {
 	struct exynos_drm_fbdev *fbdev;
@@ -211,9 +193,6 @@ int exynos_drm_fbdev_init(struct drm_device *dev)
 	if (!dev->mode_config.num_crtc || !dev->mode_config.num_connector)
 		return 0;
 
-	if (!exynos_drm_fbdev_is_anything_connected(dev))
-		return 0;
-
 	fbdev = kzalloc(sizeof(*fbdev), GFP_KERNEL);
 	if (!fbdev)
 		return -ENOMEM;
@@ -304,8 +283,5 @@ void exynos_drm_output_poll_changed(struct drm_device *dev)
 	struct exynos_drm_private *private = dev->dev_private;
 	struct drm_fb_helper *fb_helper = private->fb_helper;
 
-	if (fb_helper)
-		drm_fb_helper_hotplug_event(fb_helper);
-	else
-		exynos_drm_fbdev_init(dev);
+	drm_fb_helper_hotplug_event(fb_helper);
 }
-- 
2.11.0

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 12/12] drm/hisilicon: Remove custom FB helper deferred setup
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (10 preceding siblings ...)
  2017-06-21 18:28 ` [PATCH 11/12] drm/exynos: Remove custom FB helper " Daniel Vetter
@ 2017-06-21 18:28 ` Daniel Vetter
  2017-06-21 18:48 ` ✓ Fi.CI.BAT: success for fbdev helper locking rework and " Patchwork
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-21 18:28 UTC (permalink / raw)
  To: DRI Development
  Cc: Chen Feng, Intel Graphics Development, Xinliang Liu, Xinwei Kong,
	Daniel Vetter, Rongrong Zou, Thierry Reding

From: Thierry Reding <treding@nvidia.com>

The FB helper core now supports deferred setup, so the driver's custom
implementation can be removed.

Cc: Xinliang Liu <z.liuxinliang@hisilicon.com>
Cc: Rongrong Zou <zourongrong@gmail.com>
Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Chen Feng <puck.chen@hisilicon.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index 8065d6cb1d7f..da51befa8246 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -54,14 +54,7 @@ static void kirin_fbdev_output_poll_changed(struct drm_device *dev)
 {
 	struct kirin_drm_private *priv = dev->dev_private;
 
-	if (priv->fbdev) {
-		drm_fbdev_cma_hotplug_event(priv->fbdev);
-	} else {
-		priv->fbdev = drm_fbdev_cma_init(dev, 32,
-						 dev->mode_config.num_connector);
-		if (IS_ERR(priv->fbdev))
-			priv->fbdev = NULL;
-	}
+	drm_fbdev_cma_hotplug_event(priv->fbdev);
 }
 #endif
 
@@ -128,11 +121,19 @@ static int kirin_drm_kms_init(struct drm_device *dev)
 	/* init kms poll for handling hpd */
 	drm_kms_helper_poll_init(dev);
 
-	/* force detection after connectors init */
-	(void)drm_helper_hpd_irq_event(dev);
+	priv->fbdev = drm_fbdev_cma_init(dev, 32,
+					 dev->mode_config.num_connector);
+	if (IS_ERR(priv->fbdev)) {
+		DRM_ERROR("failed to initialize fbdev.\n");
+		ret = PTR_ERR(priv->fbdev);
+		goto err_cleanup_poll;
+	}
 
 	return 0;
 
+err_cleanup_poll:
+	drm_kms_helper_poll_fini(dev);
+	drm_vblank_cleanup(dev);
 err_unbind_all:
 	component_unbind_all(dev->dev, dev);
 err_dc_cleanup:
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* ✓ Fi.CI.BAT: success for fbdev helper locking rework and deferred setup
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (11 preceding siblings ...)
  2017-06-21 18:28 ` [PATCH 12/12] drm/hisilicon: " Daniel Vetter
@ 2017-06-21 18:48 ` Patchwork
  2017-06-21 23:01 ` [PATCH 00/12] " John Stultz
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2017-06-21 18:48 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: fbdev helper locking rework and deferred setup
URL   : https://patchwork.freedesktop.org/series/26171/
State : success

== Summary ==

Series 26171v1 fbdev helper locking rework and deferred setup
https://patchwork.freedesktop.org/api/1.0/series/26171/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_busy:
        Subgroup basic-flip-default-b:
                fail       -> DMESG-FAIL (fi-skl-6700hq) fdo#101144 +1
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> DMESG-FAIL (fi-skl-6700hq) fdo#101154 +28
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                fail       -> DMESG-FAIL (fi-skl-6700hq) fdo#100461 +1
Test kms_sink_crc_basic:
                fail       -> DMESG-FAIL (fi-skl-6700hq) fdo#101519

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144
fdo#101154 https://bugs.freedesktop.org/show_bug.cgi?id=101154
fdo#100461 https://bugs.freedesktop.org/show_bug.cgi?id=100461
fdo#101519 https://bugs.freedesktop.org/show_bug.cgi?id=101519

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:439s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:430s
fi-bsw-n3050     total:278  pass:241  dwarn:1   dfail:0   fail:0   skip:36  time:541s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:506s
fi-byt-j1900     total:278  pass:252  dwarn:2   dfail:0   fail:0   skip:24  time:485s
fi-byt-n2820     total:278  pass:248  dwarn:2   dfail:0   fail:0   skip:28  time:487s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:591s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:439s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:410s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:417s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:494s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:472s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:463s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:568s
fi-kbl-r         total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time:577s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:462s
fi-skl-6700hq    total:278  pass:219  dwarn:5   dfail:29  fail:0   skip:24  time:386s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:466s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:477s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:436s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:543s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time:399s

0a9fd1712c87aac1b6065a58cb46582a6a117cee drm-tip: 2017y-06m-21d-15h-44m-13s UTC integration manifest
8aa5567 drm/hisilicon: Remove custom FB helper deferred setup
f6fef32 drm/exynos: Remove custom FB helper deferred setup
03feca4 drm/fb-helper: Support deferred setup
e25abb8 drm/fb-helper: Split dpms handling into legacy and atomic paths
4f7bba9 drm/fb-helper: Stop using mode_config.mutex for internals
e83aa4f drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy
c0881cb drm/fb-helper: Push locking into pan_display_atomic|legacy
f12a900 drm/fb-helper: Drop locking from the vsync wait ioctl code
4ebaeac drm/fb-helper: Push locking in fb_is_bound
79c4351 drm/fb-helper: Add top-level lock
b8da451 drm/i915: Drop FBDEV #ifdev in mst code
c293208 drm/fb-helper: Push down modeset lock into FB helpers

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5018/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/12] fbdev helper locking rework and deferred setup
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (12 preceding siblings ...)
  2017-06-21 18:48 ` ✓ Fi.CI.BAT: success for fbdev helper locking rework and " Patchwork
@ 2017-06-21 23:01 ` John Stultz
  2017-06-22 14:54 ` Liviu Dudau
  2017-06-23 14:09 ` ✓ Fi.CI.BAT: success for fbdev helper locking rework and deferred setup (rev2) Patchwork
  15 siblings, 0 replies; 25+ messages in thread
From: John Stultz @ 2017-06-21 23:01 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development, Thierry Reding, DRI Development

On Wed, Jun 21, 2017 at 11:28 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Hi all,
>
> This is Thierry's deferred fbdev setup series, with the locking rework almost
> entirely redone. The much wider scope is to get rid of drm_modeset_lock_all
> calls for atomic drivers and remove users of the fairly nasty
> mode_config->acquire_ctx hack, which breaks when doing multiple atomic commits.
>
> Testing&review very much appreciated, especially from people who care about the
> various fbdev emulation things and the deferred setup stuff.
>
> Thanks, Daniel
>
> Daniel Vetter (7):
>   drm/i915: Drop FBDEV #ifdev in mst code
>   drm/fb-helper: Push locking in fb_is_bound
>   drm/fb-helper: Drop locking from the vsync wait ioctl code
>   drm/fb-helper: Push locking into pan_display_atomic|legacy
>   drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy
>   drm/fb-helper: Stop using mode_config.mutex for internals
>   drm/fb-helper: Split dpms handling into legacy and atomic paths
>
> Thierry Reding (5):
>   drm/fb-helper: Push down modeset lock into FB helpers
>   drm/fb-helper: Add top-level lock
>   drm/fb-helper: Support deferred setup
>   drm/exynos: Remove custom FB helper deferred setup
>   drm/hisilicon: Remove custom FB helper deferred setup
>
>  drivers/gpu/drm/drm_fb_helper.c                 | 361 ++++++++++++++++++------
>  drivers/gpu/drm/drm_vblank.c                    |   2 +-
>  drivers/gpu/drm/exynos/exynos_drm_drv.c         |   6 +-
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c       |  26 +-
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  21 +-
>  drivers/gpu/drm/i915/intel_dp_mst.c             |  43 +--
>  drivers/gpu/drm/radeon/radeon_dp_mst.c          |   7 -
>  include/drm/drm_fb_helper.h                     |  42 ++-
>  8 files changed, 336 insertions(+), 172 deletions(-)

So in testing this patchset against 4.12-rc6, w/ HiKey (so I had a bit
of fuzz on one patch, and skipped the drm_vblank changes and the
entire exynos patch), it seems to work ok. I rebooted 10 times and
didn't see the initialization race where multiple fbs were created
that I could regularly trigger before.

Tested-by: John Stultz <john.stultz@linaro.org>

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 09/12] drm/fb-helper: Split dpms handling into legacy and atomic paths
  2017-06-21 18:28 ` [PATCH 09/12] drm/fb-helper: Split dpms handling into legacy and atomic paths Daniel Vetter
@ 2017-06-22 10:24   ` Peter Rosin
  0 siblings, 0 replies; 25+ messages in thread
From: Peter Rosin @ 2017-06-22 10:24 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding

On 2017-06-21 20:28, Daniel Vetter wrote:
> Like with panning and modesetting, and like with those, stick with
> simple drm_modeset_locking_all for the legacy path, and the full
> atomic dance for atomic drivers.
> 
> This means a bit more boilerplate since setting up the atomic state
> machinery is rather verbose, but then this is shared code for 30+
> drivers or so, so meh.
> 
> After this patch there's only the LUT/cmap path which is still using
> drm_modeset_lock_all for an atomic driver. But Peter is already
> locking into reworking that, so I'll leave that code as-is for now.
> 
> Cc: Peter Rosin <peda@axentia.se>

For the whole series (lightly)

Tested-by: Peter Rosin <peda@axentia.se>

Cheers,
peda
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/12] fbdev helper locking rework and deferred setup
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (13 preceding siblings ...)
  2017-06-21 23:01 ` [PATCH 00/12] " John Stultz
@ 2017-06-22 14:54 ` Liviu Dudau
  2017-06-23  7:38   ` Daniel Vetter
  2017-06-23 14:09 ` ✓ Fi.CI.BAT: success for fbdev helper locking rework and deferred setup (rev2) Patchwork
  15 siblings, 1 reply; 25+ messages in thread
From: Liviu Dudau @ 2017-06-22 14:54 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development, Thierry Reding, DRI Development

On Wed, Jun 21, 2017 at 08:28:03PM +0200, Daniel Vetter wrote:
> Hi all,
> 
> This is Thierry's deferred fbdev setup series, with the locking rework almost
> entirely redone. The much wider scope is to get rid of drm_modeset_lock_all
> calls for atomic drivers and remove users of the fairly nasty
> mode_config->acquire_ctx hack, which breaks when doing multiple atomic commits.
> 
> Testing&review very much appreciated, especially from people who care about the
> various fbdev emulation things and the deferred setup stuff.

Tested on my Juno dev board on a bit of a convoluted setup: the dev board has
built into the SoC 2x HDLCD instances and I also have an FPGA daughter board
with Mali DP 650 running again on a 2x configuration. On boot I'm getting this
warning:

juno-r0-ld login: [  241.986785] INFO: task kworker/3:1:80 blocked for more than 120 seconds.                                                                                                         
[  241.993652]       Not tainted 4.12.0-rc5-01275-g1e2237a19156 #2                                                                                                                                    
[  241.999689] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.                                                                                                              
[  242.007660] kworker/3:1     D    0    80      2 0x00000000                                                                                                                                         
[  242.013340] Workqueue: events output_poll_execute [drm_kms_helper]                                                                                                                                 
[  242.019637] Call trace:                                                                                                                                                                            
[  242.022132] [<ffff000008085798>] __switch_to+0x98/0xb0                                                                                                                                             
[  242.027373] [<ffff00000886b3d4>] __schedule+0x19c/0x5e8                                                                                                                                            
[  242.032699] [<ffff00000886b858>] schedule+0x38/0xa0                                                                                                                                                
[  242.037672] [<ffff00000886bd38>] schedule_preempt_disabled+0x20/0x38                                                                                                                               
[  242.044144] [<ffff00000886c968>] __mutex_lock.isra.0+0x140/0x530                                                                                                                                   
[  242.050262] [<ffff00000886cd68>] __mutex_lock_slowpath+0x10/0x18                                                                                                                                   
[  242.056379] [<ffff00000886cda0>] mutex_lock+0x30/0x38                                                                                                                                              
[  242.061597] [<ffff000000929660>] drm_fb_helper_hotplug_event.part.22+0x20/0x100 [drm_kms_helper]                                                                                                   
[  242.070611] [<ffff000000929764>] drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]                                                                                                            
[  242.078828] [<ffff00000092a360>] drm_fbdev_cma_hotplug_event+0x10/0x20 [drm_kms_helper]                                                                                                            
[  242.086984] [<ffff000000b44494>] hdlcd_fb_output_poll_changed+0x14/0x20 [hdlcd]                                                                                                                    
[  242.094495] [<ffff000000919e90>] drm_kms_helper_hotplug_event+0x28/0x38 [drm_kms_helper]                                                                                                           
[  242.102801] [<ffff00000091a090>] output_poll_execute+0x1a0/0x1f0 [drm_kms_helper]                                                                                                                  
[  242.110424] [<ffff0000080d7fd4>] process_one_work+0x1d4/0x330                                                                                                                                      
[  242.116280] [<ffff0000080d8178>] worker_thread+0x48/0x468                                                                                                                                          
[  242.121781] [<ffff0000080ddf64>] kthread+0x12c/0x130                                                                                                                                               
[  242.126839] [<ffff000008082ec0>] ret_from_fork+0x10/0x50                                                                                                                                           
[  242.132252] INFO: task kworker/5:1:82 blocked for more than 120 seconds.                                                                                                                           
[  242.139074]       Not tainted 4.12.0-rc5-01275-g1e2237a19156 #2                                                                                                                                    
[  242.145099] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.                                                                                                              
[  242.153064] kworker/5:1     D    0    82      2 0x00000000                                                                                                                                         
[  242.158724] Workqueue: events output_poll_execute [drm_kms_helper]                                                                                                                                 
[  242.165019] Call trace:                                                                                                                                                                            
[  242.167519] [<ffff000008085798>] __switch_to+0x98/0xb0                                                                                                                                             
[  242.172755] [<ffff00000886b3d4>] __schedule+0x19c/0x5e8                                                                                                                                            
[  242.178079] [<ffff00000886b858>] schedule+0x38/0xa0                                                                                                                                                
[  242.183051] [<ffff00000886bd38>] schedule_preempt_disabled+0x20/0x38                                                                                                                               
[  242.189520] [<ffff00000886c968>] __mutex_lock.isra.0+0x140/0x530                                                                                                                                   
[  242.195639] [<ffff00000886cd68>] __mutex_lock_slowpath+0x10/0x18                                                                                                                                   
[  242.201756] [<ffff00000886cda0>] mutex_lock+0x30/0x38                                                                                                                                              
[  242.206972] [<ffff000000929660>] drm_fb_helper_hotplug_event.part.22+0x20/0x100 [drm_kms_helper]                                                                                                   
[  242.215984] [<ffff000000929764>] drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]                                                                                                            
[  242.224201] [<ffff00000092a360>] drm_fbdev_cma_hotplug_event+0x10/0x20 [drm_kms_helper]                                                                                                            
[  242.232367] [<ffff000000b543a4>] malidp_output_poll_changed+0x14/0x20 [mali_dp]                                                                                                                    
[  242.239880] [<ffff000000919e90>] drm_kms_helper_hotplug_event+0x28/0x38 [drm_kms_helper]                                                                                                           
[  242.248188] [<ffff00000091a090>] output_poll_execute+0x1a0/0x1f0 [drm_kms_helper]                                                                                                                  
[  242.255808] [<ffff0000080d7fd4>] process_one_work+0x1d4/0x330                                                                                                                                      
[  242.261664] [<ffff0000080d8178>] worker_thread+0x48/0x468                                                                                                                                          
[  242.267196] [<ffff0000080ddf64>] kthread+0x12c/0x130                                                                                                                                               
[  242.272312] [<ffff000008082ec0>] ret_from_fork+0x10/0x50  

Each hardware type has only one instance of each driver being connected to an
output, HDLCD is connected to an HDMI monitor that is not "active" (i.e.
input is switched to DP connection), while the Mali DP instance is connected to
a monitor.

Suggestions on where to look next are welcome.

Best regards,
Liviu

> 
> Thanks, Daniel
> 
> Daniel Vetter (7):
>   drm/i915: Drop FBDEV #ifdev in mst code
>   drm/fb-helper: Push locking in fb_is_bound
>   drm/fb-helper: Drop locking from the vsync wait ioctl code
>   drm/fb-helper: Push locking into pan_display_atomic|legacy
>   drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy
>   drm/fb-helper: Stop using mode_config.mutex for internals
>   drm/fb-helper: Split dpms handling into legacy and atomic paths
> 
> Thierry Reding (5):
>   drm/fb-helper: Push down modeset lock into FB helpers
>   drm/fb-helper: Add top-level lock
>   drm/fb-helper: Support deferred setup
>   drm/exynos: Remove custom FB helper deferred setup
>   drm/hisilicon: Remove custom FB helper deferred setup
> 
>  drivers/gpu/drm/drm_fb_helper.c                 | 361 ++++++++++++++++++------
>  drivers/gpu/drm/drm_vblank.c                    |   2 +-
>  drivers/gpu/drm/exynos/exynos_drm_drv.c         |   6 +-
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c       |  26 +-
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  21 +-
>  drivers/gpu/drm/i915/intel_dp_mst.c             |  43 +--
>  drivers/gpu/drm/radeon/radeon_dp_mst.c          |   7 -
>  include/drm/drm_fb_helper.h                     |  42 ++-
>  8 files changed, 336 insertions(+), 172 deletions(-)
> 
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
   _
 _|_|_
 ('_')
 (⊃  )⊃
 |_|_|
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/12] fbdev helper locking rework and deferred setup
  2017-06-22 14:54 ` Liviu Dudau
@ 2017-06-23  7:38   ` Daniel Vetter
  2017-06-23 12:34     ` Liviu Dudau
  0 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2017-06-23  7:38 UTC (permalink / raw)
  To: Liviu Dudau; +Cc: Intel Graphics Development, Thierry Reding, DRI Development

On Thu, Jun 22, 2017 at 4:54 PM, Liviu Dudau <liviu@dudau.co.uk> wrote:
> On Wed, Jun 21, 2017 at 08:28:03PM +0200, Daniel Vetter wrote:
>> Hi all,
>>
>> This is Thierry's deferred fbdev setup series, with the locking rework almost
>> entirely redone. The much wider scope is to get rid of drm_modeset_lock_all
>> calls for atomic drivers and remove users of the fairly nasty
>> mode_config->acquire_ctx hack, which breaks when doing multiple atomic commits.
>>
>> Testing&review very much appreciated, especially from people who care about the
>> various fbdev emulation things and the deferred setup stuff.
>
> Tested on my Juno dev board on a bit of a convoluted setup: the dev board has
> built into the SoC 2x HDLCD instances and I also have an FPGA daughter board
> with Mali DP 650 running again on a 2x configuration. On boot I'm getting this
> warning:
>
> juno-r0-ld login: [  241.986785] INFO: task kworker/3:1:80 blocked for more than 120 seconds.
> [  241.993652]       Not tainted 4.12.0-rc5-01275-g1e2237a19156 #2
> [  241.999689] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [  242.007660] kworker/3:1     D    0    80      2 0x00000000
> [  242.013340] Workqueue: events output_poll_execute [drm_kms_helper]
> [  242.019637] Call trace:
> [  242.022132] [<ffff000008085798>] __switch_to+0x98/0xb0
> [  242.027373] [<ffff00000886b3d4>] __schedule+0x19c/0x5e8
> [  242.032699] [<ffff00000886b858>] schedule+0x38/0xa0
> [  242.037672] [<ffff00000886bd38>] schedule_preempt_disabled+0x20/0x38
> [  242.044144] [<ffff00000886c968>] __mutex_lock.isra.0+0x140/0x530
> [  242.050262] [<ffff00000886cd68>] __mutex_lock_slowpath+0x10/0x18
> [  242.056379] [<ffff00000886cda0>] mutex_lock+0x30/0x38
> [  242.061597] [<ffff000000929660>] drm_fb_helper_hotplug_event.part.22+0x20/0x100 [drm_kms_helper]
> [  242.070611] [<ffff000000929764>] drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]
> [  242.078828] [<ffff00000092a360>] drm_fbdev_cma_hotplug_event+0x10/0x20 [drm_kms_helper]
> [  242.086984] [<ffff000000b44494>] hdlcd_fb_output_poll_changed+0x14/0x20 [hdlcd]
> [  242.094495] [<ffff000000919e90>] drm_kms_helper_hotplug_event+0x28/0x38 [drm_kms_helper]
> [  242.102801] [<ffff00000091a090>] output_poll_execute+0x1a0/0x1f0 [drm_kms_helper]
> [  242.110424] [<ffff0000080d7fd4>] process_one_work+0x1d4/0x330
> [  242.116280] [<ffff0000080d8178>] worker_thread+0x48/0x468
> [  242.121781] [<ffff0000080ddf64>] kthread+0x12c/0x130
> [  242.126839] [<ffff000008082ec0>] ret_from_fork+0x10/0x50
> [  242.132252] INFO: task kworker/5:1:82 blocked for more than 120 seconds.
> [  242.139074]       Not tainted 4.12.0-rc5-01275-g1e2237a19156 #2
> [  242.145099] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [  242.153064] kworker/5:1     D    0    82      2 0x00000000
> [  242.158724] Workqueue: events output_poll_execute [drm_kms_helper]
> [  242.165019] Call trace:
> [  242.167519] [<ffff000008085798>] __switch_to+0x98/0xb0
> [  242.172755] [<ffff00000886b3d4>] __schedule+0x19c/0x5e8
> [  242.178079] [<ffff00000886b858>] schedule+0x38/0xa0
> [  242.183051] [<ffff00000886bd38>] schedule_preempt_disabled+0x20/0x38
> [  242.189520] [<ffff00000886c968>] __mutex_lock.isra.0+0x140/0x530
> [  242.195639] [<ffff00000886cd68>] __mutex_lock_slowpath+0x10/0x18
> [  242.201756] [<ffff00000886cda0>] mutex_lock+0x30/0x38
> [  242.206972] [<ffff000000929660>] drm_fb_helper_hotplug_event.part.22+0x20/0x100 [drm_kms_helper]
> [  242.215984] [<ffff000000929764>] drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]
> [  242.224201] [<ffff00000092a360>] drm_fbdev_cma_hotplug_event+0x10/0x20 [drm_kms_helper]
> [  242.232367] [<ffff000000b543a4>] malidp_output_poll_changed+0x14/0x20 [mali_dp]
> [  242.239880] [<ffff000000919e90>] drm_kms_helper_hotplug_event+0x28/0x38 [drm_kms_helper]
> [  242.248188] [<ffff00000091a090>] output_poll_execute+0x1a0/0x1f0 [drm_kms_helper]
> [  242.255808] [<ffff0000080d7fd4>] process_one_work+0x1d4/0x330
> [  242.261664] [<ffff0000080d8178>] worker_thread+0x48/0x468
> [  242.267196] [<ffff0000080ddf64>] kthread+0x12c/0x130
> [  242.272312] [<ffff000008082ec0>] ret_from_fork+0x10/0x50
>
> Each hardware type has only one instance of each driver being connected to an
> output, HDLCD is connected to an HDMI monitor that is not "active" (i.e.
> input is switched to DP connection), while the Mali DP instance is connected to
> a monitor.
>
> Suggestions on where to look next are welcome.

I'm betting I've fumbled an unlock path somewhere and now your stuck
trying to get a lock you can't get. Can you pls recompile with lockdep
enabled and repro? That will directly tell us where and what went
wrong.

Thanks for testing.
-Daniel

>
> Best regards,
> Liviu
>
>>
>> Thanks, Daniel
>>
>> Daniel Vetter (7):
>>   drm/i915: Drop FBDEV #ifdev in mst code
>>   drm/fb-helper: Push locking in fb_is_bound
>>   drm/fb-helper: Drop locking from the vsync wait ioctl code
>>   drm/fb-helper: Push locking into pan_display_atomic|legacy
>>   drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy
>>   drm/fb-helper: Stop using mode_config.mutex for internals
>>   drm/fb-helper: Split dpms handling into legacy and atomic paths
>>
>> Thierry Reding (5):
>>   drm/fb-helper: Push down modeset lock into FB helpers
>>   drm/fb-helper: Add top-level lock
>>   drm/fb-helper: Support deferred setup
>>   drm/exynos: Remove custom FB helper deferred setup
>>   drm/hisilicon: Remove custom FB helper deferred setup
>>
>>  drivers/gpu/drm/drm_fb_helper.c                 | 361 ++++++++++++++++++------
>>  drivers/gpu/drm/drm_vblank.c                    |   2 +-
>>  drivers/gpu/drm/exynos/exynos_drm_drv.c         |   6 +-
>>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c       |  26 +-
>>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  21 +-
>>  drivers/gpu/drm/i915/intel_dp_mst.c             |  43 +--
>>  drivers/gpu/drm/radeon/radeon_dp_mst.c          |   7 -
>>  include/drm/drm_fb_helper.h                     |  42 ++-
>>  8 files changed, 336 insertions(+), 172 deletions(-)
>>
>> --
>> 2.11.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
>    _
>  _|_|_
>  ('_')
>  (⊃  )⊃
>  |_|_|



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/12] fbdev helper locking rework and deferred setup
  2017-06-23  7:38   ` Daniel Vetter
@ 2017-06-23 12:34     ` Liviu Dudau
  0 siblings, 0 replies; 25+ messages in thread
From: Liviu Dudau @ 2017-06-23 12:34 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development, Thierry Reding, DRI Development

On Fri, Jun 23, 2017 at 09:38:49AM +0200, Daniel Vetter wrote:
> On Thu, Jun 22, 2017 at 4:54 PM, Liviu Dudau <liviu@dudau.co.uk> wrote:
> > On Wed, Jun 21, 2017 at 08:28:03PM +0200, Daniel Vetter wrote:
> >> Hi all,
> >>
> >> This is Thierry's deferred fbdev setup series, with the locking rework almost
> >> entirely redone. The much wider scope is to get rid of drm_modeset_lock_all
> >> calls for atomic drivers and remove users of the fairly nasty
> >> mode_config->acquire_ctx hack, which breaks when doing multiple atomic commits.
> >>
> >> Testing&review very much appreciated, especially from people who care about the
> >> various fbdev emulation things and the deferred setup stuff.
> >
> > Tested on my Juno dev board on a bit of a convoluted setup: the dev board has
> > built into the SoC 2x HDLCD instances and I also have an FPGA daughter board
> > with Mali DP 650 running again on a 2x configuration. On boot I'm getting this
> > warning:
> >
> > juno-r0-ld login: [  241.986785] INFO: task kworker/3:1:80 blocked for more than 120 seconds.
> > [  241.993652]       Not tainted 4.12.0-rc5-01275-g1e2237a19156 #2
> > [  241.999689] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > [  242.007660] kworker/3:1     D    0    80      2 0x00000000
> > [  242.013340] Workqueue: events output_poll_execute [drm_kms_helper]
> > [  242.019637] Call trace:
> > [  242.022132] [<ffff000008085798>] __switch_to+0x98/0xb0
> > [  242.027373] [<ffff00000886b3d4>] __schedule+0x19c/0x5e8
> > [  242.032699] [<ffff00000886b858>] schedule+0x38/0xa0
> > [  242.037672] [<ffff00000886bd38>] schedule_preempt_disabled+0x20/0x38
> > [  242.044144] [<ffff00000886c968>] __mutex_lock.isra.0+0x140/0x530
> > [  242.050262] [<ffff00000886cd68>] __mutex_lock_slowpath+0x10/0x18
> > [  242.056379] [<ffff00000886cda0>] mutex_lock+0x30/0x38
> > [  242.061597] [<ffff000000929660>] drm_fb_helper_hotplug_event.part.22+0x20/0x100 [drm_kms_helper]
> > [  242.070611] [<ffff000000929764>] drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]
> > [  242.078828] [<ffff00000092a360>] drm_fbdev_cma_hotplug_event+0x10/0x20 [drm_kms_helper]
> > [  242.086984] [<ffff000000b44494>] hdlcd_fb_output_poll_changed+0x14/0x20 [hdlcd]
> > [  242.094495] [<ffff000000919e90>] drm_kms_helper_hotplug_event+0x28/0x38 [drm_kms_helper]
> > [  242.102801] [<ffff00000091a090>] output_poll_execute+0x1a0/0x1f0 [drm_kms_helper]
> > [  242.110424] [<ffff0000080d7fd4>] process_one_work+0x1d4/0x330
> > [  242.116280] [<ffff0000080d8178>] worker_thread+0x48/0x468
> > [  242.121781] [<ffff0000080ddf64>] kthread+0x12c/0x130
> > [  242.126839] [<ffff000008082ec0>] ret_from_fork+0x10/0x50
> > [  242.132252] INFO: task kworker/5:1:82 blocked for more than 120 seconds.
> > [  242.139074]       Not tainted 4.12.0-rc5-01275-g1e2237a19156 #2
> > [  242.145099] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > [  242.153064] kworker/5:1     D    0    82      2 0x00000000
> > [  242.158724] Workqueue: events output_poll_execute [drm_kms_helper]
> > [  242.165019] Call trace:
> > [  242.167519] [<ffff000008085798>] __switch_to+0x98/0xb0
> > [  242.172755] [<ffff00000886b3d4>] __schedule+0x19c/0x5e8
> > [  242.178079] [<ffff00000886b858>] schedule+0x38/0xa0
> > [  242.183051] [<ffff00000886bd38>] schedule_preempt_disabled+0x20/0x38
> > [  242.189520] [<ffff00000886c968>] __mutex_lock.isra.0+0x140/0x530
> > [  242.195639] [<ffff00000886cd68>] __mutex_lock_slowpath+0x10/0x18
> > [  242.201756] [<ffff00000886cda0>] mutex_lock+0x30/0x38
> > [  242.206972] [<ffff000000929660>] drm_fb_helper_hotplug_event.part.22+0x20/0x100 [drm_kms_helper]
> > [  242.215984] [<ffff000000929764>] drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]
> > [  242.224201] [<ffff00000092a360>] drm_fbdev_cma_hotplug_event+0x10/0x20 [drm_kms_helper]
> > [  242.232367] [<ffff000000b543a4>] malidp_output_poll_changed+0x14/0x20 [mali_dp]
> > [  242.239880] [<ffff000000919e90>] drm_kms_helper_hotplug_event+0x28/0x38 [drm_kms_helper]
> > [  242.248188] [<ffff00000091a090>] output_poll_execute+0x1a0/0x1f0 [drm_kms_helper]
> > [  242.255808] [<ffff0000080d7fd4>] process_one_work+0x1d4/0x330
> > [  242.261664] [<ffff0000080d8178>] worker_thread+0x48/0x468
> > [  242.267196] [<ffff0000080ddf64>] kthread+0x12c/0x130
> > [  242.272312] [<ffff000008082ec0>] ret_from_fork+0x10/0x50
> >
> > Each hardware type has only one instance of each driver being connected to an
> > output, HDLCD is connected to an HDMI monitor that is not "active" (i.e.
> > input is switched to DP connection), while the Mali DP instance is connected to
> > a monitor.
> >
> > Suggestions on where to look next are welcome.
> 
> I'm betting I've fumbled an unlock path somewhere and now your stuck
> trying to get a lock you can't get. Can you pls recompile with lockdep
> enabled and repro? That will directly tell us where and what went
> wrong.

This is what I've got with lockdep debugging enabled. Not sure it is the same
thing as the previous WARN, the stack trace doesn't match the one above (but I still
get that one after 120s).

[   16.536405] [drm] found ARM HDLCD version r0p0                                                                                                                                                     
[   16.670859] tda998x 0-0071: found TDA19988                                                                                                                                                         
[   16.677173] hdlcd 7ff50000.hdlcd: bound 0-0071 (ops tda998x_ops [tda998x])                                                                                                                         
[   16.686092] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).                                                                                                                            
[   16.692783] [drm] No driver support for vblank timestamp query.                                                                                                                                    
[   16.699518] [drm] No outputs connected, deferring setup                                                                                                                                            
[   16.704844]                                                                                                                                                                                        
[   16.706322] =====================================                                                                                                                                                  
[   16.710981] WARNING: bad unlock balance detected!                                                                                                                                                  
[   16.715643] 4.12.0-rc5-01275-g1e2237a19156 #3 Not tainted                                                                                                                                          
[   16.720992] -------------------------------------                                                                                                                                                  
[   16.725652] systemd-udevd/176 is trying to release lock (&dev->mode_config.mutex) at:                                                                                                              
[   16.733466] [<ffff000000a3363c>] __drm_fb_helper_initial_config+0x11c/0x570 [drm_kms_helper]                                                                                                       
[   16.741830] but there are no more locks to release!                                                                                                                                                
[   16.746661]                                                                                                                                                                                        
[   16.746661] other info that might help us debug this:                                                                                                                                              
[   16.753135] 4 locks held by systemd-udevd/176:                                                                                                                                                     
[   16.757535]  #0:  (&dev->mutex){......}, at: [<ffff00000854a134>] __driver_attach+0x4c/0xc8                                                                                                        
[   16.774279]  #1:  (&dev->mutex){......}, at: [<ffff00000854a144>] __driver_attach+0x5c/0xc8                                                                                                        
[   16.782571]  #2:  (component_mutex){+.+.+.}, at: [<ffff000008544044>] component_master_add_with_match+0x84/0xf8                                                                                    
[   16.792585]  #3:  (&helper->lock){+.+.+.}, at: [<ffff000000a33ac4>] drm_fb_helper_initial_config+0x34/0x68 [drm_kms_helper]                                                                        
[   16.803669]                                                                                                                                                                                        
[   16.803669] stack backtrace:                                                                                                                                                                       
[   16.807992] CPU: 2 PID: 176 Comm: systemd-udevd Not tainted 4.12.0-rc5-01275-g1e2237a19156 #3                                                                                                      
[   16.816442] Hardware name: ARM Juno development board (r0) (DT)                                                                                                                                    
[   16.822306] Call trace:                                                                                                                                                                            
[   16.824733] [<ffff000008088fc8>] dump_backtrace+0x0/0x238                                                                                                                                          
[   16.830083] [<ffff0000080892cc>] show_stack+0x14/0x20                                                                                                                                              
[   16.835091] [<ffff0000083e7be0>] dump_stack+0xbc/0xf4                                                                                                                                              
[   16.840097] [<ffff00000810d260>] print_unlock_imbalance_bug+0xe8/0xf0                                                                                                                              
[   16.846481] [<ffff00000811170c>] lock_release+0x174/0x368                                                                                                                                          
[   16.851832] [<ffff0000088f6eb4>] __mutex_unlock_slowpath+0x44/0x2b0                                                                                                                                
[   16.858041] [<ffff0000088f7130>] mutex_unlock+0x10/0x18                                                                                                                                            
[   16.863256] [<ffff000000a3363c>] __drm_fb_helper_initial_config+0x11c/0x570 [drm_kms_helper]                                                                                                       
[   16.871661] [<ffff000000a33ad0>] drm_fb_helper_initial_config+0x40/0x68 [drm_kms_helper]                                                                                                           
[   16.871699] [<ffff000000a34510>] drm_fbdev_cma_init_with_funcs+0x88/0x158 [drm_kms_helper]                                                                                                         
[   16.871736] [<ffff000000a345f0>] drm_fbdev_cma_init+0x10/0x20 [drm_kms_helper]                                                                                                                     
[   16.871748] [<ffff000000b5e708>] hdlcd_drm_bind+0x1f0/0x488 [hdlcd]                                                                                                                                
[   16.871752] [<ffff000008543e00>] try_to_bring_up_master+0x178/0x1d8                                                                                                                                
[   16.871756] [<ffff000008544064>] component_master_add_with_match+0xa4/0xf8                                                                                                                         
[   16.871765] [<ffff000000b5e4f0>] hdlcd_probe+0x50/0x78 [hdlcd]                                                                                                                                     
[   16.871770] [<ffff00000854bda8>] platform_drv_probe+0x58/0xb8                                                                                                                                      
[   16.871775] [<ffff00000854a03c>] driver_probe_device+0x224/0x2d0                                                                                                                                   
[   16.871779] [<ffff00000854a1ac>] __driver_attach+0xc4/0xc8                                                                                                                                         
[   16.871783] [<ffff00000854806c>] bus_for_each_dev+0x4c/0x98                                                                                                                                        
[   16.871787] [<ffff000008549948>] driver_attach+0x20/0x28                                                                                                                                           
[   16.871791] [<ffff000008549518>] bus_add_driver+0x1c0/0x230                                                                                                                                        
[   16.871794] [<ffff00000854ab60>] driver_register+0x60/0xf8                                                                                                                                         
[   16.871799] [<ffff00000854bcf8>] __platform_driver_register+0x40/0x48                                                                                                                              
[   16.871807] [<ffff000000b6a018>] hdlcd_platform_driver_init+0x18/0x1000 [hdlcd]                                                                                                                    
[   16.871812] [<ffff000008083140>] do_one_initcall+0x38/0x128                                                                                                                                        
[   16.871818] [<ffff000008195c1c>] do_init_module+0x58/0x1bc                                                                                                                                         
[   16.871823] [<ffff00000815093c>] load_module+0x1e94/0x2168                                                                                                                                         
[   16.871827] [<ffff000008150eb0>] SyS_finit_module+0xa8/0xc0                                                                                                                                        
[   16.871831] [<ffff000008082fcc>] __sys_trace_return+0x0/0x4                                                                                                                                        
[   16.872439] [drm] Initialized hdlcd 1.0.0 20151021 for 7ff50000.hdlcd on minor 0                                                                                                                   
[   16.872781] mali-dp 6f200000.malidp: assigned reserved memory node framebuffer@60000000                                                                                                            
[   16.872859] [drm] found ARM Mali-DP650 version r0p0                                                                                                                                                
[   17.038821] tda998x 1-0070: found TDA19988                                                                                                                                                         
[   17.056417] mali-dp 6f200000.malidp: bound 1-0070 (ops tda998x_ops [tda998x])                                                                                                                      
[   17.063827] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).                                                                                                                            
[   17.070435] [drm] No driver support for vblank timestamp query.                                                                                                                                    
[   17.079290] [drm] No outputs connected, deferring setup                                                                                                                                            
[   17.085265] [drm] Initialized mali-dp 1.0.0 20160106 for 6f200000.malidp on minor 1                                                                                                                
[   17.092701] random: crng init done                                                                                                                                                                 
[   17.096720] [drm] found ARM HDLCD version r0p0                                                                                                                                                     
[   17.227312] tda998x 0-0070: found TDA19988                                                                                                                                                         
[   17.233258] hdlcd 7ff60000.hdlcd: bound 0-0070 (ops tda998x_ops [tda998x])                                                                                                                         
[   17.240152] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).                                                                                                                            
[   17.246781] [drm] No driver support for vblank timestamp query.                                                                                                                                    
[   17.355359] Console: switching to colour frame buffer device 240x75                                                                                                                                
[   17.417368] hdlcd 7ff60000.hdlcd: fb0:  frame buffer device                                                                                                                                        
[   17.436616] [drm] Initialized hdlcd 1.0.0 20151021 for 7ff60000.hdlcd on minor 2

Best regards,
Liviu

> 
> Thanks for testing.
> -Daniel
> 
> >
> > Best regards,
> > Liviu
> >
> >>
> >> Thanks, Daniel
> >>
> >> Daniel Vetter (7):
> >>   drm/i915: Drop FBDEV #ifdev in mst code
> >>   drm/fb-helper: Push locking in fb_is_bound
> >>   drm/fb-helper: Drop locking from the vsync wait ioctl code
> >>   drm/fb-helper: Push locking into pan_display_atomic|legacy
> >>   drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy
> >>   drm/fb-helper: Stop using mode_config.mutex for internals
> >>   drm/fb-helper: Split dpms handling into legacy and atomic paths
> >>
> >> Thierry Reding (5):
> >>   drm/fb-helper: Push down modeset lock into FB helpers
> >>   drm/fb-helper: Add top-level lock
> >>   drm/fb-helper: Support deferred setup
> >>   drm/exynos: Remove custom FB helper deferred setup
> >>   drm/hisilicon: Remove custom FB helper deferred setup
> >>
> >>  drivers/gpu/drm/drm_fb_helper.c                 | 361 ++++++++++++++++++------
> >>  drivers/gpu/drm/drm_vblank.c                    |   2 +-
> >>  drivers/gpu/drm/exynos/exynos_drm_drv.c         |   6 +-
> >>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c       |  26 +-
> >>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  21 +-
> >>  drivers/gpu/drm/i915/intel_dp_mst.c             |  43 +--
> >>  drivers/gpu/drm/radeon/radeon_dp_mst.c          |   7 -
> >>  include/drm/drm_fb_helper.h                     |  42 ++-
> >>  8 files changed, 336 insertions(+), 172 deletions(-)
> >>
> >> --
> >> 2.11.0
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> >    _
> >  _|_|_
> >  ('_')
> >  (⊃  )⊃
> >  |_|_|
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
   _
 _|_|_
 ('_')
 (⊃  )⊃
 |_|_|
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] drm/fb-helper: Support deferred setup
  2017-06-21 18:28 ` [PATCH 10/12] drm/fb-helper: Support deferred setup Daniel Vetter
@ 2017-06-23 13:31   ` Daniel Vetter
  0 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-23 13:31 UTC (permalink / raw)
  To: DRI Development
  Cc: Liviu Dudau, Intel Graphics Development, Thierry Reding,
	John Stultz, Daniel Vetter

From: Thierry Reding <treding@nvidia.com>

FB helper code falls back to a 1024x768 mode if no outputs are connected
or don't report back any modes upon initialization. This can be annoying
because outputs that are added to FB helper later on can't be used with
FB helper if they don't support a matching mode.

The fallback is in place because VGA connectors can happen to report an
unknown connection status even when they are in fact connected.

Some drivers have custom solutions in place to defer FB helper setup
until at least one output is connected. But the logic behind these
solutions is always the same and there is nothing driver-specific about
it, so a better alterative is to fix the FB helper core and add support
for all drivers automatically.

This patch adds support for deferred FB helper setup. It checks all the
connectors for their connection status, and if all of them report to be
disconnected marks the FB helper as needing deferred setup. Whet setup
is deferred, the FB helper core will automatically retry setup after a
hotplug event, and it will keep trying until it succeeds.

v2: Rebase onto my entirely reworked fbdev helper locking. One big
difference is that this version again drops&reacquires the fbdev lock
(which is now fb_helper->lock, but before this patch series it was
mode_config->mutex), because register_framebuffer must be able to
recurse back into fbdev helper code for the initial screen setup.

v3: __drm_fb_helper_initial_config must hold fb_helper->lock upon
return, I've fumbled that in the deferred setup case (Liviu).

Cc: Liviu Dudau <liviu@dudau.co.uk>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thierry Reding <treding@nvidia.com> (v1)
Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Thierry Reding <treding@nvidia.com> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fb_helper.c | 110 ++++++++++++++++++++++++++++++----------
 include/drm/drm_fb_helper.h     |  23 +++++++++
 2 files changed, 106 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index e3d033df068f..834c7e7d7efc 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -521,6 +521,9 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 	if (!drm_fbdev_emulation)
 		return -ENODEV;
 
+	if (READ_ONCE(fb_helper->deferred_setup))
+		return 0;
+
 	mutex_lock(&fb_helper->lock);
 	ret = restore_fbdev_mode(fb_helper);
 
@@ -1693,6 +1696,23 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 }
 EXPORT_SYMBOL(drm_fb_helper_pan_display);
 
+static bool drm_fb_helper_maybe_connected(struct drm_fb_helper *helper)
+{
+	bool connected = false;
+	unsigned int i;
+
+	for (i = 0; i < helper->connector_count; i++) {
+		struct drm_fb_helper_connector *fb = helper->connector_info[i];
+
+		if (fb->connector->status != connector_status_disconnected) {
+			connected = true;
+			break;
+		}
+	}
+
+	return connected;
+}
+
 /*
  * Allocates the backing storage and sets up the fbdev info structure through
  * the ->fb_probe callback and then registers the fbdev and sets up the panic
@@ -2352,8 +2372,6 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 	int i;
 
 	DRM_DEBUG_KMS("\n");
-	if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
-		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
 
 	/* prevent concurrent modification of connector_count by hotplug */
 	lockdep_assert_held(&fb_helper->lock);
@@ -2431,6 +2449,60 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 	kfree(enabled);
 }
 
+/* Note: Drops&reacquires fb_helper->lock */
+static int __drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
+					  int bpp_sel)
+{
+	struct drm_device *dev = fb_helper->dev;
+	struct fb_info *info;
+	unsigned int width, height;
+	int ret;
+
+	width = dev->mode_config.max_width;
+	height = dev->mode_config.max_height;
+
+	if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
+		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
+
+	/*
+	 * If everything's disconnected, there's no use in attempting to set
+	 * up fbdev.
+	 */
+	if (!drm_fb_helper_maybe_connected(fb_helper)) {
+		DRM_INFO("No outputs connected, deferring setup\n");
+		fb_helper->preferred_bpp = bpp_sel;
+		fb_helper->deferred_setup = true;
+		return 0;
+	}
+
+	drm_setup_crtcs(fb_helper, width, height);
+	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
+	if (ret) {
+		mutex_unlock(&fb_helper->lock);
+		return ret;
+	}
+
+	fb_helper->deferred_setup = false;
+	mutex_unlock(&fb_helper->lock);
+
+	info = fb_helper->fbdev;
+	info->var.pixclock = 0;
+	ret = register_framebuffer(info);
+	if (ret < 0)
+		return ret;
+
+	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
+		 info->node, info->fix.id);
+
+	mutex_lock(&kernel_fb_helper_lock);
+	if (list_empty(&kernel_fb_helper_list))
+		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
+
+	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
+
+	return 0;
+}
+
 /**
  * drm_fb_helper_initial_config - setup a sane initial connector configuration
  * @fb_helper: fb_helper device struct
@@ -2475,39 +2547,16 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
  */
 int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
 {
-	struct drm_device *dev = fb_helper->dev;
-	struct fb_info *info;
 	int ret;
 
 	if (!drm_fbdev_emulation)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
-	drm_setup_crtcs(fb_helper,
-			dev->mode_config.max_width,
-			dev->mode_config.max_height);
-	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
-	mutex_unlock(&fb_helper->lock);
-	if (ret)
-		return ret;
-
-	info = fb_helper->fbdev;
-	info->var.pixclock = 0;
-	ret = register_framebuffer(info);
-	if (ret < 0)
-		return ret;
-
-	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
-		 info->node, info->fix.id);
-
-	mutex_lock(&kernel_fb_helper_lock);
-	if (list_empty(&kernel_fb_helper_list))
-		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
-
-	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
+	ret = __drm_fb_helper_initial_config(fb_helper, bpp_sel);
 	mutex_unlock(&kernel_fb_helper_lock);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_initial_config);
 
@@ -2540,6 +2589,13 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
+	if (fb_helper->deferred_setup) {
+		err = __drm_fb_helper_initial_config(fb_helper,
+						     fb_helper->preferred_bpp);
+		mutex_unlock(&fb_helper->lock);
+		return err;
+	}
+
 	if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
 		fb_helper->delayed_hotplug = true;
 		mutex_unlock(&fb_helper->lock);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index ea170b96e88d..a5ea6ffdfecc 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -232,6 +232,29 @@ struct drm_fb_helper {
 	 * needs to be reprobe when fbdev is in control again.
 	 */
 	bool delayed_hotplug;
+
+	/**
+	 * @deferred_setup:
+	 *
+	 * If no outputs are connected (disconnected or unknown) the FB helper
+	 * code will defer setup until at least one of the outputs shows up.
+	 * This field keeps track of the status so that setup can be retried
+	 * at every hotplug event until it succeeds eventually.
+	 *
+	 * Protected by @lock.
+	 */
+	bool deferred_setup;
+
+	/**
+	 * @preferred_bpp:
+	 *
+	 * Temporary storage for the driver's preferred BPP setting passed to
+	 * FB helper initialization. This needs to be tracked so that deferred
+	 * FB helper setup can pass this on.
+	 *
+	 * See also: @deferred_setup
+	 */
+	int preferred_bpp;
 };
 
 /**
-- 
2.11.0

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* ✓ Fi.CI.BAT: success for fbdev helper locking rework and deferred setup (rev2)
  2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
                   ` (14 preceding siblings ...)
  2017-06-22 14:54 ` Liviu Dudau
@ 2017-06-23 14:09 ` Patchwork
  15 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2017-06-23 14:09 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: fbdev helper locking rework and deferred setup (rev2)
URL   : https://patchwork.freedesktop.org/series/26171/
State : success

== Summary ==

Series 26171v2 fbdev helper locking rework and deferred setup
https://patchwork.freedesktop.org/api/1.0/series/26171/revisions/2/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_busy:
        Subgroup basic-flip-default-b:
                fail       -> DMESG-FAIL (fi-skl-6700hq) fdo#101144 +1
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> DMESG-FAIL (fi-skl-6700hq) fdo#101154 +27
        Subgroup basic-busy-flip-before-cursor-legacy:
                fail       -> PASS       (fi-snb-2600) fdo#100215
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                fail       -> DMESG-FAIL (fi-skl-6700hq) fdo#100461 +1
Test kms_sink_crc_basic:
                fail       -> DMESG-FAIL (fi-skl-6700hq) fdo#101519

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144
fdo#101154 https://bugs.freedesktop.org/show_bug.cgi?id=101154
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#100461 https://bugs.freedesktop.org/show_bug.cgi?id=100461
fdo#101519 https://bugs.freedesktop.org/show_bug.cgi?id=101519

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:442s
fi-bdw-gvtdvm    total:279  pass:257  dwarn:8   dfail:0   fail:0   skip:14  time:431s
fi-bsw-n3050     total:279  pass:242  dwarn:1   dfail:0   fail:0   skip:36  time:523s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:500s
fi-byt-j1900     total:279  pass:253  dwarn:2   dfail:0   fail:0   skip:24  time:481s
fi-byt-n2820     total:279  pass:249  dwarn:2   dfail:0   fail:0   skip:28  time:475s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:601s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:435s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:419s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:420s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:494s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:470s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:475s
fi-kbl-7560u     total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  time:573s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:581s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:453s
fi-skl-6700hq    total:279  pass:219  dwarn:5   dfail:29  fail:1   skip:24  time:386s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:464s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:471s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:432s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:539s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:409s

1789a777702e41b11133dd23578edbd0b47ccf40 drm-tip: 2017y-06m-23d-12h-42m-45s UTC integration manifest
4ce0eca drm/hisilicon: Remove custom FB helper deferred setup
2a29167 drm/exynos: Remove custom FB helper deferred setup
8879fee drm/fb-helper: Support deferred setup
9eb62f0 drm/fb-helper: Split dpms handling into legacy and atomic paths
30558b0 drm/fb-helper: Stop using mode_config.mutex for internals
171296b drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy
450ea9e drm/fb-helper: Push locking into pan_display_atomic|legacy
a380223 drm/fb-helper: Drop locking from the vsync wait ioctl code
aa92c2a drm/fb-helper: Push locking in fb_is_bound
8e3aee6 drm/fb-helper: Add top-level lock
2d50053 drm/i915: Drop FBDEV #ifdev in mst code
b8ea9f4 drm/fb-helper: Push down modeset lock into FB helpers

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5036/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] drm/fb-helper: Support deferred setup
  2017-06-29 10:59   ` Maarten Lankhorst
@ 2017-06-29 12:36     ` Daniel Vetter
  0 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-29 12:36 UTC (permalink / raw)
  To: Maarten Lankhorst
  Cc: Intel Graphics Development, Thierry Reding, DRI Development

On Thu, Jun 29, 2017 at 12:59 PM, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
>> @@ -2538,6 +2563,12 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>>               return 0;
>>
>>       mutex_lock(&fb_helper->lock);
>> +     if (fb_helper->deferred_setup) {
>> +             err = __drm_fb_helper_initial_config(fb_helper,
>> +                                                  fb_helper->preferred_bpp);
> Maybe rename this function to *_and_unlock? I had to read this code twice to make sure it works as intended. :)

If this is the official suffix for trickery like this I can do - I
already asked on irc what it should be called :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] drm/fb-helper: Support deferred setup
  2017-06-28 11:32 ` [PATCH] " Daniel Vetter
  2017-06-28 16:24   ` Liviu Dudau
@ 2017-06-29 10:59   ` Maarten Lankhorst
  2017-06-29 12:36     ` Daniel Vetter
  1 sibling, 1 reply; 25+ messages in thread
From: Maarten Lankhorst @ 2017-06-29 10:59 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development; +Cc: Intel Graphics Development, Thierry Reding

Op 28-06-17 om 13:32 schreef Daniel Vetter:
> FB helper code falls back to a 1024x768 mode if no outputs are connected
> or don't report back any modes upon initialization. This can be annoying
> because outputs that are added to FB helper later on can't be used with
> FB helper if they don't support a matching mode.
>
> The fallback is in place because VGA connectors can happen to report an
> unknown connection status even when they are in fact connected.
>
> Some drivers have custom solutions in place to defer FB helper setup
> until at least one output is connected. But the logic behind these
> solutions is always the same and there is nothing driver-specific about
> it, so a better alterative is to fix the FB helper core and add support
> for all drivers automatically.
>
> This patch adds support for deferred FB helper setup. It checks all the
> connectors for their connection status, and if all of them report to be
> disconnected marks the FB helper as needing deferred setup. Whet setup
> is deferred, the FB helper core will automatically retry setup after a
> hotplug event, and it will keep trying until it succeeds.
>
> v2: Rebase onto my entirely reworked fbdev helper locking. One big
> difference is that this version again drops&reacquires the fbdev lock
> (which is now fb_helper->lock, but before this patch series it was
> mode_config->mutex), because register_framebuffer must be able to
> recurse back into fbdev helper code for the initial screen setup.
>
> v3: __drm_fb_helper_initial_config must hold fb_helper->lock upon
> return, I've fumbled that in the deferred setup case (Liviu).
>
> v4: I was blind, redo this all. __drm_fb_helper_initial_config
> shouldn't need to reacquire fb_helper->lock, that just confuses
> callers. I myself got confused by kernel_fb_helper_lock and somehow
> thought it's the same as fb_helper->lock. Tsk.
>
> Also simplify the logic a bit (we don't need two functions to probe
> connectors), we can stick much closer to the existing code. And update
> some comments I've spotted that are outdated.
>
> v5: Don't pass -EAGAIN to drivers, it's just an internal error code
> (Liviu).
>
> Cc: Liviu Dudau <liviu@dudau.co.uk>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Thierry Reding <treding@nvidia.com> (v1)
> Tested-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com> (v1)
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 101 ++++++++++++++++++++++++++--------------
>  include/drm/drm_fb_helper.h     |  23 +++++++++
>  2 files changed, 89 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index bbd4c6d0378d..e49bae10f0ee 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -521,6 +521,9 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>  	if (!drm_fbdev_emulation)
>  		return -ENODEV;
>  
> +	if (READ_ONCE(fb_helper->deferred_setup))
> +		return 0;
> +
>  	mutex_lock(&fb_helper->lock);
>  	ret = restore_fbdev_mode(fb_helper);
>  
> @@ -1695,8 +1698,7 @@ EXPORT_SYMBOL(drm_fb_helper_pan_display);
>  
>  /*
>   * Allocates the backing storage and sets up the fbdev info structure through
> - * the ->fb_probe callback and then registers the fbdev and sets up the panic
> - * notifier.
> + * the ->fb_probe callback.
>   */
>  static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  					 int preferred_bpp)
> @@ -1794,13 +1796,8 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  	}
>  
>  	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
> -		/*
> -		 * hmm everyone went away - assume VGA cable just fell out
> -		 * and will come back later.
> -		 */
> -		DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
> -		sizes.fb_width = sizes.surface_width = 1024;
> -		sizes.fb_height = sizes.surface_height = 768;
> +		DRM_INFO("Cannot find any crtc or sizes\n");
> +		return -EAGAIN;
>  	}
>  
>  	/* Handle our overallocation */
> @@ -2429,6 +2426,58 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>  	kfree(enabled);
>  }
>  
> +/* Note: Drops fb_helper->lock before returning. */
> +static int __drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
> +					  int bpp_sel)
> +{
> +	struct drm_device *dev = fb_helper->dev;
> +	struct fb_info *info;
> +	unsigned int width, height;
> +	int ret;
> +
> +	width = dev->mode_config.max_width;
> +	height = dev->mode_config.max_height;
> +
> +	drm_setup_crtcs(fb_helper, width, height);
> +	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
> +	if (ret < 0) {
> +		if (ret == -EAGAIN) {
> +			fb_helper->preferred_bpp = bpp_sel;
> +			fb_helper->deferred_setup = true;
> +			ret = 0;
> +		}
> +		mutex_unlock(&fb_helper->lock);
> +
> +		return ret;
> +	}
> +
> +	fb_helper->deferred_setup = false;
> +
> +	info = fb_helper->fbdev;
> +	info->var.pixclock = 0;
> +
> +	/* Need to drop locks to avoid recursive deadlock in
> +	 * register_framebuffer. This is ok because the only thing left to do is
> +	 * register the fbdev emulation instance in kernel_fb_helper_list. */
> +	mutex_unlock(&fb_helper->lock);
> +
> +	ret = register_framebuffer(info);
> +	if (ret < 0)
> +		return ret;
> +
> +	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
> +		 info->node, info->fix.id);
> +
> +	mutex_lock(&kernel_fb_helper_lock);
> +	if (list_empty(&kernel_fb_helper_list))
> +		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
> +
> +	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
> +	mutex_unlock(&kernel_fb_helper_lock);
> +
> +	return 0;
> +}
> +
>  /**
>   * drm_fb_helper_initial_config - setup a sane initial connector configuration
>   * @fb_helper: fb_helper device struct
> @@ -2473,39 +2522,15 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>   */
>  int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
>  {
> -	struct drm_device *dev = fb_helper->dev;
> -	struct fb_info *info;
>  	int ret;
>  
>  	if (!drm_fbdev_emulation)
>  		return 0;
>  
>  	mutex_lock(&fb_helper->lock);
> -	drm_setup_crtcs(fb_helper,
> -			dev->mode_config.max_width,
> -			dev->mode_config.max_height);
> -	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
> -	mutex_unlock(&fb_helper->lock);
> -	if (ret)
> -		return ret;
> +	ret = __drm_fb_helper_initial_config(fb_helper, bpp_sel);
>  
> -	info = fb_helper->fbdev;
> -	info->var.pixclock = 0;
> -	ret = register_framebuffer(info);
> -	if (ret < 0)
> -		return ret;
> -
> -	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
> -		 info->node, info->fix.id);
> -
> -	mutex_lock(&kernel_fb_helper_lock);
> -	if (list_empty(&kernel_fb_helper_list))
> -		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
> -
> -	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
> -	mutex_unlock(&kernel_fb_helper_lock);
> -
> -	return 0;
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_fb_helper_initial_config);
>  
> @@ -2538,6 +2563,12 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>  		return 0;
>  
>  	mutex_lock(&fb_helper->lock);
> +	if (fb_helper->deferred_setup) {
> +		err = __drm_fb_helper_initial_config(fb_helper,
> +						     fb_helper->preferred_bpp);
Maybe rename this function to *_and_unlock? I had to read this code twice to make sure it works as intended. :)
> +		return err;
> +	}
> +
>  	if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
>  		fb_helper->delayed_hotplug = true;
>  		mutex_unlock(&fb_helper->lock);
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index ea170b96e88d..a5ea6ffdfecc 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -232,6 +232,29 @@ struct drm_fb_helper {
>  	 * needs to be reprobe when fbdev is in control again.
>  	 */
>  	bool delayed_hotplug;
> +
> +	/**
> +	 * @deferred_setup:
> +	 *
> +	 * If no outputs are connected (disconnected or unknown) the FB helper
> +	 * code will defer setup until at least one of the outputs shows up.
> +	 * This field keeps track of the status so that setup can be retried
> +	 * at every hotplug event until it succeeds eventually.
> +	 *
> +	 * Protected by @lock.
> +	 */
> +	bool deferred_setup;
> +
> +	/**
> +	 * @preferred_bpp:
> +	 *
> +	 * Temporary storage for the driver's preferred BPP setting passed to
> +	 * FB helper initialization. This needs to be tracked so that deferred
> +	 * FB helper setup can pass this on.
> +	 *
> +	 * See also: @deferred_setup
> +	 */
> +	int preferred_bpp;
>  };
>  
>  /**


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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] drm/fb-helper: Support deferred setup
  2017-06-28 11:32 ` [PATCH] " Daniel Vetter
@ 2017-06-28 16:24   ` Liviu Dudau
  2017-06-29 10:59   ` Maarten Lankhorst
  1 sibling, 0 replies; 25+ messages in thread
From: Liviu Dudau @ 2017-06-28 16:24 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, Thierry Reding, John Stultz, DRI Development

On Wed, Jun 28, 2017 at 01:32:01PM +0200, Daniel Vetter wrote:
> FB helper code falls back to a 1024x768 mode if no outputs are connected
> or don't report back any modes upon initialization. This can be annoying
> because outputs that are added to FB helper later on can't be used with
> FB helper if they don't support a matching mode.
> 
> The fallback is in place because VGA connectors can happen to report an
> unknown connection status even when they are in fact connected.
> 
> Some drivers have custom solutions in place to defer FB helper setup
> until at least one output is connected. But the logic behind these
> solutions is always the same and there is nothing driver-specific about
> it, so a better alterative is to fix the FB helper core and add support
> for all drivers automatically.
> 
> This patch adds support for deferred FB helper setup. It checks all the
> connectors for their connection status, and if all of them report to be
> disconnected marks the FB helper as needing deferred setup. Whet setup
> is deferred, the FB helper core will automatically retry setup after a
> hotplug event, and it will keep trying until it succeeds.
> 
> v2: Rebase onto my entirely reworked fbdev helper locking. One big
> difference is that this version again drops&reacquires the fbdev lock
> (which is now fb_helper->lock, but before this patch series it was
> mode_config->mutex), because register_framebuffer must be able to
> recurse back into fbdev helper code for the initial screen setup.
> 
> v3: __drm_fb_helper_initial_config must hold fb_helper->lock upon
> return, I've fumbled that in the deferred setup case (Liviu).
> 
> v4: I was blind, redo this all. __drm_fb_helper_initial_config
> shouldn't need to reacquire fb_helper->lock, that just confuses
> callers. I myself got confused by kernel_fb_helper_lock and somehow
> thought it's the same as fb_helper->lock. Tsk.
> 
> Also simplify the logic a bit (we don't need two functions to probe
> connectors), we can stick much closer to the existing code. And update
> some comments I've spotted that are outdated.
> 
> v5: Don't pass -EAGAIN to drivers, it's just an internal error code
> (Liviu).
> 
> Cc: Liviu Dudau <liviu@dudau.co.uk>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Thierry Reding <treding@nvidia.com> (v1)
> Tested-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com> (v1)
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Tested-by: Liviu Dudau <liviu@dudau.co.uk>

I'm going through the debugging on the deferred setup on first monitor connect,
as that seems to not do a full modeset, but that might turn out a bug in my
driver.

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 101 ++++++++++++++++++++++++++--------------
>  include/drm/drm_fb_helper.h     |  23 +++++++++
>  2 files changed, 89 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index bbd4c6d0378d..e49bae10f0ee 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -521,6 +521,9 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>  	if (!drm_fbdev_emulation)
>  		return -ENODEV;
>  
> +	if (READ_ONCE(fb_helper->deferred_setup))
> +		return 0;
> +
>  	mutex_lock(&fb_helper->lock);
>  	ret = restore_fbdev_mode(fb_helper);
>  
> @@ -1695,8 +1698,7 @@ EXPORT_SYMBOL(drm_fb_helper_pan_display);
>  
>  /*
>   * Allocates the backing storage and sets up the fbdev info structure through
> - * the ->fb_probe callback and then registers the fbdev and sets up the panic
> - * notifier.
> + * the ->fb_probe callback.
>   */
>  static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  					 int preferred_bpp)
> @@ -1794,13 +1796,8 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  	}
>  
>  	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
> -		/*
> -		 * hmm everyone went away - assume VGA cable just fell out
> -		 * and will come back later.
> -		 */
> -		DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
> -		sizes.fb_width = sizes.surface_width = 1024;
> -		sizes.fb_height = sizes.surface_height = 768;
> +		DRM_INFO("Cannot find any crtc or sizes\n");
> +		return -EAGAIN;
>  	}
>  
>  	/* Handle our overallocation */
> @@ -2429,6 +2426,58 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>  	kfree(enabled);
>  }
>  
> +/* Note: Drops fb_helper->lock before returning. */
> +static int __drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
> +					  int bpp_sel)
> +{
> +	struct drm_device *dev = fb_helper->dev;
> +	struct fb_info *info;
> +	unsigned int width, height;
> +	int ret;
> +
> +	width = dev->mode_config.max_width;
> +	height = dev->mode_config.max_height;
> +
> +	drm_setup_crtcs(fb_helper, width, height);
> +	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
> +	if (ret < 0) {
> +		if (ret == -EAGAIN) {
> +			fb_helper->preferred_bpp = bpp_sel;
> +			fb_helper->deferred_setup = true;
> +			ret = 0;
> +		}
> +		mutex_unlock(&fb_helper->lock);
> +
> +		return ret;
> +	}
> +
> +	fb_helper->deferred_setup = false;
> +
> +	info = fb_helper->fbdev;
> +	info->var.pixclock = 0;
> +
> +	/* Need to drop locks to avoid recursive deadlock in
> +	 * register_framebuffer. This is ok because the only thing left to do is
> +	 * register the fbdev emulation instance in kernel_fb_helper_list. */
> +	mutex_unlock(&fb_helper->lock);
> +
> +	ret = register_framebuffer(info);
> +	if (ret < 0)
> +		return ret;
> +
> +	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
> +		 info->node, info->fix.id);
> +
> +	mutex_lock(&kernel_fb_helper_lock);
> +	if (list_empty(&kernel_fb_helper_list))
> +		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
> +
> +	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
> +	mutex_unlock(&kernel_fb_helper_lock);
> +
> +	return 0;
> +}
> +
>  /**
>   * drm_fb_helper_initial_config - setup a sane initial connector configuration
>   * @fb_helper: fb_helper device struct
> @@ -2473,39 +2522,15 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>   */
>  int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
>  {
> -	struct drm_device *dev = fb_helper->dev;
> -	struct fb_info *info;
>  	int ret;
>  
>  	if (!drm_fbdev_emulation)
>  		return 0;
>  
>  	mutex_lock(&fb_helper->lock);
> -	drm_setup_crtcs(fb_helper,
> -			dev->mode_config.max_width,
> -			dev->mode_config.max_height);
> -	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
> -	mutex_unlock(&fb_helper->lock);
> -	if (ret)
> -		return ret;
> +	ret = __drm_fb_helper_initial_config(fb_helper, bpp_sel);
>  
> -	info = fb_helper->fbdev;
> -	info->var.pixclock = 0;
> -	ret = register_framebuffer(info);
> -	if (ret < 0)
> -		return ret;
> -
> -	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
> -		 info->node, info->fix.id);
> -
> -	mutex_lock(&kernel_fb_helper_lock);
> -	if (list_empty(&kernel_fb_helper_list))
> -		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
> -
> -	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
> -	mutex_unlock(&kernel_fb_helper_lock);
> -
> -	return 0;
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_fb_helper_initial_config);
>  
> @@ -2538,6 +2563,12 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>  		return 0;
>  
>  	mutex_lock(&fb_helper->lock);
> +	if (fb_helper->deferred_setup) {
> +		err = __drm_fb_helper_initial_config(fb_helper,
> +						     fb_helper->preferred_bpp);
> +		return err;
> +	}
> +

Minor nitpick: specially in drm_fb_helper_initial_config() you can get rid of the ret variable
and use 'return __drm_fb_helper_initial_config()'. Also here, can skip storing the result of
__drm_fb_helper_initial_config() in err.

Best regards,
Liviu


>  	if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
>  		fb_helper->delayed_hotplug = true;
>  		mutex_unlock(&fb_helper->lock);
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index ea170b96e88d..a5ea6ffdfecc 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -232,6 +232,29 @@ struct drm_fb_helper {
>  	 * needs to be reprobe when fbdev is in control again.
>  	 */
>  	bool delayed_hotplug;
> +
> +	/**
> +	 * @deferred_setup:
> +	 *
> +	 * If no outputs are connected (disconnected or unknown) the FB helper
> +	 * code will defer setup until at least one of the outputs shows up.
> +	 * This field keeps track of the status so that setup can be retried
> +	 * at every hotplug event until it succeeds eventually.
> +	 *
> +	 * Protected by @lock.
> +	 */
> +	bool deferred_setup;
> +
> +	/**
> +	 * @preferred_bpp:
> +	 *
> +	 * Temporary storage for the driver's preferred BPP setting passed to
> +	 * FB helper initialization. This needs to be tracked so that deferred
> +	 * FB helper setup can pass this on.
> +	 *
> +	 * See also: @deferred_setup
> +	 */
> +	int preferred_bpp;
>  };
>  
>  /**
> -- 
> 2.11.0
> 

-- 
   _
 _|_|_
 ('_')
 (⊃  )⊃
 |_|_|
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] drm/fb-helper: Support deferred setup
  2017-06-27 14:59 [PATCH 10/13] drm/fb-helper: Support deferred setup Daniel Vetter
@ 2017-06-28 11:32 ` Daniel Vetter
  2017-06-28 16:24   ` Liviu Dudau
  2017-06-29 10:59   ` Maarten Lankhorst
  0 siblings, 2 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-28 11:32 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding,
	John Stultz, Liviu Dudau

FB helper code falls back to a 1024x768 mode if no outputs are connected
or don't report back any modes upon initialization. This can be annoying
because outputs that are added to FB helper later on can't be used with
FB helper if they don't support a matching mode.

The fallback is in place because VGA connectors can happen to report an
unknown connection status even when they are in fact connected.

Some drivers have custom solutions in place to defer FB helper setup
until at least one output is connected. But the logic behind these
solutions is always the same and there is nothing driver-specific about
it, so a better alterative is to fix the FB helper core and add support
for all drivers automatically.

This patch adds support for deferred FB helper setup. It checks all the
connectors for their connection status, and if all of them report to be
disconnected marks the FB helper as needing deferred setup. Whet setup
is deferred, the FB helper core will automatically retry setup after a
hotplug event, and it will keep trying until it succeeds.

v2: Rebase onto my entirely reworked fbdev helper locking. One big
difference is that this version again drops&reacquires the fbdev lock
(which is now fb_helper->lock, but before this patch series it was
mode_config->mutex), because register_framebuffer must be able to
recurse back into fbdev helper code for the initial screen setup.

v3: __drm_fb_helper_initial_config must hold fb_helper->lock upon
return, I've fumbled that in the deferred setup case (Liviu).

v4: I was blind, redo this all. __drm_fb_helper_initial_config
shouldn't need to reacquire fb_helper->lock, that just confuses
callers. I myself got confused by kernel_fb_helper_lock and somehow
thought it's the same as fb_helper->lock. Tsk.

Also simplify the logic a bit (we don't need two functions to probe
connectors), we can stick much closer to the existing code. And update
some comments I've spotted that are outdated.

v5: Don't pass -EAGAIN to drivers, it's just an internal error code
(Liviu).

Cc: Liviu Dudau <liviu@dudau.co.uk>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thierry Reding <treding@nvidia.com> (v1)
Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Thierry Reding <treding@nvidia.com> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fb_helper.c | 101 ++++++++++++++++++++++++++--------------
 include/drm/drm_fb_helper.h     |  23 +++++++++
 2 files changed, 89 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index bbd4c6d0378d..e49bae10f0ee 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -521,6 +521,9 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 	if (!drm_fbdev_emulation)
 		return -ENODEV;
 
+	if (READ_ONCE(fb_helper->deferred_setup))
+		return 0;
+
 	mutex_lock(&fb_helper->lock);
 	ret = restore_fbdev_mode(fb_helper);
 
@@ -1695,8 +1698,7 @@ EXPORT_SYMBOL(drm_fb_helper_pan_display);
 
 /*
  * Allocates the backing storage and sets up the fbdev info structure through
- * the ->fb_probe callback and then registers the fbdev and sets up the panic
- * notifier.
+ * the ->fb_probe callback.
  */
 static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 					 int preferred_bpp)
@@ -1794,13 +1796,8 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	}
 
 	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
-		/*
-		 * hmm everyone went away - assume VGA cable just fell out
-		 * and will come back later.
-		 */
-		DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
-		sizes.fb_width = sizes.surface_width = 1024;
-		sizes.fb_height = sizes.surface_height = 768;
+		DRM_INFO("Cannot find any crtc or sizes\n");
+		return -EAGAIN;
 	}
 
 	/* Handle our overallocation */
@@ -2429,6 +2426,58 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 	kfree(enabled);
 }
 
+/* Note: Drops fb_helper->lock before returning. */
+static int __drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
+					  int bpp_sel)
+{
+	struct drm_device *dev = fb_helper->dev;
+	struct fb_info *info;
+	unsigned int width, height;
+	int ret;
+
+	width = dev->mode_config.max_width;
+	height = dev->mode_config.max_height;
+
+	drm_setup_crtcs(fb_helper, width, height);
+	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
+	if (ret < 0) {
+		if (ret == -EAGAIN) {
+			fb_helper->preferred_bpp = bpp_sel;
+			fb_helper->deferred_setup = true;
+			ret = 0;
+		}
+		mutex_unlock(&fb_helper->lock);
+
+		return ret;
+	}
+
+	fb_helper->deferred_setup = false;
+
+	info = fb_helper->fbdev;
+	info->var.pixclock = 0;
+
+	/* Need to drop locks to avoid recursive deadlock in
+	 * register_framebuffer. This is ok because the only thing left to do is
+	 * register the fbdev emulation instance in kernel_fb_helper_list. */
+	mutex_unlock(&fb_helper->lock);
+
+	ret = register_framebuffer(info);
+	if (ret < 0)
+		return ret;
+
+	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
+		 info->node, info->fix.id);
+
+	mutex_lock(&kernel_fb_helper_lock);
+	if (list_empty(&kernel_fb_helper_list))
+		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
+
+	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
+	mutex_unlock(&kernel_fb_helper_lock);
+
+	return 0;
+}
+
 /**
  * drm_fb_helper_initial_config - setup a sane initial connector configuration
  * @fb_helper: fb_helper device struct
@@ -2473,39 +2522,15 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
  */
 int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
 {
-	struct drm_device *dev = fb_helper->dev;
-	struct fb_info *info;
 	int ret;
 
 	if (!drm_fbdev_emulation)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
-	drm_setup_crtcs(fb_helper,
-			dev->mode_config.max_width,
-			dev->mode_config.max_height);
-	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
-	mutex_unlock(&fb_helper->lock);
-	if (ret)
-		return ret;
+	ret = __drm_fb_helper_initial_config(fb_helper, bpp_sel);
 
-	info = fb_helper->fbdev;
-	info->var.pixclock = 0;
-	ret = register_framebuffer(info);
-	if (ret < 0)
-		return ret;
-
-	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
-		 info->node, info->fix.id);
-
-	mutex_lock(&kernel_fb_helper_lock);
-	if (list_empty(&kernel_fb_helper_list))
-		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
-
-	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
-	mutex_unlock(&kernel_fb_helper_lock);
-
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_initial_config);
 
@@ -2538,6 +2563,12 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
+	if (fb_helper->deferred_setup) {
+		err = __drm_fb_helper_initial_config(fb_helper,
+						     fb_helper->preferred_bpp);
+		return err;
+	}
+
 	if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
 		fb_helper->delayed_hotplug = true;
 		mutex_unlock(&fb_helper->lock);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index ea170b96e88d..a5ea6ffdfecc 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -232,6 +232,29 @@ struct drm_fb_helper {
 	 * needs to be reprobe when fbdev is in control again.
 	 */
 	bool delayed_hotplug;
+
+	/**
+	 * @deferred_setup:
+	 *
+	 * If no outputs are connected (disconnected or unknown) the FB helper
+	 * code will defer setup until at least one of the outputs shows up.
+	 * This field keeps track of the status so that setup can be retried
+	 * at every hotplug event until it succeeds eventually.
+	 *
+	 * Protected by @lock.
+	 */
+	bool deferred_setup;
+
+	/**
+	 * @preferred_bpp:
+	 *
+	 * Temporary storage for the driver's preferred BPP setting passed to
+	 * FB helper initialization. This needs to be tracked so that deferred
+	 * FB helper setup can pass this on.
+	 *
+	 * See also: @deferred_setup
+	 */
+	int preferred_bpp;
 };
 
 /**
-- 
2.11.0

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2017-06-29 12:36 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-21 18:28 [PATCH 00/12] fbdev helper locking rework and deferred setup Daniel Vetter
2017-06-21 18:28 ` [PATCH 01/12] drm/fb-helper: Push down modeset lock into FB helpers Daniel Vetter
2017-06-21 18:28 ` [PATCH 02/12] drm/i915: Drop FBDEV #ifdev in mst code Daniel Vetter
2017-06-21 18:28 ` [PATCH 03/12] drm/fb-helper: Add top-level lock Daniel Vetter
2017-06-21 18:28 ` [PATCH 04/12] drm/fb-helper: Push locking in fb_is_bound Daniel Vetter
2017-06-21 18:28 ` [PATCH 05/12] drm/fb-helper: Drop locking from the vsync wait ioctl code Daniel Vetter
2017-06-21 18:28 ` [PATCH 06/12] drm/fb-helper: Push locking into pan_display_atomic|legacy Daniel Vetter
2017-06-21 18:28 ` [PATCH 07/12] drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy Daniel Vetter
2017-06-21 18:28 ` [PATCH 08/12] drm/fb-helper: Stop using mode_config.mutex for internals Daniel Vetter
2017-06-21 18:28 ` [PATCH 09/12] drm/fb-helper: Split dpms handling into legacy and atomic paths Daniel Vetter
2017-06-22 10:24   ` Peter Rosin
2017-06-21 18:28 ` [PATCH 10/12] drm/fb-helper: Support deferred setup Daniel Vetter
2017-06-23 13:31   ` [PATCH] " Daniel Vetter
2017-06-21 18:28 ` [PATCH 11/12] drm/exynos: Remove custom FB helper " Daniel Vetter
2017-06-21 18:28 ` [PATCH 12/12] drm/hisilicon: " Daniel Vetter
2017-06-21 18:48 ` ✓ Fi.CI.BAT: success for fbdev helper locking rework and " Patchwork
2017-06-21 23:01 ` [PATCH 00/12] " John Stultz
2017-06-22 14:54 ` Liviu Dudau
2017-06-23  7:38   ` Daniel Vetter
2017-06-23 12:34     ` Liviu Dudau
2017-06-23 14:09 ` ✓ Fi.CI.BAT: success for fbdev helper locking rework and deferred setup (rev2) Patchwork
2017-06-27 14:59 [PATCH 10/13] drm/fb-helper: Support deferred setup Daniel Vetter
2017-06-28 11:32 ` [PATCH] " Daniel Vetter
2017-06-28 16:24   ` Liviu Dudau
2017-06-29 10:59   ` Maarten Lankhorst
2017-06-29 12:36     ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.