From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sivakumar Thulasimani Subject: Re: [PATCH 2/7] drm/i915: Don't use link_bw for PLL setup Date: Tue, 07 Jul 2015 13:35:36 +0530 Message-ID: <559B8850.1000001@intel.com> References: <1436184606-18729-1-git-send-email-ville.syrjala@linux.intel.com> <1436184606-18729-3-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1785163014==" Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 45CE16E4D5 for ; Tue, 7 Jul 2015 01:06:58 -0700 (PDT) In-Reply-To: <1436184606-18729-3-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. --===============1785163014== Content-Type: multipart/alternative; boundary="------------030103060109020203010104" This is a multi-part message in MIME format. --------------030103060109020203010104 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 > > Use port_clock instead of link_bw when picking the PLL parameters for > DP. link_bw may be zero with an eDP 1.4 sink that supports > DP_LINK_RATE_SET so we shouln't use it for anything other than feed it shouldn't ? > to the sink appropriately. > > Signed-off-by: Ville Syrj=C3=A4l=C3=A4 > --- > drivers/gpu/drm/i915/intel_ddi.c | 11 ++++------ > drivers/gpu/drm/i915/intel_dp.c | 44 ++++++++++++++++++++-----------= --------- > 2 files changed, 26 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/in= tel_ddi.c > index 9a40bfb..0d3cc98 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1574,17 +1574,14 @@ skl_ddi_pll_select(struct intel_crtc *intel_crt= c, > DPLL_CFGCR2_PDIV(wrpll_params.pdiv) | > wrpll_params.central_freq; > } else if (intel_encoder->type =3D=3D INTEL_OUTPUT_DISPLAYPORT) { > - struct drm_encoder *encoder =3D &intel_encoder->base; > - struct intel_dp *intel_dp =3D enc_to_intel_dp(encoder); > - > - switch (intel_dp->link_bw) { > - case DP_LINK_BW_1_62: > + switch (crtc_state->port_clock / 2) { > + case 81000: > ctrl1 |=3D DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0); > break; > - case DP_LINK_BW_2_7: > + case 135000: > ctrl1 |=3D DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350, 0); > break; > - case DP_LINK_BW_5_4: > + case 270000: > ctrl1 |=3D DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, 0); > break; > } > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/int= el_dp.c > index decefa1..3dd4342 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -48,28 +48,28 @@ > #define INTEL_DP_RESOLUTION_FAILSAFE (3 << INTEL_DP_RESOLUTION_SHIFT_= MASK) > =20 > struct dp_link_dpll { > - int link_bw; > + int clock; > struct dpll dpll; > }; > =20 > static const struct dp_link_dpll gen4_dpll[] =3D { > - { DP_LINK_BW_1_62, > + { 162000, > { .p1 =3D 2, .p2 =3D 10, .n =3D 2, .m1 =3D 23, .m2 =3D 8 } }, > - { DP_LINK_BW_2_7, > + { 270000, > { .p1 =3D 1, .p2 =3D 10, .n =3D 1, .m1 =3D 14, .m2 =3D 2 } } > }; > =20 > static const struct dp_link_dpll pch_dpll[] =3D { > - { DP_LINK_BW_1_62, > + { 162000, > { .p1 =3D 2, .p2 =3D 10, .n =3D 1, .m1 =3D 12, .m2 =3D 9 } }, > - { DP_LINK_BW_2_7, > + { 270000, > { .p1 =3D 1, .p2 =3D 10, .n =3D 2, .m1 =3D 14, .m2 =3D 8 } } > }; > =20 > static const struct dp_link_dpll vlv_dpll[] =3D { > - { DP_LINK_BW_1_62, > + { 162000, > { .p1 =3D 3, .p2 =3D 2, .n =3D 5, .m1 =3D 3, .m2 =3D 81 } }, > - { DP_LINK_BW_2_7, > + { 270000, > { .p1 =3D 2, .p2 =3D 2, .n =3D 1, .m1 =3D 2, .m2 =3D 27 } } > }; > =20 > @@ -83,11 +83,11 @@ static const struct dp_link_dpll chv_dpll[] =3D { > * m2 is stored in fixed point format using formula below > * (m2_int << 22) | m2_fraction > */ > - { DP_LINK_BW_1_62, /* m2_int =3D 32, m2_fraction =3D 1677722 */ > + { 162000, /* m2_int =3D 32, m2_fraction =3D 1677722 */ > { .p1 =3D 4, .p2 =3D 2, .n =3D 1, .m1 =3D 2, .m2 =3D 0x819999a } }, > - { DP_LINK_BW_2_7, /* m2_int =3D 27, m2_fraction =3D 0 */ > + { 270000, /* m2_int =3D 27, m2_fraction =3D 0 */ > { .p1 =3D 4, .p2 =3D 1, .n =3D 1, .m1 =3D 2, .m2 =3D 0x6c00000 } }, > - { DP_LINK_BW_5_4, /* m2_int =3D 27, m2_fraction =3D 0 */ > + { 540000, /* m2_int =3D 27, m2_fraction =3D 0 */ > { .p1 =3D 2, .p2 =3D 1, .n =3D 1, .m1 =3D 2, .m2 =3D 0x6c00000 } } > }; > =20 > @@ -1099,7 +1099,7 @@ intel_dp_connector_unregister(struct intel_connec= tor *intel_connector) > } > =20 > static void > -skl_edp_set_pll_config(struct intel_crtc_state *pipe_config, int link_= clock) > +skl_edp_set_pll_config(struct intel_crtc_state *pipe_config) > { > u32 ctrl1; > =20 > @@ -1111,7 +1111,7 @@ skl_edp_set_pll_config(struct intel_crtc_state *p= ipe_config, int link_clock) > pipe_config->dpll_hw_state.cfgcr2 =3D 0; > =20 > ctrl1 =3D DPLL_CTRL1_OVERRIDE(SKL_DPLL0); > - switch (link_clock / 2) { > + switch (pipe_config->port_clock / 2) { > case 81000: > ctrl1 |=3D DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, > SKL_DPLL0); > @@ -1145,19 +1145,19 @@ skl_edp_set_pll_config(struct intel_crtc_state = *pipe_config, int link_clock) > } > =20 > static void > -hsw_dp_set_ddi_pll_sel(struct intel_crtc_state *pipe_config, int link_= bw) > +hsw_dp_set_ddi_pll_sel(struct intel_crtc_state *pipe_config) > { > memset(&pipe_config->dpll_hw_state, 0, > sizeof(pipe_config->dpll_hw_state)); > =20 > - switch (link_bw) { > - case DP_LINK_BW_1_62: > + switch (pipe_config->port_clock / 2) { > + case 81000: > pipe_config->ddi_pll_sel =3D PORT_CLK_SEL_LCPLL_810; > break; > - case DP_LINK_BW_2_7: > + case 135000: > pipe_config->ddi_pll_sel =3D PORT_CLK_SEL_LCPLL_1350; > break; > - case DP_LINK_BW_5_4: > + case 270000: > pipe_config->ddi_pll_sel =3D PORT_CLK_SEL_LCPLL_2700; > break; > } > @@ -1204,7 +1204,7 @@ intel_dp_source_rates(struct drm_device *dev, con= st int **source_rates) > =20 > static void > intel_dp_set_clock(struct intel_encoder *encoder, > - struct intel_crtc_state *pipe_config, int link_bw) > + struct intel_crtc_state *pipe_config) > { > struct drm_device *dev =3D encoder->base.dev; > const struct dp_link_dpll *divisor =3D NULL; > @@ -1226,7 +1226,7 @@ intel_dp_set_clock(struct intel_encoder *encoder, > =20 > if (divisor && count) { > for (i =3D 0; i < count; i++) { > - if (link_bw =3D=3D divisor[i].link_bw) { > + if (pipe_config->port_clock =3D=3D divisor[i].clock) { > pipe_config->dpll =3D divisor[i].dpll; > pipe_config->clock_set =3D true; > break; > @@ -1498,13 +1498,13 @@ found: > } > =20 > if (IS_SKYLAKE(dev) && is_edp(intel_dp)) > - skl_edp_set_pll_config(pipe_config, common_rates[clock]); > + skl_edp_set_pll_config(pipe_config); > else if (IS_BROXTON(dev)) > /* handled in ddi */; > else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) > - hsw_dp_set_ddi_pll_sel(pipe_config, intel_dp->link_bw); > + hsw_dp_set_ddi_pll_sel(pipe_config); > else > - intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw); > + intel_dp_set_clock(encoder, pipe_config); > =20 > return true; > } --=20 regards, Sivakumar --------------030103060109020203010104 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>

