Skip to contentSkip to conversation

start hiding away vbt structure from the driver

RFC Superseded Jani Nikula 2 participants 10 messages Dec 15, 2015 – Dec 16, 2015

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover start hiding away vbt structure from the driver

The VBT is a monster and it keeps growing. Originally we've extracted bits and pieces out of there, and added them cleanly to our own structures in dev_priv->vbt, with our own macros. Later on we've been slipping and we have copied stuff from VBT verbatim, using the same structs and defines as in VBT.

Start abstracting the VBT more, hiding away the VBT specific stuff that should not be fiddled with outside of intel_bios.c. Make it more explicit when we're doing so. At this stage, mostly just fork the private parts into intel_bios_private.h. As prep work, move some stuff to intel_bios.c to limit the amount of macros needing to be exposed across the whole driver.

I think this is the right direction, but this has plenty of bikeshedding potential. Therefore here's an early RFC. This is based on [1] but the conflicts are small.

BR,
Jani.

[1] http://mid.gmane.org/[email protected]

Jani Nikula (7):
drm/i915: move drmP.h include to i915_drv.h
drm/i915: move VBT based TV presence check to intel_bios.c drm/i915: move VBT based LVDS presence check to intel_bios.c drm/i915: move VBT based eDP port check to intel_bios.c drm/i915: move VBT based DSI presence check to intel_bios.c drm/i915/panel: setup pwm backlight based on connector type drm/i915/bios: hide away VBT specific things in a private bios header

 drivers/gpu/drm/i915/i915_drv.h                    |    6 +-
 drivers/gpu/drm/i915/intel_bios.c                  |  156 ++-
 drivers/gpu/drm/i915/intel_bios.h                  | 1226 ++++----------------
 .../i915/{intel_bios.h => intel_bios_private.h}    |  230 +---
 drivers/gpu/drm/i915/intel_dp.c                    |   21 +-
 drivers/gpu/drm/i915/intel_dsi.c                   |   23 +-
 drivers/gpu/drm/i915/intel_lvds.c                  |   53 +-
 drivers/gpu/drm/i915/intel_panel.c                 |    4 +-
 drivers/gpu/drm/i915/intel_tv.c                    |   43 +-
 9 files changed, 438 insertions(+), 1324 deletions(-)
 rewrite drivers/gpu/drm/i915/intel_bios.h (76%)
 rename drivers/gpu/drm/i915/{intel_bios.h => intel_bios_private.h} (81%)
Signature · 6 lines
2.1.4

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

RFC 1/7 drm/i915: move drmP.h include to i915_drv.h

The intel_bios.h header doesn't even need it, but other headers included from i915_drv.h do. Let's untangle the mess a bit.

---
 drivers/gpu/drm/i915/i915_drv.h   | 1 +
 drivers/gpu/drm/i915/intel_bios.h | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +1 -0
index 8cfac7398568..b2cf3b2c127f 100644
@@ -33,6 +33,7 @@
 #include <uapi/drm/i915_drm.h>
 #include <uapi/drm/drm_fourcc.h>
 
+#include <drm/drmP.h>
 #include "i915_reg.h"
 #include "intel_bios.h"
 #include "intel_ringbuffer.h"
drivers/gpu/drm/i915/intel_bios.h +0 -2
index 689eb42f863c..54eac1003a1e 100644
@@ -28,8 +28,6 @@
 #ifndef _I830_BIOS_H_
 #define _I830_BIOS_H_
 
-#include <drm/drmP.h>
-
 struct vbt_header {
 	u8 signature[20];		/**< Always starts with 'VBT$' */
 	u16 version;			/**< decimal */
Signature · 6 lines
2.1.4

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

RFC 2/7 drm/i915: move VBT based TV presence check to intel_bios.c

In reply to Jani Nikula
---
 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(-)
drivers/gpu/drm/i915/i915_drv.h +1 -0
index b2cf3b2c127f..a748b2c0d168 100644
@@ -3358,6 +3358,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
 /* intel_bios.c */
 int intel_bios_init(struct drm_device *dev);
 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
drivers/gpu/drm/i915/intel_bios.c +38 -0
index dc3a0fb1946c..6e3e647834af 100644
@@ -1348,3 +1348,41 @@ intel_bios_init(struct drm_device *dev)
 
 	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;
+}
drivers/gpu/drm/i915/intel_tv.c +1 -42

55 lines not on this page · raw message

Signature · 6 lines
2.1.4

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

Re: [RFC 2/7] drm/i915: move VBT based TV presence check to intel_bios.c

Quoted 25 lines · Jani Nikula
---
 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(-)
drivers/gpu/drm/i915/i915_drv.h +1 -0
index b2cf3b2c127f..a748b2c0d168 100644
@@ -3358,6 +3358,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
 /* intel_bios.c */
 int intel_bios_init(struct drm_device *dev);
 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
drivers/gpu/drm/i915/intel_bios.c +0 -0
index dc3a0fb1946c..6e3e647834af 100644
@@ -1348,3 +1348,41 @@ intel_bios_init(struct drm_device *dev)
 

drivers/gpu/drm/i915/intel_bios.c intel_bios_init(struct drm_device *dev) · line 1358

Quoted 10 lines
drivers/gpu/drm/i915/intel_bios.c +8 -0
 	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.
+ */

kerneldoc is pretty but isn't validated by 0day nor included into our docs without a stanza/section in gpu.tmpl. At least I didn't spot it, and I think it'd be good to start with that.
-Daniel

Quoted 98 lines
drivers/gpu/drm/i915/intel_bios.c +30 -0
+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;
+}
drivers/gpu/drm/i915/intel_tv.c +1 -42
index 948cbff6c62e..29e68859b9b7 100644
@@ -1529,47 +1529,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)
 {
@@ -1585,7 +1544,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;
 	}
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#4 lore raw

RFC 3/7 drm/i915: move VBT based LVDS presence check to intel_bios.c

In reply to Jani Nikula
---
 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(-)
drivers/gpu/drm/i915/i915_drv.h +1 -0
index a748b2c0d168..bb5c9e4b0af0 100644
@@ -3359,6 +3359,7 @@ extern void intel_i2c_reset(struct drm_device *dev);
 int intel_bios_init(struct drm_device *dev);
 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
drivers/gpu/drm/i915/intel_bios.c +50 -0

53 lines not on this page · raw message

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

65 lines not on this page · raw message

Signature · 6 lines
2.1.4

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

RFC 4/7 drm/i915: move VBT based eDP port check to intel_bios.c

In reply to Jani Nikula
---
 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(-)
drivers/gpu/drm/i915/i915_drv.h +1 -0
index bb5c9e4b0af0..824459c49ba6 100644
@@ -3360,6 +3360,7 @@ int intel_bios_init(struct drm_device *dev);
 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
drivers/gpu/drm/i915/intel_bios.c +33 -0
index 264451167644..bde97d4b6651 100644
@@ -1436,3 +1436,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;
+}
drivers/gpu/drm/i915/intel_dp.c +1 -20
index 31ba24114065..598fe6ff9412 100644
@@ -5070,14 +5070,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
@@ -5089,18 +5081,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
Signature · 6 lines
2.1.4

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

RFC 5/7 drm/i915: move VBT based DSI presence check to intel_bios.c

In reply to Jani Nikula
---
 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(-)
drivers/gpu/drm/i915/i915_drv.h +1 -1
index 824459c49ba6..5a7399507b84 100644
@@ -1480,7 +1480,6 @@ struct intel_vbt_data {
 
 	/* MIPI DSI */
 	struct {
-		u16 port;
 		u16 panel_id;
 		struct mipi_config *config;
 		struct mipi_pps_data *pps;
@@ -3361,6 +3360,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
drivers/gpu/drm/i915/intel_bios.c +32 -1
index bde97d4b6651..f45ebf7bdc31 100644
@@ -1153,7 +1153,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;
@@ -1469,3 +1468,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;
+}
drivers/gpu/drm/i915/intel_dsi.c +14 -9
index 44742fa2f616..696629557a99 100644
@@ -1117,9 +1117,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 {
@@ -1158,16 +1164,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) {
Signature · 6 lines
2.1.4

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

RFC 6/7 drm/i915/panel: setup pwm backlight based on connector type

In reply to 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.)

---
 drivers/gpu/drm/i915/intel_panel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_panel.c +2 -2
index 72183a057c79..381f6da6e2e9 100644
@@ -1758,7 +1758,6 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
 	struct intel_connector *intel_connector =
 		container_of(panel, struct intel_connector, panel);
 	struct drm_device *dev = intel_connector->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (IS_BROXTON(dev)) {
 		panel->backlight.setup = bxt_setup_backlight;
@@ -1785,7 +1784,8 @@ 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) || IS_CHERRYVIEW(dev)) {
-		if (dev_priv->vbt.has_mipi) {
+		if (intel_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;
Signature · 6 lines
2.1.4

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

RFC 7/7 drm/i915/bios: hide away VBT specific things in a private bios header

In reply to Jani Nikula

We've been accumulating code across the driver that depends on the VBT specific structures and defines. The VBT is an uncontrollable beast. Encourage encapsulation of the VBT data by hiding the structures and defines in a private header only to be included from intel_bios.c.

---
 drivers/gpu/drm/i915/intel_bios.c                  |    2 +-
 drivers/gpu/drm/i915/intel_bios.h                  | 1224 ++++----------------
 .../i915/{intel_bios.h => intel_bios_private.h}    |  230 +---
 3 files changed, 262 insertions(+), 1194 deletions(-)
 rewrite drivers/gpu/drm/i915/intel_bios.h (76%)
 rename drivers/gpu/drm/i915/{intel_bios.h => intel_bios_private.h} (81%)
drivers/gpu/drm/i915/intel_bios.c +1 -1
index f45ebf7bdc31..f985b6e7fb43 100644
@@ -29,7 +29,7 @@
 #include <drm/drmP.h>
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
-#include "intel_bios.h"
+#include "intel_bios_private.h"
 
 #define	SLAVE_ADDR1	0x70
 #define	SLAVE_ADDR2	0x72
drivers/gpu/drm/i915/intel_bios.h +256 -968

1,224 lines not on this page · raw message

drivers/gpu/drm/i915/intel_bios_private.h renamed from drivers/gpu/drm/i915/intel_bios.h +5 -225

269 lines not on this page · raw message

Signature · 6 lines
2.1.4

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

Re: [RFC 7/7] drm/i915/bios: hide away VBT specific things in a private bios header

Quoted 32 lines · Jani Nikula

We've been accumulating code across the driver that depends on the VBT specific structures and defines. The VBT is an uncontrollable beast. Encourage encapsulation of the VBT data by hiding the structures and defines in a private header only to be included from intel_bios.c.

---
 drivers/gpu/drm/i915/intel_bios.c                  |    2 +-
 drivers/gpu/drm/i915/intel_bios.h                  | 1224 ++++----------------
 .../i915/{intel_bios.h => intel_bios_private.h}    |  230 +---
 3 files changed, 262 insertions(+), 1194 deletions(-)
 rewrite drivers/gpu/drm/i915/intel_bios.h (76%)
 rename drivers/gpu/drm/i915/{intel_bios.h => intel_bios_private.h} (81%)
drivers/gpu/drm/i915/intel_bios.c +1 -1
index f45ebf7bdc31..f985b6e7fb43 100644
@@ -29,7 +29,7 @@
 #include <drm/drmP.h>
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
-#include "intel_bios.h"
+#include "intel_bios_private.h"
 
 #define	SLAVE_ADDR1	0x70
 #define	SLAVE_ADDR2	0x72
drivers/gpu/drm/i915/intel_bios.h +0 -0
dissimilarity index 76%
index 54eac1003a1e..929b74bf3593 100644

[snip]

Quoted 1 line

+#endif /* _INTEL_BIOS_PRIVATE_H_ */

Wrong #endif comment. And maybe we should just call this intel_vbt.h or similar, to make it a bit clearer what it's for.

Anyway I like this, but if we go to the trouble of cleaning things up I think some kerneldoc on top to explain the idea, pulled into gpu.tmpl together with the interface functions is needed.

Cheers, Daniel

Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx