[Intel-gfx] [PATCH 2/6] drm/i915: Cleaning up intel_dp_hpd_pulse

[Ander Conselvan De Oliveira]

On Tue, 2015-11-17 at 12:23 +0530, Shubhangi Shrivastava wrote:
> 
> On Monday 16 November 2015 08:16 PM, Ander Conselvan De Oliveira wrote:
> > On Mon, 2015-11-16 at 16:33 +0200, Ander Conselvan De Oliveira wrote:
> > > On Fri, 2015-11-06 at 17:58 +0530, Shubhangi Shrivastava wrote:
> > > > Current DP detection has DPCD operations split across
> > > > intel_dp_hpd_pulse and intel_dp_detect which contains
> > > > duplicates as well. Also intel_dp_detect is called
> > > > during modes enumeration as well which will result
> > > > in multiple dpcd operations. So this patch tries
> > > > to solve both these by bringing all DPCD operations
> > > > in one single function and make intel_dp_detect
> > > > use existing values instead of repeating same steps.
> > > > 
> > > > 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 | 19 ++++---------------
> > > >   1 file changed, 4 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index a0fe827..4e74cd6 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -4881,7 +4881,8 @@ intel_dp_detect(struct drm_connector *connector,
> > > > bool
> > > > force)
> > > >   		return connector_status_disconnected;
> > > >   	}
> > > >   
> > > > -	intel_dp_long_pulse(intel_dp->attached_connector);
> > > > +	if (force)
> > > > +		intel_dp_long_pulse(intel_dp->attached_connector);
> > > >   
> > > >   	if (intel_connector->detect_edid)
> > > >   		return connector_status_connected;
> > > > @@ -5211,21 +5212,9 @@ intel_dp_hpd_pulse(struct intel_digital_port
> > > > *intel_dig_port, bool long_hpd)
> > > >   		/* indicate that we need to restart link training */
> > > >   		intel_dp->train_set_valid = false;
> > > >   
> > > > -		if (!intel_digital_port_connected(dev_priv,
> > > > intel_dig_port))
> > > > -			goto mst_fail;
> > > > +		intel_dp_long_pulse(intel_dp->attached_connector);
> > > > +		goto put_power;
> > This skips the line that sets ret to IRQ_HANDLED, which will cause the
> > hotplug
> > code to "fall back to old school hpd".
> > 
> > Ander
> Notification of hotplug to user mode goes from i915_hotplug_work_func. 
> Returning IRQ_NONE from here will notify user mode about hotplug.

So if I understand correctly, previously if the device was connected, we
succeeded in getting the dpcd and the device was mst, there was no hotplug event
being sent. With the change above the hotplug event is generated
unconditionally, and we never do the error handling in mst_fail for the long pul
se.

Ander


> > 
> > > >   
> > > > -		if (!intel_dp_get_dpcd(intel_dp)) {
> > > > -			goto mst_fail;
> > > > -		}
> > > So we don't call this for eDP anymore on long pulse, which I assume is
> > > harmless
> > > since the bits we are reading from DPCD shouldn't change?
> > > 
> > > > -
> > > > -		intel_dp_probe_oui(intel_dp);
> > > > -
> > > > -		if (!intel_dp_probe_mst(intel_dp)) {
> > > > -			drm_modeset_lock(&dev
> > > > ->mode_config.connection_mutex, NULL);
> > > > -			intel_dp_check_link_status(intel_dp);
> > > > -			drm_modeset_unlock(&dev
> > > > ->mode_config.connection_mutex);
> > > > -			goto mst_fail;
> > > > -		}
> > > Hmm, so this is where that hunk from patch 1 I said should be a separate
> > > patch
> > > comes from. Looks like in belongs to this patch.
> > > 
> > > >   	} else {
> > > >   		if (intel_dp->is_mst) {
> > > >   			if (intel_dp_check_mst_status(intel_dp) == 
> > > > -EINVAL)
>