[Intel-gfx] [PATCH 04/12] drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT

[Sharma, Shashank]

Regards
Shashank

On 5/2/2016 8:09 PM, Ville Syrjälä wrote:
> On Mon, May 02, 2016 at 05:33:49PM +0300, Jani Nikula wrote:
>> On Mon, 04 Apr 2016, Shashank Sharma <shashank.sharma at intel.com> wrote:
>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>>
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>>
>>> DP dual mode type 1 DVI adaptors aren't required to implement any
>>> registers, so it's a bit hard to detect them. The best way would
>>> be to check the state of the CONFIG1 pin, but we have no way to
>>> do that. So as a last resort, check the VBT to see if the HDMI
>>> port is in fact a dual mode capable DP port.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_drv.h       |  2 ++
>>>   drivers/gpu/drm/i915/intel_bios.c     | 32 ++++++++++++++++++++++++++++++++
>>>   drivers/gpu/drm/i915/intel_dp.c       |  5 +++++
>>>   drivers/gpu/drm/i915/intel_drv.h      |  1 +
>>>   drivers/gpu/drm/i915/intel_hdmi.c     | 23 +++++++++++++++++++++--
>>>   drivers/gpu/drm/i915/intel_vbt_defs.h |  3 +++
>>>   6 files changed, 64 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index f330a53..65bb83f 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -3373,6 +3373,8 @@ 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);
>>>   bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
>>>   bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
>>> +bool intel_bios_is_dp_dual_mode(struct drm_i915_private *dev_priv,
>>> +		enum port port);
>>>
>>>   /* 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 083003b..39c520a 100644
>>> --- a/drivers/gpu/drm/i915/intel_bios.c
>>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>>> @@ -1550,6 +1550,38 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
>>>   	return false;
>>>   }
>>>
>>> +bool intel_bios_is_dp_dual_mode(struct drm_i915_private *dev_priv,
>>> +				enum port port)
>>> +{
>>> +	const union child_device_config *p_child;
>>> +	int i;
>>> +	static const short port_mapping[] = {
>>> +		[PORT_B] = DVO_PORT_DPB,
>>> +		[PORT_C] = DVO_PORT_DPC,
>>> +		[PORT_D] = DVO_PORT_DPD,
>>> +		[PORT_E] = DVO_PORT_DPE,
>>> +	};
>>> +
>>> +	if (port == PORT_A || port >= ARRAY_SIZE(port_mapping))
>>> +		return false;
>>> +
>>> +	if (!dev_priv->vbt.child_dev_num)
>>> +		return false;
>>> +
>>> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>>> +		p_child = &dev_priv->vbt.child_dev[i];
>>> +
>>> +		if (p_child->common.dvo_port == port_mapping[port] &&
>>> +		    (p_child->common.device_type &
>>> +				DEVICE_TYPE_DP_DUAL_MODE_BITS) ==
>>> +			(DEVICE_TYPE_DP_DUAL_MODE &
>>> +				DEVICE_TYPE_DP_DUAL_MODE_BITS))
>>> +			return true;
>>> +	}
>>> +	return false;
>>> +}
>>> +
>>> +
>>>   /**
>>>    * intel_bios_is_dsi_present - is DSI present in VBT
>>>    * @dev_priv:	i915 device instance
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 3ff8f1d..ba4da0c 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -5007,6 +5007,11 @@ bool intel_dp_is_edp(struct drm_device *dev, enum port port)
>>>   	return intel_bios_is_port_edp(dev_priv, port);
>>>   }
>>>
>>> +bool intel_dp_is_dual_mode(struct drm_i915_private *dev_priv, enum port port)
>>> +{
>>> +	return intel_bios_is_dp_dual_mode(dev_priv, port);
>>> +}
>>
>> Just use intel_bios_is_dp_dual_mode() where you need it.
>
> That wasn't in my original patch, and the commit message lacks any
> information that the patch has been modified by someone else than the
> original author. Such things always need to be documented properly!
>
> Anyways, I should just repost my patches separately (with reviews
> comments addresses where appropriate) so that we can get the basic
> dual mode stuff in (to fix the 12bpc regressions).
>
While porting this patch, I had to do some changes, and add wrapper 
functions due to recent changes in intel_bios.c and code movement to 
intel_vbt_def.h etc, but I couldn't remember now what was the exact 
reason. So yeah, having it in the commit message would have been a 
better idea.