[PATCH 0/5] drm/i915/icl: split pll functions

[PATCH 0/5] drm/i915/icl: split pll functions

[Lucas De Marchi]

This should achieve a similar goal as in
https://patchwork.freedesktop.org/patch/287567/?series=57116&rev=1,
decoupling the PLL ids from the type.

I actually prefer the other implementation in which we can use the
pll_info struct to store the info about the pll rather than spreading
that across several functions. It's not a strong preference though.

Anyway, this should be sufficient to allow future platforms to share
common functions.

Warning: this is only build-tested... I want to confirm if this is
the way to go.

Lucas De Marchi (5):
  drm/i915/icl: split combo and mg pll enable
  drm/i915/icl: use a function pointer for pll_write when enabling
  drm/i915/icl: split combo and mg pll disable
  drm/i915/icl: split combo and tbt pll funcs
  drm/i915/icl: remove intel_dpll_is_combophy()

 drivers/gpu/drm/i915/intel_display.c  |   3 -
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 127 ++++++++++++++++++--------
 2 files changed, 87 insertions(+), 43 deletions(-)

-- 
2.20.1

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

[PATCH 1/5] drm/i915/icl: split combo and mg pll enable

[Lucas De Marchi]

Let's start using the vfuncs to differentiate MG and Combo PLLs. The end
goal is to decouple the type of the PLL from the IDs since the latter
are likely to change from one platform to another. This also makes the
code easier to read by not having lots of if/else chains on leaf
functions.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index e4ec73d415d9..3b3de99756d6 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -3117,10 +3117,10 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv,
 }
 
 static void icl_pll_enable(struct drm_i915_private *dev_priv,
-			   struct intel_shared_dpll *pll)
+			   struct intel_shared_dpll *pll,
+			   i915_reg_t enable_reg)
 {
 	const enum intel_dpll_id id = pll->info->id;
-	i915_reg_t enable_reg = icl_pll_id_to_enable_reg(id);
 	u32 val;
 
 	val = I915_READ(enable_reg);
@@ -3157,6 +3157,23 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
 	/* DVFS post sequence would be here. See the comment above. */
 }
 
+static void combo_pll_enable(struct drm_i915_private *dev_priv,
+			     struct intel_shared_dpll *pll)
+{
+	i915_reg_t enable_reg = icl_pll_id_to_enable_reg(pll->info->id);
+
+	icl_pll_enable(dev_priv, pll, enable_reg);
+}
+
+static void mg_pll_enable(struct drm_i915_private *dev_priv,
+			  struct intel_shared_dpll *pll)
+{
+	i915_reg_t enable_reg =
+		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
+
+	icl_pll_enable(dev_priv, pll, enable_reg);
+}
+
 static void icl_pll_disable(struct drm_i915_private *dev_priv,
 			    struct intel_shared_dpll *pll)
 {
@@ -3218,13 +3235,13 @@ static void icl_dump_hw_state(struct drm_i915_private *dev_priv,
 }
 
 static const struct intel_shared_dpll_funcs icl_pll_funcs = {
-	.enable = icl_pll_enable,
+	.enable = combo_pll_enable,
 	.disable = icl_pll_disable,
 	.get_hw_state = icl_pll_get_hw_state,
 };
 
 static const struct intel_shared_dpll_funcs mg_pll_funcs = {
-	.enable = icl_pll_enable,
+	.enable = mg_pll_enable,
 	.disable = icl_pll_disable,
 	.get_hw_state = mg_pll_get_hw_state,
 };
-- 
2.20.1

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

[PATCH 2/5] drm/i915/icl: use a function pointer for pll_write when enabling

[Lucas De Marchi]

This allows us to share the icl_pll_enable() between the different types
of PLL while allowing the caller to differentiate how to write the
registers.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 3b3de99756d6..5511bc23ea3d 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -3118,9 +3118,10 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv,
 
 static void icl_pll_enable(struct drm_i915_private *dev_priv,
 			   struct intel_shared_dpll *pll,
-			   i915_reg_t enable_reg)
+			   i915_reg_t enable_reg,
+			   void (*pll_write)(struct drm_i915_private *dev_priv,
+					     struct intel_shared_dpll *pll))
 {
-	const enum intel_dpll_id id = pll->info->id;
 	u32 val;
 
 	val = I915_READ(enable_reg);
@@ -3133,12 +3134,9 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
 	 */
 	if (intel_wait_for_register(dev_priv, enable_reg, PLL_POWER_STATE,
 				    PLL_POWER_STATE, 1))
-		DRM_ERROR("PLL %d Power not enabled\n", id);
+		DRM_ERROR("PLL %d Power not enabled\n", pll->info->id);
 
-	if (intel_dpll_is_combophy(id) || id == DPLL_ID_ICL_TBTPLL)
-		icl_dpll_write(dev_priv, pll);
-	else
-		icl_mg_pll_write(dev_priv, pll);
+	pll_write(dev_priv, pll);
 
 	/*
 	 * DVFS pre sequence would be here, but in our driver the cdclk code
@@ -3152,7 +3150,7 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
 
 	if (intel_wait_for_register(dev_priv, enable_reg, PLL_LOCK, PLL_LOCK,
 				    1)) /* 600us actually. */
-		DRM_ERROR("PLL %d not locked\n", id);
+		DRM_ERROR("PLL %d not locked\n", pll->info->id);
 
 	/* DVFS post sequence would be here. See the comment above. */
 }
