All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
@ 2017-02-15  9:06 ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-02-15  9:06 UTC (permalink / raw)
  To: intel-gfx
  Cc: Chris Wilson, Lyude, Daniel Vetter, Ville Syrjälä,
	Hans de Goede, stable

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

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@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lyude <cpaul@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_hotplug.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

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_detected && dev_priv->display_irqs_enabled)
 		dev_priv->display.hpd_irq_setup(dev_priv);
 	spin_unlock(&dev_priv->irq_lock);
 
@@ -476,10 +476,11 @@ 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)
+	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup) {
+		spin_lock_irq(&dev_priv->irq_lock);
 		dev_priv->display.hpd_irq_setup(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
+		spin_unlock_irq(&dev_priv->irq_lock);
+	}
 }
 
 static void i915_hpd_poll_init_work(struct work_struct *work)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
@ 2017-02-15  9:06 ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-02-15  9:06 UTC (permalink / raw)
  To: intel-gfx; +Cc: Hans de Goede, Daniel Vetter, stable

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

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@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lyude <cpaul@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_hotplug.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

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_detected && dev_priv->display_irqs_enabled)
 		dev_priv->display.hpd_irq_setup(dev_priv);
 	spin_unlock(&dev_priv->irq_lock);
 
@@ -476,10 +476,11 @@ 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)
+	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup) {
+		spin_lock_irq(&dev_priv->irq_lock);
 		dev_priv->display.hpd_irq_setup(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
+		spin_unlock_irq(&dev_priv->irq_lock);
+	}
 }
 
 static void i915_hpd_poll_init_work(struct work_struct *work)
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
  2017-02-15  9:06 ` Chris Wilson
@ 2017-02-15 11:23   ` Ville Syrjälä
  -1 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-02-15 11:23 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, Lyude, Daniel Vetter, Hans de Goede, stable

On Wed, Feb 15, 2017 at 09:06:53AM +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
> 
> 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@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lyude <cpaul@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/intel_hotplug.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> 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)

I don't think we're setting display_irqs_enabled on any other platform
than VLV/CHV. So either we have to push this check down into
hpd_irq_setup() or just set display_irqs_enable=true somewhere for all
the other platforms.

>  		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_detected && dev_priv->display_irqs_enabled)
>  		dev_priv->display.hpd_irq_setup(dev_priv);
>  	spin_unlock(&dev_priv->irq_lock);
>  
> @@ -476,10 +476,11 @@ 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)
> +	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup) {
> +		spin_lock_irq(&dev_priv->irq_lock);
>  		dev_priv->display.hpd_irq_setup(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> +		spin_unlock_irq(&dev_priv->irq_lock);
> +	}
>  }
>  
>  static void i915_hpd_poll_init_work(struct work_struct *work)
> -- 
> 2.11.0

-- 
Ville Syrj�l�
Intel OTC

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
@ 2017-02-15 11:23   ` Ville Syrjälä
  0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-02-15 11:23 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Hans de Goede, Daniel Vetter, intel-gfx, stable

On Wed, Feb 15, 2017 at 09:06:53AM +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
> 
> 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@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lyude <cpaul@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/intel_hotplug.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> 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)

I don't think we're setting display_irqs_enabled on any other platform
than VLV/CHV. So either we have to push this check down into
hpd_irq_setup() or just set display_irqs_enable=true somewhere for all
the other platforms.

>  		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_detected && dev_priv->display_irqs_enabled)
>  		dev_priv->display.hpd_irq_setup(dev_priv);
>  	spin_unlock(&dev_priv->irq_lock);
>  
> @@ -476,10 +476,11 @@ 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)
> +	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup) {
> +		spin_lock_irq(&dev_priv->irq_lock);
>  		dev_priv->display.hpd_irq_setup(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> +		spin_unlock_irq(&dev_priv->irq_lock);
> +	}
>  }
>  
>  static void i915_hpd_poll_init_work(struct work_struct *work)
> -- 
> 2.11.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
  2017-02-15  9:06 ` Chris Wilson
  (?)
  (?)
@ 2017-02-15 11:52 ` Patchwork
  -1 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-02-15 11:52 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
URL   : https://patchwork.freedesktop.org/series/19687/
State : success

== Summary ==

Series 19687v1 drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
https://patchwork.freedesktop.org/api/1.0/series/19687/revisions/1/mbox/

fi-bdw-5557u     total:252  pass:241  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:252  pass:213  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:252  pass:225  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:252  pass:221  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:252  pass:236  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:252  pass:236  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:252  pass:202  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:252  pass:242  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:252  pass:235  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:252  pass:230  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:252  pass:242  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:252  pass:224  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:252  pass:223  dwarn:0   dfail:0   fail:0   skip:29 

78640dff39ba539190c519d5b210c9e28fb886bb drm-tip: 2017y-02m-15d-10h-08m-38s UTC integration manifest
9361547 drm/i915: Only enable hotplug interrupts if the display interrupts are enabled

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3818/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
  2017-02-15 11:23   ` Ville Syrjälä
@ 2017-02-15 12:04     ` Chris Wilson
  -1 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-02-15 12:04 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, Lyude, Daniel Vetter, Hans de Goede, stable

On Wed, Feb 15, 2017 at 01:23:35PM +0200, Ville Syrj�l� wrote:
> On Wed, Feb 15, 2017 at 09:06:53AM +0000, Chris Wilson wrote:
> > 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)
> 
> I don't think we're setting display_irqs_enabled on any other platform
> than VLV/CHV. So either we have to push this check down into
> hpd_irq_setup() or just set display_irqs_enable=true somewhere for all
> the other platforms.

Yup, only vlv/chv set display_irqs_enable. Would it not be safe to

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 99e2d659202c..7e205747f18d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4314,6 +4314,8 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
        if (!IS_GEN2(dev_priv))
                dev->vblank_disable_immediate = true;
 
+       dev_priv->display_irqs_enabled = true;
+
        dev_priv->hotplug.hpd_storm_threshold = HPD_STORM_DEFAULT_THRESHOLD;

which may then cause us to enable the display irq mask before the
powerwell is powered, but as soon as we go through a display powerwell
cycle will be back in sync? That presumes we complete such a cycle
before hotplug init touches the register - and that we can touch the irq
registers outside of the powerwell.

Or we can do something a bit more complete:

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c1b400f1ede4..681196987450 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1381,6 +1381,12 @@ struct i915_power_well;
 
 struct i915_power_well_ops {
 	/*
+	 * Initialize drm_i915_private or struct i915_power_well state,
+	 * called on driver load.
+	 */
+	void (*init)(struct drm_i915_private *dev_priv,
+		     struct i915_power_well *power_well);
+	/*
 	 * Synchronize the well's hw state to match the current sw state, for
 	 * example enable/disable it based on the current refcount. Called
 	 * during driver init and resume time, possibly after first calling
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 879567987201..4cb2b099f6c1 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1126,7 +1126,7 @@ static void vlv_init_display_clock_gating(struct drm_i915_private *dev_priv)
 		   DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 1000));
 }
 
-static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
+static void vlv_display_power_well_install(struct drm_i915_private *dev_priv)
 {
 	struct intel_encoder *encoder;
 	enum pipe pipe;
@@ -1175,7 +1175,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
 	intel_pps_unlock_regs_wa(dev_priv);
 }
 
-static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
+static void vlv_display_power_well_uninstall(struct drm_i915_private *dev_priv)
 {
 	spin_lock_irq(&dev_priv->irq_lock);
 	valleyview_disable_display_irqs(dev_priv);
@@ -1191,6 +1191,12 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
 		intel_hpd_poll_init(dev_priv);
 }
 
+static void vlv_display_power_well_init(struct drm_i915_private *dev_priv,
+					struct i915_power_well *power_well)
+{
+	dev_priv->display_irqs_enabled = false;
+}
+
 static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
 					  struct i915_power_well *power_well)
 {
@@ -1198,7 +1204,7 @@ static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
 
 	vlv_set_power_well(dev_priv, power_well, true);
 
-	vlv_display_power_well_init(dev_priv);
+	vlv_display_power_well_install(dev_priv);
 }
 
 static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
@@ -1206,7 +1212,7 @@ static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
 {
 	WARN_ON_ONCE(power_well->id != PUNIT_POWER_WELL_DISP2D);
 
-	vlv_display_power_well_deinit(dev_priv);
+	vlv_display_power_well_uninstall(dev_priv);
 
 	vlv_set_power_well(dev_priv, power_well, false);
 }
@@ -1661,6 +1667,12 @@ static void chv_set_pipe_power_well(struct drm_i915_private *dev_priv,
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
+static void chv_pipe_power_well_init(struct drm_i915_private *dev_priv,
+				     struct i915_power_well *power_well)
+{
+	dev_priv->display_irqs_enabled = false;
+}
+
 static void chv_pipe_power_well_sync_hw(struct drm_i915_private *dev_priv,
 					struct i915_power_well *power_well)
 {
@@ -1676,7 +1688,7 @@ static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
 
 	chv_set_pipe_power_well(dev_priv, power_well, true);
 
-	vlv_display_power_well_init(dev_priv);
+	vlv_display_power_well_install(dev_priv);
 }
 
 static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
@@ -1684,7 +1696,7 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
 {
 	WARN_ON_ONCE(power_well->id != PIPE_A);
 
-	vlv_display_power_well_deinit(dev_priv);
+	vlv_display_power_well_uninstall(dev_priv);
 
 	chv_set_pipe_power_well(dev_priv, power_well, false);
 }
@@ -1921,6 +1933,7 @@ static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
 };
 
 static const struct i915_power_well_ops chv_pipe_power_well_ops = {
+	.init = chv_pipe_power_well_init,
 	.sync_hw = chv_pipe_power_well_sync_hw,
 	.enable = chv_pipe_power_well_enable,
 	.disable = chv_pipe_power_well_disable,
@@ -2000,6 +2013,7 @@ static struct i915_power_well bdw_power_wells[] = {
 };
 
 static const struct i915_power_well_ops vlv_display_power_well_ops = {
+	.init = vlv_display_power_well_init,
 	.sync_hw = vlv_power_well_sync_hw,
 	.enable = vlv_display_power_well_enable,
 	.disable = vlv_display_power_well_disable,
@@ -2367,6 +2381,19 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
 	return mask;
 }
 
+static void __intel_power_domains_init(struct drm_i915_private *dev_priv)
+{
+	struct i915_power_domains *power_domains = &dev_priv->power_domains;
+	struct i915_power_well *power_well;
+	int i;
+
+	dev_priv->display_irqs_enabled = true;
+	for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
+		if (power_well->ops->init)
+			power_well->ops->init(dev_priv, power_well);
+	}
+}
+
 #define set_power_wells(power_domains, __power_wells) ({		\
 	(power_domains)->power_wells = (__power_wells);			\
 	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
@@ -2414,6 +2441,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 		set_power_wells(power_domains, i9xx_always_on_power_well);
 	}
 
+	__intel_power_domains_init(dev_priv);
+
 	return 0;
 }
 

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
@ 2017-02-15 12:04     ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-02-15 12:04 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, Lyude, Daniel Vetter, Hans de Goede, stable

On Wed, Feb 15, 2017 at 01:23:35PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 15, 2017 at 09:06:53AM +0000, Chris Wilson wrote:
> > 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)
> 
> I don't think we're setting display_irqs_enabled on any other platform
> than VLV/CHV. So either we have to push this check down into
> hpd_irq_setup() or just set display_irqs_enable=true somewhere for all
> the other platforms.

Yup, only vlv/chv set display_irqs_enable. Would it not be safe to

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 99e2d659202c..7e205747f18d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4314,6 +4314,8 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
        if (!IS_GEN2(dev_priv))
                dev->vblank_disable_immediate = true;
 
+       dev_priv->display_irqs_enabled = true;
+
        dev_priv->hotplug.hpd_storm_threshold = HPD_STORM_DEFAULT_THRESHOLD;

which may then cause us to enable the display irq mask before the
powerwell is powered, but as soon as we go through a display powerwell
cycle will be back in sync? That presumes we complete such a cycle
before hotplug init touches the register - and that we can touch the irq
registers outside of the powerwell.

Or we can do something a bit more complete:

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c1b400f1ede4..681196987450 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1381,6 +1381,12 @@ struct i915_power_well;
 
 struct i915_power_well_ops {
 	/*
+	 * Initialize drm_i915_private or struct i915_power_well state,
+	 * called on driver load.
+	 */
+	void (*init)(struct drm_i915_private *dev_priv,
+		     struct i915_power_well *power_well);
+	/*
 	 * Synchronize the well's hw state to match the current sw state, for
 	 * example enable/disable it based on the current refcount. Called
 	 * during driver init and resume time, possibly after first calling
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 879567987201..4cb2b099f6c1 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1126,7 +1126,7 @@ static void vlv_init_display_clock_gating(struct drm_i915_private *dev_priv)
 		   DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 1000));
 }
 
-static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
+static void vlv_display_power_well_install(struct drm_i915_private *dev_priv)
 {
 	struct intel_encoder *encoder;
 	enum pipe pipe;
@@ -1175,7 +1175,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
 	intel_pps_unlock_regs_wa(dev_priv);
 }
 
-static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
+static void vlv_display_power_well_uninstall(struct drm_i915_private *dev_priv)
 {
 	spin_lock_irq(&dev_priv->irq_lock);
 	valleyview_disable_display_irqs(dev_priv);
@@ -1191,6 +1191,12 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
 		intel_hpd_poll_init(dev_priv);
 }
 
+static void vlv_display_power_well_init(struct drm_i915_private *dev_priv,
+					struct i915_power_well *power_well)
+{
+	dev_priv->display_irqs_enabled = false;
+}
+
 static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
 					  struct i915_power_well *power_well)
 {
@@ -1198,7 +1204,7 @@ static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
 
 	vlv_set_power_well(dev_priv, power_well, true);
 
-	vlv_display_power_well_init(dev_priv);
+	vlv_display_power_well_install(dev_priv);
 }
 
 static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
@@ -1206,7 +1212,7 @@ static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
 {
 	WARN_ON_ONCE(power_well->id != PUNIT_POWER_WELL_DISP2D);
 
-	vlv_display_power_well_deinit(dev_priv);
+	vlv_display_power_well_uninstall(dev_priv);
 
 	vlv_set_power_well(dev_priv, power_well, false);
 }
@@ -1661,6 +1667,12 @@ static void chv_set_pipe_power_well(struct drm_i915_private *dev_priv,
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
+static void chv_pipe_power_well_init(struct drm_i915_private *dev_priv,
+				     struct i915_power_well *power_well)
+{
+	dev_priv->display_irqs_enabled = false;
+}
+
 static void chv_pipe_power_well_sync_hw(struct drm_i915_private *dev_priv,
 					struct i915_power_well *power_well)
 {
@@ -1676,7 +1688,7 @@ static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
 
 	chv_set_pipe_power_well(dev_priv, power_well, true);
 
-	vlv_display_power_well_init(dev_priv);
+	vlv_display_power_well_install(dev_priv);
 }
 
 static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
@@ -1684,7 +1696,7 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
 {
 	WARN_ON_ONCE(power_well->id != PIPE_A);
 
-	vlv_display_power_well_deinit(dev_priv);
+	vlv_display_power_well_uninstall(dev_priv);
 
 	chv_set_pipe_power_well(dev_priv, power_well, false);
 }
@@ -1921,6 +1933,7 @@ static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
 };
 
 static const struct i915_power_well_ops chv_pipe_power_well_ops = {
+	.init = chv_pipe_power_well_init,
 	.sync_hw = chv_pipe_power_well_sync_hw,
 	.enable = chv_pipe_power_well_enable,
 	.disable = chv_pipe_power_well_disable,
@@ -2000,6 +2013,7 @@ static struct i915_power_well bdw_power_wells[] = {
 };
 
 static const struct i915_power_well_ops vlv_display_power_well_ops = {
+	.init = vlv_display_power_well_init,
 	.sync_hw = vlv_power_well_sync_hw,
 	.enable = vlv_display_power_well_enable,
 	.disable = vlv_display_power_well_disable,
@@ -2367,6 +2381,19 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
 	return mask;
 }
 
+static void __intel_power_domains_init(struct drm_i915_private *dev_priv)
+{
+	struct i915_power_domains *power_domains = &dev_priv->power_domains;
+	struct i915_power_well *power_well;
+	int i;
+
+	dev_priv->display_irqs_enabled = true;
+	for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
+		if (power_well->ops->init)
+			power_well->ops->init(dev_priv, power_well);
+	}
+}
+
 #define set_power_wells(power_domains, __power_wells) ({		\
 	(power_domains)->power_wells = (__power_wells);			\
 	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
@@ -2414,6 +2441,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 		set_power_wells(power_domains, i9xx_always_on_power_well);
 	}
 
+	__intel_power_domains_init(dev_priv);
+
 	return 0;
 }
 

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
  2017-02-15 12:04     ` Chris Wilson
