Re: [Intel-gfx] [PATCH v2] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled

2017-02-16 Thread Chris Wilson 
On Wed, Feb 15, 2017 at 04:12:04PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 15, 2017 at 01:15:47PM +, 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]  ? 0xa0166000
> > [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:7fff7509a3e8 EFLAGS: 0246 ORIG_RAX: 
> > 0139
> > [4.741161] RAX: ffda RBX: 7f855aba02d1 RCX: 
> > 7f8559a140f9
> > [4.741172] RDX:  RSI: 55b6db0914f0 RDI: 
> > 0011
> > [4.741183] RBP: 0002 R08:  R09: 
> > 000e
> > [4.741193] R10: 0011 R11: 0246 R12: 
> > 55b6db0854d0
> > [4.741204] R13: 55b6db091150 R14:  R15: 
> > 55b6db035924
> > 
> > v2: Set dev_priv->display_irqs_enabled to true for all platforms other
> > than vlv/chv that manually control the display power domain.
> > 
> > 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ä 
> > Signed-off-by: Chris Wilson 
> > Cc: Lyude 
> > Cc: Daniel Vetter 
> > Cc: Ville Syrjälä 
> > Cc: Hans de Goede 
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Chris Wilson 
> 
> lgtm
> 
> Reviewed-by: Ville Syrjälä 

Thanks for the idea and fixes,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled

2017-02-15 Thread Ville Syrjälä 
On Wed, Feb 15, 2017 at 01:15:47PM +, 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]  ? 0xa0166000
> [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:7fff7509a3e8 EFLAGS: 0246 ORIG_RAX: 
> 0139
> [4.741161] RAX: ffda RBX: 7f855aba02d1 RCX: 
> 7f8559a140f9
> [4.741172] RDX:  RSI: 55b6db0914f0 RDI: 
> 0011
> [4.741183] RBP: 0002 R08:  R09: 
> 000e
> [4.741193] R10: 0011 R11: 0246 R12: 
> 55b6db0854d0
> [4.741204] R13: 55b6db091150 R14:  R15: 
> 55b6db035924
> 
> v2: Set dev_priv->display_irqs_enabled to true for all platforms other
> than vlv/chv that manually control the display power domain.
> 
> 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ä 
> Signed-off-by: Chris Wilson 
> Cc: Lyude 
> Cc: Daniel Vetter 
> Cc: Ville Syrjälä 
> Cc: Hans de Goede 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Chris Wilson 

lgtm

Reviewed-by: Ville Syrjälä 

> ---
>  drivers/gpu/drm/i915/i915_irq.c  | 10 ++
>  drivers/gpu/drm/i915/intel_hotplug.c | 11 ++-
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a887aeff4c73..91be31617e78 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4284,6 +4284,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_priv->hotplug.hpd_storm_threshold = HPD_STORM_DEFAULT_THRESHOLD;
>  
>   dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp;
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c 
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index 6a9c16508ab5..bad4f14858e3 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -224,7 +224,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_pri

[Intel-gfx] [PATCH v2] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled

2017-02-15 Thread Chris Wilson 
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]  ? 0xa0166000
[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:7fff7509a3e8 EFLAGS: 0246 ORIG_RAX: 
0139
[4.741161] RAX: ffda RBX: 7f855aba02d1 RCX: 7f8559a140f9
[4.741172] RDX:  RSI: 55b6db0914f0 RDI: 0011
[4.741183] RBP: 0002 R08:  R09: 000e
[4.741193] R10: 0011 R11: 0246 R12: 55b6db0854d0
[4.741204] R13: 55b6db091150 R14:  R15: 55b6db035924

v2: Set dev_priv->display_irqs_enabled to true for all platforms other
than vlv/chv that manually control the display power domain.

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ä 
Signed-off-by: Chris Wilson 
Cc: Lyude 
Cc: Daniel Vetter 
Cc: Ville Syrjälä 
Cc: Hans de Goede 
Cc: sta...@vger.kernel.org
Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_irq.c  | 10 ++
 drivers/gpu/drm/i915/intel_hotplug.c | 11 ++-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a887aeff4c73..91be31617e78 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4284,6 +4284,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_priv->hotplug.hpd_storm_threshold = HPD_STORM_DEFAULT_THRESHOLD;
 
dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp;
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c 
b/drivers/gpu/drm/i915/intel_hotplug.c
index 6a9c16508ab5..bad4f14858e3 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -224,7 +224,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);
 
@@ -430,7 +430,7 @@ void intel_hpd_irq_handler(struct drm_i915_private 
*dev_priv,
}
}
 
-   if (storm_detected)
+   if (storm