| Message ID | 20180508140835.20222-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 08 May 2018, Ville Syrjala <ville.syrjala@linux.intel.com> 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 > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Ondrej Zary <linux@rainbow-software.org> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105468 > References: https://lists.freedesktop.org/archives/intel-gfx/2018-March/158408.html > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/intel_bios.c | 16 +++++++++++++--- > drivers/gpu/drm/i915/intel_lvds.c | 5 ++++- > drivers/gpu/drm/i915/intel_vbt_defs.h | 2 +- > 3 files changed, 18 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 62cc9d3e20f5..f3545b5aabbc 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -518,9 +518,19 @@ 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 { > + if (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 05d012358df8..5e5a77927369 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -962,8 +962,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;
On Tue, May 08, 2018 at 05:08:35PM +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 > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Ondrej Zary <linux@rainbow-software.org> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105468 > References: https://lists.freedesktop.org/archives/intel-gfx/2018-March/158408.html > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_bios.c | 16 +++++++++++++--- > drivers/gpu/drm/i915/intel_lvds.c | 5 ++++- > drivers/gpu/drm/i915/intel_vbt_defs.h | 2 +- > 3 files changed, 18 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 62cc9d3e20f5..f3545b5aabbc 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -518,9 +518,19 @@ 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 { > + if (driver->lvds_config != BDB_DRIVER_FEATURE_INT_LVDS && > + driver->lvds_config != BDB_DRIVER_FEATURE_INT_SDVO_LVDS) > + dev_priv->vbt.int_lvds_support = 0; Ugh. Actually we can't do this. I have a 85x machine myself that has an LVDS panel but the VBT LVDS config bits set to 0 :( Looking at the revision history in some VBT spec, I see a note "0.92 | Add two definitions for VBT value of LVDS Active Config (00b and 11b values defined) | 06/13/2005". Unfortunately no information which VBT version this corresponds with. I do have a 945gm machine with VBT version 134 that does have the LVDS config bits correctly populated. I believe the 2005 date does correspond pretty well with the release of 945 chipsets, so 945gm might be the first platform where we can actually trust this. So I think we either have to just scrap this patch or limit it either to 945+ or VBT versions >= 134 (and we could refine that later if we discover older versions that have the bits populated already). As to the bug report that started this whole thing, I think I'll just go ahead an apply the DMI quirk patch as that's a 85x machine and clearly we can't use the VBT information there. > + } > > 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 05d012358df8..5e5a77927369 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -962,8 +962,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 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 62cc9d3e20f5..f3545b5aabbc 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -518,9 +518,19 @@ 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 { + if (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 05d012358df8..5e5a77927369 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -962,8 +962,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;