[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <871sv7tw0y.fsf@intel.com>
Date: Thu, 09 Feb 2017 13:59:25 +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 v2] drm/i915/debugfs: Add i915_hpd_storm_ctl
On Thu, 09 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.
>
> Changes since v1:
> - Make HPD storm interval configurable
> - Misc code cleanup
>
> Signed-off-by: Lyude <lyude@...hat.com>
> Cc: Jani Nikula <jani.nikula@...ux.intel.com>
> Cc: Tomeu Vizoso <tomeu@...euvizoso.net>
Acked-by: Jani Nikula <jani.nikula@...el.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 78 +++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/i915_drv.c | 1 +
> drivers/gpu/drm/i915/i915_drv.h | 4 ++
> drivers/gpu/drm/i915/i915_irq.c | 2 +
> drivers/gpu/drm/i915/intel_hotplug.c | 15 ++++---
> 5 files changed, 94 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1ccc297..b55547d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4641,6 +4641,81 @@ 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;
> + struct i915_hotplug *hotplug = &dev_priv->hotplug;
> +
> + seq_printf(m, "Threshold: %d\n", hotplug->hpd_storm_threshold);
> + seq_printf(m, "Detected: %s\n",
> + yesno(delayed_work_pending(&hotplug->reenable_work)));
> +
> + return 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;
> + unsigned int new_threshold;
> + int i;
> + char *newline;
> + char tmp[16];
> +
> + if (len >= sizeof(tmp))
> + return -EINVAL;
> +
> + if (copy_from_user(tmp, ubuf, len))
> + return -EFAULT;
> +
> + tmp[len] = '\0';
> +
> + /* Strip newline, if any */
> + newline = strchr(tmp, '\n');
> + if (newline)
> + *newline = '\0';
> +
> + if (strcmp(tmp, "reset") == 0)
> + new_threshold = HPD_STORM_DEFAULT_THRESHOLD;
> + else if (kstrtouint(tmp, 10, &new_threshold) != 0)
> + return -EINVAL;
> +
> + if (new_threshold > 0)
> + DRM_DEBUG_KMS("Setting HPD storm detection threshold to %d\n",
> + new_threshold);
> + else
> + DRM_DEBUG_KMS("Disabling HPD storm detection\n");
> +
> + spin_lock_irq(&dev_priv->irq_lock);
> + hotplug->hpd_storm_threshold = new_threshold;
> + /* Reset the HPD storm stats so we don't accidentally trigger a storm */
> + for_each_hpd_pin(i)
> + hotplug->stats[i].count = 0;
> + spin_unlock_irq(&dev_priv->irq_lock);
> +
> + /* Re-enable hpd immediately if we were in an irq storm */
> + flush_delayed_work(&dev_priv->hotplug.reenable_work);
> +
> + 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,
> @@ -4734,7 +4809,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.c b/drivers/gpu/drm/i915/i915_drv.c
> index d48c02a7..f23b568 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -827,6 +827,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
> spin_lock_init(&dev_priv->gpu_error.lock);
> mutex_init(&dev_priv->backlight_lock);
> spin_lock_init(&dev_priv->uncore.lock);
> +
> spin_lock_init(&dev_priv->mm.object_stat_lock);
> spin_lock_init(&dev_priv->mmio_flip_lock);
> spin_lock_init(&dev_priv->wm.dsparb_lock);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 604b20f..2b7b317 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -384,6 +384,8 @@ enum hpd_pin {
> #define for_each_hpd_pin(__pin) \
> for ((__pin) = (HPD_NONE + 1); (__pin) < HPD_NUM_PINS; (__pin)++)
>
> +#define HPD_STORM_DEFAULT_THRESHOLD 5
> +
> struct i915_hotplug {
> struct work_struct hotplug_work;
>
> @@ -407,6 +409,8 @@ struct i915_hotplug {
> struct work_struct poll_init_work;
> bool poll_enabled;
>
> + unsigned int hpd_storm_threshold;
> +
> /*
> * 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 9eb4c9c..27c13193 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4277,6 +4277,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_threshold = HPD_STORM_DEFAULT_THRESHOLD;
> +
> 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..6a9c165 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -100,7 +100,6 @@ bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port)
> }
>
> #define HPD_STORM_DETECT_PERIOD 1000
> -#define HPD_STORM_THRESHOLD 5
> #define HPD_STORM_REENABLE_DELAY (2 * 60 * 1000)
>
> /**
> @@ -112,9 +111,13 @@ bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port)
> * storms. Only the pin specific stats and state are changed, the caller is
> * responsible for further action.
> *
> - * @HPD_STORM_THRESHOLD irqs are allowed within @HPD_STORM_DETECT_PERIOD ms,
> - * otherwise it's considered an irq storm, and the irq state is set to
> - * @HPD_MARK_DISABLED.
> + * The number of irqs that are allowed within @HPD_STORM_DETECT_PERIOD is
> + * stored in @dev_priv->hotplug.hpd_storm_threshold which defaults to
> + * @HPD_STORM_DEFAULT_THRESHOLD. If this threshold is exceeded, it's
> + * considered an irq storm and the irq state is set to @HPD_MARK_DISABLED.
> + *
> + * The HPD threshold can be controlled through i915_hpd_storm_ctl in debugfs,
> + * and should only be adjusted for automated hotplug testing.
> *
> * Return true if an irq storm was detected on @pin.
> */
> @@ -123,13 +126,15 @@ static bool intel_hpd_irq_storm_detect(struct drm_i915_private *dev_priv,
> {
> unsigned long start = dev_priv->hotplug.stats[pin].last_jiffies;
> unsigned long end = start + msecs_to_jiffies(HPD_STORM_DETECT_PERIOD);
> + const int threshold = dev_priv->hotplug.hpd_storm_threshold;
> bool storm = false;
>
> if (!time_in_range(jiffies, start, end)) {
> 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 > threshold &&
> + threshold) {
> 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