On Tue, Sep 15, 2015 at 09:44:20AM +0530, 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)
> v8: Rebase it on nightly which involved skipping the hot_plug hook for now
> and letting the live_status check happen in detect until the hpd handling
> part is finalized (Daniel)
>
> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> Signed-off-by: Sonika Jindal <sonika.jindal@xxxxxxxxx>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_dp.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_hdmi.c | 28 +++++++++++++++++++++-------
> 3 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a687250..a6e22dd 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4670,7 +4670,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 08ba362..1e01350 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);
> void hsw_dp_set_ddi_pll_sel(struct intel_crtc_state *pipe_config);
>
> /* intel_dp_mst.c */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index e978c59..bb33c66 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);
>
> @@ -1372,13 +1373,26 @@ static enum drm_connector_status
> intel_hdmi_detect(struct drm_connector *connector, bool force)
> {
> enum drm_connector_status status;
> + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> + struct drm_i915_private *dev_priv = to_i915(connector->dev);
> + bool live_status = false;
> + unsigned int retry = 3;
>
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> connector->base.id, connector->name);
>
> + 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!");
> +
> intel_hdmi_unset_edid(connector);
>
> - if (intel_hdmi_set_edid(connector)) {
> + if (intel_hdmi_set_edid(connector, live_status)) {
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
>
> hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> @@ -1402,7 +1416,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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
