drm/i915: gmbus pin/port cleanup and bxt enabling

From: "A.Sunil Kamath" <[email protected]>

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 <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/i915_reg.h  |  3 +++
 drivers/gpu/drm/i915/intel_i2c.c | 29 +++++++++++++++++++++++------
 2 files changed, 26 insertions(+), 6 deletions(-)

drivers/gpu/drm/i915/i915_reg.h +3 -0

index cdc071cff001..b0dc506f2fd1 100644
@@ -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)

drivers/gpu/drm/i915/intel_i2c.c +23 -6

index ec9cc8cf642e..fd9cb6dcb84e 100644
@@ -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;

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

From: "A.Sunil Kamath" <[email protected]>

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 <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/i915_reg.h  |  3 +++
 drivers/gpu/drm/i915/intel_i2c.c | 29 +++++++++++++++++++++++------
 2 files changed, 26 insertions(+), 6 deletions(-)

drivers/gpu/drm/i915/i915_reg.h +3 -0

index cdc071cff001..b0dc506f2fd1 100644
@@ -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)

drivers/gpu/drm/i915/intel_i2c.c +23 -6

index ec9cc8cf642e..fd9cb6dcb84e 100644
@@ -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;

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/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.

Signed-off-by Jani Nikula <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/i915_drv.h +1 -2

index 3ba5de19e039..0c6024101eb9 100644
@@ -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. */

drivers/gpu/drm/i915/i915_reg.h +0 -0

index b6113c9a803b..cdc071cff001 100644

drivers/gpu/drm/i915/i915_reg.h +0 -0

 #define GMBUS1			0x5104 /* command/status */
 #define   GMBUS_SW_CLR_INT	(1<<31)
 #define   GMBUS_SW_RDY		(1<<30)

drivers/gpu/drm/i915/intel_i2c.c +38 -27

index b0003a2bd854..ff47a8fdcb6d 100644
@@ -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);
 	}
 }

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Quoted 28 lines · 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 <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/i915_drv.h +1 -2

index 3ba5de19e039..0c6024101eb9 100644
@@ -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];
 

drivers/gpu/drm/i915/i915_reg.h enum skl_disp_power_wells { · line 1800

Quoted 12 lines

drivers/gpu/drm/i915/i915_drv.h +0 -0

 	/** gmbus_mutex protects against concurrent usage of the single hw gmbus
 	 * controller on different i2c buses. */

drivers/gpu/drm/i915/i915_reg.h +1 -1

index b6113c9a803b..cdc071cff001 100644
@@ -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ä <[email protected]>

Quoted 158 lines

drivers/gpu/drm/i915/i915_reg.h +0 -0

 #define GMBUS1			0x5104 /* command/status */
 #define   GMBUS_SW_CLR_INT	(1<<31)
 #define   GMBUS_SW_RDY		(1<<30)

drivers/gpu/drm/i915/intel_i2c.c +38 -27

index b0003a2bd854..ff47a8fdcb6d 100644
@@ -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);
 	}
 }

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines

Ville Syrjälä
Intel OTC

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 <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/i915_drv.h +2 -4

index 0c6024101eb9..a1ba9bec8a6b 100644
@@ -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);

drivers/gpu/drm/i915/intel_bios.c +1 -1

index 333f40793435..ad2f3b0d922f 100644
@@ -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",

drivers/gpu/drm/i915/intel_dvo.c +1 -1

index 8d62272a3421..9670e3802939 100644
@@ -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;

drivers/gpu/drm/i915/intel_i2c.c +10 -4

index ff47a8fdcb6d..ec9cc8cf642e 100644
@@ -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];

drivers/gpu/drm/i915/intel_lvds.c +1 -1

index d61aa78ed7e3..314a5d56ace2 100644
@@ -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

drivers/gpu/drm/i915/intel_sdvo.c +2 -1

index 124992e48abd..b121796c86aa 100644
@@ -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;

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/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 <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/i915_reg.h +0 -0

index b134fa39c5b8..a2889013088f 100644

drivers/gpu/drm/i915/i915_reg.h +0 -0

 #define   GMBUS_NUM_PINS	7 /* including 0 */
 #define GMBUS1			0x5104 /* command/status */
 #define   GMBUS_SW_CLR_INT	(1<<31)

drivers/gpu/drm/i915/intel_hdmi.c +11 -3

index 26222e6c1ff3..587bab6e5f60 100644
@@ -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;

drivers/gpu/drm/i915/intel_i2c.c +26 -2

index ec9cc8cf642e..cadbc17d2775 100644
@@ -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;