Re: [PATCH RESEND 2/5] drm/i915: move VBT based LVDS presence check to intel_bios.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux