From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jani Nikula <jani.nikula@linux.intel.com>
Subject: Re: [PATCH] drm/i915: Don't fallback to ddc probe if
	downstream port is dummy
Date: Tue, 10 Sep 2013 20:01:06 +0300
Message-ID: <878uz4zk4t.fsf@intel.com>
References: <1378821618-5903-1-git-send-email-mika.kuoppala@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
	by gabe.freedesktop.org (Postfix) with ESMTP id 62F5CE730F
	for <intel-gfx@lists.freedesktop.org>;
	Tue, 10 Sep 2013 10:03:47 -0700 (PDT)
In-Reply-To: <1378821618-5903-1-git-send-email-mika.kuoppala@intel.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
To: Mika Kuoppala <mika.kuoppala@linux.intel.com>, intel-gfx@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.org

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.