From: Manasi Navare <manasi.d.navare@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 4/7] drm/i915: Add {preemph, voltage}_max() vfuncs
Date: Fri, 15 May 2020 12:09:44 -0700 [thread overview]
Message-ID: <20200515190944.GA20478@intel.com> (raw)
In-Reply-To: <20200512174145.3186-5-ville.syrjala@linux.intel.com>
On Tue, May 12, 2020 at 08:41:42PM +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.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
This is so much cleaner, thanks for the patch,
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> ---
> 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 798889f72495..f873fd03ac14 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 87876fce91a5..46e91574f178 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 4952918d0904..6d790633e667 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)
> @@ -8325,6 +8291,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.26.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
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-15 19:08 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-12 17:41 [Intel-gfx] [PATCH 0/7] drm/i915: DP vswing/preemph fixes Ville Syrjala
2020-05-12 17:41 ` [Intel-gfx] [PATCH 1/7] drm/i915: Fix cpt/ppt max pre-emphasis Ville Syrjala
2020-05-30 1:30 ` Souza, Jose
2020-05-12 17:41 ` [Intel-gfx] [PATCH 2/7] drm/i915: Fix ibx max vswing/preemph Ville Syrjala
2020-06-01 20:23 ` Souza, Jose
2020-05-12 17:41 ` [Intel-gfx] [PATCH 3/7] drm/i915: Fix ivb cpu edp vswing Ville Syrjala
2020-05-30 1:38 ` Souza, Jose
2020-05-12 17:41 ` [Intel-gfx] [PATCH 4/7] drm/i915: Add {preemph, voltage}_max() vfuncs Ville Syrjala
2020-05-15 19:09 ` Manasi Navare [this message]
2020-05-12 17:41 ` [Intel-gfx] [PATCH 5/7] drm/i915: Reverse preemph vs. voltage swing preference Ville Syrjala
2020-05-15 19:18 ` Manasi Navare
2020-05-15 19:59 ` Ville Syrjälä
2020-05-18 18:20 ` Manasi Navare
2020-05-12 17:41 ` [Intel-gfx] [PATCH 6/7] drm/i915: Fix DP_TRAIN_MAX_{PRE_EMPHASIS, SWING}_REACHED handling Ville Syrjala
2020-06-25 21:06 ` Imre Deak
2020-05-12 17:41 ` [Intel-gfx] [PATCH 7/7] drm/i915: Replace some hand rolled max()s Ville Syrjala
2020-05-15 19:21 ` Manasi Navare
2020-05-12 18:38 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: DP vswing/preemph fixes Patchwork
2020-05-12 20:11 ` [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=20200515190944.GA20478@intel.com \
--to=manasi.d.navare@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/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.