[PATCH v3 0/5] HPD support during suspend for BXT/APL.

[PATCH v3 0/5] HPD support during suspend for BXT/APL.

[Animesh Manna]

Along with below patches sharing some background details/design.
- On BXT, Display cannot generate an interrupt when in D3.
- Without display in D3, S0ix can be achieved, Power impact will be
zero if d3 is blocked. PMCSR for Graphics/Display is irrelevant, as
the power management for them is taken care by the PUNIT using RC6/DC9
hints and *not* through PMCSR write trigger.

So solution is based on below principles:
- Display should not be put into D3 for HPD to work.
- With D0+DC9 we can achieve S0ix and at the same time helps to get the
interrupt.
- Using pci_save_state() during suspend to take control from OSPM and
blocking D3, while resuming, giving back to OSPM by pci_restore_state(). 
- _DSM method is used to program pmc hpd control register to enable
hpd during suspend.

Please have a look and send your comments/suggestions.

v1: Initial version sent as RFC.

v2: Updated version after addressing review comments from Imre,David.

v3: Updated version after addressing review commnets from Imre, Ville,
    Chris.

Animesh Manna (5):
  drm/i915/bxt: Corrected the guid for bxt.
  drm/i915/bxt: VBT changes for hpd as wakeup feature
  drm/i915/bxt: Added _DSM call to set HPD_CTL.
  drm/i915/bxt: Block D3 during suspend.
  drm/i915: Enable HPD interrupts with master ctl interrupt

 drivers/gpu/drm/i915/i915_drv.c       |  5 +++
 drivers/gpu/drm/i915/i915_drv.h       |  3 ++
 drivers/gpu/drm/i915/i915_irq.c       | 73 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h       |  8 ++++
 drivers/gpu/drm/i915/intel_acpi.c     | 54 ++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_bios.c     |  4 ++
 drivers/gpu/drm/i915/intel_vbt_defs.h |  3 +-
 7 files changed, 142 insertions(+), 8 deletions(-)

-- 
2.7.4

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

[PATCH v3 1/5] drm/i915/bxt: Corrected the guid for bxt.

[Animesh Manna]

Guid is changed for bxt platform, so corrected the guid for bxt.

v1: Initial version as RFC.

v2: Based on review comment from Jani and David,
have kept guid as binary format.

v3: Based on review comment from Chris and Jani,
    - kept the const for guid.
    - corrected the file permission.

Signed-off-by: Ananth Krishna R <ananth.krishna.r@intel.com>
Signed-off-by: Bharath K Veera <bharath.k.veera@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/intel_acpi.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
index eb638a1..5c49cef 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -23,6 +23,14 @@ static const u8 intel_dsm_guid[] = {
 	0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
 };
 
