From 7a90938332d80faf973fbcffdf6e674e7b8f0914 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 26 Sep 2018 11:47:18 +0100 Subject: drm/i915: Avoid compiler warning for maybe unused gu_misc_iir /kisskb/src/drivers/gpu/drm/i915/i915_irq.c: warning: 'gu_misc_iir' may be used uninitialized in this function [-Wuninitialized]: => 3120:10 Silence the compiler warning by ensuring that the local variable is initialised and removing the guard that is confusing the older gcc. Reported-by: Geert Uytterhoeven Fixes: df0d28c185ad ("drm/i915/icl: GSE interrupt moves from DE_MISC to GU_MISC") Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Paulo Zanoni Reviewed-by: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20180926104718.17462-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_irq.c | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_irq.c') diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 10f28a2ee2e6..2e242270e270 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3088,36 +3088,27 @@ gen11_gt_irq_handler(struct drm_i915_private * const i915, spin_unlock(&i915->irq_lock); } -static void -gen11_gu_misc_irq_ack(struct drm_i915_private *dev_priv, const u32 master_ctl, - u32 *iir) +static u32 +gen11_gu_misc_irq_ack(struct drm_i915_private *dev_priv, const u32 master_ctl) { void __iomem * const regs = dev_priv->regs; + u32 iir; if (!(master_ctl & GEN11_GU_MISC_IRQ)) - return; + return 0; + + iir = raw_reg_read(regs, GEN11_GU_MISC_IIR); + if (likely(iir)) + raw_reg_write(regs, GEN11_GU_MISC_IIR, iir); - *iir = raw_reg_read(regs, GEN11_GU_MISC_IIR); - if (likely(*iir)) - raw_reg_write(regs, GEN11_GU_MISC_IIR, *iir); + return iir; } static void -gen11_gu_misc_irq_handler(struct drm_i915_private *dev_priv, - const u32 master_ctl, const u32 iir) +gen11_gu_misc_irq_handler(struct drm_i915_private *dev_priv, const u32 iir) { - if (!(master_ctl & GEN11_GU_MISC_IRQ)) - return; - - if (unlikely(!iir)) { - DRM_ERROR("GU_MISC iir blank!\n"); - return; - } - if (iir & GEN11_GU_MISC_GSE) intel_opregion_asle_intr(dev_priv); - else - DRM_ERROR("Unexpected GU_MISC interrupt 0x%x\n", iir); } static irqreturn_t gen11_irq_handler(int irq, void *arg) @@ -3154,12 +3145,12 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) enable_rpm_wakeref_asserts(i915); } - gen11_gu_misc_irq_ack(i915, master_ctl, &gu_misc_iir); + gu_misc_iir = gen11_gu_misc_irq_ack(i915, master_ctl); /* Acknowledge and enable interrupts. */ raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ | master_ctl); - gen11_gu_misc_irq_handler(i915, master_ctl, gu_misc_iir); + gen11_gu_misc_irq_handler(i915, gu_misc_iir); return IRQ_HANDLED; } -- cgit v1.2.3 From 4376b9c965c0563b1f95e8b7fae560e8e4890c00 Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Mon, 15 Oct 2018 17:14:38 +0300 Subject: drm/i915/gen8: Disable master intr before reading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Disable master interrupt before reading level indications. This will close a race where we get a level indication between reading and disabling, generating an extra interrupt where we could have avoided one. Further, as the reading acts also as a post, replace the write/post on the irq reset with the helper. On enabling side, posting doesn't serve any purpose so it can also be replaced with helper. Cc: Chris Wilson Cc: Ville Syrjälä Signed-off-by: Mika Kuoppala Reviewed-by: Chris Wilson Link: https://patchwork.freedesktop.org/patch/msgid/20181015141440.21845-1-mika.kuoppala@linux.intel.com --- drivers/gpu/drm/i915/i915_irq.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_irq.c') diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2e242270e270..cbc04dd59041 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2887,21 +2887,39 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl) return ret; } +static inline u32 gen8_master_intr_disable(void __iomem * const regs) +{ + raw_reg_write(regs, GEN8_MASTER_IRQ, 0); + + /* + * Now with master disabled, get a sample of level indications + * for this interrupt. Indications will be cleared on related acks. + * New indications can and will light up during processing, + * and will generate new interrupt after enabling master. + */ + return raw_reg_read(regs, GEN8_MASTER_IRQ); +} + +static inline void gen8_master_intr_enable(void __iomem * const regs) +{ + raw_reg_write(regs, GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); +} + static irqreturn_t gen8_irq_handler(int irq, void *arg) { struct drm_i915_private *dev_priv = to_i915(arg); + void __iomem * const regs = dev_priv->regs; u32 master_ctl; u32 gt_iir[4]; if (!intel_irqs_enabled(dev_priv)) return IRQ_NONE; - master_ctl = I915_READ_FW(GEN8_MASTER_IRQ); - master_ctl &= ~GEN8_MASTER_IRQ_CONTROL; - if (!master_ctl) + master_ctl = gen8_master_intr_disable(regs); + if (!master_ctl) { + gen8_master_intr_enable(regs); return IRQ_NONE; - - I915_WRITE_FW(GEN8_MASTER_IRQ, 0); + } /* Find, clear, then process each source of interrupt */ gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir); @@ -2913,7 +2931,7 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg) enable_rpm_wakeref_asserts(dev_priv); } - I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); + gen8_master_intr_enable(regs); gen8_gt_irq_handler(dev_priv, master_ctl, gt_iir); @@ -3598,8 +3616,7 @@ static void gen8_irq_reset(struct drm_device *dev) struct drm_i915_private *dev_priv = to_i915(dev); int pipe; - I915_WRITE(GEN8_MASTER_IRQ, 0); - POSTING_READ(GEN8_MASTER_IRQ); + gen8_master_intr_disable(dev_priv->regs); gen8_gt_irq_reset(dev_priv); @@ -4244,8 +4261,7 @@ static int gen8_irq_postinstall(struct drm_device *dev) if (HAS_PCH_SPLIT(dev_priv)) ibx_irq_postinstall(dev); - I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); - POSTING_READ(GEN8_MASTER_IRQ); + gen8_master_intr_enable(dev_priv->regs); return 0; } -- cgit v1.2.3 From 95b0e7c14c500ac7522924a54e6beddf69d383ec Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Mon, 15 Oct 2018 17:14:39 +0300 Subject: drm/i915/icl: No need to ack intr through master control All other master control register bits, except the enable, are read only and they are level indications of the second level interrupt status. Only touch enable bit and rectify the comment. Cc: Chris Wilson Cc: Dhinakaran Pandiyan Signed-off-by: Mika Kuoppala Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20181015141440.21845-2-mika.kuoppala@linux.intel.com --- drivers/gpu/drm/i915/i915_irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_irq.c') diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index cbc04dd59041..e0310ebd9c8d 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3165,8 +3165,8 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) gu_misc_iir = gen11_gu_misc_irq_ack(i915, master_ctl); - /* Acknowledge and enable interrupts. */ - raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ | master_ctl); + /* Enable interrupts. */ + raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ); gen11_gu_misc_irq_handler(i915, gu_misc_iir); -- cgit v1.2.3 From 81067b71c1d944ea1f93ee67ef0be16d8b4957b4 Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Mon, 15 Oct 2018 17:14:40 +0300 Subject: drm/i915/icl: Disable master intr before reading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Disable master interrupt before reading level indications. This will close a race where we get a level indication between reading and disabling, generating an extra interrupt where we could have avoided one. Further, as the reading acts also as a post, replace the write/post on the irq reset with the helper. On enabling side, posting doesn't serve any purpose so it can also be replaced with helper. Cc: Chris Wilson Cc: Ville Syrjälä Cc: Daniele Ceraolo Spurio Signed-off-by: Mika Kuoppala Acked-by: Chris Wilson Reviewed-by: Daniele Ceraolo Spurio Link: https://patchwork.freedesktop.org/patch/msgid/20181015141440.21845-3-mika.kuoppala@linux.intel.com --- drivers/gpu/drm/i915/i915_irq.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_irq.c') diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index e0310ebd9c8d..5d1f53723388 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3129,6 +3129,24 @@ gen11_gu_misc_irq_handler(struct drm_i915_private *dev_priv, const u32 iir) intel_opregion_asle_intr(dev_priv); } +static inline u32 gen11_master_intr_disable(void __iomem * const regs) +{ + raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, 0); + + /* + * Now with master disabled, get a sample of level indications + * for this interrupt. Indications will be cleared on related acks. + * New indications can and will light up during processing, + * and will generate new interrupt after enabling master. + */ + return raw_reg_read(regs, GEN11_GFX_MSTR_IRQ); +} + +static inline void gen11_master_intr_enable(void __iomem * const regs) +{ + raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ); +} + static irqreturn_t gen11_irq_handler(int irq, void *arg) { struct drm_i915_private * const i915 = to_i915(arg); @@ -3139,13 +3157,11 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) if (!intel_irqs_enabled(i915)) return IRQ_NONE; - master_ctl = raw_reg_read(regs, GEN11_GFX_MSTR_IRQ); - master_ctl &= ~GEN11_MASTER_IRQ; - if (!master_ctl) + master_ctl = gen11_master_intr_disable(regs); + if (!master_ctl) { + gen11_master_intr_enable(regs); return IRQ_NONE; - - /* Disable interrupts. */ - raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, 0); + } /* Find, clear, then process each source of interrupt. */ gen11_gt_irq_handler(i915, master_ctl); @@ -3165,8 +3181,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) gu_misc_iir = gen11_gu_misc_irq_ack(i915, master_ctl); - /* Enable interrupts. */ - raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ); + gen11_master_intr_enable(regs); gen11_gu_misc_irq_handler(i915, gu_misc_iir); @@ -3658,8 +3673,7 @@ static void gen11_irq_reset(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; int pipe; - I915_WRITE(GEN11_GFX_MSTR_IRQ, 0); - POSTING_READ(GEN11_GFX_MSTR_IRQ); + gen11_master_intr_disable(dev_priv->regs); gen11_gt_irq_reset(dev_priv); @@ -4323,8 +4337,7 @@ static int gen11_irq_postinstall(struct drm_device *dev) I915_WRITE(GEN11_DISPLAY_INT_CTL, GEN11_DISPLAY_IRQ_ENABLE); - I915_WRITE(GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ); - POSTING_READ(GEN11_GFX_MSTR_IRQ); + gen11_master_intr_enable(dev_priv->regs); return 0; } -- cgit v1.2.3 From 9a64c65083b910b3557b317dc56e1e93063ac350 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 6 Nov 2018 16:30:16 -0500 Subject: drm/i915: Add short HPD IRQ storm detection for non-MST systems MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unfortunately, it seems that the HPD IRQ storm problem from the early days of Intel GPUs was never entirely solved, only mostly. Within the last couple of days, I got a bug report from one of our customers who had been having issues with their machine suddenly booting up very slowly after having updated. The amount of time it took to boot went from around 30 seconds, to over 6 minutes consistently. After some investigation, I discovered that i915 was reporting massive amounts of short HPD IRQ spam on this system from the DisplayPort port, despite there not being anything actually connected. The symptoms would start with one "long" HPD IRQ being detected at boot: [ 1.891398] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00440000, dig 0x00440000, pins 0x000000a0 [ 1.891436] [drm:intel_hpd_irq_handler [i915]] digital hpd port B - long [ 1.891472] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on PIN 5 - cnt: 0 [ 1.891508] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - long [ 1.891544] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on PIN 7 - cnt: 0 [ 1.891592] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port B - long [ 1.891628] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port D - long … followed by constant short IRQs afterwards: [ 1.895091] [drm:intel_encoder_hotplug [i915]] [CONNECTOR:66:DP-1] status updated from unknown to disconnected [ 1.895129] [drm:i915_hotplug_work_func [i915]] Connector DP-3 (pin 7) received hotplug event. [ 1.895165] [drm:intel_dp_detect [i915]] [CONNECTOR:72:DP-3] [ 1.895275] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00200000, dig 0x00200000, pins 0x00000080 [ 1.895312] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short [ 1.895762] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00200000, dig 0x00200000, pins 0x00000080 [ 1.895799] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short [ 1.896239] [drm:intel_dp_aux_xfer [i915]] dp_aux_ch timeout status 0x71450085 [ 1.896293] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00200000, dig 0x00200000, pins 0x00000080 [ 1.896330] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short [ 1.896781] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00200000, dig 0x00200000, pins 0x00000080 [ 1.896817] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short [ 1.897275] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00200000, dig 0x00200000, pins 0x00000080 The customer's system in question has a GM45 GPU, which is apparently well known for hotplugging storms. So, workaround this impressively broken hardware by changing the default HPD storm threshold from 5 to 50. Then, make long IRQs count for 10, and short IRQs count for 1. This makes it so that 5 long IRQs will trigger an HPD storm, and on systems with short HPD storm detection 50 short IRQs will trigger an HPD storm. 50 short IRQs amounts to 100ms of constant pulsing, which seems like a good middleground between being too sensitive and not being sensitive enough (which would cause visible stutters in userspace every time a storm occurs). And just to be extra safe: we don't enable this by default on systems with MST support. There's too high of a chance of MST support triggering storm detection, and systems that are new enough to support MST are a lot less likely to have issues with IRQ storms anyway. As a note: this patch was tested using a ThinkPad T450s and a Chamelium to simulate the short IRQ storms. Changes since v1: - Don't use two separate thresholds, just make long IRQs count for 10 each and short IRQs count for 1. This simplifies the code a bit - Ville Syrjälä Changes since v2: - Document @long_hpd in intel_hpd_irq_storm_detect, no functional changes Changes since v4: - Remove !! in long_hpd assignment - Ville Syrjälä - queue_hp = true - Ville Syrjälä Signed-off-by: Lyude Paul Cc: Ville Syrjälä Cc: Rodrigo Vivi Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181106213017.14563-6-lyude@redhat.com --- drivers/gpu/drm/i915/i915_irq.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/gpu/drm/i915/i915_irq.c') diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 5d1f53723388..d7e47d6082de 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -4863,6 +4863,13 @@ void intel_irq_init(struct drm_i915_private *dev_priv) dev_priv->display_irqs_enabled = false; dev_priv->hotplug.hpd_storm_threshold = HPD_STORM_DEFAULT_THRESHOLD; + /* If we have MST support, we want to avoid doing short HPD IRQ storm + * detection, as short HPD storms will occur as a natural part of + * sideband messaging with MST. + * On older platforms however, IRQ storms can occur with both long and + * short pulses, as seen on some G4x systems. + */ + dev_priv->hotplug.hpd_short_storm_enabled = !HAS_DP_MST(dev_priv); dev->driver->get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos; dev->driver->get_scanout_position = i915_get_crtc_scanoutpos; -- cgit v1.2.3 From 62819dfd859e9d9960bd2ac86e0a9e356b66d03e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Roberto=20de=20Souza?= Date: Tue, 6 Nov 2018 11:08:42 -0800 Subject: drm/i915/icl: Reset PSR interruptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All other interruptions gen11 interruptions are reset in gen11_irq_reset() also it is done for other gens that supports PSR. Cc: Dhinakaran Pandiyan Cc: Paulo Zanoni Reviewed-by: Ville Syrjälä Signed-off-by: José Roberto de Souza Link: https://patchwork.freedesktop.org/patch/msgid/20181106190843.18009-3-jose.souza@intel.com --- drivers/gpu/drm/i915/i915_irq.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/gpu/drm/i915/i915_irq.c') diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d7e47d6082de..d447d7d508f4 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3679,6 +3679,9 @@ static void gen11_irq_reset(struct drm_device *dev) I915_WRITE(GEN11_DISPLAY_INT_CTL, 0); + I915_WRITE(EDP_PSR_IMR, 0xffffffff); + I915_WRITE(EDP_PSR_IIR, 0xffffffff); + for_each_pipe(dev_priv, pipe) if (intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PIPE(pipe))) -- cgit v1.2.3