[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <87tw86unib.fsf@intel.com>
Date: Tue, 07 Feb 2017 15:41:16 +0200
From: Jani Nikula <jani.nikula@...ux.intel.com>
To: Lyude <lyude@...hat.com>, intel-gfx@...ts.freedesktop.org
Cc: Lyude <lyude@...hat.com>, Tomeu Vizoso <tomeu@...euvizoso.net>,
Daniel Vetter <daniel.vetter@...el.com>,
David Airlie <airlied@...ux.ie>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/i915/debugfs: Add i915_hpd_storm_ctl
On Sat, 04 Feb 2017, Lyude <lyude@...hat.com> wrote:
> This adds a file in i915's debugfs directory that allows userspace to
> manually control HPD storm detection. This is mainly for hotplugging
> tests, where we might want to test HPD storm functionality or disable
> storm detection to speed up hotplugging tests without breaking anything.
>
> Signed-off-by: Lyude <lyude@...hat.com>
> Cc: Tomeu Vizoso <tomeu@...euvizoso.net>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 102 ++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_irq.c | 2 +
> drivers/gpu/drm/i915/intel_hotplug.c | 3 +-
> 4 files changed, 107 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3ae0656..b985c7b 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4624,6 +4624,105 @@ static int i915_forcewake_create(struct dentry *root, struct drm_minor *minor)
> return drm_add_fake_info_node(minor, ent, &i915_forcewake_fops);
> }
>
> +static int i915_hpd_storm_ctl_show(struct seq_file *m, void *data)
> +{
> + struct drm_i915_private *dev_priv = m->private;
> +
> + if (delayed_work_pending(&dev_priv->hotplug.reenable_work))
> + seq_printf(m, "detected\n");
> + else if (dev_priv->hotplug.hpd_storm_detect_enabled)
> + seq_printf(m, "on\n");
> + else
> + seq_printf(m, "off\n");
> +
> + return 0;
> +}
> +
> +static void i915_hpd_storm_reset(struct drm_i915_private *dev_priv)
> +{
> + int i;
> +
> + spin_lock_irq(&dev_priv->irq_lock);
> + for (i = 0; i < ARRAY_SIZE(dev_priv->hotplug.stats); i++) {
> + dev_priv->hotplug.stats[i].count = 0;
> + dev_priv->hotplug.stats[i].last_jiffies = jiffies;
> + }
> + spin_unlock_irq(&dev_priv->irq_lock);
> +
> + if (delayed_work_pending(&dev_priv->hotplug.reenable_work))
> + flush_delayed_work(&dev_priv->hotplug.reenable_work);
> + else
> + schedule_delayed_work(&dev_priv->hotplug.reenable_work, 0);
> +}
> +
> +static ssize_t i915_hpd_storm_ctl_write(struct file *file,
> + const char __user *ubuf, size_t len,
> + loff_t *offp)
> +{
> + struct seq_file *m = file->private_data;
> + struct drm_i915_private *dev_priv = m->private;
> + struct i915_hotplug *hotplug = &dev_priv->hotplug;
> + char tmp[16];
> + enum {
> + HPD_STORM_DISABLE = 0,
> + HPD_STORM_ENABLE,
> + HPD_STORM_RESET
> + } action;
> +
> + if (len >= sizeof(tmp))
> + return -EINVAL;
> +
> + if (copy_from_user(tmp, ubuf, len))
> + return -EFAULT;
> +
> + tmp[len] = '\0';
> +
> + if (strcmp(tmp, "off") == 0 || strcmp(tmp, "off\n") == 0)
> + action = HPD_STORM_DISABLE;
> + else if (strcmp(tmp, "on") == 0 || strcmp(tmp, "on\n") == 0)
> + action = HPD_STORM_ENABLE;
> + else if (strcmp(tmp, "reset") == 0 || strcmp(tmp, "reset\n") == 0)
> + action = HPD_STORM_RESET;
> + else
> + return -EINVAL;
> +
> + switch (action) {
> + case HPD_STORM_DISABLE:
> + DRM_DEBUG_KMS("Disabling HPD storm detection\n");
> +
> + WRITE_ONCE(hotplug->hpd_storm_detect_enabled, false);
> + i915_hpd_storm_reset(dev_priv);
> + break;
> + case HPD_STORM_ENABLE:
> + DRM_DEBUG_KMS("Enabling HPD storm detection\n");
> +
> + i915_hpd_storm_reset(dev_priv);
> + hotplug->hpd_storm_detect_enabled = true;
> + break;
> + case HPD_STORM_RESET:
> + DRM_DEBUG_KMS("Resetting HPD storm stats\n");
> +
> + i915_hpd_storm_reset(dev_priv);
> + break;
> + }
> +
> + return len;
> +}
> +
> +static int i915_hpd_storm_ctl_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, i915_hpd_storm_ctl_show, inode->i_private);
> +}
> +
> +static const struct file_operations i915_hpd_storm_ctl_fops = {
> + .owner = THIS_MODULE,
> + .open = i915_hpd_storm_ctl_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = single_release,
> + .write = i915_hpd_storm_ctl_write
> +};
> +
> static int i915_debugfs_create(struct dentry *root,
> struct drm_minor *minor,
> const char *name,
> @@ -4717,7 +4816,8 @@ static const struct i915_debugfs_files {
> {"i915_dp_test_data", &i915_displayport_test_data_fops},
> {"i915_dp_test_type", &i915_displayport_test_type_fops},
> {"i915_dp_test_active", &i915_displayport_test_active_fops},
> - {"i915_guc_log_control", &i915_guc_log_control_fops}
> + {"i915_guc_log_control", &i915_guc_log_control_fops},
> + {"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops}
> };
>
> int i915_debugfs_register(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bd5a38b..b11596f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -407,6 +407,8 @@ struct i915_hotplug {
> struct work_struct poll_init_work;
> bool poll_enabled;
>
> + bool hpd_storm_detect_enabled;
> +
> /*
> * if we get a HPD irq from DP and a HPD irq from non-DP
> * the non-DP HPD could block the workqueue on a mode config
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 059d66b..ed3292d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4225,6 +4225,8 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> if (!IS_GEN2(dev_priv))
> dev->vblank_disable_immediate = true;
>
> + dev_priv->hotplug.hpd_storm_detect_enabled = true;
> +
> dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp;
> dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index b62e3f8..c1e37dd 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -129,7 +129,8 @@ static bool intel_hpd_irq_storm_detect(struct drm_i915_private *dev_priv,
> dev_priv->hotplug.stats[pin].last_jiffies = jiffies;
> dev_priv->hotplug.stats[pin].count = 0;
> DRM_DEBUG_KMS("Received HPD interrupt on PIN %d - cnt: 0\n", pin);
> - } else if (dev_priv->hotplug.stats[pin].count > HPD_STORM_THRESHOLD) {
> + } else if (dev_priv->hotplug.stats[pin].count > HPD_STORM_THRESHOLD &&
> + dev_priv->hotplug.hpd_storm_detect_enabled) {
How about making the threshold configurable, with 0 being off?
BR,
Jani.
PS. I do wonder if anyone besides Egbert has really seen this much in
the wild...
> dev_priv->hotplug.stats[pin].state = HPD_MARK_DISABLED;
> DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", pin);
> storm = true;
--
Jani Nikula, Intel Open Source Technology Center
Powered by blists - more mailing lists