+static const u8 intel_dsm_guid_bxt[] = {
+	0xc6, 0x41, 0x5b, 0x3e,
+	0x1d, 0xeb,
+	0x60, 0x42,
+	0x9d, 0x15,
+	0xc7, 0x1f, 0xba, 0xda, 0xe4, 0x14
+};
+
 static char *intel_dsm_port_name(u8 id)
 {
 	switch (id) {
@@ -113,20 +121,30 @@ static void intel_dsm_platform_mux_info(void)
 static bool intel_dsm_pci_probe(struct pci_dev *pdev)
 {
 	acpi_handle dhandle;
+	struct drm_device *dev = pci_get_drvdata(pdev);
+	struct drm_i915_private *dev_priv = to_i915(dev);
 
 	dhandle = ACPI_HANDLE(&pdev->dev);
 	if (!dhandle)
 		return false;
 
-	if (!acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
-			    1 << INTEL_DSM_FN_PLATFORM_MUX_INFO)) {
-		DRM_DEBUG_KMS("no _DSM method for intel device\n");
-		return false;
-	}
-
 	intel_dsm_priv.dhandle = dhandle;
-	intel_dsm_platform_mux_info();
 
+	if (IS_BROXTON(dev_priv)) {
+		union acpi_object *obj;
+
+		obj = acpi_evaluate_dsm(dhandle, intel_dsm_guid_bxt,
+					INTEL_DSM_REVISION_ID, 0, NULL);
+		if (!obj)
+			return false;
+	} else {
+		if (acpi_check_dsm(dhandle, intel_dsm_guid,
+				    INTEL_DSM_REVISION_ID,
+				    1 << INTEL_DSM_FN_PLATFORM_MUX_INFO))
+			intel_dsm_platform_mux_info();
+		else
+			return false;
+	}
 	return true;
 }
 
-- 
2.7.4

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

[PATCH v3 2/5] drm/i915/bxt: VBT changes for hpd as wakeup feature

[Animesh Manna]

To support hpd during sleep a new feature flag is
added in vbt and also in dev_priv for enabling/disabling
inside driver. By default this feature will be
disabled and based on oem request this feature can
be enabled by changing vbt feature flag.

v1: Initial version as RFC.

v2: Based on review comments from Jani,
- Used bool instead of enum for hpd feature flag.
- Updating feature flag at the first place based on vbt
entry.

v3: Based on review comment from Jani,
    - removed the if condition check, directly update driver flag
    with vbt feature flag.

Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       | 3 +++
 drivers/gpu/drm/i915/intel_bios.c     | 4 ++++
 drivers/gpu/drm/i915/intel_vbt_defs.h | 3 ++-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 77d7a07..651a013 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1743,6 +1743,9 @@ struct intel_vbt_data {
 		const u8 *sequence[MIPI_SEQ_MAX];
 	} dsi;
 
+	/* HPD as wakesoure for DC9 BXT */
+	bool hpd_wakeup_enabled;
+
 	int crt_ddc_pin;
 
 	int child_dev_num;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 1cf2fa6..aacbd62 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -559,6 +559,10 @@ parse_driver_features(struct drm_i915_private *dev_priv,
 	if (driver->lvds_config == BDB_DRIVER_FEATURE_EDP)
 		dev_priv->vbt.edp.support = 1;
 
+	dev_priv->vbt.hpd_wakeup_enabled = driver->hpd_wakeup_source;
+	DRM_DEBUG_KMS("HPD as wakeup source:%s\n",
+		      yesno(driver->hpd_wakeup_source));
+
 	DRM_DEBUG_KMS("DRRS State Enabled:%d\n", driver->drrs_enabled);
 	/*
 	 * If DRRS is not supported, drrs_type has to be set to 0.
diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
index 8886cab1..2072134 100644
--- a/drivers/gpu/drm/i915/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
@@ -561,7 +561,8 @@ struct bdb_driver_features {
 	u16 tbt_enabled:1;
 	u16 psr_enabled:1;
 	u16 ips_enabled:1;
-	u16 reserved3:4;
+	u16 reserved3:3;
+	u16 hpd_wakeup_source:1;
 	u16 pc_feature_valid:1;
 } __packed;
 
-- 
2.7.4

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

[PATCH v3 3/5] drm/i915/bxt: Added _DSM call to set HPD_CTL.

[Animesh Manna]

_DSM is added to program HPD_CTL(0x1094) register
of PMC from i915 driver which will be called
based on driver feature flag.

During S0ix display engine is completely powered down. Hot plug will be
missed by display HPD detection logic. GPIO unit detect any edge on HPD
pin and then triggers PMC->Punit to power up and restore display.
By programming PMC hpd control register we are enabling the manual
trigger from Punit which will be done by writing the PM interrupt register.

Note: Display engine continue to get HPD interrupt even after DC9 but
the system is in S0 state (VNN will be on).

v1: Initial version as RFC.

v2: Modified the commit message.

Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/intel_acpi.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
index 5c49cef..47b8bbd 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -10,6 +10,8 @@
 
 #define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */
 #define INTEL_DSM_FN_PLATFORM_MUX_INFO 1 /* No args */
+#define INTEL_DSM_SET_HPD_WAKEUP 17
+#define HPD_WAKEUP_EN_VAL 0xFCF0
 
 static struct intel_dsm_priv {
 	acpi_handle dhandle;
@@ -118,6 +120,25 @@ static void intel_dsm_platform_mux_info(void)
 	ACPI_FREE(pkg);
 }
 
+static void intel_dsm_set_hpd_wakeup(const u8 *guid)
+{
+	union acpi_object *obj;
+	union acpi_object argv4 = {
+		.integer.type = ACPI_TYPE_INTEGER,
+		.integer.value = HPD_WAKEUP_EN_VAL,
+	};
+
+	obj = acpi_evaluate_dsm_typed(intel_dsm_priv.dhandle, guid,
+			INTEL_DSM_REVISION_ID, INTEL_DSM_SET_HPD_WAKEUP,
+			&argv4, ACPI_TYPE_INTEGER);
+
+	if (!obj)
+		DRM_DEBUG_DRIVER("failed to evaluate _DSM\n");
+
+	ACPI_FREE(obj);
+}
+
+
 static bool intel_dsm_pci_probe(struct pci_dev *pdev)
 {
 	acpi_handle dhandle;
@@ -131,11 +152,12 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
 	intel_dsm_priv.dhandle = dhandle;
 
 	if (IS_BROXTON(dev_priv)) {
-		union acpi_object *obj;
-
-		obj = acpi_evaluate_dsm(dhandle, intel_dsm_guid_bxt,
-					INTEL_DSM_REVISION_ID, 0, NULL);
-		if (!obj)
+		if (dev_priv->vbt.hpd_wakeup_enabled &&
+		    acpi_check_dsm(dhandle, intel_dsm_guid_bxt,
+				   INTEL_DSM_REVISION_ID,
+				   1 << INTEL_DSM_SET_HPD_WAKEUP))
+			intel_dsm_set_hpd_wakeup(intel_dsm_guid_bxt);
+		else
 			return false;
 	} else {
 		if (acpi_check_dsm(dhandle, intel_dsm_guid,
-- 
2.7.4

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

[PATCH v3 4/5] drm/i915/bxt: Block D3 during suspend.

[Animesh Manna]

For BXT, display engine can not generate interrupt when in D3.
On the othen hand S0ix can be achieved without display in D3. So,
Display should not put into D3 for HPD to work and will not
have any power impact.

v1: Initial version as RFC.

v2: Based on review comment from David,
condition check for hpd_wakeup_enabled is removed before
calling pci_restore_state.

Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6428588..e63b5f5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2382,6 +2382,9 @@ static int intel_runtime_suspend(struct device *kdev)
 	if (!IS_VALLEYVIEW(dev_priv) || !IS_CHERRYVIEW(dev_priv))
 		intel_hpd_poll_init(dev_priv);
 
+	if (dev_priv->vbt.hpd_wakeup_enabled)
+		pci_save_state(pdev);
+
 	DRM_DEBUG_KMS("Device suspended\n");
 	return 0;
 }
@@ -2398,6 +2401,8 @@ static int intel_runtime_resume(struct device *kdev)
 
 	DRM_DEBUG_KMS("Resuming device\n");
 
+	pci_restore_state(pdev);
+
 	WARN_ON_ONCE(atomic_read(&dev_priv->pm.wakeref_count));
 	disable_rpm_wakeref_asserts(dev_priv);
 
-- 
2.7.4

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

[PATCH v3 5/5] drm/i915: Enable HPD interrupts with master ctl interrupt

[Animesh Manna]

While suspending the device hpd related interrupts are enabled
to get the interrupt when device is in suspend state.

Though display is in DC9 but system can be in S0 or S0i3 state.
Hot plug during S0 state will generate de_port_interrupt but if
system is in S0i3 state then display driver will get hotplug
interrupt as pcu_hpd_interrupt which will come via pmc. So
added the interrupt handling for pcu hpd interrupt.

v1: Initial version as RFC.

v2: Based on review comments from Imre, Ville,
    - separate hpd_bxt_pcu structure added.
    - pcu_irq_ack/pcu_irq_handler pair added to handle pcu interrupt.
    - sequence for enabling pcu interrupt is modified.
    - Removed the definition of pcu interrupt mask bit as it is matching
    with enable bit.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 73 +++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h |  8 +++++
 2 files changed, 81 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a0e70f5..e5de22e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -115,6 +115,12 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
 	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
 };
 
+static const u32 hpd_bxt_pcu[HPD_NUM_PINS] = {
+	[HPD_PORT_A] = BXT_PCU_DC9_HP_DDIA,
+	[HPD_PORT_B] = BXT_PCU_DC9_HP_DDIB,
+	[HPD_PORT_C] = BXT_PCU_DC9_HP_DDIC
+};
+
 /* IIR can theoretically queue up two events. Be paranoid. */
 #define GEN8_IRQ_RESET_NDX(type, which) do { \
 	I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
@@ -2537,12 +2543,49 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 	return ret;
 }
 
+static irqreturn_t
+gen8_pcu_irq_ack(struct drm_i915_private *dev_priv,
+		 u32 master_ctl, u32 *pcu_iir)
+{
+	irqreturn_t ret = IRQ_NONE;
+
+	if (master_ctl & GEN8_PCU_IRQ) {
+		*pcu_iir = I915_READ(GEN8_PCU_IIR);
+
+		if (*pcu_iir) {
+			I915_WRITE(GEN8_PCU_IIR, *pcu_iir);
+			ret = IRQ_HANDLED;
+		} else {
+			DRM_ERROR("The master control interrupt lied (PCU)!\n");
+		}
+	}
+
+	return ret;
+}
+
+static void
+gen8_pcu_irq_handler(struct drm_i915_private *dev_priv, u32 pcu_iir)
+{
+	bool found = false;
+
+	if (IS_BROXTON(dev_priv) && (pcu_iir & BXT_PCU_DC9_HOTPLUG_MASK)) {
+		u32 tmp_mask = pcu_iir & BXT_PCU_DC9_HOTPLUG_MASK;
+
+		bxt_hpd_irq_handler(dev_priv, tmp_mask,	hpd_bxt_pcu);
+		found = true;
+	}
+
+	if (!found)
+		DRM_ERROR("Unexpected PCU interrupt\n");
+}
+
 static irqreturn_t gen8_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	u32 master_ctl;
 	u32 gt_iir[4] = {};
+	u32 pcu_iir = 0;
 	irqreturn_t ret;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -2560,7 +2603,9 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
 
 	/* Find, clear, then process each source of interrupt */
 	ret = gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir);
+	ret |= gen8_pcu_irq_ack(dev_priv, master_ctl, &pcu_iir);
 	gen8_gt_irq_handler(dev_priv, gt_iir);
+	gen8_pcu_irq_handler(dev_priv, pcu_iir);
 	ret |= gen8_de_irq_handler(dev_priv, master_ctl);
 
 	I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
@@ -4290,6 +4335,17 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
 	dev_priv->pm.irqs_enabled = false;
 }
 
+static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv)
+{
+	u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier;
+
+	de_pcu_hpd_enable_mask = BXT_PCU_DC9_HOTPLUG_MASK;
+
+	de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & ~de_pcu_hpd_enable_mask);
+	de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask);
+	GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier);
+}
+
 /**
  * intel_runtime_pm_disable_interrupts - runtime interrupt disabling
  * @dev_priv: i915 device instance
@@ -4302,6 +4358,23 @@ void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
 	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
 	dev_priv->pm.irqs_enabled = false;
 	synchronize_irq(dev_priv->drm.irq);
+
+	if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) {
+		unsigned long flags = 0;
+
+		/* Enable HPD related interrupts during DC9 for HPD wakeup */
+		spin_lock_irqsave(&dev_priv->irq_lock, flags);
+		if (dev_priv->display.hpd_irq_setup)
+			dev_priv->display.hpd_irq_setup(dev_priv);
+		spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+
+		bxt_enable_pcu_interrupt(dev_priv);
+
+		dev_priv->pm.irqs_enabled = true;
+		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
+		POSTING_READ(GEN8_MASTER_IRQ);
+
+	}
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8e47b59..19717c0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6402,6 +6402,14 @@ enum {
 #define GEN8_PCU_IIR _MMIO(0x444e8)
 #define GEN8_PCU_IER _MMIO(0x444ec)
 
+/* BXT PCU DC9 hotplug control */
+#define BXT_PCU_DC9_HP_DDIA		(1<<31)
+#define BXT_PCU_DC9_HP_DDIB		(1<<30)
+#define BXT_PCU_DC9_HP_DDIC		(1<<29)
+#define BXT_PCU_DC9_HOTPLUG_MASK	(BXT_PCU_DC9_HP_DDIA | \
+					 BXT_PCU_DC9_HP_DDIB | \
+					 BXT_PCU_DC9_HP_DDIC)
+
 #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
 /* Required on all Ironlake and Sandybridge according to the B-Spec. */
 #define  ILK_ELPIN_409_SELECT	(1 << 25)
-- 
2.7.4

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

✗ Fi.CI.BAT: failure for HPD support during suspend for BXT/APL. (rev2)

[Patchwork]

== Series Details ==

Series: HPD support during suspend for BXT/APL. (rev2)
URL   : https://patchwork.freedesktop.org/series/15833/
State : failure

== Summary ==

Series 15833v2 HPD support during suspend for BXT/APL.
https://patchwork.freedesktop.org/api/1.0/series/15833/revisions/2/mbox/

Test core_auth:
        Subgroup basic-auth:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-bxt-t5700)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-skl-6700k)
Test core_prop_blob:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-bxt-t5700)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-skl-6700k)
Test drv_getparams_basic:
        Subgroup basic-eu-total:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-bxt-t5700)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-skl-6700k)
        Subgroup basic-subslice-total:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-bxt-t5700)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-skl-6700k)
Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-bxt-t5700)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-skl-6700k)
Test drv_module_reload:
        Subgroup basic-reload:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-bxt-t5700)
        Subgroup basic-reload-final:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-bxt-t5700)
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-bxt-t5700)
Test gem_basic:
        Subgroup bad-close:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-bxt-t5700)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-skl-6700k)
        Subgroup create-close:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-bxt-t5700)
                pass       -> DMESG-WARN (fi-bdw-5557u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-skl-6700k)
        Subgroup create-fd-close:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-bxt-t5700)
WARNING: Long output truncated

60729d452290d290cd6c9553ff55402508d1d6b0 drm-tip: 2016y-12m-27d-11h-03m-47s UTC integration manifest
f213d38 drm/i915: Enable HPD interrupts with master ctl interrupt
1183d20 drm/i915/bxt: Block D3 during suspend.
cff7f17 drm/i915/bxt: Added _DSM call to set HPD_CTL.
fd5b018 drm/i915/bxt: VBT changes for hpd as wakeup feature
efaff90 drm/i915/bxt: Corrected the guid for bxt.

== Logs ==

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

[PATCH v3 5/5] drm/i915: Enable HPD interrupts with master ctl interrupt

[Animesh Manna]

While suspending the device hpd related interrupts are enabled
to get the interrupt when device is in suspend state.

Though display is in DC9 but system can be in S0 or S0i3 state.
Hot plug during S0 state will generate de_port_interrupt but if
system is in S0i3 state then display driver will get hotplug
interrupt as pcu_hpd_interrupt which will come via pmc. So
added the interrupt handling for pcu hpd interrupt.

v1: Initial version as RFC.

v2: Based on review comments from Imre, Ville,
    - separate hpd_bxt_pcu structure added.
    - pcu_irq_ack/pcu_irq_handler pair added to handle pcu interrupt.
    - sequence for enabling pcu interrupt is modified.
    - Removed the definition of pcu interrupt mask bit as it is matching
    with enable bit.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 73 +++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h |  8 +++++
 2 files changed, 81 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a0e70f5..6b34d88 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -115,6 +115,12 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
 	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
 };
 
+static const u32 hpd_bxt_pcu[HPD_NUM_PINS] = {
+	[HPD_PORT_A] = BXT_PCU_DC9_HP_DDIA,
+	[HPD_PORT_B] = BXT_PCU_DC9_HP_DDIB,
+	[HPD_PORT_C] = BXT_PCU_DC9_HP_DDIC
+};
+
 /* IIR can theoretically queue up two events. Be paranoid. */
 #define GEN8_IRQ_RESET_NDX(type, which) do { \
 	I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
@@ -2537,12 +2543,49 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 	return ret;
 }
 
+static irqreturn_t
+gen8_pcu_irq_ack(struct drm_i915_private *dev_priv,
+		 u32 master_ctl, u32 *pcu_iir)
+{
+	irqreturn_t ret = IRQ_NONE;
+
+	if (master_ctl & GEN8_PCU_IRQ) {
+		*pcu_iir = I915_READ(GEN8_PCU_IIR);
+
+		if (*pcu_iir) {
+			I915_WRITE(GEN8_PCU_IIR, *pcu_iir);
+			ret = IRQ_HANDLED;
+		} else {
+			DRM_ERROR("The master control interrupt lied (PCU)!\n");
+		}
+	}
+
+	return ret;
+}
+
+static void
+gen8_pcu_irq_handler(struct drm_i915_private *dev_priv, u32 pcu_iir)
+{
+	bool found = false;
+
+	if (IS_BROXTON(dev_priv) && (pcu_iir & BXT_PCU_DC9_HOTPLUG_MASK)) {
+		u32 tmp_mask = pcu_iir & BXT_PCU_DC9_HOTPLUG_MASK;
+
+		bxt_hpd_irq_handler(dev_priv, tmp_mask,	hpd_bxt_pcu);
+		found = true;
+	}
+
+	if ((!found) && pcu_iir)
+		DRM_ERROR("Unexpected PCU interrupt 0x%x\n", pcu_iir);
+}
+
 static irqreturn_t gen8_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	u32 master_ctl;
 	u32 gt_iir[4] = {};
+	u32 pcu_iir = 0;
 	irqreturn_t ret;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -2560,7 +2603,9 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
 
 	/* Find, clear, then process each source of interrupt */
 	ret = gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir);
+	ret |= gen8_pcu_irq_ack(dev_priv, master_ctl, &pcu_iir);
 	gen8_gt_irq_handler(dev_priv, gt_iir);
+	gen8_pcu_irq_handler(dev_priv, pcu_iir);
 	ret |= gen8_de_irq_handler(dev_priv, master_ctl);
 
 	I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
@@ -4290,6 +4335,17 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
 	dev_priv->pm.irqs_enabled = false;
 }
 
+static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv)
+{
+	u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier;
+
+	de_pcu_hpd_enable_mask = BXT_PCU_DC9_HOTPLUG_MASK;
+
+	de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & ~de_pcu_hpd_enable_mask);
+	de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask);
+	GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier);
+}
+
 /**
  * intel_runtime_pm_disable_interrupts - runtime interrupt disabling
  * @dev_priv: i915 device instance
@@ -4302,6 +4358,23 @@ void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
 	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
 	dev_priv->pm.irqs_enabled = false;
 	synchronize_irq(dev_priv->drm.irq);
+
+	if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) {
+		unsigned long flags = 0;
+
+		/* Enable HPD related interrupts during DC9 for HPD wakeup */
+		spin_lock_irqsave(&dev_priv->irq_lock, flags);
+		if (dev_priv->display.hpd_irq_setup)
+			dev_priv->display.hpd_irq_setup(dev_priv);
+		spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+
+		bxt_enable_pcu_interrupt(dev_priv);
+
+		dev_priv->pm.irqs_enabled = true;
+		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
+		POSTING_READ(GEN8_MASTER_IRQ);
+
+	}
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8e47b59..19717c0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6402,6 +6402,14 @@ enum {
 #define GEN8_PCU_IIR _MMIO(0x444e8)
 #define GEN8_PCU_IER _MMIO(0x444ec)
 
+/* BXT PCU DC9 hotplug control */
+#define BXT_PCU_DC9_HP_DDIA		(1<<31)
+#define BXT_PCU_DC9_HP_DDIB		(1<<30)
+#define BXT_PCU_DC9_HP_DDIC		(1<<29)
+#define BXT_PCU_DC9_HOTPLUG_MASK	(BXT_PCU_DC9_HP_DDIA | \
+					 BXT_PCU_DC9_HP_DDIB | \
+					 BXT_PCU_DC9_HP_DDIC)
+
 #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
 /* Required on all Ironlake and Sandybridge according to the B-Spec. */
 #define  ILK_ELPIN_409_SELECT	(1 << 25)
-- 
2.7.4

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

Re: [PATCH v3 5/5] drm/i915: Enable HPD interrupts with master ctl interrupt

[Daniel Vetter]

On Wed, Dec 28, 2016 at 01:12:09PM +0530, Animesh Manna wrote:
> While suspending the device hpd related interrupts are enabled
> to get the interrupt when device is in suspend state.
> 
> Though display is in DC9 but system can be in S0 or S0i3 state.
> Hot plug during S0 state will generate de_port_interrupt but if
> system is in S0i3 state then display driver will get hotplug
> interrupt as pcu_hpd_interrupt which will come via pmc. So
> added the interrupt handling for pcu hpd interrupt.
> 
> v1: Initial version as RFC.
> 
> v2: Based on review comments from Imre, Ville,
>     - separate hpd_bxt_pcu structure added.
>     - pcu_irq_ack/pcu_irq_handler pair added to handle pcu interrupt.
>     - sequence for enabling pcu interrupt is modified.
>     - Removed the definition of pcu interrupt mask bit as it is matching
>     with enable bit.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>

This is in-reply-to the mail from patchwork, not patch 5/5 in the series.
Which means patchwork gets confused about it. You probably now need to
resend the entire series again ...
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 73 +++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h |  8 +++++
>  2 files changed, 81 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a0e70f5..6b34d88 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -115,6 +115,12 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
>  	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
>  };
>  
> +static const u32 hpd_bxt_pcu[HPD_NUM_PINS] = {
> +	[HPD_PORT_A] = BXT_PCU_DC9_HP_DDIA,
> +	[HPD_PORT_B] = BXT_PCU_DC9_HP_DDIB,
> +	[HPD_PORT_C] = BXT_PCU_DC9_HP_DDIC
> +};
> +
>  /* IIR can theoretically queue up two events. Be paranoid. */
>  #define GEN8_IRQ_RESET_NDX(type, which) do { \
>  	I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
> @@ -2537,12 +2543,49 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  	return ret;
>  }
>  
> +static irqreturn_t
> +gen8_pcu_irq_ack(struct drm_i915_private *dev_priv,
> +		 u32 master_ctl, u32 *pcu_iir)
> +{
> +	irqreturn_t ret = IRQ_NONE;
> +
> +	if (master_ctl & GEN8_PCU_IRQ) {
> +		*pcu_iir = I915_READ(GEN8_PCU_IIR);
> +
> +		if (*pcu_iir) {
> +			I915_WRITE(GEN8_PCU_IIR, *pcu_iir);
> +			ret = IRQ_HANDLED;
> +		} else {
> +			DRM_ERROR("The master control interrupt lied (PCU)!\n");
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static void
> +gen8_pcu_irq_handler(struct drm_i915_private *dev_priv, u32 pcu_iir)
> +{
> +	bool found = false;
> +
> +	if (IS_BROXTON(dev_priv) && (pcu_iir & BXT_PCU_DC9_HOTPLUG_MASK)) {
> +		u32 tmp_mask = pcu_iir & BXT_PCU_DC9_HOTPLUG_MASK;
> +
> +		bxt_hpd_irq_handler(dev_priv, tmp_mask,	hpd_bxt_pcu);
> +		found = true;
> +	}
> +
> +	if ((!found) && pcu_iir)
> +		DRM_ERROR("Unexpected PCU interrupt 0x%x\n", pcu_iir);
> +}
> +
>  static irqreturn_t gen8_irq_handler(int irq, void *arg)
>  {
>  	struct drm_device *dev = arg;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	u32 master_ctl;
>  	u32 gt_iir[4] = {};
> +	u32 pcu_iir = 0;
>  	irqreturn_t ret;
>  
>  	if (!intel_irqs_enabled(dev_priv))
> @@ -2560,7 +2603,9 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
>  
>  	/* Find, clear, then process each source of interrupt */
>  	ret = gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir);
> +	ret |= gen8_pcu_irq_ack(dev_priv, master_ctl, &pcu_iir);
>  	gen8_gt_irq_handler(dev_priv, gt_iir);
> +	gen8_pcu_irq_handler(dev_priv, pcu_iir);
>  	ret |= gen8_de_irq_handler(dev_priv, master_ctl);
>  
>  	I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> @@ -4290,6 +4335,17 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
>  	dev_priv->pm.irqs_enabled = false;
>  }
>  
> +static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv)
> +{
> +	u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier;
> +
> +	de_pcu_hpd_enable_mask = BXT_PCU_DC9_HOTPLUG_MASK;
> +
> +	de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & ~de_pcu_hpd_enable_mask);
> +	de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask);
> +	GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier);
> +}
> +
>  /**
>   * intel_runtime_pm_disable_interrupts - runtime interrupt disabling
>   * @dev_priv: i915 device instance
> @@ -4302,6 +4358,23 @@ void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
>  	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
>  	dev_priv->pm.irqs_enabled = false;
>  	synchronize_irq(dev_priv->drm.irq);
> +
> +	if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) {
> +		unsigned long flags = 0;
> +
> +		/* Enable HPD related interrupts during DC9 for HPD wakeup */
> +		spin_lock_irqsave(&dev_priv->irq_lock, flags);
> +		if (dev_priv->display.hpd_irq_setup)
> +			dev_priv->display.hpd_irq_setup(dev_priv);
> +		spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> +
> +		bxt_enable_pcu_interrupt(dev_priv);
> +
> +		dev_priv->pm.irqs_enabled = true;
> +		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> +		POSTING_READ(GEN8_MASTER_IRQ);
> +
> +	}
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8e47b59..19717c0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6402,6 +6402,14 @@ enum {
>  #define GEN8_PCU_IIR _MMIO(0x444e8)
>  #define GEN8_PCU_IER _MMIO(0x444ec)
>  
> +/* BXT PCU DC9 hotplug control */
> +#define BXT_PCU_DC9_HP_DDIA		(1<<31)
> +#define BXT_PCU_DC9_HP_DDIB		(1<<30)
> +#define BXT_PCU_DC9_HP_DDIC		(1<<29)
> +#define BXT_PCU_DC9_HOTPLUG_MASK	(BXT_PCU_DC9_HP_DDIA | \
> +					 BXT_PCU_DC9_HP_DDIB | \
> +					 BXT_PCU_DC9_HP_DDIC)
> +
>  #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
>  /* Required on all Ironlake and Sandybridge according to the B-Spec. */
>  #define  ILK_ELPIN_409_SELECT	(1 << 25)
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 0/5] HPD support during suspend for BXT/APL.

[Imre Deak]

On Tue, Dec 27, 2016 at 06:59:51PM +0530, Animesh Manna wrote:
> Along with below patches sharing some background details/design.
> - On BXT, Display cannot generate an interrupt when in D3.
> - Without display in D3, S0ix can be achieved, Power impact will be
> zero if d3 is blocked. PMCSR for Graphics/Display is irrelevant, as
> the power management for them is taken care by the PUNIT using RC6/DC9
> hints and *not* through PMCSR write trigger.
> 
> So solution is based on below principles:
> - Display should not be put into D3 for HPD to work.
> - With D0+DC9 we can achieve S0ix and at the same time helps to get the
> interrupt.
> - Using pci_save_state() during suspend to take control from OSPM and
> blocking D3, while resuming, giving back to OSPM by pci_restore_state(). 
> - _DSM method is used to program pmc hpd control register to enable
> hpd during suspend.
> 
> Please have a look and send your comments/suggestions.

Which platform did you test this on? I tried it on both an APL/RVP and a
BXT/Joule, but couldn't see any PCU HPD interrupts generated.

> 
> v1: Initial version sent as RFC.
> 
> v2: Updated version after addressing review comments from Imre,David.
> 
> v3: Updated version after addressing review commnets from Imre, Ville,
>     Chris.
> 
> Animesh Manna (5):
>   drm/i915/bxt: Corrected the guid for bxt.
>   drm/i915/bxt: VBT changes for hpd as wakeup feature
>   drm/i915/bxt: Added _DSM call to set HPD_CTL.
>   drm/i915/bxt: Block D3 during suspend.
>   drm/i915: Enable HPD interrupts with master ctl interrupt
> 
>  drivers/gpu/drm/i915/i915_drv.c       |  5 +++
>  drivers/gpu/drm/i915/i915_drv.h       |  3 ++
>  drivers/gpu/drm/i915/i915_irq.c       | 73 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h       |  8 ++++
>  drivers/gpu/drm/i915/intel_acpi.c     | 54 ++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_bios.c     |  4 ++
>  drivers/gpu/drm/i915/intel_vbt_defs.h |  3 +-
>  7 files changed, 142 insertions(+), 8 deletions(-)
> 
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 1/5] drm/i915/bxt: Corrected the guid for bxt.

[Imre Deak]

On Tue, Dec 27, 2016 at 06:59:52PM +0530, Animesh Manna wrote:
> Guid is changed for bxt platform, so corrected the guid for bxt.
> 
> v1: Initial version as RFC.
> 
> v2: Based on review comment from Jani and David,
> have kept guid as binary format.
> 
> v3: Based on review comment from Chris and Jani,
>     - kept the const for guid.
>     - corrected the file permission.
> 
> Signed-off-by: Ananth Krishna R <ananth.krishna.r@intel.com>
> Signed-off-by: Bharath K Veera <bharath.k.veera@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_acpi.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
> index eb638a1..5c49cef 100644
> --- a/drivers/gpu/drm/i915/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/intel_acpi.c
> @@ -23,6 +23,14 @@ static const u8 intel_dsm_guid[] = {
>  	0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
>  };
>  
> +static const u8 intel_dsm_guid_bxt[] = {
> +	0xc6, 0x41, 0x5b, 0x3e,
> +	0x1d, 0xeb,
> +	0x60, 0x42,
> +	0x9d, 0x15,
> +	0xc7, 0x1f, 0xba, 0xda, 0xe4, 0x14
> +};
> +
>  static char *intel_dsm_port_name(u8 id)
>  {
>  	switch (id) {
> @@ -113,20 +121,30 @@ static void intel_dsm_platform_mux_info(void)
>  static bool intel_dsm_pci_probe(struct pci_dev *pdev)
>  {
>  	acpi_handle dhandle;
> +	struct drm_device *dev = pci_get_drvdata(pdev);
> +	struct drm_i915_private *dev_priv = to_i915(dev);
>  
>  	dhandle = ACPI_HANDLE(&pdev->dev);
>  	if (!dhandle)
>  		return false;
>  
> -	if (!acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
> -			    1 << INTEL_DSM_FN_PLATFORM_MUX_INFO)) {
> -		DRM_DEBUG_KMS("no _DSM method for intel device\n");
> -		return false;
> -	}
> -
>  	intel_dsm_priv.dhandle = dhandle;
> -	intel_dsm_platform_mux_info();
>  
> +	if (IS_BROXTON(dev_priv)) {
> +		union acpi_object *obj;
> +
> +		obj = acpi_evaluate_dsm(dhandle, intel_dsm_guid_bxt,
> +					INTEL_DSM_REVISION_ID, 0, NULL);
> +		if (!obj)
> +			return false;
> +	} else {
> +		if (acpi_check_dsm(dhandle, intel_dsm_guid,
> +				    INTEL_DSM_REVISION_ID,
> +				    1 << INTEL_DSM_FN_PLATFORM_MUX_INFO))
> +			intel_dsm_platform_mux_info();
> +		else
> +			return false;
> +	}

So the INTEL_DSM_FN_PLATFORM_MUX_INFO handling is removed for BXT, which
isn't mentioned in the commit log. It doesn't seem correct either since
it's used to display the switcheroo DSM switching method, which is
unrelated to the low-level HPD handling you are adding here. So imo just
handle the two things separately adding a new function for the HPD stuff
and calling it from intel_dsm_detect().

>  	return true;
>  }
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 3/5] drm/i915/bxt: Added _DSM call to set HPD_CTL.

[Imre Deak]

On Tue, Dec 27, 2016 at 06:59:54PM +0530, Animesh Manna wrote:
> _DSM is added to program HPD_CTL(0x1094) register
> of PMC from i915 driver which will be called
> based on driver feature flag.
> 
> During S0ix display engine is completely powered down. Hot plug will be
> missed by display HPD detection logic. GPIO unit detect any edge on HPD
> pin and then triggers PMC->Punit to power up and restore display.
> By programming PMC hpd control register we are enabling the manual
> trigger from Punit which will be done by writing the PM interrupt register.
> 
> Note: Display engine continue to get HPD interrupt even after DC9 but
> the system is in S0 state (VNN will be on).
> 
> v1: Initial version as RFC.
> 
> v2: Modified the commit message.
> 
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_acpi.c | 32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
> index 5c49cef..47b8bbd 100644
> --- a/drivers/gpu/drm/i915/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/intel_acpi.c
> @@ -10,6 +10,8 @@
>  
>  #define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */
>  #define INTEL_DSM_FN_PLATFORM_MUX_INFO 1 /* No args */
> +#define INTEL_DSM_SET_HPD_WAKEUP 17
> +#define HPD_WAKEUP_EN_VAL 0xFCF0
>  
>  static struct intel_dsm_priv {
>  	acpi_handle dhandle;
> @@ -118,6 +120,25 @@ static void intel_dsm_platform_mux_info(void)
>  	ACPI_FREE(pkg);
>  }
>  
> +static void intel_dsm_set_hpd_wakeup(const u8 *guid)
> +{
> +	union acpi_object *obj;
> +	union acpi_object argv4 = {
> +		.integer.type = ACPI_TYPE_INTEGER,
> +		.integer.value = HPD_WAKEUP_EN_VAL,
> +	};
> +
> +	obj = acpi_evaluate_dsm_typed(intel_dsm_priv.dhandle, guid,
> +			INTEL_DSM_REVISION_ID, INTEL_DSM_SET_HPD_WAKEUP,
> +			&argv4, ACPI_TYPE_INTEGER);

Did this call succeed for you? For me on all platforms I tried
it results in
ACPI Warning: \_SB.PCI0.GFX0._DSM: Argument #4 type mismatch - Found [Integer], ACPI requires [Package] (20160930/nsarguments-95)
and the above function returning NULL.

> +
> +	if (!obj)
> +		DRM_DEBUG_DRIVER("failed to evaluate _DSM\n");
> +
> +	ACPI_FREE(obj);
> +}
> +
> +
>  static bool intel_dsm_pci_probe(struct pci_dev *pdev)
>  {
>  	acpi_handle dhandle;
> @@ -131,11 +152,12 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
>  	intel_dsm_priv.dhandle = dhandle;
>  
>  	if (IS_BROXTON(dev_priv)) {
> -		union acpi_object *obj;
> -
> -		obj = acpi_evaluate_dsm(dhandle, intel_dsm_guid_bxt,
> -					INTEL_DSM_REVISION_ID, 0, NULL);
> -		if (!obj)
> +		if (dev_priv->vbt.hpd_wakeup_enabled &&
> +		    acpi_check_dsm(dhandle, intel_dsm_guid_bxt,
> +				   INTEL_DSM_REVISION_ID,
> +				   1 << INTEL_DSM_SET_HPD_WAKEUP))
> +			intel_dsm_set_hpd_wakeup(intel_dsm_guid_bxt);
> +		else
>  			return false;
>  	} else {
>  		if (acpi_check_dsm(dhandle, intel_dsm_guid,
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 5/5] drm/i915: Enable HPD interrupts with master ctl interrupt

[Imre Deak]

On Tue, Dec 27, 2016 at 06:59:56PM +0530, Animesh Manna wrote:
> While suspending the device hpd related interrupts are enabled
> to get the interrupt when device is in suspend state.
> 
> Though display is in DC9 but system can be in S0 or S0i3 state.
> Hot plug during S0 state will generate de_port_interrupt but if
> system is in S0i3 state then display driver will get hotplug
> interrupt as pcu_hpd_interrupt which will come via pmc. So
> added the interrupt handling for pcu hpd interrupt.
> 
> v1: Initial version as RFC.
> 
> v2: Based on review comments from Imre, Ville,
>     - separate hpd_bxt_pcu structure added.
>     - pcu_irq_ack/pcu_irq_handler pair added to handle pcu interrupt.
>     - sequence for enabling pcu interrupt is modified.
>     - Removed the definition of pcu interrupt mask bit as it is matching
>     with enable bit.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 73 +++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h |  8 +++++
>  2 files changed, 81 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a0e70f5..e5de22e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -115,6 +115,12 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
>  	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
>  };
>  
> +static const u32 hpd_bxt_pcu[HPD_NUM_PINS] = {
> +	[HPD_PORT_A] = BXT_PCU_DC9_HP_DDIA,
> +	[HPD_PORT_B] = BXT_PCU_DC9_HP_DDIB,
> +	[HPD_PORT_C] = BXT_PCU_DC9_HP_DDIC
> +};
> +
>  /* IIR can theoretically queue up two events. Be paranoid. */
>  #define GEN8_IRQ_RESET_NDX(type, which) do { \
>  	I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
> @@ -2537,12 +2543,49 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  	return ret;
>  }
>  
> +static irqreturn_t
> +gen8_pcu_irq_ack(struct drm_i915_private *dev_priv,
> +		 u32 master_ctl, u32 *pcu_iir)
> +{
> +	irqreturn_t ret = IRQ_NONE;
> +
> +	if (master_ctl & GEN8_PCU_IRQ) {
> +		*pcu_iir = I915_READ(GEN8_PCU_IIR);
> +
> +		if (*pcu_iir) {
> +			I915_WRITE(GEN8_PCU_IIR, *pcu_iir);
> +			ret = IRQ_HANDLED;
> +		} else {
> +			DRM_ERROR("The master control interrupt lied (PCU)!\n");
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static void
> +gen8_pcu_irq_handler(struct drm_i915_private *dev_priv, u32 pcu_iir)
> +{
> +	bool found = false;
> +
> +	if (IS_BROXTON(dev_priv) && (pcu_iir & BXT_PCU_DC9_HOTPLUG_MASK)) {
> +		u32 tmp_mask = pcu_iir & BXT_PCU_DC9_HOTPLUG_MASK;
> +
> +		bxt_hpd_irq_handler(dev_priv, tmp_mask,	hpd_bxt_pcu);

As Ville already pointed out this won't be able to distinguish between
long and short pulses. From BSpec:
"""
During DC9 hardware will dynamically transition between display engine
powered on and off, so hotplug may be detected by PMC or display engine,
and hardware will not reliably differentiate between short and long
pulse hotplug events.
"""

So it needs its own handler that assumes a long HPD happened without
relying on PCH_PORT_HOTPLUG.

> +		found = true;
> +	}
> +
> +	if (!found)
> +		DRM_ERROR("Unexpected PCU interrupt\n");
> +}
> +
>  static irqreturn_t gen8_irq_handler(int irq, void *arg)
>  {
>  	struct drm_device *dev = arg;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	u32 master_ctl;
>  	u32 gt_iir[4] = {};
> +	u32 pcu_iir = 0;
>  	irqreturn_t ret;
>  
>  	if (!intel_irqs_enabled(dev_priv))
> @@ -2560,7 +2603,9 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
>  
>  	/* Find, clear, then process each source of interrupt */
>  	ret = gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir);
> +	ret |= gen8_pcu_irq_ack(dev_priv, master_ctl, &pcu_iir);
>  	gen8_gt_irq_handler(dev_priv, gt_iir);
> +	gen8_pcu_irq_handler(dev_priv, pcu_iir);
>  	ret |= gen8_de_irq_handler(dev_priv, master_ctl);
>  
>  	I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> @@ -4290,6 +4335,17 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
>  	dev_priv->pm.irqs_enabled = false;
>  }
>  
> +static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv)
> +{
> +	u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier;
> +
> +	de_pcu_hpd_enable_mask = BXT_PCU_DC9_HOTPLUG_MASK;
> +
> +	de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & ~de_pcu_hpd_enable_mask);
> +	de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask);

No need for the outer parentheses.

> +	GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier);
> +}
> +
>  /**
>   * intel_runtime_pm_disable_interrupts - runtime interrupt disabling
>   * @dev_priv: i915 device instance
> @@ -4302,6 +4358,23 @@ void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
>  	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
>  	dev_priv->pm.irqs_enabled = false;
>  	synchronize_irq(dev_priv->drm.irq);
> +
> +	if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) {
> +		unsigned long flags = 0;
> +
> +		/* Enable HPD related interrupts during DC9 for HPD wakeup */
> +		spin_lock_irqsave(&dev_priv->irq_lock, flags);
> +		if (dev_priv->display.hpd_irq_setup)
> +			dev_priv->display.hpd_irq_setup(dev_priv);

This will complain now that interrupts have been disabled already.

> +		spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> +
> +		bxt_enable_pcu_interrupt(dev_priv);
> +
> +		dev_priv->pm.irqs_enabled = true;

This will cause a WARN in assert_can_enable_dc9() about interrupts being
enabled. You can just remove that, since its assumption doesn't hold any
more.

> +		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> +		POSTING_READ(GEN8_MASTER_IRQ);
> +
> +	}
>  }

What about the corresponding disabling of the HPD PCU interrupt during
resume?

intel_runtime_pm_disable_interrupts() is called during S3/S4 too, I
don't think we want to enable HPD for those states too.

After this both HPD IRQs and HPD polling will be enabled, we only want
HPD IRQs.

>  
>  /**
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8e47b59..19717c0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6402,6 +6402,14 @@ enum {
>  #define GEN8_PCU_IIR _MMIO(0x444e8)
>  #define GEN8_PCU_IER _MMIO(0x444ec)
>  
> +/* BXT PCU DC9 hotplug control */
> +#define BXT_PCU_DC9_HP_DDIA		(1<<31)
> +#define BXT_PCU_DC9_HP_DDIB		(1<<30)
> +#define BXT_PCU_DC9_HP_DDIC		(1<<29)
> +#define BXT_PCU_DC9_HOTPLUG_MASK	(BXT_PCU_DC9_HP_DDIA | \
> +					 BXT_PCU_DC9_HP_DDIB | \
> +					 BXT_PCU_DC9_HP_DDIC)
> +
>  #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
>  /* Required on all Ironlake and Sandybridge according to the B-Spec. */
>  #define  ILK_ELPIN_409_SELECT	(1 << 25)
> -- 
> 2.7.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 5/5] drm/i915: Enable HPD interrupts with master ctl interrupt

[Imre Deak]

On Fri, Feb 10, 2017 at 07:37:25PM +0200, Imre Deak wrote:
> On Tue, Dec 27, 2016 at 06:59:56PM +0530, Animesh Manna wrote:
> > While suspending the device hpd related interrupts are enabled
> > to get the interrupt when device is in suspend state.
> > 
> > Though display is in DC9 but system can be in S0 or S0i3 state.
> > Hot plug during S0 state will generate de_port_interrupt but if
> > system is in S0i3 state then display driver will get hotplug
> > interrupt as pcu_hpd_interrupt which will come via pmc. So
> > added the interrupt handling for pcu hpd interrupt.
> > 
> > v1: Initial version as RFC.
> > 
> > v2: Based on review comments from Imre, Ville,
> >     - separate hpd_bxt_pcu structure added.
> >     - pcu_irq_ack/pcu_irq_handler pair added to handle pcu interrupt.
> >     - sequence for enabling pcu interrupt is modified.
> >     - Removed the definition of pcu interrupt mask bit as it is matching
> >     with enable bit.
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 73 +++++++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_reg.h |  8 +++++
> >  2 files changed, 81 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index a0e70f5..e5de22e 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -115,6 +115,12 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
> >  	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
> >  };
> >  
> > +static const u32 hpd_bxt_pcu[HPD_NUM_PINS] = {
> > +	[HPD_PORT_A] = BXT_PCU_DC9_HP_DDIA,
> > +	[HPD_PORT_B] = BXT_PCU_DC9_HP_DDIB,
> > +	[HPD_PORT_C] = BXT_PCU_DC9_HP_DDIC
> > +};
> > +
> >  /* IIR can theoretically queue up two events. Be paranoid. */
> >  #define GEN8_IRQ_RESET_NDX(type, which) do { \
> >  	I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
> > @@ -2537,12 +2543,49 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> >  	return ret;
> >  }
> >  
> > +static irqreturn_t
> > +gen8_pcu_irq_ack(struct drm_i915_private *dev_priv,
> > +		 u32 master_ctl, u32 *pcu_iir)
> > +{
> > +	irqreturn_t ret = IRQ_NONE;
> > +
> > +	if (master_ctl & GEN8_PCU_IRQ) {
> > +		*pcu_iir = I915_READ(GEN8_PCU_IIR);
> > +
> > +		if (*pcu_iir) {
> > +			I915_WRITE(GEN8_PCU_IIR, *pcu_iir);
> > +			ret = IRQ_HANDLED;
> > +		} else {
> > +			DRM_ERROR("The master control interrupt lied (PCU)!\n");
> > +		}
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static void
> > +gen8_pcu_irq_handler(struct drm_i915_private *dev_priv, u32 pcu_iir)
> > +{
> > +	bool found = false;
> > +
> > +	if (IS_BROXTON(dev_priv) && (pcu_iir & BXT_PCU_DC9_HOTPLUG_MASK)) {
> > +		u32 tmp_mask = pcu_iir & BXT_PCU_DC9_HOTPLUG_MASK;
> > +
> > +		bxt_hpd_irq_handler(dev_priv, tmp_mask,	hpd_bxt_pcu);
> 
> As Ville already pointed out this won't be able to distinguish between
> long and short pulses. From BSpec:
> """
> During DC9 hardware will dynamically transition between display engine
> powered on and off, so hotplug may be detected by PMC or display engine,
> and hardware will not reliably differentiate between short and long
> pulse hotplug events.
> """
> 
> So it needs its own handler that assumes a long HPD happened without
> relying on PCH_PORT_HOTPLUG.
> 
> > +		found = true;
> > +	}
> > +
> > +	if (!found)
> > +		DRM_ERROR("Unexpected PCU interrupt\n");
> > +}
> > +
> >  static irqreturn_t gen8_irq_handler(int irq, void *arg)
> >  {
> >  	struct drm_device *dev = arg;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	u32 master_ctl;
> >  	u32 gt_iir[4] = {};
> > +	u32 pcu_iir = 0;
> >  	irqreturn_t ret;
> >  
> >  	if (!intel_irqs_enabled(dev_priv))
> > @@ -2560,7 +2603,9 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
> >  
> >  	/* Find, clear, then process each source of interrupt */
> >  	ret = gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir);
> > +	ret |= gen8_pcu_irq_ack(dev_priv, master_ctl, &pcu_iir);
> >  	gen8_gt_irq_handler(dev_priv, gt_iir);
> > +	gen8_pcu_irq_handler(dev_priv, pcu_iir);
> >  	ret |= gen8_de_irq_handler(dev_priv, master_ctl);
> >  
> >  	I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> > @@ -4290,6 +4335,17 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
> >  	dev_priv->pm.irqs_enabled = false;
> >  }
> >  
> > +static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv)
> > +{
> > +	u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier;
> > +
> > +	de_pcu_hpd_enable_mask = BXT_PCU_DC9_HOTPLUG_MASK;
> > +
> > +	de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & ~de_pcu_hpd_enable_mask);
> > +	de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask);
> 
> No need for the outer parentheses.
> 
> > +	GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier);
> > +}
> > +
> >  /**
> >   * intel_runtime_pm_disable_interrupts - runtime interrupt disabling
> >   * @dev_priv: i915 device instance
> > @@ -4302,6 +4358,23 @@ void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
> >  	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
> >  	dev_priv->pm.irqs_enabled = false;
> >  	synchronize_irq(dev_priv->drm.irq);
> > +
> > +	if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) {
> > +		unsigned long flags = 0;
> > +
> > +		/* Enable HPD related interrupts during DC9 for HPD wakeup */
> > +		spin_lock_irqsave(&dev_priv->irq_lock, flags);
> > +		if (dev_priv->display.hpd_irq_setup)
> > +			dev_priv->display.hpd_irq_setup(dev_priv);
> 
> This will complain now that interrupts have been disabled already.
> 
> > +		spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> > +
> > +		bxt_enable_pcu_interrupt(dev_priv);
> > +
> > +		dev_priv->pm.irqs_enabled = true;
> 
> This will cause a WARN in assert_can_enable_dc9() about interrupts being
> enabled. You can just remove that, since its assumption doesn't hold any
> more.
> 
> > +		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> > +		POSTING_READ(GEN8_MASTER_IRQ);
> > +
> > +	}
> >  }
> 
> What about the corresponding disabling of the HPD PCU interrupt during
> resume?

Ah, intel_runtime_pm_enable_interrupts() will take care of the above, so
that's fine.

> 
> intel_runtime_pm_disable_interrupts() is called during S3/S4 too, I
> don't think we want to enable HPD for those states too.
> 
> After this both HPD IRQs and HPD polling will be enabled, we only want
> HPD IRQs.
> 
> >  
> >  /**
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 8e47b59..19717c0 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6402,6 +6402,14 @@ enum {
> >  #define GEN8_PCU_IIR _MMIO(0x444e8)
> >  #define GEN8_PCU_IER _MMIO(0x444ec)
> >  
> > +/* BXT PCU DC9 hotplug control */
> > +#define BXT_PCU_DC9_HP_DDIA		(1<<31)
> > +#define BXT_PCU_DC9_HP_DDIB		(1<<30)
> > +#define BXT_PCU_DC9_HP_DDIC		(1<<29)
> > +#define BXT_PCU_DC9_HOTPLUG_MASK	(BXT_PCU_DC9_HP_DDIA | \
> > +					 BXT_PCU_DC9_HP_DDIB | \
> > +					 BXT_PCU_DC9_HP_DDIC)
> > +
> >  #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
> >  /* Required on all Ironlake and Sandybridge according to the B-Spec. */
> >  #define  ILK_ELPIN_409_SELECT	(1 << 25)
> > -- 
> > 2.7.4
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx