| Message ID | 1458229245-8634-2-git-send-email-cpaul@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 17 Mar 2016, Lyude <cpaul@redhat.com> wrote: > Since we've fixed up drm_dp_dpcd_read() to allow for retries when things > timeout, there's no use for having this function anymore. Good riddens. > > Signed-off-by: Lyude <cpaul@redhat.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----------------------------- > 1 file changed, 22 insertions(+), 57 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index cdc2c15..fb4cbbe5 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3190,47 +3190,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) > } > > /* > - * Native read with retry for link status and receiver capability reads for > - * cases where the sink may still be asleep. > - * > - * Sinks are *supposed* to come up within 1ms from an off state, but we're also > - * supposed to retry 3 times per the spec. > - */ > -static ssize_t > -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, > - void *buffer, size_t size) > -{ > - ssize_t ret; > - int i; > - > - /* > - * Sometime we just get the same incorrect byte repeated > - * over the entire buffer. Doing just one throw away read > - * initially seems to "solve" it. > - */ > - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); Last time around I dug out the commit adding the line above, which fixed a bug, and said it should be accounted for somehow. But I've had a change of heart. I want this function gone already. We've improved the dp helpers considerably, and if this regresses, we need to look at fixing this either at the drm level or in our ->transfer hook. Acked-by: Jani Nikula <jani.nikula@intel.com> > - > - for (i = 0; i < 3; i++) { > - ret = drm_dp_dpcd_read(aux, offset, buffer, size); > - if (ret == size) > - return ret; > - msleep(1); > - } > - > - return ret; > -} > - > -/* > * Fetch AUX CH registers 0x202 - 0x207 which contain > * link status information > */ > bool > intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) > { > - return intel_dp_dpcd_read_wake(&intel_dp->aux, > - DP_LANE0_1_STATUS, > - link_status, > - DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; > + return drm_dp_dpcd_read(&intel_dp->aux, DP_LANE0_1_STATUS, link_status, > + DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; > } > > /* These are source-specific values. */ > @@ -3865,8 +3832,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > struct drm_i915_private *dev_priv = dev->dev_private; > uint8_t rev; > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, > - sizeof(intel_dp->dpcd)) < 0) > + if (drm_dp_dpcd_read(&intel_dp->aux, 0x000, intel_dp->dpcd, > + sizeof(intel_dp->dpcd)) < 0) > return false; /* aux transfer failed */ > > DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd); > @@ -3877,9 +3844,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_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT, > - intel_dp->psr_dpcd, > - sizeof(intel_dp->psr_dpcd)); > + drm_dp_dpcd_read(&intel_dp->aux, 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"); > @@ -3890,9 +3857,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > uint8_t frame_sync_cap; > > dev_priv->psr.sink_support = true; > - intel_dp_dpcd_read_wake(&intel_dp->aux, > - DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > - &frame_sync_cap, 1); > + drm_dp_dpcd_read(&intel_dp->aux, > + DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > + &frame_sync_cap, 1); > dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false; > /* PSR2 needs frame sync as well */ > dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync; > @@ -3908,15 +3875,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > /* Intermediate frequency support */ > if (is_edp(intel_dp) && > (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & DP_DPCD_DISPLAY_CONTROL_CAPABLE) && > - (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && > + (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && > (rev >= 0x03)) { /* eDp v1.4 or higher */ > __le16 sink_rates[DP_MAX_SUPPORTED_RATES]; > int i; > > - intel_dp_dpcd_read_wake(&intel_dp->aux, > - DP_SUPPORTED_LINK_RATES, > - sink_rates, > - sizeof(sink_rates)); > + drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES, > + sink_rates, sizeof(sink_rates)); > > for (i = 0; i < ARRAY_SIZE(sink_rates); i++) { > int val = le16_to_cpu(sink_rates[i]); > @@ -3939,9 +3904,9 @@ 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_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, > - intel_dp->downstream_ports, > - DP_MAX_DOWNSTREAM_PORTS) < 0) > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, > + intel_dp->downstream_ports, > + DP_MAX_DOWNSTREAM_PORTS) < 0) > return false; /* downstream port status fetch failed */ > > return true; > @@ -3955,11 +3920,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp) > if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT)) > return; > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) > DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n", > buf[0], buf[1], buf[2]); > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", > buf[0], buf[1], buf[2]); > } > @@ -3975,7 +3940,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp) > if (intel_dp->dpcd[DP_DPCD_REV] < 0x12) > return false; > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { > if (buf[0] & DP_MST_CAP) { > DRM_DEBUG_KMS("Sink is MST capable\n"); > intel_dp->is_mst = true; > @@ -4112,7 +4077,7 @@ stop: > static bool > intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector) > { > - return intel_dp_dpcd_read_wake(&intel_dp->aux, > + return drm_dp_dpcd_read(&intel_dp->aux, > DP_DEVICE_SERVICE_IRQ_VECTOR, > sink_irq_vector, 1) == 1; > } > @@ -4122,7 +4087,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) > { > int ret; > > - ret = intel_dp_dpcd_read_wake(&intel_dp->aux, > + ret = drm_dp_dpcd_read(&intel_dp->aux, > DP_SINK_COUNT_ESI, > sink_irq_vector, 14); > if (ret != 14) > @@ -4383,7 +4348,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) > intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { > uint8_t reg; > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT, > ®, 1) < 0) > return connector_status_unknown;
On Thu, Mar 17, 2016 at 11:40:45AM -0400, Lyude wrote: > Since we've fixed up drm_dp_dpcd_read() to allow for retries when things > timeout, there's no use for having this function anymore. Good riddens. > > Signed-off-by: Lyude <cpaul@redhat.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----------------------------- > 1 file changed, 22 insertions(+), 57 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index cdc2c15..fb4cbbe5 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3190,47 +3190,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) > } > > /* > - * Native read with retry for link status and receiver capability reads for > - * cases where the sink may still be asleep. > - * > - * Sinks are *supposed* to come up within 1ms from an off state, but we're also > - * supposed to retry 3 times per the spec. > - */ > -static ssize_t > -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, > - void *buffer, size_t size) > -{ > - ssize_t ret; > - int i; > - > - /* > - * Sometime we just get the same incorrect byte repeated > - * over the entire buffer. Doing just one throw away read > - * initially seems to "solve" it. > - */ > - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); NAK If people keep intentionally breaking my shit I'm going to become really grumpy soon. > - > - for (i = 0; i < 3; i++) { > - ret = drm_dp_dpcd_read(aux, offset, buffer, size); > - if (ret == size) > - return ret; > - msleep(1); > - } > - > - return ret; > -} > - > -/* > * Fetch AUX CH registers 0x202 - 0x207 which contain > * link status information > */ > bool > intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) > { > - return intel_dp_dpcd_read_wake(&intel_dp->aux, > - DP_LANE0_1_STATUS, > - link_status, > - DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; > + return drm_dp_dpcd_read(&intel_dp->aux, DP_LANE0_1_STATUS, link_status, > + DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; > } > > /* These are source-specific values. */ > @@ -3865,8 +3832,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > struct drm_i915_private *dev_priv = dev->dev_private; > uint8_t rev; > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, > - sizeof(intel_dp->dpcd)) < 0) > + if (drm_dp_dpcd_read(&intel_dp->aux, 0x000, intel_dp->dpcd, > + sizeof(intel_dp->dpcd)) < 0) > return false; /* aux transfer failed */ > > DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd); > @@ -3877,9 +3844,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_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT, > - intel_dp->psr_dpcd, > - sizeof(intel_dp->psr_dpcd)); > + drm_dp_dpcd_read(&intel_dp->aux, 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"); > @@ -3890,9 +3857,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > uint8_t frame_sync_cap; > > dev_priv->psr.sink_support = true; > - intel_dp_dpcd_read_wake(&intel_dp->aux, > - DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > - &frame_sync_cap, 1); > + drm_dp_dpcd_read(&intel_dp->aux, > + DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > + &frame_sync_cap, 1); > dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false; > /* PSR2 needs frame sync as well */ > dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync; > @@ -3908,15 +3875,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > /* Intermediate frequency support */ > if (is_edp(intel_dp) && > (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & DP_DPCD_DISPLAY_CONTROL_CAPABLE) && > - (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && > + (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && > (rev >= 0x03)) { /* eDp v1.4 or higher */ > __le16 sink_rates[DP_MAX_SUPPORTED_RATES]; > int i; > > - intel_dp_dpcd_read_wake(&intel_dp->aux, > - DP_SUPPORTED_LINK_RATES, > - sink_rates, > - sizeof(sink_rates)); > + drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES, > + sink_rates, sizeof(sink_rates)); > > for (i = 0; i < ARRAY_SIZE(sink_rates); i++) { > int val = le16_to_cpu(sink_rates[i]); > @@ -3939,9 +3904,9 @@ 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_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, > - intel_dp->downstream_ports, > - DP_MAX_DOWNSTREAM_PORTS) < 0) > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, > + intel_dp->downstream_ports, > + DP_MAX_DOWNSTREAM_PORTS) < 0) > return false; /* downstream port status fetch failed */ > > return true; > @@ -3955,11 +3920,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp) > if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT)) > return; > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) > DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n", > buf[0], buf[1], buf[2]); > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", > buf[0], buf[1], buf[2]); > } > @@ -3975,7 +3940,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp) > if (intel_dp->dpcd[DP_DPCD_REV] < 0x12) > return false; > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { > if (buf[0] & DP_MST_CAP) { > DRM_DEBUG_KMS("Sink is MST capable\n"); > intel_dp->is_mst = true; > @@ -4112,7 +4077,7 @@ stop: > static bool > intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector) > { > - return intel_dp_dpcd_read_wake(&intel_dp->aux, > + return drm_dp_dpcd_read(&intel_dp->aux, > DP_DEVICE_SERVICE_IRQ_VECTOR, > sink_irq_vector, 1) == 1; > } > @@ -4122,7 +4087,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) > { > int ret; > > - ret = intel_dp_dpcd_read_wake(&intel_dp->aux, > + ret = drm_dp_dpcd_read(&intel_dp->aux, > DP_SINK_COUNT_ESI, > sink_irq_vector, 14); > if (ret != 14) > @@ -4383,7 +4348,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) > intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { > uint8_t reg; > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT, > ®, 1) < 0) > return connector_status_unknown; > > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Mar 17, 2016 at 07:56:33PM +0200, Jani Nikula wrote: > On Thu, 17 Mar 2016, Lyude <cpaul@redhat.com> wrote: > > Since we've fixed up drm_dp_dpcd_read() to allow for retries when things > > timeout, there's no use for having this function anymore. Good riddens. > > > > Signed-off-by: Lyude <cpaul@redhat.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----------------------------- > > 1 file changed, 22 insertions(+), 57 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index cdc2c15..fb4cbbe5 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -3190,47 +3190,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) > > } > > > > /* > > - * Native read with retry for link status and receiver capability reads for > > - * cases where the sink may still be asleep. > > - * > > - * Sinks are *supposed* to come up within 1ms from an off state, but we're also > > - * supposed to retry 3 times per the spec. > > - */ > > -static ssize_t > > -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, > > - void *buffer, size_t size) > > -{ > > - ssize_t ret; > > - int i; > > - > > - /* > > - * Sometime we just get the same incorrect byte repeated > > - * over the entire buffer. Doing just one throw away read > > - * initially seems to "solve" it. > > - */ > > - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); > > Last time around I dug out the commit adding the line above, which fixed > a bug, and said it should be accounted for somehow. But I've had a > change of heart. I want this function gone already. We've improved the > dp helpers considerably, and if this regresses, we need to look at > fixing this either at the drm level or in our ->transfer hook. > > Acked-by: Jani Nikula <jani.nikula@intel.com> Hm, my idea was actually to 1:1 move intel_dp_dpcd_read_wake logic into drm_dp_dpcd_read, without any functional change for i915.ko. And everyone else shouldn't complain about a few more retries either. That would also address Ville's grumpy nack. -Daniel > > > > - > > - for (i = 0; i < 3; i++) { > > - ret = drm_dp_dpcd_read(aux, offset, buffer, size); > > - if (ret == size) > > - return ret; > > - msleep(1); > > - } > > - > > - return ret; > > -} > > - > > -/* > > * Fetch AUX CH registers 0x202 - 0x207 which contain > > * link status information > > */ > > bool > > intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) > > { > > - return intel_dp_dpcd_read_wake(&intel_dp->aux, > > - DP_LANE0_1_STATUS, > > - link_status, > > - DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; > > + return drm_dp_dpcd_read(&intel_dp->aux, DP_LANE0_1_STATUS, link_status, > > + DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; > > } > > > > /* These are source-specific values. */ > > @@ -3865,8 +3832,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > > struct drm_i915_private *dev_priv = dev->dev_private; > > uint8_t rev; > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, > > - sizeof(intel_dp->dpcd)) < 0) > > + if (drm_dp_dpcd_read(&intel_dp->aux, 0x000, intel_dp->dpcd, > > + sizeof(intel_dp->dpcd)) < 0) > > return false; /* aux transfer failed */ > > > > DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd); > > @@ -3877,9 +3844,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_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT, > > - intel_dp->psr_dpcd, > > - sizeof(intel_dp->psr_dpcd)); > > + drm_dp_dpcd_read(&intel_dp->aux, 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"); > > @@ -3890,9 +3857,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > > uint8_t frame_sync_cap; > > > > dev_priv->psr.sink_support = true; > > - intel_dp_dpcd_read_wake(&intel_dp->aux, > > - DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > > - &frame_sync_cap, 1); > > + drm_dp_dpcd_read(&intel_dp->aux, > > + DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > > + &frame_sync_cap, 1); > > dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false; > > /* PSR2 needs frame sync as well */ > > dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync; > > @@ -3908,15 +3875,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > > /* Intermediate frequency support */ > > if (is_edp(intel_dp) && > > (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & DP_DPCD_DISPLAY_CONTROL_CAPABLE) && > > - (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && > > + (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && > > (rev >= 0x03)) { /* eDp v1.4 or higher */ > > __le16 sink_rates[DP_MAX_SUPPORTED_RATES]; > > int i; > > > > - intel_dp_dpcd_read_wake(&intel_dp->aux, > > - DP_SUPPORTED_LINK_RATES, > > - sink_rates, > > - sizeof(sink_rates)); > > + drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES, > > + sink_rates, sizeof(sink_rates)); > > > > for (i = 0; i < ARRAY_SIZE(sink_rates); i++) { > > int val = le16_to_cpu(sink_rates[i]); > > @@ -3939,9 +3904,9 @@ 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_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, > > - intel_dp->downstream_ports, > > - DP_MAX_DOWNSTREAM_PORTS) < 0) > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, > > + intel_dp->downstream_ports, > > + DP_MAX_DOWNSTREAM_PORTS) < 0) > > return false; /* downstream port status fetch failed */ > > > > return true; > > @@ -3955,11 +3920,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp) > > if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT)) > > return; > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) > > DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n", > > buf[0], buf[1], buf[2]); > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > > DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", > > buf[0], buf[1], buf[2]); > > } > > @@ -3975,7 +3940,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp) > > if (intel_dp->dpcd[DP_DPCD_REV] < 0x12) > > return false; > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { > > if (buf[0] & DP_MST_CAP) { > > DRM_DEBUG_KMS("Sink is MST capable\n"); > > intel_dp->is_mst = true; > > @@ -4112,7 +4077,7 @@ stop: > > static bool > > intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector) > > { > > - return intel_dp_dpcd_read_wake(&intel_dp->aux, > > + return drm_dp_dpcd_read(&intel_dp->aux, > > DP_DEVICE_SERVICE_IRQ_VECTOR, > > sink_irq_vector, 1) == 1; > > } > > @@ -4122,7 +4087,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) > > { > > int ret; > > > > - ret = intel_dp_dpcd_read_wake(&intel_dp->aux, > > + ret = drm_dp_dpcd_read(&intel_dp->aux, > > DP_SINK_COUNT_ESI, > > sink_irq_vector, 14); > > if (ret != 14) > > @@ -4383,7 +4348,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) > > intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { > > uint8_t reg; > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT, > > ®, 1) < 0) > > return connector_status_unknown; > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Mar 18, 2016 at 04:13:45PM +0200, Ville Syrjälä wrote: > On Thu, Mar 17, 2016 at 11:40:45AM -0400, Lyude wrote: > > Since we've fixed up drm_dp_dpcd_read() to allow for retries when things > > timeout, there's no use for having this function anymore. Good riddens. > > > > Signed-off-by: Lyude <cpaul@redhat.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----------------------------- > > 1 file changed, 22 insertions(+), 57 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index cdc2c15..fb4cbbe5 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -3190,47 +3190,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) > > } > > > > /* > > - * Native read with retry for link status and receiver capability reads for > > - * cases where the sink may still be asleep. > > - * > > - * Sinks are *supposed* to come up within 1ms from an off state, but we're also > > - * supposed to retry 3 times per the spec. > > - */ > > -static ssize_t > > -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, > > - void *buffer, size_t size) > > -{ > > - ssize_t ret; > > - int i; > > - > > - /* > > - * Sometime we just get the same incorrect byte repeated > > - * over the entire buffer. Doing just one throw away read > > - * initially seems to "solve" it. > > - */ > > - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); > > NAK > > If people keep intentionally breaking my shit I'm going to become > really grumpy soon. Oh, and just in case someone wants to come up with a better kludge, I just spent a few minutes analyzing the behavior of this crappy monitor a. What happens is that when the monitor is fully powered up (LED is blue) things are fine. After the monitor goes to sleep (LED turns orange) the first DPCD read will produce garbage. Further DPCD reads are fine, even if I wait a significant amount of time between the reads, as long as the monitor didn't do a power on->off cycle in between. So it looks like it's always just the first read after power down that gets corrupted. Now I think I'll go and test how writes behave, assuming I can find a decently sized chunk of DPCD address space I can write. And maybe I should also try i2c-over-aux... > > > - > > - for (i = 0; i < 3; i++) { > > - ret = drm_dp_dpcd_read(aux, offset, buffer, size); > > - if (ret == size) > > - return ret; > > - msleep(1); > > - } > > - > > - return ret; > > -} > > - > > -/* > > * Fetch AUX CH registers 0x202 - 0x207 which contain > > * link status information > > */ > > bool > > intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) > > { > > - return intel_dp_dpcd_read_wake(&intel_dp->aux, > > - DP_LANE0_1_STATUS, > > - link_status, > > - DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; > > + return drm_dp_dpcd_read(&intel_dp->aux, DP_LANE0_1_STATUS, link_status, > > + DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; > > } > > > > /* These are source-specific values. */ > > @@ -3865,8 +3832,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > > struct drm_i915_private *dev_priv = dev->dev_private; > > uint8_t rev; > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, > > - sizeof(intel_dp->dpcd)) < 0) > > + if (drm_dp_dpcd_read(&intel_dp->aux, 0x000, intel_dp->dpcd, > > + sizeof(intel_dp->dpcd)) < 0) > > return false; /* aux transfer failed */ > > > > DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd); > > @@ -3877,9 +3844,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_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT, > > - intel_dp->psr_dpcd, > > - sizeof(intel_dp->psr_dpcd)); > > + drm_dp_dpcd_read(&intel_dp->aux, 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"); > > @@ -3890,9 +3857,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > > uint8_t frame_sync_cap; > > > > dev_priv->psr.sink_support = true; > > - intel_dp_dpcd_read_wake(&intel_dp->aux, > > - DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > > - &frame_sync_cap, 1); > > + drm_dp_dpcd_read(&intel_dp->aux, > > + DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > > + &frame_sync_cap, 1); > > dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false; > > /* PSR2 needs frame sync as well */ > > dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync; > > @@ -3908,15 +3875,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > > /* Intermediate frequency support */ > > if (is_edp(intel_dp) && > > (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & DP_DPCD_DISPLAY_CONTROL_CAPABLE) && > > - (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && > > + (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && > > (rev >= 0x03)) { /* eDp v1.4 or higher */ > > __le16 sink_rates[DP_MAX_SUPPORTED_RATES]; > > int i; > > > > - intel_dp_dpcd_read_wake(&intel_dp->aux, > > - DP_SUPPORTED_LINK_RATES, > > - sink_rates, > > - sizeof(sink_rates)); > > + drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES, > > + sink_rates, sizeof(sink_rates)); > > > > for (i = 0; i < ARRAY_SIZE(sink_rates); i++) { > > int val = le16_to_cpu(sink_rates[i]); > > @@ -3939,9 +3904,9 @@ 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_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, > > - intel_dp->downstream_ports, > > - DP_MAX_DOWNSTREAM_PORTS) < 0) > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, > > + intel_dp->downstream_ports, > > + DP_MAX_DOWNSTREAM_PORTS) < 0) > > return false; /* downstream port status fetch failed */ > > > > return true; > > @@ -3955,11 +3920,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp) > > if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT)) > > return; > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) > > DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n", > > buf[0], buf[1], buf[2]); > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > > DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", > > buf[0], buf[1], buf[2]); > > } > > @@ -3975,7 +3940,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp) > > if (intel_dp->dpcd[DP_DPCD_REV] < 0x12) > > return false; > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { > > if (buf[0] & DP_MST_CAP) { > > DRM_DEBUG_KMS("Sink is MST capable\n"); > > intel_dp->is_mst = true; > > @@ -4112,7 +4077,7 @@ stop: > > static bool > > intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector) > > { > > - return intel_dp_dpcd_read_wake(&intel_dp->aux, > > + return drm_dp_dpcd_read(&intel_dp->aux, > > DP_DEVICE_SERVICE_IRQ_VECTOR, > > sink_irq_vector, 1) == 1; > > } > > @@ -4122,7 +4087,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) > > { > > int ret; > > > > - ret = intel_dp_dpcd_read_wake(&intel_dp->aux, > > + ret = drm_dp_dpcd_read(&intel_dp->aux, > > DP_SINK_COUNT_ESI, > > sink_irq_vector, 14); > > if (ret != 14) > > @@ -4383,7 +4348,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) > > intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { > > uint8_t reg; > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT, > > ®, 1) < 0) > > return connector_status_unknown; > > > > -- > > 2.5.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Mar 18, 2016 at 06:12:35PM +0200, Ville Syrjälä wrote: > On Fri, Mar 18, 2016 at 04:13:45PM +0200, Ville Syrjälä wrote: > > On Thu, Mar 17, 2016 at 11:40:45AM -0400, Lyude wrote: > > > Since we've fixed up drm_dp_dpcd_read() to allow for retries when things > > > timeout, there's no use for having this function anymore. Good riddens. > > > > > > Signed-off-by: Lyude <cpaul@redhat.com> > > > --- > > > drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----------------------------- > > > 1 file changed, 22 insertions(+), 57 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > index cdc2c15..fb4cbbe5 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -3190,47 +3190,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) > > > } > > > > > > /* > > > - * Native read with retry for link status and receiver capability reads for > > > - * cases where the sink may still be asleep. > > > - * > > > - * Sinks are *supposed* to come up within 1ms from an off state, but we're also > > > - * supposed to retry 3 times per the spec. > > > - */ > > > -static ssize_t > > > -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, > > > - void *buffer, size_t size) > > > -{ > > > - ssize_t ret; > > > - int i; > > > - > > > - /* > > > - * Sometime we just get the same incorrect byte repeated > > > - * over the entire buffer. Doing just one throw away read > > > - * initially seems to "solve" it. > > > - */ > > > - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); > > > > NAK > > > > If people keep intentionally breaking my shit I'm going to become > > really grumpy soon. > > Oh, and just in case someone wants to come up with a better kludge, > I just spent a few minutes analyzing the behavior of this crappy > monitor a. > > What happens is that when the monitor is fully powered up (LED is blue) > things are fine. After the monitor goes to sleep (LED turns orange) > the first DPCD read will produce garbage. Further DPCD reads are fine, > even if I wait a significant amount of time between the reads, as long > as the monitor didn't do a power on->off cycle in between. So it looks > like it's always just the first read after power down that gets > corrupted. > > Now I think I'll go and test how writes behave, assuming I can find a > decently sized chunk of DPCD address space I can write. And maybe I > should also try i2c-over-aux... The first DPCD write after powerdown also got corrupted. But i2c-over-aux seems unaffected for whatever reason. > > > > > > - > > > - for (i = 0; i < 3; i++) { > > > - ret = drm_dp_dpcd_read(aux, offset, buffer, size); > > > - if (ret == size) > > > - return ret; > > > - msleep(1); > > > - } > > > - > > > - return ret; > > > -} > > > - > > > -/* > > > * Fetch AUX CH registers 0x202 - 0x207 which contain > > > * link status information > > > */ > > > bool > > > intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) > > > { > > > - return intel_dp_dpcd_read_wake(&intel_dp->aux, > > > - DP_LANE0_1_STATUS, > > > - link_status, > > > - DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; > > > + return drm_dp_dpcd_read(&intel_dp->aux, DP_LANE0_1_STATUS, link_status, > > > + DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; > > > } > > > > > > /* These are source-specific values. */ > > > @@ -3865,8 +3832,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > uint8_t rev; > > > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, > > > - sizeof(intel_dp->dpcd)) < 0) > > > + if (drm_dp_dpcd_read(&intel_dp->aux, 0x000, intel_dp->dpcd, > > > + sizeof(intel_dp->dpcd)) < 0) > > > return false; /* aux transfer failed */ > > > > > > DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd); > > > @@ -3877,9 +3844,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_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT, > > > - intel_dp->psr_dpcd, > > > - sizeof(intel_dp->psr_dpcd)); > > > + drm_dp_dpcd_read(&intel_dp->aux, 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"); > > > @@ -3890,9 +3857,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > > > uint8_t frame_sync_cap; > > > > > > dev_priv->psr.sink_support = true; > > > - intel_dp_dpcd_read_wake(&intel_dp->aux, > > > - DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > > > - &frame_sync_cap, 1); > > > + drm_dp_dpcd_read(&intel_dp->aux, > > > + DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > > > + &frame_sync_cap, 1); > > > dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false; > > > /* PSR2 needs frame sync as well */ > > > dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync; > > > @@ -3908,15 +3875,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > > > /* Intermediate frequency support */ > > > if (is_edp(intel_dp) && > > > (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & DP_DPCD_DISPLAY_CONTROL_CAPABLE) && > > > - (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && > > > + (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && > > > (rev >= 0x03)) { /* eDp v1.4 or higher */ > > > __le16 sink_rates[DP_MAX_SUPPORTED_RATES]; > > > int i; > > > > > > - intel_dp_dpcd_read_wake(&intel_dp->aux, > > > - DP_SUPPORTED_LINK_RATES, > > > - sink_rates, > > > - sizeof(sink_rates)); > > > + drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES, > > > + sink_rates, sizeof(sink_rates)); > > > > > > for (i = 0; i < ARRAY_SIZE(sink_rates); i++) { > > > int val = le16_to_cpu(sink_rates[i]); > > > @@ -3939,9 +3904,9 @@ 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_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, > > > - intel_dp->downstream_ports, > > > - DP_MAX_DOWNSTREAM_PORTS) < 0) > > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, > > > + intel_dp->downstream_ports, > > > + DP_MAX_DOWNSTREAM_PORTS) < 0) > > > return false; /* downstream port status fetch failed */ > > > > > > return true; > > > @@ -3955,11 +3920,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp) > > > if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT)) > > > return; > > > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) > > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) > > > DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n", > > > buf[0], buf[1], buf[2]); > > > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > > > DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", > > > buf[0], buf[1], buf[2]); > > > } > > > @@ -3975,7 +3940,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp) > > > if (intel_dp->dpcd[DP_DPCD_REV] < 0x12) > > > return false; > > > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { > > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { > > > if (buf[0] & DP_MST_CAP) { > > > DRM_DEBUG_KMS("Sink is MST capable\n"); > > > intel_dp->is_mst = true; > > > @@ -4112,7 +4077,7 @@ stop: > > > static bool > > > intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector) > > > { > > > - return intel_dp_dpcd_read_wake(&intel_dp->aux, > > > + return drm_dp_dpcd_read(&intel_dp->aux, > > > DP_DEVICE_SERVICE_IRQ_VECTOR, > > > sink_irq_vector, 1) == 1; > > > } > > > @@ -4122,7 +4087,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) > > > { > > > int ret; > > > > > > - ret = intel_dp_dpcd_read_wake(&intel_dp->aux, > > > + ret = drm_dp_dpcd_read(&intel_dp->aux, > > > DP_SINK_COUNT_ESI, > > > sink_irq_vector, 14); > > > if (ret != 14) > > > @@ -4383,7 +4348,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) > > > intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { > > > uint8_t reg; > > > > > > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, > > > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT, > > > ®, 1) < 0) > > > return connector_status_unknown; > > > > > > -- > > > 2.5.0 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel OTC > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Mar 18, 2016 at 06:41:40PM +0200, Ville Syrjälä wrote: > On Fri, Mar 18, 2016 at 06:12:35PM +0200, Ville Syrjälä wrote: > > On Fri, Mar 18, 2016 at 04:13:45PM +0200, Ville Syrjälä wrote: > > > On Thu, Mar 17, 2016 at 11:40:45AM -0400, Lyude wrote: > > > > Since we've fixed up drm_dp_dpcd_read() to allow for retries when things > > > > timeout, there's no use for having this function anymore. Good riddens. > > > > > > > > Signed-off-by: Lyude <cpaul@redhat.com> > > > > --- > > > > drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----------------------------- > > > > 1 file changed, 22 insertions(+), 57 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > > index cdc2c15..fb4cbbe5 100644 > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > @@ -3190,47 +3190,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) > > > > } > > > > > > > > /* > > > > - * Native read with retry for link status and receiver capability reads for > > > > - * cases where the sink may still be asleep. > > > > - * > > > > - * Sinks are *supposed* to come up within 1ms from an off state, but we're also > > > > - * supposed to retry 3 times per the spec. > > > > - */ > > > > -static ssize_t > > > > -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, > > > > - void *buffer, size_t size) > > > > -{ > > > > - ssize_t ret; > > > > - int i; > > > > - > > > > - /* > > > > - * Sometime we just get the same incorrect byte repeated > > > > - * over the entire buffer. Doing just one throw away read > > > > - * initially seems to "solve" it. > > > > - */ > > > > - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); > > > > > > NAK > > > > > > If people keep intentionally breaking my shit I'm going to become > > > really grumpy soon. > > > > Oh, and just in case someone wants to come up with a better kludge, > > I just spent a few minutes analyzing the behavior of this crappy > > monitor a. > > > > What happens is that when the monitor is fully powered up (LED is blue) > > things are fine. After the monitor goes to sleep (LED turns orange) > > the first DPCD read will produce garbage. Further DPCD reads are fine, > > even if I wait a significant amount of time between the reads, as long > > as the monitor didn't do a power on->off cycle in between. So it looks > > like it's always just the first read after power down that gets > > corrupted. > > > > Now I think I'll go and test how writes behave, assuming I can find a > > decently sized chunk of DPCD address space I can write. And maybe I > > should also try i2c-over-aux... > > The first DPCD write after powerdown also got corrupted. But i2c-over-aux > seems unaffected for whatever reason. Do you have an amd card nearby to test there? Would be interesting to confirm that this is indeed a sink bug, and hence that it really all should be in the shared code. -Daniel
On Fri, Mar 18, 2016 at 07:00:29PM +0100, Daniel Vetter wrote: > On Fri, Mar 18, 2016 at 06:41:40PM +0200, Ville Syrjälä wrote: > > On Fri, Mar 18, 2016 at 06:12:35PM +0200, Ville Syrjälä wrote: > > > On Fri, Mar 18, 2016 at 04:13:45PM +0200, Ville Syrjälä wrote: > > > > On Thu, Mar 17, 2016 at 11:40:45AM -0400, Lyude wrote: > > > > > Since we've fixed up drm_dp_dpcd_read() to allow for retries when things > > > > > timeout, there's no use for having this function anymore. Good riddens. > > > > > > > > > > Signed-off-by: Lyude <cpaul@redhat.com> > > > > > --- > > > > > drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----------------------------- > > > > > 1 file changed, 22 insertions(+), 57 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > > > index cdc2c15..fb4cbbe5 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > > @@ -3190,47 +3190,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) > > > > > } > > > > > > > > > > /* > > > > > - * Native read with retry for link status and receiver capability reads for > > > > > - * cases where the sink may still be asleep. > > > > > - * > > > > > - * Sinks are *supposed* to come up within 1ms from an off state, but we're also > > > > > - * supposed to retry 3 times per the spec. > > > > > - */ > > > > > -static ssize_t > > > > > -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, > > > > > - void *buffer, size_t size) > > > > > -{ > > > > > - ssize_t ret; > > > > > - int i; > > > > > - > > > > > - /* > > > > > - * Sometime we just get the same incorrect byte repeated > > > > > - * over the entire buffer. Doing just one throw away read > > > > > - * initially seems to "solve" it. > > > > > - */ > > > > > - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); > > > > > > > > NAK > > > > > > > > If people keep intentionally breaking my shit I'm going to become > > > > really grumpy soon. > > > > > > Oh, and just in case someone wants to come up with a better kludge, > > > I just spent a few minutes analyzing the behavior of this crappy > > > monitor a. > > > > > > What happens is that when the monitor is fully powered up (LED is blue) > > > things are fine. After the monitor goes to sleep (LED turns orange) > > > the first DPCD read will produce garbage. Further DPCD reads are fine, > > > even if I wait a significant amount of time between the reads, as long > > > as the monitor didn't do a power on->off cycle in between. So it looks > > > like it's always just the first read after power down that gets > > > corrupted. > > > > > > Now I think I'll go and test how writes behave, assuming I can find a > > > decently sized chunk of DPCD address space I can write. And maybe I > > > should also try i2c-over-aux... > > > > The first DPCD write after powerdown also got corrupted. But i2c-over-aux > > seems unaffected for whatever reason. > > Do you have an amd card nearby to test there? Nope. > Would be interesting to > confirm that this is indeed a sink bug, and hence that it really all > should be in the shared code. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Fri, 18 Mar 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, Mar 18, 2016 at 06:12:35PM +0200, Ville Syrjälä wrote: >> On Fri, Mar 18, 2016 at 04:13:45PM +0200, Ville Syrjälä wrote: >> > On Thu, Mar 17, 2016 at 11:40:45AM -0400, Lyude wrote: >> > > - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); >> > >> > NAK >> > >> > If people keep intentionally breaking my shit I'm going to become >> > really grumpy soon. >> >> Oh, and just in case someone wants to come up with a better kludge, >> I just spent a few minutes analyzing the behavior of this crappy >> monitor a. >> >> What happens is that when the monitor is fully powered up (LED is blue) >> things are fine. After the monitor goes to sleep (LED turns orange) >> the first DPCD read will produce garbage. Further DPCD reads are fine, >> even if I wait a significant amount of time between the reads, as long >> as the monitor didn't do a power on->off cycle in between. So it looks >> like it's always just the first read after power down that gets >> corrupted. >> >> Now I think I'll go and test how writes behave, assuming I can find a >> decently sized chunk of DPCD address space I can write. And maybe I >> should also try i2c-over-aux... > > The first DPCD write after powerdown also got corrupted. But i2c-over-aux > seems unaffected for whatever reason. Did the display go to sleep on its own, or did we do something? In particular, does DPCD DP_SET_POWER register play a role? What if we skip writing D3 to it? What if we do that write as the first thing (every time)? BR, Jani.
On Mon, Mar 21, 2016 at 12:30:54PM +0200, Jani Nikula wrote: > On Fri, 18 Mar 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Fri, Mar 18, 2016 at 06:12:35PM +0200, Ville Syrjälä wrote: > >> On Fri, Mar 18, 2016 at 04:13:45PM +0200, Ville Syrjälä wrote: > >> > On Thu, Mar 17, 2016 at 11:40:45AM -0400, Lyude wrote: > >> > > - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); > >> > > >> > NAK > >> > > >> > If people keep intentionally breaking my shit I'm going to become > >> > really grumpy soon. > >> > >> Oh, and just in case someone wants to come up with a better kludge, > >> I just spent a few minutes analyzing the behavior of this crappy > >> monitor a. > >> > >> What happens is that when the monitor is fully powered up (LED is blue) > >> things are fine. After the monitor goes to sleep (LED turns orange) > >> the first DPCD read will produce garbage. Further DPCD reads are fine, > >> even if I wait a significant amount of time between the reads, as long > >> as the monitor didn't do a power on->off cycle in between. So it looks > >> like it's always just the first read after power down that gets > >> corrupted. > >> > >> Now I think I'll go and test how writes behave, assuming I can find a > >> decently sized chunk of DPCD address space I can write. And maybe I > >> should also try i2c-over-aux... > > > > The first DPCD write after powerdown also got corrupted. But i2c-over-aux > > seems unaffected for whatever reason. > > Did the display go to sleep on its own, or did we do something? In > particular, does DPCD DP_SET_POWER register play a role? What if we skip > writing D3 to it? What if we do that write as the first thing (every > time)? User pressing any of the buttons on the monitor is enough to wake it, and after a short timeout it will power down on its own, leading to the corrupted access. Keeping DP_SET_POWER at D0 doesn't change anything.
On Fri, 2016-03-18 at 20:05 +0200, Ville Syrjälä wrote: > On Fri, Mar 18, 2016 at 07:00:29PM +0100, Daniel Vetter wrote: > > > > On Fri, Mar 18, 2016 at 06:41:40PM +0200, Ville Syrjälä wrote: > > > > > > On Fri, Mar 18, 2016 at 06:12:35PM +0200, Ville Syrjälä wrote: > > > > > > > > On Fri, Mar 18, 2016 at 04:13:45PM +0200, Ville Syrjälä wrote: > > > > > > > > > > On Thu, Mar 17, 2016 at 11:40:45AM -0400, Lyude wrote: > > > > > > > > > > > > Since we've fixed up drm_dp_dpcd_read() to allow for retries when > > > > > > things > > > > > > timeout, there's no use for having this function anymore. Good > > > > > > riddens. > > > > > > > > > > > > Signed-off-by: Lyude <cpaul@redhat.com> > > > > > > --- > > > > > > drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++---------------- > > > > > > ------------- > > > > > > 1 file changed, 22 insertions(+), 57 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > > > > b/drivers/gpu/drm/i915/intel_dp.c > > > > > > index cdc2c15..fb4cbbe5 100644 > > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > > > @@ -3190,47 +3190,14 @@ static void chv_dp_post_pll_disable(struct > > > > > > intel_encoder *encoder) > > > > > > } > > > > > > > > > > > > /* > > > > > > - * Native read with retry for link status and receiver capability > > > > > > reads for > > > > > > - * cases where the sink may still be asleep. > > > > > > - * > > > > > > - * Sinks are *supposed* to come up within 1ms from an off state, > > > > > > but we're also > > > > > > - * supposed to retry 3 times per the spec. > > > > > > - */ > > > > > > -static ssize_t > > > > > > -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int > > > > > > offset, > > > > > > - void *buffer, size_t size) > > > > > > -{ > > > > > > - ssize_t ret; > > > > > > - int i; > > > > > > - > > > > > > - /* > > > > > > - * Sometime we just get the same incorrect byte repeated > > > > > > - * over the entire buffer. Doing just one throw away read > > > > > > - * initially seems to "solve" it. > > > > > > - */ > > > > > > - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); > > > > > NAK > > > > > > > > > > If people keep intentionally breaking my shit I'm going to become > > > > > really grumpy soon. > > > > Oh, and just in case someone wants to come up with a better kludge, > > > > I just spent a few minutes analyzing the behavior of this crappy > > > > monitor a. > > > > > > > > What happens is that when the monitor is fully powered up (LED is blue) > > > > things are fine. After the monitor goes to sleep (LED turns orange) > > > > the first DPCD read will produce garbage. Further DPCD reads are fine, > > > > even if I wait a significant amount of time between the reads, as long > > > > as the monitor didn't do a power on->off cycle in between. So it looks > > > > like it's always just the first read after power down that gets > > > > corrupted. > > > > > > > > Now I think I'll go and test how writes behave, assuming I can find a > > > > decently sized chunk of DPCD address space I can write. And maybe I > > > > should also try i2c-over-aux... > > > The first DPCD write after powerdown also got corrupted. But i2c-over-aux > > > seems unaffected for whatever reason. > > Do you have an amd card nearby to test there? > Nope. I also work on radeon so I have plenty. The issue with that though is that this is an issue that only really happens with the thinkpad docks, and as far as I'm aware we don't have any ThinkPads that have AMD hardware in them… > > > > Would be interesting to > > confirm that this is indeed a sink bug, and hence that it really all > > should be in the shared code. > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index cdc2c15..fb4cbbe5 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3190,47 +3190,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) } /* - * Native read with retry for link status and receiver capability reads for - * cases where the sink may still be asleep. - * - * Sinks are *supposed* to come up within 1ms from an off state, but we're also - * supposed to retry 3 times per the spec. - */ -static ssize_t -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, - void *buffer, size_t size) -{ - ssize_t ret; - int i; - - /* - * Sometime we just get the same incorrect byte repeated - * over the entire buffer. Doing just one throw away read - * initially seems to "solve" it. - */ - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); - - for (i = 0; i < 3; i++) { - ret = drm_dp_dpcd_read(aux, offset, buffer, size); - if (ret == size) - return ret; - msleep(1); - } - - return ret; -} - -/* * Fetch AUX CH registers 0x202 - 0x207 which contain * link status information */ bool intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) { - return intel_dp_dpcd_read_wake(&intel_dp->aux, - DP_LANE0_1_STATUS, - link_status, - DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; + return drm_dp_dpcd_read(&intel_dp->aux, DP_LANE0_1_STATUS, link_status, + DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; } /* These are source-specific values. */ @@ -3865,8 +3832,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) struct drm_i915_private *dev_priv = dev->dev_private; uint8_t rev; - if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, - sizeof(intel_dp->dpcd)) < 0) + if (drm_dp_dpcd_read(&intel_dp->aux, 0x000, intel_dp->dpcd, + sizeof(intel_dp->dpcd)) < 0) return false; /* aux transfer failed */ DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd); @@ -3877,9 +3844,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_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT, - intel_dp->psr_dpcd, - sizeof(intel_dp->psr_dpcd)); + drm_dp_dpcd_read(&intel_dp->aux, 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"); @@ -3890,9 +3857,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) uint8_t frame_sync_cap; dev_priv->psr.sink_support = true; - intel_dp_dpcd_read_wake(&intel_dp->aux, - DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, - &frame_sync_cap, 1); + drm_dp_dpcd_read(&intel_dp->aux, + DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, + &frame_sync_cap, 1); dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false; /* PSR2 needs frame sync as well */ dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync; @@ -3908,15 +3875,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) /* Intermediate frequency support */ if (is_edp(intel_dp) && (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & DP_DPCD_DISPLAY_CONTROL_CAPABLE) && - (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && + (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && (rev >= 0x03)) { /* eDp v1.4 or higher */ __le16 sink_rates[DP_MAX_SUPPORTED_RATES]; int i; - intel_dp_dpcd_read_wake(&intel_dp->aux, - DP_SUPPORTED_LINK_RATES, - sink_rates, - sizeof(sink_rates)); + drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES, + sink_rates, sizeof(sink_rates)); for (i = 0; i < ARRAY_SIZE(sink_rates); i++) { int val = le16_to_cpu(sink_rates[i]); @@ -3939,9 +3904,9 @@ 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_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, - intel_dp->downstream_ports, - DP_MAX_DOWNSTREAM_PORTS) < 0) + if (drm_dp_dpcd_read(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, + intel_dp->downstream_ports, + DP_MAX_DOWNSTREAM_PORTS) < 0) return false; /* downstream port status fetch failed */ return true; @@ -3955,11 +3920,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp) if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT)) return; - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n", buf[0], buf[1], buf[2]); - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) + if (drm_dp_dpcd_read(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", buf[0], buf[1], buf[2]); } @@ -3975,7 +3940,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp) if (intel_dp->dpcd[DP_DPCD_REV] < 0x12) return false; - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { + if (drm_dp_dpcd_read(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { if (buf[0] & DP_MST_CAP) { DRM_DEBUG_KMS("Sink is MST capable\n"); intel_dp->is_mst = true; @@ -4112,7 +4077,7 @@ stop: static bool intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector) { - return intel_dp_dpcd_read_wake(&intel_dp->aux, + return drm_dp_dpcd_read(&intel_dp->aux, DP_DEVICE_SERVICE_IRQ_VECTOR, sink_irq_vector, 1) == 1; } @@ -4122,7 +4087,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) { int ret; - ret = intel_dp_dpcd_read_wake(&intel_dp->aux, + ret = drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT_ESI, sink_irq_vector, 14); if (ret != 14) @@ -4383,7 +4348,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { uint8_t reg; - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT, ®, 1) < 0) return connector_status_unknown;
Since we've fixed up drm_dp_dpcd_read() to allow for retries when things timeout, there's no use for having this function anymore. Good riddens. Signed-off-by: Lyude <cpaul@redhat.com> --- drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----------------------------- 1 file changed, 22 insertions(+), 57 deletions(-)