* [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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ messages in thread
end of thread, other threads:[~2017-07-06 3:17 UTC | newest]
Thread overview: 24+ 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
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.