Re: [PATCH 1/2] drm/i915: Read sink_count dpcd always for short hpd

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 17 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
>
> Compliance test 4.2.2.8 requires driver to read the sink_count for
> short pulse interrupt even when the panel is not enabled.
> This patch performs the following
> a) reading sink_count by reusing intel_dp_detect_dpcd
> instead of using intel_dp_get_dpcd
> b) moving crtc enabled checks post sink_count read call
>
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |  117 ++++++++++++++++++++-------------------
>  1 file changed, 59 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b905c19..e4de8e5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4342,6 +4342,56 @@ go_again:
>  	return -EINVAL;
>  }
>  
> +/* XXX this is probably wrong for multiple downstream ports */
> +static enum drm_connector_status
> +intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> +{
> +	uint8_t *dpcd = intel_dp->dpcd;
> +	uint8_t type;
> +
> +	if (!intel_dp_get_dpcd(intel_dp))
> +		return connector_status_disconnected;
> +
> +	/* if there's no downstream port, we're done */
> +	if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
> +		return connector_status_connected;
> +
> +	/* If we're HPD-aware, SINK_COUNT changes dynamically */
> +	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> +	    intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
> +		uint8_t reg;
> +
> +		if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
> +					    &reg, 1) < 0)
> +			return connector_status_unknown;
> +
> +		return DP_GET_SINK_COUNT(reg) ? connector_status_connected
> +					      : connector_status_disconnected;
> +	}
> +
> +	/* If no HPD, poke DDC gently */
> +	if (drm_probe_ddc(&intel_dp->aux.ddc))
> +		return connector_status_connected;
> +
> +	/* Well we tried, say unknown for unreliable port types */
> +	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) {
> +		type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
> +		if (type == DP_DS_PORT_TYPE_VGA ||
> +		    type == DP_DS_PORT_TYPE_NON_EDID)
> +			return connector_status_unknown;
> +	} else {
> +		type = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> +			DP_DWN_STRM_PORT_TYPE_MASK;
> +		if (type == DP_DWN_STRM_PORT_TYPE_ANALOG ||
> +		    type == DP_DWN_STRM_PORT_TYPE_OTHER)
> +			return connector_status_unknown;
> +	}
> +
> +	/* Anything else is out of spec, warn and ignore */
> +	DRM_DEBUG_KMS("Broken DP branch device, ignoring\n");
> +	return connector_status_disconnected;
> +}

Please either a) just add a forward declaration for
intel_dp_detect_dpcd, or b) add a separate non-functional prep patch
that moves the function around. Please don't combine code movement with
functional changes.

BR,
Jani.


> +
>  /*
>   * According to DP spec
>   * 5.1.2:
> @@ -4362,21 +4412,22 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>  
>  	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>  
> -	if (!intel_encoder->base.crtc)
> +	/* 4.2.2.8 requires source to read link_status, 0 - 12 DPCD &
> +	 * sink_count even for short pulse irrespective of the sink is
> +	 * in use or not
> +	 */
> +	if (!intel_dp_get_link_status(intel_dp, link_status))
>  		return;
>  
> -	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> +	/* reuse to read both 0 - 12 DPCD & sink_count */
> +	if (intel_dp_detect_dpcd(intel_dp) != connector_status_connected)
>  		return;
>  
> -	/* Try to read receiver status if the link appears to be up */
> -	if (!intel_dp_get_link_status(intel_dp, link_status)) {
> +	if (!intel_encoder->base.crtc)
>  		return;
> -	}
>  
> -	/* Now read the DPCD to see if it's actually running */
> -	if (!intel_dp_get_dpcd(intel_dp)) {
> +	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
>  		return;
> -	}
>  
>  	/* Try to read the source of the interrupt */
>  	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> @@ -4401,56 +4452,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>  	}
>  }
>  
> -/* XXX this is probably wrong for multiple downstream ports */
> -static enum drm_connector_status
> -intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> -{
> -	uint8_t *dpcd = intel_dp->dpcd;
> -	uint8_t type;
> -
> -	if (!intel_dp_get_dpcd(intel_dp))
> -		return connector_status_disconnected;
> -
> -	/* if there's no downstream port, we're done */
> -	if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
> -		return connector_status_connected;
> -
> -	/* If we're HPD-aware, SINK_COUNT changes dynamically */
> -	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> -	    intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
> -		uint8_t reg;
> -
> -		if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
> -					    &reg, 1) < 0)
> -			return connector_status_unknown;
> -
> -		return DP_GET_SINK_COUNT(reg) ? connector_status_connected
> -					      : connector_status_disconnected;
> -	}
> -
> -	/* If no HPD, poke DDC gently */
> -	if (drm_probe_ddc(&intel_dp->aux.ddc))
> -		return connector_status_connected;
> -
> -	/* Well we tried, say unknown for unreliable port types */
> -	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) {
> -		type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
> -		if (type == DP_DS_PORT_TYPE_VGA ||
> -		    type == DP_DS_PORT_TYPE_NON_EDID)
> -			return connector_status_unknown;
> -	} else {
> -		type = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> -			DP_DWN_STRM_PORT_TYPE_MASK;
> -		if (type == DP_DWN_STRM_PORT_TYPE_ANALOG ||
> -		    type == DP_DWN_STRM_PORT_TYPE_OTHER)
> -			return connector_status_unknown;
> -	}
> -
> -	/* Anything else is out of spec, warn and ignore */
> -	DRM_DEBUG_KMS("Broken DP branch device, ignoring\n");
> -	return connector_status_disconnected;
> -}
> -
>  static enum drm_connector_status
>  edp_detect(struct intel_dp *intel_dp)
>  {
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx