[PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[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

[PATCH 02/10] drm/i915/cnl: Add AUX-F support

[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.
v11: Clean up squashed commit message. (David)
v12: Remove unnecessary handling for older platforms (DK)
     Adding AUX_F to PG2 following other existent ones. (DK)

Cc: David Weinehall <david.weinehall@linux.intel.com>
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>
Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@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         |  6 ++++++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 22 ++++++++++++++++++++++
 6 files changed, 45 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..934feccfecfe 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;
@@ -6224,6 +6227,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..a274e930f045 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:
@@ -1828,6 +1830,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 	BIT_ULL(POWER_DOMAIN_AUX_B) |                       \
 	BIT_ULL(POWER_DOMAIN_AUX_C) |			\
 	BIT_ULL(POWER_DOMAIN_AUX_D) |			\
+	BIT_ULL(POWER_DOMAIN_AUX_F) |			\
 	BIT_ULL(POWER_DOMAIN_AUDIO) |			\
 	BIT_ULL(POWER_DOMAIN_VGA) |				\
 	BIT_ULL(POWER_DOMAIN_INIT))
@@ -1855,6 +1858,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 +2411,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 +2532,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

[PATCH 03/10] drm/i915/cnl: Extend Wa 1178 to Aux F.

[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 a274e930f045..294b85adc413 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

[PATCH 04/10] drm/i915/cnl: Fix _CNL_PORT_TX_DW2_LN0_F definition.

[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

[PATCH 05/10] drm/i915: Fix DPLCLKA_CFGCR0 bits for Port F.

[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

[PATCH 06/10] drm/i915/cnl: Add right GMBUS pin number for HDMI on Port F.

[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 57066d6cf52f..80476bb14b58 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2175,6 +2175,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

[PATCH 07/10] drm/i915: For HPD connected port use hpd_pin instead of port.

[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 934feccfecfe..f9c90b79cbc4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4485,173 +4485,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;
 	}
 
@@ -4660,33 +4661,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 *
@@ -4745,8 +4746,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

[PATCH 08/10] drm/i915/cnl: Add HPD support for Port F.

[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 f9c90b79cbc4..ad055c527b1e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6207,8 +6207,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 80476bb14b58..f5d7bfb43006 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2331,7 +2331,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

[PATCH 09/10] drm/i915/cnl: Enable DDI-F on Cannonlake.

[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.
v8: Introduce DDI_F_LANES to PG2. (DK)
v9: Squash in the PORT_F case for enabling DP MST encoder. (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>
Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@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_dp.c         |  3 ++-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 20 +++++++++++++++++---
 6 files changed, 32 insertions(+), 5 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 67e0ec4fb5e3..cf42917725d0 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_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ad055c527b1e..86a5e8bfe2a6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6358,7 +6358,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 
 	/* init MST on ports that can support it */
 	if (HAS_DP_MST(dev_priv) && !intel_dp_is_edp(intel_dp) &&
-	    (port == PORT_B || port == PORT_C || port == PORT_D))
+	    (port == PORT_B || port == PORT_C ||
+	     port == PORT_D || port == PORT_F))
 		intel_dp_mst_encoder_init(intel_dig_port,
 					  intel_connector->base.base.id);
 
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 294b85adc413..70e659772a7a 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:
@@ -1827,6 +1831,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
 	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
 	BIT_ULL(POWER_DOMAIN_PORT_DDI_D_LANES) |		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_F_LANES) |		\
 	BIT_ULL(POWER_DOMAIN_AUX_B) |                       \
 	BIT_ULL(POWER_DOMAIN_AUX_C) |			\
 	BIT_ULL(POWER_DOMAIN_AUX_D) |			\
@@ -1861,6 +1866,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) |			\
@@ -2412,6 +2420,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,
@@ -2534,13 +2548,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

[PATCH 10/10] drm/i915/cnl: Fix DP max rate for Cannonlake with port F.

[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.
v5: Thanks CI! "error: control reaches end of non-void function"

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 | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 86a5e8bfe2a6..1f10bdb855e7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -220,15 +220,36 @@ 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;
+
+	return size;
+}
+
 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 +259,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

✓ Fi.CI.BAT: success for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Patchwork]

== Series Details ==

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

== Summary ==

Series 37309v1 series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
https://patchwork.freedesktop.org/api/1.0/series/37309/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989
                incomplete -> PASS       (fi-snb-2520m) fdo#103713
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test gem_ringfill:
        Subgroup basic-default-hang:
                dmesg-warn -> PASS       (fi-pnv-d510) fdo#101600

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:419s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:423s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:493s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:482s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:468s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:453s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:573s
fi-elk-e7500     total:224  pass:168  dwarn:9   dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:277s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:513s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:393s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:400s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:411s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:414s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:465s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:495s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:455s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:502s
fi-pnv-d510      total:288  pass:223  dwarn:0   dfail:0   fail:0   skip:65  time:560s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:422s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:511s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:529s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:491s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:484s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:418s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:431s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:528s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:393s
Blacklisted hosts:
fi-cnl-y2        total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:536s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:477s

9afefca72cf89d53e114f3974c5afea1d663d3e1 drm-tip: 2018y-01m-29d-22h-12m-08s UTC integration manifest
14d26d862f59 drm/i915/cnl: Fix DP max rate for Cannonlake with port F.
3e78baeb28d4 drm/i915/cnl: Enable DDI-F on Cannonlake.
93e8c1123894 drm/i915/cnl: Add HPD support for Port F.
c9ec4be24ee6 drm/i915: For HPD connected port use hpd_pin instead of port.
c7082bdab406 drm/i915/cnl: Add right GMBUS pin number for HDMI on Port F.
547dfae7a658 drm/i915: Fix DPLCLKA_CFGCR0 bits for Port F.
8565802eb87a drm/i915/cnl: Fix _CNL_PORT_TX_DW2_LN0_F definition.
e3fc834de5e7 drm/i915/cnl: Extend Wa 1178 to Aux F.
f867fb784b0f drm/i915/cnl: Add AUX-F support
41224a5eec38 drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

== Logs ==

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

✗ Fi.CI.IGT: warning for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Patchwork]

== Series Details ==

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

== Summary ==

Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-pri-shrfb-draw-render:
                pass       -> SKIP       (shard-snb) fdo#103167
        Subgroup fbc-rgb101010-draw-mmap-gtt:
                pass       -> SKIP       (shard-snb)
Test kms_busy:
        Subgroup extended-pageflip-hang-newfb-render-a:
                pass       -> SKIP       (shard-snb)
Test kms_rotation_crc:
        Subgroup sprite-rotation-90-flip:
                fail       -> PASS       (shard-apl) fdo#103356
Test drv_selftest:
        Subgroup live_gtt:
                pass       -> INCOMPLETE (shard-apl) fdo#103927
Test kms_cursor_legacy:
        Subgroup short-flip-before-cursor-atomic-transitions:
                pass       -> FAIL       (shard-apl) fdo#103792
Test kms_cursor_crc:
        Subgroup cursor-128x128-suspend:
                skip       -> PASS       (shard-snb) fdo#103880
Test kms_flip:
        Subgroup 2x-plain-flip-fb-recreate:
                pass       -> FAIL       (shard-hsw) fdo#100368
Test perf_pmu:
        Subgroup frequency:
                fail       -> PASS       (shard-apl) fdo#104829
Test perf:
        Subgroup enable-disable:
                pass       -> FAIL       (shard-apl) fdo#103715
Test kms_vblank:
        Subgroup pipe-b-ts-continuation-suspend:
                fail       -> PASS       (shard-snb)

fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103356 https://bugs.freedesktop.org/show_bug.cgi?id=103356
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#103792 https://bugs.freedesktop.org/show_bug.cgi?id=103792
fdo#103880 https://bugs.freedesktop.org/show_bug.cgi?id=103880
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#104829 https://bugs.freedesktop.org/show_bug.cgi?id=104829
fdo#103715 https://bugs.freedesktop.org/show_bug.cgi?id=103715

shard-apl        total:2816 pass:1726 dwarn:1   dfail:0   fail:24  skip:1064 time:12295s
shard-hsw        total:2838 pass:1733 dwarn:1   dfail:0   fail:13  skip:1090 time:12024s
shard-snb        total:2838 pass:1325 dwarn:1   dfail:0   fail:12  skip:1500 time:6624s
Blacklisted hosts:
shard-kbl        total:2838 pass:1818 dwarn:53  dfail:0   fail:25  skip:942 time:9488s

== Logs ==

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

Re: [PATCH 10/10] drm/i915/cnl: Fix DP max rate for Cannonlake with port F.

[Jani Nikula]

On Mon, 29 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> 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.
> v5: Thanks CI! "error: control reaches end of non-void function"
>
> 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 | 31 ++++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 86a5e8bfe2a6..1f10bdb855e7 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -220,15 +220,36 @@ 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;
> +
> +	return size;

IMO this splits the ARRAY_SIZE() and the (size - 2) adjustments too
much. They were tolerable within one function, but looking at this
function alone, the (size - 2) is a big WTF.

I'd just put this all in the same function.

BR,
Jani.


> +}
> +
>  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 +259,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);

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

✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Patchwork]

== Series Details ==

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

== Summary ==

Applying: drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_drv.h
M	drivers/gpu/drm/i915/i915_pci.c
M	include/drm/i915_pciids.h
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: drm/i915/cnl: Add AUX-F support
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/i915_irq.c).
error: could not build fake ancestor
Patch failed at 0002 drm/i915/cnl: Add AUX-F support
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

== Logs ==

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

Re: ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Rodrigo Vivi]

On Tue, Jan 30, 2018 at 08:05:41PM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
> URL   : https://patchwork.freedesktop.org/series/37309/
> State : failure
> 
> == Summary ==
> 
> Applying: drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.
> Using index info to reconstruct a base tree...
> M	drivers/gpu/drm/i915/i915_drv.h
> M	drivers/gpu/drm/i915/i915_pci.c
> M	include/drm/i915_pciids.h
> Falling back to patching base and 3-way merge...
> No changes -- Patch already applied.
> Applying: drm/i915/cnl: Add AUX-F support
> error: sha1 information is lacking or useless (drivers/gpu/drm/i915/i915_irq.c).
> error: could not build fake ancestor
> Patch failed at 0002 drm/i915/cnl: Add AUX-F support
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7812/issues.html

Please disregard this.
test re-trigger actually came after I had merged the series.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/10] drm/i915/cnl: Fix DP max rate for Cannonlake with port F.

[Rodrigo Vivi]

On Tue, Jan 30, 2018 at 07:45:03AM +0000, Jani Nikula wrote:
> On Mon, 29 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > 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.
> > v5: Thanks CI! "error: control reaches end of non-void function"
> >
> > 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 | 31 ++++++++++++++++++++++++-------
> >  1 file changed, 24 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 86a5e8bfe2a6..1f10bdb855e7 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -220,15 +220,36 @@ 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;
> > +
> > +	return size;
> 

ops, I had missed this email. Since I had resent the series, the old one
was on top of my inbox.

> IMO this splits the ARRAY_SIZE() and the (size - 2) adjustments too
> much. They were tolerable within one function, but looking at this
> function alone, the (size - 2) is a big WTF.
> 
> I'd just put this all in the same function.

I just split per Ville request to make conditions more readable.
I now agree that size outside of the context get uglier.

What about changing:

int num_source_rates
const int *source_rates

into:
struct {
int num;
const int *list;
int max_available;
} source_rates;

So that function or whenever we need like reading from new VBT field
we set a max_available, and when going through the list for finding
the common rate instead of relying only on num we also check max_available?

Agree?
Thoughts?

> 
> BR,
> Jani.
> 
> 
> > +}
> > +
> >  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 +259,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);
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/10] drm/i915/cnl: Fix DP max rate for Cannonlake with port F.

[Ville Syrjälä]

On Tue, Jan 30, 2018 at 12:42:12PM -0800, Rodrigo Vivi wrote:
> On Tue, Jan 30, 2018 at 07:45:03AM +0000, Jani Nikula wrote:
> > On Mon, 29 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > 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.
> > > v5: Thanks CI! "error: control reaches end of non-void function"
> > >
> > > 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 | 31 ++++++++++++++++++++++++-------
> > >  1 file changed, 24 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 86a5e8bfe2a6..1f10bdb855e7 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -220,15 +220,36 @@ 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;
> > > +
> > > +	return size;
> > 
> 
> ops, I had missed this email. Since I had resent the series, the old one
> was on top of my inbox.
> 
> > IMO this splits the ARRAY_SIZE() and the (size - 2) adjustments too
> > much. They were tolerable within one function, but looking at this
> > function alone, the (size - 2) is a big WTF.
> > 
> > I'd just put this all in the same function.
> 
> I just split per Ville request to make conditions more readable.
> I now agree that size outside of the context get uglier.
> 
> What about changing:
> 
> int num_source_rates
> const int *source_rates
> 
> into:
> struct {
> int num;
> const int *list;
> int max_available;
> } source_rates;
> 
> So that function or whenever we need like reading from new VBT field
> we set a max_available, and when going through the list for finding
> the common rate instead of relying only on num we also check max_available?
> 
> Agree?
> Thoughts?

I think the obvious solution is to just make this function
return both the array and its size. Not sure there's much point
in complicating it more than that.

> 
> > 
> > BR,
> > Jani.
> > 
> > 
> > > +}
> > > +
> > >  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 +259,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);
> > 
> > -- 
> > Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 10/10] drm/i915/cnl: Fix DP max rate for Cannonlake with port F.

[Rodrigo Vivi]

On Tue, Jan 30, 2018 at 08:46:49PM +0000, Ville Syrjälä wrote:
> On Tue, Jan 30, 2018 at 12:42:12PM -0800, Rodrigo Vivi wrote:
> > On Tue, Jan 30, 2018 at 07:45:03AM +0000, Jani Nikula wrote:
> > > On Mon, 29 Jan 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > > 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.
> > > > v5: Thanks CI! "error: control reaches end of non-void function"
> > > >
> > > > 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 | 31 ++++++++++++++++++++++++-------
> > > >  1 file changed, 24 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 86a5e8bfe2a6..1f10bdb855e7 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -220,15 +220,36 @@ 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;
> > > > +
> > > > +	return size;
> > > 
> > 
> > ops, I had missed this email. Since I had resent the series, the old one
> > was on top of my inbox.
> > 
> > > IMO this splits the ARRAY_SIZE() and the (size - 2) adjustments too
> > > much. They were tolerable within one function, but looking at this
> > > function alone, the (size - 2) is a big WTF.
> > > 
> > > I'd just put this all in the same function.
> > 
> > I just split per Ville request to make conditions more readable.
> > I now agree that size outside of the context get uglier.
> > 
> > What about changing:
> > 
> > int num_source_rates
> > const int *source_rates
> > 
> > into:
> > struct {
> > int num;
> > const int *list;
> > int max_available;
> > } source_rates;
> > 
> > So that function or whenever we need like reading from new VBT field
> > we set a max_available, and when going through the list for finding
> > the common rate instead of relying only on num we also check max_available?
> > 
> > Agree?
> > Thoughts?
> 
> I think the obvious solution is to just make this function
> return both the array and its size. Not sure there's much point
> in complicating it more than that.

fair enough. Just to confirm this simple solution you have in mind is this right:
https://pastebin.com/NcsZcfR1
?

But the point in the complicated one is to also address at the same time
the new vbt field:

u8 dp_max_link_rate:2;                                  /* 216 CNL+ */

so we would keep all the adjustments, including the VBT in a single place.

> 
> > 
> > > 
> > > BR,
> > > Jani.
> > > 
> > > 
> > > > +}
> > > > +
> > > >  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 +259,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);
> > > 
> > > -- 
> > > Jani Nikula, Intel Open Source Technology Center
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Tvrtko Ursulin]

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

Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Jani Nikula]

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

Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Pandiyan, Dhinakaran]

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

Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Jani Nikula]

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

Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Rodrigo Vivi]

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

Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Jani Nikula]

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

[PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[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

[PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[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

Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Paulo Zanoni]

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

Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Rodrigo Vivi]

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

Re: [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[Ville Syrjälä]

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

[PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU.

[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