dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/3] drm/radeon: use the new drm helpers for dp aux
  2014-03-18  3:51 Alex Deucher
@ 2014-03-18  3:51 ` Alex Deucher
  2014-03-18  7:51   ` Jani Nikula
  0 siblings, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2014-03-18  3:51 UTC (permalink / raw)
  To: dri-devel; +Cc: Alex Deucher

Switch to the new dp helpers.  The main difference is
that the DP helpers don't allow an adjustable delay in
the aux transaction, but I don't know that this is
necessary.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/atombios_dp.c       | 192 +++++++++++++----------------
 drivers/gpu/drm/radeon/radeon_connectors.c |  17 ++-
 drivers/gpu/drm/radeon/radeon_mode.h       |   2 +
 3 files changed, 104 insertions(+), 107 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
index 23189b7..8d8f846 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -142,94 +142,62 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan,
 	return recv_bytes;
 }
 
-static int radeon_dp_aux_native_write(struct radeon_connector *radeon_connector,
-				      u16 address, u8 *send, u8 send_bytes, u8 delay)
-{
-	struct radeon_connector_atom_dig *dig_connector = radeon_connector->con_priv;
-	int ret;
-	u8 msg[20];
-	int msg_bytes = send_bytes + 4;
-	u8 ack;
-	unsigned retry;
-
-	if (send_bytes > 16)
-		return -1;
+#define HEADER_SIZE 4
 
-	msg[0] = address;
-	msg[1] = address >> 8;
-	msg[2] = DP_AUX_NATIVE_WRITE << 4;
-	msg[3] = (msg_bytes << 4) | (send_bytes - 1);
-	memcpy(&msg[4], send, send_bytes);
-
-	for (retry = 0; retry < 7; retry++) {
-		ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
-					    msg, msg_bytes, NULL, 0, delay, &ack);
-		if (ret == -EBUSY)
-			continue;
-		else 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;
-	}
-
-	return -EIO;
-}
-
-static int radeon_dp_aux_native_read(struct radeon_connector *radeon_connector,
-				     u16 address, u8 *recv, int recv_bytes, u8 delay)
+static ssize_t
+radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
-	struct radeon_connector_atom_dig *dig_connector = radeon_connector->con_priv;
-	u8 msg[4];
-	int msg_bytes = 4;
-	u8 ack;
+	struct radeon_i2c_chan *chan =
+		container_of(aux, struct radeon_i2c_chan, aux);
 	int ret;
-	unsigned retry;
-
-	msg[0] = address;
-	msg[1] = address >> 8;
-	msg[2] = DP_AUX_NATIVE_READ << 4;
-	msg[3] = (msg_bytes << 4) | (recv_bytes - 1);
-
-	for (retry = 0; retry < 7; retry++) {
-		ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
-					    msg, msg_bytes, recv, recv_bytes, delay, &ack);
-		if (ret == -EBUSY)
-			continue;
-		else if (ret < 0)
-			return ret;
-		ack >>= 4;
-		if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK)
-			return ret;
-		else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
-			usleep_range(400, 500);
-		else if (ret == 0)
-			return -EPROTO;
-		else
-			return -EIO;
+	u8 tx_buf[20];
+	size_t tx_size;
+	u8 ack, delay = 0;
+
+	if (WARN_ON(msg->size > 16))
+		return -E2BIG;
+
+	tx_buf[0] = msg->address & 0xff;
+	tx_buf[1] = msg->address >> 8;
+	tx_buf[2] = msg->request << 4;
+	tx_buf[3] = msg->size - 1;
+
+	switch (msg->request & ~DP_AUX_I2C_MOT) {
+	case DP_AUX_NATIVE_WRITE:
+	case DP_AUX_I2C_WRITE:
+		tx_size = HEADER_SIZE + msg->size;
+		tx_buf[3] |= tx_size << 4;
+		memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size);
+		ret = radeon_process_aux_ch(chan,
+					    tx_buf, tx_size, NULL, 0, delay, &ack);
+		if (ret >= 0)
+			/* Return payload size. */
+			ret = msg->size;
+		break;
+	case DP_AUX_NATIVE_READ:
+	case DP_AUX_I2C_READ:
+		tx_size = HEADER_SIZE;
+		tx_buf[3] |= tx_size << 4;
+		ret = radeon_process_aux_ch(chan,
+					    tx_buf, tx_size, msg->buffer, msg->size, delay, &ack);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
 	}
 
-	return -EIO;
-}
+	if (ret > 0)
+		msg->reply = ack >> 4;
 
-static void radeon_write_dpcd_reg(struct radeon_connector *radeon_connector,
-				 u16 reg, u8 val)
-{
-	radeon_dp_aux_native_write(radeon_connector, reg, &val, 1, 0);
+	return ret;
 }
 
-static u8 radeon_read_dpcd_reg(struct radeon_connector *radeon_connector,
-			       u16 reg)
+void radeon_dp_aux_init(struct radeon_connector *radeon_connector)
 {
-	u8 val = 0;
-
-	radeon_dp_aux_native_read(radeon_connector, reg, &val, 1, 0);
+	struct radeon_connector_atom_dig *dig_connector = radeon_connector->con_priv;
 
-	return val;
+	dig_connector->dp_i2c_bus->aux.dev = radeon_connector->base.kdev;
+	dig_connector->dp_i2c_bus->aux.transfer = radeon_dp_aux_transfer;
 }
 
 int radeon_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
@@ -468,11 +436,11 @@ static void radeon_dp_probe_oui(struct radeon_connector *radeon_connector)
 	if (!(dig_connector->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT))
 		return;
 
-	if (radeon_dp_aux_native_read(radeon_connector, DP_SINK_OUI, buf, 3, 0))
+	if (drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_SINK_OUI, buf, 3))
 		DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n",
 			      buf[0], buf[1], buf[2]);
 
-	if (radeon_dp_aux_native_read(radeon_connector, DP_BRANCH_OUI, buf, 3, 0))
+	if (drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_BRANCH_OUI, buf, 3))
 		DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n",
 			      buf[0], buf[1], buf[2]);
 }
@@ -483,8 +451,8 @@ bool radeon_dp_getdpcd(struct radeon_connector *radeon_connector)
 	u8 msg[DP_DPCD_SIZE];
 	int ret, i;
 
