From: Mika Kahola <mika.kahola@intel.com>
To: Jim Bride <jim.bride@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2 14/14] drm/i915/dp/mst: Add support for upfront link training for DP MST
Date: Thu, 08 Sep 2016 13:21:22 +0300 [thread overview]
Message-ID: <1473330082.28727.32.camel@intel.com> (raw)
In-Reply-To: <20160907164022.GA29025@shiv>
On Wed, 2016-09-07 at 09:40 -0700, Jim Bride wrote:
> On Wed, Sep 07, 2016 at 01:53:31PM +0300, Mika Kahola wrote:
> >
> > On Tue, 2016-09-06 at 17:13 -0700, Manasi Navare wrote:
> > >
> > > From: Jim Bride <jim.bride@linux.intel.com>
> > >
> > > Add upfront link training to intel_dp_mst_mode_valid() so that we
> > > know
> > > topology constraints before we validate the legality of modes to
> > > be
> > > checked.
> > > Call the function that loops through the link rates and lane
> > > counts
> > > starting from highest supported link rate and lane count for
> > > training
> > > the link in compliance with DP spec
> > >
> > > v2:
> > > * Rebased on new revision of link training patch (Manasi Navare)
> > >
> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 9 ++---
> > > drivers/gpu/drm/i915/intel_dp_mst.c | 74
> > > +++++++++++++++++++++++++++----------
> > > drivers/gpu/drm/i915/intel_drv.h | 3 ++
> > > 3 files changed, 61 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 7794180..0c7674f 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -131,7 +131,7 @@ static void vlv_steal_power_sequencer(struct
> > > drm_device *dev,
> > > enum pipe pipe);
> > > static void intel_dp_unset_edid(struct intel_dp *intel_dp);
> > >
> > > -static int
> > > +int
> > > intel_dp_max_link_bw(struct intel_dp *intel_dp)
> > > {
> > > int max_link_bw = intel_dp->dpcd[DP_MAX_LINK_RATE];
> > > @@ -150,7 +150,7 @@ intel_dp_max_link_bw(struct
> > > intel_dp *intel_dp)
> > > return max_link_bw;
> > > }
> > >
> > > -static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
> > > +u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
> > > {
> > > struct intel_digital_port *intel_dig_port =
> > > dp_to_dig_port(intel_dp);
> > > u8 temp, source_max, sink_max;
> > > @@ -312,8 +312,7 @@ static int intersect_rates(const int
> > > *source_rates, int source_len,
> > > return k;
> > > }
> > >
> > > -static int intel_dp_common_rates(struct intel_dp *intel_dp,
> > > - int *common_rates)
> > > +int intel_dp_common_rates(struct intel_dp *intel_dp, int
> > > *common_rates)
> > > {
> > > const int *source_rates, *sink_rates;
> > > int source_len, sink_len;
> > > @@ -336,7 +335,7 @@ static int intel_dp_common_rates(struct
> > > intel_dp
> > > *intel_dp,
> > > common_rates);
> > > }
> > >
> > > -static bool intel_dp_upfront_link_train(struct intel_dp
> > > *intel_dp)
> > > +bool intel_dp_upfront_link_train(struct intel_dp *intel_dp)
> > > {
> > > struct intel_digital_port *intel_dig_port =
> > > dp_to_dig_port(intel_dp);
> > > struct intel_encoder *intel_encoder = &intel_dig_port-
> > > >base;
> > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> > > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > > index 54a9d76..98d45a4 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > > @@ -41,21 +41,30 @@ static bool
> > > intel_dp_mst_compute_config(struct
> > > intel_encoder *encoder,
> > > int bpp;
> > > int lane_count, slots;
> > > const struct drm_display_mode *adjusted_mode =
> > > &pipe_config-
> > > >
> > > > base.adjusted_mode;
> > > - int mst_pbn;
> > > + int mst_pbn, common_len;
> > > + int common_rates[DP_MAX_SUPPORTED_RATES] = {};
> > >
> > > pipe_config->dp_encoder_is_mst = true;
> > > pipe_config->has_pch_encoder = false;
> > > - bpp = 24;
> > > +
> > > /*
> > > - * for MST we always configure max link bw - the spec
> > > doesn't
> > > - * seem to suggest we should do otherwise.
> > > + * For MST we always configure for the maximum trainable
> > > link bw -
> > > + * the spec doesn't seem to suggest we should do
> > > otherwise. The
> > > + * calls to intel_dp_max_lane_count() and
> > > intel_dp_common_rates()
> > > + * both take successful upfront link training into
> > > account,
> > > and
> > > + * return the DisplayPort max supported values in the
> > > event
> > > that
> > > + * upfront link training was not done.
> > > */
> > > - lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> > > + lane_count = intel_dp_max_lane_count(intel_dp);
> > >
> > > pipe_config->lane_count = lane_count;
> > >
> > > - pipe_config->pipe_bpp = 24;
> > > - pipe_config->port_clock =
> > > intel_dp_max_link_rate(intel_dp);
> > > + pipe_config->pipe_bpp = bpp = 24;
> > > + common_len = intel_dp_common_rates(intel_dp,
> > > common_rates);
> > > + pipe_config->port_clock = common_rates[common_len - 1];
> > > +
> > > + DRM_DEBUG_KMS("DP MST link configured for %d lanes @
> > > %d.\n",
> > > + pipe_config->lane_count, pipe_config-
> > > >
> > > > port_clock);
> > >
> > > state = pipe_config->base.state;
> > >
> > > @@ -137,6 +146,8 @@ static void intel_mst_pre_enable_dp(struct
> > > intel_encoder *encoder,
> > > enum port port = intel_dig_port->port;
> > > struct intel_connector *connector =
> > > to_intel_connector(conn_state->connector);
> > > + struct intel_shared_dpll *pll = pipe_config-
> > > >shared_dpll;
> > > + struct intel_shared_dpll_config tmp_pll_config;
> > > int ret;
> > > uint32_t temp;
> > > int slots;
> > > @@ -150,21 +161,23 @@ static void intel_mst_pre_enable_dp(struct
> > > intel_encoder *encoder,
> > > DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
> > >
> > > if (intel_dp->active_mst_links == 0) {
> > > - intel_ddi_clk_select(&intel_dig_port->base,
> > > - pipe_config->shared_dpll);
> > > -
> > > - intel_prepare_dp_ddi_buffers(&intel_dig_port-
> > > >base);
> > > - intel_dp_set_link_params(intel_dp,
> > > - pipe_config-
> > > >port_clock,
> > > - pipe_config-
> > > >lane_count,
> > > - true);
> > > -
> > > - intel_ddi_init_dp_buf_reg(&intel_dig_port-
> > > >base);
> > >
> > > - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> > > + /* Disable the PLL since we need to acquire the
> > > PLL
> > > + * based on the link rate in the link training
> > > sequence
> > > + */
> > > + tmp_pll_config = pll->config;
> > > + pll->funcs.disable(dev_priv, pll);
> > > + pll->config.crtc_mask = 0;
> > > +
> > > + /* If Link Training fails, send a uevent to
> > > generate
> > > a
> > > + *hotplug
> > > + */
> > > + if (!(intel_ddi_link_train(intel_dp,
> > > pipe_config-
> > > >
> > > > port_clock,
> > > + pipe_config-
> > > >lane_count,
> > > true,
> > > + false)))
> > > + drm_kms_helper_hotplug_event(encoder-
> > > >
> > > > base.dev);
> > > + pll->config = tmp_pll_config;
> > >
> > > - intel_dp_start_link_train(intel_dp);
> > > - intel_dp_stop_link_train(intel_dp);
> > > }
> > >
> > > ret = drm_dp_mst_allocate_vcpi(&intel_dp->mst_mgr,
> > > @@ -336,6 +349,27 @@ intel_dp_mst_mode_valid(struct drm_connector
> > > *connector,
> > > struct drm_display_mode *mode)
> > > {
> > > int max_dotclk = to_i915(connector->dev)-
> > > >max_dotclk_freq;
> > > + struct intel_connector *intel_connector =
> > > to_intel_connector(connector);
> > > + struct intel_dp *intel_dp = intel_connector->mst_port;
> > > +
> > > + if (intel_dp->upfront_link_train && !intel_dp-
> > > >upfront_done)
> > > {
> > > + bool do_upfront_link_train;
> > > +
> > > + do_upfront_link_train = intel_dp-
> > > >
> > > > compliance_test_type !=
> > > + DP_TEST_LINK_TRAINING;
> > > + if (do_upfront_link_train) {
> > > + intel_dp->upfront_done =
> > > + intel_dp_upfront_link_train(inte
> > > l_dp
> > > );
> > > + if (intel_dp->upfront_done) {
> > > + DRM_DEBUG_KMS("MST upfront
> > > trained
> > > at "
> > > + "%d lanes @ %d.",
> > > + intel_dp-
> > > >
> > > > max_lanes_upfront,
> > > + intel_dp-
> > > >
> > > > max_link_rate_upfront);
> > > + } else
> > > + DRM_DEBUG_KMS("MST upfront link
> > > training "
> > > + "failed.");
> > Link training has failed and we have a blank screen. Should we
> > throw an
> > error here? Maybe
> >
> > return MODE_ERROR;
> This is only for upfront link training, which is helpful but not
> absolutely
> required. If this were a link training associated with a mode set
> I'd
> agree. As it is, I think a debug message is fine for this case.
>
> Jim
Ok, so at this point we are not associated with any mode so I think
debug message is just fine here.
>
>
> >
> >
> > >
> > > + }
> > > + }
> > >
> > > /* TODO - validate mode against available PBN for link
> > > */
> > > if (mode->clock < 10000)
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index a2bbf68..34af3e8 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1416,6 +1416,7 @@ void intel_edp_panel_off(struct intel_dp
> > > *intel_dp);
> > > void intel_dp_add_properties(struct intel_dp *intel_dp, struct
> > > drm_connector *connector);
> > > void intel_dp_mst_suspend(struct drm_device *dev);
> > > void intel_dp_mst_resume(struct drm_device *dev);
> > > +u8 intel_dp_max_lane_count(struct intel_dp *intel_dp);
> > > int intel_dp_max_link_rate(struct intel_dp *intel_dp);
> > > int intel_dp_link_rate_index(struct intel_dp *intel_dp, int
> > > *common_rates,
> > > int link_rate);
> > > @@ -1448,6 +1449,8 @@ intel_dp_pre_emphasis_max(struct intel_dp
> > > *intel_dp, uint8_t voltage_swing);
> > > void intel_dp_compute_rate(struct intel_dp *intel_dp, int
> > > port_clock,
> > > uint8_t *link_bw, uint8_t
> > > *rate_select);
> > > bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
> > > +int intel_dp_common_rates(struct intel_dp *intel_dp, int
> > > *common_rates);
> > > +bool intel_dp_upfront_link_train(struct intel_dp *intel_dp);
> > > bool
> > > intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t
> > > link_status[DP_LINK_STATUS_SIZE]);
> > >
--
Mika Kahola - Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-09-08 10:21 UTC|newest]
Thread overview: 81+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-01 22:08 [PATCH 00/14] Enable Upfront Link Training on DDI platforms Manasi Navare
2016-09-01 22:08 ` [PATCH v2 01/14] drm/i915: Don't pass crtc_state to intel_dp_set_link_params() Manasi Navare
2016-09-01 22:08 ` [PATCH v2 02/14] drm/i915: Remove ddi_pll_sel from intel_crtc_state Manasi Navare
2016-09-01 22:08 ` [PATCH v3 03/14] drm/i915: Split intel_ddi_pre_enable() into DP and HDMI versions Manasi Navare
2016-09-01 22:08 ` [PATCH v2 04/14] drm/i915: Split bxt_ddi_pll_select() Manasi Navare
2016-09-01 22:08 ` [PATCH 05/14] drm/i915: Split skl_get_dpll() Manasi Navare
2016-09-01 22:08 ` [PATCH 06/14] drm/i915: Split hsw_get_dpll() Manasi Navare
2016-09-01 22:08 ` [PATCH v3 07/14] drm/i915/dp: Add a standalone function to obtain shared dpll for HSW/BDW/SKL/BXT Manasi Navare
2016-09-02 20:06 ` Pandiyan, Dhinakaran
2016-09-07 22:08 ` Manasi Navare
2016-09-07 22:47 ` [PATCH v4 7/14] " Manasi Navare
2016-09-01 22:08 ` [PATCH 08/14] drm/i915/dp: Move max. vswing check to it's own function Manasi Navare
2016-09-02 8:05 ` Mika Kahola
2016-09-06 9:58 ` Mika Kahola
2016-09-06 21:25 ` Manasi Navare
2016-09-07 0:13 ` [PATCH v2 8/14] " Manasi Navare
2016-09-07 7:00 ` Mika Kahola
2016-09-07 18:28 ` [PATCH v3 " Manasi Navare
2016-09-08 7:38 ` Mika Kahola
2016-09-13 11:44 ` Jani Nikula
2016-09-01 22:08 ` [PATCH 09/14] drm/dp/i915: Make clock recovery in the link training compliant with DP Spec 1.2 Manasi Navare
2016-09-02 9:16 ` Mika Kahola
2016-09-02 17:55 ` Pandiyan, Dhinakaran
2016-09-07 0:13 ` [PATCH v2 9/14] " Manasi Navare
2016-09-07 7:33 ` Mika Kahola
2016-09-07 18:28 ` [PATCH v3 " Manasi Navare
2016-09-08 8:20 ` Mika Kahola
2016-09-01 22:08 ` [PATCH 10/14] drm/i915: Make DP link training channel equalization DP 1.2 Spec compliant Manasi Navare
2016-09-02 11:20 ` Mika Kahola
2016-09-02 19:05 ` Pandiyan, Dhinakaran
2016-09-07 7:50 ` Mika Kahola
2016-09-13 16:09 ` Rodrigo Vivi
2016-09-01 22:08 ` [PATCH 11/14] drm/i915: Fallback to lower link rate and lane count during link training Manasi Navare
2016-09-02 12:03 ` David Weinehall
2016-09-06 17:34 ` Manasi Navare
2016-09-02 12:49 ` David Weinehall
2016-09-06 17:54 ` Manasi Navare
2016-09-02 13:00 ` Mika Kahola
2016-09-06 18:01 ` Manasi Navare
2016-09-02 19:52 ` Pandiyan, Dhinakaran
2016-09-02 20:01 ` Jim Bride
2016-09-07 0:13 ` [PATCH v2 " Manasi Navare
2016-09-07 9:47 ` Mika Kahola
2016-09-07 16:47 ` Jim Bride
2016-09-07 16:48 ` Manasi Navare
2016-09-07 18:28 ` [PATCH v3 " Manasi Navare
2016-09-08 0:30 ` [PATCH v4 " Manasi Navare
2016-09-08 9:32 ` Mika Kahola
2016-09-09 1:05 ` Rodrigo Vivi
2016-09-09 7:11 ` Jani Nikula
2016-09-09 7:11 ` Jani Nikula
2016-09-09 17:13 ` Manasi Navare
2016-09-09 23:29 ` [PATCH v5 " Manasi Navare
2016-09-01 22:08 ` [PATCH 12/14] drm/i915: Reverse the loop in intel_dp_compute_config Manasi Navare
2016-09-02 13:08 ` Mika Kahola
2016-09-08 14:47 ` Manasi Navare
2016-09-02 20:24 ` Pandiyan, Dhinakaran
2016-09-08 20:02 ` [PATCH v2 12/14] drm/i915: Remove the link rate and lane count loop in compute config Manasi Navare
2016-09-13 1:14 ` Pandiyan, Dhinakaran
2016-09-14 1:05 ` Manasi Navare
2016-09-01 22:08 ` [PATCH v11 13/14] drm/i915/dp: Enable Upfront link training for typeC DP support on HSW/BDW/SKL/BXT (DDI platforms) Manasi Navare
2016-09-07 0:13 ` [PATCH v12 " Manasi Navare
2016-09-07 18:28 ` [PATCH v13 " Manasi Navare
2016-09-08 12:10 ` Mika Kahola
2016-09-08 15:06 ` Manasi Navare
2016-09-08 17:22 ` [PATCH v14 " Manasi Navare
2016-09-08 20:02 ` [PATCH v15 " Manasi Navare
2016-09-09 7:34 ` Jani Nikula
2016-09-09 23:29 ` [PATCH 13-1/14] drm/i915: Change the placement of some static functions in intel_dp.c Manasi Navare
2016-09-12 23:21 ` Rodrigo Vivi
2016-09-09 23:29 ` [PATCH v16 13-2/14] drm/i915/dp: Enable Upfront link training on HSW/BDW/SKL/BXT Manasi Navare
2016-09-13 0:22 ` Rodrigo Vivi
2016-09-09 7:31 ` [PATCH v14 13/14] drm/i915/dp: Enable Upfront link training for typeC DP support on HSW/BDW/SKL/BXT (DDI platforms) Jani Nikula
2016-09-01 22:08 ` [PATCH 14/14] drm/i915/dp/mst: Add support for upfront link training for DP MST Manasi Navare
2016-09-07 0:13 ` [PATCH v2 " Manasi Navare
2016-09-07 10:53 ` Mika Kahola
2016-09-07 16:40 ` Jim Bride
2016-09-08 10:21 ` Mika Kahola [this message]
2016-09-08 11:50 ` Mika Kahola
2016-09-01 22:48 ` ✗ Fi.CI.BAT: failure for Enable upfront link training on DDI platforms (rev3) Patchwork
2016-09-07 0:54 ` ✗ Fi.CI.BAT: warning for Enable upfront link training on DDI platforms (rev8) 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=1473330082.28727.32.camel@intel.com \
--to=mika.kahola@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jim.bride@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.