[PATCH v2 1/2] drm/bridge: analogix_dp: Remove duplicated code v2

[PATCH v2 1/2] drm/bridge: analogix_dp: Remove duplicated code v2

[Yakir Yang]

From: Tomeu Vizoso <tomeu.vizoso@collabora.com>

Remove code for reading the EDID and DPCD fields and use the helpers
instead.

Besides the obvious code reduction, other helpers are being added to the
core that could be used in this driver and will be good to be able to
use them instead of duplicating them.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Yakir Yang <ykk@rock-chips.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Yakir Yang <ykk@rock-chips.com>
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Sean Paul <seanpaul@chromium.org>
---
Changes in v2:
- A bunch of good fixes from Sean and Yakir
- Moved the transfer function to analogix_dp_reg.c
- Removed reference to the EDID from the dp struct
- Rebase on Sean's next tree
    git://people.freedesktop.org/~seanpaul/dogwood

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 263 ++++--------
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  41 +-
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 451 ++++++---------------
 3 files changed, 204 insertions(+), 551 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index efac8ab..5fe3982 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -31,6 +31,7 @@
 #include <drm/bridge/analogix_dp.h>
 
 #include "analogix_dp_core.h"
+#include "analogix_dp_reg.h"
 
 #define to_dp(nm)	container_of(nm, struct analogix_dp_device, nm)
 
@@ -174,150 +175,21 @@ static void analogix_dp_enable_sink_psr(struct analogix_dp_device *dp)
 	analogix_dp_enable_psr_crc(dp);
 }
 
-static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
-{
-	int i;
-	unsigned char sum = 0;
-
-	for (i = 0; i < EDID_BLOCK_LENGTH; i++)
-		sum = sum + edid_data[i];
-
-	return sum;
-}
-
-static int analogix_dp_read_edid(struct analogix_dp_device *dp)
-{
-	unsigned char *edid = dp->edid;
-	unsigned int extend_block = 0;
-	unsigned char sum;
-	unsigned char test_vector;
-	int retval;
-
-	/*
-	 * EDID device address is 0x50.
-	 * However, if necessary, you must have set upper address
-	 * into E-EDID in I2C device, 0x30.
-	 */
-
-	/* Read Extension Flag, Number of 128-byte EDID extension blocks */
-	retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
-						EDID_EXTENSION_FLAG,
-						&extend_block);
-	if (retval)
-		return retval;
-
-	if (extend_block > 0) {
-		dev_dbg(dp->dev, "EDID data includes a single extension!\n");
-
-		/* Read EDID data */
-		retval = analogix_dp_read_bytes_from_i2c(dp,
-						I2C_EDID_DEVICE_ADDR,
-						EDID_HEADER_PATTERN,
-						EDID_BLOCK_LENGTH,
-						&edid[EDID_HEADER_PATTERN]);
-		if (retval != 0) {
-			dev_err(dp->dev, "EDID Read failed!\n");
-			return -EIO;
-		}
-		sum = analogix_dp_calc_edid_check_sum(edid);
-		if (sum != 0) {
-			dev_err(dp->dev, "EDID bad checksum!\n");
-			return -EIO;
-		}
-
-		/* Read additional EDID data */
-		retval = analogix_dp_read_bytes_from_i2c(dp,
-				I2C_EDID_DEVICE_ADDR,
-				EDID_BLOCK_LENGTH,
-				EDID_BLOCK_LENGTH,
-				&edid[EDID_BLOCK_LENGTH]);
-		if (retval != 0) {
-			dev_err(dp->dev, "EDID Read failed!\n");
-			return -EIO;
-		}
-		sum = analogix_dp_calc_edid_check_sum(&edid[EDID_BLOCK_LENGTH]);
-		if (sum != 0) {
-			dev_err(dp->dev, "EDID bad checksum!\n");
-			return -EIO;
-		}
-
-		analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
-						&test_vector);
-		if (test_vector & DP_TEST_LINK_EDID_READ) {
-			analogix_dp_write_byte_to_dpcd(dp,
-				DP_TEST_EDID_CHECKSUM,
-				edid[EDID_BLOCK_LENGTH + EDID_CHECKSUM]);
-			analogix_dp_write_byte_to_dpcd(dp,
-				DP_TEST_RESPONSE,
-				DP_TEST_EDID_CHECKSUM_WRITE);
-		}
-	} else {
-		dev_info(dp->dev, "EDID data does not include any extensions.\n");
-
-		/* Read EDID data */
-		retval = analogix_dp_read_bytes_from_i2c(dp,
-				I2C_EDID_DEVICE_ADDR, EDID_HEADER_PATTERN,
-				EDID_BLOCK_LENGTH, &edid[EDID_HEADER_PATTERN]);
-		if (retval != 0) {
-			dev_err(dp->dev, "EDID Read failed!\n");
-			return -EIO;
-		}
-		sum = analogix_dp_calc_edid_check_sum(edid);
-		if (sum != 0) {
-			dev_err(dp->dev, "EDID bad checksum!\n");
-			return -EIO;
-		}
-
-		analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
-						&test_vector);
-		if (test_vector & DP_TEST_LINK_EDID_READ) {
-			analogix_dp_write_byte_to_dpcd(dp,
-				DP_TEST_EDID_CHECKSUM, edid[EDID_CHECKSUM]);
-			analogix_dp_write_byte_to_dpcd(dp,
-				DP_TEST_RESPONSE, DP_TEST_EDID_CHECKSUM_WRITE);
-		}
-	}
-
-	dev_dbg(dp->dev, "EDID Read success!\n");
-	return 0;
-}
-
-static int analogix_dp_handle_edid(struct analogix_dp_device *dp)
-{
-	u8 buf[12];
-	int i;
-	int retval;
-
-	/* Read DPCD DP_DPCD_REV~RECEIVE_PORT1_CAP_1 */
-	retval = analogix_dp_read_bytes_from_dpcd(dp, DP_DPCD_REV, 12, buf);
-	if (retval)
-		return retval;
-
-	/* Read EDID */
-	for (i = 0; i < 3; i++) {
-		retval = analogix_dp_read_edid(dp);
-		if (!retval)
-			break;
-	}
-
-	return retval;
-}
-
 static void
 analogix_dp_enable_rx_to_enhanced_mode(struct analogix_dp_device *dp,
 				       bool enable)
 {
 	u8 data;
 
-	analogix_dp_read_byte_from_dpcd(dp, DP_LANE_COUNT_SET, &data);
+	drm_dp_dpcd_readb(&dp->aux, DP_LANE_COUNT_SET, &data);
 
 	if (enable)
-		analogix_dp_write_byte_to_dpcd(dp, DP_LANE_COUNT_SET,
-					       DP_LANE_COUNT_ENHANCED_FRAME_EN |
-					       DPCD_LANE_COUNT_SET(data));
+		drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET,
+				   DP_LANE_COUNT_ENHANCED_FRAME_EN |
+					DPCD_LANE_COUNT_SET(data));
 	else
-		analogix_dp_write_byte_to_dpcd(dp, DP_LANE_COUNT_SET,
-					       DPCD_LANE_COUNT_SET(data));
+		drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET,
+				   DPCD_LANE_COUNT_SET(data));
 }
 
 static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp)
@@ -325,7 +197,7 @@ static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp)
 	u8 data;
 	int retval;
 
-	analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LANE_COUNT, &data);
+	drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data);
 	retval = DPCD_ENHANCED_FRAME_CAP(data);
 
 	return retval;
@@ -344,8 +216,8 @@ static void analogix_dp_training_pattern_dis(struct analogix_dp_device *dp)
 {
 	analogix_dp_set_training_pattern(dp, DP_NONE);
 
-	analogix_dp_write_byte_to_dpcd(dp, DP_TRAINING_PATTERN_SET,
-				       DP_TRAINING_PATTERN_DISABLE);
+	drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
+			   DP_TRAINING_PATTERN_DISABLE);
 }
 
 static void
@@ -390,8 +262,8 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
 	/* Setup RX configuration */
 	buf[0] = dp->link_train.link_rate;
 	buf[1] = dp->link_train.lane_count;
-	retval = analogix_dp_write_bytes_to_dpcd(dp, DP_LINK_BW_SET, 2, buf);
-	if (retval)
+	retval = drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, buf, 2);
+	if (retval < 0)
 		return retval;
 
 	/* Set TX pre-emphasis to minimum */
@@ -415,20 +287,22 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
 	analogix_dp_set_training_pattern(dp, TRAINING_PTN1);
 
 	/* Set RX training pattern */
-	retval = analogix_dp_write_byte_to_dpcd(dp,
-			DP_TRAINING_PATTERN_SET,
-			DP_LINK_SCRAMBLING_DISABLE | DP_TRAINING_PATTERN_1);
-	if (retval)
+	retval = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
+				    DP_LINK_SCRAMBLING_DISABLE |
+					DP_TRAINING_PATTERN_1);
+	if (retval < 0)
 		return retval;
 
 	for (lane = 0; lane < lane_count; lane++)
 		buf[lane] = DP_TRAIN_PRE_EMPH_LEVEL_0 |
 			    DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
 
-	retval = analogix_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET,
-						 lane_count, buf);
+	retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, buf,
+				   lane_count);
+	if (retval < 0)
+		return retval;
 
-	return retval;
+	return 0;
 }
 
 static unsigned char analogix_dp_get_lane_status(u8 link_status[2], int lane)
@@ -580,25 +454,23 @@ static int analogix_dp_process_clock_recovery(struct analogix_dp_device *dp)
 
 	lane_count = dp->link_train.lane_count;
 
-	retval =  analogix_dp_read_bytes_from_dpcd(dp,
-			DP_LANE0_1_STATUS, 2, link_status);
-	if (retval)
+	retval = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status, 2);
+	if (retval < 0)
 		return retval;
 
-	retval =  analogix_dp_read_bytes_from_dpcd(dp,
-			DP_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
-	if (retval)
+	retval = drm_dp_dpcd_read(&dp->aux, DP_ADJUST_REQUEST_LANE0_1,
+				  adjust_request, 2);
+	if (retval < 0)
 		return retval;
 
 	if (analogix_dp_clock_recovery_ok(link_status, lane_count) == 0) {
 		/* set training pattern 2 for EQ */
 		analogix_dp_set_training_pattern(dp, TRAINING_PTN2);
 
-		retval = analogix_dp_write_byte_to_dpcd(dp,
-				DP_TRAINING_PATTERN_SET,
-				DP_LINK_SCRAMBLING_DISABLE |
-				DP_TRAINING_PATTERN_2);
-		if (retval)
+		retval = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
+					    DP_LINK_SCRAMBLING_DISABLE |
+						DP_TRAINING_PATTERN_2);
+		if (retval < 0)
 			return retval;
 
 		dev_info(dp->dev, "Link Training Clock Recovery success\n");
@@ -636,13 +508,12 @@ static int analogix_dp_process_clock_recovery(struct analogix_dp_device *dp)
 		analogix_dp_set_lane_link_training(dp,
 			dp->link_train.training_lane[lane], lane);
 
-	retval = analogix_dp_write_bytes_to_dpcd(dp,
-			DP_TRAINING_LANE0_SET, lane_count,
-			dp->link_train.training_lane);
-	if (retval)
+	retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
+				   dp->link_train.training_lane, lane_count);
+	if (retval < 0)
 		return retval;
 
-	return retval;
+	return 0;
 }
 
 static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
@@ -655,9 +526,8 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
 
 	lane_count = dp->link_train.lane_count;
 
-	retval = analogix_dp_read_bytes_from_dpcd(dp,
-			DP_LANE0_1_STATUS, 2, link_status);
-	if (retval)
+	retval = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status, 2);
+	if (retval < 0)
 		return retval;
 
 	if (analogix_dp_clock_recovery_ok(link_status, lane_count)) {
@@ -665,14 +535,13 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
 		return -EIO;
 	}
 
-	retval = analogix_dp_read_bytes_from_dpcd(dp,
-			DP_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
-	if (retval)
+	retval = drm_dp_dpcd_read(&dp->aux, DP_ADJUST_REQUEST_LANE0_1, adjust_request, 2);
+	if (retval < 0)
 		return retval;
 
-	retval = analogix_dp_read_byte_from_dpcd(dp,
-			DP_LANE_ALIGN_STATUS_UPDATED, &link_align);
-	if (retval)
+	retval = drm_dp_dpcd_readb(&dp->aux, DP_LANE_ALIGN_STATUS_UPDATED,
+				   &link_align);
+	if (retval < 0)
 		return retval;
 
 	analogix_dp_get_adjust_training_lane(dp, adjust_request);
@@ -713,10 +582,12 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
 		analogix_dp_set_lane_link_training(dp,
 			dp->link_train.training_lane[lane], lane);
 
-	retval = analogix_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET,
-			lane_count, dp->link_train.training_lane);
+	retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
+				   dp->link_train.training_lane, lane_count);
+	if (retval < 0)
+		return retval;
 
-	return retval;
+	return 0;
 }
 
 static void analogix_dp_get_max_rx_bandwidth(struct analogix_dp_device *dp,
@@ -730,7 +601,7 @@ static void analogix_dp_get_max_rx_bandwidth(struct analogix_dp_device *dp,
 	 * For DP rev.1.2, Maximum link rate of Main Link lanes
 	 * 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps, 0x14 = 5.4Gbps
 	 */
-	analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LINK_RATE, &data);
+	drm_dp_dpcd_readb(&dp->aux, DP_MAX_LINK_RATE, &data);
 	*bandwidth = data;
 }
 
@@ -743,7 +614,7 @@ static void analogix_dp_get_max_rx_lane_count(struct analogix_dp_device *dp,
 	 * For DP rev.1.1, Maximum number of Main Link lanes
 	 * 0x01 = 1 lane, 0x02 = 2 lanes, 0x04 = 4 lanes
 	 */
-	analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LANE_COUNT, &data);
+	drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data);
 	*lane_count = DPCD_MAX_LANE_COUNT(data);
 }
 
@@ -912,19 +783,15 @@ static void analogix_dp_enable_scramble(struct analogix_dp_device *dp,
 	if (enable) {
 		analogix_dp_enable_scrambling(dp);
 
-		analogix_dp_read_byte_from_dpcd(dp, DP_TRAINING_PATTERN_SET,
-						&data);
-		analogix_dp_write_byte_to_dpcd(dp,
-			DP_TRAINING_PATTERN_SET,
-			(u8)(data & ~DP_LINK_SCRAMBLING_DISABLE));
+		drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data);
+		drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
+				   (u8)(data & ~DP_LINK_SCRAMBLING_DISABLE));
 	} else {
 		analogix_dp_disable_scrambling(dp);
 
-		analogix_dp_read_byte_from_dpcd(dp, DP_TRAINING_PATTERN_SET,
-						&data);
-		analogix_dp_write_byte_to_dpcd(dp,
-			DP_TRAINING_PATTERN_SET,
-			(u8)(data | DP_LINK_SCRAMBLING_DISABLE));
+		drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data);
+		drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
+				   (u8)(data | DP_LINK_SCRAMBLING_DISABLE));
 	}
 }
 
@@ -1053,7 +920,7 @@ out:
 int analogix_dp_get_modes(struct drm_connector *connector)
 {
 	struct analogix_dp_device *dp = to_dp(connector);
-	struct edid *edid = (struct edid *)dp->edid;
+	struct edid *edid;
 	int ret, num_modes = 0;
 
 	ret = analogix_dp_prepare_panel(dp, true, false);
@@ -1062,9 +929,11 @@ int analogix_dp_get_modes(struct drm_connector *connector)
 		return 0;
 	}
 
-	if (analogix_dp_handle_edid(dp) == 0) {
+	edid = drm_get_edid(connector, &dp->aux.ddc);
+	if (edid) {
 		drm_mode_connector_update_edid_property(&dp->connector, edid);
 		num_modes += drm_add_edid_modes(&dp->connector, edid);
+		kfree(edid);
 	}
 
 	if (dp->plat_data->panel)
@@ -1395,6 +1264,14 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
 	return 0;
 }
 
+static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux,
+				       struct drm_dp_aux_msg *msg)
+{
+	struct analogix_dp_device *dp = to_dp(aux);
+
+	return analogix_dp_transfer(dp, msg);
+}
+
 int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
 		     struct analogix_dp_plat_data *plat_data)
 {
@@ -1515,6 +1392,14 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
 	dp->drm_dev = drm_dev;
 	dp->encoder = dp->plat_data->encoder;
 
+	dp->aux.name = "DP-AUX";
+	dp->aux.transfer = analogix_dpaux_transfer;
+	dp->aux.dev = &pdev->dev;
+
+	ret = drm_dp_aux_register(&dp->aux);
+	if (ret)
+		goto err_disable_pm_runtime;
+
 	ret = analogix_dp_create_bridge(drm_dev, dp);
 	if (ret) {
 		DRM_ERROR("failed to create bridge (%d)\n", ret);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 473b980..a15f076 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -20,15 +20,6 @@
 #define MAX_CR_LOOP 5
 #define MAX_EQ_LOOP 5
 
-/* I2C EDID Chip ID, Slave Address */
-#define I2C_EDID_DEVICE_ADDR			0x50
-#define I2C_E_EDID_DEVICE_ADDR			0x30
-
-#define EDID_BLOCK_LENGTH			0x80
-#define EDID_HEADER_PATTERN			0x00
-#define EDID_EXTENSION_FLAG			0x7e
-#define EDID_CHECKSUM				0x7f
-
 /* DP_MAX_LANE_COUNT */
 #define DPCD_ENHANCED_FRAME_CAP(x)		(((x) >> 7) & 0x1)
 #define DPCD_MAX_LANE_COUNT(x)			((x) & 0x1f)
@@ -166,6 +157,7 @@ struct analogix_dp_device {
 	struct drm_device	*drm_dev;
 	struct drm_connector	connector;
 	struct drm_bridge	*bridge;
+	struct drm_dp_aux       aux;
 	struct clk		*clock;
 	unsigned int		irq;
 	void __iomem		*reg_base;
@@ -176,7 +168,6 @@ struct analogix_dp_device {
 	int			dpms_mode;
 	int			hpd_gpio;
 	bool                    force_hpd;
-	unsigned char           edid[EDID_BLOCK_LENGTH * 2];
 	bool			psr_support;
 
 	struct mutex		panel_lock;
@@ -210,33 +201,6 @@ void analogix_dp_reset_aux(struct analogix_dp_device *dp);
 void analogix_dp_init_aux(struct analogix_dp_device *dp);
 int analogix_dp_get_plug_in_status(struct analogix_dp_device *dp);
 void analogix_dp_enable_sw_function(struct analogix_dp_device *dp);
-int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp);
-int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp,
-				   unsigned int reg_addr,
-				   unsigned char data);
-int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp,
-				    unsigned int reg_addr,
-				    unsigned char *data);
-int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
-				    unsigned int reg_addr,
-				    unsigned int count,
-				    unsigned char data[]);
-int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp,
-				     unsigned int reg_addr,
-				     unsigned int count,
-				     unsigned char data[]);
-int analogix_dp_select_i2c_device(struct analogix_dp_device *dp,
-				  unsigned int device_addr,
-				  unsigned int reg_addr);
-int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
-				   unsigned int device_addr,
-				   unsigned int reg_addr,
-				   unsigned int *data);
-int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp,
-				    unsigned int device_addr,
-				    unsigned int reg_addr,
-				    unsigned int count,
-				    unsigned char edid[]);
 void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype);
 void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype);
 void analogix_dp_set_lane_count(struct analogix_dp_device *dp, u32 count);
