[PATCH 1/4] drm/i915: fix HPD IRQ reenable work cancelation

[PATCH 1/4] drm/i915: fix HPD IRQ reenable work cancelation

[Imre Deak]

Atm, the HPD IRQ reenable timer can get rearmed right after it's
canceled. Also to access the HPD IRQ mask registers we need to wake up
the HW.

Solve both issues by converting the reenable timer to a delayed work and
grabbing a runtime PM reference in the work. By this we can also forgo
canceling the timer during runtime suspend, since the only important
thing there is that the HW is awake when we write the registers and
that's ensured by the RPM ref. So do the cancelation only during driver
unload time; this is also a requirement for an upcoming patch where we
want to cancel all HPD related works only during system suspend and
driver unload time, but not during runtime suspend.

Note that there is still a race between the HPD IRQ reenable work and
drm_irq_uninstall() during driver unload, where the work can reenable
the HPD IRQs disabled by drm_irq_uninstall(). This isn't a problem since
the HPD IRQs will still be effectively masked by the first level
interrupt mask.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  2 +-
 drivers/gpu/drm/i915/i915_irq.c      | 33 ++++++++++++---------------------
 drivers/gpu/drm/i915/intel_display.c |  1 +
 3 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9198f1c..35d150a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1483,7 +1483,7 @@ struct drm_i915_private {
 		} hpd_mark;
 	} hpd_stats[HPD_NUM_PINS];
 	u32 hpd_event_bits;
-	struct timer_list hotplug_reenable_timer;
+	struct delayed_work hotplug_reenable_work;
 
 	struct i915_fbc fbc;
 	struct i915_drrs drrs;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8e6729e..fbf1eff 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1189,8 +1189,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
 	  * some connectors */
 	if (hpd_disabled) {
 		drm_kms_helper_poll_enable(dev);
-		mod_timer(&dev_priv->hotplug_reenable_timer,
-			  jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
+		schedule_delayed_work(&dev_priv->hotplug_reenable_work,
+			  msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
 	}
 
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
@@ -1213,11 +1213,6 @@ static void i915_hotplug_work_func(struct work_struct *work)
 		drm_kms_helper_hotplug_event(dev);
 }
 
-static void intel_hpd_irq_uninstall(struct drm_i915_private *dev_priv)
-{
-	del_timer_sync(&dev_priv->hotplug_reenable_timer);
-}
-
 static void ironlake_rps_change_irq_handler(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3886,8 +3881,6 @@ static void gen8_irq_uninstall(struct drm_device *dev)
 	if (!dev_priv)
 		return;
 
-	intel_hpd_irq_uninstall(dev_priv);
-
 	gen8_irq_reset(dev);
 }
 
@@ -3902,8 +3895,6 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
 
 	I915_WRITE(VLV_MASTER_IER, 0);
 
-	intel_hpd_irq_uninstall(dev_priv);
-
 	for_each_pipe(pipe)
 		I915_WRITE(PIPESTAT(pipe), 0xffff);
 
@@ -3982,8 +3973,6 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 	if (!dev_priv)
 		return;
 
-	intel_hpd_irq_uninstall(dev_priv);
-
 	ironlake_irq_reset(dev);
 }
 
@@ -4354,8 +4343,6 @@ static void i915_irq_uninstall(struct drm_device * dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe;
 
-	intel_hpd_irq_uninstall(dev_priv);
-
 	if (I915_HAS_HOTPLUG(dev)) {
 		I915_WRITE(PORT_HOTPLUG_EN, 0);
 		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
@@ -4591,8 +4578,6 @@ static void i965_irq_uninstall(struct drm_device * dev)
 	if (!dev_priv)
 		return;
 
-	intel_hpd_irq_uninstall(dev_priv);
-
 	I915_WRITE(PORT_HOTPLUG_EN, 0);
 	I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
 
@@ -4608,14 +4593,18 @@ static void i965_irq_uninstall(struct drm_device * dev)
 	I915_WRITE(IIR, I915_READ(IIR));
 }
 
-static void intel_hpd_irq_reenable(unsigned long data)
+static void intel_hpd_irq_reenable(struct work_struct *work)
 {
-	struct drm_i915_private *dev_priv = (struct drm_i915_private *)data;
+	struct drm_i915_private *dev_priv =
+		container_of(work, typeof(*dev_priv),
+			     hotplug_reenable_work.work);
 	struct drm_device *dev = dev_priv->dev;
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	unsigned long irqflags;
 	int i;
 
+	intel_runtime_pm_get(dev_priv);
+
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
 	for (i = (HPD_NONE + 1); i < HPD_NUM_PINS; i++) {
 		struct drm_connector *connector;
@@ -4641,6 +4630,8 @@ static void intel_hpd_irq_reenable(unsigned long data)
 	if (dev_priv->display.hpd_irq_setup)
 		dev_priv->display.hpd_irq_setup(dev);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+
+	intel_runtime_pm_put(dev_priv);
 }
 
 void intel_irq_init(struct drm_device *dev)
@@ -4663,8 +4654,8 @@ void intel_irq_init(struct drm_device *dev)
 	setup_timer(&dev_priv->gpu_error.hangcheck_timer,
 		    i915_hangcheck_elapsed,
 		    (unsigned long) dev);
-	setup_timer(&dev_priv->hotplug_reenable_timer, intel_hpd_irq_reenable,
-		    (unsigned long) dev_priv);
+	INIT_DELAYED_WORK(&dev_priv->hotplug_reenable_work,
+			  intel_hpd_irq_reenable);
 
 	pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e9b578e..b3a3168 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13101,6 +13101,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	 */
 	drm_irq_uninstall(dev);
 	cancel_work_sync(&dev_priv->hotplug_work);
+	cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work);
 	dev_priv->pm._irqs_disabled = true;
 
 	/*
-- 
1.8.4

[PATCH v2 1/5] drm/i915: take display port power domain in DP HPD handler

[Imre Deak]

Ville noticed that we can call ibx_digital_port_connected() which accesses
the HW without holding any power well/runtime pm reference. Fix this by
holding a display port power domain reference around the whole hpd_pulse
handler.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e5ada4f..0ebad42 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4037,9 +4037,12 @@ bool
 intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 {
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
+	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int ret;
+	enum intel_display_power_domain power_domain;
+	bool ret = true;
+
 	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP)
 		intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT;
 
@@ -4047,6 +4050,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 		      port_name(intel_dig_port->port),
 		      long_hpd ? "long" : "short");
 
+	power_domain = intel_display_port_power_domain(intel_encoder);
+	intel_display_power_get(dev_priv, power_domain);
+
 	if (long_hpd) {
 		if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
 			goto mst_fail;
@@ -4062,8 +4068,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 
 	} else {
 		if (intel_dp->is_mst) {
-			ret = intel_dp_check_mst_status(intel_dp);
-			if (ret == -EINVAL)
+			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
 				goto mst_fail;
 		}
 
@@ -4077,7 +4082,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 			drm_modeset_unlock(&dev->mode_config.connection_mutex);
 		}
 	}
-	return false;
+	ret = false;
+	goto put_power;
 mst_fail:
 	/* if we were in MST mode, and device is not there get out of MST mode */
 	if (intel_dp->is_mst) {
@@ -4085,7 +4091,10 @@ mst_fail:
 		intel_dp->is_mst = false;
 		drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
 	}
-	return true;
+put_power:
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
 }
 
 /* Return which DP Port should be selected for Transcoder DP control */
-- 
1.8.4

[PATCH v2 2/5] drm/i915: fix HPD IRQ reenable work cancelation

[Imre Deak]

Atm, the HPD IRQ reenable timer can get rearmed right after it's
canceled. Also to access the HPD IRQ mask registers we need to wake up
the HW.

Solve both issues by converting the reenable timer to a delayed work and
grabbing a runtime PM reference in the work. By this we can also forgo
canceling the timer during runtime suspend, since the only important
thing there is that the HW is awake when we write the registers and
that's ensured by the RPM ref. So do the cancelation only during driver
unload time; this is also a requirement for an upcoming patch where we
want to cancel all HPD related works only during system suspend and
driver unload time, but not during runtime suspend.

Note that there is still a race between the HPD IRQ reenable work and
drm_irq_uninstall() during driver unload, where the work can reenable
the HPD IRQs disabled by drm_irq_uninstall(). This isn't a problem since
the HPD IRQs will still be effectively masked by the first level
interrupt mask.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  2 +-
 drivers/gpu/drm/i915/i915_irq.c      | 33 ++++++++++++---------------------
 drivers/gpu/drm/i915/intel_display.c |  1 +
 3 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 541fb6f..9d9a2e7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1483,7 +1483,7 @@ struct drm_i915_private {
 		} hpd_mark;
 	} hpd_stats[HPD_NUM_PINS];
 	u32 hpd_event_bits;
-	struct timer_list hotplug_reenable_timer;
+	struct delayed_work hotplug_reenable_work;
 
 	struct i915_fbc fbc;
 	struct i915_drrs drrs;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b1bb88f..68fe425 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1189,8 +1189,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
 	  * some connectors */
 	if (hpd_disabled) {
 		drm_kms_helper_poll_enable(dev);
-		mod_timer(&dev_priv->hotplug_reenable_timer,
-			  jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
+		schedule_delayed_work(&dev_priv->hotplug_reenable_work,
+			  msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
 	}
 
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
@@ -1213,11 +1213,6 @@ static void i915_hotplug_work_func(struct work_struct *work)
 		drm_kms_helper_hotplug_event(dev);
 }
 
-static void intel_hpd_irq_uninstall(struct drm_i915_private *dev_priv)
-{
-	del_timer_sync(&dev_priv->hotplug_reenable_timer);
-}
-
 static void ironlake_rps_change_irq_handler(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3901,8 +3896,6 @@ static void gen8_irq_uninstall(struct drm_device *dev)
 	if (!dev_priv)
 		return;
 
-	intel_hpd_irq_uninstall(dev_priv);
-
 	gen8_irq_reset(dev);
 }
 
@@ -3917,8 +3910,6 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
 
 	I915_WRITE(VLV_MASTER_IER, 0);
 
-	intel_hpd_irq_uninstall(dev_priv);
-
 	for_each_pipe(pipe)
 		I915_WRITE(PIPESTAT(pipe), 0xffff);
 
@@ -3997,8 +3988,6 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 	if (!dev_priv)
 		return;
 
-	intel_hpd_irq_uninstall(dev_priv);
-
 	ironlake_irq_reset(dev);
 }
 
@@ -4369,8 +4358,6 @@ static void i915_irq_uninstall(struct drm_device * dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe;
 
-	intel_hpd_irq_uninstall(dev_priv);
-
 	if (I915_HAS_HOTPLUG(dev)) {
 		I915_WRITE(PORT_HOTPLUG_EN, 0);
 		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
@@ -4606,8 +4593,6 @@ static void i965_irq_uninstall(struct drm_device * dev)
 	if (!dev_priv)
 		return;
 
-	intel_hpd_irq_uninstall(dev_priv);
-
 	I915_WRITE(PORT_HOTPLUG_EN, 0);
 	I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
 
@@ -4623,14 +4608,18 @@ static void i965_irq_uninstall(struct drm_device * dev)
 	I915_WRITE(IIR, I915_READ(IIR));
 }
 
-static void intel_hpd_irq_reenable(unsigned long data)
+static void intel_hpd_irq_reenable(struct work_struct *work)
 {
-	struct drm_i915_private *dev_priv = (struct drm_i915_private *)data;
+	struct drm_i915_private *dev_priv =
+		container_of(work, typeof(*dev_priv),
+			     hotplug_reenable_work.work);
 	struct drm_device *dev = dev_priv->dev;
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	unsigned long irqflags;
 	int i;
 
+	intel_runtime_pm_get(dev_priv);
+
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
 	for (i = (HPD_NONE + 1); i < HPD_NUM_PINS; i++) {
 		struct drm_connector *connector;
@@ -4656,6 +4645,8 @@ static void intel_hpd_irq_reenable(unsigned long data)
 	if (dev_priv->display.hpd_irq_setup)
 		dev_priv->display.hpd_irq_setup(dev);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+
+	intel_runtime_pm_put(dev_priv);
 }
 
 void intel_irq_init(struct drm_device *dev)
@@ -4678,8 +4669,8 @@ void intel_irq_init(struct drm_device *dev)
 	setup_timer(&dev_priv->gpu_error.hangcheck_timer,
 		    i915_hangcheck_elapsed,
 		    (unsigned long) dev);
-	setup_timer(&dev_priv->hotplug_reenable_timer, intel_hpd_irq_reenable,
-		    (unsigned long) dev_priv);
+	INIT_DELAYED_WORK(&dev_priv->hotplug_reenable_work,
+			  intel_hpd_irq_reenable);
 
 	pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3813526..e15b155 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13117,6 +13117,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	 */
 	drm_irq_uninstall(dev);
 	cancel_work_sync(&dev_priv->hotplug_work);
+	cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work);
 	dev_priv->pm._irqs_disabled = true;
 
 	/*
-- 
1.8.4

[PATCH v2 3/5] drm/i915: cancel hotplug and dig_port work during suspend and unload

[Imre Deak]

Make sure these work handlers don't run after we system suspend or
unload the driver. Note that we don't cancel the handlers during runtime
suspend. That could lead to a lockup, since we take a runtime PM ref
from the handlers themselves. Fortunaltely canceling there is not needed
since the RPM ref itself provides for the needed serialization.

v2:
- fix the order of canceling dig_port_work wrt. hotplug_work (Ville)
- zero out {long,short}_hpd_port_mask and hpd_event_bits for speed
  (Ville)

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      | 16 ++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c |  3 +--
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 01de977..d1a29fe 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -494,6 +494,21 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
 	return true;
 }
 
+void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
+{
+	spin_lock_irq(&dev_priv->irq_lock);
+
+	dev_priv->long_hpd_port_mask = 0;
+	dev_priv->short_hpd_port_mask = 0;
+	dev_priv->hpd_event_bits = 0;
+
+	spin_unlock_irq(&dev_priv->irq_lock);
+
+	cancel_work_sync(&dev_priv->dig_port_work);
+	cancel_work_sync(&dev_priv->hotplug_work);
+	cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work);
+}
+
 static int i915_drm_freeze(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -538,6 +553,7 @@ static int i915_drm_freeze(struct drm_device *dev)
 		flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 
 		intel_runtime_pm_disable_interrupts(dev);
+		intel_hpd_cancel_work(dev_priv);
 
 		intel_suspend_gt_powersave(dev);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9d9a2e7..d8b6f67 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2219,6 +2219,7 @@ extern unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
 extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv);
 extern void i915_update_gfx_val(struct drm_i915_private *dev_priv);
 int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
+void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
 
 /* i915_irq.c */
 void i915_queue_hangcheck(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e15b155..c30b9f8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13116,8 +13116,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	 * experience fancy races otherwise.
 	 */
 	drm_irq_uninstall(dev);
-	cancel_work_sync(&dev_priv->hotplug_work);
-	cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work);
+	intel_hpd_cancel_work(dev_priv);
 	dev_priv->pm._irqs_disabled = true;
 
 	/*
-- 
1.8.4

[PATCH v2 4/5] drm/i915: make sure VDD is turned off during system suspend

[Imre Deak]

Atm we may leave eDP VDD enabled during system suspend after the CRTCs
are disabled through an HPD->DPCD read event. So disable VDD during
suspend at a point when no HPDs can occur.

Note that runtime suspend doesn't have the same problem, since there the
RPM ref held by VDD provides already the needed serialization.

v2:
- add note to commit message about the runtime suspend path (Ville)
- use edp_panel_vdd_off_sync(), so we can keep the WARN in
  edp_panel_vdd_off() (Ville)

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c  | 15 +++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c  | 11 +++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  6 ++++++
 3 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d1a29fe..5983b14 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -509,6 +509,19 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
 	cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work);
 }
 
+static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+	struct intel_encoder *intel_encoder;
+
+	drm_modeset_lock_all(dev);
+	for_each_intel_encoder(dev, intel_encoder) {
+		if (intel_encoder->suspend)
+			intel_encoder->suspend(intel_encoder);
+	}
+	drm_modeset_unlock_all(dev);
+}
+
 static int i915_drm_freeze(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -555,6 +568,8 @@ static int i915_drm_freeze(struct drm_device *dev)
 		intel_runtime_pm_disable_interrupts(dev);
 		intel_hpd_cancel_work(dev_priv);
 
+		intel_suspend_encoders(dev_priv);
+
 		intel_suspend_gt_powersave(dev);
 
 		intel_modeset_suspend_hw(dev);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0ebad42..ee982fc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4003,6 +4003,16 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 	kfree(intel_dig_port);
 }
 
+void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
+
+	if (!is_edp(intel_dp))
+		return;
+
+	edp_panel_vdd_off_sync(intel_dp);
+}
+
 static void intel_dp_encoder_reset(struct drm_encoder *encoder)
 {
 	intel_edp_panel_vdd_sanitize(to_intel_encoder(encoder));
@@ -4732,6 +4742,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 	intel_encoder->disable = intel_disable_dp;
 	intel_encoder->get_hw_state = intel_dp_get_hw_state;
 	intel_encoder->get_config = intel_dp_get_config;
+	intel_encoder->suspend = intel_dp_encoder_suspend;
 	if (IS_CHERRYVIEW(dev)) {
 		intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
 		intel_encoder->pre_enable = chv_pre_enable_dp;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3abc915..5d7b3f9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -153,6 +153,12 @@ struct intel_encoder {
 	 * be set correctly before calling this function. */
 	void (*get_config)(struct intel_encoder *,
 			   struct intel_crtc_config *pipe_config);
+	/*
+	 * Called during system suspend after all pending requests for the
+	 * encoder are flushed (for example for DP AUX transactions) and
+	 * device interrupts are disabled.
+	 */
+	void (*suspend)(struct intel_encoder *);
 	int crtc_mask;
 	enum hpd_pin hpd_pin;
 };
-- 
1.8.4

[PATCH v2 5/5] drm/i915: don't try to retrain a DP link on an inactive CRTC

[Imre Deak]

Atm we may retrain the DP link even if the CRTC is inactive through
HPD work->intel_dp_check_link_status(). This in turn can lock up the PHY
(at least on BYT), since the DP port is disabled.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81948
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ee982fc..8c1d4e1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3553,6 +3553,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 	if (WARN_ON(!intel_encoder->base.crtc))
 		return;
 
+	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
+		return;
+
 	/* Try to read receiver status if the link appears to be up */
 	if (!intel_dp_get_link_status(intel_dp, link_status)) {
 		return;
-- 
1.8.4

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

Re: [PATCH 1/4] drm/i915: fix HPD IRQ reenable work cancelation

[Ville Syrjälä]

The series seems fine to me.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
for the rest as well.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v2 1/5] drm/i915: take display port power domain in DP HPD handler

[Dave Airlie]

> Ville noticed that we can call ibx_digital_port_connected() which accesses
> the HW without holding any power well/runtime pm reference. Fix this by
> holding a display port power domain reference around the whole hpd_pulse
> handler.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Makes sense to me,

Reviewed-by: Dave Airlie <airlied@redhat.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e5ada4f..0ebad42 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4037,9 +4037,12 @@ bool
>  intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  {
>         struct intel_dp *intel_dp = &intel_dig_port->dp;
> +       struct intel_encoder *intel_encoder = &intel_dig_port->base;
>         struct drm_device *dev = intel_dig_port->base.base.dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
> -       int ret;
> +       enum intel_display_power_domain power_domain;
> +       bool ret = true;
> +
>         if (intel_dig_port->base.type != INTEL_OUTPUT_EDP)
>                 intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT;
>
> @@ -4047,6 +4050,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>                       port_name(intel_dig_port->port),
>                       long_hpd ? "long" : "short");
>
> +       power_domain = intel_display_port_power_domain(intel_encoder);
> +       intel_display_power_get(dev_priv, power_domain);
> +
>         if (long_hpd) {
>                 if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
>                         goto mst_fail;
> @@ -4062,8 +4068,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>
>         } else {
>                 if (intel_dp->is_mst) {
> -                       ret = intel_dp_check_mst_status(intel_dp);
> -                       if (ret == -EINVAL)
> +                       if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
>                                 goto mst_fail;
>                 }
>
> @@ -4077,7 +4082,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>                         drm_modeset_unlock(&dev->mode_config.connection_mutex);
>                 }
>         }
> -       return false;
> +       ret = false;
> +       goto put_power;
>  mst_fail:
>         /* if we were in MST mode, and device is not there get out of MST mode */
>         if (intel_dp->is_mst) {
> @@ -4085,7 +4091,10 @@ mst_fail:
>                 intel_dp->is_mst = false;
>                 drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
>         }
> -       return true;
> +put_power:
> +       intel_display_power_put(dev_priv, power_domain);
> +
> +       return ret;
>  }
>
>  /* Return which DP Port should be selected for Transcoder DP control */
> --
> 1.8.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: fix HPD IRQ reenable work cancelation

[Imre Deak]

On Wed, 2014-08-13 at 19:33 +0300, Ville Syrjälä wrote:
> The series seems fine to me.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> for the rest as well.

Thanks, I assume it's for v2. I'd say this is for -fixes. The problem
existed even in 3.16, but only the MST support made it apparent with the
extra HPD signaling and DP AUX activity during suspend.

--Imre 


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

Re: [PATCH 1/4] drm/i915: fix HPD IRQ reenable work cancelation

[Jani Nikula]

On Fri, 15 Aug 2014, Imre Deak <imre.deak@intel.com> wrote:
> On Wed, 2014-08-13 at 19:33 +0300, Ville Syrjälä wrote:
>> The series seems fine to me.
>> 
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> for the rest as well.
>
> Thanks, I assume it's for v2. I'd say this is for -fixes. The problem
> existed even in 3.16, but only the MST support made it apparent with the
> extra HPD signaling and DP AUX activity during suspend.

The series no longer applies on any branch. Would you mind respinning on
-fixes please?

BR,
Jani.


>
> --Imre 
>
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: fix HPD IRQ reenable work cancelation

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 953 bytes --]

On Fri, 2014-08-15 at 12:48 +0300, Jani Nikula wrote:
> On Fri, 15 Aug 2014, Imre Deak <imre.deak@intel.com> wrote:
> > On Wed, 2014-08-13 at 19:33 +0300, Ville Syrjälä wrote:
> >> The series seems fine to me.
> >> 
> >> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> for the rest as well.
> >
> > Thanks, I assume it's for v2. I'd say this is for -fixes. The problem
> > existed even in 3.16, but only the MST support made it apparent with the
> > extra HPD signaling and DP AUX activity during suspend.
> 
> The series no longer applies on any branch. Would you mind respinning on
> -fixes please?

Ok. I just noticed that it depends on the following two patches that are
only in -nightly not in -fixes:

"drm/i915: Introduce a for_each_intel_encoder() macro"
"drm/i915: lock around link status and link training."

Are you going to apply these? The second one is definitely needed in
-fixes imo.

--Imre

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 1/4] drm/i915: fix HPD IRQ reenable work cancelation

[Jani Nikula]

On Fri, 15 Aug 2014, Imre Deak <imre.deak@intel.com> wrote:
> On Fri, 2014-08-15 at 12:48 +0300, Jani Nikula wrote:
>> On Fri, 15 Aug 2014, Imre Deak <imre.deak@intel.com> wrote:
>> > On Wed, 2014-08-13 at 19:33 +0300, Ville Syrjälä wrote:
>> >> The series seems fine to me.
>> >> 
>> >> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> for the rest as well.
>> >
>> > Thanks, I assume it's for v2. I'd say this is for -fixes. The problem
>> > existed even in 3.16, but only the MST support made it apparent with the
>> > extra HPD signaling and DP AUX activity during suspend.
>> 
>> The series no longer applies on any branch. Would you mind respinning on
>> -fixes please?
>
> Ok. I just noticed that it depends on the following two patches that are
> only in -nightly not in -fixes:
>
> "drm/i915: Introduce a for_each_intel_encoder() macro"
> "drm/i915: lock around link status and link training."
>
> Are you going to apply these? The second one is definitely needed in
> -fixes imo.

I've rebased -fixes on top of 3.17-rc1, so the second patch is there
now. However patch 3 still doesn't apply. Also, if we're going to queue
these for stable to fix 3.16, it's easier if we don't depend on
for_each_intel_encoder. Could you rebase this on -fixes now please?

Thanks,
Jani.



-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx