[PATCH 0/7] drm/i915: move dpll_info inside intel_shared_dpll

[PATCH 0/7] drm/i915: move dpll_info inside intel_shared_dpll

[Lucas De Marchi]

This is an alternative to my previous patch
"drm/i915: Remove hole and padding from intel_shared_dpll".

Not sure if I split this too much, but I think it's easier to review
this way. We can always squash them if wanted.

As can be seen below this reduces .text and size of our structs, but
maybe more important it puts the const data together. 

   text	   data	    bss	    dec	    hex	filename
1753856	  69541	   5316	1828713	 1be769	drivers/gpu/drm/i915/i915.ko
1753668	  69541	   5316	1828525	 1be6ad	drivers/gpu/drm/i915/i915.ko.new

Ville, instead of adding a pointer I decided to copy the struct, just
because I thought we were going through too many indirections while for
example accesssing pll->info->funcs->get_hw_state().

struct sizes:
		       before	after
intel_shared_dpll	  152	  128
i915_drm_private	32112	31968

Lucas De Marchi (7):
  drm/i915: move dpll_info to header
  drm/i915: embed dpll_info inside intel_shared_dpll
  drm/i915: use funcs from dpll_info embedded in intel_shared_dpll
  drm/i915: use name from dpll_info embedded in intel_shared_dpll
  drm/i915: use id from dpll_info embedded in intel_shared_dpll
  drm/i915: use flags from dpll_info embedded in intel_shared_dpll
  drm/i915: rename dpll_info to intel_dpll_info

 drivers/gpu/drm/i915/i915_debugfs.c   |   3 +-
 drivers/gpu/drm/i915/intel_ddi.c      |   8 +-
 drivers/gpu/drm/i915/intel_display.c  |  25 ++---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 198 +++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_dpll_mgr.h |  53 +++++----
 5 files changed, 145 insertions(+), 142 deletions(-)

-- 
2.14.3

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

[PATCH 1/7] drm/i915: move dpll_info to header

[Lucas De Marchi]

This will allow the struct to be embedded in intel_shared_dpll.

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

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 51c5ae4e9116..52d6e731c3e9 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1877,13 +1877,6 @@ static void intel_ddi_pll_init(struct drm_device *dev)
 	}
 }
 
-struct dpll_info {
-	const char *name;
-	const int id;
-	const struct intel_shared_dpll_funcs *funcs;
-	uint32_t flags;
-};
-
 struct intel_dpll_mgr {
 	const struct dpll_info *dpll_info;
 
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index f24ccf443d25..e99d6385478a 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -205,6 +205,16 @@ struct intel_shared_dpll_funcs {
 			     struct intel_dpll_hw_state *hw_state);
 };
 
+/**
+ * struct dpll_info - display PLL platform specific info
+ */
+struct dpll_info {
+	const char *name;
+	const int id;
+	const struct intel_shared_dpll_funcs *funcs;
+	uint32_t flags;
+};
+
 /**
  * struct intel_shared_dpll - display PLL with tracked state and users
  */
-- 
2.14.3

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

[PATCH 2/7] drm/i915: embed dpll_info inside intel_shared_dpll

[Lucas De Marchi]

This way we can stop copying fields from dpll_info to intel_shared_dpll
one by one. The migration of each field will come on separate patches.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 2 ++
 drivers/gpu/drm/i915/intel_dpll_mgr.h | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 52d6e731c3e9..7ea8385cfe1d 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2410,6 +2410,8 @@ void intel_shared_dpll_init(struct drm_device *dev)
 
 	for (i = 0; dpll_info[i].id >= 0; i++) {
 		WARN_ON(i != dpll_info[i].id);
+		memcpy(&dev_priv->shared_dplls[i].info, &dpll_info[i],
+		       sizeof(struct dpll_info));
 
 		dev_priv->shared_dplls[i].id = dpll_info[i].id;
 		dev_priv->shared_dplls[i].name = dpll_info[i].name;
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index e99d6385478a..2bfec0849a5f 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -253,6 +253,11 @@ struct intel_shared_dpll {
 	 */
 	struct intel_shared_dpll_funcs funcs;
 
+	/**
+	 * @info: platform specific info
+	 */
+	struct dpll_info info;
+
 #define INTEL_DPLL_ALWAYS_ON	(1 << 0)
 	/**
 	 * @flags:
-- 
2.14.3

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

[PATCH 3/7] drm/i915: use funcs from dpll_info embedded in intel_shared_dpll

[Lucas De Marchi]

Replace all users of pll->funcs.* to use
pll->info.funcs->*. The extra indirection here is not on any critical
path and we can leave all const data together.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c  | 16 ++++++++--------
 drivers/gpu/drm/i915/intel_dpll_mgr.c |  9 ++++-----
 drivers/gpu/drm/i915/intel_dpll_mgr.h |  8 +++-----
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3e7ab75e1b41..d8e1993b883c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8768,8 +8768,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 			intel_get_shared_dpll_by_id(dev_priv, pll_id);
 		pll = pipe_config->shared_dpll;
 
-		WARN_ON(!pll->funcs.get_hw_state(dev_priv, pll,
-						 &pipe_config->dpll_hw_state));
+		WARN_ON(!pll->info.funcs->get_hw_state(dev_priv, pll,
+						&pipe_config->dpll_hw_state));
 
 		tmp = pipe_config->dpll_hw_state.dpll;
 		pipe_config->pixel_multiplier =
@@ -9245,8 +9245,8 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 
 	pll = pipe_config->shared_dpll;
 	if (pll) {
-		WARN_ON(!pll->funcs.get_hw_state(dev_priv, pll,
-						 &pipe_config->dpll_hw_state));
+		WARN_ON(!pll->info.funcs->get_hw_state(dev_priv, pll,
+						&pipe_config->dpll_hw_state));
 	}
 
 	/*
@@ -11647,7 +11647,7 @@ verify_single_dpll_state(struct drm_i915_private *dev_priv,
 
 	DRM_DEBUG_KMS("%s\n", pll->name);
 
-	active = pll->funcs.get_hw_state(dev_priv, pll, &dpll_hw_state);
+	active = pll->info.funcs->get_hw_state(dev_priv, pll, &dpll_hw_state);
 
 	if (!(pll->flags & INTEL_DPLL_ALWAYS_ON)) {
 		I915_STATE_WARN(!pll->on && pll->active_mask,
@@ -15123,8 +15123,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
 		struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
 
-		pll->on = pll->funcs.get_hw_state(dev_priv, pll,
-						  &pll->state.hw_state);
+		pll->on = pll->info.funcs->get_hw_state(dev_priv, pll,
+							&pll->state.hw_state);
 		pll->state.crtc_mask = 0;
 		for_each_intel_crtc(dev, crtc) {
 			struct intel_crtc_state *crtc_state =
@@ -15313,7 +15313,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
 
 		DRM_DEBUG_KMS("%s enabled but not in use, disabling\n", pll->name);
 
-		pll->funcs.disable(dev_priv, pll);
+		pll->info.funcs->disable(dev_priv, pll);
 		pll->on = false;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 7ea8385cfe1d..4e1afd35b38d 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -118,7 +118,7 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
 	if (WARN(!pll, "asserting DPLL %s with no DPLL\n", onoff(state)))
 		return;
 
-	cur_state = pll->funcs.get_hw_state(dev_priv, pll, &hw_state);
+	cur_state = pll->info.funcs->get_hw_state(dev_priv, pll, &hw_state);
 	I915_STATE_WARN(cur_state != state,
 	     "%s assertion failure (expected %s, current %s)\n",
 			pll->name, onoff(state), onoff(cur_state));
@@ -147,7 +147,7 @@ void intel_prepare_shared_dpll(struct intel_crtc *crtc)
 		WARN_ON(pll->on);
 		assert_shared_dpll_disabled(dev_priv, pll);
 
-		pll->funcs.prepare(dev_priv, pll);
+		pll->info.funcs->prepare(dev_priv, pll);
 	}
 	mutex_unlock(&dev_priv->dpll_lock);
 }
@@ -190,7 +190,7 @@ void intel_enable_shared_dpll(struct intel_crtc *crtc)
 	WARN_ON(pll->on);
 
 	DRM_DEBUG_KMS("enabling %s\n", pll->name);
-	pll->funcs.enable(dev_priv, pll);
+	pll->info.funcs->enable(dev_priv, pll);
 	pll->on = true;
 
 out:
@@ -232,7 +232,7 @@ void intel_disable_shared_dpll(struct intel_crtc *crtc)
 		goto out;
 
 	DRM_DEBUG_KMS("disabling %s\n", pll->name);
-	pll->funcs.disable(dev_priv, pll);
+	pll->info.funcs->disable(dev_priv, pll);
 	pll->on = false;
 
 out:
@@ -2415,7 +2415,6 @@ void intel_shared_dpll_init(struct drm_device *dev)
 
 		dev_priv->shared_dplls[i].id = dpll_info[i].id;
 		dev_priv->shared_dplls[i].name = dpll_info[i].name;
-		dev_priv->shared_dplls[i].funcs = *dpll_info[i].funcs;
 		dev_priv->shared_dplls[i].flags = dpll_info[i].flags;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index 2bfec0849a5f..131031436615 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -211,6 +211,9 @@ struct intel_shared_dpll_funcs {
 struct dpll_info {
 	const char *name;
 	const int id;
+	/**
+	 * @funcs: platform specific hooks
+	 */
 	const struct intel_shared_dpll_funcs *funcs;
 	uint32_t flags;
 };
@@ -248,11 +251,6 @@ struct intel_shared_dpll {
 	 */
 	enum intel_dpll_id id;
 
-	/**
-	 * @funcs: platform specific hooks
-	 */
-	struct intel_shared_dpll_funcs funcs;
-
 	/**
 	 * @info: platform specific info
 	 */
-- 
2.14.3

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

[PATCH 4/7] drm/i915: use name from dpll_info embedded in intel_shared_dpll

[Lucas De Marchi]

Replace all users of pll->name to use pll->info.name.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  3 ++-
 drivers/gpu/drm/i915/intel_display.c  |  7 ++++---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 26 ++++++++++++++------------
 drivers/gpu/drm/i915/intel_dpll_mgr.h |  8 +++-----
 4 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 964ea1a12357..83d59f6f8020 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3285,7 +3285,8 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
 	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
 		struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
 
-		seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->name, pll->id);
+		seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->info.name,
+			   pll->id);
 		seq_printf(m, " crtc_mask: 0x%08x, active: 0x%x, on: %s\n",
 			   pll->state.crtc_mask, pll->active_mask, yesno(pll->on));
 		seq_printf(m, " tracked hardware state:\n");
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d8e1993b883c..53fce46380e0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11645,7 +11645,7 @@ verify_single_dpll_state(struct drm_i915_private *dev_priv,
 
 	memset(&dpll_hw_state, 0, sizeof(dpll_hw_state));
 
-	DRM_DEBUG_KMS("%s\n", pll->name);
+	DRM_DEBUG_KMS("%s\n", pll->info.name);
 
 	active = pll->info.funcs->get_hw_state(dev_priv, pll, &dpll_hw_state);
 
@@ -15137,7 +15137,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 		pll->active_mask = pll->state.crtc_mask;
 
 		DRM_DEBUG_KMS("%s hw state readout: crtc_mask 0x%08x, on %i\n",
-			      pll->name, pll->state.crtc_mask, pll->on);
+			      pll->info.name, pll->state.crtc_mask, pll->on);
 	}
 
 	for_each_intel_encoder(dev, encoder) {
@@ -15311,7 +15311,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
 		if (!pll->on || pll->active_mask)
 			continue;
 
-		DRM_DEBUG_KMS("%s enabled but not in use, disabling\n", pll->name);
+		DRM_DEBUG_KMS("%s enabled but not in use, disabling\n",
+			      pll->info.name);
 
 		pll->info.funcs->disable(dev_priv, pll);
 		pll->on = false;
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 4e1afd35b38d..9fe2596d2aab 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -121,7 +121,7 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
 	cur_state = pll->info.funcs->get_hw_state(dev_priv, pll, &hw_state);
 	I915_STATE_WARN(cur_state != state,
 	     "%s assertion failure (expected %s, current %s)\n",
-			pll->name, onoff(state), onoff(cur_state));
+			pll->info.name, onoff(state), onoff(cur_state));
 }
 
 /**
@@ -143,7 +143,7 @@ void intel_prepare_shared_dpll(struct intel_crtc *crtc)
 	mutex_lock(&dev_priv->dpll_lock);
 	WARN_ON(!pll->state.crtc_mask);
 	if (!pll->active_mask) {
-		DRM_DEBUG_DRIVER("setting up %s\n", pll->name);
+		DRM_DEBUG_DRIVER("setting up %s\n", pll->info.name);
 		WARN_ON(pll->on);
 		assert_shared_dpll_disabled(dev_priv, pll);
 
@@ -179,7 +179,7 @@ void intel_enable_shared_dpll(struct intel_crtc *crtc)
 	pll->active_mask |= crtc_mask;
 
 	DRM_DEBUG_KMS("enable %s (active %x, on? %d) for crtc %d\n",
-		      pll->name, pll->active_mask, pll->on,
+		      pll->info.name, pll->active_mask, pll->on,
 		      crtc->base.base.id);
 
 	if (old_mask) {
@@ -189,7 +189,7 @@ void intel_enable_shared_dpll(struct intel_crtc *crtc)
 	}
 	WARN_ON(pll->on);
 
-	DRM_DEBUG_KMS("enabling %s\n", pll->name);
+	DRM_DEBUG_KMS("enabling %s\n", pll->info.name);
 	pll->info.funcs->enable(dev_priv, pll);
 	pll->on = true;
 
@@ -221,7 +221,7 @@ void intel_disable_shared_dpll(struct intel_crtc *crtc)
 		goto out;
 
 	DRM_DEBUG_KMS("disable %s (active %x, on? %d) for crtc %d\n",
-		      pll->name, pll->active_mask, pll->on,
+		      pll->info.name, pll->active_mask, pll->on,
 		      crtc->base.base.id);
 
 	assert_shared_dpll_enabled(dev_priv, pll);
@@ -231,7 +231,7 @@ void intel_disable_shared_dpll(struct intel_crtc *crtc)
 	if (pll->active_mask)
 		goto out;
 
-	DRM_DEBUG_KMS("disabling %s\n", pll->name);
+	DRM_DEBUG_KMS("disabling %s\n", pll->info.name);
 	pll->info.funcs->disable(dev_priv, pll);
 	pll->on = false;
 
@@ -263,7 +263,8 @@ intel_find_shared_dpll(struct intel_crtc *crtc,
 			   &shared_dpll[i].hw_state,
 			   sizeof(crtc_state->dpll_hw_state)) == 0) {
 			DRM_DEBUG_KMS("[CRTC:%d:%s] sharing existing %s (crtc mask 0x%08x, active %x)\n",
-				      crtc->base.base.id, crtc->base.name, pll->name,
+				      crtc->base.base.id, crtc->base.name,
+				      pll->info.name,
 				      shared_dpll[i].crtc_mask,
 				      pll->active_mask);
 			return pll;
@@ -275,7 +276,8 @@ intel_find_shared_dpll(struct intel_crtc *crtc,
 		pll = &dev_priv->shared_dplls[i];
 		if (shared_dpll[i].crtc_mask == 0) {
 			DRM_DEBUG_KMS("[CRTC:%d:%s] allocated %s\n",
-				      crtc->base.base.id, crtc->base.name, pll->name);
+				      crtc->base.base.id, crtc->base.name,
+				      pll->info.name);
 			return pll;
 		}
 	}
@@ -298,7 +300,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
 			crtc_state->dpll_hw_state;
 
 	crtc_state->shared_dpll = pll;
-	DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->name,
+	DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->info.name,
 			 pipe_name(crtc->pipe));
 
 	shared_dpll[pll->id].crtc_mask |= 1 << crtc->pipe;
@@ -429,7 +431,8 @@ ibx_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 		pll = &dev_priv->shared_dplls[i];
 
 		DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n",
-			      crtc->base.base.id, crtc->base.name, pll->name);
+			      crtc->base.base.id, crtc->base.name,
+			      pll->info.name);
 	} else {
 		pll = intel_find_shared_dpll(crtc, crtc_state,
 					     DPLL_ID_PCH_PLL_A,
@@ -1824,7 +1827,7 @@ bxt_get_dpll(struct intel_crtc *crtc,
 	pll = intel_get_shared_dpll_by_id(dev_priv, i);
 
 	DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n",
-		      crtc->base.base.id, crtc->base.name, pll->name);
+		      crtc->base.base.id, crtc->base.name, pll->info.name);
 
 	intel_reference_shared_dpll(pll, crtc_state);
 
@@ -2414,7 +2417,6 @@ void intel_shared_dpll_init(struct drm_device *dev)
 		       sizeof(struct dpll_info));
 
 		dev_priv->shared_dplls[i].id = dpll_info[i].id;
-		dev_priv->shared_dplls[i].name = dpll_info[i].name;
 		dev_priv->shared_dplls[i].flags = dpll_info[i].flags;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index 131031436615..16a42cf82ba8 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -209,6 +209,9 @@ struct intel_shared_dpll_funcs {
  * struct dpll_info - display PLL platform specific info
  */
 struct dpll_info {
+	/**
+	 * @name: DPLL name; used for logging
+	 */
 	const char *name;
 	const int id;
 	/**
@@ -240,11 +243,6 @@ struct intel_shared_dpll {
 	 */
 	bool on;
 
-	/**
-	 * @name: DPLL name; used for logging
-	 */
-	const char *name;
-
 	/**
 	 * @id: unique indentifier for this DPLL; should match the index in the
 	 * dev_priv->shared_dplls array
-- 
2.14.3

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

[PATCH 5/7] drm/i915: use id from dpll_info embedded in intel_shared_dpll

[Lucas De Marchi]

Replace all users of pll->id to use pll->info.id.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |   2 +-
 drivers/gpu/drm/i915/intel_ddi.c      |   8 +-
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 138 +++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_dpll_mgr.h |  10 +--
 4 files changed, 78 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 83d59f6f8020..87802704df4d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3286,7 +3286,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
 		struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
 
 		seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->info.name,
-			   pll->id);
+			   pll->info.id);
 		seq_printf(m, " crtc_mask: 0x%08x, active: 0x%x, on: %s\n",
 			   pll->state.crtc_mask, pll->active_mask, yesno(pll->on));
 		seq_printf(m, " tracked hardware state:\n");
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 8c2d778560f0..0969f2b73386 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -875,7 +875,7 @@ static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
 
 static uint32_t hsw_pll_to_ddi_pll_sel(const struct intel_shared_dpll *pll)
 {
-	switch (pll->id) {
+	switch (pll->info.id) {
 	case DPLL_ID_WRPLL1:
 		return PORT_CLK_SEL_WRPLL1;
 	case DPLL_ID_WRPLL2:
@@ -889,7 +889,7 @@ static uint32_t hsw_pll_to_ddi_pll_sel(const struct intel_shared_dpll *pll)
 	case DPLL_ID_LCPLL_2700:
 		return PORT_CLK_SEL_LCPLL_2700;
 	default:
-		MISSING_CASE(pll->id);
+		MISSING_CASE(pll->info.id);
 		return PORT_CLK_SEL_NONE;
 	}
 }
@@ -2131,7 +2131,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
 		/* Configure DPCLKA_CFGCR0 to map the DPLL to the DDI. */
 		val = I915_READ(DPCLKA_CFGCR0);
 		val &= ~DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port);
