All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
@ 2018-01-25 19:27 Rodrigo Vivi
  2018-01-25 19:27 ` [PATCH 02/10] drm/i915/cnl: Add AUX-F support Rodrigo Vivi
                   ` (11 more replies)
  0 siblings, 12 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 19:27 UTC (permalink / raw)
  To: intel-gfx
  Cc: Paulo Zanoni, Lucas De Marchi, Dhinakaran Pandiyan, Rodrigo Vivi

The only difference is that this SKUs has the full
Port A/E split named as Port F.

But since SKUs differences don't matter on the platform
definition group and ids, let's merge all off them together.

v2: Really include the PCI IDs to the picidlist[];
v3: Add the PCI Id for another SKU (Anusha).
v4: Update IDs, really include to pciidlists again.
v5: Unify all GT2 IDs.
v6: Unify in a way that we don't break early-quirks.c
v7: Remove GT reference since it doesn't matter here (Paulo)
    Also move IS_CNL_WITH_PORT_F macro to this patch to
    make it easier for review this part and also to get
    used sooner.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_pci.c |  5 ++---
 include/drm/i915_pciids.h       | 18 +++++++-----------
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 454d8f937fae..5702ebf17974 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2648,6 +2648,8 @@ intel_info(const struct drm_i915_private *dev_priv)
 				 (dev_priv)->info.gt == 2)
 #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
 				 (dev_priv)->info.gt == 3)
