Re: [Intel-gfx] [PATCH driver/intel] sna/cursor: Make sure hw cursors are disabled before disabling secondary planes
On Tue, Jun 21, 2016 at 09:25:36PM +0100, Chris Wilson wrote: > On Tue, Jun 21, 2016 at 07:34:34PM +0200, Egbert Eich wrote: > > When the hw cursors are not disabled before the cursor planes get disabled > > we may lose the cursor later on. Thus make sure the cursors are disabled > > before the cursor planes are. > > The cursor would already be controlled by the xf86SetDesiredModes(), so > we can skip disabling entirely. What we should do instead is add the > paranoia check, but I can't see an easy way to inquire what the kernel > thinks the legacy cursor handle should be. > > commit f1c757e4518f6835bbff6c940269a5c6be75f202 > Author: Chris Wilson > Date: Tue Jun 21 21:17:15 2016 +0100 > > sna: Only shutdown unknown secondary planes on CRTC we control > > In a ZaphodHead scenario, we do not own all the CRTC and so we should > not be making changes outside of our zone of control. Also, we only want > to disable secondary overlay planes and ignore the secondary cursor > planes which are controlled through the normal modesetting. > > As we are now tracking all sprite planes on a CRTC, this leads to much > simpler code. Chris, thanks for the patch! I've been able to test it now - it works. Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH driver/intel] sna/cursor: Make sure hw cursors are disabled before disabling secondary planes
When the hw cursors are not disabled before the cursor planes get disabled we may lose the cursor later on. Thus make sure the cursors are disabled before the cursor planes are. Signed-off-by: Egbert Eich --- src/sna/sna_display.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c index d790975..412c192 100644 --- a/src/sna/sna_display.c +++ b/src/sna/sna_display.c @@ -8081,6 +8081,9 @@ void sna_mode_check(struct sna *sna) if (sna->mode.hidden) return; +/* make sure the hw cursors are disabled before disabling + the secondary planes which include the cursor plane */ + sna_hide_cursors(sna->scrn); disabled = sna_mode_disable_secondary_planes(sna); /* Validate CRTC attachments and force consistency upon the kernel */ -- 2.7.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/sdvo: Use &intel_sdvo->ddc instead of intel_sdvo->i2c for DDC.
In intel_sdvo_get_lvds_modes() the wrong i2c adapter record is used for DDC. Thus the code will always have to rely on a LVDS panel mode supplied by VBT. In most cases this succeeds, so this didn't get detected for quite a while. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/intel_sdvo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 7068195..8618b15 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1848,7 +1848,7 @@ static void intel_sdvo_get_lvds_modes(struct drm_connector *connector) * Assume that the preferred modes are * arranged in priority order. */ - intel_ddc_get_modes(connector, intel_sdvo->i2c); + intel_ddc_get_modes(connector, &intel_sdvo->ddc); if (list_empty(&connector->probed_modes) == false) goto end; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/i915: Always enable the cursor right after the primary plane
On Tue, May 21, 2013 at 05:17:27PM +0200, Egbert Eich wrote: > On Wed, May 08, 2013 at 12:50:24PM +0300, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > Follow the same sequence when enabling the cursor plane during > > modeset. No point in doing this stuff in different order on different > > generations. > > > > This should also avoid a needless wait for vblank for the g4x cursor > > workaround when the cursor gets enabled anyway. > > > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_display.c | 10 -- > > 1 file changed, 4 insertions(+), 6 deletions(-) > > > > @@ -3727,6 +3725,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) > > > > intel_enable_pipe(dev_priv, pipe, false); > > intel_enable_plane(dev_priv, plane, pipe); > > + intel_crtc_update_cursor(crtc, true); > > if (IS_G4X(dev)) > > g4x_fixup_plane(dev_priv, pipe); > > > > As discussed on IRC this may interfere with > > commit 61bc95c1fbbb6a08b55bbe161fdf1ea5493fc595 > Author: Egbert Eich > Date: Mon Mar 4 09:24:38 2013 -0500 > > DRM/i915: On G45 enable cursor plane briefly after enabling the display > plane. > > described in https://bugs.freedesktop.org/show_bug.cgi?id=61457 > Today I had the chance to test this. First I tried if I can still reproduce the blank with this patch added when I disable my voodoo g4x_fixup_plane(): It turned out it still happens however very rarely (like 1 out of 20 tries). When I reenabled my voodoo the issue still occurred. I had to switch two lines around, ie: intel_enable_plane(dev_priv, plane, pipe); if (IS_G4X(dev)) g4x_fixup_plane(dev_priv, pipe); + intel_crtc_update_cursor(crtc, true); to avoid the blank screen issue - which is it didn't happen in ~75 tries. With this change: Acked-by: Egbert Eich Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: prefer VBT modes for SVDO-LVDS over EDID
Chris Wilson writes: > On Sun, Jun 09, 2013 at 11:28:12PM +0200, Daniel Vetter wrote: > > On Sun, Jun 9, 2013 at 11:16 PM, Chris Wilson > > wrote: > > > On Sun, Jun 09, 2013 at 10:58:38PM +0200, Daniel Vetter wrote: > > >> In > > >> > > >> commit 53d3b4d7778daf15900867336c85d3f8dd70600c > > >> Author: Egbert Eich > > >> Date: Tue Jun 4 17:13:21 2013 +0200 > > >> > > >> drm/i915/sdvo: Use &intel_sdvo->ddc instead of intel_sdvo->i2c for > > >> DDC > > >> > > >> Ebgert Eich fixed a long-standing bug where we simply used a > > >> non-working i2c controller to read the EDID for SDVO-LVDS panels. > > >> Unfortunately some machines seem to not be able to cope with the mode > > >> provided in the EDID (specifically they seem to not be able to cope > > >> with a 4x pixel mutliplier instead of a 2x one). > > >> > > >> Since it took forever to notice the breakage it's fairly safe to > > >> assume that at least for SDVO-LVDS panels the VBT contains fairly sane > > >> data. So just switch around the order and use VBT modes first. > > >> > > >> Cc: Egbert Eich > > >> Cc: Chris Wilson > > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65524 > > >> Cc: sta...@vger.kernel.org > > >> Signed-off-by: Daniel Vetter > > > > > > I can accept the argument that we need to prefer the VBT mode here to > > > paper over the apparent regression, but to not pass on the full EDID > > > modes seems dubious. > > > > I'm not sure there's any value in additional modes. We can't really > > support frequency switching over sdvo (it'd very likely require a > > mutliplier change) and for everything else we only ever let the fixed > > mode through the fixup hook. > > I am trying not to generalise from the broken behaviour on this machine. > On another machine, there may be some value in the extra modes. > Select the sanest default we can, then let the user go nuts with the > extra information. While I'm not a fan of setting non-native modes on panels this seems to be a requirement in some environments - not sure if there are any real world use cases but at least many QA test scenarios seem to include such modes. So adding the EDID modes (unflagging the preferred bit!) would be what I'd opt for - admittedly for a very selfish reason: it will spare me of lengthy, pointless discussions with some partners' QA departments next time we update the Intel driver. Once again we go out of our ways to accomodate the most broken devices by imposing more limitations on all others as well. At one point we may even have to give in face the grim reality and start blacklisting some of the broken crap. And since we are so much into bikeshedding here - maybe you could fix my name in the comment ;) Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/24] drm/i915: s/hotplug_irq_storm_detect/intel_hpd_irq_handler/
Daniel Vetter writes: > The combination of Paulo's fifo underrun detection code and Egbert's > hpd storm handling code unfortunately made the hpd storm handling code > racy. > > To avoid duplicating tricky interrupt locking code over all platforms > start with a bit of refactoring. This patch is the very first step > since in the end the irq storm handling code will handle all hotplug > logic (and so also encapsulate the locking nicely). > > Cc: Egbert Eich > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_irq.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > [reviewed code deleted] Reviewed-by: Egbert Eich ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/24] drm/i915: fold the queue_work into intel_hpd_irq_handler
Daniel Vetter writes: > Everywhere the same. > > Cc: Egbert Eich > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_irq.c | 11 +++ > 1 file changed, 3 insertions(+), 8 deletions(-) [reviewed code deleted] Reviewed-by: Egbert Eich ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/24] drm/i915: fold the no-irq check into intel_hpd_irq_handler
Daniel Vetter writes: > The usual pattern for our sub-function irq_handlers is that they check > for the no-irq case themselves. This results in more streamlined code > in the upper irq handlers. > > Cc: Egbert Eich > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_irq.c | 33 + > 1 file changed, 17 insertions(+), 16 deletions(-) > [reviewed code deleted] Reviewed-by: Egbert Eich ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/24] drm/i915: fix hpd interrupt register locking
Daniel Vetter writes: > Our interrupt handler (in hardird context) could race with the timer Nitpick: s/d/q/ > (in softirq context), hence we need to hold the spinlock around the > call to ->hdp_irq_setup in intel_hpd_irq_handler, too. > > But as an optimization (and more so to clarify things) we don't need > to do the irqsave/restore dance in the hardirq context. > > Note also that on ilk+ the race isn't just against the hotplug > reenable timer, but also against the fifo underrun reporting. That one > also modifies the SDEIMR register (again protected by the same > dev_priv->irq_lock). > > To lock things down again sprinkle a assert_spin_locked. But exclude > the functions touching SDEIMR for now, I want to extract them all into > a new helper function (like we do already for pipestate, display > interrupts and all the various gt interrupts). > > Cc: Egbert Eich > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_irq.c | 15 ++- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > b/drivers/gpu/drm/i915/i915_irq.c > index 1815891..95e15cd 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -880,15 +880,13 @@ static inline void intel_hpd_irq_handler(struct > drm_device *dev, > const u32 *hpd) > { > drm_i915_private_t *dev_priv = dev->dev_private; > -unsigned long irqflags; > int i; > bool storm_detected = false; > > if (!hotplug_trigger) > return; > > -spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > - > +spin_lock(&dev_priv->irq_lock); > for (i = 1; i < HPD_NUM_PINS; i++) { > > if (!(hpd[i] & hotplug_trigger) || > @@ -911,10 +909,9 @@ static inline void intel_hpd_irq_handler(struct > drm_device *dev, > } > } > > -spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > - > if (storm_detected) > dev_priv->display.hpd_irq_setup(dev); > +spin_unlock(&dev_priv->irq_lock); > > queue_work(dev_priv->wq, > &dev_priv->hotplug_work); > @@ -3327,6 +3324,8 @@ static void i915_hpd_irq_setup(struct drm_device *dev) > struct intel_encoder *intel_encoder; > u32 hotplug_en; > > +assert_spin_locked(&dev_priv->irq_lock); > + > if (I915_HAS_HOTPLUG(dev)) { > hotplug_en = I915_READ(PORT_HOTPLUG_EN); > hotplug_en &= ~HOTPLUG_INT_EN_MASK; Didn't you want to do the same for ibx_hpd_irq_setup() ? > @@ -3610,6 +3609,7 @@ void intel_hpd_init(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_mode_config *mode_config = &dev->mode_config; > struct drm_connector *connector; > +unsigned long irqflags; > int i; > > for (i = 1; i < HPD_NUM_PINS; i++) { > @@ -3622,6 +3622,11 @@ void intel_hpd_init(struct drm_device *dev) > if (!connector->polled && I915_HAS_HOTPLUG(dev) && > intel_connector->encoder->hpd_pin > HPD_NONE) > connector->polled = DRM_CONNECTOR_POLL_HPD; > } > + > +/* Interrup setup is already guaranteed to be single-threaded, this is Nitpick - missed a 't'. > + * just to make the assert_spin_locked checks happy. */ > +spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > if (dev_priv->display.hpd_irq_setup) > dev_priv->display.hpd_irq_setup(dev); > +spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > } > -- > 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/24] drm/i915: fold the hpd_irq_setup call into intel_hpd_irq_handler
Daniel Vetter writes: > We already have a vfunc for this (and other parts of the hpd storm > handling code already use it). > > Cc: Egbert Eich > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_irq.c | 29 +++-- > 1 file changed, 11 insertions(+), 18 deletions(-) > [ reviewed code deleted ] Reviewed-by: Egbert Eich ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 irq storm mitigation in 3.10
Daniel Vetter writes: > On Sun, Jul 21, 2013 at 10:23 PM, Jan Niggemann wrote: > >> But every time this happens we only let through a few interrupts, so this > >> shouldn't affect you badly. Can you please check whether those slowdowns > >> line up with 2 minute intervalls? > > > > I observed these slowdowns for a couple of weeks now. On my machine, they > > only happen once, some minutes after a cold boot. > > They last for a minute or two, and then they are gone. > > I'd have guessed that the storm detection kicks in pretty quickly after a > > storm is detected and that it would go unnoticed. > > Hm, that sounds like something doesn't quite work as expected. We > should kill things once we get 5 interrupts or so in 1 second. So if > it's bad enough that it slows your machine down it really should only > be barely noticeable. > The logs show that the disable mechanism got triggered, so there was a storm that got detected. The respective message is generated by the worker, everything up to there (detection and marking disabled) seems to be fine. I bet we are still getting interrupts but the respective bit in hpd_event_bits doesn't get set any more. Since we unconditionally queue the worker on interrupt there is surprise it is so busy. Then this points to the call to hpd_irq_setup() in intel_hpd_irq_handler() not doing what is expected, ie masking out the stormy interrupt. Could it be that we can't mask/disable an interrupt before ACKing it? @Jan, could you also specify what hardware you are using (ie give us an output of lspci -n)? Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 irq storm mitigation in 3.10
Jan Niggemann writes: > Egbert, Daniel, others, > It's a Lenovo ThinkPad T400, the model is 7434-AG2. > root@muretop:~# lspci -n > 00:00.0 0600: 8086:2a40 (rev 07) > 00:02.0 0300: 8086:2a42 (rev 07) Ok, this is a gm45. > > As to the log: I messed up the kernel parameters this morning... was > out of coffee this morning and my 1,5y daughter played around me :-) > > Here's my kernel log with drm.debug and printk.time enabled: > Uncompressed (22M): http://files.hz6.de/kern_20130722.log These logs show that interrupts are still coming thru although they should be disabled. Could you try if the patch below makes any difference? Cheers, Egbert. >From 70dae32e99799d15ddcedd5853514215624a8289 Mon Sep 17 00:00:00 2001 From: Egbert Eich Date: Mon, 22 Jul 2013 22:33:36 +0200 Subject: [PATCH] drm/i915: Make sure PORT_HOTPLUG_EN is written Add posting read to make sure PORT_HOTPLUG_EN is written in i915_hpd_irq_setup(). Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index f708e4e..e43d809 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2802,6 +2802,7 @@ static void i915_hpd_irq_setup(struct drm_device *dev) /* Ignore TV since it's buggy */ I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); + POSTING_READ(PORT_HOTPLUG_EN); } } -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 irq storm mitigation in 3.10
Hi Jan - Jan Niggemann writes: > > As to the log: I messed up the kernel parameters this morning... was > out of coffee this morning and my 1,5y daughter played around me :-) > > Here's my kernel log with drm.debug and printk.time enabled: > Uncompressed (22M): http://files.hz6.de/kern_20130722.log > bzip2'd (some 600 KB): http://files.hz6.de/kern_20130722.log.bz2 I've looked at the logs a bit more. Here's a patch adding some more debug information. Would you please apply this to your 3.10 kernel and generate a log file the same way as you did before. The driver will be even more chatty - but I don't expect any problems from this. Cheers, Egbert. diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index e43d809..46bb77c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -919,6 +919,11 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev, spin_lock(&dev_priv->irq_lock); for (i = 1; i < HPD_NUM_PINS; i++) { + if (IS_G4X(dev) && (hpd[i] & hotplug_trigger) && + dev_priv->hpd_stats[i].hpd_mark != HPD_ENABLED) + DRM_DEBUG_KMS("Received HPD intterupt although disabled\n", + I915_READ(PORT_HOTPLUG_EN)); + if (!(hpd[i] & hotplug_trigger) || dev_priv->hpd_stats[i].hpd_mark != HPD_ENABLED) continue; @@ -929,6 +934,8 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev, + msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) { dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies; dev_priv->hpd_stats[i].hpd_cnt = 0; + DRM_DEBUG_KMS("Received HPD interrupt on PIN %d - jiffies: %d\n", i, + dev_priv->hpd_stats[i].hpd_last_jiffies); } else if (dev_priv->hpd_stats[i].hpd_cnt > HPD_STORM_THRESHOLD) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; dev_priv->hpd_event_bits &= ~(1 << i); @@ -936,6 +943,8 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev, storm_detected = true; } else { dev_priv->hpd_stats[i].hpd_cnt++; + DRM_DEBUG_KMS("Received HPD interrupt on PIN %d - cnt: %d\n", i, + dev_priv->hpd_stats[i].hpd_cnt); } } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 irq storm mitigation in 3.10
Hi Jan! Jan Niggemann writes: > Hi Egbert, [...] Thanks for generating the logs! Hope you had a nice birthday dinner :) > > I applied this patch (but not the one you sent on Monday), recompiled > and logged: > uncompressed (8,2M) http://files.hz6.de/kern_20130724.log > bz2 (288 KB) http://files.hz6.de/kern_20130724.log.bz2 These logs show clearly that we are seeing interrupts which should be disabled. Could it be that we we have either the status or enable bits mixed up? Unfortunately the publically available docs for GEN4 don't show the HOTPLUG_EN and HOTPLUG_STAT registers. Daniel, could you please help me out here? Thanks a lot! Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 irq storm mitigation in 3.10
Daniel Vetter writes: > > I've checked the docs again for gm45 and we seem to have the right > values. But on early gen4 (i.e. i965g/gm) the Bspec has been wrong > about these, so I wouldn't be surprised at all if they're wrong for > the digital ports on gm45, too. > > Iirc we've already had a case like that, but there was no real > conclusion (and atm I can't find the bug). Can you pls try the below > patch (on top of Egbert's debug stuff)? > If this doesn't help, lets disable all the digital ports together for testing. > > > Egbert I think your debug patch has fairly useful information for > debugging the storm code in general, can you please submit a patch > against drm-intel-nightly? > Will do :) Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 irq storm mitigation in 3.10
Jan Niggemann writes: > Hi Daniel, > > Am 25.07.2013 11:58, schrieb Daniel Vetter: > > Can you pls try the below > > patch (on top of Egbert's debug stuff)? > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 6caa748..2d4c884 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -1925,9 +1925,9 @@ > > > > #define PORT_HOTPLUG_STAT (dev_priv->info->display_mmio_offset > > + 0x61114) > > /* HDMI/DP bits are gen4+ */ > > -#define PORTB_HOTPLUG_LIVE_STATUS (1 << 29) > > +#define PORTD_HOTPLUG_LIVE_STATUS (1 << 29) > > #define PORTC_HOTPLUG_LIVE_STATUS (1 << 28) > > -#define PORTD_HOTPLUG_LIVE_STATUS (1 << 27) > > +#define PORTB_HOTPLUG_LIVE_STATUS (1 << 27) > > #define PORTD_HOTPLUG_INT_STATUS (3 << 21) > > #define PORTC_HOTPLUG_INT_STATUS (3 << 19) > > #define PORTB_HOTPLUG_INT_STATUS (3 << 17) > I did, here are the logs: > 364K http://files.hz6.de/kern_20130724_2.log I get a 'forbidden' when I try to access this. > I don't understand what exactly this patch does, but I noticed: > - much less drm debug info, resulting in a much smaller log > - no more noticeable lag on my system (even though a storm was > detected). Ok, so this indicates that the storm detection works now :) > I double checked the latter and the lag seems indeed to be gone... Before we actually masked out individual events it was irrelevant if the bits were mixed up. Now we need to be correct on these. Jan, thanks for your help! Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Add messages usful for HPD strom detection debugging
For HPD storm detection we now mask out individual interrupt source bits. We have already seen a case where HPD interrupt enable bits were assigned to the wrong pins. To track these conditions more easily add some debugging messages. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index ee3e49c..1f3a971 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -919,6 +919,10 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev, spin_lock(&dev_priv->irq_lock); for (i = 1; i < HPD_NUM_PINS; i++) { + WARN(((hpd[i] & hotplug_trigger) && + dev_priv->hpd_stats[i].hpd_mark != HPD_ENABLED), +"Received HPD intterupt although disabled\n"); + if (!(hpd[i] & hotplug_trigger) || dev_priv->hpd_stats[i].hpd_mark != HPD_ENABLED) continue; @@ -929,6 +933,7 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev, + msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) { dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies; dev_priv->hpd_stats[i].hpd_cnt = 0; + DRM_DEBUG_KMS("Received HPD interrupt on PIN %d - cnt: 0\n", i); } else if (dev_priv->hpd_stats[i].hpd_cnt > HPD_STORM_THRESHOLD) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; dev_priv->hpd_event_bits &= ~(1 << i); @@ -936,6 +941,8 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev, storm_detected = true; } else { dev_priv->hpd_stats[i].hpd_cnt++; + DRM_DEBUG_KMS("Received HPD interrupt on PIN %d - cnt: %d\n", i, + dev_priv->hpd_stats[i].hpd_cnt); } } -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Make sure PORT_HOTPLUG_EN is written
Add posting read to make sure PORT_HOTPLUG_EN is written in i915_hpd_irq_setup(). Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 1f3a971..a38372b 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2805,6 +2805,7 @@ static void i915_hpd_irq_setup(struct drm_device *dev) /* Ignore TV since it's buggy */ I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); + POSTING_READ(PORT_HOTPLUG_EN); } } -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Make sure PORT_HOTPLUG_EN is written
Chris Wilson writes: > On Fri, Jul 26, 2013 at 12:31:30PM +0200, Egbert Eich wrote: > > Add posting read to make sure PORT_HOTPLUG_EN is written in > > i915_hpd_irq_setup(). > > It's not vital that the write be flushed here. On the deferred reenable > path a further delay in rearming is not significant, and inside the irq > handler (for disabling) the write will be flushed. > > In terms of the patch itself, you would also need to fixup > ibx_hpd_irq_setup as well. > -Chris We can probably drop this patch. It was one try figuring why HPD interrupts are still seeen on pins which have been disabled. It turned out that the root cause was a mixup in the mapping of pins to enable bits due to incorrect documentation. Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V2] drm/i915: Add messages useful for HPD storm detection debugging (v2)
For HPD storm detection we now mask out individual interrupt source bits. We have already seen a case where HPD interrupt enable bits were assigned to the wrong pins. To track these conditions more easily add some debugging messages. v2: Spelling fixes as suggested by Jani Nikula Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index ee3e49c..6a1c207 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -919,6 +919,10 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev, spin_lock(&dev_priv->irq_lock); for (i = 1; i < HPD_NUM_PINS; i++) { + WARN(((hpd[i] & hotplug_trigger) && + dev_priv->hpd_stats[i].hpd_mark != HPD_ENABLED), +"Received HPD interrupt although disabled\n"); + if (!(hpd[i] & hotplug_trigger) || dev_priv->hpd_stats[i].hpd_mark != HPD_ENABLED) continue; @@ -929,6 +933,7 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev, + msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) { dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies; dev_priv->hpd_stats[i].hpd_cnt = 0; + DRM_DEBUG_KMS("Received HPD interrupt on PIN %d - cnt: 0\n", i); } else if (dev_priv->hpd_stats[i].hpd_cnt > HPD_STORM_THRESHOLD) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; dev_priv->hpd_event_bits &= ~(1 << i); @@ -936,6 +941,8 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev, storm_detected = true; } else { dev_priv->hpd_stats[i].hpd_cnt++; + DRM_DEBUG_KMS("Received HPD interrupt on PIN %d - cnt: %d\n", i, + dev_priv->hpd_stats[i].hpd_cnt); } } -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't WARN about unexpected hpd interrupts on gmch platforms
Daniel Vetter writes: > The status bits are unconditionally set, the control bits only enable > the actual interrupt generation. Which means if we get some random > other interrupts we'll bogusly complain about them. > > So restrict the WARN to platforms with a sane hotplug interrupt > handling scheme. > > This WARN has been introduced in > > commit b8f102e8bf71cacf33326360fdf9dcfd1a63925b > Author: Egbert Eich > Date: Fri Jul 26 14:14:24 2013 +0200 > > drm/i915: Add messages useful for HPD storm detection debugging (v2) > > Cc: Egbert Eich > Cc: bitlord > Reported-by: bitlord > Cc: sta...@vger.kernel.org > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_irq.c | 18 ++ > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > b/drivers/gpu/drm/i915/i915_irq.c > index 7753249b3a95..f98ba4e6e70b 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1362,10 +1362,20 @@ static inline void intel_hpd_irq_handler(struct > drm_device *dev, > spin_lock(&dev_priv->irq_lock); > for (i = 1; i < HPD_NUM_PINS; i++) { > > -WARN_ONCE(hpd[i] & hotplug_trigger && > - dev_priv->hpd_stats[i].hpd_mark == HPD_DISABLED, > - "Received HPD interrupt (0x%08x) on pin %d (0x%08x) > although disabled\n", > - hotplug_trigger, i, hpd[i]); > +if (hpd[i] & hotplug_trigger && > +dev_priv->hpd_stats[i].hpd_mark == HPD_DISABLED) { > +/* > + * On GMCH platforms the interrupt mask bits only > + * prevent irq generation, not the setting of the > + * hotplug bits itself. So only WARN about unexpected > + * interrupts on saner platforms. > + */ > +WARN_ONCE(INTEL_INFO(dev)->gen >= 5 && > !IS_VALLEYVIEW(dev), > + "Received HPD interrupt (0x%08x) on pin %d > (0x%08x) although disabled\n", > + hotplug_trigger, i, hpd[i]); Personally I'd prefer the condition in the WARN..() macro to be the unexpected condition you want to warn about. This makes it easier for anybody not up to speed with the details of hotplug handling to understand the code. Of course the way you structure this avoids a lot of unnecessary tests. But if this is a concern maybe the entire for loop should be restructured with if (!(hpd[i] & hotplug_trigger)) continue; right at the beginning. > + > +continue; > +} > > if (!(hpd[i] & hotplug_trigger) || > dev_priv->hpd_stats[i].hpd_mark != HPD_ENABLED) > -- > 1.8.5.2 Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Set up SDVO encoder type only at detect
Depending on the SDVO output_flags SDVO may have multiple connectors. These are all initialized in intel_sdvo_output_setup(). The connector that is initialized later will override the encoder_type that has been set up by an earlier connector type initialization. Eventually the one that comes last wins. Eventually when intel_sdvo_detect() is called the active connector is determined. Delay encoder type initialization until the active connector is known and set it to the type that corresponds to this connector. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/intel_sdvo.c | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index d27155a..5043f16 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -154,6 +154,9 @@ struct intel_sdvo_connector { /* Mark the type of connector */ uint16_t output_flag; + /* store encoder type for convenience */ + int encoder_type; + enum hdmi_force_audio force_audio; /* This contains all current supported TV format */ @@ -1746,6 +1749,7 @@ intel_sdvo_detect(struct drm_connector *connector, bool force) if (response == 0) return connector_status_disconnected; + intel_sdvo->base.base.encoder_type = intel_sdvo_connector->encoder_type; intel_sdvo->attached_output = response; intel_sdvo->has_hdmi_monitor = false; @@ -2489,7 +2493,9 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device) } else { intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT; } - encoder->encoder_type = DRM_MODE_ENCODER_TMDS; + /* delay encoder_type setting until detection */ + intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_TMDS; + encoder->encoder_type = DRM_MODE_ENCODER_NONE; connector->connector_type = DRM_MODE_CONNECTOR_DVID; if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) { @@ -2524,7 +2530,9 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type) intel_connector = &intel_sdvo_connector->base; connector = &intel_connector->base; - encoder->encoder_type = DRM_MODE_ENCODER_TVDAC; + /* delay encoder_type setting until detection */ + intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_TVDAC; + encoder->encoder_type = DRM_MODE_ENCODER_NONE; connector->connector_type = DRM_MODE_CONNECTOR_SVIDEO; intel_sdvo->controlled_output |= type; @@ -2568,7 +2576,9 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device) intel_connector = &intel_sdvo_connector->base; connector = &intel_connector->base; intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT; - encoder->encoder_type = DRM_MODE_ENCODER_DAC; + /* delay encoder_type setting until detection */ + intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_DAC; + encoder->encoder_type = DRM_MODE_ENCODER_NONE; connector->connector_type = DRM_MODE_CONNECTOR_VGA; if (device == 0) { @@ -2603,7 +2613,9 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device) intel_connector = &intel_sdvo_connector->base; connector = &intel_connector->base; - encoder->encoder_type = DRM_MODE_ENCODER_LVDS; + /* delay encoder_type setting until detection */ + intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_LVDS; + encoder->encoder_type = DRM_MODE_ENCODER_NONE; connector->connector_type = DRM_MODE_CONNECTOR_LVDS; if (device == 0) { @@ -2984,7 +2996,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob) /* encoder type will be decided later */ intel_encoder = &intel_sdvo->base; intel_encoder->type = INTEL_OUTPUT_SDVO; - drm_encoder_init(dev, &intel_encoder->base, &intel_sdvo_enc_funcs, 0); + drm_encoder_init(dev, &intel_encoder->base, &intel_sdvo_enc_funcs, +DRM_MODE_ENCODER_NONE); /* Read the regs to test if we can talk to the device */ for (i = 0; i < 0x40; i++) { -- 1.8.4.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/2] Fix if multiple SDVO outputs are flagged
Currently the i915 driver can only handle well if only a single SDVO output is flagged (ie output_flags has only one bit set). If multiple outputs are flagged the side effects are only cosmetic in most cases (ie. the encoder may have the wrong type set), but there are situations (namely when intel_connector_break_all_links() is called) where this may lead to an inconsistent driver state. The following two patches fix both situations. Egbert Eich (2): drm/i915: Only break encoder linked when linked to connector drm/i915: Set up SDVO encoder type only at detect drivers/gpu/drm/i915/intel_display.c | 2 ++ drivers/gpu/drm/i915/intel_sdvo.c| 23 ++- 2 files changed, 20 insertions(+), 5 deletions(-) -- 1.8.4.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Only break encoder linked when linked to connector
Depending on the SDVO output_flags SDVO may have multiple connectors linking to the same encoder (in intel_connector->encoder->base). Only one of those connectors should be active (ie link to the encoder thru drm_connector->encoder. If intel_connector_break_all_links() is called from intel_sanitize_crtc() we may brake the crtc connection of an encoder thru an inactive connector in which case intel_connector_break_all_links() will not be called again for the active connector due to if (connector->encoder->base.crtc != &crtc->base) continue; in intel_sanitize_crtc(). This will however leave the drm_connector->encoder linkage for this active connector in place. Subsequently this will cause multiple warnings in intel_connector_check_state() to trigger and the driver will eventually die in drm_encoder_crtc_ok() (because of crtc == NULL). To avoid this break the encoder links only if the connector is active (ie. has drm_connector->encoder set). Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/intel_display.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1390ab5..041f847 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11390,6 +11390,8 @@ static void intel_connector_break_all_links(struct intel_connector *connector) { connector->base.dpms = DRM_MODE_DPMS_OFF; + if (!connector->base.encoder) + return; connector->base.encoder = NULL; connector->encoder->connectors_active = false; connector->encoder->base.crtc = NULL; -- 1.8.4.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Only break encoder linked when linked to connector
Chris Wilson writes: > On Mon, Apr 14, 2014 at 07:26:08PM +0200, Egbert Eich wrote: > > Depending on the SDVO output_flags SDVO may have multiple connectors > > linking to the same encoder (in intel_connector->encoder->base). > > Only one of those connectors should be active (ie link to the encoder > > thru drm_connector->encoder. > > If intel_connector_break_all_links() is called from intel_sanitize_crtc() > > we may brake the crtc connection of an encoder thru an inactive connector > > in which case intel_connector_break_all_links() will not be called again > > for the active connector due to > >if (connector->encoder->base.crtc != &crtc->base) > > continue; > > in intel_sanitize_crtc(). > > This will however leave the drm_connector->encoder linkage for this > > active connector in place. Subsequently this will cause multiple > > warnings in intel_connector_check_state() to trigger and the driver > > will eventually die in drm_encoder_crtc_ok() (because of crtc == NULL). > > > > To avoid this break the encoder links only if the connector is active > > (ie. has drm_connector->encoder set). > > > > Signed-off-by: Egbert Eich > > This just leaves me with a nagging doubt that not all links will then be > broken. Do we have sufficient sanity checks to detect the obverse? Would > it be worth adding a second pass over the connector_list to assert that > all links are then broken? Possibly. Maybe we should rework intel_sanitize_encoder() a bit: loop over all drm_connectors, see if a drm_connector->encoder points to the encoder in question and make sure that the intel_encoder state (ie connectors_active and base.crtc) is in sync with the results. I'll think about it some more ... Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Set up SDVO encoder type only at detect
Depending on the SDVO output_flags SDVO may have multiple connectors. These are all initialized in intel_sdvo_output_setup(). The connector that is initialized later will override the encoder_type that has been set up by an earlier connector type initialization. Eventually the one that comes last wins. Eventually when intel_sdvo_detect() is called the active connector is determined. Delay encoder type initialization until the active connector is known and set it to the type that corresponds to this connector. v2: Remove explicit encoder type initialization to DRM_MODE_ENCODER_NONE in the SDVO connector setup functions as suggested by Chris Wilson . Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/intel_sdvo.c | 21 +++-- 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index d27155a..f324ca1 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -154,6 +154,9 @@ struct intel_sdvo_connector { /* Mark the type of connector */ uint16_t output_flag; + /* store encoder type for convenience */ + int encoder_type; + enum hdmi_force_audio force_audio; /* This contains all current supported TV format */ @@ -1746,6 +1749,7 @@ intel_sdvo_detect(struct drm_connector *connector, bool force) if (response == 0) return connector_status_disconnected; + intel_sdvo->base.base.encoder_type = intel_sdvo_connector->encoder_type; intel_sdvo->attached_output = response; intel_sdvo->has_hdmi_monitor = false; @@ -2489,7 +2493,8 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device) } else { intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT; } - encoder->encoder_type = DRM_MODE_ENCODER_TMDS; + /* delay encoder_type setting until detection */ + intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_TMDS; connector->connector_type = DRM_MODE_CONNECTOR_DVID; if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) { @@ -2524,7 +2529,8 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type) intel_connector = &intel_sdvo_connector->base; connector = &intel_connector->base; - encoder->encoder_type = DRM_MODE_ENCODER_TVDAC; + /* delay encoder_type setting until detection */ + intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_TVDAC; connector->connector_type = DRM_MODE_CONNECTOR_SVIDEO; intel_sdvo->controlled_output |= type; @@ -2568,7 +2574,8 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device) intel_connector = &intel_sdvo_connector->base; connector = &intel_connector->base; intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT; - encoder->encoder_type = DRM_MODE_ENCODER_DAC; + /* delay encoder_type setting until detection */ + intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_DAC; connector->connector_type = DRM_MODE_CONNECTOR_VGA; if (device == 0) { @@ -2603,7 +2610,8 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device) intel_connector = &intel_sdvo_connector->base; connector = &intel_connector->base; - encoder->encoder_type = DRM_MODE_ENCODER_LVDS; + /* delay encoder_type setting until detection */ + intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_LVDS; connector->connector_type = DRM_MODE_CONNECTOR_LVDS; if (device == 0) { @@ -2981,10 +2989,11 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob) if (!intel_sdvo_init_ddc_proxy(intel_sdvo, dev)) goto err_i2c_bus; - /* encoder type will be decided later */ + /* encoder type will be decided in intel_sdvo_detect() */ intel_encoder = &intel_sdvo->base; intel_encoder->type = INTEL_OUTPUT_SDVO; - drm_encoder_init(dev, &intel_encoder->base, &intel_sdvo_enc_funcs, 0); + drm_encoder_init(dev, &intel_encoder->base, &intel_sdvo_enc_funcs, +DRM_MODE_ENCODER_NONE); /* Read the regs to test if we can talk to the device */ for (i = 0; i < 0x40; i++) { -- 1.8.4.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Set up SDVO encoder type only at detect
Daniel Vetter writes: > On Mon, Apr 14, 2014 at 07:26:09PM +0200, Egbert Eich wrote: > > Depending on the SDVO output_flags SDVO may have multiple connectors. > > These are all initialized in intel_sdvo_output_setup(). The connector > > that is initialized later will override the encoder_type that has been > > set up by an earlier connector type initialization. Eventually the > > one that comes last wins. > > Eventually when intel_sdvo_detect() is called the active connector is > > determined. > > Delay encoder type initialization until the active connector is known > > and set it to the type that corresponds to this connector. > > > > Signed-off-by: Egbert Eich > > Hm, has this any effect on the code itself? I think if we want to fix this > just for optics we should add a new DRM_MODE_ENCODER_MULTI. At least I > have an sdvo here which has a dac, hdmi and tv-out ... With the present logic the last connector in the list matching a flag bit will win the encoder type. The encoder type is presently just used for (debug) messages, so it is cosmetic. > > This also reminds that I should finally polish of the multi-sdvo fixes I > have around here - currently the last encoder detected wins on a > multi-encoder chip, which means if you have an lvds+hdmi combo and plug in > a screeen you'll never be able to use the lvds again until the hdmi is > unplugged. You know, from looking at the code I was wondering already if this was possible at all. I stayed away tinkering with this - there doesn't seem to be documentation on the SDVO command set publically available. Moreover I'm moslty dealing with cash registers here - so I doubt they have all the SVIDEO connectors they advertise ;) But I can't tell as I don't even have physical access! As I read the current code it seems like bad things could happen if more than one bit is set in intel_sdvo->attached_output: after all tv-out should have quite different timing requirements than a TMDS. > > Much worse if there's a tv-out detect issue ;-) ;p Cheers, Egbert. > > Cheers, Daniel > > > --- > > drivers/gpu/drm/i915/intel_sdvo.c | 23 ++- > > 1 file changed, 18 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c > > b/drivers/gpu/drm/i915/intel_sdvo.c > > index d27155a..5043f16 100644 > > --- a/drivers/gpu/drm/i915/intel_sdvo.c > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > > @@ -154,6 +154,9 @@ struct intel_sdvo_connector { > >/* Mark the type of connector */ > >uint16_t output_flag; > > > > + /* store encoder type for convenience */ > > + int encoder_type; > > + > >enum hdmi_force_audio force_audio; > > > >/* This contains all current supported TV format */ > > @@ -1746,6 +1749,7 @@ intel_sdvo_detect(struct drm_connector *connector, > > bool force) > >if (response == 0) > >return connector_status_disconnected; > > > > + intel_sdvo->base.base.encoder_type = intel_sdvo_connector->encoder_type; > >intel_sdvo->attached_output = response; > > > >intel_sdvo->has_hdmi_monitor = false; > > @@ -2489,7 +2493,9 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, > > int device) > >} else { > >intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | > > DRM_CONNECTOR_POLL_DISCONNECT; > >} > > - encoder->encoder_type = DRM_MODE_ENCODER_TMDS; > > + /* delay encoder_type setting until detection */ > > + intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_TMDS; > > + encoder->encoder_type = DRM_MODE_ENCODER_NONE; > >connector->connector_type = DRM_MODE_CONNECTOR_DVID; > > > >if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) { > > @@ -2524,7 +2530,9 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, > > int type) > > > >intel_connector = &intel_sdvo_connector->base; > >connector = &intel_connector->base; > > - encoder->encoder_type = DRM_MODE_ENCODER_TVDAC; > > + /* delay encoder_type setting until detection */ > > + intel_sdvo_connector->encoder_type = DRM_MODE_ENCODER_TVDAC; > > + encoder->encoder_type = DRM_MODE_ENCODER_NONE; > >connector->connector_type = DRM_MODE_CONNECTOR_SVIDEO; > > > >intel_sdvo->controlled_output |= type; > > @@ -2568,7 +2576,9 @@ intel_sdvo_analog_init(struct intel_sdvo > > *intel_sdvo, int device) > >intel_connector = &intel_sdvo_co
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Only break encoder linked when linked to connector
Daniel Vetter writes: > > Hm, I think to address Chris' concern we should split this into two > pieces: connector_break_all links which only breaks connector->encoder > stuff, used in both places as is. And a new encoder_break_all links which > we'll use in sanitize_crtc in a new encoder loop. I've got a different solution, although it requires a bit more code: Instead of having a single break_all_links() function I move the link breaking code into the two functions where it is used (ie sanitize_encoder() and sanitize_crtc()). This gives one a bit more flexibility in implementing what is needed and makes the code a bit clearer. I will send later - once I had the opportunity to test. Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Break encoder->crtc link separately in intel_sanitize_crtc()
Depending on the SDVO output_flags SDVO may have multiple connectors linking to the same encoder (in intel_connector->encoder->base). Only one of those connectors should be active (ie link to the encoder thru drm_connector->encoder). If intel_connector_break_all_links() is called from intel_sanitize_crtc() we may break the crtc connection of an encoder thru an inactive connector in which case intel_connector_break_all_links() will not be called again for the active connector if this happens to come later in the list due to: if (connector->encoder->base.crtc != &crtc->base) continue; in intel_sanitize_crtc(). This will however leave the drm_connector->encoder linkage for this active connector in place. Subsequently this will cause multiple warnings in intel_connector_check_state() to trigger and the driver will eventually die in drm_encoder_crtc_ok() (because of crtc == NULL). To avoid this remove intel_connector_break_all_links() and move its code to its two calling functions: intel_sanitize_crtc() and intel_sanitize_encoder(). This allows to implement the link breaking more flexibly matching the surrounding code: ie. in intel_sanitize_crtc() we can break the crtc link separatly after the links to the encoders have been broken which avoids above problem. Signed-off-by: Egbert Eich v2: This patch takes care of the concernes voiced by Chris Wilson and Daniel Vetter that only breaking links if the drm_connector is linked to an encoder may miss some links. --- drivers/gpu/drm/i915/intel_display.c | 25 + 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1390ab5..c276733 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11386,15 +11386,6 @@ void intel_modeset_init(struct drm_device *dev) } } -static void -intel_connector_break_all_links(struct intel_connector *connector) -{ - connector->base.dpms = DRM_MODE_DPMS_OFF; - connector->base.encoder = NULL; - connector->encoder->connectors_active = false; - connector->encoder->base.crtc = NULL; -} - static void intel_enable_pipe_a(struct drm_device *dev) { struct intel_connector *connector; @@ -11476,8 +11467,16 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) if (connector->encoder->base.crtc != &crtc->base) continue; - intel_connector_break_all_links(connector); + connector->base.dpms = DRM_MODE_DPMS_OFF; + connector->encoder->connectors_active = false; + connector->base.encoder = NULL; } + /* multiple connectors may have the same encoder: +* break crtc link separately */ + list_for_each_entry(connector, &dev->mode_config.connector_list, + base.head) + if (connector->encoder->base.crtc == &crtc->base) + connector->encoder->base.crtc = NULL; WARN_ON(crtc->active); crtc->base.enabled = false; @@ -11559,6 +11558,8 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder) drm_get_encoder_name(&encoder->base)); encoder->disable(encoder); } + encoder->base.crtc = NULL; + encoder->connectors_active = false; /* Inconsistent output/port/pipe state happens presumably due to * a bug in one of the get_hw_state functions. Or someplace else @@ -11569,8 +11570,8 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder) base.head) { if (connector->encoder != encoder) continue; - - intel_connector_break_all_links(connector); + connector->base.dpms = DRM_MODE_DPMS_OFF; + connector->base.encoder = NULL; } } /* Enabled encoders without active connectors will be fixed in -- 1.8.4.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Resurrect warning from intel_encoder_crtc_ok()
Bail out if crtc is NULL to keep the driver from crashing. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/intel_display.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c276733..dfebced 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10257,6 +10257,11 @@ intel_modeset_stage_output_state(struct drm_device *dev, new_crtc = set->crtc; } + if (!new_crtc) { + WARN(1, "crtc not set!"); + return -EINVAL; + } + /* Make sure the new CRTC will work with the encoder */ if (!drm_encoder_crtc_ok(&connector->new_encoder->base, new_crtc)) { -- 1.8.4.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915: Break encoder->crtc link separately in intel_sanitize_crtc()
Depending on the SDVO output_flags SDVO may have multiple connectors linking to the same encoder (in intel_connector->encoder->base). Only one of those connectors should be active (ie link to the encoder thru drm_connector->encoder). If intel_connector_break_all_links() is called from intel_sanitize_crtc() we may break the crtc connection of an encoder thru an inactive connector in which case intel_connector_break_all_links() will not be called again for the active connector if this happens to come later in the list due to: if (connector->encoder->base.crtc != &crtc->base) continue; in intel_sanitize_crtc(). This will however leave the drm_connector->encoder linkage for this active connector in place. Subsequently this will cause multiple warnings in intel_connector_check_state() to trigger and the driver will eventually die in drm_encoder_crtc_ok() (because of crtc == NULL). To avoid this remove intel_connector_break_all_links() and move its code to its two calling functions: intel_sanitize_crtc() and intel_sanitize_encoder(). This allows to implement the link breaking more flexibly matching the surrounding code: ie. in intel_sanitize_crtc() we can break the crtc link separatly after the links to the encoders have been broken which avoids above problem. This regression has been introduced in: commit 24929352481f085c5f85d4d4cbc919ddf106d381 Author: Daniel Vetter Date: Mon Jul 2 20:28:59 2012 +0200 drm/i915: read out the modeset hw state at load and resume time so goes back to the very beginning of the modeset rework. Signed-off-by: Egbert Eich Reviewed-by: Daniel Vetter Cc: sta...@vger.kernel.org Cc: Jani Nikula v2: This patch takes care of the concernes voiced by Chris Wilson and Daniel Vetter that only breaking links if the drm_connector is linked to an encoder may miss some links. v3: move all encoder handling to encoder loop as suggested by Daniel Vetter. --- drivers/gpu/drm/i915/intel_display.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b57210c..7ba8bf5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11373,15 +11373,6 @@ void intel_modeset_init(struct drm_device *dev) } } -static void -intel_connector_break_all_links(struct intel_connector *connector) -{ - connector->base.dpms = DRM_MODE_DPMS_OFF; - connector->base.encoder = NULL; - connector->encoder->connectors_active = false; - connector->encoder->base.crtc = NULL; -} - static void intel_enable_pipe_a(struct drm_device *dev) { struct intel_connector *connector; @@ -11463,8 +11454,17 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) if (connector->encoder->base.crtc != &crtc->base) continue; - intel_connector_break_all_links(connector); + connector->base.dpms = DRM_MODE_DPMS_OFF; + connector->base.encoder = NULL; } + /* multiple connectors may have the same encoder: +* handle them and break crtc link separately */ + list_for_each_entry(connector, &dev->mode_config.connector_list, + base.head) + if (connector->encoder->base.crtc == &crtc->base) { + connector->encoder->base.crtc = NULL; + connector->encoder->connectors_active = false; + } WARN_ON(crtc->active); crtc->base.enabled = false; @@ -11546,6 +11546,8 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder) drm_get_encoder_name(&encoder->base)); encoder->disable(encoder); } + encoder->base.crtc = NULL; + encoder->connectors_active = false; /* Inconsistent output/port/pipe state happens presumably due to * a bug in one of the get_hw_state functions. Or someplace else @@ -11556,8 +11558,8 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder) base.head) { if (connector->encoder != encoder) continue; - - intel_connector_break_all_links(connector); + connector->base.dpms = DRM_MODE_DPMS_OFF; + connector->base.encoder = NULL; } } /* Enabled encoders without active connectors will be fixed in -- 1.8.4.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/SDVO: For sysfs link put directory and target in correct order
When linking the i2c sysfs file into the connector's directory pass directory and link target in the right order. This code was introduced with: commit 931c1c26983b4f84e33b78579fc8d57e4a14c6b4 Author: Imre Deak Date: Tue Feb 11 17:12:51 2014 +0200 drm/i915: sdvo: add i2c sysfs symlink to the connector's directory This is the same what we do for DP connectors, so make things more consistent. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/intel_sdvo.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 5043f16..2d4d461 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -2428,8 +2428,8 @@ intel_sdvo_connector_init(struct intel_sdvo_connector *connector, if (ret < 0) goto err1; - ret = sysfs_create_link(&encoder->ddc.dev.kobj, - &drm_connector->kdev->kobj, + ret = sysfs_create_link(&drm_connector->kdev->kobj, + &encoder->ddc.dev.kobj, encoder->ddc.dev.kobj.name); if (ret < 0) goto err2; -- 1.8.4.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] HPD flood warning since b8f102e8b
Hi Jiri, Just found your email, it got missed do to a temporary inaccessibility to my email. Jiri Kosina writes: > On Thu, 3 Oct 2013, Daniel Vetter wrote: > > > Can you please attach full dmesg from boot up to the first WARN with > > drm.debug=0xe? This really shouldn't happen and indicates a bug > > somewhere ... > > A bit difficult ... I originally thought that it was reliably > reproducible, but now I didn't get it after 10 suspend/resume cycles. Will > keep following it, and once it appears, will send you the dmesg. > Could you check if you get any messages regarding HPD storms after suspend/resume ie messages like: "[drm] HPD interrupt storm detected on connector HDMI-A-1" but without the annouing warn messages? I cannot find anything obviously wrong in the code. However there are several code paths for different hardware though - could you give me an 'lspci -n' output so I can narrow them down? Thanks! Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] HPD flood warning since b8f102e8b
Jiri Kosina writes: > On Mon, 21 Oct 2013, Egbert Eich wrote: > > > > On Thu, 3 Oct 2013, Daniel Vetter wrote: > > > > > > > Can you please attach full dmesg from boot up to the first WARN with > > > > drm.debug=0xe? This really shouldn't happen and indicates a bug > > > > somewhere ... > > > > > > A bit difficult ... I originally thought that it was reliably > > > reproducible, but now I didn't get it after 10 suspend/resume cycles. > > Will > > > keep following it, and once it appears, will send you the dmesg. > > > > > Could you check if you get any messages regarding HPD storms after > > suspend/resume ie messages like: > > "[drm] HPD interrupt storm detected on connector HDMI-A-1" > > but without the annouing warn messages? > > I have this: > > [357128.184113] [drm] HPD interrupt storm detected on connector DP-3: > switching from hotplug detection to polling > > It appeared in the log approximately 5 seconds after resume has been > completed. > :( You seem to get this on different connector. Any chance for a 'lspci -n' output? Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v.2 00/12] Detect and deal with Interrupt 'Storms' from noisy Hotplug Lines.
I've reworked my 'hotplug interrupt storm detection'-patches and included most of Daniel's suggestions. I've looked into adding EDID caching but since this requires some larger scale changes and some changes outside of the Intel driver it seemed to be a good idea to propose those changes at a later time. Egbert Eich (12): DRM/i915: Remove valleyview_hpd_irq_setup. DRM/I915: Add enum hpd_pin to intel_encoder. DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in encoders. DRM/i915: Remove i965_hpd_irq_setup. DRM/i915: Get rid if the 'hotplug_supported_mask' in struct drm_i915_private. DRM/i915: Add HPD IRQ storm detection. DRM/i915: (re)init HPD interrupt storm statistics. DRM/i915: Treat hpd_irq_setup() for ironake and older generations the same way. DRM/i915: Disable HPD interrupt on pin when irq storm is detected. DRM/i915: Add Reenable Timer to turn Hotplug Detection back on. DRM/i915: Add bit field to record which pins have received HPD events. DRM/i915: Only reprobe display on encoder which has received an HPD event. drivers/gpu/drm/i915/i915_drv.h | 26 +++- drivers/gpu/drm/i915/i915_irq.c | 404 ++--- drivers/gpu/drm/i915/i915_reg.h | 32 +++- drivers/gpu/drm/i915/intel_crt.c | 10 +- drivers/gpu/drm/i915/intel_dp.c |8 +- drivers/gpu/drm/i915/intel_drv.h |5 + drivers/gpu/drm/i915/intel_hdmi.c |8 +- drivers/gpu/drm/i915/intel_sdvo.c | 17 +- drivers/gpu/drm/i915/intel_tv.c |2 +- 9 files changed, 372 insertions(+), 140 deletions(-) -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v.2 04/12] DRM/i915: Remove i965_hpd_irq_setup.
After "Convert HPD interrupts to make use of HPD pin assignment in encoders." This function is now basically the same as i915_hpd_irq_setup(). Consolidating both functions in one requires one more check for I915_HAS_HOTPLUG(dev) in the i965 code path and one more check for IS_G4X(dev) in the i915 code path. These are considered harmless. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 53 +- 1 files changed, 18 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 306cd79..c6ae7ae 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2424,25 +2424,6 @@ static int i915_irq_postinstall(struct drm_device *dev) return 0; } -static void i915_hpd_irq_setup(struct drm_device *dev) -{ - if (I915_HAS_HOTPLUG(dev)) { - drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; - struct drm_mode_config *mode_config = &dev->mode_config; - struct intel_encoder *encoder; - u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); - - hotplug_en &= ~HOTPLUG_INT_EN_MASK; - list_for_each_entry(encoder, &mode_config->encoder_list, base.head) - hotplug_en |= hpd_mask_i915[encoder->hpd_pin]; - hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50; - - /* Ignore TV since it's buggy */ - - I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); - } -} - static irqreturn_t i915_irq_handler(int irq, void *arg) { struct drm_device *dev = (struct drm_device *) arg; @@ -2649,29 +2630,31 @@ static int i965_irq_postinstall(struct drm_device *dev) return 0; } -static void i965_hpd_irq_setup(struct drm_device *dev) +static void i915_hpd_irq_setup(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; struct drm_mode_config *mode_config = &dev->mode_config; struct intel_encoder *encoder; u32 hotplug_en; - /* Note HDMI and DP share hotplug bits */ - hotplug_en = 0; - list_for_each_entry(encoder, &mode_config->encoder_list, base.head) + if (I915_HAS_HOTPLUG(dev)) { + hotplug_en = I915_READ(PORT_HOTPLUG_EN); + hotplug_en &= ~HOTPLUG_INT_EN_MASK; + /* Note HDMI and DP share hotplug bits */ /* enable bits are the same for all generations */ - hotplug_en |= hpd_mask_i915[encoder->hpd_pin]; - /* Programming the CRT detection parameters tends - to generate a spurious hotplug event about three - seconds later. So just do it once. - */ - if (IS_G4X(dev)) - hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64; - hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50; - - /* Ignore TV since it's buggy */ + list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) + hotplug_en |= hpd_mask_i915[intel_encoder->hpd_pin]; + /* Programming the CRT detection parameters tends + to generate a spurious hotplug event about three + seconds later. So just do it once. + */ + if (IS_G4X(dev)) + hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64; + hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50; - I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); + /* Ignore TV since it's buggy */ + I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); + } } static irqreturn_t i965_irq_handler(int irq, void *arg) @@ -2886,7 +2869,7 @@ void intel_irq_init(struct drm_device *dev) dev->driver->irq_postinstall = i965_irq_postinstall; dev->driver->irq_uninstall = i965_irq_uninstall; dev->driver->irq_handler = i965_irq_handler; - dev_priv->display.hpd_irq_setup = i965_hpd_irq_setup; + dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; } dev->driver->enable_vblank = i915_enable_vblank; dev->driver->disable_vblank = i915_disable_vblank; -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v.2 01/12] DRM/i915: Remove valleyview_hpd_irq_setup.
It's basically identical to i915_hpd_irq_setup(). Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 26 +- 1 files changed, 1 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2cd97d1..5fd3267 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2113,30 +2113,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev) return 0; } -static void valleyview_hpd_irq_setup(struct drm_device *dev) -{ - drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; - u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); - - /* Note HDMI and DP share bits */ - if (dev_priv->hotplug_supported_mask & PORTB_HOTPLUG_INT_STATUS) - hotplug_en |= PORTB_HOTPLUG_INT_EN; - if (dev_priv->hotplug_supported_mask & PORTC_HOTPLUG_INT_STATUS) - hotplug_en |= PORTC_HOTPLUG_INT_EN; - if (dev_priv->hotplug_supported_mask & PORTD_HOTPLUG_INT_STATUS) - hotplug_en |= PORTD_HOTPLUG_INT_EN; - if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS_I915) - hotplug_en |= SDVOC_HOTPLUG_INT_EN; - if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS_I915) - hotplug_en |= SDVOB_HOTPLUG_INT_EN; - if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS) { - hotplug_en |= CRT_HOTPLUG_INT_EN; - hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50; - } - - I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); -} - static void valleyview_irq_uninstall(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; @@ -2835,7 +2811,7 @@ void intel_irq_init(struct drm_device *dev) dev->driver->irq_uninstall = valleyview_irq_uninstall; dev->driver->enable_vblank = valleyview_enable_vblank; dev->driver->disable_vblank = valleyview_disable_vblank; - dev_priv->display.hpd_irq_setup = valleyview_hpd_irq_setup; + dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) { /* Share pre & uninstall handlers with ILK/SNB */ dev->driver->irq_handler = ivybridge_irq_handler; -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v.2 03/12] DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in encoders.
This allows to enable HPD interrupts for individual pins to only receive hotplug events from lines which are connected and working. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 160 -- drivers/gpu/drm/i915/i915_reg.h | 32 - 2 files changed, 132 insertions(+), 60 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 5fd3267..306cd79 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -36,6 +36,68 @@ #include "i915_trace.h" #include "intel_drv.h" +static const u32 hpd_ibx[] = { + 0, /* HPD_NONE */ + SDE_CRT_HOTPLUG, /* HPD_CRT */ + SDE_SDVOB_HOTPLUG, /* HPD_SDVO_B */ + 0, /* HPD_SDVO_C */ + SDE_PORTB_HOTPLUG, /* HPD_PORT_B */ + SDE_PORTC_HOTPLUG, /* HPD_PORT_C */ + SDE_PORTD_HOTPLUG /* HPD_PORT_D */ +}; + +static const u32 hpd_cpt[] = { + 0, /* HPD_NONE */ + SDE_CRT_HOTPLUG_CPT, /* HPD_CRT */ + 0, /* HPD_SDVO_B */ + 0, /* HPD_SDVO_C */ + SDE_PORTB_HOTPLUG_CPT, /* HPD_PORT_B */ + SDE_PORTC_HOTPLUG_CPT, /* HPD_PORT_C */ + SDE_PORTD_HOTPLUG_CPT /* HPD_PORT_D */ +}; + +static const u32 hpd_mask_i915[] = { + 0, /* HPD_NONE */ + CRT_HOTPLUG_INT_EN, /* HPD_CRT */ + SDVOB_HOTPLUG_INT_EN, /* HPD_SDVO_B */ + SDVOC_HOTPLUG_INT_EN, /* HPD_SDVO_C */ + PORTB_HOTPLUG_INT_EN, /* HPD_PORT_B */ + PORTC_HOTPLUG_INT_EN, /* HPD_PORT_C */ + PORTD_HOTPLUG_INT_EN /* HPD_PORT_D */ +}; + +static const u32 hpd_status_gen4[] = { + 0, /* HPD_NONE */ + CRT_HOTPLUG_INT_STATUS, /* HPD_CRT */ + SDVOB_HOTPLUG_INT_STATUS_G4X, /* HPD_SDVO_B */ + SDVOC_HOTPLUG_INT_STATUS_G4X, /* HPD_SDVO_C */ + PORTB_HOTPLUG_INT_STATUS, /* HPD_PORT_B */ + PORTC_HOTPLUG_INT_STATUS, /* HPD_PORT_C */ + PORTD_HOTPLUG_INT_STATUS /* HPD_PORT_D */ +}; + +static const u32 hpd_status_i965[] = { +0, /* HPD_NONE */ +CRT_HOTPLUG_INT_STATUS, /* HPD_CRT */ +SDVOB_HOTPLUG_INT_STATUS_I965, /* HPD_SDVO_B */ +SDVOC_HOTPLUG_INT_STATUS_I965, /* HPD_SDVO_C */ +PORTB_HOTPLUG_INT_STATUS, /* HPD_PORT_B */ +PORTC_HOTPLUG_INT_STATUS, /* HPD_PORT_C */ +PORTD_HOTPLUG_INT_STATUS /* HPD_PORT_D */ +}; + +static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */ + 0, /* HPD_NONE */ + CRT_HOTPLUG_INT_STATUS, /* HPD_CRT */ + SDVOB_HOTPLUG_INT_STATUS_I915, /* HPD_SDVO_B */ + SDVOC_HOTPLUG_INT_STATUS_I915, /* HPD_SDVO_C */ + PORTB_HOTPLUG_INT_STATUS, /* HPD_PORT_B */ + PORTC_HOTPLUG_INT_STATUS, /* HPD_PORT_C */ + PORTD_HOTPLUG_INT_STATUS /* HPD_PORT_D */ +}; + + + /* For display hotplug interrupt */ static void ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask) @@ -596,7 +658,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", hotplug_status); - if (hotplug_status & dev_priv->hotplug_supported_mask) + if (hotplug_status & HOTPLUG_INT_STATUS_I915) queue_work(dev_priv->wq, &dev_priv->hotplug_work); @@ -1940,17 +2002,21 @@ static void ibx_enable_hotplug(struct drm_device *dev) static void ibx_irq_postinstall(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; - u32 mask; - - if (HAS_PCH_IBX(dev)) - mask = SDE_HOTPLUG_MASK | - SDE_GMBUS | - SDE_AUX_MASK; - else - mask = SDE_HOTPLUG_MASK_CPT | - SDE_GMBUS_CPT | - SDE_AUX_MASK_CPT; + struct drm_mode_config *mode_config = &dev->mode_config; + struct intel_encoder *intel_encoder; + u32 mask = I915_READ(SDEIER); + if (HAS_PCH_IBX(dev)) { + mask &= ~SDE_HOTPLUG_MASK; + list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) + mask |= hpd_ibx[intel_encoder->hpd_pin]; + mask |= SDE_GMBUS | SDE_AUX_MASK; + } else { + mask &= ~SDE_HOTPLUG_MASK_CPT; + list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) + mask |= hpd_cpt[intel_encoder->hpd_pin]; + mask |= SDE_GMBUS_CPT | SDE_AUX_MASK_CPT; + } I915_WRITE(SDEIIR, I915_READ(SDEIIR)); I915_WRITE(SDEIMR, ~mask); I915_WRITE(SDEIER, mask); @@ -2360,26 +2426,16 @@ static int i915_irq_postinstall(struct drm_device *dev) static void i915_hpd_irq_setup(struct drm_device *dev) { - drm_i
[Intel-gfx] [PATCH v.2 02/12] DRM/I915: Add enum hpd_pin to intel_encoder.
To clean up hotplug support we add a new enum to intel_encoder: enum hpd_pin. It allows the encoder to request a hpd line but leave the details which IRQ is responsible on which chipset generation to i915_irq.c. This way requesting hotplug support will become really simple on the encoder/connector level. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h | 13 + drivers/gpu/drm/i915/intel_crt.c |2 ++ drivers/gpu/drm/i915/intel_dp.c |4 drivers/gpu/drm/i915/intel_drv.h |1 + drivers/gpu/drm/i915/intel_hdmi.c |4 drivers/gpu/drm/i915/intel_sdvo.c |3 +++ 6 files changed, 27 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e95337c..316747a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -86,6 +86,19 @@ enum port { }; #define port_name(p) ((p) + 'A') +enum hpd_pin { + HPD_NONE = 0, + HPD_PORT_A = HPD_NONE, /* PORT_A is internal */ + HPD_TV = HPD_NONE, /* TV is known to be unreliable */ + HPD_CRT, + HPD_SDVO_B, + HPD_SDVO_C, + HPD_PORT_B, + HPD_PORT_C, + HPD_PORT_D, + HPD_NUM_PINS +}; + #define I915_GEM_GPU_DOMAINS \ (I915_GEM_DOMAIN_RENDER | \ I915_GEM_DOMAIN_SAMPLER | \ diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index cfc9687..de43cab 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -777,6 +777,8 @@ void intel_crt_init(struct drm_device *dev) crt->base.disable = intel_disable_crt; crt->base.enable = intel_enable_crt; + if (I915_HAS_HOTPLUG(dev)) + crt->base.hpd_pin = HPD_CRT; if (HAS_DDI(dev)) crt->base.get_hw_state = intel_ddi_get_hw_state; else diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 7b8bfe8..549933a 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2835,18 +2835,22 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, /* Set up the DDC bus. */ switch (port) { case PORT_A: + intel_encoder->hpd_pin = HPD_PORT_A; name = "DPDDC-A"; break; case PORT_B: dev_priv->hotplug_supported_mask |= PORTB_HOTPLUG_INT_STATUS; + intel_encoder->hpd_pin = HPD_PORT_B; name = "DPDDC-B"; break; case PORT_C: dev_priv->hotplug_supported_mask |= PORTC_HOTPLUG_INT_STATUS; + intel_encoder->hpd_pin = HPD_PORT_C; name = "DPDDC-C"; break; case PORT_D: dev_priv->hotplug_supported_mask |= PORTD_HOTPLUG_INT_STATUS; + intel_encoder->hpd_pin = HPD_PORT_D; name = "DPDDC-D"; break; default: diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 005a91f..c9003bd 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -168,6 +168,7 @@ struct intel_encoder { * it is connected to in the pipe parameter. */ bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe); int crtc_mask; + enum hpd_pin hpd_pin; }; struct intel_panel { diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 5a6138c..8cd6bc1 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1022,17 +1022,21 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, switch (port) { case PORT_B: intel_hdmi->ddc_bus = GMBUS_PORT_DPB; + intel_encoder->hpd_pin = HPD_PORT_B; dev_priv->hotplug_supported_mask |= PORTB_HOTPLUG_INT_STATUS; break; case PORT_C: intel_hdmi->ddc_bus = GMBUS_PORT_DPC; + intel_encoder->hpd_pin = HPD_PORT_C; dev_priv->hotplug_supported_mask |= PORTC_HOTPLUG_INT_STATUS; break; case PORT_D: intel_hdmi->ddc_bus = GMBUS_PORT_DPD; + intel_encoder->hpd_pin = HPD_PORT_D; dev_priv->hotplug_supported_mask |= PORTD_HOTPLUG_INT_STATUS; break; case PORT_A: + intel_encoder->hpd_pin = HPD_PORT_A; /* Internal port only for eDP. */ default: BUG(); diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index f01063a..cf57705 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -2780,6 +2780,9 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvo
[Intel-gfx] [PATCH v.2 06/12] DRM/i915: Add HPD IRQ storm detection.
Add a hotplug IRQ storm detection (triggered when a hotplug interrupt fires more than 5 times / sec). Mask out this specific interrupt and revert to polling on the associated output. Rationale: Despite of the many attempts to fix the problem with noisy hotplug interrupt lines we are still seeing systems which have issues: Once cause of noise seems to be bad routing of the hotplug line on the board: cross talk from other signals seems to cause erronous hotplug interrupts. This has been documented as an erratum for the the i945GM chipset and thus hotplug support was disabled for this chipset model but others seem to have this problem, too. We have seen this issue on a G35 motherboard for example: Even different motherboards of the same model seem to behave differently: while some only see only around 10-100 interrupts/s others seem to see 5k or more. We've also observed a dependency on the selected video mode. Also on certain laptops interrupt noise seems to occur duing battery charging when the battery is at a certain charge levels. Thus we add a simple algorithm here that detects an 'interrupt storm' condition. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h |9 + drivers/gpu/drm/i915/i915_irq.c | 63 +++--- 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d8604a6..6ca742d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1059,6 +1059,15 @@ typedef struct drm_i915_private { /* Old dri1 support infrastructure, beware the dragons ya fools entering * here! */ struct i915_dri1_state dri1; + struct { + unsigned long hpd_last_jiffies; + int hpd_cnt; + enum { + HPD_ENABLED = 0, + HPD_DISABLED = 1, + HPD_MARK_DISABLED = 2 + } hpd_mark; + } hpd_stats[HPD_NUM_PINS]; } drm_i915_private_t; /* Iterate over initialised rings */ diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index c6ae7ae..08a0934 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -587,6 +587,34 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, queue_work(dev_priv->wq, &dev_priv->rps.work); } +static inline void hotplug_irq_storm_detect(struct drm_device *dev, + u32 hotplug_trigger, + const u32 *hpd) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + unsigned long irqflags; + int i; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + for (i = 1; i < HPD_NUM_PINS; i++) { + if ((hpd[i] & hotplug_trigger) && + dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) { + if (jiffies > (dev_priv->hpd_stats[i].hpd_last_jiffies + msecs_to_jiffies(1000)) || + jiffies < dev_priv->hpd_stats[i].hpd_last_jiffies) { + dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies; + dev_priv->hpd_stats[i].hpd_cnt = 0; + } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { + dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; + DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); + } else + dev_priv->hpd_stats[i].hpd_cnt++; + } + } + + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); +} + static void gmbus_irq_handler(struct drm_device *dev) { struct drm_i915_private *dev_priv = (drm_i915_private_t *) dev->dev_private; @@ -655,13 +683,15 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) /* Consume port. Then clear IIR or we'll miss events */ if (iir & I915_DISPLAY_PORT_INTERRUPT) { u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT); + u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915; DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", hotplug_status); - if (hotplug_status & HOTPLUG_INT_STATUS_I915) + if (hotplug_trigger) { + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915); queue_work(dev_priv->wq, &dev_priv->hotplug_work); - + } I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
[Intel-gfx] [PATCH v.2 07/12] DRM/i915: (re)init HPD interrupt storm statistics.
When an encoder is shared on several connectors there is only one hotplug line, thus this line needs to be shared among these connectors. If HPD detect only works reliably on a subset of those connectors, we want to poll the others. Thus we need to make sure that storm detection doesn't mess up the settings for those connectors. Therefore we store the settings in the intel_connector struct and restore them from there. If nothing is set but the encoder has a hpd_pin set we assume this connector is hotplug capable. On init/reset we make sure the polled state of the connectors is (re)set to the default value, the HPD interrupts are marked enabled. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 13 + drivers/gpu/drm/i915/intel_crt.c |6 ++ drivers/gpu/drm/i915/intel_dp.c |1 - drivers/gpu/drm/i915/intel_drv.h |4 drivers/gpu/drm/i915/intel_hdmi.c |1 - drivers/gpu/drm/i915/intel_sdvo.c |5 ++--- drivers/gpu/drm/i915/intel_tv.c |2 +- 7 files changed, 22 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 08a0934..d1ccff7 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2918,7 +2918,20 @@ void intel_irq_init(struct drm_device *dev) void intel_hpd_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_mode_config *mode_config = &dev->mode_config; + struct drm_connector *connector; + int i; + for (i = 1; i < HPD_NUM_PINS; i++) { + dev_priv->hpd_stats[i].hpd_cnt = 0; + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED; + } + list_for_each_entry(connector, &mode_config->connector_list, head) { + struct intel_connector *intel_connector = to_intel_connector(connector); + connector->polled = intel_connector->polled; + if (!connector->polled && I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE) + connector->polled = DRM_CONNECTOR_POLL_HPD; + } if (dev_priv->display.hpd_irq_setup) dev_priv->display.hpd_irq_setup(dev); } diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 5654583..4318f3e 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -790,10 +790,8 @@ void intel_crt_init(struct drm_device *dev) drm_sysfs_connector_add(connector); - if (I915_HAS_HOTPLUG(dev)) - connector->polled = DRM_CONNECTOR_POLL_HPD; - else - connector->polled = DRM_CONNECTOR_POLL_CONNECT; + if (!I915_HAS_HOTPLUG(dev)) + intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT; /* * Configure the automatic hotplug detection stuff diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5a77d40..66134d1 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2816,7 +2816,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, drm_connector_init(dev, connector, &intel_dp_connector_funcs, type); drm_connector_helper_add(connector, &intel_dp_connector_helper_funcs); - connector->polled = DRM_CONNECTOR_POLL_HPD; connector->interlace_allowed = true; connector->doublescan_allowed = 0; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index c9003bd..142f40a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -198,6 +198,10 @@ struct intel_connector { /* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */ struct edid *edid; + + /* since POLL and HPD connectors may use the same HPD line keep the native + state of connector->polled in case hotplug storm detection changes it */ + u8 polled; }; struct intel_crtc { diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index e48452a..82c9b9b 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1015,7 +1015,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, DRM_MODE_CONNECTOR_HDMIA); drm_connector_helper_add(connector, &intel_hdmi_connector_helper_funcs); - connector->polled = DRM_CONNECTOR_POLL_HPD; connector->interlace_allowed = 1; connector->doublescan_allowed = 0; diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index d362dd5..30635c6 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -2270,7 +2270,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_
[Intel-gfx] [PATCH v.2 05/12] DRM/i915: Get rid if the 'hotplug_supported_mask' in struct drm_i915_private.
Now since we have replaced the bits to show interest in hotplug IRQs we can go and nuke the 'hotplug_supported_mask'. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h |1 - drivers/gpu/drm/i915/intel_crt.c |2 -- drivers/gpu/drm/i915/intel_dp.c |3 --- drivers/gpu/drm/i915/intel_hdmi.c |3 --- drivers/gpu/drm/i915/intel_sdvo.c |9 +++-- 5 files changed, 3 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 316747a..d8604a6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -922,7 +922,6 @@ typedef struct drm_i915_private { u32 irq_mask; u32 gt_irq_mask; - u32 hotplug_supported_mask; struct work_struct hotplug_work; bool enable_hotplug_processing; diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index de43cab..5654583 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -800,8 +800,6 @@ void intel_crt_init(struct drm_device *dev) */ crt->force_hotplug_required = 0; - dev_priv->hotplug_supported_mask |= CRT_HOTPLUG_INT_STATUS; - /* * TODO: find a proper way to discover whether we need to set the the * polarity and link reversal bits or not, instead of relying on the diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 549933a..5a77d40 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2839,17 +2839,14 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, name = "DPDDC-A"; break; case PORT_B: - dev_priv->hotplug_supported_mask |= PORTB_HOTPLUG_INT_STATUS; intel_encoder->hpd_pin = HPD_PORT_B; name = "DPDDC-B"; break; case PORT_C: - dev_priv->hotplug_supported_mask |= PORTC_HOTPLUG_INT_STATUS; intel_encoder->hpd_pin = HPD_PORT_C; name = "DPDDC-C"; break; case PORT_D: - dev_priv->hotplug_supported_mask |= PORTD_HOTPLUG_INT_STATUS; intel_encoder->hpd_pin = HPD_PORT_D; name = "DPDDC-D"; break; diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 8cd6bc1..e48452a 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1023,17 +1023,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, case PORT_B: intel_hdmi->ddc_bus = GMBUS_PORT_DPB; intel_encoder->hpd_pin = HPD_PORT_B; - dev_priv->hotplug_supported_mask |= PORTB_HOTPLUG_INT_STATUS; break; case PORT_C: intel_hdmi->ddc_bus = GMBUS_PORT_DPC; intel_encoder->hpd_pin = HPD_PORT_C; - dev_priv->hotplug_supported_mask |= PORTC_HOTPLUG_INT_STATUS; break; case PORT_D: intel_hdmi->ddc_bus = GMBUS_PORT_DPD; intel_encoder->hpd_pin = HPD_PORT_D; - dev_priv->hotplug_supported_mask |= PORTD_HOTPLUG_INT_STATUS; break; case PORT_A: intel_encoder->hpd_pin = HPD_PORT_A; diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index cf57705..d362dd5 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -2780,6 +2780,9 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob) SDVOB_HOTPLUG_INT_STATUS_I915 : SDVOC_HOTPLUG_INT_STATUS_I915; } + /* Only enable the hotplug irq if we need it, to work around noisy +* hotplug lines. +*/ if (intel_sdvo->hotplug_active) intel_encoder->hpd_pin = HPD_SDVO_B ? HPD_SDVO_B : HPD_SDVO_C; @@ -2811,12 +2814,6 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob) */ intel_sdvo->base.cloneable = false; - /* Only enable the hotplug irq if we need it, to work around noisy -* hotplug lines. -*/ - if (intel_sdvo->hotplug_active) - dev_priv->hotplug_supported_mask |= hotplug_mask; - intel_sdvo_select_ddc_bus(dev_priv, intel_sdvo, sdvo_reg); /* Set the input timing to the screen. Assume always input 0. */ -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v.2 08/12] DRM/i915: Treat hpd_irq_setup() for ironake and older generations the same way.
When switching to enabling HPD IRQs only for lines where needed and supported this will ensure that the right lines will be enabled on all generations when intel_hpd_init() is called. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d1ccff7..e0cf629 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2033,7 +2033,7 @@ static void ibx_enable_hotplug(struct drm_device *dev) I915_WRITE(PCH_PORT_HOTPLUG, hotplug); } -static void ibx_irq_postinstall(struct drm_device *dev) +static void ibx_hpd_irq_setup(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; struct drm_mode_config *mode_config = &dev->mode_config; @@ -2055,8 +2055,6 @@ static void ibx_irq_postinstall(struct drm_device *dev) I915_WRITE(SDEIMR, ~mask); I915_WRITE(SDEIER, mask); POSTING_READ(SDEIER); - - ibx_enable_hotplug(dev); } static int ironlake_irq_postinstall(struct drm_device *dev) @@ -2094,7 +2092,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev) I915_WRITE(GTIER, render_irqs); POSTING_READ(GTIER); - ibx_irq_postinstall(dev); + ibx_enable_hotplug(dev); if (IS_IRONLAKE_M(dev)) { /* Clear & enable PCU event interrupts */ @@ -2140,7 +2138,7 @@ static int ivybridge_irq_postinstall(struct drm_device *dev) I915_WRITE(GTIER, render_irqs); POSTING_READ(GTIER); - ibx_irq_postinstall(dev); + ibx_enable_hotplug(dev); return 0; } @@ -2884,6 +2882,7 @@ void intel_irq_init(struct drm_device *dev) dev->driver->irq_uninstall = ironlake_irq_uninstall; dev->driver->enable_vblank = ivybridge_enable_vblank; dev->driver->disable_vblank = ivybridge_disable_vblank; + dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup; } else if (HAS_PCH_SPLIT(dev)) { dev->driver->irq_handler = ironlake_irq_handler; dev->driver->irq_preinstall = ironlake_irq_preinstall; @@ -2891,6 +2890,7 @@ void intel_irq_init(struct drm_device *dev) dev->driver->irq_uninstall = ironlake_irq_uninstall; dev->driver->enable_vblank = ironlake_enable_vblank; dev->driver->disable_vblank = ironlake_disable_vblank; + dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup; } else { if (INTEL_INFO(dev)->gen == 2) { dev->driver->irq_preinstall = i8xx_irq_preinstall; -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v.2 09/12] DRM/i915: Disable HPD interrupt on pin when irq storm is detected.
This patch disables hotplug interrupts if an 'interrupt storm' has been detected. Noise on the interrupt line renders the hotplug interrupt useless: each hotplug event causes the devices to be rescanned which will will only increase the system load. Thus disable the hotplug interrupts and fall back to periodic device polling. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 69 ++- 1 files changed, 53 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index e0cf629..8878fdd 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -96,7 +96,8 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */ PORTD_HOTPLUG_INT_STATUS /* HPD_PORT_D */ }; - +static void ibx_hpd_irq_setup(struct drm_device *dev); +static void i915_hpd_irq_setup(struct drm_device *dev); /* For display hotplug interrupt */ static void @@ -347,7 +348,11 @@ static void i915_hotplug_work_func(struct work_struct *work) hotplug_work); struct drm_device *dev = dev_priv->dev; struct drm_mode_config *mode_config = &dev->mode_config; - struct intel_encoder *encoder; + struct intel_connector *intel_connector; + struct intel_encoder *intel_encoder; + struct drm_connector *connector; + unsigned long irqflags; + bool connector_disabled = false; /* HPD irq before everything is fully set up. */ if (!dev_priv->enable_hotplug_processing) @@ -356,9 +361,29 @@ static void i915_hotplug_work_func(struct work_struct *work) mutex_lock(&mode_config->mutex); DRM_DEBUG_KMS("running encoder hotplug functions\n"); - list_for_each_entry(encoder, &mode_config->encoder_list, base.head) - if (encoder->hot_plug) - encoder->hot_plug(encoder); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + list_for_each_entry(connector, &mode_config->connector_list, head) { + intel_connector = to_intel_connector(connector); + intel_encoder = intel_connector->encoder; + if (intel_encoder->hpd_pin > HPD_NONE && + dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_MARK_DISABLED && + connector->polled == DRM_CONNECTOR_POLL_HPD) { + pr_warn("HPD interrupt storm detected on connector %s disabling\n", + drm_get_connector_name(connector)); + dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark = HPD_DISABLED; + connector->polled = DRM_CONNECTOR_POLL_CONNECT + | DRM_CONNECTOR_POLL_DISCONNECT; + connector_disabled = true; + } + } + if (connector_disabled) + drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */ + + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + + list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) + if (intel_encoder->hot_plug) + intel_encoder->hot_plug(encoder); mutex_unlock(&mode_config->mutex); @@ -587,13 +612,14 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, queue_work(dev_priv->wq, &dev_priv->rps.work); } -static inline void hotplug_irq_storm_detect(struct drm_device *dev, +static inline bool hotplug_irq_storm_detect(struct drm_device *dev, u32 hotplug_trigger, const u32 *hpd) { drm_i915_private_t *dev_priv = dev->dev_private; unsigned long irqflags; int i; + bool ret = false; spin_lock_irqsave(&dev_priv->irq_lock, irqflags); @@ -607,12 +633,15 @@ static inline void hotplug_irq_storm_detect(struct drm_device *dev, } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); + ret = true; } else dev_priv->hpd_stats[i].hpd_cnt++; } } spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + + return ret; } static void gmbus_irq_handler(struct drm_device *dev) @@ -688,7 +717,8 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) DRM_DEBUG_DRIVER("hotplug event received
[Intel-gfx] [PATCH v.2 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on.
We disable hoptplug detection when we encounter a hotplug event storm. Still hotplug detection is required on some outputs (like Display Port). The interrupt storm may be only temporary (on certain Dell Laptops for instance it happens at certain charging states of the system). Thus we enable it after a certain grace period (2 minutes). Should the interrupt storm persist it will be detected immediately and it will be disabled again. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h |2 + drivers/gpu/drm/i915/i915_irq.c | 53 ++- 2 files changed, 54 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6ca742d..58bee7a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1068,6 +1068,8 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; +#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000) + struct timer_list hotplug_reenable_timer; } drm_i915_private_t; /* Iterate over initialised rings */ diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 8878fdd..ba598e3 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -376,8 +376,11 @@ static void i915_hotplug_work_func(struct work_struct *work) connector_disabled = true; } } - if (connector_disabled) + if (connector_disabled) { drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */ + mod_timer(&dev_priv->hotplug_reenable_timer, + jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY)); + } spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); @@ -2253,6 +2256,8 @@ static void valleyview_irq_uninstall(struct drm_device *dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + for_each_pipe(pipe) I915_WRITE(PIPESTAT(pipe), 0x); @@ -2274,6 +2279,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + I915_WRITE(HWSTAM, 0x); I915_WRITE(DEIMR, 0x); @@ -2612,6 +2619,8 @@ static void i915_irq_uninstall(struct drm_device * dev) drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; int pipe; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + if (I915_HAS_HOTPLUG(dev)) { I915_WRITE(PORT_HOTPLUG_EN, 0); I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); @@ -2860,6 +2869,8 @@ static void i965_irq_uninstall(struct drm_device * dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + I915_WRITE(PORT_HOTPLUG_EN, 0); I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); @@ -2875,6 +2886,44 @@ static void i965_irq_uninstall(struct drm_device * dev) I915_WRITE(IIR, I915_READ(IIR)); } +static void i915_reenable_hotplug_timer_func(unsigned long data) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *)data; + struct drm_device *dev = dev_priv->dev; + struct drm_mode_config *mode_config = &dev->mode_config; + unsigned long irqflags; + int i; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + for (i = 1; i < HPD_NUM_PINS; i++) { + if (dev_priv->hpd_stats[i].hpd_mark == HPD_MARK_DISABLED) { + struct drm_connector *connector; + + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED; + list_for_each_entry(connector, &mode_config->connector_list, head) { + struct intel_connector *intel_connector = to_intel_connector(connector); + + if (intel_connector->encoder->hpd_pin == i) { + if (connector->polled != intel_connector->polled) + DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n", + drm_get_connector_name(connector)); + connector->polled = intel_connector->polled; + if (!connector->polled) + connector->polled = DRM_CONNECTOR_POLL_HPD; + } + } + + if (IS_HASWELL(dev) || + IS_IVYBRIDGE(dev) || +
[Intel-gfx] [PATCH v.2 11/12] DRM/i915: Add bit field to record which pins have received HPD events.
This way it is possible to limit 're'-detect() of displays to connectors which have received an HPD event. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h |1 + drivers/gpu/drm/i915/i915_irq.c | 10 ++ 2 files changed, 11 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 58bee7a..cd6391c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1068,6 +1068,7 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; + u32 hpd_event_bits; #define I915_REENABLE_HOTPLUG_DELAY (2*60*1000) struct timer_list hotplug_reenable_timer; } drm_i915_private_t; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index ba598e3..6e7b0b5 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -353,6 +353,7 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; unsigned long irqflags; bool connector_disabled = false; + u32 hpd_event_bits; /* HPD irq before everything is fully set up. */ if (!dev_priv->enable_hotplug_processing) @@ -362,6 +363,9 @@ static void i915_hotplug_work_func(struct work_struct *work) DRM_DEBUG_KMS("running encoder hotplug functions\n"); spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + hpd_event_bits = dev_priv->hpd_event_bits; + dev_priv->hpd_event_bits = 0; list_for_each_entry(connector, &mode_config->connector_list, head) { intel_connector = to_intel_connector(connector); intel_encoder = intel_connector->encoder; @@ -375,6 +379,10 @@ static void i915_hotplug_work_func(struct work_struct *work) | DRM_CONNECTOR_POLL_DISCONNECT; connector_disabled = true; } + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n", + drm_get_connector_name(connector), intel_encoder->hpd_pin); + } } if (connector_disabled) { drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */ @@ -629,12 +637,14 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev, for (i = 1; i < HPD_NUM_PINS; i++) { if ((hpd[i] & hotplug_trigger) && dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) { + dev_priv->hpd_event_bits |= (1 << i); if (jiffies > (dev_priv->hpd_stats[i].hpd_last_jiffies + msecs_to_jiffies(1000)) || jiffies < dev_priv->hpd_stats[i].hpd_last_jiffies) { dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies; dev_priv->hpd_stats[i].hpd_cnt = 0; } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; + dev_priv->hpd_event_bits &= ~(1 << i); DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); ret = true; } else -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v.2 12/12] DRM/i915: Only reprobe display on encoder which has received an HPD event.
Instead of calling into the DRM helper layer to poll all connectors for changes in connected displays probe only those connectors which have received a hotplug event. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 37 +++-- 1 files changed, 31 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 6e7b0b5..6c96d71 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -339,6 +339,24 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, crtc); } +/** + * drm_helper_hpd_irq_single_connector_event() - call this function with mode_config.mutex lock held + */ + +static int intel_hpd_irq_single_connector_event(struct drm_device *dev, struct drm_connector *connector) +{ + enum drm_connector_status old_status; + + old_status = connector->status; + + connector->status = connector->funcs->detect(connector, false); + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n", + connector->base.id, + drm_get_connector_name(connector), + old_status, connector->status); + return (old_status != connector->status); +} + /* * Handle hotplug events outside the interrupt handler proper. */ @@ -353,6 +371,7 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; unsigned long irqflags; bool connector_disabled = false; + bool changed = false; u32 hpd_event_bits; /* HPD irq before everything is fully set up. */ @@ -392,14 +411,20 @@ static void i915_hotplug_work_func(struct work_struct *work) spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); - list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) - if (intel_encoder->hot_plug) - intel_encoder->hot_plug(encoder); - + list_for_each_entry(connector, &mode_config->connector_list, head) { + intel_connector = to_intel_connector(connector); + intel_encoder = intel_connector->encoder; + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + if (intel_encoder->hot_plug) + intel_encoder->hot_plug(intel_encoder); + if (intel_hpd_irq_single_connector_event(dev, connector)) + changed = true; + } + } mutex_unlock(&mode_config->mutex); - /* Just fire off a uevent and let userspace tell us what to do */ - drm_helper_hpd_irq_event(dev); + if (changed) + drm_kms_helper_hotplug_event(dev); } static void ironlake_handle_rps_change(struct drm_device *dev) -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v.2 03/12] DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in encoders (v2)
This allows to enable HPD interrupts for individual pins to only receive hotplug events from lines which are connected and working. v2: Restructured initailization of const arrays following a suggstion by Chris Wilson Signed-off-by: Egbert Eich Acked-by: Chris Wilson --- drivers/gpu/drm/i915/i915_irq.c | 151 --- drivers/gpu/drm/i915/i915_reg.h | 32 - 2 files changed, 123 insertions(+), 60 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 5fd3267..7bdc90c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -36,6 +36,59 @@ #include "i915_trace.h" #include "intel_drv.h" +static const u32 hpd_ibx[] = { + [HPD_CRT] = SDE_CRT_HOTPLUG, + [HPD_SDVO_B] = SDE_SDVOB_HOTPLUG, + [HPD_PORT_B] = SDE_PORTB_HOTPLUG, + [HPD_PORT_C] = SDE_PORTC_HOTPLUG, + [HPD_PORT_D] = SDE_PORTD_HOTPLUG +}; + +static const u32 hpd_cpt[] = { + [HPD_CRT] = SDE_CRT_HOTPLUG_CPT, + [HPD_PORT_B] = SDE_PORTB_HOTPLUG_CPT, + [HPD_PORT_C] = SDE_PORTC_HOTPLUG_CPT, + [HPD_PORT_D] = SDE_PORTD_HOTPLUG_CPT +}; + +static const u32 hpd_mask_i915[] = { + [HPD_CRT] = CRT_HOTPLUG_INT_EN, + [HPD_SDVO_B] = SDVOB_HOTPLUG_INT_EN, + [HPD_SDVO_C] = SDVOC_HOTPLUG_INT_EN, + [HPD_PORT_B] = PORTB_HOTPLUG_INT_EN, + [HPD_PORT_C] = PORTC_HOTPLUG_INT_EN, + [HPD_PORT_D] = PORTD_HOTPLUG_INT_EN +}; + +static const u32 hpd_status_gen4[] = { + [HPD_CRT] = CRT_HOTPLUG_INT_STATUS, + [HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_G4X, + [HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_G4X, + [HPD_PORT_B] = PORTB_HOTPLUG_INT_STATUS, + [HPD_PORT_C] = PORTC_HOTPLUG_INT_STATUS, + [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS +}; + +static const u32 hpd_status_i965[] = { +[HPD_CRT] = CRT_HOTPLUG_INT_STATUS, +[HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_I965, +[HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_I965, +[HPD_PORT_B] = PORTB_HOTPLUG_INT_STATUS, +[HPD_PORT_C] = PORTC_HOTPLUG_INT_STATUS, +[HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS +}; + +static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */ + [HPD_CRT] = CRT_HOTPLUG_INT_STATUS, + [HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_I915, + [HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_I915, + [HPD_PORT_B] = PORTB_HOTPLUG_INT_STATUS, + [HPD_PORT_C] = PORTC_HOTPLUG_INT_STATUS, + [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS +}; + + + /* For display hotplug interrupt */ static void ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask) @@ -596,7 +649,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", hotplug_status); - if (hotplug_status & dev_priv->hotplug_supported_mask) + if (hotplug_status & HOTPLUG_INT_STATUS_I915) queue_work(dev_priv->wq, &dev_priv->hotplug_work); @@ -1940,17 +1993,21 @@ static void ibx_enable_hotplug(struct drm_device *dev) static void ibx_irq_postinstall(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; - u32 mask; - - if (HAS_PCH_IBX(dev)) - mask = SDE_HOTPLUG_MASK | - SDE_GMBUS | - SDE_AUX_MASK; - else - mask = SDE_HOTPLUG_MASK_CPT | - SDE_GMBUS_CPT | - SDE_AUX_MASK_CPT; + struct drm_mode_config *mode_config = &dev->mode_config; + struct intel_encoder *intel_encoder; + u32 mask = I915_READ(SDEIER); + if (HAS_PCH_IBX(dev)) { + mask &= ~SDE_HOTPLUG_MASK; + list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) + mask |= hpd_ibx[intel_encoder->hpd_pin]; + mask |= SDE_GMBUS | SDE_AUX_MASK; + } else { + mask &= ~SDE_HOTPLUG_MASK_CPT; + list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) + mask |= hpd_cpt[intel_encoder->hpd_pin]; + mask |= SDE_GMBUS_CPT | SDE_AUX_MASK_CPT; + } I915_WRITE(SDEIIR, I915_READ(SDEIIR)); I915_WRITE(SDEIMR, ~mask); I915_WRITE(SDEIER, mask); @@ -2360,26 +2417,16 @@ static int i915_irq_postinstall(struct drm_device *dev) static void i915_hpd_irq_setup(struct drm_device *dev) { - drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; - u32 hotplug_en; - if (I915_HAS_HOTPLUG(dev)) { - hotplug_en = I915_READ(PORT_HOTPLUG_EN); + drm
[Intel-gfx] [PATCH v.2 06/12] DRM/i915: Add HPD IRQ storm detection (v2)
Add a hotplug IRQ storm detection (triggered when a hotplug interrupt fires more than 5 times / sec). Rationale: Despite of the many attempts to fix the problem with noisy hotplug interrupt lines we are still seeing systems which have issues: Once cause of noise seems to be bad routing of the hotplug line on the board: cross talk from other signals seems to cause erronous hotplug interrupts. This has been documented as an erratum for the the i945GM chipset and thus hotplug support was disabled for this chipset model but others seem to have this problem, too. We have seen this issue on a G35 motherboard for example: Even different motherboards of the same model seem to behave differently: while some only see only around 10-100 interrupts/s others seem to see 5k or more. We've also observed a dependency on the selected video mode. Also on certain laptops interrupt noise seems to occur duing battery charging when the battery is at a certain charge levels. Thus we add a simple algorithm here that detects an 'interrupt storm' condition. v2: Fixed comment. Signed-off-by: Egbert Eich Acked-by: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.h |9 + drivers/gpu/drm/i915/i915_irq.c | 63 +++--- 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d8604a6..6ca742d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1059,6 +1059,15 @@ typedef struct drm_i915_private { /* Old dri1 support infrastructure, beware the dragons ya fools entering * here! */ struct i915_dri1_state dri1; + struct { + unsigned long hpd_last_jiffies; + int hpd_cnt; + enum { + HPD_ENABLED = 0, + HPD_DISABLED = 1, + HPD_MARK_DISABLED = 2 + } hpd_mark; + } hpd_stats[HPD_NUM_PINS]; } drm_i915_private_t; /* Iterate over initialised rings */ diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index c40c7cc..24cb6ed 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -578,6 +578,34 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, queue_work(dev_priv->wq, &dev_priv->rps.work); } +static inline void hotplug_irq_storm_detect(struct drm_device *dev, + u32 hotplug_trigger, + const u32 *hpd) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + unsigned long irqflags; + int i; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + for (i = 1; i < HPD_NUM_PINS; i++) { + if ((hpd[i] & hotplug_trigger) && + dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) { + if (jiffies > (dev_priv->hpd_stats[i].hpd_last_jiffies + msecs_to_jiffies(1000)) || + jiffies < dev_priv->hpd_stats[i].hpd_last_jiffies) { + dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies; + dev_priv->hpd_stats[i].hpd_cnt = 0; + } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { + dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; + DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); + } else + dev_priv->hpd_stats[i].hpd_cnt++; + } + } + + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); +} + static void gmbus_irq_handler(struct drm_device *dev) { struct drm_i915_private *dev_priv = (drm_i915_private_t *) dev->dev_private; @@ -646,13 +674,15 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) /* Consume port. Then clear IIR or we'll miss events */ if (iir & I915_DISPLAY_PORT_INTERRUPT) { u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT); + u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915; DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", hotplug_status); - if (hotplug_status & HOTPLUG_INT_STATUS_I915) + if (hotplug_trigger) { + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915); queue_work(dev_priv->wq, &dev_priv->hotplug_work); - + } I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
[Intel-gfx] [PATCH] DRM/i915: On G45 enable cursor plane briefly after enabling the display plane.
On G45 some low res modes (800x600 and 1024x768) produce a blank screen when the display plane is enabled with with cursor plane off. Experiments showed that this issue occurred when the following conditions were met: a. a previous mode had the cursor plane enabled (Xserver). b. this mode or the previous one was using self refresh. (Thus the problem was only seen with low res modes). The screens lit up as soon as the cursor plane got enabled. Therefore the blank screen occurred only in console mode, not when running an Xserver. It also seemed to be necessary to disable self refresh while briefly enabling the cursor plane. Signed-off-by: Egbert Eich Bugzilla: https://bugs.freedesktop.org/attachment.cgi?bugid=61457 Acked-by: Chris Wilson --- drivers/gpu/drm/i915/intel_display.c | 28 +++- 1 files changed, 27 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6eb3882..689cc3a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3593,6 +3593,30 @@ static void intel_crtc_dpms_overlay(struct intel_crtc *intel_crtc, bool enable) */ } +/** + * i9xx_fixup_plane - ugly workaround for G45 to fire up the hardware + * cursor plane briefly if not already running after enabling the display + * plane. + * This workaround avoids occasional blank screens when self refresh is + * enabled. + */ +static void +g4x_fixup_plane(struct drm_i915_private *dev_priv, enum pipe pipe) +{ + u32 cntl = I915_READ(CURCNTR(pipe)); + + if ((cntl & CURSOR_MODE) == 0) { + u32 fw_bcl_self = I915_READ(FW_BLC_SELF); + + I915_WRITE(FW_BLC_SELF, fw_bcl_self & ~FW_BLC_SELF_EN); + I915_WRITE(CURCNTR(pipe), CURSOR_MODE_64_ARGB_AX); + intel_wait_for_vblank(dev_priv->dev, pipe); + I915_WRITE(CURCNTR(pipe), cntl); + I915_WRITE(CURBASE(pipe), I915_READ(CURBASE(pipe))); + I915_WRITE(FW_BLC_SELF, fw_bcl_self); + } +} + static void i9xx_crtc_enable(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -3618,6 +3642,8 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_enable_pipe(dev_priv, pipe, false); intel_enable_plane(dev_priv, plane, pipe); + if (IS_G4X(dev)) + g4x_fixup_plane(dev_priv, pipe); intel_crtc_load_lut(crtc); intel_update_fbc(dev); @@ -6337,7 +6363,7 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc, i915_gem_detach_phys_object(dev, intel_crtc->cursor_bo); } else i915_gem_object_unpin(intel_crtc->cursor_bo); - drm_gem_object_unreference(&intel_crtc->cursor_bo->base); + drm_gem_object_unreference(&intel_crtc->cursor_bo->base); } mutex_unlock(&dev->struct_mutex); -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v.3 06/12] DRM/i915: Add HPD IRQ storm detection (v3)
Add a hotplug IRQ storm detection (triggered when a hotplug interrupt fires more than 5 times / sec). Rationale: Despite of the many attempts to fix the problem with noisy hotplug interrupt lines we are still seeing systems which have issues: Once cause of noise seems to be bad routing of the hotplug line on the board: cross talk from other signals seems to cause erronous hotplug interrupts. This has been documented as an erratum for the the i945GM chipset and thus hotplug support was disabled for this chipset model but others seem to have this problem, too. We have seen this issue on a G35 motherboard for example: Even different motherboards of the same model seem to behave differently: while some only see only around 10-100 interrupts/s others seem to see 5k or more. We've also observed a dependency on the selected video mode. Also on certain laptops interrupt noise seems to occur duing battery charging when the battery is at a certain charge levels. Thus we add a simple algorithm here that detects an 'interrupt storm' condition. v2: Fixed comment. v3: Reordered drm_i915_private: moved hpd state tracking to hotplug work stuff. Signed-off-by: Egbert Eich Acked-by: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.h |9 + drivers/gpu/drm/i915/i915_irq.c | 63 +++--- 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d8604a6..296278f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -924,6 +924,15 @@ typedef struct drm_i915_private { struct work_struct hotplug_work; bool enable_hotplug_processing; + struct { + unsigned long hpd_last_jiffies; + int hpd_cnt; + enum { + HPD_ENABLED = 0, + HPD_DISABLED = 1, + HPD_MARK_DISABLED = 2 + } hpd_mark; + } hpd_stats[HPD_NUM_PINS]; int num_pipe; int num_pch_pll; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index c40c7cc..24cb6ed 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -578,6 +578,34 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, queue_work(dev_priv->wq, &dev_priv->rps.work); } +static inline void hotplug_irq_storm_detect(struct drm_device *dev, + u32 hotplug_trigger, + const u32 *hpd) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + unsigned long irqflags; + int i; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + for (i = 1; i < HPD_NUM_PINS; i++) { + if ((hpd[i] & hotplug_trigger) && + dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) { + if (jiffies > (dev_priv->hpd_stats[i].hpd_last_jiffies + msecs_to_jiffies(1000)) || + jiffies < dev_priv->hpd_stats[i].hpd_last_jiffies) { + dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies; + dev_priv->hpd_stats[i].hpd_cnt = 0; + } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { + dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; + DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); + } else + dev_priv->hpd_stats[i].hpd_cnt++; + } + } + + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); +} + static void gmbus_irq_handler(struct drm_device *dev) { struct drm_i915_private *dev_priv = (drm_i915_private_t *) dev->dev_private; @@ -646,13 +674,15 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) /* Consume port. Then clear IIR or we'll miss events */ if (iir & I915_DISPLAY_PORT_INTERRUPT) { u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT); + u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915; DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", hotplug_status); - if (hotplug_status & HOTPLUG_INT_STATUS_I915) + if (hotplug_trigger) { + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915); queue_work(dev_priv->wq, &dev_priv->hotplug_work); - + } I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
[Intel-gfx] [PATCH v.2 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on (v2).
We disable hoptplug detection when we encounter a hotplug event storm. Still hotplug detection is required on some outputs (like Display Port). The interrupt storm may be only temporary (on certain Dell Laptops for instance it happens at certain charging states of the system). Thus we enable it after a certain grace period (2 minutes). Should the interrupt storm persist it will be detected immediately and it will be disabled again. v2: Reordered drm_i915_private: moved hotplug_reenable_timer to hpd state tracker. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h |2 + drivers/gpu/drm/i915/i915_irq.c | 53 ++- 2 files changed, 54 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 296278f..1fb7c44 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -933,6 +933,8 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; +#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000) + struct timer_list hotplug_reenable_timer; int num_pipe; int num_pch_pll; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d11534c..c688b27 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -367,8 +367,11 @@ static void i915_hotplug_work_func(struct work_struct *work) connector_disabled = true; } } - if (connector_disabled) + if (connector_disabled) { drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */ + mod_timer(&dev_priv->hotplug_reenable_timer, + jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY)); + } spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); @@ -2244,6 +2247,8 @@ static void valleyview_irq_uninstall(struct drm_device *dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + for_each_pipe(pipe) I915_WRITE(PIPESTAT(pipe), 0x); @@ -2265,6 +2270,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + I915_WRITE(HWSTAM, 0x); I915_WRITE(DEIMR, 0x); @@ -2603,6 +2610,8 @@ static void i915_irq_uninstall(struct drm_device * dev) drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; int pipe; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + if (I915_HAS_HOTPLUG(dev)) { I915_WRITE(PORT_HOTPLUG_EN, 0); I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); @@ -2851,6 +2860,8 @@ static void i965_irq_uninstall(struct drm_device * dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + I915_WRITE(PORT_HOTPLUG_EN, 0); I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); @@ -2866,6 +2877,44 @@ static void i965_irq_uninstall(struct drm_device * dev) I915_WRITE(IIR, I915_READ(IIR)); } +static void i915_reenable_hotplug_timer_func(unsigned long data) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *)data; + struct drm_device *dev = dev_priv->dev; + struct drm_mode_config *mode_config = &dev->mode_config; + unsigned long irqflags; + int i; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + for (i = 1; i < HPD_NUM_PINS; i++) { + if (dev_priv->hpd_stats[i].hpd_mark == HPD_MARK_DISABLED) { + struct drm_connector *connector; + + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED; + list_for_each_entry(connector, &mode_config->connector_list, head) { + struct intel_connector *intel_connector = to_intel_connector(connector); + + if (intel_connector->encoder->hpd_pin == i) { + if (connector->polled != intel_connector->polled) + DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n", + drm_get_connector_name(connector)); + connector->polled = intel_connector->polled; + if (!connector->polled) + connector->polled = DRM_CONNECTOR_POLL_HPD; + } + } + + if (IS
[Intel-gfx] [PATCH v.2 11/12] DRM/i915: Add bit field to record which pins have received HPD events (v2)
This way it is possible to limit 're'-detect() of displays to connectors which have received an HPD event. v2: Reordered drm_i915_private: Move hpd_event_bits to hpd state tracking. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h |1 + drivers/gpu/drm/i915/i915_irq.c | 10 ++ 2 files changed, 11 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1fb7c44..d48d1e6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -933,6 +933,7 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; + u32 hpd_event_bits; #define I915_REENABLE_HOTPLUG_DELAY (2*60*1000) struct timer_list hotplug_reenable_timer; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index c688b27..223813f 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -344,6 +344,7 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; unsigned long irqflags; bool connector_disabled = false; + u32 hpd_event_bits; /* HPD irq before everything is fully set up. */ if (!dev_priv->enable_hotplug_processing) @@ -353,6 +354,9 @@ static void i915_hotplug_work_func(struct work_struct *work) DRM_DEBUG_KMS("running encoder hotplug functions\n"); spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + hpd_event_bits = dev_priv->hpd_event_bits; + dev_priv->hpd_event_bits = 0; list_for_each_entry(connector, &mode_config->connector_list, head) { intel_connector = to_intel_connector(connector); intel_encoder = intel_connector->encoder; @@ -366,6 +370,10 @@ static void i915_hotplug_work_func(struct work_struct *work) | DRM_CONNECTOR_POLL_DISCONNECT; connector_disabled = true; } + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n", + drm_get_connector_name(connector), intel_encoder->hpd_pin); + } } if (connector_disabled) { drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */ @@ -620,12 +628,14 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev, for (i = 1; i < HPD_NUM_PINS; i++) { if ((hpd[i] & hotplug_trigger) && dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) { + dev_priv->hpd_event_bits |= (1 << i); if (jiffies > (dev_priv->hpd_stats[i].hpd_last_jiffies + msecs_to_jiffies(1000)) || jiffies < dev_priv->hpd_stats[i].hpd_last_jiffies) { dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies; dev_priv->hpd_stats[i].hpd_cnt = 0; } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; + dev_priv->hpd_event_bits &= ~(1 << i); DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); ret = true; } else -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v.3 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on (v3).
We disable hoptplug detection when we encounter a hotplug event storm. Still hotplug detection is required on some outputs (like Display Port). The interrupt storm may be only temporary (on certain Dell Laptops for instance it happens at certain charging states of the system). Thus we enable it after a certain grace period (2 minutes). Should the interrupt storm persist it will be detected immediately and it will be disabled again. v2: Reordered drm_i915_private: moved hotplug_reenable_timer to hpd state tracker. v3: Clarified loop start value, Removed superfluous test for Ivybridge and Haswell, Restructured loop to avoid deep nesting (all suggested by Ville Syrjälä) Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h |2 + drivers/gpu/drm/i915/i915_irq.c | 53 ++- 2 files changed, 54 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 296278f..1fb7c44 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -933,6 +933,8 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; +#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000) + struct timer_list hotplug_reenable_timer; int num_pipe; int num_pch_pll; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d11534c..76903af 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -367,8 +367,11 @@ static void i915_hotplug_work_func(struct work_struct *work) connector_disabled = true; } } - if (connector_disabled) + if (connector_disabled) { drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */ + mod_timer(&dev_priv->hotplug_reenable_timer, + jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY)); + } spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); @@ -2244,6 +2247,8 @@ static void valleyview_irq_uninstall(struct drm_device *dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + for_each_pipe(pipe) I915_WRITE(PIPESTAT(pipe), 0x); @@ -2265,6 +2270,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + I915_WRITE(HWSTAM, 0x); I915_WRITE(DEIMR, 0x); @@ -2603,6 +2610,8 @@ static void i915_irq_uninstall(struct drm_device * dev) drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; int pipe; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + if (I915_HAS_HOTPLUG(dev)) { I915_WRITE(PORT_HOTPLUG_EN, 0); I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); @@ -2851,6 +2860,8 @@ static void i965_irq_uninstall(struct drm_device * dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + I915_WRITE(PORT_HOTPLUG_EN, 0); I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); @@ -2866,6 +2877,44 @@ static void i965_irq_uninstall(struct drm_device * dev) I915_WRITE(IIR, I915_READ(IIR)); } +static void i915_reenable_hotplug_timer_func(unsigned long data) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *)data; + struct drm_device *dev = dev_priv->dev; + struct drm_mode_config *mode_config = &dev->mode_config; + unsigned long irqflags; + int i; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + for (i = (HPD_NONE + 1); i < HPD_NUM_PINS; i++) { + struct drm_connector *connector; + + if (dev_priv->hpd_stats[i].hpd_mark != HPD_MARK_DISABLED) + continue; + + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED; + + list_for_each_entry(connector, &mode_config->connector_list, head) { + struct intel_connector *intel_connector = to_intel_connector(connector); + + if (intel_connector->encoder->hpd_pin == i) { + if (connector->polled != intel_connector->polled) + DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n", + drm_get_connector_name(connector)); + connector->polled = intel_connector->polled; + if (!connector->polled) + connector->polle
[Intel-gfx] [PATCH v.2 09/12] DRM/i915: Disable HPD interrupt on pin when irq storm is detected (v2)
This patch disables hotplug interrupts if an 'interrupt storm' has been detected. Noise on the interrupt line renders the hotplug interrupt useless: each hotplug event causes the devices to be rescanned which will will only increase the system load. Thus disable the hotplug interrupts and fall back to periodic device polling. v2: Fixed cleanup typo. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 69 ++- 1 files changed, 53 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d9c99d7..0abef6a 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -87,7 +87,8 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */ [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS }; - +static void ibx_hpd_irq_setup(struct drm_device *dev); +static void i915_hpd_irq_setup(struct drm_device *dev); /* For display hotplug interrupt */ static void @@ -338,7 +339,11 @@ static void i915_hotplug_work_func(struct work_struct *work) hotplug_work); struct drm_device *dev = dev_priv->dev; struct drm_mode_config *mode_config = &dev->mode_config; - struct intel_encoder *encoder; + struct intel_connector *intel_connector; + struct intel_encoder *intel_encoder; + struct drm_connector *connector; + unsigned long irqflags; + bool connector_disabled = false; /* HPD irq before everything is fully set up. */ if (!dev_priv->enable_hotplug_processing) @@ -347,9 +352,29 @@ static void i915_hotplug_work_func(struct work_struct *work) mutex_lock(&mode_config->mutex); DRM_DEBUG_KMS("running encoder hotplug functions\n"); - list_for_each_entry(encoder, &mode_config->encoder_list, base.head) - if (encoder->hot_plug) - encoder->hot_plug(encoder); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + list_for_each_entry(connector, &mode_config->connector_list, head) { + intel_connector = to_intel_connector(connector); + intel_encoder = intel_connector->encoder; + if (intel_encoder->hpd_pin > HPD_NONE && + dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_MARK_DISABLED && + connector->polled == DRM_CONNECTOR_POLL_HPD) { + pr_warn("HPD interrupt storm detected on connector %s disabling\n", + drm_get_connector_name(connector)); + dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark = HPD_DISABLED; + connector->polled = DRM_CONNECTOR_POLL_CONNECT + | DRM_CONNECTOR_POLL_DISCONNECT; + connector_disabled = true; + } + } + if (connector_disabled) + drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */ + + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + + list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) + if (intel_encoder->hot_plug) + intel_encoder->hot_plug(intel_encoder); mutex_unlock(&mode_config->mutex); @@ -578,13 +603,14 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, queue_work(dev_priv->wq, &dev_priv->rps.work); } -static inline void hotplug_irq_storm_detect(struct drm_device *dev, +static inline bool hotplug_irq_storm_detect(struct drm_device *dev, u32 hotplug_trigger, const u32 *hpd) { drm_i915_private_t *dev_priv = dev->dev_private; unsigned long irqflags; int i; + bool ret = false; spin_lock_irqsave(&dev_priv->irq_lock, irqflags); @@ -598,12 +624,15 @@ static inline void hotplug_irq_storm_detect(struct drm_device *dev, } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); + ret = true; } else dev_priv->hpd_stats[i].hpd_cnt++; } } spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + + return ret; } static void gmbus_irq_handler(struct drm_device *dev) @@ -679,7 +708,8 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) DRM_DEBUG_DRIVER("hotpl
[Intel-gfx] [PATCH v.3 11/12] DRM/i915: Add bit field to record which pins have received HPD events (v3)
This way it is possible to limit 're'-detect() of displays to connectors which have received an HPD event. v2: Reordered drm_i915_private: Move hpd_event_bits to hpd state tracking. v3: Fix patch. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h |1 + drivers/gpu/drm/i915/i915_irq.c | 47 ++- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1fb7c44..d48d1e6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -933,6 +933,7 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; + u32 hpd_event_bits; #define I915_REENABLE_HOTPLUG_DELAY (2*60*1000) struct timer_list hotplug_reenable_timer; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index fd68b21..9e8d5b4 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -330,6 +330,24 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, crtc); } +/** + * drm_helper_hpd_irq_single_connector_event() - call this function with mode_config.mutex lock held + */ + +static int intel_hpd_irq_single_connector_event(struct drm_device *dev, struct drm_connector *connector) +{ + enum drm_connector_status old_status; + + old_status = connector->status; + + connector->status = connector->funcs->detect(connector, false); + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n", + connector->base.id, + drm_get_connector_name(connector), + old_status, connector->status); + return (old_status != connector->status); +} + /* * Handle hotplug events outside the interrupt handler proper. */ @@ -344,6 +362,8 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; unsigned long irqflags; bool connector_disabled = false; + bool changed = false; + u32 hpd_event_bits; /* HPD irq before everything is fully set up. */ if (!dev_priv->enable_hotplug_processing) @@ -353,6 +373,9 @@ static void i915_hotplug_work_func(struct work_struct *work) DRM_DEBUG_KMS("running encoder hotplug functions\n"); spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + hpd_event_bits = dev_priv->hpd_event_bits; + dev_priv->hpd_event_bits = 0; list_for_each_entry(connector, &mode_config->connector_list, head) { intel_connector = to_intel_connector(connector); intel_encoder = intel_connector->encoder; @@ -366,6 +389,10 @@ static void i915_hotplug_work_func(struct work_struct *work) | DRM_CONNECTOR_POLL_DISCONNECT; connector_disabled = true; } + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n", + drm_get_connector_name(connector), intel_encoder->hpd_pin); + } } if (connector_disabled) { drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */ @@ -375,14 +402,20 @@ static void i915_hotplug_work_func(struct work_struct *work) spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); - list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) - if (intel_encoder->hot_plug) - intel_encoder->hot_plug(intel_encoder); - + list_for_each_entry(connector, &mode_config->connector_list, head) { + intel_connector = to_intel_connector(connector); + intel_encoder = intel_connector->encoder; + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + if (intel_encoder->hot_plug) + intel_encoder->hot_plug(intel_encoder); + if (intel_hpd_irq_single_connector_event(dev, connector)) + changed = true; + } + } mutex_unlock(&mode_config->mutex); - /* Just fire off a uevent and let userspace tell us what to do */ - drm_helper_hpd_irq_event(dev); + if (changed) + drm_kms_helper_hotplug_event(dev); } static void ironlake_handle_rps_change(struct drm_device *dev) @@ -620,12 +653,14 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev, for (i = 1; i < HPD_NUM_PINS; i++) { i
Re: [Intel-gfx] [PATCH v.3 11/12] DRM/i915: Add bit field to record which pins have received HPD events (v3)
On Tue, Mar 05, 2013 at 08:00:30AM -0500, Egbert Eich wrote: > This way it is possible to limit 're'-detect() of displays to connectors > which have received an HPD event. > > v2: Reordered drm_i915_private: Move hpd_event_bits to hpd state tracking. > v3: Fix patch. > Oops, forget this patch. It's messed up. Sorry, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v.3 12/12] DRM/i915: Only reprobe display on encoder which has received an HPD event.
Instead of calling into the DRM helper layer to poll all connectors for changes in connected displays probe only those connectors which have received a hotplug event. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 37 +++-- 1 files changed, 31 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d65d76a..9e8d5b4 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -330,6 +330,24 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, crtc); } +/** + * drm_helper_hpd_irq_single_connector_event() - call this function with mode_config.mutex lock held + */ + +static int intel_hpd_irq_single_connector_event(struct drm_device *dev, struct drm_connector *connector) +{ + enum drm_connector_status old_status; + + old_status = connector->status; + + connector->status = connector->funcs->detect(connector, false); + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n", + connector->base.id, + drm_get_connector_name(connector), + old_status, connector->status); + return (old_status != connector->status); +} + /* * Handle hotplug events outside the interrupt handler proper. */ @@ -344,6 +362,7 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; unsigned long irqflags; bool connector_disabled = false; + bool changed = false; u32 hpd_event_bits; /* HPD irq before everything is fully set up. */ @@ -383,14 +402,20 @@ static void i915_hotplug_work_func(struct work_struct *work) spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); - list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) - if (intel_encoder->hot_plug) - intel_encoder->hot_plug(intel_encoder); - + list_for_each_entry(connector, &mode_config->connector_list, head) { + intel_connector = to_intel_connector(connector); + intel_encoder = intel_connector->encoder; + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + if (intel_encoder->hot_plug) + intel_encoder->hot_plug(intel_encoder); + if (intel_hpd_irq_single_connector_event(dev, connector)) + changed = true; + } + } mutex_unlock(&mode_config->mutex); - /* Just fire off a uevent and let userspace tell us what to do */ - drm_helper_hpd_irq_event(dev); + if (changed) + drm_kms_helper_hotplug_event(dev); } static void ironlake_handle_rps_change(struct drm_device *dev) -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] i915 fails to build on 32bit Systems
i915.ko does not build due to the following function in i915_debugfs.c: i915_min_freq_set(void *data, u64 val) { ... dev_priv->rps.min_delay = val / GT_FREQUENCY_MULTIPLIER; ... } Doing a 64bit integer division on 32bit requires a compiler run time library to be linked in, otherwise the symbol __udivdi3 will be missing. This was introduced with commit 2389cc5006861a8e024a23cb73605f2ab6a5afe8 Author: Kees Cook Date: Sun Mar 10 14:10:06 2013 -0700 drm/i915: use simple attribute in debugfs routines before this val was a simple int. Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v.2 05/12] DRM/i915: Get rid if the 'hotplug_supported_mask' in struct drm_i915_private.
Daniel Vetter writes: > On Mon, Feb 25, 2013 at 12:06:52PM -0500, Egbert Eich wrote: > > Now since we have replaced the bits to show interest in hotplug IRQs > > we can go and nuke the 'hotplug_supported_mask'. > > > > Signed-off-by: Egbert Eich > > I've applied your patch up to this one. Patch four needed some manual > convincing to fit correctly, but the new hpd infrastructure is now in > place. > > To get the actual hpd irq storm detection going I think it'd be good to > resend the remaining patches on top of latest drm-intel-nightly. I'll try > to yell at people a bit harder to review things more timely this time > around. > > Thanks for the patches. Thanks guys for reviewing the patches. I will try to spare some time tonight to look into the issues that came up. I got dragged into other tasks as well so I didn't even find the time to follow up on the patches myself. For now I need to go back and debug yet another issue with SDVO on GM45 :( Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: implement ibx_hpd_irq_setup
Sorry for replying so late, I wasn't able to task switch my brain towards this when it was discussed: Daniel Vetter writes: > diff --git a/drivers/gpu/drm/i915/i915_irq.c > b/drivers/gpu/drm/i915/i915_irq.c > index 43436e0..1279a44 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2084,7 +2084,7 @@ static void ibx_enable_hotplug(struct drm_device *dev) > I915_WRITE(PCH_PORT_HOTPLUG, hotplug); > } > > -static void ibx_irq_postinstall(struct drm_device *dev) > +static void ibx_hpd_irq_setup(struct drm_device *dev) > { > drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; > struct drm_mode_config *mode_config = &dev->mode_config; > @@ -2095,12 +2095,10 @@ static void ibx_irq_postinstall(struct drm_device > *dev) > mask &= ~SDE_HOTPLUG_MASK; ^^^ I'm missing those lines in the committed version of the patch. > list_for_each_entry(intel_encoder, &mode_config->encoder_list, > base.head) > mask |= hpd_ibx[intel_encoder->hpd_pin]; > -mask |= SDE_GMBUS | SDE_AUX_MASK; > } else { > mask &= ~SDE_HOTPLUG_MASK_CPT; ^^^ > list_for_each_entry(intel_encoder, &mode_config->encoder_list, > base.head) > mask |= hpd_cpt[intel_encoder->hpd_pin]; > -mask |= SDE_GMBUS_CPT | SDE_AUX_MASK_CPT; > } > I915_WRITE(SDEIIR, I915_READ(SDEIIR)); These are not really relevant in the present code, however they are important once I've got the hotplug stuff refitted as one needs to be able to turn off individual interrupts. I'm going to prepare a commit for this and will send it with the hpd irq storm patches. Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: implement ibx_hpd_irq_setup
Hi Daniel, Daniel Vetter writes: > On Fri, Mar 29, 2013 at 5:35 PM, Egbert Eich wrote: > > Yeah, makes sense now that I think about it - I've simply didn't look > ahead in your patch series while writing this little fixup ;-) Can you > just re-add this when resending your patches again please? > Sure, I have prepared all the patches. I just wanted to give them a try before sending them. Unfortunately I did not get around to do so over the Easter holidays. Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix SDVO connector and encoder get_hw_state functions
From: Egbert Eich The connector associated with the encoder is considered active when the output associtated with this connector is active on the encoder. The encoder itself is considered active when either there is an active output on it or the respective SDVO channel is active. Having active outputs when the SDVO channel is inactive seems to be inconsistent: such states can be found when intel_modeset_setup_hw_state() collects the hardware state set by the BIOS. This inconsistency will be fixed in intel_sanitize_crtc() (when intel_crtc_update_dpms() is called), this however only happens when the encoder is associated with a crtc. This patch also reverts: commit bd6946e87a98fea11907b2a47368e13044458a35 Author: Daniel Vetter Date: Tue Apr 2 21:30:34 2013 +0200 drm/i915: Fix sdvo connector get_hw_state function Signed-off-by: Egbert Eich Suggested-By: Daniel Vetter Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=63031 --- drivers/gpu/drm/i915/intel_sdvo.c |9 +++-- 1 files changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 298dc85..f6a9f4a 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1231,12 +1231,8 @@ static bool intel_sdvo_connector_get_hw_state(struct intel_connector *connector) struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(&connector->base); struct intel_sdvo *intel_sdvo = intel_attached_sdvo(&connector->base); - struct drm_i915_private *dev_priv = intel_sdvo->base.base.dev->dev_private; u16 active_outputs; - if (!(I915_READ(intel_sdvo->sdvo_reg) & SDVO_ENABLE)) - return false; - intel_sdvo_get_active_outputs(intel_sdvo, &active_outputs); if (active_outputs & intel_sdvo_connector->output_flag) @@ -1251,11 +1247,13 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder, struct drm_device *dev = encoder->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base); + u16 active_outputs; u32 tmp; tmp = I915_READ(intel_sdvo->sdvo_reg); + intel_sdvo_get_active_outputs(intel_sdvo, &active_outputs); - if (!(tmp & SDVO_ENABLE)) + if (!(tmp & SDVO_ENABLE) && (active_outputs == 0)) return false; if (HAS_PCH_CPT(dev)) @@ -2746,7 +2744,6 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob) struct intel_sdvo *intel_sdvo; u32 hotplug_mask; int i; - intel_sdvo = kzalloc(sizeof(struct intel_sdvo), GFP_KERNEL); if (!intel_sdvo) return false; -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 0/7] Add HPD interrupt storm detection.
This set of patches contains the remaining bits to add IRQ strom detection and disabling. Daniel has asked me to send this batch as soon as possible even though this latest batch has not yet been retested, yet. Egbert Eich (7): drm/i915: Add HPD IRQ storm detection (v4) drm/i915: (re)init HPD interrupt storm statistics drm/i915: Mask out the HPD irq bits before setting them individually. drm/i915: Disable HPD interrupt on pin when irq storm is detected (v2) drm/i915: Add Reenable Timer to turn Hotplug Detection back on (v3) drm/i915: Add bit field to record which pins have received HPD events (v2) drm/i915: Only reprobe display on encoder which has received an HPD event drivers/gpu/drm/i915/i915_drv.h | 12 ++ drivers/gpu/drm/i915/i915_irq.c | 224 ++ drivers/gpu/drm/i915/intel_crt.c | 6 +- drivers/gpu/drm/i915/intel_dp.c | 1 - drivers/gpu/drm/i915/intel_drv.h | 4 + drivers/gpu/drm/i915/intel_hdmi.c | 1 - drivers/gpu/drm/i915/intel_sdvo.c | 5 +- drivers/gpu/drm/i915/intel_tv.c | 2 +- 8 files changed, 221 insertions(+), 34 deletions(-) -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 1/7] drm/i915: Add HPD IRQ storm detection (v4)
From: Egbert Eich Add a hotplug IRQ storm detection (triggered when a hotplug interrupt fires more than 5 times / sec). Rationale: Despite of the many attempts to fix the problem with noisy hotplug interrupt lines we are still seeing systems which have issues: Once cause of noise seems to be bad routing of the hotplug line on the board: cross talk from other signals seems to cause erronous hotplug interrupts. This has been documented as an erratum for the the i945GM chipset and thus hotplug support was disabled for this chipset model but others seem to have this problem, too. We have seen this issue on a G35 motherboard for example: Even different motherboards of the same model seem to behave differently: while some only see only around 10-100 interrupts/s others seem to see 5k or more. We've also observed a dependency on the selected video mode. Also on certain laptops interrupt noise seems to occur duing battery charging when the battery is at a certain charge levels. Thus we add a simple algorithm here that detects an 'interrupt storm' condition. v2: Fixed comment. v3: Reordered drm_i915_private: moved hpd state tracking to hotplug work stuff. v4: Followed by Jesse Barnes to use a time_..() macro. Signed-off-by: Egbert Eich Acked-by: Chris Wilson Reviewed-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_drv.h | 9 ++ drivers/gpu/drm/i915/i915_irq.c | 66 + 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 44fca0b..83fc1a6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -929,6 +929,15 @@ typedef struct drm_i915_private { struct work_struct hotplug_work; bool enable_hotplug_processing; + struct { + unsigned long hpd_last_jiffies; + int hpd_cnt; + enum { + HPD_ENABLED = 0, + HPD_DISABLED = 1, + HPD_MARK_DISABLED = 2 + } hpd_mark; + } hpd_stats[HPD_NUM_PINS]; int num_pch_pll; int num_plane; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 4c5bdd0..32b5527 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -582,6 +582,37 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, queue_work(dev_priv->wq, &dev_priv->rps.work); } +#define HPD_STORM_DETECT_PERIOD 1000 + +static inline void hotplug_irq_storm_detect(struct drm_device *dev, + u32 hotplug_trigger, + const u32 *hpd) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + unsigned long irqflags; + int i; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + for (i = 1; i < HPD_NUM_PINS; i++) { + if ((hpd[i] & hotplug_trigger) && + dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) { + if (!time_in_range(jiffies, dev_priv->hpd_stats[i].hpd_last_jiffies, + dev_priv->hpd_stats[i].hpd_last_jiffies + + msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) { + dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies; + dev_priv->hpd_stats[i].hpd_cnt = 0; + } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { + dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; + DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); + } else + dev_priv->hpd_stats[i].hpd_cnt++; + } + } + + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); +} + static void gmbus_irq_handler(struct drm_device *dev) { struct drm_i915_private *dev_priv = (drm_i915_private_t *) dev->dev_private; @@ -650,13 +681,15 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) /* Consume port. Then clear IIR or we'll miss events */ if (iir & I915_DISPLAY_PORT_INTERRUPT) { u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT); + u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915; DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", hotplug_status); - if (hotplug_status & HOTPLUG_INT_STATUS_I915) + if (hotplug_trigger) { + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915);
[Intel-gfx] [PATCH v3 2/7] drm/i915: (re)init HPD interrupt storm statistics
From: Egbert Eich When an encoder is shared on several connectors there is only one hotplug line, thus this line needs to be shared among these connectors. If HPD detect only works reliably on a subset of those connectors, we want to poll the others. Thus we need to make sure that storm detection doesn't mess up the settings for those connectors. Therefore we store the settings in the intel_connector struct and restore them from there. If nothing is set but the encoder has a hpd_pin set we assume this connector is hotplug capable. On init/reset we make sure the polled state of the connectors is (re)set to the default value, the HPD interrupts are marked enabled. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 13 + drivers/gpu/drm/i915/intel_crt.c | 6 ++ drivers/gpu/drm/i915/intel_dp.c | 1 - drivers/gpu/drm/i915/intel_drv.h | 4 drivers/gpu/drm/i915/intel_hdmi.c | 1 - drivers/gpu/drm/i915/intel_sdvo.c | 5 ++--- drivers/gpu/drm/i915/intel_tv.c | 2 +- 7 files changed, 22 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 32b5527..5408a3a 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3045,7 +3045,20 @@ void intel_irq_init(struct drm_device *dev) void intel_hpd_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_mode_config *mode_config = &dev->mode_config; + struct drm_connector *connector; + int i; + for (i = 1; i < HPD_NUM_PINS; i++) { + dev_priv->hpd_stats[i].hpd_cnt = 0; + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED; + } + list_for_each_entry(connector, &mode_config->connector_list, head) { + struct intel_connector *intel_connector = to_intel_connector(connector); + connector->polled = intel_connector->polled; + if (!connector->polled && I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE) + connector->polled = DRM_CONNECTOR_POLL_HPD; + } if (dev_priv->display.hpd_irq_setup) dev_priv->display.hpd_irq_setup(dev); } diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 1ae2d7f..c063b9f 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -793,10 +793,8 @@ void intel_crt_init(struct drm_device *dev) drm_sysfs_connector_add(connector); - if (I915_HAS_HOTPLUG(dev)) - connector->polled = DRM_CONNECTOR_POLL_HPD; - else - connector->polled = DRM_CONNECTOR_POLL_CONNECT; + if (!I915_HAS_HOTPLUG(dev)) + intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT; /* * Configure the automatic hotplug detection stuff diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 482b5e5..1e9b19a 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2786,7 +2786,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, drm_connector_init(dev, connector, &intel_dp_connector_funcs, type); drm_connector_helper_add(connector, &intel_dp_connector_helper_funcs); - connector->polled = DRM_CONNECTOR_POLL_HPD; connector->interlace_allowed = true; connector->doublescan_allowed = 0; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d7bd031..a05fde7 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -171,6 +171,10 @@ struct intel_connector { /* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */ struct edid *edid; + + /* since POLL and HPD connectors may use the same HPD line keep the native + state of connector->polled in case hotplug storm detection changes it */ + u8 polled; }; struct intel_crtc_config { diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index ee4a8da..8912201 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -998,7 +998,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, DRM_MODE_CONNECTOR_HDMIA); drm_connector_helper_add(connector, &intel_hdmi_connector_helper_funcs); - connector->polled = DRM_CONNECTOR_POLL_HPD; connector->interlace_allowed = 1; connector->doublescan_allowed = 0; diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 298dc85..64b8b40 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -2276,7 +2276,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_
[Intel-gfx] [PATCH v3 3/7] drm/i915: Mask out the HPD irq bits before setting them individually.
From: Egbert Eich To disable previously enabled HPD IRQs we need to reset them and set the enabled ones individually. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 5408a3a..a3f1ac4 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2117,9 +2117,11 @@ static void ibx_hpd_irq_setup(struct drm_device *dev) u32 hotplug; if (HAS_PCH_IBX(dev)) { + mask &= ~SDE_HOTPLUG_MASK; list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) mask |= hpd_ibx[intel_encoder->hpd_pin]; } else { + mask &= ~SDE_HOTPLUG_MASK_CPT; list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) mask |= hpd_cpt[intel_encoder->hpd_pin]; } -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 6/7] drm/i915: Add bit field to record which pins have received HPD events (v2)
From: Egbert Eich This way it is possible to limit 're'-detect() of displays to connectors which have received an HPD event. v2: Reordered drm_i915_private: Move hpd_event_bits to hpd state tracking. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 10 ++ 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a3ed2e3..907e290 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -938,6 +938,7 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; + u32 hpd_event_bits; #define I915_REENABLE_HOTPLUG_DELAY (2*60*1000) struct timer_list hotplug_reenable_timer; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 1a00533..92041b9 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -348,6 +348,7 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; unsigned long irqflags; bool connector_disabled = false; + u32 hpd_event_bits; /* HPD irq before everything is fully set up. */ if (!dev_priv->enable_hotplug_processing) @@ -357,6 +358,9 @@ static void i915_hotplug_work_func(struct work_struct *work) DRM_DEBUG_KMS("running encoder hotplug functions\n"); spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + hpd_event_bits = dev_priv->hpd_event_bits; + dev_priv->hpd_event_bits = 0; list_for_each_entry(connector, &mode_config->connector_list, head) { intel_connector = to_intel_connector(connector); intel_encoder = intel_connector->encoder; @@ -370,6 +374,10 @@ static void i915_hotplug_work_func(struct work_struct *work) | DRM_CONNECTOR_POLL_DISCONNECT; connector_disabled = true; } + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n", + drm_get_connector_name(connector), intel_encoder->hpd_pin); + } } if (connector_disabled) { drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */ @@ -626,6 +634,7 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev, for (i = 1; i < HPD_NUM_PINS; i++) { if ((hpd[i] & hotplug_trigger) && dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) { + dev_priv->hpd_event_bits |= (1 << i); if (!time_in_range(jiffies, dev_priv->hpd_stats[i].hpd_last_jiffies, dev_priv->hpd_stats[i].hpd_last_jiffies + msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) { @@ -633,6 +642,7 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev, dev_priv->hpd_stats[i].hpd_cnt = 0; } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; + dev_priv->hpd_event_bits &= ~(1 << i); DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); ret = true; } else -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 7/7] drm/i915: Only reprobe display on encoder which has received an HPD event
From: Egbert Eich Instead of calling into the DRM helper layer to poll all connectors for changes in connected displays probe only those connectors which have received a hotplug event. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 37 +++-- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 92041b9..7788536 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -334,6 +334,24 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, crtc); } +/** + * drm_helper_hpd_irq_single_connector_event() - call this function with mode_config.mutex lock held + */ + +static int intel_hpd_irq_single_connector_event(struct drm_device *dev, struct drm_connector *connector) +{ + enum drm_connector_status old_status; + + old_status = connector->status; + + connector->status = connector->funcs->detect(connector, false); + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n", + connector->base.id, + drm_get_connector_name(connector), + old_status, connector->status); + return (old_status != connector->status); +} + /* * Handle hotplug events outside the interrupt handler proper. */ @@ -348,6 +366,7 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; unsigned long irqflags; bool connector_disabled = false; + bool changed = false; u32 hpd_event_bits; /* HPD irq before everything is fully set up. */ @@ -387,14 +406,20 @@ static void i915_hotplug_work_func(struct work_struct *work) spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); - list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) - if (intel_encoder->hot_plug) - intel_encoder->hot_plug(intel_encoder); - + list_for_each_entry(connector, &mode_config->connector_list, head) { + intel_connector = to_intel_connector(connector); + intel_encoder = intel_connector->encoder; + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + if (intel_encoder->hot_plug) + intel_encoder->hot_plug(intel_encoder); + if (intel_hpd_irq_single_connector_event(dev, connector)) + changed = true; + } + } mutex_unlock(&mode_config->mutex); - /* Just fire off a uevent and let userspace tell us what to do */ - drm_helper_hpd_irq_event(dev); + if (changed) + drm_kms_helper_hotplug_event(dev); } static void ironlake_handle_rps_change(struct drm_device *dev) -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 5/7] drm/i915: Add Reenable Timer to turn Hotplug Detection back on (v3)
From: Egbert Eich We disable hoptplug detection when we encounter a hotplug event storm. Still hotplug detection is required on some outputs (like Display Port). The interrupt storm may be only temporary (on certain Dell Laptops for instance it happens at certain charging states of the system). Thus we enable it after a certain grace period (2 minutes). Should the interrupt storm persist it will be detected immediately and it will be disabled again. v2: Reordered drm_i915_private: moved hotplug_reenable_timer to hpd state tracker. v3: Clarified loop start value, Removed superfluous test for Ivybridge and Haswell, Restructured loop to avoid deep nesting (all suggested by Ville Syrjälä) Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_irq.c | 49 - 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 83fc1a6..a3ed2e3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -938,6 +938,8 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; +#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000) + struct timer_list hotplug_reenable_timer; int num_pch_pll; int num_plane; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d0e6f6d..1a00533 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -371,8 +371,11 @@ static void i915_hotplug_work_func(struct work_struct *work) connector_disabled = true; } } - if (connector_disabled) + if (connector_disabled) { drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */ + mod_timer(&dev_priv->hotplug_reenable_timer, + jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY)); + } spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); @@ -2348,6 +2351,8 @@ static void valleyview_irq_uninstall(struct drm_device *dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + for_each_pipe(pipe) I915_WRITE(PIPESTAT(pipe), 0x); @@ -2369,6 +2374,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + I915_WRITE(HWSTAM, 0x); I915_WRITE(DEIMR, 0x); @@ -2745,6 +2752,8 @@ static void i915_irq_uninstall(struct drm_device * dev) drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; int pipe; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + if (I915_HAS_HOTPLUG(dev)) { I915_WRITE(PORT_HOTPLUG_EN, 0); I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); @@ -2989,6 +2998,8 @@ static void i965_irq_uninstall(struct drm_device * dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + I915_WRITE(PORT_HOTPLUG_EN, 0); I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); @@ -3004,6 +3015,40 @@ static void i965_irq_uninstall(struct drm_device * dev) I915_WRITE(IIR, I915_READ(IIR)); } +static void i915_reenable_hotplug_timer_func(unsigned long data) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *)data; + struct drm_device *dev = dev_priv->dev; + struct drm_mode_config *mode_config = &dev->mode_config; + unsigned long irqflags; + int i; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + for (i = (HPD_NONE + 1); i < HPD_NUM_PINS; i++) { + struct drm_connector *connector; + + if (dev_priv->hpd_stats[i].hpd_mark != HPD_MARK_DISABLED) + continue; + + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED; + + list_for_each_entry(connector, &mode_config->connector_list, head) { + struct intel_connector *intel_connector = to_intel_connector(connector); + + if (intel_connector->encoder->hpd_pin == i) { + if (connector->polled != intel_connector->polled) + DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n", + drm_get_connector_name(connector)); + connector->polled = intel_connector->polled; + if (!connector->polled) + conn
[Intel-gfx] [PATCH v3 4/7] drm/i915: Disable HPD interrupt on pin when irq storm is detected (v2)
From: Egbert Eich This patch disables hotplug interrupts if an 'interrupt storm' has been detected. Noise on the interrupt line renders the hotplug interrupt useless: each hotplug event causes the devices to be rescanned which will will only increase the system load. Thus disable the hotplug interrupts and fall back to periodic device polling. v2: Fixed cleanup typo. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 71 +++-- 1 file changed, 54 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index a3f1ac4..d0e6f6d 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -88,7 +88,8 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */ [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS }; - +static void ibx_hpd_irq_setup(struct drm_device *dev); +static void i915_hpd_irq_setup(struct drm_device *dev); /* For display hotplug interrupt */ static void @@ -342,7 +343,11 @@ static void i915_hotplug_work_func(struct work_struct *work) hotplug_work); struct drm_device *dev = dev_priv->dev; struct drm_mode_config *mode_config = &dev->mode_config; - struct intel_encoder *encoder; + struct intel_connector *intel_connector; + struct intel_encoder *intel_encoder; + struct drm_connector *connector; + unsigned long irqflags; + bool connector_disabled = false; /* HPD irq before everything is fully set up. */ if (!dev_priv->enable_hotplug_processing) @@ -351,9 +356,29 @@ static void i915_hotplug_work_func(struct work_struct *work) mutex_lock(&mode_config->mutex); DRM_DEBUG_KMS("running encoder hotplug functions\n"); - list_for_each_entry(encoder, &mode_config->encoder_list, base.head) - if (encoder->hot_plug) - encoder->hot_plug(encoder); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + list_for_each_entry(connector, &mode_config->connector_list, head) { + intel_connector = to_intel_connector(connector); + intel_encoder = intel_connector->encoder; + if (intel_encoder->hpd_pin > HPD_NONE && + dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_MARK_DISABLED && + connector->polled == DRM_CONNECTOR_POLL_HPD) { + pr_warn("HPD interrupt storm detected on connector %s disabling\n", + drm_get_connector_name(connector)); + dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark = HPD_DISABLED; + connector->polled = DRM_CONNECTOR_POLL_CONNECT + | DRM_CONNECTOR_POLL_DISCONNECT; + connector_disabled = true; + } + } + if (connector_disabled) + drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */ + + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + + list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) + if (intel_encoder->hot_plug) + intel_encoder->hot_plug(intel_encoder); mutex_unlock(&mode_config->mutex); @@ -584,13 +609,14 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, #define HPD_STORM_DETECT_PERIOD 1000 -static inline void hotplug_irq_storm_detect(struct drm_device *dev, +static inline bool hotplug_irq_storm_detect(struct drm_device *dev, u32 hotplug_trigger, const u32 *hpd) { drm_i915_private_t *dev_priv = dev->dev_private; unsigned long irqflags; int i; + bool ret = false; spin_lock_irqsave(&dev_priv->irq_lock, irqflags); @@ -605,12 +631,15 @@ static inline void hotplug_irq_storm_detect(struct drm_device *dev, } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); + ret = true; } else dev_priv->hpd_stats[i].hpd_cnt++; } } spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + + return ret; } static void gmbus_irq_handler(struct drm_device *dev) @@ -686,7 +715,8 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) DRM_DEBUG_DRIVER("hotplug event r
Re: [Intel-gfx] [PATCH v3 5/7] drm/i915: Add Reenable Timer to turn Hotplug Detection back on (v3)
On Thu, Apr 11, 2013 at 01:44:51PM +0300, Jani Nikula wrote: > > + > > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > > + for (i = (HPD_NONE + 1); i < HPD_NUM_PINS; i++) { > > + struct drm_connector *connector; > > + > > + if (dev_priv->hpd_stats[i].hpd_mark != HPD_MARK_DISABLED) > > I think this is wrong, skipping HPD_DISABLED. Right, this was indeed wrong. > > > + continue; > > + > > + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED; > > + > > + list_for_each_entry(connector, &mode_config->connector_list, > > head) { > > + struct intel_connector *intel_connector = > > to_intel_connector(connector); > > + > > + if (intel_connector->encoder->hpd_pin == i) { > > + if (connector->polled != > > intel_connector->polled) > > + DRM_DEBUG_DRIVER("Reenabling HPD on > > connector %s\n", > > + > > drm_get_connector_name(connector)); > > + connector->polled = intel_connector->polled; > > + if (!connector->polled) > > + connector->polled = > > DRM_CONNECTOR_POLL_HPD; > > + } > > + } > > + dev_priv->display.hpd_irq_setup(dev); > > You don't need to call this at each iteration, do you? Right, you are right here as well. > > > + } > > In fact, couldn't you just call intel_hpd_init(), or modify it to do > what you want? Keep it simple. Well, intel_hpd_init() is initializing every to dev_priv->hpd_stats[i].hpd_mark to HPD_ENABLED. Can we rule out that the timer runs between the interrupt handler and the worker - as in this state this variable might me set to HPD_MARK_DISABLED? In fact it would probably not even hurt if we changed the HPD_MARK_DISABLED to HPD_ENABLED as in a storm condition this condition will soon reappear but I'd rather avoid it. Of course we could pass an argument to the function to distinguish both conditions. This is a simplification which can still be introduced - when I'm in fact able to do some testing. Thanks a lot for the review! Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915: Disable HPD interrupt on pin when irq storm is detected (v3)
This patch disables hotplug interrupts if an 'interrupt storm' has been detected. Noise on the interrupt line renders the hotplug interrupt useless: each hotplug event causes the devices to be rescanned which will will only increase the system load. Thus disable the hotplug interrupts and fall back to periodic device polling. v2: Fixed cleanup typo. v3: Fixed format issues, clarified a variable name, changed pr_warn() to DRM_INFO() as suggested by Jani Nikula . Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 75 +++-- 1 file changed, 58 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index a3f1ac4..834c717 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -88,7 +88,8 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */ [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS }; - +static void ibx_hpd_irq_setup(struct drm_device *dev); +static void i915_hpd_irq_setup(struct drm_device *dev); /* For display hotplug interrupt */ static void @@ -342,7 +343,11 @@ static void i915_hotplug_work_func(struct work_struct *work) hotplug_work); struct drm_device *dev = dev_priv->dev; struct drm_mode_config *mode_config = &dev->mode_config; - struct intel_encoder *encoder; + struct intel_connector *intel_connector; + struct intel_encoder *intel_encoder; + struct drm_connector *connector; + unsigned long irqflags; + bool hpd_disabled = false; /* HPD irq before everything is fully set up. */ if (!dev_priv->enable_hotplug_processing) @@ -351,9 +356,33 @@ static void i915_hotplug_work_func(struct work_struct *work) mutex_lock(&mode_config->mutex); DRM_DEBUG_KMS("running encoder hotplug functions\n"); - list_for_each_entry(encoder, &mode_config->encoder_list, base.head) - if (encoder->hot_plug) - encoder->hot_plug(encoder); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + list_for_each_entry(connector, &mode_config->connector_list, head) { + intel_connector = to_intel_connector(connector); + intel_encoder = intel_connector->encoder; + if (intel_encoder->hpd_pin > HPD_NONE && + dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_MARK_DISABLED && + connector->polled == DRM_CONNECTOR_POLL_HPD) { + DRM_INFO("HPD interrupt storm detected on connector %s: " +"switching from hotplug detection to polling\n", + drm_get_connector_name(connector)); + dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark = HPD_DISABLED; + connector->polled = DRM_CONNECTOR_POLL_CONNECT + | DRM_CONNECTOR_POLL_DISCONNECT; + hpd_disabled = true; + } + } +/* if there were no outputs to poll, poll was disabled, + * therefore make sure it's enabled when disabling HPD on + * some connectors */ + if (hpd_disabled) { + drm_kms_helper_poll_enable(dev); + } + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + + list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) + if (intel_encoder->hot_plug) + intel_encoder->hot_plug(intel_encoder); mutex_unlock(&mode_config->mutex); @@ -584,13 +613,14 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, #define HPD_STORM_DETECT_PERIOD 1000 -static inline void hotplug_irq_storm_detect(struct drm_device *dev, +static inline bool hotplug_irq_storm_detect(struct drm_device *dev, u32 hotplug_trigger, const u32 *hpd) { drm_i915_private_t *dev_priv = dev->dev_private; unsigned long irqflags; int i; + bool ret = false; spin_lock_irqsave(&dev_priv->irq_lock, irqflags); @@ -605,12 +635,15 @@ static inline void hotplug_irq_storm_detect(struct drm_device *dev, } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); + ret = true; } else dev_priv->hpd_stats[i].hpd_cnt++; } } spin
[Intel-gfx] [PATCH v4] drm/i915: Add Reenable Timer to turn Hotplug Detection back on (v4)
We disable hoptplug detection when we encounter a hotplug event storm. Still hotplug detection is required on some outputs (like Display Port). The interrupt storm may be only temporary (on certain Dell Laptops for instance it happens at certain charging states of the system). Thus we enable it after a certain grace period (2 minutes). Should the interrupt storm persist it will be detected immediately and it will be disabled again. v2: Reordered drm_i915_private: moved hotplug_reenable_timer to hpd state tracker. v3: Clarified loop start value, Removed superfluous test for Ivybridge and Haswell, Restructured loop to avoid deep nesting (all suggested by Ville Syrjälä) v4: Fixed two bugs pointed out by Jani Nikula. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 50 + 2 files changed, 51 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 83fc1a6..195b9fe 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -938,6 +938,7 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; + struct timer_list hotplug_reenable_timer; int num_pch_pll; int num_plane; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 834c717..f60c643 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -337,6 +337,8 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, /* * Handle hotplug events outside the interrupt handler proper. */ +#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000) + static void i915_hotplug_work_func(struct work_struct *work) { drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t, @@ -377,7 +379,10 @@ static void i915_hotplug_work_func(struct work_struct *work) * some connectors */ if (hpd_disabled) { drm_kms_helper_poll_enable(dev); + mod_timer(&dev_priv->hotplug_reenable_timer, + jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY)); } + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) @@ -2352,6 +2357,8 @@ static void valleyview_irq_uninstall(struct drm_device *dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + for_each_pipe(pipe) I915_WRITE(PIPESTAT(pipe), 0x); @@ -2373,6 +2380,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + I915_WRITE(HWSTAM, 0x); I915_WRITE(DEIMR, 0x); @@ -2749,6 +2758,8 @@ static void i915_irq_uninstall(struct drm_device * dev) drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; int pipe; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + if (I915_HAS_HOTPLUG(dev)) { I915_WRITE(PORT_HOTPLUG_EN, 0); I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); @@ -2993,6 +3004,8 @@ static void i965_irq_uninstall(struct drm_device * dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + I915_WRITE(PORT_HOTPLUG_EN, 0); I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); @@ -3008,6 +3021,41 @@ static void i965_irq_uninstall(struct drm_device * dev) I915_WRITE(IIR, I915_READ(IIR)); } +static void i915_reenable_hotplug_timer_func(unsigned long data) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *)data; + struct drm_device *dev = dev_priv->dev; + struct drm_mode_config *mode_config = &dev->mode_config; + unsigned long irqflags; + int i; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + for (i = (HPD_NONE + 1); i < HPD_NUM_PINS; i++) { + struct drm_connector *connector; + + if (dev_priv->hpd_stats[i].hpd_mark != HPD_DISABLED) + continue; + + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED; + + list_for_each_entry(connector, &mode_config->connector_list, head) { + struct intel_connector *intel_connector = to_intel_connector(connector); + + if (intel_connector->encoder->hpd_pin == i) { + if (connector->polled != intel_connector->polled) + DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n", +
Re: [Intel-gfx] [PATCH v3 6/7] drm/i915: Add bit field to record which pins have received HPD events (v2)
On Thu, Apr 11, 2013 at 04:21:54PM +0300, Jani Nikula wrote: > > } > > + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { > > + DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug > > event.\n", > > + drm_get_connector_name(connector), > > intel_encoder->hpd_pin); > > + } > > I fear this may end up being a bit excessive debug printing. > In a storm condition this is definitely true. I still would like to keep this code in for debugging as I have some more ideas I would like to implement and test - however I will wrap this code with #if 0 .. #endif so it can be enabled easily. Eventually I will remove it entirely. Would this be ok with you? Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915: Add bit field to record which pins have received HPD events (v3)
This way it is possible to limit 're'-detect() of displays to connectors which have received an HPD event. v2: Reordered drm_i915_private: Move hpd_event_bits to hpd state tracking. v3: Fixed merge conflicts with previous patches. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 10 ++ 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 195b9fe..c0d9fb9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -938,6 +938,7 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; + u32 hpd_event_bits; struct timer_list hotplug_reenable_timer; int num_pch_pll; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index f60c643..f7219d8 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -350,6 +350,7 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; unsigned long irqflags; bool hpd_disabled = false; + u32 hpd_event_bits; /* HPD irq before everything is fully set up. */ if (!dev_priv->enable_hotplug_processing) @@ -359,6 +360,9 @@ static void i915_hotplug_work_func(struct work_struct *work) DRM_DEBUG_KMS("running encoder hotplug functions\n"); spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + hpd_event_bits = dev_priv->hpd_event_bits; + dev_priv->hpd_event_bits = 0; list_for_each_entry(connector, &mode_config->connector_list, head) { intel_connector = to_intel_connector(connector); intel_encoder = intel_connector->encoder; @@ -373,6 +377,10 @@ static void i915_hotplug_work_func(struct work_struct *work) | DRM_CONNECTOR_POLL_DISCONNECT; hpd_disabled = true; } + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n", + drm_get_connector_name(connector), intel_encoder->hpd_pin); + } } /* if there were no outputs to poll, poll was disabled, * therefore make sure it's enabled when disabling HPD on @@ -632,6 +640,7 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev, for (i = 1; i < HPD_NUM_PINS; i++) { if ((hpd[i] & hotplug_trigger) && dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) { + dev_priv->hpd_event_bits |= (1 << i); if (!time_in_range(jiffies, dev_priv->hpd_stats[i].hpd_last_jiffies, dev_priv->hpd_stats[i].hpd_last_jiffies + msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) { @@ -639,6 +648,7 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev, dev_priv->hpd_stats[i].hpd_cnt = 0; } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; + dev_priv->hpd_event_bits &= ~(1 << i); DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); ret = true; } else -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Only reprobe display on encoder which has received an HPD event (v2)
Instead of calling into the DRM helper layer to poll all connectors for changes in connected displays probe only those connectors which have received a hotplug event. Signed-off-by: Egbert Eich Reviewed-by: Jani Nikula v2: Resolved conflicts with changes in previous commits. Renamed function and and added a WARN_ON() to warn of intel_hpd_irq_event() from being called without mode_config.mutex held - suggested by Jani Nikula. --- drivers/gpu/drm/i915/i915_irq.c | 34 -- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 6e643d7..54eca33 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -334,6 +334,21 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, crtc); } +static int intel_hpd_irq_event(struct drm_device *dev, struct drm_connector *connector) +{ + enum drm_connector_status old_status; + + WARN_ON(!mutex_is_locked(&dev->mode_config.mutex)); + old_status = connector->status; + + connector->status = connector->funcs->detect(connector, false); + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n", + connector->base.id, + drm_get_connector_name(connector), + old_status, connector->status); + return (old_status != connector->status); +} + /* * Handle hotplug events outside the interrupt handler proper. */ @@ -350,6 +365,7 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; unsigned long irqflags; bool hpd_disabled = false; + bool changed = false; u32 hpd_event_bits; /* HPD irq before everything is fully set up. */ @@ -395,14 +411,20 @@ static void i915_hotplug_work_func(struct work_struct *work) spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); - list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) - if (intel_encoder->hot_plug) - intel_encoder->hot_plug(intel_encoder); - + list_for_each_entry(connector, &mode_config->connector_list, head) { + intel_connector = to_intel_connector(connector); + intel_encoder = intel_connector->encoder; + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + if (intel_encoder->hot_plug) + intel_encoder->hot_plug(intel_encoder); + if (intel_hpd_irq_event(dev, connector)) + changed = true; + } + } mutex_unlock(&mode_config->mutex); - /* Just fire off a uevent and let userspace tell us what to do */ - drm_helper_hpd_irq_event(dev); + if (changed) + drm_kms_helper_hotplug_event(dev); } static void ironlake_handle_rps_change(struct drm_device *dev) -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 Update] drm/i915: Add bit field to record which pins have received HPD events (v3)
This allows to add code which limits 're'-detect() of displays to connectors that have received an HPD event. v2: Reordered drm_i915_private: Move hpd_event_bits to hpd state tracking. v3: Fixed merge conflicts with previous patches, removed some noisy debug logging as suggested by Jani Nikula. Signed-off-by: Egbert Eich --- (Sorry for the spam, I had accidenty went the wrong version of this patch) drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 12 2 files changed, 13 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 195b9fe..c0d9fb9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -938,6 +938,7 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; + u32 hpd_event_bits; struct timer_list hotplug_reenable_timer; int num_pch_pll; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index f60c643..6e643d7 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -350,6 +350,7 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; unsigned long irqflags; bool hpd_disabled = false; + u32 hpd_event_bits; /* HPD irq before everything is fully set up. */ if (!dev_priv->enable_hotplug_processing) @@ -359,6 +360,9 @@ static void i915_hotplug_work_func(struct work_struct *work) DRM_DEBUG_KMS("running encoder hotplug functions\n"); spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + hpd_event_bits = dev_priv->hpd_event_bits; + dev_priv->hpd_event_bits = 0; list_for_each_entry(connector, &mode_config->connector_list, head) { intel_connector = to_intel_connector(connector); intel_encoder = intel_connector->encoder; @@ -373,6 +377,12 @@ static void i915_hotplug_work_func(struct work_struct *work) | DRM_CONNECTOR_POLL_DISCONNECT; hpd_disabled = true; } +#if 0 + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n", + drm_get_connector_name(connector), intel_encoder->hpd_pin); + } +#endif } /* if there were no outputs to poll, poll was disabled, * therefore make sure it's enabled when disabling HPD on @@ -632,6 +642,7 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev, for (i = 1; i < HPD_NUM_PINS; i++) { if ((hpd[i] & hotplug_trigger) && dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) { + dev_priv->hpd_event_bits |= (1 << i); if (!time_in_range(jiffies, dev_priv->hpd_stats[i].hpd_last_jiffies, dev_priv->hpd_stats[i].hpd_last_jiffies + msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) { @@ -639,6 +650,7 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev, dev_priv->hpd_stats[i].hpd_cnt = 0; } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; + dev_priv->hpd_event_bits &= ~(1 << i); DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); ret = true; } else -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/7] drm/i915: Add HPD IRQ storm detection (v4)
Hi Jani, On Thu, Apr 11, 2013 at 12:32:29PM +0300, Jani Nikula wrote: > > Hi Egbert - > > Up front, I haven't been following this series or read any of the > previous review comments. Please bear with me, and feel free to direct > me to earlier comments if I'm in contradiction. Sorry for the late reply - last week I was quite busy with other stuff, this week i'm a bit preoccupied. > > On Tue, 09 Apr 2013, Egbert Eich wrote: > > From: Egbert Eich > > > > Add a hotplug IRQ storm detection (triggered when a hotplug interrupt > > fires more than 5 times / sec). > > Okay, this is theoretical, but a display port sink could do more than > that many hpd irq requests when connected. Which leads me to wonder in > general if the storm detection should be different for hot plug > vs. unplug and hpd irq events. Agreed. During my tests I did not see any issues with the statistics I've implemented: 5/sec was an ad-hoc choice and I wanted to start with something simple. I've tested it and it seemed to work, so I didn't bother to look into this more deeply, however if you feel 5 events / sec are too few to really do a good distinction, we could easily increase this number. There have been two situations where I have seen 'interrupt storms': 1. On G35: some boxes of the affected systems do not show this issue at all while others see a very high load but are still usable. In my recollection there were in the order of some 100 interrupts/sec on these machines. Then there were systems which would 'stall' in the worker during boot due to high load. At one point the NMI watchdog kicked in and stopped this mess. There the interrupts happened at an order of magnitude if 10k! 2. A laptop with a Sandybridge chipset where the system load went high at certain stages of charging levels - when the power supply was connected. I would assume the frequency there also was around some 100 / sec. Thus if we increase the threshold frequency to some 10 / sec we would still cover all those cases. Some other issue I've seen is 'bouncing' during manual plugging I've been contemplating how to address this. There are two things to look at: 1. multiple hotplug events due to not getting a perfect connection at first 2. EDID readout happening to early when the EDID lines are not yet fully and 'permanently' connected. It might well be, that a fix for these issues might actually also adress the issues you are pointing out. I have not seen them on Intel hardware - but this may be due to the fact that the hardware I saw it on was a separate gfx card which did not have the usual mounting bracket and thus the entire setup was a bit fragile and not really suitable for hot-plugging. However I believe that these things might happen everywere for people not quite used to plugging monitors too much. > > Have you observed difference between hot plug/unplug? There seems to be a difference between monitor connected/not connected: On DVI (G35) one doesn't distinguish between plug/unplug: when the hotplug line on the connector changes state an interrupt is sent. On this system storms only happened when a monitor was conneted - since the state of the HPD pin is signalled thru different frequencies on a line across SDVO (in my recollection it was 10 vs. 20 MHz) I believe that due to cross talk the higher(?) frequency could not always reliably be measured. I did not have access to the laptop system and the customer was not patient enough to help me to debug this further with me. Generally I think we would still adress the 'strom condition' if we raised the threshold to 20 or 30 /sec. What do you think? > > Has this been a problem on PCH split platforms, i.e. since ilk/gen5? I've also observed this on Sandybridge - which would be past GEN5, wouldn't it? I will address some of the other issues mentioned in a new patch. Thanks a lot for looking at it! Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/7] drm/i915: Add HPD IRQ storm detection (v4)
Hi Jani, I've rebased and regenerated all the patches now, as there were some other bikesheds i had not adressed. I've also included all Reviewed-By: This should make it easier to integrate the patches. Some comments below: On Thu, Apr 11, 2013 at 12:32:29PM +0300, Jani Nikula wrote: > > + struct { > > + unsigned long hpd_last_jiffies; > > + int hpd_cnt; > > + enum { > > + HPD_ENABLED = 0, > > + HPD_DISABLED = 1, > > + HPD_MARK_DISABLED = 2 > > + } hpd_mark; > > + } hpd_stats[HPD_NUM_PINS]; > > With all the hotplug stuff being added, I think it's getting time to > group all the hotplug stuff under a struct. I will postpone this until I address the issues that I have on my TODO. > > > int num_pch_pll; > > int num_plane; > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index 4c5bdd0..32b5527 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -582,6 +582,37 @@ static void gen6_queue_rps_work(struct > > drm_i915_private *dev_priv, > > queue_work(dev_priv->wq, &dev_priv->rps.work); > > } > > > > +#define HPD_STORM_DETECT_PERIOD 1000 > > + > > +static inline void hotplug_irq_storm_detect(struct drm_device *dev, > > + u32 hotplug_trigger, > > + const u32 *hpd) > > I think you should just add the bool return status right here instead of > postponing until the later patch that needs it. I thought about this and left it as it is: Returning a bool status now will raise other questions as the logic in this patch doesn't require it. I'd rather have a bigger patch later which will however clearly explains the reason for the change to the function. > > > +{ > > + drm_i915_private_t *dev_priv = dev->dev_private; > > + unsigned long irqflags; > > + int i; > > + > > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > > + > > + for (i = 1; i < HPD_NUM_PINS; i++) { > > + if ((hpd[i] & hotplug_trigger) && > > + dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) { > > Bikeshed: You might get a nicer layout below by negating that if and > adding continue. Fixed. > > > + if (!time_in_range(jiffies, > > dev_priv->hpd_stats[i].hpd_last_jiffies, > > + > > dev_priv->hpd_stats[i].hpd_last_jiffies > > + + > > msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) { > > + dev_priv->hpd_stats[i].hpd_last_jiffies = > > jiffies; > > + dev_priv->hpd_stats[i].hpd_cnt = 0; > > + } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { > > + dev_priv->hpd_stats[i].hpd_mark = > > HPD_MARK_DISABLED; > > + DRM_DEBUG_KMS("HPD interrupt storm detected on > > PIN %d\n", i); > > Extra space before "PIN". Fixed. > > > + } else > > + dev_priv->hpd_stats[i].hpd_cnt++; > > If one branch requires braces, then all do. Fixed. Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/7] drm/i915: Add HPD IRQ storm detection (v5)
Add a hotplug IRQ storm detection (triggered when a hotplug interrupt fires more than 5 times / sec). Rationale: Despite of the many attempts to fix the problem with noisy hotplug interrupt lines we are still seeing systems which have issues: Once cause of noise seems to be bad routing of the hotplug line on the board: cross talk from other signals seems to cause erronous hotplug interrupts. This has been documented as an erratum for the the i945GM chipset and thus hotplug support was disabled for this chipset model but others seem to have this problem, too. We have seen this issue on a G35 motherboard for example: Even different motherboards of the same model seem to behave differently: while some only see only around 10-100 interrupts/s others seem to see 5k or more. We've also observed a dependency on the selected video mode. Also on certain laptops interrupt noise seems to occur duing battery charging when the battery is at a certain charge levels. Thus we add a simple algorithm here that detects an 'interrupt storm' condition. v2: Fixed comment. v3: Reordered drm_i915_private: moved hpd state tracking to hotplug work stuff. v4: Followed by Jesse Barnes to use a time_..() macro. v5: Fixed coding style as suggested by Jani Nikula. Signed-off-by: Egbert Eich Acked-by: Chris Wilson Reviewed-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_drv.h | 9 ++ drivers/gpu/drm/i915/i915_irq.c | 69 ++--- 2 files changed, 66 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 44fca0b..83fc1a6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -929,6 +929,15 @@ typedef struct drm_i915_private { struct work_struct hotplug_work; bool enable_hotplug_processing; + struct { + unsigned long hpd_last_jiffies; + int hpd_cnt; + enum { + HPD_ENABLED = 0, + HPD_DISABLED = 1, + HPD_MARK_DISABLED = 2 + } hpd_mark; + } hpd_stats[HPD_NUM_PINS]; int num_pch_pll; int num_plane; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 4c5bdd0..71fc238 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -582,6 +582,40 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, queue_work(dev_priv->wq, &dev_priv->rps.work); } +#define HPD_STORM_DETECT_PERIOD 1000 +#define HPD_STORM_THRESHOLD 5 + +static inline void hotplug_irq_storm_detect(struct drm_device *dev, + u32 hotplug_trigger, + const u32 *hpd) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + unsigned long irqflags; + int i; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + for (i = 1; i < HPD_NUM_PINS; i++) { + if (!(hpd[i] & hotplug_trigger) || + dev_priv->hpd_stats[i].hpd_mark != HPD_ENABLED) + continue; + + if (!time_in_range(jiffies, dev_priv->hpd_stats[i].hpd_last_jiffies, + dev_priv->hpd_stats[i].hpd_last_jiffies + + msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) { + dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies; + dev_priv->hpd_stats[i].hpd_cnt = 0; + } else if (dev_priv->hpd_stats[i].hpd_cnt > HPD_STORM_THRESHOLD) { + dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; + DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); + } else { + dev_priv->hpd_stats[i].hpd_cnt++; + } + } + + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); +} + static void gmbus_irq_handler(struct drm_device *dev) { struct drm_i915_private *dev_priv = (drm_i915_private_t *) dev->dev_private; @@ -650,13 +684,15 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) /* Consume port. Then clear IIR or we'll miss events */ if (iir & I915_DISPLAY_PORT_INTERRUPT) { u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT); + u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915; DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", hotplug_status); - if (hotplug_status & HOTPLUG_INT_STATUS_I915) + if (hotplug_trigger) { + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915);
[Intel-gfx] [PATCH 2/7] drm/i915: (re)init HPD interrupt storm statistics
When an encoder is shared on several connectors there is only one hotplug line, thus this line needs to be shared among these connectors. If HPD detect only works reliably on a subset of those connectors, we want to poll the others. Thus we need to make sure that storm detection doesn't mess up the settings for those connectors. Therefore we store the settings in the intel_connector struct and restore them from there. If nothing is set but the encoder has a hpd_pin set we assume this connector is hotplug capable. On init/reset we make sure the polled state of the connectors is (re)set to the default value, the HPD interrupts are marked enabled. Signed-off-by: Egbert Eich Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/i915_irq.c | 14 ++ drivers/gpu/drm/i915/intel_crt.c | 6 ++ drivers/gpu/drm/i915/intel_dp.c | 1 - drivers/gpu/drm/i915/intel_drv.h | 4 drivers/gpu/drm/i915/intel_hdmi.c | 1 - drivers/gpu/drm/i915/intel_sdvo.c | 5 ++--- drivers/gpu/drm/i915/intel_tv.c | 2 +- 7 files changed, 23 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 71fc238..bc00532 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -596,6 +596,7 @@ static inline void hotplug_irq_storm_detect(struct drm_device *dev, spin_lock_irqsave(&dev_priv->irq_lock, irqflags); for (i = 1; i < HPD_NUM_PINS; i++) { + if (!(hpd[i] & hotplug_trigger) || dev_priv->hpd_stats[i].hpd_mark != HPD_ENABLED) continue; @@ -3048,7 +3049,20 @@ void intel_irq_init(struct drm_device *dev) void intel_hpd_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_mode_config *mode_config = &dev->mode_config; + struct drm_connector *connector; + int i; + for (i = 1; i < HPD_NUM_PINS; i++) { + dev_priv->hpd_stats[i].hpd_cnt = 0; + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED; + } + list_for_each_entry(connector, &mode_config->connector_list, head) { + struct intel_connector *intel_connector = to_intel_connector(connector); + connector->polled = intel_connector->polled; + if (!connector->polled && I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE) + connector->polled = DRM_CONNECTOR_POLL_HPD; + } if (dev_priv->display.hpd_irq_setup) dev_priv->display.hpd_irq_setup(dev); } diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 1ae2d7f..c063b9f 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -793,10 +793,8 @@ void intel_crt_init(struct drm_device *dev) drm_sysfs_connector_add(connector); - if (I915_HAS_HOTPLUG(dev)) - connector->polled = DRM_CONNECTOR_POLL_HPD; - else - connector->polled = DRM_CONNECTOR_POLL_CONNECT; + if (!I915_HAS_HOTPLUG(dev)) + intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT; /* * Configure the automatic hotplug detection stuff diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 482b5e5..1e9b19a 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2786,7 +2786,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, drm_connector_init(dev, connector, &intel_dp_connector_funcs, type); drm_connector_helper_add(connector, &intel_dp_connector_helper_funcs); - connector->polled = DRM_CONNECTOR_POLL_HPD; connector->interlace_allowed = true; connector->doublescan_allowed = 0; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d7bd031..a05fde7 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -171,6 +171,10 @@ struct intel_connector { /* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */ struct edid *edid; + + /* since POLL and HPD connectors may use the same HPD line keep the native + state of connector->polled in case hotplug storm detection changes it */ + u8 polled; }; struct intel_crtc_config { diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index ee4a8da..8912201 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -998,7 +998,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, DRM_MODE_CONNECTOR_HDMIA); drm_connector_helper_add(connector, &intel_hdmi_connector_helper_funcs); - connector->polled = DRM_CONNECTO
[Intel-gfx] [PATCH 3/7] drm/i915: Mask out the HPD irq bits before setting them individually.
To disable previously enabled HPD IRQs we need to reset them and set the enabled ones individually. Signed-off-by: Egbert Eich Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/i915_irq.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index bc00532..4cf33b3 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2121,9 +2121,11 @@ static void ibx_hpd_irq_setup(struct drm_device *dev) u32 hotplug; if (HAS_PCH_IBX(dev)) { + mask &= ~SDE_HOTPLUG_MASK; list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) mask |= hpd_ibx[intel_encoder->hpd_pin]; } else { + mask &= ~SDE_HOTPLUG_MASK_CPT; list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) mask |= hpd_cpt[intel_encoder->hpd_pin]; } -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/7] drm/i915: Disable HPD interrupt on pin when irq storm is detected (v3)
This patch disables hotplug interrupts if an 'interrupt storm' has been detected. Noise on the interrupt line renders the hotplug interrupt useless: each hotplug event causes the devices to be rescanned which will will only increase the system load. Thus disable the hotplug interrupts and fall back to periodic device polling. v2: Fixed cleanup typo. v3: Fixed format issues, clarified a variable name, changed pr_warn() to DRM_INFO() as suggested by Jani Nikula . Signed-off-by: Egbert Eich Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/i915_irq.c | 75 +++-- 1 file changed, 58 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 4cf33b3..d059c5d 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -88,7 +88,8 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */ [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS }; - +static void ibx_hpd_irq_setup(struct drm_device *dev); +static void i915_hpd_irq_setup(struct drm_device *dev); /* For display hotplug interrupt */ static void @@ -342,7 +343,11 @@ static void i915_hotplug_work_func(struct work_struct *work) hotplug_work); struct drm_device *dev = dev_priv->dev; struct drm_mode_config *mode_config = &dev->mode_config; - struct intel_encoder *encoder; + struct intel_connector *intel_connector; + struct intel_encoder *intel_encoder; + struct drm_connector *connector; + unsigned long irqflags; + bool hpd_disabled = false; /* HPD irq before everything is fully set up. */ if (!dev_priv->enable_hotplug_processing) @@ -351,9 +356,33 @@ static void i915_hotplug_work_func(struct work_struct *work) mutex_lock(&mode_config->mutex); DRM_DEBUG_KMS("running encoder hotplug functions\n"); - list_for_each_entry(encoder, &mode_config->encoder_list, base.head) - if (encoder->hot_plug) - encoder->hot_plug(encoder); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + list_for_each_entry(connector, &mode_config->connector_list, head) { + intel_connector = to_intel_connector(connector); + intel_encoder = intel_connector->encoder; + if (intel_encoder->hpd_pin > HPD_NONE && + dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_MARK_DISABLED && + connector->polled == DRM_CONNECTOR_POLL_HPD) { + DRM_INFO("HPD interrupt storm detected on connector %s: " +"switching from hotplug detection to polling\n", + drm_get_connector_name(connector)); + dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark = HPD_DISABLED; + connector->polled = DRM_CONNECTOR_POLL_CONNECT + | DRM_CONNECTOR_POLL_DISCONNECT; + hpd_disabled = true; + } + } +/* if there were no outputs to poll, poll was disabled, + * therefore make sure it's enabled when disabling HPD on + * some connectors */ + if (hpd_disabled) + drm_kms_helper_poll_enable(dev); + + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + + list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) + if (intel_encoder->hot_plug) + intel_encoder->hot_plug(intel_encoder); mutex_unlock(&mode_config->mutex); @@ -585,13 +614,14 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, #define HPD_STORM_DETECT_PERIOD 1000 #define HPD_STORM_THRESHOLD 5 -static inline void hotplug_irq_storm_detect(struct drm_device *dev, +static inline bool hotplug_irq_storm_detect(struct drm_device *dev, u32 hotplug_trigger, const u32 *hpd) { drm_i915_private_t *dev_priv = dev->dev_private; unsigned long irqflags; int i; + bool ret = false; spin_lock_irqsave(&dev_priv->irq_lock, irqflags); @@ -609,12 +639,15 @@ static inline void hotplug_irq_storm_detect(struct drm_device *dev, } else if (dev_priv->hpd_stats[i].hpd_cnt > HPD_STORM_THRESHOLD) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); + ret = true; } else { dev_priv->hpd_stats[i].hpd_cnt++; }
[Intel-gfx] [PATCH 5/7] drm/i915: Add Reenable Timer to turn Hotplug Detection back on (v4)
We disable hoptplug detection when we encounter a hotplug event storm. Still hotplug detection is required on some outputs (like Display Port). The interrupt storm may be only temporary (on certain Dell Laptops for instance it happens at certain charging states of the system). Thus we enable it after a certain grace period (2 minutes). Should the interrupt storm persist it will be detected immediately and it will be disabled again. v2: Reordered drm_i915_private: moved hotplug_reenable_timer to hpd state tracker. v3: Clarified loop start value, Removed superfluous test for Ivybridge and Haswell, Restructured loop to avoid deep nesting (all suggested by Ville Syrjälä) v4: Fixed two bugs pointed out by Jani Nikula. Signed-off-by: Egbert Eich Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 52 - 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 83fc1a6..195b9fe 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -938,6 +938,7 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; + struct timer_list hotplug_reenable_timer; int num_pch_pll; int num_plane; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d059c5d..ae759ac 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -337,6 +337,8 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, /* * Handle hotplug events outside the interrupt handler proper. */ +#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000) + static void i915_hotplug_work_func(struct work_struct *work) { drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t, @@ -375,8 +377,11 @@ static void i915_hotplug_work_func(struct work_struct *work) /* if there were no outputs to poll, poll was disabled, * therefore make sure it's enabled when disabling HPD on * some connectors */ - if (hpd_disabled) + if (hpd_disabled) { drm_kms_helper_poll_enable(dev); + mod_timer(&dev_priv->hotplug_reenable_timer, + jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY)); + } spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); @@ -2356,6 +2361,8 @@ static void valleyview_irq_uninstall(struct drm_device *dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + for_each_pipe(pipe) I915_WRITE(PIPESTAT(pipe), 0x); @@ -2377,6 +2384,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + I915_WRITE(HWSTAM, 0x); I915_WRITE(DEIMR, 0x); @@ -2753,6 +2762,8 @@ static void i915_irq_uninstall(struct drm_device * dev) drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; int pipe; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + if (I915_HAS_HOTPLUG(dev)) { I915_WRITE(PORT_HOTPLUG_EN, 0); I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); @@ -2997,6 +3008,8 @@ static void i965_irq_uninstall(struct drm_device * dev) if (!dev_priv) return; + del_timer_sync(&dev_priv->hotplug_reenable_timer); + I915_WRITE(PORT_HOTPLUG_EN, 0); I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); @@ -3012,6 +3025,41 @@ static void i965_irq_uninstall(struct drm_device * dev) I915_WRITE(IIR, I915_READ(IIR)); } +static void i915_reenable_hotplug_timer_func(unsigned long data) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *)data; + struct drm_device *dev = dev_priv->dev; + struct drm_mode_config *mode_config = &dev->mode_config; + unsigned long irqflags; + int i; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + for (i = (HPD_NONE + 1); i < HPD_NUM_PINS; i++) { + struct drm_connector *connector; + + if (dev_priv->hpd_stats[i].hpd_mark != HPD_DISABLED) + continue; + + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED; + + list_for_each_entry(connector, &mode_config->connector_list, head) { + struct intel_connector *intel_connector = to_intel_connector(connector); + + if (intel_connector->encoder->hpd_pin == i) { + if (connector->polled != intel_connector->poll
[Intel-gfx] [PATCH 6/7] drm/i915: Add bit field to record which pins have received HPD events (v3)
This allows to add code which limits 're'-detect() of displays to connectors that have received an HPD event. v2: Reordered drm_i915_private: Move hpd_event_bits to hpd state tracking. v3: Fixed merge conflicts with previous patches, removed some noisy debug logging as suggested by Jani Nikula. Signed-off-by: Egbert Eich Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 11 +++ 2 files changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 195b9fe..c0d9fb9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -938,6 +938,7 @@ typedef struct drm_i915_private { HPD_MARK_DISABLED = 2 } hpd_mark; } hpd_stats[HPD_NUM_PINS]; + u32 hpd_event_bits; struct timer_list hotplug_reenable_timer; int num_pch_pll; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index ae759ac..e108729 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -350,6 +350,7 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; unsigned long irqflags; bool hpd_disabled = false; + u32 hpd_event_bits; /* HPD irq before everything is fully set up. */ if (!dev_priv->enable_hotplug_processing) @@ -359,6 +360,9 @@ static void i915_hotplug_work_func(struct work_struct *work) DRM_DEBUG_KMS("running encoder hotplug functions\n"); spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + hpd_event_bits = dev_priv->hpd_event_bits; + dev_priv->hpd_event_bits = 0; list_for_each_entry(connector, &mode_config->connector_list, head) { intel_connector = to_intel_connector(connector); intel_encoder = intel_connector->encoder; @@ -373,6 +377,10 @@ static void i915_hotplug_work_func(struct work_struct *work) | DRM_CONNECTOR_POLL_DISCONNECT; hpd_disabled = true; } + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n", + drm_get_connector_name(connector), intel_encoder->hpd_pin); + } } /* if there were no outputs to poll, poll was disabled, * therefore make sure it's enabled when disabling HPD on @@ -636,6 +644,8 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev, dev_priv->hpd_stats[i].hpd_mark != HPD_ENABLED) continue; + dev_priv->hpd_event_bits |= (1 << i); + if (!time_in_range(jiffies, dev_priv->hpd_stats[i].hpd_last_jiffies, dev_priv->hpd_stats[i].hpd_last_jiffies + msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) { @@ -643,6 +653,7 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev, dev_priv->hpd_stats[i].hpd_cnt = 0; } else if (dev_priv->hpd_stats[i].hpd_cnt > HPD_STORM_THRESHOLD) { dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; + dev_priv->hpd_event_bits &= ~(1 << i); DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); ret = true; } else { -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/7] drm/i915: Only reprobe display on encoder which has received an HPD event (v2)
Instead of calling into the DRM helper layer to poll all connectors for changes in connected displays probe only those connectors which have received a hotplug event. Signed-off-by: Egbert Eich Reviewed-by: Jani Nikula v2: Resolved conflicts with changes in previous commits. Renamed function and and added a WARN_ON() to warn of intel_hpd_irq_event() from being called without mode_config.mutex held - suggested by Jani Nikula. --- drivers/gpu/drm/i915/i915_irq.c | 34 -- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index e108729..dd579d7 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -334,6 +334,21 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, crtc); } +static int intel_hpd_irq_event(struct drm_device *dev, struct drm_connector *connector) +{ + enum drm_connector_status old_status; + + WARN_ON(!mutex_is_locked(&dev->mode_config.mutex)); + old_status = connector->status; + + connector->status = connector->funcs->detect(connector, false); + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n", + connector->base.id, + drm_get_connector_name(connector), + old_status, connector->status); + return (old_status != connector->status); +} + /* * Handle hotplug events outside the interrupt handler proper. */ @@ -350,6 +365,7 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_connector *connector; unsigned long irqflags; bool hpd_disabled = false; + bool changed = false; u32 hpd_event_bits; /* HPD irq before everything is fully set up. */ @@ -393,14 +409,20 @@ static void i915_hotplug_work_func(struct work_struct *work) spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); - list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head) - if (intel_encoder->hot_plug) - intel_encoder->hot_plug(intel_encoder); - + list_for_each_entry(connector, &mode_config->connector_list, head) { + intel_connector = to_intel_connector(connector); + intel_encoder = intel_connector->encoder; + if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) { + if (intel_encoder->hot_plug) + intel_encoder->hot_plug(intel_encoder); + if (intel_hpd_irq_event(dev, connector)) + changed = true; + } + } mutex_unlock(&mode_config->mutex); - /* Just fire off a uevent and let userspace tell us what to do */ - drm_helper_hpd_irq_event(dev); + if (changed) + drm_kms_helper_hotplug_event(dev); } static void ironlake_handle_rps_change(struct drm_device *dev) -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Add Reenable Timer to turn Hotplug Detection back on (v4)
On Tue, Apr 16, 2013 at 08:07:09PM +0200, Daniel Vetter wrote: > On Tue, Apr 16, 2013 at 01:36:58PM +0200, Egbert Eich wrote: > > We disable hoptplug detection when we encounter a hotplug event > > storm. Still hotplug detection is required on some outputs (like > > Display Port). The interrupt storm may be only temporary (on certain > > Dell Laptops for instance it happens at certain charging states of > > the system). Thus we enable it after a certain grace period (2 minutes). > > Should the interrupt storm persist it will be detected immediately > > and it will be disabled again. > > > > v2: Reordered drm_i915_private: moved hotplug_reenable_timer to hpd state > > tracker. > > v3: Clarified loop start value, > > Removed superfluous test for Ivybridge and Haswell, > > Restructured loop to avoid deep nesting (all suggested by Ville Syrjälä) > > v4: Fixed two bugs pointed out by Jani Nikula. > > > > Signed-off-by: Egbert Eich > > Reviewed-by: Jani Nikula > > I've queued up patches 1-5 of this series, thanks a lot for doing all > this. Wrt bikesheds: checkpatch seemed a bit unhappy about some of them, > but I've decided that I want this more for 3.10 than care about checkpatch > ;-) I've run checkpatch on every patch - here it only complains about lines longer than 80 characters. Now - if you can prove that my file introduced lines with more than 80 characters to a file that had no such lines already we can talk about fixing this ;p Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: force full modeset if the connector is in DPMS OFF mode
On Fri, May 03, 2013 at 02:22:09PM +0300, Imre Deak wrote: > Currently the driver's assumed behavior for a modeset with an attached > FB is that the corresponding connector will be switched to DPMS ON mode > if it happened to be in DPMS OFF (or another power save mode). This > wasn't enforced though if only the FB changed, everything else (format, > connector etc.) remaining the same. In this case we only set the new FB > base and left the connector in the old power save mode. > > Fix this by forcing a full modeset whenever there is an attached FB and > any affected connector is in a power save mode. > > Signed-off-by: Imre Deak [..] > @@ -8397,8 +8412,12 @@ intel_set_config_compute_mode_changes(struct > drm_mode_set *set, > } else if (set->fb->pixel_format != > set->crtc->fb->pixel_format) { > config->mode_changed = true; > - } else > + } else if (crtc_connector_off(set->crtc, *set->connectors, > + set->num_connectors)) { > + config->mode_changed = true; > + } else { > config->fb_changed = true; > + } > } > > if (set->fb && (set->x != set->crtc->x || set->y != set->crtc->y)) On https://bugs.freedesktop.org/show_bug.cgi?id=64178 I had a similar problem for which I found a very similar 'workaround': Assuming the Xserver is displaying a mode a on output A, doing a: xrandr --output A --off xrandr --output A --mode a will not light up the screen. This all sounds quite similar, to the issues describe by this patch, however the patch as is will not fix this issue: With this setup the fb does not change. The crtc_connector_off() test however only runs if set->crtc->fb != set->fb is true. Thus the connector_off() test needs to happen before: - if (set->crtc->fb != set->fb) { + if (set->connectors != NULL && + connector_off(set->crtc, *set->connectors, + set->num_connectors)) { + config->mode_changed = true; + } else if (set->crtc->fb != set->fb) { Mind the test for set->connectors != NULL as now the connectors list may be empty. Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V2] drm/i915: force full modeset if the connector is in DPMS OFF mode
From: Imre Deak Currently the driver's assumed behavior for a modeset with an attached FB is that the corresponding connector will be switched to DPMS ON mode if it happened to be in DPMS OFF (or another power save mode). This wasn't enforced though if only the FB changed, everything else (format, connector etc.) remaining the same. In this case we only set the new FB base and left the connector in the old power save mode. Fix this by forcing a full modeset whenever there is an attached FB and any affected connector is in a power save mode. V_2: Run the test for encoders in power save mode outside the the test for fb change: user space may have just disabled the encoders but left everything else in place. Make sure the connector list is not empty before running this test. Signed-off-by: Imre Deak Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/intel_display.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2939524..992d469 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8380,6 +8380,21 @@ static void intel_set_config_restore_state(struct drm_device *dev, } } +static bool +connector_off(struct drm_crtc *crtc, struct drm_connector *connectors, + int num_connectors) +{ + int i; + + for (i = 0; i < num_connectors; i++) + if (connectors[i].encoder && + connectors[i].encoder->crtc == crtc && + connectors[i].dpms != DRM_MODE_DPMS_ON) + return true; + + return false; +} + static void intel_set_config_compute_mode_changes(struct drm_mode_set *set, struct intel_set_config *config) @@ -8387,7 +8402,11 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set, /* We should be able to check here if the fb has the same properties * and then just flip_or_move it */ - if (set->crtc->fb != set->fb) { + if (set->connectors != NULL && + connector_off(set->crtc, *set->connectors, + set->num_connectors)) { + config->mode_changed = true; + } else if (set->crtc->fb != set->fb) { /* If we have no fb then treat it as a full mode set */ if (set->crtc->fb == NULL) { DRM_DEBUG_KMS("crtc has no fb, full mode set\n"); @@ -8397,8 +8416,9 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set, } else if (set->fb->pixel_format != set->crtc->fb->pixel_format) { config->mode_changed = true; - } else + } else { config->fb_changed = true; + } } if (set->fb && (set->x != set->crtc->x || set->y != set->crtc->y)) -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/i915: Always enable the cursor right after the primary plane
On Wed, May 08, 2013 at 12:50:24PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Follow the same sequence when enabling the cursor plane during > modeset. No point in doing this stuff in different order on different > generations. > > This should also avoid a needless wait for vblank for the g4x cursor > workaround when the cursor gets enabled anyway. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 10 -- > 1 file changed, 4 insertions(+), 6 deletions(-) > > @@ -3727,6 +3725,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) > > intel_enable_pipe(dev_priv, pipe, false); > intel_enable_plane(dev_priv, plane, pipe); > + intel_crtc_update_cursor(crtc, true); > if (IS_G4X(dev)) > g4x_fixup_plane(dev_priv, pipe); > As discussed on IRC this may interfere with commit 61bc95c1fbbb6a08b55bbe161fdf1ea5493fc595 Author: Egbert Eich Date: Mon Mar 4 09:24:38 2013 -0500 DRM/i915: On G45 enable cursor plane briefly after enabling the display plane. described in https://bugs.freedesktop.org/show_bug.cgi?id=61457 I cannot test on the affected hardware as it's not available to me at the moment. If Ville's G4x doesn't show the issue I will test this next time I have access to the HW in question. Therefore: Acked-by: Egbert Eich Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/i915: Always enable the cursor right after the primary plane
On Wed, May 22, 2013 at 02:15:28PM +0300, Ville Syrjälä wrote: > > I couldn't reproduce the original problem on my g4x :( Ville, just go ahead with your patch. I will test this as soon as I have access to a system where I know I can reproduce this. I'll get back to you then. > > > Therefore: > > > > Acked-by: Egbert Eich > > Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] DRM/i915: Restore sdvo_flags after dtd->mode->dtd Roundrtrip.
For TV and LVDS encoders intel_sdvo_set_input_timings_for_mode() is called to pass a mode to the sdvo chip and retrieve a dtd containing information needed to calculate the adjusted_mode which is done by intel_sdvo_get_dtd_from_mode(). To set this adjusted_mode as input mode for the sdvo chip, a dtd is recalculated using intel_sdvo_get_mode_from_dtd(). During this round trip the sdvo_flags contained in the dtd obtained from the hardware are lost. Since these flags cannot be ignored in all cases this patch preserves and restores them. This regression has been introduced in commit 6651819b4b4fc3caa6964c5d825eb4bb996f3905 Author: Daniel Vetter Date: Sun Apr 1 19:16:18 2012 +0200 drm/i915: handle input/output sdvo timings separately in mode_set Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/intel_sdvo.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 39c3198..d192ce4 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -140,6 +140,11 @@ struct intel_sdvo { /* DDC bus used by this SDVO encoder */ uint8_t ddc_bus; + + /* +* the sdvo flag gets lost in round trip: dtd->adjusted_mode->dtd +*/ + uint8_t dtd_sdvo_flags; }; struct intel_sdvo_connector { @@ -985,6 +990,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo, return false; intel_sdvo_get_mode_from_dtd(adjusted_mode, &input_dtd); + intel_sdvo->dtd_sdvo_flags = input_dtd.part2.sdvo_flags; return true; } @@ -1093,6 +1099,8 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder, * adjusted_mode. */ intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode); + if (intel_sdvo->is_tv || intel_sdvo->is_lvds) + input_dtd.part2.sdvo_flags = intel_sdvo->dtd_sdvo_flags; if (!intel_sdvo_set_input_timing(intel_sdvo, &input_dtd)) DRM_INFO("Setting input timings on %s failed\n", SDVO_NAME(intel_sdvo)); -- 1.7.6.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] DRM/i915: Don't clone SDVO LVDS with analog.
SDVO LVDS are not clonable as the input mode gets adjusted by the LVDS encoder. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/intel_sdvo.c |6 ++ 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index d192ce4..d2e8c9f 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -2286,10 +2286,8 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device) intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS1; } - /* SDVO LVDS is cloneable because the SDVO encoder does the upscaling, -* as opposed to native LVDS, where we upscale with the panel-fitter -* (and hence only the native LVDS resolution could be cloned). */ - intel_sdvo->base.cloneable = true; + /* SDVO LVDS is not cloneable because the input mode gets adjusted by the encoder */ + intel_sdvo->base.cloneable = false; intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo); if (!intel_sdvo_create_enhance_property(intel_sdvo, intel_sdvo_connector)) -- 1.7.6.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] DRM/i915: Add QUIRK_INVERT_BRIGHTNESS for NCR machines.
NCR machines with LVDS panels using Intel chipsets need to have the QUIRK_INVERT_BRIGHTNESS bit set. Unfortunately NCR doesn't set a meaningful subvendor/subdevice ID, therefore we add a DMI dependent quirk list. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/intel_display.c | 32 1 files changed, 32 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3511eff..e809fc5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7892,6 +7892,34 @@ struct intel_quirk { void (*hook)(struct drm_device *dev); }; +/* For systems that don't have a meaningful PCI subdevice/subvendor ID */ +struct intel_dmi_quirk { + void (*hook)(struct drm_device *dev); + const struct dmi_system_id (*dmi_id_list)[]; +}; + +static int intel_dmi_reverse_brightness(const struct dmi_system_id *id) +{ + DRM_INFO("Backlight polarity reversed on %s\n", id->ident); + return 1; +} + +static const struct intel_dmi_quirk intel_dmi_quirks[] = { + { + .dmi_id_list = &(const struct dmi_system_id[]) { + { + .callback = intel_dmi_reverse_brightness, + .ident = "NCR Corporation", + .matches = {DMI_MATCH(DMI_SYS_VENDOR, "NCR Corporation"), + DMI_MATCH(DMI_PRODUCT_NAME, ""), + }, + }, + { } /* terminating entry */ + }, + .hook = quirk_invert_brightness, + }, +}; + static struct intel_quirk intel_quirks[] = { /* HP Mini needs pipe A force quirk (LP: #322104) */ { 0x27ae, 0x103c, 0x361a, quirk_pipea_force }, @@ -7931,6 +7959,10 @@ static void intel_init_quirks(struct drm_device *dev) q->subsystem_device == PCI_ANY_ID)) q->hook(dev); } + for (i = 0; i < ARRAY_SIZE(intel_dmi_quirks); i++) { + if (dmi_check_system(*intel_dmi_quirks[i].dmi_id_list) != 0) + intel_dmi_quirks[i].hook(dev); + } } /* Disable the VGA plane that we never use */ -- 1.7.6.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] DRM/i915: Don't delete DPLL Multiplier during DAC init.
The DPLL multipiler is set up in intel_display.c:i9xx_update_pll() called from i9xx_crtc_mode_set(). There the DPLL multiplier is adjusted so that the SDVO gets a sufficient bus clock. When cloning a CRTC between an SDVO driven encoder and the standard DAC the DAC setup code reseted the multiplier value to 1 thus undoing the correct setup. There is no need to touch the multiplier in the DAC setup code: the correct value (i.e. 1 in case no SDVO encoder is used) is set by i9xx_update_pll() already. A comment at the code suggested that this code is a left over from the days when there was no setup for clone modes. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/intel_crt.c | 15 +-- 1 files changed, 1 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index c42b980..ae3a3d5 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -220,20 +220,7 @@ static void intel_crt_mode_set(struct drm_encoder *encoder, intel_encoder_to_crt(to_intel_encoder(encoder)); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct drm_i915_private *dev_priv = dev->dev_private; - int dpll_md_reg; - u32 adpa, dpll_md; - - dpll_md_reg = DPLL_MD(intel_crtc->pipe); - - /* -* Disable separate mode multiplier used when cloning SDVO to CRT -* XXX this needs to be adjusted when we really are cloning -*/ - if (INTEL_INFO(dev)->gen >= 4 && !HAS_PCH_SPLIT(dev)) { - dpll_md = I915_READ(dpll_md_reg); - I915_WRITE(dpll_md_reg, - dpll_md & ~DPLL_MD_UDI_MULTIPLIER_MASK); - } + u32 adpa; adpa = ADPA_HOTPLUG_BITS; if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC) -- 1.7.6.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/8] Detect and deal with Interrupt 'Storms' from noisy Hotplug Lines.
Despite the many attempts to fix the issue with noisy hotplug interrupt lines we are still seeing systems that suffer from this: Recently we encountered a rather large scale installation of Q35 systems which was hit by this issue rather severely: It seemed as if not all machines of the same model were hit equally bad, in the worst cased hotplug interrupt noise caused several 1000 interrupts / s. Those machines would not even boot, instead the interrupt handler and the scheduled workers would keep the CPU busy that eventually the watchdog would kick in and issue an NMI. Other machines only received severa 10s to 100s of interrupts per sec - those machines would run properly - just with an excessive system load. More thorough investigations seemed to indicate that this condition only happen at certain video modes. On another system - a laptop - a hotplug interrupt 'storm' occurred when it was charging and the batteries were at certain charge levels. While the system was still running fine its load was high enough that the user noticed from the fan noise that a problem existed. The latter system had a Sandybridge chipset, thus a totally different generation from the former. All those cases seemed to have been caused by cross talk on badly routed hotplug signal lines (or voltage instabilities). This led to the conclusion that instead of trying to work around these 'storms' for each individual system, there should be a generic way to detect such a condition and take appropriate action: This patch series implements a hotplug 'storm' detection, disables the respective interrupt for the hotplug pin when this condition is detected and reverts to periodic output polling on the affected connector. After a grace period of 2 minutes it will reenable hotplug on the affected line. This will take care of cases in which this condition is only temporary. Should the 'storm' condtion persist, this cycle will start over again. To implement this some rearrangements in the code were required: - The interrupt status bit which signals a hotplug needed to be recorded for each connector. - The interrupt enable functions needed to be separate, also they need to be able to enable interrupts for each hotplug line independently. Egbert Eich (8): drm/i915: Remove pch_rq_mask from struct drm_i915_private. drm/i915: Set hotplug_supported_flag for all chipset generations. drm/i915: Add hpd status bit to struct intel_connector. drm/i915: Add Hotplug IRQ Storm detection. drm/i915: Move hotplug interrupt enable for i915/i965/valleyview into a separate function. drm/i915: Only enable hotplug irq when needed on Ironlake and later chips. drm/i915: When detecting a hotplug IRQ storm disable respective IRQs. drm/i915: Add Reenable Timer to turn Hotplug Detection back on. drivers/gpu/drm/i915/i915_drv.h |7 +- drivers/gpu/drm/i915/i915_irq.c | 475 +++-- drivers/gpu/drm/i915/intel_crt.c |3 +- drivers/gpu/drm/i915/intel_dp.c |5 +- drivers/gpu/drm/i915/intel_drv.h | 11 + drivers/gpu/drm/i915/intel_hdmi.c |5 +- drivers/gpu/drm/i915/intel_sdvo.c | 23 +- 7 files changed, 383 insertions(+), 146 deletions(-) -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/8] drm/i915: Set hotplug_supported_flag for all chipset generations.
So far the hotplug_supported_mask in the struct drm_i915_private is only used for pre-Ironlake chipsets. This patch sets up the correct value for all generations. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_drv.h |4 ++ drivers/gpu/drm/i915/i915_irq.c | 71 + drivers/gpu/drm/i915/intel_crt.c |2 +- drivers/gpu/drm/i915/intel_dp.c |4 +-- drivers/gpu/drm/i915/intel_hdmi.c |4 +-- drivers/gpu/drm/i915/intel_sdvo.c | 12 +-- 6 files changed, 79 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index eeaf2a8..d7ad677 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1322,6 +1322,10 @@ extern void intel_irq_init(struct drm_device *dev); extern void intel_gt_init(struct drm_device *dev); extern void intel_gt_reset(struct drm_device *dev); +extern u32 intel_crt_hotplug_int_status(struct drm_device *dev); +extern u32 intel_sdvo_hotplug_int_status(struct drm_device *dev, bool is_sdvo_b); +extern u32 intel_hotplug_int_status(struct drm_device *dev, enum port port); + void i915_error_state_free(struct kref *error_ref); void diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index bb5e6d8..c0e302e 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2765,3 +2765,74 @@ void intel_irq_init(struct drm_device *dev) dev->driver->disable_vblank = i915_disable_vblank; } } + +u32 intel_hotplug_int_status(struct drm_device *dev, enum port port) +{ + /* SDVO is treated separately */ + if (IS_IVYBRIDGE(dev) || + IS_HASWELL(dev) || + (HAS_PCH_SPLIT(dev) && HAS_PCH_CPT(dev))) { + switch (port) { + case PORT_B: + return SDE_PORTB_HOTPLUG_CPT; + case PORT_C: + return SDE_PORTC_HOTPLUG_CPT; + case PORT_D: + return SDE_PORTD_HOTPLUG_CPT; + default: + BUG(); + } + } else if (HAS_PCH_SPLIT(dev)) { /* ! HAS_PCH_CPT(dev) */ + switch (port) { + case PORT_B: + return SDE_PORTB_HOTPLUG; + case PORT_C: + return SDE_PORTC_HOTPLUG; + case PORT_D: + return SDE_PORTD_HOTPLUG; + default: + BUG(); + } + } else { + switch (port) { + case PORT_B: + return HDMIB_HOTPLUG_INT_STATUS; /* same as PDB_HOTPLUG_INT_STATUS */ + case PORT_C: + return HDMIC_HOTPLUG_INT_STATUS; /* same as PDB_HOTPLUG_INT_STATUS */ + case PORT_D: + return HDMID_HOTPLUG_INT_STATUS; /* same as PDB_HOTPLUG_INT_STATUS */ + default: + BUG(); + } + } + return 0; +} + +u32 intel_crt_hotplug_int_status(struct drm_device *dev) +{ + if (IS_IVYBRIDGE(dev) || + IS_HASWELL(dev) || + (HAS_PCH_SPLIT(dev) && HAS_PCH_CPT(dev))) { + return SDE_CRT_HOTPLUG_CPT; + } else if (HAS_PCH_SPLIT(dev)) { /* ! HAS_PCH_CPT(dev) */ + return SDE_CRT_HOTPLUG; + } else { + return CRT_HOTPLUG_INT_STATUS; + } + return 0; +} + +u32 intel_sdvo_hotplug_int_status(struct drm_device *dev, bool is_sdvo_b) +{ + if (IS_G4X(dev)) { + return is_sdvo_b ? + SDVOB_HOTPLUG_INT_STATUS_G4X : SDVOC_HOTPLUG_INT_STATUS_G4X; + } else if (IS_GEN4(dev)) { + return is_sdvo_b ? + SDVOB_HOTPLUG_INT_STATUS_I965 : SDVOC_HOTPLUG_INT_STATUS_I965; + } else { + return is_sdvo_b ? + SDVOB_HOTPLUG_INT_STATUS_I915 : SDVOC_HOTPLUG_INT_STATUS_I915; + } + return 0; +} diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index fe20bf7..0cd9ff0 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -797,7 +797,7 @@ void intel_crt_init(struct drm_device *dev) */ crt->force_hotplug_required = 0; - dev_priv->hotplug_supported_mask |= CRT_HOTPLUG_INT_STATUS; + dev_priv->hotplug_supported_mask |= intel_crt_hotplug_int_status(dev); /* * TODO: find a proper way to discover whether we need to set the diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d76258d..1946b5b 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2754,21 +2754,19 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, name = "DPDDC-A"; break;
[Intel-gfx] [PATCH 4/8] drm/i915: Add Hotplug IRQ Storm detection.
Add a hotplug IRQ storm detection (triggered when a hotplug interrupt fires more than 5 times / sec). Mask out this specific interrupt and revert to polling on the associated output. Rationale: Despite of the many attempts to fix the problem with noisy hotplug interrupt lines we are still seeing systems which have issues: Once cause of noise seems to be bad routing of the hotplug line on the board: cross talk from other signals seems to cause erronous hotplug interrupts. This has been documented as an erratum for the the i945GM chipset and thus hotplug support was disabled for this chipset model but others seem to have this problem, too. We have seen this issue on a G35 motherboard for example: Even different motherboards of the same model seem to behave differently: while some only see only around 10-100 interrupts/s others seem to see 5k or more. We've also observed a dependency on the selected video mode. Also on certain laptops interrupt noise seems to occur duing battery charging when the battery is at a certain charge levels. Thus we add a simple algorithm here that detects an 'interrupt storm' condition. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/i915_irq.c | 74 -- drivers/gpu/drm/i915/intel_drv.h |4 ++ 2 files changed, 67 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index c0e302e..4e75df0 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -285,11 +285,24 @@ static void i915_hotplug_work_func(struct work_struct *work) hotplug_work); struct drm_device *dev = dev_priv->dev; struct drm_mode_config *mode_config = &dev->mode_config; + struct drm_connector *connector; struct intel_encoder *encoder; + unsigned long irqflags; mutex_lock(&mode_config->mutex); DRM_DEBUG_KMS("running encoder hotplug functions\n"); + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + list_for_each_entry(connector, &mode_config->connector_list, head) { + struct intel_connector *intel_connector = to_intel_connector(connector); + if (intel_connector->hpd_mark_disabled) { + intel_connector->hpd_mark_disabled = 0; + connector->polled = DRM_CONNECTOR_POLL_CONNECT +| DRM_CONNECTOR_POLL_DISCONNECT; + } + } + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + list_for_each_entry(encoder, &mode_config->encoder_list, base.head) if (encoder->hot_plug) encoder->hot_plug(encoder); @@ -300,6 +313,36 @@ static void i915_hotplug_work_func(struct work_struct *work) drm_helper_hpd_irq_event(dev); } +static inline void hotplug_irq_storm_detect(struct drm_device *dev, u32 hotplug_trigger) +{ + struct drm_mode_config *mode_config = &dev->mode_config; + drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_connector *connector; + unsigned long irqflags; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + list_for_each_entry(connector, &mode_config->connector_list, head) { + struct intel_connector *intel_connector = to_intel_connector(connector); + + if (hotplug_trigger & intel_connector->hpd_status_bit) { + if (jiffies > (intel_connector->last_hpd_jiffies + msecs_to_jiffies(1000)) || + jiffies < intel_connector->last_hpd_jiffies) { + intel_connector->last_hpd_jiffies = jiffies; + intel_connector->hpd_cnt = 0; + } else if (intel_connector->hpd_cnt > 5) { + dev_priv->hotplug_supported_mask &= ~intel_connector->hpd_status_bit; + intel_connector->hpd_mark_disabled = 1; + pr_warn("HPD interrupt storm on connector %s disabling\n", +drm_get_connector_name(connector)); + } else + intel_connector->hpd_cnt++; + } + } + + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); +} + /* defined intel_pm.c */ extern spinlock_t mchdev_lock; @@ -579,13 +622,14 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) /* Consume port. Then clear IIR or we'll miss events */ if (iir & I915_DISPLAY_PORT_INTERRUPT) { u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT); - + u32 hotplug_trigger = hotplug_
[Intel-gfx] [PATCH 3/8] drm/i915: Add hpd status bit to struct intel_connector.
To be able to map an HPD interrupt to a connector add the hpd status bit to the intel_connector structure. Signed-off-by: Egbert Eich --- drivers/gpu/drm/i915/intel_crt.c |3 ++- drivers/gpu/drm/i915/intel_dp.c |3 ++- drivers/gpu/drm/i915/intel_drv.h |3 +++ drivers/gpu/drm/i915/intel_hdmi.c |3 ++- drivers/gpu/drm/i915/intel_sdvo.c | 11 ++- 5 files changed, 19 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 0cd9ff0..75298ce 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -797,7 +797,8 @@ void intel_crt_init(struct drm_device *dev) */ crt->force_hotplug_required = 0; - dev_priv->hotplug_supported_mask |= intel_crt_hotplug_int_status(dev); + intel_connector->hpd_status_bit = intel_crt_hotplug_int_status(dev); + dev_priv->hotplug_supported_mask |= intel_connector->hpd_status_bit; /* * TODO: find a proper way to discover whether we need to set the diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1946b5b..b97eb76 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2766,7 +2766,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, WARN(1, "Invalid port %c\n", port_name(port)); break; } - dev_priv->hotplug_supported_mask |= intel_hotplug_int_status(dev, port); + intel_connector->hpd_status_bit = intel_hotplug_int_status(dev, port); + dev_priv->hotplug_supported_mask |= intel_connector->hpd_status_bit; if (is_edp(intel_dp)) intel_dp_init_panel_power_sequencer(dev, intel_dp); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8a1bd4a..a251036 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -191,6 +191,9 @@ struct intel_connector { /* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */ struct edid *edid; + + /* hpd status bit for this connector */ + u32 hpd_status_bit; }; struct intel_crtc { diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index d20f9ff..9099b6dc 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -992,7 +992,8 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, default: BUG(); } - dev_priv->hotplug_supported_mask |= intel_hotplug_int_status(dev, port); + intel_connector->hpd_status_bit = intel_hotplug_int_status(dev, port); + dev_priv->hotplug_supported_mask |= intel_connector->hpd_status_bit; if (!HAS_PCH_SPLIT(dev)) { intel_hdmi->write_infoframe = g4x_write_infoframe; diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 567846e..c99bb9d 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -2697,6 +2697,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_encoder *intel_encoder; struct intel_sdvo *intel_sdvo; + struct drm_connector *connector; u32 hotplug_mask; int i; @@ -2749,8 +2750,16 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob) /* Only enable the hotplug irq if we need it, to work around noisy * hotplug lines. */ - if (intel_sdvo->hotplug_active) + if (intel_sdvo->hotplug_active) { dev_priv->hotplug_supported_mask |= hotplug_mask; + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { + struct intel_connector *intel_connector = to_intel_connector(connector); + if (intel_connector->encoder == intel_encoder) { + intel_connector->hpd_status_bit = hotplug_mask; + break; + } + } + } intel_sdvo_select_ddc_bus(dev_priv, intel_sdvo, sdvo_reg); -- 1.7.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx