Polling for a VGA device on an old system can be quite expensive, causing latencies on the order of 600ms. As we hold the mode mutex for this time and also need the same mutex to move the cursor, we trigger a user-visible stall.
The real solution would involve improving the granulatity of the locking and so perhaps performing some of the probing not under the lock or some other updates can be done under different locks. Also reducing the cost of probing for a non-existent monitor would be worthwhile. However, exposing a parameter to disable polling is a simple workaround in the meantime.
In order to accommodate users turning polling on and off at runtime, the polling is potentially re-enabled on every probe. This is coupled to the user calling xrandr, which seems to be a vaild time to reset the polling timeout since the information on the connection has just been updated. (The presumption being that all connections are probed in a single xrandr pass, which is currently valid.)
References:
Bug 29536 - 2.6.35 causes ~600ms latency every 10s https://bugs.freedesktop.org/show_bug.cgi?id=29536
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Reported-and-tested-by: Bruno Prémont bonbons@linux-vserver.org --- drivers/gpu/drm/drm_crtc_helper.c | 17 +++++++++++++++-- 1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index a0ab1d1..13ba313 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -34,6 +34,9 @@ #include "drm_crtc_helper.h" #include "drm_fb_helper.h"
+static bool drm_kms_helper_poll = true; +module_param_named(poll, drm_kms_helper_poll, bool, 0600); + static void drm_mode_validate_flag(struct drm_connector *connector, int flags) { @@ -99,8 +102,10 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, connector->status = connector_status_disconnected; if (connector->funcs->force) connector->funcs->force(connector); - } else + } else { connector->status = connector->funcs->detect(connector); + drm_helper_hpd_irq_event(dev); + }
if (connector->status == connector_status_disconnected) { DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", @@ -982,6 +987,9 @@ static void output_poll_execute(struct work_struct *work) enum drm_connector_status old_status, status; bool repoll = false, changed = false;
+ if (!drm_kms_helper_poll) + return; + mutex_lock(&dev->mode_config.mutex); list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
@@ -1032,6 +1040,9 @@ void drm_kms_helper_poll_enable(struct drm_device *dev) bool poll = false; struct drm_connector *connector;
+ if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll) + return; + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { if (connector->polled) poll = true; @@ -1061,8 +1072,10 @@ void drm_helper_hpd_irq_event(struct drm_device *dev) { if (!dev->mode_config.poll_enabled) return; + /* kill timer and schedule immediate execution, this doesn't block */ cancel_delayed_work(&dev->mode_config.output_poll_work); - queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0); + if (drm_kms_helper_poll) + queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0); } EXPORT_SYMBOL(drm_helper_hpd_irq_event);
On Thu, Aug 12, 2010 at 3:57 PM, Chris Wilson chris@chris-wilson.co.uk wrote:
Polling for a VGA device on an old system can be quite expensive, causing latencies on the order of 600ms. As we hold the mode mutex for this time and also need the same mutex to move the cursor, we trigger a user-visible stall.
The real solution would involve improving the granulatity of the locking and so perhaps performing some of the probing not under the lock or some other updates can be done under different locks. Also reducing the cost of probing for a non-existent monitor would be worthwhile. However, exposing a parameter to disable polling is a simple workaround in the meantime.
In order to accommodate users turning polling on and off at runtime, the polling is potentially re-enabled on every probe. This is coupled to the user calling xrandr, which seems to be a vaild time to reset the polling timeout since the information on the connection has just been updated. (The presumption being that all connections are probed in a single xrandr pass, which is currently valid.)
References:
Bug 29536 - 2.6.35 causes ~600ms latency every 10s https://bugs.freedesktop.org/show_bug.cgi?id=29536
Should also fixe: https://bugs.freedesktop.org/show_bug.cgi?id=29433
Alex
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Reported-and-tested-by: Bruno Prémont bonbons@linux-vserver.org
drivers/gpu/drm/drm_crtc_helper.c | 17 +++++++++++++++-- 1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index a0ab1d1..13ba313 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -34,6 +34,9 @@ #include "drm_crtc_helper.h" #include "drm_fb_helper.h"
+static bool drm_kms_helper_poll = true; +module_param_named(poll, drm_kms_helper_poll, bool, 0600);
static void drm_mode_validate_flag(struct drm_connector *connector, int flags) { @@ -99,8 +102,10 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, connector->status = connector_status_disconnected; if (connector->funcs->force) connector->funcs->force(connector);
- } else
- } else {
connector->status = connector->funcs->detect(connector);
- drm_helper_hpd_irq_event(dev);
- }
if (connector->status == connector_status_disconnected) { DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", @@ -982,6 +987,9 @@ static void output_poll_execute(struct work_struct *work) enum drm_connector_status old_status, status; bool repoll = false, changed = false;
- if (!drm_kms_helper_poll)
- return;
mutex_lock(&dev->mode_config.mutex); list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
@@ -1032,6 +1040,9 @@ void drm_kms_helper_poll_enable(struct drm_device *dev) bool poll = false; struct drm_connector *connector;
- if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
- return;
list_for_each_entry(connector, &dev->mode_config.connector_list, head) { if (connector->polled) poll = true; @@ -1061,8 +1072,10 @@ void drm_helper_hpd_irq_event(struct drm_device *dev) { if (!dev->mode_config.poll_enabled) return;
/* kill timer and schedule immediate execution, this doesn't block */ cancel_delayed_work(&dev->mode_config.output_poll_work);
- queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0);
- if (drm_kms_helper_poll)
- queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0);
} EXPORT_SYMBOL(drm_helper_hpd_irq_event); -- 1.7.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, 2010-08-12 at 20:57 +0100, Chris Wilson wrote:
Polling for a VGA device on an old system can be quite expensive, causing latencies on the order of 600ms. As we hold the mode mutex for this time and also need the same mutex to move the cursor, we trigger a user-visible stall.
You should rebase this on -rc1 or above since the stuff in the kernel changed and we don't use the slow work stuff anymore.
Dave.
The real solution would involve improving the granulatity of the locking and so perhaps performing some of the probing not under the lock or some other updates can be done under different locks. Also reducing the cost of probing for a non-existent monitor would be worthwhile. However, exposing a parameter to disable polling is a simple workaround in the meantime.
In order to accommodate users turning polling on and off at runtime, the polling is potentially re-enabled on every probe. This is coupled to the user calling xrandr, which seems to be a vaild time to reset the polling timeout since the information on the connection has just been updated. (The presumption being that all connections are probed in a single xrandr pass, which is currently valid.)
References:
Bug 29536 - 2.6.35 causes ~600ms latency every 10s https://bugs.freedesktop.org/show_bug.cgi?id=29536
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Reported-and-tested-by: Bruno Prémont bonbons@linux-vserver.org
drivers/gpu/drm/drm_crtc_helper.c | 17 +++++++++++++++-- 1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index a0ab1d1..13ba313 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -34,6 +34,9 @@ #include "drm_crtc_helper.h" #include "drm_fb_helper.h"
+static bool drm_kms_helper_poll = true; +module_param_named(poll, drm_kms_helper_poll, bool, 0600);
static void drm_mode_validate_flag(struct drm_connector *connector, int flags) { @@ -99,8 +102,10 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, connector->status = connector_status_disconnected; if (connector->funcs->force) connector->funcs->force(connector);
- } else
} else { connector->status = connector->funcs->detect(connector);
drm_helper_hpd_irq_event(dev);}
if (connector->status == connector_status_disconnected) { DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n",
@@ -982,6 +987,9 @@ static void output_poll_execute(struct work_struct *work) enum drm_connector_status old_status, status; bool repoll = false, changed = false;
- if (!drm_kms_helper_poll)
return;- mutex_lock(&dev->mode_config.mutex); list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
@@ -1032,6 +1040,9 @@ void drm_kms_helper_poll_enable(struct drm_device *dev) bool poll = false; struct drm_connector *connector;
- if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
return;- list_for_each_entry(connector, &dev->mode_config.connector_list, head) { if (connector->polled) poll = true;
@@ -1061,8 +1072,10 @@ void drm_helper_hpd_irq_event(struct drm_device *dev) { if (!dev->mode_config.poll_enabled) return;
- /* kill timer and schedule immediate execution, this doesn't block */ cancel_delayed_work(&dev->mode_config.output_poll_work);
- queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0);
- if (drm_kms_helper_poll)
queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0);} EXPORT_SYMBOL(drm_helper_hpd_irq_event);
Polling for a VGA device on an old system can be quite expensive, causing latencies on the order of 600ms. As we hold the mode mutex for this time and also need the same mutex to move the cursor, we trigger a user-visible stall.
The real solution would involve improving the granulatity of the locking and so perhaps performing some of the probing not under the lock or some other updates can be done under different locks. Also reducing the cost of probing for a non-existent monitor would be worthwhile. However, exposing a parameter to disable polling is a simple workaround in the meantime.
In order to accommodate users turning polling on and off at runtime, the polling is potentially re-enabled on every probe. This is coupled to the user calling xrandr, which seems to be a vaild time to reset the polling timeout since the information on the connection has just been updated. (The presumption being that all connections are probed in a single xrandr pass, which is currently valid.)
References:
Bug 29536 - 2.6.35 causes ~600ms latency every 10s https://bugs.freedesktop.org/show_bug.cgi?id=29536
Bug 16265 - Why is kslowd accumulating so much CPU time? https://bugzilla.kernel.org/show_bug.cgi?id=16265
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Reported-and-tested-by: Bruno Prémont bonbons@linux-vserver.org --- drivers/gpu/drm/drm_crtc_helper.c | 17 +++++++++++++++-- 1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 7e31d43..06fc0bc 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -34,6 +34,9 @@ #include "drm_crtc_helper.h" #include "drm_fb_helper.h"
+static bool drm_kms_helper_poll = true; +module_param_named(poll, drm_kms_helper_poll, bool, 0600); + static void drm_mode_validate_flag(struct drm_connector *connector, int flags) { @@ -99,8 +102,10 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, connector->status = connector_status_disconnected; if (connector->funcs->force) connector->funcs->force(connector); - } else + } else { connector->status = connector->funcs->detect(connector); + drm_helper_hpd_irq_event(dev); + }
if (connector->status == connector_status_disconnected) { DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", @@ -840,6 +845,9 @@ static void output_poll_execute(struct work_struct *work) enum drm_connector_status old_status, status; bool repoll = false, changed = false;
+ if (!drm_kms_helper_poll) + return; + mutex_lock(&dev->mode_config.mutex); list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
@@ -890,6 +898,9 @@ void drm_kms_helper_poll_enable(struct drm_device *dev) bool poll = false; struct drm_connector *connector;
+ if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll) + return; + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { if (connector->polled) poll = true; @@ -919,8 +930,10 @@ void drm_helper_hpd_irq_event(struct drm_device *dev) { if (!dev->mode_config.poll_enabled) return; + /* kill timer and schedule immediate execution, this doesn't block */ cancel_delayed_work(&dev->mode_config.output_poll_work); - queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0); + if (drm_kms_helper_poll) + queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0); } EXPORT_SYMBOL(drm_helper_hpd_irq_event);
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Chris Wilson -
Dave Airlie