From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Lyude Paul <lyude@redhat.com>
Cc: David Airlie <airlied@linux.ie>,
intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v3 4/5] drm/i915: Clarify flow for disabling IRQs on storms
Date: Mon, 5 Nov 2018 12:38:40 -0800 [thread overview]
Message-ID: <20181105203840.GC4208@intel.com> (raw)
In-Reply-To: <20181103001909.23645-5-lyude@redhat.com>
On Fri, Nov 02, 2018 at 08:19:07PM -0400, Lyude Paul wrote:
> This is rather confusing to look at as-is:
> dev_priv->display.hpd_irq_setup(dev_priv); in intel_hpd_irq_handler()
> handles disabling the actual HPD IRQ, while
> intel_hpd_irq_storm_disable() handles moving the HPD pin state over from
> MARK_DISABLED to DISABLED along with enabling polling for it.
what about also changing the name of the function?
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
> drivers/gpu/drm/i915/intel_hotplug.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index c11d73de16f2..e5e3eeb7e482 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -351,7 +351,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
> hpd_event_bits = dev_priv->hotplug.event_bits;
> dev_priv->hotplug.event_bits = 0;
>
> - /* Disable hotplug on connectors that hit an irq storm. */
> + /* Enable polling for connectors which had HPD IRQ storms */
> intel_hpd_irq_storm_disable(dev_priv);
This now gets confusing to my brain...
comment says "enable" function name is "disable".
>
> spin_unlock_irq(&dev_priv->irq_lock);
> @@ -458,6 +458,10 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> }
> }
>
> + /*
> + * Disable any IRQs that storms were detected on. Polling enablement
> + * happens later in our hotplug work.
> + */
> if (storm_detected && dev_priv->display_irqs_enabled)
> dev_priv->display.hpd_irq_setup(dev_priv);
> spin_unlock(&dev_priv->irq_lock);
> --
> 2.19.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Lyude Paul <lyude@redhat.com>
Cc: intel-gfx@lists.freedesktop.org, David Airlie <airlied@linux.ie>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v3 4/5] drm/i915: Clarify flow for disabling IRQs on storms
Date: Mon, 5 Nov 2018 12:38:40 -0800 [thread overview]
Message-ID: <20181105203840.GC4208@intel.com> (raw)
In-Reply-To: <20181103001909.23645-5-lyude@redhat.com>
On Fri, Nov 02, 2018 at 08:19:07PM -0400, Lyude Paul wrote:
> This is rather confusing to look at as-is:
> dev_priv->display.hpd_irq_setup(dev_priv); in intel_hpd_irq_handler()
> handles disabling the actual HPD IRQ, while
> intel_hpd_irq_storm_disable() handles moving the HPD pin state over from
> MARK_DISABLED to DISABLED along with enabling polling for it.
what about also changing the name of the function?
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
> drivers/gpu/drm/i915/intel_hotplug.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index c11d73de16f2..e5e3eeb7e482 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -351,7 +351,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
> hpd_event_bits = dev_priv->hotplug.event_bits;
> dev_priv->hotplug.event_bits = 0;
>
> - /* Disable hotplug on connectors that hit an irq storm. */
> + /* Enable polling for connectors which had HPD IRQ storms */
> intel_hpd_irq_storm_disable(dev_priv);
This now gets confusing to my brain...
comment says "enable" function name is "disable".
>
> spin_unlock_irq(&dev_priv->irq_lock);
> @@ -458,6 +458,10 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> }
> }
>
> + /*
> + * Disable any IRQs that storms were detected on. Polling enablement
> + * happens later in our hotplug work.
> + */
> if (storm_detected && dev_priv->display_irqs_enabled)
> dev_priv->display.hpd_irq_setup(dev_priv);
> spin_unlock(&dev_priv->irq_lock);
> --
> 2.19.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2018-11-05 20:38 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-03 0:19 [PATCH v3 0/5] drm/i915: HPD IRQ storm detection fixes Lyude Paul
2018-11-03 0:19 ` [PATCH v3 1/5] drm/i915: Fix possible race in intel_dp_add_mst_connector() Lyude Paul
2018-11-03 0:19 ` [PATCH v3 2/5] drm/i915: Fix NULL deref when re-enabling HPD IRQs on systems with MST Lyude Paul
2018-11-03 0:19 ` [PATCH v3 3/5] drm/i915: Fix threshold check in intel_hpd_irq_storm_detect() Lyude Paul
2018-11-03 0:19 ` Lyude Paul
2018-11-03 0:19 ` [PATCH v3 4/5] drm/i915: Clarify flow for disabling IRQs on storms Lyude Paul
2018-11-05 20:38 ` Rodrigo Vivi [this message]
2018-11-05 20:38 ` [Intel-gfx] " Rodrigo Vivi
2018-11-03 0:19 ` [PATCH v3 5/5] drm/i915: Add short HPD IRQ storm detection for non-MST systems Lyude Paul
2018-11-03 0:37 ` ✗ Fi.CI.SPARSE: warning for drm/i915: HPD IRQ storm detection fixes (rev3) Patchwork
2018-11-03 0:53 ` ✓ Fi.CI.BAT: success " Patchwork
2018-11-03 4:18 ` ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181105203840.GC4208@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=airlied@linux.ie \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lyude@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.