On Tue, 30 Jun 2015, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Tue, Jun 30, 2015 at 08:45:48AM +0530, Sivakumar Thulasimani wrote:
>>
>>
>> On 6/29/2015 10:07 PM, Daniel Vetter wrote:
>> >On Mon, Jun 29, 2015 at 04:30:40PM +0530, Sivakumar Thulasimani wrote:
>> >>From: "Thulasimani, Sivakumar" <sivakumar.thulasimani@xxxxxxxxx>
>> >>
>> >>HPD storm is detected in intel_hpd_irq_handler and disabled for respective
>> >>port immediately but polling is enabled only in i915_hotplug_work_func and
>> >>not in i915_digport_work_func. This will result in disabled hpd never enabled
>> >>back again. This is fixed by calling the appropriate storm disable function
>> >>that will handle the rest of the sequence (both polling enable and reenabling
>> >>of HPD later).
>> >>---
>> >> drivers/gpu/drm/i915/intel_hotplug.c | 4 ++++
>> >> 1 file changed, 4 insertions(+)
>> >>
>> >>diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
>> >>index 3c53aac..8e18587 100644
>> >>--- a/drivers/gpu/drm/i915/intel_hotplug.c
>> >>+++ b/drivers/gpu/drm/i915/intel_hotplug.c
>> >>@@ -205,6 +205,10 @@ static void i915_digport_work_func(struct work_struct *work)
>> >> dev_priv->hotplug.long_port_mask = 0;
>> >> short_port_mask = dev_priv->hotplug.short_port_mask;
>> >> dev_priv->hotplug.short_port_mask = 0;
>> >>+
>> >>+ /* Disable hotplug on connectors that hit an irq storm. */
>> >>+ intel_hpd_irq_storm_disable(dev_priv);
>> >digport_work_func schedules the hotplug handler for everything not
>> >handled, which should result in this getting called. It really shouldn't
>> >matter when exactly it gets called.
>> >
>> >Can you please provide more data and details for your analysis? Like bug
>> >reports, backtraces and dmesg traces showing that the handler is stuck and
>> >similar things.
>> >
>> >Also your patch is missing the s-o-b line.
>> >-Daniel
>> >
>> there is no bug filed for this, it was observed as part of code analysis
>> (that is provided below)
>> I'll try to get more info as soon as i get access to a system.
>>
>> short answer:
>> the issue will be seen during hpd storm, where the last HPD is handled
>> inside intel_dp_hpd_pulse.
>> so i915_hotplug_work_func will not be queued thus missing the storm_disable
>> call.
>>
>> long answer :
>> To give a bit more background, lets assume that we get a call to
>> intel_hpd_irq_handler, with params long pulse for DP panel in PORT_B
>> on a HSW/BDW system during HPD storm scenario.
>> The following sequence will take place
>> *) is_dig_port will be set and will result in queue_dig being set as well
>> *) intel_hpd_irq_storm_detect will detect that this is 6th hpd call within
>> the
>> HPD_STORM_DETECT_PERIOD and so will mark the HPD status of PORT_B to
>> HPD_MARK_DISABLED
>> *) This will result in HPD for PORT_B being disabled immediately(masked in
>> case of LPT)
>> *) i915_digport_work_func will be queued at the end of this function, since
>> queue_dig is set
>> *) once in the i915_digport_work_func, hpd_pulse func pointer will be
>> executed since it is defined for DP
>> *) intel_dp_hpd_pulse, will have long_hpd set and since the panel is plugged
>> in still,
>> ISR will be high and so will return true.
>> *) intel_dp_get_dpcd, will succeed since DP is connected
>> *) finally IRQ_HANDLED will be returned
>> *) once call exits intel_hpd_irq_handler, HPD on port B will never be
>> enabled again
>> (unmasked in case of LPT) and no more hot plug notifications.
>
> The assumption of the storm code is that when there is a DP sink, a storm
> will never happen. We need that since otherwise the mst code (which
> creates ridiculous amounts of hpds on the DP port) will run into the storm
> detection code all the time.
>
> Might be better to document this design assumption somewhere, but it is
> baked in. Hence my question whether you've seen this happen in the real
> world - DP storms haven't been observed yet afaik, and it would be a much
> more serious problem.
The dp short hotplug irqs (used by mst) are not caught by the irq storm
code, but the long hotplug irqs are.
BR,
Jani.
> -Daniel
>
>>
>> sorry for the incomplete patch , i'll reupload again once i get some more
>> details.
>>
>> >> spin_unlock_irq(&dev_priv->irq_lock);
>> >> for (i = 0; i < I915_MAX_PORTS; i++) {
>> >>--
>> >>1.7.9.5
>> >>
>> >>_______________________________________________
>> >>Intel-gfx mailing list
>> >>Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> >>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> >
>>
>>
>> --
>> regards,
>> Sivakumar
>>
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
