Re: [PATCH] drm/i915: Don't fallback to ddc probe if downstream port is dummy

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

On Tue, 10 Sep 2013, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> If branch device advertise dummy enough sink, bail out early
> trusting to sink count instead of falling back to ddc probe
> which is deemed to fail.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=60263
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8c70a83..703767e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2786,9 +2786,15 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
>  	if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
>  		return connector_status_connected;
>  
> +	/* Analog or other */
> +#define DP_PORT_TYPE_DUMMY 0x2
> +
>  	/* If we're HPD-aware, SINK_COUNT changes dynamically */
>  	hpd = !!(intel_dp->downstream_ports[0] & DP_DS_PORT_HPD);
> -	if (hpd) {
> +
> +	/* ...and if the downstream port type is dummy enough
> +	 * dont fall into ddc probe as it will fail */
> +	if (hpd || dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_PORT_TYPE_DUMMY) {
>  		uint8_t reg;
>  		if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT,
>  						    &reg, 1))

I only started really looking into the patch, and the DP spec, now that
I tricked you into submitting it to the ml... thanks for the patch, but
I'm afraid this isn't the right thing to do. :/

The original comment above, "If we're HPD-aware, SINK_COUNT changes
dynamically", is correct, but misleading. AFAICT we can, and should,
just always read SINK_COUNT on branch devices. It should work, but it
only changes dynamically for HPD capable devices.

Further, the original code checks for HPD in ->downstream_ports, but
that's only available (and initialized!) if DPCD rev is > 0x10. That's
not checked.

Fixing this doesn't give us full support for branch devices, but would
be a baby step forward. Want to do it or shall I?

Cheers,
Jani.