@ 2017-02-15 12:13       ` Ville Syrjälä
  -1 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-02-15 12:13 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, Lyude, Daniel Vetter, Hans de Goede, stable

On Wed, Feb 15, 2017 at 12:04:18PM +0000, Chris Wilson wrote:
> On Wed, Feb 15, 2017 at 01:23:35PM +0200, Ville Syrj�l� wrote:
> > On Wed, Feb 15, 2017 at 09:06:53AM +0000, Chris Wilson wrote:
> > > 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)
> > 
> > I don't think we're setting display_irqs_enabled on any other platform
> > than VLV/CHV. So either we have to push this check down into
> > hpd_irq_setup() or just set display_irqs_enable=true somewhere for all
> > the other platforms.
> 
> Yup, only vlv/chv set display_irqs_enable. Would it not be safe to
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 99e2d659202c..7e205747f18d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4314,6 +4314,8 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>         if (!IS_GEN2(dev_priv))
>                 dev->vblank_disable_immediate = true;
>  
> +       dev_priv->display_irqs_enabled = true;
> +
>         dev_priv->hotplug.hpd_storm_threshold = HPD_STORM_DEFAULT_THRESHOLD;
> 
> which may then cause us to enable the display irq mask before the
> powerwell is powered, but as soon as we go through a display powerwell
> cycle will be back in sync? That presumes we complete such a cycle
> before hotplug init touches the register - and that we can touch the irq
> registers outside of the powerwell.

I guess that should work. Or we could just do it for !VLV/CHV if we want
to strictly maintain the current behaviour on those platforms.

> 
> Or we can do something a bit more complete:

Not sure such extra complexitry is worth the hassle for this little
thing,

> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c1b400f1ede4..681196987450 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1381,6 +1381,12 @@ struct i915_power_well;
>  
>  struct i915_power_well_ops {
>  	/*
> +	 * Initialize drm_i915_private or struct i915_power_well state,
> +	 * called on driver load.
> +	 */
> +	void (*init)(struct drm_i915_private *dev_priv,
> +		     struct i915_power_well *power_well);
> +	/*
>  	 * Synchronize the well's hw state to match the current sw state, for
>  	 * example enable/disable it based on the current refcount. Called
>  	 * during driver init and resume time, possibly after first calling
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 879567987201..4cb2b099f6c1 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1126,7 +1126,7 @@ static void vlv_init_display_clock_gating(struct drm_i915_private *dev_priv)
>  		   DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 1000));
>  }
>  
> -static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
> +static void vlv_display_power_well_install(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_encoder *encoder;
>  	enum pipe pipe;
> @@ -1175,7 +1175,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
>  	intel_pps_unlock_regs_wa(dev_priv);
>  }
>  
> -static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
> +static void vlv_display_power_well_uninstall(struct drm_i915_private *dev_priv)
>  {
>  	spin_lock_irq(&dev_priv->irq_lock);
>  	valleyview_disable_display_irqs(dev_priv);
> @@ -1191,6 +1191,12 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
>  		intel_hpd_poll_init(dev_priv);
>  }
>  
> +static void vlv_display_power_well_init(struct drm_i915_private *dev_priv,
> +					struct i915_power_well *power_well)
> +{
> +	dev_priv->display_irqs_enabled = false;
> +}
> +
>  static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
>  					  struct i915_power_well *power_well)
>  {
> @@ -1198,7 +1204,7 @@ static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
>  
>  	vlv_set_power_well(dev_priv, power_well, true);
>  
> -	vlv_display_power_well_init(dev_priv);
> +	vlv_display_power_well_install(dev_priv);
>  }
>  
>  static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
> @@ -1206,7 +1212,7 @@ static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
>  {
>  	WARN_ON_ONCE(power_well->id != PUNIT_POWER_WELL_DISP2D);
>  
> -	vlv_display_power_well_deinit(dev_priv);
> +	vlv_display_power_well_uninstall(dev_priv);
>  
>  	vlv_set_power_well(dev_priv, power_well, false);
>  }
> @@ -1661,6 +1667,12 @@ static void chv_set_pipe_power_well(struct drm_i915_private *dev_priv,
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> +static void chv_pipe_power_well_init(struct drm_i915_private *dev_priv,
> +				     struct i915_power_well *power_well)
> +{
> +	dev_priv->display_irqs_enabled = false;
> +}
> +
>  static void chv_pipe_power_well_sync_hw(struct drm_i915_private *dev_priv,
>  					struct i915_power_well *power_well)
>  {
> @@ -1676,7 +1688,7 @@ static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
>  
>  	chv_set_pipe_power_well(dev_priv, power_well, true);
>  
> -	vlv_display_power_well_init(dev_priv);
> +	vlv_display_power_well_install(dev_priv);
>  }
>  
>  static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
> @@ -1684,7 +1696,7 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
>  {
>  	WARN_ON_ONCE(power_well->id != PIPE_A);
>  
> -	vlv_display_power_well_deinit(dev_priv);
> +	vlv_display_power_well_uninstall(dev_priv);
>  
>  	chv_set_pipe_power_well(dev_priv, power_well, false);
>  }
> @@ -1921,6 +1933,7 @@ static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
>  };
>  
>  static const struct i915_power_well_ops chv_pipe_power_well_ops = {
> +	.init = chv_pipe_power_well_init,
>  	.sync_hw = chv_pipe_power_well_sync_hw,
>  	.enable = chv_pipe_power_well_enable,
>  	.disable = chv_pipe_power_well_disable,
> @@ -2000,6 +2013,7 @@ static struct i915_power_well bdw_power_wells[] = {
>  };
>  
>  static const struct i915_power_well_ops vlv_display_power_well_ops = {
> +	.init = vlv_display_power_well_init,
>  	.sync_hw = vlv_power_well_sync_hw,
>  	.enable = vlv_display_power_well_enable,
>  	.disable = vlv_display_power_well_disable,
> @@ -2367,6 +2381,19 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
>  	return mask;
>  }
>  
> +static void __intel_power_domains_init(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +	struct i915_power_well *power_well;
> +	int i;
> +
> +	dev_priv->display_irqs_enabled = true;
> +	for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
> +		if (power_well->ops->init)
> +			power_well->ops->init(dev_priv, power_well);
> +	}
> +}
> +
>  #define set_power_wells(power_domains, __power_wells) ({		\
>  	(power_domains)->power_wells = (__power_wells);			\
>  	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
> @@ -2414,6 +2441,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  		set_power_wells(power_domains, i9xx_always_on_power_well);
>  	}
>  
> +	__intel_power_domains_init(dev_priv);
> +
>  	return 0;
>  }
>  
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrj�l�
Intel OTC

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
@ 2017-02-15 12:13       ` Ville Syrjälä
  0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-02-15 12:13 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, Lyude, Daniel Vetter, Hans de Goede, stable

On Wed, Feb 15, 2017 at 12:04:18PM +0000, Chris Wilson wrote:
> On Wed, Feb 15, 2017 at 01:23:35PM +0200, Ville Syrjälä wrote:
> > On Wed, Feb 15, 2017 at 09:06:53AM +0000, Chris Wilson wrote:
> > > 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)
> > 
> > I don't think we're setting display_irqs_enabled on any other platform
> > than VLV/CHV. So either we have to push this check down into
> > hpd_irq_setup() or just set display_irqs_enable=true somewhere for all
> > the other platforms.
> 
> Yup, only vlv/chv set display_irqs_enable. Would it not be safe to
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 99e2d659202c..7e205747f18d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4314,6 +4314,8 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>         if (!IS_GEN2(dev_priv))
>                 dev->vblank_disable_immediate = true;
>  
> +       dev_priv->display_irqs_enabled = true;
> +
>         dev_priv->hotplug.hpd_storm_threshold = HPD_STORM_DEFAULT_THRESHOLD;
> 
> which may then cause us to enable the display irq mask before the
> powerwell is powered, but as soon as we go through a display powerwell
> cycle will be back in sync? That presumes we complete such a cycle
> before hotplug init touches the register - and that we can touch the irq
> registers outside of the powerwell.

I guess that should work. Or we could just do it for !VLV/CHV if we want
to strictly maintain the current behaviour on those platforms.

