[Intel-gfx] [PATCH] drm/i915: Check live status before reading edid

[Daniel Vetter]

On Tue, Jul 14, 2015 at 10:16:17AM +0530, Jindal, Sonika wrote:
> 
> 
> On 7/13/2015 8:25 PM, Daniel Vetter wrote:
> >On Mon, Jul 13, 2015 at 04:35:00PM +0530, Sonika Jindal wrote:
> >>Adding this for SKL onwards.
> >>
> >>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)
> >>
> >>Signed-off-by: Sonika Jindal <sonika.jindal at intel.com>
> >>---
> >>  drivers/gpu/drm/i915/intel_dp.c   |    4 ++--
> >>  drivers/gpu/drm/i915/intel_drv.h  |    2 ++
> >>  drivers/gpu/drm/i915/intel_hdmi.c |   42 +++++++++++++++++++++++++++++++++----
> >>  3 files changed, 42 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >>index 4ebfc3a..7b54b9d 100644
> >>--- a/drivers/gpu/drm/i915/intel_dp.c
> >>+++ b/drivers/gpu/drm/i915/intel_dp.c
> >>@@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
> >>  	return intel_dp_detect_dpcd(intel_dp);
> >>  }
> >>
> >>-static int g4x_digital_port_connected(struct drm_device *dev,
> >>-				       struct intel_digital_port *intel_dig_port)
> >>+int g4x_digital_port_connected(struct drm_device *dev,
> >>+			       struct intel_digital_port *intel_dig_port)
> >>  {
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >>  	uint32_t bit;
> >>diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >>index a47fbc3..30c8dd6 100644
> >>--- a/drivers/gpu/drm/i915/intel_drv.h
> >>+++ b/drivers/gpu/drm/i915/intel_drv.h
> >>@@ -1187,6 +1187,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);
> >>+int g4x_digital_port_connected(struct drm_device *dev,
> >>+			       struct intel_digital_port *intel_dig_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 1fb6919..7eb0d0e 100644
> >>--- a/drivers/gpu/drm/i915/intel_hdmi.c
> >>+++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >>@@ -1300,6 +1300,39 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
> >>  	to_intel_connector(connector)->detect_edid = NULL;
> >>  }
> >>
> >>+static bool bxt_port_connected(struct drm_i915_private *dev_priv,
> >>+			       struct intel_digital_port *port)
> >>+{
> >>+	u32 temp = I915_READ(GEN8_DE_PORT_ISR);
> >>+
> >>+	switch (port->port) {
> >>+	case PORT_B:
> >>+		return temp & BXT_DE_PORT_HP_DDIB;
> >>+
> >>+	case PORT_C:
> >>+		return temp & BXT_DE_PORT_HP_DDIC;
> >>+
> >>+	default:
> >>+		return false;
> >>+
> >>+	}
> >>+}
> >>+
> >>+static bool intel_hdmi_live_status(struct intel_digital_port *intel_dig_port)
> >>+{
> >>+	struct drm_device *dev = intel_dig_port->base.base.dev;
> >>+	struct drm_i915_private *dev_priv = to_i915(dev);
> >>+
> >>+	if (IS_VALLEYVIEW(dev))
> >>+		return g4x_digital_port_connected(dev, intel_dig_port);
> >>+	else if (IS_SKYLAKE(dev))
> >>+		return ibx_digital_port_connected(dev_priv, intel_dig_port);
> >>+	else if (IS_BROXTON(dev))
> >>+		return bxt_port_connected(dev_priv, intel_dig_port);
> >
> >Really I want this to be rolled out for all platforms, together with the
> >fix for handling hpd interrupts. Together with a reference to the old
> >commits we had to revert because it didn't work.
> >
> >I want to test this on my ivb (since that's the machine where I can
> >readily reproduce this bug), and if it still doesn't work there I won't
> >take this.
> >-Daniel
> Is there a formal process to raise a test for hpd on all platforms which
> might be affected by this?

Get it merged and wait for the regression reports to come in or not. The
entire problem I'm trying to explain here is that these hpd problems where
_not_ detected internally here at Intel, but only reported by external
people. Only later on did I come across a machine (by accident) which
seems to exhibit the same problem.

That's also why I want to enable it everywhere, increases the chances
we'll get a negative report if it doesn't work.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch