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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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

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