From: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2 5/7] drm/i915: Move intel_dp->lane_count into pipe_config
Date: Thu, 13 Aug 2015 12:30:53 +0530 [thread overview]
Message-ID: <55CC40A5.7060503@intel.com> (raw)
In-Reply-To: <1436189955-22210-1-git-send-email-ville.syrjala@linux.intel.com>
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
On Monday 06 July 2015 07:09 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently we clobber intel_dp->lane_count in compute config, which means
> after a rejected modeset we may no longer be able to retrain the current
> link. Move lane_count into pipe_config to avoid that.
>
> v2: Add missing ':' to the pipe config debug dump
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 3 ++
> drivers/gpu/drm/i915/intel_ddi.c | 10 +++---
> drivers/gpu/drm/i915/intel_display.c | 7 +++--
> drivers/gpu/drm/i915/intel_dp.c | 60 ++++++++++++++++++++++++------------
> drivers/gpu/drm/i915/intel_dp_mst.c | 6 +++-
> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> 6 files changed, 61 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d5c794a..f5dc75c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4096,6 +4096,7 @@ enum skl_disp_power_wells {
> /* How many wires to use. I guess 3 was too hard */
> #define DP_PORT_WIDTH(width) (((width) - 1) << 19)
> #define DP_PORT_WIDTH_MASK (7 << 19)
> +#define DP_PORT_WIDTH_SHIFT 19
>
> /* Mystic DPCD version 1.1 special mode */
> #define DP_ENHANCED_FRAMING (1 << 18)
> @@ -7128,6 +7129,8 @@ enum skl_disp_power_wells {
> #define DDI_BUF_IS_IDLE (1<<7)
> #define DDI_A_4_LANES (1<<4)
> #define DDI_PORT_WIDTH(width) (((width) - 1) << 1)
> +#define DDI_PORT_WIDTH_MASK (7 << 1)
> +#define DDI_PORT_WIDTH_SHIFT 1
> #define DDI_INIT_DISPLAY_DETECTED (1<<0)
>
> /* DDI Buffer Translations */
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 96fb472..8df596d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -723,11 +723,11 @@ void intel_ddi_init_dp_buf_reg(struct intel_encoder *encoder)
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> struct intel_digital_port *intel_dig_port =
> enc_to_dig_port(&encoder->base);
> + struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
>
> intel_dp->DP = intel_dig_port->saved_port_bits |
> DDI_BUF_CTL_ENABLE | DDI_BUF_TRANS_SELECT(0);
> - intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
> -
> + intel_dp->DP |= DDI_PORT_WIDTH(crtc->config->lane_count);
> }
>
> static struct intel_encoder *
> @@ -1913,7 +1913,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
> } else
> temp |= TRANS_DDI_MODE_SELECT_DP_SST;
>
> - temp |= DDI_PORT_WIDTH(intel_dp->lane_count);
> + temp |= DDI_PORT_WIDTH(intel_crtc->config->lane_count);
> } else if (type == INTEL_OUTPUT_DP_MST) {
> struct intel_dp *intel_dp = &enc_to_mst(encoder)->primary->dp;
>
> @@ -1922,7 +1922,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
> } else
> temp |= TRANS_DDI_MODE_SELECT_DP_SST;
>
> - temp |= DDI_PORT_WIDTH(intel_dp->lane_count);
> + temp |= DDI_PORT_WIDTH(intel_crtc->config->lane_count);
> } else {
> WARN(1, "Invalid encoder type %d for pipe %c\n",
> intel_encoder->type, pipe_name(pipe));
> @@ -3072,6 +3072,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> case TRANS_DDI_MODE_SELECT_DP_SST:
> case TRANS_DDI_MODE_SELECT_DP_MST:
> pipe_config->has_dp_encoder = true;
> + pipe_config->lane_count =
> + ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> intel_dp_get_m_n(intel_crtc, pipe_config);
> break;
> default:
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 136b533..d56973f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11981,14 +11981,16 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
> pipe_config->fdi_m_n.gmch_m, pipe_config->fdi_m_n.gmch_n,
> pipe_config->fdi_m_n.link_m, pipe_config->fdi_m_n.link_n,
> pipe_config->fdi_m_n.tu);
> - DRM_DEBUG_KMS("dp: %i, gmch_m: %u, gmch_n: %u, link_m: %u, link_n: %u, tu: %u\n",
> + DRM_DEBUG_KMS("dp: %i, lanes: %i, gmch_m: %u, gmch_n: %u, link_m: %u, link_n: %u, tu: %u\n",
> pipe_config->has_dp_encoder,
> + pipe_config->lane_count,
> pipe_config->dp_m_n.gmch_m, pipe_config->dp_m_n.gmch_n,
> pipe_config->dp_m_n.link_m, pipe_config->dp_m_n.link_n,
> pipe_config->dp_m_n.tu);
>
> - DRM_DEBUG_KMS("dp: %i, gmch_m2: %u, gmch_n2: %u, link_m2: %u, link_n2: %u, tu2: %u\n",
> + DRM_DEBUG_KMS("dp: %i, lanes: %i, gmch_m2: %u, gmch_n2: %u, link_m2: %u, link_n2: %u, tu2: %u\n",
> pipe_config->has_dp_encoder,
> + pipe_config->lane_count,
> pipe_config->dp_m2_n2.gmch_m,
> pipe_config->dp_m2_n2.gmch_n,
> pipe_config->dp_m2_n2.link_m,
> @@ -12456,6 +12458,7 @@ intel_pipe_config_compare(struct drm_device *dev,
> PIPE_CONF_CHECK_I(fdi_m_n.tu);
>
> PIPE_CONF_CHECK_I(has_dp_encoder);
> + PIPE_CONF_CHECK_I(lane_count);
>
> if (INTEL_INFO(dev)->gen < 8) {
> PIPE_CONF_CHECK_I(dp_m_n.gmch_m);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b6ba02e..da036e8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1462,7 +1462,7 @@ found:
> intel_dp->limited_color_range;
> }
>
> - intel_dp->lane_count = lane_count;
> + pipe_config->lane_count = lane_count;
>
> if (intel_dp->num_sink_rates) {
> intel_dp->link_bw = 0;
> @@ -1478,7 +1478,7 @@ found:
> pipe_config->port_clock = common_rates[clock];
>
> DRM_DEBUG_KMS("DP link bw %02x lane count %d clock %d bpp %d\n",
> - intel_dp->link_bw, intel_dp->lane_count,
> + intel_dp->link_bw, pipe_config->lane_count,
> pipe_config->port_clock, bpp);
> DRM_DEBUG_KMS("DP link bw required %i available %i\n",
> mode_rate, link_avail);
> @@ -1573,7 +1573,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder)
>
> /* Handle DP bits in common between all three register formats */
> intel_dp->DP |= DP_VOLTAGE_0_4 | DP_PRE_EMPHASIS_0;
> - intel_dp->DP |= DP_PORT_WIDTH(intel_dp->lane_count);
> + intel_dp->DP |= DP_PORT_WIDTH(crtc->config->lane_count);
>
> if (crtc->config->has_audio)
> intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE;
> @@ -2276,6 +2276,9 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>
> pipe_config->has_dp_encoder = true;
>
> + pipe_config->lane_count =
> + ((tmp & DP_PORT_WIDTH_MASK) >> DP_PORT_WIDTH_SHIFT) + 1;
> +
> intel_dp_get_m_n(crtc, pipe_config);
>
> if (port == PORT_A) {
> @@ -3301,13 +3304,15 @@ static void
> intel_get_adjust_train(struct intel_dp *intel_dp,
> const uint8_t link_status[DP_LINK_STATUS_SIZE])
> {
> + struct intel_crtc *crtc =
> + to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
> uint8_t v = 0;
> uint8_t p = 0;
> int lane;
> uint8_t voltage_max;
> uint8_t preemph_max;
>
> - for (lane = 0; lane < intel_dp->lane_count; lane++) {
> + for (lane = 0; lane < crtc->config->lane_count; lane++) {
> uint8_t this_v = drm_dp_get_adjust_request_voltage(link_status, lane);
> uint8_t this_p = drm_dp_get_adjust_request_pre_emphasis(link_status, lane);
>
> @@ -3476,8 +3481,10 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
> uint8_t dp_train_pat)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> - struct drm_device *dev = intel_dig_port->base.base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_private *dev_priv =
> + to_i915(intel_dig_port->base.base.dev);
> + struct intel_crtc *crtc =
> + to_intel_crtc(intel_dig_port->base.base.crtc);
> uint8_t buf[sizeof(intel_dp->train_set) + 1];
> int ret, len;
>
> @@ -3493,8 +3500,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
> len = 1;
> } else {
> /* DP_TRAINING_LANEx_SET follow DP_TRAINING_PATTERN_SET */
> - memcpy(buf + 1, intel_dp->train_set, intel_dp->lane_count);
> - len = intel_dp->lane_count + 1;
> + memcpy(buf + 1, intel_dp->train_set, crtc->config->lane_count);
> + len = crtc->config->lane_count + 1;
> }
>
> ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
> @@ -3518,8 +3525,10 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
> const uint8_t link_status[DP_LINK_STATUS_SIZE])
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> - struct drm_device *dev = intel_dig_port->base.base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_private *dev_priv =
> + to_i915(intel_dig_port->base.base.dev);
> + struct intel_crtc *crtc =
> + to_intel_crtc(intel_dig_port->base.base.crtc);
> int ret;
>
> intel_get_adjust_train(intel_dp, link_status);
> @@ -3529,9 +3538,9 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
> POSTING_READ(intel_dp->output_reg);
>
> ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET,
> - intel_dp->train_set, intel_dp->lane_count);
> + intel_dp->train_set, crtc->config->lane_count);
>
> - return ret == intel_dp->lane_count;
> + return ret == crtc->config->lane_count;
> }
>
> static void intel_dp_set_idle_link_train(struct intel_dp *intel_dp)
> @@ -3570,6 +3579,8 @@ void
> intel_dp_start_link_train(struct intel_dp *intel_dp)
> {
> struct drm_encoder *encoder = &dp_to_dig_port(intel_dp)->base.base;
> + struct intel_crtc *crtc =
> + to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
> struct drm_device *dev = encoder->dev;
> int i;
> uint8_t voltage;
> @@ -3582,7 +3593,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
>
> /* Write the link configuration data */
> link_config[0] = intel_dp->link_bw;
> - link_config[1] = intel_dp->lane_count;
> + link_config[1] = crtc->config->lane_count;
> if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
> link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
> @@ -3616,7 +3627,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
> break;
> }
>
> - if (drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
> + if (drm_dp_clock_recovery_ok(link_status, crtc->config->lane_count)) {
> DRM_DEBUG_KMS("clock recovery OK\n");
> break;
> }
> @@ -3639,10 +3650,10 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
> }
>
> /* Check to see if we've tried the max voltage */
> - for (i = 0; i < intel_dp->lane_count; i++)
> + for (i = 0; i < crtc->config->lane_count; i++)
> if ((intel_dp->train_set[i] & DP_TRAIN_MAX_SWING_REACHED) == 0)
> break;
> - if (i == intel_dp->lane_count) {
> + if (i == crtc->config->lane_count) {
> ++loop_tries;
> if (loop_tries == 5) {
> DRM_ERROR("too many full retries, give up\n");
> @@ -3679,6 +3690,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
> void
> intel_dp_complete_link_train(struct intel_dp *intel_dp)
> {
> + struct intel_crtc *crtc =
> + to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
> bool channel_eq = false;
> int tries, cr_tries;
> uint32_t DP = intel_dp->DP;
> @@ -3714,7 +3727,8 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
> }
>
> /* Make sure clock is still ok */
> - if (!drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
> + if (!drm_dp_clock_recovery_ok(link_status,
> + crtc->config->lane_count)) {
> intel_dp->train_set_valid = false;
> intel_dp_start_link_train(intel_dp);
> intel_dp_set_link_train(intel_dp, &DP,
> @@ -3724,7 +3738,8 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
> continue;
> }
>
> - if (drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
> + if (drm_dp_channel_eq_ok(link_status,
> + crtc->config->lane_count)) {
> channel_eq = true;
> break;
> }
> @@ -4168,6 +4183,8 @@ update_status:
> static int
> intel_dp_check_mst_status(struct intel_dp *intel_dp)
> {
> + struct intel_crtc *crtc =
> + to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
> bool bret;
>
> if (intel_dp->is_mst) {
> @@ -4180,7 +4197,8 @@ go_again:
> if (bret == true) {
>
> /* check link status - esi[10] = 0x200c */
> - if (intel_dp->active_mst_links && !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
> + if (intel_dp->active_mst_links &&
> + !drm_dp_channel_eq_ok(&esi[10], crtc->config->lane_count)) {
> DRM_DEBUG_KMS("channel EQ not ok, retraining\n");
> intel_dp_start_link_train(intel_dp);
> intel_dp_complete_link_train(intel_dp);
> @@ -4235,6 +4253,8 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
> + struct intel_crtc *crtc =
> + to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
> u8 sink_irq_vector;
> u8 link_status[DP_LINK_STATUS_SIZE];
>
> @@ -4273,7 +4293,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
> }
>
> - if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
> + if (!drm_dp_channel_eq_ok(link_status, crtc->config->lane_count)) {
> DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> intel_encoder->base.name);
> intel_dp_start_link_train(intel_dp);
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 6e4cc53..eeda730 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -65,7 +65,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
> intel_dp->rate_select = 0;
> }
>
> - intel_dp->lane_count = lane_count;
> + pipe_config->lane_count = lane_count;
>
> pipe_config->pipe_bpp = 24;
> pipe_config->port_clock = rate;
> @@ -276,6 +276,10 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
> break;
> }
> pipe_config->base.adjusted_mode.flags |= flags;
> +
> + pipe_config->lane_count =
> + ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> +
> intel_dp_get_m_n(crtc, pipe_config);
>
> intel_ddi_clock_get(&intel_dig_port->base, pipe_config);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 983a7a7..703b394 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -430,6 +430,8 @@ struct intel_crtc_state {
> /* Used by SDVO (and if we ever fix it, HDMI). */
> unsigned pixel_multiplier;
>
> + uint8_t lane_count;
> +
> /* Panel fitter controls for gen2-gen4 + VLV */
> struct {
> u32 control;
> @@ -718,7 +720,6 @@ struct intel_dp {
> bool color_range_auto;
> uint8_t link_bw;
> uint8_t rate_select;
> - uint8_t lane_count;
> uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
> uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
> uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
--
regards,
Sivakumar Thulasimani
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-08-13 7:00 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-06 12:09 [PATCH 0/7] drm/i915: Move DP link parameters out from intel_dp ville.syrjala
2015-07-06 12:10 ` [PATCH 1/7] drm/i915: Clean up DP/HDMI limited color range handling ville.syrjala
2015-08-13 6:16 ` Sivakumar Thulasimani
2015-08-13 12:01 ` Ville Syrjälä
2015-07-06 12:10 ` [PATCH 2/7] drm/i915: Don't use link_bw for PLL setup ville.syrjala
2015-07-07 8:05 ` Sivakumar Thulasimani
2015-08-11 17:21 ` [PATCH v2 " ville.syrjala
2015-07-06 12:10 ` [PATCH 3/7] drm/i915: Don't pass clock to DDI PLL select functions ville.syrjala
2015-07-07 8:07 ` Sivakumar Thulasimani
2015-07-06 12:10 ` [PATCH 4/7] drm/i915: Avoid confusion between DP and TRANS_DP_CTL in DP .get_config() ville.syrjala
2015-07-07 8:14 ` Sivakumar Thulasimani
2015-07-06 12:10 ` [PATCH 5/7] drm/i915: Move intel_dp->lane_count into pipe_config ville.syrjala
2015-07-06 13:39 ` [PATCH v2 " ville.syrjala
2015-08-13 7:00 ` Sivakumar Thulasimani [this message]
2015-07-06 12:10 ` [PATCH 6/7] drm/i915: Don't use link_bw to select between TP1 and TP3 ville.syrjala
2015-07-07 8:18 ` Sivakumar Thulasimani
2015-07-06 12:10 ` [PATCH 7/7] drm/i915: Kill intel_dp->{link_bw, rate_select} ville.syrjala
2015-07-07 8:46 ` Sivakumar Thulasimani
2015-07-10 23:13 ` shuang.he
2015-08-12 16:04 ` [PATCH 0/7] drm/i915: Move DP link parameters out from intel_dp Ville Syrjälä
2015-08-14 8:23 ` Daniel Vetter
2015-08-17 8:42 ` Maarten Lankhorst
2015-08-17 12:03 ` Ville Syrjälä
2015-08-25 12:57 ` Daniel Vetter
2015-08-25 14:11 ` Ville Syrjälä
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=55CC40A5.7060503@intel.com \
--to=sivakumar.thulasimani@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.