| Message ID | 1436265650-5201-1-git-send-email-sivakumar.thulasimani@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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
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.
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 >
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 '*'
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)