Re: [PATCH 4/5] drm/i915: Check live status before reading edid

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jul 09, 2015 at 05:34:29PM +0530, Sonika Jindal wrote:
> Adding this for SKL onwards.
> 
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>

I think this is the critical piece really, and I'd like to roll it out for
all platforms. git has the code for all the old ones, so no big deal to
digg that out. Also we need your fix for the interrupt handling first ofc,
otherwise this won't work.

Then we can apply this and give it some good testing before we start
relying on it with caching hdmi probe status. I know this means that
things will be slower, but I've been burned a bit too much the last few
times we've tried this. And I really think we need the most amount of
testing we can get (hence rolling this out for all platforms). If your
hpd irq handling bugfix is indeed the bug that will be confirmed quickly,
otherwise it means back to debugging.

> ---
>  drivers/gpu/drm/i915/intel_hdmi.c |   49 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 1fb6919..769cf4f 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1300,6 +1300,46 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
>  	to_intel_connector(connector)->detect_edid = NULL;
>  }
>  
> +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);
> +	enum port port = intel_dig_port->port;
> +
> +	if (IS_SKYLAKE(dev)) {
> +		u32 temp = I915_READ(SDEISR);
> +
> +		switch (port) {
> +		case PORT_B:
> +			return temp & SDE_PORTB_HOTPLUG_CPT;
> +
> +		case PORT_C:
> +			return temp & SDE_PORTC_HOTPLUG_CPT;
> +
> +		case PORT_D:
> +			return temp & SDE_PORTD_HOTPLUG_CPT;
> +
> +		default:
> +			return false;
> +		}

The old code had per-platform helper functions for this instead of a big
if-ladder. I think that would look better.

Also when you digg out these old versions please also cite the commit sha1
of the patches where we had to last revert this (and explain why it's now
save).
-Daniel

> +	} else if (IS_BROXTON(dev)) {
> +		u32 temp = I915_READ(GEN8_DE_PORT_ISR);
> +
> +		switch (port) {
> +		case PORT_B:
> +			return temp & BXT_DE_PORT_HP_DDIB;
> +
> +		case PORT_C:
> +			return temp & BXT_DE_PORT_HP_DDIC;
> +
> +		default:
> +			return false;
> +
> +		}
> +	}
> +	return true;
> +}
> +
>  static bool
>  intel_hdmi_set_edid(struct drm_connector *connector)
>  {
> @@ -1308,15 +1348,16 @@ intel_hdmi_set_edid(struct drm_connector *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 (intel_hdmi_live_status(hdmi_to_dig_port(intel_hdmi)))
> +		edid = drm_get_edid(connector,
> +				intel_gmbus_get_adapter(dev_priv,
> +					intel_hdmi->ddc_bus));
>  
>  	intel_display_power_put(dev_priv, power_domain);
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx