[PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c

[PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c

[Jani Nikula]

Hide knowledge about VBT child devices in intel_bios.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_bios.c | 38 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_tv.c   | 43 +--------------------------------------
 3 files changed, 40 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 77227a39f3d5..715f200cfbf5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3397,6 +3397,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
 /* intel_bios.c */
 int intel_bios_init(struct drm_i915_private *dev_priv);
 bool intel_bios_is_valid_vbt(const void *buf, size_t size);
+bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index bf62a19c8f69..2800ae50465a 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1431,3 +1431,41 @@ intel_bios_init(struct drm_i915_private *dev_priv)
 
 	return 0;
 }
+
+/**
+ * intel_bios_is_tv_present - is integrated TV present in VBT
+ * @dev_priv:	i915 device instance
+ *
+ * Return true if TV is present. If no child devices were parsed from VBT,
+ * assume TV is present.
+ */
+bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
+{
+	union child_device_config *p_child;
+	int i;
+
+	if (!dev_priv->vbt.child_dev_num)
+		return true;
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+		p_child = dev_priv->vbt.child_dev + i;
+		/*
+		 * If the device type is not TV, continue.
+		 */
+		switch (p_child->old.device_type) {
+		case DEVICE_TYPE_INT_TV:
+		case DEVICE_TYPE_TV:
+		case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
+			break;
+		default:
+			continue;
+		}
+		/* Only when the addin_offset is non-zero, it is regarded
+		 * as present.
+		 */
+		if (p_child->old.addin_offset)
+			return true;
+	}
+
+	return false;
+}
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 5034b0055169..8417fcad02d2 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1530,47 +1530,6 @@ static const struct drm_encoder_funcs intel_tv_enc_funcs = {
 	.destroy = intel_encoder_destroy,
 };
 
-/*
- * Enumerate the child dev array parsed from VBT to check whether
- * the integrated TV is present.
- * If it is present, return 1.
- * If it is not present, return false.
- * If no child dev is parsed from VBT, it assumes that the TV is present.
- */
-static int tv_is_present_in_vbt(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	union child_device_config *p_child;
-	int i, ret;
-
-	if (!dev_priv->vbt.child_dev_num)
-		return 1;
-
-	ret = 0;
-	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
-		p_child = dev_priv->vbt.child_dev + i;
-		/*
-		 * If the device type is not TV, continue.
-		 */
-		switch (p_child->old.device_type) {
-		case DEVICE_TYPE_INT_TV:
-		case DEVICE_TYPE_TV:
-		case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
-			break;
-		default:
-			continue;
-		}
-		/* Only when the addin_offset is non-zero, it is regarded
-		 * as present.
-		 */
-		if (p_child->old.addin_offset) {
-			ret = 1;
-			break;
-		}
-	}
-	return ret;
-}
-
 void
 intel_tv_init(struct drm_device *dev)
 {
@@ -1586,7 +1545,7 @@ intel_tv_init(struct drm_device *dev)
 	if ((I915_READ(TV_CTL) & TV_FUSE_STATE_MASK) == TV_FUSE_STATE_DISABLED)
 		return;
 
-	if (!tv_is_present_in_vbt(dev)) {
+	if (!intel_bios_is_tv_present(dev_priv)) {
 		DRM_DEBUG_KMS("Integrated TV is not present.\n");
 		return;
 	}
-- 
2.1.4

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

[PATCH RESEND 2/5] drm/i915: move VBT based LVDS presence check to intel_bios.c

[Jani Nikula]

Hide knowledge about VBT child devices in intel_bios.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_bios.c | 50 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lvds.c | 53 +--------------------------------------
 3 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 715f200cfbf5..d70ef71d2538 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3398,6 +3398,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
 int intel_bios_init(struct drm_i915_private *dev_priv);
 bool intel_bios_is_valid_vbt(const void *buf, size_t size);
 bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
+bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 2800ae50465a..f3f4f8e687cf 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1469,3 +1469,53 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
 
 	return false;
 }
+
+/**
+ * intel_bios_is_lvds_present - is LVDS present in VBT
+ * @dev_priv:	i915 device instance
+ * @i2c_pin:	i2c pin for LVDS if present
+ *
+ * Return true if LVDS is present. If no child devices were parsed from VBT,
+ * assume LVDS is present.
+ */
+bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
+{
+	int i;
+
+	if (!dev_priv->vbt.child_dev_num)
+		return true;
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+		union child_device_config *uchild = dev_priv->vbt.child_dev + i;
+		struct old_child_dev_config *child = &uchild->old;
+
+		/* If the device type is not LFP, continue.
+		 * We have to check both the new identifiers as well as the
+		 * old for compatibility with some BIOSes.
+		 */
+		if (child->device_type != DEVICE_TYPE_INT_LFP &&
+		    child->device_type != DEVICE_TYPE_LFP)
+			continue;
+
+		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
+		 * the VBT correctly.  Since LVDS requires additional
+		 * information from AIM blocks, a non-zero addin offset is
+		 * a good indicator that the LVDS is actually present.
+		 */
+		if (child->addin_offset)
+			return true;
+
+		/* But even then some BIOS writers perform some black magic
+		 * and instantiate the device without reference to any
+		 * additional data.  Trust that if the VBT was written into
+		 * the OpRegion then they have validated the LVDS's existence.
+		 */
+		if (dev_priv->opregion.vbt)
+			return true;
+	}
+
+	return false;
+}
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 0da0240caf81..80f8684e5137 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -772,57 +772,6 @@ static const struct dmi_system_id intel_no_lvds[] = {
 	{ }	/* terminating entry */
 };
 