-	ret = radeon_dp_aux_native_read(radeon_connector, DP_DPCD_REV, msg,
-					DP_DPCD_SIZE, 0);
+	ret = drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_DPCD_REV, msg,
+			       DP_DPCD_SIZE);
 	if (ret > 0) {
 		memcpy(dig_connector->dpcd, msg, DP_DPCD_SIZE);
 		DRM_DEBUG_KMS("DPCD: ");
@@ -506,6 +474,7 @@ int radeon_dp_get_panel_mode(struct drm_encoder *encoder,
 	struct drm_device *dev = encoder->dev;
 	struct radeon_device *rdev = dev->dev_private;
 	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
+	struct radeon_connector_atom_dig *dig_connector;
 	int panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE;
 	u16 dp_bridge = radeon_connector_encoder_get_dp_bridge_encoder_id(connector);
 	u8 tmp;
@@ -513,9 +482,15 @@ int radeon_dp_get_panel_mode(struct drm_encoder *encoder,
 	if (!ASIC_IS_DCE4(rdev))
 		return panel_mode;
 
+	if (!radeon_connector->con_priv)
+		return panel_mode;
+
+	dig_connector = radeon_connector->con_priv;
+
 	if (dp_bridge != ENCODER_OBJECT_ID_NONE) {
 		/* DP bridge chips */
-		tmp = radeon_read_dpcd_reg(radeon_connector, DP_EDP_CONFIGURATION_CAP);
+		drm_dp_dpcd_readb(&dig_connector->dp_i2c_bus->aux,
+				  DP_EDP_CONFIGURATION_CAP, &tmp);
 		if (tmp & 1)
 			panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE;
 		else if ((dp_bridge == ENCODER_OBJECT_ID_NUTMEG) ||
@@ -525,7 +500,8 @@ int radeon_dp_get_panel_mode(struct drm_encoder *encoder,
 			panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE;
 	} else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
 		/* eDP */
-		tmp = radeon_read_dpcd_reg(radeon_connector, DP_EDP_CONFIGURATION_CAP);
+		drm_dp_dpcd_readb(&dig_connector->dp_i2c_bus->aux,
+				  DP_EDP_CONFIGURATION_CAP, &tmp);
 		if (tmp & 1)
 			panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE;
 	}
@@ -576,9 +552,15 @@ int radeon_dp_mode_valid_helper(struct drm_connector *connector,
 static bool radeon_dp_get_link_status(struct radeon_connector *radeon_connector,
 				      u8 link_status[DP_LINK_STATUS_SIZE])
 {
+	struct radeon_connector_atom_dig *dig_connector;
 	int ret;
-	ret = radeon_dp_aux_native_read(radeon_connector, DP_LANE0_1_STATUS,
-					link_status, DP_LINK_STATUS_SIZE, 100);
+
+	if (!radeon_connector->con_priv)
+		return false;
+	dig_connector = radeon_connector->con_priv;
+
+	ret = drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_LANE0_1_STATUS,
+			       link_status, DP_LINK_STATUS_SIZE);
 	if (ret <= 0) {
 		return false;
 	}
@@ -612,7 +594,7 @@ void radeon_dp_set_rx_power_state(struct drm_connector *connector,
 
 	/* power up/down the sink */
 	if (dig_connector->dpcd[0] >= 0x11) {
-		radeon_write_dpcd_reg(radeon_connector,
+		drm_dp_dpcd_writeb(&dig_connector->dp_i2c_bus->aux,
 				   DP_SET_POWER, power_state);
 		usleep_range(1000, 2000);
 	}
@@ -633,6 +615,7 @@ struct radeon_dp_link_train_info {
 	u8 link_status[DP_LINK_STATUS_SIZE];
 	u8 tries;
 	bool use_dpencoder;
+	struct drm_dp_aux *aux;
 };
 
 static void radeon_dp_update_vs_emph(struct radeon_dp_link_train_info *dp_info)
@@ -643,8 +626,8 @@ static void radeon_dp_update_vs_emph(struct radeon_dp_link_train_info *dp_info)
 				       0, dp_info->train_set[0]); /* sets all lanes at once */
 
 	/* set the vs/emph on the sink */
-	radeon_dp_aux_native_write(dp_info->radeon_connector, DP_TRAINING_LANE0_SET,
-				   dp_info->train_set, dp_info->dp_lane_count, 0);
+	drm_dp_dpcd_write(dp_info->aux, DP_TRAINING_LANE0_SET,
+			  dp_info->train_set, dp_info->dp_lane_count);
 }
 
 static void radeon_dp_set_tp(struct radeon_dp_link_train_info *dp_info, int tp)
@@ -679,7 +662,7 @@ static void radeon_dp_set_tp(struct radeon_dp_link_train_info *dp_info, int tp)
 	}
 
 	/* enable training pattern on the sink */
-	radeon_write_dpcd_reg(dp_info->radeon_connector, DP_TRAINING_PATTERN_SET, tp);
+	drm_dp_dpcd_writeb(dp_info->aux, DP_TRAINING_PATTERN_SET, tp);
 }
 
 static int radeon_dp_link_train_init(struct radeon_dp_link_train_info *dp_info)
@@ -693,26 +676,26 @@ static int radeon_dp_link_train_init(struct radeon_dp_link_train_info *dp_info)
 
 	/* possibly enable downspread on the sink */
 	if (dp_info->dpcd[3] & 0x1)
-		radeon_write_dpcd_reg(dp_info->radeon_connector,
-				      DP_DOWNSPREAD_CTRL, DP_SPREAD_AMP_0_5);
+		drm_dp_dpcd_writeb(dp_info->aux,
+				   DP_DOWNSPREAD_CTRL, DP_SPREAD_AMP_0_5);
 	else
-		radeon_write_dpcd_reg(dp_info->radeon_connector,
-				      DP_DOWNSPREAD_CTRL, 0);
+		drm_dp_dpcd_writeb(dp_info->aux,
+				   DP_DOWNSPREAD_CTRL, 0);
 
 	if ((dp_info->connector->connector_type == DRM_MODE_CONNECTOR_eDP) &&
 	    (dig->panel_mode == DP_PANEL_MODE_INTERNAL_DP2_MODE)) {
-		radeon_write_dpcd_reg(dp_info->radeon_connector, DP_EDP_CONFIGURATION_SET, 1);
+		drm_dp_dpcd_writeb(dp_info->aux, DP_EDP_CONFIGURATION_SET, 1);
 	}
 
 	/* set the lane count on the sink */
 	tmp = dp_info->dp_lane_count;
 	if (drm_dp_enhanced_frame_cap(dp_info->dpcd))
 		tmp |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
-	radeon_write_dpcd_reg(dp_info->radeon_connector, DP_LANE_COUNT_SET, tmp);
+	drm_dp_dpcd_writeb(dp_info->aux, DP_LANE_COUNT_SET, tmp);
 
 	/* set the link rate on the sink */
 	tmp = drm_dp_link_rate_to_bw_code(dp_info->dp_clock);
-	radeon_write_dpcd_reg(dp_info->radeon_connector, DP_LINK_BW_SET, tmp);
+	drm_dp_dpcd_writeb(dp_info->aux, DP_LINK_BW_SET, tmp);
 
 	/* start training on the source */
 	if (ASIC_IS_DCE4(dp_info->rdev) || !dp_info->use_dpencoder)
@@ -723,9 +706,9 @@ static int radeon_dp_link_train_init(struct radeon_dp_link_train_info *dp_info)
 					  dp_info->dp_clock, dp_info->enc_id, 0);
 
 	/* disable the training pattern on the sink */
-	radeon_write_dpcd_reg(dp_info->radeon_connector,
-			      DP_TRAINING_PATTERN_SET,
-			      DP_TRAINING_PATTERN_DISABLE);
+	drm_dp_dpcd_writeb(dp_info->aux,
+			   DP_TRAINING_PATTERN_SET,
+			   DP_TRAINING_PATTERN_DISABLE);
 
 	return 0;
 }
@@ -735,9 +718,9 @@ static int radeon_dp_link_train_finish(struct radeon_dp_link_train_info *dp_info
 	udelay(400);
 
 	/* disable the training pattern on the sink */
-	radeon_write_dpcd_reg(dp_info->radeon_connector,
-			      DP_TRAINING_PATTERN_SET,
-			      DP_TRAINING_PATTERN_DISABLE);
+	drm_dp_dpcd_writeb(dp_info->aux,
+			   DP_TRAINING_PATTERN_SET,
+			   DP_TRAINING_PATTERN_DISABLE);
 
 	/* disable the training pattern on the source */
 	if (ASIC_IS_DCE4(dp_info->rdev) || !dp_info->use_dpencoder)
@@ -914,7 +897,7 @@ void radeon_dp_link_train(struct drm_encoder *encoder,
 	else
 		dp_info.enc_id |= ATOM_DP_CONFIG_LINK_A;
 
-	tmp = radeon_read_dpcd_reg(radeon_connector, DP_MAX_LANE_COUNT);
+	drm_dp_dpcd_readb(&dig_connector->dp_i2c_bus->aux, DP_MAX_LANE_COUNT, &tmp);
 	if (ASIC_IS_DCE5(rdev) && (tmp & DP_TPS3_SUPPORTED))
 		dp_info.tp3_supported = true;
 	else
@@ -927,6 +910,7 @@ void radeon_dp_link_train(struct drm_encoder *encoder,
 	dp_info.radeon_connector = radeon_connector;
 	dp_info.dp_lane_count = dig_connector->dp_lane_count;
 	dp_info.dp_clock = dig_connector->dp_clock;
+	dp_info.aux = &dig_connector->dp_i2c_bus->aux;
 
 	if (radeon_dp_link_train_init(&dp_info))
 		goto done;
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 82d4f86..ec958e86 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1595,6 +1595,7 @@ radeon_add_atom_connector(struct drm_device *dev,
 	uint32_t subpixel_order = SubPixelNone;
 	bool shared_ddc = false;
 	bool is_dp_bridge = false;
+	bool has_aux = false;
 
 	if (connector_type == DRM_MODE_CONNECTOR_Unknown)
 		return;
@@ -1672,7 +1673,9 @@ radeon_add_atom_connector(struct drm_device *dev,
 				radeon_dig_connector->dp_i2c_bus = radeon_i2c_create_dp(dev, i2c_bus, "eDP-auxch");
 			else
 				radeon_dig_connector->dp_i2c_bus = radeon_i2c_create_dp(dev, i2c_bus, "DP-auxch");
-			if (!radeon_dig_connector->dp_i2c_bus)
+			if (radeon_dig_connector->dp_i2c_bus)
+				has_aux = true;
+			else
 				DRM_ERROR("DP: Failed to assign dp ddc bus! Check dmesg for i2c errors.\n");
 			radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
 			if (!radeon_connector->ddc_bus)
@@ -1895,7 +1898,9 @@ radeon_add_atom_connector(struct drm_device *dev,
 				if (!radeon_dig_connector->dp_i2c_bus)
 					DRM_ERROR("DP: Failed to assign dp ddc bus! Check dmesg for i2c errors.\n");
 				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
-				if (!radeon_connector->ddc_bus)
+				if (radeon_connector->ddc_bus)
+					has_aux = true;
+				else
 					DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
 			}
 			subpixel_order = SubPixelHorizontalRGB;
@@ -1939,7 +1944,9 @@ radeon_add_atom_connector(struct drm_device *dev,
 			if (i2c_bus->valid) {
 				/* add DP i2c bus */
 				radeon_dig_connector->dp_i2c_bus = radeon_i2c_create_dp(dev, i2c_bus, "eDP-auxch");
-				if (!radeon_dig_connector->dp_i2c_bus)
+				if (radeon_dig_connector->dp_i2c_bus)
+					has_aux = true;
+				else
 					DRM_ERROR("DP: Failed to assign dp ddc bus! Check dmesg for i2c errors.\n");
 				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
 				if (!radeon_connector->ddc_bus)
@@ -2000,6 +2007,10 @@ radeon_add_atom_connector(struct drm_device *dev,
 
 	connector->display_info.subpixel_order = subpixel_order;
 	drm_sysfs_connector_add(connector);
+
+	if (has_aux)
+		radeon_dp_aux_init(radeon_connector);
+
 	return;
 
 failed:
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index e390f55..832d9fa 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -192,6 +192,7 @@ struct radeon_i2c_chan {
 		struct i2c_algo_dp_aux_data dp;
 	} algo;
 	struct radeon_i2c_bus_rec rec;
+	struct drm_dp_aux aux;
 };
 
 /* mostly for macs, but really any system without connector tables */
@@ -692,6 +693,7 @@ extern int radeon_dp_get_panel_mode(struct drm_encoder *encoder,
 				    struct drm_connector *connector);
 extern void radeon_dp_set_rx_power_state(struct drm_connector *connector,
 					 u8 power_state);
+extern void radeon_dp_aux_init(struct radeon_connector *radeon_connector);
 extern void atombios_dig_encoder_setup(struct drm_encoder *encoder, int action, int panel_mode);
 extern void radeon_atom_encoder_init(struct radeon_device *rdev);
 extern void radeon_atom_disp_eng_pll_init(struct radeon_device *rdev);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] drm/radeon: use the new drm helpers for dp aux
  2014-03-18  3:51 ` [PATCH 2/3] drm/radeon: use the new drm helpers for dp aux Alex Deucher
@ 2014-03-18  7:51   ` Jani Nikula
  0 siblings, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2014-03-18  7:51 UTC (permalink / raw)
  To: Alex Deucher, dri-devel; +Cc: Alex Deucher

On Tue, 18 Mar 2014, Alex Deucher <alexdeucher@gmail.com> wrote:
> Switch to the new dp helpers.  The main difference is
> that the DP helpers don't allow an adjustable delay in
> the aux transaction, but I don't know that this is
> necessary.

This is related to my comment on patch 1. We should probably work to
make the delay after native or i2c-over-aux defer smarter and adaptive
to the DP device, at the helper level. E.g. just increasing the delays
is not nice for conforming DP native sinks if the problem is with iffy
DP-to-legacy converters.

BR,
Jani.


>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/radeon/atombios_dp.c       | 192 +++++++++++++----------------
>  drivers/gpu/drm/radeon/radeon_connectors.c |  17 ++-
>  drivers/gpu/drm/radeon/radeon_mode.h       |   2 +
>  3 files changed, 104 insertions(+), 107 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
> index 23189b7..8d8f846 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -142,94 +142,62 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan,
>  	return recv_bytes;
>  }
>  
> -static int radeon_dp_aux_native_write(struct radeon_connector *radeon_connector,
> -				      u16 address, u8 *send, u8 send_bytes, u8 delay)
> -{
> -	struct radeon_connector_atom_dig *dig_connector = radeon_connector->con_priv;
> -	int ret;
> -	u8 msg[20];
> -	int msg_bytes = send_bytes + 4;
> -	u8 ack;
> -	unsigned retry;
> -
> -	if (send_bytes > 16)
> -		return -1;
> +#define HEADER_SIZE 4
>  
> -	msg[0] = address;
> -	msg[1] = address >> 8;
> -	msg[2] = DP_AUX_NATIVE_WRITE << 4;
> -	msg[3] = (msg_bytes << 4) | (send_bytes - 1);
> -	memcpy(&msg[4], send, send_bytes);
> -
> -	for (retry = 0; retry < 7; retry++) {
> -		ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
> -					    msg, msg_bytes, NULL, 0, delay, &ack);
> -		if (ret == -EBUSY)
> -			continue;
> -		else 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;
> -	}
> -
> -	return -EIO;
> -}
> -
> -static int radeon_dp_aux_native_read(struct radeon_connector *radeon_connector,
> -				     u16 address, u8 *recv, int recv_bytes, u8 delay)
> +static ssize_t
> +radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  {
> -	struct radeon_connector_atom_dig *dig_connector = radeon_connector->con_priv;
> -	u8 msg[4];
> -	int msg_bytes = 4;
> -	u8 ack;
> +	struct radeon_i2c_chan *chan =
> +		container_of(aux, struct radeon_i2c_chan, aux);
>  	int ret;
> -	unsigned retry;
> -
> -	msg[0] = address;
> -	msg[1] = address >> 8;
> -	msg[2] = DP_AUX_NATIVE_READ << 4;
> -	msg[3] = (msg_bytes << 4) | (recv_bytes - 1);
> -
> -	for (retry = 0; retry < 7; retry++) {
> -		ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
> -					    msg, msg_bytes, recv, recv_bytes, delay, &ack);
> -		if (ret == -EBUSY)
> -			continue;
> -		else if (ret < 0)
> -			return ret;
> -		ack >>= 4;
> -		if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK)
> -			return ret;
> -		else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
> -			usleep_range(400, 500);
> -		else if (ret == 0)
> -			return -EPROTO;
> -		else
> -			return -EIO;
> +	u8 tx_buf[20];
> +	size_t tx_size;
> +	u8 ack, delay = 0;
> +
> +	if (WARN_ON(msg->size > 16))
> +		return -E2BIG;
> +
> +	tx_buf[0] = msg->address & 0xff;
> +	tx_buf[1] = msg->address >> 8;
> +	tx_buf[2] = msg->request << 4;
> +	tx_buf[3] = msg->size - 1;
> +
> +	switch (msg->request & ~DP_AUX_I2C_MOT) {
> +	case DP_AUX_NATIVE_WRITE:
> +	case DP_AUX_I2C_WRITE:
> +		tx_size = HEADER_SIZE + msg->size;
> +		tx_buf[3] |= tx_size << 4;
> +		memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size);
> +		ret = radeon_process_aux_ch(chan,
> +					    tx_buf, tx_size, NULL, 0, delay, &ack);
> +		if (ret >= 0)
> +			/* Return payload size. */
> +			ret = msg->size;
> +		break;
> +	case DP_AUX_NATIVE_READ:
> +	case DP_AUX_I2C_READ:
> +		tx_size = HEADER_SIZE;
> +		tx_buf[3] |= tx_size << 4;
> +		ret = radeon_process_aux_ch(chan,
> +					    tx_buf, tx_size, msg->buffer, msg->size, delay, &ack);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
>  	}
>  
> -	return -EIO;
> -}
> +	if (ret > 0)
> +		msg->reply = ack >> 4;
>  
> -static void radeon_write_dpcd_reg(struct radeon_connector *radeon_connector,
> -				 u16 reg, u8 val)
> -{
> -	radeon_dp_aux_native_write(radeon_connector, reg, &val, 1, 0);
> +	return ret;
>  }
>  
> -static u8 radeon_read_dpcd_reg(struct radeon_connector *radeon_connector,
> -			       u16 reg)
> +void radeon_dp_aux_init(struct radeon_connector *radeon_connector)
>  {
> -	u8 val = 0;
> -
> -	radeon_dp_aux_native_read(radeon_connector, reg, &val, 1, 0);
> +	struct radeon_connector_atom_dig *dig_connector = radeon_connector->con_priv;
>  
> -	return val;
> +	dig_connector->dp_i2c_bus->aux.dev = radeon_connector->base.kdev;
> +	dig_connector->dp_i2c_bus->aux.transfer = radeon_dp_aux_transfer;
>  }
>  
>  int radeon_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
> @@ -468,11 +436,11 @@ static void radeon_dp_probe_oui(struct radeon_connector *radeon_connector)
>  	if (!(dig_connector->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT))
>  		return;
>  
> -	if (radeon_dp_aux_native_read(radeon_connector, DP_SINK_OUI, buf, 3, 0))
> +	if (drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_SINK_OUI, buf, 3))
>  		DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n",
>  			      buf[0], buf[1], buf[2]);
>  
> -	if (radeon_dp_aux_native_read(radeon_connector, DP_BRANCH_OUI, buf, 3, 0))
> +	if (drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_BRANCH_OUI, buf, 3))
>  		DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n",
>  			      buf[0], buf[1], buf[2]);
>  }
> @@ -483,8 +451,8 @@ bool radeon_dp_getdpcd(struct radeon_connector *radeon_connector)
>  	u8 msg[DP_DPCD_SIZE];
>  	int ret, i;
>  
> -	ret = radeon_dp_aux_native_read(radeon_connector, DP_DPCD_REV, msg,
> -					DP_DPCD_SIZE, 0);
> +	ret = drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_DPCD_REV, msg,
> +			       DP_DPCD_SIZE);
>  	if (ret > 0) {
>  		memcpy(dig_connector->dpcd, msg, DP_DPCD_SIZE);
>  		DRM_DEBUG_KMS("DPCD: ");
> @@ -506,6 +474,7 @@ int radeon_dp_get_panel_mode(struct drm_encoder *encoder,
>  	struct drm_device *dev = encoder->dev;
>  	struct radeon_device *rdev = dev->dev_private;
>  	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> +	struct radeon_connector_atom_dig *dig_connector;
>  	int panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE;
>  	u16 dp_bridge = radeon_connector_encoder_get_dp_bridge_encoder_id(connector);
>  	u8 tmp;
> @@ -513,9 +482,15 @@ int radeon_dp_get_panel_mode(struct drm_encoder *encoder,
>  	if (!ASIC_IS_DCE4(rdev))
>  		return panel_mode;
>  
> +	if (!radeon_connector->con_priv)
> +		return panel_mode;
> +
> +	dig_connector = radeon_connector->con_priv;
> +
>  	if (dp_bridge != ENCODER_OBJECT_ID_NONE) {
>  		/* DP bridge chips */
> -		tmp = radeon_read_dpcd_reg(radeon_connector, DP_EDP_CONFIGURATION_CAP);
> +		drm_dp_dpcd_readb(&dig_connector->dp_i2c_bus->aux,
> +				  DP_EDP_CONFIGURATION_CAP, &tmp);
>  		if (tmp & 1)
>  			panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE;
>  		else if ((dp_bridge == ENCODER_OBJECT_ID_NUTMEG) ||
> @@ -525,7 +500,8 @@ int radeon_dp_get_panel_mode(struct drm_encoder *encoder,
>  			panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE;
>  	} else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
>  		/* eDP */
> -		tmp = radeon_read_dpcd_reg(radeon_connector, DP_EDP_CONFIGURATION_CAP);
> +		drm_dp_dpcd_readb(&dig_connector->dp_i2c_bus->aux,
> +				  DP_EDP_CONFIGURATION_CAP, &tmp);
>  		if (tmp & 1)
>  			panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE;
>  	}
> @@ -576,9 +552,15 @@ int radeon_dp_mode_valid_helper(struct drm_connector *connector,
>  static bool radeon_dp_get_link_status(struct radeon_connector *radeon_connector,
>  				      u8 link_status[DP_LINK_STATUS_SIZE])
>  {
> +	struct radeon_connector_atom_dig *dig_connector;
>  	int ret;
> -	ret = radeon_dp_aux_native_read(radeon_connector, DP_LANE0_1_STATUS,
> -					link_status, DP_LINK_STATUS_SIZE, 100);
> +
> +	if (!radeon_connector->con_priv)
> +		return false;
> +	dig_connector = radeon_connector->con_priv;
> +
> +	ret = drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_LANE0_1_STATUS,
> +			       link_status, DP_LINK_STATUS_SIZE);
>  	if (ret <= 0) {
>  		return false;
>  	}
> @@ -612,7 +594,7 @@ void radeon_dp_set_rx_power_state(struct drm_connector *connector,
>  
>  	/* power up/down the sink */
>  	if (dig_connector->dpcd[0] >= 0x11) {
> -		radeon_write_dpcd_reg(radeon_connector,
> +		drm_dp_dpcd_writeb(&dig_connector->dp_i2c_bus->aux,
>  				   DP_SET_POWER, power_state);
>  		usleep_range(1000, 2000);
>  	}
> @@ -633,6 +615,7 @@ struct radeon_dp_link_train_info {
>  	u8 link_status[DP_LINK_STATUS_SIZE];
>  	u8 tries;
>  	bool use_dpencoder;
> +	struct drm_dp_aux *aux;
>  };
>  
>  static void radeon_dp_update_vs_emph(struct radeon_dp_link_train_info *dp_info)
> @@ -643,8 +626,8 @@ static void radeon_dp_update_vs_emph(struct radeon_dp_link_train_info *dp_info)
>  				       0, dp_info->train_set[0]); /* sets all lanes at once */
>  
>  	/* set the vs/emph on the sink */
> -	radeon_dp_aux_native_write(dp_info->radeon_connector, DP_TRAINING_LANE0_SET,
> -				   dp_info->train_set, dp_info->dp_lane_count, 0);
> +	drm_dp_dpcd_write(dp_info->aux, DP_TRAINING_LANE0_SET,
> +			  dp_info->train_set, dp_info->dp_lane_count);
>  }
>  
>  static void radeon_dp_set_tp(struct radeon_dp_link_train_info *dp_info, int tp)
> @@ -679,7 +662,7 @@ static void radeon_dp_set_tp(struct radeon_dp_link_train_info *dp_info, int tp)
>  	}
>  
>  	/* enable training pattern on the sink */
> -	radeon_write_dpcd_reg(dp_info->radeon_connector, DP_TRAINING_PATTERN_SET, tp);
> +	drm_dp_dpcd_writeb(dp_info->aux, DP_TRAINING_PATTERN_SET, tp);
>  }
>  
>  static int radeon_dp_link_train_init(struct radeon_dp_link_train_info *dp_info)
> @@ -693,26 +676,26 @@ static int radeon_dp_link_train_init(struct radeon_dp_link_train_info *dp_info)
>  
>  	/* possibly enable downspread on the sink */
>  	if (dp_info->dpcd[3] & 0x1)
> -		radeon_write_dpcd_reg(dp_info->radeon_connector,
> -				      DP_DOWNSPREAD_CTRL, DP_SPREAD_AMP_0_5);
> +		drm_dp_dpcd_writeb(dp_info->aux,
> +				   DP_DOWNSPREAD_CTRL, DP_SPREAD_AMP_0_5);
>  	else
> -		radeon_write_dpcd_reg(dp_info->radeon_connector,
> -				      DP_DOWNSPREAD_CTRL, 0);
> +		drm_dp_dpcd_writeb(dp_info->aux,
> +				   DP_DOWNSPREAD_CTRL, 0);
>  
>  	if ((dp_info->connector->connector_type == DRM_MODE_CONNECTOR_eDP) &&
>  	    (dig->panel_mode == DP_PANEL_MODE_INTERNAL_DP2_MODE)) {
> -		radeon_write_dpcd_reg(dp_info->radeon_connector, DP_EDP_CONFIGURATION_SET, 1);
> +		drm_dp_dpcd_writeb(dp_info->aux, DP_EDP_CONFIGURATION_SET, 1);
>  	}
>  
>  	/* set the lane count on the sink */
>  	tmp = dp_info->dp_lane_count;
>  	if (drm_dp_enhanced_frame_cap(dp_info->dpcd))
>  		tmp |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> -	radeon_write_dpcd_reg(dp_info->radeon_connector, DP_LANE_COUNT_SET, tmp);
> +	drm_dp_dpcd_writeb(dp_info->aux, DP_LANE_COUNT_SET, tmp);
>  
>  	/* set the link rate on the sink */
>  	tmp = drm_dp_link_rate_to_bw_code(dp_info->dp_clock);
> -	radeon_write_dpcd_reg(dp_info->radeon_connector, DP_LINK_BW_SET, tmp);
> +	drm_dp_dpcd_writeb(dp_info->aux, DP_LINK_BW_SET, tmp);
>  
>  	/* start training on the source */
>  	if (ASIC_IS_DCE4(dp_info->rdev) || !dp_info->use_dpencoder)
> @@ -723,9 +706,9 @@ static int radeon_dp_link_train_init(struct radeon_dp_link_train_info *dp_info)
>  					  dp_info->dp_clock, dp_info->enc_id, 0);
>  
>  	/* disable the training pattern on the sink */
> -	radeon_write_dpcd_reg(dp_info->radeon_connector,
> -			      DP_TRAINING_PATTERN_SET,
> -			      DP_TRAINING_PATTERN_DISABLE);
> +	drm_dp_dpcd_writeb(dp_info->aux,
> +			   DP_TRAINING_PATTERN_SET,
> +			   DP_TRAINING_PATTERN_DISABLE);
>  
>  	return 0;
>  }
> @@ -735,9 +718,9 @@ static int radeon_dp_link_train_finish(struct radeon_dp_link_train_info *dp_info
>  	udelay(400);
>  
>  	/* disable the training pattern on the sink */
> -	radeon_write_dpcd_reg(dp_info->radeon_connector,
> -			      DP_TRAINING_PATTERN_SET,
> -			      DP_TRAINING_PATTERN_DISABLE);
> +	drm_dp_dpcd_writeb(dp_info->aux,
> +			   DP_TRAINING_PATTERN_SET,
> +			   DP_TRAINING_PATTERN_DISABLE);
>  
>  	/* disable the training pattern on the source */
>  	if (ASIC_IS_DCE4(dp_info->rdev) || !dp_info->use_dpencoder)
> @@ -914,7 +897,7 @@ void radeon_dp_link_train(struct drm_encoder *encoder,
>  	else
>  		dp_info.enc_id |= ATOM_DP_CONFIG_LINK_A;
>  
> -	tmp = radeon_read_dpcd_reg(radeon_connector, DP_MAX_LANE_COUNT);
> +	drm_dp_dpcd_readb(&dig_connector->dp_i2c_bus->aux, DP_MAX_LANE_COUNT, &tmp);
>  	if (ASIC_IS_DCE5(rdev) && (tmp & DP_TPS3_SUPPORTED))
>  		dp_info.tp3_supported = true;
>  	else
> @@ -927,6 +910,7 @@ void radeon_dp_link_train(struct drm_encoder *encoder,
>  	dp_info.radeon_connector = radeon_connector;
>  	dp_info.dp_lane_count = dig_connector->dp_lane_count;
>  	dp_info.dp_clock = dig_connector->dp_clock;
> +	dp_info.aux = &dig_connector->dp_i2c_bus->aux;
>  
>  	if (radeon_dp_link_train_init(&dp_info))
>  		goto done;
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
> index 82d4f86..ec958e86 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -1595,6 +1595,7 @@ radeon_add_atom_connector(struct drm_device *dev,
>  	uint32_t subpixel_order = SubPixelNone;
>  	bool shared_ddc = false;
>  	bool is_dp_bridge = false;
> +	bool has_aux = false;
>  
>  	if (connector_type == DRM_MODE_CONNECTOR_Unknown)
>  		return;
> @@ -1672,7 +1673,9 @@ radeon_add_atom_connector(struct drm_device *dev,
>  				radeon_dig_connector->dp_i2c_bus = radeon_i2c_create_dp(dev, i2c_bus, "eDP-auxch");
>  			else
>  				radeon_dig_connector->dp_i2c_bus = radeon_i2c_create_dp(dev, i2c_bus, "DP-auxch");
> -			if (!radeon_dig_connector->dp_i2c_bus)
> +			if (radeon_dig_connector->dp_i2c_bus)
> +				has_aux = true;
> +			else
>  				DRM_ERROR("DP: Failed to assign dp ddc bus! Check dmesg for i2c errors.\n");
>  			radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
>  			if (!radeon_connector->ddc_bus)
> @@ -1895,7 +1898,9 @@ radeon_add_atom_connector(struct drm_device *dev,
>  				if (!radeon_dig_connector->dp_i2c_bus)
>  					DRM_ERROR("DP: Failed to assign dp ddc bus! Check dmesg for i2c errors.\n");
>  				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> -				if (!radeon_connector->ddc_bus)
> +				if (radeon_connector->ddc_bus)
> +					has_aux = true;
> +				else
>  					DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
>  			}
>  			subpixel_order = SubPixelHorizontalRGB;
> @@ -1939,7 +1944,9 @@ radeon_add_atom_connector(struct drm_device *dev,
>  			if (i2c_bus->valid) {
>  				/* add DP i2c bus */
>  				radeon_dig_connector->dp_i2c_bus = radeon_i2c_create_dp(dev, i2c_bus, "eDP-auxch");
> -				if (!radeon_dig_connector->dp_i2c_bus)
> +				if (radeon_dig_connector->dp_i2c_bus)
> +					has_aux = true;
> +				else
>  					DRM_ERROR("DP: Failed to assign dp ddc bus! Check dmesg for i2c errors.\n");
>  				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
>  				if (!radeon_connector->ddc_bus)
> @@ -2000,6 +2007,10 @@ radeon_add_atom_connector(struct drm_device *dev,
>  
>  	connector->display_info.subpixel_order = subpixel_order;
>  	drm_sysfs_connector_add(connector);
> +
> +	if (has_aux)
> +		radeon_dp_aux_init(radeon_connector);
> +
>  	return;
>  
>  failed:
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
> index e390f55..832d9fa 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -192,6 +192,7 @@ struct radeon_i2c_chan {
>  		struct i2c_algo_dp_aux_data dp;
>  	} algo;
>  	struct radeon_i2c_bus_rec rec;
> +	struct drm_dp_aux aux;
>  };
>  
>  /* mostly for macs, but really any system without connector tables */
> @@ -692,6 +693,7 @@ extern int radeon_dp_get_panel_mode(struct drm_encoder *encoder,
>  				    struct drm_connector *connector);
>  extern void radeon_dp_set_rx_power_state(struct drm_connector *connector,
>  					 u8 power_state);
> +extern void radeon_dp_aux_init(struct radeon_connector *radeon_connector);
>  extern void atombios_dig_encoder_setup(struct drm_encoder *encoder, int action, int panel_mode);
>  extern void radeon_atom_encoder_init(struct radeon_device *rdev);
>  extern void radeon_atom_disp_eng_pll_init(struct radeon_device *rdev);
> -- 
> 1.8.3.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] drm/dp: make aux retries less chatty
@ 2014-03-21 14:34 Alex Deucher
  2014-03-21 14:34 ` [PATCH 2/3] drm/radeon: use the new drm helpers for dp aux Alex Deucher
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Alex Deucher @ 2014-03-21 14:34 UTC (permalink / raw)
  To: dri-devel, christian.koenig; +Cc: Alex Deucher

Switch to debug only to avoid flooding the logs.
This mirrors the behavior in some other drivers.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_dp_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 35251af..74724aa 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -402,7 +402,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
 		}
 	}
 
-	DRM_ERROR("too many retries, giving up\n");
+	DRM_DEBUG_KMS("too many retries, giving up\n");
 	return -EIO;
 }
 
@@ -656,7 +656,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 		}
 	}
 
-	DRM_ERROR("too many retries, giving up\n");
+	DRM_DEBUG_KMS("too many retries, giving up\n");
 	return -EREMOTEIO;
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] drm/radeon: use the new drm helpers for dp aux
  2014-03-21 14:34 [PATCH 1/3] drm/dp: make aux retries less chatty Alex Deucher
@ 2014-03-21 14:34 ` Alex Deucher
  2014-03-21 14:34 ` [PATCH 3/3] drm/radeon: use drm_dp_dpcd_read_link_status() Alex Deucher
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2014-03-21 14:34 UTC (permalink / raw)
  To: dri-devel, christian.koenig; +Cc: Alex Deucher

Switch to the new dp helpers.  The main difference is
that the DP helpers don't allow an adjustable delay in
the aux transaction, but I don't know that this is
necessary.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/atombios_dp.c       | 192 +++++++++++++----------------
 drivers/gpu/drm/radeon/radeon_connectors.c |  17 ++-
 drivers/gpu/drm/radeon/radeon_mode.h       |   2 +
 3 files changed, 104 insertions(+), 107 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
index 23189b7..8d8f846 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -142,94 +142,62 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan,
 	return recv_bytes;
 }
 
-static int radeon_dp_aux_native_write(struct radeon_connector *radeon_connector,
-				      u16 address, u8 *send, u8 send_bytes, u8 delay)
-{
-	struct radeon_connector_atom_dig *dig_connector = radeon_connector->con_priv;
-	int ret;
-	u8 msg[20];
-	int msg_bytes = send_bytes + 4;
-	u8 ack;
-	unsigned retry;
-
-	if (send_bytes > 16)
-		return -1;
+#define HEADER_SIZE 4
 
-	msg[0] = address;
-	msg[1] = address >> 8;
-	msg[2] = DP_AUX_NATIVE_WRITE << 4;
-	msg[3] = (msg_bytes << 4) | (send_bytes - 1);
-	memcpy(&msg[4], send, send_bytes);
-
-	for (retry = 0; retry < 7; retry++) {
-		ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
-					    msg, msg_bytes, NULL, 0, delay, &ack);
-		if (ret == -EBUSY)
-			continue;
-		else 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;
-	}
-
-	return -EIO;
-}
-
-static int radeon_dp_aux_native_read(struct radeon_connector *radeon_connector,
-				     u16 address, u8 *recv, int recv_bytes, u8 delay)
+static ssize_t
+radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
-	struct radeon_connector_atom_dig *dig_connector = radeon_connector->con_priv;
-	u8 msg[4];
-	int msg_bytes = 4;
-	u8 ack;
+	struct radeon_i2c_chan *chan =
+		container_of(aux, struct radeon_i2c_chan, aux);
 	int ret;
-	unsigned retry;
-
-	msg[0] = address;
-	msg[1] = address >> 8;
-	msg[2] = DP_AUX_NATIVE_READ << 4;
-	msg[3] = (msg_bytes << 4) | (recv_bytes - 1);
-
-	for (retry = 0; retry < 7; retry++) {
-		ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
-					    msg, msg_bytes, recv, recv_bytes, delay, &ack);
-		if (ret == -EBUSY)
-			continue;
-		else if (ret < 0)
-			return ret;
-		ack >>= 4;
-		if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK)
-			return ret;
-		else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
-			usleep_range(400, 500);
-		else if (ret == 0)
-			return -EPROTO;
-		else
-			return -EIO;
+	u8 tx_buf[20];
+	size_t tx_size;
+	u8 ack, delay = 0;
+
+	if (WARN_ON(msg->size > 16))
+		return -E2BIG;
+
+	tx_buf[0] = msg->address & 0xff;
+	tx_buf[1] = msg->address >> 8;
+	tx_buf[2] = msg->request << 4;
+	tx_buf[3] = msg->size - 1;
+
+	switch (msg->request & ~DP_AUX_I2C_MOT) {
+	case DP_AUX_NATIVE_WRITE:
+	case DP_AUX_I2C_WRITE:
+		tx_size = HEADER_SIZE + msg->size;
+		tx_buf[3] |= tx_size << 4;
+		memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size);
+		ret = radeon_process_aux_ch(chan,
+					    tx_buf, tx_size, NULL, 0, delay, &ack);
+		if (ret >= 0)
+			/* Return payload size. */
+			ret = msg->size;
+		break;
+	case DP_AUX_NATIVE_READ:
+	case DP_AUX_I2C_READ:
+		tx_size = HEADER_SIZE;
+		tx_buf[3] |= tx_size << 4;
+		ret = radeon_process_aux_ch(chan,
+					    tx_buf, tx_size, msg->buffer, msg->size, delay, &ack);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
 	}
 
-	return -EIO;
-}
+	if (ret > 0)
+		msg->reply = ack >> 4;
 
-static void radeon_write_dpcd_reg(struct radeon_connector *radeon_connector,
-				 u16 reg, u8 val)
-{
-	radeon_dp_aux_native_write(radeon_connector, reg, &val, 1, 0);
+	return ret;
 }
 
-static u8 radeon_read_dpcd_reg(struct radeon_connector *radeon_connector,
-			       u16 reg)
+void radeon_dp_aux_init(struct radeon_connector *radeon_connector)
 {
-	u8 val = 0;
-
-	radeon_dp_aux_native_read(radeon_connector, reg, &val, 1, 0);
+	struct radeon_connector_atom_dig *dig_connector = radeon_connector->con_priv;
 
-	return val;
+	dig_connector->dp_i2c_bus->aux.dev = radeon_connector->base.kdev;
+	dig_connector->dp_i2c_bus->aux.transfer = radeon_dp_aux_transfer;
 }
 
 int radeon_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
@@ -468,11 +436,11 @@ static void radeon_dp_probe_oui(struct radeon_connector *radeon_connector)
 	if (!(dig_connector->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT))
 		return;
 
-	if (radeon_dp_aux_native_read(radeon_connector, DP_SINK_OUI, buf, 3, 0))
+	if (drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_SINK_OUI, buf, 3))
 		DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n",
 			      buf[0], buf[1], buf[2]);
 
-	if (radeon_dp_aux_native_read(radeon_connector, DP_BRANCH_OUI, buf, 3, 0))
+	if (drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_BRANCH_OUI, buf, 3))
 		DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n",
 			      buf[0], buf[1], buf[2]);
 }
@@ -483,8 +451,8 @@ bool radeon_dp_getdpcd(struct radeon_connector *radeon_connector)
 	u8 msg[DP_DPCD_SIZE];
 	int ret, i;
 
-	ret = radeon_dp_aux_native_read(radeon_connector, DP_DPCD_REV, msg,
-					DP_DPCD_SIZE, 0);
+	ret = drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_DPCD_REV, msg,
+			       DP_DPCD_SIZE);
 	if (ret > 0) {
 		memcpy(dig_connector->dpcd, msg, DP_DPCD_SIZE);
 		DRM_DEBUG_KMS("DPCD: ");
@@ -506,6 +474,7 @@ int radeon_dp_get_panel_mode(struct drm_encoder *encoder,
 	struct drm_device *dev = encoder->dev;
 	struct radeon_device *rdev = dev->dev_private;
 	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
+	struct radeon_connector_atom_dig *dig_connector;
 	int panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE;
 	u16 dp_bridge = radeon_connector_encoder_get_dp_bridge_encoder_id(connector);
 	u8 tmp;
@@ -513,9 +482,15 @@ int radeon_dp_get_panel_mode(struct drm_encoder *encoder,
 	if (!ASIC_IS_DCE4(rdev))
 		return panel_mode;
 
+	if (!radeon_connector->con_priv)
+		return panel_mode;
+
+	dig_connector = radeon_connector->con_priv;
+
 	if (dp_bridge != ENCODER_OBJECT_ID_NONE) {
 		/* DP bridge chips */
-		tmp = radeon_read_dpcd_reg(radeon_connector, DP_EDP_CONFIGURATION_CAP);
+		drm_dp_dpcd_readb(&dig_connector->dp_i2c_bus->aux,
+				  DP_EDP_CONFIGURATION_CAP, &tmp);
 		if (tmp & 1)
 			panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE;
 		else if ((dp_bridge == ENCODER_OBJECT_ID_NUTMEG) ||
@@ -525,7 +500,8 @@ int radeon_dp_get_panel_mode(struct drm_encoder *encoder,
 			panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE;
 	} else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
 		/* eDP */
-		tmp = radeon_read_dpcd_reg(radeon_connector, DP_EDP_CONFIGURATION_CAP);
+		drm_dp_dpcd_readb(&dig_connector->dp_i2c_bus->aux,
+				  DP_EDP_CONFIGURATION_CAP, &tmp);
 		if (tmp & 1)
 			panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE;
 	}
@@ -576,9 +552,15 @@ int radeon_dp_mode_valid_helper(struct drm_connector *connector,
 static bool radeon_dp_get_link_status(struct radeon_connector *radeon_connector,
 				      u8 link_status[DP_LINK_STATUS_SIZE])
 {
+	struct radeon_connector_atom_dig *dig_connector;
 	int ret;
-	ret = radeon_dp_aux_native_read(radeon_connector, DP_LANE0_1_STATUS,
-					link_status, DP_LINK_STATUS_SIZE, 100);
+
+	if (!radeon_connector->con_priv)
+		return false;
+	dig_connector = radeon_connector->con_priv;
+
+	ret = drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_LANE0_1_STATUS,
+			       link_status, DP_LINK_STATUS_SIZE);
 	if (ret <= 0) {
 		return false;
 	}
@@ -612,7 +594,7 @@ void radeon_dp_set_rx_power_state(struct drm_connector *connector,
 
 	/* power up/down the sink */
 	if (dig_connector->dpcd[0] >= 0x11) {
-		radeon_write_dpcd_reg(radeon_connector,
+		drm_dp_dpcd_writeb(&dig_connector->dp_i2c_bus->aux,
 				   DP_SET_POWER, power_state);
 		usleep_range(1000, 2000);
 	}
@@ -633,6 +615,7 @@ struct radeon_dp_link_train_info {
 	u8 link_status[DP_LINK_STATUS_SIZE];
 	u8 tries;
 	bool use_dpencoder;
+	struct drm_dp_aux *aux;
 };
 
 static void radeon_dp_update_vs_emph(struct radeon_dp_link_train_info *dp_info)
@@ -643,8 +626,8 @@ static void radeon_dp_update_vs_emph(struct radeon_dp_link_train_info *dp_info)
 				       0, dp_info->train_set[0]); /* sets all lanes at once */
 
 	/* set the vs/emph on the sink */
-	radeon_dp_aux_native_write(dp_info->radeon_connector, DP_TRAINING_LANE0_SET,
-				   dp_info->train_set, dp_info->dp_lane_count, 0);
+	drm_dp_dpcd_write(dp_info->aux, DP_TRAINING_LANE0_SET,
+			  dp_info->train_set, dp_info->dp_lane_count);
 }
 
 static void radeon_dp_set_tp(struct radeon_dp_link_train_info *dp_info, int tp)
@@ -679,7 +662,7 @@ static void radeon_dp_set_tp(struct radeon_dp_link_train_info *dp_info, int tp)
 	}
 
 	/* enable training pattern on the sink */
-	radeon_write_dpcd_reg(dp_info->radeon_connector, DP_TRAINING_PATTERN_SET, tp);
+	drm_dp_dpcd_writeb(dp_info->aux, DP_TRAINING_PATTERN_SET, tp);
 }
 
 static int radeon_dp_link_train_init(struct radeon_dp_link_train_info *dp_info)
@@ -693,26 +676,26 @@ static int radeon_dp_link_train_init(struct radeon_dp_link_train_info *dp_info)
 
 	/* possibly enable downspread on the sink */
 	if (dp_info->dpcd[3] & 0x1)
-		radeon_write_dpcd_reg(dp_info->radeon_connector,
-				      DP_DOWNSPREAD_CTRL, DP_SPREAD_AMP_0_5);
+		drm_dp_dpcd_writeb(dp_info->aux,
+				   DP_DOWNSPREAD_CTRL, DP_SPREAD_AMP_0_5);
 	else
-		radeon_write_dpcd_reg(dp_info->radeon_connector,
-				      DP_DOWNSPREAD_CTRL, 0);
+		drm_dp_dpcd_writeb(dp_info->aux,
+				   DP_DOWNSPREAD_CTRL, 0);
 
 	if ((dp_info->connector->connector_type == DRM_MODE_CONNECTOR_eDP) &&
 	    (dig->panel_mode == DP_PANEL_MODE_INTERNAL_DP2_MODE)) {
-		radeon_write_dpcd_reg(dp_info->radeon_connector, DP_EDP_CONFIGURATION_SET, 1);
+		drm_dp_dpcd_writeb(dp_info->aux, DP_EDP_CONFIGURATION_SET, 1);
 	}
 
 	/* set the lane count on the sink */
 	tmp = dp_info->dp_lane_count;
 	if (drm_dp_enhanced_frame_cap(dp_info->dpcd))
 		tmp |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
-	radeon_write_dpcd_reg(dp_info->radeon_connector, DP_LANE_COUNT_SET, tmp);
+	drm_dp_dpcd_writeb(dp_info->aux, DP_LANE_COUNT_SET, tmp);
 
 	/* set the link rate on the sink */
 	tmp = drm_dp_link_rate_to_bw_code(dp_info->dp_clock);
-	radeon_write_dpcd_reg(dp_info->radeon_connector, DP_LINK_BW_SET, tmp);
+	drm_dp_dpcd_writeb(dp_info->aux, DP_LINK_BW_SET, tmp);
 
 	/* start training on the source */
 	if (ASIC_IS_DCE4(dp_info->rdev) || !dp_info->use_dpencoder)
@@ -723,9 +706,9 @@ static int radeon_dp_link_train_init(struct radeon_dp_link_train_info *dp_info)
 					  dp_info->dp_clock, dp_info->enc_id, 0);
 
 	/* disable the training pattern on the sink */
-	radeon_write_dpcd_reg(dp_info->radeon_connector,
-			      DP_TRAINING_PATTERN_SET,
-			      DP_TRAINING_PATTERN_DISABLE);
+	drm_dp_dpcd_writeb(dp_info->aux,
+			   DP_TRAINING_PATTERN_SET,
+			   DP_TRAINING_PATTERN_DISABLE);
 
 	return 0;
 }
@@ -735,9 +718,9 @@ static int radeon_dp_link_train_finish(struct radeon_dp_link_train_info *dp_info
 	udelay(400);
 
 	/* disable the training pattern on the sink */
-	radeon_write_dpcd_reg(dp_info->radeon_connector,
-			      DP_TRAINING_PATTERN_SET,
-			      DP_TRAINING_PATTERN_DISABLE);
+	drm_dp_dpcd_writeb(dp_info->aux,
+			   DP_TRAINING_PATTERN_SET,
+			   DP_TRAINING_PATTERN_DISABLE);
 
 	/* disable the training pattern on the source */
 	if (ASIC_IS_DCE4(dp_info->rdev) || !dp_info->use_dpencoder)
@@ -914,7 +897,7 @@ void radeon_dp_link_train(struct drm_encoder *encoder,
 	else
 		dp_info.enc_id |= ATOM_DP_CONFIG_LINK_A;
 
-	tmp = radeon_read_dpcd_reg(radeon_connector, DP_MAX_LANE_COUNT);
+	drm_dp_dpcd_readb(&dig_connector->dp_i2c_bus->aux, DP_MAX_LANE_COUNT, &tmp);
 	if (ASIC_IS_DCE5(rdev) && (tmp & DP_TPS3_SUPPORTED))
 		dp_info.tp3_supported = true;
 	else
@@ -927,6 +910,7 @@ void radeon_dp_link_train(struct drm_encoder *encoder,
 	dp_info.radeon_connector = radeon_connector;
 	dp_info.dp_lane_count = dig_connector->dp_lane_count;
 	dp_info.dp_clock = dig_connector->dp_clock;
+	dp_info.aux = &dig_connector->dp_i2c_bus->aux;
 
 	if (radeon_dp_link_train_init(&dp_info))
 		goto done;
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 82d4f86..ec958e86 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1595,6 +1595,7 @@ radeon_add_atom_connector(struct drm_device *dev,
 	uint32_t subpixel_order = SubPixelNone;
 	bool shared_ddc = false;
 	bool is_dp_bridge = false;
+	bool has_aux = false;
 
 	if (connector_type == DRM_MODE_CONNECTOR_Unknown)
 		return;
@@ -1672,7 +1673,9 @@ radeon_add_atom_connector(struct drm_device *dev,
 				radeon_dig_connector->dp_i2c_bus = radeon_i2c_create_dp(dev, i2c_bus, "eDP-auxch");
 			else
 				radeon_dig_connector->dp_i2c_bus = radeon_i2c_create_dp(dev, i2c_bus, "DP-auxch");
-			if (!radeon_dig_connector->dp_i2c_bus)
+			if (radeon_dig_connector->dp_i2c_bus)
+				has_aux = true;
+			else
 				DRM_ERROR("DP: Failed to assign dp ddc bus! Check dmesg for i2c errors.\n");
 			radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
 			if (!radeon_connector->ddc_bus)
@@ -1895,7 +1898,9 @@ radeon_add_atom_connector(struct drm_device *dev,
 				if (!radeon_dig_connector->dp_i2c_bus)
 					DRM_ERROR("DP: Failed to assign dp ddc bus! Check dmesg for i2c errors.\n");
 				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
-				if (!radeon_connector->ddc_bus)
+				if (radeon_connector->ddc_bus)
+					has_aux = true;
+				else
 					DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
 			}
 			subpixel_order = SubPixelHorizontalRGB;
@@ -1939,7 +1944,9 @@ radeon_add_atom_connector(struct drm_device *dev,
 			if (i2c_bus->valid) {
 				/* add DP i2c bus */
 				radeon_dig_connector->dp_i2c_bus = radeon_i2c_create_dp(dev, i2c_bus, "eDP-auxch");
-				if (!radeon_dig_connector->dp_i2c_bus)
+				if (radeon_dig_connector->dp_i2c_bus)
+					has_aux = true;
+				else
 					DRM_ERROR("DP: Failed to assign dp ddc bus! Check dmesg for i2c errors.\n");
 				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
 				if (!radeon_connector->ddc_bus)
@@ -2000,6 +2007,10 @@ radeon_add_atom_connector(struct drm_device *dev,
 
 	connector->display_info.subpixel_order = subpixel_order;
 	drm_sysfs_connector_add(connector);
+
+	if (has_aux)
+		radeon_dp_aux_init(radeon_connector);
+
 	return;
 
 failed:
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index e390f55..832d9fa 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -192,6 +192,7 @@ struct radeon_i2c_chan {
 		struct i2c_algo_dp_aux_data dp;
 	} algo;
 	struct radeon_i2c_bus_rec rec;
+	struct drm_dp_aux aux;
 };
 
 /* mostly for macs, but really any system without connector tables */
@@ -692,6 +693,7 @@ extern int radeon_dp_get_panel_mode(struct drm_encoder *encoder,
 				    struct drm_connector *connector);
 extern void radeon_dp_set_rx_power_state(struct drm_connector *connector,
 					 u8 power_state);
+extern void radeon_dp_aux_init(struct radeon_connector *radeon_connector);
 extern void atombios_dig_encoder_setup(struct drm_encoder *encoder, int action, int panel_mode);
 extern void radeon_atom_encoder_init(struct radeon_device *rdev);
 extern void radeon_atom_disp_eng_pll_init(struct radeon_device *rdev);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] drm/radeon: use drm_dp_dpcd_read_link_status()
  2014-03-21 14:34 [PATCH 1/3] drm/dp: make aux retries less chatty Alex Deucher
  2014-03-21 14:34 ` [PATCH 2/3] drm/radeon: use the new drm helpers for dp aux Alex Deucher
@ 2014-03-21 14:34 ` Alex Deucher
  2014-03-21 18:01 ` [PATCH 1/3] drm/dp: make aux retries less chatty Daniel Vetter
  2014-03-21 19:06 ` Christian König
  3 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2014-03-21 14:34 UTC (permalink / raw)
  To: dri-devel, christian.koenig; +Cc: Alex Deucher

Replace the radeon specific version with the generic version.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/atombios_dp.c | 30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
index 8d8f846..8b0ab17 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -549,32 +549,12 @@ int radeon_dp_mode_valid_helper(struct drm_connector *connector,
 	return MODE_OK;
 }
 
-static bool radeon_dp_get_link_status(struct radeon_connector *radeon_connector,
-				      u8 link_status[DP_LINK_STATUS_SIZE])
-{
-	struct radeon_connector_atom_dig *dig_connector;
-	int ret;
-
-	if (!radeon_connector->con_priv)
-		return false;
-	dig_connector = radeon_connector->con_priv;
-
-	ret = drm_dp_dpcd_read(&dig_connector->dp_i2c_bus->aux, DP_LANE0_1_STATUS,
-			       link_status, DP_LINK_STATUS_SIZE);
-	if (ret <= 0) {
-		return false;
-	}
-
-	DRM_DEBUG_KMS("link status %6ph\n", link_status);
-	return true;
-}
-
 bool radeon_dp_needs_link_train(struct radeon_connector *radeon_connector)
 {
 	u8 link_status[DP_LINK_STATUS_SIZE];
 	struct radeon_connector_atom_dig *dig = radeon_connector->con_priv;
 
-	if (!radeon_dp_get_link_status(radeon_connector, link_status))
+	if (drm_dp_dpcd_read_link_status(&dig->dp_i2c_bus->aux, link_status) <= 0)
 		return false;
 	if (drm_dp_channel_eq_ok(link_status, dig->dp_lane_count))
 		return false;
@@ -605,7 +585,6 @@ struct radeon_dp_link_train_info {
 	struct radeon_device *rdev;
 	struct drm_encoder *encoder;
 	struct drm_connector *connector;
-	struct radeon_connector *radeon_connector;
 	int enc_id;
 	int dp_clock;
 	int dp_lane_count;
@@ -752,7 +731,8 @@ static int radeon_dp_link_train_cr(struct radeon_dp_link_train_info *dp_info)
 	while (1) {
 		drm_dp_link_train_clock_recovery_delay(dp_info->dpcd);
 
-		if (!radeon_dp_get_link_status(dp_info->radeon_connector, dp_info->link_status)) {
+		if (drm_dp_dpcd_read_link_status(dp_info->aux,
+						 dp_info->link_status) <= 0) {
 			DRM_ERROR("displayport link status failed\n");
 			break;
 		}
@@ -814,7 +794,8 @@ static int radeon_dp_link_train_ce(struct radeon_dp_link_train_info *dp_info)
 	while (1) {
 		drm_dp_link_train_channel_eq_delay(dp_info->dpcd);
 
-		if (!radeon_dp_get_link_status(dp_info->radeon_connector, dp_info->link_status)) {
+		if (drm_dp_dpcd_read_link_status(dp_info->aux,
+						 dp_info->link_status) <= 0) {
 			DRM_ERROR("displayport link status failed\n");
 			break;
 		}
@@ -907,7 +888,6 @@ void radeon_dp_link_train(struct drm_encoder *encoder,
 	dp_info.rdev = rdev;
 	dp_info.encoder = encoder;
 	dp_info.connector = connector;
-	dp_info.radeon_connector = radeon_connector;
 	dp_info.dp_lane_count = dig_connector->dp_lane_count;
 	dp_info.dp_clock = dig_connector->dp_clock;
 	dp_info.aux = &dig_connector->dp_i2c_bus->aux;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] drm/dp: make aux retries less chatty
  2014-03-21 14:34 [PATCH 1/3] drm/dp: make aux retries less chatty Alex Deucher
  2014-03-21 14:34 ` [PATCH 2/3] drm/radeon: use the new drm helpers for dp aux Alex Deucher
  2014-03-21 14:34 ` [PATCH 3/3] drm/radeon: use drm_dp_dpcd_read_link_status() Alex Deucher
@ 2014-03-21 18:01 ` Daniel Vetter
  2014-03-21 19:06 ` Christian König
  3 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2014-03-21 18:01 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Alex Deucher, christian.koenig, dri-devel

On Fri, Mar 21, 2014 at 10:34:06AM -0400, Alex Deucher wrote:
> Switch to debug only to avoid flooding the logs.
> This mirrors the behavior in some other drivers.
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Btw I've just noticed that we have a 2nd retry loop with 3 retries and a 1
msec delay which apparently is needed to kick suspended dp sinks out of
their sleep. See intel_dp_dpcd_read_wake in intel_dp.c. Should we maybe
move that 2nd loop into these helpers, too?
-Daniel

> ---
>  drivers/gpu/drm/drm_dp_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 35251af..74724aa 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -402,7 +402,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>  		}
>  	}
>  
> -	DRM_ERROR("too many retries, giving up\n");
> +	DRM_DEBUG_KMS("too many retries, giving up\n");
>  	return -EIO;
>  }
>  
> @@ -656,7 +656,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  		}
>  	}
>  
> -	DRM_ERROR("too many retries, giving up\n");
> +	DRM_DEBUG_KMS("too many retries, giving up\n");
>  	return -EREMOTEIO;
>  }
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] drm/dp: make aux retries less chatty
  2014-03-21 14:34 [PATCH 1/3] drm/dp: make aux retries less chatty Alex Deucher
                   ` (2 preceding siblings ...)
  2014-03-21 18:01 ` [PATCH 1/3] drm/dp: make aux retries less chatty Daniel Vetter
@ 2014-03-21 19:06 ` Christian König
  3 siblings, 0 replies; 7+ messages in thread
From: Christian König @ 2014-03-21 19:06 UTC (permalink / raw)
  To: Alex Deucher, dri-devel; +Cc: Alex Deucher

Am 21.03.2014 15:34, schrieb Alex Deucher:
> Switch to debug only to avoid flooding the logs.
> This mirrors the behavior in some other drivers.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Added to my 3.15 queue 
(http://cgit.freedesktop.org/~deathsimple/linux/log/?h=drm-next-3.15-wip).

Thanks,
Christian.

> ---
>   drivers/gpu/drm/drm_dp_helper.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 35251af..74724aa 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -402,7 +402,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>   		}
>   	}
>   
> -	DRM_ERROR("too many retries, giving up\n");
> +	DRM_DEBUG_KMS("too many retries, giving up\n");
>   	return -EIO;
>   }
>   
> @@ -656,7 +656,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>   		}
>   	}
>   
> -	DRM_ERROR("too many retries, giving up\n");
> +	DRM_DEBUG_KMS("too many retries, giving up\n");
>   	return -EREMOTEIO;
>   }
>   

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-03-21 19:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-21 14:34 [PATCH 1/3] drm/dp: make aux retries less chatty Alex Deucher
2014-03-21 14:34 ` [PATCH 2/3] drm/radeon: use the new drm helpers for dp aux Alex Deucher
2014-03-21 14:34 ` [PATCH 3/3] drm/radeon: use drm_dp_dpcd_read_link_status() Alex Deucher
2014-03-21 18:01 ` [PATCH 1/3] drm/dp: make aux retries less chatty Daniel Vetter
2014-03-21 19:06 ` Christian König
  -- strict thread matches above, loose matches on Subject: below --
2014-03-18  3:51 Alex Deucher
2014-03-18  3:51 ` [PATCH 2/3] drm/radeon: use the new drm helpers for dp aux Alex Deucher
2014-03-18  7:51   ` Jani Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox