Acked-by: Lyude <lyude@xxxxxxxxxx>
On Mon, 2017-03-13 at 17:02 +0000, Chris Wilson wrote:
> In order to prevent accessing the hpd registers outside of the
> display
> power wells, we should refrain from writing to the registers before
> the
> display interrupts are enabled.
>
> [ 4.740136] WARNING: CPU: 1 PID: 221 at
> drivers/gpu/drm/i915/intel_uncore.c:795
> __unclaimed_reg_debug+0x44/0x50 [i915]
> [ 4.740155] Unclaimed read from register 0x1e1110
> [ 4.740168] Modules linked in: i915(+) intel_gtt drm_kms_helper
> prime_numbers
> [ 4.740190] CPU: 1 PID: 221 Comm: systemd-udevd Not tainted
> 4.10.0-rc6+ #384
> [ 4.740203] Hardware name: / , BIOS
> PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> [ 4.740220] Call Trace:
> [ 4.740236] dump_stack+0x4d/0x6f
> [ 4.740251] __warn+0xc1/0xe0
> [ 4.740265] warn_slowpath_fmt+0x4a/0x50
> [ 4.740281] ? insert_work+0x77/0xc0
> [ 4.740355] ? fwtable_write32+0x90/0x130 [i915]
> [ 4.740431] __unclaimed_reg_debug+0x44/0x50 [i915]
> [ 4.740507] fwtable_read32+0xd8/0x130 [i915]
> [ 4.740575] i915_hpd_irq_setup+0xa5/0x100 [i915]
> [ 4.740649] intel_hpd_init+0x68/0x80 [i915]
> [ 4.740716] i915_driver_load+0xe19/0x1380 [i915]
> [ 4.740784] i915_pci_probe+0x32/0x90 [i915]
> [ 4.740799] pci_device_probe+0x8b/0xf0
> [ 4.740815] driver_probe_device+0x2b6/0x450
> [ 4.740828] __driver_attach+0xda/0xe0
> [ 4.740841] ? driver_probe_device+0x450/0x450
> [ 4.740853] bus_for_each_dev+0x5b/0x90
> [ 4.740865] driver_attach+0x19/0x20
> [ 4.740878] bus_add_driver+0x166/0x260
> [ 4.740892] driver_register+0x5b/0xd0
> [ 4.740906] ? 0xffffffffa0166000
> [ 4.740920] __pci_register_driver+0x47/0x50
> [ 4.740985] i915_init+0x5c/0x5e [i915]
> [ 4.740999] do_one_initcall+0x3e/0x160
> [ 4.741015] ? __vunmap+0x7c/0xc0
> [ 4.741029] ? kmem_cache_alloc+0xcf/0x120
> [ 4.741045] do_init_module+0x55/0x1c4
> [ 4.741060] load_module+0x1f3f/0x25b0
> [ 4.741073] ? __symbol_put+0x40/0x40
> [ 4.741086] ? kernel_read_file+0x100/0x190
> [ 4.741100] SYSC_finit_module+0xbc/0xf0
> [ 4.741112] SyS_finit_module+0x9/0x10
> [ 4.741125] entry_SYSCALL_64_fastpath+0x17/0x98
> [ 4.741135] RIP: 0033:0x7f8559a140f9
> [ 4.741145] RSP: 002b:00007fff7509a3e8 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000139
> [ 4.741161] RAX: ffffffffffffffda RBX: 00007f855aba02d1 RCX:
> 00007f8559a140f9
> [ 4.741172] RDX: 0000000000000000 RSI: 000055b6db0914f0 RDI:
> 0000000000000011
> [ 4.741183] RBP: 0000000000020000 R08: 0000000000000000 R09:
> 000000000000000e
> [ 4.741193] R10: 0000000000000011 R11: 0000000000000246 R12:
> 000055b6db0854d0
> [ 4.741204] R13: 000055b6db091150 R14: 0000000000000000 R15:
> 000055b6db035924
>
> v2: Set dev_priv->display_irqs_enabled to true for all platforms
> other
> than vlv/chv that manually control the display power domain.
>
> Cherry-pick: 262fd485ac6b ("drm/i915: Only enable hotplug interrupts
> if the display interrupts are enabled")
> Fixes: 19625e85c6ec ("drm/i915: Enable polling when we don't have
> hpd")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97798
> Suggested-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Lyude <cpaul@xxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Hans de Goede <jwrdegoede@xxxxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Link: http://patchwork.freedesktop.org/patch/msgid/20170215131547.506
> 4-1-chris@xxxxxxxxxxxxxxxxxx
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++
> drivers/gpu/drm/i915/intel_hotplug.c | 14 ++++++++------
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index e6ffef2f707a..4fc8973744b4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4266,6 +4266,16 @@ void intel_irq_init(struct drm_i915_private
> *dev_priv)
> if (!IS_GEN2(dev_priv))
> dev->vblank_disable_immediate = true;
>
> + /* Most platforms treat the display irq block as an always-
> on
> + * power domain. vlv/chv can disable it at runtime and need
> + * special care to avoid writing any of the display block
> registers
> + * outside of the power domain. We defer setting up the
> display irqs
> + * in this case to the runtime pm.
> + */
> + dev_priv->display_irqs_enabled = true;
> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> + dev_priv->display_irqs_enabled = false;
> +
> dev->driver->get_vblank_timestamp =
> i915_get_vblank_timestamp;
> dev->driver->get_scanout_position =
> i915_get_crtc_scanoutpos;
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index b62e3f8ad415..54208bef7a83 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -219,7 +219,7 @@ static void
> intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> }
> }
> }
> - if (dev_priv->display.hpd_irq_setup)
> + if (dev_priv->display_irqs_enabled && dev_priv-
> >display.hpd_irq_setup)
> dev_priv->display.hpd_irq_setup(dev_priv);
> spin_unlock_irq(&dev_priv->irq_lock);
>
> @@ -425,7 +425,7 @@ void intel_hpd_irq_handler(struct
> drm_i915_private *dev_priv,
> }
> }
>
> - if (storm_detected)
> + if (storm_detected && dev_priv->display_irqs_enabled)
> dev_priv->display.hpd_irq_setup(dev_priv);
> spin_unlock(&dev_priv->irq_lock);
>
> @@ -471,10 +471,12 @@ void intel_hpd_init(struct drm_i915_private
> *dev_priv)
> * Interrupt setup is already guaranteed to be single-
> threaded, this is
> * just to make the assert_spin_locked checks happy.
> */
> - spin_lock_irq(&dev_priv->irq_lock);
> - if (dev_priv->display.hpd_irq_setup)
> - dev_priv->display.hpd_irq_setup(dev_priv);
> - spin_unlock_irq(&dev_priv->irq_lock);
> + if (dev_priv->display_irqs_enabled && dev_priv-
> >display.hpd_irq_setup) {
> + spin_lock_irq(&dev_priv->irq_lock);
> + if (dev_priv->display_irqs_enabled)
> + dev_priv->display.hpd_irq_setup(dev_priv);
> + spin_unlock_irq(&dev_priv->irq_lock);
> + }
> }
>
> static void i915_hpd_poll_init_work(struct work_struct *work)
--
Cheers,
Lyude