All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915/icp: Add Interrupt Support
@ 2018-06-20 21:36 Anusha Srivatsa
  2018-06-20 21:36 ` [PATCH 2/2] drm/i915/icl: implement icl_digital_port_connected() Anusha Srivatsa
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Anusha Srivatsa @ 2018-06-20 21:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni

This patch addresses Interrupts from south display engine (SDE).

ICP has two registers - SHOTPLUG_CTL_DDI and SHOTPLUG_CTL_TC.
Introduce these registers and their intended values.

Introduce icp_irq_handler().

v2:
- remove redundant register defines.(Lucas)
- Change register names to be more consistent with
previous platforms (Lucas)

Cc: Lucas De Marchi <lucas.de.marchi@gmail.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
[Paulo: coding style bikesheds and rebases].
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 134 +++++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h |  40 ++++++++++++
 2 files changed, 172 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 46aaef5..7a7c4a2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -122,6 +122,15 @@ static const u32 hpd_gen11[HPD_NUM_PINS] = {
 	[HPD_PORT_F] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG
 };
 
+static const u32 hpd_icp[HPD_NUM_PINS] = {
+	[HPD_PORT_A] = SDE_DDIA_HOTPLUG_ICP,
+	[HPD_PORT_B] = SDE_DDIB_HOTPLUG_ICP,
+	[HPD_PORT_C] = SDE_TC1_HOTPLUG_ICP,
+	[HPD_PORT_D] = SDE_TC2_HOTPLUG_ICP,
+	[HPD_PORT_E] = SDE_TC3_HOTPLUG_ICP,
+	[HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP
+};
+
 /* IIR can theoretically queue up two events. Be paranoid. */
 #define GEN8_IRQ_RESET_NDX(type, which) do { \
 	I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
@@ -1586,6 +1595,34 @@ static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
 	}
 }
 
+static bool icp_ddi_port_hotplug_long_detect(enum port port, u32 val)
+{
+	switch (port) {
+	case PORT_A:
+		return val & ICP_DDIA_HPD_LONG_DETECT;
+	case PORT_B:
+		return val & ICP_DDIB_HPD_LONG_DETECT;
+	default:
+		return false;
+	}
+}
+
+static bool icp_tc_port_hotplug_long_detect(enum port port, u32 val)
+{
+	switch (port) {
+	case PORT_C:
+		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1);
+	case PORT_D:
+		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2);
+	case PORT_E:
+		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3);
+	case PORT_F:
+		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4);
+	default:
+		return false;
+	}
+}
+
 static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
 {
 	switch (port) {
@@ -2385,6 +2422,43 @@ static void cpt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
 		cpt_serr_int_handler(dev_priv);
 }
 
+static void icp_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
+{
+	u32 ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP;
+	u32 tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP;
+	u32 pin_mask = 0, long_mask = 0;
+
+	if (ddi_hotplug_trigger) {
+		u32 dig_hotplug_reg;
+
+		dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_DDI);
+		I915_WRITE(SHOTPLUG_CTL_DDI, dig_hotplug_reg);
+
+		intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
+				   ddi_hotplug_trigger,
+				   dig_hotplug_reg, hpd_icp,
+				   icp_ddi_port_hotplug_long_detect);
+	}
+
+	if (tc_hotplug_trigger) {
+		u32 dig_hotplug_reg;
+
+		dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_TC);
+		I915_WRITE(SHOTPLUG_CTL_TC, dig_hotplug_reg);
+
+		intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
+				   tc_hotplug_trigger,
+				   dig_hotplug_reg, hpd_icp,
+				   icp_tc_port_hotplug_long_detect);
+	}
+
+	if (pin_mask)
+		intel_hpd_irq_handler(dev_priv, pin_mask, long_mask);
+
+	if (pch_iir & SDE_GMBUS_ICP)
+		gmbus_irq_handler(dev_priv);
+}
+
 static void spt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
 {
 	u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT &
@@ -2804,8 +2878,11 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 			I915_WRITE(SDEIIR, iir);
 			ret = IRQ_HANDLED;
 
-			if (HAS_PCH_SPT(dev_priv) || HAS_PCH_KBP(dev_priv) ||
-			    HAS_PCH_CNP(dev_priv))
+			if (HAS_PCH_ICP(dev_priv))
+				icp_irq_handler(dev_priv, iir);
+			else if (HAS_PCH_SPT(dev_priv) ||
+				 HAS_PCH_KBP(dev_priv) ||
+				 HAS_PCH_CNP(dev_priv))
 				spt_irq_handler(dev_priv, iir);
 			else
 				cpt_irq_handler(dev_priv, iir);
@@ -3584,6 +3661,9 @@ static void gen11_irq_reset(struct drm_device *dev)
 	GEN3_IRQ_RESET(GEN11_DE_HPD_);
 	GEN3_IRQ_RESET(GEN11_GU_MISC_);
 	GEN3_IRQ_RESET(GEN8_PCU_);
+
+	if (HAS_PCH_ICP(dev_priv))
+		GEN3_IRQ_RESET(SDE);
 }
 
 void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
@@ -3700,6 +3780,35 @@ static void ibx_hpd_irq_setup(struct drm_i915_private *dev_priv)
 	ibx_hpd_detection_setup(dev_priv);
 }
 
+static void icp_hpd_detection_setup(struct drm_i915_private *dev_priv)
+{
+	u32 hotplug;
+
+	hotplug = I915_READ(SHOTPLUG_CTL_DDI);
+	hotplug |= ICP_DDIA_HPD_ENABLE |
+		   ICP_DDIB_HPD_ENABLE;
+	I915_WRITE(SHOTPLUG_CTL_DDI, hotplug);
+
+	hotplug = I915_READ(SHOTPLUG_CTL_TC);
+	hotplug |= ICP_TC_HPD_ENABLE(PORT_TC1) |
+		   ICP_TC_HPD_ENABLE(PORT_TC2) |
+		   ICP_TC_HPD_ENABLE(PORT_TC3) |
+		   ICP_TC_HPD_ENABLE(PORT_TC4);
+	I915_WRITE(SHOTPLUG_CTL_TC, hotplug);
+}
+
+static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv)
+{
+	u32 hotplug_irqs, enabled_irqs;
+
+	hotplug_irqs = SDE_DDI_MASK_ICP | SDE_TC_MASK_ICP;
+	enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_icp);
+
+	ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
+
+	icp_hpd_detection_setup(dev_priv);
+}
+
 static void gen11_hpd_detection_setup(struct drm_i915_private *dev_priv)
 {
 	u32 hotplug;
@@ -3733,6 +3842,9 @@ static void gen11_hpd_irq_setup(struct drm_i915_private *dev_priv)
 	POSTING_READ(GEN11_DE_HPD_IMR);
 
 	gen11_hpd_detection_setup(dev_priv);
+
+	if (HAS_PCH_ICP(dev_priv))
+		icp_hpd_irq_setup(dev_priv);
 }
 
 static void spt_hpd_detection_setup(struct drm_i915_private *dev_priv)
@@ -4168,11 +4280,29 @@ static void gen11_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_MASK,  ~0);
 }
 
+static void icp_irq_postinstall(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	u32 mask = SDE_GMBUS_ICP;
+
+	WARN_ON(I915_READ(SDEIER) != 0);
+	I915_WRITE(SDEIER, 0xffffffff);
+	POSTING_READ(SDEIER);
+
+	gen3_assert_iir_is_zero(dev_priv, SDEIIR);
+	I915_WRITE(SDEIMR, ~mask);
+
+	icp_hpd_detection_setup(dev_priv);
+}
+
 static int gen11_irq_postinstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 gu_misc_masked = GEN11_GU_MISC_GSE;
 
+	if (HAS_PCH_ICP(dev_priv))
+		icp_irq_postinstall(dev);
+
 	gen11_gt_irq_postinstall(dev_priv);
 	gen8_de_irq_postinstall(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4bfd7a9..e347055 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7517,6 +7517,46 @@ enum {
 #define  PORTE_HOTPLUG_SHORT_DETECT	(1 << 0)
 #define  PORTE_HOTPLUG_LONG_DETECT	(2 << 0)
 
+/* ICP */
+#define   SDE_TC4_HOTPLUG_ICP		(1 << 27)
+#define   SDE_TC3_HOTPLUG_ICP		(1 << 26)
+#define   SDE_TC2_HOTPLUG_ICP		(1 << 25)
+#define   SDE_TC1_HOTPLUG_ICP		(1 << 24)
+#define   SDE_GMBUS_ICP			(1 << 23)
+#define   SDE_DDIB_HOTPLUG_ICP		(1 << 17)
+#define   SDE_DDIA_HOTPLUG_ICP		(1 << 16)
+
+#define SDE_DDI_MASK_ICP		(SDE_DDIB_HOTPLUG_ICP |	\
+					 SDE_DDIA_HOTPLUG_ICP)
+
+#define SDE_TC_MASK_ICP			(SDE_TC4_HOTPLUG_ICP |	\
+					 SDE_TC3_HOTPLUG_ICP |	\
+					 SDE_TC2_HOTPLUG_ICP |	\
+					 SDE_TC1_HOTPLUG_ICP)
+
+/* This register is a reuse of PCH_PORT_HOTPLUG register. The Spec
+ * has the name SHOTPLUG_CTL_DDI for ICP. Let us stick to the Spec.
+ */
+
+#define SHOTPLUG_CTL_DDI			_MMIO(0xc4030)
+#define   ICP_DDIB_HPD_ENABLE			(1 << 7)
+#define   ICP_DDIB_HPD_STATUS_MASK		(3 << 4)
+#define   ICP_DDIB_HPD_NO_DETECT		(0 << 4)
+#define   ICP_DDIB_HPD_SHORT_DETECT		(1 << 4)
+#define   ICP_DDIB_HPD_LONG_DETECT		(2 << 4)
+#define   ICP_DDIB_HPD_SHORT_LONG_DETECT	(3 << 4)
+#define   ICP_DDIA_HPD_ENABLE			(1 << 3)
+#define   ICP_DDIA_HPD_STATUS_MASK		(3 << 0)
+#define   ICP_DDIA_HPD_NO_DETECT		(0 << 0)
+#define   ICP_DDIA_HPD_SHORT_DETECT		(1 << 0)
+#define   ICP_DDIA_HPD_LONG_DETECT		(2 << 0)
+#define   ICP_DDIA_HPD_SHORT_LONG_DETECT	(3 << 0)
+
+#define SHOTPLUG_CTL_TC				_MMIO(0xc4034)
+#define   ICP_TC_HPD_ENABLE(tc_port)		(8 << (tc_port) * 4)
+#define   ICP_TC_HPD_LONG_DETECT(tc_port)	(2 << (tc_port) * 4)
+#define   ICP_TC_HPD_SHORT_DETECT(tc_port)	(1 << (tc_port) * 4)
+
 #define PCH_GPIOA               _MMIO(0xc5010)
 #define PCH_GPIOB               _MMIO(0xc5014)
 #define PCH_GPIOC               _MMIO(0xc5018)
-- 
2.7.4

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

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

* [PATCH 2/2] drm/i915/icl: implement icl_digital_port_connected()
  2018-06-20 21:36 [PATCH 1/2] drm/i915/icp: Add Interrupt Support Anusha Srivatsa
@ 2018-06-20 21:36 ` Anusha Srivatsa
  2018-06-30  6:42   ` Lucas De Marchi
  2018-06-20 22:11 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/icp: Add Interrupt Support Patchwork
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Anusha Srivatsa @ 2018-06-20 21:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi, Paulo Zanoni, Rodrigo Vivi

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Do like the other functions and check for the ISR bits. We have plans
to add a few more checks in this code in the next patches, that's why
it's a little more verbose than it could be.

v2:
- Change the register names, to be consistent with
the rest of the platforms.

Cc: Animesh Manna <animesh.manna@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  5 ++++
 drivers/gpu/drm/i915/intel_dp.c | 57 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e347055..f55e889 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7093,6 +7093,7 @@ enum {
 #define  GEN11_TC3_HOTPLUG			(1 << 18)
 #define  GEN11_TC2_HOTPLUG			(1 << 17)
 #define  GEN11_TC1_HOTPLUG			(1 << 16)
+#define  GEN11_TC_HOTPLUG(tc_port)		(1 << ((tc_port) + 16))
 #define  GEN11_DE_TC_HOTPLUG_MASK		(GEN11_TC4_HOTPLUG | \
 						 GEN11_TC3_HOTPLUG | \
 						 GEN11_TC2_HOTPLUG | \
@@ -7101,6 +7102,7 @@ enum {
 #define  GEN11_TBT3_HOTPLUG			(1 << 2)
 #define  GEN11_TBT2_HOTPLUG			(1 << 1)
 #define  GEN11_TBT1_HOTPLUG			(1 << 0)
+#define  GEN11_TBT_HOTPLUG(tc_port)		(1 << (tc_port))
 #define  GEN11_DE_TBT_HOTPLUG_MASK		(GEN11_TBT4_HOTPLUG | \
 						 GEN11_TBT3_HOTPLUG | \
 						 GEN11_TBT2_HOTPLUG | \
@@ -7525,6 +7527,9 @@ enum {
 #define   SDE_GMBUS_ICP			(1 << 23)
 #define   SDE_DDIB_HOTPLUG_ICP		(1 << 17)
 #define   SDE_DDIA_HOTPLUG_ICP		(1 << 16)
+#define SDE_TC_HOTPLUG_ICP(tc_port)		(1 << ((tc_port) + 24))
+#define SDE_DDI_HOTPLUG_ICP(port)		(1 << ((port) + 16))
+
 
 #define SDE_DDI_MASK_ICP		(SDE_DDIB_HOTPLUG_ICP |	\
 					 SDE_DDIA_HOTPLUG_ICP)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6ac6c87..224cc71 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4750,6 +4750,61 @@ static bool bxt_digital_port_connected(struct intel_encoder *encoder)
 	return I915_READ(GEN8_DE_PORT_ISR) & bit;
 }
 
+static bool icl_combo_port_connected(struct drm_i915_private *dev_priv,
+				     struct intel_digital_port *intel_dig_port)
+{
+	enum port port = intel_dig_port->base.port;
+
+	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
+}
+
+static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
+				  struct intel_digital_port *intel_dig_port)
+{
+	enum port port = intel_dig_port->base.port;
+	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
+	u32 legacy_bit = SDE_TC_HOTPLUG_ICP(tc_port);
+	u32 typec_bit = GEN11_TC_HOTPLUG(tc_port);
+	u32 tbt_bit = GEN11_TBT_HOTPLUG(tc_port);
+	bool is_legacy = false, is_typec = false, is_tbt = false;
+	u32 cpu_isr;
+
+	if (I915_READ(SDEISR) & legacy_bit)
+		is_legacy = true;
+
+	cpu_isr = I915_READ(GEN11_DE_HPD_ISR);
+	if (cpu_isr & typec_bit)
+		is_typec = true;
+	if (cpu_isr & tbt_bit)
+		is_tbt = true;
+
+	WARN_ON(is_legacy + is_typec + is_tbt > 1);
+	if (!is_legacy && !is_typec && !is_tbt)
+		return false;
+
+	return true;
+}
+
+static bool icl_digital_port_connected(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+
+	switch (encoder->hpd_pin) {
+	case HPD_PORT_A:
+	case HPD_PORT_B:
+		return icl_combo_port_connected(dev_priv, dig_port);
+	case HPD_PORT_C:
+	case HPD_PORT_D:
+	case HPD_PORT_E:
+	case HPD_PORT_F:
+		return icl_tc_port_connected(dev_priv, dig_port);
+	default:
+		MISSING_CASE(encoder->hpd_pin);
+		return false;
+	}
+}
+
 /*
  * intel_digital_port_connected - is the specified port connected?
  * @encoder: intel_encoder
@@ -4777,6 +4832,8 @@ bool intel_digital_port_connected(struct intel_encoder *encoder)
 		return bdw_digital_port_connected(encoder);
 	else if (IS_GEN9_LP(dev_priv))
 		return bxt_digital_port_connected(encoder);
+	else if (IS_ICELAKE(dev_priv))
+		return icl_digital_port_connected(encoder);
 	else
 		return spt_digital_port_connected(encoder);
 }
-- 
2.7.4

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

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

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/icp: Add Interrupt Support
  2018-06-20 21:36 [PATCH 1/2] drm/i915/icp: Add Interrupt Support Anusha Srivatsa
  2018-06-20 21:36 ` [PATCH 2/2] drm/i915/icl: implement icl_digital_port_connected() Anusha Srivatsa
@ 2018-06-20 22:11 ` Patchwork
  2018-06-21  6:37 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-06-20 22:11 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/icp: Add Interrupt Support
URL   : https://patchwork.freedesktop.org/series/45124/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4354 -> Patchwork_9375 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45124/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9375 that come from known issues:

  === IGT changes ===

    ==== Possible fixes ====

    igt@gem_ctx_create@basic-files:
      fi-kbl-7567u:       INCOMPLETE -> PASS

    igt@gem_exec_gttfill@basic:
      fi-byt-n2820:       FAIL (fdo#106744) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#106744 https://bugs.freedesktop.org/show_bug.cgi?id=106744


== Participating hosts (43 -> 38) ==

  Additional (1): fi-byt-j1900 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-glk-dsi fi-bsw-cyan fi-ctg-p8600 fi-kbl-x1275 


== Build changes ==

    * Linux: CI_DRM_4354 -> Patchwork_9375

  CI_DRM_4354: 370dde179b55a18cdc2a32d83a9dafcea1e434f4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4527: 04afec3ccfcb35e994f2e78254ff499f6b94f097 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9375: 51fd78b85ea1794cfab59cee53d3b6ada5af8ac1 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

51fd78b85ea1 drm/i915/icl: implement icl_digital_port_connected()
afaa9a9ef7cb drm/i915/icp: Add Interrupt Support

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9375/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/icp: Add Interrupt Support
  2018-06-20 21:36 [PATCH 1/2] drm/i915/icp: Add Interrupt Support Anusha Srivatsa
  2018-06-20 21:36 ` [PATCH 2/2] drm/i915/icl: implement icl_digital_port_connected() Anusha Srivatsa
  2018-06-20 22:11 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/icp: Add Interrupt Support Patchwork
@ 2018-06-21  6:37 ` Patchwork
  2018-06-25 23:17 ` [PATCH 1/2] " Paulo Zanoni
  2018-07-23 17:05 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] " Patchwork
  4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-06-21  6:37 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/icp: Add Interrupt Support
URL   : https://patchwork.freedesktop.org/series/45124/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4354_full -> Patchwork_9375_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9375_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9375_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9375_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_mocs_settings@mocs-rc6-blt:
      shard-kbl:          PASS -> SKIP

    igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic:
      shard-snb:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_9375_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@pi-ringfull-render:
      shard-apl:          NOTRUN -> FAIL (fdo#103158)

    igt@kms_atomic_transition@plane-all-transition-nonblocking:
      shard-glk:          PASS -> INCOMPLETE (fdo#103359, k.org#198133)

    igt@kms_cursor_legacy@cursora-vs-flipa-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763)

    igt@kms_flip@flip-vs-blocking-wf-vblank:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_flip_tiling@flip-to-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724) +1

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724, fdo#103822)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      shard-kbl:          NOTRUN -> INCOMPLETE (fdo#103665)

    igt@kms_sysfs_edid_timing:
      shard-apl:          NOTRUN -> FAIL (fdo#100047)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_evict:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@drv_selftest@live_gtt:
      shard-kbl:          FAIL (fdo#105347) -> PASS
      shard-glk:          FAIL (fdo#105347) -> PASS
      shard-apl:          FAIL (fdo#105347) -> PASS

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          DMESG-WARN (fdo#105763, fdo#106538) -> PASS

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_flip@2x-plain-flip-fb-recreate:
      shard-glk:          FAIL (fdo#100368) -> PASS

    
  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4354 -> Patchwork_9375

  CI_DRM_4354: 370dde179b55a18cdc2a32d83a9dafcea1e434f4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4527: 04afec3ccfcb35e994f2e78254ff499f6b94f097 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9375: 51fd78b85ea1794cfab59cee53d3b6ada5af8ac1 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9375/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/2] drm/i915/icp: Add Interrupt Support
  2018-06-20 21:36 [PATCH 1/2] drm/i915/icp: Add Interrupt Support Anusha Srivatsa
                   ` (2 preceding siblings ...)
  2018-06-21  6:37 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-06-25 23:17 ` Paulo Zanoni
  2018-06-26 18:32   ` Srivatsa, Anusha
  2018-07-23 17:05 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] " Patchwork
  4 siblings, 1 reply; 10+ messages in thread
From: Paulo Zanoni @ 2018-06-25 23:17 UTC (permalink / raw)
  To: Anusha Srivatsa, intel-gfx; +Cc: Dhinakaran Pandiyan

Em Qua, 2018-06-20 às 14:36 -0700, Anusha Srivatsa escreveu:
> This patch addresses Interrupts from south display engine (SDE).
> 
> ICP has two registers - SHOTPLUG_CTL_DDI and SHOTPLUG_CTL_TC.
> Introduce these registers and their intended values.
> 
> Introduce icp_irq_handler().
> 
> v2:
> - remove redundant register defines.(Lucas)
> - Change register names to be more consistent with
> previous platforms (Lucas)
> 
> Cc: Lucas De Marchi <lucas.de.marchi@gmail.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> [Paulo: coding style bikesheds and rebases].
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 134
> +++++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h |  40 ++++++++++++
>  2 files changed, 172 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 46aaef5..7a7c4a2 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -122,6 +122,15 @@ static const u32 hpd_gen11[HPD_NUM_PINS] = {
>  	[HPD_PORT_F] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG
>  };
>  
> +static const u32 hpd_icp[HPD_NUM_PINS] = {
> +	[HPD_PORT_A] = SDE_DDIA_HOTPLUG_ICP,
> +	[HPD_PORT_B] = SDE_DDIB_HOTPLUG_ICP,
> +	[HPD_PORT_C] = SDE_TC1_HOTPLUG_ICP,
> +	[HPD_PORT_D] = SDE_TC2_HOTPLUG_ICP,
> +	[HPD_PORT_E] = SDE_TC3_HOTPLUG_ICP,
> +	[HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP
> +};
> +
>  /* IIR can theoretically queue up two events. Be paranoid. */
>  #define GEN8_IRQ_RESET_NDX(type, which) do { \
>  	I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
> @@ -1586,6 +1595,34 @@ static bool bxt_port_hotplug_long_detect(enum
> port port, u32 val)
>  	}
>  }
>  
> +static bool icp_ddi_port_hotplug_long_detect(enum port port, u32
> val)
> +{
> +	switch (port) {
> +	case PORT_A:
> +		return val & ICP_DDIA_HPD_LONG_DETECT;
> +	case PORT_B:
> +		return val & ICP_DDIB_HPD_LONG_DETECT;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool icp_tc_port_hotplug_long_detect(enum port port, u32 val)
> +{
> +	switch (port) {
> +	case PORT_C:
> +		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1);
> +	case PORT_D:
> +		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2);
> +	case PORT_E:
> +		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3);
> +	case PORT_F:
> +		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4);
> +	default:
> +		return false;
> +	}
> +}
> +
>  static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
>  {
>  	switch (port) {
> @@ -2385,6 +2422,43 @@ static void cpt_irq_handler(struct
> drm_i915_private *dev_priv, u32 pch_iir)
>  		cpt_serr_int_handler(dev_priv);
>  }
>  
> +static void icp_irq_handler(struct drm_i915_private *dev_priv, u32
> pch_iir)
> +{
> +	u32 ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP;
> +	u32 tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP;
> +	u32 pin_mask = 0, long_mask = 0;
> +
> +	if (ddi_hotplug_trigger) {
> +		u32 dig_hotplug_reg;
> +
> +		dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_DDI);
> +		I915_WRITE(SHOTPLUG_CTL_DDI, dig_hotplug_reg);
> +
> +		intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> +				   ddi_hotplug_trigger,
> +				   dig_hotplug_reg, hpd_icp,
> +				   icp_ddi_port_hotplug_long_detect)
> ;
> +	}
> +
> +	if (tc_hotplug_trigger) {
> +		u32 dig_hotplug_reg;
> +
> +		dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_TC);
> +		I915_WRITE(SHOTPLUG_CTL_TC, dig_hotplug_reg);
> +
> +		intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> +				   tc_hotplug_trigger,
> +				   dig_hotplug_reg, hpd_icp,
> +				   icp_tc_port_hotplug_long_detect);
> +	}
> +
> +	if (pin_mask)
> +		intel_hpd_irq_handler(dev_priv, pin_mask,
> long_mask);
> +
> +	if (pch_iir & SDE_GMBUS_ICP)
> +		gmbus_irq_handler(dev_priv);
> +}
> +
>  static void spt_irq_handler(struct drm_i915_private *dev_priv, u32
> pch_iir)
>  {
>  	u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT &
> @@ -2804,8 +2878,11 @@ gen8_de_irq_handler(struct drm_i915_private
> *dev_priv, u32 master_ctl)
>  			I915_WRITE(SDEIIR, iir);
>  			ret = IRQ_HANDLED;
>  
> -			if (HAS_PCH_SPT(dev_priv) ||
> HAS_PCH_KBP(dev_priv) ||
> -			    HAS_PCH_CNP(dev_priv))
> +			if (HAS_PCH_ICP(dev_priv))
> +				icp_irq_handler(dev_priv, iir);
> +			else if (HAS_PCH_SPT(dev_priv) ||
> +				 HAS_PCH_KBP(dev_priv) ||
> +				 HAS_PCH_CNP(dev_priv))
>  				spt_irq_handler(dev_priv, iir);
>  			else
>  				cpt_irq_handler(dev_priv, iir);
> @@ -3584,6 +3661,9 @@ static void gen11_irq_reset(struct drm_device
> *dev)
>  	GEN3_IRQ_RESET(GEN11_DE_HPD_);
>  	GEN3_IRQ_RESET(GEN11_GU_MISC_);
>  	GEN3_IRQ_RESET(GEN8_PCU_);
> +
> +	if (HAS_PCH_ICP(dev_priv))
> +		GEN3_IRQ_RESET(SDE);
>  }
>  
>  void gen8_irq_power_well_post_enable(struct drm_i915_private
> *dev_priv,
> @@ -3700,6 +3780,35 @@ static void ibx_hpd_irq_setup(struct
> drm_i915_private *dev_priv)
>  	ibx_hpd_detection_setup(dev_priv);
>  }
>  
> +static void icp_hpd_detection_setup(struct drm_i915_private
> *dev_priv)
> +{
> +	u32 hotplug;
> +
> +	hotplug = I915_READ(SHOTPLUG_CTL_DDI);
> +	hotplug |= ICP_DDIA_HPD_ENABLE |
> +		   ICP_DDIB_HPD_ENABLE;
> +	I915_WRITE(SHOTPLUG_CTL_DDI, hotplug);
> +
> +	hotplug = I915_READ(SHOTPLUG_CTL_TC);
> +	hotplug |= ICP_TC_HPD_ENABLE(PORT_TC1) |
> +		   ICP_TC_HPD_ENABLE(PORT_TC2) |
> +		   ICP_TC_HPD_ENABLE(PORT_TC3) |
> +		   ICP_TC_HPD_ENABLE(PORT_TC4);
> +	I915_WRITE(SHOTPLUG_CTL_TC, hotplug);
> +}
> +
> +static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv)
> +{
> +	u32 hotplug_irqs, enabled_irqs;
> +
> +	hotplug_irqs = SDE_DDI_MASK_ICP | SDE_TC_MASK_ICP;
> +	enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_icp);
> +
> +	ibx_display_interrupt_update(dev_priv, hotplug_irqs,
> enabled_irqs);
> +
> +	icp_hpd_detection_setup(dev_priv);
> +}
> +
>  static void gen11_hpd_detection_setup(struct drm_i915_private
> *dev_priv)
>  {
>  	u32 hotplug;
> @@ -3733,6 +3842,9 @@ static void gen11_hpd_irq_setup(struct
> drm_i915_private *dev_priv)
>  	POSTING_READ(GEN11_DE_HPD_IMR);
>  
>  	gen11_hpd_detection_setup(dev_priv);
> +
> +	if (HAS_PCH_ICP(dev_priv))
> +		icp_hpd_irq_setup(dev_priv);
>  }
>  
>  static void spt_hpd_detection_setup(struct drm_i915_private
> *dev_priv)
> @@ -4168,11 +4280,29 @@ static void gen11_gt_irq_postinstall(struct
> drm_i915_private *dev_priv)
>  	I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_MASK,  ~0);
>  }
>  
> +static void icp_irq_postinstall(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	u32 mask = SDE_GMBUS_ICP;
> +
> +	WARN_ON(I915_READ(SDEIER) != 0);
> +	I915_WRITE(SDEIER, 0xffffffff);
> +	POSTING_READ(SDEIER);
> +
> +	gen3_assert_iir_is_zero(dev_priv, SDEIIR);
> +	I915_WRITE(SDEIMR, ~mask);
> +
> +	icp_hpd_detection_setup(dev_priv);

Having a sentence/paragraph on the commit message explaining why we can
afford to do this instead of going the ibx_irq_pre_postinstall()
followed by ibx_irq_postinstall() done by the previous platforms would
help.

My understanding is that this is OK, but a confirmation would always be
fine.


> +}
> +
>  static int gen11_irq_postinstall(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 gu_misc_masked = GEN11_GU_MISC_GSE;
>  
> +	if (HAS_PCH_ICP(dev_priv))
> +		icp_irq_postinstall(dev);
> +
>  	gen11_gt_irq_postinstall(dev_priv);
>  	gen8_de_irq_postinstall(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 4bfd7a9..e347055 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7517,6 +7517,46 @@ enum {
>  #define  PORTE_HOTPLUG_SHORT_DETECT	(1 << 0)
>  #define  PORTE_HOTPLUG_LONG_DETECT	(2 << 0)
>  
> +/* ICP */
> +#define   SDE_TC4_HOTPLUG_ICP		(1 << 27)
> +#define   SDE_TC3_HOTPLUG_ICP		(1 << 26)
> +#define   SDE_TC2_HOTPLUG_ICP		(1 << 25)
> +#define   SDE_TC1_HOTPLUG_ICP		(1 << 24)
> +#define   SDE_GMBUS_ICP			(1 << 23)
> +#define   SDE_DDIB_HOTPLUG_ICP		(1 << 17)
> +#define   SDE_DDIA_HOTPLUG_ICP		(1 << 16)
> +
> +#define SDE_DDI_MASK_ICP		(SDE_DDIB_HOTPLUG_ICP |	
> \
> +					 SDE_DDIA_HOTPLUG_ICP)
> +
> +#define SDE_TC_MASK_ICP			(SDE_TC4_HOTPLUG_ICP
> |	\
> +					 SDE_TC3_HOTPLUG_ICP |	
> \
> +					 SDE_TC2_HOTPLUG_ICP |	
> \
> +					 SDE_TC1_HOTPLUG_ICP)

Now these definitions are right below PCH_PORT_HOTPLUG2, but these bits
are not for that register. Please move the definitions to the place
that has the other SDE definitions. Also please change that /* ICP */
comment to match the style used by the other PCHs: /* south display
engine interrupt: ICP */ (also possibly fixing the "CPT/PPT" comment to
"CPT - CNP" or something like that.


> +
> +/* This register is a reuse of PCH_PORT_HOTPLUG register. The Spec
> + * has the name SHOTPLUG_CTL_DDI for ICP. Let us stick to the Spec.

I think the best way to describe would be to say that what was
previously done by a single register (PCH_PORT_HOTPLUG) is now done by
two registers, one for DDI and the other for TC, and that's why we have
brand new definitions.

Otherwise, looks good.

Thanks,
Paulo

> + */
> +
> +#define SHOTPLUG_CTL_DDI			_MMIO(0xc4030)
> +#define   ICP_DDIB_HPD_ENABLE			(1 << 7)
> +#define   ICP_DDIB_HPD_STATUS_MASK		(3 << 4)
> +#define   ICP_DDIB_HPD_NO_DETECT		(0 << 4)
> +#define   ICP_DDIB_HPD_SHORT_DETECT		(1 << 4)
> +#define   ICP_DDIB_HPD_LONG_DETECT		(2 << 4)
> +#define   ICP_DDIB_HPD_SHORT_LONG_DETECT	(3 << 4)
> +#define   ICP_DDIA_HPD_ENABLE			(1 << 3)
> +#define   ICP_DDIA_HPD_STATUS_MASK		(3 << 0)
> +#define   ICP_DDIA_HPD_NO_DETECT		(0 << 0)
> +#define   ICP_DDIA_HPD_SHORT_DETECT		(1 << 0)
> +#define   ICP_DDIA_HPD_LONG_DETECT		(2 << 0)
> +#define   ICP_DDIA_HPD_SHORT_LONG_DETECT	(3 << 0)
> +
> +#define SHOTPLUG_CTL_TC				_MMIO(0xc4034
> )
> +#define   ICP_TC_HPD_ENABLE(tc_port)		(8 << (tc_port)
> * 4)
> +#define   ICP_TC_HPD_LONG_DETECT(tc_port)	(2 << (tc_port) *
> 4)
> +#define   ICP_TC_HPD_SHORT_DETECT(tc_port)	(1 << (tc_port) *
> 4)
> +
>  #define PCH_GPIOA               _MMIO(0xc5010)
>  #define PCH_GPIOB               _MMIO(0xc5014)
>  #define PCH_GPIOC               _MMIO(0xc5018)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/2] drm/i915/icp: Add Interrupt Support
  2018-06-25 23:17 ` [PATCH 1/2] " Paulo Zanoni
@ 2018-06-26 18:32   ` Srivatsa, Anusha
  2018-06-26 20:09     ` Paulo Zanoni
  0 siblings, 1 reply; 10+ messages in thread
From: Srivatsa, Anusha @ 2018-06-26 18:32 UTC (permalink / raw)
  To: Zanoni, Paulo R, intel-gfx; +Cc: Pandiyan, Dhinakaran


________________________________________
From: Zanoni, Paulo R
Sent: Monday, June 25, 2018 4:17 PM
To: Srivatsa, Anusha; intel-gfx@lists.freedesktop.org
Cc: Pandiyan, Dhinakaran
Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/icp: Add Interrupt Support

Em Qua, 2018-06-20 às 14:36 -0700, Anusha Srivatsa escreveu:
> This patch addresses Interrupts from south display engine (SDE).
>
> ICP has two registers - SHOTPLUG_CTL_DDI and SHOTPLUG_CTL_TC.
> Introduce these registers and their intended values.
>
> Introduce icp_irq_handler().
>
> v2:
> - remove redundant register defines.(Lucas)
> - Change register names to be more consistent with
> previous platforms (Lucas)
>
> Cc: Lucas De Marchi <lucas.de.marchi@gmail.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> [Paulo: coding style bikesheds and rebases].
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 134
> +++++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h |  40 ++++++++++++
>  2 files changed, 172 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 46aaef5..7a7c4a2 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -122,6 +122,15 @@ static const u32 hpd_gen11[HPD_NUM_PINS] = {
>       [HPD_PORT_F] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG
>  };
>
> +static const u32 hpd_icp[HPD_NUM_PINS] = {
> +     [HPD_PORT_A] = SDE_DDIA_HOTPLUG_ICP,
> +     [HPD_PORT_B] = SDE_DDIB_HOTPLUG_ICP,
> +     [HPD_PORT_C] = SDE_TC1_HOTPLUG_ICP,
> +     [HPD_PORT_D] = SDE_TC2_HOTPLUG_ICP,
> +     [HPD_PORT_E] = SDE_TC3_HOTPLUG_ICP,
> +     [HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP
> +};
> +
>  /* IIR can theoretically queue up two events. Be paranoid. */
>  #define GEN8_IRQ_RESET_NDX(type, which) do { \
>       I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
> @@ -1586,6 +1595,34 @@ static bool bxt_port_hotplug_long_detect(enum
> port port, u32 val)
>       }
>  }
>
> +static bool icp_ddi_port_hotplug_long_detect(enum port port, u32
> val)
> +{
> +     switch (port) {
> +     case PORT_A:
> +             return val & ICP_DDIA_HPD_LONG_DETECT;
> +     case PORT_B:
> +             return val & ICP_DDIB_HPD_LONG_DETECT;
> +     default:
> +             return false;
> +     }
> +}
> +
> +static bool icp_tc_port_hotplug_long_detect(enum port port, u32 val)
> +{
> +     switch (port) {
> +     case PORT_C:
> +             return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1);
> +     case PORT_D:
> +             return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2);
> +     case PORT_E:
> +             return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3);
> +     case PORT_F:
> +             return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4);
> +     default:
> +             return false;
> +     }
> +}
> +
>  static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
>  {
>       switch (port) {
> @@ -2385,6 +2422,43 @@ static void cpt_irq_handler(struct
> drm_i915_private *dev_priv, u32 pch_iir)
>               cpt_serr_int_handler(dev_priv);
>  }
>
> +static void icp_irq_handler(struct drm_i915_private *dev_priv, u32
> pch_iir)
> +{
> +     u32 ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP;
> +     u32 tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP;
> +     u32 pin_mask = 0, long_mask = 0;
> +
> +     if (ddi_hotplug_trigger) {
> +             u32 dig_hotplug_reg;
> +
> +             dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_DDI);
> +             I915_WRITE(SHOTPLUG_CTL_DDI, dig_hotplug_reg);
> +
> +             intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> +                                ddi_hotplug_trigger,
> +                                dig_hotplug_reg, hpd_icp,
> +                                icp_ddi_port_hotplug_long_detect)
> ;
> +     }
> +
> +     if (tc_hotplug_trigger) {
> +             u32 dig_hotplug_reg;
> +
> +             dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_TC);
> +             I915_WRITE(SHOTPLUG_CTL_TC, dig_hotplug_reg);
> +
> +             intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> +                                tc_hotplug_trigger,
> +                                dig_hotplug_reg, hpd_icp,
> +                                icp_tc_port_hotplug_long_detect);
> +     }
> +
> +     if (pin_mask)
> +             intel_hpd_irq_handler(dev_priv, pin_mask,
> long_mask);
> +
> +     if (pch_iir & SDE_GMBUS_ICP)
> +             gmbus_irq_handler(dev_priv);
> +}
> +
>  static void spt_irq_handler(struct drm_i915_private *dev_priv, u32
> pch_iir)
>  {
>       u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT &
> @@ -2804,8 +2878,11 @@ gen8_de_irq_handler(struct drm_i915_private
> *dev_priv, u32 master_ctl)
>                       I915_WRITE(SDEIIR, iir);
>                       ret = IRQ_HANDLED;
>
> -                     if (HAS_PCH_SPT(dev_priv) ||
> HAS_PCH_KBP(dev_priv) ||
> -                         HAS_PCH_CNP(dev_priv))
> +                     if (HAS_PCH_ICP(dev_priv))
> +                             icp_irq_handler(dev_priv, iir);
> +                     else if (HAS_PCH_SPT(dev_priv) ||
> +                              HAS_PCH_KBP(dev_priv) ||
> +                              HAS_PCH_CNP(dev_priv))
>                               spt_irq_handler(dev_priv, iir);
>                       else
>                               cpt_irq_handler(dev_priv, iir);
> @@ -3584,6 +3661,9 @@ static void gen11_irq_reset(struct drm_device
> *dev)
>       GEN3_IRQ_RESET(GEN11_DE_HPD_);
>       GEN3_IRQ_RESET(GEN11_GU_MISC_);
>       GEN3_IRQ_RESET(GEN8_PCU_);
> +
> +     if (HAS_PCH_ICP(dev_priv))
> +             GEN3_IRQ_RESET(SDE);
>  }
>
>  void gen8_irq_power_well_post_enable(struct drm_i915_private
> *dev_priv,
> @@ -3700,6 +3780,35 @@ static void ibx_hpd_irq_setup(struct
> drm_i915_private *dev_priv)
>       ibx_hpd_detection_setup(dev_priv);
>  }
>
> +static void icp_hpd_detection_setup(struct drm_i915_private
> *dev_priv)
> +{
> +     u32 hotplug;
> +
> +     hotplug = I915_READ(SHOTPLUG_CTL_DDI);
> +     hotplug |= ICP_DDIA_HPD_ENABLE |
> +                ICP_DDIB_HPD_ENABLE;
> +     I915_WRITE(SHOTPLUG_CTL_DDI, hotplug);
> +
> +     hotplug = I915_READ(SHOTPLUG_CTL_TC);
> +     hotplug |= ICP_TC_HPD_ENABLE(PORT_TC1) |
> +                ICP_TC_HPD_ENABLE(PORT_TC2) |
> +                ICP_TC_HPD_ENABLE(PORT_TC3) |
> +                ICP_TC_HPD_ENABLE(PORT_TC4);
> +     I915_WRITE(SHOTPLUG_CTL_TC, hotplug);
> +}
> +
> +static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv)
> +{
> +     u32 hotplug_irqs, enabled_irqs;
> +
> +     hotplug_irqs = SDE_DDI_MASK_ICP | SDE_TC_MASK_ICP;
> +     enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_icp);
> +
> +     ibx_display_interrupt_update(dev_priv, hotplug_irqs,
> enabled_irqs);
> +
> +     icp_hpd_detection_setup(dev_priv);
> +}
> +
>  static void gen11_hpd_detection_setup(struct drm_i915_private
> *dev_priv)
>  {
>       u32 hotplug;
> @@ -3733,6 +3842,9 @@ static void gen11_hpd_irq_setup(struct
> drm_i915_private *dev_priv)
>       POSTING_READ(GEN11_DE_HPD_IMR);
>
>       gen11_hpd_detection_setup(dev_priv);
> +
> +     if (HAS_PCH_ICP(dev_priv))
> +             icp_hpd_irq_setup(dev_priv);
>  }
>
>  static void spt_hpd_detection_setup(struct drm_i915_private
> *dev_priv)
> @@ -4168,11 +4280,29 @@ static void gen11_gt_irq_postinstall(struct
> drm_i915_private *dev_priv)
>       I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_MASK,  ~0);
>  }
>
> +static void icp_irq_postinstall(struct drm_device *dev)
> +{
> +     struct drm_i915_private *dev_priv = to_i915(dev);
> +     u32 mask = SDE_GMBUS_ICP;
> +
> +     WARN_ON(I915_READ(SDEIER) != 0);
> +     I915_WRITE(SDEIER, 0xffffffff);
> +     POSTING_READ(SDEIER);
> +
> +     gen3_assert_iir_is_zero(dev_priv, SDEIIR);
> +     I915_WRITE(SDEIMR, ~mask);
> +
> +     icp_hpd_detection_setup(dev_priv);

Having a sentence/paragraph on the commit message explaining why we can
afford to do this instead of going the ibx_irq_pre_postinstall()
followed by ibx_irq_postinstall() done by the previous platforms would
help.

My understanding is that this is OK, but a confirmation would always be
fine.


> +}
> +
>  static int gen11_irq_postinstall(struct drm_device *dev)
>  {
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       u32 gu_misc_masked = GEN11_GU_MISC_GSE;
>
> +     if (HAS_PCH_ICP(dev_priv))
> +             icp_irq_postinstall(dev);
> +
>       gen11_gt_irq_postinstall(dev_priv);
>       gen8_de_irq_postinstall(dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 4bfd7a9..e347055 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7517,6 +7517,46 @@ enum {
>  #define  PORTE_HOTPLUG_SHORT_DETECT  (1 << 0)
>  #define  PORTE_HOTPLUG_LONG_DETECT   (2 << 0)
>
> +/* ICP */
> +#define   SDE_TC4_HOTPLUG_ICP                (1 << 27)
> +#define   SDE_TC3_HOTPLUG_ICP                (1 << 26)
> +#define   SDE_TC2_HOTPLUG_ICP                (1 << 25)
> +#define   SDE_TC1_HOTPLUG_ICP                (1 << 24)
> +#define   SDE_GMBUS_ICP                      (1 << 23)
> +#define   SDE_DDIB_HOTPLUG_ICP               (1 << 17)
> +#define   SDE_DDIA_HOTPLUG_ICP               (1 << 16)
> +
> +#define SDE_DDI_MASK_ICP             (SDE_DDIB_HOTPLUG_ICP |
> \
> +                                      SDE_DDIA_HOTPLUG_ICP)
> +
> +#define SDE_TC_MASK_ICP                      (SDE_TC4_HOTPLUG_ICP
> |     \
> +                                      SDE_TC3_HOTPLUG_ICP |
> \
> +                                      SDE_TC2_HOTPLUG_ICP |
> \
> +                                      SDE_TC1_HOTPLUG_ICP)

Now these definitions are right below PCH_PORT_HOTPLUG2, but these bits
are not for that register. Please move the definitions to the place
that has the other SDE definitions. Also please change that /* ICP */
comment to match the style used by the other PCHs: /* south display
engine interrupt: ICP */ (also possibly fixing the "CPT/PPT" comment to
"CPT - CNP" or something like that.
Yes. That is good point.
QQ, the change of "CPT/PPT...." will be a separate patch though, right?

> +
> +/* This register is a reuse of PCH_PORT_HOTPLUG register. The Spec
> + * has the name SHOTPLUG_CTL_DDI for ICP. Let us stick to the Spec.

I think the best way to describe would be to say that what was
previously done by a single register (PCH_PORT_HOTPLUG) is now done by
two registers, one for DDI and the other for TC, and that's why we have
brand new definitions.

Thanks paulo for the reviews. Will make the changes.

Anusha 
Otherwise, looks good.

Thanks,
Paulo

> + */
> +
> +#define SHOTPLUG_CTL_DDI                     _MMIO(0xc4030)
> +#define   ICP_DDIB_HPD_ENABLE                        (1 << 7)
> +#define   ICP_DDIB_HPD_STATUS_MASK           (3 << 4)
> +#define   ICP_DDIB_HPD_NO_DETECT             (0 << 4)
> +#define   ICP_DDIB_HPD_SHORT_DETECT          (1 << 4)
> +#define   ICP_DDIB_HPD_LONG_DETECT           (2 << 4)
> +#define   ICP_DDIB_HPD_SHORT_LONG_DETECT     (3 << 4)
> +#define   ICP_DDIA_HPD_ENABLE                        (1 << 3)
> +#define   ICP_DDIA_HPD_STATUS_MASK           (3 << 0)
> +#define   ICP_DDIA_HPD_NO_DETECT             (0 << 0)
> +#define   ICP_DDIA_HPD_SHORT_DETECT          (1 << 0)
> +#define   ICP_DDIA_HPD_LONG_DETECT           (2 << 0)
> +#define   ICP_DDIA_HPD_SHORT_LONG_DETECT     (3 << 0)
> +
> +#define SHOTPLUG_CTL_TC                              _MMIO(0xc4034
> )
> +#define   ICP_TC_HPD_ENABLE(tc_port)         (8 << (tc_port)
> * 4)
> +#define   ICP_TC_HPD_LONG_DETECT(tc_port)    (2 << (tc_port) *
> 4)
> +#define   ICP_TC_HPD_SHORT_DETECT(tc_port)   (1 << (tc_port) *
> 4)
> +
>  #define PCH_GPIOA               _MMIO(0xc5010)
>  #define PCH_GPIOB               _MMIO(0xc5014)
>  #define PCH_GPIOC               _MMIO(0xc5018)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/2] drm/i915/icp: Add Interrupt Support
  2018-06-26 18:32   ` Srivatsa, Anusha
@ 2018-06-26 20:09     ` Paulo Zanoni
  0 siblings, 0 replies; 10+ messages in thread
From: Paulo Zanoni @ 2018-06-26 20:09 UTC (permalink / raw)
  To: Srivatsa, Anusha, intel-gfx; +Cc: Pandiyan, Dhinakaran

Em Ter, 2018-06-26 às 11:32 -0700, Srivatsa, Anusha escreveu:
> ________________________________________
> From: Zanoni, Paulo R
> Sent: Monday, June 25, 2018 4:17 PM
> To: Srivatsa, Anusha; intel-gfx@lists.freedesktop.org
> Cc: Pandiyan, Dhinakaran
> Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/icp: Add Interrupt
> Support
> 
> Em Qua, 2018-06-20 às 14:36 -0700, Anusha Srivatsa escreveu:
> > This patch addresses Interrupts from south display engine (SDE).
> > 
> > ICP has two registers - SHOTPLUG_CTL_DDI and SHOTPLUG_CTL_TC.
> > Introduce these registers and their intended values.
> > 
> > Introduce icp_irq_handler().
> > 
> > v2:
> > - remove redundant register defines.(Lucas)
> > - Change register names to be more consistent with
> > previous platforms (Lucas)
> > 
> > Cc: Lucas De Marchi <lucas.de.marchi@gmail.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > [Paulo: coding style bikesheds and rebases].
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 134
> > +++++++++++++++++++++++++++++++++++++++-
> >  drivers/gpu/drm/i915/i915_reg.h |  40 ++++++++++++
> >  2 files changed, 172 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 46aaef5..7a7c4a2 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -122,6 +122,15 @@ static const u32 hpd_gen11[HPD_NUM_PINS] = {
> >       [HPD_PORT_F] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG
> >  };
> > 
> > +static const u32 hpd_icp[HPD_NUM_PINS] = {
> > +     [HPD_PORT_A] = SDE_DDIA_HOTPLUG_ICP,
> > +     [HPD_PORT_B] = SDE_DDIB_HOTPLUG_ICP,
> > +     [HPD_PORT_C] = SDE_TC1_HOTPLUG_ICP,
> > +     [HPD_PORT_D] = SDE_TC2_HOTPLUG_ICP,
> > +     [HPD_PORT_E] = SDE_TC3_HOTPLUG_ICP,
> > +     [HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP
> > +};
> > +
> >  /* IIR can theoretically queue up two events. Be paranoid. */
> >  #define GEN8_IRQ_RESET_NDX(type, which) do { \
> >       I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
> > @@ -1586,6 +1595,34 @@ static bool
> > bxt_port_hotplug_long_detect(enum
> > port port, u32 val)
> >       }
> >  }
> > 
> > +static bool icp_ddi_port_hotplug_long_detect(enum port port, u32
> > val)
> > +{
> > +     switch (port) {
> > +     case PORT_A:
> > +             return val & ICP_DDIA_HPD_LONG_DETECT;
> > +     case PORT_B:
> > +             return val & ICP_DDIB_HPD_LONG_DETECT;
> > +     default:
> > +             return false;
> > +     }
> > +}
> > +
> > +static bool icp_tc_port_hotplug_long_detect(enum port port, u32
> > val)
> > +{
> > +     switch (port) {
> > +     case PORT_C:
> > +             return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1);
> > +     case PORT_D:
> > +             return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2);
> > +     case PORT_E:
> > +             return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3);
> > +     case PORT_F:
> > +             return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4);
> > +     default:
> > +             return false;
> > +     }
> > +}
> > +
> >  static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
> >  {
> >       switch (port) {
> > @@ -2385,6 +2422,43 @@ static void cpt_irq_handler(struct
> > drm_i915_private *dev_priv, u32 pch_iir)
> >               cpt_serr_int_handler(dev_priv);
> >  }
> > 
> > +static void icp_irq_handler(struct drm_i915_private *dev_priv, u32
> > pch_iir)
> > +{
> > +     u32 ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP;
> > +     u32 tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP;
> > +     u32 pin_mask = 0, long_mask = 0;
> > +
> > +     if (ddi_hotplug_trigger) {
> > +             u32 dig_hotplug_reg;
> > +
> > +             dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_DDI);
> > +             I915_WRITE(SHOTPLUG_CTL_DDI, dig_hotplug_reg);
> > +
> > +             intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> > +                                ddi_hotplug_trigger,
> > +                                dig_hotplug_reg, hpd_icp,
> > +                                icp_ddi_port_hotplug_long_detect)
> > ;
> > +     }
> > +
> > +     if (tc_hotplug_trigger) {
> > +             u32 dig_hotplug_reg;
> > +
> > +             dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_TC);
> > +             I915_WRITE(SHOTPLUG_CTL_TC, dig_hotplug_reg);
> > +
> > +             intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> > +                                tc_hotplug_trigger,
> > +                                dig_hotplug_reg, hpd_icp,
> > +                                icp_tc_port_hotplug_long_detect);
> > +     }
> > +
> > +     if (pin_mask)
> > +             intel_hpd_irq_handler(dev_priv, pin_mask,
> > long_mask);
> > +
> > +     if (pch_iir & SDE_GMBUS_ICP)
> > +             gmbus_irq_handler(dev_priv);
> > +}
> > +
> >  static void spt_irq_handler(struct drm_i915_private *dev_priv, u32
> > pch_iir)
> >  {
> >       u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT &
> > @@ -2804,8 +2878,11 @@ gen8_de_irq_handler(struct drm_i915_private
> > *dev_priv, u32 master_ctl)
> >                       I915_WRITE(SDEIIR, iir);
> >                       ret = IRQ_HANDLED;
> > 
> > -                     if (HAS_PCH_SPT(dev_priv) ||
> > HAS_PCH_KBP(dev_priv) ||
> > -                         HAS_PCH_CNP(dev_priv))
> > +                     if (HAS_PCH_ICP(dev_priv))
> > +                             icp_irq_handler(dev_priv, iir);
> > +                     else if (HAS_PCH_SPT(dev_priv) ||
> > +                              HAS_PCH_KBP(dev_priv) ||
> > +                              HAS_PCH_CNP(dev_priv))
> >                               spt_irq_handler(dev_priv, iir);
> >                       else
> >                               cpt_irq_handler(dev_priv, iir);
> > @@ -3584,6 +3661,9 @@ static void gen11_irq_reset(struct drm_device
> > *dev)
> >       GEN3_IRQ_RESET(GEN11_DE_HPD_);
> >       GEN3_IRQ_RESET(GEN11_GU_MISC_);
> >       GEN3_IRQ_RESET(GEN8_PCU_);
> > +
> > +     if (HAS_PCH_ICP(dev_priv))
> > +             GEN3_IRQ_RESET(SDE);
> >  }
> > 
> >  void gen8_irq_power_well_post_enable(struct drm_i915_private
> > *dev_priv,
> > @@ -3700,6 +3780,35 @@ static void ibx_hpd_irq_setup(struct
> > drm_i915_private *dev_priv)
> >       ibx_hpd_detection_setup(dev_priv);
> >  }
> > 
> > +static void icp_hpd_detection_setup(struct drm_i915_private
> > *dev_priv)
> > +{
> > +     u32 hotplug;
> > +
> > +     hotplug = I915_READ(SHOTPLUG_CTL_DDI);
> > +     hotplug |= ICP_DDIA_HPD_ENABLE |
> > +                ICP_DDIB_HPD_ENABLE;
> > +     I915_WRITE(SHOTPLUG_CTL_DDI, hotplug);
> > +
> > +     hotplug = I915_READ(SHOTPLUG_CTL_TC);
> > +     hotplug |= ICP_TC_HPD_ENABLE(PORT_TC1) |
> > +                ICP_TC_HPD_ENABLE(PORT_TC2) |
> > +                ICP_TC_HPD_ENABLE(PORT_TC3) |
> > +                ICP_TC_HPD_ENABLE(PORT_TC4);
> > +     I915_WRITE(SHOTPLUG_CTL_TC, hotplug);
> > +}
> > +
> > +static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv)
> > +{
> > +     u32 hotplug_irqs, enabled_irqs;
> > +
> > +     hotplug_irqs = SDE_DDI_MASK_ICP | SDE_TC_MASK_ICP;
> > +     enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_icp);
> > +
> > +     ibx_display_interrupt_update(dev_priv, hotplug_irqs,
> > enabled_irqs);
> > +
> > +     icp_hpd_detection_setup(dev_priv);
> > +}
> > +
> >  static void gen11_hpd_detection_setup(struct drm_i915_private
> > *dev_priv)
> >  {
> >       u32 hotplug;
> > @@ -3733,6 +3842,9 @@ static void gen11_hpd_irq_setup(struct
> > drm_i915_private *dev_priv)
> >       POSTING_READ(GEN11_DE_HPD_IMR);
> > 
> >       gen11_hpd_detection_setup(dev_priv);
> > +
> > +     if (HAS_PCH_ICP(dev_priv))
> > +             icp_hpd_irq_setup(dev_priv);
> >  }
> > 
> >  static void spt_hpd_detection_setup(struct drm_i915_private
> > *dev_priv)
> > @@ -4168,11 +4280,29 @@ static void gen11_gt_irq_postinstall(struct
> > drm_i915_private *dev_priv)
> >       I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_MASK,  ~0);
> >  }
> > 
> > +static void icp_irq_postinstall(struct drm_device *dev)
> > +{
> > +     struct drm_i915_private *dev_priv = to_i915(dev);
> > +     u32 mask = SDE_GMBUS_ICP;
> > +
> > +     WARN_ON(I915_READ(SDEIER) != 0);
> > +     I915_WRITE(SDEIER, 0xffffffff);
> > +     POSTING_READ(SDEIER);
> > +
> > +     gen3_assert_iir_is_zero(dev_priv, SDEIIR);
> > +     I915_WRITE(SDEIMR, ~mask);
> > +
> > +     icp_hpd_detection_setup(dev_priv);
> 
> Having a sentence/paragraph on the commit message explaining why we
> can
> afford to do this instead of going the ibx_irq_pre_postinstall()
> followed by ibx_irq_postinstall() done by the previous platforms
> would
> help.
> 
> My understanding is that this is OK, but a confirmation would always
> be
> fine.
> 
> 
> > +}
> > +
> >  static int gen11_irq_postinstall(struct drm_device *dev)
> >  {
> >       struct drm_i915_private *dev_priv = dev->dev_private;
> >       u32 gu_misc_masked = GEN11_GU_MISC_GSE;
> > 
> > +     if (HAS_PCH_ICP(dev_priv))
> > +             icp_irq_postinstall(dev);
> > +
> >       gen11_gt_irq_postinstall(dev_priv);
> >       gen8_de_irq_postinstall(dev_priv);
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 4bfd7a9..e347055 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7517,6 +7517,46 @@ enum {
> >  #define  PORTE_HOTPLUG_SHORT_DETECT  (1 << 0)
> >  #define  PORTE_HOTPLUG_LONG_DETECT   (2 << 0)
> > 
> > +/* ICP */
> > +#define   SDE_TC4_HOTPLUG_ICP                (1 << 27)
> > +#define   SDE_TC3_HOTPLUG_ICP                (1 << 26)
> > +#define   SDE_TC2_HOTPLUG_ICP                (1 << 25)
> > +#define   SDE_TC1_HOTPLUG_ICP                (1 << 24)
> > +#define   SDE_GMBUS_ICP                      (1 << 23)
> > +#define   SDE_DDIB_HOTPLUG_ICP               (1 << 17)
> > +#define   SDE_DDIA_HOTPLUG_ICP               (1 << 16)
> > +
> > +#define SDE_DDI_MASK_ICP             (SDE_DDIB_HOTPLUG_ICP |
> > \
> > +                                      SDE_DDIA_HOTPLUG_ICP)
> > +
> > +#define SDE_TC_MASK_ICP                      (SDE_TC4_HOTPLUG_ICP
> > >     \
> > 
> > +                                      SDE_TC3_HOTPLUG_ICP |
> > \
> > +                                      SDE_TC2_HOTPLUG_ICP |
> > \
> > +                                      SDE_TC1_HOTPLUG_ICP)
> 
> Now these definitions are right below PCH_PORT_HOTPLUG2, but these
> bits
> are not for that register. Please move the definitions to the place
> that has the other SDE definitions. Also please change that /* ICP */
> comment to match the style used by the other PCHs: /* south display
> engine interrupt: ICP */ (also possibly fixing the "CPT/PPT" comment
> to
> "CPT - CNP" or something like that.
> Yes. That is good point.
> QQ, the change of "CPT/PPT...." will be a separate patch though,
> right?

Since it's just a single line comment change and it's related to your
current change (update to the bits of the register in question), I
would say there's no need to be in a separate patch IMHO, but of course
we would also accept the change in a separate patch.


> 
> > +
> > +/* This register is a reuse of PCH_PORT_HOTPLUG register. The Spec
> > + * has the name SHOTPLUG_CTL_DDI for ICP. Let us stick to the
> > Spec.
> 
> I think the best way to describe would be to say that what was
> previously done by a single register (PCH_PORT_HOTPLUG) is now done
> by
> two registers, one for DDI and the other for TC, and that's why we
> have
> brand new definitions.
> 
> Thanks paulo for the reviews. Will make the changes.
> 
> Anusha 
> Otherwise, looks good.

Please fix your email client to add ">" when you reply to emails, this
email was super confusing.

> 
> Thanks,
> Paulo
> 
> > + */
> > +
> > +#define SHOTPLUG_CTL_DDI                     _MMIO(0xc4030)
> > +#define   ICP_DDIB_HPD_ENABLE                        (1 << 7)
> > +#define   ICP_DDIB_HPD_STATUS_MASK           (3 << 4)
> > +#define   ICP_DDIB_HPD_NO_DETECT             (0 << 4)
> > +#define   ICP_DDIB_HPD_SHORT_DETECT          (1 << 4)
> > +#define   ICP_DDIB_HPD_LONG_DETECT           (2 << 4)
> > +#define   ICP_DDIB_HPD_SHORT_LONG_DETECT     (3 << 4)
> > +#define   ICP_DDIA_HPD_ENABLE                        (1 << 3)
> > +#define   ICP_DDIA_HPD_STATUS_MASK           (3 << 0)
> > +#define   ICP_DDIA_HPD_NO_DETECT             (0 << 0)
> > +#define   ICP_DDIA_HPD_SHORT_DETECT          (1 << 0)
> > +#define   ICP_DDIA_HPD_LONG_DETECT           (2 << 0)
> > +#define   ICP_DDIA_HPD_SHORT_LONG_DETECT     (3 << 0)
> > +
> > +#define SHOTPLUG_CTL_TC                              _MMIO(0xc4034
> > )
> > +#define   ICP_TC_HPD_ENABLE(tc_port)         (8 << (tc_port)
> > * 4)
> > +#define   ICP_TC_HPD_LONG_DETECT(tc_port)    (2 << (tc_port) *
> > 4)
> > +#define   ICP_TC_HPD_SHORT_DETECT(tc_port)   (1 << (tc_port) *
> > 4)
> > +
> >  #define PCH_GPIOA               _MMIO(0xc5010)
> >  #define PCH_GPIOB               _MMIO(0xc5014)
> >  #define PCH_GPIOC               _MMIO(0xc5018)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/2] drm/i915/icl: implement icl_digital_port_connected()
  2018-06-20 21:36 ` [PATCH 2/2] drm/i915/icl: implement icl_digital_port_connected() Anusha Srivatsa
@ 2018-06-30  6:42   ` Lucas De Marchi
  0 siblings, 0 replies; 10+ messages in thread
From: Lucas De Marchi @ 2018-06-30  6:42 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx, Lucas De Marchi, Paulo Zanoni, Rodrigo Vivi

On Wed, Jun 20, 2018 at 02:36:05PM -0700, Anusha Srivatsa wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Do like the other functions and check for the ISR bits. We have plans
> to add a few more checks in this code in the next patches, that's why
> it's a little more verbose than it could be.
> 
> v2:
> - Change the register names, to be consistent with
> the rest of the platforms.
> 
> Cc: Animesh Manna <animesh.manna@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

My review stands for v2 as done in v1:

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

> ---
>  drivers/gpu/drm/i915/i915_reg.h |  5 ++++
>  drivers/gpu/drm/i915/intel_dp.c | 57 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e347055..f55e889 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7093,6 +7093,7 @@ enum {
>  #define  GEN11_TC3_HOTPLUG			(1 << 18)
>  #define  GEN11_TC2_HOTPLUG			(1 << 17)
>  #define  GEN11_TC1_HOTPLUG			(1 << 16)
> +#define  GEN11_TC_HOTPLUG(tc_port)		(1 << ((tc_port) + 16))
>  #define  GEN11_DE_TC_HOTPLUG_MASK		(GEN11_TC4_HOTPLUG | \
>  						 GEN11_TC3_HOTPLUG | \
>  						 GEN11_TC2_HOTPLUG | \
> @@ -7101,6 +7102,7 @@ enum {
>  #define  GEN11_TBT3_HOTPLUG			(1 << 2)
>  #define  GEN11_TBT2_HOTPLUG			(1 << 1)
>  #define  GEN11_TBT1_HOTPLUG			(1 << 0)
> +#define  GEN11_TBT_HOTPLUG(tc_port)		(1 << (tc_port))
>  #define  GEN11_DE_TBT_HOTPLUG_MASK		(GEN11_TBT4_HOTPLUG | \
>  						 GEN11_TBT3_HOTPLUG | \
>  						 GEN11_TBT2_HOTPLUG | \
> @@ -7525,6 +7527,9 @@ enum {
>  #define   SDE_GMBUS_ICP			(1 << 23)
>  #define   SDE_DDIB_HOTPLUG_ICP		(1 << 17)
>  #define   SDE_DDIA_HOTPLUG_ICP		(1 << 16)
> +#define SDE_TC_HOTPLUG_ICP(tc_port)		(1 << ((tc_port) + 24))
> +#define SDE_DDI_HOTPLUG_ICP(port)		(1 << ((port) + 16))
> +
>  
>  #define SDE_DDI_MASK_ICP		(SDE_DDIB_HOTPLUG_ICP |	\
>  					 SDE_DDIA_HOTPLUG_ICP)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 6ac6c87..224cc71 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4750,6 +4750,61 @@ static bool bxt_digital_port_connected(struct intel_encoder *encoder)
>  	return I915_READ(GEN8_DE_PORT_ISR) & bit;
>  }
>  
> +static bool icl_combo_port_connected(struct drm_i915_private *dev_priv,
> +				     struct intel_digital_port *intel_dig_port)
> +{
> +	enum port port = intel_dig_port->base.port;
> +
> +	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
> +}
> +
> +static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
> +				  struct intel_digital_port *intel_dig_port)
> +{
> +	enum port port = intel_dig_port->base.port;
> +	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> +	u32 legacy_bit = SDE_TC_HOTPLUG_ICP(tc_port);
> +	u32 typec_bit = GEN11_TC_HOTPLUG(tc_port);
> +	u32 tbt_bit = GEN11_TBT_HOTPLUG(tc_port);
> +	bool is_legacy = false, is_typec = false, is_tbt = false;
> +	u32 cpu_isr;
> +
> +	if (I915_READ(SDEISR) & legacy_bit)
> +		is_legacy = true;
> +
> +	cpu_isr = I915_READ(GEN11_DE_HPD_ISR);
> +	if (cpu_isr & typec_bit)
> +		is_typec = true;
> +	if (cpu_isr & tbt_bit)
> +		is_tbt = true;
> +
> +	WARN_ON(is_legacy + is_typec + is_tbt > 1);
> +	if (!is_legacy && !is_typec && !is_tbt)
> +		return false;
> +
> +	return true;
> +}
> +
> +static bool icl_digital_port_connected(struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> +
> +	switch (encoder->hpd_pin) {
> +	case HPD_PORT_A:
> +	case HPD_PORT_B:
> +		return icl_combo_port_connected(dev_priv, dig_port);
> +	case HPD_PORT_C:
> +	case HPD_PORT_D:
> +	case HPD_PORT_E:
> +	case HPD_PORT_F:
> +		return icl_tc_port_connected(dev_priv, dig_port);
> +	default:
> +		MISSING_CASE(encoder->hpd_pin);
> +		return false;
> +	}
> +}
> +
>  /*
>   * intel_digital_port_connected - is the specified port connected?
>   * @encoder: intel_encoder
> @@ -4777,6 +4832,8 @@ bool intel_digital_port_connected(struct intel_encoder *encoder)
>  		return bdw_digital_port_connected(encoder);
>  	else if (IS_GEN9_LP(dev_priv))
>  		return bxt_digital_port_connected(encoder);
> +	else if (IS_ICELAKE(dev_priv))
> +		return icl_digital_port_connected(encoder);
>  	else
>  		return spt_digital_port_connected(encoder);
>  }
> -- 
> 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

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

* ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/icp: Add Interrupt Support
  2018-06-20 21:36 [PATCH 1/2] drm/i915/icp: Add Interrupt Support Anusha Srivatsa
                   ` (3 preceding siblings ...)
  2018-06-25 23:17 ` [PATCH 1/2] " Paulo Zanoni
@ 2018-07-23 17:05 ` Patchwork
  4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-07-23 17:05 UTC (permalink / raw)
  To: Srivatsa, Anusha; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/icp: Add Interrupt Support
URL   : https://patchwork.freedesktop.org/series/45124/
State : failure

== Summary ==

Applying: drm/i915/icp: Add Interrupt Support
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_irq.c
M	drivers/gpu/drm/i915/i915_reg.h
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/i915_reg.h
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_reg.h
Auto-merging drivers/gpu/drm/i915/i915_irq.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_irq.c
error: Failed to merge in the changes.
Patch failed at 0001 drm/i915/icp: Add Interrupt Support
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9375/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 1/2] drm/i915/icp: Add Interrupt Support
@ 2018-06-20 21:02 Anusha Srivatsa
  0 siblings, 0 replies; 10+ messages in thread
From: Anusha Srivatsa @ 2018-06-20 21:02 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni

This patch addresses Interrupts from south display engine (SDE).

ICP has two registers - SHOTPLUG_CTL_DDI and SHOTPLUG_CTL_TC.
Introduce these registers and their intended values.

Introduce icp_irq_handler().

v2:
- remove redundant register defines.(Lucas)
- Change register names to be more consistent with
previous platforms (Lucas)

Cc: Lucas De Marchi <lucas.de.marchi@gmail.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
[Paulo: coding style bikesheds and rebases].
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 134 +++++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h |  40 ++++++++++++
 2 files changed, 172 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 46aaef5..7a7c4a2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -122,6 +122,15 @@ static const u32 hpd_gen11[HPD_NUM_PINS] = {
 	[HPD_PORT_F] = GEN11_TC4_HOTPLUG | GEN11_TBT4_HOTPLUG
 };
 
+static const u32 hpd_icp[HPD_NUM_PINS] = {
+	[HPD_PORT_A] = SDE_DDIA_HOTPLUG_ICP,
+	[HPD_PORT_B] = SDE_DDIB_HOTPLUG_ICP,
+	[HPD_PORT_C] = SDE_TC1_HOTPLUG_ICP,
+	[HPD_PORT_D] = SDE_TC2_HOTPLUG_ICP,
+	[HPD_PORT_E] = SDE_TC3_HOTPLUG_ICP,
+	[HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP
+};
+
 /* IIR can theoretically queue up two events. Be paranoid. */
 #define GEN8_IRQ_RESET_NDX(type, which) do { \
 	I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
@@ -1586,6 +1595,34 @@ static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
 	}
 }
 
+static bool icp_ddi_port_hotplug_long_detect(enum port port, u32 val)
+{
+	switch (port) {
+	case PORT_A:
+		return val & ICP_DDIA_HPD_LONG_DETECT;
+	case PORT_B:
+		return val & ICP_DDIB_HPD_LONG_DETECT;
+	default:
+		return false;
+	}
+}
+
+static bool icp_tc_port_hotplug_long_detect(enum port port, u32 val)
+{
+	switch (port) {
+	case PORT_C:
+		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1);
+	case PORT_D:
+		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2);
+	case PORT_E:
+		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3);
+	case PORT_F:
+		return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4);
+	default:
+		return false;
+	}
+}
+
 static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
 {
 	switch (port) {
@@ -2385,6 +2422,43 @@ static void cpt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
 		cpt_serr_int_handler(dev_priv);
 }
 
+static void icp_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
+{
+	u32 ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP;
+	u32 tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP;
+	u32 pin_mask = 0, long_mask = 0;
+
+	if (ddi_hotplug_trigger) {
+		u32 dig_hotplug_reg;
+
+		dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_DDI);
+		I915_WRITE(SHOTPLUG_CTL_DDI, dig_hotplug_reg);
+
+		intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
+				   ddi_hotplug_trigger,
+				   dig_hotplug_reg, hpd_icp,
+				   icp_ddi_port_hotplug_long_detect);
+	}
+
+	if (tc_hotplug_trigger) {
+		u32 dig_hotplug_reg;
+
+		dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_TC);
+		I915_WRITE(SHOTPLUG_CTL_TC, dig_hotplug_reg);
+
+		intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
+				   tc_hotplug_trigger,
+				   dig_hotplug_reg, hpd_icp,
+				   icp_tc_port_hotplug_long_detect);
+	}
+
+	if (pin_mask)
+		intel_hpd_irq_handler(dev_priv, pin_mask, long_mask);
+
+	if (pch_iir & SDE_GMBUS_ICP)
+		gmbus_irq_handler(dev_priv);
+}
+
 static void spt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
 {
 	u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT &
@@ -2804,8 +2878,11 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 			I915_WRITE(SDEIIR, iir);
 			ret = IRQ_HANDLED;
 
-			if (HAS_PCH_SPT(dev_priv) || HAS_PCH_KBP(dev_priv) ||
-			    HAS_PCH_CNP(dev_priv))
+			if (HAS_PCH_ICP(dev_priv))
+				icp_irq_handler(dev_priv, iir);
+			else if (HAS_PCH_SPT(dev_priv) ||
+				 HAS_PCH_KBP(dev_priv) ||
+				 HAS_PCH_CNP(dev_priv))
 				spt_irq_handler(dev_priv, iir);
 			else
 				cpt_irq_handler(dev_priv, iir);
@@ -3584,6 +3661,9 @@ static void gen11_irq_reset(struct drm_device *dev)
 	GEN3_IRQ_RESET(GEN11_DE_HPD_);
 	GEN3_IRQ_RESET(GEN11_GU_MISC_);
 	GEN3_IRQ_RESET(GEN8_PCU_);
+
+	if (HAS_PCH_ICP(dev_priv))
+		GEN3_IRQ_RESET(SDE);
 }
 
 void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
@@ -3700,6 +3780,35 @@ static void ibx_hpd_irq_setup(struct drm_i915_private *dev_priv)
 	ibx_hpd_detection_setup(dev_priv);
 }
 
+static void icp_hpd_detection_setup(struct drm_i915_private *dev_priv)
+{
+	u32 hotplug;
+
+	hotplug = I915_READ(SHOTPLUG_CTL_DDI);
+	hotplug |= ICP_DDIA_HPD_ENABLE |
+		   ICP_DDIB_HPD_ENABLE;
+	I915_WRITE(SHOTPLUG_CTL_DDI, hotplug);
+
+	hotplug = I915_READ(SHOTPLUG_CTL_TC);
+	hotplug |= ICP_TC_HPD_ENABLE(PORT_TC1) |
+		   ICP_TC_HPD_ENABLE(PORT_TC2) |
+		   ICP_TC_HPD_ENABLE(PORT_TC3) |
+		   ICP_TC_HPD_ENABLE(PORT_TC4);
+	I915_WRITE(SHOTPLUG_CTL_TC, hotplug);
+}
+
+static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv)
+{
+	u32 hotplug_irqs, enabled_irqs;
+
+	hotplug_irqs = SDE_DDI_MASK_ICP | SDE_TC_MASK_ICP;
+	enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_icp);
+
+	ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
+
+	icp_hpd_detection_setup(dev_priv);
+}
+
 static void gen11_hpd_detection_setup(struct drm_i915_private *dev_priv)
 {
 	u32 hotplug;
@@ -3733,6 +3842,9 @@ static void gen11_hpd_irq_setup(struct drm_i915_private *dev_priv)
 	POSTING_READ(GEN11_DE_HPD_IMR);
 
 	gen11_hpd_detection_setup(dev_priv);
+
+	if (HAS_PCH_ICP(dev_priv))
+		icp_hpd_irq_setup(dev_priv);
 }
 
 static void spt_hpd_detection_setup(struct drm_i915_private *dev_priv)
@@ -4168,11 +4280,29 @@ static void gen11_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_MASK,  ~0);
 }
 
+static void icp_irq_postinstall(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	u32 mask = SDE_GMBUS_ICP;
+
+	WARN_ON(I915_READ(SDEIER) != 0);
+	I915_WRITE(SDEIER, 0xffffffff);
+	POSTING_READ(SDEIER);
+
+	gen3_assert_iir_is_zero(dev_priv, SDEIIR);
+	I915_WRITE(SDEIMR, ~mask);
+
+	icp_hpd_detection_setup(dev_priv);
+}
+
 static int gen11_irq_postinstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 gu_misc_masked = GEN11_GU_MISC_GSE;
 
+	if (HAS_PCH_ICP(dev_priv))
+		icp_irq_postinstall(dev);
+
 	gen11_gt_irq_postinstall(dev_priv);
 	gen8_de_irq_postinstall(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4bfd7a9..e347055 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7517,6 +7517,46 @@ enum {
 #define  PORTE_HOTPLUG_SHORT_DETECT	(1 << 0)
 #define  PORTE_HOTPLUG_LONG_DETECT	(2 << 0)
 
+/* ICP */
+#define   SDE_TC4_HOTPLUG_ICP		(1 << 27)
+#define   SDE_TC3_HOTPLUG_ICP		(1 << 26)
+#define   SDE_TC2_HOTPLUG_ICP		(1 << 25)
+#define   SDE_TC1_HOTPLUG_ICP		(1 << 24)
+#define   SDE_GMBUS_ICP			(1 << 23)
+#define   SDE_DDIB_HOTPLUG_ICP		(1 << 17)
+#define   SDE_DDIA_HOTPLUG_ICP		(1 << 16)
+
+#define SDE_DDI_MASK_ICP		(SDE_DDIB_HOTPLUG_ICP |	\
+					 SDE_DDIA_HOTPLUG_ICP)
+
+#define SDE_TC_MASK_ICP			(SDE_TC4_HOTPLUG_ICP |	\
+					 SDE_TC3_HOTPLUG_ICP |	\
+					 SDE_TC2_HOTPLUG_ICP |	\
+					 SDE_TC1_HOTPLUG_ICP)
+
+/* This register is a reuse of PCH_PORT_HOTPLUG register. The Spec
+ * has the name SHOTPLUG_CTL_DDI for ICP. Let us stick to the Spec.
+ */
+
+#define SHOTPLUG_CTL_DDI			_MMIO(0xc4030)
+#define   ICP_DDIB_HPD_ENABLE			(1 << 7)
+#define   ICP_DDIB_HPD_STATUS_MASK		(3 << 4)
+#define   ICP_DDIB_HPD_NO_DETECT		(0 << 4)
+#define   ICP_DDIB_HPD_SHORT_DETECT		(1 << 4)
+#define   ICP_DDIB_HPD_LONG_DETECT		(2 << 4)
+#define   ICP_DDIB_HPD_SHORT_LONG_DETECT	(3 << 4)
+#define   ICP_DDIA_HPD_ENABLE			(1 << 3)
+#define   ICP_DDIA_HPD_STATUS_MASK		(3 << 0)
+#define   ICP_DDIA_HPD_NO_DETECT		(0 << 0)
+#define   ICP_DDIA_HPD_SHORT_DETECT		(1 << 0)
+#define   ICP_DDIA_HPD_LONG_DETECT		(2 << 0)
+#define   ICP_DDIA_HPD_SHORT_LONG_DETECT	(3 << 0)
+
+#define SHOTPLUG_CTL_TC				_MMIO(0xc4034)
+#define   ICP_TC_HPD_ENABLE(tc_port)		(8 << (tc_port) * 4)
+#define   ICP_TC_HPD_LONG_DETECT(tc_port)	(2 << (tc_port) * 4)
+#define   ICP_TC_HPD_SHORT_DETECT(tc_port)	(1 << (tc_port) * 4)
+
 #define PCH_GPIOA               _MMIO(0xc5010)
 #define PCH_GPIOB               _MMIO(0xc5014)
 #define PCH_GPIOC               _MMIO(0xc5018)
-- 
2.7.4

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

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

end of thread, other threads:[~2018-07-23 17:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-20 21:36 [PATCH 1/2] drm/i915/icp: Add Interrupt Support Anusha Srivatsa
2018-06-20 21:36 ` [PATCH 2/2] drm/i915/icl: implement icl_digital_port_connected() Anusha Srivatsa
2018-06-30  6:42   ` Lucas De Marchi
2018-06-20 22:11 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/icp: Add Interrupt Support Patchwork
2018-06-21  6:37 ` ✓ Fi.CI.IGT: " Patchwork
2018-06-25 23:17 ` [PATCH 1/2] " Paulo Zanoni
2018-06-26 18:32   ` Srivatsa, Anusha
2018-06-26 20:09     ` Paulo Zanoni
2018-07-23 17:05 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2018-06-20 21:02 [PATCH 1/2] " Anusha Srivatsa

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