From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 4/9] drm/i915: Add {preemph, voltage}_max() vfuncs
Date: Thu, 7 May 2020 10:09:07 +0300 [thread overview]
Message-ID: <20200507070907.GZ6112@intel.com> (raw)
In-Reply-To: <20200506112328.16562-5-ville.syrjala@linux.intel.com>
On Wed, May 06, 2020 at 02:23:23PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Different platforms have different max vswing/preemph settings.
> Turn that into a pair vfuncs so we can decouple intel_dp.c and
> intel_ddi.c further.
Forgot to mention that it not only depends on the platform, but also
the link rate which is a runtime thing. So unfortunately can't just
make these into fixed init time assignments.
Though I have been pondering about making an init time split
between eDP vs. eDP low vswing vs. DP vs. HDMI though. That
could potentially make some of the if ladders in the ddi dbuf
trans code less confusing. Haven't actually tried to code
it up to see how it would look in the end.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 21 ++----
> drivers/gpu/drm/i915/display/intel_ddi.h | 3 -
> .../drm/i915/display/intel_display_types.h | 3 +
> drivers/gpu/drm/i915/display/intel_dp.c | 67 ++++++-------------
> drivers/gpu/drm/i915/display/intel_dp.h | 4 --
> .../drm/i915/display/intel_dp_link_training.c | 20 +++++-
> 6 files changed, 49 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 5601673c3f30..409b8a68570f 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2095,10 +2095,10 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
> ddi_translations[level].deemphasis);
> }
>
> -u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
> +static u8 intel_ddi_dp_voltage_max(struct intel_dp *intel_dp)
> {
> + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> - struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> enum port port = encoder->port;
> enum phy phy = intel_port_to_phy(dev_priv, port);
> int n_entries;
> @@ -2151,19 +2151,9 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
> * used on all DDI platforms. Should that change we need to
> * rethink this code.
> */
> -u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder, u8 voltage_swing)
> +static u8 intel_ddi_dp_preemph_max(struct intel_dp *intel_dp)
> {
> - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> - return DP_TRAIN_PRE_EMPH_LEVEL_3;
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> - return DP_TRAIN_PRE_EMPH_LEVEL_2;
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> - return DP_TRAIN_PRE_EMPH_LEVEL_1;
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> - default:
> - return DP_TRAIN_PRE_EMPH_LEVEL_0;
> - }
> + return DP_TRAIN_PRE_EMPH_LEVEL_3;
> }
>
> static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
> @@ -4510,6 +4500,9 @@ intel_ddi_init_dp_connector(struct intel_digital_port *intel_dig_port)
> else
> intel_dig_port->dp.set_signal_levels = hsw_set_signal_levels;
>
> + intel_dig_port->dp.voltage_max = intel_ddi_dp_voltage_max;
> + intel_dig_port->dp.preemph_max = intel_ddi_dp_preemph_max;
> +
> if (INTEL_GEN(dev_priv) < 12) {
> intel_dig_port->dp.regs.dp_tp_ctl = DP_TP_CTL(port);
> intel_dig_port->dp.regs.dp_tp_status = DP_TP_STATUS(port);
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h b/drivers/gpu/drm/i915/display/intel_ddi.h
> index fbdf8ddde486..077e9dbbe367 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.h
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.h
> @@ -42,9 +42,6 @@ void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
> struct intel_crtc_state *crtc_state);
> u32 bxt_signal_levels(struct intel_dp *intel_dp);
> u32 ddi_signal_levels(struct intel_dp *intel_dp);
> -u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder);
> -u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder,
> - u8 voltage_swing);
> int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> bool enable);
> void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 9488449e4b94..e0384af489c2 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1371,6 +1371,9 @@ struct intel_dp {
> void (*set_idle_link_train)(struct intel_dp *intel_dp);
> void (*set_signal_levels)(struct intel_dp *intel_dp);
>
> + u8 (*preemph_max)(struct intel_dp *intel_dp);
> + u8 (*voltage_max)(struct intel_dp *intel_dp);
> +
> /* Displayport compliance testing */
> struct intel_dp_compliance compliance;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index a52f01c48644..1b786d5af383 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3947,58 +3947,24 @@ intel_dp_get_link_status(struct intel_dp *intel_dp, u8 link_status[DP_LINK_STATU
> DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
> }
>
> -/* These are source-specific values. */
> -u8
> -intel_dp_voltage_max(struct intel_dp *intel_dp)
> +static u8 intel_dp_voltage_max_2(struct intel_dp *intel_dp)
> {
> - struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> - enum port port = encoder->port;
> + return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> +}
>
> - if (HAS_DDI(dev_priv))
> - return intel_ddi_dp_voltage_max(encoder);
> - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv) ||
> - (HAS_PCH_SPLIT(dev_priv) && port != PORT_A))
> - return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> - else
> - return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> +static u8 intel_dp_voltage_max_3(struct intel_dp *intel_dp)
> +{
> + return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> }
>
> -u8
> -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, u8 voltage_swing)
> +static u8 intel_dp_pre_empemph_max_2(struct intel_dp *intel_dp)
> {
> - struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> - struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> - enum port port = encoder->port;
> + return DP_TRAIN_PRE_EMPH_LEVEL_2;
> +}
>
> - if (HAS_DDI(dev_priv)) {
> - return intel_ddi_dp_pre_emphasis_max(encoder, voltage_swing);
> - } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv) ||
> - (HAS_PCH_SPLIT(dev_priv) && port != PORT_A)) {
> - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> - return DP_TRAIN_PRE_EMPH_LEVEL_3;
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> - return DP_TRAIN_PRE_EMPH_LEVEL_2;
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> - return DP_TRAIN_PRE_EMPH_LEVEL_1;
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> - default:
> - return DP_TRAIN_PRE_EMPH_LEVEL_0;
> - }
> - } else {
> - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> - return DP_TRAIN_PRE_EMPH_LEVEL_2;
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> - return DP_TRAIN_PRE_EMPH_LEVEL_2;
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> - return DP_TRAIN_PRE_EMPH_LEVEL_1;
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> - default:
> - return DP_TRAIN_PRE_EMPH_LEVEL_0;
> - }
> - }
> +static u8 intel_dp_pre_empemph_max_3(struct intel_dp *intel_dp)
> +{
> + return DP_TRAIN_PRE_EMPH_LEVEL_3;
> }
>
> static void vlv_set_signal_levels(struct intel_dp *intel_dp)
> @@ -8491,6 +8457,15 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
> else
> intel_dig_port->dp.set_signal_levels = g4x_set_signal_levels;
>
> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv) ||
> + (HAS_PCH_SPLIT(dev_priv) && port != PORT_A)) {
> + intel_dig_port->dp.preemph_max = intel_dp_pre_empemph_max_3;
> + intel_dig_port->dp.voltage_max = intel_dp_voltage_max_3;
> + } else {
> + intel_dig_port->dp.preemph_max = intel_dp_pre_empemph_max_2;
> + intel_dig_port->dp.voltage_max = intel_dp_voltage_max_2;
> + }
> +
> intel_dig_port->dp.output_reg = output_reg;
> intel_dig_port->max_lanes = 4;
> intel_dig_port->dp.regs.dp_tp_ctl = DP_TP_CTL(port);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 6659ce15a693..e8375a75c3ec 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -91,10 +91,6 @@ intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
> void
> intel_dp_set_signal_levels(struct intel_dp *intel_dp);
> void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
> -u8
> -intel_dp_voltage_max(struct intel_dp *intel_dp);
> -u8
> -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, u8 voltage_swing);
> void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
> u8 *link_bw, u8 *rate_select);
> bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index e4f1843170b7..171d9e842fc0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -34,6 +34,21 @@ intel_dp_dump_link_status(const u8 link_status[DP_LINK_STATUS_SIZE])
> link_status[3], link_status[4], link_status[5]);
> }
>
> +static u8 dp_pre_emphasis_max(u8 voltage_swing)
> +{
> + switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> + return DP_TRAIN_PRE_EMPH_LEVEL_3;
> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> + return DP_TRAIN_PRE_EMPH_LEVEL_2;
> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> + return DP_TRAIN_PRE_EMPH_LEVEL_1;
> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> + default:
> + return DP_TRAIN_PRE_EMPH_LEVEL_0;
> + }
> +}
> +
> void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
> const u8 link_status[DP_LINK_STATUS_SIZE])
> {
> @@ -53,11 +68,12 @@ void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
> p = this_p;
> }
>
> - voltage_max = intel_dp_voltage_max(intel_dp);
> + voltage_max = intel_dp->voltage_max(intel_dp);
> if (v >= voltage_max)
> v = voltage_max | DP_TRAIN_MAX_SWING_REACHED;
>
> - preemph_max = intel_dp_pre_emphasis_max(intel_dp, v);
> + preemph_max = min(intel_dp->preemph_max(intel_dp),
> + dp_pre_emphasis_max(v));
> if (p >= preemph_max)
> p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
>
> --
> 2.24.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-05-07 7:16 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-06 11:23 [Intel-gfx] [PATCH 0/9] drm/i915: Plumb crtc state to link training code Ville Syrjala
2020-05-06 11:23 ` [Intel-gfx] [PATCH 1/9] drm/i915: Fix cpt/ppt max pre-emphasis Ville Syrjala
2020-05-06 11:23 ` [Intel-gfx] [PATCH 2/9] drm/i915: Fix ibx max vswing/preemph Ville Syrjala
2020-05-06 11:23 ` [Intel-gfx] [PATCH 3/9] drm/i915: Fix ivb cpu edp vswing Ville Syrjala
2020-05-06 11:23 ` [Intel-gfx] [PATCH 4/9] drm/i915: Add {preemph, voltage}_max() vfuncs Ville Syrjala
2020-05-07 7:09 ` Ville Syrjälä [this message]
2020-05-06 11:23 ` [Intel-gfx] [PATCH 5/9] drm/i915: Reverse preemph vs. voltage swing preference Ville Syrjala
2020-05-06 11:23 ` [Intel-gfx] [PATCH 6/9] drm/i915: Fix DP_TRAIN_MAX_{PRE_EMPHASIS, SWING}_REACHED handling Ville Syrjala
2020-05-06 11:23 ` [Intel-gfx] [PATCH 7/9] drm/i915: Replace some hand rolled max()s Ville Syrjala
2020-05-06 11:23 ` [Intel-gfx] [PATCH 8/9] drm/i915: Plumb crtc_state to link training Ville Syrjala
2020-05-06 12:05 ` [Intel-gfx] [PATCH v2 " Ville Syrjala
2020-05-06 11:23 ` [Intel-gfx] [PATCH 9/9] drm/i915: Eliminate intel_dp.regs.dp_tp_{ctl, status} Ville Syrjala
2020-05-06 11:29 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Plumb crtc state to link training code Patchwork
2020-05-06 12:49 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Plumb crtc state to link training code (rev2) Patchwork
2020-05-06 13:48 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200507070907.GZ6112@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.