-		val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->id, port);
+		val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info.id, port);
 		I915_WRITE(DPCLKA_CFGCR0, val);
 
 		/*
@@ -2148,7 +2148,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
 
 		val &= ~(DPLL_CTRL2_DDI_CLK_OFF(port) |
 			 DPLL_CTRL2_DDI_CLK_SEL_MASK(port));
-		val |= (DPLL_CTRL2_DDI_CLK_SEL(pll->id, port) |
+		val |= (DPLL_CTRL2_DDI_CLK_SEL(pll->info.id, port) |
 			DPLL_CTRL2_DDI_SEL_OVERRIDE(port));
 
 		I915_WRITE(DPLL_CTRL2, val);
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 9fe2596d2aab..823752f725d3 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -291,7 +291,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
 {
 	struct intel_shared_dpll_state *shared_dpll;
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	enum intel_dpll_id i = pll->id;
+	const enum intel_dpll_id i = pll->info.id;
 
 	shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state);
 
@@ -303,7 +303,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
 	DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->info.name,
 			 pipe_name(crtc->pipe));
 
-	shared_dpll[pll->id].crtc_mask |= 1 << crtc->pipe;
+	shared_dpll[i].crtc_mask |= 1 << crtc->pipe;
 }
 
 /**
@@ -348,10 +348,10 @@ static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv,
 	if (!intel_display_power_get_if_enabled(dev_priv, POWER_DOMAIN_PLLS))
 		return false;
 
-	val = I915_READ(PCH_DPLL(pll->id));
+	val = I915_READ(PCH_DPLL(pll->info.id));
 	hw_state->dpll = val;
-	hw_state->fp0 = I915_READ(PCH_FP0(pll->id));
-	hw_state->fp1 = I915_READ(PCH_FP1(pll->id));
+	hw_state->fp0 = I915_READ(PCH_FP0(pll->info.id));
+	hw_state->fp1 = I915_READ(PCH_FP1(pll->info.id));
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
 
@@ -361,8 +361,8 @@ static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv,
 static void ibx_pch_dpll_prepare(struct drm_i915_private *dev_priv,
 				 struct intel_shared_dpll *pll)
 {
-	I915_WRITE(PCH_FP0(pll->id), pll->state.hw_state.fp0);
-	I915_WRITE(PCH_FP1(pll->id), pll->state.hw_state.fp1);
+	I915_WRITE(PCH_FP0(pll->info.id), pll->state.hw_state.fp0);
+	I915_WRITE(PCH_FP1(pll->info.id), pll->state.hw_state.fp1);
 }
 
 static void ibx_assert_pch_refclk_enabled(struct drm_i915_private *dev_priv)
@@ -384,10 +384,10 @@ static void ibx_pch_dpll_enable(struct drm_i915_private *dev_priv,
 	/* PCH refclock must be enabled first */
 	ibx_assert_pch_refclk_enabled(dev_priv);
 
-	I915_WRITE(PCH_DPLL(pll->id), pll->state.hw_state.dpll);
+	I915_WRITE(PCH_DPLL(pll->info.id), pll->state.hw_state.dpll);
 
 	/* Wait for the clocks to stabilize. */
-	POSTING_READ(PCH_DPLL(pll->id));
+	POSTING_READ(PCH_DPLL(pll->info.id));
 	udelay(150);
 
 	/* The pixel multiplier can only be updated once the
@@ -395,8 +395,8 @@ static void ibx_pch_dpll_enable(struct drm_i915_private *dev_priv,
 	 *
 	 * So write it again.
 	 */