Use port_clock instead of link_bw when picking the PLL parameters for
DP. link_bw may be zero with an eDP 1.4 sink that supports
DP_LINK_RATE_SET so we shouln't use it for anything other than feed it
shouldn't ? 
to the sink appropriately.

Signed-off-by: Ville Syrj=C3=A4l=C3=A4 <ville.syrjala@linux.in=
tel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 11 ++++------
 drivers/gpu/drm/i915/intel_dp.c  | 44 ++++++++++++++++++++--------------=
------
 2 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/inte=
l_ddi.c
index 9a40bfb..0d3cc98 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1574,17 +1574,14 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc,
 			 DPLL_CFGCR2_PDIV(wrpll_params.pdiv) |
 			 wrpll_params.central_freq;
 	} else if (intel_encoder->type =3D=3D INTEL_OUTPUT_DISPLAYPORT) {
-		struct drm_encoder *encoder =3D &intel_encoder->base;
-		struct intel_dp *intel_dp =3D enc_to_intel_dp(encoder);
-
-		switch (intel_dp->link_bw) {
-		case DP_LINK_BW_1_62:
+		switch (crtc_state->port_clock / 2) {
+		case 81000:
 			ctrl1 |=3D DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0);
 			break;
-		case DP_LINK_BW_2_7:
+		case 135000:
 			ctrl1 |=3D DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350, 0);
 			break;
-		case DP_LINK_BW_5_4:
+		case 270000:
 			ctrl1 |=3D DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, 0);
 			break;
 		}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel=
_dp.c
index decefa1..3dd4342 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -48,28 +48,28 @@
 #define INTEL_DP_RESOLUTION_FAILSAFE	(3 << INTEL_DP_RESOLUTION_SHI=
FT_MASK)
=20
 struct dp_link_dpll {
-	int link_bw;
+	int clock;
 	struct dpll dpll;
 };
=20
 static const struct dp_link_dpll gen4_dpll[] =3D {
-	{ DP_LINK_BW_1_62,
+	{ 162000,
 		{ .p1 =3D 2, .p2 =3D 10, .n =3D 2, .m1 =3D 23, .m2 =3D 8 } },
-	{ DP_LINK_BW_2_7,
+	{ 270000,
 		{ .p1 =3D 1, .p2 =3D 10, .n =3D 1, .m1 =3D 14, .m2 =3D 2 } }
 };
=20
 static const struct dp_link_dpll pch_dpll[] =3D {
-	{ DP_LINK_BW_1_62,
+	{ 162000,
 		{ .p1 =3D 2, .p2 =3D 10, .n =3D 1, .m1 =3D 12, .m2 =3D 9 } },
-	{ DP_LINK_BW_2_7,
+	{ 270000,
 		{ .p1 =3D 1, .p2 =3D 10, .n =3D 2, .m1 =3D 14, .m2 =3D 8 } }
 };
=20
 static const struct dp_link_dpll vlv_dpll[] =3D {
-	{ DP_LINK_BW_1_62,
+	{ 162000,
 		{ .p1 =3D 3, .p2 =3D 2, .n =3D 5, .m1 =3D 3, .m2 =3D 81 } },
-	{ DP_LINK_BW_2_7,
+	{ 270000,
 		{ .p1 =3D 2, .p2 =3D 2, .n =3D 1, .m1 =3D 2, .m2 =3D 27 } }
 };
=20
@@ -83,11 +83,11 @@ static const struct dp_link_dpll chv_dpll[] =3D {
 	 * m2 is stored in fixed point format using formula below
 	 * (m2_int << 22) | m2_fraction
 	 */
-	{ DP_LINK_BW_1_62,	/* m2_int =3D 32, m2_fraction =3D 1677722 */
+	{ 162000,	/* m2_int =3D 32, m2_fraction =3D 1677722 */
 		{ .p1 =3D 4, .p2 =3D 2, .n =3D 1, .m1 =3D 2, .m2 =3D 0x819999a } },
-	{ DP_LINK_BW_2_7,	/* m2_int =3D 27, m2_fraction =3D 0 */
+	{ 270000,	/* m2_int =3D 27, m2_fraction =3D 0 */
 		{ .p1 =3D 4, .p2 =3D 1, .n =3D 1, .m1 =3D 2, .m2 =3D 0x6c00000 } },
-	{ DP_LINK_BW_5_4,	/* m2_int =3D 27, m2_fraction =3D 0 */
+	{ 540000,	/* m2_int =3D 27, m2_fraction =3D 0 */
 		{ .p1 =3D 2, .p2 =3D 1, .n =3D 1, .m1 =3D 2, .m2 =3D 0x6c00000 } }
 };
=20
@@ -1099,7 +1099,7 @@ intel_dp_connector_unregister(struct intel_connecto=
r *intel_connector)
 }
=20
 static void
-skl_edp_set_pll_config(struct intel_crtc_state *pipe_config, int link_cl=
ock)
+skl_edp_set_pll_config(struct intel_crtc_state *pipe_config)
 {
 	u32 ctrl1;
=20
@@ -1111,7 +1111,7 @@ skl_edp_set_pll_config(struct intel_crtc_state *pip=
e_config, int link_clock)
 	pipe_config->dpll_hw_state.cfgcr2 =3D 0;
=20
 	ctrl1 =3D DPLL_CTRL1_OVERRIDE(SKL_DPLL0);
-	switch (link_clock / 2) {
+	switch (pipe_config->port_clock / 2) {
 	case 81000:
 		ctrl1 |=3D DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810,
 					      SKL_DPLL0);
@@ -1145,19 +1145,19 @@ skl_edp_set_pll_config(struct intel_crtc_state *p=
ipe_config, int link_clock)
 }
=20
 static void
-hsw_dp_set_ddi_pll_sel(struct intel_crtc_state *pipe_config, int link_bw=
)
+hsw_dp_set_ddi_pll_sel(struct intel_crtc_state *pipe_config)
 {
 	memset(&pipe_config->dpll_hw_state, 0,
 	       sizeof(pipe_config->dpll_hw_state));
=20
-	switch (link_bw) {
-	case DP_LINK_BW_1_62:
+	switch (pipe_config->port_clock / 2) {
+	case 81000:
 		pipe_config->ddi_pll_sel =3D PORT_CLK_SEL_LCPLL_810;
 		break;
-	case DP_LINK_BW_2_7:
+	case 135000:
 		pipe_config->ddi_pll_sel =3D PORT_CLK_SEL_LCPLL_1350;
 		break;
-	case DP_LINK_BW_5_4:
+	case 270000:
 		pipe_config->ddi_pll_sel =3D PORT_CLK_SEL_LCPLL_2700;
 		break;
 	}
@@ -1204,7 +1204,7 @@ intel_dp_source_rates(struct drm_device *dev, const=
 int **source_rates)
=20
 static void
 intel_dp_set_clock(struct intel_encoder *encoder,
-		   struct intel_crtc_state *pipe_config, int link_bw)
+		   struct intel_crtc_state *pipe_config)
 {
 	struct drm_device *dev =3D encoder->base.dev;
 	const struct dp_link_dpll *divisor =3D NULL;
@@ -1226,7 +1226,7 @@ intel_dp_set_clock(struct intel_encoder *encoder,
=20
 	if (divisor && count) {
 		for (i =3D 0; i < count; i++) {
-			if (link_bw =3D=3D divisor[i].link_bw) {
+			if (pipe_config->port_clock =3D=3D divisor[i].clock) {
 				pipe_config->dpll =3D divisor[i].dpll;
 				pipe_config->clock_set =3D true;
 				break;
@@ -1498,13 +1498,13 @@ found:
 	}
=20
 	if (IS_SKYLAKE(dev) && is_edp(intel_dp))
-		skl_edp_set_pll_config(pipe_config, common_rates[clock]);
+		skl_edp_set_pll_config(pipe_config);
 	else if (IS_BROXTON(dev))
 		/* handled in ddi */;
 	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
-		hsw_dp_set_ddi_pll_sel(pipe_config, intel_dp->link_bw);
+		hsw_dp_set_ddi_pll_sel(pipe_config);
 	else
-		intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
+		intel_dp_set_clock(encoder, pipe_config);
=20
 	return true;
 }


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