drm/i915: storm detection documentation update
diff mbox

Message ID 1436265650-5201-1-git-send-email-sivakumar.thulasimani@intel.com
State New
Headers show

Commit Message

Sivakumar Thulasimani July 7, 2015, 10:40 a.m. UTC
From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>

Update the hotplug documentation to explain that hotplug storm
is not expected for Display port panels and hence is not handled
in current code.

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
---
 drivers/gpu/drm/i915/intel_hotplug.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Daniel Vetter July 7, 2015, 11:31 a.m. UTC | #1
On Tue, Jul 07, 2015 at 04:10:49PM +0530, Sivakumar Thulasimani wrote:
> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
> 
> Update the hotplug documentation to explain that hotplug storm
> is not expected for Display port panels and hence is not handled
> in current code.
> 
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> ---
>  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 bac91a1..7dc5e6a 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -66,6 +66,10 @@
>   * while before being re-enabled. The intention is to mitigate issues raising
>   * from broken hardware triggering massive amounts of interrupts and grinding
>   * the system to a halt.
> + *
> + * Hotplug interrupt storm is not expected on Display port panel, hence the
> + * current code only handles disabling and later enabling of HPD interrupts
> + * for HDMI panels through the storm handling set of functions.

This isn't accurate, we handle storms on everything _but_ DP (tv, vga,
sdvo, ...). I'd go with

 * Hotplug interrupt storm is not expected on Display port panel, hence the
 * current code doesn't handle irq reenabling when a DP sink is connected
 * and the hpd is handled by the DP callbacks. But on DP+ ports
 * storms are still handled correctly in all other cases (e.g. due to HDMI
 * sinks).

Could still be improved I think.

-Daniel
Sivakumar Thulasimani July 8, 2015, 1:24 p.m. UTC | #2
On 7/7/2015 5:01 PM, Daniel Vetter wrote:
> On Tue, Jul 07, 2015 at 04:10:49PM +0530, Sivakumar Thulasimani wrote:
>> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
>>
>> Update the hotplug documentation to explain that hotplug storm
>> is not expected for Display port panels and hence is not handled
>> in current code.
>>
>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>> ---
>>   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 bac91a1..7dc5e6a 100644
>> --- a/drivers/gpu/drm/i915/intel_hotplug.c
>> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
>> @@ -66,6 +66,10 @@
>>    * while before being re-enabled. The intention is to mitigate issues raising
>>    * from broken hardware triggering massive amounts of interrupts and grinding
>>    * the system to a halt.
>> + *
>> + * Hotplug interrupt storm is not expected on Display port panel, hence the
>> + * current code only handles disabling and later enabling of HPD interrupts
>> + * for HDMI panels through the storm handling set of functions.
> This isn't accurate, we handle storms on everything _but_ DP (tv, vga,
> sdvo, ...). I'd go with
>
>   * Hotplug interrupt storm is not expected on Display port panel, hence the
>   * current code doesn't handle irq reenabling when a DP sink is connected
>   * and the hpd is handled by the DP callbacks. But on DP+ ports
>   * storms are still handled correctly in all other cases (e.g. due to HDMI
>   * sinks).
>
> Could still be improved I think.
>
> -Daniel
>
Sorry i don't get the "DP+" reference here. By DP+, do you mean dongles ?
passive dongles will report them selves as HDMI to respective detect 
routines
active dongles will be still treated as DP hence not handling HPD will 
apply to any
display connected through active dongle as well.
Daniel Vetter July 8, 2015, 3:20 p.m. UTC | #3
On Wed, Jul 08, 2015 at 06:54:06PM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 7/7/2015 5:01 PM, Daniel Vetter wrote:
> >On Tue, Jul 07, 2015 at 04:10:49PM +0530, Sivakumar Thulasimani wrote:
> >>From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
> >>
> >>Update the hotplug documentation to explain that hotplug storm
> >>is not expected for Display port panels and hence is not handled
> >>in current code.
> >>
> >>Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> >>---
> >>  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 bac91a1..7dc5e6a 100644
> >>--- a/drivers/gpu/drm/i915/intel_hotplug.c
> >>+++ b/drivers/gpu/drm/i915/intel_hotplug.c
> >>@@ -66,6 +66,10 @@
> >>   * while before being re-enabled. The intention is to mitigate issues raising
> >>   * from broken hardware triggering massive amounts of interrupts and grinding
> >>   * the system to a halt.
> >>+ *
> >>+ * Hotplug interrupt storm is not expected on Display port panel, hence the
> >>+ * current code only handles disabling and later enabling of HPD interrupts
> >>+ * for HDMI panels through the storm handling set of functions.
> >This isn't accurate, we handle storms on everything _but_ DP (tv, vga,
> >sdvo, ...). I'd go with
> >
> >  * Hotplug interrupt storm is not expected on Display port panel, hence the
> >  * current code doesn't handle irq reenabling when a DP sink is connected
> >  * and the hpd is handled by the DP callbacks. But on DP+ ports
> >  * storms are still handled correctly in all other cases (e.g. due to HDMI
> >  * sinks).
> >
> >Could still be improved I think.
> >
> >-Daniel
> >
> Sorry i don't get the "DP+" reference here. By DP+, do you mean dongles ?
> passive dongles will report them selves as HDMI to respective detect
> routines
> active dongles will be still treated as DP hence not handling HPD will apply
> to any
> display connected through active dongle as well.

