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

[PATCH 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.

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       | 54 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_reg.h       | 12 +++++++
 drivers/gpu/drm/i915/intel_acpi.c     | 60 +++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_bios.c     |  6 ++++
 drivers/gpu/drm/i915/intel_vbt_defs.h |  3 +-
 7 files changed, 132 insertions(+), 11 deletions(-)
 mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c
 mode change 100644 => 100755 drivers/gpu/drm/i915/intel_acpi.c

-- 
1.9.1

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

[PATCH 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.

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 | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)
 mode change 100644 => 100755 drivers/gpu/drm/i915/intel_acpi.c

diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
old mode 100644
new mode 100755
index eb638a1..8c878ab
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -15,7 +15,7 @@
 	acpi_handle dhandle;
 } intel_dsm_priv;
 
-static const u8 intel_dsm_guid[] = {
+static u8 intel_dsm_guid[] = {
 	0xd3, 0x73, 0xd8, 0x7e,
 	0xd0, 0xc2,
 	0x4f, 0x4e,
@@ -23,6 +23,14 @@
 	0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
 };
 
+static 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,12 +121,20 @@ 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 = dev->dev_private;
+	u8 *guid;
 
 	dhandle = ACPI_HANDLE(&pdev->dev);
 	if (!dhandle)
 		return false;
 
-	if (!acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
+	if (IS_BROXTON(dev_priv))
+		guid = intel_dsm_guid_bxt;
+	else
+		guid = intel_dsm_guid;
+
+	if (!acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
 			    1 << INTEL_DSM_FN_PLATFORM_MUX_INFO)) {
 		DRM_DEBUG_KMS("no _DSM method for intel device\n");
 		return false;
-- 
1.9.1

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

[PATCH 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.

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     | 6 ++++++
 drivers/gpu/drm/i915/intel_vbt_defs.h | 3 ++-
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4e7148a..1c3cf31 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1583,6 +1583,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 7ffab1a..da4a7fd 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -557,6 +557,12 @@ static int intel_bios_ssc_frequency(struct drm_i915_private *dev_priv,
 	if (driver->lvds_config == BDB_DRIVER_FEATURE_EDP)
 		dev_priv->vbt.edp.support = 1;
 
+	if (driver->hpd_wakeup_source) {
+		dev_priv->vbt.hpd_wakeup_enabled = true;
+		DRM_DEBUG_KMS("HPD as wakeup feature is enabled\n");
+	} else
+		DRM_DEBUG_KMS("HPD wakeup source feature is disabled in VBT\n");
+
 	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;
 
-- 
1.9.1

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

[PATCH 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. PMC hpd control register
programming will enable PMC to get hpd interrupt
during dc9.

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

diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
index 8c878ab..15d3b84 100755
--- 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(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;
@@ -134,14 +155,23 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
 	else
 		guid = intel_dsm_guid;
 
-	if (!acpi_check_dsm(dhandle, 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 (acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
+				1 << INTEL_DSM_FN_PLATFORM_MUX_INFO))
+		intel_dsm_platform_mux_info();
+	else
+		DRM_DEBUG_KMS("no _DSM method for mux-info\n");
+
+	/* Need to ensure vbt parsing is completed. */
+	if (dev_priv->vbt.hpd_wakeup_enabled &&
+		acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
+				1 << INTEL_DSM_SET_HPD_WAKEUP))
+		intel_dsm_set_hpd_wakeup(guid);
+	else {
+		dev_priv->vbt.hpd_wakeup_enabled = false;
+		DRM_DEBUG_KMS("no _DSM method for hpd-enabling\n");
+	}
 
 	return true;
 }
-- 
1.9.1

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

[PATCH 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 4f0e56d..bc48642 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2381,6 +2381,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;
 }
@@ -2397,6 +2400,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);
 
-- 
1.9.1

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

[PATCH 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.

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 | 56 ++++++++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++
 2 files changed, 65 insertions(+), 3 deletions(-)
 mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
old mode 100644
new mode 100755
index cb8a75f..2f9b604
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -110,9 +110,9 @@
 
 /* BXT hpd list */
 static const u32 hpd_bxt[HPD_NUM_PINS] = {
-	[HPD_PORT_A] = BXT_DE_PORT_HP_DDIA,
-	[HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
-	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
+	[HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA),
+	[HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB),
+	[HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC)
 };
 
 /* IIR can theoretically queue up two events. Be paranoid. */
@@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv,
 			DRM_ERROR("The master control interrupt lied (DE PORT)!\n");
 	}
 
+	if (master_ctl & GEN8_PCU_IRQ) {
+		iir = I915_READ(GEN8_PCU_IIR);
+		if (iir) {
+			u32 tmp_mask;
+
+			I915_WRITE(GEN8_PCU_IIR, iir);
+			ret = IRQ_HANDLED;
+			if (IS_BROXTON(dev_priv)) {
+				tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK;
+				if (tmp_mask)
+					bxt_hpd_irq_handler(dev_priv, tmp_mask,
+							hpd_bxt);
+			} else
+				DRM_ERROR("Unexpected PCU interrupt\n");
+		} else
+			DRM_ERROR("The master control interrupt lied (PCU)!\n");
+	}
+
 	for_each_pipe(dev_priv, pipe) {
 		u32 flip_done, fault_errors;
 
@@ -4294,6 +4312,19 @@ 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 = GEN9_DE_PCU_PORTA_HOTPLUG |
+				GEN9_DE_PCU_PORTB_HOTPLUG |
+				GEN9_DE_PCU_PORTC_HOTPLUG;
+
+	de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0);
+	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
@@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
  */
 void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
 {
+	unsigned long flags = 0;
+
 	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
 	dev_priv->pm.irqs_enabled = false;
+
+	if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) {
+
+		/* Enable HPD related interrupts during DC9 for HPD wakeup */
+
+		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
+		POSTING_READ(GEN8_MASTER_IRQ);
+
+		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;
+	}
 	synchronize_irq(dev_priv->drm.irq);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3361d7f..df89025 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6030,6 +6030,18 @@ 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 GEN9_DE_PCU_PORTA_HOTPLUG      (1 << 31)
+#define GEN9_DE_PCU_PORTB_HOTPLUG      (1 << 30)
+#define GEN9_DE_PCU_PORTC_HOTPLUG      (1 << 29)
+
 #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
 /* Required on all Ironlake and Sandybridge according to the B-Spec. */
 #define  ILK_ELPIN_409_SELECT	(1 << 25)
-- 
1.9.1

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

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

[Chris Wilson]

On Wed, Nov 23, 2016 at 09:48:23PM +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.
> 
> 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 | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>  mode change 100644 => 100755 drivers/gpu/drm/i915/intel_acpi.c
> 
> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
> old mode 100644
> new mode 100755

3 people handled this patch and none complained about making
intel_acpi.c executable? What does happen when you try to execute it?

> index eb638a1..8c878ab
> --- a/drivers/gpu/drm/i915/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/intel_acpi.c
> @@ -15,7 +15,7 @@
>  	acpi_handle dhandle;
>  } intel_dsm_priv;
>  
> -static const u8 intel_dsm_guid[] = {
> +static u8 intel_dsm_guid[] = {

Why drop the const?

>  	0xd3, 0x73, 0xd8, 0x7e,
>  	0xd0, 0xc2,
>  	0x4f, 0x4e,
> @@ -23,6 +23,14 @@
>  	0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
>  };
>  
> +static u8 intel_dsm_guid_bxt[] = {

Missing const.

> +	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,12 +121,20 @@ 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 = dev->dev_private;

dev == dev_priv, just a rose by another name. Use to_i915();

> +	u8 *guid;

Missing const.

>  
>  	dhandle = ACPI_HANDLE(&pdev->dev);
>  	if (!dhandle)
>  		return false;
>  
> -	if (!acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
> +	if (IS_BROXTON(dev_priv))
> +		guid = intel_dsm_guid_bxt;
> +	else
> +		guid = intel_dsm_guid;
> +
> +	if (!acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
>  			    1 << INTEL_DSM_FN_PLATFORM_MUX_INFO)) {
>  		DRM_DEBUG_KMS("no _DSM method for intel device\n");
>  		return false;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

[Imre Deak]

On Wed, 2016-11-23 at 21:48 +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.
> 
> 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 | 56 ++++++++++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++
>  2 files changed, 65 insertions(+), 3 deletions(-)
>  mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> old mode 100644
> new mode 100755
> index cb8a75f..2f9b604
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -110,9 +110,9 @@
>  
>  /* BXT hpd list */
>  static const u32 hpd_bxt[HPD_NUM_PINS] = {
> -	[HPD_PORT_A] = BXT_DE_PORT_HP_DDIA,
> -	[HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
> -	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
> +	[HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA),
> +	[HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB),
> +	[HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC)

These are bits programmed to GEN8_DE_PORT_*, so adding the PCU bits
here is bogus.

>  };
>  
>  /* IIR can theoretically queue up two events. Be paranoid. */
> @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  			DRM_ERROR("The master control interrupt lied (DE PORT)!\n");
>  	}
>  
> +	if (master_ctl & GEN8_PCU_IRQ) {
> +		iir = I915_READ(GEN8_PCU_IIR);
> +		if (iir) {
> +			u32 tmp_mask;
> +
> +			I915_WRITE(GEN8_PCU_IIR, iir);
> +			ret = IRQ_HANDLED;
> +			if (IS_BROXTON(dev_priv)) {
> +				tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK;
> +				if (tmp_mask)
> +					bxt_hpd_irq_handler(dev_priv, tmp_mask,
> +							hpd_bxt);
> +			} else
> +				DRM_ERROR("Unexpected PCU interrupt\n");
> +		} else
> +			DRM_ERROR("The master control interrupt lied (PCU)!\n");
> +	}
> +
>  	for_each_pipe(dev_priv, pipe) {
>  		u32 flip_done, fault_errors;
>  
> @@ -4294,6 +4312,19 @@ 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 = GEN9_DE_PCU_PORTA_HOTPLUG |
> +				GEN9_DE_PCU_PORTB_HOTPLUG |
> +				GEN9_DE_PCU_PORTC_HOTPLUG;
> +
> +	de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0);

Typo.

> +	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
> @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
>   */
>  void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
>  {
> +	unsigned long flags = 0;
> +
>  	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
>  	dev_priv->pm.irqs_enabled = false;
> +
> +	if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) {
> +
> +		/* Enable HPD related interrupts during DC9 for HPD wakeup */
> +
> +		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> +		POSTING_READ(GEN8_MASTER_IRQ);
> +
> +		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);

The lock should be around the whole IRQ programming sequence.

> +
> +		dev_priv->pm.irqs_enabled = true;
> +	}
>  	synchronize_irq(dev_priv->drm.irq);

This should come before the IRQ enabling.

>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3361d7f..df89025 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6030,6 +6030,18 @@ 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 GEN9_DE_PCU_PORTA_HOTPLUG      (1 << 31)
> +#define GEN9_DE_PCU_PORTB_HOTPLUG      (1 << 30)
> +#define GEN9_DE_PCU_PORTC_HOTPLUG      (1 << 29)

Redundant copy of the BXT_ defines?

> +
>  #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
>  /* Required on all Ironlake and Sandybridge according to the B-Spec. */
>  #define  ILK_ELPIN_409_SELECT	(1 << 25)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Wed, Nov 23, 2016 at 09:48:27PM +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.
> 
> 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 | 56 ++++++++++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++
>  2 files changed, 65 insertions(+), 3 deletions(-)
>  mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> old mode 100644
> new mode 100755
> index cb8a75f..2f9b604
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -110,9 +110,9 @@
>  
>  /* BXT hpd list */
>  static const u32 hpd_bxt[HPD_NUM_PINS] = {
> -	[HPD_PORT_A] = BXT_DE_PORT_HP_DDIA,
> -	[HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
> -	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
> +	[HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA),
> +	[HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB),
> +	[HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC)

hpd_bxt_pcu[]

>  };
>  
>  /* IIR can theoretically queue up two events. Be paranoid. */
> @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  			DRM_ERROR("The master control interrupt lied (DE PORT)!\n");
>  	}
>  
> +	if (master_ctl & GEN8_PCU_IRQ) {
> +		iir = I915_READ(GEN8_PCU_IIR);
> +		if (iir) {
> +			u32 tmp_mask;
> +

Please add a proper pcu irq ack/handler pair. I actually have such a
thing sitting on a branch:

git://github.com/vsyrjala/linux.git pcode_irq

> +			I915_WRITE(GEN8_PCU_IIR, iir);
> +			ret = IRQ_HANDLED;
> +			if (IS_BROXTON(dev_priv)) {
> +				tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK;
> +				if (tmp_mask)
> +					bxt_hpd_irq_handler(dev_priv, tmp_mask,
> +							hpd_bxt);

Umm. Does the PCH_HOTPLUG reg actually work in this "hpd redirected to
pcu" state? As in are we going to detect long vs. short pulses correctly
if you just use bxt_hpd_irq_handler()?

> +			} else
> +				DRM_ERROR("Unexpected PCU interrupt\n");
> +		} else
> +			DRM_ERROR("The master control interrupt lied (PCU)!\n");
> +	}
> +
>  	for_each_pipe(dev_priv, pipe) {
>  		u32 flip_done, fault_errors;
>  
> @@ -4294,6 +4312,19 @@ 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 = GEN9_DE_PCU_PORTA_HOTPLUG |
> +				GEN9_DE_PCU_PORTB_HOTPLUG |
> +				GEN9_DE_PCU_PORTC_HOTPLUG;
> +
> +	de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0);
> +	de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask);

You'll want a bdw_update_pcu_irq() or some such thing.

> +	GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier);
> +}
> +
>  /**
>   * intel_runtime_pm_disable_interrupts - runtime interrupt disabling
>   * @dev_priv: i915 device instance
> @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
>   */
>  void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
>  {
> +	unsigned long flags = 0;
> +
>  	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
>  	dev_priv->pm.irqs_enabled = false;
> +
> +	if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) {
> +
> +		/* Enable HPD related interrupts during DC9 for HPD wakeup */
> +
> +		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> +		POSTING_READ(GEN8_MASTER_IRQ);
> +
> +		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;
> +	}
>  	synchronize_irq(dev_priv->drm.irq);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3361d7f..df89025 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6030,6 +6030,18 @@ 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 GEN9_DE_PCU_PORTA_HOTPLUG      (1 << 31)
> +#define GEN9_DE_PCU_PORTB_HOTPLUG      (1 << 30)
> +#define GEN9_DE_PCU_PORTC_HOTPLUG      (1 << 29)

Two names for the same bits?

> +
>  #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
>  /* Required on all Ironlake and Sandybridge according to the B-Spec. */
>  #define  ILK_ELPIN_409_SELECT	(1 << 25)
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

✗ Fi.CI.BAT: warning for HPD support during suspend for BXT/APL.

[Patchwork]

== Series Details ==

Series: HPD support during suspend for BXT/APL.
URL   : https://patchwork.freedesktop.org/series/15833/
State : warning

== Summary ==

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

Test drv_module_reload_basic:
                pass       -> DMESG-WARN (fi-bxt-t5700)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (fi-bxt-t5700)
        Subgroup basic-rte:
                pass       -> DMESG-WARN (fi-bxt-t5700)
Test prime_vgem:
        Subgroup basic-fence-flip:
                pass       -> DMESG-WARN (fi-bxt-t5700)

fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:204  dwarn:0   dfail:0   fail:0   skip:40 
fi-bxt-t5700     total:244  pass:212  dwarn:4   dfail:0   fail:0   skip:28 
fi-byt-j1900     total:244  pass:216  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-n2820     total:244  pass:212  dwarn:0   dfail:0   fail:0   skip:32 
fi-hsw-4770      total:244  pass:224  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:244  pass:224  dwarn:0   dfail:0   fail:0   skip:20 
fi-ilk-650       total:244  pass:191  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ivb-3770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-kbl-7200u     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:223  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6700k     total:244  pass:222  dwarn:1   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:244  pass:212  dwarn:0   dfail:0   fail:0   skip:32 
fi-snb-2600      total:244  pass:211  dwarn:0   dfail:0   fail:0   skip:33 

01896e61d9cc0cad08e19990cd095cdf679f6142 drm-intel-nightly: 2016y-11m-23d-14h-45m-53s UTC integration manifest
acc7df4 drm/i915: Enable HPD interrupts with master ctl interrupt
eb7a5df drm/i915/bxt: Block D3 during suspend.
8834d93 drm/i915/bxt: Added _DSM call to set HPD_CTL.
1d08e3f drm/i915/bxt: VBT changes for hpd as wakeup feature
f0e96b6 drm/i915/bxt: Corrected the guid for bxt.

== Logs ==

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

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

[Ville Syrjälä]

On Wed, Nov 23, 2016 at 09:48:25PM +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. PMC hpd control register
> programming will enable PMC to get hpd interrupt
> during dc9.
> 
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_acpi.c | 44 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 37 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
> index 8c878ab..15d3b84 100755
> --- 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(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;
> @@ -134,14 +155,23 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
>  	else
>  		guid = intel_dsm_guid;
>  
> -	if (!acpi_check_dsm(dhandle, 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 (acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
> +				1 << INTEL_DSM_FN_PLATFORM_MUX_INFO))
> +		intel_dsm_platform_mux_info();
> +	else
> +		DRM_DEBUG_KMS("no _DSM method for mux-info\n");
> +
> +	/* Need to ensure vbt parsing is completed. */

Eh?

> +	if (dev_priv->vbt.hpd_wakeup_enabled &&
> +		acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
> +				1 << INTEL_DSM_SET_HPD_WAKEUP))
> +		intel_dsm_set_hpd_wakeup(guid);

So we're permanently routing hpds to the pcu? Won't that mess up
stuff like short pulse detection?

I was expecting that we'd switch between the PCU and not during
runtime suspend/resume.

> +	else {
> +		dev_priv->vbt.hpd_wakeup_enabled = false;
> +		DRM_DEBUG_KMS("no _DSM method for hpd-enabling\n");
> +	}
>  
>  	return true;
>  }
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

[Jani Nikula]

On Wed, 23 Nov 2016, Animesh Manna <animesh.manna@intel.com> wrote:
> _DSM is added to program HPD_CTL(0x1094) register
> of PMC from i915 driver which will be called
> based on driver feature flag. PMC hpd control register
> programming will enable PMC to get hpd interrupt
> during dc9.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_acpi.c | 44 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
> index 8c878ab..15d3b84 100755
> --- 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(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;
> @@ -134,14 +155,23 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
>  	else
>  		guid = intel_dsm_guid;
>  
> -	if (!acpi_check_dsm(dhandle, 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 (acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
> +				1 << INTEL_DSM_FN_PLATFORM_MUX_INFO))
> +		intel_dsm_platform_mux_info();
> +	else
> +		DRM_DEBUG_KMS("no _DSM method for mux-info\n");
> +
> +	/* Need to ensure vbt parsing is completed. */
> +	if (dev_priv->vbt.hpd_wakeup_enabled &&
> +		acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
> +				1 << INTEL_DSM_SET_HPD_WAKEUP))
> +		intel_dsm_set_hpd_wakeup(guid);
> +	else {
> +		dev_priv->vbt.hpd_wakeup_enabled = false;

Please don't change dev_priv->vbt at runtime.

BR,
Jani.

> +		DRM_DEBUG_KMS("no _DSM method for hpd-enabling\n");
> +	}
>  
>  	return true;
>  }

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

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

[Jani Nikula]

On Wed, 23 Nov 2016, Animesh Manna <animesh.manna@intel.com> wrote:
> 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.
>
> 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     | 6 ++++++
>  drivers/gpu/drm/i915/intel_vbt_defs.h | 3 ++-
>  3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4e7148a..1c3cf31 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1583,6 +1583,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 7ffab1a..da4a7fd 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -557,6 +557,12 @@ static int intel_bios_ssc_frequency(struct drm_i915_private *dev_priv,
>  	if (driver->lvds_config == BDB_DRIVER_FEATURE_EDP)
>  		dev_priv->vbt.edp.support = 1;
>  
> +	if (driver->hpd_wakeup_source) {
> +		dev_priv->vbt.hpd_wakeup_enabled = true;
> +		DRM_DEBUG_KMS("HPD as wakeup feature is enabled\n");
> +	} else
> +		DRM_DEBUG_KMS("HPD wakeup source feature is disabled in VBT\n");
> +

How about just

	dev_priv->vbt.hpd_wakeup_enabled = driver->hpd_wakeup_source;

(Which kind of makes you wonder why you use different names for the
two.)

and then

	DRM_DEBUG_KMS("HPD as wakeup source: %s", yesno(driver->hpd_wakeup_source));

BR,
Jani.


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

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

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

[Animesh Manna]

On 11/23/2016 10:02 PM, Chris Wilson wrote:
> On Wed, Nov 23, 2016 at 09:48:23PM +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.
>>
>> 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 | 20 ++++++++++++++++++--
>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>   mode change 100644 => 100755 drivers/gpu/drm/i915/intel_acpi.c
>>
>> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
>> old mode 100644
>> new mode 100755
> 3 people handled this patch and none complained about making
> intel_acpi.c executable? What does happen when you try to execute it?
oh, will correct it in the next patchset.
>
>> index eb638a1..8c878ab
>> --- a/drivers/gpu/drm/i915/intel_acpi.c
>> +++ b/drivers/gpu/drm/i915/intel_acpi.c
>> @@ -15,7 +15,7 @@
>>   	acpi_handle dhandle;
>>   } intel_dsm_priv;
>>   
>> -static const u8 intel_dsm_guid[] = {
>> +static u8 intel_dsm_guid[] = {
> Why drop the const?
intel_dsm_guid is not updated anywhere, it used to assign it to a common 
pointer based on platform check in my current implementation.
we can explicitly typecast to avoid compilation warning to a normal 
pointer which will be used during dsm probe. Hope it will be fine.
Please let me know for any concern/suggestion.

>
>>   	0xd3, 0x73, 0xd8, 0x7e,
>>   	0xd0, 0xc2,
>>   	0x4f, 0x4e,
>> @@ -23,6 +23,14 @@
>>   	0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
>>   };
>>   
>> +static u8 intel_dsm_guid_bxt[] = {
> Missing const.
Explained above.
>
>> +	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,12 +121,20 @@ 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 = dev->dev_private;
> dev == dev_priv, just a rose by another name. Use to_i915();
>
>> +	u8 *guid;
> Missing const.
Explained above.
>
>>   
>>   	dhandle = ACPI_HANDLE(&pdev->dev);
>>   	if (!dhandle)
>>   		return false;
>>   
>> -	if (!acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
>> +	if (IS_BROXTON(dev_priv))
>> +		guid = intel_dsm_guid_bxt;
>> +	else
>> +		guid = intel_dsm_guid;
>> +
>> +	if (!acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
>>   			    1 << INTEL_DSM_FN_PLATFORM_MUX_INFO)) {
>>   		DRM_DEBUG_KMS("no _DSM method for intel device\n");
>>   		return false;
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> 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 1/5] drm/i915/bxt: Corrected the guid for bxt.

[Jani Nikula]

On Mon, 28 Nov 2016, Animesh Manna <animesh.manna@intel.com> wrote:
> On 11/23/2016 10:02 PM, Chris Wilson wrote:
>> On Wed, Nov 23, 2016 at 09:48:23PM +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.
>>>
>>> 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 | 20 ++++++++++++++++++--
>>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>>   mode change 100644 => 100755 drivers/gpu/drm/i915/intel_acpi.c
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
>>> old mode 100644
>>> new mode 100755
>> 3 people handled this patch and none complained about making
>> intel_acpi.c executable? What does happen when you try to execute it?
> oh, will correct it in the next patchset.
>>
>>> index eb638a1..8c878ab
>>> --- a/drivers/gpu/drm/i915/intel_acpi.c
>>> +++ b/drivers/gpu/drm/i915/intel_acpi.c
>>> @@ -15,7 +15,7 @@
>>>   	acpi_handle dhandle;
>>>   } intel_dsm_priv;
>>>   
>>> -static const u8 intel_dsm_guid[] = {
>>> +static u8 intel_dsm_guid[] = {
>> Why drop the const?
> intel_dsm_guid is not updated anywhere, it used to assign it to a common 
> pointer based on platform check in my current implementation.
> we can explicitly typecast to avoid compilation warning to a normal 
> pointer which will be used during dsm probe. Hope it will be fine.
> Please let me know for any concern/suggestion.

Doh, you use 'const u8 *guid', of course. The acpi_check_dsm uuid
parameter is also const u8 *.

BR,
Jani.

>
>>
>>>   	0xd3, 0x73, 0xd8, 0x7e,
>>>   	0xd0, 0xc2,
>>>   	0x4f, 0x4e,
>>> @@ -23,6 +23,14 @@
>>>   	0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
>>>   };
>>>   
>>> +static u8 intel_dsm_guid_bxt[] = {
>> Missing const.
> Explained above.
>>
>>> +	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,12 +121,20 @@ 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 = dev->dev_private;
>> dev == dev_priv, just a rose by another name. Use to_i915();
>>
>>> +	u8 *guid;
>> Missing const.
> Explained above.

>>
>>>   
>>>   	dhandle = ACPI_HANDLE(&pdev->dev);
>>>   	if (!dhandle)
>>>   		return false;
>>>   
>>> -	if (!acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
>>> +	if (IS_BROXTON(dev_priv))
>>> +		guid = intel_dsm_guid_bxt;
>>> +	else
>>> +		guid = intel_dsm_guid;
>>> +
>>> +	if (!acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
>>>   			    1 << INTEL_DSM_FN_PLATFORM_MUX_INFO)) {
>>>   		DRM_DEBUG_KMS("no _DSM method for intel device\n");
>>>   		return false;
>>> -- 
>>> 1.9.1
>>>
>>> _______________________________________________
>>> 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

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

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

[Animesh Manna]

On 11/23/2016 10:31 PM, Imre Deak wrote:
> On Wed, 2016-11-23 at 21:48 +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.
>>
>> 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 | 56 ++++++++++++++++++++++++++++++++++++++---
>>   drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++
>>   2 files changed, 65 insertions(+), 3 deletions(-)
>>   mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> old mode 100644
>> new mode 100755
>> index cb8a75f..2f9b604
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -110,9 +110,9 @@
>>   
>>   /* BXT hpd list */
>>   static const u32 hpd_bxt[HPD_NUM_PINS] = {
>> -	[HPD_PORT_A] = BXT_DE_PORT_HP_DDIA,
>> -	[HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
>> -	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
>> +	[HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA),
>> +	[HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB),
>> +	[HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC)
> These are bits programmed to GEN8_DE_PORT_*, so adding the PCU bits
> here is bogus.
Thanks Imre for review. I understood the "hpd_bxt" array is to store all 
bits which validate hpd interrupt in irq_handler from the respective port.
Previously only from DE_PORT interrupt used to come and after 
implementing HPD as wake feature interrupt source will be both DE_PORT 
and PCU.
So added pcu related bits in the same array.
Do you want two different array for DE_PORT and PCU. I can do it by 
creating a new array named "hpd_bxt_pcu" and change the existing one as 
"hpd_bxt_de".
Please let me know your suggestion.
>>   };
>>   
>>   /* IIR can theoretically queue up two events. Be paranoid. */
>> @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv,
>>   			DRM_ERROR("The master control interrupt lied (DE PORT)!\n");
>>   	}
>>   
>> +	if (master_ctl & GEN8_PCU_IRQ) {
>> +		iir = I915_READ(GEN8_PCU_IIR);
>> +		if (iir) {
>> +			u32 tmp_mask;
>> +
>> +			I915_WRITE(GEN8_PCU_IIR, iir);
>> +			ret = IRQ_HANDLED;
>> +			if (IS_BROXTON(dev_priv)) {
>> +				tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK;
>> +				if (tmp_mask)
>> +					bxt_hpd_irq_handler(dev_priv, tmp_mask,
>> +							hpd_bxt);
>> +			} else
>> +				DRM_ERROR("Unexpected PCU interrupt\n");
>> +		} else
>> +			DRM_ERROR("The master control interrupt lied (PCU)!\n");
>> +	}
>> +
>>   	for_each_pipe(dev_priv, pipe) {
>>   		u32 flip_done, fault_errors;
>>   
>> @@ -4294,6 +4312,19 @@ 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 = GEN9_DE_PCU_PORTA_HOTPLUG |
>> +				GEN9_DE_PCU_PORTB_HOTPLUG |
>> +				GEN9_DE_PCU_PORTC_HOTPLUG;
>> +
>> +	de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0);
> Typo.
Will remove "de" tag from all pcu related variables, for anything else 
let me know.
GEN8 and GEN9 is using same pcu interrupt registers so used the same 
macro "GEN8_PCU_IMR".
>
>> +	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
>> @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
>>    */
>>   void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
>>   {
>> +	unsigned long flags = 0;
>> +
>>   	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
>>   	dev_priv->pm.irqs_enabled = false;
>> +
>> +	if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) {
>> +
>> +		/* Enable HPD related interrupts during DC9 for HPD wakeup */
>> +
>> +		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
>> +		POSTING_READ(GEN8_MASTER_IRQ);
>> +
>> +		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);
> The lock should be around the whole IRQ programming sequence.
Just to make happy the "assert_spin_locked" which present inside 
hpd_irq_setup() taken the irq_lock.
As we are disabling all the inerrupts in initial stage, is it ok to take 
lock before and after enabling display port and pcu interrupts.
BTW, intel_runtime_pm_disable_interrupts function don't have any lock 
before.
>
>> +
>> +		dev_priv->pm.irqs_enabled = true;
>> +	}
>>   	synchronize_irq(dev_priv->drm.irq);
> This should come before the IRQ enabling.
Sure, will do it in next patchset.
>
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 3361d7f..df89025 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6030,6 +6030,18 @@ 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 GEN9_DE_PCU_PORTA_HOTPLUG      (1 << 31)
>> +#define GEN9_DE_PCU_PORTB_HOTPLUG      (1 << 30)
>> +#define GEN9_DE_PCU_PORTC_HOTPLUG      (1 << 29)
> Redundant copy of the BXT_ defines?
will remove in the next patchset.
>
>> +
>>   #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
>>   /* Required on all Ironlake and Sandybridge according to the B-Spec. */
>>   #define  ILK_ELPIN_409_SELECT	(1 << 25)

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

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

[Imre Deak]

On ma, 2016-11-28 at 19:09 +0530, Animesh Manna wrote:
> 
> On 11/23/2016 10:31 PM, Imre Deak wrote:
> > On Wed, 2016-11-23 at 21:48 +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.
> > > 
> > > 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 | 56 ++++++++++++++++++++++++++++++++++++++---
> > >   drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++
> > >   2 files changed, 65 insertions(+), 3 deletions(-)
> > >   mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > old mode 100644
> > > new mode 100755
> > > index cb8a75f..2f9b604
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -110,9 +110,9 @@
> > >   
> > >   /* BXT hpd list */
> > >   static const u32 hpd_bxt[HPD_NUM_PINS] = {
> > > -	[HPD_PORT_A] = BXT_DE_PORT_HP_DDIA,
> > > -	[HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
> > > -	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
> > > +	[HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA),
> > > +	[HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB),
> > > +	[HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC)
> > These are bits programmed to GEN8_DE_PORT_*, so adding the PCU bits
> > here is bogus.
> Thanks Imre for review. I understood the "hpd_bxt" array is to store all 
> bits which validate hpd interrupt in irq_handler from the respective port.
> Previously only from DE_PORT interrupt used to come and after 
> implementing HPD as wake feature interrupt source will be both DE_PORT 
> and PCU.
> So added pcu related bits in the same array.
> Do you want two different array for DE_PORT and PCU. I can do it by 
> creating a new array named "hpd_bxt_pcu" and change the existing one as 
> "hpd_bxt_de".
> Please let me know your suggestion.

The problem is that - for example - bxt_hpd_irq_setup() will program
now these PCU bits to GEN8_DE_PORT_IMR which is wrong. There's also a
WARN that will trigger because of this.

Yes, using a separate struct would work I think.

> > >   };
> > >   
> > >   /* IIR can theoretically queue up two events. Be paranoid. */
> > > @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv,
> > >   			DRM_ERROR("The master control interrupt lied (DE PORT)!\n");
> > >   	}
> > >   
> > > +	if (master_ctl & GEN8_PCU_IRQ) {
> > > +		iir = I915_READ(GEN8_PCU_IIR);
> > > +		if (iir) {
> > > +			u32 tmp_mask;
> > > +
> > > +			I915_WRITE(GEN8_PCU_IIR, iir);
> > > +			ret = IRQ_HANDLED;
> > > +			if (IS_BROXTON(dev_priv)) {
> > > +				tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK;
> > > +				if (tmp_mask)
> > > +					bxt_hpd_irq_handler(dev_priv, tmp_mask,
> > > +							hpd_bxt);
> > > +			} else
> > > +				DRM_ERROR("Unexpected PCU interrupt\n");
> > > +		} else
> > > +			DRM_ERROR("The master control interrupt lied (PCU)!\n");
> > > +	}
> > > +
> > >   	for_each_pipe(dev_priv, pipe) {
> > >   		u32 flip_done, fault_errors;
> > >   
> > > @@ -4294,6 +4312,19 @@ 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 = GEN9_DE_PCU_PORTA_HOTPLUG |
> > > +				GEN9_DE_PCU_PORTB_HOTPLUG |
> > > +				GEN9_DE_PCU_PORTC_HOTPLUG;
> > > +
> > > +	de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0);
> > Typo.
> Will remove "de" tag from all pcu related variables, for anything else 
> let me know.
> GEN8 and GEN9 is using same pcu interrupt registers so used the same 
> macro "GEN8_PCU_IMR".

I meant the '& 0x0' part, looks like '& ~de_pcu_hpd_enable_mask' is
what you meant.

> > 
> > > +	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
> > > @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
> > >    */
> > >   void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
> > >   {
> > > +	unsigned long flags = 0;
> > > +
> > >   	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
> > >   	dev_priv->pm.irqs_enabled = false;
> > > +
> > > +	if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) {
> > > +
> > > +		/* Enable HPD related interrupts during DC9 for HPD wakeup */
> > > +
> > > +		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> > > +		POSTING_READ(GEN8_MASTER_IRQ);
> > > +
> > > +		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);
> > The lock should be around the whole IRQ programming sequence.
> Just to make happy the "assert_spin_locked" which present inside 
> hpd_irq_setup() taken the irq_lock.
> As we are disabling all the inerrupts in initial stage, is it ok to take 
> lock before and after enabling display port and pcu interrupts.
> BTW, intel_runtime_pm_disable_interrupts function don't have any lock 
> before.

The problem is that with the above sequence once you enable interrupts
the handler may be called with irqs_enabled = false and ignoring the
interrupt. But looking at it again, taking the lock around the whole
sequence won't actually solve that, you need to set
GEN8_MASTER_IRQ_CONTROL as the last step (and move synchronize_irq
before the enabling).

> > 
> > > +
> > > +		dev_priv->pm.irqs_enabled = true;
> > > +	}
> > >   	synchronize_irq(dev_priv->drm.irq);
> > This should come before the IRQ enabling.
> Sure, will do it in next patchset.
> > 
> > >   }
> > >   
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 3361d7f..df89025 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6030,6 +6030,18 @@ 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 GEN9_DE_PCU_PORTA_HOTPLUG      (1 << 31)
> > > +#define GEN9_DE_PCU_PORTB_HOTPLUG      (1 << 30)
> > > +#define GEN9_DE_PCU_PORTC_HOTPLUG      (1 << 29)
> > Redundant copy of the BXT_ defines?
> will remove in the next patchset.
> > 
> > > +
> > >   #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
> > >   /* Required on all Ironlake and Sandybridge according to the B-Spec. */
> > >   #define  ILK_ELPIN_409_SELECT	(1 << 25)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Animesh Manna]

On 11/23/2016 10:40 PM, Ville Syrjälä wrote:
> On Wed, Nov 23, 2016 at 09:48:27PM +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.
>>
>> 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 | 56 ++++++++++++++++++++++++++++++++++++++---
>>   drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++
>>   2 files changed, 65 insertions(+), 3 deletions(-)
>>   mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> old mode 100644
>> new mode 100755
>> index cb8a75f..2f9b604
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -110,9 +110,9 @@
>>   
>>   /* BXT hpd list */
>>   static const u32 hpd_bxt[HPD_NUM_PINS] = {
>> -	[HPD_PORT_A] = BXT_DE_PORT_HP_DDIA,
>> -	[HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
>> -	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
>> +	[HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA),
>> +	[HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB),
>> +	[HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC)
> hpd_bxt_pcu[]
Thanks Ville for review.
yes, will add a separate struct for pcu.
>
>>   };
>>   
>>   /* IIR can theoretically queue up two events. Be paranoid. */
>> @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv,
>>   			DRM_ERROR("The master control interrupt lied (DE PORT)!\n");
>>   	}
>>   
>> +	if (master_ctl & GEN8_PCU_IRQ) {
>> +		iir = I915_READ(GEN8_PCU_IIR);
>> +		if (iir) {
>> +			u32 tmp_mask;
>> +
> Please add a proper pcu irq ack/handler pair. I actually have such a
> thing sitting on a branch:
>
> git://github.com/vsyrjala/linux.git pcode_irq
Downloaded the code, can you please help with the commit you are referring.
I can see some old commit as last commit (will try to sync with you over 
irc):
amanna@DispDev:~/PROJECT_CODEBASE/drm-intel-ville/pcode_irq$ git log
commit 20624d17963c737bbd9f242402bf3136cb664d10
Merge: 9a08da1 f4274e2
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Wed Apr 8 15:12:25 2015 -0700

     Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux


>
>> +			I915_WRITE(GEN8_PCU_IIR, iir);
>> +			ret = IRQ_HANDLED;
>> +			if (IS_BROXTON(dev_priv)) {
>> +				tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK;
>> +				if (tmp_mask)
>> +					bxt_hpd_irq_handler(dev_priv, tmp_mask,
>> +							hpd_bxt);
> Umm. Does the PCH_HOTPLUG reg actually work in this "hpd redirected to
> pcu" state? As in are we going to detect long vs. short pulses correctly
> if you just use bxt_hpd_irq_handler()?
After programming the PMC_HPD_CTL register we can get PCU interrupt only 
during S0ix for HPD.
AFAIK for both DP and HDMI we will be getting long pulse for hotplug.
And In non-S0ix scenario HPD interrupt will always come from DE_PORT. So 
do we need to handle short pulse interrupt?
>
>> +			} else
>> +				DRM_ERROR("Unexpected PCU interrupt\n");
>> +		} else
>> +			DRM_ERROR("The master control interrupt lied (PCU)!\n");
>> +	}
>> +
>>   	for_each_pipe(dev_priv, pipe) {
>>   		u32 flip_done, fault_errors;
>>   
>> @@ -4294,6 +4312,19 @@ 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 = GEN9_DE_PCU_PORTA_HOTPLUG |
>> +				GEN9_DE_PCU_PORTB_HOTPLUG |
>> +				GEN9_DE_PCU_PORTC_HOTPLUG;
>> +
>> +	de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0);
>> +	de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask);
> You'll want a bdw_update_pcu_irq() or some such thing.
Yes, better to use bdw tag, will update in next patchset.
>
>> +	GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier);
>> +}
>> +
>>   /**
>>    * intel_runtime_pm_disable_interrupts - runtime interrupt disabling
>>    * @dev_priv: i915 device instance
>> @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
>>    */
>>   void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
>>   {
>> +	unsigned long flags = 0;
>> +
>>   	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
>>   	dev_priv->pm.irqs_enabled = false;
>> +
>> +	if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) {
>> +
>> +		/* Enable HPD related interrupts during DC9 for HPD wakeup */
>> +
>> +		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
>> +		POSTING_READ(GEN8_MASTER_IRQ);
>> +
>> +		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;
>> +	}
>>   	synchronize_irq(dev_priv->drm.irq);
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 3361d7f..df89025 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6030,6 +6030,18 @@ 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 GEN9_DE_PCU_PORTA_HOTPLUG      (1 << 31)
>> +#define GEN9_DE_PCU_PORTB_HOTPLUG      (1 << 30)
>> +#define GEN9_DE_PCU_PORTC_HOTPLUG      (1 << 29)
> Two names for the same bits?
will correct it.

- Animesh
>
>> +
>>   #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
>>   /* Required on all Ironlake and Sandybridge according to the B-Spec. */
>>   #define  ILK_ELPIN_409_SELECT	(1 << 25)
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> 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 5/5] drm/i915: Enable HPD interrupts with master ctl interrupt

[Ville Syrjälä]

On Mon, Nov 28, 2016 at 09:19:36PM +0530, Animesh Manna wrote:
> 
> 
> On 11/23/2016 10:40 PM, Ville Syrjälä wrote:
> > On Wed, Nov 23, 2016 at 09:48:27PM +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.
> >>
> >> 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 | 56 ++++++++++++++++++++++++++++++++++++++---
> >>   drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++
> >>   2 files changed, 65 insertions(+), 3 deletions(-)
> >>   mode change 100644 => 100755 drivers/gpu/drm/i915/i915_irq.c
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> old mode 100644
> >> new mode 100755
> >> index cb8a75f..2f9b604
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -110,9 +110,9 @@
> >>   
> >>   /* BXT hpd list */
> >>   static const u32 hpd_bxt[HPD_NUM_PINS] = {
> >> -	[HPD_PORT_A] = BXT_DE_PORT_HP_DDIA,
> >> -	[HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
> >> -	[HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
> >> +	[HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA),
> >> +	[HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB),
> >> +	[HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC)
> > hpd_bxt_pcu[]
> Thanks Ville for review.
> yes, will add a separate struct for pcu.
> >
> >>   };
> >>   
> >>   /* IIR can theoretically queue up two events. Be paranoid. */
> >> @@ -2463,6 +2463,24 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv,
> >>   			DRM_ERROR("The master control interrupt lied (DE PORT)!\n");
> >>   	}
> >>   
> >> +	if (master_ctl & GEN8_PCU_IRQ) {
> >> +		iir = I915_READ(GEN8_PCU_IIR);
> >> +		if (iir) {
> >> +			u32 tmp_mask;
> >> +
> > Please add a proper pcu irq ack/handler pair. I actually have such a
> > thing sitting on a branch:
> >
> > git://github.com/vsyrjala/linux.git pcode_irq
> Downloaded the code, can you please help with the commit you are referring.
> I can see some old commit as last commit (will try to sync with you over 
> irc):
> amanna@DispDev:~/PROJECT_CODEBASE/drm-intel-ville/pcode_irq$ git log
> commit 20624d17963c737bbd9f242402bf3136cb664d10
> Merge: 9a08da1 f4274e2
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date:   Wed Apr 8 15:12:25 2015 -0700
> 
>      Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux

git checkout pcode_irq

> 
> 
> >
> >> +			I915_WRITE(GEN8_PCU_IIR, iir);
> >> +			ret = IRQ_HANDLED;
> >> +			if (IS_BROXTON(dev_priv)) {
> >> +				tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK;
> >> +				if (tmp_mask)
> >> +					bxt_hpd_irq_handler(dev_priv, tmp_mask,
> >> +							hpd_bxt);
> > Umm. Does the PCH_HOTPLUG reg actually work in this "hpd redirected to
> > pcu" state? As in are we going to detect long vs. short pulses correctly
> > if you just use bxt_hpd_irq_handler()?
> After programming the PMC_HPD_CTL register we can get PCU interrupt only 
> during S0ix for HPD.
> AFAIK for both DP and HDMI we will be getting long pulse for hotplug.
> And In non-S0ix scenario HPD interrupt will always come from DE_PORT. So 
> do we need to handle short pulse interrupt?

Short pulse probably isn't needed during s0ix since I presume we can't
have any active displays anyway. The only slight concern might be DP
branch devices which would signal downstream port HPDs via a short
pulse. I have a vague recollection that there might be a way to aske
them for a long HPD instead via some DPCD register.

> >
> >> +			} else
> >> +				DRM_ERROR("Unexpected PCU interrupt\n");
> >> +		} else
> >> +			DRM_ERROR("The master control interrupt lied (PCU)!\n");
> >> +	}
> >> +
> >>   	for_each_pipe(dev_priv, pipe) {
> >>   		u32 flip_done, fault_errors;
> >>   
> >> @@ -4294,6 +4312,19 @@ 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 = GEN9_DE_PCU_PORTA_HOTPLUG |
> >> +				GEN9_DE_PCU_PORTB_HOTPLUG |
> >> +				GEN9_DE_PCU_PORTC_HOTPLUG;
> >> +
> >> +	de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0);
> >> +	de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask);
> > You'll want a bdw_update_pcu_irq() or some such thing.
> Yes, better to use bdw tag, will update in next patchset.
> >
> >> +	GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier);
> >> +}
> >> +
> >>   /**
> >>    * intel_runtime_pm_disable_interrupts - runtime interrupt disabling
> >>    * @dev_priv: i915 device instance
> >> @@ -4303,8 +4334,27 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
> >>    */
> >>   void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
> >>   {
> >> +	unsigned long flags = 0;
> >> +
> >>   	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
> >>   	dev_priv->pm.irqs_enabled = false;
> >> +
> >> +	if (IS_BROXTON(dev_priv) && dev_priv->vbt.hpd_wakeup_enabled) {
> >> +
> >> +		/* Enable HPD related interrupts during DC9 for HPD wakeup */
> >> +
> >> +		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> >> +		POSTING_READ(GEN8_MASTER_IRQ);
> >> +
> >> +		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;
> >> +	}
> >>   	synchronize_irq(dev_priv->drm.irq);
> >>   }
> >>   
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> index 3361d7f..df89025 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -6030,6 +6030,18 @@ 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 GEN9_DE_PCU_PORTA_HOTPLUG      (1 << 31)
> >> +#define GEN9_DE_PCU_PORTB_HOTPLUG      (1 << 30)
> >> +#define GEN9_DE_PCU_PORTC_HOTPLUG      (1 << 29)
> > Two names for the same bits?
> will correct it.
> 
> - Animesh
> >
> >> +
> >>   #define ILK_DISPLAY_CHICKEN2	_MMIO(0x42004)
> >>   /* Required on all Ironlake and Sandybridge according to the B-Spec. */
> >>   #define  ILK_ELPIN_409_SELECT	(1 << 25)
> >> -- 
> >> 1.9.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

