| Message ID | ef5f900f8b60298923bc68c42a7b11cd82391099.1391520164.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 04, 2014 at 03:40:44PM +0200, Jani Nikula wrote: > intel_dp_aux_native_read_retry() is only needed when the sink might be > asleep. Use the regular read without retries otherwise. I guess I should repeat here what I mentioned to Jani: The DP spec seems to indicate that AUX transactions should be performed only when the sink is in on/standby states. When it's in the sleep state, it may only monitor the AUX channel enough to detect something's happening. When it detects the AUX activity, it may take up to 1ms (20ms for "embedded connections") to respond. The state machine diagram shows that the only valid AUX transaction here is a write of 1h to the 600h register to wake the sink up to standby state. After that further AUX transfers can be peformed normally. And once the source is done with the AUX transfers, it can either proceed w/ link training and move the sink to the on state, or it may put it back to sleep. Currently we're doing all kinds of AUX transfers while the sink is still in the sleep state. This doesn't appear valid according to the spec, and even if it were, I think we'd need to use the _retry variant pretty much always since there would no guarantee that the sink wouldn't put the AUX channel back into the monitor-only mode between transfers. So it seems we'd need to keep better track of the sink power state while doing AUX transfers. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 33 +++++++++++++-------------------- > 1 file changed, 13 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index f1ef3d482c87..19ff1b161ffb 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -2871,9 +2871,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > /* Check if the panel supports PSR */ > memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd)); > if (is_edp(intel_dp)) { > - intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT, > - intel_dp->psr_dpcd, > - sizeof(intel_dp->psr_dpcd)); > + intel_dp_aux_native_read(intel_dp, DP_PSR_SUPPORT, > + intel_dp->psr_dpcd, > + sizeof(intel_dp->psr_dpcd)); > if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) { > dev_priv->psr.sink_support = true; > DRM_DEBUG_KMS("Detected EDP PSR Panel.\n"); > @@ -2895,9 +2895,10 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > if (intel_dp->dpcd[DP_DPCD_REV] == 0x10) > return true; /* no per-port downstream info */ > > - if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0, > - intel_dp->downstream_ports, > - DP_MAX_DOWNSTREAM_PORTS) == 0) > + if (intel_dp_aux_native_read(intel_dp, DP_DOWNSTREAM_PORT_0, > + intel_dp->downstream_ports, > + DP_MAX_DOWNSTREAM_PORTS) != > + DP_MAX_DOWNSTREAM_PORTS) > return false; /* downstream port status fetch failed */ > > return true; > @@ -2913,11 +2914,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp) > > edp_panel_vdd_on(intel_dp); > > - if (intel_dp_aux_native_read_retry(intel_dp, DP_SINK_OUI, buf, 3)) > + if (intel_dp_aux_native_read(intel_dp, DP_SINK_OUI, buf, 3) == 3) > DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n", > buf[0], buf[1], buf[2]); > > - if (intel_dp_aux_native_read_retry(intel_dp, DP_BRANCH_OUI, buf, 3)) > + if (intel_dp_aux_native_read(intel_dp, DP_BRANCH_OUI, buf, 3) == 3) > DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", > buf[0], buf[1], buf[2]); > > @@ -2953,18 +2954,11 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) > return 0; > } > > -static bool > +static inline bool > intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector) > { > - int ret; > - > - ret = intel_dp_aux_native_read_retry(intel_dp, > - DP_DEVICE_SERVICE_IRQ_VECTOR, > - sink_irq_vector, 1); > - if (!ret) > - return false; > - > - return true; > + return intel_dp_aux_native_read(intel_dp, DP_DEVICE_SERVICE_IRQ_VECTOR, > + sink_irq_vector, 1) == 1; > } > > static void > @@ -3047,8 +3041,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) > if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && > intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { > uint8_t reg; > - if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT, > - ®, 1)) > + if (intel_dp_aux_native_read(intel_dp, DP_SINK_COUNT, ®, 1) != 1) > return connector_status_unknown; > return DP_GET_SINK_COUNT(reg) ? connector_status_connected > : connector_status_disconnected; > -- > 1.7.9.5
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index f1ef3d482c87..19ff1b161ffb 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2871,9 +2871,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) /* Check if the panel supports PSR */ memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd)); if (is_edp(intel_dp)) { - intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT, - intel_dp->psr_dpcd, - sizeof(intel_dp->psr_dpcd)); + intel_dp_aux_native_read(intel_dp, DP_PSR_SUPPORT, + intel_dp->psr_dpcd, + sizeof(intel_dp->psr_dpcd)); if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) { dev_priv->psr.sink_support = true; DRM_DEBUG_KMS("Detected EDP PSR Panel.\n"); @@ -2895,9 +2895,10 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) if (intel_dp->dpcd[DP_DPCD_REV] == 0x10) return true; /* no per-port downstream info */ - if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0, - intel_dp->downstream_ports, - DP_MAX_DOWNSTREAM_PORTS) == 0) + if (intel_dp_aux_native_read(intel_dp, DP_DOWNSTREAM_PORT_0, + intel_dp->downstream_ports, + DP_MAX_DOWNSTREAM_PORTS) != + DP_MAX_DOWNSTREAM_PORTS) return false; /* downstream port status fetch failed */ return true; @@ -2913,11 +2914,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp) edp_panel_vdd_on(intel_dp); - if (intel_dp_aux_native_read_retry(intel_dp, DP_SINK_OUI, buf, 3)) + if (intel_dp_aux_native_read(intel_dp, DP_SINK_OUI, buf, 3) == 3) DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n", buf[0], buf[1], buf[2]); - if (intel_dp_aux_native_read_retry(intel_dp, DP_BRANCH_OUI, buf, 3)) + if (intel_dp_aux_native_read(intel_dp, DP_BRANCH_OUI, buf, 3) == 3) DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", buf[0], buf[1], buf[2]); @@ -2953,18 +2954,11 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) return 0; } -static bool +static inline bool intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector) { - int ret; - - ret = intel_dp_aux_native_read_retry(intel_dp, - DP_DEVICE_SERVICE_IRQ_VECTOR, - sink_irq_vector, 1); - if (!ret) - return false; - - return true; + return intel_dp_aux_native_read(intel_dp, DP_DEVICE_SERVICE_IRQ_VECTOR, + sink_irq_vector, 1) == 1; } static void @@ -3047,8 +3041,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { uint8_t reg; - if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT, - ®, 1)) + if (intel_dp_aux_native_read(intel_dp, DP_SINK_COUNT, ®, 1) != 1) return connector_status_unknown; return DP_GET_SINK_COUNT(reg) ? connector_status_connected : connector_status_disconnected;
intel_dp_aux_native_read_retry() is only needed when the sink might be asleep. Use the regular read without retries otherwise. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-)