diff mbox

[v4,1/1] drm/i915: Consult VBT "LVDS config" bits to determine whether internal LVDS is present

Message ID 20180518150138.18361-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä May 18, 2018, 3:01 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

VBT seems to have some bits to tell us whether the internal LVDS port
has something hooked up. In theory one might expect the VBT to not have
a child device for the LVDS port if there's no panel hooked up, but
in practice many VBTs still add the child device. The "LVDS config" bits
seem more reliable though, so let's check those.

So far we've used the "LVDS config" bits to check for eDP support on
ILK+, and disable the internal LVDS when the value is 3. That value
is actually documented as "Both internal LVDS and SDVO LVDS", but in
practice it looks to mean "eDP" on all the ilk+ VBTs I've seen. So let's
keep that interpretation, but for pre-ILK we will consider the value
3 to also indicate the presence of the internal LVDS.

Currently we have 25 DMI matches for the "no internal LVDS" quirk. In an
effort to reduce that let's toss in a WARN when the DMI match and VBT
both tell us that the internal LVDS is not present. The hope is that
people will report a bug, and then we can just nuke the corresponding
entry from the DMI quirk list. Credits to Jani for this idea.

v2: Split the basic int_lvds_support thing to a separate patch (Jani)
v3: Rebase
v4: Limit this to VBT version >= 134

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ondrej Zary <linux@rainbow-software.org>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c     | 28 +++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_lvds.c     |  5 ++++-
 drivers/gpu/drm/i915/intel_vbt_defs.h |  2 +-
 3 files changed, 30 insertions(+), 5 deletions(-)

Comments

Ville Syrjälä May 25, 2018, 3:02 p.m. UTC | #1 
On Fri, May 18, 2018 at 06:01:38PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> VBT seems to have some bits to tell us whether the internal LVDS port
> has something hooked up. In theory one might expect the VBT to not have
> a child device for the LVDS port if there's no panel hooked up, but
> in practice many VBTs still add the child device. The "LVDS config" bits
> seem more reliable though, so let's check those.
> 
> So far we've used the "LVDS config" bits to check for eDP support on
> ILK+, and disable the internal LVDS when the value is 3. That value
> is actually documented as "Both internal LVDS and SDVO LVDS", but in
> practice it looks to mean "eDP" on all the ilk+ VBTs I've seen. So let's
> keep that interpretation, but for pre-ILK we will consider the value
> 3 to also indicate the presence of the internal LVDS.
> 
> Currently we have 25 DMI matches for the "no internal LVDS" quirk. In an
> effort to reduce that let's toss in a WARN when the DMI match and VBT
> both tell us that the internal LVDS is not present. The hope is that
> people will report a bug, and then we can just nuke the corresponding
> entry from the DMI quirk list. Credits to Jani for this idea.
> 
> v2: Split the basic int_lvds_support thing to a separate patch (Jani)
> v3: Rebase
> v4: Limit this to VBT version >= 134
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ondrej Zary <linux@rainbow-software.org>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I picked up Jani's r-b from v3 and pushed to dinq. Let's see how badly
I'll end up regretting this one...

> ---
>  drivers/gpu/drm/i915/intel_bios.c     | 28 +++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_lvds.c     |  5 ++++-
>  drivers/gpu/drm/i915/intel_vbt_defs.h |  2 +-
>  3 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 5c863e2714ec..26b59118ef7b 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -518,9 +518,31 @@ parse_driver_features(struct drm_i915_private *dev_priv,
>  	if (!driver)
>  		return;
>  
> -	if (INTEL_GEN(dev_priv) >= 5 &&
> -	    driver->lvds_config == BDB_DRIVER_FEATURE_EDP)
> -		dev_priv->vbt.int_lvds_support = 0;
> +	if (INTEL_GEN(dev_priv) >= 5) {
> +		/*
> +		 * Note that we consider BDB_DRIVER_FEATURE_INT_SDVO_LVDS
> +		 * to mean "eDP". The VBT spec doesn't agree with that
> +		 * interpretation, but real world VBTs seem to.
> +		 */
> +		if (driver->lvds_config != BDB_DRIVER_FEATURE_INT_LVDS)
> +			dev_priv->vbt.int_lvds_support = 0;
> +	} else {
> +		/*
> +		 * FIXME it's not clear which BDB version has the LVDS config
> +		 * bits defined. Revision history in the VBT spec says:
> +		 * "0.92 | Add two definitions for VBT value of LVDS Active
> +		 *  Config (00b and 11b values defined) | 06/13/2005"
> +		 * but does not the specify the BDB version.
> +		 *
> +		 * So far version 134 (on i945gm) is the oldest VBT observed
> +		 * in the wild with the bits correctly populated. Version
> +		 * 108 (on i85x) does not have the bits correctly populated.
> +		 */
> +		if (bdb->version >= 134 &&
> +		    driver->lvds_config != BDB_DRIVER_FEATURE_INT_LVDS &&
> +		    driver->lvds_config != BDB_DRIVER_FEATURE_INT_SDVO_LVDS)
> +			dev_priv->vbt.int_lvds_support = 0;
> +	}
>  
>  	DRM_DEBUG_KMS("DRRS State Enabled:%d\n", driver->drrs_enabled);
>  	/*
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index cda46bfa7041..a78d2b181819 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -973,8 +973,11 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
>  		return;
>  
>  	/* Skip init on machines we know falsely report LVDS */
> -	if (dmi_check_system(intel_no_lvds))
> +	if (dmi_check_system(intel_no_lvds)) {
> +		WARN(!dev_priv->vbt.int_lvds_support,
> +		     "Useless DMI match. Internal LVDS support disabled by VBT\n");
>  		return;
> +	}
>  
>  	if (!dev_priv->vbt.int_lvds_support) {
>  		DRM_DEBUG_KMS("Internal LVDS support disabled by VBT\n");
> diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
> index 458468237b5f..39c804624179 100644
> --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> @@ -635,7 +635,7 @@ struct bdb_sdvo_lvds_options {
>  #define BDB_DRIVER_FEATURE_NO_LVDS		0
>  #define BDB_DRIVER_FEATURE_INT_LVDS		1
>  #define BDB_DRIVER_FEATURE_SDVO_LVDS		2
> -#define BDB_DRIVER_FEATURE_EDP			3
> +#define BDB_DRIVER_FEATURE_INT_SDVO_LVDS	3
>  
>  struct bdb_driver_features {
>  	u8 boot_dev_algorithm:1;
> -- 
> 2.16.1
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 5c863e2714ec..26b59118ef7b 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -518,9 +518,31 @@  parse_driver_features(struct drm_i915_private *dev_priv,
 	if (!driver)
 		return;
 
-	if (INTEL_GEN(dev_priv) >= 5 &&
-	    driver->lvds_config == BDB_DRIVER_FEATURE_EDP)
-		dev_priv->vbt.int_lvds_support = 0;
+	if (INTEL_GEN(dev_priv) >= 5) {
+		/*
+		 * Note that we consider BDB_DRIVER_FEATURE_INT_SDVO_LVDS
+		 * to mean "eDP". The VBT spec doesn't agree with that
+		 * interpretation, but real world VBTs seem to.
+		 */
+		if (driver->lvds_config != BDB_DRIVER_FEATURE_INT_LVDS)
+			dev_priv->vbt.int_lvds_support = 0;
+	} else {
+		/*
+		 * FIXME it's not clear which BDB version has the LVDS config
+		 * bits defined. Revision history in the VBT spec says:
+		 * "0.92 | Add two definitions for VBT value of LVDS Active
+		 *  Config (00b and 11b values defined) | 06/13/2005"
+		 * but does not the specify the BDB version.
+		 *
+		 * So far version 134 (on i945gm) is the oldest VBT observed
+		 * in the wild with the bits correctly populated. Version
+		 * 108 (on i85x) does not have the bits correctly populated.
+		 */
+		if (bdb->version >= 134 &&
+		    driver->lvds_config != BDB_DRIVER_FEATURE_INT_LVDS &&
+		    driver->lvds_config != BDB_DRIVER_FEATURE_INT_SDVO_LVDS)
+			dev_priv->vbt.int_lvds_support = 0;
+	}
 
 	DRM_DEBUG_KMS("DRRS State Enabled:%d\n", driver->drrs_enabled);
 	/*
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index cda46bfa7041..a78d2b181819 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -973,8 +973,11 @@  void intel_lvds_init(struct drm_i915_private *dev_priv)
 		return;
 
 	/* Skip init on machines we know falsely report LVDS */
-	if (dmi_check_system(intel_no_lvds))
+	if (dmi_check_system(intel_no_lvds)) {
+		WARN(!dev_priv->vbt.int_lvds_support,
+		     "Useless DMI match. Internal LVDS support disabled by VBT\n");
 		return;
+	}
 
 	if (!dev_priv->vbt.int_lvds_support) {
 		DRM_DEBUG_KMS("Internal LVDS support disabled by VBT\n");
diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
index 458468237b5f..39c804624179 100644
--- a/drivers/gpu/drm/i915/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
@@ -635,7 +635,7 @@  struct bdb_sdvo_lvds_options {
 #define BDB_DRIVER_FEATURE_NO_LVDS		0
 #define BDB_DRIVER_FEATURE_INT_LVDS		1
 #define BDB_DRIVER_FEATURE_SDVO_LVDS		2
-#define BDB_DRIVER_FEATURE_EDP			3
+#define BDB_DRIVER_FEATURE_INT_SDVO_LVDS	3
 
 struct bdb_driver_features {
 	u8 boot_dev_algorithm:1;