@@ -285,5 +249,6 @@ void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
 void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
 void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
 			      struct edp_vsc_psr *vsc);
-
+ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
+			     struct drm_dp_aux_msg *msg);
 #endif /* _ANALOGIX_DP_CORE_H */
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 52c1b6b..a4d17b8 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -585,330 +585,6 @@ int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp,
 	return retval;
 }
 
-int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp,
-				    unsigned int reg_addr,
-				    unsigned char *data)
-{
-	u32 reg;
-	int i;
-	int retval;
-
-	for (i = 0; i < 3; i++) {
-		/* Clear AUX CH data buffer */
-		reg = BUF_CLR;
-		writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
-
-		/* Select DPCD device address */
-		reg = AUX_ADDR_7_0(reg_addr);
-		writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
-		reg = AUX_ADDR_15_8(reg_addr);
-		writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
-		reg = AUX_ADDR_19_16(reg_addr);
-		writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
-
-		/*
-		 * Set DisplayPort transaction and read 1 byte
-		 * If bit 3 is 1, DisplayPort transaction.
-		 * If Bit 3 is 0, I2C transaction.
-		 */
-		reg = AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_READ;
-		writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
-
-		/* Start AUX transaction */
-		retval = analogix_dp_start_aux_transaction(dp);
-		if (retval == 0)
-			break;
-
-		dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
-	}
-
-	/* Read data buffer */
-	reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0);
-	*data = (unsigned char)(reg & 0xff);
-
-	return retval;
-}
-
-int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
-				    unsigned int reg_addr,
-				    unsigned int count,
-				    unsigned char data[])
-{
-	u32 reg;
-	unsigned int start_offset;
-	unsigned int cur_data_count;
-	unsigned int cur_data_idx;
-	int i;
-	int retval = 0;
-
-	/* Clear AUX CH data buffer */
-	reg = BUF_CLR;
-	writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
-
-	start_offset = 0;
-	while (start_offset < count) {
-		/* Buffer size of AUX CH is 16 * 4bytes */
-		if ((count - start_offset) > 16)
-			cur_data_count = 16;
-		else
-			cur_data_count = count - start_offset;
-
-		for (i = 0; i < 3; i++) {
-			/* Select DPCD device address */
-			reg = AUX_ADDR_7_0(reg_addr + start_offset);
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
-			reg = AUX_ADDR_15_8(reg_addr + start_offset);
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
-			reg = AUX_ADDR_19_16(reg_addr + start_offset);
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
-
-			for (cur_data_idx = 0; cur_data_idx < cur_data_count;
-			     cur_data_idx++) {
-				reg = data[start_offset + cur_data_idx];
-				writel(reg, dp->reg_base +
-				       ANALOGIX_DP_BUF_DATA_0 +
-				       4 * cur_data_idx);
-			}
-
-			/*
-			 * Set DisplayPort transaction and write
-			 * If bit 3 is 1, DisplayPort transaction.
-			 * If Bit 3 is 0, I2C transaction.
-			 */
-			reg = AUX_LENGTH(cur_data_count) |
-				AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_WRITE;
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
-
-			/* Start AUX transaction */
-			retval = analogix_dp_start_aux_transaction(dp);
-			if (retval == 0)
-				break;
-
-			dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
-				__func__);
-		}
-
-		start_offset += cur_data_count;
-	}
-
-	return retval;
-}
-
-int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp,
-				     unsigned int reg_addr,
-				     unsigned int count,
-				     unsigned char data[])
-{
-	u32 reg;
-	unsigned int start_offset;
-	unsigned int cur_data_count;
-	unsigned int cur_data_idx;
-	int i;
-	int retval = 0;
-
-	/* Clear AUX CH data buffer */
-	reg = BUF_CLR;
-	writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
-
-	start_offset = 0;
-	while (start_offset < count) {
-		/* Buffer size of AUX CH is 16 * 4bytes */
-		if ((count - start_offset) > 16)
-			cur_data_count = 16;
-		else
-			cur_data_count = count - start_offset;
-
-		/* AUX CH Request Transaction process */
-		for (i = 0; i < 3; i++) {
-			/* Select DPCD device address */
-			reg = AUX_ADDR_7_0(reg_addr + start_offset);
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
-			reg = AUX_ADDR_15_8(reg_addr + start_offset);
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
-			reg = AUX_ADDR_19_16(reg_addr + start_offset);
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
-
-			/*
-			 * Set DisplayPort transaction and read
-			 * If bit 3 is 1, DisplayPort transaction.
-			 * If Bit 3 is 0, I2C transaction.
-			 */
-			reg = AUX_LENGTH(cur_data_count) |
-				AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_READ;
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
-
-			/* Start AUX transaction */
-			retval = analogix_dp_start_aux_transaction(dp);
-			if (retval == 0)
-				break;
-
-			dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
-				__func__);
-		}
-
-		for (cur_data_idx = 0; cur_data_idx < cur_data_count;
-		    cur_data_idx++) {
-			reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0
-						 + 4 * cur_data_idx);
-			data[start_offset + cur_data_idx] =
-				(unsigned char)reg;
-		}
-
-		start_offset += cur_data_count;
-	}
-
-	return retval;
-}
-
-int analogix_dp_select_i2c_device(struct analogix_dp_device *dp,
-				  unsigned int device_addr,
-				  unsigned int reg_addr)
-{
-	u32 reg;
-	int retval;
-
-	/* Set EDID device address */
-	reg = device_addr;
-	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
-	writel(0x0, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
-	writel(0x0, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
-
-	/* Set offset from base address of EDID device */
-	writel(reg_addr, dp->reg_base + ANALOGIX_DP_BUF_DATA_0);
-
-	/*
-	 * Set I2C transaction and write address
-	 * If bit 3 is 1, DisplayPort transaction.
-	 * If Bit 3 is 0, I2C transaction.
-	 */
-	reg = AUX_TX_COMM_I2C_TRANSACTION | AUX_TX_COMM_MOT |
-		AUX_TX_COMM_WRITE;
-	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
-
-	/* Start AUX transaction */
-	retval = analogix_dp_start_aux_transaction(dp);
-	if (retval != 0)
-		dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
-
-	return retval;
-}
-
-int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
-				   unsigned int device_addr,
-				   unsigned int reg_addr,
-				   unsigned int *data)
-{
-	u32 reg;
-	int i;
-	int retval;
-
-	for (i = 0; i < 3; i++) {
-		/* Clear AUX CH data buffer */
-		reg = BUF_CLR;
-		writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
-
-		/* Select EDID device */
-		retval = analogix_dp_select_i2c_device(dp, device_addr,
-						       reg_addr);
-		if (retval != 0)
-			continue;
-
-		/*
-		 * Set I2C transaction and read data
-		 * If bit 3 is 1, DisplayPort transaction.
-		 * If Bit 3 is 0, I2C transaction.
-		 */
-		reg = AUX_TX_COMM_I2C_TRANSACTION |
-			AUX_TX_COMM_READ;
-		writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
-
-		/* Start AUX transaction */
-		retval = analogix_dp_start_aux_transaction(dp);
-		if (retval == 0)
-			break;
-
-		dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
-	}
-
-	/* Read data */
-	if (retval == 0)
-		*data = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0);
-
-	return retval;
-}
-
-int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp,
-				    unsigned int device_addr,
-				    unsigned int reg_addr,
-				    unsigned int count,
-				    unsigned char edid[])
-{
-	u32 reg;
-	unsigned int i, j;
-	unsigned int cur_data_idx;
-	unsigned int defer = 0;
-	int retval = 0;
-
-	for (i = 0; i < count; i += 16) {
-		for (j = 0; j < 3; j++) {
-			/* Clear AUX CH data buffer */
-			reg = BUF_CLR;
-			writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
-
-			/* Set normal AUX CH command */
-			reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
-			reg &= ~ADDR_ONLY;
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
-
-			/*
-			 * If Rx sends defer, Tx sends only reads
-			 * request without sending address
-			 */
-			if (!defer)
-				retval = analogix_dp_select_i2c_device(dp,
-						device_addr, reg_addr + i);
-			else
-				defer = 0;
-
-			if (retval == 0) {
-				/*
-				 * Set I2C transaction and write data
-				 * If bit 3 is 1, DisplayPort transaction.
-				 * If Bit 3 is 0, I2C transaction.
-				 */
-				reg = AUX_LENGTH(16) |
-					AUX_TX_COMM_I2C_TRANSACTION |
-					AUX_TX_COMM_READ;
-				writel(reg, dp->reg_base +
-					ANALOGIX_DP_AUX_CH_CTL_1);
-
-				/* Start AUX transaction */
-				retval = analogix_dp_start_aux_transaction(dp);
-				if (retval == 0)
-					break;
-
-				dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
-					__func__);
-			}
-			/* Check if Rx sends defer */
-			reg = readl(dp->reg_base + ANALOGIX_DP_AUX_RX_COMM);
-			if (reg == AUX_RX_COMM_AUX_DEFER ||
-			    reg == AUX_RX_COMM_I2C_DEFER) {
-				dev_err(dp->dev, "Defer: %d\n\n", reg);
-				defer = 1;
-			}
-		}
-
-		for (cur_data_idx = 0; cur_data_idx < 16; cur_data_idx++) {
-			reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0
-						 + 4 * cur_data_idx);
-			edid[i + cur_data_idx] = (unsigned char)reg;
-		}
-	}
-
-	return retval;
-}
-
 void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype)
 {
 	u32 reg;
@@ -1373,3 +1049,130 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
 	val |= IF_EN;
 	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
 }
+
+ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
+			     struct drm_dp_aux_msg *msg)
+{
+	u32 reg;
+	u8 *buffer = msg->buffer;
+	int timeout_loop = 0;
+	unsigned int i;
+	int num_transferred = 0;
+
+	/* Buffer size of AUX CH is 16 bytes */
+	if (WARN_ON(msg->size > 16))
+		return -E2BIG;
+
+	/* Clear AUX CH data buffer */
+	reg = BUF_CLR;
+	writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
+
+	switch (msg->request & ~DP_AUX_I2C_MOT) {
+	case DP_AUX_I2C_WRITE:
+		reg = AUX_TX_COMM_WRITE | AUX_TX_COMM_I2C_TRANSACTION;
+		if (msg->request & DP_AUX_I2C_MOT)
+			reg |= AUX_TX_COMM_MOT;
+		break;
+
+	case DP_AUX_I2C_READ:
+		reg = AUX_TX_COMM_READ | AUX_TX_COMM_I2C_TRANSACTION;
+		if (msg->request & DP_AUX_I2C_MOT)
+			reg |= AUX_TX_COMM_MOT;
+		break;
+
+	case DP_AUX_NATIVE_WRITE:
+		reg = AUX_TX_COMM_WRITE | AUX_TX_COMM_DP_TRANSACTION;
+		break;
+
+	case DP_AUX_NATIVE_READ:
+		reg = AUX_TX_COMM_READ | AUX_TX_COMM_DP_TRANSACTION;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	reg |= AUX_LENGTH(msg->size);
+	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
+
+	/* Select DPCD device address */
+	reg = AUX_ADDR_7_0(msg->address);
+	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
+	reg = AUX_ADDR_15_8(msg->address);
+	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
+	reg = AUX_ADDR_19_16(msg->address);
+	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
+
+	if (!(msg->request & DP_AUX_I2C_READ)) {
+		for (i = 0; i < msg->size; i++) {
+			reg = buffer[i];
+			writel(reg, dp->reg_base + ANALOGIX_DP_BUF_DATA_0 +
+			       4 * i);
+			num_transferred++;
+		}
+	}
+
+	/* Enable AUX CH operation */
+	reg = AUX_EN;
+
+	/* Zero-sized messages specify address-only transactions. */
+	if (msg->size < 1)
+		reg |= ADDR_ONLY;
+
+	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
+
+	/* Is AUX CH command reply received? */
+	/* TODO: Wait for an interrupt instead of looping? */
+	reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
+	while (!(reg & RPLY_RECEIV)) {
+		timeout_loop++;
+		if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
+			dev_err(dp->dev, "AUX CH command reply failed!\n");
+			return -ETIMEDOUT;
+		}
+		reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
+		usleep_range(10, 11);
+	}
+
+	/* Clear interrupt source for AUX CH command reply */
+	writel(RPLY_RECEIV, dp->reg_base + ANALOGIX_DP_INT_STA);
+
+	/* Clear interrupt source for AUX CH access error */
+	reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
+	if (reg & AUX_ERR) {
+		writel(AUX_ERR, dp->reg_base + ANALOGIX_DP_INT_STA);
+		return -EREMOTEIO;
+	}
+
+	/* Check AUX CH error access status */
+	reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_STA);
+	if ((reg & AUX_STATUS_MASK)) {
+		dev_err(dp->dev, "AUX CH error happened: %d\n\n",
+			reg & AUX_STATUS_MASK);
+		return -EREMOTEIO;
+	}
+
+	if (msg->request & DP_AUX_I2C_READ) {
+		for (i = 0; i < msg->size; i++) {
+			reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0 +
+				    4 * i);
+			buffer[i] = (unsigned char)reg;
+			num_transferred++;
+		}
+	}
+
+	/* Check if Rx sends defer */
+	reg = readl(dp->reg_base + ANALOGIX_DP_AUX_RX_COMM);
+	if (reg == AUX_RX_COMM_AUX_DEFER)
+		msg->reply = DP_AUX_NATIVE_REPLY_DEFER;
+	else if (reg == AUX_RX_COMM_I2C_DEFER)
+		msg->reply = DP_AUX_I2C_REPLY_DEFER;
+	else if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE ||
+		 (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_READ)
+		msg->reply = DP_AUX_I2C_REPLY_ACK;
+	else if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_WRITE ||
+		 (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_READ)
+		msg->reply = DP_AUX_NATIVE_REPLY_ACK;
+
+	return num_transferred;
+}
-- 
1.9.1

[PATCH v2 1/2] drm/bridge: analogix_dp: Remove duplicated code v2

[Yakir Yang]

From: Tomeu Vizoso <tomeu.vizoso@collabora.com>

Remove code for reading the EDID and DPCD fields and use the helpers
instead.

Besides the obvious code reduction, other helpers are being added to the
core that could be used in this driver and will be good to be able to
use them instead of duplicating them.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Yakir Yang <ykk@rock-chips.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Yakir Yang <ykk@rock-chips.com>
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Sean Paul <seanpaul@chromium.org>
---
Changes in v2:
- A bunch of good fixes from Sean and Yakir
- Moved the transfer function to analogix_dp_reg.c
- Removed reference to the EDID from the dp struct
- Rebase on Sean's next tree
    git://people.freedesktop.org/~seanpaul/dogwood

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 263 ++++--------
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  41 +-
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 451 ++++++---------------
 3 files changed, 204 insertions(+), 551 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index efac8ab..5fe3982 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -31,6 +31,7 @@
 #include <drm/bridge/analogix_dp.h>
 
 #include "analogix_dp_core.h"
+#include "analogix_dp_reg.h"
 
 #define to_dp(nm)	container_of(nm, struct analogix_dp_device, nm)
 
@@ -174,150 +175,21 @@ static void analogix_dp_enable_sink_psr(struct analogix_dp_device *dp)
 	analogix_dp_enable_psr_crc(dp);
 }
 
-static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
-{
-	int i;
-	unsigned char sum = 0;
-
-	for (i = 0; i < EDID_BLOCK_LENGTH; i++)
-		sum = sum + edid_data[i];
-
-	return sum;
-}
-
-static int analogix_dp_read_edid(struct analogix_dp_device *dp)
-{
-	unsigned char *edid = dp->edid;
-	unsigned int extend_block = 0;
-	unsigned char sum;
-	unsigned char test_vector;
-	int retval;
-
-	/*
-	 * EDID device address is 0x50.
-	 * However, if necessary, you must have set upper address
-	 * into E-EDID in I2C device, 0x30.
-	 */
-
-	/* Read Extension Flag, Number of 128-byte EDID extension blocks */
-	retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
-						EDID_EXTENSION_FLAG,
-						&extend_block);
-	if (retval)
-		return retval;
-
-	if (extend_block > 0) {
-		dev_dbg(dp->dev, "EDID data includes a single extension!\n");
-
-		/* Read EDID data */
-		retval = analogix_dp_read_bytes_from_i2c(dp,
-						I2C_EDID_DEVICE_ADDR,
-						EDID_HEADER_PATTERN,
-						EDID_BLOCK_LENGTH,
-						&edid[EDID_HEADER_PATTERN]);
-		if (retval != 0) {
-			dev_err(dp->dev, "EDID Read failed!\n");
-			return -EIO;
-		}
-		sum = analogix_dp_calc_edid_check_sum(edid);
-		if (sum != 0) {
-			dev_err(dp->dev, "EDID bad checksum!\n");
-			return -EIO;
-		}
-
-		/* Read additional EDID data */
-		retval = analogix_dp_read_bytes_from_i2c(dp,
-				I2C_EDID_DEVICE_ADDR,
-				EDID_BLOCK_LENGTH,
-				EDID_BLOCK_LENGTH,
-				&edid[EDID_BLOCK_LENGTH]);
-		if (retval != 0) {
-			dev_err(dp->dev, "EDID Read failed!\n");
-			return -EIO;
-		}
-		sum = analogix_dp_calc_edid_check_sum(&edid[EDID_BLOCK_LENGTH]);
-		if (sum != 0) {
-			dev_err(dp->dev, "EDID bad checksum!\n");
-			return -EIO;
-		}
-
-		analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
-						&test_vector);
-		if (test_vector & DP_TEST_LINK_EDID_READ) {
-			analogix_dp_write_byte_to_dpcd(dp,
-				DP_TEST_EDID_CHECKSUM,
-				edid[EDID_BLOCK_LENGTH + EDID_CHECKSUM]);
-			analogix_dp_write_byte_to_dpcd(dp,
-				DP_TEST_RESPONSE,
-				DP_TEST_EDID_CHECKSUM_WRITE);
-		}
-	} else {
-		dev_info(dp->dev, "EDID data does not include any extensions.\n");
-
-		/* Read EDID data */
-		retval = analogix_dp_read_bytes_from_i2c(dp,
-				I2C_EDID_DEVICE_ADDR, EDID_HEADER_PATTERN,
-				EDID_BLOCK_LENGTH, &edid[EDID_HEADER_PATTERN]);
-		if (retval != 0) {
-			dev_err(dp->dev, "EDID Read failed!\n");
-			return -EIO;
-		}
-		sum = analogix_dp_calc_edid_check_sum(edid);
-		if (sum != 0) {
-			dev_err(dp->dev, "EDID bad checksum!\n");
-			return -EIO;
-		}
-
-		analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
-						&test_vector);
-		if (test_vector & DP_TEST_LINK_EDID_READ) {
-			analogix_dp_write_byte_to_dpcd(dp,
-				DP_TEST_EDID_CHECKSUM, edid[EDID_CHECKSUM]);
-			analogix_dp_write_byte_to_dpcd(dp,
-				DP_TEST_RESPONSE, DP_TEST_EDID_CHECKSUM_WRITE);
-		}
-	}
-
-	dev_dbg(dp->dev, "EDID Read success!\n");
-	return 0;
-}
-
-static int analogix_dp_handle_edid(struct analogix_dp_device *dp)
-{
-	u8 buf[12];
-	int i;
-	int retval;
-
-	/* Read DPCD DP_DPCD_REV~RECEIVE_PORT1_CAP_1 */
-	retval = analogix_dp_read_bytes_from_dpcd(dp, DP_DPCD_REV, 12, buf);
-	if (retval)
-		return retval;
-
-	/* Read EDID */
-	for (i = 0; i < 3; i++) {
-		retval = analogix_dp_read_edid(dp);
-		if (!retval)
-			break;
-	}
-
-	return retval;
-}
-
 static void
 analogix_dp_enable_rx_to_enhanced_mode(struct analogix_dp_device *dp,
 				       bool enable)
 {
 	u8 data;
 
-	analogix_dp_read_byte_from_dpcd(dp, DP_LANE_COUNT_SET, &data);
+	drm_dp_dpcd_readb(&dp->aux, DP_LANE_COUNT_SET, &data);
 
 	if (enable)
-		analogix_dp_write_byte_to_dpcd(dp, DP_LANE_COUNT_SET,
-					       DP_LANE_COUNT_ENHANCED_FRAME_EN |
-					       DPCD_LANE_COUNT_SET(data));
+		drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET,
+				   DP_LANE_COUNT_ENHANCED_FRAME_EN |
+					DPCD_LANE_COUNT_SET(data));
 	else
