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

Hi all,

Here's the locking patches respun with Maarten's review. I think that part is
ready for merging. The deferred setup itself needs more thought, since both
Maarten and Liviu are still unhappy with what happens between the deferred setup
and fbcon.

Cheers, Daniel

Daniel Vetter (9):
  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
  drm/fb-helper: Support deferred setup
  drm/i915: Protect against deferred fbdev setup

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

 drivers/gpu/drm/drm_fb_helper.c                 | 359 ++++++++++++++----------
 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 |  20 +-
 drivers/gpu/drm/i915/i915_debugfs.c             |   2 +-
 drivers/gpu/drm/i915/i915_drv.h                 |   2 -
 drivers/gpu/drm/i915/intel_dp_mst.c             |  47 +---
 drivers/gpu/drm/i915/intel_fbdev.c              |  16 +-
 drivers/gpu/drm/radeon/radeon_dp_mst.c          |   7 -
 include/drm/drm_fb_helper.h                     |  42 ++-
 11 files changed, 297 insertions(+), 232 deletions(-)

-- 
2.13.2

_______________________________________________
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/13] drm/fb-helper: Push down modeset lock into FB helpers
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
@ 2017-07-04 15:18 ` Daniel Vetter
  2017-07-04 15:18 ` [PATCH 02/13] drm/i915: Drop FBDEV #ifdev in mst code Daniel Vetter
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 15:18 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Alex Deucher,
	Thierry Reding, Christian König

From: Thierry Reding <treding@nvidia.com>

Move the modeset locking from drivers into FB helpers.

v2: Also handle intel_connector_add_to_fbdev.

v3: Prevent race in intel_dp_mst with ->detect (Maarten)

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
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    |  9 +++-----
 drivers/gpu/drm/radeon/radeon_dp_mst.c |  7 ------
 3 files changed, 37 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..9aa959284497 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,15 @@ 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);
+	/* prevent race with the check in ->detect */
+	drm_modeset_lock(&connector->dev->mode_config.connection_mutex, NULL);
 	intel_connector->mst_port = NULL;
-	drm_modeset_unlock_all(dev);
+	drm_modeset_unlock(&connector->dev->mode_config.connection_mutex);
 
 	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.13.2

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

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

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

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

v2: We also need to drop the #ifdef from headers. Seems like a small
price to pay for slightly cleaner code.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |  2 --
 drivers/gpu/drm/i915/intel_dp_mst.c | 38 ++++++++++---------------------------
 2 files changed, 10 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0029bb949e90..3749208b7200 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2304,11 +2304,9 @@ struct drm_i915_private {
 
 	struct drm_i915_gem_object *vlv_pctx;
 
-#ifdef CONFIG_DRM_FBDEV_EMULATION
 	/* list of fbdev register on this device */
 	struct intel_fbdev *fbdev;
 	struct work_struct fbdev_suspend_work;
-#endif
 
 	struct drm_property *broadcast_rgb_property;
 	struct drm_property *force_audio_property;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 9aa959284497..e4ea968b1d6b 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,28 +478,32 @@ 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);
 	/* prevent race with the check in ->detect */
 	drm_modeset_lock(&connector->dev->mode_config.connection_mutex, NULL);
 	intel_connector->mst_port = NULL;
 	drm_modeset_unlock(&connector->dev->mode_config.connection_mutex);
 
-	drm_connector_unreference(&intel_connector->base);
+	drm_connector_unreference(connector);
 	DRM_DEBUG_KMS("\n");
 }
 
-- 
2.13.2

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

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

* [PATCH 03/13] drm/fb-helper: Add top-level lock
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
  2017-07-04 15:18 ` [PATCH 01/13] drm/fb-helper: Push down modeset lock into FB helpers Daniel Vetter
  2017-07-04 15:18 ` [PATCH 02/13] drm/i915: Drop FBDEV #ifdev in mst code Daniel Vetter
@ 2017-07-04 15:18 ` Daniel Vetter
  2017-07-04 15:18 ` [PATCH 04/13] drm/fb-helper: Push locking in fb_is_bound Daniel Vetter
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 15:18 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.13.2

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

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

* [PATCH 04/13] drm/fb-helper: Push locking in fb_is_bound
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
                   ` (2 preceding siblings ...)
  2017-07-04 15:18 ` [PATCH 03/13] drm/fb-helper: Add top-level lock Daniel Vetter
@ 2017-07-04 15:18 ` Daniel Vetter
  2017-07-04 15:18 ` [PATCH 05/13] drm/fb-helper: Drop locking from the vsync wait ioctl code Daniel Vetter
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 15:18 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding,
	John Stultz, 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.13.2

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

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

* [PATCH 05/13] drm/fb-helper: Drop locking from the vsync wait ioctl code
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
                   ` (3 preceding siblings ...)
  2017-07-04 15:18 ` [PATCH 04/13] drm/fb-helper: Push locking in fb_is_bound Daniel Vetter
@ 2017-07-04 15:18 ` Daniel Vetter
  2017-07-04 15:18 ` [PATCH 06/13] drm/fb-helper: Push locking into pan_display_atomic|legacy Daniel Vetter
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 15:18 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.13.2

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

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

* [PATCH 06/13] drm/fb-helper: Push locking into pan_display_atomic|legacy
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
                   ` (4 preceding siblings ...)
  2017-07-04 15:18 ` [PATCH 05/13] drm/fb-helper: Drop locking from the vsync wait ioctl code Daniel Vetter
@ 2017-07-04 15:18 ` Daniel Vetter
  2017-07-04 15:18 ` [PATCH 07/13] drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy Daniel Vetter
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 15:18 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding,
	John Stultz, 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.13.2

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

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

* [PATCH 07/13] drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
                   ` (5 preceding siblings ...)
  2017-07-04 15:18 ` [PATCH 06/13] drm/fb-helper: Push locking into pan_display_atomic|legacy Daniel Vetter
@ 2017-07-04 15:18 ` Daniel Vetter
  2017-07-04 15:18 ` [PATCH 08/13] drm/fb-helper: Stop using mode_config.mutex for internals Daniel Vetter
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 15:18 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding,
	John Stultz, 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.13.2

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

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

* [PATCH 08/13] drm/fb-helper: Stop using mode_config.mutex for internals
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
                   ` (6 preceding siblings ...)
  2017-07-04 15:18 ` [PATCH 07/13] drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy Daniel Vetter
@ 2017-07-04 15:18 ` Daniel Vetter
  2017-07-04 15:40   ` Ville Syrjälä
  2017-07-05  4:56   ` [PATCH] " Daniel Vetter
  2017-07-04 15:18 ` [PATCH 09/13] drm/fb-helper: Split dpms handling into legacy and atomic paths Daniel Vetter
                   ` (6 subsequent siblings)
  14 siblings, 2 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 15:18 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.

v3: I forgot to hold mode_config.mutex while looking at
connector->status and the mode list. Also, we need to patch up the
i915 ->initial_config callback to grab the locks it needs to inspect
the modeset state recovered from the fw.

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    | 33 ++++++++-------------------------
 drivers/gpu/drm/drm_vblank.c       |  2 +-
 drivers/gpu/drm/i915/intel_fbdev.c | 16 ++++++++++++----
 3 files changed, 21 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 400bbb07eff2..59e2916471b2 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;
@@ -1893,12 +1882,11 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe
 EXPORT_SYMBOL(drm_fb_helper_fill_var);
 
 static int drm_fb_helper_probe_connector_modes(struct drm_fb_helper *fb_helper,
-					       uint32_t maxX,
-					       uint32_t maxY)
+						uint32_t maxX,
+						uint32_t maxY)
 {
 	struct drm_connector *connector;
-	int count = 0;
-	int i;
+	int i, count = 0;
 
 	drm_fb_helper_for_each_connector(fb_helper, i) {
 		connector = fb_helper->connector_info[i]->connector;
@@ -2296,12 +2284,8 @@ 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);
-	lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
 
 	crtcs = kcalloc(fb_helper->connector_count,
 			sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL);
@@ -2316,7 +2300,10 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 		goto out;
 	}
 
+	mutex_lock(&fb_helper->dev->mode_config.mutex);
 	drm_enable_connectors(fb_helper, enabled);
+	if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
+		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
 
 	if (!(fb_helper->funcs->initial_config &&
 	      fb_helper->funcs->initial_config(fb_helper, crtcs, modes,
@@ -2337,6 +2324,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 
 		drm_pick_crtcs(fb_helper, crtcs, modes, 0, width, height);
 	}
+	mutex_unlock(&fb_helper->dev->mode_config.mutex);
 
 	/* need to set the modesets up here for use later */
 	/* fill out the connector<->crtc mappings into the modesets */
@@ -2428,12 +2416,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 +2482,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 bfe86fa2d3b1..70f2b9593edc 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.
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 03347c6ae599..460ca0b3fb88 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -352,14 +352,20 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 	unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG);
 	int i, j;
 	bool *save_enabled;
-	bool fallback = true;
+	bool fallback = true, ret = true;
 	int num_connectors_enabled = 0;
 	int num_connectors_detected = 0;
+	struct drm_modeset_acquire_ctx ctx;
 
 	save_enabled = kcalloc(count, sizeof(bool), GFP_KERNEL);
 	if (!save_enabled)
 		return false;
 
+	drm_modeset_acquire_init(&ctx, 0);
+
+	while (drm_modeset_lock_all_ctx(fb_helper->dev, &ctx) != 0)
+		drm_modeset_backoff(&ctx);
+
 	memcpy(save_enabled, enabled, count);
 	mask = GENMASK(count - 1, 0);
 	conn_configured = 0;
@@ -509,12 +515,14 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 bail:
 		DRM_DEBUG_KMS("Not using firmware configuration\n");
 		memcpy(enabled, save_enabled, count);
-		kfree(save_enabled);
-		return false;
+		ret = false;
 	}
 
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
 	kfree(save_enabled);
-	return true;
+	return ret;
 }
 
 static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
-- 
2.13.2

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

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

* [PATCH 09/13] drm/fb-helper: Split dpms handling into legacy and atomic paths
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
                   ` (7 preceding siblings ...)
  2017-07-04 15:18 ` [PATCH 08/13] drm/fb-helper: Stop using mode_config.mutex for internals Daniel Vetter
@ 2017-07-04 15:18 ` Daniel Vetter
  2017-07-04 15:18 ` [PATCH 10/13] drm/fb-helper: Support deferred setup Daniel Vetter
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 15:18 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, 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.

v2: Squash in patches from Maarten to unify all the various atomic
paths into just one atomic update function for fbdev overall. On top
do one s/restore_fbdev_mode/restore_fbdev_mode_atomic/ so that we have
all-atomic callchains after the first check.

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

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 59e2916471b2..a35a2ab8e630 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -378,7 +378,7 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
 }
 EXPORT_SYMBOL(drm_fb_helper_debug_leave);
 
-static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
+static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool active)
 {
 	struct drm_device *dev = fb_helper->dev;
 	struct drm_plane *plane;
@@ -427,6 +427,17 @@ 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 out_state;
+
+		/*
+		 * __drm_atomic_helper_set_config() sets active when a
+		 * mode is set, unconditionally clear it if we force DPMS off
+		 */
+		if (!active) {
+			struct drm_crtc *crtc = mode_set->crtc;
+			struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+
+			crtc_state->active = false;
+		}
 	}
 
 	ret = drm_atomic_commit(state);
@@ -497,7 +508,7 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 	struct drm_device *dev = fb_helper->dev;
 
 	if (drm_drv_uses_atomic_modeset(dev))
-		return restore_fbdev_mode_atomic(fb_helper);
+		return restore_fbdev_mode_atomic(fb_helper, true);
 	else
 		return restore_fbdev_mode_legacy(fb_helper);
 }
@@ -616,23 +627,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 +650,25 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
 		}
 	}
 	drm_modeset_unlock_all(dev);
+}
+
+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))
+		restore_fbdev_mode_atomic(fb_helper, dpms_mode == DRM_MODE_DPMS_ON);
+	else
+		dpms_legacy(fb_helper, dpms_mode);
 	mutex_unlock(&fb_helper->lock);
 }
 
@@ -1503,70 +1523,36 @@ int drm_fb_helper_set_par(struct fb_info *info)
 }
 EXPORT_SYMBOL(drm_fb_helper_set_par);
 
-static int pan_display_atomic(struct fb_var_screeninfo *var,
-			      struct fb_info *info)
+static void pan_set(struct drm_fb_helper *fb_helper, int x, int y)
 {
-	struct drm_fb_helper *fb_helper = info->par;
-	struct drm_device *dev = fb_helper->dev;
-	struct drm_atomic_state *state;
-	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) {
-		ret = -ENOMEM;
-		goto out_ctx;
-	}
+	int i;
 
-	state->acquire_ctx = &ctx;
-retry:
-	plane_mask = 0;
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		struct drm_mode_set *mode_set;
 
 		mode_set = &fb_helper->crtc_info[i].mode_set;
 
-		mode_set->x = var->xoffset;
-		mode_set->y = var->yoffset;
-
-		ret = __drm_atomic_helper_set_config(mode_set, state);
-		if (ret != 0)
-			goto out_state;
-
-		plane = mode_set->crtc->primary;
-		plane_mask |= (1 << drm_plane_index(plane));
-		plane->old_fb = plane->fb;
+		mode_set->x = x;
+		mode_set->y = y;
 	}
+}
 
-	ret = drm_atomic_commit(state);
-	if (ret != 0)
-		goto out_state;
-
-	info->var.xoffset = var->xoffset;
-	info->var.yoffset = var->yoffset;
-
-out_state:
-	drm_atomic_clean_old_fb(dev, plane_mask, ret);
+static int pan_display_atomic(struct fb_var_screeninfo *var,
+			      struct fb_info *info)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	int ret;
 
-	if (ret == -EDEADLK)
-		goto backoff;
+	pan_set(fb_helper, var->xoffset, var->yoffset);
 
-	drm_atomic_state_put(state);
-out_ctx:
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
+	ret = restore_fbdev_mode_atomic(fb_helper, true);
+	if (!ret) {
+		info->var.xoffset = var->xoffset;
+		info->var.yoffset = var->yoffset;
+	} else
+		pan_set(fb_helper, info->var.xoffset, info->var.yoffset);
 
 	return ret;
-
-backoff:
-	drm_atomic_state_clear(state);
-	drm_modeset_backoff(&ctx);
-
-	goto retry;
 }
 
 static int pan_display_legacy(struct fb_var_screeninfo *var,
@@ -2467,7 +2453,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.13.2

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

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

* [PATCH 10/13] drm/fb-helper: Support deferred setup
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
                   ` (8 preceding siblings ...)
  2017-07-04 15:18 ` [PATCH 09/13] drm/fb-helper: Split dpms handling into legacy and atomic paths Daniel Vetter
@ 2017-07-04 15:18 ` Daniel Vetter
  2017-07-04 15:29   ` Liviu Dudau
  2017-07-05 11:20   ` Maarten Lankhorst
  2017-07-04 15:18 ` [PATCH 11/13] drm/i915: Protect against deferred fbdev setup Daniel Vetter
                   ` (4 subsequent siblings)
  14 siblings, 2 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 15:18 UTC (permalink / raw)
  To: DRI Development
  Cc: Liviu Dudau, Daniel Vetter, Intel Graphics Development,
	John Stultz, Thierry Reding

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).

v6: Add _and_unlock suffix to clarify locking (Maarten)

Cc: Liviu Dudau <liviu@dudau.co.uk>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
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 | 102 ++++++++++++++++++++++++++--------------
 include/drm/drm_fb_helper.h     |  23 +++++++++
 2 files changed, 90 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index a35a2ab8e630..135367548027 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -532,6 +532,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);
 
@@ -1616,8 +1619,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)
@@ -1715,13 +1717,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 */
@@ -2350,6 +2347,59 @@ 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_and_unlock(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
@@ -2394,39 +2444,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_and_unlock(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);
 
@@ -2459,6 +2485,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_and_unlock(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.13.2

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

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

* [PATCH 11/13] drm/i915: Protect against deferred fbdev setup
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
                   ` (9 preceding siblings ...)
  2017-07-04 15:18 ` [PATCH 10/13] drm/fb-helper: Support deferred setup Daniel Vetter
@ 2017-07-04 15:18 ` Daniel Vetter
  2017-07-05 10:08   ` Maarten Lankhorst
  2017-07-04 15:18 ` [PATCH 12/13] drm/exynos: Remove custom FB helper deferred setup Daniel Vetter
                   ` (3 subsequent siblings)
  14 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 15:18 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter

We could probably hit this already with our current async fbdev init,
but it's much easier to hit this with the new deferred fbdev setup
that I'm working on polishing.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 580bd4f4a49e..a4224566ebca 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1931,7 +1931,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
 		return ret;
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
-	if (dev_priv->fbdev) {
+	if (dev_priv->fbdev && dev_priv->fbdev->helper.fb) {
 		fbdev_fb = to_intel_framebuffer(dev_priv->fbdev->helper.fb);
 
 		seq_printf(m, "fbcon size: %d x %d, depth %d, %d bpp, modifier 0x%llx, refcount %d, obj ",
-- 
2.13.2

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

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

* [PATCH 12/13] drm/exynos: Remove custom FB helper deferred setup
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
                   ` (10 preceding siblings ...)
  2017-07-04 15:18 ` [PATCH 11/13] drm/i915: Protect against deferred fbdev setup Daniel Vetter
@ 2017-07-04 15:18 ` Daniel Vetter
  2017-07-06  3:16   ` Inki Dae
  2017-07-04 15:18 ` [PATCH 13/13] drm/hisilicon: " Daniel Vetter
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 15:18 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Seung-Woo Kim,
	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.13.2

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

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

* [PATCH 13/13] drm/hisilicon: Remove custom FB helper deferred setup
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
                   ` (11 preceding siblings ...)
  2017-07-04 15:18 ` [PATCH 12/13] drm/exynos: Remove custom FB helper deferred setup Daniel Vetter
@ 2017-07-04 15:18 ` Daniel Vetter
  2017-07-04 16:39 ` ✓ Fi.CI.BAT: success for fbdev locking + deferred setup take 3 Patchwork
  2017-07-05  5:14 ` ✓ Fi.CI.BAT: success for fbdev locking + deferred setup take 3 (rev2) Patchwork
  14 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 15:18 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.

v2: Dont' resurrect drm_vblank_cleanup.

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>
Signed-off-by: Thierry Reding <treding@nvidia.com> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 20 ++++++++++----------
 1 file changed, 10 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..1178341c3858 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,18 @@ 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);
 err_unbind_all:
 	component_unbind_all(dev->dev, dev);
 err_dc_cleanup:
-- 
2.13.2

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

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

* Re: [PATCH 10/13] drm/fb-helper: Support deferred setup
  2017-07-04 15:18 ` [PATCH 10/13] drm/fb-helper: Support deferred setup Daniel Vetter
@ 2017-07-04 15:29   ` Liviu Dudau
  2017-07-05 11:20   ` Maarten Lankhorst
  1 sibling, 0 replies; 25+ messages in thread
From: Liviu Dudau @ 2017-07-04 15:29 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development, Thierry Reding, DRI Development

On Tue, Jul 04, 2017 at 05:18:30PM +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).
> 
> v6: Add _and_unlock suffix to clarify locking (Maarten)
> 
> Cc: Liviu Dudau <liviu@dudau.co.uk>

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

We can keep the discussion on the need for set_par separate on the thread
for the patch that I have sent.

Best regards,
Liviu

> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 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 | 102 ++++++++++++++++++++++++++--------------
>  include/drm/drm_fb_helper.h     |  23 +++++++++
>  2 files changed, 90 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index a35a2ab8e630..135367548027 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -532,6 +532,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);
>  
> @@ -1616,8 +1619,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)
> @@ -1715,13 +1717,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 */
> @@ -2350,6 +2347,59 @@ 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_and_unlock(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
> @@ -2394,39 +2444,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_and_unlock(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);
>  
> @@ -2459,6 +2485,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_and_unlock(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.13.2
> 

-- 
   _
 _|_|_
 ('_')
 (⊃  )⊃
 |_|_|
_______________________________________________
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 08/13] drm/fb-helper: Stop using mode_config.mutex for internals
  2017-07-04 15:18 ` [PATCH 08/13] drm/fb-helper: Stop using mode_config.mutex for internals Daniel Vetter
@ 2017-07-04 15:40   ` Ville Syrjälä
  2017-07-04 16:19     ` Daniel Vetter
  2017-07-05  4:56   ` [PATCH] " Daniel Vetter
  1 sibling, 1 reply; 25+ messages in thread
From: Ville Syrjälä @ 2017-07-04 15:40 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding,
	DRI Development

On Tue, Jul 04, 2017 at 05:18:28PM +0200, Daniel Vetter wrote:
> Those are now all protected using fb_helper->lock.
> 
> v2: We still need to hold mode_config.mutex right around calling
> connector->fill_modes.
> 
> v3: I forgot to hold mode_config.mutex while looking at
> connector->status and the mode list. Also, we need to patch up the
> i915 ->initial_config callback to grab the locks it needs to inspect
> the modeset state recovered from the fw.
> 
> 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    | 33 ++++++++-------------------------
>  drivers/gpu/drm/drm_vblank.c       |  2 +-
>  drivers/gpu/drm/i915/intel_fbdev.c | 16 ++++++++++++----
>  3 files changed, 21 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 400bbb07eff2..59e2916471b2 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
<snip>
> @@ -2296,12 +2284,8 @@ 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);
> -	lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
>  
>  	crtcs = kcalloc(fb_helper->connector_count,
>  			sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL);
> @@ -2316,7 +2300,10 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>  		goto out;
>  	}
>  
> +	mutex_lock(&fb_helper->dev->mode_config.mutex);
>  	drm_enable_connectors(fb_helper, enabled);
> +	if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)

This changes the order of these two functions calls. Looks to me like
drm_fb_helper_probe_connector_modes() updates connector->status, and
drm_enable_connectors() depends on it. So either these shouldn't get 
reordered or there's some not immediately obvious reason why that's OK.

> +		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
>  
>  	if (!(fb_helper->funcs->initial_config &&
>  	      fb_helper->funcs->initial_config(fb_helper, crtcs, modes,

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
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 08/13] drm/fb-helper: Stop using mode_config.mutex for internals
  2017-07-04 15:40   ` Ville Syrjälä
@ 2017-07-04 16:19     ` Daniel Vetter
  0 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-04 16:19 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding,
	DRI Development

On Tue, Jul 4, 2017 at 5:40 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>> +     mutex_lock(&fb_helper->dev->mode_config.mutex);
>>       drm_enable_connectors(fb_helper, enabled);
>> +     if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
>
> This changes the order of these two functions calls. Looks to me like
> drm_fb_helper_probe_connector_modes() updates connector->status, and
> drm_enable_connectors() depends on it. So either these shouldn't get
> reordered or there's some not immediately obvious reason why that's OK.

I think some rebase fail somewhere ... this should indeed not get
reordered. Thanks for spotting.
-Daniel
-- 
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

* ✓ Fi.CI.BAT: success for fbdev locking + deferred setup take 3
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
                   ` (12 preceding siblings ...)
  2017-07-04 15:18 ` [PATCH 13/13] drm/hisilicon: " Daniel Vetter
@ 2017-07-04 16:39 ` Patchwork
  2017-07-05  5:14 ` ✓ Fi.CI.BAT: success for fbdev locking + deferred setup take 3 (rev2) Patchwork
  14 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2017-07-04 16:39 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: fbdev locking + deferred setup take 3
URL   : https://patchwork.freedesktop.org/series/26819/
State : success

== Summary ==

Series 26819v1 fbdev locking + deferred setup take 3
https://patchwork.freedesktop.org/api/1.0/series/26819/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100125
Test kms_busy:
        Subgroup basic-flip-default-b:
                dmesg-warn -> PASS       (fi-skl-6700hq) fdo#101144
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-pnv-d510) fdo#101597 +1
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-j1900) fdo#101516

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101516 https://bugs.freedesktop.org/show_bug.cgi?id=101516

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:446s
fi-bdw-gvtdvm    total:279  pass:257  dwarn:8   dfail:0   fail:0   skip:14  time:427s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:355s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:533s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:509s
fi-byt-j1900     total:279  pass:255  dwarn:0   dfail:0   fail:0   skip:24  time:492s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:490s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:608s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:438s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:415s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:433s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:497s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:476s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:469s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:572s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:572s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:557s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:455s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:587s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:462s
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:430s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:539s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:407s

24346e831017070c18f3c33b74a7b098682e20f7 drm-tip: 2017y-07m-04d-15h-39m-34s UTC integration manifest
0ac1a92 drm/hisilicon: Remove custom FB helper deferred setup
aef1096 drm/exynos: Remove custom FB helper deferred setup
da41051 drm/i915: Protect against deferred fbdev setup
315f49a drm/fb-helper: Support deferred setup
e0e1d98 drm/fb-helper: Split dpms handling into legacy and atomic paths
d350ff6 drm/fb-helper: Stop using mode_config.mutex for internals
ba01e68 drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy
c154f0c drm/fb-helper: Push locking into pan_display_atomic|legacy
eddd815 drm/fb-helper: Drop locking from the vsync wait ioctl code
05168f2 drm/fb-helper: Push locking in fb_is_bound
44cf723 drm/fb-helper: Add top-level lock
ff551f6 drm/i915: Drop FBDEV #ifdev in mst code
1159cc5 drm/fb-helper: Push down modeset lock into FB helpers

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5108/
_______________________________________________
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: Stop using mode_config.mutex for internals
  2017-07-04 15:18 ` [PATCH 08/13] drm/fb-helper: Stop using mode_config.mutex for internals Daniel Vetter
  2017-07-04 15:40   ` Ville Syrjälä
@ 2017-07-05  4:56   ` Daniel Vetter
  1 sibling, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-05  4:56 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter, Thierry Reding

Those are now all protected using fb_helper->lock.

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

v3: I forgot to hold mode_config.mutex while looking at
connector->status and the mode list. Also, we need to patch up the
i915 ->initial_config callback to grab the locks it needs to inspect
the modeset state recovered from the fw.

v4: Don't reorder the probe too much (Ville).

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
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    | 33 ++++++++-------------------------
 drivers/gpu/drm/drm_vblank.c       |  2 +-
 drivers/gpu/drm/i915/intel_fbdev.c | 16 ++++++++++++----
 3 files changed, 21 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index c5bf37f3bcdd..5d3d776508b3 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;
@@ -1879,12 +1868,11 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe
 EXPORT_SYMBOL(drm_fb_helper_fill_var);
 
 static int drm_fb_helper_probe_connector_modes(struct drm_fb_helper *fb_helper,
-					       uint32_t maxX,
-					       uint32_t maxY)
+						uint32_t maxX,
+						uint32_t maxY)
 {
 	struct drm_connector *connector;
-	int count = 0;
-	int i;
+	int i, count = 0;
 
 	drm_fb_helper_for_each_connector(fb_helper, i) {
 		connector = fb_helper->connector_info[i]->connector;
@@ -2282,12 +2270,8 @@ 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);
-	lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
 
 	crtcs = kcalloc(fb_helper->connector_count,
 			sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL);
@@ -2302,6 +2286,9 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 		goto out;
 	}
 
+	mutex_lock(&fb_helper->dev->mode_config.mutex);
+	if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
+		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
 	drm_enable_connectors(fb_helper, enabled);
 
 	if (!(fb_helper->funcs->initial_config &&
@@ -2323,6 +2310,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 
 		drm_pick_crtcs(fb_helper, crtcs, modes, 0, width, height);
 	}
+	mutex_unlock(&fb_helper->dev->mode_config.mutex);
 
 	/* need to set the modesets up here for use later */
 	/* fill out the connector<->crtc mappings into the modesets */
@@ -2414,12 +2402,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;
@@ -2482,10 +2468,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 bfe86fa2d3b1..70f2b9593edc 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.
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 0c4cde6b2e6f..f11ee039ff45 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -352,14 +352,20 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 	unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG);
 	int i, j;
 	bool *save_enabled;
-	bool fallback = true;
+	bool fallback = true, ret = true;
 	int num_connectors_enabled = 0;
 	int num_connectors_detected = 0;
+	struct drm_modeset_acquire_ctx ctx;
 
 	save_enabled = kcalloc(count, sizeof(bool), GFP_KERNEL);
 	if (!save_enabled)
 		return false;
 
+	drm_modeset_acquire_init(&ctx, 0);
+
+	while (drm_modeset_lock_all_ctx(fb_helper->dev, &ctx) != 0)
+		drm_modeset_backoff(&ctx);
+
 	memcpy(save_enabled, enabled, count);
 	mask = GENMASK(count - 1, 0);
 	conn_configured = 0;
@@ -509,12 +515,14 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 bail:
 		DRM_DEBUG_KMS("Not using firmware configuration\n");
 		memcpy(enabled, save_enabled, count);
-		kfree(save_enabled);
-		return false;
+		ret = false;
 	}
 
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
 	kfree(save_enabled);
-	return true;
+	return ret;
 }
 
 static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
-- 
2.13.2

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

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

* ✓ Fi.CI.BAT: success for fbdev locking + deferred setup take 3 (rev2)
  2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
                   ` (13 preceding siblings ...)
  2017-07-04 16:39 ` ✓ Fi.CI.BAT: success for fbdev locking + deferred setup take 3 Patchwork
@ 2017-07-05  5:14 ` Patchwork
  14 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2017-07-05  5:14 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: fbdev locking + deferred setup take 3 (rev2)
URL   : https://patchwork.freedesktop.org/series/26819/
State : success

== Summary ==

Series 26819v2 fbdev locking + deferred setup take 3
https://patchwork.freedesktop.org/api/1.0/series/26819/revisions/2/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100125
Test kms_busy:
        Subgroup basic-flip-default-b:
                dmesg-warn -> PASS       (fi-skl-6700hq) fdo#101144
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-pnv-d510) fdo#101597

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:439s
fi-bdw-gvtdvm    total:279  pass:257  dwarn:8   dfail:0   fail:0   skip:14  time:426s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:356s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:528s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:505s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:485s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:479s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:595s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:437s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:416s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:414s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:498s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:483s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:473s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:568s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:585s
fi-pnv-d510      total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  time:558s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:454s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:586s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:463s
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:437s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:541s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:405s

24346e831017070c18f3c33b74a7b098682e20f7 drm-tip: 2017y-07m-04d-15h-39m-34s UTC integration manifest
865abf9 drm/hisilicon: Remove custom FB helper deferred setup
56ee0ac drm/exynos: Remove custom FB helper deferred setup
e37ccea drm/i915: Protect against deferred fbdev setup
ef196c3 drm/fb-helper: Support deferred setup
faca99d drm/fb-helper: Split dpms handling into legacy and atomic paths
6c6c028 drm/fb-helper: Stop using mode_config.mutex for internals
93efd50 drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy
2d45204 drm/fb-helper: Push locking into pan_display_atomic|legacy
b9c8043 drm/fb-helper: Drop locking from the vsync wait ioctl code
2a653dd drm/fb-helper: Push locking in fb_is_bound
99cc87c drm/fb-helper: Add top-level lock
26e2fe9 drm/i915: Drop FBDEV #ifdev in mst code
4b91ddd drm/fb-helper: Push down modeset lock into FB helpers

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5110/
_______________________________________________
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 11/13] drm/i915: Protect against deferred fbdev setup
  2017-07-04 15:18 ` [PATCH 11/13] drm/i915: Protect against deferred fbdev setup Daniel Vetter
@ 2017-07-05 10:08   ` Maarten Lankhorst
  2017-07-05 20:17     ` Daniel Vetter
  0 siblings, 1 reply; 25+ messages in thread
From: Maarten Lankhorst @ 2017-07-05 10:08 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development; +Cc: Daniel Vetter, Intel Graphics Development

Op 04-07-17 om 17:18 schreef Daniel Vetter:
> We could probably hit this already with our current async fbdev init,
> but it's much easier to hit this with the new deferred fbdev setup
> that I'm working on polishing.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 580bd4f4a49e..a4224566ebca 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1931,7 +1931,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
>  		return ret;
>  
>  #ifdef CONFIG_DRM_FBDEV_EMULATION
> -	if (dev_priv->fbdev) {
> +	if (dev_priv->fbdev && dev_priv->fbdev->helper.fb) {
>  		fbdev_fb = to_intel_framebuffer(dev_priv->fbdev->helper.fb);
>  
>  		seq_printf(m, "fbcon size: %d x %d, depth %d, %d bpp, modifier 0x%llx, refcount %d, obj ",

Since patch 10/13 makes it way more likely we don't allocate a FB, this patch should be commited before it?

_______________________________________________
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 10/13] drm/fb-helper: Support deferred setup
  2017-07-04 15:18 ` [PATCH 10/13] drm/fb-helper: Support deferred setup Daniel Vetter
  2017-07-04 15:29   ` Liviu Dudau
@ 2017-07-05 11:20   ` Maarten Lankhorst
  1 sibling, 0 replies; 25+ messages in thread
From: Maarten Lankhorst @ 2017-07-05 11:20 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development; +Cc: Intel Graphics Development, Thierry Reding

Op 04-07-17 om 17:18 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).
>
> v6: Add _and_unlock suffix to clarify locking (Maarten)
If you remove intel_fbdev_output_poll_changed's null check for ifbdev->fb,
and apply 11 before this one, then for the first 11 patches:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@intel.com>

It should probably be noted in this commit that this change results in no longer registering the intel fb if no output is connected,
in which case dummy fb is always used.

_______________________________________________
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 11/13] drm/i915: Protect against deferred fbdev setup
  2017-07-05 10:08   ` Maarten Lankhorst
@ 2017-07-05 20:17     ` Daniel Vetter
  0 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-07-05 20:17 UTC (permalink / raw)
  To: Maarten Lankhorst
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	Daniel Vetter

On Wed, Jul 05, 2017 at 12:08:15PM +0200, Maarten Lankhorst wrote:
> Op 04-07-17 om 17:18 schreef Daniel Vetter:
> > We could probably hit this already with our current async fbdev init,
> > but it's much easier to hit this with the new deferred fbdev setup
> > that I'm working on polishing.
> >
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 580bd4f4a49e..a4224566ebca 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -1931,7 +1931,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
> >  		return ret;
> >  
> >  #ifdef CONFIG_DRM_FBDEV_EMULATION
> > -	if (dev_priv->fbdev) {
> > +	if (dev_priv->fbdev && dev_priv->fbdev->helper.fb) {
> >  		fbdev_fb = to_intel_framebuffer(dev_priv->fbdev->helper.fb);
> >  
> >  		seq_printf(m, "fbcon size: %d x %d, depth %d, %d bpp, modifier 0x%llx, refcount %d, obj ",
> 
> Since patch 10/13 makes it way more likely we don't allocate a FB, this patch should be commited before it?

Yeah I think I need to reshuffle, and add the patch to remove the
ifbdev->vma check in output_changed too. I think I'll merge patches 1-9
tomorrow meanwhile.

Thanks a lot for your review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
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 12/13] drm/exynos: Remove custom FB helper deferred setup
  2017-07-04 15:18 ` [PATCH 12/13] drm/exynos: Remove custom FB helper deferred setup Daniel Vetter
@ 2017-07-06  3:16   ` Inki Dae
  0 siblings, 0 replies; 25+ messages in thread
From: Inki Dae @ 2017-07-06  3:16 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Kyungmin Park, Intel Graphics Development, Thierry Reding, Seung-Woo Kim



2017년 07월 05일 00:18에 Daniel Vetter 이(가) 쓴 글:
> From: Thierry Reding <treding@nvidia.com>
> 
> The FB helper core now supports deferred setup, so the driver's custom
> implementation can be removed.

Reviewed-by: Inki Dae <inki.dae@samsung.com>
Tested-by: Inki Dae <inki.dae@samsung.com>

Thanks,
Inki Dae

> 
> 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);
>  }
> 
_______________________________________________
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/13] drm/fb-helper: Push locking in fb_is_bound
  2017-06-27 14:59 [PATCH 00/13] fbdev locking rework and deferred setup, take 2 Daniel Vetter
@ 2017-06-27 14:59 ` Daniel Vetter
  0 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-06-27 14:59 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Thierry Reding,
	John Stultz, 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

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

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

end of thread, other threads:[~2017-07-06  3:17 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-04 15:18 [PATCH 00/13] fbdev locking + deferred setup take 3 Daniel Vetter
2017-07-04 15:18 ` [PATCH 01/13] drm/fb-helper: Push down modeset lock into FB helpers Daniel Vetter
2017-07-04 15:18 ` [PATCH 02/13] drm/i915: Drop FBDEV #ifdev in mst code Daniel Vetter
2017-07-04 15:18 ` [PATCH 03/13] drm/fb-helper: Add top-level lock Daniel Vetter
2017-07-04 15:18 ` [PATCH 04/13] drm/fb-helper: Push locking in fb_is_bound Daniel Vetter
2017-07-04 15:18 ` [PATCH 05/13] drm/fb-helper: Drop locking from the vsync wait ioctl code Daniel Vetter
2017-07-04 15:18 ` [PATCH 06/13] drm/fb-helper: Push locking into pan_display_atomic|legacy Daniel Vetter
2017-07-04 15:18 ` [PATCH 07/13] drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy Daniel Vetter
2017-07-04 15:18 ` [PATCH 08/13] drm/fb-helper: Stop using mode_config.mutex for internals Daniel Vetter
2017-07-04 15:40   ` Ville Syrjälä
2017-07-04 16:19     ` Daniel Vetter
2017-07-05  4:56   ` [PATCH] " Daniel Vetter
2017-07-04 15:18 ` [PATCH 09/13] drm/fb-helper: Split dpms handling into legacy and atomic paths Daniel Vetter
2017-07-04 15:18 ` [PATCH 10/13] drm/fb-helper: Support deferred setup Daniel Vetter
2017-07-04 15:29   ` Liviu Dudau
2017-07-05 11:20   ` Maarten Lankhorst
2017-07-04 15:18 ` [PATCH 11/13] drm/i915: Protect against deferred fbdev setup Daniel Vetter
2017-07-05 10:08   ` Maarten Lankhorst
2017-07-05 20:17     ` Daniel Vetter
2017-07-04 15:18 ` [PATCH 12/13] drm/exynos: Remove custom FB helper deferred setup Daniel Vetter
2017-07-06  3:16   ` Inki Dae
2017-07-04 15:18 ` [PATCH 13/13] drm/hisilicon: " Daniel Vetter
2017-07-04 16:39 ` ✓ Fi.CI.BAT: success for fbdev locking + deferred setup take 3 Patchwork
2017-07-05  5:14 ` ✓ Fi.CI.BAT: success for fbdev locking + deferred setup take 3 (rev2) Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2017-06-27 14:59 [PATCH 00/13] fbdev locking rework and deferred setup, take 2 Daniel Vetter
2017-06-27 14:59 ` [PATCH 04/13] drm/fb-helper: Push locking in fb_is_bound 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.