| Message ID | 1409009095-3621-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 26.08.2014 01:24, Jesse Barnes wrote: > This happens in irq_postinstall before we've set the pm._irqs_disabled flag, > but shouldn't warn. So add a nowarn variant to allow this to happen w/o > a backtrace and keep the rest of the IRQ tracking code happy. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> Reported-by: Oliver Hartkopp <socketcan@hartkopp.net> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net> Yeah. That fixed it. Thanks Jesse! [ 2.900147] Linux agpgart interface v0.103 [ 2.900475] agpgart-intel 0000:00:00.0: Intel HD Graphics Chipset [ 2.900579] agpgart-intel 0000:00:00.0: detected gtt size: 2097152K total, 262144K mappable [ 2.901354] agpgart-intel 0000:00:00.0: detected 32768K stolen memory [ 2.901743] agpgart-intel 0000:00:00.0: AGP aperture is 256M @ 0xe0000000 [ 2.901995] [drm] Initialized drm 1.1.0 20060810 [ 2.904212] [drm] Memory usable by graphics device = 2048M [ 2.904295] [drm] Replacing VGA console driver [ 2.904961] Console: switching to colour dummy device 80x25 [ 2.937950] i915 0000:00:02.0: irq 24 for MSI/MSI-X [ 2.937962] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [ 2.937965] [drm] Driver supports precise vblank timestamp query. [ 2.938310] vgaarb: device changed decodes: PCI:0000:00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem [ 3.030687] fbcon: inteldrmfb (fb0) is primary device [ 3.494467] tsc: Refined TSC clocksource calibration: 2792.999 MHz [ 4.176644] [drm:intel_dp_start_link_train] *ERROR* too many full retries, give up [ 4.495814] Switched to clocksource tsc [ 4.529922] Console: switching to colour frame buffer device 240x67 [ 4.536702] i915 0000:00:02.0: fb0: inteldrmfb frame buffer device [ 4.536769] i915 0000:00:02.0: registered panic notifier [ 4.548333] ACPI: Video Device [VID2] (multi-head: yes rom: no post: no) [ 4.900233] acpi device:45: registered as cooling_device0 [ 4.924178] input: Video Bus as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/LNXVIDEO:02/input/input4 [ 4.924371] [drm] Initialized i915 1.6.0 20140725 for 0000:00:02.0 on minor 0 Btw. do you know what [drm:intel_dp_start_link_train] *ERROR* too many full retries, give up is about? Regards, Oliver > --- > drivers/gpu/drm/i915/i915_irq.c | 18 ++++++++++++------ > 1 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index d5445e7..ec1d9fe 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -132,6 +132,16 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */ > > /* For display hotplug interrupt */ > static void > +ironlake_enable_display_irq_nowarn(struct drm_i915_private *dev_priv, u32 mask) > +{ > + if ((dev_priv->irq_mask & mask) != 0) { > + dev_priv->irq_mask &= ~mask; > + I915_WRITE(DEIMR, dev_priv->irq_mask); > + POSTING_READ(DEIMR); > + } > +} > + > +static void > ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask) > { > assert_spin_locked(&dev_priv->irq_lock); > @@ -139,11 +149,7 @@ ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask) > if (WARN_ON(!intel_irqs_enabled(dev_priv))) > return; > > - if ((dev_priv->irq_mask & mask) != 0) { > - dev_priv->irq_mask &= ~mask; > - I915_WRITE(DEIMR, dev_priv->irq_mask); > - POSTING_READ(DEIMR); > - } > + ironlake_enable_display_irq_nowarn(dev_priv, mask); > } > > static void > @@ -3665,7 +3671,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev) > * setup is guaranteed to run in single-threaded context. But we > * need it to make the assert_spin_locked happy. */ > spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > - ironlake_enable_display_irq(dev_priv, DE_PCU_EVENT); > + ironlake_enable_display_irq_nowarn(dev_priv, DE_PCU_EVENT); > spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > } > >
On Mon, Aug 25, 2014 at 04:24:55PM -0700, Jesse Barnes wrote: > This happens in irq_postinstall before we've set the pm._irqs_disabled flag, > but shouldn't warn. So add a nowarn variant to allow this to happen w/o > a backtrace and keep the rest of the IRQ tracking code happy. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> Shouldn't we instead just move the pm._irqs_disabled = false in i915_dma.c right above the drm_irq_install call? In intel_runtime_pm_restore_interrupts we also set it to false before we call the various hooks. Also the commit message is a bit thin on the usual details like which commits introduced this regression, so that maintainers know where to apply this to. -Daniel > --- > drivers/gpu/drm/i915/i915_irq.c | 18 ++++++++++++------ > 1 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index d5445e7..ec1d9fe 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -132,6 +132,16 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */ > > /* For display hotplug interrupt */ > static void > +ironlake_enable_display_irq_nowarn(struct drm_i915_private *dev_priv, u32 mask) > +{ > + if ((dev_priv->irq_mask & mask) != 0) { > + dev_priv->irq_mask &= ~mask; > + I915_WRITE(DEIMR, dev_priv->irq_mask); > + POSTING_READ(DEIMR); > + } > +} > + > +static void > ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask) > { > assert_spin_locked(&dev_priv->irq_lock); > @@ -139,11 +149,7 @@ ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask) > if (WARN_ON(!intel_irqs_enabled(dev_priv))) > return; > > - if ((dev_priv->irq_mask & mask) != 0) { > - dev_priv->irq_mask &= ~mask; > - I915_WRITE(DEIMR, dev_priv->irq_mask); > - POSTING_READ(DEIMR); > - } > + ironlake_enable_display_irq_nowarn(dev_priv, mask); > } > > static void > @@ -3665,7 +3671,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev) > * setup is guaranteed to run in single-threaded context. But we > * need it to make the assert_spin_locked happy. */ > spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > - ironlake_enable_display_irq(dev_priv, DE_PCU_EVENT); > + ironlake_enable_display_irq_nowarn(dev_priv, DE_PCU_EVENT); > spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > } > > -- > 1.7.5.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 26 Aug 2014 09:23:54 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Aug 25, 2014 at 04:24:55PM -0700, Jesse Barnes wrote: > > This happens in irq_postinstall before we've set the pm._irqs_disabled flag, > > but shouldn't warn. So add a nowarn variant to allow this to happen w/o > > a backtrace and keep the rest of the IRQ tracking code happy. > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > Shouldn't we instead just move the pm._irqs_disabled = false in i915_dma.c > right above the drm_irq_install call? In > intel_runtime_pm_restore_interrupts we also set it to false before we call > the various hooks. I didn't check on all the paths whether that was safe, we have a lot of warnings. And really this init time thing is a special case, so it made sense to me. > Also the commit message is a bit thin on the usual details like which > commits introduced this regression, so that maintainers know where to > apply this to. I don't have the commit... Oliver do you have it handy?
On 26.08.2014 20:52, Jesse Barnes wrote: > On Tue, 26 Aug 2014 09:23:54 +0200 > Daniel Vetter <daniel@ffwll.ch> wrote: >>> This happens in irq_postinstall before we've set the pm._irqs_disabled flag, >>> but shouldn't warn. So add a nowarn variant to allow this to happen w/o >>> a backtrace and keep the rest of the IRQ tracking code happy. >>> >> Also the commit message is a bit thin on the usual details like which >> commits introduced this regression, so that maintainers know where to >> apply this to. > > I don't have the commit... Oliver do you have it handy? > Hm - I really can not tell what has been done to introduce this regression. I just saw the warning on my machine after upgrading to 3.17 ... You can ask me about linux/net/can/ but not the drm stuff ;-) Cheers, Oliver
On Tue, 26 Aug 2014 21:03:11 +0200 Oliver Hartkopp <socketcan@hartkopp.net> wrote: > On 26.08.2014 20:52, Jesse Barnes wrote: > > On Tue, 26 Aug 2014 09:23:54 +0200 > > Daniel Vetter <daniel@ffwll.ch> wrote: > > >>> This happens in irq_postinstall before we've set the pm._irqs_disabled flag, > >>> but shouldn't warn. So add a nowarn variant to allow this to happen w/o > >>> a backtrace and keep the rest of the IRQ tracking code happy. > >>> > > >> Also the commit message is a bit thin on the usual details like which > >> commits introduced this regression, so that maintainers know where to > >> apply this to. > > > > I don't have the commit... Oliver do you have it handy? > > > > Hm - I really can not tell what has been done to introduce this regression. > I just saw the warning on my machine after upgrading to 3.17 ... > > You can ask me about linux/net/can/ but not the drm stuff ;-) I think it was this one Daniel, or the combination of patches around it: commit 95f25beddba2ec9510b249740bacc11eca70cf75 Author: Jesse Barnes <jbarnes@virtuousgeek.org> Date: Fri Jun 20 09:29:22 2014 -0700 drm/i915: set pm._irqs_disabled at IRQ init time
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d5445e7..ec1d9fe 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -132,6 +132,16 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */ /* For display hotplug interrupt */ static void +ironlake_enable_display_irq_nowarn(struct drm_i915_private *dev_priv, u32 mask) +{ + if ((dev_priv->irq_mask & mask) != 0) { + dev_priv->irq_mask &= ~mask; + I915_WRITE(DEIMR, dev_priv->irq_mask); + POSTING_READ(DEIMR); + } +} + +static void ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask) { assert_spin_locked(&dev_priv->irq_lock); @@ -139,11 +149,7 @@ ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask) if (WARN_ON(!intel_irqs_enabled(dev_priv))) return; - if ((dev_priv->irq_mask & mask) != 0) { - dev_priv->irq_mask &= ~mask; - I915_WRITE(DEIMR, dev_priv->irq_mask); - POSTING_READ(DEIMR); - } + ironlake_enable_display_irq_nowarn(dev_priv, mask); } static void @@ -3665,7 +3671,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev) * setup is guaranteed to run in single-threaded context. But we * need it to make the assert_spin_locked happy. */ spin_lock_irqsave(&dev_priv->irq_lock, irqflags); - ironlake_enable_display_irq(dev_priv, DE_PCU_EVENT); + ironlake_enable_display_irq_nowarn(dev_priv, DE_PCU_EVENT); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); }
This happens in irq_postinstall before we've set the pm._irqs_disabled flag, but shouldn't warn. So add a nowarn variant to allow this to happen w/o a backtrace and keep the rest of the IRQ tracking code happy. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/i915_irq.c | 18 ++++++++++++------ 1 files changed, 12 insertions(+), 6 deletions(-)