-		analogix_dp_write_byte_to_dpcd(dp, DP_LANE_COUNT_SET,
-					       DPCD_LANE_COUNT_SET(data));
+		drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET,
+				   DPCD_LANE_COUNT_SET(data));
 }
 
 static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp)
@@ -325,7 +197,7 @@ static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp)
 	u8 data;
 	int retval;
 
-	analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LANE_COUNT, &data);
+	drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data);
 	retval = DPCD_ENHANCED_FRAME_CAP(data);
 
 	return retval;
@@ -344,8 +216,8 @@ static void analogix_dp_training_pattern_dis(struct analogix_dp_device *dp)
 {
 	analogix_dp_set_training_pattern(dp, DP_NONE);
 
-	analogix_dp_write_byte_to_dpcd(dp, DP_TRAINING_PATTERN_SET,
-				       DP_TRAINING_PATTERN_DISABLE);
+	drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
+			   DP_TRAINING_PATTERN_DISABLE);
 }
 
 static void
@@ -390,8 +262,8 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
 	/* Setup RX configuration */
 	buf[0] = dp->link_train.link_rate;
 	buf[1] = dp->link_train.lane_count;
-	retval = analogix_dp_write_bytes_to_dpcd(dp, DP_LINK_BW_SET, 2, buf);
-	if (retval)
+	retval = drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, buf, 2);
+	if (retval < 0)
 		return retval;
 
 	/* Set TX pre-emphasis to minimum */
@@ -415,20 +287,22 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
 	analogix_dp_set_training_pattern(dp, TRAINING_PTN1);
 
 	/* Set RX training pattern */
-	retval = analogix_dp_write_byte_to_dpcd(dp,
-			DP_TRAINING_PATTERN_SET,
-			DP_LINK_SCRAMBLING_DISABLE | DP_TRAINING_PATTERN_1);
-	if (retval)
+	retval = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
+				    DP_LINK_SCRAMBLING_DISABLE |
+					DP_TRAINING_PATTERN_1);
+	if (retval < 0)
 		return retval;
 
 	for (lane = 0; lane < lane_count; lane++)
 		buf[lane] = DP_TRAIN_PRE_EMPH_LEVEL_0 |
 			    DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
 
-	retval = analogix_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET,
-						 lane_count, buf);
+	retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, buf,
+				   lane_count);
+	if (retval < 0)
+		return retval;
 
-	return retval;
+	return 0;
 }
 
 static unsigned char analogix_dp_get_lane_status(u8 link_status[2], int lane)
@@ -580,25 +454,23 @@ static int analogix_dp_process_clock_recovery(struct analogix_dp_device *dp)
 
 	lane_count = dp->link_train.lane_count;
 
-	retval =  analogix_dp_read_bytes_from_dpcd(dp,
-			DP_LANE0_1_STATUS, 2, link_status);
-	if (retval)
+	retval = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status, 2);
+	if (retval < 0)
 		return retval;
 
-	retval =  analogix_dp_read_bytes_from_dpcd(dp,
-			DP_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
-	if (retval)
+	retval = drm_dp_dpcd_read(&dp->aux, DP_ADJUST_REQUEST_LANE0_1,
+				  adjust_request, 2);
+	if (retval < 0)
 		return retval;
 
 	if (analogix_dp_clock_recovery_ok(link_status, lane_count) == 0) {
 		/* set training pattern 2 for EQ */
 		analogix_dp_set_training_pattern(dp, TRAINING_PTN2);
 
-		retval = analogix_dp_write_byte_to_dpcd(dp,
-				DP_TRAINING_PATTERN_SET,
-				DP_LINK_SCRAMBLING_DISABLE |
-				DP_TRAINING_PATTERN_2);
-		if (retval)
+		retval = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
+					    DP_LINK_SCRAMBLING_DISABLE |
+						DP_TRAINING_PATTERN_2);
+		if (retval < 0)
 			return retval;
 
 		dev_info(dp->dev, "Link Training Clock Recovery success\n");
@@ -636,13 +508,12 @@ static int analogix_dp_process_clock_recovery(struct analogix_dp_device *dp)
 		analogix_dp_set_lane_link_training(dp,
 			dp->link_train.training_lane[lane], lane);
 
-	retval = analogix_dp_write_bytes_to_dpcd(dp,
-			DP_TRAINING_LANE0_SET, lane_count,
-			dp->link_train.training_lane);
-	if (retval)
+	retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
+				   dp->link_train.training_lane, lane_count);
+	if (retval < 0)
 		return retval;
 
-	return retval;
+	return 0;
 }
 
 static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
@@ -655,9 +526,8 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
 
 	lane_count = dp->link_train.lane_count;
 
-	retval = analogix_dp_read_bytes_from_dpcd(dp,
-			DP_LANE0_1_STATUS, 2, link_status);
-	if (retval)
+	retval = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status, 2);
+	if (retval < 0)
 		return retval;
 
 	if (analogix_dp_clock_recovery_ok(link_status, lane_count)) {
@@ -665,14 +535,13 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
 		return -EIO;
 	}
 
-	retval = analogix_dp_read_bytes_from_dpcd(dp,
-			DP_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
-	if (retval)
+	retval = drm_dp_dpcd_read(&dp->aux, DP_ADJUST_REQUEST_LANE0_1, adjust_request, 2);
+	if (retval < 0)
 		return retval;
 
-	retval = analogix_dp_read_byte_from_dpcd(dp,
-			DP_LANE_ALIGN_STATUS_UPDATED, &link_align);
-	if (retval)
+	retval = drm_dp_dpcd_readb(&dp->aux, DP_LANE_ALIGN_STATUS_UPDATED,
+				   &link_align);
+	if (retval < 0)
 		return retval;
 
 	analogix_dp_get_adjust_training_lane(dp, adjust_request);
@@ -713,10 +582,12 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
 		analogix_dp_set_lane_link_training(dp,
 			dp->link_train.training_lane[lane], lane);
 
-	retval = analogix_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET,
-			lane_count, dp->link_train.training_lane);
+	retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
+				   dp->link_train.training_lane, lane_count);
+	if (retval < 0)
+		return retval;
 
-	return retval;
+	return 0;
 }
 
 static void analogix_dp_get_max_rx_bandwidth(struct analogix_dp_device *dp,
@@ -730,7 +601,7 @@ static void analogix_dp_get_max_rx_bandwidth(struct analogix_dp_device *dp,
 	 * For DP rev.1.2, Maximum link rate of Main Link lanes
 	 * 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps, 0x14 = 5.4Gbps
 	 */
-	analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LINK_RATE, &data);
+	drm_dp_dpcd_readb(&dp->aux, DP_MAX_LINK_RATE, &data);
 	*bandwidth = data;
 }
 
@@ -743,7 +614,7 @@ static void analogix_dp_get_max_rx_lane_count(struct analogix_dp_device *dp,
 	 * For DP rev.1.1, Maximum number of Main Link lanes
 	 * 0x01 = 1 lane, 0x02 = 2 lanes, 0x04 = 4 lanes
 	 */
-	analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LANE_COUNT, &data);
+	drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data);
 	*lane_count = DPCD_MAX_LANE_COUNT(data);
 }
 
@@ -912,19 +783,15 @@ static void analogix_dp_enable_scramble(struct analogix_dp_device *dp,
 	if (enable) {
 		analogix_dp_enable_scrambling(dp);
 
-		analogix_dp_read_byte_from_dpcd(dp, DP_TRAINING_PATTERN_SET,
-						&data);
-		analogix_dp_write_byte_to_dpcd(dp,
-			DP_TRAINING_PATTERN_SET,
-			(u8)(data & ~DP_LINK_SCRAMBLING_DISABLE));
+		drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data);
+		drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
+				   (u8)(data & ~DP_LINK_SCRAMBLING_DISABLE));
 	} else {
 		analogix_dp_disable_scrambling(dp);
 
-		analogix_dp_read_byte_from_dpcd(dp, DP_TRAINING_PATTERN_SET,
-						&data);
-		analogix_dp_write_byte_to_dpcd(dp,
-			DP_TRAINING_PATTERN_SET,
-			(u8)(data | DP_LINK_SCRAMBLING_DISABLE));
+		drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data);
+		drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
+				   (u8)(data | DP_LINK_SCRAMBLING_DISABLE));
 	}
 }
 
@@ -1053,7 +920,7 @@ out:
 int analogix_dp_get_modes(struct drm_connector *connector)
 {
 	struct analogix_dp_device *dp = to_dp(connector);
-	struct edid *edid = (struct edid *)dp->edid;
+	struct edid *edid;
 	int ret, num_modes = 0;
 
 	ret = analogix_dp_prepare_panel(dp, true, false);
@@ -1062,9 +929,11 @@ int analogix_dp_get_modes(struct drm_connector *connector)
 		return 0;
 	}
 
-	if (analogix_dp_handle_edid(dp) == 0) {
+	edid = drm_get_edid(connector, &dp->aux.ddc);
+	if (edid) {
 		drm_mode_connector_update_edid_property(&dp->connector, edid);
 		num_modes += drm_add_edid_modes(&dp->connector, edid);
+		kfree(edid);
 	}
 
 	if (dp->plat_data->panel)
@@ -1395,6 +1264,14 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
 	return 0;
 }
 
+static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux,
+				       struct drm_dp_aux_msg *msg)
+{
+	struct analogix_dp_device *dp = to_dp(aux);
+
+	return analogix_dp_transfer(dp, msg);
+}
+
 int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
 		     struct analogix_dp_plat_data *plat_data)
 {
@@ -1515,6 +1392,14 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
 	dp->drm_dev = drm_dev;
 	dp->encoder = dp->plat_data->encoder;
 
+	dp->aux.name = "DP-AUX";
+	dp->aux.transfer = analogix_dpaux_transfer;
+	dp->aux.dev = &pdev->dev;
+
+	ret = drm_dp_aux_register(&dp->aux);
+	if (ret)
+		goto err_disable_pm_runtime;
+
 	ret = analogix_dp_create_bridge(drm_dev, dp);
 	if (ret) {
 		DRM_ERROR("failed to create bridge (%d)\n", ret);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 473b980..a15f076 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -20,15 +20,6 @@
 #define MAX_CR_LOOP 5
 #define MAX_EQ_LOOP 5
 
-/* I2C EDID Chip ID, Slave Address */
-#define I2C_EDID_DEVICE_ADDR			0x50
-#define I2C_E_EDID_DEVICE_ADDR			0x30
-
-#define EDID_BLOCK_LENGTH			0x80
-#define EDID_HEADER_PATTERN			0x00
-#define EDID_EXTENSION_FLAG			0x7e
-#define EDID_CHECKSUM				0x7f
-
 /* DP_MAX_LANE_COUNT */
 #define DPCD_ENHANCED_FRAME_CAP(x)		(((x) >> 7) & 0x1)
 #define DPCD_MAX_LANE_COUNT(x)			((x) & 0x1f)
@@ -166,6 +157,7 @@ struct analogix_dp_device {
 	struct drm_device	*drm_dev;
 	struct drm_connector	connector;
 	struct drm_bridge	*bridge;
+	struct drm_dp_aux       aux;
 	struct clk		*clock;
 	unsigned int		irq;
 	void __iomem		*reg_base;
@@ -176,7 +168,6 @@ struct analogix_dp_device {
 	int			dpms_mode;
 	int			hpd_gpio;
 	bool                    force_hpd;
-	unsigned char           edid[EDID_BLOCK_LENGTH * 2];
 	bool			psr_support;
 
 	struct mutex		panel_lock;
@@ -210,33 +201,6 @@ void analogix_dp_reset_aux(struct analogix_dp_device *dp);
 void analogix_dp_init_aux(struct analogix_dp_device *dp);
 int analogix_dp_get_plug_in_status(struct analogix_dp_device *dp);
 void analogix_dp_enable_sw_function(struct analogix_dp_device *dp);
-int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp);
-int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp,
-				   unsigned int reg_addr,
-				   unsigned char data);
-int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp,
-				    unsigned int reg_addr,
-				    unsigned char *data);
-int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
-				    unsigned int reg_addr,
-				    unsigned int count,
-				    unsigned char data[]);
-int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp,
-				     unsigned int reg_addr,
-				     unsigned int count,
-				     unsigned char data[]);
-int analogix_dp_select_i2c_device(struct analogix_dp_device *dp,
-				  unsigned int device_addr,
-				  unsigned int reg_addr);
-int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
-				   unsigned int device_addr,
-				   unsigned int reg_addr,
-				   unsigned int *data);
-int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp,
-				    unsigned int device_addr,
-				    unsigned int reg_addr,
-				    unsigned int count,
-				    unsigned char edid[]);
 void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype);
 void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype);
 void analogix_dp_set_lane_count(struct analogix_dp_device *dp, u32 count);
@@ -285,5 +249,6 @@ void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
 void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
 void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
 			      struct edp_vsc_psr *vsc);
-
+ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
+			     struct drm_dp_aux_msg *msg);
 #endif /* _ANALOGIX_DP_CORE_H */
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 52c1b6b..a4d17b8 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -585,330 +585,6 @@ int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp,
 	return retval;
 }
 
-int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp,
-				    unsigned int reg_addr,
-				    unsigned char *data)
-{
-	u32 reg;
-	int i;
-	int retval;
-
-	for (i = 0; i < 3; i++) {
-		/* Clear AUX CH data buffer */
-		reg = BUF_CLR;
-		writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
-
-		/* Select DPCD device address */
-		reg = AUX_ADDR_7_0(reg_addr);
-		writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
-		reg = AUX_ADDR_15_8(reg_addr);
-		writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
-		reg = AUX_ADDR_19_16(reg_addr);
-		writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
-
-		/*
-		 * Set DisplayPort transaction and read 1 byte
-		 * If bit 3 is 1, DisplayPort transaction.
-		 * If Bit 3 is 0, I2C transaction.
-		 */
-		reg = AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_READ;
-		writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
-
-		/* Start AUX transaction */
-		retval = analogix_dp_start_aux_transaction(dp);
-		if (retval == 0)
-			break;
-
-		dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
-	}
-
-	/* Read data buffer */
-	reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0);
-	*data = (unsigned char)(reg & 0xff);
-
-	return retval;
-}
-
-int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
-				    unsigned int reg_addr,
-				    unsigned int count,
-				    unsigned char data[])
-{
-	u32 reg;
-	unsigned int start_offset;
-	unsigned int cur_data_count;
-	unsigned int cur_data_idx;
-	int i;
-	int retval = 0;
-
-	/* Clear AUX CH data buffer */
-	reg = BUF_CLR;
-	writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
-
-	start_offset = 0;
-	while (start_offset < count) {
-		/* Buffer size of AUX CH is 16 * 4bytes */
-		if ((count - start_offset) > 16)
-			cur_data_count = 16;
-		else
-			cur_data_count = count - start_offset;
-
-		for (i = 0; i < 3; i++) {
-			/* Select DPCD device address */
-			reg = AUX_ADDR_7_0(reg_addr + start_offset);
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
-			reg = AUX_ADDR_15_8(reg_addr + start_offset);
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
-			reg = AUX_ADDR_19_16(reg_addr + start_offset);
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
-
-			for (cur_data_idx = 0; cur_data_idx < cur_data_count;
-			     cur_data_idx++) {
-				reg = data[start_offset + cur_data_idx];
-				writel(reg, dp->reg_base +
-				       ANALOGIX_DP_BUF_DATA_0 +
-				       4 * cur_data_idx);
-			}
-
-			/*
-			 * Set DisplayPort transaction and write
-			 * If bit 3 is 1, DisplayPort transaction.
-			 * If Bit 3 is 0, I2C transaction.
-			 */
-			reg = AUX_LENGTH(cur_data_count) |
-				AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_WRITE;
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
-
-			/* Start AUX transaction */
-			retval = analogix_dp_start_aux_transaction(dp);
-			if (retval == 0)
-				break;
-
-			dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
-				__func__);
-		}
-
-		start_offset += cur_data_count;
-	}
-
-	return retval;
-}
-
-int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp,
-				     unsigned int reg_addr,
-				     unsigned int count,
-				     unsigned char data[])
-{
-	u32 reg;
-	unsigned int start_offset;
-	unsigned int cur_data_count;
-	unsigned int cur_data_idx;
-	int i;
-	int retval = 0;
-
-	/* Clear AUX CH data buffer */
-	reg = BUF_CLR;
-	writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
-
-	start_offset = 0;
-	while (start_offset < count) {
-		/* Buffer size of AUX CH is 16 * 4bytes */
-		if ((count - start_offset) > 16)
-			cur_data_count = 16;
-		else
-			cur_data_count = count - start_offset;
-
-		/* AUX CH Request Transaction process */
-		for (i = 0; i < 3; i++) {
-			/* Select DPCD device address */
-			reg = AUX_ADDR_7_0(reg_addr + start_offset);
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
-			reg = AUX_ADDR_15_8(reg_addr + start_offset);
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
-			reg = AUX_ADDR_19_16(reg_addr + start_offset);
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
-
-			/*
-			 * Set DisplayPort transaction and read
-			 * If bit 3 is 1, DisplayPort transaction.
-			 * If Bit 3 is 0, I2C transaction.
-			 */
-			reg = AUX_LENGTH(cur_data_count) |
-				AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_READ;
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
-
-			/* Start AUX transaction */
-			retval = analogix_dp_start_aux_transaction(dp);
-			if (retval == 0)
-				break;
-
-			dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
-				__func__);
-		}
-
-		for (cur_data_idx = 0; cur_data_idx < cur_data_count;
-		    cur_data_idx++) {
-			reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0
-						 + 4 * cur_data_idx);
-			data[start_offset + cur_data_idx] =
-				(unsigned char)reg;
-		}
-
-		start_offset += cur_data_count;
-	}
-
-	return retval;
-}
-
-int analogix_dp_select_i2c_device(struct analogix_dp_device *dp,
-				  unsigned int device_addr,
-				  unsigned int reg_addr)
-{
-	u32 reg;
-	int retval;
-
-	/* Set EDID device address */
-	reg = device_addr;
-	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
-	writel(0x0, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
-	writel(0x0, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
-
-	/* Set offset from base address of EDID device */
-	writel(reg_addr, dp->reg_base + ANALOGIX_DP_BUF_DATA_0);
-
-	/*
-	 * Set I2C transaction and write address
-	 * If bit 3 is 1, DisplayPort transaction.
-	 * If Bit 3 is 0, I2C transaction.
-	 */
-	reg = AUX_TX_COMM_I2C_TRANSACTION | AUX_TX_COMM_MOT |
-		AUX_TX_COMM_WRITE;
-	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
-
-	/* Start AUX transaction */
-	retval = analogix_dp_start_aux_transaction(dp);
-	if (retval != 0)
-		dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
-
-	return retval;
-}
-
-int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
-				   unsigned int device_addr,
-				   unsigned int reg_addr,
-				   unsigned int *data)
-{
-	u32 reg;
-	int i;
-	int retval;
-
-	for (i = 0; i < 3; i++) {
-		/* Clear AUX CH data buffer */
-		reg = BUF_CLR;
-		writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
-
-		/* Select EDID device */
-		retval = analogix_dp_select_i2c_device(dp, device_addr,
-						       reg_addr);
-		if (retval != 0)
-			continue;
-
-		/*
-		 * Set I2C transaction and read data
-		 * If bit 3 is 1, DisplayPort transaction.
-		 * If Bit 3 is 0, I2C transaction.
-		 */
-		reg = AUX_TX_COMM_I2C_TRANSACTION |
-			AUX_TX_COMM_READ;
-		writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
-
-		/* Start AUX transaction */
-		retval = analogix_dp_start_aux_transaction(dp);
-		if (retval == 0)
-			break;
-
-		dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
-	}
-
-	/* Read data */
-	if (retval == 0)
-		*data = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0);
-
-	return retval;
-}
-
-int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp,
-				    unsigned int device_addr,
-				    unsigned int reg_addr,
-				    unsigned int count,
-				    unsigned char edid[])
-{
-	u32 reg;
-	unsigned int i, j;
-	unsigned int cur_data_idx;
-	unsigned int defer = 0;
-	int retval = 0;
-
-	for (i = 0; i < count; i += 16) {
-		for (j = 0; j < 3; j++) {
-			/* Clear AUX CH data buffer */
-			reg = BUF_CLR;
-			writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
-
-			/* Set normal AUX CH command */
-			reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
-			reg &= ~ADDR_ONLY;
-			writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
-
-			/*
-			 * If Rx sends defer, Tx sends only reads
-			 * request without sending address
-			 */
-			if (!defer)
-				retval = analogix_dp_select_i2c_device(dp,
-						device_addr, reg_addr + i);
-			else
-				defer = 0;
-
-			if (retval == 0) {
-				/*
-				 * Set I2C transaction and write data
-				 * If bit 3 is 1, DisplayPort transaction.
-				 * If Bit 3 is 0, I2C transaction.
-				 */
-				reg = AUX_LENGTH(16) |
-					AUX_TX_COMM_I2C_TRANSACTION |
-					AUX_TX_COMM_READ;
-				writel(reg, dp->reg_base +
-					ANALOGIX_DP_AUX_CH_CTL_1);
-
-				/* Start AUX transaction */
-				retval = analogix_dp_start_aux_transaction(dp);
-				if (retval == 0)
-					break;
-
-				dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
-					__func__);
-			}
-			/* Check if Rx sends defer */
-			reg = readl(dp->reg_base + ANALOGIX_DP_AUX_RX_COMM);
-			if (reg == AUX_RX_COMM_AUX_DEFER ||
-			    reg == AUX_RX_COMM_I2C_DEFER) {
-				dev_err(dp->dev, "Defer: %d\n\n", reg);
-				defer = 1;
-			}
-		}
-
-		for (cur_data_idx = 0; cur_data_idx < 16; cur_data_idx++) {
-			reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0
-						 + 4 * cur_data_idx);
-			edid[i + cur_data_idx] = (unsigned char)reg;
-		}
-	}
-
-	return retval;
-}
-
 void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype)
 {
 	u32 reg;
@@ -1373,3 +1049,130 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
 	val |= IF_EN;
 	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
 }
+
+ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
+			     struct drm_dp_aux_msg *msg)
+{
+	u32 reg;
+	u8 *buffer = msg->buffer;
+	int timeout_loop = 0;
+	unsigned int i;
+	int num_transferred = 0;
+
+	/* Buffer size of AUX CH is 16 bytes */
+	if (WARN_ON(msg->size > 16))
+		return -E2BIG;
+
+	/* Clear AUX CH data buffer */
+	reg = BUF_CLR;
+	writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
+
+	switch (msg->request & ~DP_AUX_I2C_MOT) {
+	case DP_AUX_I2C_WRITE:
+		reg = AUX_TX_COMM_WRITE | AUX_TX_COMM_I2C_TRANSACTION;
+		if (msg->request & DP_AUX_I2C_MOT)
+			reg |= AUX_TX_COMM_MOT;
+		break;
+
+	case DP_AUX_I2C_READ:
+		reg = AUX_TX_COMM_READ | AUX_TX_COMM_I2C_TRANSACTION;
+		if (msg->request & DP_AUX_I2C_MOT)
+			reg |= AUX_TX_COMM_MOT;
+		break;
+
+	case DP_AUX_NATIVE_WRITE:
+		reg = AUX_TX_COMM_WRITE | AUX_TX_COMM_DP_TRANSACTION;
+		break;
+
+	case DP_AUX_NATIVE_READ:
+		reg = AUX_TX_COMM_READ | AUX_TX_COMM_DP_TRANSACTION;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	reg |= AUX_LENGTH(msg->size);
+	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
+
+	/* Select DPCD device address */
+	reg = AUX_ADDR_7_0(msg->address);
+	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
+	reg = AUX_ADDR_15_8(msg->address);
+	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
+	reg = AUX_ADDR_19_16(msg->address);
+	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
+
+	if (!(msg->request & DP_AUX_I2C_READ)) {
+		for (i = 0; i < msg->size; i++) {
+			reg = buffer[i];
+			writel(reg, dp->reg_base + ANALOGIX_DP_BUF_DATA_0 +
+			       4 * i);
+			num_transferred++;
+		}
+	}
+
+	/* Enable AUX CH operation */
+	reg = AUX_EN;
+
+	/* Zero-sized messages specify address-only transactions. */
+	if (msg->size < 1)
+		reg |= ADDR_ONLY;
+
+	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
+
+	/* Is AUX CH command reply received? */
+	/* TODO: Wait for an interrupt instead of looping? */
+	reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
+	while (!(reg & RPLY_RECEIV)) {
+		timeout_loop++;
+		if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
+			dev_err(dp->dev, "AUX CH command reply failed!\n");
+			return -ETIMEDOUT;
+		}
+		reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
+		usleep_range(10, 11);
+	}
+
+	/* Clear interrupt source for AUX CH command reply */
+	writel(RPLY_RECEIV, dp->reg_base + ANALOGIX_DP_INT_STA);
+
+	/* Clear interrupt source for AUX CH access error */
+	reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
+	if (reg & AUX_ERR) {
+		writel(AUX_ERR, dp->reg_base + ANALOGIX_DP_INT_STA);
+		return -EREMOTEIO;
+	}
+
+	/* Check AUX CH error access status */
+	reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_STA);
+	if ((reg & AUX_STATUS_MASK)) {
+		dev_err(dp->dev, "AUX CH error happened: %d\n\n",
+			reg & AUX_STATUS_MASK);
+		return -EREMOTEIO;
+	}
+
+	if (msg->request & DP_AUX_I2C_READ) {
+		for (i = 0; i < msg->size; i++) {
+			reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0 +
+				    4 * i);
+			buffer[i] = (unsigned char)reg;
+			num_transferred++;
+		}
+	}
+
+	/* Check if Rx sends defer */
+	reg = readl(dp->reg_base + ANALOGIX_DP_AUX_RX_COMM);
+	if (reg == AUX_RX_COMM_AUX_DEFER)
+		msg->reply = DP_AUX_NATIVE_REPLY_DEFER;
+	else if (reg == AUX_RX_COMM_I2C_DEFER)
+		msg->reply = DP_AUX_I2C_REPLY_DEFER;
+	else if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE ||
+		 (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_READ)
+		msg->reply = DP_AUX_I2C_REPLY_ACK;
+	else if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_WRITE ||
+		 (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_READ)
+		msg->reply = DP_AUX_NATIVE_REPLY_ACK;
+
+	return num_transferred;
+}
-- 
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/2] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR

[Yakir Yang]

Make sure the request PSR state could effect in analogix_dp_send_psr_spd()
function, or printing the error Sink PSR state if we failed to effect
the request PSR setting.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v2:
- A bunch of good fixes from Sean

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  6 ++----
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  4 ++--
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 25 ++++++++++++++++++++--
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5fe3982..c0ce16a 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
 	psr_vsc.DB0 = 0;
 	psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
 
-	analogix_dp_send_psr_spd(dp, &psr_vsc);
-	return 0;
+	return analogix_dp_send_psr_spd(dp, &psr_vsc);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
 
@@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
 	psr_vsc.DB0 = 0;
 	psr_vsc.DB1 = 0;
 
-	analogix_dp_send_psr_spd(dp, &psr_vsc);
-	return 0;
+	return analogix_dp_send_psr_spd(dp, &psr_vsc);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
 
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index a15f076..6c07a50 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -247,8 +247,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
 void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
 void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
 void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
-			      struct edp_vsc_psr *vsc);
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+			     struct edp_vsc_psr *vsc);
 ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
 			     struct drm_dp_aux_msg *msg);
 #endif /* _ANALOGIX_DP_CORE_H */
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index a4d17b8..09d703b 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
 	writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
 }
 
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
-			      struct edp_vsc_psr *vsc)
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+			     struct edp_vsc_psr *vsc)
 {
+	unsigned long timeout;
 	unsigned int val;
+	u8 sink;
 
 	/* don't send info frame */
 	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
@@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
 	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
 	val |= IF_EN;
 	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+
+	timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_COUNT);
+	while (time_before(jiffies, timeout)) {
+		val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
+		if (val != 1) {
+			dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
+			return val;
+		}
+
+		if (vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB ||
+		    !vsc->DB1 && sink == DP_PSR_SINK_INACTIVE)
+			break;
+
+		usleep_range(1000, 1500);
+	}
+
+	dev_warn(dp->dev, "Failed to effect PSR: %x", sink);
+
+	return -ETIMEDOUT;
 }
 
 ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
-- 
1.9.1

[PATCH v2 2/2] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR

[Yakir Yang]

Make sure the request PSR state could effect in analogix_dp_send_psr_spd()
function, or printing the error Sink PSR state if we failed to effect
the request PSR setting.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v2:
- A bunch of good fixes from Sean

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  6 ++----
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  4 ++--
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 25 ++++++++++++++++++++--
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5fe3982..c0ce16a 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
 	psr_vsc.DB0 = 0;
 	psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
 
-	analogix_dp_send_psr_spd(dp, &psr_vsc);
-	return 0;
+	return analogix_dp_send_psr_spd(dp, &psr_vsc);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
 
@@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
 	psr_vsc.DB0 = 0;
 	psr_vsc.DB1 = 0;
 
-	analogix_dp_send_psr_spd(dp, &psr_vsc);
-	return 0;
+	return analogix_dp_send_psr_spd(dp, &psr_vsc);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
 
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index a15f076..6c07a50 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -247,8 +247,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
 void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
 void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
 void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
-			      struct edp_vsc_psr *vsc);
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+			     struct edp_vsc_psr *vsc);
 ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
 			     struct drm_dp_aux_msg *msg);
 #endif /* _ANALOGIX_DP_CORE_H */
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index a4d17b8..09d703b 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
 	writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
 }
 
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
-			      struct edp_vsc_psr *vsc)
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+			     struct edp_vsc_psr *vsc)
 {
+	unsigned long timeout;
 	unsigned int val;
+	u8 sink;
 
 	/* don't send info frame */
 	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
@@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
 	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
 	val |= IF_EN;
 	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+
+	timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_COUNT);
+	while (time_before(jiffies, timeout)) {
+		val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
+		if (val != 1) {
+			dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
+			return val;
+		}
+
+		if (vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB ||
+		    !vsc->DB1 && sink == DP_PSR_SINK_INACTIVE)
+			break;
+
+		usleep_range(1000, 1500);
+	}
+
+	dev_warn(dp->dev, "Failed to effect PSR: %x", sink);
+
+	return -ETIMEDOUT;
 }
 
 ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
-- 
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/2] drm/bridge: analogix_dp: Remove duplicated code v2

[Sean Paul]

On Wed, Sep 7, 2016 at 11:48 PM, Yakir Yang <ykk@rock-chips.com> wrote:
> From: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>
> Remove code for reading the EDID and DPCD fields and use the helpers
> instead.
>
> Besides the obvious code reduction, other helpers are being added to the
> core that could be used in this driver and will be good to be able to
> use them instead of duplicating them.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Cc: Yakir Yang <ykk@rock-chips.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Yakir Yang <ykk@rock-chips.com>
> Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Tested-by: Sean Paul <seanpaul@chromium.org>
> ---
> Changes in v2:
> - A bunch of good fixes from Sean and Yakir
> - Moved the transfer function to analogix_dp_reg.c
> - Removed reference to the EDID from the dp struct
> - Rebase on Sean's next tree
>     git://people.freedesktop.org/~seanpaul/dogwood
>


This patch has already been merged to my 'base' branch, I suppose
since no one has picked it up yet, I'll pull it in my rockchip
for-next.

Thanks,

Sean




>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 263 ++++--------
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  41 +-
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 451 ++++++---------------
>  3 files changed, 204 insertions(+), 551 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index efac8ab..5fe3982 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -31,6 +31,7 @@
>  #include <drm/bridge/analogix_dp.h>
>
>  #include "analogix_dp_core.h"
> +#include "analogix_dp_reg.h"
>
>  #define to_dp(nm)      container_of(nm, struct analogix_dp_device, nm)
>
> @@ -174,150 +175,21 @@ static void analogix_dp_enable_sink_psr(struct analogix_dp_device *dp)
>         analogix_dp_enable_psr_crc(dp);
>  }
>
> -static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
> -{
> -       int i;
> -       unsigned char sum = 0;
> -
> -       for (i = 0; i < EDID_BLOCK_LENGTH; i++)
> -               sum = sum + edid_data[i];
> -
> -       return sum;
> -}
> -
> -static int analogix_dp_read_edid(struct analogix_dp_device *dp)
> -{
> -       unsigned char *edid = dp->edid;
> -       unsigned int extend_block = 0;
> -       unsigned char sum;
> -       unsigned char test_vector;
> -       int retval;
> -
> -       /*
> -        * EDID device address is 0x50.
> -        * However, if necessary, you must have set upper address
> -        * into E-EDID in I2C device, 0x30.
> -        */
> -
> -       /* Read Extension Flag, Number of 128-byte EDID extension blocks */
> -       retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
> -                                               EDID_EXTENSION_FLAG,
> -                                               &extend_block);
> -       if (retval)
> -               return retval;
> -
> -       if (extend_block > 0) {
> -               dev_dbg(dp->dev, "EDID data includes a single extension!\n");
> -
> -               /* Read EDID data */
> -               retval = analogix_dp_read_bytes_from_i2c(dp,
> -                                               I2C_EDID_DEVICE_ADDR,
> -                                               EDID_HEADER_PATTERN,
> -                                               EDID_BLOCK_LENGTH,
> -                                               &edid[EDID_HEADER_PATTERN]);
> -               if (retval != 0) {
> -                       dev_err(dp->dev, "EDID Read failed!\n");
> -                       return -EIO;
> -               }
> -               sum = analogix_dp_calc_edid_check_sum(edid);
> -               if (sum != 0) {
> -                       dev_err(dp->dev, "EDID bad checksum!\n");
> -                       return -EIO;
> -               }
> -
> -               /* Read additional EDID data */
> -               retval = analogix_dp_read_bytes_from_i2c(dp,
> -                               I2C_EDID_DEVICE_ADDR,
> -                               EDID_BLOCK_LENGTH,
> -                               EDID_BLOCK_LENGTH,
> -                               &edid[EDID_BLOCK_LENGTH]);
> -               if (retval != 0) {
> -                       dev_err(dp->dev, "EDID Read failed!\n");
> -                       return -EIO;
> -               }
> -               sum = analogix_dp_calc_edid_check_sum(&edid[EDID_BLOCK_LENGTH]);
> -               if (sum != 0) {
> -                       dev_err(dp->dev, "EDID bad checksum!\n");
> -                       return -EIO;
> -               }
> -
> -               analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
> -                                               &test_vector);
> -               if (test_vector & DP_TEST_LINK_EDID_READ) {
> -                       analogix_dp_write_byte_to_dpcd(dp,
> -                               DP_TEST_EDID_CHECKSUM,
> -                               edid[EDID_BLOCK_LENGTH + EDID_CHECKSUM]);
> -                       analogix_dp_write_byte_to_dpcd(dp,
> -                               DP_TEST_RESPONSE,
> -                               DP_TEST_EDID_CHECKSUM_WRITE);
> -               }
> -       } else {
> -               dev_info(dp->dev, "EDID data does not include any extensions.\n");
> -
> -               /* Read EDID data */
> -               retval = analogix_dp_read_bytes_from_i2c(dp,
> -                               I2C_EDID_DEVICE_ADDR, EDID_HEADER_PATTERN,
> -                               EDID_BLOCK_LENGTH, &edid[EDID_HEADER_PATTERN]);
> -               if (retval != 0) {
> -                       dev_err(dp->dev, "EDID Read failed!\n");
> -                       return -EIO;
> -               }
> -               sum = analogix_dp_calc_edid_check_sum(edid);
> -               if (sum != 0) {
> -                       dev_err(dp->dev, "EDID bad checksum!\n");
> -                       return -EIO;
> -               }
> -
> -               analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
> -                                               &test_vector);
> -               if (test_vector & DP_TEST_LINK_EDID_READ) {
> -                       analogix_dp_write_byte_to_dpcd(dp,
> -                               DP_TEST_EDID_CHECKSUM, edid[EDID_CHECKSUM]);
> -                       analogix_dp_write_byte_to_dpcd(dp,
> -                               DP_TEST_RESPONSE, DP_TEST_EDID_CHECKSUM_WRITE);
> -               }
> -       }
> -
> -       dev_dbg(dp->dev, "EDID Read success!\n");
> -       return 0;
> -}
> -
> -static int analogix_dp_handle_edid(struct analogix_dp_device *dp)
> -{
> -       u8 buf[12];
> -       int i;
> -       int retval;
> -
> -       /* Read DPCD DP_DPCD_REV~RECEIVE_PORT1_CAP_1 */
> -       retval = analogix_dp_read_bytes_from_dpcd(dp, DP_DPCD_REV, 12, buf);
> -       if (retval)
> -               return retval;
> -
> -       /* Read EDID */
> -       for (i = 0; i < 3; i++) {
> -               retval = analogix_dp_read_edid(dp);
> -               if (!retval)
> -                       break;
> -       }
> -
> -       return retval;
> -}
> -
>  static void
>  analogix_dp_enable_rx_to_enhanced_mode(struct analogix_dp_device *dp,
>                                        bool enable)
>  {
>         u8 data;
>
> -       analogix_dp_read_byte_from_dpcd(dp, DP_LANE_COUNT_SET, &data);
> +       drm_dp_dpcd_readb(&dp->aux, DP_LANE_COUNT_SET, &data);
>
>         if (enable)
> -               analogix_dp_write_byte_to_dpcd(dp, DP_LANE_COUNT_SET,
> -                                              DP_LANE_COUNT_ENHANCED_FRAME_EN |
> -                                              DPCD_LANE_COUNT_SET(data));
> +               drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET,
> +                                  DP_LANE_COUNT_ENHANCED_FRAME_EN |
> +                                       DPCD_LANE_COUNT_SET(data));
>         else
> -               analogix_dp_write_byte_to_dpcd(dp, DP_LANE_COUNT_SET,
> -                                              DPCD_LANE_COUNT_SET(data));
> +               drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET,
> +                                  DPCD_LANE_COUNT_SET(data));
>  }
>
>  static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp)
> @@ -325,7 +197,7 @@ static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp)
>         u8 data;
>         int retval;
>
> -       analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LANE_COUNT, &data);
> +       drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data);
>         retval = DPCD_ENHANCED_FRAME_CAP(data);
>
>         return retval;
> @@ -344,8 +216,8 @@ static void analogix_dp_training_pattern_dis(struct analogix_dp_device *dp)
>  {
>         analogix_dp_set_training_pattern(dp, DP_NONE);
>
> -       analogix_dp_write_byte_to_dpcd(dp, DP_TRAINING_PATTERN_SET,
> -                                      DP_TRAINING_PATTERN_DISABLE);
> +       drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
> +                          DP_TRAINING_PATTERN_DISABLE);
>  }
>
>  static void
> @@ -390,8 +262,8 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
>         /* Setup RX configuration */
>         buf[0] = dp->link_train.link_rate;
>         buf[1] = dp->link_train.lane_count;
> -       retval = analogix_dp_write_bytes_to_dpcd(dp, DP_LINK_BW_SET, 2, buf);
> -       if (retval)
> +       retval = drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, buf, 2);
> +       if (retval < 0)
>                 return retval;
>
>         /* Set TX pre-emphasis to minimum */
> @@ -415,20 +287,22 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
>         analogix_dp_set_training_pattern(dp, TRAINING_PTN1);
>
>         /* Set RX training pattern */
> -       retval = analogix_dp_write_byte_to_dpcd(dp,
> -                       DP_TRAINING_PATTERN_SET,
> -                       DP_LINK_SCRAMBLING_DISABLE | DP_TRAINING_PATTERN_1);
> -       if (retval)
> +       retval = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
> +                                   DP_LINK_SCRAMBLING_DISABLE |
> +                                       DP_TRAINING_PATTERN_1);
> +       if (retval < 0)
>                 return retval;
>
>         for (lane = 0; lane < lane_count; lane++)
>                 buf[lane] = DP_TRAIN_PRE_EMPH_LEVEL_0 |
>                             DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
>
> -       retval = analogix_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET,
> -                                                lane_count, buf);
> +       retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, buf,
> +                                  lane_count);
> +       if (retval < 0)
> +               return retval;
>
> -       return retval;
> +       return 0;
>  }
>
>  static unsigned char analogix_dp_get_lane_status(u8 link_status[2], int lane)
> @@ -580,25 +454,23 @@ static int analogix_dp_process_clock_recovery(struct analogix_dp_device *dp)
>
>         lane_count = dp->link_train.lane_count;
>
> -       retval =  analogix_dp_read_bytes_from_dpcd(dp,
> -                       DP_LANE0_1_STATUS, 2, link_status);
> -       if (retval)
> +       retval = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status, 2);
> +       if (retval < 0)
>                 return retval;
>
> -       retval =  analogix_dp_read_bytes_from_dpcd(dp,
> -                       DP_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
> -       if (retval)
> +       retval = drm_dp_dpcd_read(&dp->aux, DP_ADJUST_REQUEST_LANE0_1,
> +                                 adjust_request, 2);
> +       if (retval < 0)
>                 return retval;
>
>         if (analogix_dp_clock_recovery_ok(link_status, lane_count) == 0) {
>                 /* set training pattern 2 for EQ */
>                 analogix_dp_set_training_pattern(dp, TRAINING_PTN2);
>
> -               retval = analogix_dp_write_byte_to_dpcd(dp,
> -                               DP_TRAINING_PATTERN_SET,
> -                               DP_LINK_SCRAMBLING_DISABLE |
> -                               DP_TRAINING_PATTERN_2);
> -               if (retval)
> +               retval = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
> +                                           DP_LINK_SCRAMBLING_DISABLE |
> +                                               DP_TRAINING_PATTERN_2);
> +               if (retval < 0)
>                         return retval;
>
>                 dev_info(dp->dev, "Link Training Clock Recovery success\n");
> @@ -636,13 +508,12 @@ static int analogix_dp_process_clock_recovery(struct analogix_dp_device *dp)
>                 analogix_dp_set_lane_link_training(dp,
>                         dp->link_train.training_lane[lane], lane);
>
> -       retval = analogix_dp_write_bytes_to_dpcd(dp,
> -                       DP_TRAINING_LANE0_SET, lane_count,
> -                       dp->link_train.training_lane);
> -       if (retval)
> +       retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
> +                                  dp->link_train.training_lane, lane_count);
> +       if (retval < 0)
>                 return retval;
>
> -       return retval;
> +       return 0;
>  }
>
>  static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
> @@ -655,9 +526,8 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
>
>         lane_count = dp->link_train.lane_count;
>
> -       retval = analogix_dp_read_bytes_from_dpcd(dp,
> -                       DP_LANE0_1_STATUS, 2, link_status);
> -       if (retval)
> +       retval = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status, 2);
> +       if (retval < 0)
>                 return retval;
>
>         if (analogix_dp_clock_recovery_ok(link_status, lane_count)) {
> @@ -665,14 +535,13 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
>                 return -EIO;
>         }
>
> -       retval = analogix_dp_read_bytes_from_dpcd(dp,
> -                       DP_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
> -       if (retval)
> +       retval = drm_dp_dpcd_read(&dp->aux, DP_ADJUST_REQUEST_LANE0_1, adjust_request, 2);
> +       if (retval < 0)
>                 return retval;
>
> -       retval = analogix_dp_read_byte_from_dpcd(dp,
> -                       DP_LANE_ALIGN_STATUS_UPDATED, &link_align);
> -       if (retval)
> +       retval = drm_dp_dpcd_readb(&dp->aux, DP_LANE_ALIGN_STATUS_UPDATED,
> +                                  &link_align);
> +       if (retval < 0)
>                 return retval;
>
>         analogix_dp_get_adjust_training_lane(dp, adjust_request);
> @@ -713,10 +582,12 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
>                 analogix_dp_set_lane_link_training(dp,
>                         dp->link_train.training_lane[lane], lane);
>
> -       retval = analogix_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET,
> -                       lane_count, dp->link_train.training_lane);
> +       retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
> +                                  dp->link_train.training_lane, lane_count);
> +       if (retval < 0)
> +               return retval;
>
> -       return retval;
> +       return 0;
>  }
>
>  static void analogix_dp_get_max_rx_bandwidth(struct analogix_dp_device *dp,
> @@ -730,7 +601,7 @@ static void analogix_dp_get_max_rx_bandwidth(struct analogix_dp_device *dp,
>          * For DP rev.1.2, Maximum link rate of Main Link lanes
>          * 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps, 0x14 = 5.4Gbps
>          */
> -       analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LINK_RATE, &data);
> +       drm_dp_dpcd_readb(&dp->aux, DP_MAX_LINK_RATE, &data);
>         *bandwidth = data;
>  }
>
> @@ -743,7 +614,7 @@ static void analogix_dp_get_max_rx_lane_count(struct analogix_dp_device *dp,
>          * For DP rev.1.1, Maximum number of Main Link lanes
>          * 0x01 = 1 lane, 0x02 = 2 lanes, 0x04 = 4 lanes
>          */
> -       analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LANE_COUNT, &data);
> +       drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data);
>         *lane_count = DPCD_MAX_LANE_COUNT(data);
>  }
>
> @@ -912,19 +783,15 @@ static void analogix_dp_enable_scramble(struct analogix_dp_device *dp,
>         if (enable) {
>                 analogix_dp_enable_scrambling(dp);
>
> -               analogix_dp_read_byte_from_dpcd(dp, DP_TRAINING_PATTERN_SET,
> -                                               &data);
> -               analogix_dp_write_byte_to_dpcd(dp,
> -                       DP_TRAINING_PATTERN_SET,
> -                       (u8)(data & ~DP_LINK_SCRAMBLING_DISABLE));
> +               drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data);
> +               drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
> +                                  (u8)(data & ~DP_LINK_SCRAMBLING_DISABLE));
>         } else {
>                 analogix_dp_disable_scrambling(dp);
>
> -               analogix_dp_read_byte_from_dpcd(dp, DP_TRAINING_PATTERN_SET,
> -                                               &data);
> -               analogix_dp_write_byte_to_dpcd(dp,
> -                       DP_TRAINING_PATTERN_SET,
> -                       (u8)(data | DP_LINK_SCRAMBLING_DISABLE));
> +               drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data);
> +               drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
> +                                  (u8)(data | DP_LINK_SCRAMBLING_DISABLE));
>         }
>  }
>
> @@ -1053,7 +920,7 @@ out:
>  int analogix_dp_get_modes(struct drm_connector *connector)
>  {
>         struct analogix_dp_device *dp = to_dp(connector);
> -       struct edid *edid = (struct edid *)dp->edid;
> +       struct edid *edid;
>         int ret, num_modes = 0;
>
>         ret = analogix_dp_prepare_panel(dp, true, false);
> @@ -1062,9 +929,11 @@ int analogix_dp_get_modes(struct drm_connector *connector)
>                 return 0;
>         }
>
> -       if (analogix_dp_handle_edid(dp) == 0) {
> +       edid = drm_get_edid(connector, &dp->aux.ddc);
> +       if (edid) {
>                 drm_mode_connector_update_edid_property(&dp->connector, edid);
>                 num_modes += drm_add_edid_modes(&dp->connector, edid);
> +               kfree(edid);
>         }
>
>         if (dp->plat_data->panel)
> @@ -1395,6 +1264,14 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
>         return 0;
>  }
>
> +static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux,
> +                                      struct drm_dp_aux_msg *msg)
> +{
> +       struct analogix_dp_device *dp = to_dp(aux);
> +
> +       return analogix_dp_transfer(dp, msg);
> +}
> +
>  int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
>                      struct analogix_dp_plat_data *plat_data)
>  {
> @@ -1515,6 +1392,14 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
>         dp->drm_dev = drm_dev;
>         dp->encoder = dp->plat_data->encoder;
>
> +       dp->aux.name = "DP-AUX";
> +       dp->aux.transfer = analogix_dpaux_transfer;
> +       dp->aux.dev = &pdev->dev;
> +
> +       ret = drm_dp_aux_register(&dp->aux);
> +       if (ret)
> +               goto err_disable_pm_runtime;
> +
>         ret = analogix_dp_create_bridge(drm_dev, dp);
>         if (ret) {
>                 DRM_ERROR("failed to create bridge (%d)\n", ret);
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index 473b980..a15f076 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -20,15 +20,6 @@
>  #define MAX_CR_LOOP 5
>  #define MAX_EQ_LOOP 5
>
> -/* I2C EDID Chip ID, Slave Address */
> -#define I2C_EDID_DEVICE_ADDR                   0x50
> -#define I2C_E_EDID_DEVICE_ADDR                 0x30
> -
> -#define EDID_BLOCK_LENGTH                      0x80
> -#define EDID_HEADER_PATTERN                    0x00
> -#define EDID_EXTENSION_FLAG                    0x7e
> -#define EDID_CHECKSUM                          0x7f
> -
>  /* DP_MAX_LANE_COUNT */
>  #define DPCD_ENHANCED_FRAME_CAP(x)             (((x) >> 7) & 0x1)
>  #define DPCD_MAX_LANE_COUNT(x)                 ((x) & 0x1f)
> @@ -166,6 +157,7 @@ struct analogix_dp_device {
>         struct drm_device       *drm_dev;
>         struct drm_connector    connector;
>         struct drm_bridge       *bridge;
> +       struct drm_dp_aux       aux;
>         struct clk              *clock;
>         unsigned int            irq;
>         void __iomem            *reg_base;
> @@ -176,7 +168,6 @@ struct analogix_dp_device {
>         int                     dpms_mode;
>         int                     hpd_gpio;
>         bool                    force_hpd;
> -       unsigned char           edid[EDID_BLOCK_LENGTH * 2];
>         bool                    psr_support;
>
>         struct mutex            panel_lock;
> @@ -210,33 +201,6 @@ void analogix_dp_reset_aux(struct analogix_dp_device *dp);
>  void analogix_dp_init_aux(struct analogix_dp_device *dp);
>  int analogix_dp_get_plug_in_status(struct analogix_dp_device *dp);
>  void analogix_dp_enable_sw_function(struct analogix_dp_device *dp);
> -int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp);
> -int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp,
> -                                  unsigned int reg_addr,
> -                                  unsigned char data);
> -int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp,
> -                                   unsigned int reg_addr,
> -                                   unsigned char *data);
> -int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
> -                                   unsigned int reg_addr,
> -                                   unsigned int count,
> -                                   unsigned char data[]);
> -int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp,
> -                                    unsigned int reg_addr,
> -                                    unsigned int count,
> -                                    unsigned char data[]);
> -int analogix_dp_select_i2c_device(struct analogix_dp_device *dp,
> -                                 unsigned int device_addr,
> -                                 unsigned int reg_addr);
> -int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
> -                                  unsigned int device_addr,
> -                                  unsigned int reg_addr,
> -                                  unsigned int *data);
> -int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp,
> -                                   unsigned int device_addr,
> -                                   unsigned int reg_addr,
> -                                   unsigned int count,
> -                                   unsigned char edid[]);
>  void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype);
>  void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype);
>  void analogix_dp_set_lane_count(struct analogix_dp_device *dp, u32 count);
> @@ -285,5 +249,6 @@ void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>  void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
>  void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>                               struct edp_vsc_psr *vsc);
> -
> +ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> +                            struct drm_dp_aux_msg *msg);
>  #endif /* _ANALOGIX_DP_CORE_H */
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index 52c1b6b..a4d17b8 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -585,330 +585,6 @@ int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp,
>         return retval;
>  }
>
> -int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp,
> -                                   unsigned int reg_addr,
> -                                   unsigned char *data)
> -{
> -       u32 reg;
> -       int i;
> -       int retval;
> -
> -       for (i = 0; i < 3; i++) {
> -               /* Clear AUX CH data buffer */
> -               reg = BUF_CLR;
> -               writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
> -
> -               /* Select DPCD device address */
> -               reg = AUX_ADDR_7_0(reg_addr);
> -               writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
> -               reg = AUX_ADDR_15_8(reg_addr);
> -               writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
> -               reg = AUX_ADDR_19_16(reg_addr);
> -               writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
> -
> -               /*
> -                * Set DisplayPort transaction and read 1 byte
> -                * If bit 3 is 1, DisplayPort transaction.
> -                * If Bit 3 is 0, I2C transaction.
> -                */
> -               reg = AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_READ;
> -               writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
> -
> -               /* Start AUX transaction */
> -               retval = analogix_dp_start_aux_transaction(dp);
> -               if (retval == 0)
> -                       break;
> -
> -               dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
> -       }
> -
> -       /* Read data buffer */
> -       reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0);
> -       *data = (unsigned char)(reg & 0xff);
> -
> -       return retval;
> -}
> -
> -int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
> -                                   unsigned int reg_addr,
> -                                   unsigned int count,
> -                                   unsigned char data[])
> -{
> -       u32 reg;
> -       unsigned int start_offset;
> -       unsigned int cur_data_count;
> -       unsigned int cur_data_idx;
> -       int i;
> -       int retval = 0;
> -
> -       /* Clear AUX CH data buffer */
> -       reg = BUF_CLR;
> -       writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
> -
> -       start_offset = 0;
> -       while (start_offset < count) {
> -               /* Buffer size of AUX CH is 16 * 4bytes */
> -               if ((count - start_offset) > 16)
> -                       cur_data_count = 16;
> -               else
> -                       cur_data_count = count - start_offset;
> -
> -               for (i = 0; i < 3; i++) {
> -                       /* Select DPCD device address */
> -                       reg = AUX_ADDR_7_0(reg_addr + start_offset);
> -                       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
> -                       reg = AUX_ADDR_15_8(reg_addr + start_offset);
> -                       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
> -                       reg = AUX_ADDR_19_16(reg_addr + start_offset);
> -                       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
> -
> -                       for (cur_data_idx = 0; cur_data_idx < cur_data_count;
> -                            cur_data_idx++) {
> -                               reg = data[start_offset + cur_data_idx];
> -                               writel(reg, dp->reg_base +
> -                                      ANALOGIX_DP_BUF_DATA_0 +
> -                                      4 * cur_data_idx);
> -                       }
> -
> -                       /*
> -                        * Set DisplayPort transaction and write
> -                        * If bit 3 is 1, DisplayPort transaction.
> -                        * If Bit 3 is 0, I2C transaction.
> -                        */
> -                       reg = AUX_LENGTH(cur_data_count) |
> -                               AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_WRITE;
> -                       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
> -
> -                       /* Start AUX transaction */
> -                       retval = analogix_dp_start_aux_transaction(dp);
> -                       if (retval == 0)
> -                               break;
> -
> -                       dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
> -                               __func__);
> -               }
> -
> -               start_offset += cur_data_count;
> -       }
> -
> -       return retval;
> -}
> -
> -int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp,
> -                                    unsigned int reg_addr,
> -                                    unsigned int count,
> -                                    unsigned char data[])
> -{
> -       u32 reg;
> -       unsigned int start_offset;
> -       unsigned int cur_data_count;
> -       unsigned int cur_data_idx;
> -       int i;
> -       int retval = 0;
> -
> -       /* Clear AUX CH data buffer */
> -       reg = BUF_CLR;
> -       writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
> -
> -       start_offset = 0;
> -       while (start_offset < count) {
> -               /* Buffer size of AUX CH is 16 * 4bytes */
> -               if ((count - start_offset) > 16)
> -                       cur_data_count = 16;
> -               else
> -                       cur_data_count = count - start_offset;
> -
> -               /* AUX CH Request Transaction process */
> -               for (i = 0; i < 3; i++) {
> -                       /* Select DPCD device address */
> -                       reg = AUX_ADDR_7_0(reg_addr + start_offset);
> -                       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
> -                       reg = AUX_ADDR_15_8(reg_addr + start_offset);
> -                       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
> -                       reg = AUX_ADDR_19_16(reg_addr + start_offset);
> -                       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
> -
> -                       /*
> -                        * Set DisplayPort transaction and read
> -                        * If bit 3 is 1, DisplayPort transaction.
> -                        * If Bit 3 is 0, I2C transaction.
> -                        */
> -                       reg = AUX_LENGTH(cur_data_count) |
> -                               AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_READ;
> -                       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
> -
> -                       /* Start AUX transaction */
> -                       retval = analogix_dp_start_aux_transaction(dp);
> -                       if (retval == 0)
> -                               break;
> -
> -                       dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
> -                               __func__);
> -               }
> -
> -               for (cur_data_idx = 0; cur_data_idx < cur_data_count;
> -                   cur_data_idx++) {
> -                       reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0
> -                                                + 4 * cur_data_idx);
> -                       data[start_offset + cur_data_idx] =
> -                               (unsigned char)reg;
> -               }
> -
> -               start_offset += cur_data_count;
> -       }
> -
> -       return retval;
> -}
> -
> -int analogix_dp_select_i2c_device(struct analogix_dp_device *dp,
> -                                 unsigned int device_addr,
> -                                 unsigned int reg_addr)
> -{
> -       u32 reg;
> -       int retval;
> -
> -       /* Set EDID device address */
> -       reg = device_addr;
> -       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
> -       writel(0x0, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
> -       writel(0x0, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
> -
> -       /* Set offset from base address of EDID device */
> -       writel(reg_addr, dp->reg_base + ANALOGIX_DP_BUF_DATA_0);
> -
> -       /*
> -        * Set I2C transaction and write address
> -        * If bit 3 is 1, DisplayPort transaction.
> -        * If Bit 3 is 0, I2C transaction.
> -        */
> -       reg = AUX_TX_COMM_I2C_TRANSACTION | AUX_TX_COMM_MOT |
> -               AUX_TX_COMM_WRITE;
> -       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
> -
> -       /* Start AUX transaction */
> -       retval = analogix_dp_start_aux_transaction(dp);
> -       if (retval != 0)
> -               dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
> -
> -       return retval;
> -}
> -
> -int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
> -                                  unsigned int device_addr,
> -                                  unsigned int reg_addr,
> -                                  unsigned int *data)
> -{
> -       u32 reg;
> -       int i;
> -       int retval;
> -
> -       for (i = 0; i < 3; i++) {
> -               /* Clear AUX CH data buffer */
> -               reg = BUF_CLR;
> -               writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
> -
> -               /* Select EDID device */
> -               retval = analogix_dp_select_i2c_device(dp, device_addr,
> -                                                      reg_addr);
> -               if (retval != 0)
> -                       continue;
> -
> -               /*
> -                * Set I2C transaction and read data
> -                * If bit 3 is 1, DisplayPort transaction.
> -                * If Bit 3 is 0, I2C transaction.
> -                */
> -               reg = AUX_TX_COMM_I2C_TRANSACTION |
> -                       AUX_TX_COMM_READ;
> -               writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
> -
> -               /* Start AUX transaction */
> -               retval = analogix_dp_start_aux_transaction(dp);
> -               if (retval == 0)
> -                       break;
> -
> -               dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
> -       }
> -
> -       /* Read data */
> -       if (retval == 0)
> -               *data = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0);
> -
> -       return retval;
> -}
> -
> -int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp,
> -                                   unsigned int device_addr,
> -                                   unsigned int reg_addr,
> -                                   unsigned int count,
> -                                   unsigned char edid[])
> -{
> -       u32 reg;
> -       unsigned int i, j;
> -       unsigned int cur_data_idx;
> -       unsigned int defer = 0;
> -       int retval = 0;
> -
> -       for (i = 0; i < count; i += 16) {
> -               for (j = 0; j < 3; j++) {
> -                       /* Clear AUX CH data buffer */
> -                       reg = BUF_CLR;
> -                       writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
> -
> -                       /* Set normal AUX CH command */
> -                       reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
> -                       reg &= ~ADDR_ONLY;
> -                       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
> -
> -                       /*
> -                        * If Rx sends defer, Tx sends only reads
> -                        * request without sending address
> -                        */
> -                       if (!defer)
> -                               retval = analogix_dp_select_i2c_device(dp,
> -                                               device_addr, reg_addr + i);
> -                       else
> -                               defer = 0;
> -
> -                       if (retval == 0) {
> -                               /*
> -                                * Set I2C transaction and write data
> -                                * If bit 3 is 1, DisplayPort transaction.
> -                                * If Bit 3 is 0, I2C transaction.
> -                                */
> -                               reg = AUX_LENGTH(16) |
> -                                       AUX_TX_COMM_I2C_TRANSACTION |
> -                                       AUX_TX_COMM_READ;
> -                               writel(reg, dp->reg_base +
> -                                       ANALOGIX_DP_AUX_CH_CTL_1);
> -
> -                               /* Start AUX transaction */
> -                               retval = analogix_dp_start_aux_transaction(dp);
> -                               if (retval == 0)
> -                                       break;
> -
> -                               dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
> -                                       __func__);
> -                       }
> -                       /* Check if Rx sends defer */
> -                       reg = readl(dp->reg_base + ANALOGIX_DP_AUX_RX_COMM);
> -                       if (reg == AUX_RX_COMM_AUX_DEFER ||
> -                           reg == AUX_RX_COMM_I2C_DEFER) {
> -                               dev_err(dp->dev, "Defer: %d\n\n", reg);
> -                               defer = 1;
> -                       }
> -               }
> -
> -               for (cur_data_idx = 0; cur_data_idx < 16; cur_data_idx++) {
> -                       reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0
> -                                                + 4 * cur_data_idx);
> -                       edid[i + cur_data_idx] = (unsigned char)reg;
> -               }
> -       }
> -
> -       return retval;
> -}
> -
>  void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype)
>  {
>         u32 reg;
> @@ -1373,3 +1049,130 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>         val |= IF_EN;
>         writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>  }
> +
> +ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> +                            struct drm_dp_aux_msg *msg)
> +{
> +       u32 reg;
> +       u8 *buffer = msg->buffer;
> +       int timeout_loop = 0;
> +       unsigned int i;
> +       int num_transferred = 0;
> +
> +       /* Buffer size of AUX CH is 16 bytes */
> +       if (WARN_ON(msg->size > 16))
> +               return -E2BIG;
> +
> +       /* Clear AUX CH data buffer */
> +       reg = BUF_CLR;
> +       writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
> +
> +       switch (msg->request & ~DP_AUX_I2C_MOT) {
> +       case DP_AUX_I2C_WRITE:
> +               reg = AUX_TX_COMM_WRITE | AUX_TX_COMM_I2C_TRANSACTION;
> +               if (msg->request & DP_AUX_I2C_MOT)
> +                       reg |= AUX_TX_COMM_MOT;
> +               break;
> +
> +       case DP_AUX_I2C_READ:
> +               reg = AUX_TX_COMM_READ | AUX_TX_COMM_I2C_TRANSACTION;
> +               if (msg->request & DP_AUX_I2C_MOT)
> +                       reg |= AUX_TX_COMM_MOT;
> +               break;
> +
> +       case DP_AUX_NATIVE_WRITE:
> +               reg = AUX_TX_COMM_WRITE | AUX_TX_COMM_DP_TRANSACTION;
> +               break;
> +
> +       case DP_AUX_NATIVE_READ:
> +               reg = AUX_TX_COMM_READ | AUX_TX_COMM_DP_TRANSACTION;
> +               break;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       reg |= AUX_LENGTH(msg->size);
> +       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1);
> +
> +       /* Select DPCD device address */
> +       reg = AUX_ADDR_7_0(msg->address);
> +       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0);
> +       reg = AUX_ADDR_15_8(msg->address);
> +       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8);
> +       reg = AUX_ADDR_19_16(msg->address);
> +       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16);
> +
> +       if (!(msg->request & DP_AUX_I2C_READ)) {
> +               for (i = 0; i < msg->size; i++) {
> +                       reg = buffer[i];
> +                       writel(reg, dp->reg_base + ANALOGIX_DP_BUF_DATA_0 +
> +                              4 * i);
> +                       num_transferred++;
> +               }
> +       }
> +
> +       /* Enable AUX CH operation */
> +       reg = AUX_EN;
> +
> +       /* Zero-sized messages specify address-only transactions. */
> +       if (msg->size < 1)
> +               reg |= ADDR_ONLY;
> +
> +       writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
> +
> +       /* Is AUX CH command reply received? */
> +       /* TODO: Wait for an interrupt instead of looping? */
> +       reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
> +       while (!(reg & RPLY_RECEIV)) {
> +               timeout_loop++;
> +               if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
> +                       dev_err(dp->dev, "AUX CH command reply failed!\n");
> +                       return -ETIMEDOUT;
> +               }
> +               reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
> +               usleep_range(10, 11);
> +       }
> +
> +       /* Clear interrupt source for AUX CH command reply */
> +       writel(RPLY_RECEIV, dp->reg_base + ANALOGIX_DP_INT_STA);
> +
> +       /* Clear interrupt source for AUX CH access error */
> +       reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
> +       if (reg & AUX_ERR) {
> +               writel(AUX_ERR, dp->reg_base + ANALOGIX_DP_INT_STA);
> +               return -EREMOTEIO;
> +       }
> +
> +       /* Check AUX CH error access status */
> +       reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_STA);
> +       if ((reg & AUX_STATUS_MASK)) {
> +               dev_err(dp->dev, "AUX CH error happened: %d\n\n",
> +                       reg & AUX_STATUS_MASK);
> +               return -EREMOTEIO;
> +       }
> +
> +       if (msg->request & DP_AUX_I2C_READ) {
> +               for (i = 0; i < msg->size; i++) {
> +                       reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0 +
> +                                   4 * i);
> +                       buffer[i] = (unsigned char)reg;
> +                       num_transferred++;
> +               }
> +       }
> +
> +       /* Check if Rx sends defer */
> +       reg = readl(dp->reg_base + ANALOGIX_DP_AUX_RX_COMM);
> +       if (reg == AUX_RX_COMM_AUX_DEFER)
> +               msg->reply = DP_AUX_NATIVE_REPLY_DEFER;
> +       else if (reg == AUX_RX_COMM_I2C_DEFER)
> +               msg->reply = DP_AUX_I2C_REPLY_DEFER;
> +       else if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE ||
> +                (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_READ)
> +               msg->reply = DP_AUX_I2C_REPLY_ACK;
> +       else if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_WRITE ||
> +                (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_READ)
> +               msg->reply = DP_AUX_NATIVE_REPLY_ACK;
> +
> +       return num_transferred;
> +}
> --
> 1.9.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR

[Sean Paul]

On Wed, Sep 7, 2016 at 11:48 PM, Yakir Yang <ykk@rock-chips.com> wrote:
> Make sure the request PSR state could effect in analogix_dp_send_psr_spd()
> function, or printing the error Sink PSR state if we failed to effect
> the request PSR setting.
>


Let's change to:

Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
function, or print the sink PSR error state if we failed to apply the
requested PSR
setting.

> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
> Changes in v2:
> - A bunch of good fixes from Sean
>
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  6 ++----
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  4 ++--
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 25 ++++++++++++++++++++--
>  3 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 5fe3982..c0ce16a 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
>         psr_vsc.DB0 = 0;
>         psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>
> -       analogix_dp_send_psr_spd(dp, &psr_vsc);
> -       return 0;
> +       return analogix_dp_send_psr_spd(dp, &psr_vsc);
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>
> @@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
>         psr_vsc.DB0 = 0;
>         psr_vsc.DB1 = 0;
>
> -       analogix_dp_send_psr_spd(dp, &psr_vsc);
> -       return 0;
> +       return analogix_dp_send_psr_spd(dp, &psr_vsc);
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index a15f076..6c07a50 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -247,8 +247,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>  void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>  void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>  void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> -                             struct edp_vsc_psr *vsc);
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> +                            struct edp_vsc_psr *vsc);
>  ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>                              struct drm_dp_aux_msg *msg);
>  #endif /* _ANALOGIX_DP_CORE_H */
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index a4d17b8..09d703b 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
>         writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
>  }
>
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> -                             struct edp_vsc_psr *vsc)
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> +                            struct edp_vsc_psr *vsc)
>  {
> +       unsigned long timeout;
>         unsigned int val;
> +       u8 sink;
>
>         /* don't send info frame */
>         val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> @@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>         val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>         val |= IF_EN;
>         writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +
> +       timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_COUNT);

Mismatched units here. DP_TIMEOUT_LOOP_COUNT is defined as number of
retries, whereas you're using it as number of ms. Fortunately, the
retry number is so high that this works out :)

In a separate patch preceding this one, can you change
DP_TIMEOUT_LOOP_COUNT to DP_TIMEOUT_LOOP_MS and alter the other
timeout loops to use time_before() like this one instead of blindly
looping 100 times? After that, you can use DP_TIMEOUT_LOOP_MS here.

> +       while (time_before(jiffies, timeout)) {
> +               val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
> +               if (val != 1) {
> +                       dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
> +                       return val;

Ok, since this is my snippet this comment is my fault, and I apologize
for that :). However, this could return 0. If drm_dp_dpcd_readb
returns 0, you probably want to retry (same as -EBUSY).

> +               }
> +
> +               if (vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB ||
> +                   !vsc->DB1 && sink == DP_PSR_SINK_INACTIVE)
> +                       break;
> +
> +               usleep_range(1000, 1500);
> +       }
> +
> +       dev_warn(dp->dev, "Failed to effect PSR: %x", sink);

Nit: I think you want to say "PSR failed to take effect" or "Failed to
apply PSR"

Sean

> +
> +       return -ETIMEDOUT;
>  }
>
>  ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> --
> 1.9.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR

[Yakir Yang]

On 09/08/2016 10:12 PM, Sean Paul wrote:
> On Wed, Sep 7, 2016 at 11:48 PM, Yakir Yang <ykk@rock-chips.com> wrote:
>> Make sure the request PSR state could effect in analogix_dp_send_psr_spd()
>> function, or printing the error Sink PSR state if we failed to effect
>> the request PSR setting.
>>
>
> Let's change to:
>
> Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
> function, or print the sink PSR error state if we failed to apply the
> requested PSR
> setting.
Done,
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> ---
>> Changes in v2:
>> - A bunch of good fixes from Sean
>>
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  6 ++----
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  4 ++--
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 25 ++++++++++++++++++++--
>>   3 files changed, 27 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 5fe3982..c0ce16a 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
>>          psr_vsc.DB0 = 0;
>>          psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>>
>> -       analogix_dp_send_psr_spd(dp, &psr_vsc);
>> -       return 0;
>> +       return analogix_dp_send_psr_spd(dp, &psr_vsc);
>>   }
>>   EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>>
>> @@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
>>          psr_vsc.DB0 = 0;
>>          psr_vsc.DB1 = 0;
>>
>> -       analogix_dp_send_psr_spd(dp, &psr_vsc);
>> -       return 0;
>> +       return analogix_dp_send_psr_spd(dp, &psr_vsc);
>>   }
>>   EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> index a15f076..6c07a50 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> @@ -247,8 +247,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>>   void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>>   void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>>   void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
>> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> -                             struct edp_vsc_psr *vsc);
>> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> +                            struct edp_vsc_psr *vsc);
>>   ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>>                               struct drm_dp_aux_msg *msg);
>>   #endif /* _ANALOGIX_DP_CORE_H */
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> index a4d17b8..09d703b 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> @@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
>>          writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
>>   }
>>
>> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> -                             struct edp_vsc_psr *vsc)
>> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> +                            struct edp_vsc_psr *vsc)
>>   {
>> +       unsigned long timeout;
>>          unsigned int val;
>> +       u8 sink;
>>
>>          /* don't send info frame */
>>          val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> @@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>>          val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>>          val |= IF_EN;
>>          writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> +
>> +       timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_COUNT);
> Mismatched units here. DP_TIMEOUT_LOOP_COUNT is defined as number of
> retries, whereas you're using it as number of ms. Fortunately, the
> retry number is so high that this works out :)
>
> In a separate patch preceding this one, can you change
> DP_TIMEOUT_LOOP_COUNT to DP_TIMEOUT_LOOP_MS and alter the other
> timeout loops to use time_before() like this one instead of blindly
> looping 100 times? After that, you can use DP_TIMEOUT_LOOP_MS here.

Done, and after do some experiments, I found we need to set the timeout 
to 300ms. Cause in some case we would take about 290ms here to get the 
right psr state.


>> +       while (time_before(jiffies, timeout)) {
>> +               val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
>> +               if (val != 1) {
>> +                       dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
>> +                       return val;
> Ok, since this is my snippet this comment is my fault, and I apologize
> for that :). However, this could return 0. If drm_dp_dpcd_readb
> returns 0, you probably want to retry (same as -EBUSY).
done, just return -EBUSY
>
>> +               }
>> +
>> +               if (vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB ||
>> +                   !vsc->DB1 && sink == DP_PSR_SINK_INACTIVE)
>> +                       break;
>> +
>> +               usleep_range(1000, 1500);
>> +       }
>> +
>> +       dev_warn(dp->dev, "Failed to effect PSR: %x", sink);
> Nit: I think you want to say "PSR failed to take effect" or "Failed to
> apply PSR"
Done

- Yakir
> Sean
>
>> +
>> +       return -ETIMEDOUT;
>>   }
>>
>>   ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>> --
>> 1.9.1
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

Re: [PATCH v2 2/2] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR

[Yakir Yang]

On 09/08/2016 10:12 PM, Sean Paul wrote:
> On Wed, Sep 7, 2016 at 11:48 PM, Yakir Yang <ykk@rock-chips.com> wrote:
>> Make sure the request PSR state could effect in analogix_dp_send_psr_spd()
>> function, or printing the error Sink PSR state if we failed to effect
>> the request PSR setting.
>>
>
> Let's change to:
>
> Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
> function, or print the sink PSR error state if we failed to apply the
> requested PSR
> setting.
Done,
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> ---
>> Changes in v2:
>> - A bunch of good fixes from Sean
>>
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  6 ++----
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  4 ++--
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 25 ++++++++++++++++++++--
>>   3 files changed, 27 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 5fe3982..c0ce16a 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
>>          psr_vsc.DB0 = 0;
>>          psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>>
>> -       analogix_dp_send_psr_spd(dp, &psr_vsc);
>> -       return 0;
>> +       return analogix_dp_send_psr_spd(dp, &psr_vsc);
>>   }
>>   EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>>
>> @@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
>>          psr_vsc.DB0 = 0;
>>          psr_vsc.DB1 = 0;
>>
>> -       analogix_dp_send_psr_spd(dp, &psr_vsc);
>> -       return 0;
>> +       return analogix_dp_send_psr_spd(dp, &psr_vsc);
>>   }
>>   EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> index a15f076..6c07a50 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> @@ -247,8 +247,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>>   void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>>   void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>>   void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
>> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> -                             struct edp_vsc_psr *vsc);
>> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> +                            struct edp_vsc_psr *vsc);
>>   ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>>                               struct drm_dp_aux_msg *msg);
>>   #endif /* _ANALOGIX_DP_CORE_H */
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> index a4d17b8..09d703b 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> @@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
>>          writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
>>   }
>>
>> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> -                             struct edp_vsc_psr *vsc)
>> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> +                            struct edp_vsc_psr *vsc)
>>   {
>> +       unsigned long timeout;
>>          unsigned int val;
>> +       u8 sink;
>>
>>          /* don't send info frame */
>>          val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> @@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>>          val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>>          val |= IF_EN;
>>          writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> +
>> +       timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_COUNT);
> Mismatched units here. DP_TIMEOUT_LOOP_COUNT is defined as number of
> retries, whereas you're using it as number of ms. Fortunately, the
> retry number is so high that this works out :)
>
> In a separate patch preceding this one, can you change
> DP_TIMEOUT_LOOP_COUNT to DP_TIMEOUT_LOOP_MS and alter the other
> timeout loops to use time_before() like this one instead of blindly
> looping 100 times? After that, you can use DP_TIMEOUT_LOOP_MS here.

Done, and after do some experiments, I found we need to set the timeout 
to 300ms. Cause in some case we would take about 290ms here to get the 
right psr state.


>> +       while (time_before(jiffies, timeout)) {
>> +               val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
>> +               if (val != 1) {
>> +                       dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
>> +                       return val;
> Ok, since this is my snippet this comment is my fault, and I apologize
> for that :). However, this could return 0. If drm_dp_dpcd_readb
> returns 0, you probably want to retry (same as -EBUSY).
done, just return -EBUSY
>
>> +               }
>> +
>> +               if (vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB ||
>> +                   !vsc->DB1 && sink == DP_PSR_SINK_INACTIVE)
>> +                       break;
>> +
>> +               usleep_range(1000, 1500);
>> +       }
>> +
>> +       dev_warn(dp->dev, "Failed to effect PSR: %x", sink);
> Nit: I think you want to say "PSR failed to take effect" or "Failed to
> apply PSR"
Done

- Yakir
> Sean
>
>> +
>> +       return -ETIMEDOUT;
>>   }
>>
>>   ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>> --
>> 1.9.1
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 2/3] drm/bridge: analogix_dp: use jiffies to simulate timeout loop

[Yakir Yang]

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v3:
- Suggested by Sean

Changes in v2: None

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  3 ++-
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 18 +++++++++---------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index a15f076..d564e90 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -16,10 +16,11 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_dp_helper.h>
 
-#define DP_TIMEOUT_LOOP_COUNT 100
 #define MAX_CR_LOOP 5
 #define MAX_EQ_LOOP 5
 
+#define DP_TIMEOUT_LOOP_MS			msecs_to_jiffies(1)
+
 /* DP_MAX_LANE_COUNT */
 #define DPCD_ENHANCED_FRAME_CAP(x)		(((x) >> 7) & 0x1)
 #define DPCD_MAX_LANE_COUNT(x)			((x) & 0x1f)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index a4d17b8..15a4cf0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -335,7 +335,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
 void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
 {
 	u32 reg;
-	int timeout_loop = 0;
+	unsigned long timeout;
 
 	analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
 
@@ -350,9 +350,9 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
 	if (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
 		analogix_dp_set_pll_power_down(dp, 0);
 
+		timeout = jiffies + DP_TIMEOUT_LOOP_MS;
 		while (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
-			timeout_loop++;
-			if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
+			if (time_after(jiffies, timeout)) {
 				dev_err(dp->dev, "failed to get pll lock status\n");
 				return;
 			}
@@ -501,7 +501,7 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
 {
 	int reg;
 	int retval = 0;
-	int timeout_loop = 0;
+	unsigned long timeout;
 
 	/* Enable AUX CH operation */
 	reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
@@ -509,10 +509,10 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
 	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
 
 	/* Is AUX CH command reply received? */
+	timeout = jiffies + DP_TIMEOUT_LOOP_MS;
 	reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
 	while (!(reg & RPLY_RECEIV)) {
-		timeout_loop++;
-		if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
+		if (time_after(jiffies, timeout)) {
 			dev_err(dp->dev, "AUX CH command reply failed!\n");
 			return -ETIMEDOUT;
 		}
@@ -1055,7 +1055,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
 {
 	u32 reg;
 	u8 *buffer = msg->buffer;
-	int timeout_loop = 0;
+	unsigned long timeout;
 	unsigned int i;
 	int num_transferred = 0;
 
@@ -1123,10 +1123,10 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
 
 	/* Is AUX CH command reply received? */
 	/* TODO: Wait for an interrupt instead of looping? */
+	timeout = jiffies + DP_TIMEOUT_LOOP_MS;
 	reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
 	while (!(reg & RPLY_RECEIV)) {
-		timeout_loop++;
-		if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
+		if (time_after(jiffies, timeout)) {
 			dev_err(dp->dev, "AUX CH command reply failed!\n");
 			return -ETIMEDOUT;
 		}
-- 
1.9.1

[PATCH v3 2/3] drm/bridge: analogix_dp: use jiffies to simulate timeout loop

[Yakir Yang]

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v3:
- Suggested by Sean

Changes in v2: None

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  3 ++-
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 18 +++++++++---------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index a15f076..d564e90 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -16,10 +16,11 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_dp_helper.h>
 
-#define DP_TIMEOUT_LOOP_COUNT 100
 #define MAX_CR_LOOP 5
 #define MAX_EQ_LOOP 5
 
+#define DP_TIMEOUT_LOOP_MS			msecs_to_jiffies(1)
+
 /* DP_MAX_LANE_COUNT */
 #define DPCD_ENHANCED_FRAME_CAP(x)		(((x) >> 7) & 0x1)
 #define DPCD_MAX_LANE_COUNT(x)			((x) & 0x1f)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index a4d17b8..15a4cf0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -335,7 +335,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
 void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
 {
 	u32 reg;
-	int timeout_loop = 0;
+	unsigned long timeout;
 
 	analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
 
@@ -350,9 +350,9 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
 	if (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
 		analogix_dp_set_pll_power_down(dp, 0);
 
+		timeout = jiffies + DP_TIMEOUT_LOOP_MS;
 		while (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
-			timeout_loop++;
-			if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
+			if (time_after(jiffies, timeout)) {
 				dev_err(dp->dev, "failed to get pll lock status\n");
 				return;
 			}
@@ -501,7 +501,7 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
 {
 	int reg;
 	int retval = 0;
-	int timeout_loop = 0;
+	unsigned long timeout;
 
 	/* Enable AUX CH operation */
 	reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
@@ -509,10 +509,10 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
 	writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
 
 	/* Is AUX CH command reply received? */
+	timeout = jiffies + DP_TIMEOUT_LOOP_MS;
 	reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
 	while (!(reg & RPLY_RECEIV)) {
-		timeout_loop++;
-		if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
+		if (time_after(jiffies, timeout)) {
 			dev_err(dp->dev, "AUX CH command reply failed!\n");
 			return -ETIMEDOUT;
 		}
@@ -1055,7 +1055,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
 {
 	u32 reg;
 	u8 *buffer = msg->buffer;
-	int timeout_loop = 0;
+	unsigned long timeout;
 	unsigned int i;
 	int num_transferred = 0;
 
@@ -1123,10 +1123,10 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
 
 	/* Is AUX CH command reply received? */
 	/* TODO: Wait for an interrupt instead of looping? */
+	timeout = jiffies + DP_TIMEOUT_LOOP_MS;
 	reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
 	while (!(reg & RPLY_RECEIV)) {
-		timeout_loop++;
-		if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
+		if (time_after(jiffies, timeout)) {
 			dev_err(dp->dev, "AUX CH command reply failed!\n");
 			return -ETIMEDOUT;
 		}
-- 
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 3/3] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR

[Yakir Yang]

Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
function, or print the sink PSR error state if we failed to apply the
requested PSR setting.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v3:
- Update commit message
- Add DP_TIMEOUT_PSR_LOOP_MS marcos
- Correct the return values of analogix_dp_send_psr_spd()

Changes in v2:
- A bunch of good fixes from Sean

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  6 ++----
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  5 +++--
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 25 ++++++++++++++++++++--
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5fe3982..c0ce16a 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
 	psr_vsc.DB0 = 0;
 	psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
 
-	analogix_dp_send_psr_spd(dp, &psr_vsc);
-	return 0;
+	return analogix_dp_send_psr_spd(dp, &psr_vsc);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
 
@@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
 	psr_vsc.DB0 = 0;
 	psr_vsc.DB1 = 0;
 
-	analogix_dp_send_psr_spd(dp, &psr_vsc);
-	return 0;
+	return analogix_dp_send_psr_spd(dp, &psr_vsc);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
 
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index d564e90..a27f1e3 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -20,6 +20,7 @@
 #define MAX_EQ_LOOP 5
 
 #define DP_TIMEOUT_LOOP_MS			msecs_to_jiffies(1)
+#define DP_TIMEOUT_PSR_LOOP_MS			msecs_to_jiffies(300)
 
 /* DP_MAX_LANE_COUNT */
 #define DPCD_ENHANCED_FRAME_CAP(x)		(((x) >> 7) & 0x1)
@@ -248,8 +249,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
 void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
 void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
 void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
-			      struct edp_vsc_psr *vsc);
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+			     struct edp_vsc_psr *vsc);
 ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
 			     struct drm_dp_aux_msg *msg);
 #endif /* _ANALOGIX_DP_CORE_H */
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 15a4cf0..7fd4ed0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
 	writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
 }
 
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
-			      struct edp_vsc_psr *vsc)
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+			     struct edp_vsc_psr *vsc)
 {
+	unsigned long timeout;
 	unsigned int val;
+	u8 sink;
 
 	/* don't send info frame */
 	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
@@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
 	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
 	val |= IF_EN;
 	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+
+	timeout = jiffies + DP_TIMEOUT_PSR_LOOP_MS;
+	while (time_before(jiffies, timeout)) {
+		val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
+		if (val != 1) {
+			dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
+			return -EBUSY;
+		}
+
+		if ((vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB) ||
+		    (!vsc->DB1 && sink == DP_PSR_SINK_INACTIVE))
+			return 0;
+
+		usleep_range(1000, 1500);
+	}
+
+	dev_warn(dp->dev, "Failed to apply PSR, sink state was [%x]", sink);
+
+	return -ETIMEDOUT;
 }
 
 ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
-- 
1.9.1

[PATCH v3 3/3] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR

[Yakir Yang]

Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
function, or print the sink PSR error state if we failed to apply the
requested PSR setting.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v3:
- Update commit message
- Add DP_TIMEOUT_PSR_LOOP_MS marcos
- Correct the return values of analogix_dp_send_psr_spd()

Changes in v2:
- A bunch of good fixes from Sean

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  6 ++----
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  5 +++--
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 25 ++++++++++++++++++++--
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5fe3982..c0ce16a 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
 	psr_vsc.DB0 = 0;
 	psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
 
-	analogix_dp_send_psr_spd(dp, &psr_vsc);
-	return 0;
+	return analogix_dp_send_psr_spd(dp, &psr_vsc);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
 
@@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
 	psr_vsc.DB0 = 0;
 	psr_vsc.DB1 = 0;
 
-	analogix_dp_send_psr_spd(dp, &psr_vsc);
-	return 0;
+	return analogix_dp_send_psr_spd(dp, &psr_vsc);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
 
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index d564e90..a27f1e3 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -20,6 +20,7 @@
 #define MAX_EQ_LOOP 5
 
 #define DP_TIMEOUT_LOOP_MS			msecs_to_jiffies(1)
+#define DP_TIMEOUT_PSR_LOOP_MS			msecs_to_jiffies(300)
 
 /* DP_MAX_LANE_COUNT */
 #define DPCD_ENHANCED_FRAME_CAP(x)		(((x) >> 7) & 0x1)
@@ -248,8 +249,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
 void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
 void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
 void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
-			      struct edp_vsc_psr *vsc);
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+			     struct edp_vsc_psr *vsc);
 ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
 			     struct drm_dp_aux_msg *msg);
 #endif /* _ANALOGIX_DP_CORE_H */
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 15a4cf0..7fd4ed0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
 	writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
 }
 
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
-			      struct edp_vsc_psr *vsc)
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+			     struct edp_vsc_psr *vsc)
 {
+	unsigned long timeout;
 	unsigned int val;
+	u8 sink;
 
 	/* don't send info frame */
 	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
@@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
 	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
 	val |= IF_EN;
 	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+
+	timeout = jiffies + DP_TIMEOUT_PSR_LOOP_MS;
+	while (time_before(jiffies, timeout)) {
+		val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
+		if (val != 1) {
+			dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
+			return -EBUSY;
+		}
+
+		if ((vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB) ||
+		    (!vsc->DB1 && sink == DP_PSR_SINK_INACTIVE))
+			return 0;
+
+		usleep_range(1000, 1500);
+	}
+
+	dev_warn(dp->dev, "Failed to apply PSR, sink state was [%x]", sink);
+
+	return -ETIMEDOUT;
 }
 
 ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
-- 
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/3] drm/bridge: analogix_dp: use jiffies to simulate timeout loop

[Sean Paul]

On Fri, Sep 9, 2016 at 5:44 AM, Yakir Yang <ykk@rock-chips.com> wrote:
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
> Changes in v3:
> - Suggested by Sean
>
> Changes in v2: None
>
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  3 ++-
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 18 +++++++++---------
>  2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index a15f076..d564e90 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -16,10 +16,11 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_dp_helper.h>
>
> -#define DP_TIMEOUT_LOOP_COUNT 100
>  #define MAX_CR_LOOP 5
>  #define MAX_EQ_LOOP 5
>
> +#define DP_TIMEOUT_LOOP_MS                     msecs_to_jiffies(1)

The name suggests the units here are ms, but you're storing jiffies.
Do the msecs_to_jiffies conversion down below.

> +
>  /* DP_MAX_LANE_COUNT */
>  #define DPCD_ENHANCED_FRAME_CAP(x)             (((x) >> 7) & 0x1)
>  #define DPCD_MAX_LANE_COUNT(x)                 ((x) & 0x1f)
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index a4d17b8..15a4cf0 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -335,7 +335,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
>  void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
>  {
>         u32 reg;
> -       int timeout_loop = 0;
> +       unsigned long timeout;
>
>         analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
>
> @@ -350,9 +350,9 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
>         if (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
>                 analogix_dp_set_pll_power_down(dp, 0);
>
> +               timeout = jiffies + DP_TIMEOUT_LOOP_MS;

timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_MS);

>                 while (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
> -                       timeout_loop++;
> -                       if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
> +                       if (time_after(jiffies, timeout)) {
>                                 dev_err(dp->dev, "failed to get pll lock status\n");
>                                 return;
>                         }
> @@ -501,7 +501,7 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
>  {
>         int reg;
>         int retval = 0;
> -       int timeout_loop = 0;
> +       unsigned long timeout;
>
>         /* Enable AUX CH operation */
>         reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
> @@ -509,10 +509,10 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
>         writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
>
>         /* Is AUX CH command reply received? */
> +       timeout = jiffies + DP_TIMEOUT_LOOP_MS;
>         reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
>         while (!(reg & RPLY_RECEIV)) {
> -               timeout_loop++;
> -               if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
> +               if (time_after(jiffies, timeout)) {
>                         dev_err(dp->dev, "AUX CH command reply failed!\n");
>                         return -ETIMEDOUT;
>                 }
> @@ -1055,7 +1055,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>  {
>         u32 reg;
>         u8 *buffer = msg->buffer;
> -       int timeout_loop = 0;
> +       unsigned long timeout;
>         unsigned int i;
>         int num_transferred = 0;
>
> @@ -1123,10 +1123,10 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>
>         /* Is AUX CH command reply received? */
>         /* TODO: Wait for an interrupt instead of looping? */
> +       timeout = jiffies + DP_TIMEOUT_LOOP_MS;
>         reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
>         while (!(reg & RPLY_RECEIV)) {
> -               timeout_loop++;
> -               if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
> +               if (time_after(jiffies, timeout)) {
>                         dev_err(dp->dev, "AUX CH command reply failed!\n");
>                         return -ETIMEDOUT;
>                 }
> --
> 1.9.1
>
>

Re: [PATCH v3 2/3] drm/bridge: analogix_dp: use jiffies to simulate timeout loop

[Sean Paul]

On Fri, Sep 9, 2016 at 5:44 AM, Yakir Yang <ykk@rock-chips.com> wrote:
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
> Changes in v3:
> - Suggested by Sean
>
> Changes in v2: None
>
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  3 ++-
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 18 +++++++++---------
>  2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index a15f076..d564e90 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -16,10 +16,11 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_dp_helper.h>
>
> -#define DP_TIMEOUT_LOOP_COUNT 100
>  #define MAX_CR_LOOP 5
>  #define MAX_EQ_LOOP 5
>
> +#define DP_TIMEOUT_LOOP_MS                     msecs_to_jiffies(1)

The name suggests the units here are ms, but you're storing jiffies.
Do the msecs_to_jiffies conversion down below.

> +
>  /* DP_MAX_LANE_COUNT */
>  #define DPCD_ENHANCED_FRAME_CAP(x)             (((x) >> 7) & 0x1)
>  #define DPCD_MAX_LANE_COUNT(x)                 ((x) & 0x1f)
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index a4d17b8..15a4cf0 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -335,7 +335,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
>  void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
>  {
>         u32 reg;
> -       int timeout_loop = 0;
> +       unsigned long timeout;
>
>         analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
>
> @@ -350,9 +350,9 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
>         if (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
>                 analogix_dp_set_pll_power_down(dp, 0);
>
> +               timeout = jiffies + DP_TIMEOUT_LOOP_MS;

timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_MS);

>                 while (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
> -                       timeout_loop++;
> -                       if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
> +                       if (time_after(jiffies, timeout)) {
>                                 dev_err(dp->dev, "failed to get pll lock status\n");
>                                 return;
>                         }
> @@ -501,7 +501,7 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
>  {
>         int reg;
>         int retval = 0;
> -       int timeout_loop = 0;
> +       unsigned long timeout;
>
>         /* Enable AUX CH operation */
>         reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
> @@ -509,10 +509,10 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
>         writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
>
>         /* Is AUX CH command reply received? */
> +       timeout = jiffies + DP_TIMEOUT_LOOP_MS;
>         reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
>         while (!(reg & RPLY_RECEIV)) {
> -               timeout_loop++;
> -               if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
> +               if (time_after(jiffies, timeout)) {
>                         dev_err(dp->dev, "AUX CH command reply failed!\n");
>                         return -ETIMEDOUT;
>                 }
> @@ -1055,7 +1055,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>  {
>         u32 reg;
>         u8 *buffer = msg->buffer;
> -       int timeout_loop = 0;
> +       unsigned long timeout;
>         unsigned int i;
>         int num_transferred = 0;
>
> @@ -1123,10 +1123,10 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>
>         /* Is AUX CH command reply received? */
>         /* TODO: Wait for an interrupt instead of looping? */
> +       timeout = jiffies + DP_TIMEOUT_LOOP_MS;
>         reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
>         while (!(reg & RPLY_RECEIV)) {
> -               timeout_loop++;
> -               if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
> +               if (time_after(jiffies, timeout)) {
>                         dev_err(dp->dev, "AUX CH command reply failed!\n");
>                         return -ETIMEDOUT;
>                 }
> --
> 1.9.1
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 3/3] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR

[Sean Paul]

On Fri, Sep 9, 2016 at 5:45 AM, Yakir Yang <ykk@rock-chips.com> wrote:
> Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
> function, or print the sink PSR error state if we failed to apply the
> requested PSR setting.
>
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
> Changes in v3:
> - Update commit message
> - Add DP_TIMEOUT_PSR_LOOP_MS marcos
> - Correct the return values of analogix_dp_send_psr_spd()
>
> Changes in v2:
> - A bunch of good fixes from Sean
>
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  6 ++----
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  5 +++--
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 25 ++++++++++++++++++++--
>  3 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 5fe3982..c0ce16a 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
>         psr_vsc.DB0 = 0;
>         psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>
> -       analogix_dp_send_psr_spd(dp, &psr_vsc);
> -       return 0;
> +       return analogix_dp_send_psr_spd(dp, &psr_vsc);
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>
> @@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
>         psr_vsc.DB0 = 0;
>         psr_vsc.DB1 = 0;
>
> -       analogix_dp_send_psr_spd(dp, &psr_vsc);
> -       return 0;
> +       return analogix_dp_send_psr_spd(dp, &psr_vsc);
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index d564e90..a27f1e3 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -20,6 +20,7 @@
>  #define MAX_EQ_LOOP 5
>
>  #define DP_TIMEOUT_LOOP_MS                     msecs_to_jiffies(1)
> +#define DP_TIMEOUT_PSR_LOOP_MS                 msecs_to_jiffies(300)

Same comment here re: units.

300ms seems like a really long time. Why does it take this long?

Sean


>
>  /* DP_MAX_LANE_COUNT */
>  #define DPCD_ENHANCED_FRAME_CAP(x)             (((x) >> 7) & 0x1)
> @@ -248,8 +249,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>  void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>  void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>  void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> -                             struct edp_vsc_psr *vsc);
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> +                            struct edp_vsc_psr *vsc);
>  ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>                              struct drm_dp_aux_msg *msg);
>  #endif /* _ANALOGIX_DP_CORE_H */
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index 15a4cf0..7fd4ed0 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
>         writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
>  }
>
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> -                             struct edp_vsc_psr *vsc)
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> +                            struct edp_vsc_psr *vsc)
>  {
> +       unsigned long timeout;
>         unsigned int val;
> +       u8 sink;
>
>         /* don't send info frame */
>         val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> @@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>         val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>         val |= IF_EN;
>         writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +
> +       timeout = jiffies + DP_TIMEOUT_PSR_LOOP_MS;
> +       while (time_before(jiffies, timeout)) {
> +               val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
> +               if (val != 1) {
> +                       dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
> +                       return -EBUSY;
> +               }
> +
> +               if ((vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB) ||
> +                   (!vsc->DB1 && sink == DP_PSR_SINK_INACTIVE))
> +                       return 0;
> +
> +               usleep_range(1000, 1500);
> +       }
> +
> +       dev_warn(dp->dev, "Failed to apply PSR, sink state was [%x]", sink);
> +
> +       return -ETIMEDOUT;
>  }
>
>  ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> --
> 1.9.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/3] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR

[Sean Paul]

On Fri, Sep 9, 2016 at 5:45 AM, Yakir Yang <ykk@rock-chips.com> wrote:
> Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
> function, or print the sink PSR error state if we failed to apply the
> requested PSR setting.
>
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
> Changes in v3:
> - Update commit message
> - Add DP_TIMEOUT_PSR_LOOP_MS marcos
> - Correct the return values of analogix_dp_send_psr_spd()
>
> Changes in v2:
> - A bunch of good fixes from Sean
>
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  6 ++----
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  5 +++--
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 25 ++++++++++++++++++++--
>  3 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 5fe3982..c0ce16a 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
>         psr_vsc.DB0 = 0;
>         psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>
> -       analogix_dp_send_psr_spd(dp, &psr_vsc);
> -       return 0;
> +       return analogix_dp_send_psr_spd(dp, &psr_vsc);
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>
> @@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
>         psr_vsc.DB0 = 0;
>         psr_vsc.DB1 = 0;
>
> -       analogix_dp_send_psr_spd(dp, &psr_vsc);
> -       return 0;
> +       return analogix_dp_send_psr_spd(dp, &psr_vsc);
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index d564e90..a27f1e3 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -20,6 +20,7 @@
>  #define MAX_EQ_LOOP 5
>
>  #define DP_TIMEOUT_LOOP_MS                     msecs_to_jiffies(1)
> +#define DP_TIMEOUT_PSR_LOOP_MS                 msecs_to_jiffies(300)

Same comment here re: units.

300ms seems like a really long time. Why does it take this long?

Sean


>
>  /* DP_MAX_LANE_COUNT */
>  #define DPCD_ENHANCED_FRAME_CAP(x)             (((x) >> 7) & 0x1)
> @@ -248,8 +249,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>  void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>  void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>  void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> -                             struct edp_vsc_psr *vsc);
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> +                            struct edp_vsc_psr *vsc);
>  ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>                              struct drm_dp_aux_msg *msg);
>  #endif /* _ANALOGIX_DP_CORE_H */
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index 15a4cf0..7fd4ed0 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
>         writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
>  }
>
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> -                             struct edp_vsc_psr *vsc)
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> +                            struct edp_vsc_psr *vsc)
>  {
> +       unsigned long timeout;
>         unsigned int val;
> +       u8 sink;
>
>         /* don't send info frame */
>         val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> @@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>         val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>         val |= IF_EN;
>         writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +
> +       timeout = jiffies + DP_TIMEOUT_PSR_LOOP_MS;
> +       while (time_before(jiffies, timeout)) {
> +               val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
> +               if (val != 1) {
> +                       dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
> +                       return -EBUSY;
> +               }
> +
> +               if ((vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB) ||
> +                   (!vsc->DB1 && sink == DP_PSR_SINK_INACTIVE))
> +                       return 0;
> +
> +               usleep_range(1000, 1500);
> +       }
> +
> +       dev_warn(dp->dev, "Failed to apply PSR, sink state was [%x]", sink);
> +
> +       return -ETIMEDOUT;
>  }
>
>  ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> --
> 1.9.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/3] drm/bridge: analogix_dp: use jiffies to simulate timeout loop

[Yakir Yang]

Hi Sean,


On 09/12/2016 09:51 PM, Sean Paul wrote:
> On Fri, Sep 9, 2016 at 5:44 AM, Yakir Yang <ykk@rock-chips.com> wrote:
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> ---
>> Changes in v3:
>> - Suggested by Sean
>>
>> Changes in v2: None
>>
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  3 ++-
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 18 +++++++++---------
>>   2 files changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> index a15f076..d564e90 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> @@ -16,10 +16,11 @@
>>   #include <drm/drm_crtc.h>
>>   #include <drm/drm_dp_helper.h>
>>
>> -#define DP_TIMEOUT_LOOP_COUNT 100
>>   #define MAX_CR_LOOP 5
>>   #define MAX_EQ_LOOP 5
>>
>> +#define DP_TIMEOUT_LOOP_MS                     msecs_to_jiffies(1)
> The name suggests the units here are ms, but you're storing jiffies.
> Do the msecs_to_jiffies conversion down below.

I suddenly realized that 'analogix_dp_core.c' also used the 
'DP_TIMEOUT_LOOP_COUNT' macros, and 'analogix_dp_core.c' have four kinds 
of timeout,
   - DP_TIMEOUT_LOOP_COUNT * 1us
   - DP_TIMEOUT_LOOP_COUNT * 10us
   - DP_TIMEOUT_LOOP_COUNT * 100us
   - DP_TIMEOUT_LOOP_COUNT * 1000us

I may guess it's not necessary to replace the 'DP_TIMEOUT_LOOP_COUNT' 
now  :-)

- Yakir

>
>> +
>>   /* DP_MAX_LANE_COUNT */
>>   #define DPCD_ENHANCED_FRAME_CAP(x)             (((x) >> 7) & 0x1)
>>   #define DPCD_MAX_LANE_COUNT(x)                 ((x) & 0x1f)
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> index a4d17b8..15a4cf0 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> @@ -335,7 +335,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
>>   void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
>>   {
>>          u32 reg;
>> -       int timeout_loop = 0;
>> +       unsigned long timeout;
>>
>>          analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
>>
>> @@ -350,9 +350,9 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
>>          if (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
>>                  analogix_dp_set_pll_power_down(dp, 0);
>>
>> +               timeout = jiffies + DP_TIMEOUT_LOOP_MS;
> timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_MS);
>
>>                  while (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
>> -                       timeout_loop++;
>> -                       if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
>> +                       if (time_after(jiffies, timeout)) {
>>                                  dev_err(dp->dev, "failed to get pll lock status\n");
>>                                  return;
>>                          }
>> @@ -501,7 +501,7 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
>>   {
>>          int reg;
>>          int retval = 0;
>> -       int timeout_loop = 0;
>> +       unsigned long timeout;
>>
>>          /* Enable AUX CH operation */
>>          reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
>> @@ -509,10 +509,10 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
>>          writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
>>
>>          /* Is AUX CH command reply received? */
>> +       timeout = jiffies + DP_TIMEOUT_LOOP_MS;
>>          reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
>>          while (!(reg & RPLY_RECEIV)) {
>> -               timeout_loop++;
>> -               if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
>> +               if (time_after(jiffies, timeout)) {
>>                          dev_err(dp->dev, "AUX CH command reply failed!\n");
>>                          return -ETIMEDOUT;
>>                  }
>> @@ -1055,7 +1055,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>>   {
>>          u32 reg;
>>          u8 *buffer = msg->buffer;
>> -       int timeout_loop = 0;
>> +       unsigned long timeout;
>>          unsigned int i;
>>          int num_transferred = 0;
>>
>> @@ -1123,10 +1123,10 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>>
>>          /* Is AUX CH command reply received? */
>>          /* TODO: Wait for an interrupt instead of looping? */
>> +       timeout = jiffies + DP_TIMEOUT_LOOP_MS;
>>          reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
>>          while (!(reg & RPLY_RECEIV)) {
>> -               timeout_loop++;
>> -               if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
>> +               if (time_after(jiffies, timeout)) {
>>                          dev_err(dp->dev, "AUX CH command reply failed!\n");
>>                          return -ETIMEDOUT;
>>                  }
>> --
>> 1.9.1
>>
>>
>
>

Re: [PATCH v3 2/3] drm/bridge: analogix_dp: use jiffies to simulate timeout loop

[Yakir Yang]

Hi Sean,


On 09/12/2016 09:51 PM, Sean Paul wrote:
> On Fri, Sep 9, 2016 at 5:44 AM, Yakir Yang <ykk@rock-chips.com> wrote:
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> ---
>> Changes in v3:
>> - Suggested by Sean
>>
>> Changes in v2: None
>>
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  3 ++-
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 18 +++++++++---------
>>   2 files changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> index a15f076..d564e90 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> @@ -16,10 +16,11 @@
>>   #include <drm/drm_crtc.h>
>>   #include <drm/drm_dp_helper.h>
>>
>> -#define DP_TIMEOUT_LOOP_COUNT 100
>>   #define MAX_CR_LOOP 5
>>   #define MAX_EQ_LOOP 5
>>
>> +#define DP_TIMEOUT_LOOP_MS                     msecs_to_jiffies(1)
> The name suggests the units here are ms, but you're storing jiffies.
> Do the msecs_to_jiffies conversion down below.

I suddenly realized that 'analogix_dp_core.c' also used the 
'DP_TIMEOUT_LOOP_COUNT' macros, and 'analogix_dp_core.c' have four kinds 
of timeout,
   - DP_TIMEOUT_LOOP_COUNT * 1us
   - DP_TIMEOUT_LOOP_COUNT * 10us
   - DP_TIMEOUT_LOOP_COUNT * 100us
   - DP_TIMEOUT_LOOP_COUNT * 1000us

I may guess it's not necessary to replace the 'DP_TIMEOUT_LOOP_COUNT' 
now  :-)

- Yakir

>
>> +
>>   /* DP_MAX_LANE_COUNT */
>>   #define DPCD_ENHANCED_FRAME_CAP(x)             (((x) >> 7) & 0x1)
>>   #define DPCD_MAX_LANE_COUNT(x)                 ((x) & 0x1f)
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> index a4d17b8..15a4cf0 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> @@ -335,7 +335,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
>>   void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
>>   {
>>          u32 reg;
>> -       int timeout_loop = 0;
>> +       unsigned long timeout;
>>
>>          analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
>>
>> @@ -350,9 +350,9 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
>>          if (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
>>                  analogix_dp_set_pll_power_down(dp, 0);
>>
>> +               timeout = jiffies + DP_TIMEOUT_LOOP_MS;
> timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_MS);
>
>>                  while (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
>> -                       timeout_loop++;
>> -                       if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
>> +                       if (time_after(jiffies, timeout)) {
>>                                  dev_err(dp->dev, "failed to get pll lock status\n");
>>                                  return;
>>                          }
>> @@ -501,7 +501,7 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
>>   {
>>          int reg;
>>          int retval = 0;
>> -       int timeout_loop = 0;
>> +       unsigned long timeout;
>>
>>          /* Enable AUX CH operation */
>>          reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
>> @@ -509,10 +509,10 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
>>          writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
>>
>>          /* Is AUX CH command reply received? */
>> +       timeout = jiffies + DP_TIMEOUT_LOOP_MS;
>>          reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
>>          while (!(reg & RPLY_RECEIV)) {
>> -               timeout_loop++;
>> -               if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
>> +               if (time_after(jiffies, timeout)) {
>>                          dev_err(dp->dev, "AUX CH command reply failed!\n");
>>                          return -ETIMEDOUT;
>>                  }
>> @@ -1055,7 +1055,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>>   {
>>          u32 reg;
>>          u8 *buffer = msg->buffer;
>> -       int timeout_loop = 0;
>> +       unsigned long timeout;
>>          unsigned int i;
>>          int num_transferred = 0;
>>
>> @@ -1123,10 +1123,10 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>>
>>          /* Is AUX CH command reply received? */
>>          /* TODO: Wait for an interrupt instead of looping? */
>> +       timeout = jiffies + DP_TIMEOUT_LOOP_MS;
>>          reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
>>          while (!(reg & RPLY_RECEIV)) {
>> -               timeout_loop++;
>> -               if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
>> +               if (time_after(jiffies, timeout)) {
>>                          dev_err(dp->dev, "AUX CH command reply failed!\n");
>>                          return -ETIMEDOUT;
>>                  }
>> --
>> 1.9.1
>>
>>
>
>


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 3/3] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR

[Yakir Yang]

Hi Sean,

On 09/12/2016 09:52 PM, Sean Paul wrote:
> On Fri, Sep 9, 2016 at 5:45 AM, Yakir Yang <ykk@rock-chips.com> wrote:
>> Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
>> function, or print the sink PSR error state if we failed to apply the
>> requested PSR setting.
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> ---
>> Changes in v3:
>> - Update commit message
>> - Add DP_TIMEOUT_PSR_LOOP_MS marcos
>> - Correct the return values of analogix_dp_send_psr_spd()
>>
>> Changes in v2:
>> - A bunch of good fixes from Sean
>>
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  6 ++----
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  5 +++--
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 25 ++++++++++++++++++++--
>>   3 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 5fe3982..c0ce16a 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
>>          psr_vsc.DB0 = 0;
>>          psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>>
>> -       analogix_dp_send_psr_spd(dp, &psr_vsc);
>> -       return 0;
>> +       return analogix_dp_send_psr_spd(dp, &psr_vsc);
>>   }
>>   EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>>
>> @@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
>>          psr_vsc.DB0 = 0;
>>          psr_vsc.DB1 = 0;
>>
>> -       analogix_dp_send_psr_spd(dp, &psr_vsc);
>> -       return 0;
>> +       return analogix_dp_send_psr_spd(dp, &psr_vsc);
>>   }
>>   EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> index d564e90..a27f1e3 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> @@ -20,6 +20,7 @@
>>   #define MAX_EQ_LOOP 5
>>
>>   #define DP_TIMEOUT_LOOP_MS                     msecs_to_jiffies(1)
>> +#define DP_TIMEOUT_PSR_LOOP_MS                 msecs_to_jiffies(300)
> Same comment here re: units.
>
> 300ms seems like a really long time. Why does it take this long?

This magic number '300ms' just come from my test, I haven't found the 
description in eDP 1.4a Spec about what exact time should Sink take to 
entry PSR.

- Yakir

> Sean
>
>
>>   /* DP_MAX_LANE_COUNT */
>>   #define DPCD_ENHANCED_FRAME_CAP(x)             (((x) >> 7) & 0x1)
>> @@ -248,8 +249,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>>   void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>>   void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>>   void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
>> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> -                             struct edp_vsc_psr *vsc);
>> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> +                            struct edp_vsc_psr *vsc);
>>   ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>>                               struct drm_dp_aux_msg *msg);
>>   #endif /* _ANALOGIX_DP_CORE_H */
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> index 15a4cf0..7fd4ed0 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> @@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
>>          writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
>>   }
>>
>> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> -                             struct edp_vsc_psr *vsc)
>> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> +                            struct edp_vsc_psr *vsc)
>>   {
>> +       unsigned long timeout;
>>          unsigned int val;
>> +       u8 sink;
>>
>>          /* don't send info frame */
>>          val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> @@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>>          val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>>          val |= IF_EN;
>>          writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> +
>> +       timeout = jiffies + DP_TIMEOUT_PSR_LOOP_MS;
>> +       while (time_before(jiffies, timeout)) {
>> +               val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
>> +               if (val != 1) {
>> +                       dev_err(dp->dev, "PSR_STATUS read failed ret=%d", val);
>> +                       return -EBUSY;
>> +               }
>> +
>> +               if ((vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB) ||
>> +                   (!vsc->DB1 && sink == DP_PSR_SINK_INACTIVE))
>> +                       return 0;
>> +
>> +               usleep_range(1000, 1500);
>> +       }
>> +
>> +       dev_warn(dp->dev, "Failed to apply PSR, sink state was [%x]", sink);
>> +
>> +       return -ETIMEDOUT;
>>   }
>>
>>   ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>> --
>> 1.9.1
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>