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(-)
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)
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);
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;
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
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]);
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,
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; };
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,
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;
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
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,
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;
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
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
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
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);
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);
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
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.
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Jani Nikula -
Thierry Reding