Re: [PATCH] drm/i915/hotplug: Fixing storm handling for digital ports

[Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>]

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@intel.com>
>>
>> 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.

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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>


-- 
regards,
Sivakumar


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx