[PATCH 0/5] drm/i915: gmbus pin/port cleanup and bxt enabling

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/5] drm/i915: gmbus pin/port cleanup and bxt enabling
@ 2015-03-26 22:20 Jani Nikula
  2015-03-26 22:20 ` [PATCH 1/5] drm/i915: rename GMBUS_PORT_* macros as GMBUS_PIN_* Jani Nikula
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Jani Nikula @ 2015-03-26 22:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Hi all, Imre in particular -

This series spun out of reviewing the bxt gmbus enabling patches [1],
[2] and [3] that were getting, at least for my poor brain, hard to
follow.

The first four are platform independent cleanup and prep work that could
be merged before any of the bxt enabling series. It's all pretty
straightforward too.

The last patch enables gmbus on bxt.

Together this series replaces the three referenced patches.

As a follow-up, it's fairly easy to e.g. add the correct names for the
pins for CHV, and drop legacy pins (ssc, lvds) for hsw+ that we still
register adapters for! I just kept this series small for starters.

BR,
Jani.


[1] http://mid.gmane.org/1426585215-8788-21-git-send-email-imre.deak@intel.com
[2] http://mid.gmane.org/1426585215-8788-22-git-send-email-imre.deak@intel.com
[3] http://mid.gmane.org/1426585215-8788-23-git-send-email-imre.deak@intel.com



A.Sunil Kamath (1):
  drm/i915: add bxt gmbus support

Jani Nikula (4):
  drm/i915: rename GMBUS_PORT_* macros as GMBUS_PIN_*
  drm/i915: refer to pin instead of port in the intel_i2c.c interfaces
  drm/i915: index gmbus tables using the pin pair number
  drm/i915: base gmbus pin validity check on the gmbus pin map array

 drivers/gpu/drm/i915/i915_drv.h   | 13 ++----
 drivers/gpu/drm/i915/i915_reg.h   | 23 ++++++----
 drivers/gpu/drm/i915/intel_bios.c |  4 +-
 drivers/gpu/drm/i915/intel_crt.c  |  2 +-
 drivers/gpu/drm/i915/intel_dvo.c  | 10 ++--
 drivers/gpu/drm/i915/intel_hdmi.c |  8 ++--
 drivers/gpu/drm/i915/intel_i2c.c  | 96 ++++++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_lvds.c |  4 +-
 drivers/gpu/drm/i915/intel_sdvo.c |  5 +-
 9 files changed, 100 insertions(+), 65 deletions(-)

-- 
2.1.4

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

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

* [PATCH 1/5] drm/i915: rename GMBUS_PORT_* macros as GMBUS_PIN_*
  2015-03-26 22:20 [PATCH 0/5] drm/i915: gmbus pin/port cleanup and bxt enabling Jani Nikula
@ 2015-03-26 22:20 ` Jani Nikula
  2015-03-26 22:20 ` [PATCH 2/5] drm/i915: refer to pin instead of port in the intel_i2c.c interfaces Jani Nikula
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2015-03-26 22:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

The specs refer to pin pairs. Start moving towards using pin rather than
port all around to avoid confusion. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 +-
 drivers/gpu/drm/i915/i915_reg.h   | 20 ++++++++++----------
 drivers/gpu/drm/i915/intel_bios.c |  2 +-
 drivers/gpu/drm/i915/intel_crt.c  |  2 +-
 drivers/gpu/drm/i915/intel_dvo.c  |  8 ++++----
 drivers/gpu/drm/i915/intel_hdmi.c |  8 ++++----
 drivers/gpu/drm/i915/intel_lvds.c |  2 +-
 drivers/gpu/drm/i915/intel_sdvo.c |  2 +-
 8 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 256369f29975..a1d9d302278f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3051,7 +3051,7 @@ extern int intel_setup_gmbus(struct drm_device *dev);
 extern void intel_teardown_gmbus(struct drm_device *dev);
 static inline bool intel_gmbus_is_port_valid(unsigned port)
 {
-	return (port >= GMBUS_PORT_SSC && port <= GMBUS_PORT_DPD);
+	return (port >= GMBUS_PIN_SSC && port <= GMBUS_PIN_DPD);
 }
 
 extern struct i2c_adapter *intel_gmbus_get_adapter(
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b522eb6e59a4..b6113c9a803b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1788,16 +1788,16 @@ enum skl_disp_power_wells {
 #define   GMBUS_RATE_400KHZ	(2<<8) /* reserved on Pineview */
 #define   GMBUS_RATE_1MHZ	(3<<8) /* reserved on Pineview */
 #define   GMBUS_HOLD_EXT	(1<<7) /* 300ns hold time, rsvd on Pineview */
-#define   GMBUS_PORT_DISABLED	0
-#define   GMBUS_PORT_SSC	1
-#define   GMBUS_PORT_VGADDC	2
-#define   GMBUS_PORT_PANEL	3
-#define   GMBUS_PORT_DPD_CHV	3 /* HDMID_CHV */
-#define   GMBUS_PORT_DPC	4 /* HDMIC */
-#define   GMBUS_PORT_DPB	5 /* SDVO, HDMIB */
-#define   GMBUS_PORT_DPD	6 /* HDMID */
-#define   GMBUS_PORT_RESERVED	7 /* 7 reserved */
-#define   GMBUS_NUM_PORTS	(GMBUS_PORT_DPD - GMBUS_PORT_SSC + 1)
+#define   GMBUS_PIN_DISABLED	0
+#define   GMBUS_PIN_SSC		1
+#define   GMBUS_PIN_VGADDC	2
+#define   GMBUS_PIN_PANEL	3
+#define   GMBUS_PIN_DPD_CHV	3 /* HDMID_CHV */
+#define   GMBUS_PIN_DPC		4 /* HDMIC */
+#define   GMBUS_PIN_DPB		5 /* SDVO, HDMIB */
+#define   GMBUS_PIN_DPD		6 /* HDMID */
+#define   GMBUS_PIN_RESERVED	7 /* 7 reserved */
+#define   GMBUS_NUM_PORTS	(GMBUS_PIN_DPD - GMBUS_PIN_SSC + 1)
 #define GMBUS1			0x5104 /* command/status */
 #define   GMBUS_SW_CLR_INT	(1<<31)
 #define   GMBUS_SW_RDY		(1<<30)
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index c684085cb56a..40c8375a4477 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1133,7 +1133,7 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
 	struct drm_device *dev = dev_priv->dev;
 	enum port port;
 
-	dev_priv->vbt.crt_ddc_pin = GMBUS_PORT_VGADDC;
+	dev_priv->vbt.crt_ddc_pin = GMBUS_PIN_VGADDC;
 
 	/* Default to having backlight */
 	dev_priv->vbt.backlight.present = true;
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 573aaff3389a..61a174db8a89 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -745,7 +745,7 @@ static int intel_crt_get_modes(struct drm_connector *connector)
 		goto out;
 
 	/* Try to probe digital port for output in DVI-I -> VGA mode. */
-	i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PORT_DPB);
+	i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PIN_DPB);
 	ret = intel_crt_ddc_get_modes(connector, i2c);
 
 out:
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 4ccd6c3f133d..fe13bbbf301c 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -80,7 +80,7 @@ static const struct intel_dvo_device intel_dvo_devices[] = {
 		.name = "ch7017",
 		.dvo_reg = DVOC,
 		.slave_addr = 0x75,
-		.gpio = GMBUS_PORT_DPB,
+		.gpio = GMBUS_PIN_DPB,
 		.dev_ops = &ch7017_ops,
 	},
 	{
@@ -364,7 +364,7 @@ static int intel_dvo_get_modes(struct drm_connector *connector)
 	 * that's not the case.
 	 */
 	intel_ddc_get_modes(connector,
-			    intel_gmbus_get_adapter(dev_priv, GMBUS_PORT_DPC));
+			    intel_gmbus_get_adapter(dev_priv, GMBUS_PIN_DPC));
 	if (!list_empty(&connector->probed_modes))
 		return 1;
 
@@ -503,9 +503,9 @@ void intel_dvo_init(struct drm_device *dev)
 		if (intel_gmbus_is_port_valid(dvo->gpio))
 			gpio = dvo->gpio;
 		else if (dvo->type == INTEL_DVO_CHIP_LVDS)
-			gpio = GMBUS_PORT_SSC;
+			gpio = GMBUS_PIN_SSC;
 		else
-			gpio = GMBUS_PORT_DPB;
+			gpio = GMBUS_PIN_DPB;
 
 		/* Set up the I2C bus necessary for the chip we're probing.
 		 * It appears that everything is on GPIOE except for panels
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index cacbafdad3ab..26222e6c1ff3 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1681,18 +1681,18 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 
 	switch (port) {
 	case PORT_B:
-		intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
+		intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
 		intel_encoder->hpd_pin = HPD_PORT_B;
 		break;
 	case PORT_C:
-		intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
+		intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
 		intel_encoder->hpd_pin = HPD_PORT_C;
 		break;
 	case PORT_D:
 		if (IS_CHERRYVIEW(dev))
-			intel_hdmi->ddc_bus = GMBUS_PORT_DPD_CHV;
+			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
 		else
-			intel_hdmi->ddc_bus = GMBUS_PORT_DPD;
+			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
 		intel_encoder->hpd_pin = HPD_PORT_D;
 		break;
 	case PORT_A:
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 06d2da336f7c..875916152645 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -921,7 +921,7 @@ void intel_lvds_init(struct drm_device *dev)
 	if (dmi_check_system(intel_no_lvds))
 		return;
 
-	pin = GMBUS_PORT_PANEL;
+	pin = GMBUS_PIN_PANEL;
 	if (!lvds_is_present_in_vbt(dev, &pin)) {
 		DRM_DEBUG_KMS("LVDS is not present in VBT\n");
 		return;
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index f5b7e1e7c5e0..3e0e9b0e3503 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2294,7 +2294,7 @@ intel_sdvo_select_i2c_bus(struct drm_i915_private *dev_priv,
 	if (mapping->initialized && intel_gmbus_is_port_valid(mapping->i2c_pin))
 		pin = mapping->i2c_pin;
 	else
-		pin = GMBUS_PORT_DPB;
+		pin = GMBUS_PIN_DPB;
 
 	sdvo->i2c = intel_gmbus_get_adapter(dev_priv, pin);
 
-- 
2.1.4

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

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

* [PATCH 2/5] drm/i915: refer to pin instead of port in the intel_i2c.c interfaces
  2015-03-26 22:20 [PATCH 0/5] drm/i915: gmbus pin/port cleanup and bxt enabling Jani Nikula
  2015-03-26 22:20 ` [PATCH 1/5] drm/i915: rename GMBUS_PORT_* macros as GMBUS_PIN_* Jani Nikula
@ 2015-03-26 22:20 ` Jani Nikula
  2015-03-26 22:20 ` [PATCH 3/5] drm/i915: index gmbus tables using the pin pair number Jani Nikula
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2015-03-26 22:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Rename intel_gmbus_is_port_valid to intel_gmbus_is_valid_pin, and rename
port parameters to pin as well. This matches usage all around, as
usually a pin is passed to the validity check function. No functional
changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   | 8 ++++----
 drivers/gpu/drm/i915/intel_bios.c | 2 +-
 drivers/gpu/drm/i915/intel_dvo.c  | 2 +-
 drivers/gpu/drm/i915/intel_i2c.c  | 8 ++++----
 drivers/gpu/drm/i915/intel_lvds.c | 2 +-
 drivers/gpu/drm/i915/intel_sdvo.c | 2 +-
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a1d9d302278f..3ba5de19e039 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3049,13 +3049,13 @@ void i915_teardown_sysfs(struct drm_device *dev_priv);
 /* intel_i2c.c */
 extern int intel_setup_gmbus(struct drm_device *dev);
 extern void intel_teardown_gmbus(struct drm_device *dev);
-static inline bool intel_gmbus_is_port_valid(unsigned port)
+static inline bool intel_gmbus_is_valid_pin(unsigned int pin)
 {
-	return (port >= GMBUS_PIN_SSC && port <= GMBUS_PIN_DPD);
+	return (pin >= GMBUS_PIN_SSC && pin <= GMBUS_PIN_DPD);
 }
 
-extern struct i2c_adapter *intel_gmbus_get_adapter(
-		struct drm_i915_private *dev_priv, unsigned port);
+extern struct i2c_adapter *
+intel_gmbus_get_adapter(struct drm_i915_private *dev_priv, unsigned int pin);
 extern void intel_gmbus_set_speed(struct i2c_adapter *adapter, int speed);
 extern void intel_gmbus_force_bit(struct i2c_adapter *adapter, bool force_bit);
 static inline bool intel_gmbus_is_forced_bit(struct i2c_adapter *adapter)
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 40c8375a4477..333f40793435 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -438,7 +438,7 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
 		if (block_size >= sizeof(*general)) {
 			int bus_pin = general->crt_ddc_gmbus_pin;
 			DRM_DEBUG_KMS("crt_ddc_bus_pin: %d\n", bus_pin);
-			if (intel_gmbus_is_port_valid(bus_pin))
+			if (intel_gmbus_is_valid_pin(bus_pin))
 				dev_priv->vbt.crt_ddc_pin = bus_pin;
 		} else {
 			DRM_DEBUG_KMS("BDB_GD too small (%d). Invalid.\n",
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index fe13bbbf301c..8d62272a3421 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -500,7 +500,7 @@ void intel_dvo_init(struct drm_device *dev)
 		 * special cases, but otherwise default to what's defined
 		 * in the spec.
 		 */
-		if (intel_gmbus_is_port_valid(dvo->gpio))
+		if (intel_gmbus_is_valid_pin(dvo->gpio))
 			gpio = dvo->gpio;
 		else if (dvo->type == INTEL_DVO_CHIP_LVDS)
 			gpio = GMBUS_PIN_SSC;
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index b31088a551f2..b0003a2bd854 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -574,12 +574,12 @@ err:
 }
 
 struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv,
-					    unsigned port)
+					    unsigned int pin)
 {
-	WARN_ON(!intel_gmbus_is_port_valid(port));
+	WARN_ON(!intel_gmbus_is_valid_pin(pin));
 	/* -1 to map pin pair to gmbus index */
-	return (intel_gmbus_is_port_valid(port)) ?
-		&dev_priv->gmbus[port - 1].adapter : NULL;
+	return (intel_gmbus_is_valid_pin(pin)) ?
+		&dev_priv->gmbus[pin - 1].adapter : NULL;
 }
 
 void intel_gmbus_set_speed(struct i2c_adapter *adapter, int speed)
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 875916152645..d61aa78ed7e3 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -781,7 +781,7 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev,
 		    child->device_type != DEVICE_TYPE_LFP)
 			continue;
 
-		if (intel_gmbus_is_port_valid(child->i2c_pin))
+		if (intel_gmbus_is_valid_pin(child->i2c_pin))
 			*i2c_pin = child->i2c_pin;
 
 		/* However, we cannot trust the BIOS writers to populate
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 3e0e9b0e3503..124992e48abd 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2291,7 +2291,7 @@ intel_sdvo_select_i2c_bus(struct drm_i915_private *dev_priv,
 	else
 		mapping = &dev_priv->sdvo_mappings[1];
 
-	if (mapping->initialized && intel_gmbus_is_port_valid(mapping->i2c_pin))
+	if (mapping->initialized && intel_gmbus_is_valid_pin(mapping->i2c_pin))
 		pin = mapping->i2c_pin;
 	else
 		pin = GMBUS_PIN_DPB;
-- 
2.1.4

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

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

* [PATCH 3/5] drm/i915: index gmbus tables using the pin pair number
  2015-03-26 22:20 [PATCH 0/5] drm/i915: gmbus pin/port cleanup and bxt enabling Jani Nikula
  2015-03-26 22:20 ` [PATCH 1/5] drm/i915: rename GMBUS_PORT_* macros as GMBUS_PIN_* Jani Nikula
  2015-03-26 22:20 ` [PATCH 2/5] drm/i915: refer to pin instead of port in the intel_i2c.c interfaces Jani Nikula
@ 2015-03-26 22:20 ` Jani Nikula
  2015-03-27 15:00   ` Ville Syrjälä
  2015-03-26 22:20 ` [PATCH 4/5] drm/i915: base gmbus pin validity check on the gmbus pin map array Jani Nikula
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Jani Nikula @ 2015-03-26 22:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Index the gmbus tables directly using the pin instead of having a
confusing "port = i + 1" mapping. This finishes off removing the "gmbus
port" as a notion, and leaves us with just the "gmbus pin".

As pin 0 is invalid by definition and the gmbus tables will have a gap
at that index, add pin validity check to all the loops. This will be
benefitial for supporting platforms that have different numbers of pins,
or gaps.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  3 +-
 drivers/gpu/drm/i915/i915_reg.h  |  2 +-
 drivers/gpu/drm/i915/intel_i2c.c | 65 +++++++++++++++++++++++-----------------
 3 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3ba5de19e039..0c6024101eb9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1574,8 +1574,7 @@ struct drm_i915_private {
 
 	struct i915_virtual_gpu vgpu;
 
-	struct intel_gmbus gmbus[GMBUS_NUM_PORTS];
-
+	struct intel_gmbus gmbus[GMBUS_PIN_MAX];
 
 	/** gmbus_mutex protects against concurrent usage of the single hw gmbus
 	 * controller on different i2c buses. */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b6113c9a803b..cdc071cff001 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1797,7 +1797,7 @@ enum skl_disp_power_wells {
 #define   GMBUS_PIN_DPB		5 /* SDVO, HDMIB */
 #define   GMBUS_PIN_DPD		6 /* HDMID */
 #define   GMBUS_PIN_RESERVED	7 /* 7 reserved */
-#define   GMBUS_NUM_PORTS	(GMBUS_PIN_DPD - GMBUS_PIN_SSC + 1)
+#define   GMBUS_PIN_MAX		7 /* not inclusive */
 #define GMBUS1			0x5104 /* command/status */
 #define   GMBUS_SW_CLR_INT	(1<<31)
 #define   GMBUS_SW_RDY		(1<<30)
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index b0003a2bd854..ff47a8fdcb6d 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -34,18 +34,19 @@
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 
-struct gmbus_port {
+struct gmbus_pin {
 	const char *name;
 	int reg;
 };
 
-static const struct gmbus_port gmbus_ports[] = {
-	{ "ssc", GPIOB },
-	{ "vga", GPIOA },
-	{ "panel", GPIOC },
-	{ "dpc", GPIOD },
-	{ "dpb", GPIOE },
-	{ "dpd", GPIOF },
+/* Map gmbus pin pairs to names and registers. */
+static const struct gmbus_pin gmbus_pins[] = {
+	[GMBUS_PIN_SSC] = { "ssc", GPIOB },
+	[GMBUS_PIN_VGADDC] = { "vga", GPIOA },
+	[GMBUS_PIN_PANEL] = { "panel", GPIOC },
+	[GMBUS_PIN_DPC] = { "dpc", GPIOD },
+	[GMBUS_PIN_DPB] = { "dpb", GPIOE },
+	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
 };
 
 /* Intel GPIO access functions */
@@ -182,15 +183,14 @@ intel_gpio_post_xfer(struct i2c_adapter *adapter)
 }
 
 static void
-intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
+intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
 {
 	struct drm_i915_private *dev_priv = bus->dev_priv;
 	struct i2c_algo_bit_data *algo;
 
 	algo = &bus->bit_algo;
 
-	/* -1 to map pin pair to gmbus index */
-	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_ports[pin - 1].reg;
+	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
 
 	bus->adapter.algo_data = algo;
 	algo->setsda = set_data;
@@ -517,7 +517,9 @@ static const struct i2c_algorithm gmbus_algorithm = {
 int intel_setup_gmbus(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int ret, i;
+	struct intel_gmbus *bus;
+	unsigned int pin;
+	int ret;
 
 	if (HAS_PCH_NOP(dev))
 		return 0;
@@ -531,16 +533,18 @@ int intel_setup_gmbus(struct drm_device *dev)
 	mutex_init(&dev_priv->gmbus_mutex);
 	init_waitqueue_head(&dev_priv->gmbus_wait_queue);
 
-	for (i = 0; i < GMBUS_NUM_PORTS; i++) {
-		struct intel_gmbus *bus = &dev_priv->gmbus[i];
-		u32 port = i + 1; /* +1 to map gmbus index to pin pair */
+	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
+		if (!intel_gmbus_is_valid_pin(pin))
+			continue;
+
+		bus = &dev_priv->gmbus[pin];
 
 		bus->adapter.owner = THIS_MODULE;
 		bus->adapter.class = I2C_CLASS_DDC;
 		snprintf(bus->adapter.name,
 			 sizeof(bus->adapter.name),
 			 "i915 gmbus %s",
-			 gmbus_ports[i].name);
+			 gmbus_pins[pin].name);
 
 		bus->adapter.dev.parent = &dev->pdev->dev;
 		bus->dev_priv = dev_priv;
@@ -548,13 +552,13 @@ int intel_setup_gmbus(struct drm_device *dev)
 		bus->adapter.algo = &gmbus_algorithm;
 
 		/* By default use a conservative clock rate */
-		bus->reg0 = port | GMBUS_RATE_100KHZ;
+		bus->reg0 = pin | GMBUS_RATE_100KHZ;
 
 		/* gmbus seems to be broken on i830 */
 		if (IS_I830(dev))
 			bus->force_bit = 1;
 
-		intel_gpio_setup(bus, port);
+		intel_gpio_setup(bus, pin);
 
 		ret = i2c_add_adapter(&bus->adapter);
 		if (ret)
@@ -566,8 +570,11 @@ int intel_setup_gmbus(struct drm_device *dev)
 	return 0;
 
 err:
-	while (--i) {
-		struct intel_gmbus *bus = &dev_priv->gmbus[i];
+	while (--pin) {
+		if (!intel_gmbus_is_valid_pin(pin))
+			continue;
+
+		bus = &dev_priv->gmbus[pin];
 		i2c_del_adapter(&bus->adapter);
 	}
 	return ret;
@@ -576,10 +583,10 @@ err:
 struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv,
 					    unsigned int pin)
 {
-	WARN_ON(!intel_gmbus_is_valid_pin(pin));
-	/* -1 to map pin pair to gmbus index */
-	return (intel_gmbus_is_valid_pin(pin)) ?
-		&dev_priv->gmbus[pin - 1].adapter : NULL;
+	if (WARN_ON(!intel_gmbus_is_valid_pin(pin)))
+		return NULL;
+
+	return &dev_priv->gmbus[pin].adapter;
 }
 
 void intel_gmbus_set_speed(struct i2c_adapter *adapter, int speed)
@@ -602,10 +609,14 @@ void intel_gmbus_force_bit(struct i2c_adapter *adapter, bool force_bit)
 void intel_teardown_gmbus(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int i;
+	struct intel_gmbus *bus;
+	unsigned int pin;
+
+	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
+		if (!intel_gmbus_is_valid_pin(pin))
+			continue;
 
-	for (i = 0; i < GMBUS_NUM_PORTS; i++) {
-		struct intel_gmbus *bus = &dev_priv->gmbus[i];
+		bus = &dev_priv->gmbus[pin];
 		i2c_del_adapter(&bus->adapter);
 	}
 }
-- 
2.1.4

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

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

* Re: [PATCH 3/5] drm/i915: index gmbus tables using the pin pair number
  2015-03-26 22:20 ` [PATCH 3/5] drm/i915: index gmbus tables using the pin pair number Jani Nikula
@ 2015-03-27 15:00   ` Ville Syrjälä
  2015-03-27 16:27     ` Jani Nikula
  2015-04-01  7:55     ` [PATCH v2] " Jani Nikula
  0 siblings, 2 replies; 19+ messages in thread
From: Ville Syrjälä @ 2015-03-27 15:00 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Fri, Mar 27, 2015 at 12:20:21AM +0200, Jani Nikula wrote:
> Index the gmbus tables directly using the pin instead of having a
> confusing "port = i + 1" mapping. This finishes off removing the "gmbus
> port" as a notion, and leaves us with just the "gmbus pin".
> 
> As pin 0 is invalid by definition and the gmbus tables will have a gap
> at that index, add pin validity check to all the loops. This will be
> benefitial for supporting platforms that have different numbers of pins,
> or gaps.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  3 +-
>  drivers/gpu/drm/i915/i915_reg.h  |  2 +-
>  drivers/gpu/drm/i915/intel_i2c.c | 65 +++++++++++++++++++++++-----------------
>  3 files changed, 40 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3ba5de19e039..0c6024101eb9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1574,8 +1574,7 @@ struct drm_i915_private {
>  
>  	struct i915_virtual_gpu vgpu;
>  
> -	struct intel_gmbus gmbus[GMBUS_NUM_PORTS];
> -
> +	struct intel_gmbus gmbus[GMBUS_PIN_MAX];
>  
>  	/** gmbus_mutex protects against concurrent usage of the single hw gmbus
>  	 * controller on different i2c buses. */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b6113c9a803b..cdc071cff001 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1797,7 +1797,7 @@ enum skl_disp_power_wells {
>  #define   GMBUS_PIN_DPB		5 /* SDVO, HDMIB */
>  #define   GMBUS_PIN_DPD		6 /* HDMID */
>  #define   GMBUS_PIN_RESERVED	7 /* 7 reserved */
> -#define   GMBUS_NUM_PORTS	(GMBUS_PIN_DPD - GMBUS_PIN_SSC + 1)
> +#define   GMBUS_PIN_MAX		7 /* not inclusive */

PIN_MAX makes me think it's inclusive. NUM_PINS maybe?

w/ or w/o that particular bikeshed patches 1-4 look fine to me, so:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  #define GMBUS1			0x5104 /* command/status */
>  #define   GMBUS_SW_CLR_INT	(1<<31)
>  #define   GMBUS_SW_RDY		(1<<30)
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index b0003a2bd854..ff47a8fdcb6d 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -34,18 +34,19 @@
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
>  
> -struct gmbus_port {
> +struct gmbus_pin {
>  	const char *name;
>  	int reg;
>  };
>  
> -static const struct gmbus_port gmbus_ports[] = {
> -	{ "ssc", GPIOB },
> -	{ "vga", GPIOA },
> -	{ "panel", GPIOC },
> -	{ "dpc", GPIOD },
> -	{ "dpb", GPIOE },
> -	{ "dpd", GPIOF },
> +/* Map gmbus pin pairs to names and registers. */
> +static const struct gmbus_pin gmbus_pins[] = {
> +	[GMBUS_PIN_SSC] = { "ssc", GPIOB },
> +	[GMBUS_PIN_VGADDC] = { "vga", GPIOA },
> +	[GMBUS_PIN_PANEL] = { "panel", GPIOC },
> +	[GMBUS_PIN_DPC] = { "dpc", GPIOD },
> +	[GMBUS_PIN_DPB] = { "dpb", GPIOE },
> +	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
>  };
>  
>  /* Intel GPIO access functions */
> @@ -182,15 +183,14 @@ intel_gpio_post_xfer(struct i2c_adapter *adapter)
>  }
>  
>  static void
> -intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
> +intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
>  {
>  	struct drm_i915_private *dev_priv = bus->dev_priv;
>  	struct i2c_algo_bit_data *algo;
>  
>  	algo = &bus->bit_algo;
>  
> -	/* -1 to map pin pair to gmbus index */
> -	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_ports[pin - 1].reg;
> +	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
>  
>  	bus->adapter.algo_data = algo;
>  	algo->setsda = set_data;
> @@ -517,7 +517,9 @@ static const struct i2c_algorithm gmbus_algorithm = {
>  int intel_setup_gmbus(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int ret, i;
> +	struct intel_gmbus *bus;
> +	unsigned int pin;
> +	int ret;
>  
>  	if (HAS_PCH_NOP(dev))
>  		return 0;
> @@ -531,16 +533,18 @@ int intel_setup_gmbus(struct drm_device *dev)
>  	mutex_init(&dev_priv->gmbus_mutex);
>  	init_waitqueue_head(&dev_priv->gmbus_wait_queue);
>  
> -	for (i = 0; i < GMBUS_NUM_PORTS; i++) {
> -		struct intel_gmbus *bus = &dev_priv->gmbus[i];
> -		u32 port = i + 1; /* +1 to map gmbus index to pin pair */
> +	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
> +		if (!intel_gmbus_is_valid_pin(pin))
> +			continue;
> +
> +		bus = &dev_priv->gmbus[pin];
>  
>  		bus->adapter.owner = THIS_MODULE;
>  		bus->adapter.class = I2C_CLASS_DDC;
>  		snprintf(bus->adapter.name,
>  			 sizeof(bus->adapter.name),
>  			 "i915 gmbus %s",
> -			 gmbus_ports[i].name);
> +			 gmbus_pins[pin].name);
>  
>  		bus->adapter.dev.parent = &dev->pdev->dev;
>  		bus->dev_priv = dev_priv;
> @@ -548,13 +552,13 @@ int intel_setup_gmbus(struct drm_device *dev)
>  		bus->adapter.algo = &gmbus_algorithm;
>  
>  		/* By default use a conservative clock rate */
> -		bus->reg0 = port | GMBUS_RATE_100KHZ;
> +		bus->reg0 = pin | GMBUS_RATE_100KHZ;
>  
>  		/* gmbus seems to be broken on i830 */
>  		if (IS_I830(dev))
>  			bus->force_bit = 1;
>  
> -		intel_gpio_setup(bus, port);
> +		intel_gpio_setup(bus, pin);
>  
>  		ret = i2c_add_adapter(&bus->adapter);
>  		if (ret)
> @@ -566,8 +570,11 @@ int intel_setup_gmbus(struct drm_device *dev)
>  	return 0;
>  
>  err:
> -	while (--i) {
> -		struct intel_gmbus *bus = &dev_priv->gmbus[i];
> +	while (--pin) {
> +		if (!intel_gmbus_is_valid_pin(pin))
> +			continue;
> +
> +		bus = &dev_priv->gmbus[pin];
>  		i2c_del_adapter(&bus->adapter);
>  	}
>  	return ret;
> @@ -576,10 +583,10 @@ err:
>  struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv,
>  					    unsigned int pin)
>  {
> -	WARN_ON(!intel_gmbus_is_valid_pin(pin));
> -	/* -1 to map pin pair to gmbus index */
> -	return (intel_gmbus_is_valid_pin(pin)) ?
> -		&dev_priv->gmbus[pin - 1].adapter : NULL;
> +	if (WARN_ON(!intel_gmbus_is_valid_pin(pin)))
> +		return NULL;
> +
> +	return &dev_priv->gmbus[pin].adapter;
>  }
>  
>  void intel_gmbus_set_speed(struct i2c_adapter *adapter, int speed)
> @@ -602,10 +609,14 @@ void intel_gmbus_force_bit(struct i2c_adapter *adapter, bool force_bit)
>  void intel_teardown_gmbus(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int i;
> +	struct intel_gmbus *bus;
> +	unsigned int pin;
> +
> +	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
> +		if (!intel_gmbus_is_valid_pin(pin))
> +			continue;
>  
> -	for (i = 0; i < GMBUS_NUM_PORTS; i++) {
> -		struct intel_gmbus *bus = &dev_priv->gmbus[i];
> +		bus = &dev_priv->gmbus[pin];
>  		i2c_del_adapter(&bus->adapter);
>  	}
>  }
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH 3/5] drm/i915: index gmbus tables using the pin pair number
  2015-03-27 15:00   ` Ville Syrjälä
@ 2015-03-27 16:27     ` Jani Nikula
  2015-04-01  7:55     ` [PATCH v2] " Jani Nikula
  1 sibling, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2015-03-27 16:27 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Fri, 27 Mar 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Mar 27, 2015 at 12:20:21AM +0200, Jani Nikula wrote:
>> Index the gmbus tables directly using the pin instead of having a
>> confusing "port = i + 1" mapping. This finishes off removing the "gmbus
>> port" as a notion, and leaves us with just the "gmbus pin".
>> 
>> As pin 0 is invalid by definition and the gmbus tables will have a gap
>> at that index, add pin validity check to all the loops. This will be
>> benefitial for supporting platforms that have different numbers of pins,
>> or gaps.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |  3 +-
>>  drivers/gpu/drm/i915/i915_reg.h  |  2 +-
>>  drivers/gpu/drm/i915/intel_i2c.c | 65 +++++++++++++++++++++++-----------------
>>  3 files changed, 40 insertions(+), 30 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 3ba5de19e039..0c6024101eb9 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1574,8 +1574,7 @@ struct drm_i915_private {
>>  
>>  	struct i915_virtual_gpu vgpu;
>>  
>> -	struct intel_gmbus gmbus[GMBUS_NUM_PORTS];
>> -
>> +	struct intel_gmbus gmbus[GMBUS_PIN_MAX];
>>  
>>  	/** gmbus_mutex protects against concurrent usage of the single hw gmbus
>>  	 * controller on different i2c buses. */
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index b6113c9a803b..cdc071cff001 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -1797,7 +1797,7 @@ enum skl_disp_power_wells {
>>  #define   GMBUS_PIN_DPB		5 /* SDVO, HDMIB */
>>  #define   GMBUS_PIN_DPD		6 /* HDMID */
>>  #define   GMBUS_PIN_RESERVED	7 /* 7 reserved */
>> -#define   GMBUS_NUM_PORTS	(GMBUS_PIN_DPD - GMBUS_PIN_SSC + 1)
>> +#define   GMBUS_PIN_MAX		7 /* not inclusive */
>
> PIN_MAX makes me think it's inclusive. NUM_PINS maybe?

I wanted to clearly distinguish it from the old NUM_PORTS which included
only the valid ones, while this is used for the gmbus[] array field size
in struct drm_i915_private only.

An alternative would be to make GMBUS_PIN_MAX inclusive and to use
gmbus[GMBUS_PIN_MAX + 1] in struct drm_i915_private.

> w/ or w/o that particular bikeshed patches 1-4 look fine to me, so:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks for the review.

BR,
Jani.

>
>>  #define GMBUS1			0x5104 /* command/status */
>>  #define   GMBUS_SW_CLR_INT	(1<<31)
>>  #define   GMBUS_SW_RDY		(1<<30)
>> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
>> index b0003a2bd854..ff47a8fdcb6d 100644
>> --- a/drivers/gpu/drm/i915/intel_i2c.c
>> +++ b/drivers/gpu/drm/i915/intel_i2c.c
>> @@ -34,18 +34,19 @@
>>  #include <drm/i915_drm.h>
>>  #include "i915_drv.h"
>>  
>> -struct gmbus_port {
>> +struct gmbus_pin {
>>  	const char *name;
>>  	int reg;
>>  };
>>  
>> -static const struct gmbus_port gmbus_ports[] = {
>> -	{ "ssc", GPIOB },
>> -	{ "vga", GPIOA },
>> -	{ "panel", GPIOC },
>> -	{ "dpc", GPIOD },
>> -	{ "dpb", GPIOE },
>> -	{ "dpd", GPIOF },
>> +/* Map gmbus pin pairs to names and registers. */
>> +static const struct gmbus_pin gmbus_pins[] = {
>> +	[GMBUS_PIN_SSC] = { "ssc", GPIOB },
>> +	[GMBUS_PIN_VGADDC] = { "vga", GPIOA },
>> +	[GMBUS_PIN_PANEL] = { "panel", GPIOC },
>> +	[GMBUS_PIN_DPC] = { "dpc", GPIOD },
>> +	[GMBUS_PIN_DPB] = { "dpb", GPIOE },
>> +	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
>>  };
>>  
>>  /* Intel GPIO access functions */
>> @@ -182,15 +183,14 @@ intel_gpio_post_xfer(struct i2c_adapter *adapter)
>>  }
>>  
>>  static void
>> -intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
>> +intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
>>  {
>>  	struct drm_i915_private *dev_priv = bus->dev_priv;
>>  	struct i2c_algo_bit_data *algo;
>>  
>>  	algo = &bus->bit_algo;
>>  
>> -	/* -1 to map pin pair to gmbus index */
>> -	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_ports[pin - 1].reg;
>> +	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
>>  
>>  	bus->adapter.algo_data = algo;
>>  	algo->setsda = set_data;
>> @@ -517,7 +517,9 @@ static const struct i2c_algorithm gmbus_algorithm = {
>>  int intel_setup_gmbus(struct drm_device *dev)
>>  {
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>> -	int ret, i;
>> +	struct intel_gmbus *bus;
>> +	unsigned int pin;
>> +	int ret;
>>  
>>  	if (HAS_PCH_NOP(dev))
>>  		return 0;
>> @@ -531,16 +533,18 @@ int intel_setup_gmbus(struct drm_device *dev)
>>  	mutex_init(&dev_priv->gmbus_mutex);
>>  	init_waitqueue_head(&dev_priv->gmbus_wait_queue);
>>  
>> -	for (i = 0; i < GMBUS_NUM_PORTS; i++) {
>> -		struct intel_gmbus *bus = &dev_priv->gmbus[i];
>> -		u32 port = i + 1; /* +1 to map gmbus index to pin pair */
>> +	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
>> +		if (!intel_gmbus_is_valid_pin(pin))
>> +			continue;
>> +
>> +		bus = &dev_priv->gmbus[pin];
>>  
>>  		bus->adapter.owner = THIS_MODULE;
>>  		bus->adapter.class = I2C_CLASS_DDC;
>>  		snprintf(bus->adapter.name,
>>  			 sizeof(bus->adapter.name),
>>  			 "i915 gmbus %s",
>> -			 gmbus_ports[i].name);
>> +			 gmbus_pins[pin].name);
>>  
>>  		bus->adapter.dev.parent = &dev->pdev->dev;
>>  		bus->dev_priv = dev_priv;
>> @@ -548,13 +552,13 @@ int intel_setup_gmbus(struct drm_device *dev)
>>  		bus->adapter.algo = &gmbus_algorithm;
>>  
>>  		/* By default use a conservative clock rate */
>> -		bus->reg0 = port | GMBUS_RATE_100KHZ;
>> +		bus->reg0 = pin | GMBUS_RATE_100KHZ;
>>  
>>  		/* gmbus seems to be broken on i830 */
>>  		if (IS_I830(dev))
>>  			bus->force_bit = 1;
>>  
>> -		intel_gpio_setup(bus, port);
>> +		intel_gpio_setup(bus, pin);
>>  
>>  		ret = i2c_add_adapter(&bus->adapter);
>>  		if (ret)
>> @@ -566,8 +570,11 @@ int intel_setup_gmbus(struct drm_device *dev)
>>  	return 0;
>>  
>>  err:
>> -	while (--i) {
>> -		struct intel_gmbus *bus = &dev_priv->gmbus[i];
>> +	while (--pin) {
>> +		if (!intel_gmbus_is_valid_pin(pin))
>> +			continue;
>> +
>> +		bus = &dev_priv->gmbus[pin];
>>  		i2c_del_adapter(&bus->adapter);
>>  	}
>>  	return ret;
>> @@ -576,10 +583,10 @@ err:
>>  struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv,
>>  					    unsigned int pin)
>>  {
>> -	WARN_ON(!intel_gmbus_is_valid_pin(pin));
>> -	/* -1 to map pin pair to gmbus index */
>> -	return (intel_gmbus_is_valid_pin(pin)) ?
>> -		&dev_priv->gmbus[pin - 1].adapter : NULL;
>> +	if (WARN_ON(!intel_gmbus_is_valid_pin(pin)))
>> +		return NULL;
>> +
>> +	return &dev_priv->gmbus[pin].adapter;
>>  }
>>  
>>  void intel_gmbus_set_speed(struct i2c_adapter *adapter, int speed)
>> @@ -602,10 +609,14 @@ void intel_gmbus_force_bit(struct i2c_adapter *adapter, bool force_bit)
>>  void intel_teardown_gmbus(struct drm_device *dev)
>>  {
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>> -	int i;
>> +	struct intel_gmbus *bus;
>> +	unsigned int pin;
>> +
>> +	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
>> +		if (!intel_gmbus_is_valid_pin(pin))
>> +			continue;
>>  
>> -	for (i = 0; i < GMBUS_NUM_PORTS; i++) {
>> -		struct intel_gmbus *bus = &dev_priv->gmbus[i];
>> +		bus = &dev_priv->gmbus[pin];
>>  		i2c_del_adapter(&bus->adapter);
>>  	}
>>  }
>> -- 
>> 2.1.4
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Ville Syrjälä
> Intel OTC

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

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

* [PATCH v2] drm/i915: index gmbus tables using the pin pair number
  2015-03-27 15:00   ` Ville Syrjälä
  2015-03-27 16:27     ` Jani Nikula
@ 2015-04-01  7:55     ` Jani Nikula
  1 sibling, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2015-04-01  7:55 UTC (permalink / raw)
  To: Ville Syrjälä, Jani Nikula; +Cc: intel-gfx

Index the gmbus tables directly using the pin instead of having a
confusing "port = i + 1" mapping. This finishes off removing the "gmbus
port" as a notion, and leaves us with just the "gmbus pin".

As pin 0 is invalid by definition and the gmbus tables will have a gap
at that index, add pin validity check to all the loops. This will be
benefitial for supporting platforms that have different numbers of pins,
or gaps.

v2: s/GMBUS_PIN_MAX/GMBUS_NUM_PINS/ (Ville, Daniel)

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  3 +-
 drivers/gpu/drm/i915/i915_reg.h  |  2 +-
 drivers/gpu/drm/i915/intel_i2c.c | 65 +++++++++++++++++++++++-----------------
 3 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e7db1e1fcd16..e50b32772bb4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1574,8 +1574,7 @@ struct drm_i915_private {
 
 	struct i915_virtual_gpu vgpu;
 
-	struct intel_gmbus gmbus[GMBUS_NUM_PORTS];
-
+	struct intel_gmbus gmbus[GMBUS_NUM_PINS];
 
 	/** gmbus_mutex protects against concurrent usage of the single hw gmbus
 	 * controller on different i2c buses. */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8167ef53b02c..b134fa39c5b8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1797,7 +1797,7 @@ enum skl_disp_power_wells {
 #define   GMBUS_PIN_DPB		5 /* SDVO, HDMIB */
 #define   GMBUS_PIN_DPD		6 /* HDMID */
 #define   GMBUS_PIN_RESERVED	7 /* 7 reserved */
-#define   GMBUS_NUM_PORTS	(GMBUS_PIN_DPD - GMBUS_PIN_SSC + 1)
+#define   GMBUS_NUM_PINS	7 /* including 0 */
 #define GMBUS1			0x5104 /* command/status */
 #define   GMBUS_SW_CLR_INT	(1<<31)
 #define   GMBUS_SW_RDY		(1<<30)
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index b0003a2bd854..ff47a8fdcb6d 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -34,18 +34,19 @@
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 
-struct gmbus_port {
+struct gmbus_pin {
 	const char *name;
 	int reg;
 };
 
-static const struct gmbus_port gmbus_ports[] = {
-	{ "ssc", GPIOB },
-	{ "vga", GPIOA },
-	{ "panel", GPIOC },
-	{ "dpc", GPIOD },
-	{ "dpb", GPIOE },
-	{ "dpd", GPIOF },
+/* Map gmbus pin pairs to names and registers. */
+static const struct gmbus_pin gmbus_pins[] = {
+	[GMBUS_PIN_SSC] = { "ssc", GPIOB },
+	[GMBUS_PIN_VGADDC] = { "vga", GPIOA },
+	[GMBUS_PIN_PANEL] = { "panel", GPIOC },
+	[GMBUS_PIN_DPC] = { "dpc", GPIOD },
+	[GMBUS_PIN_DPB] = { "dpb", GPIOE },
+	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
 };
 
 /* Intel GPIO access functions */
@@ -182,15 +183,14 @@ intel_gpio_post_xfer(struct i2c_adapter *adapter)
 }
 
 static void
-intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
+intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
 {
 	struct drm_i915_private *dev_priv = bus->dev_priv;
 	struct i2c_algo_bit_data *algo;
 
 	algo = &bus->bit_algo;
 
-	/* -1 to map pin pair to gmbus index */
-	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_ports[pin - 1].reg;
+	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
 
 	bus->adapter.algo_data = algo;
 	algo->setsda = set_data;
@@ -517,7 +517,9 @@ static const struct i2c_algorithm gmbus_algorithm = {
 int intel_setup_gmbus(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int ret, i;
+	struct intel_gmbus *bus;
+	unsigned int pin;
+	int ret;
 
 	if (HAS_PCH_NOP(dev))
 		return 0;
@@ -531,16 +533,18 @@ int intel_setup_gmbus(struct drm_device *dev)
 	mutex_init(&dev_priv->gmbus_mutex);
 	init_waitqueue_head(&dev_priv->gmbus_wait_queue);
 
-	for (i = 0; i < GMBUS_NUM_PORTS; i++) {
-		struct intel_gmbus *bus = &dev_priv->gmbus[i];
-		u32 port = i + 1; /* +1 to map gmbus index to pin pair */
+	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
+		if (!intel_gmbus_is_valid_pin(pin))
+			continue;
+
+		bus = &dev_priv->gmbus[pin];
 
 		bus->adapter.owner = THIS_MODULE;
 		bus->adapter.class = I2C_CLASS_DDC;
 		snprintf(bus->adapter.name,
 			 sizeof(bus->adapter.name),
 			 "i915 gmbus %s",
-			 gmbus_ports[i].name);
+			 gmbus_pins[pin].name);
 
 		bus->adapter.dev.parent = &dev->pdev->dev;
 		bus->dev_priv = dev_priv;
@@ -548,13 +552,13 @@ int intel_setup_gmbus(struct drm_device *dev)
 		bus->adapter.algo = &gmbus_algorithm;
 
 		/* By default use a conservative clock rate */
-		bus->reg0 = port | GMBUS_RATE_100KHZ;
+		bus->reg0 = pin | GMBUS_RATE_100KHZ;
 
 		/* gmbus seems to be broken on i830 */
 		if (IS_I830(dev))
 			bus->force_bit = 1;
 
-		intel_gpio_setup(bus, port);
+		intel_gpio_setup(bus, pin);
 
 		ret = i2c_add_adapter(&bus->adapter);
 		if (ret)
@@ -566,8 +570,11 @@ int intel_setup_gmbus(struct drm_device *dev)
 	return 0;
 
 err:
-	while (--i) {
-		struct intel_gmbus *bus = &dev_priv->gmbus[i];
+	while (--pin) {
+		if (!intel_gmbus_is_valid_pin(pin))
+			continue;
+
+		bus = &dev_priv->gmbus[pin];
 		i2c_del_adapter(&bus->adapter);
 	}
 	return ret;
@@ -576,10 +583,10 @@ err:
 struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv,
 					    unsigned int pin)
 {
-	WARN_ON(!intel_gmbus_is_valid_pin(pin));
-	/* -1 to map pin pair to gmbus index */
-	return (intel_gmbus_is_valid_pin(pin)) ?
-		&dev_priv->gmbus[pin - 1].adapter : NULL;
+	if (WARN_ON(!intel_gmbus_is_valid_pin(pin)))
+		return NULL;
+
+	return &dev_priv->gmbus[pin].adapter;
 }
 
 void intel_gmbus_set_speed(struct i2c_adapter *adapter, int speed)
@@ -602,10 +609,14 @@ void intel_gmbus_force_bit(struct i2c_adapter *adapter, bool force_bit)
 void intel_teardown_gmbus(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int i;
+	struct intel_gmbus *bus;
+	unsigned int pin;
+
+	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
+		if (!intel_gmbus_is_valid_pin(pin))
+			continue;
 
-	for (i = 0; i < GMBUS_NUM_PORTS; i++) {
-		struct intel_gmbus *bus = &dev_priv->gmbus[i];
+		bus = &dev_priv->gmbus[pin];
 		i2c_del_adapter(&bus->adapter);
 	}
 }
-- 
2.1.4

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

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

* [PATCH 4/5] drm/i915: base gmbus pin validity check on the gmbus pin map array
  2015-03-26 22:20 [PATCH 0/5] drm/i915: gmbus pin/port cleanup and bxt enabling Jani Nikula
                   ` (2 preceding siblings ...)
  2015-03-26 22:20 ` [PATCH 3/5] drm/i915: index gmbus tables using the pin pair number Jani Nikula
@ 2015-03-26 22:20 ` Jani Nikula
  2015-04-01 12:12   ` Daniel Vetter
  2015-03-26 22:20 ` [PATCH 5/5] drm/i915: add bxt gmbus support Jani Nikula
  2015-03-27 10:05 ` [PATCH 1/2] drm/i915: don't register nonexisting gmbus pins for bdw Jani Nikula
  5 siblings, 1 reply; 19+ messages in thread
From: Jani Nikula @ 2015-03-26 22:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

This will be helpful for adding future platforms. It is better to keep
the information in the single point of truth (the table) instead of
duplicating it into the validity function.

While at it, add dev_priv parameter to the function, also to prepare for
adding future platform support.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  6 ++----
 drivers/gpu/drm/i915/intel_bios.c |  2 +-
 drivers/gpu/drm/i915/intel_dvo.c  |  2 +-
 drivers/gpu/drm/i915/intel_i2c.c  | 14 ++++++++++----
 drivers/gpu/drm/i915/intel_lvds.c |  2 +-
 drivers/gpu/drm/i915/intel_sdvo.c |  3 ++-
 6 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0c6024101eb9..a1ba9bec8a6b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3048,10 +3048,8 @@ void i915_teardown_sysfs(struct drm_device *dev_priv);
 /* intel_i2c.c */
 extern int intel_setup_gmbus(struct drm_device *dev);
 extern void intel_teardown_gmbus(struct drm_device *dev);
-static inline bool intel_gmbus_is_valid_pin(unsigned int pin)
-{
-	return (pin >= GMBUS_PIN_SSC && pin <= GMBUS_PIN_DPD);
-}
+extern bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
+				     unsigned int pin);
 
 extern struct i2c_adapter *
 intel_gmbus_get_adapter(struct drm_i915_private *dev_priv, unsigned int pin);
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 333f40793435..ad2f3b0d922f 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -438,7 +438,7 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
 		if (block_size >= sizeof(*general)) {
 			int bus_pin = general->crt_ddc_gmbus_pin;
 			DRM_DEBUG_KMS("crt_ddc_bus_pin: %d\n", bus_pin);
-			if (intel_gmbus_is_valid_pin(bus_pin))
+			if (intel_gmbus_is_valid_pin(dev_priv, bus_pin))
 				dev_priv->vbt.crt_ddc_pin = bus_pin;
 		} else {
 			DRM_DEBUG_KMS("BDB_GD too small (%d). Invalid.\n",
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 8d62272a3421..9670e3802939 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -500,7 +500,7 @@ void intel_dvo_init(struct drm_device *dev)
 		 * special cases, but otherwise default to what's defined
 		 * in the spec.
 		 */
-		if (intel_gmbus_is_valid_pin(dvo->gpio))
+		if (intel_gmbus_is_valid_pin(dev_priv, dvo->gpio))
 			gpio = dvo->gpio;
 		else if (dvo->type == INTEL_DVO_CHIP_LVDS)
 			gpio = GMBUS_PIN_SSC;
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index ff47a8fdcb6d..ec9cc8cf642e 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -49,6 +49,12 @@ static const struct gmbus_pin gmbus_pins[] = {
 	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
 };
 
+bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
+			      unsigned int pin)
+{
+	return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
+}
+
 /* Intel GPIO access functions */
 
 #define I2C_RISEFALL_TIME 10
@@ -534,7 +540,7 @@ int intel_setup_gmbus(struct drm_device *dev)
 	init_waitqueue_head(&dev_priv->gmbus_wait_queue);
 
 	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
-		if (!intel_gmbus_is_valid_pin(pin))
+		if (!intel_gmbus_is_valid_pin(dev_priv, pin))
 			continue;
 
 		bus = &dev_priv->gmbus[pin];
@@ -571,7 +577,7 @@ int intel_setup_gmbus(struct drm_device *dev)
 
 err:
 	while (--pin) {
-		if (!intel_gmbus_is_valid_pin(pin))
+		if (!intel_gmbus_is_valid_pin(dev_priv, pin))
 			continue;
 
 		bus = &dev_priv->gmbus[pin];
@@ -583,7 +589,7 @@ err:
 struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv,
 					    unsigned int pin)
 {
-	if (WARN_ON(!intel_gmbus_is_valid_pin(pin)))
+	if (WARN_ON(!intel_gmbus_is_valid_pin(dev_priv, pin)))
 		return NULL;
 
 	return &dev_priv->gmbus[pin].adapter;
@@ -613,7 +619,7 @@ void intel_teardown_gmbus(struct drm_device *dev)
 	unsigned int pin;
 
 	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
-		if (!intel_gmbus_is_valid_pin(pin))
+		if (!intel_gmbus_is_valid_pin(dev_priv, pin))
 			continue;
 
 		bus = &dev_priv->gmbus[pin];
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index d61aa78ed7e3..314a5d56ace2 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -781,7 +781,7 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev,
 		    child->device_type != DEVICE_TYPE_LFP)
 			continue;
 
-		if (intel_gmbus_is_valid_pin(child->i2c_pin))
+		if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
 			*i2c_pin = child->i2c_pin;
 
 		/* However, we cannot trust the BIOS writers to populate
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 124992e48abd..b121796c86aa 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2291,7 +2291,8 @@ intel_sdvo_select_i2c_bus(struct drm_i915_private *dev_priv,
 	else
 		mapping = &dev_priv->sdvo_mappings[1];
 
-	if (mapping->initialized && intel_gmbus_is_valid_pin(mapping->i2c_pin))
+	if (mapping->initialized &&
+	    intel_gmbus_is_valid_pin(dev_priv, mapping->i2c_pin))
 		pin = mapping->i2c_pin;
 	else
 		pin = GMBUS_PIN_DPB;
-- 
2.1.4

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

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

* Re: [PATCH 4/5] drm/i915: base gmbus pin validity check on the gmbus pin map array
  2015-03-26 22:20 ` [PATCH 4/5] drm/i915: base gmbus pin validity check on the gmbus pin map array Jani Nikula
@ 2015-04-01 12:12   ` Daniel Vetter
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2015-04-01 12:12 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Fri, Mar 27, 2015 at 12:20:22AM +0200, Jani Nikula wrote:
> This will be helpful for adding future platforms. It is better to keep
> the information in the single point of truth (the table) instead of
> duplicating it into the validity function.
> 
> While at it, add dev_priv parameter to the function, also to prepare for
> adding future platform support.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Merged up to this patch, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  6 ++----
>  drivers/gpu/drm/i915/intel_bios.c |  2 +-
>  drivers/gpu/drm/i915/intel_dvo.c  |  2 +-
>  drivers/gpu/drm/i915/intel_i2c.c  | 14 ++++++++++----
>  drivers/gpu/drm/i915/intel_lvds.c |  2 +-
>  drivers/gpu/drm/i915/intel_sdvo.c |  3 ++-
>  6 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0c6024101eb9..a1ba9bec8a6b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3048,10 +3048,8 @@ void i915_teardown_sysfs(struct drm_device *dev_priv);
>  /* intel_i2c.c */
>  extern int intel_setup_gmbus(struct drm_device *dev);
>  extern void intel_teardown_gmbus(struct drm_device *dev);
> -static inline bool intel_gmbus_is_valid_pin(unsigned int pin)
> -{
> -	return (pin >= GMBUS_PIN_SSC && pin <= GMBUS_PIN_DPD);
> -}
> +extern bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
> +				     unsigned int pin);
>  
>  extern struct i2c_adapter *
>  intel_gmbus_get_adapter(struct drm_i915_private *dev_priv, unsigned int pin);
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 333f40793435..ad2f3b0d922f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -438,7 +438,7 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>  		if (block_size >= sizeof(*general)) {
>  			int bus_pin = general->crt_ddc_gmbus_pin;
>  			DRM_DEBUG_KMS("crt_ddc_bus_pin: %d\n", bus_pin);
> -			if (intel_gmbus_is_valid_pin(bus_pin))
> +			if (intel_gmbus_is_valid_pin(dev_priv, bus_pin))
>  				dev_priv->vbt.crt_ddc_pin = bus_pin;
>  		} else {
>  			DRM_DEBUG_KMS("BDB_GD too small (%d). Invalid.\n",
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 8d62272a3421..9670e3802939 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -500,7 +500,7 @@ void intel_dvo_init(struct drm_device *dev)
>  		 * special cases, but otherwise default to what's defined
>  		 * in the spec.
>  		 */
> -		if (intel_gmbus_is_valid_pin(dvo->gpio))
> +		if (intel_gmbus_is_valid_pin(dev_priv, dvo->gpio))
>  			gpio = dvo->gpio;
>  		else if (dvo->type == INTEL_DVO_CHIP_LVDS)
>  			gpio = GMBUS_PIN_SSC;
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index ff47a8fdcb6d..ec9cc8cf642e 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -49,6 +49,12 @@ static const struct gmbus_pin gmbus_pins[] = {
>  	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
>  };
>  
> +bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
> +			      unsigned int pin)
> +{
> +	return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
> +}
> +
>  /* Intel GPIO access functions */
>  
>  #define I2C_RISEFALL_TIME 10
> @@ -534,7 +540,7 @@ int intel_setup_gmbus(struct drm_device *dev)
>  	init_waitqueue_head(&dev_priv->gmbus_wait_queue);
>  
>  	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
> -		if (!intel_gmbus_is_valid_pin(pin))
> +		if (!intel_gmbus_is_valid_pin(dev_priv, pin))
>  			continue;
>  
>  		bus = &dev_priv->gmbus[pin];
> @@ -571,7 +577,7 @@ int intel_setup_gmbus(struct drm_device *dev)
>  
>  err:
>  	while (--pin) {
> -		if (!intel_gmbus_is_valid_pin(pin))
> +		if (!intel_gmbus_is_valid_pin(dev_priv, pin))
>  			continue;
>  
>  		bus = &dev_priv->gmbus[pin];
> @@ -583,7 +589,7 @@ err:
>  struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv,
>  					    unsigned int pin)
>  {
> -	if (WARN_ON(!intel_gmbus_is_valid_pin(pin)))
> +	if (WARN_ON(!intel_gmbus_is_valid_pin(dev_priv, pin)))
>  		return NULL;
>  
>  	return &dev_priv->gmbus[pin].adapter;
> @@ -613,7 +619,7 @@ void intel_teardown_gmbus(struct drm_device *dev)
>  	unsigned int pin;
>  
>  	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
> -		if (!intel_gmbus_is_valid_pin(pin))
> +		if (!intel_gmbus_is_valid_pin(dev_priv, pin))
>  			continue;
>  
>  		bus = &dev_priv->gmbus[pin];
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index d61aa78ed7e3..314a5d56ace2 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -781,7 +781,7 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev,
>  		    child->device_type != DEVICE_TYPE_LFP)
>  			continue;
>  
> -		if (intel_gmbus_is_valid_pin(child->i2c_pin))
> +		if (intel_gmbus_is_valid_pin(dev_priv, child->i2c_pin))
>  			*i2c_pin = child->i2c_pin;
>  
>  		/* However, we cannot trust the BIOS writers to populate
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 124992e48abd..b121796c86aa 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2291,7 +2291,8 @@ intel_sdvo_select_i2c_bus(struct drm_i915_private *dev_priv,
>  	else
>  		mapping = &dev_priv->sdvo_mappings[1];
>  
> -	if (mapping->initialized && intel_gmbus_is_valid_pin(mapping->i2c_pin))
> +	if (mapping->initialized &&
> +	    intel_gmbus_is_valid_pin(dev_priv, mapping->i2c_pin))
>  		pin = mapping->i2c_pin;
>  	else
>  		pin = GMBUS_PIN_DPB;
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* [PATCH 5/5] drm/i915: add bxt gmbus support
  2015-03-26 22:20 [PATCH 0/5] drm/i915: gmbus pin/port cleanup and bxt enabling Jani Nikula
                   ` (3 preceding siblings ...)
  2015-03-26 22:20 ` [PATCH 4/5] drm/i915: base gmbus pin validity check on the gmbus pin map array Jani Nikula
@ 2015-03-26 22:20 ` Jani Nikula
  2015-03-26 22:38   ` Jani Nikula
                     ` (3 more replies)
  2015-03-27 10:05 ` [PATCH 1/2] drm/i915: don't register nonexisting gmbus pins for bdw Jani Nikula
  5 siblings, 4 replies; 19+ messages in thread
From: Jani Nikula @ 2015-03-26 22:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

From: "A.Sunil Kamath" <sunil.kamath@intel.com>

For BXT gmbus is pulled from GPU to CPU. From implementation point of
view only pin pair configuration will change. The existing
implementation supports all platforms previous to GEN8 and also SKL. But
for BXT pin pair configuration is completely different than SKL or other
previous GEN's. This patch introduces the new pin pair configuration
structure specific to BXT and also ensures every real gmbus port has a
gpio pin.

v3 by Jani: with the platform independent prep work in place, the bxt
enabling reduces to a fairly trivial patch. Credits are due Sunil for
giving me the ideas (with his patches) what the platform independent
parts should look like.

Issue: VIZ-3574
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  3 +++
 drivers/gpu/drm/i915/intel_i2c.c | 29 +++++++++++++++++++++++------
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cdc071cff001..b0dc506f2fd1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1797,6 +1797,9 @@ enum skl_disp_power_wells {
 #define   GMBUS_PIN_DPB		5 /* SDVO, HDMIB */
 #define   GMBUS_PIN_DPD		6 /* HDMID */
 #define   GMBUS_PIN_RESERVED	7 /* 7 reserved */
+#define   GMBUS_PIN_1_BXT	1
+#define   GMBUS_PIN_2_BXT	2
+#define   GMBUS_PIN_3_BXT	3
 #define   GMBUS_PIN_MAX		7 /* not inclusive */
 #define GMBUS1			0x5104 /* command/status */
 #define   GMBUS_SW_CLR_INT	(1<<31)
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index ec9cc8cf642e..fd9cb6dcb84e 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -49,10 +49,20 @@ static const struct gmbus_pin gmbus_pins[] = {
 	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
 };
 
+static const struct gmbus_pin gmbus_pins_bxt[] = {
+	[GMBUS_PIN_1_BXT] = { "dpb", PCH_GPIOB },
+	[GMBUS_PIN_2_BXT] = { "dpc", PCH_GPIOC },
+	[GMBUS_PIN_3_BXT] = { "misc", PCH_GPIOD },
+};
+
 bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
 			      unsigned int pin)
 {
-	return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
+	if (IS_BROXTON(dev_priv))
+		return pin < ARRAY_SIZE(gmbus_pins_bxt) &&
+			gmbus_pins_bxt[pin].reg;
+	else
+		return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
 }
 
 /* Intel GPIO access functions */
@@ -196,7 +206,10 @@ intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
 
 	algo = &bus->bit_algo;
 
-	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
+	if (IS_BROXTON(dev_priv))
+		bus->gpio_reg = gmbus_pins_bxt[pin].reg;
+	else
+		bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
 
 	bus->adapter.algo_data = algo;
 	algo->setsda = set_data;
@@ -540,6 +553,8 @@ int intel_setup_gmbus(struct drm_device *dev)
 	init_waitqueue_head(&dev_priv->gmbus_wait_queue);
 
 	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
+		const char *name;
+
 		if (!intel_gmbus_is_valid_pin(dev_priv, pin))
 			continue;
 
@@ -547,10 +562,12 @@ int intel_setup_gmbus(struct drm_device *dev)
 
 		bus->adapter.owner = THIS_MODULE;
 		bus->adapter.class = I2C_CLASS_DDC;
-		snprintf(bus->adapter.name,
-			 sizeof(bus->adapter.name),
-			 "i915 gmbus %s",
-			 gmbus_pins[pin].name);
+		if (IS_BROXTON(dev_priv))
+			name = gmbus_pins_bxt[pin].name;
+		else
+			name = gmbus_pins[pin].name;
+		snprintf(bus->adapter.name, sizeof(bus->adapter.name),
+			 "i915 gmbus %s", name);
 
 		bus->adapter.dev.parent = &dev->pdev->dev;
 		bus->dev_priv = dev_priv;
-- 
2.1.4

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

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

* Re: [PATCH 5/5] drm/i915: add bxt gmbus support
  2015-03-26 22:20 ` [PATCH 5/5] drm/i915: add bxt gmbus support Jani Nikula
@ 2015-03-26 22:38   ` Jani Nikula
  2015-03-27  8:42   ` Daniel Vetter
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2015-03-26 22:38 UTC (permalink / raw)
  To: intel-gfx

On Fri, 27 Mar 2015, Jani Nikula <jani.nikula@intel.com> wrote:
> From: "A.Sunil Kamath" <sunil.kamath@intel.com>
>
> For BXT gmbus is pulled from GPU to CPU. From implementation point of
> view only pin pair configuration will change. The existing
> implementation supports all platforms previous to GEN8 and also SKL. But
> for BXT pin pair configuration is completely different than SKL or other
> previous GEN's. This patch introduces the new pin pair configuration
> structure specific to BXT and also ensures every real gmbus port has a
> gpio pin.
>
> v3 by Jani: with the platform independent prep work in place, the bxt
> enabling reduces to a fairly trivial patch. Credits are due Sunil for
> giving me the ideas (with his patches) what the platform independent
> parts should look like.

Okay, git fail, this still needs a bxt specific fix in
intel_hdmi_init_connector a bit similar to what chv does there, but too
tired now. x_X

BR,
Jani.


>
> Issue: VIZ-3574
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  3 +++
>  drivers/gpu/drm/i915/intel_i2c.c | 29 +++++++++++++++++++++++------
>  2 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cdc071cff001..b0dc506f2fd1 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1797,6 +1797,9 @@ enum skl_disp_power_wells {
>  #define   GMBUS_PIN_DPB		5 /* SDVO, HDMIB */
>  #define   GMBUS_PIN_DPD		6 /* HDMID */
>  #define   GMBUS_PIN_RESERVED	7 /* 7 reserved */
> +#define   GMBUS_PIN_1_BXT	1
> +#define   GMBUS_PIN_2_BXT	2
> +#define   GMBUS_PIN_3_BXT	3
>  #define   GMBUS_PIN_MAX		7 /* not inclusive */
>  #define GMBUS1			0x5104 /* command/status */
>  #define   GMBUS_SW_CLR_INT	(1<<31)
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index ec9cc8cf642e..fd9cb6dcb84e 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -49,10 +49,20 @@ static const struct gmbus_pin gmbus_pins[] = {
>  	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
>  };
>  
> +static const struct gmbus_pin gmbus_pins_bxt[] = {
> +	[GMBUS_PIN_1_BXT] = { "dpb", PCH_GPIOB },
> +	[GMBUS_PIN_2_BXT] = { "dpc", PCH_GPIOC },
> +	[GMBUS_PIN_3_BXT] = { "misc", PCH_GPIOD },
> +};
> +
>  bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
>  			      unsigned int pin)
>  {
> -	return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
> +	if (IS_BROXTON(dev_priv))
> +		return pin < ARRAY_SIZE(gmbus_pins_bxt) &&
> +			gmbus_pins_bxt[pin].reg;
> +	else
> +		return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
>  }
>  
>  /* Intel GPIO access functions */
> @@ -196,7 +206,10 @@ intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
>  
>  	algo = &bus->bit_algo;
>  
> -	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
> +	if (IS_BROXTON(dev_priv))
> +		bus->gpio_reg = gmbus_pins_bxt[pin].reg;
> +	else
> +		bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
>  
>  	bus->adapter.algo_data = algo;
>  	algo->setsda = set_data;
> @@ -540,6 +553,8 @@ int intel_setup_gmbus(struct drm_device *dev)
>  	init_waitqueue_head(&dev_priv->gmbus_wait_queue);
>  
>  	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
> +		const char *name;
> +
>  		if (!intel_gmbus_is_valid_pin(dev_priv, pin))
>  			continue;
>  
> @@ -547,10 +562,12 @@ int intel_setup_gmbus(struct drm_device *dev)
>  
>  		bus->adapter.owner = THIS_MODULE;
>  		bus->adapter.class = I2C_CLASS_DDC;
> -		snprintf(bus->adapter.name,
> -			 sizeof(bus->adapter.name),
> -			 "i915 gmbus %s",
> -			 gmbus_pins[pin].name);
> +		if (IS_BROXTON(dev_priv))
> +			name = gmbus_pins_bxt[pin].name;
> +		else
> +			name = gmbus_pins[pin].name;
> +		snprintf(bus->adapter.name, sizeof(bus->adapter.name),
> +			 "i915 gmbus %s", name);
>  
>  		bus->adapter.dev.parent = &dev->pdev->dev;
>  		bus->dev_priv = dev_priv;
> -- 
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH 5/5] drm/i915: add bxt gmbus support
  2015-03-26 22:20 ` [PATCH 5/5] drm/i915: add bxt gmbus support Jani Nikula
  2015-03-26 22:38   ` Jani Nikula
@ 2015-03-27  8:42   ` Daniel Vetter
  2015-03-27  8:59   ` [PATCH] " Jani Nikula
  2015-03-27  9:00   ` [PATCH v4] " Jani Nikula
  3 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2015-03-27  8:42 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Fri, Mar 27, 2015 at 12:20:23AM +0200, Jani Nikula wrote:
> From: "A.Sunil Kamath" <sunil.kamath@intel.com>
> 
> For BXT gmbus is pulled from GPU to CPU. From implementation point of

s/GPU/PCH/
-Daniel

> view only pin pair configuration will change. The existing
> implementation supports all platforms previous to GEN8 and also SKL. But
> for BXT pin pair configuration is completely different than SKL or other
> previous GEN's. This patch introduces the new pin pair configuration
> structure specific to BXT and also ensures every real gmbus port has a
> gpio pin.
> 
> v3 by Jani: with the platform independent prep work in place, the bxt
> enabling reduces to a fairly trivial patch. Credits are due Sunil for
> giving me the ideas (with his patches) what the platform independent
> parts should look like.
> 
> Issue: VIZ-3574
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  3 +++
>  drivers/gpu/drm/i915/intel_i2c.c | 29 +++++++++++++++++++++++------
>  2 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cdc071cff001..b0dc506f2fd1 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1797,6 +1797,9 @@ enum skl_disp_power_wells {
>  #define   GMBUS_PIN_DPB		5 /* SDVO, HDMIB */
>  #define   GMBUS_PIN_DPD		6 /* HDMID */
>  #define   GMBUS_PIN_RESERVED	7 /* 7 reserved */
> +#define   GMBUS_PIN_1_BXT	1
> +#define   GMBUS_PIN_2_BXT	2
> +#define   GMBUS_PIN_3_BXT	3
>  #define   GMBUS_PIN_MAX		7 /* not inclusive */
>  #define GMBUS1			0x5104 /* command/status */
>  #define   GMBUS_SW_CLR_INT	(1<<31)
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index ec9cc8cf642e..fd9cb6dcb84e 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -49,10 +49,20 @@ static const struct gmbus_pin gmbus_pins[] = {
>  	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
>  };
>  
> +static const struct gmbus_pin gmbus_pins_bxt[] = {
> +	[GMBUS_PIN_1_BXT] = { "dpb", PCH_GPIOB },
> +	[GMBUS_PIN_2_BXT] = { "dpc", PCH_GPIOC },
> +	[GMBUS_PIN_3_BXT] = { "misc", PCH_GPIOD },
> +};
> +
>  bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
>  			      unsigned int pin)
>  {
> -	return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
> +	if (IS_BROXTON(dev_priv))
> +		return pin < ARRAY_SIZE(gmbus_pins_bxt) &&
> +			gmbus_pins_bxt[pin].reg;
> +	else
> +		return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
>  }
>  
>  /* Intel GPIO access functions */
> @@ -196,7 +206,10 @@ intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
>  
>  	algo = &bus->bit_algo;
>  
> -	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
> +	if (IS_BROXTON(dev_priv))
> +		bus->gpio_reg = gmbus_pins_bxt[pin].reg;
> +	else
> +		bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
>  
>  	bus->adapter.algo_data = algo;
>  	algo->setsda = set_data;
> @@ -540,6 +553,8 @@ int intel_setup_gmbus(struct drm_device *dev)
>  	init_waitqueue_head(&dev_priv->gmbus_wait_queue);
>  
>  	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
> +		const char *name;
> +
>  		if (!intel_gmbus_is_valid_pin(dev_priv, pin))
>  			continue;
>  
> @@ -547,10 +562,12 @@ int intel_setup_gmbus(struct drm_device *dev)
>  
>  		bus->adapter.owner = THIS_MODULE;
>  		bus->adapter.class = I2C_CLASS_DDC;
> -		snprintf(bus->adapter.name,
> -			 sizeof(bus->adapter.name),
> -			 "i915 gmbus %s",
> -			 gmbus_pins[pin].name);
> +		if (IS_BROXTON(dev_priv))
> +			name = gmbus_pins_bxt[pin].name;
> +		else
> +			name = gmbus_pins[pin].name;
> +		snprintf(bus->adapter.name, sizeof(bus->adapter.name),
> +			 "i915 gmbus %s", name);
>  
>  		bus->adapter.dev.parent = &dev->pdev->dev;
>  		bus->dev_priv = dev_priv;
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* [PATCH] drm/i915: add bxt gmbus support
  2015-03-26 22:20 ` [PATCH 5/5] drm/i915: add bxt gmbus support Jani Nikula
  2015-03-26 22:38   ` Jani Nikula
  2015-03-27  8:42   ` Daniel Vetter
@ 2015-03-27  8:59   ` Jani Nikula
  2015-03-27  9:00   ` [PATCH v4] " Jani Nikula
  3 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2015-03-27  8:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

From: "A.Sunil Kamath" <sunil.kamath@intel.com>

For BXT gmbus is pulled from PCH to CPU. From implementation point of
view only pin pair configuration will change. The existing
implementation supports all platforms previous to GEN8 and also SKL. But
for BXT pin pair configuration is completely different than SKL or other
previous GEN's. This patch introduces the new pin pair configuration
structure specific to BXT and also ensures every real gmbus port has a
gpio pin.

v3 by Jani: with the platform independent prep work in place, the bxt
enabling reduces to a fairly trivial patch. Credits are due Sunil for
giving me the ideas (with his patches) what the platform independent
parts should look like.

v4: Fix intel_hdmi_init_connector() for bxt. Abstract gmbus_pin access
more. s/GPU/PCH/ in commit message.

Issue: VIZ-3574
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h   |  3 +++
 drivers/gpu/drm/i915/intel_hdmi.c | 14 +++++++++++---
 drivers/gpu/drm/i915/intel_i2c.c  | 30 +++++++++++++++++++++++++++---
 3 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cdc071cff001..b0dc506f2fd1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1797,6 +1797,9 @@ enum skl_disp_power_wells {
 #define   GMBUS_PIN_DPB		5 /* SDVO, HDMIB */
 #define   GMBUS_PIN_DPD		6 /* HDMID */
 #define   GMBUS_PIN_RESERVED	7 /* 7 reserved */
+#define   GMBUS_PIN_1_BXT	1
+#define   GMBUS_PIN_2_BXT	2
+#define   GMBUS_PIN_3_BXT	3
 #define   GMBUS_PIN_MAX		7 /* not inclusive */
 #define GMBUS1			0x5104 /* command/status */
 #define   GMBUS_SW_CLR_INT	(1<<31)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 26222e6c1ff3..587bab6e5f60 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1681,15 +1681,23 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 
 	switch (port) {
 	case PORT_B:
-		intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
+		if (IS_BROXTON(dev_priv))
+			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
+		else
+			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
 		intel_encoder->hpd_pin = HPD_PORT_B;
 		break;
 	case PORT_C:
-		intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
+		if (IS_BROXTON(dev_priv))
+			intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
+		else
+			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
 		intel_encoder->hpd_pin = HPD_PORT_C;
 		break;
 	case PORT_D:
-		if (IS_CHERRYVIEW(dev))
+		if (WARN_ON(IS_BROXTON(dev_priv)))
+			intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
+		else if (IS_CHERRYVIEW(dev_priv))
 			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
 		else
 			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index ec9cc8cf642e..cadbc17d2775 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -49,10 +49,33 @@ static const struct gmbus_pin gmbus_pins[] = {
 	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
 };
 
+static const struct gmbus_pin gmbus_pins_bxt[] = {
+	[GMBUS_PIN_1_BXT] = { "dpb", PCH_GPIOB },
+	[GMBUS_PIN_2_BXT] = { "dpc", PCH_GPIOC },
+	[GMBUS_PIN_3_BXT] = { "misc", PCH_GPIOD },
+};
+
+/* pin is expected to be valid */
+static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
+					     unsigned int pin)
+{
+	if (IS_BROXTON(dev_priv))
+		return &gmbus_pins_bxt[pin];
+	else
+		return &gmbus_pins[pin];
+}
+
 bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
 			      unsigned int pin)
 {
-	return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
+	unsigned int size;
+
+	if (IS_BROXTON(dev_priv))
+		size = ARRAY_SIZE(gmbus_pins_bxt);
+	else
+		size = ARRAY_SIZE(gmbus_pins);
+
+	return pin < size && get_gmbus_pin(dev_priv, pin)->reg;
 }
 
 /* Intel GPIO access functions */
@@ -196,7 +219,8 @@ intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
 
 	algo = &bus->bit_algo;
 
-	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
+	bus->gpio_reg = dev_priv->gpio_mmio_base +
+		get_gmbus_pin(dev_priv, pin)->reg;
 
 	bus->adapter.algo_data = algo;
 	algo->setsda = set_data;
@@ -550,7 +574,7 @@ int intel_setup_gmbus(struct drm_device *dev)
 		snprintf(bus->adapter.name,
 			 sizeof(bus->adapter.name),
 			 "i915 gmbus %s",
-			 gmbus_pins[pin].name);
+			 get_gmbus_pin(dev_priv, pin)->name);
 
 		bus->adapter.dev.parent = &dev->pdev->dev;
 		bus->dev_priv = dev_priv;
-- 
2.1.4

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

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

* [PATCH v4] drm/i915: add bxt gmbus support
  2015-03-26 22:20 ` [PATCH 5/5] drm/i915: add bxt gmbus support Jani Nikula
                     ` (2 preceding siblings ...)
  2015-03-27  8:59   ` [PATCH] " Jani Nikula
@ 2015-03-27  9:00   ` Jani Nikula
  2015-04-01  7:58     ` [PATCH v5] " Jani Nikula
  3 siblings, 1 reply; 19+ messages in thread
From: Jani Nikula @ 2015-03-27  9:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

From: "A.Sunil Kamath" <sunil.kamath@intel.com>

For BXT gmbus is pulled from PCH to CPU. From implementation point of
view only pin pair configuration will change. The existing
implementation supports all platforms previous to GEN8 and also SKL. But
for BXT pin pair configuration is completely different than SKL or other
previous GEN's. This patch introduces the new pin pair configuration
structure specific to BXT and also ensures every real gmbus port has a
gpio pin.

v3 by Jani: with the platform independent prep work in place, the bxt
enabling reduces to a fairly trivial patch. Credits are due Sunil for
giving me the ideas (with his patches) what the platform independent
parts should look like.

v4: Fix intel_hdmi_init_connector() for bxt. Abstract gmbus_pin access
more. s/GPU/PCH/ in commit message.

Issue: VIZ-3574
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h   |  3 +++
 drivers/gpu/drm/i915/intel_hdmi.c | 14 +++++++++++---
 drivers/gpu/drm/i915/intel_i2c.c  | 30 +++++++++++++++++++++++++++---
 3 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cdc071cff001..b0dc506f2fd1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1797,6 +1797,9 @@ enum skl_disp_power_wells {
 #define   GMBUS_PIN_DPB		5 /* SDVO, HDMIB */
 #define   GMBUS_PIN_DPD		6 /* HDMID */
 #define   GMBUS_PIN_RESERVED	7 /* 7 reserved */
+#define   GMBUS_PIN_1_BXT	1
+#define   GMBUS_PIN_2_BXT	2
+#define   GMBUS_PIN_3_BXT	3
 #define   GMBUS_PIN_MAX		7 /* not inclusive */
 #define GMBUS1			0x5104 /* command/status */
 #define   GMBUS_SW_CLR_INT	(1<<31)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 26222e6c1ff3..587bab6e5f60 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1681,15 +1681,23 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 
 	switch (port) {
 	case PORT_B:
-		intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
+		if (IS_BROXTON(dev_priv))
+			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
+		else
+			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
 		intel_encoder->hpd_pin = HPD_PORT_B;
 		break;
 	case PORT_C:
-		intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
+		if (IS_BROXTON(dev_priv))
+			intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
+		else
+			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
 		intel_encoder->hpd_pin = HPD_PORT_C;
 		break;
 	case PORT_D:
-		if (IS_CHERRYVIEW(dev))
+		if (WARN_ON(IS_BROXTON(dev_priv)))
+			intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
+		else if (IS_CHERRYVIEW(dev_priv))
 			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
 		else
 			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index ec9cc8cf642e..cadbc17d2775 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -49,10 +49,33 @@ static const struct gmbus_pin gmbus_pins[] = {
 	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
 };
 
+static const struct gmbus_pin gmbus_pins_bxt[] = {
+	[GMBUS_PIN_1_BXT] = { "dpb", PCH_GPIOB },
+	[GMBUS_PIN_2_BXT] = { "dpc", PCH_GPIOC },
+	[GMBUS_PIN_3_BXT] = { "misc", PCH_GPIOD },
+};
+
+/* pin is expected to be valid */
+static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
+					     unsigned int pin)
+{
+	if (IS_BROXTON(dev_priv))
+		return &gmbus_pins_bxt[pin];
+	else
+		return &gmbus_pins[pin];
+}
+
 bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
 			      unsigned int pin)
 {
-	return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
+	unsigned int size;
+
+	if (IS_BROXTON(dev_priv))
+		size = ARRAY_SIZE(gmbus_pins_bxt);
+	else
+		size = ARRAY_SIZE(gmbus_pins);
+
+	return pin < size && get_gmbus_pin(dev_priv, pin)->reg;
 }
 
 /* Intel GPIO access functions */
@@ -196,7 +219,8 @@ intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
 
 	algo = &bus->bit_algo;
 
-	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
+	bus->gpio_reg = dev_priv->gpio_mmio_base +
+		get_gmbus_pin(dev_priv, pin)->reg;
 
 	bus->adapter.algo_data = algo;
 	algo->setsda = set_data;
@@ -550,7 +574,7 @@ int intel_setup_gmbus(struct drm_device *dev)
 		snprintf(bus->adapter.name,
 			 sizeof(bus->adapter.name),
 			 "i915 gmbus %s",
-			 gmbus_pins[pin].name);
+			 get_gmbus_pin(dev_priv, pin)->name);
 
 		bus->adapter.dev.parent = &dev->pdev->dev;
 		bus->dev_priv = dev_priv;
-- 
2.1.4

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

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

* [PATCH v5] drm/i915: add bxt gmbus support
  2015-03-27  9:00   ` [PATCH v4] " Jani Nikula
@ 2015-04-01  7:58     ` Jani Nikula
  2015-04-09 10:33       ` shuang.he
  2015-04-09 11:12       ` Imre Deak
  0 siblings, 2 replies; 19+ messages in thread
From: Jani Nikula @ 2015-04-01  7:58 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx

For BXT gmbus is pulled from PCH to CPU. From implementation point of
view only pin pair configuration will change. The existing
implementation supports all platforms previous to GEN8 and also SKL. But
for BXT pin pair configuration is completely different than SKL or other
previous GEN's. This patch introduces the new pin pair configuration
structure specific to BXT and also ensures every real gmbus port has a
gpio pin.

v3 by Jani: with the platform independent prep work in place, the bxt
enabling reduces to a fairly trivial patch. Credits are due Sunil for
giving me the ideas (with his patches) what the platform independent
parts should look like.

v4: Fix intel_hdmi_init_connector() for bxt. Abstract gmbus_pin access
more. s/GPU/PCH/ in commit message.

v5: Rebase.

Issue: VIZ-3574
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h   |  3 +++
 drivers/gpu/drm/i915/intel_hdmi.c | 14 +++++++++++---
 drivers/gpu/drm/i915/intel_i2c.c  | 30 +++++++++++++++++++++++++++---
 3 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b134fa39c5b8..a2889013088f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1797,6 +1797,9 @@ enum skl_disp_power_wells {
 #define   GMBUS_PIN_DPB		5 /* SDVO, HDMIB */
 #define   GMBUS_PIN_DPD		6 /* HDMID */
 #define   GMBUS_PIN_RESERVED	7 /* 7 reserved */
+#define   GMBUS_PIN_1_BXT	1
+#define   GMBUS_PIN_2_BXT	2
+#define   GMBUS_PIN_3_BXT	3
 #define   GMBUS_NUM_PINS	7 /* including 0 */
 #define GMBUS1			0x5104 /* command/status */
 #define   GMBUS_SW_CLR_INT	(1<<31)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 26222e6c1ff3..587bab6e5f60 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1681,15 +1681,23 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 
 	switch (port) {
 	case PORT_B:
-		intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
+		if (IS_BROXTON(dev_priv))
+			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
+		else
+			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
 		intel_encoder->hpd_pin = HPD_PORT_B;
 		break;
 	case PORT_C:
-		intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
+		if (IS_BROXTON(dev_priv))
+			intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
+		else
+			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
 		intel_encoder->hpd_pin = HPD_PORT_C;
 		break;
 	case PORT_D:
-		if (IS_CHERRYVIEW(dev))
+		if (WARN_ON(IS_BROXTON(dev_priv)))
+			intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
+		else if (IS_CHERRYVIEW(dev_priv))
 			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
 		else
 			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index ec9cc8cf642e..cadbc17d2775 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -49,10 +49,33 @@ static const struct gmbus_pin gmbus_pins[] = {
 	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
 };
 
+static const struct gmbus_pin gmbus_pins_bxt[] = {
+	[GMBUS_PIN_1_BXT] = { "dpb", PCH_GPIOB },
+	[GMBUS_PIN_2_BXT] = { "dpc", PCH_GPIOC },
+	[GMBUS_PIN_3_BXT] = { "misc", PCH_GPIOD },
+};
+
+/* pin is expected to be valid */
+static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
+					     unsigned int pin)
+{
+	if (IS_BROXTON(dev_priv))
+		return &gmbus_pins_bxt[pin];
+	else
+		return &gmbus_pins[pin];
+}
+
 bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
 			      unsigned int pin)
 {
-	return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
+	unsigned int size;
+
+	if (IS_BROXTON(dev_priv))
+		size = ARRAY_SIZE(gmbus_pins_bxt);
+	else
+		size = ARRAY_SIZE(gmbus_pins);
+
+	return pin < size && get_gmbus_pin(dev_priv, pin)->reg;
 }
 
 /* Intel GPIO access functions */
@@ -196,7 +219,8 @@ intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
 
 	algo = &bus->bit_algo;
 
-	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
+	bus->gpio_reg = dev_priv->gpio_mmio_base +
+		get_gmbus_pin(dev_priv, pin)->reg;
 
 	bus->adapter.algo_data = algo;
 	algo->setsda = set_data;
@@ -550,7 +574,7 @@ int intel_setup_gmbus(struct drm_device *dev)
 		snprintf(bus->adapter.name,
 			 sizeof(bus->adapter.name),
 			 "i915 gmbus %s",
-			 gmbus_pins[pin].name);
+			 get_gmbus_pin(dev_priv, pin)->name);
 
 		bus->adapter.dev.parent = &dev->pdev->dev;
 		bus->dev_priv = dev_priv;
-- 
2.1.4

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

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

* Re: [PATCH v5] drm/i915: add bxt gmbus support
  2015-04-01  7:58     ` [PATCH v5] " Jani Nikula
@ 2015-04-09 10:33       ` shuang.he
  2015-04-09 11:12       ` Imre Deak
  1 sibling, 0 replies; 19+ messages in thread
From: shuang.he @ 2015-04-09 10:33 UTC (permalink / raw)
  To: shuang.he, ethan.gao, intel-gfx, jani.nikula

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6110
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  313/313              313/313
IVB                                  337/337              337/337
BYT                                  286/286              286/286
HSW                                  395/395              395/395
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v5] drm/i915: add bxt gmbus support
  2015-04-01  7:58     ` [PATCH v5] " Jani Nikula
  2015-04-09 10:33       ` shuang.he
@ 2015-04-09 11:12       ` Imre Deak
  1 sibling, 0 replies; 19+ messages in thread
From: Imre Deak @ 2015-04-09 11:12 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, 2015-04-01 at 10:58 +0300, Jani Nikula wrote:
> For BXT gmbus is pulled from PCH to CPU. From implementation point of
> view only pin pair configuration will change. The existing
> implementation supports all platforms previous to GEN8 and also SKL. But
> for BXT pin pair configuration is completely different than SKL or other
> previous GEN's. This patch introduces the new pin pair configuration
> structure specific to BXT and also ensures every real gmbus port has a
> gpio pin.
> 
> v3 by Jani: with the platform independent prep work in place, the bxt
> enabling reduces to a fairly trivial patch. Credits are due Sunil for
> giving me the ideas (with his patches) what the platform independent
> parts should look like.
> 
> v4: Fix intel_hdmi_init_connector() for bxt. Abstract gmbus_pin access
> more. s/GPU/PCH/ in commit message.
> 
> v5: Rebase.
> 
> Issue: VIZ-3574
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h   |  3 +++
>  drivers/gpu/drm/i915/intel_hdmi.c | 14 +++++++++++---
>  drivers/gpu/drm/i915/intel_i2c.c  | 30 +++++++++++++++++++++++++++---
>  3 files changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b134fa39c5b8..a2889013088f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1797,6 +1797,9 @@ enum skl_disp_power_wells {
>  #define   GMBUS_PIN_DPB		5 /* SDVO, HDMIB */
>  #define   GMBUS_PIN_DPD		6 /* HDMID */
>  #define   GMBUS_PIN_RESERVED	7 /* 7 reserved */
> +#define   GMBUS_PIN_1_BXT	1
> +#define   GMBUS_PIN_2_BXT	2
> +#define   GMBUS_PIN_3_BXT	3

I would have called these GMBUS_PIN_DDI_{B,C,A}_BXT to have a more
descriptive name (and since the pin->port mapping is fixed). Either way
the patch looks ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>

>  #define   GMBUS_NUM_PINS	7 /* including 0 */
>  #define GMBUS1			0x5104 /* command/status */
>  #define   GMBUS_SW_CLR_INT	(1<<31)
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 26222e6c1ff3..587bab6e5f60 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1681,15 +1681,23 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  
>  	switch (port) {
>  	case PORT_B:
> -		intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> +		if (IS_BROXTON(dev_priv))
> +			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> +		else
> +			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
>  		intel_encoder->hpd_pin = HPD_PORT_B;
>  		break;
>  	case PORT_C:
> -		intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> +		if (IS_BROXTON(dev_priv))
> +			intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> +		else
> +			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
>  		intel_encoder->hpd_pin = HPD_PORT_C;
>  		break;
>  	case PORT_D:
> -		if (IS_CHERRYVIEW(dev))
> +		if (WARN_ON(IS_BROXTON(dev_priv)))
> +			intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> +		else if (IS_CHERRYVIEW(dev_priv))
>  			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
>  		else
>  			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index ec9cc8cf642e..cadbc17d2775 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -49,10 +49,33 @@ static const struct gmbus_pin gmbus_pins[] = {
>  	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
>  };
>  
> +static const struct gmbus_pin gmbus_pins_bxt[] = {
> +	[GMBUS_PIN_1_BXT] = { "dpb", PCH_GPIOB },
> +	[GMBUS_PIN_2_BXT] = { "dpc", PCH_GPIOC },
> +	[GMBUS_PIN_3_BXT] = { "misc", PCH_GPIOD },
> +};
> +
> +/* pin is expected to be valid */
> +static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
> +					     unsigned int pin)
> +{
> +	if (IS_BROXTON(dev_priv))
> +		return &gmbus_pins_bxt[pin];
> +	else
> +		return &gmbus_pins[pin];
> +}
> +
>  bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
>  			      unsigned int pin)
>  {
> -	return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
> +	unsigned int size;
> +
> +	if (IS_BROXTON(dev_priv))
> +		size = ARRAY_SIZE(gmbus_pins_bxt);
> +	else
> +		size = ARRAY_SIZE(gmbus_pins);
> +
> +	return pin < size && get_gmbus_pin(dev_priv, pin)->reg;
>  }
>  
>  /* Intel GPIO access functions */
> @@ -196,7 +219,8 @@ intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
>  
>  	algo = &bus->bit_algo;
>  
> -	bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
> +	bus->gpio_reg = dev_priv->gpio_mmio_base +
> +		get_gmbus_pin(dev_priv, pin)->reg;
>  
>  	bus->adapter.algo_data = algo;
>  	algo->setsda = set_data;
> @@ -550,7 +574,7 @@ int intel_setup_gmbus(struct drm_device *dev)
>  		snprintf(bus->adapter.name,
>  			 sizeof(bus->adapter.name),
>  			 "i915 gmbus %s",
> -			 gmbus_pins[pin].name);
> +			 get_gmbus_pin(dev_priv, pin)->name);
>  
>  		bus->adapter.dev.parent = &dev->pdev->dev;
>  		bus->dev_priv = dev_priv;


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

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

* [PATCH 1/2] drm/i915: don't register nonexisting gmbus pins for bdw
  2015-03-26 22:20 [PATCH 0/5] drm/i915: gmbus pin/port cleanup and bxt enabling Jani Nikula
                   ` (4 preceding siblings ...)
  2015-03-26 22:20 ` [PATCH 5/5] drm/i915: add bxt gmbus support Jani Nikula
@ 2015-03-27 10:05 ` Jani Nikula
  2015-03-27 10:05   ` [PATCH 2/2] drm/i915: don't register nonexisting gmbus pins for skl Jani Nikula
  5 siblings, 1 reply; 19+ messages in thread
From: Jani Nikula @ 2015-03-27 10:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_i2c.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index cadbc17d2775..61931ca17cd9 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -49,6 +49,13 @@ static const struct gmbus_pin gmbus_pins[] = {
 	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
 };
 
+static const struct gmbus_pin gmbus_pins_bdw[] = {
+	[GMBUS_PIN_VGADDC] = { "vga", GPIOA },
+	[GMBUS_PIN_DPC] = { "dpc", GPIOD },
+	[GMBUS_PIN_DPB] = { "dpb", GPIOE },
+	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
+};
+
 static const struct gmbus_pin gmbus_pins_bxt[] = {
 	[GMBUS_PIN_1_BXT] = { "dpb", PCH_GPIOB },
 	[GMBUS_PIN_2_BXT] = { "dpc", PCH_GPIOC },
@@ -61,6 +68,8 @@ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
 {
 	if (IS_BROXTON(dev_priv))
 		return &gmbus_pins_bxt[pin];
+	else if (IS_BROADWELL(dev_priv))
+		return &gmbus_pins_bdw[pin];
 	else
 		return &gmbus_pins[pin];
 }
@@ -72,6 +81,8 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
 
 	if (IS_BROXTON(dev_priv))
 		size = ARRAY_SIZE(gmbus_pins_bxt);
+	else if (IS_BROADWELL(dev_priv))
+		size = ARRAY_SIZE(gmbus_pins_bdw);
 	else
 		size = ARRAY_SIZE(gmbus_pins);
 
-- 
2.1.4

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

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

* [PATCH 2/2] drm/i915: don't register nonexisting gmbus pins for skl
  2015-03-27 10:05 ` [PATCH 1/2] drm/i915: don't register nonexisting gmbus pins for bdw Jani Nikula
@ 2015-03-27 10:05   ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2015-03-27 10:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_i2c.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 61931ca17cd9..9696b738a691 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -56,6 +56,12 @@ static const struct gmbus_pin gmbus_pins_bdw[] = {
 	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
 };
 
+static const struct gmbus_pin gmbus_pins_skl[] = {
+	[GMBUS_PIN_DPC] = { "dpc", GPIOD },
+	[GMBUS_PIN_DPB] = { "dpb", GPIOE },
+	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
+};
+
 static const struct gmbus_pin gmbus_pins_bxt[] = {
 	[GMBUS_PIN_1_BXT] = { "dpb", PCH_GPIOB },
 	[GMBUS_PIN_2_BXT] = { "dpc", PCH_GPIOC },
@@ -68,6 +74,8 @@ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
 {
 	if (IS_BROXTON(dev_priv))
 		return &gmbus_pins_bxt[pin];
+	else if (IS_SKYLAKE(dev_priv))
+		return &gmbus_pins_skl[pin];
 	else if (IS_BROADWELL(dev_priv))
 		return &gmbus_pins_bdw[pin];
 	else
@@ -81,6 +89,8 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
 
 	if (IS_BROXTON(dev_priv))
 		size = ARRAY_SIZE(gmbus_pins_bxt);
+	else if (IS_SKYLAKE(dev_priv))
+		size = ARRAY_SIZE(gmbus_pins_skl);
 	else if (IS_BROADWELL(dev_priv))
 		size = ARRAY_SIZE(gmbus_pins_bdw);
 	else
-- 
2.1.4

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

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

end of thread, other threads:[~2015-04-09 11:12 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-26 22:20 [PATCH 0/5] drm/i915: gmbus pin/port cleanup and bxt enabling Jani Nikula
2015-03-26 22:20 ` [PATCH 1/5] drm/i915: rename GMBUS_PORT_* macros as GMBUS_PIN_* Jani Nikula
2015-03-26 22:20 ` [PATCH 2/5] drm/i915: refer to pin instead of port in the intel_i2c.c interfaces Jani Nikula
2015-03-26 22:20 ` [PATCH 3/5] drm/i915: index gmbus tables using the pin pair number Jani Nikula
2015-03-27 15:00   ` Ville Syrjälä
2015-03-27 16:27     ` Jani Nikula
2015-04-01  7:55     ` [PATCH v2] " Jani Nikula
2015-03-26 22:20 ` [PATCH 4/5] drm/i915: base gmbus pin validity check on the gmbus pin map array Jani Nikula
2015-04-01 12:12   ` Daniel Vetter
2015-03-26 22:20 ` [PATCH 5/5] drm/i915: add bxt gmbus support Jani Nikula
2015-03-26 22:38   ` Jani Nikula
2015-03-27  8:42   ` Daniel Vetter
2015-03-27  8:59   ` [PATCH] " Jani Nikula
2015-03-27  9:00   ` [PATCH v4] " Jani Nikula
2015-04-01  7:58     ` [PATCH v5] " Jani Nikula
2015-04-09 10:33       ` shuang.he
2015-04-09 11:12       ` Imre Deak
2015-03-27 10:05 ` [PATCH 1/2] drm/i915: don't register nonexisting gmbus pins for bdw Jani Nikula
2015-03-27 10:05   ` [PATCH 2/2] drm/i915: don't register nonexisting gmbus pins for skl Jani Nikula

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