[Animesh Manna]

On 11/23/2016 11:47 PM, Ville Syrjälä wrote:
> On Wed, Nov 23, 2016 at 09:48:25PM +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. PMC hpd control register
>> programming will enable PMC to get hpd interrupt
>> during dc9.
>>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_acpi.c | 44 ++++++++++++++++++++++++++++++++-------
>>   1 file changed, 37 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
>> index 8c878ab..15d3b84 100755
>> --- 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(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;
>> @@ -134,14 +155,23 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
>>   	else
>>   		guid = intel_dsm_guid;
>>   
>> -	if (!acpi_check_dsm(dhandle, 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 (acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
>> +				1 << INTEL_DSM_FN_PLATFORM_MUX_INFO))
>> +		intel_dsm_platform_mux_info();
>> +	else
>> +		DRM_DEBUG_KMS("no _DSM method for mux-info\n");
>> +
>> +	/* Need to ensure vbt parsing is completed. */
> Eh?
There is a feature flag for oem to enable/disable HPD as wake feature in 
vbt and on the basis of that PMC_HPD_CTL register will be programmed.
So added a comment just to have that info captured in code-comment, for 
any concern let me know, will update accordingly.
>
>> +	if (dev_priv->vbt.hpd_wakeup_enabled &&
>> +		acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
>> +				1 << INTEL_DSM_SET_HPD_WAKEUP))
>> +		intel_dsm_set_hpd_wakeup(guid);
> So we're permanently routing hpds to the pcu? Won't that mess up
> stuff like short pulse detection?
>
> I was expecting that we'd switch between the PCU and not during
> runtime suspend/resume.
Routing hpd to the pcu will only happen during S0ix. As per my 
understanding for both HDMI and DP we will be getting long pulse during 
hotplug at S0ix. So I think it will not mess up.
Correct me if I am missing something.

- Animesh
>
>> +	else {
>> +		dev_priv->vbt.hpd_wakeup_enabled = false;
>> +		DRM_DEBUG_KMS("no _DSM method for hpd-enabling\n");
>> +	}
>>   
>>   	return true;
>>   }
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> 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 1/5] drm/i915/bxt: Corrected the guid for bxt.

[Animesh Manna]

On 11/28/2016 4:54 PM, Jani Nikula wrote:
> On Mon, 28 Nov 2016, Animesh Manna <animesh.manna@intel.com> wrote:
>> On 11/23/2016 10:02 PM, Chris Wilson wrote:
>>> On Wed, Nov 23, 2016 at 09:48:23PM +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.
>>>>
>>>> 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 | 20 ++++++++++++++++++--
>>>>    1 file changed, 18 insertions(+), 2 deletions(-)
>>>>    mode change 100644 => 100755 drivers/gpu/drm/i915/intel_acpi.c
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
>>>> old mode 100644
>>>> new mode 100755
>>> 3 people handled this patch and none complained about making
>>> intel_acpi.c executable? What does happen when you try to execute it?
>> oh, will correct it in the next patchset.
>>>> index eb638a1..8c878ab
>>>> --- a/drivers/gpu/drm/i915/intel_acpi.c
>>>> +++ b/drivers/gpu/drm/i915/intel_acpi.c
>>>> @@ -15,7 +15,7 @@
>>>>    	acpi_handle dhandle;
>>>>    } intel_dsm_priv;
>>>>    
>>>> -static const u8 intel_dsm_guid[] = {
>>>> +static u8 intel_dsm_guid[] = {
>>> Why drop the const?
>> intel_dsm_guid is not updated anywhere, it used to assign it to a common
>> pointer based on platform check in my current implementation.
>> we can explicitly typecast to avoid compilation warning to a normal
>> pointer which will be used during dsm probe. Hope it will be fine.
>> Please let me know for any concern/suggestion.
> Doh, you use 'const u8 *guid', of course. The acpi_check_dsm uuid
> parameter is also const u8 *.
>
> BR,
> Jani.
Ok.


>
>>>>    	0xd3, 0x73, 0xd8, 0x7e,
>>>>    	0xd0, 0xc2,
>>>>    	0x4f, 0x4e,
>>>> @@ -23,6 +23,14 @@
>>>>    	0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
>>>>    };
>>>>    
>>>> +static u8 intel_dsm_guid_bxt[] = {
>>> Missing const.
>> Explained above.
>>>> +	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,12 +121,20 @@ 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 = dev->dev_private;
>>> dev == dev_priv, just a rose by another name. Use to_i915();
>>>
>>>> +	u8 *guid;
>>> Missing const.
>> Explained above.
>>>>    
>>>>    	dhandle = ACPI_HANDLE(&pdev->dev);
>>>>    	if (!dhandle)
>>>>    		return false;
>>>>    
>>>> -	if (!acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
>>>> +	if (IS_BROXTON(dev_priv))
>>>> +		guid = intel_dsm_guid_bxt;
>>>> +	else
>>>> +		guid = intel_dsm_guid;
>>>> +
>>>> +	if (!acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
>>>>    			    1 << INTEL_DSM_FN_PLATFORM_MUX_INFO)) {
>>>>    		DRM_DEBUG_KMS("no _DSM method for intel device\n");
>>>>    		return false;
>>>> -- 
>>>> 1.9.1
>>>>
>>>> _______________________________________________
>>>> 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

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

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

[Ville Syrjälä]

On Mon, Nov 28, 2016 at 09:36:51PM +0530, Animesh Manna wrote:
> 
> 
> On 11/23/2016 11:47 PM, Ville Syrjälä wrote:
> > On Wed, Nov 23, 2016 at 09:48:25PM +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. PMC hpd control register
> >> programming will enable PMC to get hpd interrupt
> >> during dc9.
> >>
> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_acpi.c | 44 ++++++++++++++++++++++++++++++++-------
> >>   1 file changed, 37 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
> >> index 8c878ab..15d3b84 100755
> >> --- 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(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;
> >> @@ -134,14 +155,23 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
> >>   	else
> >>   		guid = intel_dsm_guid;
> >>   
> >> -	if (!acpi_check_dsm(dhandle, 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 (acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
> >> +				1 << INTEL_DSM_FN_PLATFORM_MUX_INFO))
> >> +		intel_dsm_platform_mux_info();
> >> +	else
> >> +		DRM_DEBUG_KMS("no _DSM method for mux-info\n");
> >> +
> >> +	/* Need to ensure vbt parsing is completed. */
> > Eh?
> There is a feature flag for oem to enable/disable HPD as wake feature in 
> vbt and on the basis of that PMC_HPD_CTL register will be programmed.
> So added a comment just to have that info captured in code-comment, for 
> any concern let me know, will update accordingly.

The comment is making me thing the VBT parsing would happen
asynchronously somehow, and this would need then to block until it's
finsiehd. Doesn't make any sense to me.

> >
> >> +	if (dev_priv->vbt.hpd_wakeup_enabled &&
> >> +		acpi_check_dsm(dhandle, guid, INTEL_DSM_REVISION_ID,
> >> +				1 << INTEL_DSM_SET_HPD_WAKEUP))
> >> +		intel_dsm_set_hpd_wakeup(guid);
> > So we're permanently routing hpds to the pcu? Won't that mess up
> > stuff like short pulse detection?
> >
> > I was expecting that we'd switch between the PCU and not during
> > runtime suspend/resume.
> Routing hpd to the pcu will only happen during S0ix. As per my 
> understanding for both HDMI and DP we will be getting long pulse during 
> hotplug at S0ix. So I think it will not mess up.
> Correct me if I am missing something.

I have no idea. I don't have the hardware to verify any of that. Please
do that yourself, and add a convicing comment/commit message with your
findings documented.

> 
> - Animesh
> >
> >> +	else {
> >> +		dev_priv->vbt.hpd_wakeup_enabled = false;
> >> +		DRM_DEBUG_KMS("no _DSM method for hpd-enabling\n");
> >> +	}
> >>   
> >>   	return true;
> >>   }
> >> -- 
> >> 1.9.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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