All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll()
@ 2019-02-07 17:32 Ville Syrjala
  2019-02-07 17:32 ` [PATCH 02/13] drm/i915: Don't pass crtc to intel_get_shared_dpll() and .get_dpll() Ville Syrjala
                   ` (15 more replies)
  0 siblings, 16 replies; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Passing both crtc and its state is redundant. Pass just the state.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 0a42d11c4c33..1d1a2c456257 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -241,11 +241,11 @@ void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
 }
 
 static struct intel_shared_dpll *
-intel_find_shared_dpll(struct intel_crtc *crtc,
-		       struct intel_crtc_state *crtc_state,
+intel_find_shared_dpll(struct intel_crtc_state *crtc_state,
 		       enum intel_dpll_id range_min,
 		       enum intel_dpll_id range_max)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_shared_dpll *pll, *unused_pll = NULL;
 	struct intel_shared_dpll_state *shared_dpll;
@@ -436,7 +436,7 @@ ibx_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 			      crtc->base.base.id, crtc->base.name,
 			      pll->info->name);
 	} else {
-		pll = intel_find_shared_dpll(crtc, crtc_state,
+		pll = intel_find_shared_dpll(crtc_state,
 					     DPLL_ID_PCH_PLL_A,
 					     DPLL_ID_PCH_PLL_B);
 	}
@@ -780,7 +780,7 @@ static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(int clock,
 
 	crtc_state->dpll_hw_state.wrpll = val;
 
-	pll = intel_find_shared_dpll(crtc, crtc_state,
+	pll = intel_find_shared_dpll(crtc_state,
 				     DPLL_ID_WRPLL1, DPLL_ID_WRPLL2);
 
 	if (!pll)
@@ -840,7 +840,7 @@ hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 		crtc_state->dpll_hw_state.spll =
 			SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC;
 
-		pll = intel_find_shared_dpll(crtc, crtc_state,
+		pll = intel_find_shared_dpll(crtc_state,
 					     DPLL_ID_SPLL, DPLL_ID_SPLL);
 	} else {
 		return NULL;
@@ -1411,11 +1411,11 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	}
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
-		pll = intel_find_shared_dpll(crtc, crtc_state,
+		pll = intel_find_shared_dpll(crtc_state,
 					     DPLL_ID_SKL_DPLL0,
 					     DPLL_ID_SKL_DPLL0);
 	else
-		pll = intel_find_shared_dpll(crtc, crtc_state,
+		pll = intel_find_shared_dpll(crtc_state,
 					     DPLL_ID_SKL_DPLL1,
 					     DPLL_ID_SKL_DPLL3);
 	if (!pll)
@@ -2390,7 +2390,7 @@ cnl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 		return NULL;
 	}
 
-	pll = intel_find_shared_dpll(crtc, crtc_state,
+	pll = intel_find_shared_dpll(crtc_state,
 				     DPLL_ID_SKL_DPLL0,
 				     DPLL_ID_SKL_DPLL2);
 	if (!pll) {
@@ -2945,7 +2945,7 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 
 	crtc_state->dpll_hw_state = pll_state;
 
-	pll = intel_find_shared_dpll(crtc, crtc_state, min, max);
+	pll = intel_find_shared_dpll(crtc_state, min, max);
 	if (!pll) {
 		DRM_DEBUG_KMS("No PLL selected\n");
 		return NULL;
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 02/13] drm/i915: Don't pass crtc to intel_get_shared_dpll() and .get_dpll()
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
@ 2019-02-07 17:32 ` Ville Syrjala
  2019-02-07 17:32 ` [PATCH 03/13] drm/i915: Pass crtc_state down to skl dpll funcs Ville Syrjala
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Passing both crtc and its state is redundant. Pass just the state.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c  |  4 +-
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 56 +++++++++++++--------------
 drivers/gpu/drm/i915/intel_dpll_mgr.h |  3 +-
 3 files changed, 30 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 91b274ea16c4..28f9807bbf6c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8837,7 +8837,7 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 
 	ironlake_compute_dpll(crtc, crtc_state, NULL);
 
-	if (!intel_get_shared_dpll(crtc, crtc_state, NULL)) {
+	if (!intel_get_shared_dpll(crtc_state, NULL)) {
 		DRM_DEBUG_KMS("failed to find PLL for pipe %c\n",
 			      pipe_name(crtc->pipe));
 		return -EINVAL;
@@ -9431,7 +9431,7 @@ static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
 		struct intel_encoder *encoder =
 			intel_get_crtc_new_encoder(state, crtc_state);
 
-		if (!intel_get_shared_dpll(crtc, crtc_state, encoder)) {
+		if (!intel_get_shared_dpll(crtc_state, encoder)) {
 			DRM_DEBUG_KMS("failed to find PLL for pipe %c\n",
 				      pipe_name(crtc->pipe));
 			return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 1d1a2c456257..66069a58b786 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -420,9 +420,10 @@ static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv,
 }
 
 static struct intel_shared_dpll *
-ibx_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
+ibx_get_dpll(struct intel_crtc_state *crtc_state,
 	     struct intel_encoder *encoder)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_shared_dpll *pll;
 	enum intel_dpll_id i;
@@ -764,15 +765,13 @@ hsw_ddi_calculate_wrpll(int clock /* in Hz */,
 	*r2_out = best.r2;
 }
 
-static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(int clock,
-						       struct intel_crtc *crtc,
-						       struct intel_crtc_state *crtc_state)
+static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(struct intel_crtc_state *crtc_state)
 {
 	struct intel_shared_dpll *pll;
 	u32 val;
 	unsigned int p, n2, r2;
 
-	hsw_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
+	hsw_ddi_calculate_wrpll(crtc_state->port_clock * 1000, &r2, &n2, &p);
 
 	val = WRPLL_PLL_ENABLE | WRPLL_PLL_LCPLL |
 	      WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
@@ -790,11 +789,12 @@ static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(int clock,
 }
 
 static struct intel_shared_dpll *
-hsw_ddi_dp_get_dpll(struct intel_encoder *encoder, int clock)
+hsw_ddi_dp_get_dpll(struct intel_crtc_state *crtc_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 	struct intel_shared_dpll *pll;
 	enum intel_dpll_id pll_id;
+	int clock = crtc_state->port_clock;
 
 	switch (clock / 2) {
 	case 81000:
@@ -820,19 +820,18 @@ hsw_ddi_dp_get_dpll(struct intel_encoder *encoder, int clock)
 }
 
 static struct intel_shared_dpll *
-hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
+hsw_get_dpll(struct intel_crtc_state *crtc_state,
 	     struct intel_encoder *encoder)
 {
 	struct intel_shared_dpll *pll;
-	int clock = crtc_state->port_clock;
 
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
-		pll = hsw_ddi_hdmi_get_dpll(clock, crtc, crtc_state);
+		pll = hsw_ddi_hdmi_get_dpll(crtc_state);
 	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
-		pll = hsw_ddi_dp_get_dpll(encoder, clock);
+		pll = hsw_ddi_dp_get_dpll(crtc_state);
 	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
 		if (WARN_ON(crtc_state->port_clock / 2 != 135000))
 			return NULL;
@@ -1383,9 +1382,10 @@ skl_ddi_dp_set_dpll_hw_state(int clock,
 }
 
 static struct intel_shared_dpll *
-skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
+skl_get_dpll(struct intel_crtc_state *crtc_state,
 	     struct intel_encoder *encoder)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct intel_shared_dpll *pll;
 	int clock = crtc_state->port_clock;
 	bool bret;
@@ -1827,10 +1827,10 @@ bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc *intel_crtc,
 }
 
 static struct intel_shared_dpll *
-bxt_get_dpll(struct intel_crtc *crtc,
-		struct intel_crtc_state *crtc_state,
-		struct intel_encoder *encoder)
+bxt_get_dpll(struct intel_crtc_state *crtc_state,
+	     struct intel_encoder *encoder)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct intel_dpll_hw_state dpll_hw_state = { };
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_shared_dpll *pll;
@@ -1911,8 +1911,7 @@ static void intel_ddi_pll_init(struct drm_device *dev)
 struct intel_dpll_mgr {
 	const struct dpll_info *dpll_info;
 
-	struct intel_shared_dpll *(*get_dpll)(struct intel_crtc *crtc,
-					      struct intel_crtc_state *crtc_state,
+	struct intel_shared_dpll *(*get_dpll)(struct intel_crtc_state *crtc_state,
 					      struct intel_encoder *encoder);
 
 	void (*dump_hw_state)(struct drm_i915_private *dev_priv,
@@ -2361,9 +2360,10 @@ cnl_ddi_dp_set_dpll_hw_state(int clock,
 }
 
 static struct intel_shared_dpll *
-cnl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
+cnl_get_dpll(struct intel_crtc_state *crtc_state,
 	     struct intel_encoder *encoder)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct intel_shared_dpll *pll;
 	int clock = crtc_state->port_clock;
 	bool bret;
@@ -2892,10 +2892,10 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
 }
 
 static struct intel_shared_dpll *
-icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
+icl_get_dpll(struct intel_crtc_state *crtc_state,
 	     struct intel_encoder *encoder)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 	struct intel_digital_port *intel_dig_port;
 	struct intel_shared_dpll *pll;
 	struct intel_dpll_hw_state pll_state = {};
@@ -3271,31 +3271,29 @@ void intel_shared_dpll_init(struct drm_device *dev)
 
 /**
  * intel_get_shared_dpll - get a shared DPLL for CRTC and encoder combination
- * @crtc: CRTC
- * @crtc_state: atomic state for @crtc
+ * @crtc_state: atomic state for the crtc
  * @encoder: encoder
  *
  * Find an appropriate DPLL for the given CRTC and encoder combination. A
- * reference from the @crtc to the returned pll is registered in the atomic
- * state. That configuration is made effective by calling
+ * reference from the @crtc_state to the returned pll is registered in the
+ * atomic state. That configuration is made effective by calling
  * intel_shared_dpll_swap_state(). The reference should be released by calling
  * intel_release_shared_dpll().
  *
  * Returns:
- * A shared DPLL to be used by @crtc and @encoder with the given @crtc_state.
+ * A shared DPLL to be used by @crtc_state and @encoder.
  */
 struct intel_shared_dpll *
-intel_get_shared_dpll(struct intel_crtc *crtc,
-		      struct intel_crtc_state *crtc_state,
+intel_get_shared_dpll(struct intel_crtc_state *crtc_state,
 		      struct intel_encoder *encoder)
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 	const struct intel_dpll_mgr *dpll_mgr = dev_priv->dpll_mgr;
 
 	if (WARN_ON(!dpll_mgr))
 		return NULL;
 
-	return dpll_mgr->get_dpll(crtc, crtc_state, encoder);
+	return dpll_mgr->get_dpll(crtc_state, encoder);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index 40e8391a92f2..3a2df77c39c4 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -327,8 +327,7 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
 			bool state);
 #define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
 #define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
-struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
-						struct intel_crtc_state *state,
+struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc_state *state,
 						struct intel_encoder *encoder);
 void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
 			       struct intel_crtc *crtc,
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 03/13] drm/i915: Pass crtc_state down to skl dpll funcs
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
  2019-02-07 17:32 ` [PATCH 02/13] drm/i915: Don't pass crtc to intel_get_shared_dpll() and .get_dpll() Ville Syrjala
@ 2019-02-07 17:32 ` Ville Syrjala
  2019-02-07 17:32 ` [PATCH 04/13] drm/i915: Remove redundant on stack dpll_hw_state from skl_get_dpll() Ville Syrjala
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Simplify the calling convention of the skl dpll funcs by plumbing
the crtc state deeper.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 66069a58b786..ebfd5c7cd864 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1307,9 +1307,7 @@ skl_ddi_calculate_wrpll(int clock /* in Hz */,
 	return true;
 }
 
-static bool skl_ddi_hdmi_pll_dividers(struct intel_crtc *crtc,
-				      struct intel_crtc_state *crtc_state,
-				      int clock)
+static bool skl_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state)
 {
 	u32 ctrl1, cfgcr1, cfgcr2;
 	struct skl_wrpll_params wrpll_params = { 0, };
@@ -1322,7 +1320,8 @@ static bool skl_ddi_hdmi_pll_dividers(struct intel_crtc *crtc,
 
 	ctrl1 |= DPLL_CTRL1_HDMI_MODE(0);
 
-	if (!skl_ddi_calculate_wrpll(clock * 1000, &wrpll_params))
+	if (!skl_ddi_calculate_wrpll(crtc_state->port_clock * 1000,
+				     &wrpll_params))
 		return false;
 
 	cfgcr1 = DPLL_CFGCR1_FREQ_ENABLE |
@@ -1345,7 +1344,7 @@ static bool skl_ddi_hdmi_pll_dividers(struct intel_crtc *crtc,
 }
 
 static bool
-skl_ddi_dp_set_dpll_hw_state(int clock,
+skl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
 			     struct intel_dpll_hw_state *dpll_hw_state)
 {
 	u32 ctrl1;
@@ -1355,7 +1354,7 @@ skl_ddi_dp_set_dpll_hw_state(int clock,
 	 * as the DPLL id in this function.
 	 */
 	ctrl1 = DPLL_CTRL1_OVERRIDE(0);
-	switch (clock / 2) {
+	switch (crtc_state->port_clock / 2) {
 	case 81000:
 		ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0);
 		break;
@@ -1385,22 +1384,20 @@ static struct intel_shared_dpll *
 skl_get_dpll(struct intel_crtc_state *crtc_state,
 	     struct intel_encoder *encoder)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct intel_shared_dpll *pll;
-	int clock = crtc_state->port_clock;
 	bool bret;
 	struct intel_dpll_hw_state dpll_hw_state;
 
 	memset(&dpll_hw_state, 0, sizeof(dpll_hw_state));
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
-		bret = skl_ddi_hdmi_pll_dividers(crtc, crtc_state, clock);
+		bret = skl_ddi_hdmi_pll_dividers(crtc_state);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not get HDMI pll dividers.\n");
 			return NULL;
 		}
 	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
-		bret = skl_ddi_dp_set_dpll_hw_state(clock, &dpll_hw_state);
+		bret = skl_ddi_dp_set_dpll_hw_state(crtc_state, &dpll_hw_state);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not set DP dpll HW state.\n");
 			return NULL;
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 04/13] drm/i915: Remove redundant on stack dpll_hw_state from skl_get_dpll()
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
  2019-02-07 17:32 ` [PATCH 02/13] drm/i915: Don't pass crtc to intel_get_shared_dpll() and .get_dpll() Ville Syrjala
  2019-02-07 17:32 ` [PATCH 03/13] drm/i915: Pass crtc_state down to skl dpll funcs Ville Syrjala
@ 2019-02-07 17:32 ` Ville Syrjala
  2019-02-07 17:32 ` [PATCH 05/13] drm/i915: Pass crtc_state down to bxt dpll funcs Ville Syrjala
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Just store the stuff directly into crtc_state->dpll_hw_state rather
than to a temp and copying the whole thing over.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index ebfd5c7cd864..f0e3a0a62db0 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1344,8 +1344,7 @@ static bool skl_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state)
 }
 
 static bool
-skl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
-			     struct intel_dpll_hw_state *dpll_hw_state)
+skl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
 {
 	u32 ctrl1;
 
@@ -1376,7 +1375,11 @@ skl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
 		break;
 	}
 
-	dpll_hw_state->ctrl1 = ctrl1;
+	memset(&crtc_state->dpll_hw_state, 0,
+	       sizeof(crtc_state->dpll_hw_state));
+
+	crtc_state->dpll_hw_state.ctrl1 = ctrl1;
+
 	return true;
 }
 
@@ -1386,9 +1389,6 @@ skl_get_dpll(struct intel_crtc_state *crtc_state,
 {
 	struct intel_shared_dpll *pll;
 	bool bret;
-	struct intel_dpll_hw_state dpll_hw_state;
-
-	memset(&dpll_hw_state, 0, sizeof(dpll_hw_state));
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
 		bret = skl_ddi_hdmi_pll_dividers(crtc_state);
@@ -1397,12 +1397,11 @@ skl_get_dpll(struct intel_crtc_state *crtc_state,
 			return NULL;
 		}
 	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
-		bret = skl_ddi_dp_set_dpll_hw_state(crtc_state, &dpll_hw_state);
+		bret = skl_ddi_dp_set_dpll_hw_state(crtc_state);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not set DP dpll HW state.\n");
 			return NULL;
 		}
-		crtc_state->dpll_hw_state = dpll_hw_state;
 	} else {
 		return NULL;
 	}
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 05/13] drm/i915: Pass crtc_state down to bxt dpll funcs
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (2 preceding siblings ...)
  2019-02-07 17:32 ` [PATCH 04/13] drm/i915: Remove redundant on stack dpll_hw_state from skl_get_dpll() Ville Syrjala
@ 2019-02-07 17:32 ` Ville Syrjala
  2019-02-07 17:32 ` [PATCH 06/13] drm/i915: Remove redundant on stack dpll_hw_state from bxt_get_dpll() Ville Syrjala
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Simplify the calling convention of the dpll funcs by plumbing
the crtc state deeper.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c  |  5 +--
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 48 ++++++++++++++-------------
 drivers/gpu/drm/i915/intel_drv.h      |  2 +-
 3 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 28f9807bbf6c..cc83206fe166 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -942,14 +942,15 @@ chv_find_best_dpll(const struct intel_limit *limit,
 	return found;
 }
 
-bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
+bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state,
 			struct dpll *best_clock)
 {
 	int refclk = 100000;
 	const struct intel_limit *limit = &intel_limits_bxt;
 
 	return chv_find_best_dpll(limit, crtc_state,
-				  target_clock, refclk, NULL, best_clock);
+				  crtc_state->port_clock, refclk,
+				  NULL, best_clock);
 }
 
 bool intel_crtc_active(struct intel_crtc *crtc)
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index f0e3a0a62db0..3de62c704580 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1688,10 +1688,10 @@ static const struct bxt_clk_div bxt_dp_clk_val[] = {
 };
 
 static bool
-bxt_ddi_hdmi_pll_dividers(struct intel_crtc *intel_crtc,
-			  struct intel_crtc_state *crtc_state, int clock,
+bxt_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state,
 			  struct bxt_clk_div *clk_div)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct dpll best_clock;
 
 	/* Calculate HDMI div */
@@ -1699,9 +1699,10 @@ bxt_ddi_hdmi_pll_dividers(struct intel_crtc *intel_crtc,
 	 * FIXME: tie the following calculation into
 	 * i9xx_crtc_compute_clock
 	 */
-	if (!bxt_find_best_dpll(crtc_state, clock, &best_clock)) {
+	if (!bxt_find_best_dpll(crtc_state, &best_clock)) {
 		DRM_DEBUG_DRIVER("no PLL dividers found for clock %d pipe %c\n",
-				 clock, pipe_name(intel_crtc->pipe));
+				 crtc_state->port_clock,
+				 pipe_name(crtc->pipe));
 		return false;
 	}
 
@@ -1718,8 +1719,10 @@ bxt_ddi_hdmi_pll_dividers(struct intel_crtc *intel_crtc,
 	return true;
 }
 
-static void bxt_ddi_dp_pll_dividers(int clock, struct bxt_clk_div *clk_div)
+static void bxt_ddi_dp_pll_dividers(struct intel_crtc_state *crtc_state,
+				    struct bxt_clk_div *clk_div)
 {
+	int clock = crtc_state->port_clock;
 	int i;
 
 	*clk_div = bxt_dp_clk_val[0];
@@ -1733,10 +1736,11 @@ static void bxt_ddi_dp_pll_dividers(int clock, struct bxt_clk_div *clk_div)
 	clk_div->vco = clock * 10 / 2 * clk_div->p1 * clk_div->p2;
 }
 
-static bool bxt_ddi_set_dpll_hw_state(int clock,
-			  struct bxt_clk_div *clk_div,
-			  struct intel_dpll_hw_state *dpll_hw_state)
+static bool bxt_ddi_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
+				      struct bxt_clk_div *clk_div,
+				      struct intel_dpll_hw_state *dpll_hw_state)
 {
+	int clock = crtc_state->port_clock;
 	int vco = clk_div->vco;
 	u32 prop_coef, int_coef, gain_ctl, targ_cnt;
 	u32 lanestagger;
@@ -1800,26 +1804,25 @@ static bool bxt_ddi_set_dpll_hw_state(int clock,
 }
 
 static bool
-bxt_ddi_dp_set_dpll_hw_state(int clock,
+bxt_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
 			     struct intel_dpll_hw_state *dpll_hw_state)
 {
-	struct bxt_clk_div clk_div = {0};
+	struct bxt_clk_div clk_div = {};
 
-	bxt_ddi_dp_pll_dividers(clock, &clk_div);
+	bxt_ddi_dp_pll_dividers(crtc_state, &clk_div);
 
-	return bxt_ddi_set_dpll_hw_state(clock, &clk_div, dpll_hw_state);
+	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div, dpll_hw_state);
 }
 
 static bool
-bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc *intel_crtc,
-			       struct intel_crtc_state *crtc_state, int clock,
+bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
 			       struct intel_dpll_hw_state *dpll_hw_state)
 {
-	struct bxt_clk_div clk_div = { };
+	struct bxt_clk_div clk_div = {};
 
-	bxt_ddi_hdmi_pll_dividers(intel_crtc, crtc_state, clock, &clk_div);
+	bxt_ddi_hdmi_pll_dividers(crtc_state, &clk_div);
 
-	return bxt_ddi_set_dpll_hw_state(clock, &clk_div, dpll_hw_state);
+	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div, dpll_hw_state);
 }
 
 static struct intel_shared_dpll *
@@ -1830,15 +1833,14 @@ bxt_get_dpll(struct intel_crtc_state *crtc_state,
 	struct intel_dpll_hw_state dpll_hw_state = { };
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_shared_dpll *pll;
-	int i, clock = crtc_state->port_clock;
+	enum intel_dpll_id id;
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
-	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc, crtc_state, clock,
-					    &dpll_hw_state))
+	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc_state, &dpll_hw_state))
 		return NULL;
 
 	if (intel_crtc_has_dp_encoder(crtc_state) &&
-	    !bxt_ddi_dp_set_dpll_hw_state(clock, &dpll_hw_state))
+	    !bxt_ddi_dp_set_dpll_hw_state(crtc_state, &dpll_hw_state))
 		return NULL;
 
 	memset(&crtc_state->dpll_hw_state, 0,
@@ -1847,8 +1849,8 @@ bxt_get_dpll(struct intel_crtc_state *crtc_state,
 	crtc_state->dpll_hw_state = dpll_hw_state;
 
 	/* 1:1 mapping between ports and PLLs */
-	i = (enum intel_dpll_id) encoder->port;
-	pll = intel_get_shared_dpll_by_id(dev_priv, i);
+	id = (enum intel_dpll_id) encoder->port;
+	pll = intel_get_shared_dpll_by_id(dev_priv, id);
 
 	DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n",
 		      crtc->base.base.id, crtc->base.name, pll->info->name);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9ec3d00fbd19..aabc3ffdb95b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1738,7 +1738,7 @@ void intel_dp_get_m_n(struct intel_crtc *crtc,
 void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
 		      enum link_m_n_set m_n);
 int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n);
-bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
+bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state,
 			struct dpll *best_clock);
 int chv_calc_dpll_params(int refclk, struct dpll *pll_clock);
 
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 06/13] drm/i915: Remove redundant on stack dpll_hw_state from bxt_get_dpll()
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (3 preceding siblings ...)
  2019-02-07 17:32 ` [PATCH 05/13] drm/i915: Pass crtc_state down to bxt dpll funcs Ville Syrjala
@ 2019-02-07 17:32 ` Ville Syrjala
  2019-02-07 17:32 ` [PATCH 07/13] drm/i915: Pass crtc_state down to cnl dpll funcs Ville Syrjala
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Just store the stuff directly into crtc_state->dpll_hw_state rather
than to a temp and copying the whole thing over.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 3de62c704580..c428dcd06ed0 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1737,14 +1737,16 @@ static void bxt_ddi_dp_pll_dividers(struct intel_crtc_state *crtc_state,
 }
 
 static bool bxt_ddi_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
-				      struct bxt_clk_div *clk_div,
-				      struct intel_dpll_hw_state *dpll_hw_state)
+				      const struct bxt_clk_div *clk_div)
 {
+	struct intel_dpll_hw_state *dpll_hw_state = &crtc_state->dpll_hw_state;
 	int clock = crtc_state->port_clock;
 	int vco = clk_div->vco;
 	u32 prop_coef, int_coef, gain_ctl, targ_cnt;
 	u32 lanestagger;
 
+	memset(dpll_hw_state, 0, sizeof(*dpll_hw_state));
+
 	if (vco >= 6200000 && vco <= 6700000) {
 		prop_coef = 4;
 		int_coef = 9;
@@ -1804,25 +1806,23 @@ static bool bxt_ddi_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
 }
 
 static bool
-bxt_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
-			     struct intel_dpll_hw_state *dpll_hw_state)
+bxt_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
 {
 	struct bxt_clk_div clk_div = {};
 
 	bxt_ddi_dp_pll_dividers(crtc_state, &clk_div);
 
-	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div, dpll_hw_state);
+	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
 }
 
 static bool
-bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
-			       struct intel_dpll_hw_state *dpll_hw_state)
+bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
 {
 	struct bxt_clk_div clk_div = {};
 
 	bxt_ddi_hdmi_pll_dividers(crtc_state, &clk_div);
 
-	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div, dpll_hw_state);
+	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
 }
 
 static struct intel_shared_dpll *
@@ -1830,24 +1830,18 @@ bxt_get_dpll(struct intel_crtc_state *crtc_state,
 	     struct intel_encoder *encoder)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct intel_dpll_hw_state dpll_hw_state = { };
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_shared_dpll *pll;
 	enum intel_dpll_id id;
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
-	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc_state, &dpll_hw_state))
+	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc_state))
 		return NULL;
 
 	if (intel_crtc_has_dp_encoder(crtc_state) &&
-	    !bxt_ddi_dp_set_dpll_hw_state(crtc_state, &dpll_hw_state))
+	    !bxt_ddi_dp_set_dpll_hw_state(crtc_state))
 		return NULL;
 
-	memset(&crtc_state->dpll_hw_state, 0,
-	       sizeof(crtc_state->dpll_hw_state));
-
-	crtc_state->dpll_hw_state = dpll_hw_state;
-
 	/* 1:1 mapping between ports and PLLs */
 	id = (enum intel_dpll_id) encoder->port;
 	pll = intel_get_shared_dpll_by_id(dev_priv, id);
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 07/13] drm/i915: Pass crtc_state down to cnl dpll funcs
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (4 preceding siblings ...)
  2019-02-07 17:32 ` [PATCH 06/13] drm/i915: Remove redundant on stack dpll_hw_state from bxt_get_dpll() Ville Syrjala
@ 2019-02-07 17:32 ` Ville Syrjala
  2019-02-07 17:32 ` [PATCH 08/13] drm/i915: Remove redundant on stack dpll_hw_state from cnl_get_dpll() Ville Syrjala
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Simplify the calling convention of the dpll funcs by plumbing
the crtc state deeper.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 29 +++++++++++----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index c428dcd06ed0..b34d457cf7aa 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2233,11 +2233,11 @@ int cnl_hdmi_pll_ref_clock(struct drm_i915_private *dev_priv)
 }
 
 static bool
-cnl_ddi_calculate_wrpll(int clock,
-			struct drm_i915_private *dev_priv,
+cnl_ddi_calculate_wrpll(struct intel_crtc_state *crtc_state,
 			struct skl_wrpll_params *wrpll_params)
 {
-	u32 afe_clock = clock * 5;
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+	u32 afe_clock = crtc_state->port_clock * 5;
 	u32 ref_clock;
 	u32 dco_min = 7998000;
 	u32 dco_max = 10000000;
@@ -2273,23 +2273,20 @@ cnl_ddi_calculate_wrpll(int clock,
 
 	ref_clock = cnl_hdmi_pll_ref_clock(dev_priv);
 
-	cnl_wrpll_params_populate(wrpll_params, best_dco, ref_clock, pdiv, qdiv,
-				  kdiv);
+	cnl_wrpll_params_populate(wrpll_params, best_dco, ref_clock,
+				  pdiv, qdiv, kdiv);
 
 	return true;
 }
 
-static bool cnl_ddi_hdmi_pll_dividers(struct intel_crtc *crtc,
-				      struct intel_crtc_state *crtc_state,
-				      int clock)
+static bool cnl_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	u32 cfgcr0, cfgcr1;
 	struct skl_wrpll_params wrpll_params = { 0, };
 
 	cfgcr0 = DPLL_CFGCR0_HDMI_MODE;
 
-	if (!cnl_ddi_calculate_wrpll(clock, dev_priv, &wrpll_params))
+	if (!cnl_ddi_calculate_wrpll(crtc_state, &wrpll_params))
 		return false;
 
 	cfgcr0 |= DPLL_CFGCR0_DCO_FRACTION(wrpll_params.dco_fraction) |
@@ -2310,14 +2307,14 @@ static bool cnl_ddi_hdmi_pll_dividers(struct intel_crtc *crtc,
 }
 
 static bool
-cnl_ddi_dp_set_dpll_hw_state(int clock,
+cnl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
 			     struct intel_dpll_hw_state *dpll_hw_state)
 {
 	u32 cfgcr0;
 
 	cfgcr0 = DPLL_CFGCR0_SSC_ENABLE;
 
-	switch (clock / 2) {
+	switch (crtc_state->port_clock / 2) {
 	case 81000:
 		cfgcr0 |= DPLL_CFGCR0_LINK_RATE_810;
 		break;
@@ -2355,22 +2352,20 @@ static struct intel_shared_dpll *
 cnl_get_dpll(struct intel_crtc_state *crtc_state,
 	     struct intel_encoder *encoder)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct intel_shared_dpll *pll;
-	int clock = crtc_state->port_clock;
 	bool bret;
 	struct intel_dpll_hw_state dpll_hw_state;
 
 	memset(&dpll_hw_state, 0, sizeof(dpll_hw_state));
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
-		bret = cnl_ddi_hdmi_pll_dividers(crtc, crtc_state, clock);
+		bret = cnl_ddi_hdmi_pll_dividers(crtc_state);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not get HDMI pll dividers.\n");
 			return NULL;
 		}
 	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
-		bret = cnl_ddi_dp_set_dpll_hw_state(clock, &dpll_hw_state);
+		bret = cnl_ddi_dp_set_dpll_hw_state(crtc_state, &dpll_hw_state);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not set DP dpll HW state.\n");
 			return NULL;
@@ -2539,7 +2534,7 @@ static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
 		ret = icl_calc_tbt_pll(dev_priv, clock, &pll_params);
 	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) ||
 		 intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
-		ret = cnl_ddi_calculate_wrpll(clock, dev_priv, &pll_params);
+		ret = cnl_ddi_calculate_wrpll(crtc_state, &pll_params);
 	else
 		ret = icl_calc_dp_combo_pll(dev_priv, clock, &pll_params);
 
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 08/13] drm/i915: Remove redundant on stack dpll_hw_state from cnl_get_dpll()
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (5 preceding siblings ...)
  2019-02-07 17:32 ` [PATCH 07/13] drm/i915: Pass crtc_state down to cnl dpll funcs Ville Syrjala
@ 2019-02-07 17:32 ` Ville Syrjala
  2019-02-07 17:32 ` [PATCH 09/13] drm/i915: Pass crtc_state down to icl dpll funcs Ville Syrjala
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Just store the stuff directly into crtc_state->dpll_hw_state rather
than to a temp and copying the whole thing over.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index b34d457cf7aa..5937fe2af9e9 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2307,8 +2307,7 @@ static bool cnl_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state)
 }
 
 static bool
-cnl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
-			     struct intel_dpll_hw_state *dpll_hw_state)
+cnl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
 {
 	u32 cfgcr0;
 
@@ -2344,7 +2343,11 @@ cnl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
 		break;
 	}
 
-	dpll_hw_state->cfgcr0 = cfgcr0;
+	memset(&crtc_state->dpll_hw_state, 0,
+	       sizeof(crtc_state->dpll_hw_state));
+
+	crtc_state->dpll_hw_state.cfgcr0 = cfgcr0;
+
 	return true;
 }
 
@@ -2354,9 +2357,6 @@ cnl_get_dpll(struct intel_crtc_state *crtc_state,
 {
 	struct intel_shared_dpll *pll;
 	bool bret;
-	struct intel_dpll_hw_state dpll_hw_state;
-
-	memset(&dpll_hw_state, 0, sizeof(dpll_hw_state));
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
 		bret = cnl_ddi_hdmi_pll_dividers(crtc_state);
@@ -2365,12 +2365,11 @@ cnl_get_dpll(struct intel_crtc_state *crtc_state,
 			return NULL;
 		}
 	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
-		bret = cnl_ddi_dp_set_dpll_hw_state(crtc_state, &dpll_hw_state);
+		bret = cnl_ddi_dp_set_dpll_hw_state(crtc_state);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not set DP dpll HW state.\n");
 			return NULL;
 		}
-		crtc_state->dpll_hw_state = dpll_hw_state;
 	} else {
 		DRM_DEBUG_KMS("Skip DPLL setup for output_types 0x%x\n",
 			      crtc_state->output_types);
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 09/13] drm/i915: Pass crtc_state down to icl dpll funcs
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (6 preceding siblings ...)
  2019-02-07 17:32 ` [PATCH 08/13] drm/i915: Remove redundant on stack dpll_hw_state from cnl_get_dpll() Ville Syrjala
@ 2019-02-07 17:32 ` Ville Syrjala
  2019-02-07 17:32 ` [PATCH 10/13] drm/i915: Remove redundant on stack dpll_hw_state from icl_get_dpll() Ville Syrjala
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Simplify the calling convention of the dpll funcs by plumbing
the crtc state deeper.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 5937fe2af9e9..879f405d9742 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2470,10 +2470,12 @@ static const struct skl_wrpll_params icl_tbt_pll_19_2MHz_values = {
 	.pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0,
 };
 
-static bool icl_calc_dp_combo_pll(struct drm_i915_private *dev_priv, int clock,
+static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
 				  struct skl_wrpll_params *pll_params)
 {
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 	const struct skl_wrpll_params *params;
+	int clock = crtc_state->port_clock;
 
 	params = dev_priv->cdclk.hw.ref == 24000 ?
 			icl_dp_combo_pll_24MHz_values :
@@ -2512,9 +2514,11 @@ static bool icl_calc_dp_combo_pll(struct drm_i915_private *dev_priv, int clock,
 	return true;
 }
 
-static bool icl_calc_tbt_pll(struct drm_i915_private *dev_priv, int clock,
+static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
 			     struct skl_wrpll_params *pll_params)
 {
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+
 	*pll_params = dev_priv->cdclk.hw.ref == 24000 ?
 			icl_tbt_pll_24MHz_values : icl_tbt_pll_19_2MHz_values;
 	return true;
@@ -2530,12 +2534,12 @@ static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
 	bool ret;
 
 	if (intel_port_is_tc(dev_priv, encoder->port))
-		ret = icl_calc_tbt_pll(dev_priv, clock, &pll_params);
+		ret = icl_calc_tbt_pll(crtc_state, &pll_params);
 	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) ||
 		 intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
 		ret = cnl_ddi_calculate_wrpll(crtc_state, &pll_params);
 	else
-		ret = icl_calc_dp_combo_pll(dev_priv, clock, &pll_params);
+		ret = icl_calc_dp_combo_pll(crtc_state, &pll_params);
 
 	if (!ret)
 		return false;
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 10/13] drm/i915: Remove redundant on stack dpll_hw_state from icl_get_dpll()
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (7 preceding siblings ...)
  2019-02-07 17:32 ` [PATCH 09/13] drm/i915: Pass crtc_state down to icl dpll funcs Ville Syrjala
@ 2019-02-07 17:32 ` Ville Syrjala
  2019-02-07 17:32 ` [PATCH 11/13] drm/i915: Fix readout for cnl DPLL kdiv==3 Ville Syrjala
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Just store the stuff directly into crtc_state->dpll_hw_state rather
than to a temp and copying the whole thing over.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 35 +++++++++++++--------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 879f405d9742..fa2e5aae3f72 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2525,10 +2525,9 @@ static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
 }
 
 static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
-				struct intel_encoder *encoder, int clock,
-				struct intel_dpll_hw_state *pll_state)
+				struct intel_encoder *encoder)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 	u32 cfgcr0, cfgcr1;
 	struct skl_wrpll_params pll_params = { 0 };
 	bool ret;
@@ -2553,8 +2552,12 @@ static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
 		 DPLL_CFGCR1_PDIV(pll_params.pdiv) |
 		 DPLL_CFGCR1_CENTRAL_FREQ_8400;
 
-	pll_state->cfgcr0 = cfgcr0;
-	pll_state->cfgcr1 = cfgcr1;
+	memset(&crtc_state->dpll_hw_state, 0,
+	       sizeof(crtc_state->dpll_hw_state));
+
+	crtc_state->dpll_hw_state.cfgcr0 = cfgcr0;
+	crtc_state->dpll_hw_state.cfgcr1 = cfgcr1;
+
 	return true;
 }
 
@@ -2718,12 +2721,12 @@ static bool icl_mg_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
  * The specification for this function uses real numbers, so the math had to be
  * adapted to integer-only calculation, that's why it looks so different.
  */
-static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
-				  struct intel_encoder *encoder, int clock,
-				  struct intel_dpll_hw_state *pll_state)
+static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+	struct intel_dpll_hw_state *pll_state = &crtc_state->dpll_hw_state;
 	int refclk_khz = dev_priv->cdclk.hw.ref;
+	int clock = crtc_state->port_clock;
 	u32 dco_khz, m1div, m2div_int, m2div_rem, m2div_frac;
 	u32 iref_ndiv, iref_trim, iref_pulse_w;
 	u32 prop_coeff, int_coeff;
@@ -2733,6 +2736,8 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
 	bool use_ssc = false;
 	bool is_dp = !intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI);
 
+	memset(pll_state, 0, sizeof(*pll_state));
+
 	if (!icl_mg_pll_find_divisors(clock, is_dp, use_ssc, &dco_khz,
 				      pll_state)) {
 		DRM_DEBUG_KMS("Failed to find divisors for clock %d\n", clock);
@@ -2888,17 +2893,14 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 	struct intel_digital_port *intel_dig_port;
 	struct intel_shared_dpll *pll;
-	struct intel_dpll_hw_state pll_state = {};
 	enum port port = encoder->port;
 	enum intel_dpll_id min, max;
-	int clock = crtc_state->port_clock;
 	bool ret;
 
 	if (intel_port_is_combophy(dev_priv, port)) {
 		min = DPLL_ID_ICL_DPLL0;
 		max = DPLL_ID_ICL_DPLL1;
-		ret = icl_calc_dpll_state(crtc_state, encoder, clock,
-					  &pll_state);
+		ret = icl_calc_dpll_state(crtc_state, encoder);
 	} else if (intel_port_is_tc(dev_priv, port)) {
 		if (encoder->type == INTEL_OUTPUT_DP_MST) {
 			struct intel_dp_mst_encoder *mst_encoder;
@@ -2912,16 +2914,14 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
 		if (intel_dig_port->tc_type == TC_PORT_TBT) {
 			min = DPLL_ID_ICL_TBTPLL;
 			max = min;
-			ret = icl_calc_dpll_state(crtc_state, encoder, clock,
-						  &pll_state);
+			ret = icl_calc_dpll_state(crtc_state, encoder);
 		} else {
 			enum tc_port tc_port;
 
 			tc_port = intel_port_to_tc(dev_priv, port);
 			min = icl_tc_port_to_pll_id(tc_port);
 			max = min;
-			ret = icl_calc_mg_pll_state(crtc_state, encoder, clock,
-						    &pll_state);
+			ret = icl_calc_mg_pll_state(crtc_state);
 		}
 	} else {
 		MISSING_CASE(port);
@@ -2933,7 +2933,6 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
 		return NULL;
 	}
 
-	crtc_state->dpll_hw_state = pll_state;
 
 	pll = intel_find_shared_dpll(crtc_state, min, max);
 	if (!pll) {
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 11/13] drm/i915: Fix readout for cnl DPLL kdiv==3
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (8 preceding siblings ...)
  2019-02-07 17:32 ` [PATCH 10/13] drm/i915: Remove redundant on stack dpll_hw_state from icl_get_dpll() Ville Syrjala
@ 2019-02-07 17:32 ` Ville Syrjala
  2019-02-07 22:48   ` Lucas De Marchi
  2019-02-07 17:32 ` [PATCH 12/13] drm/i915: Nuke icl_calc_dp_combo_pll_link() Ville Syrjala
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The readout code thinks that kdiv of 3 is 4. Fix it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 2 +-
 drivers/gpu/drm/i915/intel_ddi.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 638a586469f9..f8c15148927a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9741,7 +9741,7 @@ enum skl_power_gate {
 #define  DPLL_CFGCR1_KDIV(x)		((x) << 6)
 #define  DPLL_CFGCR1_KDIV_1		(1 << 6)
 #define  DPLL_CFGCR1_KDIV_2		(2 << 6)
-#define  DPLL_CFGCR1_KDIV_4		(4 << 6)
+#define  DPLL_CFGCR1_KDIV_3		(4 << 6)
 #define  DPLL_CFGCR1_PDIV_MASK		(0xf << 2)
 #define  DPLL_CFGCR1_PDIV_SHIFT		(2)
 #define  DPLL_CFGCR1_PDIV(x)		((x) << 2)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ca705546a0ab..fd192baf93b2 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1349,8 +1349,8 @@ int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
 	case DPLL_CFGCR1_KDIV_2:
 		p2 = 2;
 		break;
-	case DPLL_CFGCR1_KDIV_4:
-		p2 = 4;
+	case DPLL_CFGCR1_KDIV_3:
+		p2 = 3;
 		break;
 	}
 
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 12/13] drm/i915: Nuke icl_calc_dp_combo_pll_link()
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (9 preceding siblings ...)
  2019-02-07 17:32 ` [PATCH 11/13] drm/i915: Fix readout for cnl DPLL kdiv==3 Ville Syrjala
@ 2019-02-07 17:32 ` Ville Syrjala
  2019-02-07 17:32 ` [PATCH 13/13] drm/i915: Remove the fragile array index -> link rate mapping Ville Syrjala
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We already have the code to calculate the WRPLL output clock from
the register values, but for some reason we're only using it for
HDMI and not DP. Throw out the inflexible DP DPLL table lookup and
just call the HDMI code which decodes the actual register values.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c      |  6 +--
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 70 ---------------------------
 drivers/gpu/drm/i915/intel_dpll_mgr.h |  2 -
 3 files changed, 1 insertion(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index fd192baf93b2..9c25837507fc 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1477,11 +1477,7 @@ static void icl_ddi_clock_get(struct intel_encoder *encoder,
 
 	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
 	if (intel_port_is_combophy(dev_priv, port)) {
-		if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
-			link_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
-		else
-			link_clock = icl_calc_dp_combo_pll_link(dev_priv,
-								pll_id);
+		link_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
 	} else {
 		if (pll_id == DPLL_ID_ICL_TBTPLL)
 			link_clock = icl_calc_tbt_pll_link(dev_priv, port);
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index fa2e5aae3f72..78dd28aa123c 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2561,76 +2561,6 @@ static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
 	return true;
 }
 
-int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv,
-			       u32 pll_id)
-{
-	u32 cfgcr0, cfgcr1;
-	u32 pdiv, kdiv, qdiv_mode, qdiv_ratio, dco_integer, dco_fraction;
-	const struct skl_wrpll_params *params;
-	int index, n_entries, link_clock;
-
-	/* Read back values from DPLL CFGCR registers */
-	cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(pll_id));
-	cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(pll_id));
-
-	dco_integer = cfgcr0 & DPLL_CFGCR0_DCO_INTEGER_MASK;
-	dco_fraction = (cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
-		DPLL_CFGCR0_DCO_FRACTION_SHIFT;
-	pdiv = (cfgcr1 & DPLL_CFGCR1_PDIV_MASK) >> DPLL_CFGCR1_PDIV_SHIFT;
-	kdiv = (cfgcr1 & DPLL_CFGCR1_KDIV_MASK) >> DPLL_CFGCR1_KDIV_SHIFT;
-	qdiv_mode = (cfgcr1 & DPLL_CFGCR1_QDIV_MODE(1)) >>
-		DPLL_CFGCR1_QDIV_MODE_SHIFT;
-	qdiv_ratio = (cfgcr1 & DPLL_CFGCR1_QDIV_RATIO_MASK) >>
-		DPLL_CFGCR1_QDIV_RATIO_SHIFT;
-
-	params = dev_priv->cdclk.hw.ref == 24000 ?
-		icl_dp_combo_pll_24MHz_values :
-		icl_dp_combo_pll_19_2MHz_values;
-	n_entries = ARRAY_SIZE(icl_dp_combo_pll_24MHz_values);
-
-	for (index = 0; index < n_entries; index++) {
-		if (dco_integer == params[index].dco_integer &&
-		    dco_fraction == params[index].dco_fraction &&
-		    pdiv == params[index].pdiv &&
-		    kdiv == params[index].kdiv &&
-		    qdiv_mode == params[index].qdiv_mode &&
-		    qdiv_ratio == params[index].qdiv_ratio)
-			break;
-	}
-
-	/* Map PLL Index to Link Clock */
-	switch (index) {
-	default:
-		MISSING_CASE(index);
-		/* fall through */
-	case 0:
-		link_clock = 540000;
-		break;
-	case 1:
-		link_clock = 270000;
-		break;
-	case 2:
-		link_clock = 162000;
-		break;
-	case 3:
-		link_clock = 324000;
-		break;
-	case 4:
-		link_clock = 216000;
-		break;
-	case 5:
-		link_clock = 432000;
-		break;
-	case 6:
-		link_clock = 648000;
-		break;
-	case 7:
-		link_clock = 810000;
-		break;
-	}
-
-	return link_clock;
-}
 
 static enum tc_port icl_pll_id_to_tc_port(enum intel_dpll_id id)
 {
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index 3a2df77c39c4..bd8124cc81ed 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -340,8 +340,6 @@ void intel_shared_dpll_init(struct drm_device *dev);
 
 void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv,
 			      struct intel_dpll_hw_state *hw_state);
-int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv,
-			       u32 pll_id);
 int cnl_hdmi_pll_ref_clock(struct drm_i915_private *dev_priv);
 enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port);
 bool intel_dpll_is_combophy(enum intel_dpll_id id);
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 13/13] drm/i915: Remove the fragile array index -> link rate mapping
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (10 preceding siblings ...)
  2019-02-07 17:32 ` [PATCH 12/13] drm/i915: Nuke icl_calc_dp_combo_pll_link() Ville Syrjala
@ 2019-02-07 17:32 ` Ville Syrjala
  2019-02-07 23:11   ` Lucas De Marchi
  2019-02-07 18:01 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Patchwork
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjala @ 2019-02-07 17:32 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rather than try to maintain some magic relationship between the link
rates and the index into the wrpll params array let's just store
the link rate in the array itself. Much less fragile.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 136 +++++++++++++-------------
 1 file changed, 68 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 78dd28aa123c..9aacd19aede1 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2417,47 +2417,69 @@ static const struct intel_dpll_mgr cnl_pll_mgr = {
 	.dump_hw_state = cnl_dump_hw_state,
 };
 
+struct icl_combo_pll_params {
+	int clock;
+	struct skl_wrpll_params wrpll;
+};
+
 /*
  * These values alrea already adjusted: they're the bits we write to the
  * registers, not the logical values.
  */
-static const struct skl_wrpll_params icl_dp_combo_pll_24MHz_values[] = {
-	{ .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [0]: 5.4 */
-	  .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [1]: 2.7 */
-	  .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [2]: 1.62 */
-	  .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [3]: 3.24 */
-	  .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x168, .dco_fraction = 0x0000,		/* [4]: 2.16 */
-	  .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2},
-	{ .dco_integer = 0x168, .dco_fraction = 0x0000,		/* [5]: 4.32 */
-	  .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x195, .dco_fraction = 0x0000,		/* [6]: 6.48 */
-	  .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [7]: 8.1 */
-	  .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
+static const struct icl_combo_pll_params icl_dp_combo_pll_24MHz_values[] = {
+	{ 540000,
+	  { .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [0]: 5.4 */
+	    .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 270000,
+	  { .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [1]: 2.7 */
+	    .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 162000,
+	  { .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [2]: 1.62 */
+	    .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 324000,
+	  { .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [3]: 3.24 */
+	    .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 216000,
+	  { .dco_integer = 0x168, .dco_fraction = 0x0000,		/* [4]: 2.16 */
+	    .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
+	{ 432000,
+	  { .dco_integer = 0x168, .dco_fraction = 0x0000,		/* [5]: 4.32 */
+	    .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 648000,
+	  { .dco_integer = 0x195, .dco_fraction = 0x0000,		/* [6]: 6.48 */
+	    .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 810000,
+	  { .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [7]: 8.1 */
+	    .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
 };
 
+
 /* Also used for 38.4 MHz values. */
-static const struct skl_wrpll_params icl_dp_combo_pll_19_2MHz_values[] = {
-	{ .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [0]: 5.4 */
-	  .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [1]: 2.7 */
-	  .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [2]: 1.62 */
-	  .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [3]: 3.24 */
-	  .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x1C2, .dco_fraction = 0x0000,		/* [4]: 2.16 */
-	  .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2},
-	{ .dco_integer = 0x1C2, .dco_fraction = 0x0000,		/* [5]: 4.32 */
-	  .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x1FA, .dco_fraction = 0x2000,		/* [6]: 6.48 */
-	  .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [7]: 8.1 */
-	  .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
+static const struct icl_combo_pll_params icl_dp_combo_pll_19_2MHz_values[] = {
+	{ 540000,
+	  { .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [0]: 5.4 */
+	    .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 270000,
+	  { .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [1]: 2.7 */
+	    .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 162000,
+	  { .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [2]: 1.62 */
+	    .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 324000,
+	  { .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [3]: 3.24 */
+	    .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 216000,
+	  { .dco_integer = 0x1C2, .dco_fraction = 0x0000,		/* [4]: 2.16 */
+	    .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
+	{ 432000,
+	  { .dco_integer = 0x1C2, .dco_fraction = 0x0000,		/* [5]: 4.32 */
+	    .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 648000,
+	  { .dco_integer = 0x1FA, .dco_fraction = 0x2000,		/* [6]: 6.48 */
+	    .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 810000,
+	  { .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [7]: 8.1 */
+	    .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
 };
 
 static const struct skl_wrpll_params icl_tbt_pll_24MHz_values = {
@@ -2474,44 +2496,22 @@ static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
 				  struct skl_wrpll_params *pll_params)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
-	const struct skl_wrpll_params *params;
+	const struct icl_combo_pll_params *params =
+		dev_priv->cdclk.hw.ref == 24000 ?
+		icl_dp_combo_pll_24MHz_values :
+		icl_dp_combo_pll_19_2MHz_values;
 	int clock = crtc_state->port_clock;
+	int i;
 
-	params = dev_priv->cdclk.hw.ref == 24000 ?
-			icl_dp_combo_pll_24MHz_values :
-			icl_dp_combo_pll_19_2MHz_values;
-
-	switch (clock) {
-	case 540000:
-		*pll_params = params[0];
-		break;
-	case 270000:
-		*pll_params = params[1];
-		break;
-	case 162000:
-		*pll_params = params[2];
-		break;
-	case 324000:
-		*pll_params = params[3];
-		break;
-	case 216000:
-		*pll_params = params[4];
-		break;
-	case 432000:
-		*pll_params = params[5];
-		break;
-	case 648000:
-		*pll_params = params[6];
-		break;
-	case 810000:
-		*pll_params = params[7];
-		break;
-	default:
-		MISSING_CASE(clock);
-		return false;
+	for (i = 0; i < ARRAY_SIZE(icl_dp_combo_pll_24MHz_values); i++) {
+		if (clock == params[i].clock) {
+			*pll_params = params[i].wrpll;
+			return true;
+		}
 	}
 
-	return true;
+	MISSING_CASE(clock);
+	return false;
 }
 
 static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll()
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (11 preceding siblings ...)
  2019-02-07 17:32 ` [PATCH 13/13] drm/i915: Remove the fragile array index -> link rate mapping Ville Syrjala
@ 2019-02-07 18:01 ` Patchwork
  2019-02-07 18:19 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-02-07 18:01 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll()
URL   : https://patchwork.freedesktop.org/series/56354/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
4a516bd3c8c7 drm/i915: Don't pass crtc to intel_find_shared_dpll()
bf85a4dc6c43 drm/i915: Don't pass crtc to intel_get_shared_dpll() and .get_dpll()
a6bc002df368 drm/i915: Pass crtc_state down to skl dpll funcs
11ad00fc5691 drm/i915: Remove redundant on stack dpll_hw_state from skl_get_dpll()
f0140456a110 drm/i915: Pass crtc_state down to bxt dpll funcs
-:153: CHECK:SPACING: No space is necessary after a cast
#153: FILE: drivers/gpu/drm/i915/intel_dpll_mgr.c:1852:
+	id = (enum intel_dpll_id) encoder->port;

total: 0 errors, 0 warnings, 1 checks, 136 lines checked
366662df0b7f drm/i915: Remove redundant on stack dpll_hw_state from bxt_get_dpll()
befe48f125b5 drm/i915: Pass crtc_state down to cnl dpll funcs
b78d192ed8bd drm/i915: Remove redundant on stack dpll_hw_state from cnl_get_dpll()
fd96d0dd0da2 drm/i915: Pass crtc_state down to icl dpll funcs
0dd1a1e3a232 drm/i915: Remove redundant on stack dpll_hw_state from icl_get_dpll()
502bd2131842 drm/i915: Fix readout for cnl DPLL kdiv==3
bdf9bfde9ad8 drm/i915: Nuke icl_calc_dp_combo_pll_link()
afee63b9e324 drm/i915: Remove the fragile array index -> link rate mapping
-:76: CHECK:LINE_SPACING: Please don't use multiple blank lines
#76: FILE: drivers/gpu/drm/i915/intel_dpll_mgr.c:2456:
 
+

total: 0 errors, 0 warnings, 1 checks, 159 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll()
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (12 preceding siblings ...)
  2019-02-07 18:01 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Patchwork
@ 2019-02-07 18:19 ` Patchwork
  2019-02-07 20:47 ` ✓ Fi.CI.IGT: " Patchwork
  2019-03-02  0:28 ` [PATCH 01/13] " Lucas De Marchi
  15 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-02-07 18:19 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll()
URL   : https://patchwork.freedesktop.org/series/56354/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5561 -> Patchwork_12169
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56354/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12169:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@pm_rpm@module-reload:
    - {fi-icl-y}:         PASS -> INCOMPLETE

  
Known issues
------------

  Here are the changes found in Patchwork_12169 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@pm_rpm@basic-rte:
    - fi-bsw-kefka:       PASS -> FAIL [fdo#108800]

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         PASS -> DMESG-FAIL [fdo#103182]

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     FAIL [fdo#103167] -> PASS

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         FAIL [fdo#104008] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104008]: https://bugs.freedesktop.org/show_bug.cgi?id=104008
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (49 -> 45)
------------------------------

  Missing    (4): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


Build changes
-------------

    * Linux: CI_DRM_5561 -> Patchwork_12169

  CI_DRM_5561: 07e241128d99c103fa40a50d24128fcdcb3cf223 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4813: 09f506726d0e115ee7f4a1604ae71adcf9f12690 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12169: afee63b9e324dca1ff898c12684878144fc8afd5 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

afee63b9e324 drm/i915: Remove the fragile array index -> link rate mapping
bdf9bfde9ad8 drm/i915: Nuke icl_calc_dp_combo_pll_link()
502bd2131842 drm/i915: Fix readout for cnl DPLL kdiv==3
0dd1a1e3a232 drm/i915: Remove redundant on stack dpll_hw_state from icl_get_dpll()
fd96d0dd0da2 drm/i915: Pass crtc_state down to icl dpll funcs
b78d192ed8bd drm/i915: Remove redundant on stack dpll_hw_state from cnl_get_dpll()
befe48f125b5 drm/i915: Pass crtc_state down to cnl dpll funcs
366662df0b7f drm/i915: Remove redundant on stack dpll_hw_state from bxt_get_dpll()
f0140456a110 drm/i915: Pass crtc_state down to bxt dpll funcs
11ad00fc5691 drm/i915: Remove redundant on stack dpll_hw_state from skl_get_dpll()
a6bc002df368 drm/i915: Pass crtc_state down to skl dpll funcs
bf85a4dc6c43 drm/i915: Don't pass crtc to intel_get_shared_dpll() and .get_dpll()
4a516bd3c8c7 drm/i915: Don't pass crtc to intel_find_shared_dpll()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12169/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll()
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (13 preceding siblings ...)
  2019-02-07 18:19 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-02-07 20:47 ` Patchwork
  2019-03-02  0:28 ` [PATCH 01/13] " Lucas De Marchi
  15 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-02-07 20:47 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll()
URL   : https://patchwork.freedesktop.org/series/56354/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5561_full -> Patchwork_12169_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12169_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-apl:          PASS -> FAIL [fdo#106641]

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-apl:          PASS -> FAIL [fdo#106510] / [fdo#108145]

  * igt@kms_color@pipe-b-degamma:
    - shard-apl:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-64x21-onscreen:
    - shard-glk:          PASS -> FAIL [fdo#103232]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-glk:          PASS -> FAIL [fdo#103167] +4

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1

  
#### Possible fixes ####

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-glk:          FAIL [fdo#106641] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-apl:          FAIL [fdo#109350] -> PASS

  * igt@kms_flip@modeset-vs-vblank-race-interruptible:
    - shard-apl:          FAIL [fdo#103060] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-glk:          FAIL [fdo#103167] -> PASS

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-glk:          FAIL [fdo#103166] -> PASS

  * igt@kms_setmode@basic:
    - shard-hsw:          FAIL [fdo#99912] -> PASS
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  * igt@perf@short-reads:
    - shard-kbl:          FAIL [fdo#103183] -> PASS

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103183]: https://bugs.freedesktop.org/show_bug.cgi?id=103183
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#106510]: https://bugs.freedesktop.org/show_bug.cgi?id=106510
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * Linux: CI_DRM_5561 -> Patchwork_12169

  CI_DRM_5561: 07e241128d99c103fa40a50d24128fcdcb3cf223 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4813: 09f506726d0e115ee7f4a1604ae71adcf9f12690 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12169: afee63b9e324dca1ff898c12684878144fc8afd5 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12169/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 11/13] drm/i915: Fix readout for cnl DPLL kdiv==3
  2019-02-07 17:32 ` [PATCH 11/13] drm/i915: Fix readout for cnl DPLL kdiv==3 Ville Syrjala
@ 2019-02-07 22:48   ` Lucas De Marchi
  0 siblings, 0 replies; 20+ messages in thread
From: Lucas De Marchi @ 2019-02-07 22:48 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: Intel Graphics

On Thu, Feb 7, 2019 at 9:33 AM Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The readout code thinks that kdiv of 3 is 4. Fix it.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 2 +-
>  drivers/gpu/drm/i915/intel_ddi.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 638a586469f9..f8c15148927a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9741,7 +9741,7 @@ enum skl_power_gate {
>  #define  DPLL_CFGCR1_KDIV(x)           ((x) << 6)
>  #define  DPLL_CFGCR1_KDIV_1            (1 << 6)
>  #define  DPLL_CFGCR1_KDIV_2            (2 << 6)
> -#define  DPLL_CFGCR1_KDIV_4            (4 << 6)
> +#define  DPLL_CFGCR1_KDIV_3            (4 << 6)
>  #define  DPLL_CFGCR1_PDIV_MASK         (0xf << 2)
>  #define  DPLL_CFGCR1_PDIV_SHIFT                (2)
>  #define  DPLL_CFGCR1_PDIV(x)           ((x) << 2)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index ca705546a0ab..fd192baf93b2 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1349,8 +1349,8 @@ int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
>         case DPLL_CFGCR1_KDIV_2:
>                 p2 = 2;
>                 break;
> -       case DPLL_CFGCR1_KDIV_4:
> -               p2 = 4;
> +       case DPLL_CFGCR1_KDIV_3:
> +               p2 = 3;

confusing in the spec because kdiv 3 has the value 4. And no idea why
we call it p2
rather than kdiv, that IMO whould make more sense.

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

>                 break;
>         }
>
> --
> 2.19.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 13/13] drm/i915: Remove the fragile array index -> link rate mapping
  2019-02-07 17:32 ` [PATCH 13/13] drm/i915: Remove the fragile array index -> link rate mapping Ville Syrjala
@ 2019-02-07 23:11   ` Lucas De Marchi
  2019-02-08 12:24     ` Ville Syrjälä
  0 siblings, 1 reply; 20+ messages in thread
From: Lucas De Marchi @ 2019-02-07 23:11 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: Intel Graphics

On Thu, Feb 7, 2019 at 9:33 AM Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rather than try to maintain some magic relationship between the link
> rates and the index into the wrpll params array let's just store
> the link rate in the array itself. Much less fragile.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 136 +++++++++++++-------------
>  1 file changed, 68 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 78dd28aa123c..9aacd19aede1 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2417,47 +2417,69 @@ static const struct intel_dpll_mgr cnl_pll_mgr = {
>         .dump_hw_state = cnl_dump_hw_state,
>  };
>
> +struct icl_combo_pll_params {
> +       int clock;

isn't clock confusing here since we have the reference clock and here
is the link rate?

> +       struct skl_wrpll_params wrpll;

humn... since we will look for the clock to lookup the entry, we could
go one step further and
join the 2 tables:

struct icl_combo_pll_params {
       int clock;
       struct skl_wrpll_params wrpll[2];
};

> +};
> +
>  /*
>   * These values alrea already adjusted: they're the bits we write to the
>   * registers, not the logical values.
>   */
> -static const struct skl_wrpll_params icl_dp_combo_pll_24MHz_values[] = {
> -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [0]: 5.4 */
> -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [1]: 2.7 */
> -         .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [2]: 1.62 */
> -         .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [3]: 3.24 */
> -         .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x168, .dco_fraction = 0x0000,         /* [4]: 2.16 */
> -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2},
> -       { .dco_integer = 0x168, .dco_fraction = 0x0000,         /* [5]: 4.32 */
> -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x195, .dco_fraction = 0x0000,         /* [6]: 6.48 */
> -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [7]: 8.1 */
> -         .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> +static const struct icl_combo_pll_params icl_dp_combo_pll_24MHz_values[] = {
> +       { 540000,
> +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [0]: 5.4 */
> +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },

I'm almost always for designated initializer, but in this table that
comes directly from spec, wouldn't
be easier  to read by skipping the initializer and just aligning the
numbers? IMO the lines below seem
easier to read:

 static const struct skl_wrpll_params icl_dp_combo_pll_24MHz_values[] = {
         /* clock, dco_int,  dco_frac, kdiv, qdiv_mode, qdiv_ratio */
         { 5400,  {  0x151,    0x4000,    1,        0,          0 } },
...
}

Lucas De Marchi

> +       { 270000,
> +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [1]: 2.7 */
> +           .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 162000,
> +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [2]: 1.62 */
> +           .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 324000,
> +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [3]: 3.24 */
> +           .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 216000,
> +         { .dco_integer = 0x168, .dco_fraction = 0x0000,               /* [4]: 2.16 */
> +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
> +       { 432000,
> +         { .dco_integer = 0x168, .dco_fraction = 0x0000,               /* [5]: 4.32 */
> +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 648000,
> +         { .dco_integer = 0x195, .dco_fraction = 0x0000,               /* [6]: 6.48 */
> +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 810000,
> +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [7]: 8.1 */
> +           .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
>  };
>
> +
>  /* Also used for 38.4 MHz values. */
> -static const struct skl_wrpll_params icl_dp_combo_pll_19_2MHz_values[] = {
> -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [0]: 5.4 */
> -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [1]: 2.7 */
> -         .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [2]: 1.62 */
> -         .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [3]: 3.24 */
> -         .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x1C2, .dco_fraction = 0x0000,         /* [4]: 2.16 */
> -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2},
> -       { .dco_integer = 0x1C2, .dco_fraction = 0x0000,         /* [5]: 4.32 */
> -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x1FA, .dco_fraction = 0x2000,         /* [6]: 6.48 */
> -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [7]: 8.1 */
> -         .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> +static const struct icl_combo_pll_params icl_dp_combo_pll_19_2MHz_values[] = {
> +       { 540000,
> +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [0]: 5.4 */
> +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 270000,
> +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [1]: 2.7 */
> +           .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 162000,
> +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [2]: 1.62 */
> +           .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 324000,
> +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [3]: 3.24 */
> +           .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 216000,
> +         { .dco_integer = 0x1C2, .dco_fraction = 0x0000,               /* [4]: 2.16 */
> +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
> +       { 432000,
> +         { .dco_integer = 0x1C2, .dco_fraction = 0x0000,               /* [5]: 4.32 */
> +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 648000,
> +         { .dco_integer = 0x1FA, .dco_fraction = 0x2000,               /* [6]: 6.48 */
> +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 810000,
> +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [7]: 8.1 */
> +           .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
>  };
>
>  static const struct skl_wrpll_params icl_tbt_pll_24MHz_values = {
> @@ -2474,44 +2496,22 @@ static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
>                                   struct skl_wrpll_params *pll_params)
>  {
>         struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> -       const struct skl_wrpll_params *params;
> +       const struct icl_combo_pll_params *params =
> +               dev_priv->cdclk.hw.ref == 24000 ?
> +               icl_dp_combo_pll_24MHz_values :
> +               icl_dp_combo_pll_19_2MHz_values;
>         int clock = crtc_state->port_clock;
> +       int i;
>
> -       params = dev_priv->cdclk.hw.ref == 24000 ?
> -                       icl_dp_combo_pll_24MHz_values :
> -                       icl_dp_combo_pll_19_2MHz_values;
> -
> -       switch (clock) {
> -       case 540000:
> -               *pll_params = params[0];
> -               break;
> -       case 270000:
> -               *pll_params = params[1];
> -               break;
> -       case 162000:
> -               *pll_params = params[2];
> -               break;
> -       case 324000:
> -               *pll_params = params[3];
> -               break;
> -       case 216000:
> -               *pll_params = params[4];
> -               break;
> -       case 432000:
> -               *pll_params = params[5];
> -               break;
> -       case 648000:
> -               *pll_params = params[6];
> -               break;
> -       case 810000:
> -               *pll_params = params[7];
> -               break;
> -       default:
> -               MISSING_CASE(clock);
> -               return false;
> +       for (i = 0; i < ARRAY_SIZE(icl_dp_combo_pll_24MHz_values); i++) {
> +               if (clock == params[i].clock) {
> +                       *pll_params = params[i].wrpll;
> +                       return true;
> +               }
>         }
>
> -       return true;
> +       MISSING_CASE(clock);
> +       return false;
>  }
>
>  static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
> --
> 2.19.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 13/13] drm/i915: Remove the fragile array index -> link rate mapping
  2019-02-07 23:11   ` Lucas De Marchi
@ 2019-02-08 12:24     ` Ville Syrjälä
  0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2019-02-08 12:24 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: Intel Graphics

On Thu, Feb 07, 2019 at 03:11:59PM -0800, Lucas De Marchi wrote:
> On Thu, Feb 7, 2019 at 9:33 AM Ville Syrjala
> <ville.syrjala@linux.intel.com> wrote:
> >
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Rather than try to maintain some magic relationship between the link
> > rates and the index into the wrpll params array let's just store
> > the link rate in the array itself. Much less fragile.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 136 +++++++++++++-------------
> >  1 file changed, 68 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > index 78dd28aa123c..9aacd19aede1 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > @@ -2417,47 +2417,69 @@ static const struct intel_dpll_mgr cnl_pll_mgr = {
> >         .dump_hw_state = cnl_dump_hw_state,
> >  };
> >
> > +struct icl_combo_pll_params {
> > +       int clock;
> 
> isn't clock confusing here since we have the reference clock and here
> is the link rate?

This is PLL code. There is no such thing as link rate in a PLL.

> 
> > +       struct skl_wrpll_params wrpll;
> 
> humn... since we will look for the clock to lookup the entry, we could
> go one step further and
> join the 2 tables:
> 
> struct icl_combo_pll_params {
>        int clock;
>        struct skl_wrpll_params wrpll[2];
> };

That would add another magic array index. Which wouldn't be consistent
with the tbt pll params. We'd have to add some kind of enum for the
index and the convert all of these. I certainly don't care enough to
do that myself.

> 
> > +};
> > +
> >  /*
> >   * These values alrea already adjusted: they're the bits we write to the
> >   * registers, not the logical values.
> >   */
> > -static const struct skl_wrpll_params icl_dp_combo_pll_24MHz_values[] = {
> > -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [0]: 5.4 */
> > -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [1]: 2.7 */
> > -         .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [2]: 1.62 */
> > -         .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [3]: 3.24 */
> > -         .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x168, .dco_fraction = 0x0000,         /* [4]: 2.16 */
> > -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2},
> > -       { .dco_integer = 0x168, .dco_fraction = 0x0000,         /* [5]: 4.32 */
> > -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x195, .dco_fraction = 0x0000,         /* [6]: 6.48 */
> > -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [7]: 8.1 */
> > -         .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > +static const struct icl_combo_pll_params icl_dp_combo_pll_24MHz_values[] = {
> > +       { 540000,
> > +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [0]: 5.4 */
> > +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> 
> I'm almost always for designated initializer, but in this table that
> comes directly from spec, wouldn't
> be easier  to read by skipping the initializer and just aligning the
> numbers? IMO the lines below seem
> easier to read:
> 
>  static const struct skl_wrpll_params icl_dp_combo_pll_24MHz_values[] = {
>          /* clock, dco_int,  dco_frac, kdiv, qdiv_mode, qdiv_ratio */
>          { 5400,  {  0x151,    0x4000,    1,        0,          0 } },
> ...
> }

With the comment it might be OK. But it'll certainly cause cscope to
miss stuff if you look for eg. qdiv_mode by name. So not sure if it's
better or worse tbh.

> 
> Lucas De Marchi
> 
> > +       { 270000,
> > +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [1]: 2.7 */
> > +           .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 162000,
> > +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [2]: 1.62 */
> > +           .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 324000,
> > +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [3]: 3.24 */
> > +           .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 216000,
> > +         { .dco_integer = 0x168, .dco_fraction = 0x0000,               /* [4]: 2.16 */
> > +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
> > +       { 432000,
> > +         { .dco_integer = 0x168, .dco_fraction = 0x0000,               /* [5]: 4.32 */
> > +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 648000,
> > +         { .dco_integer = 0x195, .dco_fraction = 0x0000,               /* [6]: 6.48 */
> > +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 810000,
> > +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [7]: 8.1 */
> > +           .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> >  };
> >
> > +
> >  /* Also used for 38.4 MHz values. */
> > -static const struct skl_wrpll_params icl_dp_combo_pll_19_2MHz_values[] = {
> > -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [0]: 5.4 */
> > -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [1]: 2.7 */
> > -         .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [2]: 1.62 */
> > -         .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [3]: 3.24 */
> > -         .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x1C2, .dco_fraction = 0x0000,         /* [4]: 2.16 */
> > -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2},
> > -       { .dco_integer = 0x1C2, .dco_fraction = 0x0000,         /* [5]: 4.32 */
> > -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x1FA, .dco_fraction = 0x2000,         /* [6]: 6.48 */
> > -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [7]: 8.1 */
> > -         .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > +static const struct icl_combo_pll_params icl_dp_combo_pll_19_2MHz_values[] = {
> > +       { 540000,
> > +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [0]: 5.4 */
> > +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 270000,
> > +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [1]: 2.7 */
> > +           .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 162000,
> > +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [2]: 1.62 */
> > +           .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 324000,
> > +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [3]: 3.24 */
> > +           .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 216000,
> > +         { .dco_integer = 0x1C2, .dco_fraction = 0x0000,               /* [4]: 2.16 */
> > +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
> > +       { 432000,
> > +         { .dco_integer = 0x1C2, .dco_fraction = 0x0000,               /* [5]: 4.32 */
> > +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 648000,
> > +         { .dco_integer = 0x1FA, .dco_fraction = 0x2000,               /* [6]: 6.48 */
> > +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 810000,
> > +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [7]: 8.1 */
> > +           .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> >  };
> >
> >  static const struct skl_wrpll_params icl_tbt_pll_24MHz_values = {
> > @@ -2474,44 +2496,22 @@ static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
> >                                   struct skl_wrpll_params *pll_params)
> >  {
> >         struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > -       const struct skl_wrpll_params *params;
> > +       const struct icl_combo_pll_params *params =
> > +               dev_priv->cdclk.hw.ref == 24000 ?
> > +               icl_dp_combo_pll_24MHz_values :
> > +               icl_dp_combo_pll_19_2MHz_values;
> >         int clock = crtc_state->port_clock;
> > +       int i;
> >
> > -       params = dev_priv->cdclk.hw.ref == 24000 ?
> > -                       icl_dp_combo_pll_24MHz_values :
> > -                       icl_dp_combo_pll_19_2MHz_values;
> > -
> > -       switch (clock) {
> > -       case 540000:
> > -               *pll_params = params[0];
> > -               break;
> > -       case 270000:
> > -               *pll_params = params[1];
> > -               break;
> > -       case 162000:
> > -               *pll_params = params[2];
> > -               break;
> > -       case 324000:
> > -               *pll_params = params[3];
> > -               break;
> > -       case 216000:
> > -               *pll_params = params[4];
> > -               break;
> > -       case 432000:
> > -               *pll_params = params[5];
> > -               break;
> > -       case 648000:
> > -               *pll_params = params[6];
> > -               break;
> > -       case 810000:
> > -               *pll_params = params[7];
> > -               break;
> > -       default:
> > -               MISSING_CASE(clock);
> > -               return false;
> > +       for (i = 0; i < ARRAY_SIZE(icl_dp_combo_pll_24MHz_values); i++) {
> > +               if (clock == params[i].clock) {
> > +                       *pll_params = params[i].wrpll;
> > +                       return true;
> > +               }
> >         }
> >
> > -       return true;
> > +       MISSING_CASE(clock);
> > +       return false;
> >  }
> >
> >  static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
> > --
> > 2.19.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Lucas De Marchi

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll()
  2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
                   ` (14 preceding siblings ...)
  2019-02-07 20:47 ` ✓ Fi.CI.IGT: " Patchwork
@ 2019-03-02  0:28 ` Lucas De Marchi
  15 siblings, 0 replies; 20+ messages in thread
From: Lucas De Marchi @ 2019-03-02  0:28 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: Intel Graphics

On Thu, Feb 7, 2019 at 9:32 AM Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Passing both crtc and its state is redundant. Pass just the state.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

for the series.

Lucas De Marchi

> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 0a42d11c4c33..1d1a2c456257 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -241,11 +241,11 @@ void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
>  }
>
>  static struct intel_shared_dpll *
> -intel_find_shared_dpll(struct intel_crtc *crtc,
> -                      struct intel_crtc_state *crtc_state,
> +intel_find_shared_dpll(struct intel_crtc_state *crtc_state,
>                        enum intel_dpll_id range_min,
>                        enum intel_dpll_id range_max)
>  {
> +       struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>         struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>         struct intel_shared_dpll *pll, *unused_pll = NULL;
>         struct intel_shared_dpll_state *shared_dpll;
> @@ -436,7 +436,7 @@ ibx_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>                               crtc->base.base.id, crtc->base.name,
>                               pll->info->name);
>         } else {
> -               pll = intel_find_shared_dpll(crtc, crtc_state,
> +               pll = intel_find_shared_dpll(crtc_state,
>                                              DPLL_ID_PCH_PLL_A,
>                                              DPLL_ID_PCH_PLL_B);
>         }
> @@ -780,7 +780,7 @@ static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(int clock,
>
>         crtc_state->dpll_hw_state.wrpll = val;
>
> -       pll = intel_find_shared_dpll(crtc, crtc_state,
> +       pll = intel_find_shared_dpll(crtc_state,
>                                      DPLL_ID_WRPLL1, DPLL_ID_WRPLL2);
>
>         if (!pll)
> @@ -840,7 +840,7 @@ hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>                 crtc_state->dpll_hw_state.spll =
>                         SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC;
>
> -               pll = intel_find_shared_dpll(crtc, crtc_state,
> +               pll = intel_find_shared_dpll(crtc_state,
>                                              DPLL_ID_SPLL, DPLL_ID_SPLL);
>         } else {
>                 return NULL;
> @@ -1411,11 +1411,11 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>         }
>
>         if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
> -               pll = intel_find_shared_dpll(crtc, crtc_state,
> +               pll = intel_find_shared_dpll(crtc_state,
>                                              DPLL_ID_SKL_DPLL0,
>                                              DPLL_ID_SKL_DPLL0);
>         else
> -               pll = intel_find_shared_dpll(crtc, crtc_state,
> +               pll = intel_find_shared_dpll(crtc_state,
>                                              DPLL_ID_SKL_DPLL1,
>                                              DPLL_ID_SKL_DPLL3);
>         if (!pll)
> @@ -2390,7 +2390,7 @@ cnl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>                 return NULL;
>         }
>
> -       pll = intel_find_shared_dpll(crtc, crtc_state,
> +       pll = intel_find_shared_dpll(crtc_state,
>                                      DPLL_ID_SKL_DPLL0,
>                                      DPLL_ID_SKL_DPLL2);
>         if (!pll) {
> @@ -2945,7 +2945,7 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>
>         crtc_state->dpll_hw_state = pll_state;
>
> -       pll = intel_find_shared_dpll(crtc, crtc_state, min, max);
> +       pll = intel_find_shared_dpll(crtc_state, min, max);
>         if (!pll) {
>                 DRM_DEBUG_KMS("No PLL selected\n");
>                 return NULL;
> --
> 2.19.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-03-02  0:28 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-07 17:32 [PATCH 01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Ville Syrjala
2019-02-07 17:32 ` [PATCH 02/13] drm/i915: Don't pass crtc to intel_get_shared_dpll() and .get_dpll() Ville Syrjala
2019-02-07 17:32 ` [PATCH 03/13] drm/i915: Pass crtc_state down to skl dpll funcs Ville Syrjala
2019-02-07 17:32 ` [PATCH 04/13] drm/i915: Remove redundant on stack dpll_hw_state from skl_get_dpll() Ville Syrjala
2019-02-07 17:32 ` [PATCH 05/13] drm/i915: Pass crtc_state down to bxt dpll funcs Ville Syrjala
2019-02-07 17:32 ` [PATCH 06/13] drm/i915: Remove redundant on stack dpll_hw_state from bxt_get_dpll() Ville Syrjala
2019-02-07 17:32 ` [PATCH 07/13] drm/i915: Pass crtc_state down to cnl dpll funcs Ville Syrjala
2019-02-07 17:32 ` [PATCH 08/13] drm/i915: Remove redundant on stack dpll_hw_state from cnl_get_dpll() Ville Syrjala
2019-02-07 17:32 ` [PATCH 09/13] drm/i915: Pass crtc_state down to icl dpll funcs Ville Syrjala
2019-02-07 17:32 ` [PATCH 10/13] drm/i915: Remove redundant on stack dpll_hw_state from icl_get_dpll() Ville Syrjala
2019-02-07 17:32 ` [PATCH 11/13] drm/i915: Fix readout for cnl DPLL kdiv==3 Ville Syrjala
2019-02-07 22:48   ` Lucas De Marchi
2019-02-07 17:32 ` [PATCH 12/13] drm/i915: Nuke icl_calc_dp_combo_pll_link() Ville Syrjala
2019-02-07 17:32 ` [PATCH 13/13] drm/i915: Remove the fragile array index -> link rate mapping Ville Syrjala
2019-02-07 23:11   ` Lucas De Marchi
2019-02-08 12:24     ` Ville Syrjälä
2019-02-07 18:01 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() Patchwork
2019-02-07 18:19 ` ✓ Fi.CI.BAT: success " Patchwork
2019-02-07 20:47 ` ✓ Fi.CI.IGT: " Patchwork
2019-03-02  0:28 ` [PATCH 01/13] " Lucas De Marchi

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.