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

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

[Chris Wilson]

In order to prevent accessing the hpd registers outside of the display
power wells, we should refrain from writing to the registers before the
display interrupts are enabled.

[    4.740136] WARNING: CPU: 1 PID: 221 at drivers/gpu/drm/i915/intel_uncore.c:795 __unclaimed_reg_debug+0x44/0x50 [i915]
[    4.740155] Unclaimed read from register 0x1e1110
[    4.740168] Modules linked in: i915(+) intel_gtt drm_kms_helper prime_numbers
[    4.740190] CPU: 1 PID: 221 Comm: systemd-udevd Not tainted 4.10.0-rc6+ #384
[    4.740203] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[    4.740220] Call Trace:
[    4.740236]  dump_stack+0x4d/0x6f
[    4.740251]  __warn+0xc1/0xe0
[    4.740265]  warn_slowpath_fmt+0x4a/0x50
[    4.740281]  ? insert_work+0x77/0xc0
[    4.740355]  ? fwtable_write32+0x90/0x130 [i915]
[    4.740431]  __unclaimed_reg_debug+0x44/0x50 [i915]
[    4.740507]  fwtable_read32+0xd8/0x130 [i915]
[    4.740575]  i915_hpd_irq_setup+0xa5/0x100 [i915]
[    4.740649]  intel_hpd_init+0x68/0x80 [i915]
[    4.740716]  i915_driver_load+0xe19/0x1380 [i915]
[    4.740784]  i915_pci_probe+0x32/0x90 [i915]
[    4.740799]  pci_device_probe+0x8b/0xf0
[    4.740815]  driver_probe_device+0x2b6/0x450
[    4.740828]  __driver_attach+0xda/0xe0
[    4.740841]  ? driver_probe_device+0x450/0x450
[    4.740853]  bus_for_each_dev+0x5b/0x90
[    4.740865]  driver_attach+0x19/0x20
[    4.740878]  bus_add_driver+0x166/0x260
[    4.740892]  driver_register+0x5b/0xd0
[    4.740906]  ? 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

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

[Chris Wilson]

In order to prevent accessing the hpd registers outside of the display
power wells, we should refrain from writing to the registers before the
display interrupts are enabled.

[    4.740136] WARNING: CPU: 1 PID: 221 at drivers/gpu/drm/i915/intel_uncore.c:795 __unclaimed_reg_debug+0x44/0x50 [i915]
[    4.740155] Unclaimed read from register 0x1e1110
[    4.740168] Modules linked in: i915(+) intel_gtt drm_kms_helper prime_numbers
[    4.740190] CPU: 1 PID: 221 Comm: systemd-udevd Not tainted 4.10.0-rc6+ #384
[    4.740203] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[    4.740220] Call Trace:
[    4.740236]  dump_stack+0x4d/0x6f
[    4.740251]  __warn+0xc1/0xe0
[    4.740265]  warn_slowpath_fmt+0x4a/0x50
[    4.740281]  ? insert_work+0x77/0xc0
[    4.740355]  ? fwtable_write32+0x90/0x130 [i915]
[    4.740431]  __unclaimed_reg_debug+0x44/0x50 [i915]
[    4.740507]  fwtable_read32+0xd8/0x130 [i915]
[    4.740575]  i915_hpd_irq_setup+0xa5/0x100 [i915]
[    4.740649]  intel_hpd_init+0x68/0x80 [i915]
[    4.740716]  i915_driver_load+0xe19/0x1380 [i915]
[    4.740784]  i915_pci_probe+0x32/0x90 [i915]
[    4.740799]  pci_device_probe+0x8b/0xf0
[    4.740815]  driver_probe_device+0x2b6/0x450
[    4.740828]  __driver_attach+0xda/0xe0
[    4.740841]  ? driver_probe_device+0x450/0x450
[    4.740853]  bus_for_each_dev+0x5b/0x90
[    4.740865]  driver_attach+0x19/0x20
[    4.740878]  bus_add_driver+0x166/0x260
[    4.740892]  driver_register+0x5b/0xd0
[    4.740906]  ? 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

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

[Ville Syrjälä]

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

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

[Ville Syrjälä]

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

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

[Chris Wilson]

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

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

[Chris Wilson]

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

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

[Ville Syrjälä]

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

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

[Ville Syrjälä]

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

✓ Fi.CI.BAT: success for drm/i915: Only enable hotplug interrupts if the display interrupts are enabled

[Patchwork]

== 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

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

[Chris Wilson]

In order to prevent accessing the hpd registers outside of the display
power wells, we should refrain from writing to the registers before the
display interrupts are enabled.

[    4.740136] WARNING: CPU: 1 PID: 221 at drivers/gpu/drm/i915/intel_uncore.c:795 __unclaimed_reg_debug+0x44/0x50 [i915]
[    4.740155] Unclaimed read from register 0x1e1110
[    4.740168] Modules linked in: i915(+) intel_gtt drm_kms_helper prime_numbers
[    4.740190] CPU: 1 PID: 221 Comm: systemd-udevd Not tainted 4.10.0-rc6+ #384
[    4.740203] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[    4.740220] Call Trace:
[    4.740236]  dump_stack+0x4d/0x6f
[    4.740251]  __warn+0xc1/0xe0
[    4.740265]  warn_slowpath_fmt+0x4a/0x50
[    4.740281]  ? insert_work+0x77/0xc0
[    4.740355]  ? fwtable_write32+0x90/0x130 [i915]
[    4.740431]  __unclaimed_reg_debug+0x44/0x50 [i915]
[    4.740507]  fwtable_read32+0xd8/0x130 [i915]
[    4.740575]  i915_hpd_irq_setup+0xa5/0x100 [i915]
[    4.740649]  intel_hpd_init+0x68/0x80 [i915]
[    4.740716]  i915_driver_load+0xe19/0x1380 [i915]
[    4.740784]  i915_pci_probe+0x32/0x90 [i915]
[    4.740799]  pci_device_probe+0x8b/0xf0
[    4.740815]  driver_probe_device+0x2b6/0x450
[    4.740828]  __driver_attach+0xda/0xe0
[    4.740841]  ? driver_probe_device+0x450/0x450
[    4.740853]  bus_for_each_dev+0x5b/0x90
[    4.740865]  driver_attach+0x19/0x20
[    4.740878]  bus_add_driver+0x166/0x260
[    4.740892]  driver_register+0x5b/0xd0
[    4.740906]  ? 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

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

[Chris Wilson]

In order to prevent accessing the hpd registers outside of the display
power wells, we should refrain from writing to the registers before the
display interrupts are enabled.

[    4.740136] WARNING: CPU: 1 PID: 221 at drivers/gpu/drm/i915/intel_uncore.c:795 __unclaimed_reg_debug+0x44/0x50 [i915]
[    4.740155] Unclaimed read from register 0x1e1110
[    4.740168] Modules linked in: i915(+) intel_gtt drm_kms_helper prime_numbers
[    4.740190] CPU: 1 PID: 221 Comm: systemd-udevd Not tainted 4.10.0-rc6+ #384
[    4.740203] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[    4.740220] Call Trace:
[    4.740236]  dump_stack+0x4d/0x6f
[    4.740251]  __warn+0xc1/0xe0
[    4.740265]  warn_slowpath_fmt+0x4a/0x50
[    4.740281]  ? insert_work+0x77/0xc0
[    4.740355]  ? fwtable_write32+0x90/0x130 [i915]
[    4.740431]  __unclaimed_reg_debug+0x44/0x50 [i915]
[    4.740507]  fwtable_read32+0xd8/0x130 [i915]
[    4.740575]  i915_hpd_irq_setup+0xa5/0x100 [i915]
[    4.740649]  intel_hpd_init+0x68/0x80 [i915]
[    4.740716]  i915_driver_load+0xe19/0x1380 [i915]
[    4.740784]  i915_pci_probe+0x32/0x90 [i915]
[    4.740799]  pci_device_probe+0x8b/0xf0
[    4.740815]  driver_probe_device+0x2b6/0x450
[    4.740828]  __driver_attach+0xda/0xe0
[    4.740841]  ? driver_probe_device+0x450/0x450
[    4.740853]  bus_for_each_dev+0x5b/0x90
[    4.740865]  driver_attach+0x19/0x20
[    4.740878]  bus_add_driver+0x166/0x260
[    4.740892]  driver_register+0x5b/0xd0
[    4.740906]  ? 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

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

[Ville Syrjälä]

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

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

[Ville Syrjälä]

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

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

[Chris Wilson]

On Wed, Feb 15, 2017 at 04:12:04PM +0200, Ville Syrj�l� wrote:
> On Wed, Feb 15, 2017 at 01:15:47PM +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

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

[Chris Wilson]

On Wed, Feb 15, 2017 at 04:12:04PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 15, 2017 at 01:15:47PM +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

✓ Fi.CI.BAT: success for drm/i915: Only enable hotplug interrupts if the display interrupts are enabled (rev3)

[Patchwork]

== 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