DP+ is DP with support for HDMI with level shifter cables (i.e. all the
ports on intel chips). I just wanted to make it clear that storm handling
doesn't work if we have a confirmed DP sink on the port, but will work in
any other cases (nothing or HDMI sink or DVI or whatever connected). Like
I said some room for improvement ;-)
-Daniel
Sivakumar Thulasimani July 9, 2015, 12:52 p.m. UTC | #4
On 7/8/2015 8:50 PM, Daniel Vetter wrote:
> On Wed, Jul 08, 2015 at 06:54:06PM +0530, Sivakumar Thulasimani wrote:
>>
>> On 7/7/2015 5:01 PM, Daniel Vetter wrote:
>>> On Tue, Jul 07, 2015 at 04:10:49PM +0530, Sivakumar Thulasimani wrote:
>>>> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
>>>>
>>>> Update the hotplug documentation to explain that hotplug storm
>>>> is not expected for Display port panels and hence is not handled
>>>> in current code.
>>>>
>>>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>>>> ---
>>>>   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 bac91a1..7dc5e6a 100644
>>>> --- a/drivers/gpu/drm/i915/intel_hotplug.c
>>>> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
>>>> @@ -66,6 +66,10 @@
>>>>    * while before being re-enabled. The intention is to mitigate issues raising
>>>>    * from broken hardware triggering massive amounts of interrupts and grinding
>>>>    * the system to a halt.
>>>> + *
>>>> + * Hotplug interrupt storm is not expected on Display port panel, hence the
>>>> + * current code only handles disabling and later enabling of HPD interrupts
>>>> + * for HDMI panels through the storm handling set of functions.
>>> This isn't accurate, we handle storms on everything _but_ DP (tv, vga,
>>> sdvo, ...). I'd go with
>>>
>>>   * Hotplug interrupt storm is not expected on Display port panel, hence the
>>>   * current code doesn't handle irq reenabling when a DP sink is connected
>>>   * and the hpd is handled by the DP callbacks. But on DP+ ports
>>>   * storms are still handled correctly in all other cases (e.g. due to HDMI
>>>   * sinks).
>>>
>>> Could still be improved I think.
>>>
>>> -Daniel
>>>
>> Sorry i don't get the "DP+" reference here. By DP+, do you mean dongles ?
>> passive dongles will report them selves as HDMI to respective detect
>> routines
>> active dongles will be still treated as DP hence not handling HPD will apply
>> to any
>> display connected through active dongle as well.
> DP+ is DP with support for HDMI with level shifter cables (i.e. all the
> ports on intel chips). I just wanted to make it clear that storm handling
> doesn't work if we have a confirmed DP sink on the port, but will work in
> any other cases (nothing or HDMI sink or DVI or whatever connected). Like
> I said some room for improvement ;-)
> -Daniel
how about the following ?

  * current implementation expects that hotplug interrupt storm will not be
  * seen when display port sink is connected, hence on platforms whose DP
  * callback is handled by i915_digport_work_func reenabling of hpd is not
  * performed ( it was never expected to be disabled in the first place ;) )
  * this is specific to DP sinks handled by this routine and any other display
  * such as HDMI or DVI enabled on the same port will have proper logic since
  * it will use i915_hotplug_work_func where this logic is handled.

please provide any corrections or new comment to be used, i'll upload a 
new patch with
the final approved text block.
Daniel Vetter July 9, 2015, 4:19 p.m. UTC | #5
On Thu, Jul 09, 2015 at 06:22:01PM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 7/8/2015 8:50 PM, Daniel Vetter wrote:
> >On Wed, Jul 08, 2015 at 06:54:06PM +0530, Sivakumar Thulasimani wrote:
> >>
> >>On 7/7/2015 5:01 PM, Daniel Vetter wrote:
> >>>On Tue, Jul 07, 2015 at 04:10:49PM +0530, Sivakumar Thulasimani wrote:
> >>>>From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
> >>>>
> >>>>Update the hotplug documentation to explain that hotplug storm
> >>>>is not expected for Display port panels and hence is not handled
> >>>>in current code.
> >>>>
> >>>>Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> >>>>---
> >>>>  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 bac91a1..7dc5e6a 100644
> >>>>--- a/drivers/gpu/drm/i915/intel_hotplug.c
> >>>>+++ b/drivers/gpu/drm/i915/intel_hotplug.c
> >>>>@@ -66,6 +66,10 @@
> >>>>   * while before being re-enabled. The intention is to mitigate issues raising
> >>>>   * from broken hardware triggering massive amounts of interrupts and grinding
> >>>>   * the system to a halt.
> >>>>+ *
> >>>>+ * Hotplug interrupt storm is not expected on Display port panel, hence the
> >>>>+ * current code only handles disabling and later enabling of HPD interrupts
> >>>>+ * for HDMI panels through the storm handling set of functions.
> >>>This isn't accurate, we handle storms on everything _but_ DP (tv, vga,
> >>>sdvo, ...). I'd go with
> >>>
> >>>  * Hotplug interrupt storm is not expected on Display port panel, hence the
> >>>  * current code doesn't handle irq reenabling when a DP sink is connected
> >>>  * and the hpd is handled by the DP callbacks. But on DP+ ports
> >>>  * storms are still handled correctly in all other cases (e.g. due to HDMI
> >>>  * sinks).
> >>>
> >>>Could still be improved I think.
> >>>
> >>>-Daniel
> >>>
> >>Sorry i don't get the "DP+" reference here. By DP+, do you mean dongles ?
> >>passive dongles will report them selves as HDMI to respective detect
> >>routines
> >>active dongles will be still treated as DP hence not handling HPD will apply
> >>to any
> >>display connected through active dongle as well.
> >DP+ is DP with support for HDMI with level shifter cables (i.e. all the
> >ports on intel chips). I just wanted to make it clear that storm handling
> >doesn't work if we have a confirmed DP sink on the port, but will work in
> >any other cases (nothing or HDMI sink or DVI or whatever connected). Like
> >I said some room for improvement ;-)
> >-Daniel
> how about the following ?
> 
>  * current implementation expects that hotplug interrupt storm will not be

s/current/Current/

>  * seen when display port sink is connected, hence on platforms whose DP
>  * callback is handled by i915_digport_work_func reenabling of hpd is not
>  * performed ( it was never expected to be disabled in the first place ;) )
>  * this is specific to DP sinks handled by this routine and any other display
>  * such as HDMI or DVI enabled on the same port will have proper logic since
>  * it will use i915_hotplug_work_func where this logic is handled.

lgtm.
-Daniel

> 
> please provide any corrections or new comment to be used, i'll upload a new
> patch with
> the final approved text block.
> 
> -- 
> regards,
> Sivakumar
>
Shuang He July 11, 2015, 1:17 a.m. UTC | #6
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6743
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                                  303/303              303/303
SNB              +3                 309/316              312/316
IVB                                  343/343              343/343
BYT                 -1              285/285              284/285
HSW              +13                 367/381              380/381
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*SNB  igt@kms_mmio_vs_cs_flip@setcrtc_vs_cs_flip      DMESG_WARN(1)      PASS(1)
*SNB  igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip      DMESG_WARN(1)      PASS(1)
*SNB  igt@pm_rpm@cursor      DMESG_WARN(1)      PASS(1)
*SNB  igt@pm_rpm@cursor-dpms      DMESG_FAIL(1)      FAIL(1)
*BYT  igt@gem_partial_pwrite_pread@reads-uncached      PASS(1)      FAIL(1)
*HSW  igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_lpsp@non-edp      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@debugfs-read      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@gem-idle      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@gem-mmap-gtt      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@gem-pread      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@i2c      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@modeset-non-lpsp      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@modeset-non-lpsp-stress-no-wait      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@pci-d3-state      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@reg-read-ioctl      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@rte      DMESG_WARN(1)      PASS(1)
*HSW  igt@pm_rpm@sysfs-read      DMESG_WARN(1)      PASS(1)
Note: You need to pay more attention to line start with '*'

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index bac91a1..7dc5e6a 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -66,6 +66,10 @@ 
  * while before being re-enabled. The intention is to mitigate issues raising
  * from broken hardware triggering massive amounts of interrupts and grinding
  * the system to a halt.
+ *
+ * Hotplug interrupt storm is not expected on Display port panel, hence the
+ * current code only handles disabling and later enabling of HPD interrupts
+ * for HDMI panels through the storm handling set of functions.
  */
 
 enum port intel_hpd_pin_to_port(enum hpd_pin pin)