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

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

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