thr3ads.net - Nouveau - [Nouveau] [PATCH 00/10] extract dp helper functions [Oct 2012]

If this information is useful, please help other people find it:
Share via:

Daniel Vetter

2012-Oct-18 08:15 UTC

[Nouveau] [PATCH 00/10] extract dp helper functions

Hi all,

I've frustrated myself the last few days yelling at our link training code.
Comparing the i915 code to radeon and nouveau I've noticed the lack of a
nice
set of dp helper functions. So I've started to extract a few.

There's lots more that we can do I think (link configuration selection, the
i2c
over aux retry stuff which diverges already between i915 and radeon, maybe more
higher level parts of the training sequence). But there the drivers diverge
quite a bit (e.g. the link configuration is driver by different things in each
driver: coded link bw from the dp spec, link clock or required bw vs avialable
bw), so that's more work and probably best done when reworking these
functions
for other reasons.

I've also tried to put the new helpers a bit to use in nouveau, but due to
lack
of hw that part is untested.

Comments and testing highly welcome.

Yours, Daniel

Daniel Vetter (10):
  drm: rename drm_dp_i2c_helper.c to drm_dp_helper.c
  drm: dp helper: extract drm_dp_channel_eq_ok
  drm: dp helper: extract drm_dp_clock_recovery_ok
  drm/nouveau: use the cr_ok/chanel_eq_ok helpers
  drm: extract helpers to compute new training values from sink request
  drm/nouveau: use dp link train request helper
  drm: extract dp link train delay functions from radeon
  drm/i915: use the new dp train delay helpers
  drm: extract dp link bw helpers
  drm: extract drm_dp_max_lane_count helper

 drivers/gpu/drm/Makefile             |   2 +-
 drivers/gpu/drm/drm_dp_helper.c      | 328 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_dp_i2c_helper.c  | 208 ----------------------
 drivers/gpu/drm/i915/intel_dp.c      |  98 ++---------
 drivers/gpu/drm/nouveau/nouveau_dp.c |  35 +---
 drivers/gpu/drm/radeon/atombios_dp.c | 147 ++--------------
 drivers/gpu/drm/radeon/radeon_mode.h |   2 +-
 include/drm/drm_dp_helper.h          |  31 ++++
 8 files changed, 400 insertions(+), 451 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_dp_helper.c
 delete mode 100644 drivers/gpu/drm/drm_dp_i2c_helper.c

-- 
1.7.11.4

Daniel Vetter

2012-Oct-18 08:15 UTC

head link

[Nouveau] [PATCH 01/10] drm: rename drm_dp_i2c_helper.c to drm_dp_helper.c

I want to move some dp link training helpers into this place, so in
the future this won't be just about i2c any longer.

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/Makefile            |   2 +-
 drivers/gpu/drm/drm_dp_helper.c     | 208 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_dp_i2c_helper.c | 208 ------------------------------------
 3 files changed, 209 insertions(+), 209 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_dp_helper.c
 delete mode 100644 drivers/gpu/drm/drm_dp_i2c_helper.c

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 2ff5cef..dc4e88f 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -19,7 +19,7 @@ drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
 
 drm-usb-y   := drm_usb.o
 
-drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_i2c_helper.o
+drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o drm_dp_helper.o
 drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
 drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
 
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
new file mode 100644
index 0000000..f7eba0a
--- /dev/null
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -0,0 +1,208 @@
+/*
+ * Copyright ? 2009 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided
"as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/sched.h>
+#include <linux/i2c.h>
+#include "drm_dp_helper.h"
+#include "drmP.h"
+
+/* Run a single AUX_CH I2C transaction, writing/reading data as necessary */
+static int
+i2c_algo_dp_aux_transaction(struct i2c_adapter *adapter, int mode,
+			    uint8_t write_byte, uint8_t *read_byte)
+{
+	struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
+	int ret;
+	
+	ret = (*algo_data->aux_ch)(adapter, mode,
+				   write_byte, read_byte);
+	return ret;
+}
+
+/*
+ * I2C over AUX CH
+ */
+
+/*
+ * Send the address. If the I2C link is running, this 'restarts'
+ * the connection with the new address, this is used for doing
+ * a write followed by a read (as needed for DDC)
+ */
+static int
+i2c_algo_dp_aux_address(struct i2c_adapter *adapter, u16 address, bool reading)
+{
+	struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
+	int mode = MODE_I2C_START;
+	int ret;
+
+	if (reading)
+		mode |= MODE_I2C_READ;
+	else
+		mode |= MODE_I2C_WRITE;
+	algo_data->address = address;
+	algo_data->running = true;
+	ret = i2c_algo_dp_aux_transaction(adapter, mode, 0, NULL);
+	return ret;
+}
+
+/*
+ * Stop the I2C transaction. This closes out the link, sending
+ * a bare address packet with the MOT bit turned off
+ */
+static void
+i2c_algo_dp_aux_stop(struct i2c_adapter *adapter, bool reading)
+{
+	struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
+	int mode = MODE_I2C_STOP;
+
+	if (reading)
+		mode |= MODE_I2C_READ;
+	else
+		mode |= MODE_I2C_WRITE;
+	if (algo_data->running) {
+		(void) i2c_algo_dp_aux_transaction(adapter, mode, 0, NULL);
+		algo_data->running = false;
+	}
+}
+
+/*
+ * Write a single byte to the current I2C address, the
+ * the I2C link must be running or this returns -EIO
+ */
+static int
+i2c_algo_dp_aux_put_byte(struct i2c_adapter *adapter, u8 byte)
+{
+	struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
+	int ret;
+
+	if (!algo_data->running)
+		return -EIO;
+
+	ret = i2c_algo_dp_aux_transaction(adapter, MODE_I2C_WRITE, byte, NULL);
+	return ret;
+}
+
+/*
+ * Read a single byte from the current I2C address, the
+ * I2C link must be running or this returns -EIO
+ */
+static int
+i2c_algo_dp_aux_get_byte(struct i2c_adapter *adapter, u8 *byte_ret)
+{
+	struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
+	int ret;
+
+	if (!algo_data->running)
+		return -EIO;
+
+	ret = i2c_algo_dp_aux_transaction(adapter, MODE_I2C_READ, 0, byte_ret);
+	return ret;
+}
+
+static int
+i2c_algo_dp_aux_xfer(struct i2c_adapter *adapter,
+		     struct i2c_msg *msgs,
+		     int num)
+{
+	int ret = 0;
+	bool reading = false;
+	int m;
+	int b;
+
+	for (m = 0; m < num; m++) {
+		u16 len = msgs[m].len;
+		u8 *buf = msgs[m].buf;
+		reading = (msgs[m].flags & I2C_M_RD) != 0;
+		ret = i2c_algo_dp_aux_address(adapter, msgs[m].addr, reading);
+		if (ret < 0)
+			break;
+		if (reading) {
+			for (b = 0; b < len; b++) {
+				ret = i2c_algo_dp_aux_get_byte(adapter, &buf[b]);
+				if (ret < 0)
+					break;
+			}
+		} else {
+			for (b = 0; b < len; b++) {
+				ret = i2c_algo_dp_aux_put_byte(adapter, buf[b]);
+				if (ret < 0)
+					break;
+			}
+		}
+		if (ret < 0)
+			break;
+	}
+	if (ret >= 0)
+		ret = num;
+	i2c_algo_dp_aux_stop(adapter, reading);
+	DRM_DEBUG_KMS("dp_aux_xfer return %d\n", ret);
+	return ret;
+}
+
+static u32
+i2c_algo_dp_aux_functionality(struct i2c_adapter *adapter)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
+	       I2C_FUNC_SMBUS_READ_BLOCK_DATA |
+	       I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
+	       I2C_FUNC_10BIT_ADDR;
+}
+
+static const struct i2c_algorithm i2c_dp_aux_algo = {
+	.master_xfer	= i2c_algo_dp_aux_xfer,
+	.functionality	= i2c_algo_dp_aux_functionality,
+};
+
+static void
+i2c_dp_aux_reset_bus(struct i2c_adapter *adapter)
+{
+	(void) i2c_algo_dp_aux_address(adapter, 0, false);
+	(void) i2c_algo_dp_aux_stop(adapter, false);
+					   
+}
+
+static int
+i2c_dp_aux_prepare_bus(struct i2c_adapter *adapter)
+{
+	adapter->algo = &i2c_dp_aux_algo;
+	adapter->retries = 3;
+	i2c_dp_aux_reset_bus(adapter);
+	return 0;
+}
+
+int
+i2c_dp_aux_add_bus(struct i2c_adapter *adapter)
+{
+	int error;
+	
+	error = i2c_dp_aux_prepare_bus(adapter);
+	if (error)
+		return error;
+	error = i2c_add_adapter(adapter);
+	return error;
+}
+EXPORT_SYMBOL(i2c_dp_aux_add_bus);
diff --git a/drivers/gpu/drm/drm_dp_i2c_helper.c
b/drivers/gpu/drm/drm_dp_i2c_helper.c
deleted file mode 100644
index f7eba0a..0000000
--- a/drivers/gpu/drm/drm_dp_i2c_helper.c
+++ /dev/null
@@ -1,208 +0,0 @@
-/*
- * Copyright ? 2009 Keith Packard
- *
- * Permission to use, copy, modify, distribute, and sell this software and its
- * documentation for any purpose is hereby granted without fee, provided that
- * the above copyright notice appear in all copies and that both that copyright
- * notice and this permission notice appear in supporting documentation, and
- * that the name of the copyright holders not be used in advertising or
- * publicity pertaining to distribution of the software without specific,
- * written prior permission.  The copyright holders make no representations
- * about the suitability of this software for any purpose.  It is provided
"as
- * is" without express or implied warranty.
- *
- * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
- * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
- * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
- * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
- * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
- * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
- * OF THIS SOFTWARE.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/delay.h>
-#include <linux/init.h>
-#include <linux/errno.h>
-#include <linux/sched.h>
-#include <linux/i2c.h>
-#include "drm_dp_helper.h"
-#include "drmP.h"
-
-/* Run a single AUX_CH I2C transaction, writing/reading data as necessary */
-static int
-i2c_algo_dp_aux_transaction(struct i2c_adapter *adapter, int mode,
-			    uint8_t write_byte, uint8_t *read_byte)
-{
-	struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
-	int ret;
-	
-	ret = (*algo_data->aux_ch)(adapter, mode,
-				   write_byte, read_byte);
-	return ret;
-}
-
-/*
- * I2C over AUX CH
- */
-
-/*
- * Send the address. If the I2C link is running, this 'restarts'
- * the connection with the new address, this is used for doing
- * a write followed by a read (as needed for DDC)
- */
-static int
-i2c_algo_dp_aux_address(struct i2c_adapter *adapter, u16 address, bool reading)
-{
-	struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
-	int mode = MODE_I2C_START;
-	int ret;
-
-	if (reading)
-		mode |= MODE_I2C_READ;
-	else
-		mode |= MODE_I2C_WRITE;
-	algo_data->address = address;
-	algo_data->running = true;
-	ret = i2c_algo_dp_aux_transaction(adapter, mode, 0, NULL);
-	return ret;
-}
-
-/*
- * Stop the I2C transaction. This closes out the link, sending
- * a bare address packet with the MOT bit turned off
- */
-static void
-i2c_algo_dp_aux_stop(struct i2c_adapter *adapter, bool reading)
-{
-	struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
-	int mode = MODE_I2C_STOP;
-
-	if (reading)
-		mode |= MODE_I2C_READ;
-	else
-		mode |= MODE_I2C_WRITE;
-	if (algo_data->running) {
-		(void) i2c_algo_dp_aux_transaction(adapter, mode, 0, NULL);
-		algo_data->running = false;
-	}
-}
-
-/*
- * Write a single byte to the current I2C address, the
- * the I2C link must be running or this returns -EIO
- */
-static int
-i2c_algo_dp_aux_put_byte(struct i2c_adapter *adapter, u8 byte)
-{
-	struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
-	int ret;
-
-	if (!algo_data->running)
-		return -EIO;
-
-	ret = i2c_algo_dp_aux_transaction(adapter, MODE_I2C_WRITE, byte, NULL);
-	return ret;
-}
-
-/*
- * Read a single byte from the current I2C address, the
- * I2C link must be running or this returns -EIO
- */
-static int
-i2c_algo_dp_aux_get_byte(struct i2c_adapter *adapter, u8 *byte_ret)
-{
-	struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
-	int ret;
-
-	if (!algo_data->running)
-		return -EIO;
-
-	ret = i2c_algo_dp_aux_transaction(adapter, MODE_I2C_READ, 0, byte_ret);
-	return ret;
-}
-
-static int
-i2c_algo_dp_aux_xfer(struct i2c_adapter *adapter,
-		     struct i2c_msg *msgs,
-		     int num)
-{
-	int ret = 0;
-	bool reading = false;
-	int m;
-	int b;
-
-	for (m = 0; m < num; m++) {
-		u16 len = msgs[m].len;
-		u8 *buf = msgs[m].buf;
-		reading = (msgs[m].flags & I2C_M_RD) != 0;
-		ret = i2c_algo_dp_aux_address(adapter, msgs[m].addr, reading);
-		if (ret < 0)
-			break;
-		if (reading) {
-			for (b = 0; b < len; b++) {
-				ret = i2c_algo_dp_aux_get_byte(adapter, &buf[b]);
-				if (ret < 0)
-					break;
-			}
-		} else {
-			for (b = 0; b < len; b++) {
-				ret = i2c_algo_dp_aux_put_byte(adapter, buf[b]);
-				if (ret < 0)
-					break;
-			}
-		}
-		if (ret < 0)
-			break;
-	}
-	if (ret >= 0)
-		ret = num;
-	i2c_algo_dp_aux_stop(adapter, reading);
-	DRM_DEBUG_KMS("dp_aux_xfer return %d\n", ret);
-	return ret;
-}
-
-static u32
-i2c_algo_dp_aux_functionality(struct i2c_adapter *adapter)
-{
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
-	       I2C_FUNC_SMBUS_READ_BLOCK_DATA |
-	       I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
-	       I2C_FUNC_10BIT_ADDR;
-}
-
-static const struct i2c_algorithm i2c_dp_aux_algo = {
-	.master_xfer	= i2c_algo_dp_aux_xfer,
-	.functionality	= i2c_algo_dp_aux_functionality,
-};
-
-static void
-i2c_dp_aux_reset_bus(struct i2c_adapter *adapter)
-{
-	(void) i2c_algo_dp_aux_address(adapter, 0, false);
-	(void) i2c_algo_dp_aux_stop(adapter, false);
-					   
-}
-
-static int
-i2c_dp_aux_prepare_bus(struct i2c_adapter *adapter)
-{
-	adapter->algo = &i2c_dp_aux_algo;
-	adapter->retries = 3;
-	i2c_dp_aux_reset_bus(adapter);
-	return 0;
-}
-
-int
-i2c_dp_aux_add_bus(struct i2c_adapter *adapter)
-{
-	int error;
-	
-	error = i2c_dp_aux_prepare_bus(adapter);
-	if (error)
-		return error;
-	error = i2c_add_adapter(adapter);
-	return error;
-}
-EXPORT_SYMBOL(i2c_dp_aux_add_bus);
-- 
1.7.11.4

Daniel Vetter

2012-Oct-18 08:15 UTC

head link

[Nouveau] [PATCH 02/10] drm: dp helper: extract drm_dp_channel_eq_ok

radeon and intel use the exact same definition.

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/drm_dp_helper.c      | 50 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c      | 35 ++-----------------------
 drivers/gpu/drm/radeon/atombios_dp.c | 24 ++---------------
 include/drm/drm_dp_helper.h          |  5 ++++
 4 files changed, 59 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index f7eba0a..9dde04a 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -206,3 +206,53 @@ i2c_dp_aux_add_bus(struct i2c_adapter *adapter)
 	return error;
 }
 EXPORT_SYMBOL(i2c_dp_aux_add_bus);
+
+/* Helpers for DP link training */
+static u8 dp_link_status(u8 link_status[DP_LINK_STATUS_SIZE], int r)
+{
+	return link_status[r - DP_LANE0_1_STATUS];
+}
+
+static u8 dp_get_lane_status(u8 link_status[DP_LINK_STATUS_SIZE],
+			     int lane)
+{
+	int i = DP_LANE0_1_STATUS + (lane >> 1);
+	int s = (lane & 1) * 4;
+	u8 l = dp_link_status(link_status, i);
+	return (l >> s) & 0xf;
+}
+
+bool drm_dp_channel_eq_ok(u8 link_status[DP_LINK_STATUS_SIZE],
+			  int lane_count)
+{
+	u8 lane_align;
+	u8 lane_status;
+	int lane;
+
+	lane_align = dp_link_status(link_status,
+				    DP_LANE_ALIGN_STATUS_UPDATED);
+	if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0)
+		return false;
+	for (lane = 0; lane < lane_count; lane++) {
+		lane_status = dp_get_lane_status(link_status, lane);
+		if ((lane_status & DP_CHANNEL_EQ_BITS) != DP_CHANNEL_EQ_BITS)
+			return false;
+	}
+	return true;
+}
+EXPORT_SYMBOL(drm_dp_channel_eq_ok);
+
+bool drm_dp_clock_recovery_ok(u8 link_status[DP_LINK_STATUS_SIZE],
+			      int lane_count)
+{
+	int lane;
+	u8 lane_status;
+
+	for (lane = 0; lane < lane_count; lane++) {
+		lane_status = dp_get_lane_status(link_status, lane);
+		if ((lane_status & DP_LANE_CR_DONE) == 0)
+			return false;
+	}
+	return true;
+}
+EXPORT_SYMBOL(drm_dp_clock_recovery_ok);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f2c9ea6..d3f4db0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,7 +38,6 @@
 #include "i915_drv.h"
 
 #define DP_RECEIVER_CAP_SIZE	0xf
-#define DP_LINK_STATUS_SIZE	6
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
 /**
@@ -1401,13 +1400,6 @@ intel_dp_get_link_status(struct intel_dp *intel_dp,
uint8_t link_status[DP_LINK_
 }
 
 static uint8_t
-intel_dp_link_status(uint8_t link_status[DP_LINK_STATUS_SIZE],
-		     int r)
-{
-	return link_status[r - DP_LANE0_1_STATUS];
-}
-
-static uint8_t
 intel_get_adjust_request_voltage(uint8_t adjust_request[2],
 				 int lane)
 {
@@ -1692,29 +1684,6 @@ intel_clock_recovery_ok(uint8_t
link_status[DP_LINK_STATUS_SIZE], int lane_count
 	return true;
 }
 
-/* Check to see if channel eq is done on all channels */
-#define CHANNEL_EQ_BITS (DP_LANE_CR_DONE|\
-			 DP_LANE_CHANNEL_EQ_DONE|\
-			 DP_LANE_SYMBOL_LOCKED)
-static bool
-intel_channel_eq_ok(struct intel_dp *intel_dp, uint8_t
link_status[DP_LINK_STATUS_SIZE])
-{
-	uint8_t lane_align;
-	uint8_t lane_status;
-	int lane;
-
-	lane_align = intel_dp_link_status(link_status,
-					  DP_LANE_ALIGN_STATUS_UPDATED);
-	if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0)
-		return false;
-	for (lane = 0; lane < intel_dp->lane_count; lane++) {
-		lane_status = intel_get_lane_status(link_status, lane);
-		if ((lane_status & CHANNEL_EQ_BITS) != CHANNEL_EQ_BITS)
-			return false;
-	}
-	return true;
-}
-
 static bool
 intel_dp_set_link_train(struct intel_dp *intel_dp,
 			uint32_t dp_reg_value,
@@ -1969,7 +1938,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 			continue;
 		}
 
-		if (intel_channel_eq_ok(intel_dp, link_status)) {
+		if (drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
 			channel_eq = true;
 			break;
 		}
@@ -2170,7 +2139,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
 
-	if (!intel_channel_eq_ok(intel_dp, link_status)) {
+	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
 		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
 			      drm_get_encoder_name(&intel_dp->base.base));
 		intel_dp_start_link_train(intel_dp);
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
b/drivers/gpu/drm/radeon/atombios_dp.c
index d48224b..8aa8187 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -34,7 +34,6 @@
 
 /* move these to drm_dp_helper.c/h */
 #define DP_LINK_CONFIGURATION_SIZE 9
-#define DP_LINK_STATUS_SIZE	   6
 #define DP_DPCD_SIZE	           8
 
 static char *voltage_names[] = {
@@ -318,25 +317,6 @@ static bool dp_clock_recovery_ok(u8
link_status[DP_LINK_STATUS_SIZE],
 	return true;
 }
 
-static bool dp_channel_eq_ok(u8 link_status[DP_LINK_STATUS_SIZE],
-			     int lane_count)
-{
-	u8 lane_align;
-	u8 lane_status;
-	int lane;
-
-	lane_align = dp_link_status(link_status,
-				    DP_LANE_ALIGN_STATUS_UPDATED);
-	if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0)
-		return false;
-	for (lane = 0; lane < lane_count; lane++) {
-		lane_status = dp_get_lane_status(link_status, lane);
-		if ((lane_status & DP_CHANNEL_EQ_BITS) != DP_CHANNEL_EQ_BITS)
-			return false;
-	}
-	return true;
-}
-
 static u8 dp_get_adjust_request_voltage(u8 link_status[DP_LINK_STATUS_SIZE],
 					int lane)
 
@@ -664,7 +644,7 @@ bool radeon_dp_needs_link_train(struct radeon_connector
*radeon_connector)
 
 	if (!radeon_dp_get_link_status(radeon_connector, link_status))
 		return false;
-	if (dp_channel_eq_ok(link_status, dig->dp_lane_count))
+	if (drm_dp_channel_eq_ok(link_status, dig->dp_lane_count))
 		return false;
 	return true;
 }
@@ -896,7 +876,7 @@ static int radeon_dp_link_train_ce(struct
radeon_dp_link_train_info *dp_info)
 			break;
 		}
 
-		if (dp_channel_eq_ok(dp_info->link_status, dp_info->dp_lane_count)) {
+		if (drm_dp_channel_eq_ok(dp_info->link_status, dp_info->dp_lane_count))
{
 			channel_eq = true;
 			break;
 		}
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index fe06148..9e10420 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -322,4 +322,9 @@ struct i2c_algo_dp_aux_data {
 int
 i2c_dp_aux_add_bus(struct i2c_adapter *adapter);
 
+
+#define DP_LINK_STATUS_SIZE	   6
+bool drm_dp_channel_eq_ok(u8 link_status[DP_LINK_STATUS_SIZE],
+			  int lane_count);
+
 #endif /* _DRM_DP_HELPER_H_ */
-- 
1.7.11.4

Daniel Vetter

2012-Oct-18 08:15 UTC

head link

[Nouveau] [PATCH 03/10] drm: dp helper: extract drm_dp_clock_recovery_ok

radeon and intel use the exact same definition.

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c      |  4 ++--
 drivers/gpu/drm/radeon/atombios_dp.c | 25 +------------------------
 include/drm/drm_dp_helper.h          |  2 ++
 3 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d3f4db0..5a9ab3a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1845,7 +1845,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 			break;
 		}
 
-		if (intel_clock_recovery_ok(link_status, intel_dp->lane_count)) {
+		if (drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
 			DRM_DEBUG_KMS("clock recovery OK\n");
 			clock_recovery = true;
 			break;
@@ -1932,7 +1932,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 			break;
 
 		/* Make sure clock is still ok */
-		if (!intel_clock_recovery_ok(link_status, intel_dp->lane_count)) {
+		if (!drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
 			intel_dp_start_link_train(intel_dp);
 			cr_tries++;
 			continue;
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
b/drivers/gpu/drm/radeon/atombios_dp.c
index 8aa8187..75fdf34 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -294,29 +294,6 @@ static u8 dp_link_status(u8
link_status[DP_LINK_STATUS_SIZE], int r)
 	return link_status[r - DP_LANE0_1_STATUS];
 }
 
-static u8 dp_get_lane_status(u8 link_status[DP_LINK_STATUS_SIZE],
-			     int lane)
-{
-	int i = DP_LANE0_1_STATUS + (lane >> 1);
-	int s = (lane & 1) * 4;
-	u8 l = dp_link_status(link_status, i);
-	return (l >> s) & 0xf;
-}
-
-static bool dp_clock_recovery_ok(u8 link_status[DP_LINK_STATUS_SIZE],
-				 int lane_count)
-{
-	int lane;
-	u8 lane_status;
-
-	for (lane = 0; lane < lane_count; lane++) {
-		lane_status = dp_get_lane_status(link_status, lane);
-		if ((lane_status & DP_LANE_CR_DONE) == 0)
-			return false;
-	}
-	return true;
-}
-
 static u8 dp_get_adjust_request_voltage(u8 link_status[DP_LINK_STATUS_SIZE],
 					int lane)
 
@@ -811,7 +788,7 @@ static int radeon_dp_link_train_cr(struct
radeon_dp_link_train_info *dp_info)
 			break;
 		}
 
-		if (dp_clock_recovery_ok(dp_info->link_status, dp_info->dp_lane_count))
{
+		if (drm_dp_clock_recovery_ok(dp_info->link_status,
dp_info->dp_lane_count)) {
 			clock_recovery = true;
 			break;
 		}
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 9e10420..89e92c9 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -326,5 +326,7 @@ i2c_dp_aux_add_bus(struct i2c_adapter *adapter);
 #define DP_LINK_STATUS_SIZE	   6
 bool drm_dp_channel_eq_ok(u8 link_status[DP_LINK_STATUS_SIZE],
 			  int lane_count);
+bool drm_dp_clock_recovery_ok(u8 link_status[DP_LINK_STATUS_SIZE],
+			      int lane_count);
 
 #endif /* _DRM_DP_HELPER_H_ */
-- 
1.7.11.4

Daniel Vetter

2012-Oct-18 08:15 UTC

head link

[Nouveau] [PATCH 04/10] drm/nouveau: use the cr_ok/chanel_eq_ok helpers

Only compile-tested, due to a lack of nouveau dp hw. Should be
equivalent code though.

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/nouveau/nouveau_dp.c | 26 +++++---------------------
 1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c
b/drivers/gpu/drm/nouveau/nouveau_dp.c
index e754aa3..60d561e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -298,7 +298,7 @@ dp_link_train_cr(struct drm_device *dev, struct dp_state
*dp)
 {
 	bool cr_done = false, abort = false;
 	int voltage = dp->conf[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
-	int tries = 0, i;
+	int tries = 0;
 
 	dp_set_training_pattern(dev, dp, DP_TRAINING_PATTERN_1);
 
@@ -307,16 +307,7 @@ dp_link_train_cr(struct drm_device *dev, struct dp_state
*dp)
 		    dp_link_train_update(dev, dp, 100))
 			break;
 
-		cr_done = true;
-		for (i = 0; i < dp->link_nr; i++) {
-			u8 lane = (dp->stat[i >> 1] >> ((i & 1) * 4)) & 0xf;
-			if (!(lane & DP_LANE_CR_DONE)) {
-				cr_done = false;
-				if (dp->conf[i] & DP_TRAIN_MAX_SWING_REACHED)
-					abort = true;
-				break;
-			}
-		}
+		cr_done = drm_dp_clock_recovery_ok(dp->stat, dp->link_nr);
 
 		if ((dp->conf[0] & DP_TRAIN_VOLTAGE_SWING_MASK) != voltage) {
 			voltage = dp->conf[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
@@ -331,7 +322,7 @@ static int
 dp_link_train_eq(struct drm_device *dev, struct dp_state *dp)
 {
 	bool eq_done, cr_done = true;
-	int tries = 0, i;
+	int tries = 0;
 
 	dp_set_training_pattern(dev, dp, DP_TRAINING_PATTERN_2);
 
@@ -339,15 +330,8 @@ dp_link_train_eq(struct drm_device *dev, struct dp_state
*dp)
 		if (dp_link_train_update(dev, dp, 400))
 			break;
 
-		eq_done = !!(dp->stat[2] & DP_INTERLANE_ALIGN_DONE);
-		for (i = 0; i < dp->link_nr && eq_done; i++) {
-			u8 lane = (dp->stat[i >> 1] >> ((i & 1) * 4)) & 0xf;
-			if (!(lane & DP_LANE_CR_DONE))
-				cr_done = false;
-			if (!(lane & DP_LANE_CHANNEL_EQ_DONE) ||
-			    !(lane & DP_LANE_SYMBOL_LOCKED))
-				eq_done = false;
-		}
+		eq_done = drm_dp_channel_eq_ok(dp->stat, dp->link_nr);
+		cr_done = drm_dp_clock_recovery_ok(dp->stat, dp->link_nr);
 
 		if (dp_link_train_commit(dev, dp))
 			break;
-- 
1.7.11.4

Daniel Vetter

2012-Oct-18 08:15 UTC

head link

[Nouveau] [PATCH 05/10] drm: extract helpers to compute new training values from sink request

Safe for the minor difference that the intel versions get an offset
into the link_status as an argument, both are the same again.

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/drm_dp_helper.c      | 27 +++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c      | 30 ++----------------------------
 drivers/gpu/drm/radeon/atombios_dp.c | 34 ++--------------------------------
 include/drm/drm_dp_helper.h          |  4 ++++
 4 files changed, 35 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 9dde04a..d1a196f 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -256,3 +256,30 @@ bool drm_dp_clock_recovery_ok(u8
link_status[DP_LINK_STATUS_SIZE],
 	return true;
 }
 EXPORT_SYMBOL(drm_dp_clock_recovery_ok);
+
+u8 drm_dp_get_adjust_request_voltage(u8 link_status[DP_LINK_STATUS_SIZE],
+				     int lane)
+{
+	int i = DP_ADJUST_REQUEST_LANE0_1 + (lane >> 1);
+	int s = ((lane & 1) ?
+		 DP_ADJUST_VOLTAGE_SWING_LANE1_SHIFT :
+		 DP_ADJUST_VOLTAGE_SWING_LANE0_SHIFT);
+	u8 l = dp_link_status(link_status, i);
+
+	return ((l >> s) & 0x3) << DP_TRAIN_VOLTAGE_SWING_SHIFT;
+}
+EXPORT_SYMBOL(drm_dp_get_adjust_request_voltage);
+
+u8 drm_dp_get_adjust_request_pre_emphasis(u8 link_status[DP_LINK_STATUS_SIZE],
+					  int lane)
+{
+	int i = DP_ADJUST_REQUEST_LANE0_1 + (lane >> 1);
+	int s = ((lane & 1) ?
+		 DP_ADJUST_PRE_EMPHASIS_LANE1_SHIFT :
+		 DP_ADJUST_PRE_EMPHASIS_LANE0_SHIFT);
+	u8 l = dp_link_status(link_status, i);
+
+	return ((l >> s) & 0x3) << DP_TRAIN_PRE_EMPHASIS_SHIFT;
+}
+EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
+
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5a9ab3a..4cd957a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1399,31 +1399,6 @@ intel_dp_get_link_status(struct intel_dp *intel_dp,
uint8_t link_status[DP_LINK_
 					      DP_LINK_STATUS_SIZE);
 }
 
-static uint8_t
-intel_get_adjust_request_voltage(uint8_t adjust_request[2],
-				 int lane)
-{
-	int	    s = ((lane & 1) ?
-			 DP_ADJUST_VOLTAGE_SWING_LANE1_SHIFT :
-			 DP_ADJUST_VOLTAGE_SWING_LANE0_SHIFT);
-	uint8_t l = adjust_request[lane>>1];
-
-	return ((l >> s) & 3) << DP_TRAIN_VOLTAGE_SWING_SHIFT;
-}
-
-static uint8_t
-intel_get_adjust_request_pre_emphasis(uint8_t adjust_request[2],
-				      int lane)
-{
-	int	    s = ((lane & 1) ?
-			 DP_ADJUST_PRE_EMPHASIS_LANE1_SHIFT :
-			 DP_ADJUST_PRE_EMPHASIS_LANE0_SHIFT);
-	uint8_t l = adjust_request[lane>>1];
-
-	return ((l >> s) & 3) << DP_TRAIN_PRE_EMPHASIS_SHIFT;
-}
-
-
 #if 0
 static char	*voltage_names[] = {
 	"0.4V", "0.6V", "0.8V", "1.2V"
@@ -1502,13 +1477,12 @@ intel_get_adjust_train(struct intel_dp *intel_dp,
uint8_t link_status[DP_LINK_ST
 	uint8_t v = 0;
 	uint8_t p = 0;
 	int lane;
-	uint8_t	*adjust_request = link_status + (DP_ADJUST_REQUEST_LANE0_1 -
DP_LANE0_1_STATUS);
 	uint8_t voltage_max;
 	uint8_t preemph_max;
 
 	for (lane = 0; lane < intel_dp->lane_count; lane++) {
-		uint8_t this_v = intel_get_adjust_request_voltage(adjust_request, lane);
-		uint8_t this_p = intel_get_adjust_request_pre_emphasis(adjust_request, lane);
+		uint8_t this_v = drm_dp_get_adjust_request_voltage(link_status, lane);
+		uint8_t this_p = drm_dp_get_adjust_request_pre_emphasis(link_status, lane);
 
 		if (this_v > v)
 			v = this_v;
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
b/drivers/gpu/drm/radeon/atombios_dp.c
index 75fdf34..5479832 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -289,36 +289,6 @@ int radeon_dp_i2c_aux_ch(struct i2c_adapter *adapter, int
mode,
 
 /***** general DP utility functions *****/
 
-static u8 dp_link_status(u8 link_status[DP_LINK_STATUS_SIZE], int r)
-{
-	return link_status[r - DP_LANE0_1_STATUS];
-}
-
-static u8 dp_get_adjust_request_voltage(u8 link_status[DP_LINK_STATUS_SIZE],
-					int lane)
-
-{
-	int i = DP_ADJUST_REQUEST_LANE0_1 + (lane >> 1);
-	int s = ((lane & 1) ?
-		 DP_ADJUST_VOLTAGE_SWING_LANE1_SHIFT :
-		 DP_ADJUST_VOLTAGE_SWING_LANE0_SHIFT);
-	u8 l = dp_link_status(link_status, i);
-
-	return ((l >> s) & 0x3) << DP_TRAIN_VOLTAGE_SWING_SHIFT;
-}
-
-static u8 dp_get_adjust_request_pre_emphasis(u8
link_status[DP_LINK_STATUS_SIZE],
-					     int lane)
-{
-	int i = DP_ADJUST_REQUEST_LANE0_1 + (lane >> 1);
-	int s = ((lane & 1) ?
-		 DP_ADJUST_PRE_EMPHASIS_LANE1_SHIFT :
-		 DP_ADJUST_PRE_EMPHASIS_LANE0_SHIFT);
-	u8 l = dp_link_status(link_status, i);
-
-	return ((l >> s) & 0x3) << DP_TRAIN_PRE_EMPHASIS_SHIFT;
-}
-
 #define DP_VOLTAGE_MAX         DP_TRAIN_VOLTAGE_SWING_1200
 #define DP_PRE_EMPHASIS_MAX    DP_TRAIN_PRE_EMPHASIS_9_5
 
@@ -331,8 +301,8 @@ static void dp_get_adjust_train(u8
link_status[DP_LINK_STATUS_SIZE],
 	int lane;
 
 	for (lane = 0; lane < lane_count; lane++) {
-		u8 this_v = dp_get_adjust_request_voltage(link_status, lane);
-		u8 this_p = dp_get_adjust_request_pre_emphasis(link_status, lane);
+		u8 this_v = drm_dp_get_adjust_request_voltage(link_status, lane);
+		u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status, lane);
 
 		DRM_DEBUG_KMS("requested signal parameters: lane %d voltage %s pre_emph
%s\n",
 			  lane,
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 89e92c9..57e6dbd 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -328,5 +328,9 @@ bool drm_dp_channel_eq_ok(u8
link_status[DP_LINK_STATUS_SIZE],
 			  int lane_count);
 bool drm_dp_clock_recovery_ok(u8 link_status[DP_LINK_STATUS_SIZE],
 			      int lane_count);
+u8 drm_dp_get_adjust_request_voltage(u8 link_status[DP_LINK_STATUS_SIZE],
+				     int lane);
+u8 drm_dp_get_adjust_request_pre_emphasis(u8 link_status[DP_LINK_STATUS_SIZE],
+					  int lane);
 
 #endif /* _DRM_DP_HELPER_H_ */
-- 
1.7.11.4

Daniel Vetter

2012-Oct-18 08:15 UTC

head link

[Nouveau] [PATCH 06/10] drm/nouveau: use dp link train request helper

nouveau again score with an impressive density of magic numbers.

Again only compile-tested due to lack of hw, but should be equivalent
code.

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/nouveau/nouveau_dp.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c
b/drivers/gpu/drm/nouveau/nouveau_dp.c
index 60d561e..d46a8ff 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -261,11 +261,10 @@ dp_link_train_commit(struct drm_device *dev, struct
dp_state *dp)
 	int i;
 
 	for (i = 0; i < dp->link_nr; i++) {
-		u8 lane = (dp->stat[4 + (i >> 1)] >> ((i & 1) * 4)) &
0xf;
-		u8 lpre = (lane & 0x0c) >> 2;
-		u8 lvsw = (lane & 0x03) >> 0;
+		u8 lpre = drm_dp_get_adjust_request_pre_emphasis(dp->stat, i);
+		u8 lvsw = drm_dp_get_adjust_request_voltage(dp->stat, i);
 
-		dp->conf[i] = (lpre << 3) | lvsw;
+		dp->conf[i] = lpre | lvsw;
 		if (lvsw == DP_TRAIN_VOLTAGE_SWING_1200)
 			dp->conf[i] |= DP_TRAIN_MAX_SWING_REACHED;
 		if ((lpre << 3) == DP_TRAIN_PRE_EMPHASIS_9_5)
-- 
1.7.11.4

Daniel Vetter

2012-Oct-18 08:15 UTC

head link

[Nouveau] [PATCH 07/10] drm: extract dp link train delay functions from radeon

This requires a few changes since that dpcd value is above the
range currently cached by radeon. I've check the dp specs, and
above 0xf there's a big gap and nothing that looks like we should
cache it while a given device is plugged in. It's also the same value
that i915.ko uses.

Hence extend the various dpcd arrays in the radeon driver, use
proper symbolic constants where applicable (one place overallocated
the dpcd array to 25 bytes). Then also drop the rd_interval cache -
radeon_dp_link_train_init re-reads the dpcd block, so the values we'll
consume in train_cr and train_ce will always be fresh.

To avoid needless diff-churn, #define the old size of dpcd as the new
one and keep it around.

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/drm_dp_helper.c      | 15 +++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c      |  1 -
 drivers/gpu/drm/radeon/atombios_dp.c | 25 +++++++++----------------
 drivers/gpu/drm/radeon/radeon_mode.h |  2 +-
 include/drm/drm_dp_helper.h          |  5 +++++
 5 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index d1a196f..e43ddde 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -283,3 +283,18 @@ u8 drm_dp_get_adjust_request_pre_emphasis(u8
link_status[DP_LINK_STATUS_SIZE],
 }
 EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
 
+void drm_dp_link_train_clock_recovery_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
+	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
+		udelay(100);
+	else
+		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
+}
+EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
+
+void drm_dp_link_train_channel_eq_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
+	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
+		udelay(400);
+	else
+		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
+}
+EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4cd957a..aa1a28c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -37,7 +37,6 @@
 #include "i915_drm.h"
 #include "i915_drv.h"
 
-#define DP_RECEIVER_CAP_SIZE	0xf
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
 /**
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
b/drivers/gpu/drm/radeon/atombios_dp.c
index 5479832..4551ea5 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -34,7 +34,7 @@
 
 /* move these to drm_dp_helper.c/h */
 #define DP_LINK_CONFIGURATION_SIZE 9
-#define DP_DPCD_SIZE	           8
+#define DP_DPCD_SIZE DP_RECEIVER_CAP_SIZE
 
 static char *voltage_names[] = {
         "0.4V", "0.6V", "0.8V", "1.2V"
@@ -478,14 +478,15 @@ static void radeon_dp_probe_oui(struct radeon_connector
*radeon_connector)
 bool radeon_dp_getdpcd(struct radeon_connector *radeon_connector)
 {
 	struct radeon_connector_atom_dig *dig_connector =
radeon_connector->con_priv;
-	u8 msg[25];
+	u8 msg[DP_DPCD_SIZE];
 	int ret, i;
 
-	ret = radeon_dp_aux_native_read(radeon_connector, DP_DPCD_REV, msg, 8, 0);
+	ret = radeon_dp_aux_native_read(radeon_connector, DP_DPCD_REV, msg,
+					DP_DPCD_SIZE, 0);
 	if (ret > 0) {
-		memcpy(dig_connector->dpcd, msg, 8);
+		memcpy(dig_connector->dpcd, msg, DP_DPCD_SIZE);
 		DRM_DEBUG_KMS("DPCD: ");
-		for (i = 0; i < 8; i++)
+		for (i = 0; i < DP_DPCD_SIZE; i++)
 			DRM_DEBUG_KMS("%02x ", msg[i]);
 		DRM_DEBUG_KMS("\n");
 
@@ -604,9 +605,8 @@ struct radeon_dp_link_train_info {
 	int enc_id;
 	int dp_clock;
 	int dp_lane_count;
-	int rd_interval;
 	bool tp3_supported;
-	u8 dpcd[8];
+	u8 dpcd[DP_RECEIVER_CAP_SIZE];
 	u8 train_set[4];
 	u8 link_status[DP_LINK_STATUS_SIZE];
 	u8 tries;
@@ -748,10 +748,7 @@ static int radeon_dp_link_train_cr(struct
radeon_dp_link_train_info *dp_info)
 	dp_info->tries = 0;
 	voltage = 0xff;
 	while (1) {
-		if (dp_info->rd_interval == 0)
-			udelay(100);
-		else
-			mdelay(dp_info->rd_interval * 4);
+		drm_dp_link_train_clock_recovery_delay(dp_info->dpcd);
 
 		if (!radeon_dp_get_link_status(dp_info->radeon_connector,
dp_info->link_status)) {
 			DRM_ERROR("displayport link status failed\n");
@@ -813,10 +810,7 @@ static int radeon_dp_link_train_ce(struct
radeon_dp_link_train_info *dp_info)
 	dp_info->tries = 0;
 	channel_eq = false;
 	while (1) {
-		if (dp_info->rd_interval == 0)
-			udelay(400);
-		else
-			mdelay(dp_info->rd_interval * 4);
+		drm_dp_link_train_channel_eq_delay(dp_info->dpcd);
 
 		if (!radeon_dp_get_link_status(dp_info->radeon_connector,
dp_info->link_status)) {
 			DRM_ERROR("displayport link status failed\n");
@@ -901,7 +895,6 @@ void radeon_dp_link_train(struct drm_encoder *encoder,
 	else
 		dp_info.enc_id |= ATOM_DP_CONFIG_LINK_A;
 
-	dp_info.rd_interval = radeon_read_dpcd_reg(radeon_connector,
DP_TRAINING_AUX_RD_INTERVAL);
 	tmp = radeon_read_dpcd_reg(radeon_connector, DP_MAX_LANE_COUNT);
 	if (ASIC_IS_DCE5(rdev) && (tmp & DP_TPS3_SUPPORTED))
 		dp_info.tp3_supported = true;
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h
b/drivers/gpu/drm/radeon/radeon_mode.h
index d569789..e5b668e 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -403,7 +403,7 @@ struct radeon_connector_atom_dig {
 	uint32_t igp_lane_info;
 	/* displayport */
 	struct radeon_i2c_chan *dp_i2c_bus;
-	u8 dpcd[8];
+	u8 dpcd[DP_RECEIVER_CAP_SIZE];
 	u8 dp_sink_type;
 	int dp_clock;
 	int dp_lane_count;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 57e6dbd..60bd8d3 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -25,6 +25,7 @@
 
 #include <linux/types.h>
 #include <linux/i2c.h>
+#include <linux/delay.h>
 
 /*
  * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
@@ -333,4 +334,8 @@ u8 drm_dp_get_adjust_request_voltage(u8
link_status[DP_LINK_STATUS_SIZE],
 u8 drm_dp_get_adjust_request_pre_emphasis(u8 link_status[DP_LINK_STATUS_SIZE],
 					  int lane);
 
+#define DP_RECEIVER_CAP_SIZE	0xf
+void drm_dp_link_train_clock_recovery_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
+void drm_dp_link_train_channel_eq_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
+
 #endif /* _DRM_DP_HELPER_H_ */
-- 
1.7.11.4

Daniel Vetter

2012-Oct-18 08:15 UTC

head link

[Nouveau] [PATCH 08/10] drm/i915: use the new dp train delay helpers

Only really required for dp 1.2. I've hoped this would help with some
link training woes I'm fighting, but alas those are only dp 1.1
devices.

Also move a comment that went misplaced in the recent refactorings to
the right spot again.

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index aa1a28c..865f7f3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1806,13 +1806,13 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 		DRM_DEBUG_KMS("training pattern 1 signal levels %08x\n",
 			      signal_levels);
 
+		/* Set training pattern 1 */
 		if (!intel_dp_set_link_train(intel_dp, DP,
 					     DP_TRAINING_PATTERN_1 |
 					     DP_LINK_SCRAMBLING_DISABLE))
 			break;
-		/* Set training pattern 1 */
 
-		udelay(100);
+		drm_dp_link_train_clock_recovery_delay(intel_dp->dpcd);
 		if (!intel_dp_get_link_status(intel_dp, link_status)) {
 			DRM_ERROR("failed to get link status\n");
 			break;
@@ -1900,7 +1900,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 					     DP_LINK_SCRAMBLING_DISABLE))
 			break;
 
-		udelay(400);
+		drm_dp_link_train_channel_eq_delay(intel_dp->dpcd);
 		if (!intel_dp_get_link_status(intel_dp, link_status))
 			break;
 
-- 
1.7.11.4

Daniel Vetter

2012-Oct-18 08:15 UTC

head link

[Nouveau] [PATCH 09/10] drm: extract dp link bw helpers

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/drm_dp_helper.c      | 28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c      |  5 +----
 drivers/gpu/drm/radeon/atombios_dp.c | 32 +++-----------------------------
 include/drm/drm_dp_helper.h          |  8 ++++++++
 4 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index e43ddde..d8a4189 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -298,3 +298,31 @@ void drm_dp_link_train_channel_eq_delay(u8
dpcd[DP_RECEIVER_CAP_SIZE]) {
 		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
 }
 EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
+
+u8 drm_dp_link_rate_to_bw_code(int link_rate)
+{
+	switch (link_rate) {
+	case 162000:
+	default:
+		return DP_LINK_BW_1_62;
+	case 270000:
+		return DP_LINK_BW_2_7;
+	case 540000:
+		return DP_LINK_BW_5_4;
+	}
+}
+EXPORT_SYMBOL(drm_dp_link_rate_to_bw_code);
+
+int drm_dp_bw_code_to_link_rate(u8 link_bw)
+{
+	switch (link_bw) {
+	case DP_LINK_BW_1_62:
+	default:
+		return 162000;
+	case DP_LINK_BW_2_7:
+		return 270000;
+	case DP_LINK_BW_5_4:
+		return 540000;
+	}
+}
+EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 865f7f3..fea768d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -111,10 +111,7 @@ intel_edp_link_config(struct intel_encoder *intel_encoder,
 	struct intel_dp *intel_dp = container_of(intel_encoder, struct intel_dp,
base);
 
 	*lane_num = intel_dp->lane_count;
-	if (intel_dp->link_bw == DP_LINK_BW_1_62)
-		*link_bw = 162000;
-	else if (intel_dp->link_bw == DP_LINK_BW_2_7)
-		*link_bw = 270000;
+	*link_bw = drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
 }
 
 int
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
b/drivers/gpu/drm/radeon/atombios_dp.c
index 4551ea5..aebd4d3 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -347,37 +347,11 @@ static int dp_get_max_dp_pix_clock(int link_rate,
 	return (link_rate * lane_num * 8) / bpp;
 }
 
-static int dp_get_max_link_rate(u8 dpcd[DP_DPCD_SIZE])
-{
-	switch (dpcd[DP_MAX_LINK_RATE]) {
-	case DP_LINK_BW_1_62:
-	default:
-		return 162000;
-	case DP_LINK_BW_2_7:
-		return 270000;
-	case DP_LINK_BW_5_4:
-		return 540000;
-	}
-}
-
 static u8 dp_get_max_lane_number(u8 dpcd[DP_DPCD_SIZE])
 {
 	return dpcd[DP_MAX_LANE_COUNT] & DP_MAX_LANE_COUNT_MASK;
 }
 
-static u8 dp_get_dp_link_rate_coded(int link_rate)
-{
-	switch (link_rate) {
-	case 162000:
-	default:
-		return DP_LINK_BW_1_62;
-	case 270000:
-		return DP_LINK_BW_2_7;
-	case 540000:
-		return DP_LINK_BW_5_4;
-	}
-}
-
 /***** radeon specific DP functions *****/
 
 /* First get the min lane# when low rate is used according to pixel clock
@@ -389,7 +363,7 @@ static int radeon_dp_get_dp_lane_number(struct drm_connector
*connector,
 					int pix_clock)
 {
 	int bpp = convert_bpc_to_bpp(radeon_get_monitor_bpc(connector));
-	int max_link_rate = dp_get_max_link_rate(dpcd);
+	int max_link_rate = drm_dp_max_link_rate(dpcd);
 	int max_lane_num = dp_get_max_lane_number(dpcd);
 	int lane_num;
 	int max_dp_pix_clock;
@@ -427,7 +401,7 @@ static int radeon_dp_get_dp_link_clock(struct drm_connector
*connector,
 			return 540000;
 	}
 
-	return dp_get_max_link_rate(dpcd);
+	return drm_dp_max_link_rate(dpcd);
 }
 
 static u8 radeon_dp_encoder_service(struct radeon_device *rdev,
@@ -692,7 +666,7 @@ static int radeon_dp_link_train_init(struct
radeon_dp_link_train_info *dp_info)
 	radeon_write_dpcd_reg(dp_info->radeon_connector, DP_LANE_COUNT_SET, tmp);
 
 	/* set the link rate on the sink */
-	tmp = dp_get_dp_link_rate_coded(dp_info->dp_clock);
+	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);
 
 	/* start training on the source */
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 60bd8d3..455f8e0 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -338,4 +338,12 @@ u8 drm_dp_get_adjust_request_pre_emphasis(u8
link_status[DP_LINK_STATUS_SIZE],
 void drm_dp_link_train_clock_recovery_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
 void drm_dp_link_train_channel_eq_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
 
+u8 drm_dp_link_rate_to_bw_code(int link_rate);
+int drm_dp_bw_code_to_link_rate(u8 link_bw);
+
+static inline int
+drm_dp_max_link_rate(u8 dpcd[DP_RECEIVER_CAP_SIZE])
+{
+	return drm_dp_bw_code_to_link_rate(dpcd[DP_MAX_LINK_RATE]);
+}
 #endif /* _DRM_DP_HELPER_H_ */
-- 
1.7.11.4

Daniel Vetter

2012-Oct-18 08:15 UTC

head link

[Nouveau] [PATCH 10/10] drm: extract drm_dp_max_lane_count helper

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c      | 17 ++---------------
 drivers/gpu/drm/nouveau/nouveau_dp.c |  2 +-
 drivers/gpu/drm/radeon/atombios_dp.c |  7 +------
 include/drm/drm_dp_helper.h          |  7 +++++++
 4 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fea768d..72fbd6c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -127,19 +127,6 @@ intel_edp_target_clock(struct intel_encoder *intel_encoder,
 }
 
 static int
-intel_dp_max_lane_count(struct intel_dp *intel_dp)
-{
-	int max_lane_count = intel_dp->dpcd[DP_MAX_LANE_COUNT] & 0x1f;
-	switch (max_lane_count) {
-	case 1: case 2: case 4:
-		break;
-	default:
-		max_lane_count = 4;
-	}
-	return max_lane_count;
-}
-
-static int
 intel_dp_max_link_bw(struct intel_dp *intel_dp)
 {
 	int max_link_bw = intel_dp->dpcd[DP_MAX_LINK_RATE];
@@ -199,7 +186,7 @@ intel_dp_adjust_dithering(struct intel_dp *intel_dp,
 			  bool adjust_mode)
 {
 	int max_link_clock = intel_dp_link_clock(intel_dp_max_link_bw(intel_dp));
-	int max_lanes = intel_dp_max_lane_count(intel_dp);
+	int max_lanes = drm_dp_max_lane_count(intel_dp->dpcd);
 	int max_rate, mode_rate;
 
 	mode_rate = intel_dp_link_required(mode->clock, 24);
@@ -675,7 +662,7 @@ intel_dp_mode_fixup(struct drm_encoder *encoder,
 	struct drm_device *dev = encoder->dev;
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 	int lane_count, clock;
-	int max_lane_count = intel_dp_max_lane_count(intel_dp);
+	int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
 	int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
 	int bpp, mode_rate;
 	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c
b/drivers/gpu/drm/nouveau/nouveau_dp.c
index d46a8ff..2786594 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -528,7 +528,7 @@ nouveau_dp_detect(struct drm_encoder *encoder)
 		return false;
 
 	nv_encoder->dp.link_bw = 27000 * dpcd[1];
-	nv_encoder->dp.link_nr = dpcd[2] & DP_MAX_LANE_COUNT_MASK;
+	nv_encoder->dp.link_nr = drm_dp_max_lane_count(dpcd);
 
 	NV_DEBUG_KMS(dev, "display: %dx%d dpcd 0x%02x\n",
 		     nv_encoder->dp.link_nr, nv_encoder->dp.link_bw, dpcd[0]);
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
b/drivers/gpu/drm/radeon/atombios_dp.c
index aebd4d3..d808cb4 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -347,11 +347,6 @@ static int dp_get_max_dp_pix_clock(int link_rate,
 	return (link_rate * lane_num * 8) / bpp;
 }
 
-static u8 dp_get_max_lane_number(u8 dpcd[DP_DPCD_SIZE])
-{
-	return dpcd[DP_MAX_LANE_COUNT] & DP_MAX_LANE_COUNT_MASK;
-}
-
 /***** radeon specific DP functions *****/
 
 /* First get the min lane# when low rate is used according to pixel clock
@@ -364,7 +359,7 @@ static int radeon_dp_get_dp_lane_number(struct drm_connector
*connector,
 {
 	int bpp = convert_bpc_to_bpp(radeon_get_monitor_bpc(connector));
 	int max_link_rate = drm_dp_max_link_rate(dpcd);
-	int max_lane_num = dp_get_max_lane_number(dpcd);
+	int max_lane_num = drm_dp_max_lane_count(dpcd);
 	int lane_num;
 	int max_dp_pix_clock;
 
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 455f8e0..c09d367 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -346,4 +346,11 @@ drm_dp_max_link_rate(u8 dpcd[DP_RECEIVER_CAP_SIZE])
 {
 	return drm_dp_bw_code_to_link_rate(dpcd[DP_MAX_LINK_RATE]);
 }
+
+static inline u8
+drm_dp_max_lane_count(u8 dpcd[DP_RECEIVER_CAP_SIZE])
+{
+	return dpcd[DP_MAX_LANE_COUNT] & DP_MAX_LANE_COUNT_MASK;
+}
+
 #endif /* _DRM_DP_HELPER_H_ */
-- 
1.7.11.4

Alex Deucher

2012-Oct-18 13:30 UTC

head link

[Nouveau] [PATCH 00/10] extract dp helper functions

On Thu, Oct 18, 2012 at 4:15 AM, Daniel Vetter <daniel.vetter at ffwll.ch>
wrote:> Hi all,
>
> I've frustrated myself the last few days yelling at our link training
code.
> Comparing the i915 code to radeon and nouveau I've noticed the lack of
a nice
> set of dp helper functions. So I've started to extract a few.
>
> There's lots more that we can do I think (link configuration selection,
the i2c
> over aux retry stuff which diverges already between i915 and radeon, maybe
more
> higher level parts of the training sequence). But there the drivers diverge
> quite a bit (e.g. the link configuration is driver by different things in
each
> driver: coded link bw from the dp spec, link clock or required bw vs
avialable
> bw), so that's more work and probably best done when reworking these
functions
> for other reasons.
>
> I've also tried to put the new helpers a bit to use in nouveau, but due
to lack
> of hw that part is untested.
>
> Comments and testing highly welcome.
Looks good to me.  Other than the minor change required in patch7,
this series is:

Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
>
> Yours, Daniel
>
> Daniel Vetter (10):
>   drm: rename drm_dp_i2c_helper.c to drm_dp_helper.c
>   drm: dp helper: extract drm_dp_channel_eq_ok
>   drm: dp helper: extract drm_dp_clock_recovery_ok
>   drm/nouveau: use the cr_ok/chanel_eq_ok helpers
>   drm: extract helpers to compute new training values from sink request
>   drm/nouveau: use dp link train request helper
>   drm: extract dp link train delay functions from radeon
>   drm/i915: use the new dp train delay helpers
>   drm: extract dp link bw helpers
>   drm: extract drm_dp_max_lane_count helper
>
>  drivers/gpu/drm/Makefile             |   2 +-
>  drivers/gpu/drm/drm_dp_helper.c      | 328
+++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_dp_i2c_helper.c  | 208 ----------------------
>  drivers/gpu/drm/i915/intel_dp.c      |  98 ++---------
>  drivers/gpu/drm/nouveau/nouveau_dp.c |  35 +---
>  drivers/gpu/drm/radeon/atombios_dp.c | 147 ++--------------
>  drivers/gpu/drm/radeon/radeon_mode.h |   2 +-
>  include/drm/drm_dp_helper.h          |  31 ++++
>  8 files changed, 400 insertions(+), 451 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_dp_helper.c
>  delete mode 100644 drivers/gpu/drm/drm_dp_i2c_helper.c
>
> --
> 1.7.11.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Alex Deucher

2012-Oct-18 13:48 UTC

head link

[Nouveau] [PATCH 00/10] extract dp helper functions

On Thu, Oct 18, 2012 at 4:15 AM, Daniel Vetter <daniel.vetter at ffwll.ch>
wrote:> Hi all,
>
> I've frustrated myself the last few days yelling at our link training
code.
> Comparing the i915 code to radeon and nouveau I've noticed the lack of
a nice
> set of dp helper functions. So I've started to extract a few.
>
> There's lots more that we can do I think (link configuration selection,
the i2c
> over aux retry stuff which diverges already between i915 and radeon, maybe
more
> higher level parts of the training sequence). But there the drivers diverge
> quite a bit (e.g. the link configuration is driver by different things in
each
> driver: coded link bw from the dp spec, link clock or required bw vs
avialable
> bw), so that's more work and probably best done when reworking these
functions
> for other reasons.
In theory we could provide a helper function to do the entire link
training in common code.  We'd just need a a couple of function
callbacks:

dp_aux_read()
dp_aux_write()
dp_link_train_init()
dp_set_src_training_pattern()
dp_link_train_fini()

Obviously some drivers may want to do their own thing, so it would
just be a helper.  Still for DP 1.2, it would be nice to have the
option of sharing more code.
>
> I've also tried to put the new helpers a bit to use in nouveau, but due
to lack
> of hw that part is untested.
>
> Comments and testing highly welcome.
>
> Yours, Daniel
>
> Daniel Vetter (10):
>   drm: rename drm_dp_i2c_helper.c to drm_dp_helper.c
>   drm: dp helper: extract drm_dp_channel_eq_ok
>   drm: dp helper: extract drm_dp_clock_recovery_ok
>   drm/nouveau: use the cr_ok/chanel_eq_ok helpers
>   drm: extract helpers to compute new training values from sink request
>   drm/nouveau: use dp link train request helper
>   drm: extract dp link train delay functions from radeon
>   drm/i915: use the new dp train delay helpers
>   drm: extract dp link bw helpers
>   drm: extract drm_dp_max_lane_count helper
>
>  drivers/gpu/drm/Makefile             |   2 +-
>  drivers/gpu/drm/drm_dp_helper.c      | 328
+++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_dp_i2c_helper.c  | 208 ----------------------
>  drivers/gpu/drm/i915/intel_dp.c      |  98 ++---------
>  drivers/gpu/drm/nouveau/nouveau_dp.c |  35 +---
>  drivers/gpu/drm/radeon/atombios_dp.c | 147 ++--------------
>  drivers/gpu/drm/radeon/radeon_mode.h |   2 +-
>  include/drm/drm_dp_helper.h          |  31 ++++
>  8 files changed, 400 insertions(+), 451 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_dp_helper.c
>  delete mode 100644 drivers/gpu/drm/drm_dp_i2c_helper.c
>
> --
> 1.7.11.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Daniel Vetter

2012-Oct-18 14:12 UTC

head link

[Nouveau] [PATCH 00/10] extract dp helper functions

On Thu, Oct 18, 2012 at 3:48 PM, Alex Deucher <alexdeucher at gmail.com>
wrote:> On Thu, Oct 18, 2012 at 4:15 AM, Daniel Vetter <daniel.vetter at
ffwll.ch> wrote:
>> Hi all,
>>
>> I've frustrated myself the last few days yelling at our link
training code.
>> Comparing the i915 code to radeon and nouveau I've noticed the lack
of a nice
>> set of dp helper functions. So I've started to extract a few.
>>
>> There's lots more that we can do I think (link configuration
selection, the i2c
>> over aux retry stuff which diverges already between i915 and radeon,
maybe more
>> higher level parts of the training sequence). But there the drivers
diverge
>> quite a bit (e.g. the link configuration is driver by different things
in each
>> driver: coded link bw from the dp spec, link clock or required bw vs
avialable
>> bw), so that's more work and probably best done when reworking
these functions
>> for other reasons.
>
> In theory we could provide a helper function to do the entire link
> training in common code.  We'd just need a a couple of function
> callbacks:
>
> dp_aux_read()
> dp_aux_write()
> dp_link_train_init()
> dp_set_src_training_pattern()
> dp_link_train_fini()
>
> Obviously some drivers may want to do their own thing, so it would
> just be a helper.  Still for DP 1.2, it would be nice to have the
> option of sharing more code.
Yeah, that's one of the ideas. Although I think we should start with a
few smaller things like e.g. the bandwidth stuff or the dp aux i2c
logic. Those need a subset of the above interfaces only and are less
intrusive. And since the link training in i915 seems to be still
rather broken and it in decent flux due to hsw enabling in general, I
want to wait and see a bit first until we have the corner cases
nailed.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Possibly Parallel Threads

Search for more seemingly similar threads

Nouveau - Oct 2012 - [PATCH 00/10] extract dp helper functions

[Nouveau] [PATCH 00/10] extract dp helper functions

[Nouveau] [PATCH 01/10] drm: rename drm_dp_i2c_helper.c to drm_dp_helper.c

[Nouveau] [PATCH 02/10] drm: dp helper: extract drm_dp_channel_eq_ok

[Nouveau] [PATCH 03/10] drm: dp helper: extract drm_dp_clock_recovery_ok

[Nouveau] [PATCH 04/10] drm/nouveau: use the cr_ok/chanel_eq_ok helpers

[Nouveau] [PATCH 05/10] drm: extract helpers to compute new training values from sink request

[Nouveau] [PATCH 06/10] drm/nouveau: use dp link train request helper

[Nouveau] [PATCH 07/10] drm: extract dp link train delay functions from radeon

[Nouveau] [PATCH 08/10] drm/i915: use the new dp train delay helpers

[Nouveau] [PATCH 09/10] drm: extract dp link bw helpers

[Nouveau] [PATCH 10/10] drm: extract drm_dp_max_lane_count helper

[Nouveau] [PATCH 00/10] extract dp helper functions

[Nouveau] [PATCH 00/10] extract dp helper functions

[Nouveau] [PATCH 00/10] extract dp helper functions

Possibly Parallel Threads