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

[Dave Airlie]

On 22 June 2015 at 23:02, Daniel Vetter <daniel at ffwll.ch> wrote:
> 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 at 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.

In my tree currently this code grabs mode_config->mutex, so it should
be fine wrt
the connector list disappearing, as removing connectors is protected
my mode_config->mutex
as well.

if we do add a spinlock, we need to be careful about dropping it and restarting
the loops etc, as I would guess there are a fair few paths we don't
want to descend
holding the spin lock, due to possible sleeping/scheduling etc.

I think the intel_hpd_irq_reenable_work is the one that stands out to
me, the short
hotplug paths don't traverse connector lists deliberately to avoid the problem.

Dave.