[RFC PATCH 5/7] drm/i915/dp: use the new drm helpers for dp aux

The main differences are:

* Native aux has retry limit of 7 instead of infinite retry.

* Sleep in native reply defer increases from udelay(100) to usleep_range(400, 500).

* Unknown native reply results in retry instead of -EIO.

* Lower level -EBUSY results in retry instead of fail.

* Write return of 0 results in -EPROTO instead of (questionable) success.

Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 249 ++++++++++++++++++-------------------- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 120 insertions(+), 130 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 2eb6715f78df..835f16da80af 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -575,90 +575,68 @@ 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;

- 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 (;;) { - 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) - break; - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER) - udelay(100); - else - return -EIO; - } - return send_bytes; -} + switch (msg->request & ~DP_AUX_I2C_MOT) { + case DP_AUX_NATIVE_WRITE: + case DP_AUX_I2C_WRITE: + txsize = HEADER_SIZE + msg->size; + rxsize = 1;

-/* 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); -} + if (WARN_ON(txsize > 20)) + return -E2BIG;

-/* 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; + memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);

- if (WARN_ON(recv_bytes > 19)) - return -E2BIG; + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); + if (ret > 0) { + msg->reply = rxbuf[0] >> 4;

- 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; + /* Return payload size. */ + ret = msg->size; + } + break;

- msg_bytes = 4; - reply_bytes = recv_bytes + 1; + case DP_AUX_NATIVE_READ: + case DP_AUX_I2C_READ: + txsize = HEADER_SIZE; + rxsize = msg->size + 1;

- for (;;) { - 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; + if (WARN_ON(rxsize > 20)) + return -E2BIG; + + 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) - udelay(100); - else - return -EIO; + break; + + default: + ret = -EINVAL; + break; } + + return ret; }

static int @@ -805,6 +783,13 @@ intel_dp_i2c_init(struct intel_dp *intel_dp, }

static void +intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector) +{ + intel_dp->aux.dev = connector->base.kdev; + intel_dp->aux.transfer = intel_dp_aux_transfer; +} + +static void intel_dp_set_clock(struct intel_encoder *encoder, struct intel_crtc_config *pipe_config, int link_bw) { @@ -1444,8 +1429,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 { @@ -1454,9 +1439,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); @@ -1673,13 +1657,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); @@ -1989,26 +1971,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; }

/* @@ -2018,10 +1999,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; }

/* @@ -2535,8 +2516,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; } @@ -2565,9 +2546,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; } @@ -2623,11 +2603,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;

@@ -2867,11 +2847,12 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); struct drm_device *dev = dig_port->base.base.dev; struct drm_i915_private *dev_priv = dev->dev_private; + int ret;

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))) + 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), @@ -2884,9 +2865,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(intel_dp, 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"); @@ -2908,10 +2889,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(intel_dp, DP_DOWNSTREAM_PORT_0, - intel_dp->downstream_ports, - DP_MAX_DOWNSTREAM_PORTS) != - DP_MAX_DOWNSTREAM_PORTS) + ret = drm_dp_dpcd_read(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, + intel_dp->downstream_ports, DP_MAX_DOWNSTREAM_PORTS); + if (ret != DP_MAX_DOWNSTREAM_PORTS) return false; /* downstream port status fetch failed */

return true; @@ -2927,11 +2907,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)

edp_panel_vdd_on(intel_dp);

- if (intel_dp_aux_native_read(intel_dp, 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_aux_native_read(intel_dp, 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]);

@@ -2945,40 +2925,43 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) struct intel_crtc *intel_crtc = to_intel_crtc(intel_dig_port->base.base.crtc); u8 buf[1]; + int ret;

- if (intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1) != 1) + ret = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf); + if (ret != 1) 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) != 1) + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, DP_TEST_SINK_START); + if (ret != 1) 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) != 6) + ret = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6); + if (ret != 6) 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 inline bool intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector) { - return intel_dp_aux_native_read(intel_dp, DP_DEVICE_SERVICE_IRQ_VECTOR, - sink_irq_vector, 1) == 1; + return drm_dp_dpcd_readb(&intel_dp->aux, DP_DEVICE_SERVICE_IRQ_VECTOR, + sink_irq_vector) == 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); }

/* @@ -3017,9 +3000,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); @@ -3042,6 +3025,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) { uint8_t *dpcd = intel_dp->dpcd; uint8_t type; + int ret;

if (!intel_dp_get_dpcd(intel_dp)) return connector_status_disconnected; @@ -3054,8 +3038,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(intel_dp, DP_SINK_COUNT, &reg, 1) != 1) + + ret = drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_COUNT, &reg); + if (ret != 1) return connector_status_unknown; + return DP_GET_SINK_COUNT(reg) ? connector_status_connected : connector_status_disconnected; } @@ -3855,6 +3842,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 44067bce5e04..75a1329297d9 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -479,6 +479,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;