[PATCH 00/12] drm/dp: Extend DRM DP link helpers

[PATCH 00/12] drm/dp: Extend DRM DP link helpers

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This set of patches extends the existing DRM DP link helpers by caching
additional capabilities. It also reworks the helpers to make use of the
existing parsing helpers to reduce code duplication.

Thierry

Thierry Reding (12):
  drm/dp: Add drm_dp_link_reset() implementation
  drm/dp: link: Track capabilities alongside settings
  drm/dp: Turn link capabilities into booleans
  drm/dp: Probe link using existing parsing helpers
  drm/dp: Read fast training capability from link
  drm/dp: Read TPS3 capability from sink
  drm/dp: Set channel coding on link configuration
  drm/dp: Read eDP version from DPCD
  drm/dp: Add helper to get post-cursor adjustments
  drm/dp: Enable alternate scrambler when supported
  drm/dp: Read AUX read interval from DPCD
  drm/dp: Add drm_dp_link_choose() helper

 drivers/gpu/drm/drm_dp_helper.c    | 177 ++++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/msm/edp/edp_ctrl.c |  12 +--
 drivers/gpu/drm/tegra/dpaux.c      |   8 +-
 drivers/gpu/drm/tegra/sor.c        |  33 ++++---
 include/drm/drm_dp_helper.h        |  77 +++++++++++++++-
 5 files changed, 266 insertions(+), 41 deletions(-)

-- 
2.5.0

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

[PATCH 01/12] drm/dp: Add drm_dp_link_reset() implementation

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Subsequent patches will add non-volatile fields to struct drm_dp_link,
so introduce a function to zero out only the volatile fields.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 9535c5b60387..841b49652cc5 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -284,6 +284,17 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 }
 EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
 
+static void drm_dp_link_reset(struct drm_dp_link *link)
+{
+	if (!link)
+		return;
+
+	link->revision = 0;
+	link->rate = 0;
+	link->num_lanes = 0;
+	link->capabilities = 0;
+}
+
 /**
  * drm_dp_link_probe() - probe a DisplayPort link for capabilities
  * @aux: DisplayPort AUX channel
@@ -300,7 +311,7 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	u8 values[3];
 	int err;
 
-	memset(link, 0, sizeof(*link));
+	drm_dp_link_reset(link);
 
 	err = drm_dp_dpcd_read(aux, DP_DPCD_REV, values, sizeof(values));
 	if (err < 0)
-- 
2.5.0

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

[PATCH 02/12] drm/dp: link: Track capabilities alongside settings

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Store capabilities in max_* fields and add separate fields for the
currently selected settings. This is useful to allow the current link
configuration to be stored without overwriting the capabilities.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c    | 16 +++++++++++-----
 drivers/gpu/drm/msm/edp/edp_ctrl.c |  8 ++++----
 drivers/gpu/drm/tegra/dpaux.c      |  8 ++++----
 drivers/gpu/drm/tegra/sor.c        | 29 ++++++++++++++---------------
 include/drm/drm_dp_helper.h        |  7 +++++--
 5 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 841b49652cc5..c00896ab8b04 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -290,9 +290,12 @@ static void drm_dp_link_reset(struct drm_dp_link *link)
 		return;
 
 	link->revision = 0;
-	link->rate = 0;
-	link->num_lanes = 0;
+	link->max_rate = 0;
+	link->max_lanes = 0;
 	link->capabilities = 0;
+
+	link->rate = 0;
+	link->lanes = 0;
 }
 
 /**
@@ -318,12 +321,15 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 		return err;
 
 	link->revision = values[0];
-	link->rate = drm_dp_bw_code_to_link_rate(values[1]);
-	link->num_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
+	link->max_rate = drm_dp_bw_code_to_link_rate(values[1]);
+	link->max_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
 
 	if (values[2] & DP_ENHANCED_FRAME_CAP)
 		link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
 
+	link->rate = link->max_rate;
+	link->lanes = link->max_lanes;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_link_probe);
@@ -410,7 +416,7 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	int err;
 
 	values[0] = drm_dp_link_rate_to_bw_code(link->rate);
-	values[1] = link->num_lanes;
+	values[1] = link->lanes;
 
 	if (link->capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
 		values[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c b/drivers/gpu/drm/msm/edp/edp_ctrl.c
index 81200e9be382..c87a03561246 100644
--- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
+++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
@@ -416,7 +416,7 @@ static void edp_fill_link_cfg(struct edp_ctrl *ctrl)
 	u32 prate;
 	u32 lrate;
 	u32 bpp;
-	u8 max_lane = ctrl->dp_link.num_lanes;
+	u8 max_lane = ctrl->dp_link.max_lanes;
 	u8 lane;
 
 	prate = ctrl->pixel_rate;
@@ -426,7 +426,7 @@ static void edp_fill_link_cfg(struct edp_ctrl *ctrl)
 	 * By default, use the maximum link rate and minimum lane count,
 	 * so that we can do rate down shift during link training.
 	 */
-	ctrl->link_rate = drm_dp_link_rate_to_bw_code(ctrl->dp_link.rate);
+	ctrl->link_rate = drm_dp_link_rate_to_bw_code(ctrl->dp_link.max_rate);
 
 	prate *= bpp;
 	prate /= 8; /* in kByte */
@@ -714,7 +714,7 @@ static int edp_link_rate_down_shift(struct edp_ctrl *ctrl)
 
 	rate = ctrl->link_rate;
 	lane = ctrl->lane_cnt;
-	max_lane = ctrl->dp_link.num_lanes;
+	max_lane = ctrl->dp_link.max_lanes;
 
 	bpp = ctrl->color_depth * 3;
 	prate = ctrl->pixel_rate;
@@ -772,7 +772,7 @@ static int edp_do_link_train(struct edp_ctrl *ctrl)
 	 * Set the current link rate and lane cnt to panel. They may have been
 	 * adjusted and the values are different from them in DPCD CAP
 	 */
-	dp_link.num_lanes = ctrl->lane_cnt;
+	dp_link.lanes = ctrl->lane_cnt;
 	dp_link.rate = drm_dp_bw_code_to_link_rate(ctrl->link_rate);
 	dp_link.capabilities = ctrl->dp_link.capabilities;
 	if (drm_dp_link_configure(ctrl->drm_aux, &dp_link) < 0)
diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index 6aecb6647313..2c5480265ccd 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -578,14 +578,14 @@ int tegra_dpaux_train(struct tegra_dpaux *dpaux, struct drm_dp_link *link,
 	if (tp == DP_TRAINING_PATTERN_DISABLE)
 		return 0;
 
-	for (i = 0; i < link->num_lanes; i++)
+	for (i = 0; i < link->lanes; i++)
 		values[i] = DP_TRAIN_MAX_PRE_EMPHASIS_REACHED |
 			    DP_TRAIN_PRE_EMPH_LEVEL_0 |
 			    DP_TRAIN_MAX_SWING_REACHED |
 			    DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
 
 	err = drm_dp_dpcd_write(&dpaux->aux, DP_TRAINING_LANE0_SET, values,
-				link->num_lanes);
+				link->lanes);
 	if (err < 0)
 		return err;
 
@@ -597,13 +597,13 @@ int tegra_dpaux_train(struct tegra_dpaux *dpaux, struct drm_dp_link *link,
 
 	switch (tp) {
 	case DP_TRAINING_PATTERN_1:
-		if (!drm_dp_clock_recovery_ok(status, link->num_lanes))
+		if (!drm_dp_clock_recovery_ok(status, link->lanes))
 			return -EAGAIN;
 
 		break;
 
 	case DP_TRAINING_PATTERN_2:
-		if (!drm_dp_channel_eq_ok(status, link->num_lanes))
+		if (!drm_dp_channel_eq_ok(status, link->lanes))
 			return -EAGAIN;
 
 		break;
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 3eff7cf75d25..7bf444bd1588 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -277,7 +277,7 @@ static int tegra_sor_dp_train_fast(struct tegra_sor *sor,
 	if (err < 0)
 		return err;
 
-	for (i = 0, value = 0; i < link->num_lanes; i++) {
+	for (i = 0, value = 0; i < link->lanes; i++) {
 		unsigned long lane = SOR_DP_TPG_CHANNEL_CODING |
 				     SOR_DP_TPG_SCRAMBLER_NONE |
 				     SOR_DP_TPG_PATTERN_TRAIN1;
@@ -298,7 +298,7 @@ static int tegra_sor_dp_train_fast(struct tegra_sor *sor,
 	value |= SOR_DP_SPARE_MACRO_SOR_CLK;
 	tegra_sor_writel(sor, value, SOR_DP_SPARE0);
 
-	for (i = 0, value = 0; i < link->num_lanes; i++) {
+	for (i = 0, value = 0; i < link->lanes; i++) {
 		unsigned long lane = SOR_DP_TPG_CHANNEL_CODING |
 				     SOR_DP_TPG_SCRAMBLER_NONE |
 				     SOR_DP_TPG_PATTERN_TRAIN2;
@@ -313,7 +313,7 @@ static int tegra_sor_dp_train_fast(struct tegra_sor *sor,
 	if (err < 0)
 		return err;
 
-	for (i = 0, value = 0; i < link->num_lanes; i++) {
+	for (i = 0, value = 0; i < link->lanes; i++) {
 		unsigned long lane = SOR_DP_TPG_CHANNEL_CODING |
 				     SOR_DP_TPG_SCRAMBLER_GALIOS |
 				     SOR_DP_TPG_PATTERN_NONE;
@@ -581,11 +581,11 @@ static int tegra_sor_calc_config(struct tegra_sor *sor,
 	u32 num_syms_per_line;
 	unsigned int i;
 
-	if (!link_rate || !link->num_lanes || !pclk || !config->bits_per_pixel)
+	if (!link_rate || !link->lanes || !pclk || !config->bits_per_pixel)
 		return -EINVAL;
 
-	output = link_rate * 8 * link->num_lanes;
 	input = pclk * config->bits_per_pixel;
+	output = link_rate * 8 * link->lanes;
 
 	if (input >= output)
 		return -ERANGE;
@@ -628,7 +628,7 @@ static int tegra_sor_calc_config(struct tegra_sor *sor,
 	watermark = div_u64(watermark + params.error, f);
 	config->watermark = watermark + (config->bits_per_pixel / 8) + 2;
 	num_syms_per_line = (mode->hdisplay * config->bits_per_pixel) *
-			    (link->num_lanes * 8);
+			    (link->lanes * 8);
 
 	if (config->watermark > 30) {
 		config->watermark = 30;
@@ -648,12 +648,12 @@ static int tegra_sor_calc_config(struct tegra_sor *sor,
 	if (link->capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
 		config->hblank_symbols -= 3;
 
-	config->hblank_symbols -= 12 / link->num_lanes;
+	config->hblank_symbols -= 12 / link->lanes;
 
 	/* compute the number of symbols per vertical blanking interval */
 	num = (mode->hdisplay - 25) * link_rate;
 	config->vblank_symbols = div_u64(num, pclk);
-	config->vblank_symbols -= 36 / link->num_lanes + 4;
+	config->vblank_symbols -= 36 / link->lanes + 4;
 
 	dev_dbg(sor->dev, "blank symbols: H:%u V:%u\n", config->hblank_symbols,
 		config->vblank_symbols);
@@ -1340,17 +1340,17 @@ static void tegra_sor_edp_enable(struct drm_encoder *encoder)
 	/* power DP lanes */
 	value = tegra_sor_readl(sor, SOR_DP_PADCTL0);
 
-	if (link.num_lanes <= 2)
+	if (link.lanes <= 2)
 		value &= ~(SOR_DP_PADCTL_PD_TXD_3 | SOR_DP_PADCTL_PD_TXD_2);
 	else
 		value |= SOR_DP_PADCTL_PD_TXD_3 | SOR_DP_PADCTL_PD_TXD_2;
 
-	if (link.num_lanes <= 1)
+	if (link.lanes <= 1)
 		value &= ~SOR_DP_PADCTL_PD_TXD_1;
 	else
 		value |= SOR_DP_PADCTL_PD_TXD_1;
 
-	if (link.num_lanes == 0)
+	if (link.lanes == 0)
 		value &= ~SOR_DP_PADCTL_PD_TXD_0;
 	else
 		value |= SOR_DP_PADCTL_PD_TXD_0;
@@ -1359,7 +1359,7 @@ static void tegra_sor_edp_enable(struct drm_encoder *encoder)
 
 	value = tegra_sor_readl(sor, SOR_DP_LINKCTL0);
 	value &= ~SOR_DP_LINKCTL_LANE_COUNT_MASK;
-	value |= SOR_DP_LINKCTL_LANE_COUNT(link.num_lanes);
+	value |= SOR_DP_LINKCTL_LANE_COUNT(link.lanes);
 	tegra_sor_writel(sor, value, SOR_DP_LINKCTL0);
 
 	/* start lane sequencer */
@@ -1453,7 +1453,7 @@ static void tegra_sor_edp_enable(struct drm_encoder *encoder)
 				err);
 
 		rate = drm_dp_link_rate_to_bw_code(link.rate);
-		lanes = link.num_lanes;
+		lanes = link.lanes;
 
 		value = tegra_sor_readl(sor, SOR_CLK_CNTRL);
 		value &= ~SOR_CLK_CNTRL_DP_LINK_SPEED_MASK;
@@ -1470,8 +1470,7 @@ static void tegra_sor_edp_enable(struct drm_encoder *encoder)
 		tegra_sor_writel(sor, value, SOR_DP_LINKCTL0);
 
 		/* disable training pattern generator */
-
-		for (i = 0; i < link.num_lanes; i++) {
+		for (i = 0; i < link.lanes; i++) {
 			unsigned long lane = SOR_DP_TPG_CHANNEL_CODING |
 					     SOR_DP_TPG_SCRAMBLER_GALIOS |
 					     SOR_DP_TPG_PATTERN_NONE;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index bb9d0deca07c..124fea3ffd57 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -757,9 +757,12 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 
 struct drm_dp_link {
 	unsigned char revision;
-	unsigned int rate;
-	unsigned int num_lanes;
+	unsigned int max_rate;
+	unsigned int max_lanes;
 	unsigned long capabilities;
+
+	unsigned int rate;
+	unsigned int lanes;
 };
 
 int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
-- 
2.5.0

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

[PATCH 03/12] drm/dp: Turn link capabilities into booleans

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Rather than storing capabilities as flags in an integer, use a separate
boolean per capability. This simplifies the code that checks for these
capabilities.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c    | 18 +++++++++++++++---
 drivers/gpu/drm/msm/edp/edp_ctrl.c |  4 ++--
 drivers/gpu/drm/tegra/sor.c        |  4 ++--
 include/drm/drm_dp_helper.h        | 10 ++++++++--
 4 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index c00896ab8b04..cdc0da67e8ad 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -284,6 +284,17 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 }
 EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
 
+static void drm_dp_link_caps_reset(struct drm_dp_link_caps *caps)
+{
+	caps->enhanced_framing = false;
+}
+
+void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
+			   const struct drm_dp_link_caps *src)
+{
+	dest->enhanced_framing = src->enhanced_framing;
+}
+
 static void drm_dp_link_reset(struct drm_dp_link *link)
 {
 	if (!link)
@@ -292,7 +303,8 @@ static void drm_dp_link_reset(struct drm_dp_link *link)
 	link->revision = 0;
 	link->max_rate = 0;
 	link->max_lanes = 0;
-	link->capabilities = 0;
+
+	drm_dp_link_caps_reset(&link->caps);
 
 	link->rate = 0;
 	link->lanes = 0;
@@ -325,7 +337,7 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	link->max_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
 
 	if (values[2] & DP_ENHANCED_FRAME_CAP)
-		link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
+		link->caps.enhanced_framing = true;
 
 	link->rate = link->max_rate;
 	link->lanes = link->max_lanes;
@@ -418,7 +430,7 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	values[0] = drm_dp_link_rate_to_bw_code(link->rate);
 	values[1] = link->lanes;
 
-	if (link->capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
+	if (link->caps.enhanced_framing)
 		values[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
 
 	err = drm_dp_dpcd_write(aux, DP_LINK_BW_SET, values, sizeof(values));
diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c b/drivers/gpu/drm/msm/edp/edp_ctrl.c
index c87a03561246..12e2a5cd9582 100644
--- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
+++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
@@ -452,7 +452,7 @@ static void edp_config_ctrl(struct edp_ctrl *ctrl)
 
 	data = EDP_CONFIGURATION_CTRL_LANES(ctrl->lane_cnt - 1);
 
-	if (ctrl->dp_link.capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
+	if (ctrl->dp_link.caps.enhanced_framing)
 		data |= EDP_CONFIGURATION_CTRL_ENHANCED_FRAMING;
 
 	depth = EDP_6BIT;
@@ -774,7 +774,7 @@ static int edp_do_link_train(struct edp_ctrl *ctrl)
 	 */
 	dp_link.lanes = ctrl->lane_cnt;
 	dp_link.rate = drm_dp_bw_code_to_link_rate(ctrl->link_rate);
-	dp_link.capabilities = ctrl->dp_link.capabilities;
+	drm_dp_link_caps_copy(&dp_link.caps, &ctrl->dp_link.caps);
 	if (drm_dp_link_configure(ctrl->drm_aux, &dp_link) < 0)
 		return EDP_TRAIN_FAIL;
 
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 7bf444bd1588..f26dfaca7586 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -645,7 +645,7 @@ static int tegra_sor_calc_config(struct tegra_sor *sor,
 	num = ((mode->htotal - mode->hdisplay) - 7) * link_rate;
 	config->hblank_symbols = div_u64(num, pclk);
 
-	if (link->capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
+	if (link->caps.enhanced_framing)
 		config->hblank_symbols -= 3;
 
 	config->hblank_symbols -= 12 / link->lanes;
@@ -1464,7 +1464,7 @@ static void tegra_sor_edp_enable(struct drm_encoder *encoder)
 		value &= ~SOR_DP_LINKCTL_LANE_COUNT_MASK;
 		value |= SOR_DP_LINKCTL_LANE_COUNT(lanes);
 
-		if (link.capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
+		if (link.caps.enhanced_framing)
 			value |= SOR_DP_LINKCTL_ENHANCED_FRAME;
 
 		tegra_sor_writel(sor, value, SOR_DP_LINKCTL0);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 124fea3ffd57..20e4fe2e13c5 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -753,13 +753,19 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 /*
  * DisplayPort link
  */
-#define DP_LINK_CAP_ENHANCED_FRAMING (1 << 0)
+struct drm_dp_link_caps {
+	bool enhanced_framing;
+};
+
+void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
+			   const struct drm_dp_link_caps *src);
 
 struct drm_dp_link {
 	unsigned char revision;
 	unsigned int max_rate;
 	unsigned int max_lanes;
-	unsigned long capabilities;
+
+	struct drm_dp_link_caps caps;
 
 	unsigned int rate;
 	unsigned int lanes;
-- 
2.5.0

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

[PATCH 04/12] drm/dp: Probe link using existing parsing helpers

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Use existing parsing helpers to probe a DisplayPort link.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 29 ++++++++++++++++++++++-------
 include/drm/drm_dp_helper.h     |  2 ++
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index cdc0da67e8ad..54d8398cf3a6 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -269,6 +269,22 @@ ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset,
 EXPORT_SYMBOL(drm_dp_dpcd_write);
 
 /**
+ * drm_dp_dpcd_read_link_caps() - read DPCD link capabilities
+ * @aux: DisplayPort AUX channel
+ * @caps: buffer to store the link capabilities in
+ *
+ * Returns:
+ * The number of bytes transferred on success or a negative error code on
+ * failure.
+ */
+int drm_dp_dpcd_read_link_caps(struct drm_dp_aux *aux,
+			       u8 caps[DP_RECEIVER_CAP_SIZE])
+{
+	return drm_dp_dpcd_read(aux, DP_DPCD_REV, caps, DP_RECEIVER_CAP_SIZE);
+}
+EXPORT_SYMBOL(drm_dp_dpcd_read_link_caps);
+
+/**
  * drm_dp_dpcd_read_link_status() - read DPCD link status (bytes 0x202-0x207)
  * @aux: DisplayPort AUX channel
  * @status: buffer to store the link status in (must be at least 6 bytes)
@@ -323,21 +339,20 @@ static void drm_dp_link_reset(struct drm_dp_link *link)
  */
 int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 {
-	u8 values[3];
+	u8 values[DP_RECEIVER_CAP_SIZE];
 	int err;
 
 	drm_dp_link_reset(link);
 
-	err = drm_dp_dpcd_read(aux, DP_DPCD_REV, values, sizeof(values));
+	err = drm_dp_dpcd_read_link_caps(aux, values);
 	if (err < 0)
 		return err;
 
-	link->revision = values[0];
-	link->max_rate = drm_dp_bw_code_to_link_rate(values[1]);
-	link->max_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
+	link->revision = values[DP_DPCD_REV];
+	link->max_rate = drm_dp_max_link_rate(values);
+	link->max_lanes = drm_dp_max_lane_count(values);
 
-	if (values[2] & DP_ENHANCED_FRAME_CAP)
-		link->caps.enhanced_framing = true;
+	link->caps.enhanced_framing = drm_dp_enhanced_frame_cap(values);
 
 	link->rate = link->max_rate;
 	link->lanes = link->max_lanes;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 20e4fe2e13c5..4929b3ea30e3 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -747,6 +747,8 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux,
 	return drm_dp_dpcd_write(aux, offset, &value, 1);
 }
 
+int drm_dp_dpcd_read_link_caps(struct drm_dp_aux *aux,
+			       u8 caps[DP_RECEIVER_CAP_SIZE]);
 int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 				 u8 status[DP_LINK_STATUS_SIZE]);
 
-- 
2.5.0

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

[PATCH 05/12] drm/dp: Read fast training capability from link

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

While probing the DisplayPort link, query the fast training capability.
If supported, drivers can use the fast link training sequence instead of
the more involved full link training sequence.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 3 +++
 include/drm/drm_dp_helper.h     | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 54d8398cf3a6..0211502f63af 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -303,12 +303,14 @@ EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
 static void drm_dp_link_caps_reset(struct drm_dp_link_caps *caps)
 {
 	caps->enhanced_framing = false;
+	caps->fast_training = false;
 }
 
 void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
 			   const struct drm_dp_link_caps *src)
 {
 	dest->enhanced_framing = src->enhanced_framing;
+	dest->fast_training = src->fast_training;
 }
 
 static void drm_dp_link_reset(struct drm_dp_link *link)
@@ -353,6 +355,7 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	link->max_lanes = drm_dp_max_lane_count(values);
 
 	link->caps.enhanced_framing = drm_dp_enhanced_frame_cap(values);
+	link->caps.fast_training = drm_dp_fast_training_cap(values);
 
 	link->rate = link->max_rate;
 	link->lanes = link->max_lanes;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 4929b3ea30e3..a01ae89cee9c 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -645,6 +645,13 @@ drm_dp_tps3_supported(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
 		dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED;
 }
 
+static inline bool
+drm_dp_fast_training_cap(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
+{
+	return dpcd[DP_DPCD_REV] >= 0x11 &&
+		(dpcd[DP_MAX_DOWNSPREAD] & DP_NO_AUX_HANDSHAKE_LINK_TRAINING);
+}
+
 /*
  * DisplayPort AUX channel
  */
@@ -757,6 +764,7 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
  */
 struct drm_dp_link_caps {
 	bool enhanced_framing;
+	bool fast_training;
 };
 
 void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
-- 
2.5.0

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

[PATCH 06/12] drm/dp: Read TPS3 capability from sink

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The TPS3 capability can be exposed by DP 1.2 and later sinks if they
support the alternative training pattern for channel equalization.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 3 +++
 include/drm/drm_dp_helper.h     | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 0211502f63af..2c774ffa2943 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -303,6 +303,7 @@ EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
 static void drm_dp_link_caps_reset(struct drm_dp_link_caps *caps)
 {
 	caps->enhanced_framing = false;
+	caps->tps3_supported = false;
 	caps->fast_training = false;
 }
 
@@ -310,6 +311,7 @@ void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
 			   const struct drm_dp_link_caps *src)
 {
 	dest->enhanced_framing = src->enhanced_framing;
+	dest->tps3_supported = src->tps3_supported;
 	dest->fast_training = src->fast_training;
 }
 
@@ -355,6 +357,7 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	link->max_lanes = drm_dp_max_lane_count(values);
 
 	link->caps.enhanced_framing = drm_dp_enhanced_frame_cap(values);
+	link->caps.tps3_supported = drm_dp_tps3_supported(values);
 	link->caps.fast_training = drm_dp_fast_training_cap(values);
 
 	link->rate = link->max_rate;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index a01ae89cee9c..bf280a4230fa 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -764,6 +764,7 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
  */
 struct drm_dp_link_caps {
 	bool enhanced_framing;
+	bool tps3_supported;
 	bool fast_training;
 };
 
-- 
2.5.0

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

[PATCH 07/12] drm/dp: Set channel coding on link configuration

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Make use of ANSI 8B/10B channel coding if the DisplayPort sink supports
it.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 12 +++++++++++-
 include/drm/drm_dp_helper.h     |  8 ++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 2c774ffa2943..0cf5b1c278ae 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -305,6 +305,7 @@ static void drm_dp_link_caps_reset(struct drm_dp_link_caps *caps)
 	caps->enhanced_framing = false;
 	caps->tps3_supported = false;
 	caps->fast_training = false;
+	caps->channel_coding = false;
 }
 
 void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
@@ -313,6 +314,7 @@ void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
 	dest->enhanced_framing = src->enhanced_framing;
 	dest->tps3_supported = src->tps3_supported;
 	dest->fast_training = src->fast_training;
+	dest->channel_coding = src->channel_coding;
 }
 
 static void drm_dp_link_reset(struct drm_dp_link *link)
@@ -359,6 +361,7 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	link->caps.enhanced_framing = drm_dp_enhanced_frame_cap(values);
 	link->caps.tps3_supported = drm_dp_tps3_supported(values);
 	link->caps.fast_training = drm_dp_fast_training_cap(values);
+	link->caps.channel_coding = drm_dp_channel_coding_supported(values);
 
 	link->rate = link->max_rate;
 	link->lanes = link->max_lanes;
@@ -445,7 +448,7 @@ EXPORT_SYMBOL(drm_dp_link_power_down);
  */
 int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
 {
-	u8 values[2];
+	u8 values[2], value = 0;
 	int err;
 
 	values[0] = drm_dp_link_rate_to_bw_code(link->rate);
@@ -458,6 +461,13 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	if (err < 0)
 		return err;
 
+	if (link->caps.channel_coding)
+		value = DP_SET_ANSI_8B10B;
+
+	err = drm_dp_dpcd_writeb(aux, DP_MAIN_LINK_CHANNEL_CODING_SET, value);
+	if (err < 0)
+		return err;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_link_configure);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index bf280a4230fa..201e654b2257 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -88,6 +88,7 @@
 # define DP_DETAILED_CAP_INFO_AVAILABLE	    (1 << 4) /* DPI */
 
 #define DP_MAIN_LINK_CHANNEL_CODING         0x006
+# define DP_CAP_ANSI_8B10B		    (1 << 0)
 
 #define DP_DOWN_STREAM_PORT_COUNT	    0x007
 # define DP_PORT_COUNT_MASK		    0x0f
@@ -652,6 +653,12 @@ drm_dp_fast_training_cap(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
 		(dpcd[DP_MAX_DOWNSPREAD] & DP_NO_AUX_HANDSHAKE_LINK_TRAINING);
 }
 
+static inline bool
+drm_dp_channel_coding_supported(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
+{
+	return dpcd[DP_MAIN_LINK_CHANNEL_CODING] & DP_CAP_ANSI_8B10B;
+}
+
 /*
  * DisplayPort AUX channel
  */
@@ -766,6 +773,7 @@ struct drm_dp_link_caps {
 	bool enhanced_framing;
 	bool tps3_supported;
 	bool fast_training;
+	bool channel_coding;
 };
 
 void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
-- 
2.5.0

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

[PATCH 08/12] drm/dp: Read eDP version from DPCD

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

If the sink supports eDP, read the eDP revision from it's DPCD.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 15 +++++++++++++++
 include/drm/drm_dp_helper.h     |  8 ++++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 0cf5b1c278ae..8d8112a4fee0 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -327,6 +327,7 @@ static void drm_dp_link_reset(struct drm_dp_link *link)
 	link->max_lanes = 0;
 
 	drm_dp_link_caps_reset(&link->caps);
+	link->edp = 0;
 
 	link->rate = 0;
 	link->lanes = 0;
@@ -363,6 +364,20 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	link->caps.fast_training = drm_dp_fast_training_cap(values);
 	link->caps.channel_coding = drm_dp_channel_coding_supported(values);
 
+	if (drm_dp_alternate_scrambler_reset_cap(values)) {
+		static const u8 edp_revs[] = { 0x11, 0x12, 0x13, 0x14 };
+		u8 value;
+
+		err = drm_dp_dpcd_readb(aux, DP_EDP_DPCD_REV, &value);
+		if (err < 0)
+			return err;
+
+		if (value >= ARRAY_SIZE(edp_revs))
+			DRM_ERROR("unsupported eDP version: %02x\n", value);
+		else
+			link->edp = edp_revs[value];
+	}
+
 	link->rate = link->max_rate;
 	link->lanes = link->max_lanes;
 
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 201e654b2257..1f7b137d01b6 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -659,6 +659,13 @@ drm_dp_channel_coding_supported(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
 	return dpcd[DP_MAIN_LINK_CHANNEL_CODING] & DP_CAP_ANSI_8B10B;
 }
 
+static inline bool
+drm_dp_alternate_scrambler_reset_cap(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
+{
+	return dpcd[DP_EDP_CONFIGURATION_CAP] &
+			DP_ALTERNATE_SCRAMBLER_RESET_CAP;
+}
+
 /*
  * DisplayPort AUX channel
  */
@@ -785,6 +792,7 @@ struct drm_dp_link {
 	unsigned int max_lanes;
 
 	struct drm_dp_link_caps caps;
+	unsigned char edp;
 
 	unsigned int rate;
 	unsigned int lanes;
-- 
2.5.0

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

[PATCH 09/12] drm/dp: Add helper to get post-cursor adjustments

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

If the transmitter supports pre-emphasis post cursor2 the sink will
request adjustments in a similar way to how it requests adjustments to
the voltage swing and pre-emphasis settings.

Add a helper to extract these adjustments on a per-lane basis from the
DPCD link status.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 10 ++++++++++
 include/drm/drm_dp_helper.h     | 12 ++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 8d8112a4fee0..e3a289793c73 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -115,6 +115,16 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI
 }
 EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
 
+u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZE],
+					 unsigned int lane)
+{
+	unsigned int offset = DP_ADJUST_REQUEST_POST_CURSOR2;
+	u8 value = dp_link_status(link_status, offset);
+
+	return (value >> (lane << 1)) & 0x3;
+}
+EXPORT_SYMBOL(drm_dp_get_adjust_request_post_cursor);
+
 void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
 	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
 		udelay(100);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 1f7b137d01b6..2efa6539c783 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -400,6 +400,16 @@
 # define DP_ADJUST_PRE_EMPHASIS_LANE1_MASK   0xc0
 # define DP_ADJUST_PRE_EMPHASIS_LANE1_SHIFT  6
 
+#define DP_ADJUST_REQUEST_POST_CURSOR2	    0x20c
+# define DP_ADJUST_POST_CURSOR2_LANE0_MASK  0x03
+# define DP_ADJUST_POST_CURSOR2_LANE0_SHIFT 0
+# define DP_ADJUST_POST_CURSOR2_LANE1_MASK  0x0c
+# define DP_ADJUST_POST_CURSOR2_LANE1_SHIFT 2
+# define DP_ADJUST_POST_CURSOR2_LANE2_MASK  0x30
+# define DP_ADJUST_POST_CURSOR2_LANE2_SHIFT 4
+# define DP_ADJUST_POST_CURSOR2_LANE3_MASK  0xc0
+# define DP_ADJUST_POST_CURSOR2_LANE3_SHIFT 6
+
 #define DP_TEST_REQUEST			    0x218
 # define DP_TEST_LINK_TRAINING		    (1 << 0)
 # define DP_TEST_LINK_VIDEO_PATTERN	    (1 << 1)
@@ -582,6 +592,8 @@ u8 drm_dp_get_adjust_request_voltage(const u8 link_status[DP_LINK_STATUS_SIZE],
 				     int lane);
 u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SIZE],
 					  int lane);
+u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZE],
+					 unsigned int lane);
 
 #define DP_BRANCH_OUI_HEADER_SIZE	0xc
 #define DP_RECEIVER_CAP_SIZE		0xf
-- 
2.5.0

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

[PATCH 10/12] drm/dp: Enable alternate scrambler when supported

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

If the sink is eDP and supports the alternate scrambler reset, enable
it.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 11 +++++++++++
 include/drm/drm_dp_helper.h     |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index e3a289793c73..76ac68bc1042 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -316,6 +316,7 @@ static void drm_dp_link_caps_reset(struct drm_dp_link_caps *caps)
 	caps->tps3_supported = false;
 	caps->fast_training = false;
 	caps->channel_coding = false;
+	caps->alternate_scrambler_reset = false;
 }
 
 void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
@@ -325,6 +326,7 @@ void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
 	dest->tps3_supported = src->tps3_supported;
 	dest->fast_training = src->fast_training;
 	dest->channel_coding = src->channel_coding;
+	dest->alternate_scrambler_reset = src->alternate_scrambler_reset;
 }
 
 static void drm_dp_link_reset(struct drm_dp_link *link)
@@ -378,6 +380,8 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 		static const u8 edp_revs[] = { 0x11, 0x12, 0x13, 0x14 };
 		u8 value;
 
+		link->caps.alternate_scrambler_reset = true;
+
 		err = drm_dp_dpcd_readb(aux, DP_EDP_DPCD_REV, &value);
 		if (err < 0)
 			return err;
@@ -493,6 +497,13 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
 	if (err < 0)
 		return err;
 
+	if (link->caps.alternate_scrambler_reset) {
+		err = drm_dp_dpcd_writeb(aux, DP_EDP_CONFIGURATION_SET,
+					 DP_ALTERNATE_SCRAMBLER_RESET_ENABLE);
+		if (err < 0)
+			return err;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_link_configure);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 2efa6539c783..825aaf4e8c71 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -793,6 +793,7 @@ struct drm_dp_link_caps {
 	bool tps3_supported;
 	bool fast_training;
 	bool channel_coding;
+	bool alternate_scrambler_reset;
 };
 
 void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
-- 
2.5.0

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

[PATCH 11/12] drm/dp: Read AUX read interval from DPCD

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Store the AUX read interval from DPCD, so that it can be used to wait
for the durations given in the specification during link training.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c |  4 ++++
 include/drm/drm_dp_helper.h     | 17 +++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 76ac68bc1042..da519acfeba7 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -339,6 +339,7 @@ static void drm_dp_link_reset(struct drm_dp_link *link)
 	link->max_lanes = 0;
 
 	drm_dp_link_caps_reset(&link->caps);
+	link->aux_rd_interval = 0;
 	link->edp = 0;
 
 	link->rate = 0;
@@ -392,6 +393,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
 			link->edp = edp_revs[value];
 	}
 
+	/* DP_TRAINING_AUX_RD_INTERVAL is in units of 4 milliseconds */
+	link->aux_rd_interval = drm_dp_aux_rd_interval(values);
+
 	link->rate = link->max_rate;
 	link->lanes = link->max_lanes;
 
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 825aaf4e8c71..20ae0e413b64 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -678,6 +678,22 @@ drm_dp_alternate_scrambler_reset_cap(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
 			DP_ALTERNATE_SCRAMBLER_RESET_CAP;
 }
 
+/**
+ * drm_dp_read_aux_interval() - read the AUX read interval from the DPCD
+ * @dpcd: receiver capacity buffer
+ *
+ * Reads the AUX read interval (in microseconds) from the DPCD. Note that the
+ * TRAINING_AUX_RD_INTERVAL stores the value in units of 4 milliseconds.
+ *
+ * Returns:
+ * The read AUX interval in microseconds.
+ */
+static inline unsigned int
+drm_dp_aux_rd_interval(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
+{
+	return dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4000;
+}
+
 /*
  * DisplayPort AUX channel
  */
@@ -805,6 +821,7 @@ struct drm_dp_link {
 	unsigned int max_lanes;
 
 	struct drm_dp_link_caps caps;
+	unsigned int aux_rd_interval;
 	unsigned char edp;
 
 	unsigned int rate;
-- 
2.5.0

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

[PATCH 12/12] drm/dp: Add drm_dp_link_choose() helper

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This helper chooses an appropriate configuration, according to the
bitrate requirements of the video mode and the capabilities of the
DisplayPort sink.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 55 +++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_dp_helper.h     |  5 ++++
 2 files changed, 60 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index da519acfeba7..95825155dc89 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -512,6 +512,61 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
 }
 EXPORT_SYMBOL(drm_dp_link_configure);
 
+/**
+ * drm_dp_link_choose() - choose the lowest possible configuration for a mode
+ * @link: DRM DP link object
+ * @mode: DRM display mode
+ * @info: DRM display information
+ *
+ * According to the eDP specification, a source should select a configuration
+ * with the lowest number of lanes and the lowest possible link rate that can
+ * match the bitrate requirements of a video mode. However it must ensure not
+ * to exceed the capabilities of the sink.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+int drm_dp_link_choose(struct drm_dp_link *link,
+		       const struct drm_display_mode *mode,
+		       const struct drm_display_info *info)
+{
+	/* available link symbol clock rates */
+	static const unsigned int rates[3] = { 162000, 270000, 540000 };
+	/* available number of lanes */
+	static const unsigned int lanes[3] = { 1, 2, 4 };
+	unsigned long requirement, capacity;
+	unsigned int rate = link->max_rate;
+	unsigned int i, j;
+
+	/* bandwidth requirement */
+	requirement = mode->clock * info->bpc * 3;
+
+	for (i = 0; i < ARRAY_SIZE(lanes) && lanes[i] <= link->max_lanes; i++) {
+		for (j = 0; j < ARRAY_SIZE(rates) && rates[j] <= rate; j++) {
+			/*
+			 * Capacity for this combination of lanes and rate,
+			 * factoring in the ANSI 8B/10B encoding.
+			 *
+			 * Link rates in the DRM DP helpers are really link
+			 * symbol frequencies, so a tenth of the actual rate
+			 * of the link.
+			 */
+			capacity = lanes[i] * (rates[j] * 10) * 8 / 10;
+
+			if (capacity >= requirement) {
+				DRM_DEBUG_KMS("using %u lanes at %u kHz (%lu/%lu kbps)\n",
+					      lanes[i], rates[j], requirement,
+					      capacity);
+				link->lanes = lanes[i];
+				link->rate = rates[j];
+				return 0;
+			}
+		}
+	}
+
+	return -ERANGE;
+}
+EXPORT_SYMBOL(drm_dp_link_choose);
+
 /*
  * I2C-over-AUX implementation
  */
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 20ae0e413b64..f3eacf62add8 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -27,6 +27,8 @@
 #include <linux/i2c.h>
 #include <linux/delay.h>
 
+#include <drm/drm_crtc.h>
+
 /*
  * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
  * DP and DPCD versions are independent.  Differences from 1.0 are not noted,
@@ -832,6 +834,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
 int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link);
 int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link);
 int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link);
+int drm_dp_link_choose(struct drm_dp_link *link,
+		       const struct drm_display_mode *mode,
+		       const struct drm_display_info *info);
 
 int drm_dp_aux_register(struct drm_dp_aux *aux);
 void drm_dp_aux_unregister(struct drm_dp_aux *aux);
-- 
2.5.0

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

Re: [PATCH 03/12] drm/dp: Turn link capabilities into booleans

[Daniel Vetter]

On Mon, Dec 14, 2015 at 01:55:55PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Rather than storing capabilities as flags in an integer, use a separate
> boolean per capability. This simplifies the code that checks for these
> capabilities.
> 
> Cc: Rob Clark <robdclark@gmail.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

I still don't really see all that much benefit in storing this in decoded
form, but explicit bools instead of creating a new bitfiled with new DP_*
#defines that look awfully close to the spec ones but arent is much
better. This way there's at least no confusion.

Besides that bikeshed I think this patch series looks like a good step
towards unifying our dp helpers again overall. Tow comments below,
otherwise Ack on the approach from my side. Didn't do a detailed review
though.
> ---
>  drivers/gpu/drm/drm_dp_helper.c    | 18 +++++++++++++++---
>  drivers/gpu/drm/msm/edp/edp_ctrl.c |  4 ++--
>  drivers/gpu/drm/tegra/sor.c        |  4 ++--
>  include/drm/drm_dp_helper.h        | 10 ++++++++--
>  4 files changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index c00896ab8b04..cdc0da67e8ad 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -284,6 +284,17 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>  }
>  EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
>  
> +static void drm_dp_link_caps_reset(struct drm_dp_link_caps *caps)
> +{
> +	caps->enhanced_framing = false;

memset imo.

> +}
> +
> +void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
> +			   const struct drm_dp_link_caps *src)
> +{
> +	dest->enhanced_framing = src->enhanced_framing;

memcpy imo.

-Daniel

> +}
> +
>  static void drm_dp_link_reset(struct drm_dp_link *link)
>  {
>  	if (!link)
> @@ -292,7 +303,8 @@ static void drm_dp_link_reset(struct drm_dp_link *link)
>  	link->revision = 0;
>  	link->max_rate = 0;
>  	link->max_lanes = 0;
> -	link->capabilities = 0;
> +
> +	drm_dp_link_caps_reset(&link->caps);
>  
>  	link->rate = 0;
>  	link->lanes = 0;
> @@ -325,7 +337,7 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
>  	link->max_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
>  
>  	if (values[2] & DP_ENHANCED_FRAME_CAP)
> -		link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
> +		link->caps.enhanced_framing = true;
>  
>  	link->rate = link->max_rate;
>  	link->lanes = link->max_lanes;
> @@ -418,7 +430,7 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
>  	values[0] = drm_dp_link_rate_to_bw_code(link->rate);
>  	values[1] = link->lanes;
>  
> -	if (link->capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
> +	if (link->caps.enhanced_framing)
>  		values[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
>  
>  	err = drm_dp_dpcd_write(aux, DP_LINK_BW_SET, values, sizeof(values));
> diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c b/drivers/gpu/drm/msm/edp/edp_ctrl.c
> index c87a03561246..12e2a5cd9582 100644
> --- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
> +++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
> @@ -452,7 +452,7 @@ static void edp_config_ctrl(struct edp_ctrl *ctrl)
>  
>  	data = EDP_CONFIGURATION_CTRL_LANES(ctrl->lane_cnt - 1);
>  
> -	if (ctrl->dp_link.capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
> +	if (ctrl->dp_link.caps.enhanced_framing)
>  		data |= EDP_CONFIGURATION_CTRL_ENHANCED_FRAMING;
>  
>  	depth = EDP_6BIT;
> @@ -774,7 +774,7 @@ static int edp_do_link_train(struct edp_ctrl *ctrl)
>  	 */
>  	dp_link.lanes = ctrl->lane_cnt;
>  	dp_link.rate = drm_dp_bw_code_to_link_rate(ctrl->link_rate);
> -	dp_link.capabilities = ctrl->dp_link.capabilities;
> +	drm_dp_link_caps_copy(&dp_link.caps, &ctrl->dp_link.caps);
>  	if (drm_dp_link_configure(ctrl->drm_aux, &dp_link) < 0)
>  		return EDP_TRAIN_FAIL;
>  
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index 7bf444bd1588..f26dfaca7586 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -645,7 +645,7 @@ static int tegra_sor_calc_config(struct tegra_sor *sor,
>  	num = ((mode->htotal - mode->hdisplay) - 7) * link_rate;
>  	config->hblank_symbols = div_u64(num, pclk);
>  
> -	if (link->capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
> +	if (link->caps.enhanced_framing)
>  		config->hblank_symbols -= 3;
>  
>  	config->hblank_symbols -= 12 / link->lanes;
> @@ -1464,7 +1464,7 @@ static void tegra_sor_edp_enable(struct drm_encoder *encoder)
>  		value &= ~SOR_DP_LINKCTL_LANE_COUNT_MASK;
>  		value |= SOR_DP_LINKCTL_LANE_COUNT(lanes);
>  
> -		if (link.capabilities & DP_LINK_CAP_ENHANCED_FRAMING)
> +		if (link.caps.enhanced_framing)
>  			value |= SOR_DP_LINKCTL_ENHANCED_FRAME;
>  
>  		tegra_sor_writel(sor, value, SOR_DP_LINKCTL0);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 124fea3ffd57..20e4fe2e13c5 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -753,13 +753,19 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>  /*
>   * DisplayPort link
>   */
> -#define DP_LINK_CAP_ENHANCED_FRAMING (1 << 0)
> +struct drm_dp_link_caps {
> +	bool enhanced_framing;
> +};
> +
> +void drm_dp_link_caps_copy(struct drm_dp_link_caps *dest,
> +			   const struct drm_dp_link_caps *src);
>  
>  struct drm_dp_link {
>  	unsigned char revision;
>  	unsigned int max_rate;
>  	unsigned int max_lanes;
> -	unsigned long capabilities;
> +
> +	struct drm_dp_link_caps caps;
>  
>  	unsigned int rate;
>  	unsigned int lanes;
> -- 
> 2.5.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/12] drm/dp: Read AUX read interval from DPCD

[Daniel Vetter]

On Mon, Dec 14, 2015 at 01:56:03PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Store the AUX read interval from DPCD, so that it can be used to wait
> for the durations given in the specification during link training.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c |  4 ++++
>  include/drm/drm_dp_helper.h     | 17 +++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 76ac68bc1042..da519acfeba7 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -339,6 +339,7 @@ static void drm_dp_link_reset(struct drm_dp_link *link)
>  	link->max_lanes = 0;
>  
>  	drm_dp_link_caps_reset(&link->caps);
> +	link->aux_rd_interval = 0;
>  	link->edp = 0;
>  
>  	link->rate = 0;
> @@ -392,6 +393,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
>  			link->edp = edp_revs[value];
>  	}
>  
> +	/* DP_TRAINING_AUX_RD_INTERVAL is in units of 4 milliseconds */
> +	link->aux_rd_interval = drm_dp_aux_rd_interval(values);

Hm, just wondering a bit of the relationship between link and cap. Is link
all about sink really, and not the source? At least in my experience it
makes a lot of sense to strictly keep these two separate, since otherwise
you'll have lots of fun aligning things in generic code. Anyway, just a
thougth.

> +
>  	link->rate = link->max_rate;
>  	link->lanes = link->max_lanes;
>  
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 825aaf4e8c71..20ae0e413b64 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -678,6 +678,22 @@ drm_dp_alternate_scrambler_reset_cap(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
>  			DP_ALTERNATE_SCRAMBLER_RESET_CAP;
>  }
>  
> +/**
> + * drm_dp_read_aux_interval() - read the AUX read interval from the DPCD
> + * @dpcd: receiver capacity buffer
> + *
> + * Reads the AUX read interval (in microseconds) from the DPCD. Note that the
> + * TRAINING_AUX_RD_INTERVAL stores the value in units of 4 milliseconds.
> + *
> + * Returns:
> + * The read AUX interval in microseconds.
> + */
> +static inline unsigned int
> +drm_dp_aux_rd_interval(const u8 dpcd[DP_RECEIVER_CAP_SIZE])

We should use this one here in the 2 delay helpers for channel_eq and
clock_recovery imo.
-Daniel

> +{
> +	return dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4000;
> +}
> +
>  /*
>   * DisplayPort AUX channel
>   */
> @@ -805,6 +821,7 @@ struct drm_dp_link {
>  	unsigned int max_lanes;
>  
>  	struct drm_dp_link_caps caps;
> +	unsigned int aux_rd_interval;
>  	unsigned char edp;
>  
>  	unsigned int rate;
> -- 
> 2.5.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 12/12] drm/dp: Add drm_dp_link_choose() helper

[Jani Nikula]

On Mon, 14 Dec 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> This helper chooses an appropriate configuration, according to the
> bitrate requirements of the video mode and the capabilities of the
> DisplayPort sink.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 55 +++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_dp_helper.h     |  5 ++++
>  2 files changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index da519acfeba7..95825155dc89 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -512,6 +512,61 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
>  }
>  EXPORT_SYMBOL(drm_dp_link_configure);
>  
> +/**
> + * drm_dp_link_choose() - choose the lowest possible configuration for a mode
> + * @link: DRM DP link object
> + * @mode: DRM display mode
> + * @info: DRM display information
> + *
> + * According to the eDP specification, a source should select a configuration
> + * with the lowest number of lanes and the lowest possible link rate that can
> + * match the bitrate requirements of a video mode. However it must ensure not
> + * to exceed the capabilities of the sink.

Eventually this would have to take into account the intersection of
per-sink and per-source supported rates, including the intermediate
frequencies. Until then, i915 couldn't switch over.

BR,
Jani.


> + *
> + * Returns: 0 on success or a negative error code on failure.
> + */
> +int drm_dp_link_choose(struct drm_dp_link *link,
> +		       const struct drm_display_mode *mode,
> +		       const struct drm_display_info *info)
> +{
> +	/* available link symbol clock rates */
> +	static const unsigned int rates[3] = { 162000, 270000, 540000 };
> +	/* available number of lanes */
> +	static const unsigned int lanes[3] = { 1, 2, 4 };
> +	unsigned long requirement, capacity;
> +	unsigned int rate = link->max_rate;
> +	unsigned int i, j;
> +
> +	/* bandwidth requirement */
> +	requirement = mode->clock * info->bpc * 3;
> +
> +	for (i = 0; i < ARRAY_SIZE(lanes) && lanes[i] <= link->max_lanes; i++) {
> +		for (j = 0; j < ARRAY_SIZE(rates) && rates[j] <= rate; j++) {
> +			/*
> +			 * Capacity for this combination of lanes and rate,
> +			 * factoring in the ANSI 8B/10B encoding.
> +			 *
> +			 * Link rates in the DRM DP helpers are really link
> +			 * symbol frequencies, so a tenth of the actual rate
> +			 * of the link.
> +			 */
> +			capacity = lanes[i] * (rates[j] * 10) * 8 / 10;
> +
> +			if (capacity >= requirement) {
> +				DRM_DEBUG_KMS("using %u lanes at %u kHz (%lu/%lu kbps)\n",
> +					      lanes[i], rates[j], requirement,
> +					      capacity);
> +				link->lanes = lanes[i];
> +				link->rate = rates[j];
> +				return 0;
> +			}
> +		}
> +	}
> +
> +	return -ERANGE;
> +}
> +EXPORT_SYMBOL(drm_dp_link_choose);
> +
>  /*
>   * I2C-over-AUX implementation
>   */
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 20ae0e413b64..f3eacf62add8 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -27,6 +27,8 @@
>  #include <linux/i2c.h>
>  #include <linux/delay.h>
>  
> +#include <drm/drm_crtc.h>
> +
>  /*
>   * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
>   * DP and DPCD versions are independent.  Differences from 1.0 are not noted,
> @@ -832,6 +834,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link);
> +int drm_dp_link_choose(struct drm_dp_link *link,
> +		       const struct drm_display_mode *mode,
> +		       const struct drm_display_info *info);
>  
>  int drm_dp_aux_register(struct drm_dp_aux *aux);
>  void drm_dp_aux_unregister(struct drm_dp_aux *aux);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 12/12] drm/dp: Add drm_dp_link_choose() helper

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2226 bytes --]

On Sun, Jan 31, 2016 at 04:39:51PM +0200, Jani Nikula wrote:
> On Mon, 14 Dec 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > This helper chooses an appropriate configuration, according to the
> > bitrate requirements of the video mode and the capabilities of the
> > DisplayPort sink.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 55 +++++++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_dp_helper.h     |  5 ++++
> >  2 files changed, 60 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > index da519acfeba7..95825155dc89 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -512,6 +512,61 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
> >  }
> >  EXPORT_SYMBOL(drm_dp_link_configure);
> >  
> > +/**
> > + * drm_dp_link_choose() - choose the lowest possible configuration for a mode
> > + * @link: DRM DP link object
> > + * @mode: DRM display mode
> > + * @info: DRM display information
> > + *
> > + * According to the eDP specification, a source should select a configuration
> > + * with the lowest number of lanes and the lowest possible link rate that can
> > + * match the bitrate requirements of a video mode. However it must ensure not
> > + * to exceed the capabilities of the sink.
> 
> Eventually this would have to take into account the intersection of
> per-sink and per-source supported rates, including the intermediate
> frequencies. Until then, i915 couldn't switch over.

I'm not sure I understand what you're saying. The idea is that drivers
will call the drm_dp_link_probe() helper to probe the sink for supported
frequencies, number of lanes and capabilities.

After that, drivers are supposed to adjust the maximum values to account
for their limitations. Hence the name for struct drm_dp_*link*, because
it contains the negotiated parameters for the link between source and
sink.

That seems to me like the logical procedure when following the spec. Is
that not how i915 works?

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 11/12] drm/dp: Read AUX read interval from DPCD

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 3980 bytes --]

On Tue, Dec 15, 2015 at 11:38:10AM +0100, Daniel Vetter wrote:
> On Mon, Dec 14, 2015 at 01:56:03PM +0100, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Store the AUX read interval from DPCD, so that it can be used to wait
> > for the durations given in the specification during link training.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c |  4 ++++
> >  include/drm/drm_dp_helper.h     | 17 +++++++++++++++++
> >  2 files changed, 21 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > index 76ac68bc1042..da519acfeba7 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -339,6 +339,7 @@ static void drm_dp_link_reset(struct drm_dp_link *link)
> >  	link->max_lanes = 0;
> >  
> >  	drm_dp_link_caps_reset(&link->caps);
> > +	link->aux_rd_interval = 0;
> >  	link->edp = 0;
> >  
> >  	link->rate = 0;
> > @@ -392,6 +393,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
> >  			link->edp = edp_revs[value];
> >  	}
> >  
> > +	/* DP_TRAINING_AUX_RD_INTERVAL is in units of 4 milliseconds */
> > +	link->aux_rd_interval = drm_dp_aux_rd_interval(values);
> 
> Hm, just wondering a bit of the relationship between link and cap. Is link
> all about sink really, and not the source? At least in my experience it
> makes a lot of sense to strictly keep these two separate, since otherwise
> you'll have lots of fun aligning things in generic code. Anyway, just a
> thougth.

The idea is that the link is the intersection between sink and source
capabilities. Drivers are supposed to call drm_dp_link_probe() to obtain
the capabilities of the sink and then adjust the struct drm_dp_link
according to their limitations (e.g. decrease the maximum rate if they
don't support 5.4 GHz, reduce the number of lanes if they only support
two, ...).

Once that's done the drivers can call drm_dp_link_choose() to select the
"best" set of configuration parameters given the link capabilities.

Note that this is strictly deriven from reading the specification under
the assumption that this is how things work in real life. My, arguably
limited, experience with Tegra shows that this is true. But perhaps that
is overly naive.

But I'd like to better understand what other drivers require so that
these helpers can be improved and be useful by more than a single
driver. Currently every driver implements their own DP stack, which I
think is rather unfortunate because we end up with vastly different
behaviour depending on which driver is in use.

Of course if that's what's desired, I'm more than happy to move this
code into the Tegra driver. I might have to duplicate the code that's
shared with MSM, but it's really not a lot compared to what's coming
up.

> > +
> >  	link->rate = link->max_rate;
> >  	link->lanes = link->max_lanes;
> >  
> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > index 825aaf4e8c71..20ae0e413b64 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -678,6 +678,22 @@ drm_dp_alternate_scrambler_reset_cap(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> >  			DP_ALTERNATE_SCRAMBLER_RESET_CAP;
> >  }
> >  
> > +/**
> > + * drm_dp_read_aux_interval() - read the AUX read interval from the DPCD
> > + * @dpcd: receiver capacity buffer
> > + *
> > + * Reads the AUX read interval (in microseconds) from the DPCD. Note that the
> > + * TRAINING_AUX_RD_INTERVAL stores the value in units of 4 milliseconds.
> > + *
> > + * Returns:
> > + * The read AUX interval in microseconds.
> > + */
> > +static inline unsigned int
> > +drm_dp_aux_rd_interval(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> 
> We should use this one here in the 2 delay helpers for channel_eq and
> clock_recovery imo.

Agreed.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 12/12] drm/dp: Add drm_dp_link_choose() helper

[Jani Nikula]

On Mon, 29 Feb 2016, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Sun, Jan 31, 2016 at 04:39:51PM +0200, Jani Nikula wrote:
>> On Mon, 14 Dec 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
>> > From: Thierry Reding <treding@nvidia.com>
>> >
>> > This helper chooses an appropriate configuration, according to the
>> > bitrate requirements of the video mode and the capabilities of the
>> > DisplayPort sink.
>> >
>> > Signed-off-by: Thierry Reding <treding@nvidia.com>
>> > ---
>> >  drivers/gpu/drm/drm_dp_helper.c | 55 +++++++++++++++++++++++++++++++++++++++++
>> >  include/drm/drm_dp_helper.h     |  5 ++++
>> >  2 files changed, 60 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> > index da519acfeba7..95825155dc89 100644
>> > --- a/drivers/gpu/drm/drm_dp_helper.c
>> > +++ b/drivers/gpu/drm/drm_dp_helper.c
>> > @@ -512,6 +512,61 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link)
>> >  }
>> >  EXPORT_SYMBOL(drm_dp_link_configure);
>> >  
>> > +/**
>> > + * drm_dp_link_choose() - choose the lowest possible configuration for a mode
>> > + * @link: DRM DP link object
>> > + * @mode: DRM display mode
>> > + * @info: DRM display information
>> > + *
>> > + * According to the eDP specification, a source should select a configuration
>> > + * with the lowest number of lanes and the lowest possible link rate that can
>> > + * match the bitrate requirements of a video mode. However it must ensure not
>> > + * to exceed the capabilities of the sink.
>> 
>> Eventually this would have to take into account the intersection of
>> per-sink and per-source supported rates, including the intermediate
>> frequencies. Until then, i915 couldn't switch over.
>
> I'm not sure I understand what you're saying. The idea is that drivers
> will call the drm_dp_link_probe() helper to probe the sink for supported
> frequencies, number of lanes and capabilities.
>
> After that, drivers are supposed to adjust the maximum values to account
> for their limitations. Hence the name for struct drm_dp_*link*, because
> it contains the negotiated parameters for the link between source and
> sink.
>
> That seems to me like the logical procedure when following the spec. Is
> that not how i915 works?

eDP v1.4 introduces more link rates. DPCD 10h through 1fh may contain a
table of the supported link rates, AFAICT the spec is written in a way
to allow arbitrary rates, but at least 1.62, 2.16, 2.43, 2.7, 3.24,
4.32, and 5.4 Gbps are cited and more details are provided for
those. The support is not limited by the maximum only, so an
intersection of the rates supported by source and sink will be required.

Obviously _link_probe() needs to be updated to figure out the supported
rates, not just the max, and _link_choose() can't assume a fixed set of
rates either.

i915 supports this. I'm not saying that what you're doing must support
all of this from the start, but I think you should get the eDP v1.4 spec
to make sure adding the support later on will be as straightforward as
possible.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/12] drm/dp: Read AUX read interval from DPCD

[Daniel Vetter]

On Mon, Feb 29, 2016 at 07:38:21AM +0100, Thierry Reding wrote:
> On Tue, Dec 15, 2015 at 11:38:10AM +0100, Daniel Vetter wrote:
> > On Mon, Dec 14, 2015 at 01:56:03PM +0100, Thierry Reding wrote:
> > > From: Thierry Reding <treding@nvidia.com>
> > > 
> > > Store the AUX read interval from DPCD, so that it can be used to wait
> > > for the durations given in the specification during link training.
> > > 
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > >  drivers/gpu/drm/drm_dp_helper.c |  4 ++++
> > >  include/drm/drm_dp_helper.h     | 17 +++++++++++++++++
> > >  2 files changed, 21 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > > index 76ac68bc1042..da519acfeba7 100644
> > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > @@ -339,6 +339,7 @@ static void drm_dp_link_reset(struct drm_dp_link *link)
> > >  	link->max_lanes = 0;
> > >  
> > >  	drm_dp_link_caps_reset(&link->caps);
> > > +	link->aux_rd_interval = 0;
> > >  	link->edp = 0;
> > >  
> > >  	link->rate = 0;
> > > @@ -392,6 +393,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
> > >  			link->edp = edp_revs[value];
> > >  	}
> > >  
> > > +	/* DP_TRAINING_AUX_RD_INTERVAL is in units of 4 milliseconds */
> > > +	link->aux_rd_interval = drm_dp_aux_rd_interval(values);
> > 
> > Hm, just wondering a bit of the relationship between link and cap. Is link
> > all about sink really, and not the source? At least in my experience it
> > makes a lot of sense to strictly keep these two separate, since otherwise
> > you'll have lots of fun aligning things in generic code. Anyway, just a
> > thougth.
> 
> The idea is that the link is the intersection between sink and source
> capabilities. Drivers are supposed to call drm_dp_link_probe() to obtain
> the capabilities of the sink and then adjust the struct drm_dp_link
> according to their limitations (e.g. decrease the maximum rate if they
> don't support 5.4 GHz, reduce the number of lanes if they only support
> two, ...).
> 
> Once that's done the drivers can call drm_dp_link_choose() to select the
> "best" set of configuration parameters given the link capabilities.
> 
> Note that this is strictly deriven from reading the specification under
> the assumption that this is how things work in real life. My, arguably
> limited, experience with Tegra shows that this is true. But perhaps that
> is overly naive.
> 
> But I'd like to better understand what other drivers require so that
> these helpers can be improved and be useful by more than a single
> driver. Currently every driver implements their own DP stack, which I
> think is rather unfortunate because we end up with vastly different
> behaviour depending on which driver is in use.
> 
> Of course if that's what's desired, I'm more than happy to move this
> code into the Tegra driver. I might have to duplicate the code that's
> shared with MSM, but it's really not a lot compared to what's coming
> up.

Ah, that makes sense to have a link structure for the negotioted values
that fit both sink and source. In that case I think making it a bit leaner
would be good, and only include parameters which actually need to be
negiotated (link rate, lanes, ...). aux retry intervals and stuff like
that should be agnostic, or at least only depend upon the sink (I hope).

Given that I don't think we should store such values, but instead
recompute them from the read/cached dcpd blocks. That would be more
in-line with the style of the helpers used by i915/radeon. This way the
actually negotiated stuff sticks out more. And like you say we need that
in some struct so that drivers can apply quirks/tricks (i.e. simply to
avoid the midlayer mistake). E.g. one thing i915 does is up-front link
training, to be able to handle usb type C correctly.

Another reason why I think we should charge ahead with caching source/sink
invariant values too much is that I still believe extracting SST helpers
from MST helpers is a good long-term plan. Keeping is simple (and
accepting more copypasting due to that) will make the inevitable
refactoring easier I hope.

Cheers, Daniel

> > > +
> > >  	link->rate = link->max_rate;
> > >  	link->lanes = link->max_lanes;
> > >  
> > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > > index 825aaf4e8c71..20ae0e413b64 100644
> > > --- a/include/drm/drm_dp_helper.h
> > > +++ b/include/drm/drm_dp_helper.h
> > > @@ -678,6 +678,22 @@ drm_dp_alternate_scrambler_reset_cap(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> > >  			DP_ALTERNATE_SCRAMBLER_RESET_CAP;
> > >  }
> > >  
> > > +/**
> > > + * drm_dp_read_aux_interval() - read the AUX read interval from the DPCD
> > > + * @dpcd: receiver capacity buffer
> > > + *
> > > + * Reads the AUX read interval (in microseconds) from the DPCD. Note that the
> > > + * TRAINING_AUX_RD_INTERVAL stores the value in units of 4 milliseconds.
> > > + *
> > > + * Returns:
> > > + * The read AUX interval in microseconds.
> > > + */
> > > +static inline unsigned int
> > > +drm_dp_aux_rd_interval(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> > 
> > We should use this one here in the 2 delay helpers for channel_eq and
> > clock_recovery imo.
> 
> Agreed.
> 
> Thierry



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel