Message ID | 1447741601-32700-1-git-send-email-shubhangi.shrivastava@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, 2015-11-17 at 11:56 +0530, Shubhangi Shrivastava wrote: > This patch moves probing for panel, DPCD read etc to another > function intel_dp_long_pulse, while intel_dp_detect returns > the status as connected or disconnected depending on > whether the edid is available or not. May i suggest a slight rephrase? "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." I think we also need a note why the EDID change is necessary and at the same doesn't break anything. > This change will be required by further patches in the series > to avoid performing multiple DPCD operations and removing > their duplication. I found this last part confusing. Maybe replace "avoid multiple DPCD operations and removing their duplication" with "avoid duplicated DPCD operations on hotplug", since we are not going into details here. > > v2: Moved a hunk to next patch of the series. > Moved intel_dp_unset_edid to out. (Ander) > > Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 52 ++++++++++++++++++++++++++++------------ > - > 1 file changed, 36 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 8071247..dd2b9da 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4631,9 +4631,10 @@ intel_dp_power_put(struct intel_dp *dp, > intel_display_power_put(to_i915(encoder->base.dev), power_domain); > } > > -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; > @@ -4643,17 +4644,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_dp_power_get(intel_dp); > > /* Can't disconnect eDP, but you can close the lid... */ > @@ -4670,8 +4660,10 @@ intel_dp_detect(struct drm_connector *connector, bool > force) > > 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 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; > status = connector_status_disconnected; > @@ -4699,8 +4691,36 @@ intel_dp_detect(struct drm_connector *connector, bool > force) > } > > out: > + if (status != connector_status_connected) > + intel_dp_unset_edid(intel_dp); I didn't notice this at first, but intel_dp_set_edid() doesn't free the previous edid, so I think we also need to call it just before intel_dp_set_edid(). Patch looks good otherwise. Ander > intel_dp_power_put(intel_dp, 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
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 8071247..dd2b9da 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4631,9 +4631,10 @@ intel_dp_power_put(struct intel_dp *dp, intel_display_power_put(to_i915(encoder->base.dev), power_domain); } -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; @@ -4643,17 +4644,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_dp_power_get(intel_dp); /* Can't disconnect eDP, but you can close the lid... */ @@ -4670,8 +4660,10 @@ intel_dp_detect(struct drm_connector *connector, bool force) 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 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; status = connector_status_disconnected; @@ -4699,8 +4691,36 @@ intel_dp_detect(struct drm_connector *connector, bool force) } out: + if (status != connector_status_connected) + intel_dp_unset_edid(intel_dp); intel_dp_power_put(intel_dp, 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