-	I915_WRITE(PCH_DPLL(pll->id), pll->state.hw_state.dpll);
-	POSTING_READ(PCH_DPLL(pll->id));
+	I915_WRITE(PCH_DPLL(pll->info.id), pll->state.hw_state.dpll);
+	POSTING_READ(PCH_DPLL(pll->info.id));
 	udelay(200);
 }
 
@@ -412,8 +412,8 @@ static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv,
 			assert_pch_transcoder_disabled(dev_priv, crtc->pipe);
 	}
 
-	I915_WRITE(PCH_DPLL(pll->id), 0);
-	POSTING_READ(PCH_DPLL(pll->id));
+	I915_WRITE(PCH_DPLL(pll->info.id), 0);
+	POSTING_READ(PCH_DPLL(pll->info.id));
 	udelay(200);
 }
 
@@ -469,8 +469,8 @@ static const struct intel_shared_dpll_funcs ibx_pch_dpll_funcs = {
 static void hsw_ddi_wrpll_enable(struct drm_i915_private *dev_priv,
 			       struct intel_shared_dpll *pll)
 {
-	I915_WRITE(WRPLL_CTL(pll->id), pll->state.hw_state.wrpll);
-	POSTING_READ(WRPLL_CTL(pll->id));
+	I915_WRITE(WRPLL_CTL(pll->info.id), pll->state.hw_state.wrpll);
+	POSTING_READ(WRPLL_CTL(pll->info.id));
 	udelay(20);
 }
 
@@ -487,9 +487,9 @@ static void hsw_ddi_wrpll_disable(struct drm_i915_private *dev_priv,
 {
 	uint32_t val;
 
-	val = I915_READ(WRPLL_CTL(pll->id));
-	I915_WRITE(WRPLL_CTL(pll->id), val & ~WRPLL_PLL_ENABLE);
-	POSTING_READ(WRPLL_CTL(pll->id));
+	val = I915_READ(WRPLL_CTL(pll->info.id));
+	I915_WRITE(WRPLL_CTL(pll->info.id), val & ~WRPLL_PLL_ENABLE);
+	POSTING_READ(WRPLL_CTL(pll->info.id));
 }
 
 static void hsw_ddi_spll_disable(struct drm_i915_private *dev_priv,
@@ -511,7 +511,7 @@ static bool hsw_ddi_wrpll_get_hw_state(struct drm_i915_private *dev_priv,
 	if (!intel_display_power_get_if_enabled(dev_priv, POWER_DOMAIN_PLLS))
 		return false;
 
-	val = I915_READ(WRPLL_CTL(pll->id));
+	val = I915_READ(WRPLL_CTL(pll->info.id));
 	hw_state->wrpll = val;
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
@@ -921,9 +921,10 @@ static void skl_ddi_pll_write_ctrl1(struct drm_i915_private *dev_priv,
 
 	val = I915_READ(DPLL_CTRL1);
 
-	val &= ~(DPLL_CTRL1_HDMI_MODE(pll->id) | DPLL_CTRL1_SSC(pll->id) |
-		 DPLL_CTRL1_LINK_RATE_MASK(pll->id));
-	val |= pll->state.hw_state.ctrl1 << (pll->id * 6);
+	val &= ~(DPLL_CTRL1_HDMI_MODE(pll->info.id) |
+		 DPLL_CTRL1_SSC(pll->info.id) |
+		 DPLL_CTRL1_LINK_RATE_MASK(pll->info.id));
+	val |= pll->state.hw_state.ctrl1 << (pll->info.id * 6);
 
 	I915_WRITE(DPLL_CTRL1, val);
 	POSTING_READ(DPLL_CTRL1);
@@ -936,21 +937,21 @@ static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 
 	skl_ddi_pll_write_ctrl1(dev_priv, pll);
 
-	I915_WRITE(regs[pll->id].cfgcr1, pll->state.hw_state.cfgcr1);
-	I915_WRITE(regs[pll->id].cfgcr2, pll->state.hw_state.cfgcr2);
-	POSTING_READ(regs[pll->id].cfgcr1);
-	POSTING_READ(regs[pll->id].cfgcr2);
+	I915_WRITE(regs[pll->info.id].cfgcr1, pll->state.hw_state.cfgcr1);
+	I915_WRITE(regs[pll->info.id].cfgcr2, pll->state.hw_state.cfgcr2);
+	POSTING_READ(regs[pll->info.id].cfgcr1);
+	POSTING_READ(regs[pll->info.id].cfgcr2);
 
 	/* the enable bit is always bit 31 */
-	I915_WRITE(regs[pll->id].ctl,
-		   I915_READ(regs[pll->id].ctl) | LCPLL_PLL_ENABLE);
+	I915_WRITE(regs[pll->info.id].ctl,
+		   I915_READ(regs[pll->info.id].ctl) | LCPLL_PLL_ENABLE);
 
 	if (intel_wait_for_register(dev_priv,
 				    DPLL_STATUS,
-				    DPLL_LOCK(pll->id),
-				    DPLL_LOCK(pll->id),
+				    DPLL_LOCK(pll->info.id),
+				    DPLL_LOCK(pll->info.id),
 				    5))
-		DRM_ERROR("DPLL %d not locked\n", pll->id);
+		DRM_ERROR("DPLL %d not locked\n", pll->info.id);
 }
 
 static void skl_ddi_dpll0_enable(struct drm_i915_private *dev_priv,
@@ -965,9 +966,9 @@ static void skl_ddi_pll_disable(struct drm_i915_private *dev_priv,
 	const struct skl_dpll_regs *regs = skl_dpll_regs;
 
 	/* the enable bit is always bit 31 */
-	I915_WRITE(regs[pll->id].ctl,
-		   I915_READ(regs[pll->id].ctl) & ~LCPLL_PLL_ENABLE);
-	POSTING_READ(regs[pll->id].ctl);
+	I915_WRITE(regs[pll->info.id].ctl,
+		   I915_READ(regs[pll->info.id].ctl) & ~LCPLL_PLL_ENABLE);
+	POSTING_READ(regs[pll->info.id].ctl);
 }
 
 static void skl_ddi_dpll0_disable(struct drm_i915_private *dev_priv,
@@ -988,17 +989,17 @@ static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 
 	ret = false;
 
-	val = I915_READ(regs[pll->id].ctl);
+	val = I915_READ(regs[pll->info.id].ctl);
 	if (!(val & LCPLL_PLL_ENABLE))
 		goto out;
 
 	val = I915_READ(DPLL_CTRL1);
-	hw_state->ctrl1 = (val >> (pll->id * 6)) & 0x3f;
+	hw_state->ctrl1 = (val >> (pll->info.id * 6)) & 0x3f;
 
 	/* avoid reading back stale values if HDMI mode is not enabled */
-	if (val & DPLL_CTRL1_HDMI_MODE(pll->id)) {
-		hw_state->cfgcr1 = I915_READ(regs[pll->id].cfgcr1);
-		hw_state->cfgcr2 = I915_READ(regs[pll->id].cfgcr2);
+	if (val & DPLL_CTRL1_HDMI_MODE(pll->info.id)) {
+		hw_state->cfgcr1 = I915_READ(regs[pll->info.id].cfgcr1);
+		hw_state->cfgcr2 = I915_READ(regs[pll->info.id].cfgcr2);
 	}
 	ret = true;
 
@@ -1022,12 +1023,12 @@ static bool skl_ddi_dpll0_get_hw_state(struct drm_i915_private *dev_priv,
 	ret = false;
 
 	/* DPLL0 is always enabled since it drives CDCLK */
-	val = I915_READ(regs[pll->id].ctl);
+	val = I915_READ(regs[pll->info.id].ctl);
 	if (WARN_ON(!(val & LCPLL_PLL_ENABLE)))
 		goto out;
 
 	val = I915_READ(DPLL_CTRL1);
-	hw_state->ctrl1 = (val >> (pll->id * 6)) & 0x3f;
+	hw_state->ctrl1 = (val >> (pll->info.id * 6)) & 0x3f;
 
 	ret = true;
 
@@ -1427,7 +1428,7 @@ static void bxt_ddi_pll_enable(struct drm_i915_private *dev_priv,
 				struct intel_shared_dpll *pll)
 {
 	uint32_t temp;
-	enum port port = (enum port)pll->id;	/* 1:1 port->PLL mapping */
+	enum port port = (enum port)pll->info.id; /* 1:1 port->PLL mapping */
 	enum dpio_phy phy;
 	enum dpio_channel ch;
 
@@ -1546,7 +1547,7 @@ static void bxt_ddi_pll_enable(struct drm_i915_private *dev_priv,
 static void bxt_ddi_pll_disable(struct drm_i915_private *dev_priv,
 					struct intel_shared_dpll *pll)
 {
-	enum port port = (enum port)pll->id;	/* 1:1 port->PLL mapping */
+	enum port port = (enum port)pll->info.id; /* 1:1 port->PLL mapping */
 	uint32_t temp;
 
 	temp = I915_READ(BXT_PORT_PLL_ENABLE(port));
@@ -1569,7 +1570,7 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 					struct intel_shared_dpll *pll,
 					struct intel_dpll_hw_state *hw_state)
 {
-	enum port port = (enum port)pll->id;	/* 1:1 port->PLL mapping */
+	enum port port = (enum port)pll->info.id; /* 1:1 port->PLL mapping */
 	uint32_t val;
 	bool ret;
 	enum dpio_phy phy;
@@ -1952,35 +1953,35 @@ static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 	uint32_t val;
 
 	/* 1. Enable DPLL power in DPLL_ENABLE. */
-	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
+	val = I915_READ(CNL_DPLL_ENABLE(pll->info.id));
 	val |= PLL_POWER_ENABLE;
-	I915_WRITE(CNL_DPLL_ENABLE(pll->id), val);
+	I915_WRITE(CNL_DPLL_ENABLE(pll->info.id), val);
 
 	/* 2. Wait for DPLL power state enabled in DPLL_ENABLE. */
 	if (intel_wait_for_register(dev_priv,
-				    CNL_DPLL_ENABLE(pll->id),
+				    CNL_DPLL_ENABLE(pll->info.id),
 				    PLL_POWER_STATE,
 				    PLL_POWER_STATE,
 				    5))
-		DRM_ERROR("PLL %d Power not enabled\n", pll->id);
+		DRM_ERROR("PLL %d Power not enabled\n", pll->info.id);
 
 	/*
 	 * 3. Configure DPLL_CFGCR0 to set SSC enable/disable,
 	 * select DP mode, and set DP link rate.
 	 */
 	val = pll->state.hw_state.cfgcr0;
-	I915_WRITE(CNL_DPLL_CFGCR0(pll->id), val);
+	I915_WRITE(CNL_DPLL_CFGCR0(pll->info.id), val);
 
 	/* 4. Reab back to ensure writes completed */
-	POSTING_READ(CNL_DPLL_CFGCR0(pll->id));
+	POSTING_READ(CNL_DPLL_CFGCR0(pll->info.id));
 
 	/* 3. Configure DPLL_CFGCR0 */
 	/* Avoid touch CFGCR1 if HDMI mode is not enabled */
 	if (pll->state.hw_state.cfgcr0 & DPLL_CFGCR0_HDMI_MODE) {
 		val = pll->state.hw_state.cfgcr1;
-		I915_WRITE(CNL_DPLL_CFGCR1(pll->id), val);
+		I915_WRITE(CNL_DPLL_CFGCR1(pll->info.id), val);
 		/* 4. Reab back to ensure writes completed */
-		POSTING_READ(CNL_DPLL_CFGCR1(pll->id));
+		POSTING_READ(CNL_DPLL_CFGCR1(pll->info.id));
 	}
 
 	/*
@@ -1993,17 +1994,17 @@ static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 	 */
 
 	/* 6. Enable DPLL in DPLL_ENABLE. */
-	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
+	val = I915_READ(CNL_DPLL_ENABLE(pll->info.id));
 	val |= PLL_ENABLE;
-	I915_WRITE(CNL_DPLL_ENABLE(pll->id), val);
+	I915_WRITE(CNL_DPLL_ENABLE(pll->info.id), val);
 
 	/* 7. Wait for PLL lock status in DPLL_ENABLE. */
 	if (intel_wait_for_register(dev_priv,
-				    CNL_DPLL_ENABLE(pll->id),
+				    CNL_DPLL_ENABLE(pll->info.id),
 				    PLL_LOCK,
 				    PLL_LOCK,
 				    5))
-		DRM_ERROR("PLL %d not locked\n", pll->id);
+		DRM_ERROR("PLL %d not locked\n", pll->info.id);
 
 	/*
 	 * 8. If the frequency will result in a change to the voltage
@@ -2040,17 +2041,17 @@ static void cnl_ddi_pll_disable(struct drm_i915_private *dev_priv,
 	 */
 
 	/* 3. Disable DPLL through DPLL_ENABLE. */
-	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
+	val = I915_READ(CNL_DPLL_ENABLE(pll->info.id));
 	val &= ~PLL_ENABLE;
-	I915_WRITE(CNL_DPLL_ENABLE(pll->id), val);
+	I915_WRITE(CNL_DPLL_ENABLE(pll->info.id), val);
 
 	/* 4. Wait for PLL not locked status in DPLL_ENABLE. */
 	if (intel_wait_for_register(dev_priv,
-				    CNL_DPLL_ENABLE(pll->id),
+				    CNL_DPLL_ENABLE(pll->info.id),
 				    PLL_LOCK,
 				    0,
 				    5))
-		DRM_ERROR("PLL %d locked\n", pll->id);
+		DRM_ERROR("PLL %d locked\n", pll->info.id);
 
 	/*
 	 * 5. If the frequency will result in a change to the voltage
@@ -2062,17 +2063,17 @@ static void cnl_ddi_pll_disable(struct drm_i915_private *dev_priv,
 	 */
 
 	/* 6. Disable DPLL power in DPLL_ENABLE. */
-	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
+	val = I915_READ(CNL_DPLL_ENABLE(pll->info.id));
 	val &= ~PLL_POWER_ENABLE;
-	I915_WRITE(CNL_DPLL_ENABLE(pll->id), val);
+	I915_WRITE(CNL_DPLL_ENABLE(pll->info.id), val);
 
 	/* 7. Wait for DPLL power state disabled in DPLL_ENABLE. */
 	if (intel_wait_for_register(dev_priv,
-				    CNL_DPLL_ENABLE(pll->id),
+				    CNL_DPLL_ENABLE(pll->info.id),
 				    PLL_POWER_STATE,
 				    0,
 				    5))
-		DRM_ERROR("PLL %d Power not disabled\n", pll->id);
+		DRM_ERROR("PLL %d Power not disabled\n", pll->info.id);
 }
 
 static bool cnl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
@@ -2087,16 +2088,16 @@ static bool cnl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 
 	ret = false;
 
-	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
+	val = I915_READ(CNL_DPLL_ENABLE(pll->info.id));
 	if (!(val & PLL_ENABLE))
 		goto out;
 
-	val = I915_READ(CNL_DPLL_CFGCR0(pll->id));
+	val = I915_READ(CNL_DPLL_CFGCR0(pll->info.id));
 	hw_state->cfgcr0 = val;
 
 	/* avoid reading back stale values if HDMI mode is not enabled */
 	if (val & DPLL_CFGCR0_HDMI_MODE) {
-		hw_state->cfgcr1 = I915_READ(CNL_DPLL_CFGCR1(pll->id));
+		hw_state->cfgcr1 = I915_READ(CNL_DPLL_CFGCR1(pll->info.id));
 	}
 	ret = true;
 
@@ -2416,7 +2417,6 @@ void intel_shared_dpll_init(struct drm_device *dev)
 		memcpy(&dev_priv->shared_dplls[i].info, &dpll_info[i],
 		       sizeof(struct dpll_info));
 
-		dev_priv->shared_dplls[i].id = dpll_info[i].id;
 		dev_priv->shared_dplls[i].flags = dpll_info[i].flags;
 	}
 
@@ -2477,7 +2477,7 @@ void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
 	struct intel_shared_dpll_state *shared_dpll_state;
 
 	shared_dpll_state = intel_atomic_get_shared_dpll_state(state);
-	shared_dpll_state[dpll->id].crtc_mask &= ~(1 << crtc->pipe);
+	shared_dpll_state[dpll->info.id].crtc_mask &= ~(1 << crtc->pipe);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index 16a42cf82ba8..0fe5f2beefd5 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -213,6 +213,10 @@ struct dpll_info {
 	 * @name: DPLL name; used for logging
 	 */
 	const char *name;
+	/**
+	 * @id: unique indentifier for this DPLL; should match the index in the
+	 * dev_priv->shared_dplls array
+	 */
 	const int id;
 	/**
 	 * @funcs: platform specific hooks
@@ -243,12 +247,6 @@ struct intel_shared_dpll {
 	 */
 	bool on;
 
-	/**
-	 * @id: unique indentifier for this DPLL; should match the index in the
-	 * dev_priv->shared_dplls array
-	 */
-	enum intel_dpll_id id;
-
 	/**
 	 * @info: platform specific info
 	 */
-- 
2.14.3

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

[PATCH 6/7] drm/i915: use flags from dpll_info embedded in intel_shared_dpll

[Lucas De Marchi]

Replace all users of pll->flags to use pll->info.flags.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 53fce46380e0..1dfaf28bc641 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11649,7 +11649,7 @@ verify_single_dpll_state(struct drm_i915_private *dev_priv,
 
 	active = pll->info.funcs->get_hw_state(dev_priv, pll, &dpll_hw_state);
 
-	if (!(pll->flags & INTEL_DPLL_ALWAYS_ON)) {
+	if (!(pll->info.flags & INTEL_DPLL_ALWAYS_ON)) {
 		I915_STATE_WARN(!pll->on && pll->active_mask,
 		     "pll in active use but not on in sw tracking\n");
 		I915_STATE_WARN(pll->on && !pll->active_mask,
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 823752f725d3..436fca8efe8c 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2416,8 +2416,6 @@ void intel_shared_dpll_init(struct drm_device *dev)
 		WARN_ON(i != dpll_info[i].id);
 		memcpy(&dev_priv->shared_dplls[i].info, &dpll_info[i],
 		       sizeof(struct dpll_info));
-
-		dev_priv->shared_dplls[i].flags = dpll_info[i].flags;
 	}
 
 	dev_priv->dpll_mgr = dpll_mgr;
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index 0fe5f2beefd5..4f21fb9108b5 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -222,6 +222,14 @@ struct dpll_info {
 	 * @funcs: platform specific hooks
 	 */
 	const struct intel_shared_dpll_funcs *funcs;
+#define INTEL_DPLL_ALWAYS_ON	(1 << 0)
+	/**
+	 * @flags:
+	 *
+	 * INTEL_DPLL_ALWAYS_ON
+	 *     Inform the state checker that the DPLL is kept enabled even if
+	 *     not in use by any CRTC.
+	 */
 	uint32_t flags;
 };
 
@@ -251,16 +259,6 @@ struct intel_shared_dpll {
 	 * @info: platform specific info
 	 */
 	struct dpll_info info;
-
-#define INTEL_DPLL_ALWAYS_ON	(1 << 0)
-	/**
-	 * @flags:
-	 *
-	 * INTEL_DPLL_ALWAYS_ON
-	 *     Inform the state checker that the DPLL is kept enabled even if
-	 *     not in use by any CRTC.
-	 */
-	uint32_t flags;
 };
 
 #define SKL_DPLL0 0
-- 
2.14.3

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

[PATCH 7/7] drm/i915: rename dpll_info to intel_dpll_info

[Lucas De Marchi]

Now that the struct is moved to the header, let's add the proper prefix.

git grep -lz "struct dpll_info"  -- drivers/gpu/drm/i915/ | \
	xargs -0 sed -i 's/struct dpll_info/struct intel_dpll_info/g'

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 16 ++++++++--------
 drivers/gpu/drm/i915/intel_dpll_mgr.h |  6 +++---
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 436fca8efe8c..3fec3cfdc6af 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1882,7 +1882,7 @@ static void intel_ddi_pll_init(struct drm_device *dev)
 }
 
 struct intel_dpll_mgr {
-	const struct dpll_info *dpll_info;
+	const struct intel_dpll_info *dpll_info;
 
 	struct intel_shared_dpll *(*get_dpll)(struct intel_crtc *crtc,
 					      struct intel_crtc_state *crtc_state,
@@ -1892,7 +1892,7 @@ struct intel_dpll_mgr {
 			      struct intel_dpll_hw_state *hw_state);
 };
 
-static const struct dpll_info pch_plls[] = {
+static const struct intel_dpll_info pch_plls[] = {
 	{ "PCH DPLL A", DPLL_ID_PCH_PLL_A, &ibx_pch_dpll_funcs, 0 },
 	{ "PCH DPLL B", DPLL_ID_PCH_PLL_B, &ibx_pch_dpll_funcs, 0 },
 	{ NULL, -1, NULL, 0 },
@@ -1904,7 +1904,7 @@ static const struct intel_dpll_mgr pch_pll_mgr = {
 	.dump_hw_state = ibx_dump_hw_state,
 };
 
-static const struct dpll_info hsw_plls[] = {
+static const struct intel_dpll_info hsw_plls[] = {
 	{ "WRPLL 1",    DPLL_ID_WRPLL1,     &hsw_ddi_wrpll_funcs, 0 },
 	{ "WRPLL 2",    DPLL_ID_WRPLL2,     &hsw_ddi_wrpll_funcs, 0 },
 	{ "SPLL",       DPLL_ID_SPLL,       &hsw_ddi_spll_funcs,  0 },
@@ -1920,7 +1920,7 @@ static const struct intel_dpll_mgr hsw_pll_mgr = {
 	.dump_hw_state = hsw_dump_hw_state,
 };
 
-static const struct dpll_info skl_plls[] = {
+static const struct intel_dpll_info skl_plls[] = {
 	{ "DPLL 0", DPLL_ID_SKL_DPLL0, &skl_ddi_dpll0_funcs, INTEL_DPLL_ALWAYS_ON },
 	{ "DPLL 1", DPLL_ID_SKL_DPLL1, &skl_ddi_pll_funcs,   0 },
 	{ "DPLL 2", DPLL_ID_SKL_DPLL2, &skl_ddi_pll_funcs,   0 },
@@ -1934,7 +1934,7 @@ static const struct intel_dpll_mgr skl_pll_mgr = {
 	.dump_hw_state = skl_dump_hw_state,
 };
 
-static const struct dpll_info bxt_plls[] = {
+static const struct intel_dpll_info bxt_plls[] = {
 	{ "PORT PLL A", DPLL_ID_SKL_DPLL0, &bxt_ddi_pll_funcs, 0 },
 	{ "PORT PLL B", DPLL_ID_SKL_DPLL1, &bxt_ddi_pll_funcs, 0 },
 	{ "PORT PLL C", DPLL_ID_SKL_DPLL2, &bxt_ddi_pll_funcs, 0 },
@@ -2368,7 +2368,7 @@ static const struct intel_shared_dpll_funcs cnl_ddi_pll_funcs = {
 	.get_hw_state = cnl_ddi_pll_get_hw_state,
 };
 
-static const struct dpll_info cnl_plls[] = {
+static const struct intel_dpll_info cnl_plls[] = {
 	{ "DPLL 0", DPLL_ID_SKL_DPLL0, &cnl_ddi_pll_funcs, 0 },
 	{ "DPLL 1", DPLL_ID_SKL_DPLL1, &cnl_ddi_pll_funcs, 0 },
 	{ "DPLL 2", DPLL_ID_SKL_DPLL2, &cnl_ddi_pll_funcs, 0 },
@@ -2391,7 +2391,7 @@ void intel_shared_dpll_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	const struct intel_dpll_mgr *dpll_mgr = NULL;
-	const struct dpll_info *dpll_info;
+	const struct intel_dpll_info *dpll_info;
 	int i;
 
 	if (IS_CANNONLAKE(dev_priv))
@@ -2415,7 +2415,7 @@ void intel_shared_dpll_init(struct drm_device *dev)
 	for (i = 0; dpll_info[i].id >= 0; i++) {
 		WARN_ON(i != dpll_info[i].id);
 		memcpy(&dev_priv->shared_dplls[i].info, &dpll_info[i],
-		       sizeof(struct dpll_info));
+		       sizeof(struct intel_dpll_info));
 	}
 
 	dev_priv->dpll_mgr = dpll_mgr;
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index 4f21fb9108b5..e7ff45c11242 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -206,9 +206,9 @@ struct intel_shared_dpll_funcs {
 };
 
 /**
- * struct dpll_info - display PLL platform specific info
+ * struct intel_dpll_info - display PLL platform specific info
  */
-struct dpll_info {
+struct intel_dpll_info {
 	/**
 	 * @name: DPLL name; used for logging
 	 */
@@ -258,7 +258,7 @@ struct intel_shared_dpll {
 	/**
 	 * @info: platform specific info
 	 */
-	struct dpll_info info;
+	struct intel_dpll_info info;
 };
 
 #define SKL_DPLL0 0
-- 
2.14.3

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915: move dpll_info inside intel_shared_dpll

[Patchwork]

== Series Details ==

Series: drm/i915: move dpll_info inside intel_shared_dpll
URL   : https://patchwork.freedesktop.org/series/40251/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
7636902c0601 drm/i915: move dpll_info to header
1783e9c8ea30 drm/i915: embed dpll_info inside intel_shared_dpll
1f8a09f1818e drm/i915: use funcs from dpll_info embedded in intel_shared_dpll
-:24: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#24: FILE: drivers/gpu/drm/i915/intel_display.c:8772:
+		WARN_ON(!pll->info.funcs->get_hw_state(dev_priv, pll,
+						&pipe_config->dpll_hw_state));

-:35: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#35: FILE: drivers/gpu/drm/i915/intel_display.c:9249:
+		WARN_ON(!pll->info.funcs->get_hw_state(dev_priv, pll,
+						&pipe_config->dpll_hw_state));

total: 0 errors, 0 warnings, 2 checks, 105 lines checked
e4d2094fed15 drm/i915: use name from dpll_info embedded in intel_shared_dpll
e159ec0f240d drm/i915: use id from dpll_info embedded in intel_shared_dpll
0bad30025000 drm/i915: use flags from dpll_info embedded in intel_shared_dpll
ccf2665dd184 drm/i915: rename dpll_info to intel_dpll_info

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

✗ Fi.CI.BAT: failure for drm/i915: move dpll_info inside intel_shared_dpll

[Patchwork]

== Series Details ==

Series: drm/i915: move dpll_info inside intel_shared_dpll
URL   : https://patchwork.freedesktop.org/series/40251/
State : failure

== Summary ==

Series 40251v1 drm/i915: move dpll_info inside intel_shared_dpll
https://patchwork.freedesktop.org/api/1.0/series/40251/revisions/1/mbox/

---- Possible new issues:

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> DMESG-WARN (fi-kbl-7567u)
Test drv_module_reload:
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (fi-kbl-7567u)
        Subgroup basic-reload:
                pass       -> DMESG-WARN (fi-kbl-7567u)
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (fi-kbl-7567u)
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> INCOMPLETE (fi-hsw-4770)
Test kms_busy:
        Subgroup basic-flip-a:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-flip-b:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-flip-c:
                pass       -> SKIP       (fi-kbl-7567u)
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-flip-after-cursor-atomic:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-flip-after-cursor-legacy:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-flip-after-cursor-varying-size:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-flip-before-cursor-atomic:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-flip-before-cursor-legacy:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-flip-before-cursor-varying-size:
                pass       -> SKIP       (fi-kbl-7567u)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-plain-flip:
                pass       -> SKIP       (fi-kbl-7567u)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> SKIP       (fi-kbl-7567u)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup hang-read-crc-pipe-b:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup hang-read-crc-pipe-c:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup nonblocking-crc-pipe-a:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup nonblocking-crc-pipe-b:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup nonblocking-crc-pipe-c:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup read-crc-pipe-a:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup read-crc-pipe-b:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup read-crc-pipe-c:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup read-crc-pipe-c-frame-sequence:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> SKIP       (fi-kbl-7567u)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> SKIP       (fi-kbl-7567u)
        Subgroup basic-rte:
                pass       -> SKIP       (fi-kbl-7567u)
Test prime_vgem:
        Subgroup basic-fence-flip:
                pass       -> SKIP       (fi-kbl-7567u)

WARNING: Long output truncated

141def2a45f4a3ad7c7e9144cd26e97bb1298397 drm-tip: 2018y-03m-19d-23h-48m-43s UTC integration manifest
ccf2665dd184 drm/i915: rename dpll_info to intel_dpll_info
0bad30025000 drm/i915: use flags from dpll_info embedded in intel_shared_dpll
e159ec0f240d drm/i915: use id from dpll_info embedded in intel_shared_dpll
e4d2094fed15 drm/i915: use name from dpll_info embedded in intel_shared_dpll
1f8a09f1818e drm/i915: use funcs from dpll_info embedded in intel_shared_dpll
1783e9c8ea30 drm/i915: embed dpll_info inside intel_shared_dpll
7636902c0601 drm/i915: move dpll_info to header

== Logs ==

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

Re: [PATCH 1/7] drm/i915: move dpll_info to header

[Ville Syrjälä]

On Mon, Mar 19, 2018 at 11:24:17PM -0700, Lucas De Marchi wrote:
> This will allow the struct to be embedded in intel_shared_dpll.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c |  7 -------
>  drivers/gpu/drm/i915/intel_dpll_mgr.h | 10 ++++++++++
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 51c5ae4e9116..52d6e731c3e9 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -1877,13 +1877,6 @@ static void intel_ddi_pll_init(struct drm_device *dev)
>  	}
>  }
>  
> -struct dpll_info {
> -	const char *name;
> -	const int id;
> -	const struct intel_shared_dpll_funcs *funcs;
> -	uint32_t flags;
> -};
> -
>  struct intel_dpll_mgr {
>  	const struct dpll_info *dpll_info;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> index f24ccf443d25..e99d6385478a 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> @@ -205,6 +205,16 @@ struct intel_shared_dpll_funcs {
>  			     struct intel_dpll_hw_state *hw_state);
>  };
>  
> +/**
> + * struct dpll_info - display PLL platform specific info
> + */
> +struct dpll_info {
> +	const char *name;
> +	const int id;

The const here seems pointless. Well, I guess if we go with the copy
then it might not be. Although then we get to wonder why 'flags' isn't
const.

> +	const struct intel_shared_dpll_funcs *funcs;
> +	uint32_t flags;

This structure seems to be poorly organized for 64bit machines.

> +};
> +
>  /**
>   * struct intel_shared_dpll - display PLL with tracked state and users
>   */
> -- 
> 2.14.3

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

Re: [PATCH 0/7] drm/i915: move dpll_info inside intel_shared_dpll

[Ville Syrjälä]

On Mon, Mar 19, 2018 at 11:24:16PM -0700, Lucas De Marchi wrote:
> 
> This is an alternative to my previous patch
> "drm/i915: Remove hole and padding from intel_shared_dpll".
> 
> Not sure if I split this too much, but I think it's easier to review
> this way. We can always squash them if wanted.
> 
> As can be seen below this reduces .text and size of our structs, but
> maybe more important it puts the const data together. 
> 
>    text	   data	    bss	    dec	    hex	filename
> 1753856	  69541	   5316	1828713	 1be769	drivers/gpu/drm/i915/i915.ko
> 1753668	  69541	   5316	1828525	 1be6ad	drivers/gpu/drm/i915/i915.ko.new
> 
> Ville, instead of adding a pointer I decided to copy the struct, just
> because I thought we were going through too many indirections while for
> example accesssing pll->info->funcs->get_hw_state().

I don't particularly like the copy since it means no const.

> 
> struct sizes:
> 		       before	after
> intel_shared_dpll	  152	  128
> i915_drm_private	32112	31968
> 
> Lucas De Marchi (7):
>   drm/i915: move dpll_info to header
>   drm/i915: embed dpll_info inside intel_shared_dpll
>   drm/i915: use funcs from dpll_info embedded in intel_shared_dpll
>   drm/i915: use name from dpll_info embedded in intel_shared_dpll
>   drm/i915: use id from dpll_info embedded in intel_shared_dpll
>   drm/i915: use flags from dpll_info embedded in intel_shared_dpll
>   drm/i915: rename dpll_info to intel_dpll_info
> 
>  drivers/gpu/drm/i915/i915_debugfs.c   |   3 +-
>  drivers/gpu/drm/i915/intel_ddi.c      |   8 +-
>  drivers/gpu/drm/i915/intel_display.c  |  25 ++---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 198 +++++++++++++++++-----------------
>  drivers/gpu/drm/i915/intel_dpll_mgr.h |  53 +++++----
>  5 files changed, 145 insertions(+), 142 deletions(-)
> 
> -- 
> 2.14.3

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

Re: [PATCH 1/7] drm/i915: move dpll_info to header

[Lucas De Marchi]

On Tue, Mar 20, 2018 at 2:56 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Mon, Mar 19, 2018 at 11:24:17PM -0700, Lucas De Marchi wrote:
>> This will allow the struct to be embedded in intel_shared_dpll.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dpll_mgr.c |  7 -------
>>  drivers/gpu/drm/i915/intel_dpll_mgr.h | 10 ++++++++++
>>  2 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> index 51c5ae4e9116..52d6e731c3e9 100644
>> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> @@ -1877,13 +1877,6 @@ static void intel_ddi_pll_init(struct drm_device *dev)
>>       }
>>  }
>>
>> -struct dpll_info {
>> -     const char *name;
>> -     const int id;
>> -     const struct intel_shared_dpll_funcs *funcs;
>> -     uint32_t flags;
>> -};
>> -
>>  struct intel_dpll_mgr {
>>       const struct dpll_info *dpll_info;
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
>> index f24ccf443d25..e99d6385478a 100644
>> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
>> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
>> @@ -205,6 +205,16 @@ struct intel_shared_dpll_funcs {
>>                            struct intel_dpll_hw_state *hw_state);
>>  };
>>
>> +/**
>> + * struct dpll_info - display PLL platform specific info
>> + */
>> +struct dpll_info {
>> +     const char *name;
>> +     const int id;
>
> The const here seems pointless. Well, I guess if we go with the copy
> then it might not be. Although then we get to wonder why 'flags' isn't
> const.

Here I wanted to do just a code move. Any change would be on top. Since it seems
the pointer approach is preferred, I'm just going ahead and dropping this.

>
>> +     const struct intel_shared_dpll_funcs *funcs;
>> +     uint32_t flags;
>
> This structure seems to be poorly organized for 64bit machines.


Yes, there's a 4-bytes  right there.  Fixing this unfortunately
involves changing
all tables inside dpll_mgr.c. I'm sending the reordering as the last
patch in v2.

thanks
Lucas De Marchi

>
>> +};
>> +
>>  /**
>>   * struct intel_shared_dpll - display PLL with tracked state and users
>>   */
>> --
>> 2.14.3
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> 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

Re: [PATCH 1/7] drm/i915: move dpll_info to header

[Ville Syrjälä]

On Tue, Mar 20, 2018 at 02:50:26PM -0700, Lucas De Marchi wrote:
> On Tue, Mar 20, 2018 at 2:56 AM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Mon, Mar 19, 2018 at 11:24:17PM -0700, Lucas De Marchi wrote:
> >> This will allow the struct to be embedded in intel_shared_dpll.
> >>
> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_dpll_mgr.c |  7 -------
> >>  drivers/gpu/drm/i915/intel_dpll_mgr.h | 10 ++++++++++
> >>  2 files changed, 10 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> >> index 51c5ae4e9116..52d6e731c3e9 100644
> >> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> >> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> >> @@ -1877,13 +1877,6 @@ static void intel_ddi_pll_init(struct drm_device *dev)
> >>       }
> >>  }
> >>
> >> -struct dpll_info {
> >> -     const char *name;
> >> -     const int id;
> >> -     const struct intel_shared_dpll_funcs *funcs;
> >> -     uint32_t flags;
> >> -};
> >> -
> >>  struct intel_dpll_mgr {
> >>       const struct dpll_info *dpll_info;
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> >> index f24ccf443d25..e99d6385478a 100644
> >> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> >> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> >> @@ -205,6 +205,16 @@ struct intel_shared_dpll_funcs {
> >>                            struct intel_dpll_hw_state *hw_state);
> >>  };
> >>
> >> +/**
> >> + * struct dpll_info - display PLL platform specific info
> >> + */
> >> +struct dpll_info {
> >> +     const char *name;
> >> +     const int id;
> >
> > The const here seems pointless. Well, I guess if we go with the copy
> > then it might not be. Although then we get to wonder why 'flags' isn't
> > const.
> 
> Here I wanted to do just a code move. Any change would be on top. Since it seems
> the pointer approach is preferred, I'm just going ahead and dropping this.
> 
> >
> >> +     const struct intel_shared_dpll_funcs *funcs;
> >> +     uint32_t flags;
> >
> > This structure seems to be poorly organized for 64bit machines.
> 
> 
> Yes, there's a 4-bytes  right there.  Fixing this unfortunately
> involves changing
> all tables inside dpll_mgr.c.

Those aren't using named initializers?

> I'm sending the reordering as the last
> patch in v2.
> 
> thanks
> Lucas De Marchi
> 
> >
> >> +};
> >> +
> >>  /**
> >>   * struct intel_shared_dpll - display PLL with tracked state and users
> >>   */
> >> --
> >> 2.14.3
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Lucas De Marchi

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

Re: [PATCH 1/7] drm/i915: move dpll_info to header

[Lucas De Marchi]

On Wed, Mar 21, 2018 at 12:25:40PM +0200, Ville Syrjälä wrote:
> > > This structure seems to be poorly organized for 64bit machines.
> > 
> > 
> > Yes, there's a 4-bytes  right there.  Fixing this unfortunately
> > involves changing
> > all tables inside dpll_mgr.c.
> 
> Those aren't using named initializers?

No. Most of the tables aren't using named initializers. On v2 of the
patch set I did the reordering, but without adding named initializers.

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