Thanks Reviewed-by: Rodrigo Vivi On Fri, Sep 11, 2015 at 4:38 AM Sonika Jindal wrote: > The Bspec is very clear that Live status must be checked about before > trying to read EDID over DDC channel. This patch makes sure that HDMI > EDID is read only when live status is up. > > The live status doesn't seem to perform very consistent across various > platforms when tested with different monitors. The reason behind that is > some monitors are late to provide right voltage to set live_status up. > So, after getting the interrupt, for a small duration, live status reg > fluctuates, and then settles down showing the correct staus. > > This is explained here in, in a rough way: > HPD line ________________ > |\ T1 = Monitor Hotplug causing IRQ > | \______________________________________ > | | > | | > | | T2 = Live status is stable > | | _____________________________________ > | | /| > Live status _____________|_|/ | > | | | > | | | > | | | > T0 T1 T2 > > (Between T1 and T2 Live status fluctuates or can be even low, depending on > the monitor) > > After several experiments, we have concluded that a max delay > of 30ms is enough to allow the live status to settle down with > most of the monitors. This total delay of 30ms has been split into > a resolution of 3 retries of 10ms each, for the better cases. > > This delay is kept at 30ms, keeping in consideration that, HDCP compliance > expect the HPD handler to respond a plug out in 100ms, by disabling port. > > v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions > to check digital port status. Adding a separate function to get bxt live > status (Daniel) > v3: Using intel_encoder->hpd_pin to check the live status (Siva) > Moving the live status read to intel_hdmi_probe and passing parameter > to read/not to read the edid. (me) > v4: > * Added live status check for all platforms using > intel_digital_port_connected. > * Rebased on top of Jani's DP cleanup series > * Some monitors take time in setting the live status. So retry for few > times if this is a connect HPD > v5: Removed extra "drm/i915" from commit message. Adding Shashank's sob > which was missed. > v6: Drop the (!detect_edid && !live_status check) check because for DDI > ports which are enumerated as hdmi as well as DP, we don't have a > mechanism to differentiate between DP and hdmi inside the encoder's > hot_plug. This leads to call to the hdmi's hot_plug hook for DP as well > as hdmi which leads to issues during unplug because of the above check. > v7: Make intel_digital_port_connected global in this patch, some > reformatting of while loop, adding a print when live status is not > up. (Rodrigo) > > Signed-off-by: Shashank Sharma > Signed-off-by: Sonika Jindal > --- > drivers/gpu/drm/i915/intel_dp.c | 2 +- > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_hdmi.c | 26 +++++++++++++++++++------- > 3 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > b/drivers/gpu/drm/i915/intel_dp.c > index bf17030..fedf6d1 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4695,7 +4695,7 @@ static bool bxt_digital_port_connected(struct > drm_i915_private *dev_priv, > * > * Return %true if @port is connected, %false otherwise. > */ > -static bool intel_digital_port_connected(struct drm_i915_private > *dev_priv, > +bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > struct intel_digital_port *port) > { > if (HAS_PCH_IBX(dev_priv)) > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index b6c2c20..ac6d748 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1210,6 +1210,8 @@ void intel_edp_drrs_disable(struct intel_dp > *intel_dp); > void intel_edp_drrs_invalidate(struct drm_device *dev, > unsigned frontbuffer_bits); > void intel_edp_drrs_flush(struct drm_device *dev, unsigned > frontbuffer_bits); > +bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > + struct intel_digital_port *port); > > /* intel_dp_mst.c */ > int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, > int conn_id); > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 1eda71a..d366ca5 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -1329,22 +1329,23 @@ intel_hdmi_unset_edid(struct drm_connector > *connector) > } > > static bool > -intel_hdmi_set_edid(struct drm_connector *connector) > +intel_hdmi_set_edid(struct drm_connector *connector, bool force) > { > struct drm_i915_private *dev_priv = to_i915(connector->dev); > struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); > struct intel_encoder *intel_encoder = > &hdmi_to_dig_port(intel_hdmi)->base; > enum intel_display_power_domain power_domain; > - struct edid *edid; > + struct edid *edid = NULL; > bool connected = false; > > power_domain = intel_display_port_power_domain(intel_encoder); > intel_display_power_get(dev_priv, power_domain); > > - edid = drm_get_edid(connector, > - intel_gmbus_get_adapter(dev_priv, > - intel_hdmi->ddc_bus)); > + if (force) > + edid = drm_get_edid(connector, > + intel_gmbus_get_adapter(dev_priv, > + intel_hdmi->ddc_bus)); > > intel_display_power_put(dev_priv, power_domain); > > @@ -1374,14 +1375,25 @@ void intel_hdmi_probe(struct intel_encoder > *intel_encoder) > enc_to_intel_hdmi(&intel_encoder->base); > struct intel_connector *intel_connector = > intel_hdmi->attached_connector; > + struct drm_i915_private *dev_priv = > to_i915(intel_encoder->base.dev); > + bool live_status = false; > + unsigned int retry = 3; > + > + while (!live_status && --retry) { > + live_status = intel_digital_port_connected(dev_priv, > + hdmi_to_dig_port(intel_hdmi)); > + mdelay(10); > + } > > + if (!live_status) > + DRM_DEBUG_KMS("Live status not up!"); > /* > * We are here, means there is a hotplug or a force > * detection. Clear the cached EDID and probe the > * DDC bus to check the current status of HDMI. > */ > intel_hdmi_unset_edid(&intel_connector->base); > - if (intel_hdmi_set_edid(&intel_connector->base)) > + if (intel_hdmi_set_edid(&intel_connector->base, live_status)) > DRM_DEBUG_DRIVER("DDC probe: got EDID\n"); > else > DRM_DEBUG_DRIVER("DDC probe: no EDID\n"); > @@ -1432,7 +1444,7 @@ intel_hdmi_force(struct drm_connector *connector) > if (connector->status != connector_status_connected) > return; > > - intel_hdmi_set_edid(connector); > + intel_hdmi_set_edid(connector, true); > hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI; > } > > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >