On Tue, 09 Apr 2013, Egbert Eich <eich at freedesktop.org> wrote:
> From: Egbert Eich <eich at suse.de>
>
> We disable hoptplug detection when we encounter a hotplug event
> storm. Still hotplug detection is required on some outputs (like
> Display Port). The interrupt storm may be only temporary (on certain
> Dell Laptops for instance it happens at certain charging states of
> the system). Thus we enable it after a certain grace period (2 minutes).
> Should the interrupt storm persist it will be detected immediately
> and it will be disabled again.
>
> v2: Reordered drm_i915_private: moved hotplug_reenable_timer to hpd state tracker.
> v3: Clarified loop start value,
> Removed superfluous test for Ivybridge and Haswell,
> Restructured loop to avoid deep nesting (all suggested by Ville Syrj?l?)
>
> Signed-off-by: Egbert Eich <eich at suse.de>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_irq.c | 49 ++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 83fc1a6..a3ed2e3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -938,6 +938,8 @@ typedef struct drm_i915_private {
> HPD_MARK_DISABLED = 2
> } hpd_mark;
> } hpd_stats[HPD_NUM_PINS];
> +#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000)
It's already too crowded here, please move this #define to i915_irq.c.
> + struct timer_list hotplug_reenable_timer;
>
> int num_pch_pll;
> int num_plane;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d0e6f6d..1a00533 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -371,8 +371,11 @@ static void i915_hotplug_work_func(struct work_struct *work)
> connector_disabled = true;
> }
> }
> - if (connector_disabled)
> + if (connector_disabled) {
> drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */
> + mod_timer(&dev_priv->hotplug_reenable_timer,
> + jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
> + }
>
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
> @@ -2348,6 +2351,8 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
> if (!dev_priv)
> return;
>
> + del_timer_sync(&dev_priv->hotplug_reenable_timer);
> +
> for_each_pipe(pipe)
> I915_WRITE(PIPESTAT(pipe), 0xffff);
>
> @@ -2369,6 +2374,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
> if (!dev_priv)
> return;
>
> + del_timer_sync(&dev_priv->hotplug_reenable_timer);
> +
> I915_WRITE(HWSTAM, 0xffffffff);
>
> I915_WRITE(DEIMR, 0xffffffff);
> @@ -2745,6 +2752,8 @@ static void i915_irq_uninstall(struct drm_device * dev)
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> int pipe;
>
> + del_timer_sync(&dev_priv->hotplug_reenable_timer);
> +
> if (I915_HAS_HOTPLUG(dev)) {
> I915_WRITE(PORT_HOTPLUG_EN, 0);
> I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
> @@ -2989,6 +2998,8 @@ static void i965_irq_uninstall(struct drm_device * dev)
> if (!dev_priv)
> return;
>
> + del_timer_sync(&dev_priv->hotplug_reenable_timer);
> +
> I915_WRITE(PORT_HOTPLUG_EN, 0);
> I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
>
> @@ -3004,6 +3015,40 @@ static void i965_irq_uninstall(struct drm_device * dev)
> I915_WRITE(IIR, I915_READ(IIR));
> }
>
> +static void i915_reenable_hotplug_timer_func(unsigned long data)
> +{
> + drm_i915_private_t *dev_priv = (drm_i915_private_t *)data;
> + struct drm_device *dev = dev_priv->dev;
> + struct drm_mode_config *mode_config = &dev->mode_config;
> + unsigned long irqflags;
> + int i;
> +
> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> + for (i = (HPD_NONE + 1); i < HPD_NUM_PINS; i++) {
> + struct drm_connector *connector;
> +
> + if (dev_priv->hpd_stats[i].hpd_mark != HPD_MARK_DISABLED)
I think this is wrong, skipping HPD_DISABLED.
> + continue;
> +
> + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED;
> +
> + list_for_each_entry(connector, &mode_config->connector_list, head) {
> + struct intel_connector *intel_connector = to_intel_connector(connector);
> +
> + if (intel_connector->encoder->hpd_pin == i) {
> + if (connector->polled != intel_connector->polled)
> + DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
> + drm_get_connector_name(connector));
> + connector->polled = intel_connector->polled;
> + if (!connector->polled)
> + connector->polled = DRM_CONNECTOR_POLL_HPD;
> + }
> + }
> + dev_priv->display.hpd_irq_setup(dev);
You don't need to call this at each iteration, do you?
> + }
In fact, couldn't you just call intel_hpd_init(), or modify it to do
what you want? Keep it simple.
> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> +}
> +
> void intel_irq_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3016,6 +3061,8 @@ void intel_irq_init(struct drm_device *dev)
> setup_timer(&dev_priv->gpu_error.hangcheck_timer,
> i915_hangcheck_elapsed,
> (unsigned long) dev);
> + setup_timer(&dev_priv->hotplug_reenable_timer, i915_reenable_hotplug_timer_func,
> + (unsigned long) dev_priv);
>
> pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
>
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
