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

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

[Mika Kuoppala]

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))
-- 
1.7.9.5

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

[Jani Nikula]

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.

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

[Mika Kuoppala]

Jani Nikula <jani.nikula@linux.intel.com> writes:

> 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?

As it seems you have much better grasp of what needs to be done,
please do.

-Mika