@@ -3162,7 +3160,7 @@ static void combo_pll_enable(struct drm_i915_private *dev_priv,
 {
 	i915_reg_t enable_reg = icl_pll_id_to_enable_reg(pll->info->id);
 
-	icl_pll_enable(dev_priv, pll, enable_reg);
+	icl_pll_enable(dev_priv, pll, enable_reg, icl_dpll_write);
 }
 
 static void mg_pll_enable(struct drm_i915_private *dev_priv,
@@ -3171,7 +3169,7 @@ static void mg_pll_enable(struct drm_i915_private *dev_priv,
 	i915_reg_t enable_reg =
 		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
 
-	icl_pll_enable(dev_priv, pll, enable_reg);
+	icl_pll_enable(dev_priv, pll, enable_reg, icl_mg_pll_write);
 }
 
 static void icl_pll_disable(struct drm_i915_private *dev_priv,
-- 
2.20.1

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

[PATCH 3/5] drm/i915/icl: split combo and mg pll disable

[Lucas De Marchi]

Like was done in the enable case, split the implementation of the
disable for MG and Combo PLLs.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 30 ++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 5511bc23ea3d..d334dd9bcae6 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -3173,10 +3173,9 @@ static void mg_pll_enable(struct drm_i915_private *dev_priv,
 }
 
 static void icl_pll_disable(struct drm_i915_private *dev_priv,
-			    struct intel_shared_dpll *pll)
+			    struct intel_shared_dpll *pll,
+			    i915_reg_t enable_reg)
 {
-	const enum intel_dpll_id id = pll->info->id;
-	i915_reg_t enable_reg = icl_pll_id_to_enable_reg(id);
 	u32 val;
 
 	/* The first steps are done by intel_ddi_post_disable(). */
@@ -3193,7 +3192,7 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
 
 	/* Timeout is actually 1us. */
 	if (intel_wait_for_register(dev_priv, enable_reg, PLL_LOCK, 0, 1))
-		DRM_ERROR("PLL %d locked\n", id);
+		DRM_ERROR("PLL %d locked\n", pll->info->id);
 
 	/* DVFS post sequence would be here. See the comment above. */
 
@@ -3207,7 +3206,24 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
 	 */
 	if (intel_wait_for_register(dev_priv, enable_reg, PLL_POWER_STATE, 0,
 				    1))
-		DRM_ERROR("PLL %d Power not disabled\n", id);
+		DRM_ERROR("PLL %d Power not disabled\n", pll->info->id);
+}
+
+static void combo_pll_disable(struct drm_i915_private *dev_priv,
+			      struct intel_shared_dpll *pll)
+{
+	i915_reg_t enable_reg = icl_pll_id_to_enable_reg(pll->info->id);
+
+	icl_pll_disable(dev_priv, pll, enable_reg);
+}
+
+static void mg_pll_disable(struct drm_i915_private *dev_priv,
+			   struct intel_shared_dpll *pll)
+{
+	i915_reg_t enable_reg =
+		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
+
+	icl_pll_disable(dev_priv, pll, enable_reg);
 }
 
 static void icl_dump_hw_state(struct drm_i915_private *dev_priv,
@@ -3234,13 +3250,13 @@ static void icl_dump_hw_state(struct drm_i915_private *dev_priv,
 
 static const struct intel_shared_dpll_funcs icl_pll_funcs = {
 	.enable = combo_pll_enable,
-	.disable = icl_pll_disable,
+	.disable = combo_pll_disable,
 	.get_hw_state = icl_pll_get_hw_state,
 };
 
 static const struct intel_shared_dpll_funcs mg_pll_funcs = {
 	.enable = mg_pll_enable,
-	.disable = icl_pll_disable,
+	.disable = mg_pll_disable,
 	.get_hw_state = mg_pll_get_hw_state,
 };
 
-- 
2.20.1

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

[PATCH 4/5] drm/i915/icl: split combo and tbt pll funcs

[Lucas De Marchi]

Like was done for MG and combo, now finish the per-type split of the
vfunc by moving TBT out of the combo functions. Now we can completely
remove icl_pll_id_to_enable_reg() since each PLL type pass as all the
information via arguments.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 63 ++++++++++++++++++---------
 1 file changed, 42 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index d334dd9bcae6..d26e6f41cb30 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2956,16 +2956,6 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	return pll;
 }
 
-static i915_reg_t icl_pll_id_to_enable_reg(enum intel_dpll_id id)
-{
-	if (intel_dpll_is_combophy(id))
-		return CNL_DPLL_ENABLE(id);
-	else if (id == DPLL_ID_ICL_TBTPLL)
-		return TBT_PLL_ENABLE;
-
-	return MG_PLL_ENABLE(icl_pll_id_to_tc_port(id));
-}
-
 static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				struct intel_shared_dpll *pll,
 				struct intel_dpll_hw_state *hw_state)
@@ -3030,7 +3020,8 @@ static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
 
 static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				 struct intel_shared_dpll *pll,
-				 struct intel_dpll_hw_state *hw_state)
+				 struct intel_dpll_hw_state *hw_state,
+				 i915_reg_t enable_reg)
 {
 	const enum intel_dpll_id id = pll->info->id;
 	intel_wakeref_t wakeref;
@@ -3042,7 +3033,7 @@ static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv,
 	if (!wakeref)
 		return false;
 
-	val = I915_READ(icl_pll_id_to_enable_reg(id));
+	val = I915_READ(enable_reg);
 	if (!(val & PLL_ENABLE))
 		goto out;
 
@@ -3055,6 +3046,21 @@ static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv,
 	return ret;
 }
 
+static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
+				   struct intel_shared_dpll *pll,
+				   struct intel_dpll_hw_state *hw_state)
+{
+	return icl_pll_get_hw_state(dev_priv, pll, hw_state,
+				    CNL_DPLL_ENABLE(pll->info->id));
+}
+
+static bool tbt_pll_get_hw_state(struct drm_i915_private *dev_priv,
+				 struct intel_shared_dpll *pll,
+				 struct intel_dpll_hw_state *hw_state)
+{
+	return icl_pll_get_hw_state(dev_priv, pll, hw_state, TBT_PLL_ENABLE);
+}
+
 static void icl_dpll_write(struct drm_i915_private *dev_priv,
 			   struct intel_shared_dpll *pll)
 {
@@ -3158,9 +3164,14 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
 static void combo_pll_enable(struct drm_i915_private *dev_priv,
 			     struct intel_shared_dpll *pll)
 {
-	i915_reg_t enable_reg = icl_pll_id_to_enable_reg(pll->info->id);
+	icl_pll_enable(dev_priv, pll, CNL_DPLL_ENABLE(pll->info->id),
+		       icl_dpll_write);
+}
 
-	icl_pll_enable(dev_priv, pll, enable_reg, icl_dpll_write);
+static void tbt_pll_enable(struct drm_i915_private *dev_priv,
+			   struct intel_shared_dpll *pll)
+{
+	icl_pll_enable(dev_priv, pll, TBT_PLL_ENABLE, icl_dpll_write);
 }
 
 static void mg_pll_enable(struct drm_i915_private *dev_priv,
@@ -3212,9 +3223,13 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
 static void combo_pll_disable(struct drm_i915_private *dev_priv,
 			      struct intel_shared_dpll *pll)
 {
-	i915_reg_t enable_reg = icl_pll_id_to_enable_reg(pll->info->id);
+	icl_pll_disable(dev_priv, pll, CNL_DPLL_ENABLE(pll->info->id));
+}
 
-	icl_pll_disable(dev_priv, pll, enable_reg);
+static void tbt_pll_disable(struct drm_i915_private *dev_priv,
+			    struct intel_shared_dpll *pll)
+{
+	icl_pll_disable(dev_priv, pll, TBT_PLL_ENABLE);
 }
 
 static void mg_pll_disable(struct drm_i915_private *dev_priv,
@@ -3248,10 +3263,16 @@ static void icl_dump_hw_state(struct drm_i915_private *dev_priv,
 		      hw_state->mg_pll_tdc_coldst_bias);
 }
 
-static const struct intel_shared_dpll_funcs icl_pll_funcs = {
+static const struct intel_shared_dpll_funcs combo_pll_funcs = {
 	.enable = combo_pll_enable,
 	.disable = combo_pll_disable,
-	.get_hw_state = icl_pll_get_hw_state,
+	.get_hw_state = combo_pll_get_hw_state,
+};
+
+static const struct intel_shared_dpll_funcs tbt_pll_funcs = {
+	.enable = tbt_pll_enable,
+	.disable = tbt_pll_disable,
+	.get_hw_state = tbt_pll_get_hw_state,
 };
 
 static const struct intel_shared_dpll_funcs mg_pll_funcs = {
@@ -3261,9 +3282,9 @@ static const struct intel_shared_dpll_funcs mg_pll_funcs = {
 };
 
 static const struct dpll_info icl_plls[] = {
-	{ "DPLL 0",   &icl_pll_funcs, DPLL_ID_ICL_DPLL0,  0 },
-	{ "DPLL 1",   &icl_pll_funcs, DPLL_ID_ICL_DPLL1,  0 },
-	{ "TBT PLL",  &icl_pll_funcs, DPLL_ID_ICL_TBTPLL, 0 },
+	{ "DPLL 0",   &combo_pll_funcs, DPLL_ID_ICL_DPLL0,  0 },
+	{ "DPLL 1",   &combo_pll_funcs, DPLL_ID_ICL_DPLL1,  0 },
+	{ "TBT PLL",  &tbt_pll_funcs, DPLL_ID_ICL_TBTPLL, 0 },
 	{ "MG PLL 1", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1, 0 },
 	{ "MG PLL 2", &mg_pll_funcs, DPLL_ID_ICL_MGPLL2, 0 },
 	{ "MG PLL 3", &mg_pll_funcs, DPLL_ID_ICL_MGPLL3, 0 },
-- 
2.20.1

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

[PATCH 5/5] drm/i915/icl: remove intel_dpll_is_combophy()

[Lucas De Marchi]

This is only used in intel_display() and shouldn't be needed there.
We don't want to keep converting from pll id to pll type so just remove
the function.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c  | 3 ---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 5 -----
 2 files changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d852cb282060..c2558231dcb2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9592,9 +9592,6 @@ static void icelake_get_ddi_pll(struct drm_i915_private *dev_priv,
 		temp = I915_READ(DPCLKA_CFGCR0_ICL) &
 		       DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port);
 		id = temp >> DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port);
-
-		if (WARN_ON(!intel_dpll_is_combophy(id)))
-			return;
 	} else if (intel_port_is_tc(dev_priv, port)) {
 		id = icl_tc_port_to_pll_id(intel_port_to_tc(dev_priv, port));
 	} else {
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index d26e6f41cb30..5147e0cabb58 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2649,11 +2649,6 @@ enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port)
 	return tc_port + DPLL_ID_ICL_MGPLL1;
 }
 
-bool intel_dpll_is_combophy(enum intel_dpll_id id)
-{
-	return id == DPLL_ID_ICL_DPLL0 || id == DPLL_ID_ICL_DPLL1;
-}
-
 static bool icl_mg_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
 				     u32 *target_dco_khz,
 				     struct intel_dpll_hw_state *state)
-- 
2.20.1

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

✓ Fi.CI.BAT: success for drm/i915/icl: split pll functions

[Patchwork]

== Series Details ==

Series: drm/i915/icl: split pll functions
URL   : https://patchwork.freedesktop.org/series/57618/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5708 -> Patchwork_12386
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Suppressed ####

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

  * igt@gem_render_tiled_blits@basic:
    - {fi-icl-y}:         PASS -> INCOMPLETE

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#105602] / [fdo#108529] +1

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#108529]

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       PASS -> DMESG-FAIL [fdo#105079]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
    - fi-kbl-7567u:       PASS -> SKIP [fdo#109271] +33

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

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

  
#### Possible fixes ####

  * igt@gem_mmap@basic-small-bo:
    - {fi-icl-y}:         DMESG-WARN -> PASS

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-7560u:       CRASH -> PASS

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

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

  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (47 -> 42)
------------------------------

  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bdw-samus 


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

    * Linux: CI_DRM_5708 -> Patchwork_12386

  CI_DRM_5708: afd34c5dec857362de91fb3044f09d90e83ad6a5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4871: 8feb147562ba1b364615ddacd44c3846f0250d37 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12386: aa1bb01b628e15767e705bdaef5e14b3aa76f96e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

aa1bb01b628e drm/i915/icl: remove intel_dpll_is_combophy()
c990b01020e4 drm/i915/icl: split combo and tbt pll funcs
afabb4fade35 drm/i915/icl: split combo and mg pll disable
c94f26a3f8e1 drm/i915/icl: use a function pointer for pll_write when enabling
6052e88ffb68 drm/i915/icl: split combo and mg pll enable

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915/icl: split pll functions

[Patchwork]

== Series Details ==

Series: drm/i915/icl: split pll functions
URL   : https://patchwork.freedesktop.org/series/57618/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5708_full -> Patchwork_12386_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@preempt-other-chain-blt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +76

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-snb:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-d:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-pageflip-hang-newfb-render-f:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2

  * igt@kms_chv_cursor_fail@pipe-c-256x256-bottom-edge:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +10

  * igt@kms_color@pipe-a-legacy-gamma:
    - shard-apl:          PASS -> FAIL [fdo#104782] / [fdo#108145]

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

  * igt@kms_cursor_crc@cursor-256x85-onscreen:
    - shard-skl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +4

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          PASS -> FAIL [fdo#105767]

  * igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled:
    - shard-skl:          PASS -> FAIL [fdo#103184]

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

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

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +28

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
    - shard-skl:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +15

  * igt@kms_panel_fitting@legacy:
    - shard-skl:          NOTRUN -> FAIL [fdo#105456]

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-glk:          PASS -> FAIL [fdo#108948]

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          PASS -> FAIL [fdo#109016]

  * igt@kms_rotation_crc@primary-rotation-270:
    - shard-skl:          PASS -> FAIL [fdo#103925] / [fdo#107815]

  
#### Possible fixes ####

  * igt@i915_pm_rpm@gem-execbuf-stress-extra-wait:
    - shard-skl:          INCOMPLETE [fdo#107803] / [fdo#107807] -> PASS

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-kbl:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_chv_cursor_fail@pipe-a-256x256-bottom-edge:
    - shard-skl:          FAIL [fdo#104671] -> PASS

  * igt@kms_cursor_crc@cursor-256x85-offscreen:
    - shard-skl:          FAIL [fdo#103232] -> PASS

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

  * igt@kms_cursor_crc@cursor-size-change:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          FAIL [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-mmap-cpu:
    - shard-skl:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-skl:          FAIL [fdo#105682] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-wc:
    - shard-skl:          FAIL [fdo#103167] / [fdo#105682] -> PASS

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          FAIL [fdo#107815] / [fdo#108145] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_universal_plane@universal-plane-pipe-c-functional:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  
#### Warnings ####

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-skl:          DMESG-FAIL [fdo#103166] / [fdo#106885] -> FAIL [fdo#103166]

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

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103925]: https://bugs.freedesktop.org/show_bug.cgi?id=103925
  [fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#107803]: https://bugs.freedesktop.org/show_bug.cgi?id=107803
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350


Participating hosts (6 -> 6)
------------------------------

  No changes in participating hosts


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

    * Linux: CI_DRM_5708 -> Patchwork_12386

  CI_DRM_5708: afd34c5dec857362de91fb3044f09d90e83ad6a5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4871: 8feb147562ba1b364615ddacd44c3846f0250d37 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12386: aa1bb01b628e15767e705bdaef5e14b3aa76f96e @ 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_12386/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/5] drm/i915/icl: use a function pointer for pll_write when enabling

[Ville Syrjälä]

On Tue, Mar 05, 2019 at 05:26:33PM -0800, Lucas De Marchi wrote:
> This allows us to share the icl_pll_enable() between the different types
> of PLL while allowing the caller to differentiate how to write the
> registers.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 3b3de99756d6..5511bc23ea3d 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -3118,9 +3118,10 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv,
>  
>  static void icl_pll_enable(struct drm_i915_private *dev_priv,
>  			   struct intel_shared_dpll *pll,
> -			   i915_reg_t enable_reg)
> +			   i915_reg_t enable_reg,
> +			   void (*pll_write)(struct drm_i915_private *dev_priv,
> +					     struct intel_shared_dpll *pll))
>  {
> -	const enum intel_dpll_id id = pll->info->id;
>  	u32 val;
>  
>  	val = I915_READ(enable_reg);
> @@ -3133,12 +3134,9 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
>  	 */
>  	if (intel_wait_for_register(dev_priv, enable_reg, PLL_POWER_STATE,
>  				    PLL_POWER_STATE, 1))
> -		DRM_ERROR("PLL %d Power not enabled\n", id);
> +		DRM_ERROR("PLL %d Power not enabled\n", pll->info->id);
>  
> -	if (intel_dpll_is_combophy(id) || id == DPLL_ID_ICL_TBTPLL)
> -		icl_dpll_write(dev_priv, pll);
> -	else
> -		icl_mg_pll_write(dev_priv, pll);
> +	pll_write(dev_priv, pll);

Hmm. Would it be cleaner to just exract the pll power
enable/disable and pll enable/disable parts into small helpers?
It looks like like glk/cnl also follow this same pattern, so
there may be a chance to reuse the code on those platforms
as well.

>  
>  	/*
>  	 * DVFS pre sequence would be here, but in our driver the cdclk code
> @@ -3152,7 +3150,7 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
>  
>  	if (intel_wait_for_register(dev_priv, enable_reg, PLL_LOCK, PLL_LOCK,
>  				    1)) /* 600us actually. */
> -		DRM_ERROR("PLL %d not locked\n", id);
> +		DRM_ERROR("PLL %d not locked\n", pll->info->id);
>  
>  	/* DVFS post sequence would be here. See the comment above. */
>  }
> @@ -3162,7 +3160,7 @@ static void combo_pll_enable(struct drm_i915_private *dev_priv,
>  {
>  	i915_reg_t enable_reg = icl_pll_id_to_enable_reg(pll->info->id);
>  
> -	icl_pll_enable(dev_priv, pll, enable_reg);
> +	icl_pll_enable(dev_priv, pll, enable_reg, icl_dpll_write);
>  }
>  
>  static void mg_pll_enable(struct drm_i915_private *dev_priv,
> @@ -3171,7 +3169,7 @@ static void mg_pll_enable(struct drm_i915_private *dev_priv,
>  	i915_reg_t enable_reg =
>  		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
>  
> -	icl_pll_enable(dev_priv, pll, enable_reg);
> +	icl_pll_enable(dev_priv, pll, enable_reg, icl_mg_pll_write);
>  }
>  
>  static void icl_pll_disable(struct drm_i915_private *dev_priv,
> -- 
> 2.20.1

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