> 
> Or we can do something a bit more complete:

Not sure such extra complexitry is worth the hassle for this little
thing,

> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c1b400f1ede4..681196987450 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1381,6 +1381,12 @@ struct i915_power_well;
>  
>  struct i915_power_well_ops {
>  	/*
> +	 * Initialize drm_i915_private or struct i915_power_well state,
> +	 * called on driver load.
> +	 */
> +	void (*init)(struct drm_i915_private *dev_priv,
> +		     struct i915_power_well *power_well);
> +	/*
>  	 * Synchronize the well's hw state to match the current sw state, for
>  	 * example enable/disable it based on the current refcount. Called
>  	 * during driver init and resume time, possibly after first calling
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 879567987201..4cb2b099f6c1 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1126,7 +1126,7 @@ static void vlv_init_display_clock_gating(struct drm_i915_private *dev_priv)
>  		   DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 1000));
>  }
>  
> -static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
> +static void vlv_display_power_well_install(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_encoder *encoder;
>  	enum pipe pipe;
> @@ -1175,7 +1175,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
>  	intel_pps_unlock_regs_wa(dev_priv);
>  }
>  
> -static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
> +static void vlv_display_power_well_uninstall(struct drm_i915_private *dev_priv)
>  {
>  	spin_lock_irq(&dev_priv->irq_lock);
>  	valleyview_disable_display_irqs(dev_priv);
> @@ -1191,6 +1191,12 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
>  		intel_hpd_poll_init(dev_priv);
>  }
>  
> +static void vlv_display_power_well_init(struct drm_i915_private *dev_priv,
> +					struct i915_power_well *power_well)
> +{
> +	dev_priv->display_irqs_enabled = false;
> +}
> +
>  static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
>  					  struct i915_power_well *power_well)
>  {
> @@ -1198,7 +1204,7 @@ static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
>  
>  	vlv_set_power_well(dev_priv, power_well, true);
>  
> -	vlv_display_power_well_init(dev_priv);
> +	vlv_display_power_well_install(dev_priv);
>  }
>  
>  static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
> @@ -1206,7 +1212,7 @@ static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
>  {
>  	WARN_ON_ONCE(power_well->id != PUNIT_POWER_WELL_DISP2D);
>  
> -	vlv_display_power_well_deinit(dev_priv);
> +	vlv_display_power_well_uninstall(dev_priv);
>  
>  	vlv_set_power_well(dev_priv, power_well, false);
>  }
> @@ -1661,6 +1667,12 @@ static void chv_set_pipe_power_well(struct drm_i915_private *dev_priv,
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> +static void chv_pipe_power_well_init(struct drm_i915_private *dev_priv,
> +				     struct i915_power_well *power_well)
> +{
> +	dev_priv->display_irqs_enabled = false;
> +}
> +
>  static void chv_pipe_power_well_sync_hw(struct drm_i915_private *dev_priv,
>  					struct i915_power_well *power_well)
>  {
> @@ -1676,7 +1688,7 @@ static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
>  
>  	chv_set_pipe_power_well(dev_priv, power_well, true);
>  
> -	vlv_display_power_well_init(dev_priv);
> +	vlv_display_power_well_install(dev_priv);
>  }
>  
>  static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
> @@ -1684,7 +1696,7 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
>  {
>  	WARN_ON_ONCE(power_well->id != PIPE_A);
>  
> -	vlv_display_power_well_deinit(dev_priv);
> +	vlv_display_power_well_uninstall(dev_priv);
>  
>  	chv_set_pipe_power_well(dev_priv, power_well, false);
>  }
> @@ -1921,6 +1933,7 @@ static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
>  };
>  
>  static const struct i915_power_well_ops chv_pipe_power_well_ops = {
> +	.init = chv_pipe_power_well_init,
>  	.sync_hw = chv_pipe_power_well_sync_hw,
>  	.enable = chv_pipe_power_well_enable,
>  	.disable = chv_pipe_power_well_disable,
> @@ -2000,6 +2013,7 @@ static struct i915_power_well bdw_power_wells[] = {
>  };
>  
>  static const struct i915_power_well_ops vlv_display_power_well_ops = {
> +	.init = vlv_display_power_well_init,
>  	.sync_hw = vlv_power_well_sync_hw,
>  	.enable = vlv_display_power_well_enable,
>  	.disable = vlv_display_power_well_disable,
> @@ -2367,6 +2381,19 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
>  	return mask;
>  }
>  
> +static void __intel_power_domains_init(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +	struct i915_power_well *power_well;
> +	int i;
> +
> +	dev_priv->display_irqs_enabled = true;
> +	for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
> +		if (power_well->ops->init)
> +			power_well->ops->init(dev_priv, power_well);
> +	}
> +}
> +
>  #define set_power_wells(power_domains, __power_wells) ({		\
>  	(power_domains)->power_wells = (__power_wells);			\
>  	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
> @@ -2414,6 +2441,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  		set_power_wells(power_domains, i9xx_always_on_power_well);
>  	}
>  
> +	__intel_power_domains_init(dev_priv);
> +
>  	return 0;
>  }
>  
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
  2017-02-15  9:06 ` Chris Wilson
@ 2017-02-15 13:15   ` Chris Wilson
  -1 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-02-15 13:15 UTC (permalink / raw)
  To: intel-gfx
  Cc: Chris Wilson, Lyude, Daniel Vetter, Ville Syrjälä,
	Hans de Goede, stable

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.

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@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lyude <cpaul@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
Cc: stable@vger.kernel.org
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 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_detected && dev_priv->display_irqs_enabled)
 		dev_priv->display.hpd_irq_setup(dev_priv);
 	spin_unlock(&dev_priv->irq_lock);
 
@@ -476,10 +476,11 @@ 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)
+	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup) {
+		spin_lock_irq(&dev_priv->irq_lock);
 		dev_priv->display.hpd_irq_setup(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
+		spin_unlock_irq(&dev_priv->irq_lock);
+	}
 }
 
 static void i915_hpd_poll_init_work(struct work_struct *work)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
@ 2017-02-15 13:15   ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-02-15 13:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: Hans de Goede, Daniel Vetter, stable

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.

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@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lyude <cpaul@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
Cc: stable@vger.kernel.org
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 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_detected && dev_priv->display_irqs_enabled)
 		dev_priv->display.hpd_irq_setup(dev_priv);
 	spin_unlock(&dev_priv->irq_lock);
 
@@ -476,10 +476,11 @@ 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)
+	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup) {
+		spin_lock_irq(&dev_priv->irq_lock);
 		dev_priv->display.hpd_irq_setup(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
+		spin_unlock_irq(&dev_priv->irq_lock);
+	}
 }
 
 static void i915_hpd_poll_init_work(struct work_struct *work)
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
  2017-02-15 13:15   ` Chris Wilson
@ 2017-02-15 14:12     ` Ville Syrjälä
  -1 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-02-15 14:12 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, Lyude, Daniel Vetter, Hans de Goede, stable

On Wed, Feb 15, 2017 at 01:15:47PM +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.
> 
> 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@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lyude <cpaul@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

lgtm

Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>

> ---
>  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_detected && dev_priv->display_irqs_enabled)
>  		dev_priv->display.hpd_irq_setup(dev_priv);
>  	spin_unlock(&dev_priv->irq_lock);
>  
> @@ -476,10 +476,11 @@ 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)
> +	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup) {
> +		spin_lock_irq(&dev_priv->irq_lock);
>  		dev_priv->display.hpd_irq_setup(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> +		spin_unlock_irq(&dev_priv->irq_lock);
> +	}
>  }
>  
>  static void i915_hpd_poll_init_work(struct work_struct *work)
> -- 
> 2.11.0

-- 
Ville Syrj�l�
Intel OTC

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
@ 2017-02-15 14:12     ` Ville Syrjälä
  0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-02-15 14:12 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, Lyude, Daniel Vetter, Hans de Goede, stable

On Wed, Feb 15, 2017 at 01:15:47PM +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.
> 
> 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@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lyude <cpaul@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

lgtm

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  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_detected && dev_priv->display_irqs_enabled)
>  		dev_priv->display.hpd_irq_setup(dev_priv);
>  	spin_unlock(&dev_priv->irq_lock);
>  
> @@ -476,10 +476,11 @@ 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)
> +	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup) {
> +		spin_lock_irq(&dev_priv->irq_lock);
>  		dev_priv->display.hpd_irq_setup(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> +		spin_unlock_irq(&dev_priv->irq_lock);
> +	}
>  }
>  
>  static void i915_hpd_poll_init_work(struct work_struct *work)
> -- 
> 2.11.0

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Only enable hotplug interrupts if the display interrupts are enabled (rev3)
  2017-02-15  9:06 ` Chris Wilson
                   ` (3 preceding siblings ...)
  (?)
@ 2017-02-15 15:12 ` Patchwork
  -1 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-02-15 15:12 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Only enable hotplug interrupts if the display interrupts are enabled (rev3)
URL   : https://patchwork.freedesktop.org/series/19687/
State : success

== Summary ==

Series 19687v3 drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
https://patchwork.freedesktop.org/api/1.0/series/19687/revisions/3/mbox/

Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-a:
                dmesg-fail -> PASS       (fi-snb-2520m)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                dmesg-fail -> PASS       (fi-snb-2520m)
        Subgroup nonblocking-crc-pipe-b:
                dmesg-fail -> PASS       (fi-snb-2520m)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                dmesg-fail -> PASS       (fi-snb-2520m)
        Subgroup read-crc-pipe-a:
                incomplete -> PASS       (fi-snb-2520m)

fi-bdw-5557u     total:252  pass:238  dwarn:3   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:252  pass:210  dwarn:3   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:252  pass:230  dwarn:3   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:252  pass:222  dwarn:3   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:252  pass:218  dwarn:3   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:252  pass:233  dwarn:3   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:252  pass:233  dwarn:3   dfail:0   fail:0   skip:16 
fi-ilk-650       total:252  pass:199  dwarn:3   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:252  pass:231  dwarn:3   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:252  pass:231  dwarn:3   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:252  pass:231  dwarn:3   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:252  pass:239  dwarn:3   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:252  pass:232  dwarn:3   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:252  pass:230  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:252  pass:239  dwarn:3   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:252  pass:221  dwarn:3   dfail:0   fail:0   skip:28 
fi-snb-2600      total:252  pass:220  dwarn:3   dfail:0   fail:0   skip:29 

cc11223a7f11b4e2d15f1c645326ac6f34568d88 drm-tip: 2017y-02m-15d-13h-44m-31s UTC integration manifest
af6d56d drm/i915: Only enable hotplug interrupts if the display interrupts are enabled

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3824/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
  2017-02-15 14:12     ` Ville Syrjälä
@ 2017-02-16  9:59       ` Chris Wilson
  -1 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-02-16  9:59 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, Lyude, Daniel Vetter, Hans de Goede, stable

On Wed, Feb 15, 2017 at 04:12:04PM +0200, Ville Syrj�l� wrote:
> On Wed, Feb 15, 2017 at 01:15:47PM +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.
> > 
> > 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@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Lyude <cpaul@redhat.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> lgtm
> 
> Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>

Thanks for the idea and fixes,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
@ 2017-02-16  9:59       ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-02-16  9:59 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, Lyude, Daniel Vetter, Hans de Goede, stable

On Wed, Feb 15, 2017 at 04:12:04PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 15, 2017 at 01:15:47PM +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.
> > 
> > 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@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Lyude <cpaul@redhat.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> lgtm
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks for the idea and fixes,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
  2017-03-13 17:02   ` Chris Wilson
@ 2017-03-15 20:39     ` Lyude Paul
  -1 siblings, 0 replies; 20+ messages in thread
From: Lyude Paul @ 2017-03-15 20:39 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx
  Cc: jani.nikula, Daniel Vetter, Ville Syrjälä,
	Hans de Goede, stable

Acked-by: Lyude <lyude@redhat.com>

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@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lyude <cpaul@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> Cc: stable@vger.kernel.org
> Link: http://patchwork.freedesktop.org/patch/msgid/20170215131547.506
> 4-1-chris@chris-wilson.co.uk
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
@ 2017-03-15 20:39     ` Lyude Paul
  0 siblings, 0 replies; 20+ messages in thread
From: Lyude Paul @ 2017-03-15 20:39 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: jani.nikula, Daniel Vetter, stable, Hans de Goede

Acked-by: Lyude <lyude@redhat.com>

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@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lyude <cpaul@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> Cc: stable@vger.kernel.org
> Link: http://patchwork.freedesktop.org/patch/msgid/20170215131547.506
> 4-1-chris@chris-wilson.co.uk
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
  2017-03-13 16:14 Fixes that failed to backport to v4.11-rc1 Jani Nikula
@ 2017-03-13 17:02   ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-03-13 17:02 UTC (permalink / raw)
  To: intel-gfx
  Cc: jani.nikula, Chris Wilson, Lyude, Daniel Vetter,
	Ville Syrjälä,
	Hans de Goede, stable

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@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lyude <cpaul@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
Cc: stable@vger.kernel.org
Link: http://patchwork.freedesktop.org/patch/msgid/20170215131547.5064-1-chris@chris-wilson.co.uk
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 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)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
@ 2017-03-13 17:02   ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-03-13 17:02 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, Daniel Vetter, stable, Hans de Goede

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@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lyude <cpaul@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
Cc: stable@vger.kernel.org
Link: http://patchwork.freedesktop.org/patch/msgid/20170215131547.5064-1-chris@chris-wilson.co.uk
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 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)
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2017-03-15 20:40 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-15  9:06 [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled Chris Wilson
2017-02-15  9:06 ` Chris Wilson
2017-02-15 11:23 ` Ville Syrjälä
2017-02-15 11:23   ` Ville Syrjälä
2017-02-15 12:04   ` Chris Wilson
2017-02-15 12:04     ` Chris Wilson
2017-02-15 12:13     ` Ville Syrjälä
2017-02-15 12:13       ` Ville Syrjälä
2017-02-15 11:52 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-02-15 13:15 ` [PATCH v2] " Chris Wilson
2017-02-15 13:15   ` Chris Wilson
2017-02-15 14:12   ` Ville Syrjälä
2017-02-15 14:12     ` Ville Syrjälä
2017-02-16  9:59     ` Chris Wilson
2017-02-16  9:59       ` Chris Wilson
2017-02-15 15:12 ` ✓ Fi.CI.BAT: success for drm/i915: Only enable hotplug interrupts if the display interrupts are enabled (rev3) Patchwork
2017-03-13 16:14 Fixes that failed to backport to v4.11-rc1 Jani Nikula
2017-03-13 17:02 ` [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled Chris Wilson
2017-03-13 17:02   ` Chris Wilson
2017-03-15 20:39   ` Lyude Paul
2017-03-15 20:39     ` Lyude Paul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.