Re: [PATCH v2 5/7] drm/i915: Move intel_dp->lane_count into pipe_config

Sivakumar Thulasimani
2015-08-13T07:00:27Z

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