On Mon, 08 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@xxxxxxxxx> 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@xxxxxxxxx> >> --- >> 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx