[Intel-gfx] [PATCH 1/6] drm/i915: Splitting intel_dp_detect

[Shubhangi Shrivastava]

On Wednesday 13 January 2016 07:03 PM, Ander Conselvan De Oliveira wrote:
> On Wed, 2016-01-13 at 13:20 +0200, Ander Conselvan De Oliveira wrote:
>> On Tue, 2016-01-05 at 18:20 +0530, Shubhangi Shrivastava wrote:
>>> intel_dp_detect() is called for not just detection but
>>> during modes enumeration as well. Repeating the whole
>>> sequence during each of these calls is wasteful and
>>> time consuming.
>>> This patch moves probing for panel, DPCD read etc done in
>>> intel_dp_detect() to a new function intel_dp_long_pulse().
>>> Note that the behavior of intel_dp_detect() is changed to
>>> report connected or disconnected depending on whether the
>>> EDID is available or not.
>>> This change will be required by further patches in the series
>>> to avoid performing duplicated DPCD operations on hotplug.
>>>
>>> v2: Moved a hunk to next patch of the series.
>>>      Moved intel_dp_unset_edid to out. (Ander)
>>> v3: Rephrased commit message and intel_dp_unset_dp() is called
>>>      within intel_dp_set_dp() to free the previous EDID. (Ander)
>>>
>>> Tested-by: Nathan D Ciobanu <nathan.d.ciobanu at intel.com>
>>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani at intel.com>
>>> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava at intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_dp.c | 56 +++++++++++++++++++++++++-------------
>>> --
>>> -
>>>   1 file changed, 35 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>>> b/drivers/gpu/drm/i915/intel_dp.c
>>> index 796e3d3..e3b4208 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -129,6 +129,7 @@ static void edp_panel_vdd_off(struct intel_dp *intel_dp,
>>> bool sync);
>>>   static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp);
>>>   static void vlv_steal_power_sequencer(struct drm_device *dev,
>>>   				      enum pipe pipe);
>>> +static void intel_dp_unset_edid(struct intel_dp *intel_dp);
>>>   
>>>   static unsigned int intel_dp_unused_lane_mask(int lane_count)
>>>   {
>>> @@ -4587,6 +4588,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
>>>   	struct intel_connector *intel_connector = intel_dp
>>> ->attached_connector;
>>>   	struct edid *edid;
>>>   
>>> +	intel_dp_unset_edid(intel_dp);
>>>   	edid = intel_dp_get_edid(intel_dp);
>>>   	intel_connector->detect_edid = edid;
>>>   
>>> @@ -4607,9 +4609,10 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>>>   	intel_dp->has_audio = false;
>>>   }
>>>   
>>> -static enum drm_connector_status
>>> -intel_dp_detect(struct drm_connector *connector, bool force)
>>> +static void
>>> +intel_dp_long_pulse(struct intel_connector *intel_connector)
>>>   {
>>> +	struct drm_connector *connector = &intel_connector->base;
>>>   	struct intel_dp *intel_dp = intel_attached_dp(connector);
>>>   	struct intel_digital_port *intel_dig_port =
>>> dp_to_dig_port(intel_dp);
>>>   	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>>> @@ -4619,17 +4622,6 @@ intel_dp_detect(struct drm_connector *connector, bool
>>> force)
>>>   	bool ret;
>>>   	u8 sink_irq_vector;
>>>   
>>> -	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>>> -		      connector->base.id, connector->name);
>>> -	intel_dp_unset_edid(intel_dp);
>>> -
>>> -	if (intel_dp->is_mst) {
>>> -		/* MST devices are disconnected from a monitor POV */
>>> -		if (intel_encoder->type != INTEL_OUTPUT_EDP)
>>> -			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>>> -		return connector_status_disconnected;
>>> -	}
>>> -
>>>   	power_domain = intel_display_port_aux_power_domain(intel_encoder);
>>>   	intel_display_power_get(to_i915(dev), power_domain);
>>>   
>>> @@ -4653,14 +4645,8 @@ intel_dp_detect(struct drm_connector *connector, bool
>>> force)
>>>   	intel_dp_probe_oui(intel_dp);
>>>   
>>>   	ret = intel_dp_probe_mst(intel_dp);
>>> -	if (ret) {
>>> -		/* if we are in MST mode then this connector
>>> -		   won't appear connected or have anything with EDID on it
>>> */
>>> -		if (intel_encoder->type != INTEL_OUTPUT_EDP)
>>> -			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>> This deletion is new in this version of the patch. I think we still need the
>> hunk above, otherwise we might not properly update the encoder type when we
>> switch from an HDMI sink connected through a level shifter to an MST sink.
>>
>> Ander
>>
Encoder type setting for MST is being done in intel_dp_detect(). So, 
don't find a need to add it here.
>>> -		status = connector_status_disconnected;
>>> +	if (ret)
>>>   		goto out;
> Also, there is no call to intel_dp_unset_edid() for this case, since the code
> will reach the label 'out' with status being connected. So in this case the
> return value of intel_dp_detect() will depend on the stale value of
> intel_dp->detect_edid.
>
> Ander
Yes.. Thats right.. Will add a call to intel_dp_unset_edid() in is_mst() 
check of intel_dp_detect().
>
>>> -	}
>>>   
>>>   	/*
>>>   	 * Clearing NACK and defer counts to get their exact values
>>> @@ -4691,8 +4677,36 @@ intel_dp_detect(struct drm_connector *connector, bool
>>> force)
>>>   	}
>>>   
>>>   out:
>>> +	if (status != connector_status_connected)
>>> +		intel_dp_unset_edid(intel_dp);
>>>   	intel_display_power_put(to_i915(dev), power_domain);
>>> -	return status;
>>> +	return;
>>> +}
>>> +
>>> +static enum drm_connector_status
>>> +intel_dp_detect(struct drm_connector *connector, bool force)
>>> +{
>>> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
>>> +	struct intel_digital_port *intel_dig_port =
>>> dp_to_dig_port(intel_dp);
>>> +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>>> +	struct intel_connector *intel_connector =
>>> to_intel_connector(connector);
>>> +
>>> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>>> +		     connector->base.id, connector->name);
>>> +
>>> +	if (intel_dp->is_mst) {
>>> +		/* MST devices are disconnected from a monitor POV */
>>> +		if (intel_encoder->type != INTEL_OUTPUT_EDP)
>>> +			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>>> +		return connector_status_disconnected;
>>> +	}
>>> +
>>> +	intel_dp_long_pulse(intel_dp->attached_connector);
>>> +
>>> +	if (intel_connector->detect_edid)
>>> +		return connector_status_connected;
>>> +	else
>>> +		return connector_status_disconnected;
>>>   }
>>>   
>>>   static void