From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sivakumar Thulasimani Subject: Re: [PATCH 7/7] drm/i915: Kill intel_dp->{link_bw, rate_select} Date: Tue, 07 Jul 2015 14:16:15 +0530 Message-ID: <559B91D7.4010904@intel.com> References: <1436184606-18729-1-git-send-email-ville.syrjala@linux.intel.com> <1436184606-18729-8-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1390274349==" Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTP id 661C96E99B for ; Tue, 7 Jul 2015 01:47:37 -0700 (PDT) In-Reply-To: <1436184606-18729-8-git-send-email-ville.syrjala@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org This is a multi-part message in MIME format. --===============1390274349== Content-Type: multipart/alternative; boundary="------------000209050808070403020904" This is a multi-part message in MIME format. --------------000209050808070403020904 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Reviewed-by: Sivakumar Thulasimani On 7/6/2015 5:40 PM, ville.syrjala@linux.intel.com wrote: > From: Ville Syrj=C3=A4l=C3=A4 > > 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=C3=A4l=C3=A4 > --- > 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/int= el_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); > } > =20 > +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 =3D 0; > + *rate_select =3D > + intel_dp_rate_select(intel_dp, port_clock); > + } else { > + *link_bw =3D drm_dp_link_rate_to_bw_code(port_clock); > + *rate_select =3D 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 *enc= oder, > int link_avail, link_clock; > int common_rates[DP_MAX_SUPPORTED_RATES] =3D {}; > int common_len; > + uint8_t link_bw, rate_select; > =20 > common_len =3D intel_dp_common_rates(intel_dp, common_rates); > =20 > @@ -1464,21 +1478,14 @@ found: > =20 > pipe_config->lane_count =3D lane_count; > =20 > - if (intel_dp->num_sink_rates) { > - intel_dp->link_bw =3D 0; > - intel_dp->rate_select =3D > - intel_dp_rate_select(intel_dp, common_rates[clock]); > - } else { > - intel_dp->link_bw =3D > - drm_dp_link_rate_to_bw_code(common_rates[clock]); > - intel_dp->rate_select =3D 0; > - } > - > pipe_config->pipe_bpp =3D bpp; > pipe_config->port_clock =3D common_rates[clock]; > =20 > - 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 *inte= l_dp) > int voltage_tries, loop_tries; > uint32_t DP =3D intel_dp->DP; > uint8_t link_config[2]; > + uint8_t link_bw, rate_select; > =20 > if (HAS_DDI(dev)) > intel_ddi_prepare_link_retrain(encoder); > =20 > + intel_dp_compute_rate(intel_dp, crtc->config->port_clock, > + &link_bw, &rate_select); > + > /* Write the link configuration data */ > - link_config[0] =3D intel_dp->link_bw; > + link_config[0] =3D link_bw; > link_config[1] =3D crtc->config->lane_count; > if (drm_dp_enhanced_frame_cap(intel_dp->dpcd)) > link_config[1] |=3D 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); > =20 > link_config[0] =3D 0; > link_config[1] =3D 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 =3D &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 =3D &pipe_config->base.adjust= ed_mode; > struct drm_connector *drm_connector; > struct intel_connector *connector, *found =3D NULL; > @@ -55,20 +55,11 @@ static bool intel_dp_mst_compute_config(struct inte= l_encoder *encoder, > */ > lane_count =3D drm_dp_max_lane_count(intel_dp->dpcd); > =20 > - rate =3D intel_dp_max_link_rate(intel_dp); > - > - if (intel_dp->num_sink_rates) { > - intel_dp->link_bw =3D 0; > - intel_dp->rate_select =3D intel_dp_rate_select(intel_dp, rate); > - } else { > - intel_dp->link_bw =3D drm_dp_link_rate_to_bw_code(rate); > - intel_dp->rate_select =3D 0; > - } > =20 > pipe_config->lane_count =3D lane_count; > =20 > pipe_config->pipe_bpp =3D 24; > - pipe_config->port_clock =3D rate; > + pipe_config->port_clock =3D intel_dp_max_link_rate(intel_dp); > =20 > state =3D pipe_config->base.state; > =20 > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/in= tel_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]; --=20 regards, Sivakumar --------------000209050808070403020904 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable

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

On 7/6/2015 5:40 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrj=C3=A4l=C3=A4 <ville.syrj=
ala@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=C3=A4l=C3=A4 <ville.syrjala@linux.in=
tel.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);
 }
=20
+static void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_cl=
ock,
+				  uint8_t *link_bw, uint8_t *rate_select)
+{
+	if (intel_dp->num_sink_rates) {
+		*link_bw =3D 0;
+		*rate_select =3D
+			intel_dp_rate_select(intel_dp, port_clock);
+	} else {
+		*link_bw =3D drm_dp_link_rate_to_bw_code(port_clock);
+		*rate_select =3D 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 *encod=
er,
 	int link_avail, link_clock;
 	int common_rates[DP_MAX_SUPPORTED_RATES] =3D {};
 	int common_len;
+	uint8_t link_bw, rate_select;
=20
 	common_len =3D intel_dp_common_rates(intel_dp, common_rates);
=20
@@ -1464,21 +1478,14 @@ found:
=20
 	pipe_config->lane_count =3D lane_count;
=20
-	if (intel_dp->num_sink_rates) {
-		intel_dp->link_bw =3D 0;
-		intel_dp->rate_select =3D
-			intel_dp_rate_select(intel_dp, common_rates[clock]);
-	} else {
-		intel_dp->link_bw =3D
-			drm_dp_link_rate_to_bw_code(common_rates[clock]);
-		intel_dp->rate_select =3D 0;
-	}
-
 	pipe_config->pipe_bpp =3D bpp;
 	pipe_config->port_clock =3D common_rates[clock];
=20
-	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 =3D intel_dp->DP;
 	uint8_t link_config[2];
+	uint8_t link_bw, rate_select;
=20
 	if (HAS_DDI(dev))
 		intel_ddi_prepare_link_retrain(encoder);
=20
+	intel_dp_compute_rate(intel_dp, crtc->config->port_clock,
+			      &link_bw, &rate_select);
+
 	/* Write the link configuration data */
-	link_config[0] =3D intel_dp->link_bw;
+	link_config[0] =3D link_bw;
 	link_config[1] =3D crtc->config->lane_count;
 	if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
 		link_config[1] |=3D 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);
=20
 	link_config[0] =3D 0;
 	link_config[1] =3D DP_SET_ANSI_8B10B;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/i=
ntel_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_en=
coder *encoder,
 	struct intel_dp *intel_dp =3D &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 =3D &pipe_config->base.ad=
justed_mode;
 	struct drm_connector *drm_connector;
 	struct intel_connector *connector, *found =3D NULL;
@@ -55,20 +55,11 @@ static bool intel_dp_mst_compute_config(struct intel_=
encoder *encoder,
 	 */
 	lane_count =3D drm_dp_max_lane_count(intel_dp->dpcd);
=20
-	rate =3D intel_dp_max_link_rate(intel_dp);
-
-	if (intel_dp->num_sink_rates) {
-		intel_dp->link_bw =3D 0;
-		intel_dp->rate_select =3D intel_dp_rate_select(intel_dp, rate);
-	} else {
-		intel_dp->link_bw =3D drm_dp_link_rate_to_bw_code(rate);
-		intel_dp->rate_select =3D 0;
-	}
=20
 	pipe_config->lane_count =3D lane_count;
=20
 	pipe_config->pipe_bpp =3D 24;
-	pipe_config->port_clock =3D rate;
+	pipe_config->port_clock =3D intel_dp_max_link_rate(intel_dp);
=20
 	state =3D pipe_config->base.state;
=20
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/inte=
l_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];


--=20
regards,
Sivakumar
--------------000209050808070403020904-- --===============1390274349== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============1390274349==--