Re: [Intel-gfx] [PATCH 2/5] drm/i915/irq: abstract irq storm hotplug disabling

From: Daniel Vetter <daniel@ffwll.ch>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH 2/5] drm/i915/irq: abstract irq storm hotplug disabling
Date: Mon, 22 Jun 2015 15:02:30 +0200	[thread overview]
Message-ID: <20150622130230.GY25769@phenom.ffwll.local> (raw)
In-Reply-To: <c3e22fbaed7b3fbdd54846f7e492bfee31d20149.1434621691.git.jani.nikula@intel.com>

On Thu, Jun 18, 2015 at 01:06:14PM +0300, Jani Nikula wrote:
> Continue abstracting hotplug storm related functions to clarify the
> code. This time, abstract hotplug irq storm related hotplug
> disabling. While at it, clean up the loop iterating over connectors for
> readability.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 77 ++++++++++++++++++++++++++---------------
>  1 file changed, 50 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d64d6895a2e5..bf4c15d0ea2b 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -879,7 +879,7 @@ static void i915_digport_work_func(struct work_struct *work)
>  /*
>   * Handle hotplug events outside the interrupt handler proper.
>   */
> -#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000)
> +static void intel_hpd_irq_storm_disable(struct drm_i915_private *dev_priv);
>  
>  static void i915_hotplug_work_func(struct work_struct *work)
>  {
> @@ -890,7 +890,6 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  	struct intel_connector *intel_connector;
>  	struct intel_encoder *intel_encoder;
>  	struct drm_connector *connector;
> -	bool hpd_disabled = false;
>  	bool changed = false;
>  	u32 hpd_event_bits;
>  
> @@ -901,31 +900,9 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  
>  	hpd_event_bits = dev_priv->hotplug.event_bits;
>  	dev_priv->hotplug.event_bits = 0;
> -	list_for_each_entry(connector, &mode_config->connector_list, head) {

Random comment: We have piles of connector_list walking in probe codde,
and DP MST adds/removes them without much thought really users of these.
Dave? Do we need a connector_list spinlock?

Just grabbing one of the modeset locks won't cut it I think since it'll
serialize way too much.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel