| Message ID | b994d5c8a7097c82a45945add06b073bf95057af.1394808141.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula <jani.nikula@intel.com> wrote: > Functionality remains largely the same as before. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 257 +++++++++++++++++--------------------- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 2 files changed, 116 insertions(+), 142 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 22134edc03dd..24cbf4bd36c4 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -575,97 +575,77 @@ out: > return ret; > } > > -/* Write data to the aux channel in native mode */ > -static int > -intel_dp_aux_native_write(struct intel_dp *intel_dp, > - uint16_t address, uint8_t *send, int send_bytes) > +#define HEADER_SIZE 4 > +static ssize_t > +intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > { > + struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux); > + uint8_t txbuf[20], rxbuf[20]; > + size_t txsize, rxsize; > int ret; > - uint8_t msg[20]; > - int msg_bytes; > - uint8_t ack; > - int retry; > > - if (WARN_ON(send_bytes > 16)) > - return -E2BIG; > + txbuf[0] = msg->request << 4; > + txbuf[1] = msg->address >> 8; > + txbuf[2] = msg->address & 0xff; > + txbuf[3] = msg->size - 1; > > - intel_dp_check_edp(intel_dp); > - msg[0] = DP_AUX_NATIVE_WRITE << 4; > - msg[1] = address >> 8; > - msg[2] = address & 0xff; > - msg[3] = send_bytes - 1; > - memcpy(&msg[4], send, send_bytes); > - msg_bytes = send_bytes + 4; > - for (retry = 0; retry < 7; retry++) { we were retrying 7 times always, but now we are now retrying only 3 times or even none. Couldn't it trigger some new bug or regression? > - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1); > - if (ret < 0) > - return ret; > - ack >>= 4; > - if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) > - return send_bytes; > - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER) > - usleep_range(400, 500); we are also not checking this ack x defer (here nor in native_write) replies anymore. Don't we need it anymore? why? > - else > - return -EIO; > - } > + switch (msg->request & ~DP_AUX_I2C_MOT) { > + case DP_AUX_NATIVE_WRITE: > + case DP_AUX_I2C_WRITE: > + txsize = HEADER_SIZE + msg->size; > + rxsize = 1; > > - DRM_ERROR("too many retries, giving up\n"); > - return -EIO; > -} > + if (WARN_ON(txsize > 20)) > + return -E2BIG; > > -/* Write a single byte to the aux channel in native mode */ > -static int > -intel_dp_aux_native_write_1(struct intel_dp *intel_dp, > - uint16_t address, uint8_t byte) > -{ > - return intel_dp_aux_native_write(intel_dp, address, &byte, 1); > -} > + memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size); > > -/* read bytes from a native aux channel */ > -static int > -intel_dp_aux_native_read(struct intel_dp *intel_dp, > - uint16_t address, uint8_t *recv, int recv_bytes) > -{ > - uint8_t msg[4]; > - int msg_bytes; > - uint8_t reply[20]; > - int reply_bytes; > - uint8_t ack; > - int ret; > - int retry; > + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); > + if (ret > 0) { > + msg->reply = rxbuf[0] >> 4; > > - if (WARN_ON(recv_bytes > 19)) > - return -E2BIG; > + /* Return payload size. */ > + ret = msg->size; > + } > + break; > > - intel_dp_check_edp(intel_dp); > - msg[0] = DP_AUX_NATIVE_READ << 4; > - msg[1] = address >> 8; > - msg[2] = address & 0xff; > - msg[3] = recv_bytes - 1; > + case DP_AUX_NATIVE_READ: > + case DP_AUX_I2C_READ: > + txsize = HEADER_SIZE; > + rxsize = msg->size + 1; > > - msg_bytes = 4; > - reply_bytes = recv_bytes + 1; > + if (WARN_ON(rxsize > 20)) > + return -E2BIG; > > - for (retry = 0; retry < 7; retry++) { > - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, > - reply, reply_bytes); > - if (ret == 0) > - return -EPROTO; > - if (ret < 0) > - return ret; > - ack = reply[0] >> 4; > - if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) { > - memcpy(recv, reply + 1, ret - 1); > - return ret - 1; > + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); > + if (ret > 0) { > + msg->reply = rxbuf[0] >> 4; > + /* > + * Assume happy day, and copy the data. The caller is > + * expected to check msg->reply before touching it. > + * > + * Return payload size. > + */ > + ret--; > + memcpy(msg->buffer, rxbuf + 1, ret); > } > - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER) > - usleep_range(400, 500); > - else > - return -EIO; > + break; > + > + default: > + ret = -EINVAL; > + break; > } > > - DRM_ERROR("too many retries, giving up\n"); > - return -EIO; > + return ret; > +} > + > +static void > +intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector) > +{ > + struct drm_device *dev = intel_dp_to_dev(intel_dp); > + > + intel_dp->aux.dev = dev->dev; > + intel_dp->aux.transfer = intel_dp_aux_transfer; > } > > static int > @@ -1472,8 +1452,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) > return; > > if (mode != DRM_MODE_DPMS_ON) { > - ret = intel_dp_aux_native_write_1(intel_dp, DP_SET_POWER, > - DP_SET_POWER_D3); > + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, > + DP_SET_POWER_D3); > if (ret != 1) > DRM_DEBUG_DRIVER("failed to write sink power state\n"); > } else { > @@ -1482,9 +1462,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) > * time to wake up. > */ > for (i = 0; i < 3; i++) { > - ret = intel_dp_aux_native_write_1(intel_dp, > - DP_SET_POWER, > - DP_SET_POWER_D0); > + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, > + DP_SET_POWER_D0); > if (ret == 1) > break; > msleep(1); > @@ -1708,13 +1687,11 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp) > > /* Enable PSR in sink */ > if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) > - intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, > - DP_PSR_ENABLE & > - ~DP_PSR_MAIN_LINK_ACTIVE); > + drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, > + DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE); > else > - intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, > - DP_PSR_ENABLE | > - DP_PSR_MAIN_LINK_ACTIVE); > + drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, > + DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE); > > /* Setup AUX registers */ > I915_WRITE(EDP_PSR_AUX_DATA1(dev), EDP_PSR_DPCD_COMMAND); > @@ -2026,26 +2003,25 @@ static void vlv_dp_pre_pll_enable(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 bool > -intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address, > - uint8_t *recv, int recv_bytes) > +static ssize_t > +intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, > + void *buffer, size_t size) > { > - int ret, i; > + ssize_t ret; > + int i; > > - /* > - * Sinks are *supposed* to come up within 1ms from an off state, > - * but we're also supposed to retry 3 times per the spec. > - */ > for (i = 0; i < 3; i++) { > - ret = intel_dp_aux_native_read(intel_dp, address, recv, > - recv_bytes); > - if (ret == recv_bytes) > - return true; > + ret = drm_dp_dpcd_read(aux, offset, buffer, size); > + if (ret == size) > + return ret; > msleep(1); > } > > - return false; > + return ret; > } > > /* > @@ -2055,10 +2031,10 @@ intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address, > static bool > intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) > { > - return intel_dp_aux_native_read_retry(intel_dp, > - DP_LANE0_1_STATUS, > - 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; > } > > /* > @@ -2572,8 +2548,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, > len = intel_dp->lane_count + 1; > } > > - ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_PATTERN_SET, > - buf, len); > + ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET, > + buf, len); > > return ret == len; > } > @@ -2602,9 +2578,8 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP, > I915_WRITE(intel_dp->output_reg, *DP); > POSTING_READ(intel_dp->output_reg); > > - ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_LANE0_SET, > - intel_dp->train_set, > - intel_dp->lane_count); > + ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET, > + intel_dp->train_set, intel_dp->lane_count); > > return ret == intel_dp->lane_count; > } > @@ -2660,11 +2635,11 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) > link_config[1] = intel_dp->lane_count; > if (drm_dp_enhanced_frame_cap(intel_dp->dpcd)) > link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN; > - intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, link_config, 2); > + drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2); > > link_config[0] = 0; > link_config[1] = DP_SET_ANSI_8B10B; > - intel_dp_aux_native_write(intel_dp, DP_DOWNSPREAD_CTRL, link_config, 2); > + drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2); > > DP |= DP_PORT_EN; > > @@ -2907,8 +2882,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > > char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3]; > > - if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd, > - sizeof(intel_dp->dpcd)) == 0) > + if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, > + sizeof(intel_dp->dpcd)) < 0) > return false; /* aux transfer failed */ > > hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd), > @@ -2921,9 +2896,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_dpcd_read_wake(&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"); > @@ -2945,9 +2920,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_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0, > - intel_dp->downstream_ports, > - DP_MAX_DOWNSTREAM_PORTS) == 0) > + if (intel_dp_dpcd_read_wake(&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; > @@ -2963,11 +2938,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_dpcd_read_wake(&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_aux_native_read_retry(intel_dp, DP_BRANCH_OUI, buf, 3)) > + if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) > DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", > buf[0], buf[1], buf[2]); > > @@ -2982,46 +2957,40 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) > to_intel_crtc(intel_dig_port->base.base.crtc); > u8 buf[1]; > > - if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1)) > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0); > return -EAGAIN; > > if (!(buf[0] & DP_TEST_CRC_SUPPORTED)) > return -ENOTTY; > > - if (!intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, > - DP_TEST_SINK_START)) > + if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, > + DP_TEST_SINK_START) < 0) > return -EAGAIN; > > /* Wait 2 vblanks to be sure we will have the correct CRC value */ > intel_wait_for_vblank(dev, intel_crtc->pipe); > intel_wait_for_vblank(dev, intel_crtc->pipe); > > - if (!intel_dp_aux_native_read(intel_dp, DP_TEST_CRC_R_CR, crc, 6)) > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) > return -EAGAIN; > > - intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, 0); > + drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0); > return 0; > } > > static 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_dpcd_read_wake(&intel_dp->aux, > + DP_DEVICE_SERVICE_IRQ_VECTOR, > + sink_irq_vector, 1) == 1; > } > > static void > intel_dp_handle_test_request(struct intel_dp *intel_dp) > { > /* NAK by default */ > - intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, DP_TEST_NAK); > + drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_RESPONSE, DP_TEST_NAK); > } > > /* > @@ -3060,9 +3029,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) > if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && > intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) { > /* Clear interrupt source */ > - intel_dp_aux_native_write_1(intel_dp, > - DP_DEVICE_SERVICE_IRQ_VECTOR, > - sink_irq_vector); > + drm_dp_dpcd_writeb(&intel_dp->aux, > + DP_DEVICE_SERVICE_IRQ_VECTOR, > + sink_irq_vector); > > if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST) > intel_dp_handle_test_request(intel_dp); > @@ -3097,9 +3066,11 @@ 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_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, > + ®, 1) < 0) > return connector_status_unknown; > + > return DP_GET_SINK_COUNT(reg) ? connector_status_connected > : connector_status_disconnected; > } > @@ -3925,6 +3896,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq); > } > > + intel_dp_aux_init(intel_dp, intel_connector); > + > error = intel_dp_i2c_init(intel_dp, intel_connector, name); > WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n", > error, port_name(port)); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 2546cae0b4f0..561af4b013b1 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -491,6 +491,7 @@ struct intel_dp { > uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; > struct i2c_adapter adapter; > struct i2c_algo_dp_aux_data algo; > + struct drm_dp_aux aux; > uint8_t train_set[4]; > int panel_power_up_delay; > int panel_power_down_delay; > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Mar 17, 2014 at 11:08 AM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote: > On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula <jani.nikula@intel.com> wrote: >> Functionality remains largely the same as before. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 257 +++++++++++++++++--------------------- >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> 2 files changed, 116 insertions(+), 142 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 22134edc03dd..24cbf4bd36c4 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -575,97 +575,77 @@ out: >> return ret; >> } >> >> -/* Write data to the aux channel in native mode */ >> -static int >> -intel_dp_aux_native_write(struct intel_dp *intel_dp, >> - uint16_t address, uint8_t *send, int send_bytes) >> +#define HEADER_SIZE 4 >> +static ssize_t >> +intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >> { >> + struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux); >> + uint8_t txbuf[20], rxbuf[20]; >> + size_t txsize, rxsize; >> int ret; >> - uint8_t msg[20]; >> - int msg_bytes; >> - uint8_t ack; >> - int retry; >> >> - if (WARN_ON(send_bytes > 16)) >> - return -E2BIG; >> + txbuf[0] = msg->request << 4; >> + txbuf[1] = msg->address >> 8; >> + txbuf[2] = msg->address & 0xff; >> + txbuf[3] = msg->size - 1; >> >> - intel_dp_check_edp(intel_dp); >> - msg[0] = DP_AUX_NATIVE_WRITE << 4; >> - msg[1] = address >> 8; >> - msg[2] = address & 0xff; >> - msg[3] = send_bytes - 1; >> - memcpy(&msg[4], send, send_bytes); >> - msg_bytes = send_bytes + 4; >> - for (retry = 0; retry < 7; retry++) { > > we were retrying 7 times always, but now we are now retrying only 3 > times or even none. > Couldn't it trigger some new bug or regression? > >> - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1); >> - if (ret < 0) >> - return ret; >> - ack >>= 4; >> - if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) >> - return send_bytes; >> - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER) >> - usleep_range(400, 500); > > we are also not checking this ack x defer (here nor in native_write) > replies anymore. > Don't we need it anymore? why? > >> - else >> - return -EIO; >> - } >> + switch (msg->request & ~DP_AUX_I2C_MOT) { >> + case DP_AUX_NATIVE_WRITE: >> + case DP_AUX_I2C_WRITE: >> + txsize = HEADER_SIZE + msg->size; >> + rxsize = 1; >> >> - DRM_ERROR("too many retries, giving up\n"); >> - return -EIO; >> -} >> + if (WARN_ON(txsize > 20)) >> + return -E2BIG; >> >> -/* Write a single byte to the aux channel in native mode */ >> -static int >> -intel_dp_aux_native_write_1(struct intel_dp *intel_dp, >> - uint16_t address, uint8_t byte) >> -{ >> - return intel_dp_aux_native_write(intel_dp, address, &byte, 1); >> -} >> + memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size); >> >> -/* read bytes from a native aux channel */ >> -static int >> -intel_dp_aux_native_read(struct intel_dp *intel_dp, >> - uint16_t address, uint8_t *recv, int recv_bytes) >> -{ >> - uint8_t msg[4]; >> - int msg_bytes; >> - uint8_t reply[20]; >> - int reply_bytes; >> - uint8_t ack; >> - int ret; >> - int retry; >> + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); >> + if (ret > 0) { >> + msg->reply = rxbuf[0] >> 4; >> >> - if (WARN_ON(recv_bytes > 19)) >> - return -E2BIG; >> + /* Return payload size. */ >> + ret = msg->size; >> + } >> + break; >> >> - intel_dp_check_edp(intel_dp); >> - msg[0] = DP_AUX_NATIVE_READ << 4; >> - msg[1] = address >> 8; >> - msg[2] = address & 0xff; >> - msg[3] = recv_bytes - 1; >> + case DP_AUX_NATIVE_READ: >> + case DP_AUX_I2C_READ: >> + txsize = HEADER_SIZE; >> + rxsize = msg->size + 1; >> >> - msg_bytes = 4; >> - reply_bytes = recv_bytes + 1; >> + if (WARN_ON(rxsize > 20)) >> + return -E2BIG; >> >> - for (retry = 0; retry < 7; retry++) { >> - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, >> - reply, reply_bytes); >> - if (ret == 0) >> - return -EPROTO; >> - if (ret < 0) >> - return ret; >> - ack = reply[0] >> 4; >> - if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) { >> - memcpy(recv, reply + 1, ret - 1); >> - return ret - 1; >> + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); >> + if (ret > 0) { >> + msg->reply = rxbuf[0] >> 4; >> + /* >> + * Assume happy day, and copy the data. The caller is >> + * expected to check msg->reply before touching it. >> + * >> + * Return payload size. >> + */ >> + ret--; >> + memcpy(msg->buffer, rxbuf + 1, ret); >> } >> - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER) >> - usleep_range(400, 500); >> - else >> - return -EIO; >> + break; >> + >> + default: >> + ret = -EINVAL; >> + break; >> } >> >> - DRM_ERROR("too many retries, giving up\n"); >> - return -EIO; >> + return ret; >> +} >> + >> +static void >> +intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector) >> +{ >> + struct drm_device *dev = intel_dp_to_dev(intel_dp); >> + >> + intel_dp->aux.dev = dev->dev; >> + intel_dp->aux.transfer = intel_dp_aux_transfer; >> } >> >> static int >> @@ -1472,8 +1452,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) >> return; >> >> if (mode != DRM_MODE_DPMS_ON) { >> - ret = intel_dp_aux_native_write_1(intel_dp, DP_SET_POWER, >> - DP_SET_POWER_D3); >> + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, >> + DP_SET_POWER_D3); >> if (ret != 1) >> DRM_DEBUG_DRIVER("failed to write sink power state\n"); >> } else { >> @@ -1482,9 +1462,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) >> * time to wake up. >> */ >> for (i = 0; i < 3; i++) { >> - ret = intel_dp_aux_native_write_1(intel_dp, >> - DP_SET_POWER, >> - DP_SET_POWER_D0); >> + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, >> + DP_SET_POWER_D0); >> if (ret == 1) >> break; >> msleep(1); >> @@ -1708,13 +1687,11 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp) >> >> /* Enable PSR in sink */ >> if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) >> - intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, >> - DP_PSR_ENABLE & >> - ~DP_PSR_MAIN_LINK_ACTIVE); >> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, >> + DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE); >> else >> - intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, >> - DP_PSR_ENABLE | >> - DP_PSR_MAIN_LINK_ACTIVE); >> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, >> + DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE); >> >> /* Setup AUX registers */ >> I915_WRITE(EDP_PSR_AUX_DATA1(dev), EDP_PSR_DPCD_COMMAND); >> @@ -2026,26 +2003,25 @@ static void vlv_dp_pre_pll_enable(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 bool >> -intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address, >> - uint8_t *recv, int recv_bytes) >> +static ssize_t >> +intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, >> + void *buffer, size_t size) >> { >> - int ret, i; >> + ssize_t ret; >> + int i; >> >> - /* >> - * Sinks are *supposed* to come up within 1ms from an off state, >> - * but we're also supposed to retry 3 times per the spec. >> - */ >> for (i = 0; i < 3; i++) { >> - ret = intel_dp_aux_native_read(intel_dp, address, recv, >> - recv_bytes); >> - if (ret == recv_bytes) >> - return true; >> + ret = drm_dp_dpcd_read(aux, offset, buffer, size); >> + if (ret == size) >> + return ret; >> msleep(1); >> } >> >> - return false; >> + return ret; >> } >> >> /* >> @@ -2055,10 +2031,10 @@ intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address, >> static bool >> intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) >> { >> - return intel_dp_aux_native_read_retry(intel_dp, >> - DP_LANE0_1_STATUS, >> - 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; >> } >> >> /* >> @@ -2572,8 +2548,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, >> len = intel_dp->lane_count + 1; >> } >> >> - ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_PATTERN_SET, >> - buf, len); >> + ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET, >> + buf, len); >> >> return ret == len; >> } >> @@ -2602,9 +2578,8 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP, >> I915_WRITE(intel_dp->output_reg, *DP); >> POSTING_READ(intel_dp->output_reg); >> >> - ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_LANE0_SET, >> - intel_dp->train_set, >> - intel_dp->lane_count); >> + ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET, >> + intel_dp->train_set, intel_dp->lane_count); >> >> return ret == intel_dp->lane_count; >> } >> @@ -2660,11 +2635,11 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) >> link_config[1] = intel_dp->lane_count; >> if (drm_dp_enhanced_frame_cap(intel_dp->dpcd)) >> link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN; >> - intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, link_config, 2); >> + drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2); >> >> link_config[0] = 0; >> link_config[1] = DP_SET_ANSI_8B10B; >> - intel_dp_aux_native_write(intel_dp, DP_DOWNSPREAD_CTRL, link_config, 2); >> + drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2); >> >> DP |= DP_PORT_EN; >> >> @@ -2907,8 +2882,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) >> >> char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3]; >> >> - if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd, >> - sizeof(intel_dp->dpcd)) == 0) >> + if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, >> + sizeof(intel_dp->dpcd)) < 0) >> return false; /* aux transfer failed */ >> >> hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd), >> @@ -2921,9 +2896,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_dpcd_read_wake(&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"); >> @@ -2945,9 +2920,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_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0, >> - intel_dp->downstream_ports, >> - DP_MAX_DOWNSTREAM_PORTS) == 0) >> + if (intel_dp_dpcd_read_wake(&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; >> @@ -2963,11 +2938,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_dpcd_read_wake(&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_aux_native_read_retry(intel_dp, DP_BRANCH_OUI, buf, 3)) >> + if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) >> DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", >> buf[0], buf[1], buf[2]); >> >> @@ -2982,46 +2957,40 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) >> to_intel_crtc(intel_dig_port->base.base.crtc); >> u8 buf[1]; >> >> - if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1)) >> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0); wrong ";" here.. >> return -EAGAIN; >> >> if (!(buf[0] & DP_TEST_CRC_SUPPORTED)) >> return -ENOTTY; >> >> - if (!intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, >> - DP_TEST_SINK_START)) >> + if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, >> + DP_TEST_SINK_START) < 0) >> return -EAGAIN; >> >> /* Wait 2 vblanks to be sure we will have the correct CRC value */ >> intel_wait_for_vblank(dev, intel_crtc->pipe); >> intel_wait_for_vblank(dev, intel_crtc->pipe); >> >> - if (!intel_dp_aux_native_read(intel_dp, DP_TEST_CRC_R_CR, crc, 6)) >> + if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) >> return -EAGAIN; >> >> - intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, 0); >> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0); >> return 0; >> } >> >> static 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_dpcd_read_wake(&intel_dp->aux, >> + DP_DEVICE_SERVICE_IRQ_VECTOR, >> + sink_irq_vector, 1) == 1; >> } >> >> static void >> intel_dp_handle_test_request(struct intel_dp *intel_dp) >> { >> /* NAK by default */ >> - intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, DP_TEST_NAK); >> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_RESPONSE, DP_TEST_NAK); >> } >> >> /* >> @@ -3060,9 +3029,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) >> if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && >> intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) { >> /* Clear interrupt source */ >> - intel_dp_aux_native_write_1(intel_dp, >> - DP_DEVICE_SERVICE_IRQ_VECTOR, >> - sink_irq_vector); >> + drm_dp_dpcd_writeb(&intel_dp->aux, >> + DP_DEVICE_SERVICE_IRQ_VECTOR, >> + sink_irq_vector); >> >> if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST) >> intel_dp_handle_test_request(intel_dp); >> @@ -3097,9 +3066,11 @@ 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_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, >> + ®, 1) < 0) >> return connector_status_unknown; >> + >> return DP_GET_SINK_COUNT(reg) ? connector_status_connected >> : connector_status_disconnected; >> } >> @@ -3925,6 +3896,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, >> intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq); >> } >> >> + intel_dp_aux_init(intel_dp, intel_connector); >> + >> error = intel_dp_i2c_init(intel_dp, intel_connector, name); >> WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n", >> error, port_name(port)); >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 2546cae0b4f0..561af4b013b1 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -491,6 +491,7 @@ struct intel_dp { >> uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; >> struct i2c_adapter adapter; >> struct i2c_algo_dp_aux_data algo; >> + struct drm_dp_aux aux; >> uint8_t train_set[4]; >> int panel_power_up_delay; >> int panel_power_down_delay; >> -- >> 1.7.9.5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br
On Mon, 17 Mar 2014, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote: > On Mon, Mar 17, 2014 at 11:08 AM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote: >> On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula <jani.nikula@intel.com> wrote: >>> - if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1)) >>> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0); > > wrong ";" here.. Yikes, good catch. I've been rebasing this way too many times... Jani.
On Mon, 17 Mar 2014, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote: > On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula <jani.nikula@intel.com> wrote: >> Functionality remains largely the same as before. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 257 +++++++++++++++++--------------------- >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> 2 files changed, 116 insertions(+), 142 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 22134edc03dd..24cbf4bd36c4 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -575,97 +575,77 @@ out: >> return ret; >> } >> >> -/* Write data to the aux channel in native mode */ >> -static int >> -intel_dp_aux_native_write(struct intel_dp *intel_dp, >> - uint16_t address, uint8_t *send, int send_bytes) >> +#define HEADER_SIZE 4 >> +static ssize_t >> +intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >> { >> + struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux); >> + uint8_t txbuf[20], rxbuf[20]; >> + size_t txsize, rxsize; >> int ret; >> - uint8_t msg[20]; >> - int msg_bytes; >> - uint8_t ack; >> - int retry; >> >> - if (WARN_ON(send_bytes > 16)) >> - return -E2BIG; >> + txbuf[0] = msg->request << 4; >> + txbuf[1] = msg->address >> 8; >> + txbuf[2] = msg->address & 0xff; >> + txbuf[3] = msg->size - 1; >> >> - intel_dp_check_edp(intel_dp); >> - msg[0] = DP_AUX_NATIVE_WRITE << 4; >> - msg[1] = address >> 8; >> - msg[2] = address & 0xff; >> - msg[3] = send_bytes - 1; >> - memcpy(&msg[4], send, send_bytes); >> - msg_bytes = send_bytes + 4; >> - for (retry = 0; retry < 7; retry++) { > > we were retrying 7 times always, but now we are now retrying only 3 > times or even none. I'm confused, what are you referring to? drm_dp_dpcd_access() in drm_dp_helper.c has the retry loop now. I think you're mixing this with the intel_dp_dpcd_read_wake() wrapper, but that ends up in drm_dp_dpcd_access() as well. > Couldn't it trigger some new bug or regression? > >> - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1); >> - if (ret < 0) >> - return ret; >> - ack >>= 4; >> - if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) >> - return send_bytes; >> - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER) >> - usleep_range(400, 500); > > we are also not checking this ack x defer (here nor in native_write) > replies anymore. Ditto, we store ack >>= 4 into msg->reply which gets checked in drm_dp_dpcd_access(). BR, Jani. > Don't we need it anymore? why? > >> - else >> - return -EIO; >> - } >> + switch (msg->request & ~DP_AUX_I2C_MOT) { >> + case DP_AUX_NATIVE_WRITE: >> + case DP_AUX_I2C_WRITE: >> + txsize = HEADER_SIZE + msg->size; >> + rxsize = 1; >> >> - DRM_ERROR("too many retries, giving up\n"); >> - return -EIO; >> -} >> + if (WARN_ON(txsize > 20)) >> + return -E2BIG; >> >> -/* Write a single byte to the aux channel in native mode */ >> -static int >> -intel_dp_aux_native_write_1(struct intel_dp *intel_dp, >> - uint16_t address, uint8_t byte) >> -{ >> - return intel_dp_aux_native_write(intel_dp, address, &byte, 1); >> -} >> + memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size); >> >> -/* read bytes from a native aux channel */ >> -static int >> -intel_dp_aux_native_read(struct intel_dp *intel_dp, >> - uint16_t address, uint8_t *recv, int recv_bytes) >> -{ >> - uint8_t msg[4]; >> - int msg_bytes; >> - uint8_t reply[20]; >> - int reply_bytes; >> - uint8_t ack; >> - int ret; >> - int retry; >> + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); >> + if (ret > 0) { >> + msg->reply = rxbuf[0] >> 4; >> >> - if (WARN_ON(recv_bytes > 19)) >> - return -E2BIG; >> + /* Return payload size. */ >> + ret = msg->size; >> + } >> + break; >> >> - intel_dp_check_edp(intel_dp); >> - msg[0] = DP_AUX_NATIVE_READ << 4; >> - msg[1] = address >> 8; >> - msg[2] = address & 0xff; >> - msg[3] = recv_bytes - 1; >> + case DP_AUX_NATIVE_READ: >> + case DP_AUX_I2C_READ: >> + txsize = HEADER_SIZE; >> + rxsize = msg->size + 1; >> >> - msg_bytes = 4; >> - reply_bytes = recv_bytes + 1; >> + if (WARN_ON(rxsize > 20)) >> + return -E2BIG; >> >> - for (retry = 0; retry < 7; retry++) { >> - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, >> - reply, reply_bytes); >> - if (ret == 0) >> - return -EPROTO; >> - if (ret < 0) >> - return ret; >> - ack = reply[0] >> 4; >> - if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) { >> - memcpy(recv, reply + 1, ret - 1); >> - return ret - 1; >> + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); >> + if (ret > 0) { >> + msg->reply = rxbuf[0] >> 4; >> + /* >> + * Assume happy day, and copy the data. The caller is >> + * expected to check msg->reply before touching it. >> + * >> + * Return payload size. >> + */ >> + ret--; >> + memcpy(msg->buffer, rxbuf + 1, ret); >> } >> - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER) >> - usleep_range(400, 500); >> - else >> - return -EIO; >> + break; >> + >> + default: >> + ret = -EINVAL; >> + break; >> } >> >> - DRM_ERROR("too many retries, giving up\n"); >> - return -EIO; >> + return ret; >> +} >> + >> +static void >> +intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector) >> +{ >> + struct drm_device *dev = intel_dp_to_dev(intel_dp); >> + >> + intel_dp->aux.dev = dev->dev; >> + intel_dp->aux.transfer = intel_dp_aux_transfer; >> } >> >> static int >> @@ -1472,8 +1452,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) >> return; >> >> if (mode != DRM_MODE_DPMS_ON) { >> - ret = intel_dp_aux_native_write_1(intel_dp, DP_SET_POWER, >> - DP_SET_POWER_D3); >> + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, >> + DP_SET_POWER_D3); >> if (ret != 1) >> DRM_DEBUG_DRIVER("failed to write sink power state\n"); >> } else { >> @@ -1482,9 +1462,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) >> * time to wake up. >> */ >> for (i = 0; i < 3; i++) { >> - ret = intel_dp_aux_native_write_1(intel_dp, >> - DP_SET_POWER, >> - DP_SET_POWER_D0); >> + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, >> + DP_SET_POWER_D0); >> if (ret == 1) >> break; >> msleep(1); >> @@ -1708,13 +1687,11 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp) >> >> /* Enable PSR in sink */ >> if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) >> - intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, >> - DP_PSR_ENABLE & >> - ~DP_PSR_MAIN_LINK_ACTIVE); >> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, >> + DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE); >> else >> - intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, >> - DP_PSR_ENABLE | >> - DP_PSR_MAIN_LINK_ACTIVE); >> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, >> + DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE); >> >> /* Setup AUX registers */ >> I915_WRITE(EDP_PSR_AUX_DATA1(dev), EDP_PSR_DPCD_COMMAND); >> @@ -2026,26 +2003,25 @@ static void vlv_dp_pre_pll_enable(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 bool >> -intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address, >> - uint8_t *recv, int recv_bytes) >> +static ssize_t >> +intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, >> + void *buffer, size_t size) >> { >> - int ret, i; >> + ssize_t ret; >> + int i; >> >> - /* >> - * Sinks are *supposed* to come up within 1ms from an off state, >> - * but we're also supposed to retry 3 times per the spec. >> - */ >> for (i = 0; i < 3; i++) { >> - ret = intel_dp_aux_native_read(intel_dp, address, recv, >> - recv_bytes); >> - if (ret == recv_bytes) >> - return true; >> + ret = drm_dp_dpcd_read(aux, offset, buffer, size); >> + if (ret == size) >> + return ret; >> msleep(1); >> } >> >> - return false; >> + return ret; >> } >> >> /* >> @@ -2055,10 +2031,10 @@ intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address, >> static bool >> intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) >> { >> - return intel_dp_aux_native_read_retry(intel_dp, >> - DP_LANE0_1_STATUS, >> - 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; >> } >> >> /* >> @@ -2572,8 +2548,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, >> len = intel_dp->lane_count + 1; >> } >> >> - ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_PATTERN_SET, >> - buf, len); >> + ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET, >> + buf, len); >> >> return ret == len; >> } >> @@ -2602,9 +2578,8 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP, >> I915_WRITE(intel_dp->output_reg, *DP); >> POSTING_READ(intel_dp->output_reg); >> >> - ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_LANE0_SET, >> - intel_dp->train_set, >> - intel_dp->lane_count); >> + ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET, >> + intel_dp->train_set, intel_dp->lane_count); >> >> return ret == intel_dp->lane_count; >> } >> @@ -2660,11 +2635,11 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) >> link_config[1] = intel_dp->lane_count; >> if (drm_dp_enhanced_frame_cap(intel_dp->dpcd)) >> link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN; >> - intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, link_config, 2); >> + drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2); >> >> link_config[0] = 0; >> link_config[1] = DP_SET_ANSI_8B10B; >> - intel_dp_aux_native_write(intel_dp, DP_DOWNSPREAD_CTRL, link_config, 2); >> + drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2); >> >> DP |= DP_PORT_EN; >> >> @@ -2907,8 +2882,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) >> >> char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3]; >> >> - if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd, >> - sizeof(intel_dp->dpcd)) == 0) >> + if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, >> + sizeof(intel_dp->dpcd)) < 0) >> return false; /* aux transfer failed */ >> >> hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd), >> @@ -2921,9 +2896,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_dpcd_read_wake(&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"); >> @@ -2945,9 +2920,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_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0, >> - intel_dp->downstream_ports, >> - DP_MAX_DOWNSTREAM_PORTS) == 0) >> + if (intel_dp_dpcd_read_wake(&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; >> @@ -2963,11 +2938,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_dpcd_read_wake(&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_aux_native_read_retry(intel_dp, DP_BRANCH_OUI, buf, 3)) >> + if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) >> DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", >> buf[0], buf[1], buf[2]); >> >> @@ -2982,46 +2957,40 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) >> to_intel_crtc(intel_dig_port->base.base.crtc); >> u8 buf[1]; >> >> - if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1)) >> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0); >> return -EAGAIN; >> >> if (!(buf[0] & DP_TEST_CRC_SUPPORTED)) >> return -ENOTTY; >> >> - if (!intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, >> - DP_TEST_SINK_START)) >> + if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, >> + DP_TEST_SINK_START) < 0) >> return -EAGAIN; >> >> /* Wait 2 vblanks to be sure we will have the correct CRC value */ >> intel_wait_for_vblank(dev, intel_crtc->pipe); >> intel_wait_for_vblank(dev, intel_crtc->pipe); >> >> - if (!intel_dp_aux_native_read(intel_dp, DP_TEST_CRC_R_CR, crc, 6)) >> + if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) >> return -EAGAIN; >> >> - intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, 0); >> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0); >> return 0; >> } >> >> static 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_dpcd_read_wake(&intel_dp->aux, >> + DP_DEVICE_SERVICE_IRQ_VECTOR, >> + sink_irq_vector, 1) == 1; >> } >> >> static void >> intel_dp_handle_test_request(struct intel_dp *intel_dp) >> { >> /* NAK by default */ >> - intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, DP_TEST_NAK); >> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_RESPONSE, DP_TEST_NAK); >> } >> >> /* >> @@ -3060,9 +3029,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) >> if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && >> intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) { >> /* Clear interrupt source */ >> - intel_dp_aux_native_write_1(intel_dp, >> - DP_DEVICE_SERVICE_IRQ_VECTOR, >> - sink_irq_vector); >> + drm_dp_dpcd_writeb(&intel_dp->aux, >> + DP_DEVICE_SERVICE_IRQ_VECTOR, >> + sink_irq_vector); >> >> if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST) >> intel_dp_handle_test_request(intel_dp); >> @@ -3097,9 +3066,11 @@ 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_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, >> + ®, 1) < 0) >> return connector_status_unknown; >> + >> return DP_GET_SINK_COUNT(reg) ? connector_status_connected >> : connector_status_disconnected; >> } >> @@ -3925,6 +3896,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, >> intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq); >> } >> >> + intel_dp_aux_init(intel_dp, intel_connector); >> + >> error = intel_dp_i2c_init(intel_dp, intel_connector, name); >> WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n", >> error, port_name(port)); >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 2546cae0b4f0..561af4b013b1 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -491,6 +491,7 @@ struct intel_dp { >> uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; >> struct i2c_adapter adapter; >> struct i2c_algo_dp_aux_data algo; >> + struct drm_dp_aux aux; >> uint8_t train_set[4]; >> int panel_power_up_delay; >> int panel_power_down_delay; >> -- >> 1.7.9.5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br
Thanks for explanations Jani and Daniel. Feel free to use Reviewe-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> On Tue, Mar 18, 2014 at 7:54 AM, Jani Nikula <jani.nikula@intel.com> wrote: > On Mon, 17 Mar 2014, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote: >> On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula <jani.nikula@intel.com> wrote: >>> Functionality remains largely the same as before. >>> >>> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_dp.c | 257 +++++++++++++++++--------------------- >>> drivers/gpu/drm/i915/intel_drv.h | 1 + >>> 2 files changed, 116 insertions(+), 142 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >>> index 22134edc03dd..24cbf4bd36c4 100644 >>> --- a/drivers/gpu/drm/i915/intel_dp.c >>> +++ b/drivers/gpu/drm/i915/intel_dp.c >>> @@ -575,97 +575,77 @@ out: >>> return ret; >>> } >>> >>> -/* Write data to the aux channel in native mode */ >>> -static int >>> -intel_dp_aux_native_write(struct intel_dp *intel_dp, >>> - uint16_t address, uint8_t *send, int send_bytes) >>> +#define HEADER_SIZE 4 >>> +static ssize_t >>> +intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >>> { >>> + struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux); >>> + uint8_t txbuf[20], rxbuf[20]; >>> + size_t txsize, rxsize; >>> int ret; >>> - uint8_t msg[20]; >>> - int msg_bytes; >>> - uint8_t ack; >>> - int retry; >>> >>> - if (WARN_ON(send_bytes > 16)) >>> - return -E2BIG; >>> + txbuf[0] = msg->request << 4; >>> + txbuf[1] = msg->address >> 8; >>> + txbuf[2] = msg->address & 0xff; >>> + txbuf[3] = msg->size - 1; >>> >>> - intel_dp_check_edp(intel_dp); >>> - msg[0] = DP_AUX_NATIVE_WRITE << 4; >>> - msg[1] = address >> 8; >>> - msg[2] = address & 0xff; >>> - msg[3] = send_bytes - 1; >>> - memcpy(&msg[4], send, send_bytes); >>> - msg_bytes = send_bytes + 4; >>> - for (retry = 0; retry < 7; retry++) { >> >> we were retrying 7 times always, but now we are now retrying only 3 >> times or even none. > > I'm confused, what are you referring to? drm_dp_dpcd_access() in > drm_dp_helper.c has the retry loop now. > > I think you're mixing this with the intel_dp_dpcd_read_wake() wrapper, > but that ends up in drm_dp_dpcd_access() as well. > >> Couldn't it trigger some new bug or regression? >> >>> - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1); >>> - if (ret < 0) >>> - return ret; >>> - ack >>= 4; >>> - if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) >>> - return send_bytes; >>> - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER) >>> - usleep_range(400, 500); >> >> we are also not checking this ack x defer (here nor in native_write) >> replies anymore. > > Ditto, we store ack >>= 4 into msg->reply which gets checked in > drm_dp_dpcd_access(). > > BR, > Jani. > >> Don't we need it anymore? why? >> >>> - else >>> - return -EIO; >>> - } >>> + switch (msg->request & ~DP_AUX_I2C_MOT) { >>> + case DP_AUX_NATIVE_WRITE: >>> + case DP_AUX_I2C_WRITE: >>> + txsize = HEADER_SIZE + msg->size; >>> + rxsize = 1; >>> >>> - DRM_ERROR("too many retries, giving up\n"); >>> - return -EIO; >>> -} >>> + if (WARN_ON(txsize > 20)) >>> + return -E2BIG; >>> >>> -/* Write a single byte to the aux channel in native mode */ >>> -static int >>> -intel_dp_aux_native_write_1(struct intel_dp *intel_dp, >>> - uint16_t address, uint8_t byte) >>> -{ >>> - return intel_dp_aux_native_write(intel_dp, address, &byte, 1); >>> -} >>> + memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size); >>> >>> -/* read bytes from a native aux channel */ >>> -static int >>> -intel_dp_aux_native_read(struct intel_dp *intel_dp, >>> - uint16_t address, uint8_t *recv, int recv_bytes) >>> -{ >>> - uint8_t msg[4]; >>> - int msg_bytes; >>> - uint8_t reply[20]; >>> - int reply_bytes; >>> - uint8_t ack; >>> - int ret; >>> - int retry; >>> + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); >>> + if (ret > 0) { >>> + msg->reply = rxbuf[0] >> 4; >>> >>> - if (WARN_ON(recv_bytes > 19)) >>> - return -E2BIG; >>> + /* Return payload size. */ >>> + ret = msg->size; >>> + } >>> + break; >>> >>> - intel_dp_check_edp(intel_dp); >>> - msg[0] = DP_AUX_NATIVE_READ << 4; >>> - msg[1] = address >> 8; >>> - msg[2] = address & 0xff; >>> - msg[3] = recv_bytes - 1; >>> + case DP_AUX_NATIVE_READ: >>> + case DP_AUX_I2C_READ: >>> + txsize = HEADER_SIZE; >>> + rxsize = msg->size + 1; >>> >>> - msg_bytes = 4; >>> - reply_bytes = recv_bytes + 1; >>> + if (WARN_ON(rxsize > 20)) >>> + return -E2BIG; >>> >>> - for (retry = 0; retry < 7; retry++) { >>> - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, >>> - reply, reply_bytes); >>> - if (ret == 0) >>> - return -EPROTO; >>> - if (ret < 0) >>> - return ret; >>> - ack = reply[0] >> 4; >>> - if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) { >>> - memcpy(recv, reply + 1, ret - 1); >>> - return ret - 1; >>> + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); >>> + if (ret > 0) { >>> + msg->reply = rxbuf[0] >> 4; >>> + /* >>> + * Assume happy day, and copy the data. The caller is >>> + * expected to check msg->reply before touching it. >>> + * >>> + * Return payload size. >>> + */ >>> + ret--; >>> + memcpy(msg->buffer, rxbuf + 1, ret); >>> } >>> - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER) >>> - usleep_range(400, 500); >>> - else >>> - return -EIO; >>> + break; >>> + >>> + default: >>> + ret = -EINVAL; >>> + break; >>> } >>> >>> - DRM_ERROR("too many retries, giving up\n"); >>> - return -EIO; >>> + return ret; >>> +} >>> + >>> +static void >>> +intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector) >>> +{ >>> + struct drm_device *dev = intel_dp_to_dev(intel_dp); >>> + >>> + intel_dp->aux.dev = dev->dev; >>> + intel_dp->aux.transfer = intel_dp_aux_transfer; >>> } >>> >>> static int >>> @@ -1472,8 +1452,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) >>> return; >>> >>> if (mode != DRM_MODE_DPMS_ON) { >>> - ret = intel_dp_aux_native_write_1(intel_dp, DP_SET_POWER, >>> - DP_SET_POWER_D3); >>> + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, >>> + DP_SET_POWER_D3); >>> if (ret != 1) >>> DRM_DEBUG_DRIVER("failed to write sink power state\n"); >>> } else { >>> @@ -1482,9 +1462,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) >>> * time to wake up. >>> */ >>> for (i = 0; i < 3; i++) { >>> - ret = intel_dp_aux_native_write_1(intel_dp, >>> - DP_SET_POWER, >>> - DP_SET_POWER_D0); >>> + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, >>> + DP_SET_POWER_D0); >>> if (ret == 1) >>> break; >>> msleep(1); >>> @@ -1708,13 +1687,11 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp) >>> >>> /* Enable PSR in sink */ >>> if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) >>> - intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, >>> - DP_PSR_ENABLE & >>> - ~DP_PSR_MAIN_LINK_ACTIVE); >>> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, >>> + DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE); >>> else >>> - intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, >>> - DP_PSR_ENABLE | >>> - DP_PSR_MAIN_LINK_ACTIVE); >>> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, >>> + DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE); >>> >>> /* Setup AUX registers */ >>> I915_WRITE(EDP_PSR_AUX_DATA1(dev), EDP_PSR_DPCD_COMMAND); >>> @@ -2026,26 +2003,25 @@ static void vlv_dp_pre_pll_enable(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 bool >>> -intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address, >>> - uint8_t *recv, int recv_bytes) >>> +static ssize_t >>> +intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, >>> + void *buffer, size_t size) >>> { >>> - int ret, i; >>> + ssize_t ret; >>> + int i; >>> >>> - /* >>> - * Sinks are *supposed* to come up within 1ms from an off state, >>> - * but we're also supposed to retry 3 times per the spec. >>> - */ >>> for (i = 0; i < 3; i++) { >>> - ret = intel_dp_aux_native_read(intel_dp, address, recv, >>> - recv_bytes); >>> - if (ret == recv_bytes) >>> - return true; >>> + ret = drm_dp_dpcd_read(aux, offset, buffer, size); >>> + if (ret == size) >>> + return ret; >>> msleep(1); >>> } >>> >>> - return false; >>> + return ret; >>> } >>> >>> /* >>> @@ -2055,10 +2031,10 @@ intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address, >>> static bool >>> intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) >>> { >>> - return intel_dp_aux_native_read_retry(intel_dp, >>> - DP_LANE0_1_STATUS, >>> - 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; >>> } >>> >>> /* >>> @@ -2572,8 +2548,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, >>> len = intel_dp->lane_count + 1; >>> } >>> >>> - ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_PATTERN_SET, >>> - buf, len); >>> + ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET, >>> + buf, len); >>> >>> return ret == len; >>> } >>> @@ -2602,9 +2578,8 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP, >>> I915_WRITE(intel_dp->output_reg, *DP); >>> POSTING_READ(intel_dp->output_reg); >>> >>> - ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_LANE0_SET, >>> - intel_dp->train_set, >>> - intel_dp->lane_count); >>> + ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET, >>> + intel_dp->train_set, intel_dp->lane_count); >>> >>> return ret == intel_dp->lane_count; >>> } >>> @@ -2660,11 +2635,11 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) >>> link_config[1] = intel_dp->lane_count; >>> if (drm_dp_enhanced_frame_cap(intel_dp->dpcd)) >>> link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN; >>> - intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, link_config, 2); >>> + drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2); >>> >>> link_config[0] = 0; >>> link_config[1] = DP_SET_ANSI_8B10B; >>> - intel_dp_aux_native_write(intel_dp, DP_DOWNSPREAD_CTRL, link_config, 2); >>> + drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2); >>> >>> DP |= DP_PORT_EN; >>> >>> @@ -2907,8 +2882,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) >>> >>> char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3]; >>> >>> - if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd, >>> - sizeof(intel_dp->dpcd)) == 0) >>> + if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, >>> + sizeof(intel_dp->dpcd)) < 0) >>> return false; /* aux transfer failed */ >>> >>> hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd), >>> @@ -2921,9 +2896,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_dpcd_read_wake(&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"); >>> @@ -2945,9 +2920,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_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0, >>> - intel_dp->downstream_ports, >>> - DP_MAX_DOWNSTREAM_PORTS) == 0) >>> + if (intel_dp_dpcd_read_wake(&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; >>> @@ -2963,11 +2938,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_dpcd_read_wake(&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_aux_native_read_retry(intel_dp, DP_BRANCH_OUI, buf, 3)) >>> + if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) >>> DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", >>> buf[0], buf[1], buf[2]); >>> >>> @@ -2982,46 +2957,40 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) >>> to_intel_crtc(intel_dig_port->base.base.crtc); >>> u8 buf[1]; >>> >>> - if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1)) >>> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0); >>> return -EAGAIN; >>> >>> if (!(buf[0] & DP_TEST_CRC_SUPPORTED)) >>> return -ENOTTY; >>> >>> - if (!intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, >>> - DP_TEST_SINK_START)) >>> + if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, >>> + DP_TEST_SINK_START) < 0) >>> return -EAGAIN; >>> >>> /* Wait 2 vblanks to be sure we will have the correct CRC value */ >>> intel_wait_for_vblank(dev, intel_crtc->pipe); >>> intel_wait_for_vblank(dev, intel_crtc->pipe); >>> >>> - if (!intel_dp_aux_native_read(intel_dp, DP_TEST_CRC_R_CR, crc, 6)) >>> + if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) >>> return -EAGAIN; >>> >>> - intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, 0); >>> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0); >>> return 0; >>> } >>> >>> static 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_dpcd_read_wake(&intel_dp->aux, >>> + DP_DEVICE_SERVICE_IRQ_VECTOR, >>> + sink_irq_vector, 1) == 1; >>> } >>> >>> static void >>> intel_dp_handle_test_request(struct intel_dp *intel_dp) >>> { >>> /* NAK by default */ >>> - intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, DP_TEST_NAK); >>> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_RESPONSE, DP_TEST_NAK); >>> } >>> >>> /* >>> @@ -3060,9 +3029,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) >>> if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && >>> intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) { >>> /* Clear interrupt source */ >>> - intel_dp_aux_native_write_1(intel_dp, >>> - DP_DEVICE_SERVICE_IRQ_VECTOR, >>> - sink_irq_vector); >>> + drm_dp_dpcd_writeb(&intel_dp->aux, >>> + DP_DEVICE_SERVICE_IRQ_VECTOR, >>> + sink_irq_vector); >>> >>> if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST) >>> intel_dp_handle_test_request(intel_dp); >>> @@ -3097,9 +3066,11 @@ 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_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, >>> + ®, 1) < 0) >>> return connector_status_unknown; >>> + >>> return DP_GET_SINK_COUNT(reg) ? connector_status_connected >>> : connector_status_disconnected; >>> } >>> @@ -3925,6 +3896,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, >>> intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq); >>> } >>> >>> + intel_dp_aux_init(intel_dp, intel_connector); >>> + >>> error = intel_dp_i2c_init(intel_dp, intel_connector, name); >>> WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n", >>> error, port_name(port)); >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >>> index 2546cae0b4f0..561af4b013b1 100644 >>> --- a/drivers/gpu/drm/i915/intel_drv.h >>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>> @@ -491,6 +491,7 @@ struct intel_dp { >>> uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; >>> struct i2c_adapter adapter; >>> struct i2c_algo_dp_aux_data algo; >>> + struct drm_dp_aux aux; >>> uint8_t train_set[4]; >>> int panel_power_up_delay; >>> int panel_power_down_delay; >>> -- >>> 1.7.9.5 >>> >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> >> >> -- >> Rodrigo Vivi >> Blog: http://blog.vivi.eng.br > > -- > Jani Nikula, Intel Open Source Technology Center
On Tue, Mar 18, 2014 at 3:02 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote: > Thanks for explanations Jani and Daniel. > > Feel free to use Reviewe-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> Thanks to patches & review, all pulled into a topic branch, will probably merge into dinq in a few days. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 22134edc03dd..24cbf4bd36c4 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -575,97 +575,77 @@ out: return ret; } -/* Write data to the aux channel in native mode */ -static int -intel_dp_aux_native_write(struct intel_dp *intel_dp, - uint16_t address, uint8_t *send, int send_bytes) +#define HEADER_SIZE 4 +static ssize_t +intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) { + struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux); + uint8_t txbuf[20], rxbuf[20]; + size_t txsize, rxsize; int ret; - uint8_t msg[20]; - int msg_bytes; - uint8_t ack; - int retry; - if (WARN_ON(send_bytes > 16)) - return -E2BIG; + txbuf[0] = msg->request << 4; + txbuf[1] = msg->address >> 8; + txbuf[2] = msg->address & 0xff; + txbuf[3] = msg->size - 1; - intel_dp_check_edp(intel_dp); - msg[0] = DP_AUX_NATIVE_WRITE << 4; - msg[1] = address >> 8; - msg[2] = address & 0xff; - msg[3] = send_bytes - 1; - memcpy(&msg[4], send, send_bytes); - msg_bytes = send_bytes + 4; - for (retry = 0; retry < 7; retry++) { - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1); - if (ret < 0) - return ret; - ack >>= 4; - if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) - return send_bytes; - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER) - usleep_range(400, 500); - else - return -EIO; - } + switch (msg->request & ~DP_AUX_I2C_MOT) { + case DP_AUX_NATIVE_WRITE: + case DP_AUX_I2C_WRITE: + txsize = HEADER_SIZE + msg->size; + rxsize = 1; - DRM_ERROR("too many retries, giving up\n"); - return -EIO; -} + if (WARN_ON(txsize > 20)) + return -E2BIG; -/* Write a single byte to the aux channel in native mode */ -static int -intel_dp_aux_native_write_1(struct intel_dp *intel_dp, - uint16_t address, uint8_t byte) -{ - return intel_dp_aux_native_write(intel_dp, address, &byte, 1); -} + memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size); -/* read bytes from a native aux channel */ -static int -intel_dp_aux_native_read(struct intel_dp *intel_dp, - uint16_t address, uint8_t *recv, int recv_bytes) -{ - uint8_t msg[4]; - int msg_bytes; - uint8_t reply[20]; - int reply_bytes; - uint8_t ack; - int ret; - int retry; + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); + if (ret > 0) { + msg->reply = rxbuf[0] >> 4; - if (WARN_ON(recv_bytes > 19)) - return -E2BIG; + /* Return payload size. */ + ret = msg->size; + } + break; - intel_dp_check_edp(intel_dp); - msg[0] = DP_AUX_NATIVE_READ << 4; - msg[1] = address >> 8; - msg[2] = address & 0xff; - msg[3] = recv_bytes - 1; + case DP_AUX_NATIVE_READ: + case DP_AUX_I2C_READ: + txsize = HEADER_SIZE; + rxsize = msg->size + 1; - msg_bytes = 4; - reply_bytes = recv_bytes + 1; + if (WARN_ON(rxsize > 20)) + return -E2BIG; - for (retry = 0; retry < 7; retry++) { - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, - reply, reply_bytes); - if (ret == 0) - return -EPROTO; - if (ret < 0) - return ret; - ack = reply[0] >> 4; - if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) { - memcpy(recv, reply + 1, ret - 1); - return ret - 1; + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); + if (ret > 0) { + msg->reply = rxbuf[0] >> 4; + /* + * Assume happy day, and copy the data. The caller is + * expected to check msg->reply before touching it. + * + * Return payload size. + */ + ret--; + memcpy(msg->buffer, rxbuf + 1, ret); } - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER) - usleep_range(400, 500); - else - return -EIO; + break; + + default: + ret = -EINVAL; + break; } - DRM_ERROR("too many retries, giving up\n"); - return -EIO; + return ret; +} + +static void +intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector) +{ + struct drm_device *dev = intel_dp_to_dev(intel_dp); + + intel_dp->aux.dev = dev->dev; + intel_dp->aux.transfer = intel_dp_aux_transfer; } static int @@ -1472,8 +1452,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) return; if (mode != DRM_MODE_DPMS_ON) { - ret = intel_dp_aux_native_write_1(intel_dp, DP_SET_POWER, - DP_SET_POWER_D3); + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, + DP_SET_POWER_D3); if (ret != 1) DRM_DEBUG_DRIVER("failed to write sink power state\n"); } else { @@ -1482,9 +1462,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) * time to wake up. */ for (i = 0; i < 3; i++) { - ret = intel_dp_aux_native_write_1(intel_dp, - DP_SET_POWER, - DP_SET_POWER_D0); + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, + DP_SET_POWER_D0); if (ret == 1) break; msleep(1); @@ -1708,13 +1687,11 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp) /* Enable PSR in sink */ if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) - intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, - DP_PSR_ENABLE & - ~DP_PSR_MAIN_LINK_ACTIVE); + drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, + DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE); else - intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, - DP_PSR_ENABLE | - DP_PSR_MAIN_LINK_ACTIVE); + drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, + DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE); /* Setup AUX registers */ I915_WRITE(EDP_PSR_AUX_DATA1(dev), EDP_PSR_DPCD_COMMAND); @@ -2026,26 +2003,25 @@ static void vlv_dp_pre_pll_enable(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 bool -intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address, - uint8_t *recv, int recv_bytes) +static ssize_t +intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, + void *buffer, size_t size) { - int ret, i; + ssize_t ret; + int i; - /* - * Sinks are *supposed* to come up within 1ms from an off state, - * but we're also supposed to retry 3 times per the spec. - */ for (i = 0; i < 3; i++) { - ret = intel_dp_aux_native_read(intel_dp, address, recv, - recv_bytes); - if (ret == recv_bytes) - return true; + ret = drm_dp_dpcd_read(aux, offset, buffer, size); + if (ret == size) + return ret; msleep(1); } - return false; + return ret; } /* @@ -2055,10 +2031,10 @@ intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address, static bool intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) { - return intel_dp_aux_native_read_retry(intel_dp, - DP_LANE0_1_STATUS, - 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; } /* @@ -2572,8 +2548,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, len = intel_dp->lane_count + 1; } - ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_PATTERN_SET, - buf, len); + ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET, + buf, len); return ret == len; } @@ -2602,9 +2578,8 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP, I915_WRITE(intel_dp->output_reg, *DP); POSTING_READ(intel_dp->output_reg); - ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_LANE0_SET, - intel_dp->train_set, - intel_dp->lane_count); + ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET, + intel_dp->train_set, intel_dp->lane_count); return ret == intel_dp->lane_count; } @@ -2660,11 +2635,11 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) link_config[1] = intel_dp->lane_count; if (drm_dp_enhanced_frame_cap(intel_dp->dpcd)) link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN; - intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, link_config, 2); + drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2); link_config[0] = 0; link_config[1] = DP_SET_ANSI_8B10B; - intel_dp_aux_native_write(intel_dp, DP_DOWNSPREAD_CTRL, link_config, 2); + drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2); DP |= DP_PORT_EN; @@ -2907,8 +2882,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3]; - if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd, - sizeof(intel_dp->dpcd)) == 0) + if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, + sizeof(intel_dp->dpcd)) < 0) return false; /* aux transfer failed */ hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd), @@ -2921,9 +2896,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_dpcd_read_wake(&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"); @@ -2945,9 +2920,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_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0, - intel_dp->downstream_ports, - DP_MAX_DOWNSTREAM_PORTS) == 0) + if (intel_dp_dpcd_read_wake(&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; @@ -2963,11 +2938,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_dpcd_read_wake(&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_aux_native_read_retry(intel_dp, DP_BRANCH_OUI, buf, 3)) + if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", buf[0], buf[1], buf[2]); @@ -2982,46 +2957,40 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) to_intel_crtc(intel_dig_port->base.base.crtc); u8 buf[1]; - if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1)) + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0); return -EAGAIN; if (!(buf[0] & DP_TEST_CRC_SUPPORTED)) return -ENOTTY; - if (!intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, - DP_TEST_SINK_START)) + if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, + DP_TEST_SINK_START) < 0) return -EAGAIN; /* Wait 2 vblanks to be sure we will have the correct CRC value */ intel_wait_for_vblank(dev, intel_crtc->pipe); intel_wait_for_vblank(dev, intel_crtc->pipe); - if (!intel_dp_aux_native_read(intel_dp, DP_TEST_CRC_R_CR, crc, 6)) + if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) return -EAGAIN; - intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, 0); + drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0); return 0; } static 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_dpcd_read_wake(&intel_dp->aux, + DP_DEVICE_SERVICE_IRQ_VECTOR, + sink_irq_vector, 1) == 1; } static void intel_dp_handle_test_request(struct intel_dp *intel_dp) { /* NAK by default */ - intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, DP_TEST_NAK); + drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_RESPONSE, DP_TEST_NAK); } /* @@ -3060,9 +3029,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) { /* Clear interrupt source */ - intel_dp_aux_native_write_1(intel_dp, - DP_DEVICE_SERVICE_IRQ_VECTOR, - sink_irq_vector); + drm_dp_dpcd_writeb(&intel_dp->aux, + DP_DEVICE_SERVICE_IRQ_VECTOR, + sink_irq_vector); if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST) intel_dp_handle_test_request(intel_dp); @@ -3097,9 +3066,11 @@ 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_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, + ®, 1) < 0) return connector_status_unknown; + return DP_GET_SINK_COUNT(reg) ? connector_status_connected : connector_status_disconnected; } @@ -3925,6 +3896,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq); } + intel_dp_aux_init(intel_dp, intel_connector); + error = intel_dp_i2c_init(intel_dp, intel_connector, name); WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n", error, port_name(port)); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 2546cae0b4f0..561af4b013b1 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -491,6 +491,7 @@ struct intel_dp { uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; struct i2c_adapter adapter; struct i2c_algo_dp_aux_data algo; + struct drm_dp_aux aux; uint8_t train_set[4]; int panel_power_up_delay; int panel_power_down_delay;
Functionality remains largely the same as before. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 257 +++++++++++++++++--------------------- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 116 insertions(+), 142 deletions(-)