-/*
- * Enumerate the child dev array parsed from VBT to check whether
- * the LVDS is present.
- * If it is present, return 1.
- * If it is not present, return false.
- * If no child dev is parsed from VBT, it assumes that the LVDS is present.
- */
-static bool lvds_is_present_in_vbt(struct drm_device *dev,
-				   u8 *i2c_pin)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int i;
-
-	if (!dev_priv->vbt.child_dev_num)
-		return true;
-
-	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
-		union child_device_config *uchild = dev_priv->vbt.child_dev + i;
-		struct old_child_dev_config *child = &uchild->old;
-
-		/* If the device type is not LFP, continue.
-		 * We have to check both the new identifiers as well as the
-		 * old for compatibility with some BIOSes.
-		 */
-		if (child->device_type != DEVICE_TYPE_INT_LFP &&
-		    child->device_type != DEVICE_TYPE_LFP)
-			continue;
-
-		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
-		 * the VBT correctly.  Since LVDS requires additional
-		 * information from AIM blocks, a non-zero addin offset is
-		 * a good indicator that the LVDS is actually present.
-		 */
-		if (child->addin_offset)
-			return true;
-
-		/* But even then some BIOS writers perform some black magic
-		 * and instantiate the device without reference to any
-		 * additional data.  Trust that if the VBT was written into
-		 * the OpRegion then they have validated the LVDS's existence.
-		 */
-		if (dev_priv->opregion.vbt)
-			return true;
-	}
-
-	return false;
-}
-
 static int intel_dual_link_lvds_callback(const struct dmi_system_id *id)
 {
 	DRM_INFO("Forcing lvds to dual link mode on %s\n", id->ident);
@@ -979,7 +928,7 @@ void intel_lvds_init(struct drm_device *dev)
 	}
 
 	pin = GMBUS_PIN_PANEL;
-	if (!lvds_is_present_in_vbt(dev, &pin)) {
+	if (!intel_bios_is_lvds_present(dev_priv, &pin)) {
 		if ((lvds & LVDS_PORT_EN) == 0) {
 			DRM_DEBUG_KMS("LVDS is not present in VBT\n");
 			return;
-- 
2.1.4

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

[PATCH RESEND 3/5] drm/i915: move VBT based eDP port check to intel_bios.c

[Jani Nikula]

Hide knowledge about VBT child devices in intel_bios.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_bios.c | 33 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c   | 21 +--------------------
 3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d70ef71d2538..234f33708a31 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3399,6 +3399,7 @@ int intel_bios_init(struct drm_i915_private *dev_priv);
 bool intel_bios_is_valid_vbt(const void *buf, size_t size);
 bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
 bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
+bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index f3f4f8e687cf..1429d8214885 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1519,3 +1519,36 @@ bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
 
 	return false;
 }
+
+/**
+ * intel_bios_is_port_edp - is the device in given port eDP
+ * @dev_priv:	i915 device instance
+ * @port:	port to check
+ *
+ * Return true if the device in %port is eDP.
+ */
+bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
+{
+	union child_device_config *p_child;
+	static const short port_mapping[] = {
+		[PORT_B] = DVO_PORT_DPB,
+		[PORT_C] = DVO_PORT_DPC,
+		[PORT_D] = DVO_PORT_DPD,
+		[PORT_E] = DVO_PORT_DPE,
+	};
+	int i;
+
+	if (!dev_priv->vbt.child_dev_num)
+		return false;
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+		p_child = dev_priv->vbt.child_dev + i;
+
+		if (p_child->common.dvo_port == port_mapping[port] &&
+		    (p_child->common.device_type & DEVICE_TYPE_eDP_BITS) ==
+		    (DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
+			return true;
+	}
+
+	return false;
+}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a073f04a5330..9a4cfdf0cd70 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5065,14 +5065,6 @@ put_power:
 bool intel_dp_is_edp(struct drm_device *dev, enum port port)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	union child_device_config *p_child;
-	int i;
-	static const short port_mapping[] = {
-		[PORT_B] = DVO_PORT_DPB,
-		[PORT_C] = DVO_PORT_DPC,
-		[PORT_D] = DVO_PORT_DPD,
-		[PORT_E] = DVO_PORT_DPE,
-	};
 
 	/*
 	 * eDP not supported on g4x. so bail out early just
@@ -5084,18 +5076,7 @@ bool intel_dp_is_edp(struct drm_device *dev, enum port port)
 	if (port == PORT_A)
 		return true;
 
-	if (!dev_priv->vbt.child_dev_num)
-		return false;
-
-	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
-		p_child = dev_priv->vbt.child_dev + i;
-
-		if (p_child->common.dvo_port == port_mapping[port] &&
-		    (p_child->common.device_type & DEVICE_TYPE_eDP_BITS) ==
-		    (DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
-			return true;
-	}
-	return false;
+	return intel_bios_is_port_edp(dev_priv, port);
 }
 
 void
-- 
2.1.4

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

[PATCH RESEND 4/5] drm/i915: move VBT based DSI presence check to intel_bios.c

[Jani Nikula]

Hide knowledge about VBT child devices in intel_bios.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 +-
 drivers/gpu/drm/i915/intel_bios.c | 33 ++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_dsi.c  | 23 ++++++++++++++---------
 3 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 234f33708a31..cd2595c25f8d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1516,7 +1516,6 @@ struct intel_vbt_data {
 
 	/* MIPI DSI */
 	struct {
-		u16 port;
 		u16 panel_id;
 		struct mipi_config *config;
 		struct mipi_pps_data *pps;
@@ -3400,6 +3399,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size);
 bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
 bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
 bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
+bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 1429d8214885..b0cf33234c33 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1237,7 +1237,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 		    &&p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) {
 			DRM_DEBUG_KMS("Found MIPI as LFP\n");
 			dev_priv->vbt.has_mipi = 1;
-			dev_priv->vbt.dsi.port = p_child->common.dvo_port;
 		}
 
 		child_dev_ptr = dev_priv->vbt.child_dev + count;
@@ -1552,3 +1551,35 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
 
 	return false;
 }
+
+/**
+ * intel_bios_is_dsi_present - is DSI present in VBT
+ * @dev_priv:	i915 device instance
+ * @port:	port for DSI if present
+ *
+ * Return true if DSI is present, and return the port in %port.
+ */
+bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
+			       enum port *port)
+{
+	union child_device_config *p_child;
+	int i;
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+		p_child = dev_priv->vbt.child_dev + i;
+
+		if (!(p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT))
+			continue;
+
+		switch (p_child->common.dvo_port) {
+		case DVO_PORT_MIPIA:
+		case DVO_PORT_MIPIB:
+		case DVO_PORT_MIPIC:
+		case DVO_PORT_MIPID:
+			*port = p_child->common.dvo_port - DVO_PORT_MIPIA;
+			return true;
+		}
+	}
+
+	return false;
+}
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 378f879f4015..5b6ec567e3ce 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1112,9 +1112,15 @@ void intel_dsi_init(struct drm_device *dev)
 	DRM_DEBUG_KMS("\n");
 
 	/* There is no detection method for MIPI so rely on VBT */
-	if (!dev_priv->vbt.has_mipi)
+	if (!intel_bios_is_dsi_present(dev_priv, &port))
 		return;
 
+	if (port != PORT_A && port != PORT_C) {
+		DRM_DEBUG_KMS("VBT has unsupported DSI port %c\n",
+			      port_name(port));
+		return;
+	}
+
 	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
 		dev_priv->mipi_mmio_base = VLV_MIPI_BASE;
 	} else {
@@ -1153,16 +1159,15 @@ void intel_dsi_init(struct drm_device *dev)
 	intel_connector->unregister = intel_connector_unregister;
 
 	/* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */
-	if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) {
-		intel_encoder->crtc_mask = (1 << PIPE_A);
-		intel_dsi->ports = (1 << PORT_A);
-	} else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) {
-		intel_encoder->crtc_mask = (1 << PIPE_B);
-		intel_dsi->ports = (1 << PORT_C);
-	}
+	if (port == PORT_A)
+		intel_encoder->crtc_mask = 1 << PIPE_A;
+	else
+		intel_encoder->crtc_mask = 1 << PIPE_B;
 
 	if (dev_priv->vbt.dsi.config->dual_link)
-		intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
+		intel_dsi->ports = (1 << PORT_A) | (1 << PORT_C);
+	else
+		intel_dsi->ports = 1 << port;
 
 	/* Create a DSI host (and a device) for each port. */
 	for_each_dsi_port(port, intel_dsi->ports) {
-- 
2.1.4

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

[PATCH RESEND 5/5] drm/i915/panel: setup pwm backlight based on connector type

[Jani Nikula]

Use the connector type instead of VBT directly to decide which backlight
mechanism to use on VLV/CHV. (Indirectly, this is the same thing, but
hides the VBT use.)

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

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 21ee6477bf98..f7fbb7c223b1 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1745,7 +1745,7 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
 		panel->backlight.get = pch_get_backlight;
 		panel->backlight.hz_to_pwm = pch_hz_to_pwm;
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		if (dev_priv->vbt.has_mipi) {
+		if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI) {
 			panel->backlight.setup = pwm_setup_backlight;
 			panel->backlight.enable = pwm_enable_backlight;
 			panel->backlight.disable = pwm_disable_backlight;
-- 
2.1.4

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

✗ Fi.CI.BAT: failure for series starting with [RESEND,1/5] drm/i915: move VBT based TV presence check to intel_bios.c

[Patchwork]

== Summary ==

Series 3121v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/3121/revisions/1/mbox/

Test drv_module_reload_basic:
                pass       -> DMESG-WARN (ilk-hp8440p)
Test gem_mmap_gtt:
        Subgroup basic-small-bo:
                pass       -> DMESG-WARN (ilk-hp8440p)
Test gem_sync:
        Subgroup basic-bsd:
                pass       -> DMESG-FAIL (ilk-hp8440p)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (ilk-hp8440p) UNSTABLE
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-warn -> PASS       (skl-i5k-2)

bdw-nuci7        total:161  pass:152  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:164  pass:152  dwarn:0   dfail:0   fail:0   skip:12 
byt-nuc          total:164  pass:141  dwarn:0   dfail:0   fail:0   skip:23 
hsw-brixbox      total:164  pass:151  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2          total:164  pass:154  dwarn:0   dfail:0   fail:0   skip:10 
ilk-hp8440p      total:164  pass:113  dwarn:2   dfail:1   fail:0   skip:48 
ivb-t430s        total:164  pass:150  dwarn:0   dfail:0   fail:0   skip:14 
skl-i5k-2        total:164  pass:149  dwarn:1   dfail:0   fail:0   skip:14 
skl-i7k-2        total:164  pass:149  dwarn:1   dfail:0   fail:0   skip:14 
snb-dellxps      total:164  pass:142  dwarn:0   dfail:0   fail:0   skip:22 
snb-x220t        total:164  pass:142  dwarn:0   dfail:0   fail:1   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1368/

57e229193395068adcb34c5266d54194e652869f drm-intel-nightly: 2016y-02m-04d-18h-38m-55s UTC integration manifest
31400d242652b6705a98f9539c685daf4f8d46c9 drm/i915/panel: setup pwm backlight based on connector type
fb6c1a1a33ed8c55450251bace69f42b98f23e9b drm/i915: move VBT based DSI presence check to intel_bios.c
dbc3f4f96e24ae58849c4775f8c1cae3c0d70065 drm/i915: move VBT based eDP port check to intel_bios.c
3389e0bfcd2611ac99db098fddb936f9eb6f9259 drm/i915: move VBT based LVDS presence check to intel_bios.c
4a1453567944519983f85e212a8eb93084834656 drm/i915: move VBT based TV presence check to intel_bios.c

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

Re: [PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c

[Thulasimani, Sivakumar]

On 2/5/2016 4:06 PM, Jani Nikula wrote:
> Hide knowledge about VBT child devices in intel_bios.c.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h   |  1 +
>   drivers/gpu/drm/i915/intel_bios.c | 38 ++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_tv.c   | 43 +--------------------------------------
>   3 files changed, 40 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 77227a39f3d5..715f200cfbf5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3397,6 +3397,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
>   /* intel_bios.c */
>   int intel_bios_init(struct drm_i915_private *dev_priv);
>   bool intel_bios_is_valid_vbt(const void *buf, size_t size);
> +bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>   
>   /* intel_opregion.c */
>   #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index bf62a19c8f69..2800ae50465a 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1431,3 +1431,41 @@ intel_bios_init(struct drm_i915_private *dev_priv)
>   
>   	return 0;
>   }
> +
> +/**
> + * intel_bios_is_tv_present - is integrated TV present in VBT
> + * @dev_priv:	i915 device instance
> + *
> + * Return true if TV is present. If no child devices were parsed from VBT,
> + * assume TV is present.
> + */
> +bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
> +{
> +	union child_device_config *p_child;
> +	int i;
> +
> +	if (!dev_priv->vbt.child_dev_num)
> +		return true;
> +
This is old code moved here, but should we return true
when child_dev_num == 0 ? I understand we shouldn't make
functional changes when moving code but just pointing out
to check if this can be fixed in the further patches.
> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> +		p_child = dev_priv->vbt.child_dev + i;
> +		/*
> +		 * If the device type is not TV, continue.
> +		 */
> +		switch (p_child->old.device_type) {
> +		case DEVICE_TYPE_INT_TV:
> +		case DEVICE_TYPE_TV:
> +		case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
> +			break;
> +		default:
> +			continue;
> +		}
> +		/* Only when the addin_offset is non-zero, it is regarded
> +		 * as present.
> +		 */
> +		if (p_child->old.addin_offset)
> +			return true;
> +	}
> +
> +	return false;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 5034b0055169..8417fcad02d2 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1530,47 +1530,6 @@ static const struct drm_encoder_funcs intel_tv_enc_funcs = {
>   	.destroy = intel_encoder_destroy,
>   };
>   
> -/*
> - * Enumerate the child dev array parsed from VBT to check whether
> - * the integrated TV is present.
> - * If it is present, return 1.
> - * If it is not present, return false.
> - * If no child dev is parsed from VBT, it assumes that the TV is present.
> - */
> -static int tv_is_present_in_vbt(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	union child_device_config *p_child;
> -	int i, ret;
> -
> -	if (!dev_priv->vbt.child_dev_num)
> -		return 1;
> -
> -	ret = 0;
> -	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> -		p_child = dev_priv->vbt.child_dev + i;
> -		/*
> -		 * If the device type is not TV, continue.
> -		 */
> -		switch (p_child->old.device_type) {
> -		case DEVICE_TYPE_INT_TV:
> -		case DEVICE_TYPE_TV:
> -		case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
> -			break;
> -		default:
> -			continue;
> -		}
> -		/* Only when the addin_offset is non-zero, it is regarded
> -		 * as present.
> -		 */
> -		if (p_child->old.addin_offset) {
> -			ret = 1;
> -			break;
> -		}
> -	}
> -	return ret;
> -}
> -
>   void
>   intel_tv_init(struct drm_device *dev)
>   {
> @@ -1586,7 +1545,7 @@ intel_tv_init(struct drm_device *dev)
>   	if ((I915_READ(TV_CTL) & TV_FUSE_STATE_MASK) == TV_FUSE_STATE_DISABLED)
>   		return;
>   
> -	if (!tv_is_present_in_vbt(dev)) {
> +	if (!intel_bios_is_tv_present(dev_priv)) {
>   		DRM_DEBUG_KMS("Integrated TV is not present.\n");
>   		return;
>   	}
saw below line immediately after the tv_is_present_in_vbt() call.
can you move this inside the new function as well since we
consider it as well before enumerating tv encoder.

1593         /* Even if we have an encoder we may not have a connector */
1594         if (!dev_priv->vbt.int_tv_support)
1595                 return;
1596

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

Re: [PATCH RESEND 2/5] drm/i915: move VBT based LVDS presence check to intel_bios.c

[Thulasimani, Sivakumar]

On 2/5/2016 4:06 PM, Jani Nikula wrote:
> Hide knowledge about VBT child devices in intel_bios.c.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h   |  1 +
>   drivers/gpu/drm/i915/intel_bios.c | 50 ++++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_lvds.c | 53 +--------------------------------------
>   3 files changed, 52 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 715f200cfbf5..d70ef71d2538 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3398,6 +3398,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
>   int intel_bios_init(struct drm_i915_private *dev_priv);
>   bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>   bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
> +bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
>   
>   /* intel_opregion.c */
>   #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 2800ae50465a..f3f4f8e687cf 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1469,3 +1469,53 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
>   
>   	return false;
>   }
> +
> +/**
> + * intel_bios_is_lvds_present - is LVDS present in VBT
> + * @dev_priv:	i915 device instance
> + * @i2c_pin:	i2c pin for LVDS if present
> + *
> + * Return true if LVDS is present. If no child devices were parsed from VBT,
> + * assume LVDS is present.
> + */
> +bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
> +{
> +	int i;
> +
> +	if (!dev_priv->vbt.child_dev_num)
> +		return true;
> +
we check if eDP is supported through following code before the call to
this function. please add this check here so we can avoid vbt 
referencing for that.

intel_lvds.c
  972         if (HAS_PCH_SPLIT(dev)) {
  973                 if ((lvds & LVDS_DETECTED) == 0)
  974                         return;
  975                 if (dev_priv->vbt.edp_support) {
  976                         DRM_DEBUG_KMS("disable LVDS for eDP 
support\n");
  977                         return;
  978                 }
  979         }

Sivakumar
> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> +		union child_device_config *uchild = dev_priv->vbt.child_dev + i;
> +		struct old_child_dev_config *child = &uchild->old;
> +
> +		/* If the device type is not LFP, continue.
> +		 * We have to check both the new identifiers as well as the
> +		 * old for compatibility with some BIOSes.
> +		 */
> +		if (child->device_type != DEVICE_TYPE_INT_LFP &&
> +		    child->device_type != DEVICE_TYPE_LFP)
> +			continue;
> +
> +		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
> +		 * the VBT correctly.  Since LVDS requires additional
> +		 * information from AIM blocks, a non-zero addin offset is
> +		 * a good indicator that the LVDS is actually present.
> +		 */
> +		if (child->addin_offset)
> +			return true;
> +
> +		/* But even then some BIOS writers perform some black magic
> +		 * and instantiate the device without reference to any
> +		 * additional data.  Trust that if the VBT was written into
> +		 * the OpRegion then they have validated the LVDS's existence.
> +		 */
> +		if (dev_priv->opregion.vbt)
> +			return true;
> +	}
> +
> +	return false;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 0da0240caf81..80f8684e5137 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -772,57 +772,6 @@ static const struct dmi_system_id intel_no_lvds[] = {
>   	{ }	/* terminating entry */
>   };
>   
> -/*
> - * Enumerate the child dev array parsed from VBT to check whether
> - * the LVDS is present.
> - * If it is present, return 1.
> - * If it is not present, return false.
> - * If no child dev is parsed from VBT, it assumes that the LVDS is present.
> - */
> -static bool lvds_is_present_in_vbt(struct drm_device *dev,
> -				   u8 *i2c_pin)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int i;
> -
> -	if (!dev_priv->vbt.child_dev_num)
> -		return true;
> -
> -	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> -		union child_device_config *uchild = dev_priv->vbt.child_dev + i;
> -		struct old_child_dev_config *child = &uchild->old;
> -
> -		/* If the device type is not LFP, continue.
> -		 * We have to check both the new identifiers as well as the
> -		 * old for compatibility with some BIOSes.
> -		 */
> -		if (child->device_type != DEVICE_TYPE_INT_LFP &&
> -		    child->device_type != DEVICE_TYPE_LFP)
> -			continue;
> -
> -		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
> -		 * the VBT correctly.  Since LVDS requires additional
> -		 * information from AIM blocks, a non-zero addin offset is
> -		 * a good indicator that the LVDS is actually present.
> -		 */
> -		if (child->addin_offset)
> -			return true;
> -
> -		/* But even then some BIOS writers perform some black magic
> -		 * and instantiate the device without reference to any
> -		 * additional data.  Trust that if the VBT was written into
> -		 * the OpRegion then they have validated the LVDS's existence.
> -		 */
> -		if (dev_priv->opregion.vbt)
> -			return true;
> -	}
> -
> -	return false;
> -}
> -
>   static int intel_dual_link_lvds_callback(const struct dmi_system_id *id)
>   {
>   	DRM_INFO("Forcing lvds to dual link mode on %s\n", id->ident);
> @@ -979,7 +928,7 @@ void intel_lvds_init(struct drm_device *dev)
>   	}
>   
>   	pin = GMBUS_PIN_PANEL;
> -	if (!lvds_is_present_in_vbt(dev, &pin)) {
> +	if (!intel_bios_is_lvds_present(dev_priv, &pin)) {
>   		if ((lvds & LVDS_PORT_EN) == 0) {
>   			DRM_DEBUG_KMS("LVDS is not present in VBT\n");
>   			return;

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

Re: [PATCH RESEND 4/5] drm/i915: move VBT based DSI presence check to intel_bios.c

[Thulasimani, Sivakumar]

On 2/5/2016 4:06 PM, Jani Nikula wrote:
> Hide knowledge about VBT child devices in intel_bios.c.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h   |  2 +-
>   drivers/gpu/drm/i915/intel_bios.c | 33 ++++++++++++++++++++++++++++++++-
>   drivers/gpu/drm/i915/intel_dsi.c  | 23 ++++++++++++++---------
>   3 files changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 234f33708a31..cd2595c25f8d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1516,7 +1516,6 @@ struct intel_vbt_data {
>   
>   	/* MIPI DSI */
>   	struct {
> -		u16 port;
>   		u16 panel_id;
>   		struct mipi_config *config;
>   		struct mipi_pps_data *pps;
> @@ -3400,6 +3399,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>   bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>   bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
>   bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
> +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
>   
>   /* intel_opregion.c */
>   #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 1429d8214885..b0cf33234c33 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1237,7 +1237,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>   		    &&p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) {
>   			DRM_DEBUG_KMS("Found MIPI as LFP\n");
>   			dev_priv->vbt.has_mipi = 1;
> -			dev_priv->vbt.dsi.port = p_child->common.dvo_port;
>   		}
>   
>   		child_dev_ptr = dev_priv->vbt.child_dev + count;
> @@ -1552,3 +1551,35 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
>   
>   	return false;
>   }
> +
> +/**
> + * intel_bios_is_dsi_present - is DSI present in VBT
> + * @dev_priv:	i915 device instance
> + * @port:	port for DSI if present
> + *
> + * Return true if DSI is present, and return the port in %port.
> + */
> +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
> +			       enum port *port)
> +{
> +	union child_device_config *p_child;
> +	int i;
> +
> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> +		p_child = dev_priv->vbt.child_dev + i;
> +
> +		if (!(p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT))
> +			continue;
> +
> +		switch (p_child->common.dvo_port) {
> +		case DVO_PORT_MIPIA:
> +		case DVO_PORT_MIPIB:
> +		case DVO_PORT_MIPIC:
> +		case DVO_PORT_MIPID:
> +			*port = p_child->common.dvo_port - DVO_PORT_MIPIA;
> +			return true;
> +		}
> +	}
can we add platform check & vbt.has_mipi check ? it will avoid the need
to loop through the array.
Also since all platforms so far (VLV/CHV/BXT) has only MIPI A & MIPI C
is B & D needed here ? we can directly fail here for unsupported ports
rather than in dsi_init as special check.
> +
> +	return false;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 378f879f4015..5b6ec567e3ce 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1112,9 +1112,15 @@ void intel_dsi_init(struct drm_device *dev)
>   	DRM_DEBUG_KMS("\n");
>   
>   	/* There is no detection method for MIPI so rely on VBT */
> -	if (!dev_priv->vbt.has_mipi)
> +	if (!intel_bios_is_dsi_present(dev_priv, &port))
>   		return;
>   
> +	if (port != PORT_A && port != PORT_C) {
> +		DRM_DEBUG_KMS("VBT has unsupported DSI port %c\n",
> +			      port_name(port));
> +		return;
> +	}
> +
>   	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
>   		dev_priv->mipi_mmio_base = VLV_MIPI_BASE;
>   	} else {
> @@ -1153,16 +1159,15 @@ void intel_dsi_init(struct drm_device *dev)
>   	intel_connector->unregister = intel_connector_unregister;
>   
>   	/* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */
> -	if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) {
> -		intel_encoder->crtc_mask = (1 << PIPE_A);
> -		intel_dsi->ports = (1 << PORT_A);
> -	} else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) {
> -		intel_encoder->crtc_mask = (1 << PIPE_B);
> -		intel_dsi->ports = (1 << PORT_C);
> -	}
> +	if (port == PORT_A)
> +		intel_encoder->crtc_mask = 1 << PIPE_A;
> +	else
> +		intel_encoder->crtc_mask = 1 << PIPE_B;
>   
The above limitation applies only to CHT/VLV.
>   	if (dev_priv->vbt.dsi.config->dual_link)
> -		intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
> +		intel_dsi->ports = (1 << PORT_A) | (1 << PORT_C);
> +	else
> +		intel_dsi->ports = 1 << port;
>   
>   	/* Create a DSI host (and a device) for each port. */
>   	for_each_dsi_port(port, intel_dsi->ports) {

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

Re: [PATCH RESEND 2/5] drm/i915: move VBT based LVDS presence check to intel_bios.c

[Jani Nikula]

On Mon, 08 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> wrote:
> On 2/5/2016 4:06 PM, Jani Nikula wrote:
>> Hide knowledge about VBT child devices in intel_bios.c.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h   |  1 +
>>   drivers/gpu/drm/i915/intel_bios.c | 50 ++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_lvds.c | 53 +--------------------------------------
>>   3 files changed, 52 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 715f200cfbf5..d70ef71d2538 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3398,6 +3398,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
>>   int intel_bios_init(struct drm_i915_private *dev_priv);
>>   bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>>   bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>> +bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
>>   
>>   /* intel_opregion.c */
>>   #ifdef CONFIG_ACPI
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 2800ae50465a..f3f4f8e687cf 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1469,3 +1469,53 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
>>   
>>   	return false;
>>   }
>> +
>> +/**
>> + * intel_bios_is_lvds_present - is LVDS present in VBT
>> + * @dev_priv:	i915 device instance
>> + * @i2c_pin:	i2c pin for LVDS if present
>> + *
>> + * Return true if LVDS is present. If no child devices were parsed from VBT,
>> + * assume LVDS is present.
>> + */
>> +bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin)
>> +{
>> +	int i;
>> +
>> +	if (!dev_priv->vbt.child_dev_num)
>> +		return true;
>> +
> we check if eDP is supported through following code before the call to
> this function. please add this check here so we can avoid vbt 
> referencing for that.

Unfortunately, that would change the logic and I want to avoid making
that change at this point. For PCH split, we respect
dev_priv->vbt.edp_support, and bail out early, but otherwise we ignore
VBT if the BIOS has enabled LVDS anyway. I can only guess it's some
hackery to work around some weird setups out there.

BR,
Jani.


>
> intel_lvds.c
>   972         if (HAS_PCH_SPLIT(dev)) {
>   973                 if ((lvds & LVDS_DETECTED) == 0)
>   974                         return;
>   975                 if (dev_priv->vbt.edp_support) {
>   976                         DRM_DEBUG_KMS("disable LVDS for eDP 
> support\n");
>   977                         return;
>   978                 }
>   979         }
>
> Sivakumar
>> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> +		union child_device_config *uchild = dev_priv->vbt.child_dev + i;
>> +		struct old_child_dev_config *child = &uchild->old;
>> +
>> +		/* If the device type is not LFP, continue.
>> +		 * We have to check both the new identifiers as well as the
>> +		 * old for compatibility with some BIOSes.
>> +		 */
>> +		if (child->device_type != DEVICE_TYPE_INT_LFP &&
>> +		    child->device_type != DEVICE_TYPE_LFP)
>> +			continue;
>> +
>> +		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
>> +		 * the VBT correctly.  Since LVDS requires additional
>> +		 * information from AIM blocks, a non-zero addin offset is
>> +		 * a good indicator that the LVDS is actually present.
>> +		 */
>> +		if (child->addin_offset)
>> +			return true;
>> +
>> +		/* But even then some BIOS writers perform some black magic
>> +		 * and instantiate the device without reference to any
>> +		 * additional data.  Trust that if the VBT was written into
>> +		 * the OpRegion then they have validated the LVDS's existence.
>> +		 */
>> +		if (dev_priv->opregion.vbt)
>> +			return true;
>> +	}
>> +
>> +	return false;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
>> index 0da0240caf81..80f8684e5137 100644
>> --- a/drivers/gpu/drm/i915/intel_lvds.c
>> +++ b/drivers/gpu/drm/i915/intel_lvds.c
>> @@ -772,57 +772,6 @@ static const struct dmi_system_id intel_no_lvds[] = {
>>   	{ }	/* terminating entry */
>>   };
>>   
>> -/*
>> - * Enumerate the child dev array parsed from VBT to check whether
>> - * the LVDS is present.
>> - * If it is present, return 1.
>> - * If it is not present, return false.
>> - * If no child dev is parsed from VBT, it assumes that the LVDS is present.
>> - */
>> -static bool lvds_is_present_in_vbt(struct drm_device *dev,
>> -				   u8 *i2c_pin)
>> -{
>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>> -	int i;
>> -
>> -	if (!dev_priv->vbt.child_dev_num)
>> -		return true;
>> -
>> -	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> -		union child_device_config *uchild = dev_priv->vbt.child_dev + i;
>> -		struct old_child_dev_config *child = &uchild->old;
>> -
>> -		/* If the device type is not LFP, continue.
>> -		 * We have to check both the new identifiers as well as the
>> -		 * old for compatibility with some BIOSes.
>> -		 */
>> -		if (child->device_type != DEVICE_TYPE_INT_LFP &&
>> -		    child->device_type != DEVICE_TYPE_LFP)
>> -			continue;
>> -
>> -		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
>> -		 * the VBT correctly.  Since LVDS requires additional
>> -		 * information from AIM blocks, a non-zero addin offset is
>> -		 * a good indicator that the LVDS is actually present.
>> -		 */
>> -		if (child->addin_offset)
>> -			return true;
>> -
>> -		/* But even then some BIOS writers perform some black magic
>> -		 * and instantiate the device without reference to any
>> -		 * additional data.  Trust that if the VBT was written into
>> -		 * the OpRegion then they have validated the LVDS's existence.
>> -		 */
>> -		if (dev_priv->opregion.vbt)
>> -			return true;
>> -	}
>> -
>> -	return false;
>> -}
>> -
>>   static int intel_dual_link_lvds_callback(const struct dmi_system_id *id)
>>   {
>>   	DRM_INFO("Forcing lvds to dual link mode on %s\n", id->ident);
>> @@ -979,7 +928,7 @@ void intel_lvds_init(struct drm_device *dev)
>>   	}
>>   
>>   	pin = GMBUS_PIN_PANEL;
>> -	if (!lvds_is_present_in_vbt(dev, &pin)) {
>> +	if (!intel_bios_is_lvds_present(dev_priv, &pin)) {
>>   		if ((lvds & LVDS_PORT_EN) == 0) {
>>   			DRM_DEBUG_KMS("LVDS is not present in VBT\n");
>>   			return;
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH RESEND 4/5] drm/i915: move VBT based DSI presence check to intel_bios.c

[Jani Nikula]

On Mon, 08 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> wrote:
> On 2/5/2016 4:06 PM, Jani Nikula wrote:
>> Hide knowledge about VBT child devices in intel_bios.c.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h   |  2 +-
>>   drivers/gpu/drm/i915/intel_bios.c | 33 ++++++++++++++++++++++++++++++++-
>>   drivers/gpu/drm/i915/intel_dsi.c  | 23 ++++++++++++++---------
>>   3 files changed, 47 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 234f33708a31..cd2595c25f8d 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1516,7 +1516,6 @@ struct intel_vbt_data {
>>   
>>   	/* MIPI DSI */
>>   	struct {
>> -		u16 port;
>>   		u16 panel_id;
>>   		struct mipi_config *config;
>>   		struct mipi_pps_data *pps;
>> @@ -3400,6 +3399,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>>   bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>>   bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
>>   bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
>> +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
>>   
>>   /* intel_opregion.c */
>>   #ifdef CONFIG_ACPI
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 1429d8214885..b0cf33234c33 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1237,7 +1237,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>>   		    &&p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) {
>>   			DRM_DEBUG_KMS("Found MIPI as LFP\n");
>>   			dev_priv->vbt.has_mipi = 1;
>> -			dev_priv->vbt.dsi.port = p_child->common.dvo_port;
>>   		}
>>   
>>   		child_dev_ptr = dev_priv->vbt.child_dev + count;
>> @@ -1552,3 +1551,35 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
>>   
>>   	return false;
>>   }
>> +
>> +/**
>> + * intel_bios_is_dsi_present - is DSI present in VBT
>> + * @dev_priv:	i915 device instance
>> + * @port:	port for DSI if present
>> + *
>> + * Return true if DSI is present, and return the port in %port.
>> + */
>> +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
>> +			       enum port *port)
>> +{
>> +	union child_device_config *p_child;
>> +	int i;
>> +
>> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> +		p_child = dev_priv->vbt.child_dev + i;
>> +
>> +		if (!(p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT))
>> +			continue;
>> +
>> +		switch (p_child->common.dvo_port) {
>> +		case DVO_PORT_MIPIA:
>> +		case DVO_PORT_MIPIB:
>> +		case DVO_PORT_MIPIC:
>> +		case DVO_PORT_MIPID:
>> +			*port = p_child->common.dvo_port - DVO_PORT_MIPIA;
>> +			return true;
>> +		}
>> +	}
> can we add platform check & vbt.has_mipi check ? it will avoid the need
> to loop through the array.

I'm actually going to remove has_mipi altogether as unnecessary
duplication of information. We run the loop very few times anyway. Also,
I don't want to duplicate the information about platforms, that stuff is
in intel_setup_outputs and in the encoder inits. Single point of truth.

> Also since all platforms so far (VLV/CHV/BXT) has only MIPI A & MIPI C
> is B & D needed here ? we can directly fail here for unsupported ports
> rather than in dsi_init as special check.

Agreed.

>> +
>> +	return false;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 378f879f4015..5b6ec567e3ce 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -1112,9 +1112,15 @@ void intel_dsi_init(struct drm_device *dev)
>>   	DRM_DEBUG_KMS("\n");
>>   
>>   	/* There is no detection method for MIPI so rely on VBT */
>> -	if (!dev_priv->vbt.has_mipi)
>> +	if (!intel_bios_is_dsi_present(dev_priv, &port))
>>   		return;
>>   
>> +	if (port != PORT_A && port != PORT_C) {
>> +		DRM_DEBUG_KMS("VBT has unsupported DSI port %c\n",
>> +			      port_name(port));
>> +		return;
>> +	}
>> +
>>   	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
>>   		dev_priv->mipi_mmio_base = VLV_MIPI_BASE;
>>   	} else {
>> @@ -1153,16 +1159,15 @@ void intel_dsi_init(struct drm_device *dev)
>>   	intel_connector->unregister = intel_connector_unregister;
>>   
>>   	/* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */
>> -	if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) {
>> -		intel_encoder->crtc_mask = (1 << PIPE_A);
>> -		intel_dsi->ports = (1 << PORT_A);
>> -	} else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) {
>> -		intel_encoder->crtc_mask = (1 << PIPE_B);
>> -		intel_dsi->ports = (1 << PORT_C);
>> -	}
>> +	if (port == PORT_A)
>> +		intel_encoder->crtc_mask = 1 << PIPE_A;
>> +	else
>> +		intel_encoder->crtc_mask = 1 << PIPE_B;
>>   
> The above limitation applies only to CHT/VLV.

True, but I'm again reluctant to make the change at this point. It
should be added as part of BXT DSI instead.

BR,
Jani.

>>   	if (dev_priv->vbt.dsi.config->dual_link)
>> -		intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
>> +		intel_dsi->ports = (1 << PORT_A) | (1 << PORT_C);
>> +	else
>> +		intel_dsi->ports = 1 << port;
>>   
>>   	/* Create a DSI host (and a device) for each port. */
>>   	for_each_dsi_port(port, intel_dsi->ports) {
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH RESEND 1/5] drm/i915: move VBT based TV presence check to intel_bios.c

[Jani Nikula]

On Mon, 08 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> wrote:
> On 2/5/2016 4:06 PM, Jani Nikula wrote:
>> Hide knowledge about VBT child devices in intel_bios.c.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h   |  1 +
>>   drivers/gpu/drm/i915/intel_bios.c | 38 ++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_tv.c   | 43 +--------------------------------------
>>   3 files changed, 40 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 77227a39f3d5..715f200cfbf5 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3397,6 +3397,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
>>   /* intel_bios.c */
>>   int intel_bios_init(struct drm_i915_private *dev_priv);
>>   bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>> +bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>>   
>>   /* intel_opregion.c */
>>   #ifdef CONFIG_ACPI
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index bf62a19c8f69..2800ae50465a 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1431,3 +1431,41 @@ intel_bios_init(struct drm_i915_private *dev_priv)
>>   
>>   	return 0;
>>   }
>> +
>> +/**
>> + * intel_bios_is_tv_present - is integrated TV present in VBT
>> + * @dev_priv:	i915 device instance
>> + *
>> + * Return true if TV is present. If no child devices were parsed from VBT,
>> + * assume TV is present.
>> + */
>> +bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv)
>> +{
>> +	union child_device_config *p_child;
>> +	int i;
>> +
>> +	if (!dev_priv->vbt.child_dev_num)
>> +		return true;
>> +
> This is old code moved here, but should we return true
> when child_dev_num == 0 ? I understand we shouldn't make
> functional changes when moving code but just pointing out
> to check if this can be fixed in the further patches.

I have no idea, but I'm guessing it's there for historical reasons. No
reason to change it now as the code gets run only for some gen 3/4
hardware.

>> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> +		p_child = dev_priv->vbt.child_dev + i;
>> +		/*
>> +		 * If the device type is not TV, continue.
>> +		 */
>> +		switch (p_child->old.device_type) {
>> +		case DEVICE_TYPE_INT_TV:
>> +		case DEVICE_TYPE_TV:
>> +		case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
>> +			break;
>> +		default:
>> +			continue;
>> +		}
>> +		/* Only when the addin_offset is non-zero, it is regarded
>> +		 * as present.
>> +		 */
>> +		if (p_child->old.addin_offset)
>> +			return true;
>> +	}
>> +
>> +	return false;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
>> index 5034b0055169..8417fcad02d2 100644
>> --- a/drivers/gpu/drm/i915/intel_tv.c
>> +++ b/drivers/gpu/drm/i915/intel_tv.c
>> @@ -1530,47 +1530,6 @@ static const struct drm_encoder_funcs intel_tv_enc_funcs = {
>>   	.destroy = intel_encoder_destroy,
>>   };
>>   
>> -/*
>> - * Enumerate the child dev array parsed from VBT to check whether
>> - * the integrated TV is present.
>> - * If it is present, return 1.
>> - * If it is not present, return false.
>> - * If no child dev is parsed from VBT, it assumes that the TV is present.
>> - */
>> -static int tv_is_present_in_vbt(struct drm_device *dev)
>> -{
>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>> -	union child_device_config *p_child;
>> -	int i, ret;
>> -
>> -	if (!dev_priv->vbt.child_dev_num)
>> -		return 1;
>> -
>> -	ret = 0;
>> -	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> -		p_child = dev_priv->vbt.child_dev + i;
>> -		/*
>> -		 * If the device type is not TV, continue.
>> -		 */
>> -		switch (p_child->old.device_type) {
>> -		case DEVICE_TYPE_INT_TV:
>> -		case DEVICE_TYPE_TV:
>> -		case DEVICE_TYPE_TV_SVIDEO_COMPOSITE:
>> -			break;
>> -		default:
>> -			continue;
>> -		}
>> -		/* Only when the addin_offset is non-zero, it is regarded
>> -		 * as present.
>> -		 */
>> -		if (p_child->old.addin_offset) {
>> -			ret = 1;
>> -			break;
>> -		}
>> -	}
>> -	return ret;
>> -}
>> -
>>   void
>>   intel_tv_init(struct drm_device *dev)
>>   {
>> @@ -1586,7 +1545,7 @@ intel_tv_init(struct drm_device *dev)
>>   	if ((I915_READ(TV_CTL) & TV_FUSE_STATE_MASK) == TV_FUSE_STATE_DISABLED)
>>   		return;
>>   
>> -	if (!tv_is_present_in_vbt(dev)) {
>> +	if (!intel_bios_is_tv_present(dev_priv)) {
>>   		DRM_DEBUG_KMS("Integrated TV is not present.\n");
>>   		return;
>>   	}
> saw below line immediately after the tv_is_present_in_vbt() call.
> can you move this inside the new function as well since we
> consider it as well before enumerating tv encoder.

Makes sense.

BR,
Jani.

>
> 1593         /* Even if we have an encoder we may not have a connector */
> 1594         if (!dev_priv->vbt.int_tv_support)
> 1595                 return;
> 1596
>
> regards,
> Sivakumar
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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