Re: [PATCH] drm/kms: Add a module parameter to disable polling

[Dave Airlie <airlied@redhat.com>]

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


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