| Message ID | 20190702144947.13127-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: synchronize_irq() against the actual irq | expand |
Quoting Ville Syrjala (2019-07-02 15:49:47) > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > When eliminating our use of drm_irq_install() I failed to convert > all our synchronize_irq() calls to consult pdev->irq instead of > dev_priv->drm.irq. As we no longer populate dev_priv->drm.irq > we're no longer synchronizing against anything. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Reported-by: Imre Deak <imre.deak@intel.com> > Fixes: b318b82455bd ("drm/i915: Nuke drm_driver irq vfuncs") > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111012 > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Oops. Lots of duplication there, I don't think an static inline void intel_synchronize_irq(struct drm_i915_private *i915) { synchronize_irq(i915->drm.pdev->irq); } (intel_ or i915_ depending on taste) would go amiss. Sadly kernel/irq/irqdesc.c doesn't report a bogus irq number or else we could have marked the drm.irq as bad. Kudos to Imre for figuring out the link as that bug report had been worrying me, and never once did I suspect it was the irq serialisation. All callsites converted, Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Tue, Jul 02, 2019 at 03:58:46PM +0100, Chris Wilson wrote: > Quoting Ville Syrjala (2019-07-02 15:49:47) > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > When eliminating our use of drm_irq_install() I failed to convert > > all our synchronize_irq() calls to consult pdev->irq instead of > > dev_priv->drm.irq. As we no longer populate dev_priv->drm.irq > > we're no longer synchronizing against anything. > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Reported-by: Imre Deak <imre.deak@intel.com> > > Fixes: b318b82455bd ("drm/i915: Nuke drm_driver irq vfuncs") > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111012 > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Oops. > > Lots of duplication there, I don't think an > > static inline void intel_synchronize_irq(struct drm_i915_private *i915) > { > synchronize_irq(i915->drm.pdev->irq); > } > > (intel_ or i915_ depending on taste) > > would go amiss. Sadly kernel/irq/irqdesc.c doesn't report a bogus irq > number or else we could have marked the drm.irq as bad. > > Kudos to Imre for figuring out the link as that bug report had been > worrying me, and never once did I suspect it was the irq serialisation. The wakeref count tracking gave the clue and then what the common thing on the path could be for HSW..ICL (probably not irq_reset()!) :) > > All callsites converted, > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > -Chris
Quoting Patchwork (2019-07-02 22:28:10) > #### Possible fixes #### > > * igt@i915_pm_rpm@module-reload: > - fi-kbl-r: [DMESG-WARN][9] ([fdo#111012]) -> [PASS][10] > [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/fi-kbl-r/igt@i915_pm_rpm@module-reload.html > [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13494/fi-kbl-r/igt@i915_pm_rpm@module-reload.html > - fi-hsw-peppy: [DMESG-WARN][11] ([fdo#111012]) -> [PASS][12] > [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/fi-hsw-peppy/igt@i915_pm_rpm@module-reload.html > [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13494/fi-hsw-peppy/igt@i915_pm_rpm@module-reload.html Pushed for the BAT cleanup. -Chris
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index 86a38116dc3a..118b0808f77a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -1158,7 +1158,7 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv) spin_unlock_irq(&dev_priv->irq_lock); /* make sure we're done processing display irqs */ - synchronize_irq(dev_priv->drm.irq); + synchronize_irq(dev_priv->drm.pdev->irq); intel_power_sequencer_reset(dev_priv); diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c index 1e2c4307d05a..aa975834f4dc 100644 --- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c +++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c @@ -667,5 +667,5 @@ void intel_crtc_disable_pipe_crc(struct intel_crtc *intel_crtc) I915_WRITE(PIPE_CRC_CTL(crtc->index), 0); POSTING_READ(PIPE_CRC_CTL(crtc->index)); - synchronize_irq(dev_priv->drm.irq); + synchronize_irq(dev_priv->drm.pdev->irq); } diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index d1508f0b4c84..c1fb5fa3952e 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -1162,7 +1162,7 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine) if (execlists_active(&engine->execlists)) { struct tasklet_struct *t = &engine->execlists.tasklet; - synchronize_hardirq(engine->i915->drm.irq); + synchronize_hardirq(engine->i915->drm.pdev->irq); local_bh_disable(); if (tasklet_trylock(t)) { diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index eeecdad0e3ca..781d7dcaa1bf 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -4081,7 +4081,7 @@ static int i915_hpd_storm_ctl_show(struct seq_file *m, void *data) /* Synchronize with everything first in case there's been an HPD * storm, but we haven't finished handling it in the kernel yet */ - synchronize_irq(dev_priv->drm.irq); + synchronize_irq(dev_priv->drm.pdev->irq); flush_work(&dev_priv->hotplug.dig_port_work); flush_work(&dev_priv->hotplug.hotplug_work); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 73f0338faf9f..0230ef43fb12 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -580,7 +580,7 @@ void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv) gen6_disable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS); spin_unlock_irq(&dev_priv->irq_lock); - synchronize_irq(dev_priv->drm.irq); + synchronize_irq(dev_priv->drm.pdev->irq); /* Now that we will not be generating any more work, flush any * outstanding tasks. As we are called on the RPS idle path, @@ -627,7 +627,7 @@ void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv) gen6_disable_pm_irq(dev_priv, dev_priv->pm_guc_events); spin_unlock_irq(&dev_priv->irq_lock); - synchronize_irq(dev_priv->drm.irq); + synchronize_irq(dev_priv->drm.pdev->irq); gen9_reset_guc_interrupts(dev_priv); } @@ -663,7 +663,7 @@ void gen11_disable_guc_interrupts(struct drm_i915_private *dev_priv) I915_WRITE(GEN11_GUC_SG_INTR_ENABLE, 0); spin_unlock_irq(&dev_priv->irq_lock); - synchronize_irq(dev_priv->drm.irq); + synchronize_irq(dev_priv->drm.pdev->irq); gen11_reset_guc_interrupts(dev_priv); } @@ -3680,7 +3680,7 @@ void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv, spin_unlock_irq(&dev_priv->irq_lock); /* make sure we're done processing display irqs */ - synchronize_irq(dev_priv->drm.irq); + synchronize_irq(dev_priv->drm.pdev->irq); } static void cherryview_irq_reset(struct drm_i915_private *dev_priv) @@ -4970,7 +4970,7 @@ void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv) { intel_irq_reset(dev_priv); dev_priv->runtime_pm.irqs_enabled = false; - synchronize_irq(dev_priv->drm.irq); + synchronize_irq(dev_priv->drm.pdev->irq); } /** diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index e3b83ecb90b5..bc3ae7db170b 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -615,7 +615,7 @@ void intel_guc_log_relay_close(struct intel_guc_log *log) struct drm_i915_private *i915 = guc_to_i915(guc); guc_log_disable_flush_events(log); - synchronize_irq(i915->drm.irq); + synchronize_irq(i915->drm.pdev->irq); flush_work(&log->relay.flush_work);