Re: [PATCH 7/7] drm/i915: Kill intel_dp->{link_bw, rate_select}

Sivakumar Thulasimani
2015-07-07T08:47:37Z

[-- Attachment #1.1: Type: text/plain, Size: 5936 bytes --]

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

On 7/6/2015 5:40 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We only need the link_bw/rate_select parameters when starting link
> training, and they should be computed based on the currently active
> config, so throw them out from intel_dp and just compute on demand.
>
> Toss in an extra debug print to see rate_select in addition to link_bw,
> as the latter may be 0 for eDP 1.4.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 39 ++++++++++++++++++++++++-------------
> drivers/gpu/drm/i915/intel_dp_mst.c | 13 ++-----------
> drivers/gpu/drm/i915/intel_drv.h | 2 --
> 3 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 46b734b..e88cec2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1342,6 +1342,19 @@ int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
> return rate_to_index(rate, intel_dp->sink_rates);
> }
>
> +static void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
> + uint8_t *link_bw, uint8_t *rate_select)
> +{
> + if (intel_dp->num_sink_rates) {
> + *link_bw = 0;
> + *rate_select =
> + intel_dp_rate_select(intel_dp, port_clock);
> + } else {
> + *link_bw = drm_dp_link_rate_to_bw_code(port_clock);
> + *rate_select = 0;
> + }
> +}
> +
> bool
> intel_dp_compute_config(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config)
> @@ -1363,6 +1376,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> int link_avail, link_clock;
> int common_rates[DP_MAX_SUPPORTED_RATES] = {};
> int common_len;
> + uint8_t link_bw, rate_select;
>
> common_len = intel_dp_common_rates(intel_dp, common_rates);
>
> @@ -1464,21 +1478,14 @@ found:
>
> pipe_config->lane_count = lane_count;
>
> - if (intel_dp->num_sink_rates) {
> - intel_dp->link_bw = 0;
> - intel_dp->rate_select =
> - intel_dp_rate_select(intel_dp, common_rates[clock]);
> - } else {
> - intel_dp->link_bw =
> - drm_dp_link_rate_to_bw_code(common_rates[clock]);
> - intel_dp->rate_select = 0;
> - }
> -
> pipe_config->pipe_bpp = bpp;
> 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, pipe_config->lane_count,
> + intel_dp_compute_rate(intel_dp, pipe_config->port_clock,
> + &link_bw, &rate_select);
> +
> + DRM_DEBUG_KMS("DP link bw %02x rate select %02x lane count %d clock %d bpp %d\n",
> + link_bw, rate_select, pipe_config->lane_count,
> pipe_config->port_clock, bpp);
> DRM_DEBUG_KMS("DP link bw required %i available %i\n",
> mode_rate, link_avail);
> @@ -3587,19 +3594,23 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
> int voltage_tries, loop_tries;
> uint32_t DP = intel_dp->DP;
> uint8_t link_config[2];
> + uint8_t link_bw, rate_select;
>
> if (HAS_DDI(dev))
> intel_ddi_prepare_link_retrain(encoder);
>
> + intel_dp_compute_rate(intel_dp, crtc->config->port_clock,
> + &link_bw, &rate_select);
> +
> /* Write the link configuration data */
> - link_config[0] = intel_dp->link_bw;
> + link_config[0] = link_bw;
> 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);
> if (intel_dp->num_sink_rates)
> drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_RATE_SET,
> - &intel_dp->rate_select, 1);
> + &rate_select, 1);
>
> link_config[0] = 0;
> link_config[1] = DP_SET_ANSI_8B10B;
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index eeda730..3dc08da 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -38,7 +38,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
> struct intel_dp *intel_dp = &intel_dig_port->dp;
> struct drm_atomic_state *state;
> int bpp, i;
> - int lane_count, slots, rate;
> + int lane_count, slots;
> struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> struct drm_connector *drm_connector;
> struct intel_connector *connector, *found = NULL;
> @@ -55,20 +55,11 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
> */
> lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
>
> - rate = intel_dp_max_link_rate(intel_dp);
> -
> - if (intel_dp->num_sink_rates) {
> - intel_dp->link_bw = 0;
> - intel_dp->rate_select = intel_dp_rate_select(intel_dp, rate);
> - } else {
> - intel_dp->link_bw = drm_dp_link_rate_to_bw_code(rate);
> - intel_dp->rate_select = 0;
> - }
>
> pipe_config->lane_count = lane_count;
>
> pipe_config->pipe_bpp = 24;
> - pipe_config->port_clock = rate;
> + pipe_config->port_clock = intel_dp_max_link_rate(intel_dp);
>
> state = pipe_config->base.state;
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 703b394..709de50 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -718,8 +718,6 @@ struct intel_dp {
> enum hdmi_force_audio force_audio;
> bool limited_color_range;
> bool color_range_auto;
> - uint8_t link_bw;
> - uint8_t rate_select;
> 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

[-- Attachment #1.2: Type: text/html, Size: 6786 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]