+#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
+					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
 
 #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
 
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index f28c165fc49d..7eb3d5e4350e 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -571,7 +571,7 @@ static const struct intel_device_info intel_coffeelake_gt3_info __initconst = {
 	.ddb_size = 1024, \
 	GLK_COLORS
 
-static const struct intel_device_info intel_cannonlake_gt2_info __initconst = {
+static const struct intel_device_info intel_cannonlake_info __initconst = {
 	GEN10_FEATURES,
 	.is_alpha_support = 1,
 	.platform = INTEL_CANNONLAKE,
@@ -649,8 +649,7 @@ static const struct pci_device_id pciidlist[] = {
 	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
 	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
 	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
-	INTEL_CNL_U_GT2_IDS(&intel_cannonlake_gt2_info),
-	INTEL_CNL_Y_GT2_IDS(&intel_cannonlake_gt2_info),
+	INTEL_CNL_IDS(&intel_cannonlake_info),
 	{0, 0, 0}
 };
 MODULE_DEVICE_TABLE(pci, pciidlist);
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 5db0458dd832..9e1fe6634424 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -414,24 +414,20 @@
 	INTEL_CFL_U_GT2_IDS(info), \
 	INTEL_CFL_U_GT3_IDS(info)
 
-/* CNL U 2+2 */
-#define INTEL_CNL_U_GT2_IDS(info) \
+/* CNL */
+#define INTEL_CNL_IDS(info) \
 	INTEL_VGA_DEVICE(0x5A52, info), \
 	INTEL_VGA_DEVICE(0x5A5A, info), \
 	INTEL_VGA_DEVICE(0x5A42, info), \
-	INTEL_VGA_DEVICE(0x5A4A, info)
-
-/* CNL Y 2+2 */
-#define INTEL_CNL_Y_GT2_IDS(info) \
+	INTEL_VGA_DEVICE(0x5A4A, info), \
 	INTEL_VGA_DEVICE(0x5A51, info), \
 	INTEL_VGA_DEVICE(0x5A59, info), \
 	INTEL_VGA_DEVICE(0x5A41, info), \
 	INTEL_VGA_DEVICE(0x5A49, info), \
 	INTEL_VGA_DEVICE(0x5A71, info), \
-	INTEL_VGA_DEVICE(0x5A79, info)
-
-#define INTEL_CNL_IDS(info) \
-	INTEL_CNL_U_GT2_IDS(info), \
-	INTEL_CNL_Y_GT2_IDS(info)
+	INTEL_VGA_DEVICE(0x5A79, info), \
+	INTEL_VGA_DEVICE(0x5A54, info), \
+	INTEL_VGA_DEVICE(0x5A5C, info), \
+	INTEL_VGA_DEVICE(0x5A44, info)
 
 #endif /* _I915_PCIIDS_H */
-- 
2.13.6

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

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

* [PATCH 02/10] drm/i915/cnl: Add AUX-F support
  2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
@ 2018-01-25 19:27 ` Rodrigo Vivi
  2018-01-25 19:27 ` [PATCH 03/10] drm/i915/cnl: Extend Wa 1178 to Aux F Rodrigo Vivi
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 19:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi, Dhinakaran Pandiyan, Rodrigo Vivi

On some Cannonlake SKUs we have a dedicated Aux for port F,
that is only the full split between port A and port E.

There is still no Aux E for Port E, as in previous platforms,
because port_E still means shared lanes with port A.

v2: Rebase.
v3: Add couple missed PORT_F cases on intel_dp.
v4: Rebase and fix commit message.
v5: Squash Imre's "drm/i915: Add missing AUX_F power well string"
v6: Rebase on top of display headers rework.
v7: s/IS_CANNONLAKE/IS_CNL_WITH_PORT_F (DK)
v8: Fix Aux bits for Port F (DK)
v9: Fix VBT definition of Port F (DK).
v10: Squash power well addition to this patch to avoid
     warns as pointed by DK.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

drm/i915/cnl: Don't try to manage Port F power wells on all CNL.

SKUs that lacks on the full port F split will just time out
when touching this power well bits, causing a noisy warn.

v2: Suggested-by: Imre. Temporarily remove the aux pw id after setting
    it instead of duplicating and redefining everything.
v3: Simplify even more the logic, using one that don't mix the
    array size with the pw bits. Also add a comment.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 +
 drivers/gpu/drm/i915/i915_irq.c         |  6 ++++++
 drivers/gpu/drm/i915/i915_reg.h         |  9 +++++++++
 drivers/gpu/drm/i915/intel_display.h    |  1 +
 drivers/gpu/drm/i915/intel_dp.c         |  8 ++++++++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 21 +++++++++++++++++++++
 6 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5702ebf17974..f89a1a0a25c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1255,6 +1255,7 @@ enum modeset_restore {
 #define DP_AUX_B 0x10
 #define DP_AUX_C 0x20
 #define DP_AUX_D 0x30
+#define DP_AUX_F 0x60
 
 #define DDC_PIN_B  0x05
 #define DDC_PIN_C  0x04
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f643d5ad7ff7..4d84b1c41a94 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2585,6 +2585,9 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 					    GEN9_AUX_CHANNEL_C |
 					    GEN9_AUX_CHANNEL_D;
 
+			if (IS_CNL_WITH_PORT_F(dev_priv))
+				tmp_mask |= CNL_AUX_CHANNEL_F;
+
 			if (iir & tmp_mask) {
 				dp_aux_irq_handler(dev_priv);
 				found = true;
@@ -3623,6 +3626,9 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 		de_pipe_masked |= GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
 	}
 
+	if (IS_CNL_WITH_PORT_F(dev_priv))
+		de_port_masked |= CNL_AUX_CHANNEL_F;
+
 	de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
 					   GEN8_PIPE_FIFO_UNDERRUN;
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 64933fd74cb6..44f46593172f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1312,6 +1312,7 @@ enum i915_power_well_id {
 	CNL_DISP_PW_AUX_B = GLK_DISP_PW_AUX_B,
 	CNL_DISP_PW_AUX_C = GLK_DISP_PW_AUX_C,
 	CNL_DISP_PW_AUX_D,
+	CNL_DISP_PW_AUX_F,
 
 	SKL_DISP_PW_1 = 14,
 	SKL_DISP_PW_2,
@@ -5284,6 +5285,13 @@ enum {
 #define _DPD_AUX_CH_DATA4	(dev_priv->info.display_mmio_offset + 0x64320)
 #define _DPD_AUX_CH_DATA5	(dev_priv->info.display_mmio_offset + 0x64324)
 
+#define _DPF_AUX_CH_CTL		(dev_priv->info.display_mmio_offset + 0x64510)
+#define _DPF_AUX_CH_DATA1	(dev_priv->info.display_mmio_offset + 0x64514)
+#define _DPF_AUX_CH_DATA2	(dev_priv->info.display_mmio_offset + 0x64518)
+#define _DPF_AUX_CH_DATA3	(dev_priv->info.display_mmio_offset + 0x6451c)
+#define _DPF_AUX_CH_DATA4	(dev_priv->info.display_mmio_offset + 0x64520)
+#define _DPF_AUX_CH_DATA5	(dev_priv->info.display_mmio_offset + 0x64524)
+
 #define DP_AUX_CH_CTL(port)	_MMIO_PORT(port, _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL)
 #define DP_AUX_CH_DATA(port, i)	_MMIO(_PORT(port, _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
 
@@ -6939,6 +6947,7 @@ enum {
 #define GEN8_DE_PORT_IMR _MMIO(0x44444)
 #define GEN8_DE_PORT_IIR _MMIO(0x44448)
 #define GEN8_DE_PORT_IER _MMIO(0x4444c)
+#define  CNL_AUX_CHANNEL_F		(1 << 28)
 #define  GEN9_AUX_CHANNEL_D		(1 << 27)
 #define  GEN9_AUX_CHANNEL_C		(1 << 26)
 #define  GEN9_AUX_CHANNEL_B		(1 << 25)
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index e47638931b51..30fa2041a45f 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -172,6 +172,7 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_AUX_C,
 	POWER_DOMAIN_AUX_D,
+	POWER_DOMAIN_AUX_F,
 	POWER_DOMAIN_GMBUS,
 	POWER_DOMAIN_MODESET,
 	POWER_DOMAIN_GT_IRQ,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a2e887999915..ae3b0b030177 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1323,6 +1323,9 @@ static enum port intel_aux_port(struct drm_i915_private *dev_priv,
 	case DP_AUX_D:
 		aux_port = PORT_D;
 		break;
+	case DP_AUX_F:
+		aux_port = PORT_F;
+		break;
 	default:
 		MISSING_CASE(info->alternate_aux_channel);
 		aux_port = PORT_A;
@@ -1342,6 +1345,7 @@ static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
 	case PORT_B:
 	case PORT_C:
 	case PORT_D:
+	case PORT_F:
 		return DP_AUX_CH_CTL(port);
 	default:
 		MISSING_CASE(port);
@@ -1356,6 +1360,7 @@ static i915_reg_t g4x_aux_data_reg(struct drm_i915_private *dev_priv,
 	case PORT_B:
 	case PORT_C:
 	case PORT_D:
+	case PORT_F:
 		return DP_AUX_CH_DATA(port, index);
 	default:
 		MISSING_CASE(port);
@@ -6224,6 +6229,9 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
 		/* FIXME: Check VBT for actual wiring of PORT E */
 		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
 		break;
+	case PORT_F:
+		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F;
+		break;
 	default:
 		MISSING_CASE(encoder->port);
 	}
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 5b1aa4b9c72c..93cc07998cc0 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -124,6 +124,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
 		return "AUX_C";
 	case POWER_DOMAIN_AUX_D:
 		return "AUX_D";
+	case POWER_DOMAIN_AUX_F:
+		return "AUX_F";
 	case POWER_DOMAIN_GMBUS:
 		return "GMBUS";
 	case POWER_DOMAIN_INIT:
@@ -1855,6 +1857,9 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 #define CNL_DISPLAY_AUX_D_POWER_DOMAINS (		\
 	BIT_ULL(POWER_DOMAIN_AUX_D) |			\
 	BIT_ULL(POWER_DOMAIN_INIT))
+#define CNL_DISPLAY_AUX_F_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_AUX_F) |			\
+	BIT_ULL(POWER_DOMAIN_INIT))
 #define CNL_DISPLAY_DC_OFF_POWER_DOMAINS (		\
 	CNL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
 	BIT_ULL(POWER_DOMAIN_GT_IRQ) |			\
@@ -2405,6 +2410,12 @@ static struct i915_power_well cnl_power_wells[] = {
 		.ops = &hsw_power_well_ops,
 		.id = SKL_DISP_PW_DDI_D,
 	},
+	{
+		.name = "AUX F",
+		.domains = CNL_DISPLAY_AUX_F_POWER_DOMAINS,
+		.ops = &hsw_power_well_ops,
+		.id = CNL_DISP_PW_AUX_F,
+	},
 };
 
 static int
@@ -2520,6 +2531,16 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 		set_power_wells(power_domains, skl_power_wells);
 	} else if (IS_CANNONLAKE(dev_priv)) {
 		set_power_wells(power_domains, cnl_power_wells);
+
+		/*
+		 * Aux IO is getting enabled for all ports
+		 * regardless the presence or use. So, in order to avoid
+		 * timeouts, lets remove it from the list
+		 * for the SKUs without port F.
+		 */
+		if (!IS_CNL_WITH_PORT_F(dev_priv))
+			power_domains->power_well_count -= 1;
+
 	} else if (IS_BROXTON(dev_priv)) {
 		set_power_wells(power_domains, bxt_power_wells);
 	} else if (IS_GEMINILAKE(dev_priv)) {
-- 
2.13.6

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

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

* [PATCH 03/10] drm/i915/cnl: Extend Wa 1178 to Aux F.
  2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
  2018-01-25 19:27 ` [PATCH 02/10] drm/i915/cnl: Add AUX-F support Rodrigo Vivi
@ 2018-01-25 19:27 ` Rodrigo Vivi
  2018-01-25 19:27 ` [PATCH 04/10] drm/i915/cnl: Fix _CNL_PORT_TX_DW2_LN0_F definition Rodrigo Vivi
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 19:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi, Dhinakaran Pandiyan, Rodrigo Vivi

We also need to extend this WA to Aux F.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         | 4 +++-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 44f46593172f..dda77443cb89 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8439,10 +8439,12 @@ enum skl_power_gate {
 #define _CNL_AUX_ANAOVRD1_B		0x162250
 #define _CNL_AUX_ANAOVRD1_C		0x162210
 #define _CNL_AUX_ANAOVRD1_D		0x1622D0
+#define _CNL_AUX_ANAOVRD1_F		0x162A90
 #define CNL_AUX_ANAOVRD1(pw)		_MMIO(_PICK(_CNL_AUX_REG_IDX(pw), \
 						    _CNL_AUX_ANAOVRD1_B, \
 						    _CNL_AUX_ANAOVRD1_C, \
-						    _CNL_AUX_ANAOVRD1_D))
+						    _CNL_AUX_ANAOVRD1_D, \
+						    _CNL_AUX_ANAOVRD1_F))
 #define   CNL_AUX_ANAOVRD1_ENABLE	(1<<16)
 #define   CNL_AUX_ANAOVRD1_LDO_BYPASS	(1<<23)
 
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 93cc07998cc0..3526b563b8ec 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -395,7 +395,7 @@ static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
 	/* Display WA #1178: cnl */
 	if (IS_CANNONLAKE(dev_priv) &&
 	    (id == CNL_DISP_PW_AUX_B || id == CNL_DISP_PW_AUX_C ||
-	     id == CNL_DISP_PW_AUX_D)) {
+	     id == CNL_DISP_PW_AUX_D || id == CNL_DISP_PW_AUX_F)) {
 		val = I915_READ(CNL_AUX_ANAOVRD1(id));
 		val |= CNL_AUX_ANAOVRD1_ENABLE | CNL_AUX_ANAOVRD1_LDO_BYPASS;
 		I915_WRITE(CNL_AUX_ANAOVRD1(id), val);
-- 
2.13.6

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

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

* [PATCH 04/10] drm/i915/cnl: Fix _CNL_PORT_TX_DW2_LN0_F definition.
  2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
  2018-01-25 19:27 ` [PATCH 02/10] drm/i915/cnl: Add AUX-F support Rodrigo Vivi
  2018-01-25 19:27 ` [PATCH 03/10] drm/i915/cnl: Extend Wa 1178 to Aux F Rodrigo Vivi
@ 2018-01-25 19:27 ` Rodrigo Vivi
  2018-01-25 19:27 ` [PATCH 05/10] drm/i915: Fix DPLCLKA_CFGCR0 bits for Port F Rodrigo Vivi
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 19:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi, Rodrigo Vivi

This was wrong since its introduction on commit '04416108ccea
("drm/i915/cnl: Add registers related to voltage swing sequences.")'

But since no Port F was needed so far we don't need to
propagate fixes back there.

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dda77443cb89..31e2fa602620 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1964,7 +1964,7 @@ enum i915_power_well_id {
 #define _CNL_PORT_TX_DW2_LN0_B		0x162648
 #define _CNL_PORT_TX_DW2_LN0_C		0x162C48
 #define _CNL_PORT_TX_DW2_LN0_D		0x162E48
-#define _CNL_PORT_TX_DW2_LN0_F		0x162A48
+#define _CNL_PORT_TX_DW2_LN0_F		0x162848
 #define CNL_PORT_TX_DW2_GRP(port)	_MMIO_PORT6(port, \
 						    _CNL_PORT_TX_DW2_GRP_AE, \
 						    _CNL_PORT_TX_DW2_GRP_B, \
-- 
2.13.6

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

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

* [PATCH 05/10] drm/i915: Fix DPLCLKA_CFGCR0 bits for Port F.
  2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
                   ` (2 preceding siblings ...)
  2018-01-25 19:27 ` [PATCH 04/10] drm/i915/cnl: Fix _CNL_PORT_TX_DW2_LN0_F definition Rodrigo Vivi
@ 2018-01-25 19:27 ` Rodrigo Vivi
  2018-01-25 19:27 ` [PATCH 06/10] drm/i915/cnl: Add right GMBUS pin number for HDMI on " Rodrigo Vivi
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 19:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi, Dhinakaran Pandiyan, Rodrigo Vivi

Since when it got introduced with commit '555e38d27317
("drm/i915/cnl: DDI - PLL mapping")' the support for Port F
was wrong, because Port F bits are far from bits used
for A to E.

Since Port F is not used so far we don't need to propagate
Fixes back there.

v2: Reuse _SHIFT definition to avoid complicated duplication (DK).

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 31e2fa602620..076a49107e02 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8844,10 +8844,12 @@ enum skl_power_gate {
  * CNL Clocks
  */
 #define DPCLKA_CFGCR0				_MMIO(0x6C200)
-#define  DPCLKA_CFGCR0_DDI_CLK_OFF(port)	(1 << ((port)+10))
-#define  DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port)	(3 << ((port)*2))
-#define  DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port)	((port)*2)
-#define  DPCLKA_CFGCR0_DDI_CLK_SEL(pll, port)	((pll) << ((port)*2))
+#define  DPCLKA_CFGCR0_DDI_CLK_OFF(port)	(1 << ((port) ==  PORT_F ? 23 : \
+						      (port)+10))
+#define  DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port)	((port) == PORT_F ? 21 : \
+						(port)*2)
+#define  DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port)	(3 << DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port))
+#define  DPCLKA_CFGCR0_DDI_CLK_SEL(pll, port)	((pll) << DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port))
 
 /* CNL PLL */
 #define DPLL0_ENABLE		0x46010
-- 
2.13.6

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

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

* [PATCH 06/10] drm/i915/cnl: Add right GMBUS pin number for HDMI on Port F.
  2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
                   ` (3 preceding siblings ...)
  2018-01-25 19:27 ` [PATCH 05/10] drm/i915: Fix DPLCLKA_CFGCR0 bits for Port F Rodrigo Vivi
@ 2018-01-25 19:27 ` Rodrigo Vivi
  2018-01-25 19:27 ` [PATCH 07/10] drm/i915: For HPD connected port use hpd_pin instead of port Rodrigo Vivi
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 19:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi, Rodrigo Vivi

On CNP Pin 3 is for misc of Port F usage depending on the
configuration. For CNL that uses Port F, pin 3 is the one.

v2: Make it more generic and update commit message.

Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 978f22bb96c2..0d924ba42075 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2178,6 +2178,9 @@ static u8 cnp_port_to_ddc_pin(struct drm_i915_private *dev_priv,
 	case PORT_D:
 		ddc_pin = GMBUS_PIN_4_CNP;
 		break;
+	case PORT_F:
+		ddc_pin = GMBUS_PIN_3_BXT;
+		break;
 	default:
 		MISSING_CASE(port);
 		ddc_pin = GMBUS_PIN_1_BXT;
-- 
2.13.6

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

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

* [PATCH 07/10] drm/i915: For HPD connected port use hpd_pin instead of port.
  2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
                   ` (4 preceding siblings ...)
  2018-01-25 19:27 ` [PATCH 06/10] drm/i915/cnl: Add right GMBUS pin number for HDMI on " Rodrigo Vivi
@ 2018-01-25 19:27 ` Rodrigo Vivi
  2018-01-25 21:37   ` [PATCH] drm/i915/cnp: Properly handle VBT ddc pin out of bounds Rodrigo Vivi
  2018-01-25 19:27 ` [PATCH 08/10] drm/i915/cnl: Add HPD support for Port F Rodrigo Vivi
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 19:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi, Rodrigo Vivi

Let's try to simplify this mapping to hpd_pin -> bit
instead using port.
So for CNL with port F where we have this port using
hdp_pin and bits of other ports we don't need to duplicated
the mapping.

But for now this is only a re-org with no functional change
expected.

v2: Add missing lines and nuke @port reference from code
    documentation. (Ville)

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c     | 150 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h    |   3 +-
 drivers/gpu/drm/i915/intel_lspcon.c |   3 +-
 3 files changed, 77 insertions(+), 79 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ae3b0b030177..8579d2d09231 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4487,173 +4487,174 @@ edp_detect(struct intel_dp *intel_dp)
 	return status;
 }
 
-static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
-				       struct intel_digital_port *port)
+static bool ibx_digital_port_connected(struct intel_encoder *encoder)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 bit;
 
-	switch (port->base.port) {
-	case PORT_B:
+	switch (encoder->hpd_pin) {
+	case HPD_PORT_B:
 		bit = SDE_PORTB_HOTPLUG;
 		break;
-	case PORT_C:
+	case HPD_PORT_C:
 		bit = SDE_PORTC_HOTPLUG;
 		break;
-	case PORT_D:
+	case HPD_PORT_D:
 		bit = SDE_PORTD_HOTPLUG;
 		break;
 	default:
-		MISSING_CASE(port->base.port);
+		MISSING_CASE(encoder->hpd_pin);
 		return false;
 	}
 
 	return I915_READ(SDEISR) & bit;
 }
 
-static bool cpt_digital_port_connected(struct drm_i915_private *dev_priv,
-				       struct intel_digital_port *port)
+static bool cpt_digital_port_connected(struct intel_encoder *encoder)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 bit;
 
-	switch (port->base.port) {
-	case PORT_B:
+	switch (encoder->hpd_pin) {
+	case HPD_PORT_B:
 		bit = SDE_PORTB_HOTPLUG_CPT;
 		break;
-	case PORT_C:
+	case HPD_PORT_C:
 		bit = SDE_PORTC_HOTPLUG_CPT;
 		break;
-	case PORT_D:
+	case HPD_PORT_D:
 		bit = SDE_PORTD_HOTPLUG_CPT;
 		break;
 	default:
-		MISSING_CASE(port->base.port);
+		MISSING_CASE(encoder->hpd_pin);
 		return false;
 	}
 
 	return I915_READ(SDEISR) & bit;
 }
 
-static bool spt_digital_port_connected(struct drm_i915_private *dev_priv,
-				       struct intel_digital_port *port)
+static bool spt_digital_port_connected(struct intel_encoder *encoder)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 bit;
 
-	switch (port->base.port) {
-	case PORT_A:
+	switch (encoder->hpd_pin) {
+	case HPD_PORT_A:
 		bit = SDE_PORTA_HOTPLUG_SPT;
 		break;
-	case PORT_E:
+	case HPD_PORT_E:
 		bit = SDE_PORTE_HOTPLUG_SPT;
 		break;
 	default:
-		return cpt_digital_port_connected(dev_priv, port);
+		return cpt_digital_port_connected(encoder);
 	}
 
 	return I915_READ(SDEISR) & bit;
 }
 
-static bool g4x_digital_port_connected(struct drm_i915_private *dev_priv,
-				       struct intel_digital_port *port)
+static bool g4x_digital_port_connected(struct intel_encoder *encoder)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 bit;
 
-	switch (port->base.port) {
-	case PORT_B:
+	switch (encoder->hpd_pin) {
+	case HPD_PORT_B:
 		bit = PORTB_HOTPLUG_LIVE_STATUS_G4X;
 		break;
-	case PORT_C:
+	case HPD_PORT_C:
 		bit = PORTC_HOTPLUG_LIVE_STATUS_G4X;
 		break;
-	case PORT_D:
+	case HPD_PORT_D:
 		bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
 		break;
 	default:
-		MISSING_CASE(port->base.port);
+		MISSING_CASE(encoder->hpd_pin);
 		return false;
 	}
 
 	return I915_READ(PORT_HOTPLUG_STAT) & bit;
 }
 
-static bool gm45_digital_port_connected(struct drm_i915_private *dev_priv,
-					struct intel_digital_port *port)
+static bool gm45_digital_port_connected(struct intel_encoder *encoder)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 bit;
 
-	switch (port->base.port) {
-	case PORT_B:
+	switch (encoder->hpd_pin) {
+	case HPD_PORT_B:
 		bit = PORTB_HOTPLUG_LIVE_STATUS_GM45;
 		break;
-	case PORT_C:
+	case HPD_PORT_C:
 		bit = PORTC_HOTPLUG_LIVE_STATUS_GM45;
 		break;
-	case PORT_D:
+	case HPD_PORT_D:
 		bit = PORTD_HOTPLUG_LIVE_STATUS_GM45;
 		break;
 	default:
-		MISSING_CASE(port->base.port);
+		MISSING_CASE(encoder->hpd_pin);
 		return false;
 	}
 
 	return I915_READ(PORT_HOTPLUG_STAT) & bit;
 }
 
-static bool ilk_digital_port_connected(struct drm_i915_private *dev_priv,
-				       struct intel_digital_port *port)
+static bool ilk_digital_port_connected(struct intel_encoder *encoder)
 {
-	if (port->base.port == PORT_A)
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
+	if (encoder->hpd_pin == HPD_PORT_A)
 		return I915_READ(DEISR) & DE_DP_A_HOTPLUG;
 	else
-		return ibx_digital_port_connected(dev_priv, port);
+		return ibx_digital_port_connected(encoder);
 }
 
-static bool snb_digital_port_connected(struct drm_i915_private *dev_priv,
-				       struct intel_digital_port *port)
+static bool snb_digital_port_connected(struct intel_encoder *encoder)
 {
-	if (port->base.port == PORT_A)
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
+	if (encoder->hpd_pin == HPD_PORT_A)
 		return I915_READ(DEISR) & DE_DP_A_HOTPLUG;
 	else
-		return cpt_digital_port_connected(dev_priv, port);
+		return cpt_digital_port_connected(encoder);
 }
 
-static bool ivb_digital_port_connected(struct drm_i915_private *dev_priv,
-				       struct intel_digital_port *port)
+static bool ivb_digital_port_connected(struct intel_encoder *encoder)
 {
-	if (port->base.port == PORT_A)
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
+	if (encoder->hpd_pin == HPD_PORT_A)
 		return I915_READ(DEISR) & DE_DP_A_HOTPLUG_IVB;
 	else
-		return cpt_digital_port_connected(dev_priv, port);
+		return cpt_digital_port_connected(encoder);
 }
 
-static bool bdw_digital_port_connected(struct drm_i915_private *dev_priv,
-				       struct intel_digital_port *port)
+static bool bdw_digital_port_connected(struct intel_encoder *encoder)
 {
-	if (port->base.port == PORT_A)
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
+	if (encoder->hpd_pin == HPD_PORT_A)
 		return I915_READ(GEN8_DE_PORT_ISR) & GEN8_PORT_DP_A_HOTPLUG;
 	else
-		return cpt_digital_port_connected(dev_priv, port);
+		return cpt_digital_port_connected(encoder);
 }
 
-static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
-				       struct intel_digital_port *intel_dig_port)
+static bool bxt_digital_port_connected(struct intel_encoder *encoder)
 {
-	struct intel_encoder *intel_encoder = &intel_dig_port->base;
-	enum port port;
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 bit;
 
-	port = intel_hpd_pin_to_port(intel_encoder->hpd_pin);
-	switch (port) {
-	case PORT_A:
+	switch (encoder->hpd_pin) {
+	case HPD_PORT_A:
 		bit = BXT_DE_PORT_HP_DDIA;
 		break;
-	case PORT_B:
+	case HPD_PORT_B:
 		bit = BXT_DE_PORT_HP_DDIB;
 		break;
-	case PORT_C:
+	case HPD_PORT_C:
 		bit = BXT_DE_PORT_HP_DDIC;
 		break;
 	default:
-		MISSING_CASE(port);
+		MISSING_CASE(encoder->hpd_pin);
 		return false;
 	}
 
@@ -4662,33 +4663,33 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
 
 /*
  * intel_digital_port_connected - is the specified port connected?
- * @dev_priv: i915 private structure
- * @port: the port to test
+ * @encoder: intel_encoder
  *
- * Return %true if @port is connected, %false otherwise.
+ * Return %true if port is connected, %false otherwise.
  */
-bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
-				  struct intel_digital_port *port)
+bool intel_digital_port_connected(struct intel_encoder *encoder)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
 	if (HAS_GMCH_DISPLAY(dev_priv)) {
 		if (IS_GM45(dev_priv))
-			return gm45_digital_port_connected(dev_priv, port);
+			return gm45_digital_port_connected(encoder);
 		else
-			return g4x_digital_port_connected(dev_priv, port);
+			return g4x_digital_port_connected(encoder);
 	}
 
 	if (IS_GEN5(dev_priv))
-		return ilk_digital_port_connected(dev_priv, port);
+		return ilk_digital_port_connected(encoder);
 	else if (IS_GEN6(dev_priv))
-		return snb_digital_port_connected(dev_priv, port);
+		return snb_digital_port_connected(encoder);
 	else if (IS_GEN7(dev_priv))
-		return ivb_digital_port_connected(dev_priv, port);
+		return ivb_digital_port_connected(encoder);
 	else if (IS_GEN8(dev_priv))
-		return bdw_digital_port_connected(dev_priv, port);
+		return bdw_digital_port_connected(encoder);
 	else if (IS_GEN9_LP(dev_priv))
-		return bxt_digital_port_connected(dev_priv, port);
+		return bxt_digital_port_connected(encoder);
 	else
-		return spt_digital_port_connected(dev_priv, port);
+		return spt_digital_port_connected(encoder);
 }
 
 static struct edid *
@@ -4747,8 +4748,7 @@ intel_dp_long_pulse(struct intel_connector *connector)
 	/* Can't disconnect eDP, but you can close the lid... */
 	if (intel_dp_is_edp(intel_dp))
 		status = edp_detect(intel_dp);
-	else if (intel_digital_port_connected(dev_priv,
-					      dp_to_dig_port(intel_dp)))
+	else if (intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base))
 		status = intel_dp_detect_dpcd(intel_dp);
 	else
 		status = connector_status_disconnected;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3cee54bc0352..2632ae50ca33 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1670,8 +1670,7 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
 bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
 int intel_dp_link_required(int pixel_clock, int bpp);
 int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
-bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
-				  struct intel_digital_port *port);
+bool intel_digital_port_connected(struct intel_encoder *encoder);
 
 /* intel_dp_aux_backlight.c */
 int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index dcbc786479f9..8ae8f42f430a 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -167,11 +167,10 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
 {
 	struct intel_dp *intel_dp = lspcon_to_intel_dp(lspcon);
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
 	unsigned long start = jiffies;
 
 	while (1) {
-		if (intel_digital_port_connected(dev_priv, dig_port)) {
+		if (intel_digital_port_connected(&dig_port->base)) {
 			DRM_DEBUG_KMS("LSPCON recovering in PCON mode after %u ms\n",
 				      jiffies_to_msecs(jiffies - start));
 			return;
-- 
2.13.6

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

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

* [PATCH 08/10] drm/i915/cnl: Add HPD support for Port F.
  2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
                   ` (5 preceding siblings ...)
  2018-01-25 19:27 ` [PATCH 07/10] drm/i915: For HPD connected port use hpd_pin instead of port Rodrigo Vivi
@ 2018-01-25 19:27 ` Rodrigo Vivi
  2018-01-25 19:27 ` [PATCH 09/10] drm/i915/cnl: Enable DDI-F on Cannonlake Rodrigo Vivi
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 19:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi, Dhinakaran Pandiyan, Rodrigo Vivi

On CNP boards that are using DDI F,
bit 25 (SDE_PORTE_HOTPLUG_SPT) is representing
the Digital Port F hotplug line when the Digital
Port F hotplug detect input is enabled.

v2: Reuse all existent structure instead of adding a
new HPD_PORT_F pointing to pin of port E.
v3: Use IS_CNL_WITH_PORT_F so we can start upstreaming
    this right now. If that SKU ever get a proper name
    we come back and update it.
v4: Rebase on top of digital connected port using encoder
    instead of port.
v5: Moved IS_CNL_WITH_PORT_F definition to the PCI IDs patch.

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  6 ++++--
 drivers/gpu/drm/i915/i915_irq.c      | 35 +++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_dp.c      |  4 +++-
 drivers/gpu/drm/i915/intel_hdmi.c    |  2 +-
 drivers/gpu/drm/i915/intel_hotplug.c | 19 +++++++++++++++----
 5 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f89a1a0a25c8..388b72cccde4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2958,8 +2958,10 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 void intel_hpd_init(struct drm_i915_private *dev_priv);
 void intel_hpd_init_work(struct drm_i915_private *dev_priv);
 void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
-enum port intel_hpd_pin_to_port(enum hpd_pin pin);
-enum hpd_pin intel_hpd_pin(enum port port);
+enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
+				enum hpd_pin pin);
+enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
+				   enum port port);
 bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin);
 void intel_hpd_enable(struct drm_i915_private *dev_priv, enum hpd_pin pin);
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4d84b1c41a94..0213860963ab 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1574,10 +1574,11 @@ static bool i9xx_port_hotplug_long_detect(enum port port, u32 val)
  *
  * Note that the caller is expected to zero out the masks initially.
  */
-static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
-			     u32 hotplug_trigger, u32 dig_hotplug_reg,
-			     const u32 hpd[HPD_NUM_PINS],
-			     bool long_pulse_detect(enum port port, u32 val))
+static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
+			       u32 *pin_mask, u32 *long_mask,
+			       u32 hotplug_trigger, u32 dig_hotplug_reg,
+			       const u32 hpd[HPD_NUM_PINS],
+			       bool long_pulse_detect(enum port port, u32 val))
 {
 	enum port port;
 	int i;
@@ -1588,7 +1589,7 @@ static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
 
 		*pin_mask |= BIT(i);
 
-		port = intel_hpd_pin_to_port(i);
+		port = intel_hpd_pin_to_port(dev_priv, i);
 		if (port == PORT_NONE)
 			continue;
 
@@ -1976,8 +1977,9 @@ static void i9xx_hpd_irq_handler(struct drm_i915_private *dev_priv,
 		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
 
 		if (hotplug_trigger) {
-			intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
-					   hotplug_trigger, hpd_status_g4x,
+			intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
+					   hotplug_trigger, hotplug_trigger,
+					   hpd_status_g4x,
 					   i9xx_port_hotplug_long_detect);
 
 			intel_hpd_irq_handler(dev_priv, pin_mask, long_mask);
@@ -1989,8 +1991,9 @@ static void i9xx_hpd_irq_handler(struct drm_i915_private *dev_priv,
 		u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
 
 		if (hotplug_trigger) {
-			intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
-					   hotplug_trigger, hpd_status_i915,
+			intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
+					   hotplug_trigger, hotplug_trigger,
+					   hpd_status_i915,
 					   i9xx_port_hotplug_long_detect);
 			intel_hpd_irq_handler(dev_priv, pin_mask, long_mask);
 		}
@@ -2191,7 +2194,7 @@ static void ibx_hpd_irq_handler(struct drm_i915_private *dev_priv,
 	if (!hotplug_trigger)
 		return;
 
-	intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+	intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask, hotplug_trigger,
 			   dig_hotplug_reg, hpd,
 			   pch_port_hotplug_long_detect);
 
@@ -2333,8 +2336,8 @@ static void spt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
 		dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
 		I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
 
-		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
-				   dig_hotplug_reg, hpd_spt,
+		intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
+				   hotplug_trigger, dig_hotplug_reg, hpd_spt,
 				   spt_port_hotplug_long_detect);
 	}
 
@@ -2344,8 +2347,8 @@ static void spt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
 		dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
 		I915_WRITE(PCH_PORT_HOTPLUG2, dig_hotplug_reg);
 
-		intel_get_hpd_pins(&pin_mask, &long_mask, hotplug2_trigger,
-				   dig_hotplug_reg, hpd_spt,
+		intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
+				   hotplug2_trigger, dig_hotplug_reg, hpd_spt,
 				   spt_port_hotplug2_long_detect);
 	}
 
@@ -2365,7 +2368,7 @@ static void ilk_hpd_irq_handler(struct drm_i915_private *dev_priv,
 	dig_hotplug_reg = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
 	I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, dig_hotplug_reg);
 
-	intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+	intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask, hotplug_trigger,
 			   dig_hotplug_reg, hpd,
 			   ilk_port_hotplug_long_detect);
 
@@ -2542,7 +2545,7 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv,
 	dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
 	I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
 
-	intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+	intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask, hotplug_trigger,
 			   dig_hotplug_reg, hpd,
 			   bxt_port_hotplug_long_detect);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8579d2d09231..2b26ffe100b1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6209,8 +6209,10 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
 {
 	struct intel_encoder *encoder = &intel_dig_port->base;
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
+	struct intel_encoder *intel_encoder = &intel_dig_port->base;
+	struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
 
-	encoder->hpd_pin = intel_hpd_pin(encoder->port);
+	encoder->hpd_pin = intel_hpd_pin_default(dev_priv, encoder->port);
 
 	switch (encoder->port) {
 	case PORT_A:
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 0d924ba42075..f9a1a32fd272 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2334,7 +2334,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 
 	if (WARN_ON(port == PORT_A))
 		return;
-	intel_encoder->hpd_pin = intel_hpd_pin(port);
+	intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
 
 	if (HAS_DDI(dev_priv))
 		intel_connector->get_hw_state = intel_ddi_connector_get_hw_state;
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 875d5d218d5c..fe28c1ea84a5 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -78,12 +78,14 @@
 
 /**
  * intel_hpd_port - return port hard associated with certain pin.
+ * @dev_priv: private driver data pointer
  * @pin: the hpd pin to get associated port
  *
  * Return port that is associatade with @pin and PORT_NONE if no port is
  * hard associated with that @pin.
  */
-enum port intel_hpd_pin_to_port(enum hpd_pin pin)
+enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
+				enum hpd_pin pin)
 {
 	switch (pin) {
 	case HPD_PORT_A:
@@ -95,6 +97,8 @@ enum port intel_hpd_pin_to_port(enum hpd_pin pin)
 	case HPD_PORT_D:
 		return PORT_D;
 	case HPD_PORT_E:
+		if (IS_CNL_WITH_PORT_F(dev_priv))
+			return PORT_F;
 		return PORT_E;
 	default:
 		return PORT_NONE; /* no port for this pin */
@@ -102,13 +106,17 @@ enum port intel_hpd_pin_to_port(enum hpd_pin pin)
 }
 
 /**
- * intel_hpd_pin - return pin hard associated with certain port.
+ * intel_hpd_pin_default - return default pin associated with certain port.
+ * @dev_priv: private driver data pointer
  * @port: the hpd port to get associated pin
  *
+ * It is only valid and used by digital port encoder.
+ *
  * Return pin that is associatade with @port and HDP_NONE if no pin is
  * hard associated with that @port.
  */
-enum hpd_pin intel_hpd_pin(enum port port)
+enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
+				   enum port port)
 {
 	switch (port) {
 	case PORT_A:
@@ -121,6 +129,9 @@ enum hpd_pin intel_hpd_pin(enum port port)
 		return HPD_PORT_D;
 	case PORT_E:
 		return HPD_PORT_E;
+	case PORT_F:
+		if (IS_CNL_WITH_PORT_F(dev_priv))
+			return HPD_PORT_E;
 	default:
 		MISSING_CASE(port);
 		return HPD_NONE;
@@ -417,7 +428,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 		if (!(BIT(i) & pin_mask))
 			continue;
 
-		port = intel_hpd_pin_to_port(i);
+		port = intel_hpd_pin_to_port(dev_priv, i);
 		is_dig_port = port != PORT_NONE &&
 			dev_priv->hotplug.irq_port[port];
 
-- 
2.13.6

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

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

* [PATCH 09/10] drm/i915/cnl: Enable DDI-F on Cannonlake.
  2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
                   ` (6 preceding siblings ...)
  2018-01-25 19:27 ` [PATCH 08/10] drm/i915/cnl: Add HPD support for Port F Rodrigo Vivi
@ 2018-01-25 19:27 ` Rodrigo Vivi
  2018-01-25 19:27 ` [PATCH 10/10] drm/i915/cnl: Fix DP max rate for Cannonlake with port F Rodrigo Vivi
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 19:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi

Now let's finish the Port-F support by adding the
proper port F detection, irq and power well support.

v2: Rebase
v3: Use BIT_ULL
v4: Cover missed case on ddi init.
v5: Update commit message.
v6: Rebase on top of display headers rework.
v7: Squash power-well handling related to DDI F to this
    patch to avoid warns as pointed out by DK.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  2 ++
 drivers/gpu/drm/i915/intel_ddi.c        |  4 ++++
 drivers/gpu/drm/i915/intel_display.c    |  6 +++++-
 drivers/gpu/drm/i915/intel_display.h    |  2 ++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 19 ++++++++++++++++---
 5 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 076a49107e02..8261fe4c4316 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1304,6 +1304,7 @@ enum i915_power_well_id {
 	SKL_DISP_PW_DDI_B,
 	SKL_DISP_PW_DDI_C,
 	SKL_DISP_PW_DDI_D,
+	CNL_DISP_PW_DDI_F = 6,
 
 	GLK_DISP_PW_AUX_A = 8,
 	GLK_DISP_PW_AUX_B,
@@ -8945,6 +8946,7 @@ enum skl_power_gate {
 #define  SFUSE_STRAP_RAW_FREQUENCY	(1<<8)
 #define  SFUSE_STRAP_DISPLAY_DISABLED	(1<<7)
 #define  SFUSE_STRAP_CRT_DISABLED	(1<<6)
+#define  SFUSE_STRAP_DDIF_DETECTED	(1<<3)
 #define  SFUSE_STRAP_DDIB_DETECTED	(1<<2)
 #define  SFUSE_STRAP_DDIC_DETECTED	(1<<1)
 #define  SFUSE_STRAP_DDID_DETECTED	(1<<0)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e51559be2e3b..cfcd9cb37d5d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2946,6 +2946,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 		intel_dig_port->ddi_io_power_domain =
 			POWER_DOMAIN_PORT_DDI_E_IO;
 		break;
+	case PORT_F:
+		intel_dig_port->ddi_io_power_domain =
+			POWER_DOMAIN_PORT_DDI_F_IO;
+		break;
 	default:
 		MISSING_CASE(port);
 	}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 83de43ce1f3b..fe3c09184c2e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5647,6 +5647,8 @@ enum intel_display_power_domain intel_port_to_power_domain(enum port port)
 		return POWER_DOMAIN_PORT_DDI_D_LANES;
 	case PORT_E:
 		return POWER_DOMAIN_PORT_DDI_E_LANES;
+	case PORT_F:
+		return POWER_DOMAIN_PORT_DDI_F_LANES;
 	default:
 		MISSING_CASE(port);
 		return POWER_DOMAIN_PORT_OTHER;
@@ -13619,7 +13621,7 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
 		if (found || IS_GEN9_BC(dev_priv))
 			intel_ddi_init(dev_priv, PORT_A);
 
-		/* DDI B, C and D detection is indicated by the SFUSE_STRAP
+		/* DDI B, C, D, and F detection is indicated by the SFUSE_STRAP
 		 * register */
 		found = I915_READ(SFUSE_STRAP);
 
@@ -13629,6 +13631,8 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
 			intel_ddi_init(dev_priv, PORT_C);
 		if (found & SFUSE_STRAP_DDID_DETECTED)
 			intel_ddi_init(dev_priv, PORT_D);
+		if (found & SFUSE_STRAP_DDIF_DETECTED)
+			intel_ddi_init(dev_priv, PORT_F);
 		/*
 		 * On SKL we don't have a way to detect DDI-E so we rely on VBT.
 		 */
diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index 30fa2041a45f..c4042e342f50 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -157,11 +157,13 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_PORT_DDI_C_LANES,
 	POWER_DOMAIN_PORT_DDI_D_LANES,
 	POWER_DOMAIN_PORT_DDI_E_LANES,
+	POWER_DOMAIN_PORT_DDI_F_LANES,
 	POWER_DOMAIN_PORT_DDI_A_IO,
 	POWER_DOMAIN_PORT_DDI_B_IO,
 	POWER_DOMAIN_PORT_DDI_C_IO,
 	POWER_DOMAIN_PORT_DDI_D_IO,
 	POWER_DOMAIN_PORT_DDI_E_IO,
+	POWER_DOMAIN_PORT_DDI_F_IO,
 	POWER_DOMAIN_PORT_DSI,
 	POWER_DOMAIN_PORT_CRT,
 	POWER_DOMAIN_PORT_OTHER,
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 3526b563b8ec..30e50ea16960 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -94,6 +94,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
 		return "PORT_DDI_D_LANES";
 	case POWER_DOMAIN_PORT_DDI_E_LANES:
 		return "PORT_DDI_E_LANES";
+	case POWER_DOMAIN_PORT_DDI_F_LANES:
+		return "PORT_DDI_F_LANES";
 	case POWER_DOMAIN_PORT_DDI_A_IO:
 		return "PORT_DDI_A_IO";
 	case POWER_DOMAIN_PORT_DDI_B_IO:
@@ -104,6 +106,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
 		return "PORT_DDI_D_IO";
 	case POWER_DOMAIN_PORT_DDI_E_IO:
 		return "PORT_DDI_E_IO";
+	case POWER_DOMAIN_PORT_DDI_F_IO:
+		return "PORT_DDI_F_IO";
 	case POWER_DOMAIN_PORT_DSI:
 		return "PORT_DSI";
 	case POWER_DOMAIN_PORT_CRT:
@@ -1860,6 +1864,9 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 #define CNL_DISPLAY_AUX_F_POWER_DOMAINS (		\
 	BIT_ULL(POWER_DOMAIN_AUX_F) |			\
 	BIT_ULL(POWER_DOMAIN_INIT))
+#define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) |		\
+	BIT_ULL(POWER_DOMAIN_INIT))
 #define CNL_DISPLAY_DC_OFF_POWER_DOMAINS (		\
 	CNL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
 	BIT_ULL(POWER_DOMAIN_GT_IRQ) |			\
@@ -2411,6 +2418,12 @@ static struct i915_power_well cnl_power_wells[] = {
 		.id = SKL_DISP_PW_DDI_D,
 	},
 	{
+		.name = "DDI F IO power well",
+		.domains = CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS,
+		.ops = &hsw_power_well_ops,
+		.id = CNL_DISP_PW_DDI_F,
+	},
+	{
 		.name = "AUX F",
 		.domains = CNL_DISPLAY_AUX_F_POWER_DOMAINS,
 		.ops = &hsw_power_well_ops,
@@ -2533,13 +2546,13 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 		set_power_wells(power_domains, cnl_power_wells);
 
 		/*
-		 * Aux IO is getting enabled for all ports
+		 * DDI and Aux IO are getting enabled for all ports
 		 * regardless the presence or use. So, in order to avoid
-		 * timeouts, lets remove it from the list
+		 * timeouts, lets remove them from the list
 		 * for the SKUs without port F.
 		 */
 		if (!IS_CNL_WITH_PORT_F(dev_priv))
-			power_domains->power_well_count -= 1;
+			power_domains->power_well_count -= 2;
 
 	} else if (IS_BROXTON(dev_priv)) {
 		set_power_wells(power_domains, bxt_power_wells);
-- 
2.13.6

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

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

* [PATCH 10/10] drm/i915/cnl: Fix DP max rate for Cannonlake with port F.
  2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
                   ` (7 preceding siblings ...)
  2018-01-25 19:27 ` [PATCH 09/10] drm/i915/cnl: Enable DDI-F on Cannonlake Rodrigo Vivi
@ 2018-01-25 19:27 ` Rodrigo Vivi
  2018-01-25 19:35 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Patchwork
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 19:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi, Rodrigo Vivi

On CNL SKUs that uses port F,  max DP rate is 8.1G for all
ports when we have the elevated voltage (higher than 0.85V).

v2: Make commit message more generic.
v3: Move conditions to a helper to get easier to read. (Ville).
v4: Add a mention to the numerical voltage on commit
    message per Manasi request.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2b26ffe100b1..31a968f20761 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -220,15 +220,34 @@ intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp)
 	return max_dotclk;
 }
 
+static int cnl_adjusted_max_rate(struct intel_dp *intel_dp, int size)
+{
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
+	enum port port = dig_port->base.port;
+
+	u32 voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
+
+	/* Low voltage SKUs are limited to max of 5.4G */
+	if (voltage == VOLTAGE_INFO_0_85V)
+		return size - 2;
+
+	/* For this SKU 8.1G is supported in all ports */
+	if(IS_CNL_WITH_PORT_F(dev_priv))
+		return size;
+
+	/* For other SKUs, max rate on ports A and B is 5.4G */
+	if (port == PORT_A || port == PORT_D)
+		return size - 2;
+}
+
 static void
 intel_dp_set_source_rates(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
-	enum port port = dig_port->base.port;
 	const int *source_rates;
 	int size;
-	u32 voltage;
 
 	/* This should only be done once */
 	WARN_ON(intel_dp->source_rates || intel_dp->num_source_rates);
@@ -238,11 +257,7 @@ intel_dp_set_source_rates(struct intel_dp *intel_dp)
 		size = ARRAY_SIZE(bxt_rates);
 	} else if (IS_CANNONLAKE(dev_priv)) {
 		source_rates = cnl_rates;
-		size = ARRAY_SIZE(cnl_rates);
-		voltage = I915_READ(CNL_PORT_COMP_DW3) & VOLTAGE_INFO_MASK;
-		if (port == PORT_A || port == PORT_D ||
-		    voltage == VOLTAGE_INFO_0_85V)
-			size -= 2;
+		size = cnl_adjusted_max_rate(intel_dp, ARRAY_SIZE(cnl_rates));
 	} else if (IS_GEN9_BC(dev_priv)) {
 		source_rates = skl_rates;
 		size = ARRAY_SIZE(skl_rates);
-- 
2.13.6

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

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

* ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
  2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
                   ` (8 preceding siblings ...)
  2018-01-25 19:27 ` [PATCH 10/10] drm/i915/cnl: Fix DP max rate for Cannonlake with port F Rodrigo Vivi
@ 2018-01-25 19:35 ` Patchwork
  2018-01-25 21:51 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev2) Patchwork
  2018-01-26 10:12 ` [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Jani Nikula
  11 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2018-01-25 19:35 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
URL   : https://patchwork.freedesktop.org/series/37129/
State : failure

== Summary ==

  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CHK     include/generated/bounds.h
  CHK     include/generated/timeconst.h
  CHK     include/generated/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  DESCEND  objtool
  CHK     scripts/mod/devicetable-offsets.h
  CHK     include/generated/compile.h
  CC      init/version.o
  AR      init/built-in.o
  CC      arch/x86/kernel/early-quirks.o
  AR      arch/x86/kernel/built-in.o
  AR      arch/x86/built-in.o
  CC      kernel/sys.o
  CC      kernel/trace/trace.o
  AR      kernel/trace/built-in.o
  CC      kernel/module.o
  AR      kernel/built-in.o
  GZIP    kernel/config_data.gz
  CHK     kernel/config_data.h
  UPD     kernel/config_data.h
  CC [M]  kernel/configs.o
  CC      drivers/base/firmware_class.o
  AR      drivers/base/built-in.o
  CC [M]  drivers/gpu/drm/i915/intel_dp.o
drivers/gpu/drm/i915/intel_dp.c: In function ‘cnl_adjusted_max_rate’:
drivers/gpu/drm/i915/intel_dp.c:242:1: error: control reaches end of non-void function [-Werror=return-type]
 }
 ^
cc1: all warnings being treated as errors
scripts/Makefile.build:316: recipe for target 'drivers/gpu/drm/i915/intel_dp.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_dp.o] Error 1
scripts/Makefile.build:575: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:575: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:575: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1018: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

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

* [PATCH] drm/i915/cnp: Properly handle VBT ddc pin out of bounds.
  2018-01-25 19:27 ` [PATCH 07/10] drm/i915: For HPD connected port use hpd_pin instead of port Rodrigo Vivi
@ 2018-01-25 21:37   ` Rodrigo Vivi
  2018-01-25 22:01     ` Rodrigo Vivi
  0 siblings, 1 reply; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 21:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Kai Heng Feng, Rodrigo Vivi

If the table result is out of bounds on the array map
there is something really wrong with VBT pin so we don't
return that vbt_pin, but only return 0 instead.

This basically reverts commit 'a8e6f3888b05 ("drm/i915/cnp:
Ignore VBT request for know invalid DDC pin.")'

Also this properly fixes commit 9c3b2689d01f ("drm/i915/cnl:
Map VBT DDC Pin to BSpec DDC Pin.")

v2: Do in a way that we don't break other platforms. (Jani)
v3: Keep debug message (Jani)

Fixes: a8e6f3888b05 ("drm/i915/cnp: Ignore VBT request for know invalid DDC pin.")
Fixes: 9c3b2689d01f ("drm/i915/cnl: Map VBT DDC Pin to BSpec DDC Pin.")
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Kai Heng Feng <kai.heng.feng@canonical.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 95f0b310d656..06526b17a011 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1116,9 +1116,9 @@ static const u8 cnp_ddc_pin_map[] = {
 static u8 map_ddc_pin(struct drm_i915_private *dev_priv, u8 vbt_pin)
 {
 	if (HAS_PCH_CNP(dev_priv)) {
-		if (vbt_pin > 0 && vbt_pin < ARRAY_SIZE(cnp_ddc_pin_map))
+		if (vbt_pin > 0 && vbt_pin < ARRAY_SIZE(cnp_ddc_pin_map)) {
 			return cnp_ddc_pin_map[vbt_pin];
-		if (vbt_pin > GMBUS_PIN_4_CNP) {
+		} else {
 			DRM_DEBUG_KMS("Ignoring alternate pin: VBT claims DDC pin %d, which is not valid for this platform\n", vbt_pin);
 			return 0;
 		}
-- 
2.13.6

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

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

* ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev2)
  2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
                   ` (9 preceding siblings ...)
  2018-01-25 19:35 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Patchwork
@ 2018-01-25 21:51 ` Patchwork
  2018-01-26 10:12 ` [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Jani Nikula
  11 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2018-01-25 21:51 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev2)
URL   : https://patchwork.freedesktop.org/series/37129/
State : failure

== Summary ==

  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CHK     include/generated/bounds.h
  CHK     include/generated/timeconst.h
  CHK     include/generated/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  DESCEND  objtool
  CHK     scripts/mod/devicetable-offsets.h
  CHK     include/generated/compile.h
  CC      init/version.o
  AR      init/built-in.o
  CC      arch/x86/kernel/early-quirks.o
  AR      arch/x86/kernel/built-in.o
  AR      arch/x86/built-in.o
  CC      kernel/sys.o
  CC      kernel/trace/trace.o
  AR      kernel/trace/built-in.o
  CC      kernel/module.o
  AR      kernel/built-in.o
  GZIP    kernel/config_data.gz
  CHK     kernel/config_data.h
  UPD     kernel/config_data.h
  CC [M]  kernel/configs.o
  CC      drivers/base/firmware_class.o
  AR      drivers/base/built-in.o
  CC [M]  drivers/gpu/drm/i915/intel_dp.o
drivers/gpu/drm/i915/intel_dp.c: In function ‘bxt_digital_port_connected’:
drivers/gpu/drm/i915/intel_dp.c:4659:31: error: incompatible type for argument 1 of ‘intel_hpd_pin_to_port’
  port = intel_hpd_pin_to_port(intel_encoder->hpd_pin);
                               ^~~~~~~~~~~~~
In file included from drivers/gpu/drm/i915/intel_drv.h:33:0,
                 from drivers/gpu/drm/i915/intel_dp.c:42:
drivers/gpu/drm/i915/i915_drv.h:2961:11: note: expected ‘struct drm_i915_private *’ but argument is of type ‘enum hpd_pin’
 enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
           ^~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/intel_dp.c:4659:9: error: too few arguments to function ‘intel_hpd_pin_to_port’
  port = intel_hpd_pin_to_port(intel_encoder->hpd_pin);
         ^~~~~~~~~~~~~~~~~~~~~
In file included from drivers/gpu/drm/i915/intel_drv.h:33:0,
                 from drivers/gpu/drm/i915/intel_dp.c:42:
drivers/gpu/drm/i915/i915_drv.h:2961:11: note: declared here
 enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
           ^~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/intel_dp.c: In function ‘cnl_adjusted_max_rate’:
drivers/gpu/drm/i915/intel_dp.c:242:1: error: control reaches end of non-void function [-Werror=return-type]
 }
 ^
cc1: all warnings being treated as errors
scripts/Makefile.build:316: recipe for target 'drivers/gpu/drm/i915/intel_dp.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_dp.o] Error 1
scripts/Makefile.build:575: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:575: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:575: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1018: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

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

* Re: [PATCH] drm/i915/cnp: Properly handle VBT ddc pin out of bounds.
  2018-01-25 21:37   ` [PATCH] drm/i915/cnp: Properly handle VBT ddc pin out of bounds Rodrigo Vivi
@ 2018-01-25 22:01     ` Rodrigo Vivi
  0 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 22:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Kai Heng Feng

On Thu, Jan 25, 2018 at 09:37:56PM +0000, Rodrigo Vivi wrote:

please ignore this one...

now I guess I will have to resend the entire series to avoid
CI confusion... :/

> If the table result is out of bounds on the array map
> there is something really wrong with VBT pin so we don't
> return that vbt_pin, but only return 0 instead.
> 
> This basically reverts commit 'a8e6f3888b05 ("drm/i915/cnp:
> Ignore VBT request for know invalid DDC pin.")'
> 
> Also this properly fixes commit 9c3b2689d01f ("drm/i915/cnl:
> Map VBT DDC Pin to BSpec DDC Pin.")
> 
> v2: Do in a way that we don't break other platforms. (Jani)
> v3: Keep debug message (Jani)
> 
> Fixes: a8e6f3888b05 ("drm/i915/cnp: Ignore VBT request for know invalid DDC pin.")
> Fixes: 9c3b2689d01f ("drm/i915/cnl: Map VBT DDC Pin to BSpec DDC Pin.")
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Kai Heng Feng <kai.heng.feng@canonical.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_bios.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 95f0b310d656..06526b17a011 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1116,9 +1116,9 @@ static const u8 cnp_ddc_pin_map[] = {
>  static u8 map_ddc_pin(struct drm_i915_private *dev_priv, u8 vbt_pin)
>  {
>  	if (HAS_PCH_CNP(dev_priv)) {
> -		if (vbt_pin > 0 && vbt_pin < ARRAY_SIZE(cnp_ddc_pin_map))
> +		if (vbt_pin > 0 && vbt_pin < ARRAY_SIZE(cnp_ddc_pin_map)) {
>  			return cnp_ddc_pin_map[vbt_pin];
> -		if (vbt_pin > GMBUS_PIN_4_CNP) {
> +		} else {
>  			DRM_DEBUG_KMS("Ignoring alternate pin: VBT claims DDC pin %d, which is not valid for this platform\n", vbt_pin);
>  			return 0;
>  		}
> -- 
> 2.13.6
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
  2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
                   ` (10 preceding siblings ...)
  2018-01-25 21:51 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev2) Patchwork
@ 2018-01-26 10:12 ` Jani Nikula
  2018-01-26 17:26   ` Rodrigo Vivi
  11 siblings, 1 reply; 26+ messages in thread
From: Jani Nikula @ 2018-01-26 10:12 UTC (permalink / raw)
  To: intel-gfx
  Cc: Rodrigo Vivi, Lucas De Marchi, Paulo Zanoni, Dhinakaran Pandiyan

On Thu, 25 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> The only difference is that this SKUs has the full
> Port A/E split named as Port F.
>
> But since SKUs differences don't matter on the platform
> definition group and ids, let's merge all off them together.
>
> v2: Really include the PCI IDs to the picidlist[];
> v3: Add the PCI Id for another SKU (Anusha).
> v4: Update IDs, really include to pciidlists again.
> v5: Unify all GT2 IDs.
> v6: Unify in a way that we don't break early-quirks.c
> v7: Remove GT reference since it doesn't matter here (Paulo)
>     Also move IS_CNL_WITH_PORT_F macro to this patch to
>     make it easier for review this part and also to get
>     used sooner.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>  drivers/gpu/drm/i915/i915_pci.c |  5 ++---
>  include/drm/i915_pciids.h       | 18 +++++++-----------
>  3 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 454d8f937fae..5702ebf17974 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2648,6 +2648,8 @@ intel_info(const struct drm_i915_private *dev_priv)
>  				 (dev_priv)->info.gt == 2)
>  #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
>  				 (dev_priv)->info.gt == 3)
> +#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
> +					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)

I'd be happy if bit 2 in device id actually meant "port F", but I'm not
so happy with coming up with rules like this for coincidental things.

More generally, I'm not all that happy about *any* of the INTEL_DEVID
uses in i915_drv.h because it spreads out the device id information, so
I'd rather not add more. I'd rather move towards single point of truth
for device ids.

Okay, so this is late in the review cycles, and part of a more general
problem, so we should probably just go with this for now and come back
to this later.


BR,
Jani.




>  
>  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
>  
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index f28c165fc49d..7eb3d5e4350e 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -571,7 +571,7 @@ static const struct intel_device_info intel_coffeelake_gt3_info __initconst = {
>  	.ddb_size = 1024, \
>  	GLK_COLORS
>  
> -static const struct intel_device_info intel_cannonlake_gt2_info __initconst = {
> +static const struct intel_device_info intel_cannonlake_info __initconst = {
>  	GEN10_FEATURES,
>  	.is_alpha_support = 1,
>  	.platform = INTEL_CANNONLAKE,
> @@ -649,8 +649,7 @@ static const struct pci_device_id pciidlist[] = {
>  	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
>  	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
>  	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
> -	INTEL_CNL_U_GT2_IDS(&intel_cannonlake_gt2_info),
> -	INTEL_CNL_Y_GT2_IDS(&intel_cannonlake_gt2_info),
> +	INTEL_CNL_IDS(&intel_cannonlake_info),
>  	{0, 0, 0}
>  };
>  MODULE_DEVICE_TABLE(pci, pciidlist);
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index 5db0458dd832..9e1fe6634424 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -414,24 +414,20 @@
>  	INTEL_CFL_U_GT2_IDS(info), \
>  	INTEL_CFL_U_GT3_IDS(info)
>  
> -/* CNL U 2+2 */
> -#define INTEL_CNL_U_GT2_IDS(info) \
> +/* CNL */
> +#define INTEL_CNL_IDS(info) \
>  	INTEL_VGA_DEVICE(0x5A52, info), \
>  	INTEL_VGA_DEVICE(0x5A5A, info), \
>  	INTEL_VGA_DEVICE(0x5A42, info), \
> -	INTEL_VGA_DEVICE(0x5A4A, info)
> -
> -/* CNL Y 2+2 */
> -#define INTEL_CNL_Y_GT2_IDS(info) \
> +	INTEL_VGA_DEVICE(0x5A4A, info), \
>  	INTEL_VGA_DEVICE(0x5A51, info), \
>  	INTEL_VGA_DEVICE(0x5A59, info), \
>  	INTEL_VGA_DEVICE(0x5A41, info), \
>  	INTEL_VGA_DEVICE(0x5A49, info), \
>  	INTEL_VGA_DEVICE(0x5A71, info), \
> -	INTEL_VGA_DEVICE(0x5A79, info)
> -
> -#define INTEL_CNL_IDS(info) \
> -	INTEL_CNL_U_GT2_IDS(info), \
> -	INTEL_CNL_Y_GT2_IDS(info)
> +	INTEL_VGA_DEVICE(0x5A79, info), \
> +	INTEL_VGA_DEVICE(0x5A54, info), \
> +	INTEL_VGA_DEVICE(0x5A5C, info), \
> +	INTEL_VGA_DEVICE(0x5A44, info)
>  
>  #endif /* _I915_PCIIDS_H */

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

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

* Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
  2018-01-26 10:12 ` [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Jani Nikula
@ 2018-01-26 17:26   ` Rodrigo Vivi
  2018-01-27  9:05     ` Jani Nikula
  0 siblings, 1 reply; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-26 17:26 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Lucas De Marchi, Paulo Zanoni, Dhinakaran Pandiyan

On Fri, Jan 26, 2018 at 10:12:00AM +0000, Jani Nikula wrote:
> On Thu, 25 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > The only difference is that this SKUs has the full
> > Port A/E split named as Port F.
> >
> > But since SKUs differences don't matter on the platform
> > definition group and ids, let's merge all off them together.
> >
> > v2: Really include the PCI IDs to the picidlist[];
> > v3: Add the PCI Id for another SKU (Anusha).
> > v4: Update IDs, really include to pciidlists again.
> > v5: Unify all GT2 IDs.
> > v6: Unify in a way that we don't break early-quirks.c
> > v7: Remove GT reference since it doesn't matter here (Paulo)
> >     Also move IS_CNL_WITH_PORT_F macro to this patch to
> >     make it easier for review this part and also to get
> >     used sooner.
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h |  2 ++
> >  drivers/gpu/drm/i915/i915_pci.c |  5 ++---
> >  include/drm/i915_pciids.h       | 18 +++++++-----------
> >  3 files changed, 11 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 454d8f937fae..5702ebf17974 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2648,6 +2648,8 @@ intel_info(const struct drm_i915_private *dev_priv)
> >  				 (dev_priv)->info.gt == 2)
> >  #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
> >  				 (dev_priv)->info.gt == 3)
> > +#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
> > +					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
> 
> I'd be happy if bit 2 in device id actually meant "port F", but I'm not
> so happy with coming up with rules like this for coincidental things.

the port F thing is just that we have no name for that SKU :(
but from the spec organization this bit seems to represent that
group. Although I fully agree with you that this is horrible.

> 
> More generally, I'm not all that happy about *any* of the INTEL_DEVID
> uses in i915_drv.h because it spreads out the device id information, so
> I'd rather not add more. I'd rather move towards single point of truth
> for device ids.

Yeah. I guess I agree with you.
There should be something inside the device info right?
even if we have to separate that in two groups.

If you agree with this approach I can follow up with a patch/series
that kill INTEL_DEVID in favor of device info structs.

Or do you have any other idea for solving this?

on this particular case here I considered that, but
since I had no good name and INTEL_DEVID was there I've
chosen the lazy path.

> 
> Okay, so this is late in the review cycles, and part of a more general
> problem, so we should probably just go with this for now and come back
> to this later.

It is never too late ;)

But in this case I'd prefer moving with this series as is
to avoid blocking Paulo with ICL enabling and minimize
the conflicts and rebase pain to only one area of the code.

Thanks for bringing it up,
Rodrigo.

> 
> 
> BR,
> Jani.
> 
> 
> 
> 
> >  
> >  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> > index f28c165fc49d..7eb3d5e4350e 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -571,7 +571,7 @@ static const struct intel_device_info intel_coffeelake_gt3_info __initconst = {
> >  	.ddb_size = 1024, \
> >  	GLK_COLORS
> >  
> > -static const struct intel_device_info intel_cannonlake_gt2_info __initconst = {
> > +static const struct intel_device_info intel_cannonlake_info __initconst = {
> >  	GEN10_FEATURES,
> >  	.is_alpha_support = 1,
> >  	.platform = INTEL_CANNONLAKE,
> > @@ -649,8 +649,7 @@ static const struct pci_device_id pciidlist[] = {
> >  	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
> >  	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
> >  	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
> > -	INTEL_CNL_U_GT2_IDS(&intel_cannonlake_gt2_info),
> > -	INTEL_CNL_Y_GT2_IDS(&intel_cannonlake_gt2_info),
> > +	INTEL_CNL_IDS(&intel_cannonlake_info),
> >  	{0, 0, 0}
> >  };
> >  MODULE_DEVICE_TABLE(pci, pciidlist);
> > diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> > index 5db0458dd832..9e1fe6634424 100644
> > --- a/include/drm/i915_pciids.h
> > +++ b/include/drm/i915_pciids.h
> > @@ -414,24 +414,20 @@
> >  	INTEL_CFL_U_GT2_IDS(info), \
> >  	INTEL_CFL_U_GT3_IDS(info)
> >  
> > -/* CNL U 2+2 */
> > -#define INTEL_CNL_U_GT2_IDS(info) \
> > +/* CNL */
> > +#define INTEL_CNL_IDS(info) \
> >  	INTEL_VGA_DEVICE(0x5A52, info), \
> >  	INTEL_VGA_DEVICE(0x5A5A, info), \
> >  	INTEL_VGA_DEVICE(0x5A42, info), \
> > -	INTEL_VGA_DEVICE(0x5A4A, info)
> > -
> > -/* CNL Y 2+2 */
> > -#define INTEL_CNL_Y_GT2_IDS(info) \
> > +	INTEL_VGA_DEVICE(0x5A4A, info), \
> >  	INTEL_VGA_DEVICE(0x5A51, info), \
> >  	INTEL_VGA_DEVICE(0x5A59, info), \
> >  	INTEL_VGA_DEVICE(0x5A41, info), \
> >  	INTEL_VGA_DEVICE(0x5A49, info), \
> >  	INTEL_VGA_DEVICE(0x5A71, info), \
> > -	INTEL_VGA_DEVICE(0x5A79, info)
> > -
> > -#define INTEL_CNL_IDS(info) \
> > -	INTEL_CNL_U_GT2_IDS(info), \
> > -	INTEL_CNL_Y_GT2_IDS(info)
> > +	INTEL_VGA_DEVICE(0x5A79, info), \
> > +	INTEL_VGA_DEVICE(0x5A54, info), \
> > +	INTEL_VGA_DEVICE(0x5A5C, info), \
> > +	INTEL_VGA_DEVICE(0x5A44, info)
> >  
> >  #endif /* _I915_PCIIDS_H */
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
  2018-01-26 17:26   ` Rodrigo Vivi
@ 2018-01-27  9:05     ` Jani Nikula
  2018-01-29 19:09       ` Pandiyan, Dhinakaran
  2018-01-30 10:48       ` Tvrtko Ursulin
  0 siblings, 2 replies; 26+ messages in thread
From: Jani Nikula @ 2018-01-27  9:05 UTC (permalink / raw)
  To: Rodrigo Vivi
  Cc: Paulo Zanoni, intel-gfx, Lucas De Marchi, Dhinakaran Pandiyan

On Fri, 26 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Fri, Jan 26, 2018 at 10:12:00AM +0000, Jani Nikula wrote:
>> On Thu, 25 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>> > The only difference is that this SKUs has the full
>> > Port A/E split named as Port F.
>> >
>> > But since SKUs differences don't matter on the platform
>> > definition group and ids, let's merge all off them together.
>> >
>> > v2: Really include the PCI IDs to the picidlist[];
>> > v3: Add the PCI Id for another SKU (Anusha).
>> > v4: Update IDs, really include to pciidlists again.
>> > v5: Unify all GT2 IDs.
>> > v6: Unify in a way that we don't break early-quirks.c
>> > v7: Remove GT reference since it doesn't matter here (Paulo)
>> >     Also move IS_CNL_WITH_PORT_F macro to this patch to
>> >     make it easier for review this part and also to get
>> >     used sooner.
>> >
>> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>> >  drivers/gpu/drm/i915/i915_pci.c |  5 ++---
>> >  include/drm/i915_pciids.h       | 18 +++++++-----------
>> >  3 files changed, 11 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> > index 454d8f937fae..5702ebf17974 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -2648,6 +2648,8 @@ intel_info(const struct drm_i915_private *dev_priv)
>> >  				 (dev_priv)->info.gt == 2)
>> >  #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
>> >  				 (dev_priv)->info.gt == 3)
>> > +#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
>> > +					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
>> 
>> I'd be happy if bit 2 in device id actually meant "port F", but I'm not
>> so happy with coming up with rules like this for coincidental things.
>
> the port F thing is just that we have no name for that SKU :(
> but from the spec organization this bit seems to represent that
> group. Although I fully agree with you that this is horrible.
>
>> 
>> More generally, I'm not all that happy about *any* of the INTEL_DEVID
>> uses in i915_drv.h because it spreads out the device id information, so
>> I'd rather not add more. I'd rather move towards single point of truth
>> for device ids.
>
> Yeah. I guess I agree with you.
> There should be something inside the device info right?
> even if we have to separate that in two groups.

That's the thing. It also worries me to add everything in device info,
because it grows the device info structs for all devices, and you also
need to add more device infos to cover all combinations. Unless you
initialize them runtime, which is also a bit meh.

> If you agree with this approach I can follow up with a patch/series
> that kill INTEL_DEVID in favor of device info structs.
>
> Or do you have any other idea for solving this?

Not really. Cc: Chris and Tvrtko who seem to come up with ideas for
things like this on a regular basis. ;)

BR,
Jani.

>
> on this particular case here I considered that, but
> since I had no good name and INTEL_DEVID was there I've
> chosen the lazy path.
>
>> 
>> Okay, so this is late in the review cycles, and part of a more general
>> problem, so we should probably just go with this for now and come back
>> to this later.
>
> It is never too late ;)
>
> But in this case I'd prefer moving with this series as is
> to avoid blocking Paulo with ICL enabling and minimize
> the conflicts and rebase pain to only one area of the code.
>
> Thanks for bringing it up,
> Rodrigo.
>
>> 
>> 
>> BR,
>> Jani.
>> 
>> 
>> 
>> 
>> >  
>> >  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
>> >  
>> > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> > index f28c165fc49d..7eb3d5e4350e 100644
>> > --- a/drivers/gpu/drm/i915/i915_pci.c
>> > +++ b/drivers/gpu/drm/i915/i915_pci.c
>> > @@ -571,7 +571,7 @@ static const struct intel_device_info intel_coffeelake_gt3_info __initconst = {
>> >  	.ddb_size = 1024, \
>> >  	GLK_COLORS
>> >  
>> > -static const struct intel_device_info intel_cannonlake_gt2_info __initconst = {
>> > +static const struct intel_device_info intel_cannonlake_info __initconst = {
>> >  	GEN10_FEATURES,
>> >  	.is_alpha_support = 1,
>> >  	.platform = INTEL_CANNONLAKE,
>> > @@ -649,8 +649,7 @@ static const struct pci_device_id pciidlist[] = {
>> >  	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
>> >  	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
>> >  	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
>> > -	INTEL_CNL_U_GT2_IDS(&intel_cannonlake_gt2_info),
>> > -	INTEL_CNL_Y_GT2_IDS(&intel_cannonlake_gt2_info),
>> > +	INTEL_CNL_IDS(&intel_cannonlake_info),
>> >  	{0, 0, 0}
>> >  };
>> >  MODULE_DEVICE_TABLE(pci, pciidlist);
>> > diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
>> > index 5db0458dd832..9e1fe6634424 100644
>> > --- a/include/drm/i915_pciids.h
>> > +++ b/include/drm/i915_pciids.h
>> > @@ -414,24 +414,20 @@
>> >  	INTEL_CFL_U_GT2_IDS(info), \
>> >  	INTEL_CFL_U_GT3_IDS(info)
>> >  
>> > -/* CNL U 2+2 */
>> > -#define INTEL_CNL_U_GT2_IDS(info) \
>> > +/* CNL */
>> > +#define INTEL_CNL_IDS(info) \
>> >  	INTEL_VGA_DEVICE(0x5A52, info), \
>> >  	INTEL_VGA_DEVICE(0x5A5A, info), \
>> >  	INTEL_VGA_DEVICE(0x5A42, info), \
>> > -	INTEL_VGA_DEVICE(0x5A4A, info)
>> > -
>> > -/* CNL Y 2+2 */
>> > -#define INTEL_CNL_Y_GT2_IDS(info) \
>> > +	INTEL_VGA_DEVICE(0x5A4A, info), \
>> >  	INTEL_VGA_DEVICE(0x5A51, info), \
>> >  	INTEL_VGA_DEVICE(0x5A59, info), \
>> >  	INTEL_VGA_DEVICE(0x5A41, info), \
>> >  	INTEL_VGA_DEVICE(0x5A49, info), \
>> >  	INTEL_VGA_DEVICE(0x5A71, info), \
>> > -	INTEL_VGA_DEVICE(0x5A79, info)
>> > -
>> > -#define INTEL_CNL_IDS(info) \
>> > -	INTEL_CNL_U_GT2_IDS(info), \
>> > -	INTEL_CNL_Y_GT2_IDS(info)
>> > +	INTEL_VGA_DEVICE(0x5A79, info), \
>> > +	INTEL_VGA_DEVICE(0x5A54, info), \
>> > +	INTEL_VGA_DEVICE(0x5A5C, info), \
>> > +	INTEL_VGA_DEVICE(0x5A44, info)
>> >  
>> >  #endif /* _I915_PCIIDS_H */
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center

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

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

* Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
  2018-01-27  9:05     ` Jani Nikula
@ 2018-01-29 19:09       ` Pandiyan, Dhinakaran
  2018-01-29 20:35         ` Jani Nikula
  2018-01-30 10:48       ` Tvrtko Ursulin
  1 sibling, 1 reply; 26+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-01-29 19:09 UTC (permalink / raw)
  To: jani.nikula; +Cc: intel-gfx, De Marchi, Lucas, Zanoni, Paulo R, Vivi, Rodrigo




On Sat, 2018-01-27 at 11:05 +0200, Jani Nikula wrote:
> On Fri, 26 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > On Fri, Jan 26, 2018 at 10:12:00AM +0000, Jani Nikula wrote:
> >> On Thu, 25 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> >> > The only difference is that this SKUs has the full
> >> > Port A/E split named as Port F.
> >> >
> >> > But since SKUs differences don't matter on the platform
> >> > definition group and ids, let's merge all off them together.
> >> >
> >> > v2: Really include the PCI IDs to the picidlist[];
> >> > v3: Add the PCI Id for another SKU (Anusha).
> >> > v4: Update IDs, really include to pciidlists again.
> >> > v5: Unify all GT2 IDs.
> >> > v6: Unify in a way that we don't break early-quirks.c
> >> > v7: Remove GT reference since it doesn't matter here (Paulo)
> >> >     Also move IS_CNL_WITH_PORT_F macro to this patch to
> >> >     make it easier for review this part and also to get
> >> >     used sooner.
> >> >
> >> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> >> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/i915_drv.h |  2 ++
> >> >  drivers/gpu/drm/i915/i915_pci.c |  5 ++---
> >> >  include/drm/i915_pciids.h       | 18 +++++++-----------
> >> >  3 files changed, 11 insertions(+), 14 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> > index 454d8f937fae..5702ebf17974 100644
> >> > --- a/drivers/gpu/drm/i915/i915_drv.h
> >> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> > @@ -2648,6 +2648,8 @@ intel_info(const struct drm_i915_private *dev_priv)
> >> >  				 (dev_priv)->info.gt == 2)
> >> >  #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
> >> >  				 (dev_priv)->info.gt == 3)
> >> > +#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
> >> > +					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
> >> 
> >> I'd be happy if bit 2 in device id actually meant "port F", but I'm not
> >> so happy with coming up with rules like this for coincidental things.
> >
> > the port F thing is just that we have no name for that SKU :(
> > but from the spec organization this bit seems to represent that
> > group. Although I fully agree with you that this is horrible.
> >
> >> 
> >> More generally, I'm not all that happy about *any* of the INTEL_DEVID
> >> uses in i915_drv.h because it spreads out the device id information, so
> >> I'd rather not add more. I'd rather move towards single point of truth
> >> for device ids.
> >
> > Yeah. I guess I agree with you.
> > There should be something inside the device info right?
> > even if we have to separate that in two groups.
> 
> That's the thing. It also worries me to add everything in device info,
> because it grows the device info structs for all devices, and you also
> need to add more device infos to cover all combinations. Unless you
> initialize them runtime, which is also a bit meh.
> 

I am wondering if we can augment the device info struct at run-time with
a union of platform-specific key-value pairs.


> > If you agree with this approach I can follow up with a patch/series
> > that kill INTEL_DEVID in favor of device info structs.
> >
> > Or do you have any other idea for solving this?
> 
> Not really. Cc: Chris and Tvrtko who seem to come up with ideas for
> things like this on a regular basis. ;)
> 
> BR,
> Jani.
> 
> >
> > on this particular case here I considered that, but
> > since I had no good name and INTEL_DEVID was there I've
> > chosen the lazy path.
> >
> >> 
> >> Okay, so this is late in the review cycles, and part of a more general
> >> problem, so we should probably just go with this for now and come back
> >> to this later.
> >
> > It is never too late ;)
> >
> > But in this case I'd prefer moving with this series as is
> > to avoid blocking Paulo with ICL enabling and minimize
> > the conflicts and rebase pain to only one area of the code.
> >
> > Thanks for bringing it up,
> > Rodrigo.
> >
> >> 
> >> 
> >> BR,
> >> Jani.
> >> 
> >> 
> >> 
> >> 
> >> >  
> >> >  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
> >> >  
> >> > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> >> > index f28c165fc49d..7eb3d5e4350e 100644
> >> > --- a/drivers/gpu/drm/i915/i915_pci.c
> >> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> >> > @@ -571,7 +571,7 @@ static const struct intel_device_info intel_coffeelake_gt3_info __initconst = {
> >> >  	.ddb_size = 1024, \
> >> >  	GLK_COLORS
> >> >  
> >> > -static const struct intel_device_info intel_cannonlake_gt2_info __initconst = {
> >> > +static const struct intel_device_info intel_cannonlake_info __initconst = {
> >> >  	GEN10_FEATURES,
> >> >  	.is_alpha_support = 1,
> >> >  	.platform = INTEL_CANNONLAKE,
> >> > @@ -649,8 +649,7 @@ static const struct pci_device_id pciidlist[] = {
> >> >  	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
> >> >  	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
> >> >  	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
> >> > -	INTEL_CNL_U_GT2_IDS(&intel_cannonlake_gt2_info),
> >> > -	INTEL_CNL_Y_GT2_IDS(&intel_cannonlake_gt2_info),
> >> > +	INTEL_CNL_IDS(&intel_cannonlake_info),
> >> >  	{0, 0, 0}
> >> >  };
> >> >  MODULE_DEVICE_TABLE(pci, pciidlist);
> >> > diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> >> > index 5db0458dd832..9e1fe6634424 100644
> >> > --- a/include/drm/i915_pciids.h
> >> > +++ b/include/drm/i915_pciids.h
> >> > @@ -414,24 +414,20 @@
> >> >  	INTEL_CFL_U_GT2_IDS(info), \
> >> >  	INTEL_CFL_U_GT3_IDS(info)
> >> >  
> >> > -/* CNL U 2+2 */
> >> > -#define INTEL_CNL_U_GT2_IDS(info) \
> >> > +/* CNL */
> >> > +#define INTEL_CNL_IDS(info) \
> >> >  	INTEL_VGA_DEVICE(0x5A52, info), \
> >> >  	INTEL_VGA_DEVICE(0x5A5A, info), \
> >> >  	INTEL_VGA_DEVICE(0x5A42, info), \
> >> > -	INTEL_VGA_DEVICE(0x5A4A, info)
> >> > -
> >> > -/* CNL Y 2+2 */
> >> > -#define INTEL_CNL_Y_GT2_IDS(info) \
> >> > +	INTEL_VGA_DEVICE(0x5A4A, info), \
> >> >  	INTEL_VGA_DEVICE(0x5A51, info), \
> >> >  	INTEL_VGA_DEVICE(0x5A59, info), \
> >> >  	INTEL_VGA_DEVICE(0x5A41, info), \
> >> >  	INTEL_VGA_DEVICE(0x5A49, info), \
> >> >  	INTEL_VGA_DEVICE(0x5A71, info), \
> >> > -	INTEL_VGA_DEVICE(0x5A79, info)
> >> > -
> >> > -#define INTEL_CNL_IDS(info) \
> >> > -	INTEL_CNL_U_GT2_IDS(info), \
> >> > -	INTEL_CNL_Y_GT2_IDS(info)
> >> > +	INTEL_VGA_DEVICE(0x5A79, info), \
> >> > +	INTEL_VGA_DEVICE(0x5A54, info), \
> >> > +	INTEL_VGA_DEVICE(0x5A5C, info), \
> >> > +	INTEL_VGA_DEVICE(0x5A44, info)
> >> >  
> >> >  #endif /* _I915_PCIIDS_H */
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Technology Center
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
  2018-01-29 19:09       ` Pandiyan, Dhinakaran
@ 2018-01-29 20:35         ` Jani Nikula
  0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2018-01-29 20:35 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran
  Cc: intel-gfx, De Marchi, Lucas, Zanoni, Paulo R, Vivi, Rodrigo

On Mon, 29 Jan 2018, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote:
> On Sat, 2018-01-27 at 11:05 +0200, Jani Nikula wrote:
>> On Fri, 26 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>> > On Fri, Jan 26, 2018 at 10:12:00AM +0000, Jani Nikula wrote:
>> >> On Thu, 25 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>> >> > The only difference is that this SKUs has the full
>> >> > Port A/E split named as Port F.
>> >> >
>> >> > But since SKUs differences don't matter on the platform
>> >> > definition group and ids, let's merge all off them together.
>> >> >
>> >> > v2: Really include the PCI IDs to the picidlist[];
>> >> > v3: Add the PCI Id for another SKU (Anusha).
>> >> > v4: Update IDs, really include to pciidlists again.
>> >> > v5: Unify all GT2 IDs.
>> >> > v6: Unify in a way that we don't break early-quirks.c
>> >> > v7: Remove GT reference since it doesn't matter here (Paulo)
>> >> >     Also move IS_CNL_WITH_PORT_F macro to this patch to
>> >> >     make it easier for review this part and also to get
>> >> >     used sooner.
>> >> >
>> >> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> >> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> >> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> >> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> >> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> >> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> >> > ---
>> >> >  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>> >> >  drivers/gpu/drm/i915/i915_pci.c |  5 ++---
>> >> >  include/drm/i915_pciids.h       | 18 +++++++-----------
>> >> >  3 files changed, 11 insertions(+), 14 deletions(-)
>> >> >
>> >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> >> > index 454d8f937fae..5702ebf17974 100644
>> >> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> >> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> >> > @@ -2648,6 +2648,8 @@ intel_info(const struct drm_i915_private *dev_priv)
>> >> >  				 (dev_priv)->info.gt == 2)
>> >> >  #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
>> >> >  				 (dev_priv)->info.gt == 3)
>> >> > +#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
>> >> > +					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
>> >> 
>> >> I'd be happy if bit 2 in device id actually meant "port F", but I'm not
>> >> so happy with coming up with rules like this for coincidental things.
>> >
>> > the port F thing is just that we have no name for that SKU :(
>> > but from the spec organization this bit seems to represent that
>> > group. Although I fully agree with you that this is horrible.
>> >
>> >> 
>> >> More generally, I'm not all that happy about *any* of the INTEL_DEVID
>> >> uses in i915_drv.h because it spreads out the device id information, so
>> >> I'd rather not add more. I'd rather move towards single point of truth
>> >> for device ids.
>> >
>> > Yeah. I guess I agree with you.
>> > There should be something inside the device info right?
>> > even if we have to separate that in two groups.
>> 
>> That's the thing. It also worries me to add everything in device info,
>> because it grows the device info structs for all devices, and you also
>> need to add more device infos to cover all combinations. Unless you
>> initialize them runtime, which is also a bit meh.
>> 
>
> I am wondering if we can augment the device info struct at run-time with
> a union of platform-specific key-value pairs.

That occurred to me too, but I'm not thrilled about the prospect of
maintaining the union and ensuring we only ever access the fields within
correct platform specific code or conditions.

BR,
Jani.


>
>
>> > If you agree with this approach I can follow up with a patch/series
>> > that kill INTEL_DEVID in favor of device info structs.
>> >
>> > Or do you have any other idea for solving this?
>> 
>> Not really. Cc: Chris and Tvrtko who seem to come up with ideas for
>> things like this on a regular basis. ;)
>> 
>> BR,
>> Jani.
>> 
>> >
>> > on this particular case here I considered that, but
>> > since I had no good name and INTEL_DEVID was there I've
>> > chosen the lazy path.
>> >
>> >> 
>> >> Okay, so this is late in the review cycles, and part of a more general
>> >> problem, so we should probably just go with this for now and come back
>> >> to this later.
>> >
>> > It is never too late ;)
>> >
>> > But in this case I'd prefer moving with this series as is
>> > to avoid blocking Paulo with ICL enabling and minimize
>> > the conflicts and rebase pain to only one area of the code.
>> >
>> > Thanks for bringing it up,
>> > Rodrigo.
>> >
>> >> 
>> >> 
>> >> BR,
>> >> Jani.
>> >> 
>> >> 
>> >> 
>> >> 
>> >> >  
>> >> >  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
>> >> >  
>> >> > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> >> > index f28c165fc49d..7eb3d5e4350e 100644
>> >> > --- a/drivers/gpu/drm/i915/i915_pci.c
>> >> > +++ b/drivers/gpu/drm/i915/i915_pci.c
>> >> > @@ -571,7 +571,7 @@ static const struct intel_device_info intel_coffeelake_gt3_info __initconst = {
>> >> >  	.ddb_size = 1024, \
>> >> >  	GLK_COLORS
>> >> >  
>> >> > -static const struct intel_device_info intel_cannonlake_gt2_info __initconst = {
>> >> > +static const struct intel_device_info intel_cannonlake_info __initconst = {
>> >> >  	GEN10_FEATURES,
>> >> >  	.is_alpha_support = 1,
>> >> >  	.platform = INTEL_CANNONLAKE,
>> >> > @@ -649,8 +649,7 @@ static const struct pci_device_id pciidlist[] = {
>> >> >  	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
>> >> >  	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
>> >> >  	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
>> >> > -	INTEL_CNL_U_GT2_IDS(&intel_cannonlake_gt2_info),
>> >> > -	INTEL_CNL_Y_GT2_IDS(&intel_cannonlake_gt2_info),
>> >> > +	INTEL_CNL_IDS(&intel_cannonlake_info),
>> >> >  	{0, 0, 0}
>> >> >  };
>> >> >  MODULE_DEVICE_TABLE(pci, pciidlist);
>> >> > diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
>> >> > index 5db0458dd832..9e1fe6634424 100644
>> >> > --- a/include/drm/i915_pciids.h
>> >> > +++ b/include/drm/i915_pciids.h
>> >> > @@ -414,24 +414,20 @@
>> >> >  	INTEL_CFL_U_GT2_IDS(info), \
>> >> >  	INTEL_CFL_U_GT3_IDS(info)
>> >> >  
>> >> > -/* CNL U 2+2 */
>> >> > -#define INTEL_CNL_U_GT2_IDS(info) \
>> >> > +/* CNL */
>> >> > +#define INTEL_CNL_IDS(info) \
>> >> >  	INTEL_VGA_DEVICE(0x5A52, info), \
>> >> >  	INTEL_VGA_DEVICE(0x5A5A, info), \
>> >> >  	INTEL_VGA_DEVICE(0x5A42, info), \
>> >> > -	INTEL_VGA_DEVICE(0x5A4A, info)
>> >> > -
>> >> > -/* CNL Y 2+2 */
>> >> > -#define INTEL_CNL_Y_GT2_IDS(info) \
>> >> > +	INTEL_VGA_DEVICE(0x5A4A, info), \
>> >> >  	INTEL_VGA_DEVICE(0x5A51, info), \
>> >> >  	INTEL_VGA_DEVICE(0x5A59, info), \
>> >> >  	INTEL_VGA_DEVICE(0x5A41, info), \
>> >> >  	INTEL_VGA_DEVICE(0x5A49, info), \
>> >> >  	INTEL_VGA_DEVICE(0x5A71, info), \
>> >> > -	INTEL_VGA_DEVICE(0x5A79, info)
>> >> > -
>> >> > -#define INTEL_CNL_IDS(info) \
>> >> > -	INTEL_CNL_U_GT2_IDS(info), \
>> >> > -	INTEL_CNL_Y_GT2_IDS(info)
>> >> > +	INTEL_VGA_DEVICE(0x5A79, info), \
>> >> > +	INTEL_VGA_DEVICE(0x5A54, info), \
>> >> > +	INTEL_VGA_DEVICE(0x5A5C, info), \
>> >> > +	INTEL_VGA_DEVICE(0x5A44, info)
>> >> >  
>> >> >  #endif /* _I915_PCIIDS_H */
>> >> 
>> >> -- 
>> >> Jani Nikula, Intel Open Source Technology Center
>> 

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

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

* Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
  2018-01-27  9:05     ` Jani Nikula
  2018-01-29 19:09       ` Pandiyan, Dhinakaran
@ 2018-01-30 10:48       ` Tvrtko Ursulin
  1 sibling, 0 replies; 26+ messages in thread
From: Tvrtko Ursulin @ 2018-01-30 10:48 UTC (permalink / raw)
  To: Jani Nikula, Rodrigo Vivi
  Cc: intel-gfx, Lucas De Marchi, Paulo Zanoni, Dhinakaran Pandiyan


On 27/01/2018 09:05, Jani Nikula wrote:
> On Fri, 26 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>> On Fri, Jan 26, 2018 at 10:12:00AM +0000, Jani Nikula wrote:
>>> On Thu, 25 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>>>> The only difference is that this SKUs has the full
>>>> Port A/E split named as Port F.
>>>>
>>>> But since SKUs differences don't matter on the platform
>>>> definition group and ids, let's merge all off them together.
>>>>
>>>> v2: Really include the PCI IDs to the picidlist[];
>>>> v3: Add the PCI Id for another SKU (Anusha).
>>>> v4: Update IDs, really include to pciidlists again.
>>>> v5: Unify all GT2 IDs.
>>>> v6: Unify in a way that we don't break early-quirks.c
>>>> v7: Remove GT reference since it doesn't matter here (Paulo)
>>>>      Also move IS_CNL_WITH_PORT_F macro to this patch to
>>>>      make it easier for review this part and also to get
>>>>      used sooner.
>>>>
>>>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>>>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/i915_drv.h |  2 ++
>>>>   drivers/gpu/drm/i915/i915_pci.c |  5 ++---
>>>>   include/drm/i915_pciids.h       | 18 +++++++-----------
>>>>   3 files changed, 11 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>> index 454d8f937fae..5702ebf17974 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>> @@ -2648,6 +2648,8 @@ intel_info(const struct drm_i915_private *dev_priv)
>>>>   				 (dev_priv)->info.gt == 2)
>>>>   #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
>>>>   				 (dev_priv)->info.gt == 3)
>>>> +#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
>>>> +					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
>>>
>>> I'd be happy if bit 2 in device id actually meant "port F", but I'm not
>>> so happy with coming up with rules like this for coincidental things.
>>
>> the port F thing is just that we have no name for that SKU :(
>> but from the spec organization this bit seems to represent that
>> group. Although I fully agree with you that this is horrible.
>>
>>>
>>> More generally, I'm not all that happy about *any* of the INTEL_DEVID
>>> uses in i915_drv.h because it spreads out the device id information, so
>>> I'd rather not add more. I'd rather move towards single point of truth
>>> for device ids.
>>
>> Yeah. I guess I agree with you.
>> There should be something inside the device info right?
>> even if we have to separate that in two groups.
> 
> That's the thing. It also worries me to add everything in device info,
> because it grows the device info structs for all devices, and you also
> need to add more device infos to cover all combinations. Unless you
> initialize them runtime, which is also a bit meh.
> 
>> If you agree with this approach I can follow up with a patch/series
>> that kill INTEL_DEVID in favor of device info structs.
>>
>> Or do you have any other idea for solving this?
> 
> Not really. Cc: Chris and Tvrtko who seem to come up with ideas for
> things like this on a regular basis. ;)

If I understood the gist, if single point of truth for device ids is the 
desire, that seems to imply there has to some place where flags are 
toggled at runtime.

Getting rid of INTEL_DEVID checks sprinkled around randomly, as courtesy 
of i915_drv.h "is-platform-variant" macros would be nice, I agree.

So perhaps a runtime pass which would set a new field in device_info, 
like INTEL_INFO()->platform_variant wouldn't be that bad. Device ID 
checks would then be moved from i915_drv.h into intel_device_info.c, and 
the i915_drv.h macros would become simple checks.

Wrt size consideration, I am not sure if it would be feasible to split 
struct intel_device_info in const friendly part and the rest - so that 
the instances in i915_pci.c could be smaller, and copied over to 
larger/complete version in struct drm_i915_private. On a quick glance it 
doesn't seem that there are many, if any, fields which are set at 
runtime only.

So no magical ideas from me I'm afraid.

Regards,

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

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

* [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
@ 2018-01-29 23:22 Rodrigo Vivi
  0 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-29 23:22 UTC (permalink / raw)
  To: intel-gfx
  Cc: Paulo Zanoni, Lucas De Marchi, Dhinakaran Pandiyan, Rodrigo Vivi

The only difference is that this SKUs has the full
Port A/E split named as Port F.

But since SKUs differences don't matter on the platform
definition group and ids, let's merge all off them together.

v2: Really include the PCI IDs to the picidlist[];
v3: Add the PCI Id for another SKU (Anusha).
v4: Update IDs, really include to pciidlists again.
v5: Unify all GT2 IDs.
v6: Unify in a way that we don't break early-quirks.c
v7: Remove GT reference since it doesn't matter here (Paulo)
    Also move IS_CNL_WITH_PORT_F macro to this patch to
    make it easier for review this part and also to get
    used sooner.
v8: Rebased on top of commit 5db47e37b387 ("Revert "drm/i915:
mark all device info struct with __initconst"")

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_pci.c |  5 ++---
 include/drm/i915_pciids.h       | 18 +++++++-----------
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 454d8f937fae..5702ebf17974 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2648,6 +2648,8 @@ intel_info(const struct drm_i915_private *dev_priv)
 				 (dev_priv)->info.gt == 2)
 #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
 				 (dev_priv)->info.gt == 3)
+#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
+					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
 
 #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
 
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 138228dd7782..4e7a10c89782 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -571,7 +571,7 @@ static const struct intel_device_info intel_coffeelake_gt3_info = {
 	.ddb_size = 1024, \
 	GLK_COLORS
 
-static const struct intel_device_info intel_cannonlake_gt2_info = {
+static const struct intel_device_info intel_cannonlake_info = {
 	GEN10_FEATURES,
 	.is_alpha_support = 1,
 	.platform = INTEL_CANNONLAKE,
@@ -649,8 +649,7 @@ static const struct pci_device_id pciidlist[] = {
 	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
 	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
 	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
-	INTEL_CNL_U_GT2_IDS(&intel_cannonlake_gt2_info),
-	INTEL_CNL_Y_GT2_IDS(&intel_cannonlake_gt2_info),
+	INTEL_CNL_IDS(&intel_cannonlake_info),
 	{0, 0, 0}
 };
 MODULE_DEVICE_TABLE(pci, pciidlist);
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 5db0458dd832..9e1fe6634424 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -414,24 +414,20 @@
 	INTEL_CFL_U_GT2_IDS(info), \
 	INTEL_CFL_U_GT3_IDS(info)
 
-/* CNL U 2+2 */
-#define INTEL_CNL_U_GT2_IDS(info) \
+/* CNL */
+#define INTEL_CNL_IDS(info) \
 	INTEL_VGA_DEVICE(0x5A52, info), \
 	INTEL_VGA_DEVICE(0x5A5A, info), \
 	INTEL_VGA_DEVICE(0x5A42, info), \
-	INTEL_VGA_DEVICE(0x5A4A, info)
-
-/* CNL Y 2+2 */
-#define INTEL_CNL_Y_GT2_IDS(info) \
+	INTEL_VGA_DEVICE(0x5A4A, info), \
 	INTEL_VGA_DEVICE(0x5A51, info), \
 	INTEL_VGA_DEVICE(0x5A59, info), \
 	INTEL_VGA_DEVICE(0x5A41, info), \
 	INTEL_VGA_DEVICE(0x5A49, info), \
 	INTEL_VGA_DEVICE(0x5A71, info), \
-	INTEL_VGA_DEVICE(0x5A79, info)
-
-#define INTEL_CNL_IDS(info) \
-	INTEL_CNL_U_GT2_IDS(info), \
-	INTEL_CNL_Y_GT2_IDS(info)
+	INTEL_VGA_DEVICE(0x5A79, info), \
+	INTEL_VGA_DEVICE(0x5A54, info), \
+	INTEL_VGA_DEVICE(0x5A5C, info), \
+	INTEL_VGA_DEVICE(0x5A44, info)
 
 #endif /* _I915_PCIIDS_H */
-- 
2.13.6

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

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

* [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
@ 2018-01-25 22:03 Rodrigo Vivi
  0 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-25 22:03 UTC (permalink / raw)
  To: intel-gfx
  Cc: Paulo Zanoni, Lucas De Marchi, Dhinakaran Pandiyan, Rodrigo Vivi

The only difference is that this SKUs has the full
Port A/E split named as Port F.

But since SKUs differences don't matter on the platform
definition group and ids, let's merge all off them together.

v2: Really include the PCI IDs to the picidlist[];
v3: Add the PCI Id for another SKU (Anusha).
v4: Update IDs, really include to pciidlists again.
v5: Unify all GT2 IDs.
v6: Unify in a way that we don't break early-quirks.c
v7: Remove GT reference since it doesn't matter here (Paulo)
    Also move IS_CNL_WITH_PORT_F macro to this patch to
    make it easier for review this part and also to get
    used sooner.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_pci.c |  5 ++---
 include/drm/i915_pciids.h       | 18 +++++++-----------
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 454d8f937fae..5702ebf17974 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2648,6 +2648,8 @@ intel_info(const struct drm_i915_private *dev_priv)
 				 (dev_priv)->info.gt == 2)
 #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
 				 (dev_priv)->info.gt == 3)
+#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
+					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
 
 #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
 
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index f28c165fc49d..7eb3d5e4350e 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -571,7 +571,7 @@ static const struct intel_device_info intel_coffeelake_gt3_info __initconst = {
 	.ddb_size = 1024, \
 	GLK_COLORS
 
-static const struct intel_device_info intel_cannonlake_gt2_info __initconst = {
+static const struct intel_device_info intel_cannonlake_info __initconst = {
 	GEN10_FEATURES,
 	.is_alpha_support = 1,
 	.platform = INTEL_CANNONLAKE,
@@ -649,8 +649,7 @@ static const struct pci_device_id pciidlist[] = {
 	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
 	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
 	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
-	INTEL_CNL_U_GT2_IDS(&intel_cannonlake_gt2_info),
-	INTEL_CNL_Y_GT2_IDS(&intel_cannonlake_gt2_info),
+	INTEL_CNL_IDS(&intel_cannonlake_info),
 	{0, 0, 0}
 };
 MODULE_DEVICE_TABLE(pci, pciidlist);
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 5db0458dd832..9e1fe6634424 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -414,24 +414,20 @@
 	INTEL_CFL_U_GT2_IDS(info), \
 	INTEL_CFL_U_GT3_IDS(info)
 
-/* CNL U 2+2 */
-#define INTEL_CNL_U_GT2_IDS(info) \
+/* CNL */
+#define INTEL_CNL_IDS(info) \
 	INTEL_VGA_DEVICE(0x5A52, info), \
 	INTEL_VGA_DEVICE(0x5A5A, info), \
 	INTEL_VGA_DEVICE(0x5A42, info), \
-	INTEL_VGA_DEVICE(0x5A4A, info)
-
-/* CNL Y 2+2 */
-#define INTEL_CNL_Y_GT2_IDS(info) \
+	INTEL_VGA_DEVICE(0x5A4A, info), \
 	INTEL_VGA_DEVICE(0x5A51, info), \
 	INTEL_VGA_DEVICE(0x5A59, info), \
 	INTEL_VGA_DEVICE(0x5A41, info), \
 	INTEL_VGA_DEVICE(0x5A49, info), \
 	INTEL_VGA_DEVICE(0x5A71, info), \
-	INTEL_VGA_DEVICE(0x5A79, info)
-
-#define INTEL_CNL_IDS(info) \
-	INTEL_CNL_U_GT2_IDS(info), \
-	INTEL_CNL_Y_GT2_IDS(info)
+	INTEL_VGA_DEVICE(0x5A79, info), \
+	INTEL_VGA_DEVICE(0x5A54, info), \
+	INTEL_VGA_DEVICE(0x5A5C, info), \
+	INTEL_VGA_DEVICE(0x5A44, info)
 
 #endif /* _I915_PCIIDS_H */
-- 
2.13.6

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

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

* Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
  2018-01-20  0:05 Rodrigo Vivi
  2018-01-22 16:56 ` Ville Syrjälä
@ 2018-01-25 17:40 ` Paulo Zanoni
  1 sibling, 0 replies; 26+ messages in thread
From: Paulo Zanoni @ 2018-01-25 17:40 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: Lucas De Marchi, Dhinakaran Pandiyan

Em Sex, 2018-01-19 às 16:05 -0800, Rodrigo Vivi escreveu:
> The only difference is that this SKUs has the full
> Port A/E split named as Port F.
> 
> But since SKUs differences don't matter on the platform
> definition group and ids, let's merge all off them together.
> 
> v2: Really include the PCI IDs to the picidlist[];
> v3: Add the PCI Id for another SKU (Anusha).
> v4: Update IDs, really include to pciidlists again.
> v5: Unify all GT2 IDs.
> v6: Unify in a way that we don't break early-quirks.c
> v7: Remove GT reference since it doesn't matter here (Paulo)
>     Also move IS_CNL_WITH_PORT_F macro to this patch to
>     make it easier for review this part and also to get
>     used sooner.
> 

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

(although I still would have preferred the split)

> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>  drivers/gpu/drm/i915/i915_pci.c |  5 ++---
>  include/drm/i915_pciids.h       | 18 +++++++-----------
>  3 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 8333692dac5a..3d3727829ac7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2647,6 +2647,8 @@ intel_info(const struct drm_i915_private
> *dev_priv)
>  				 (dev_priv)->info.gt == 2)
>  #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
>  				 (dev_priv)->info.gt == 3)
> +#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
> +					(INTEL_DEVID(dev_priv) &
> 0x0004) == 0x0004)
>  
>  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)-
> >is_alpha_support)
>  
> diff --git a/drivers/gpu/drm/i915/i915_pci.c
> b/drivers/gpu/drm/i915/i915_pci.c
> index f28c165fc49d..7eb3d5e4350e 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -571,7 +571,7 @@ static const struct intel_device_info
> intel_coffeelake_gt3_info __initconst = {
>  	.ddb_size = 1024, \
>  	GLK_COLORS
>  
> -static const struct intel_device_info intel_cannonlake_gt2_info
> __initconst = {
> +static const struct intel_device_info intel_cannonlake_info
> __initconst = {
>  	GEN10_FEATURES,
>  	.is_alpha_support = 1,
>  	.platform = INTEL_CANNONLAKE,
> @@ -649,8 +649,7 @@ static const struct pci_device_id pciidlist[] = {
>  	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
>  	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
>  	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
> -	INTEL_CNL_U_GT2_IDS(&intel_cannonlake_gt2_info),
> -	INTEL_CNL_Y_GT2_IDS(&intel_cannonlake_gt2_info),
> +	INTEL_CNL_IDS(&intel_cannonlake_info),
>  	{0, 0, 0}
>  };
>  MODULE_DEVICE_TABLE(pci, pciidlist);
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index 5db0458dd832..9e1fe6634424 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -414,24 +414,20 @@
>  	INTEL_CFL_U_GT2_IDS(info), \
>  	INTEL_CFL_U_GT3_IDS(info)
>  
> -/* CNL U 2+2 */
> -#define INTEL_CNL_U_GT2_IDS(info) \
> +/* CNL */
> +#define INTEL_CNL_IDS(info) \
>  	INTEL_VGA_DEVICE(0x5A52, info), \
>  	INTEL_VGA_DEVICE(0x5A5A, info), \
>  	INTEL_VGA_DEVICE(0x5A42, info), \
> -	INTEL_VGA_DEVICE(0x5A4A, info)
> -
> -/* CNL Y 2+2 */
> -#define INTEL_CNL_Y_GT2_IDS(info) \
> +	INTEL_VGA_DEVICE(0x5A4A, info), \
>  	INTEL_VGA_DEVICE(0x5A51, info), \
>  	INTEL_VGA_DEVICE(0x5A59, info), \
>  	INTEL_VGA_DEVICE(0x5A41, info), \
>  	INTEL_VGA_DEVICE(0x5A49, info), \
>  	INTEL_VGA_DEVICE(0x5A71, info), \
> -	INTEL_VGA_DEVICE(0x5A79, info)
> -
> -#define INTEL_CNL_IDS(info) \
> -	INTEL_CNL_U_GT2_IDS(info), \
> -	INTEL_CNL_Y_GT2_IDS(info)
> +	INTEL_VGA_DEVICE(0x5A79, info), \
> +	INTEL_VGA_DEVICE(0x5A54, info), \
> +	INTEL_VGA_DEVICE(0x5A5C, info), \
> +	INTEL_VGA_DEVICE(0x5A44, info)
>  
>  #endif /* _I915_PCIIDS_H */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
  2018-01-22 16:56 ` Ville Syrjälä
@ 2018-01-22 23:00   ` Rodrigo Vivi
  0 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-22 23:00 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, Lucas De Marchi, Paulo Zanoni, Dhinakaran Pandiyan

On Mon, Jan 22, 2018 at 04:56:10PM +0000, Ville Syrjälä wrote:
> On Fri, Jan 19, 2018 at 04:05:15PM -0800, Rodrigo Vivi wrote:
> > The only difference is that this SKUs has the full
> > Port A/E split named as Port F.
> > 
> > But since SKUs differences don't matter on the platform
> > definition group and ids, let's merge all off them together.
> > 
> > v2: Really include the PCI IDs to the picidlist[];
> > v3: Add the PCI Id for another SKU (Anusha).
> > v4: Update IDs, really include to pciidlists again.
> > v5: Unify all GT2 IDs.
> > v6: Unify in a way that we don't break early-quirks.c
> > v7: Remove GT reference since it doesn't matter here (Paulo)
> >     Also move IS_CNL_WITH_PORT_F macro to this patch to
> >     make it easier for review this part and also to get
> >     used sooner.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h |  2 ++
> >  drivers/gpu/drm/i915/i915_pci.c |  5 ++---
> >  include/drm/i915_pciids.h       | 18 +++++++-----------
> >  3 files changed, 11 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 8333692dac5a..3d3727829ac7 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2647,6 +2647,8 @@ intel_info(const struct drm_i915_private *dev_priv)
> >  				 (dev_priv)->info.gt == 2)
> >  #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
> >  				 (dev_priv)->info.gt == 3)
> > +#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
> > +					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
> 
> I wonder if we should generalize this sort of thing into some kind of
> device info port_mask. Though our port namespace currently only covers
> the various digital port type, so listing all possible ports there
> wouldn't currently be possible.

well... the right way actually would be having a proper name for this SKU,
but unfortunately we don't :/

> 
> Perhaps not worth the hassle right now.

yeap... I don't feel it is worth right now.

> 
> >  
> >  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> > index f28c165fc49d..7eb3d5e4350e 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -571,7 +571,7 @@ static const struct intel_device_info intel_coffeelake_gt3_info __initconst = {
> >  	.ddb_size = 1024, \
> >  	GLK_COLORS
> >  
> > -static const struct intel_device_info intel_cannonlake_gt2_info __initconst = {
> > +static const struct intel_device_info intel_cannonlake_info __initconst = {
> >  	GEN10_FEATURES,
> >  	.is_alpha_support = 1,
> >  	.platform = INTEL_CANNONLAKE,
> > @@ -649,8 +649,7 @@ static const struct pci_device_id pciidlist[] = {
> >  	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
> >  	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
> >  	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
> > -	INTEL_CNL_U_GT2_IDS(&intel_cannonlake_gt2_info),
> > -	INTEL_CNL_Y_GT2_IDS(&intel_cannonlake_gt2_info),
> > +	INTEL_CNL_IDS(&intel_cannonlake_info),
> >  	{0, 0, 0}
> >  };
> >  MODULE_DEVICE_TABLE(pci, pciidlist);
> > diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> > index 5db0458dd832..9e1fe6634424 100644
> > --- a/include/drm/i915_pciids.h
> > +++ b/include/drm/i915_pciids.h
> > @@ -414,24 +414,20 @@
> >  	INTEL_CFL_U_GT2_IDS(info), \
> >  	INTEL_CFL_U_GT3_IDS(info)
> >  
> > -/* CNL U 2+2 */
> > -#define INTEL_CNL_U_GT2_IDS(info) \
> > +/* CNL */
> > +#define INTEL_CNL_IDS(info) \
> >  	INTEL_VGA_DEVICE(0x5A52, info), \
> >  	INTEL_VGA_DEVICE(0x5A5A, info), \
> >  	INTEL_VGA_DEVICE(0x5A42, info), \
> > -	INTEL_VGA_DEVICE(0x5A4A, info)
> > -
> > -/* CNL Y 2+2 */
> > -#define INTEL_CNL_Y_GT2_IDS(info) \
> > +	INTEL_VGA_DEVICE(0x5A4A, info), \
> >  	INTEL_VGA_DEVICE(0x5A51, info), \
> >  	INTEL_VGA_DEVICE(0x5A59, info), \
> >  	INTEL_VGA_DEVICE(0x5A41, info), \
> >  	INTEL_VGA_DEVICE(0x5A49, info), \
> >  	INTEL_VGA_DEVICE(0x5A71, info), \
> > -	INTEL_VGA_DEVICE(0x5A79, info)
> > -
> > -#define INTEL_CNL_IDS(info) \
> > -	INTEL_CNL_U_GT2_IDS(info), \
> > -	INTEL_CNL_Y_GT2_IDS(info)
> > +	INTEL_VGA_DEVICE(0x5A79, info), \
> > +	INTEL_VGA_DEVICE(0x5A54, info), \
> > +	INTEL_VGA_DEVICE(0x5A5C, info), \
> > +	INTEL_VGA_DEVICE(0x5A44, info)
> >  
> >  #endif /* _I915_PCIIDS_H */
> > -- 
> > 2.13.6
> > 
> > _______________________________________________
> > 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

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

* Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
  2018-01-20  0:05 Rodrigo Vivi
@ 2018-01-22 16:56 ` Ville Syrjälä
  2018-01-22 23:00   ` Rodrigo Vivi
  2018-01-25 17:40 ` Paulo Zanoni
  1 sibling, 1 reply; 26+ messages in thread
From: Ville Syrjälä @ 2018-01-22 16:56 UTC (permalink / raw)
  To: Rodrigo Vivi
  Cc: intel-gfx, Lucas De Marchi, Paulo Zanoni, Dhinakaran Pandiyan

On Fri, Jan 19, 2018 at 04:05:15PM -0800, Rodrigo Vivi wrote:
> The only difference is that this SKUs has the full
> Port A/E split named as Port F.
> 
> But since SKUs differences don't matter on the platform
> definition group and ids, let's merge all off them together.
> 
> v2: Really include the PCI IDs to the picidlist[];
> v3: Add the PCI Id for another SKU (Anusha).
> v4: Update IDs, really include to pciidlists again.
> v5: Unify all GT2 IDs.
> v6: Unify in a way that we don't break early-quirks.c
> v7: Remove GT reference since it doesn't matter here (Paulo)
>     Also move IS_CNL_WITH_PORT_F macro to this patch to
>     make it easier for review this part and also to get
>     used sooner.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>  drivers/gpu/drm/i915/i915_pci.c |  5 ++---
>  include/drm/i915_pciids.h       | 18 +++++++-----------
>  3 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8333692dac5a..3d3727829ac7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2647,6 +2647,8 @@ intel_info(const struct drm_i915_private *dev_priv)
>  				 (dev_priv)->info.gt == 2)
>  #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
>  				 (dev_priv)->info.gt == 3)
> +#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
> +					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)

I wonder if we should generalize this sort of thing into some kind of
device info port_mask. Though our port namespace currently only covers
the various digital port type, so listing all possible ports there
wouldn't currently be possible.

Perhaps not worth the hassle right now.

>  
>  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
>  
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index f28c165fc49d..7eb3d5e4350e 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -571,7 +571,7 @@ static const struct intel_device_info intel_coffeelake_gt3_info __initconst = {
>  	.ddb_size = 1024, \
>  	GLK_COLORS
>  
> -static const struct intel_device_info intel_cannonlake_gt2_info __initconst = {
> +static const struct intel_device_info intel_cannonlake_info __initconst = {
>  	GEN10_FEATURES,
>  	.is_alpha_support = 1,
>  	.platform = INTEL_CANNONLAKE,
> @@ -649,8 +649,7 @@ static const struct pci_device_id pciidlist[] = {
>  	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
>  	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
>  	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
> -	INTEL_CNL_U_GT2_IDS(&intel_cannonlake_gt2_info),
> -	INTEL_CNL_Y_GT2_IDS(&intel_cannonlake_gt2_info),
> +	INTEL_CNL_IDS(&intel_cannonlake_info),
>  	{0, 0, 0}
>  };
>  MODULE_DEVICE_TABLE(pci, pciidlist);
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index 5db0458dd832..9e1fe6634424 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -414,24 +414,20 @@
>  	INTEL_CFL_U_GT2_IDS(info), \
>  	INTEL_CFL_U_GT3_IDS(info)
>  
> -/* CNL U 2+2 */
> -#define INTEL_CNL_U_GT2_IDS(info) \
> +/* CNL */
> +#define INTEL_CNL_IDS(info) \
>  	INTEL_VGA_DEVICE(0x5A52, info), \
>  	INTEL_VGA_DEVICE(0x5A5A, info), \
>  	INTEL_VGA_DEVICE(0x5A42, info), \
> -	INTEL_VGA_DEVICE(0x5A4A, info)
> -
> -/* CNL Y 2+2 */
> -#define INTEL_CNL_Y_GT2_IDS(info) \
> +	INTEL_VGA_DEVICE(0x5A4A, info), \
>  	INTEL_VGA_DEVICE(0x5A51, info), \
>  	INTEL_VGA_DEVICE(0x5A59, info), \
>  	INTEL_VGA_DEVICE(0x5A41, info), \
>  	INTEL_VGA_DEVICE(0x5A49, info), \
>  	INTEL_VGA_DEVICE(0x5A71, info), \
> -	INTEL_VGA_DEVICE(0x5A79, info)
> -
> -#define INTEL_CNL_IDS(info) \
> -	INTEL_CNL_U_GT2_IDS(info), \
> -	INTEL_CNL_Y_GT2_IDS(info)
> +	INTEL_VGA_DEVICE(0x5A79, info), \
> +	INTEL_VGA_DEVICE(0x5A54, info), \
> +	INTEL_VGA_DEVICE(0x5A5C, info), \
> +	INTEL_VGA_DEVICE(0x5A44, info)
>  
>  #endif /* _I915_PCIIDS_H */
> -- 
> 2.13.6
> 
> _______________________________________________
> 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

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

* [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
@ 2018-01-20  0:05 Rodrigo Vivi
  2018-01-22 16:56 ` Ville Syrjälä
  2018-01-25 17:40 ` Paulo Zanoni
  0 siblings, 2 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2018-01-20  0:05 UTC (permalink / raw)
  To: intel-gfx
  Cc: Paulo Zanoni, Lucas De Marchi, Dhinakaran Pandiyan, Rodrigo Vivi

The only difference is that this SKUs has the full
Port A/E split named as Port F.

But since SKUs differences don't matter on the platform
definition group and ids, let's merge all off them together.

v2: Really include the PCI IDs to the picidlist[];
v3: Add the PCI Id for another SKU (Anusha).
v4: Update IDs, really include to pciidlists again.
v5: Unify all GT2 IDs.
v6: Unify in a way that we don't break early-quirks.c
v7: Remove GT reference since it doesn't matter here (Paulo)
    Also move IS_CNL_WITH_PORT_F macro to this patch to
    make it easier for review this part and also to get
    used sooner.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_pci.c |  5 ++---
 include/drm/i915_pciids.h       | 18 +++++++-----------
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8333692dac5a..3d3727829ac7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2647,6 +2647,8 @@ intel_info(const struct drm_i915_private *dev_priv)
 				 (dev_priv)->info.gt == 2)
 #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
 				 (dev_priv)->info.gt == 3)
+#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
+					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
 
 #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
 
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index f28c165fc49d..7eb3d5e4350e 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -571,7 +571,7 @@ static const struct intel_device_info intel_coffeelake_gt3_info __initconst = {
 	.ddb_size = 1024, \
 	GLK_COLORS
 
-static const struct intel_device_info intel_cannonlake_gt2_info __initconst = {
+static const struct intel_device_info intel_cannonlake_info __initconst = {
 	GEN10_FEATURES,
 	.is_alpha_support = 1,
 	.platform = INTEL_CANNONLAKE,
@@ -649,8 +649,7 @@ static const struct pci_device_id pciidlist[] = {
 	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
 	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
 	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
-	INTEL_CNL_U_GT2_IDS(&intel_cannonlake_gt2_info),
-	INTEL_CNL_Y_GT2_IDS(&intel_cannonlake_gt2_info),
+	INTEL_CNL_IDS(&intel_cannonlake_info),
 	{0, 0, 0}
 };
 MODULE_DEVICE_TABLE(pci, pciidlist);
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 5db0458dd832..9e1fe6634424 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -414,24 +414,20 @@
 	INTEL_CFL_U_GT2_IDS(info), \
 	INTEL_CFL_U_GT3_IDS(info)
 
-/* CNL U 2+2 */
-#define INTEL_CNL_U_GT2_IDS(info) \
+/* CNL */
+#define INTEL_CNL_IDS(info) \
 	INTEL_VGA_DEVICE(0x5A52, info), \
 	INTEL_VGA_DEVICE(0x5A5A, info), \
 	INTEL_VGA_DEVICE(0x5A42, info), \
-	INTEL_VGA_DEVICE(0x5A4A, info)
-
-/* CNL Y 2+2 */
-#define INTEL_CNL_Y_GT2_IDS(info) \
+	INTEL_VGA_DEVICE(0x5A4A, info), \
 	INTEL_VGA_DEVICE(0x5A51, info), \
 	INTEL_VGA_DEVICE(0x5A59, info), \
 	INTEL_VGA_DEVICE(0x5A41, info), \
 	INTEL_VGA_DEVICE(0x5A49, info), \
 	INTEL_VGA_DEVICE(0x5A71, info), \
-	INTEL_VGA_DEVICE(0x5A79, info)
-
-#define INTEL_CNL_IDS(info) \
-	INTEL_CNL_U_GT2_IDS(info), \
-	INTEL_CNL_Y_GT2_IDS(info)
+	INTEL_VGA_DEVICE(0x5A79, info), \
+	INTEL_VGA_DEVICE(0x5A54, info), \
+	INTEL_VGA_DEVICE(0x5A5C, info), \
+	INTEL_VGA_DEVICE(0x5A44, info)
 
 #endif /* _I915_PCIIDS_H */
-- 
2.13.6

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

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

end of thread, other threads:[~2018-01-30 10:48 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-25 19:27 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
2018-01-25 19:27 ` [PATCH 02/10] drm/i915/cnl: Add AUX-F support Rodrigo Vivi
2018-01-25 19:27 ` [PATCH 03/10] drm/i915/cnl: Extend Wa 1178 to Aux F Rodrigo Vivi
2018-01-25 19:27 ` [PATCH 04/10] drm/i915/cnl: Fix _CNL_PORT_TX_DW2_LN0_F definition Rodrigo Vivi
2018-01-25 19:27 ` [PATCH 05/10] drm/i915: Fix DPLCLKA_CFGCR0 bits for Port F Rodrigo Vivi
2018-01-25 19:27 ` [PATCH 06/10] drm/i915/cnl: Add right GMBUS pin number for HDMI on " Rodrigo Vivi
2018-01-25 19:27 ` [PATCH 07/10] drm/i915: For HPD connected port use hpd_pin instead of port Rodrigo Vivi
2018-01-25 21:37   ` [PATCH] drm/i915/cnp: Properly handle VBT ddc pin out of bounds Rodrigo Vivi
2018-01-25 22:01     ` Rodrigo Vivi
2018-01-25 19:27 ` [PATCH 08/10] drm/i915/cnl: Add HPD support for Port F Rodrigo Vivi
2018-01-25 19:27 ` [PATCH 09/10] drm/i915/cnl: Enable DDI-F on Cannonlake Rodrigo Vivi
2018-01-25 19:27 ` [PATCH 10/10] drm/i915/cnl: Fix DP max rate for Cannonlake with port F Rodrigo Vivi
2018-01-25 19:35 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Patchwork
2018-01-25 21:51 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev2) Patchwork
2018-01-26 10:12 ` [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Jani Nikula
2018-01-26 17:26   ` Rodrigo Vivi
2018-01-27  9:05     ` Jani Nikula
2018-01-29 19:09       ` Pandiyan, Dhinakaran
2018-01-29 20:35         ` Jani Nikula
2018-01-30 10:48       ` Tvrtko Ursulin
  -- strict thread matches above, loose matches on Subject: below --
2018-01-29 23:22 Rodrigo Vivi
2018-01-25 22:03 Rodrigo Vivi
2018-01-20  0:05 Rodrigo Vivi
2018-01-22 16:56 ` Ville Syrjälä
2018-01-22 23:00   ` Rodrigo Vivi
2018-01-25 17:40 ` Paulo Zanoni

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.