| Message ID | 1347977073.25266.78.camel@atropine (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
At Tue, 18 Sep 2012 10:04:33 -0400, Adam Jackson wrote: > > On Tue, 2012-09-18 at 13:01 +0200, Takashi Iwai wrote: > > > > I started a patch series for this a bit ago, I'll send it on > > > momentarily. > > > > Thanks! I evaluated it now (with a typo fix suggested by Jani). > > Unfortunately, it doesn't improve the situation. > > > > The fetch of downstream ports succeeds, and it gets 0x09. So, this > > indicates again it's a VGA downstream port. But that's all, so far. > > The 0x09 is reported no matter whether the VGA cable is plugged or > > not, so this can't be used as the detection of the downstream port > > plug state. > > Sorry, there's a bug in the patch. link_configuration[0] is not > DP_SINK_COUNT, I have no idea why I thought it was. Try this on top of > the series: > > === > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 9809c53..b6b9a18 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -2098,15 +2098,22 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) > > 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 */ > hpd = !!(intel_dp->downstream_ports[0] & DP_DS_PORT_HPD); > - if (hpd && (intel_dp->link_configuration[0] & DP_SINK_COUNT_MASK)) > - return connector_status_connected; > + if (hpd) { > + uint8_t sink_count; > + if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT, > + &sink_count, 1)) > + return connector_status_unknown; > + sink_count &= DP_SINK_COUNT_MASK; > + return sink_count ? connector_status_connected > + : connector_status_disconnected; > + } > > /* If no HPD, poke DDC gently */ > if (drm_probe_ddc(&intel_dp->adapter)) > === Woohoo, the patch works! This also avoids the driver spewing tons of error messages [drm:intel_dp_i2c_aux_ch] *ERROR* too many retries, giving up and [drm:intel_dp_complete_link_train] *ERROR* failed to train DP, aborting I guess it's because now the driver detects the disconnection properly. The hotplug / -unplug also seems working. Could you brush up and resend the patches for merging to 3.7 kernel? Or, would you like to fix the multiple branch devices first? When you resubmit patches, feel free to add: Tested-by: Takashi Iwai <tiwai@suse.de> Thanks! Takashi > If that doesn't work then the HPD-capable bit is useless - or if we're > lucky just needs quirking by branch OUI - and we should just fall > through to the drm_probe_ddc() path. What is the branch OUI, anyway? > > There's a third possibility, which is that HPD does work but that we're > not doing enough to enable it. The DP 1.1a spec has a non-normative > appendix describing one way a device could go about doing that as an > optional feature, but the method described does not match how we're > currently handling sink-specific IRQs. I have no idea what the 1.2 spec > says on this point though. > > - ajax
On Tue, 2012-09-18 at 16:32 +0200, Takashi Iwai wrote: > Woohoo, the patch works! \o/ > Could you brush up and resend the patches for merging to 3.7 kernel? > Or, would you like to fix the multiple branch devices first? I'm not too worried about branch devices with multiple downstreams just yet. I don't know of any you can buy anyway. Even if you could there's some open questions about how we'd present them to the user, and the whole concept will need rethinking once we get multiple-stream support. The only case I can think of where this might not work sanely is an active DP->HDMI adaptor with sinks for both audio and video? But again, I don't think those exist. If they do you'd expect both the A and V sinks to plug simultaneously, so sink count should always be either zero or non-zero, so it should work anyway. > When you resubmit patches, feel free to add: > Tested-by: Takashi Iwai <tiwai@suse.de> Will do, thanks for testing! - ajax
At Tue, 18 Sep 2012 10:48:11 -0400, Adam Jackson wrote: > > On Tue, 2012-09-18 at 16:32 +0200, Takashi Iwai wrote: > > > Woohoo, the patch works! > > \o/ > > > Could you brush up and resend the patches for merging to 3.7 kernel? > > Or, would you like to fix the multiple branch devices first? > > I'm not too worried about branch devices with multiple downstreams just > yet. I don't know of any you can buy anyway. Even if you could there's > some open questions about how we'd present them to the user, and the > whole concept will need rethinking once we get multiple-stream support. > > The only case I can think of where this might not work sanely is an > active DP->HDMI adaptor with sinks for both audio and video? But again, > I don't think those exist. If they do you'd expect both the A and V > sinks to plug simultaneously, so sink count should always be either zero > or non-zero, so it should work anyway. FWIW, I tried an active DP->DVI adapter, and it seems working, too. But the sink count is 1, so it doesn't look like A+V sinks. Takashi
=== diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 9809c53..b6b9a18 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2098,15 +2098,22 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) 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 */ hpd = !!(intel_dp->downstream_ports[0] & DP_DS_PORT_HPD); - if (hpd && (intel_dp->link_configuration[0] & DP_SINK_COUNT_MASK)) - return connector_status_connected; + if (hpd) { + uint8_t sink_count; + if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT, + &sink_count, 1)) + return connector_status_unknown; + sink_count &= DP_SINK_COUNT_MASK; + return sink_count ? connector_status_connected + : connector_status_disconnected; + } /* If no HPD, poke DDC gently */ if (drm_probe_ddc(&intel_dp->adapter))