[PATCH 0/6] Move dpio access out of intel_display.c

[PATCH 0/6] Move dpio access out of intel_display.c

[Ander Conselvan de Oliveira]

Hi,

This series moves all of the calls to vlv_dpio_{read,write} to
intel_dpio_phy.c. I think it simplifies the surrounding code a bit.

Thanks,
Ander

Ander Conselvan de Oliveira (6):
  drm/i915: Rename struct dpll to struct intel_dpll
  drm/i915: Move dpio code of VLV/CHV dpll enabling to intel_dpio_phy.c
  drm/i915: Merge vlv/chv _prepare_pll() with their enable counterpart
  drm/i915: Move VLV divider readout to intel_dpio_phy.c
  drm/i915: Move CHV divider readout to intel_dpio_phy.c
  drm/i915: Move toggling of CHV DPIO_DCLKP_EN to intel_dpio_phy.c

 drivers/gpu/drm/i915/i915_drv.h       |  14 +-
 drivers/gpu/drm/i915/intel_ddi.c      |   2 +-
 drivers/gpu/drm/i915/intel_display.c  | 395 ++++++----------------------------
 drivers/gpu/drm/i915/intel_dp.c       |   2 +-
 drivers/gpu/drm/i915/intel_dpio_phy.c | 260 ++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dpll_mgr.c |   2 +-
 drivers/gpu/drm/i915/intel_drv.h      |  10 +-
 drivers/gpu/drm/i915/intel_sdvo.c     |   2 +-
 8 files changed, 350 insertions(+), 337 deletions(-)

-- 
2.5.5

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

[PATCH 1/6] drm/i915: Rename struct dpll to struct intel_dpll

[Ander Conselvan de Oliveira]

Prefix struct dpll with intel_ to follow the convention in the driver.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  2 +-
 drivers/gpu/drm/i915/intel_ddi.c      |  2 +-
 drivers/gpu/drm/i915/intel_display.c  | 76 +++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_dp.c       |  2 +-
 drivers/gpu/drm/i915/intel_dpll_mgr.c |  2 +-
 drivers/gpu/drm/i915/intel_drv.h      | 10 ++---
 drivers/gpu/drm/i915/intel_sdvo.c     |  2 +-
 7 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7a0b513..5f9dda2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -577,7 +577,7 @@ struct intel_crtc_state;
 struct intel_initial_plane_config;
 struct intel_crtc;
 struct intel_limit;
-struct dpll;
+struct intel_dpll;
 
 struct drm_i915_display_funcs {
 	int (*get_display_clock_speed)(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c454744..1387acd 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -948,7 +948,7 @@ static int bxt_calc_pll_link(struct drm_i915_private *dev_priv,
 {
 	struct intel_shared_dpll *pll;
 	struct intel_dpll_hw_state *state;
-	struct dpll clock;
+	struct intel_dpll clock;
 
 	/* For DDI ports we always use a shared PLL. */
 	if (WARN_ON(dpll == DPLL_ID_PRIVATE))
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 05c7533..048a5bf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -578,7 +578,7 @@ static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
  * divided-down version of it.
  */
 /* m1 is reserved as 0 in Pineview, n is a ring counter */
-static int pnv_calc_dpll_params(int refclk, struct dpll *clock)
+static int pnv_calc_dpll_params(int refclk, struct intel_dpll *clock)
 {
 	clock->m = clock->m2 + 2;
 	clock->p = clock->p1 * clock->p2;
@@ -590,12 +590,12 @@ static int pnv_calc_dpll_params(int refclk, struct dpll *clock)
 	return clock->dot;
 }
 
-static uint32_t i9xx_dpll_compute_m(struct dpll *dpll)
+static uint32_t i9xx_dpll_compute_m(struct intel_dpll *dpll)
 {
 	return 5 * (dpll->m1 + 2) + (dpll->m2 + 2);
 }
 
-static int i9xx_calc_dpll_params(int refclk, struct dpll *clock)
+static int i9xx_calc_dpll_params(int refclk, struct intel_dpll *clock)
 {
 	clock->m = i9xx_dpll_compute_m(clock);
 	clock->p = clock->p1 * clock->p2;
@@ -607,7 +607,7 @@ static int i9xx_calc_dpll_params(int refclk, struct dpll *clock)
 	return clock->dot;
 }
 
-static int vlv_calc_dpll_params(int refclk, struct dpll *clock)
+static int vlv_calc_dpll_params(int refclk, struct intel_dpll *clock)
 {
 	clock->m = clock->m1 * clock->m2;
 	clock->p = clock->p1 * clock->p2;
@@ -619,7 +619,7 @@ static int vlv_calc_dpll_params(int refclk, struct dpll *clock)
 	return clock->dot / 5;
 }
 
-int chv_calc_dpll_params(int refclk, struct dpll *clock)
+int chv_calc_dpll_params(int refclk, struct intel_dpll *clock)
 {
 	clock->m = clock->m1 * clock->m2;
 	clock->p = clock->p1 * clock->p2;
@@ -640,7 +640,7 @@ int chv_calc_dpll_params(int refclk, struct dpll *clock)
 
 static bool intel_PLL_is_valid(struct drm_device *dev,
 			       const struct intel_limit *limit,
-			       const struct dpll *clock)
+			       const struct intel_dpll *clock)
 {
 	if (clock->n   < limit->n.min   || limit->n.max   < clock->n)
 		INTELPllInvalid("n out of range\n");
@@ -712,11 +712,11 @@ i9xx_select_p2_div(const struct intel_limit *limit,
 static bool
 i9xx_find_best_dpll(const struct intel_limit *limit,
 		    struct intel_crtc_state *crtc_state,
-		    int target, int refclk, struct dpll *match_clock,
-		    struct dpll *best_clock)
+		    int target, int refclk, struct intel_dpll *match_clock,
+		    struct intel_dpll *best_clock)
 {
 	struct drm_device *dev = crtc_state->base.crtc->dev;
-	struct dpll clock;
+	struct intel_dpll clock;
 	int err = target;
 
 	memset(best_clock, 0, sizeof(*best_clock));
@@ -769,11 +769,11 @@ i9xx_find_best_dpll(const struct intel_limit *limit,
 static bool
 pnv_find_best_dpll(const struct intel_limit *limit,
 		   struct intel_crtc_state *crtc_state,
-		   int target, int refclk, struct dpll *match_clock,
-		   struct dpll *best_clock)
+		   int target, int refclk, struct intel_dpll *match_clock,
+		   struct intel_dpll *best_clock)
 {
 	struct drm_device *dev = crtc_state->base.crtc->dev;
-	struct dpll clock;
+	struct intel_dpll clock;
 	int err = target;
 
 	memset(best_clock, 0, sizeof(*best_clock));
@@ -824,11 +824,11 @@ pnv_find_best_dpll(const struct intel_limit *limit,
 static bool
 g4x_find_best_dpll(const struct intel_limit *limit,
 		   struct intel_crtc_state *crtc_state,
-		   int target, int refclk, struct dpll *match_clock,
-		   struct dpll *best_clock)
+		   int target, int refclk, struct intel_dpll *match_clock,
+		   struct intel_dpll *best_clock)
 {
 	struct drm_device *dev = crtc_state->base.crtc->dev;
-	struct dpll clock;
+	struct intel_dpll clock;
 	int max_n;
 	bool found = false;
 	/* approximately equals target * 0.00585 */
@@ -874,8 +874,8 @@ g4x_find_best_dpll(const struct intel_limit *limit,
  * best configuration and error found so far. Return the calculated error.
  */
 static bool vlv_PLL_is_optimal(struct drm_device *dev, int target_freq,
-			       const struct dpll *calculated_clock,
-			       const struct dpll *best_clock,
+			       const struct intel_dpll *calculated_clock,
+			       const struct intel_dpll *best_clock,
 			       unsigned int best_error_ppm,
 			       unsigned int *error_ppm)
 {
@@ -917,12 +917,12 @@ static bool vlv_PLL_is_optimal(struct drm_device *dev, int target_freq,
 static bool
 vlv_find_best_dpll(const struct intel_limit *limit,
 		   struct intel_crtc_state *crtc_state,
-		   int target, int refclk, struct dpll *match_clock,
-		   struct dpll *best_clock)
+		   int target, int refclk, struct intel_dpll *match_clock,
+		   struct intel_dpll *best_clock)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
-	struct dpll clock;
+	struct intel_dpll clock;
 	unsigned int bestppm = 1000000;
 	/* min update 19.2 MHz */
 	int max_n = min(limit->n.max, refclk / 19200);
@@ -976,13 +976,13 @@ vlv_find_best_dpll(const struct intel_limit *limit,
 static bool
 chv_find_best_dpll(const struct intel_limit *limit,
 		   struct intel_crtc_state *crtc_state,
-		   int target, int refclk, struct dpll *match_clock,
-		   struct dpll *best_clock)
+		   int target, int refclk, struct intel_dpll *match_clock,
+		   struct intel_dpll *best_clock)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	unsigned int best_error_ppm;
-	struct dpll clock;
+	struct intel_dpll clock;
 	uint64_t m2;
 	int found = false;
 
@@ -1032,7 +1032,7 @@ chv_find_best_dpll(const struct intel_limit *limit,
 }
 
 bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
-			struct dpll *best_clock)
+			struct intel_dpll *best_clock)
 {
 	int refclk = 100000;
 	const struct intel_limit *limit = &intel_limits_bxt;
@@ -7041,19 +7041,19 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
 }
 
-static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
+static uint32_t pnv_dpll_compute_fp(struct intel_dpll *dpll)
 {
 	return (1 << dpll->n) << 16 | dpll->m2;
 }
 
-static uint32_t i9xx_dpll_compute_fp(struct dpll *dpll)
+static uint32_t i9xx_dpll_compute_fp(struct intel_dpll *dpll)
 {
 	return dpll->n << 16 | dpll->m1 << 8 | dpll->m2;
 }
 
 static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
 				     struct intel_crtc_state *crtc_state,
-				     struct dpll *reduced_clock)
+				     struct intel_dpll *reduced_clock)
 {
 	struct drm_device *dev = crtc->base.dev;
 	u32 fp, fp2 = 0;
@@ -7430,7 +7430,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
  * be enabled.
  */
 int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
-		     const struct dpll *dpll)
+		     const struct intel_dpll *dpll)
 {
 	struct intel_crtc *crtc =
 		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
@@ -7477,13 +7477,13 @@ void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe)
 
 static void i9xx_compute_dpll(struct intel_crtc *crtc,
 			      struct intel_crtc_state *crtc_state,
-			      struct dpll *reduced_clock)
+			      struct intel_dpll *reduced_clock)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 dpll;
 	bool is_sdvo;
-	struct dpll *clock = &crtc_state->dpll;
+	struct intel_dpll *clock = &crtc_state->dpll;
 
 	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
 
@@ -7553,12 +7553,12 @@ static void i9xx_compute_dpll(struct intel_crtc *crtc,
 
 static void i8xx_compute_dpll(struct intel_crtc *crtc,
 			      struct intel_crtc_state *crtc_state,
-			      struct dpll *reduced_clock)
+			      struct intel_dpll *reduced_clock)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 dpll;
-	struct dpll *clock = &crtc_state->dpll;
+	struct intel_dpll *clock = &crtc_state->dpll;
 
 	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
 
@@ -8024,7 +8024,7 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe = pipe_config->cpu_transcoder;
-	struct dpll clock;
+	struct intel_dpll clock;
 	u32 mdiv;
 	int refclk = 100000;
 
@@ -8121,7 +8121,7 @@ static void chv_crtc_clock_get(struct intel_crtc *crtc,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe = pipe_config->cpu_transcoder;
 	enum dpio_channel port = vlv_pipe_to_channel(pipe);
-	struct dpll clock;
+	struct intel_dpll clock;
 	u32 cmn_dw13, pll_dw0, pll_dw1, pll_dw2, pll_dw3;
 	int refclk = 100000;
 
@@ -8777,14 +8777,14 @@ int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp)
 	return DIV_ROUND_UP(bps, link_bw * 8);
 }
 
-static bool ironlake_needs_fb_cb_tune(struct dpll *dpll, int factor)
+static bool ironlake_needs_fb_cb_tune(struct intel_dpll *dpll, int factor)
 {
 	return i9xx_dpll_compute_m(dpll) < factor * dpll->n;
 }
 
 static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 				  struct intel_crtc_state *crtc_state,
-				  struct dpll *reduced_clock)
+				  struct intel_dpll *reduced_clock)
 {
 	struct drm_crtc *crtc = &intel_crtc->base;
 	struct drm_device *dev = crtc->dev;
@@ -8892,7 +8892,7 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct dpll reduced_clock;
+	struct intel_dpll reduced_clock;
 	bool has_reduced_clock = false;
 	struct intel_shared_dpll *pll;
 	const struct intel_limit *limit;
@@ -10622,7 +10622,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 	int pipe = pipe_config->cpu_transcoder;
 	u32 dpll = pipe_config->dpll_hw_state.dpll;
 	u32 fp;
-	struct dpll clock;
+	struct intel_dpll clock;
 	int port_clock;
 	int refclk = i9xx_pll_refclk(dev, pipe_config);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3633002..e85711a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -49,7 +49,7 @@
 
 struct dp_link_dpll {
 	int clock;
-	struct dpll dpll;
+	struct intel_dpll dpll;
 };
 
 static const struct dp_link_dpll gen4_dpll[] = {
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index c283ba4..3499ed2 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1508,7 +1508,7 @@ bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	int clock = crtc_state->port_clock;
 
 	if (encoder->type == INTEL_OUTPUT_HDMI) {
-		struct dpll best_clock;
+		struct intel_dpll best_clock;
 
 		/* Calculate HDMI div */
 		/*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8405ff7..2033ff1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -266,7 +266,7 @@ struct intel_connector {
 	struct intel_dp *mst_port;
 };
 
-struct dpll {
+struct intel_dpll {
 	/* given values */
 	int n;
 	int m1, m2;
@@ -484,7 +484,7 @@ struct intel_crtc_state {
 
 	/* Settings for the intel dpll used on pretty much everything but
 	 * haswell. */
-	struct dpll dpll;
+	struct intel_dpll dpll;
 
 	/* Selected dpll when shared or NULL. */
 	struct intel_shared_dpll *shared_dpll;
@@ -1206,7 +1206,7 @@ void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
 				    enum pipe pipe);
 
 int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
-		     const struct dpll *dpll);
+		     const struct intel_dpll *dpll);
 void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe);
 int lpt_get_iclkip(struct drm_i915_private *dev_priv);
 
@@ -1255,8 +1255,8 @@ void intel_dp_get_m_n(struct intel_crtc *crtc,
 void intel_dp_set_m_n(struct intel_crtc *crtc, 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,
-			struct dpll *best_clock);
-int chv_calc_dpll_params(int refclk, struct dpll *pll_clock);
+			struct intel_dpll *best_clock);
+int chv_calc_dpll_params(int refclk, struct intel_dpll *pll_clock);
 
 bool intel_crtc_active(struct drm_crtc *crtc);
 void hsw_enable_ips(struct intel_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 2128fae..96bedd0 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1101,7 +1101,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
 static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state *pipe_config)
 {
 	unsigned dotclock = pipe_config->port_clock;
-	struct dpll *clock = &pipe_config->dpll;
+	struct intel_dpll *clock = &pipe_config->dpll;
 
 	/* SDVO TV has fixed PLL values depend on its clock range,
 	   this mirrors vbios setting. */
-- 
2.5.5

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

[PATCH 2/6] drm/i915: Move dpio code of VLV/CHV dpll enabling to intel_dpio_phy.c

[Ander Conselvan de Oliveira]

Hide the dpio details of setting up the dplls on VLV/CHV to
intel_dpio_phy.c. This will allow the prepare and enable pll functions
to be merged in a follow up. It also creates a better split of the code
where most of the dpio access are concentrated in one place.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |   6 +
 drivers/gpu/drm/i915/intel_display.c  | 207 ++--------------------------------
 drivers/gpu/drm/i915/intel_dpio_phy.c | 202 +++++++++++++++++++++++++++++++++
 3 files changed, 217 insertions(+), 198 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5f9dda2..99dfacd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3627,6 +3627,9 @@ void chv_phy_pre_pll_enable(struct intel_encoder *encoder);
 void chv_phy_pre_encoder_enable(struct intel_encoder *encoder);
 void chv_phy_release_cl2_override(struct intel_encoder *encoder);
 void chv_phy_post_pll_disable(struct intel_encoder *encoder);
+void chv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
+			 u32 bestm1, u32 bestm2, u32 bestp1, u32 bestp2,
+			 int vco);
 
 void vlv_set_phy_signal_level(struct intel_encoder *encoder,
 			      u32 demph_reg_value, u32 preemph_reg_value,
@@ -3634,6 +3637,9 @@ void vlv_set_phy_signal_level(struct intel_encoder *encoder,
 void vlv_phy_pre_pll_enable(struct intel_encoder *encoder);
 void vlv_phy_pre_encoder_enable(struct intel_encoder *encoder);
 void vlv_phy_reset_lanes(struct intel_encoder *encoder);
+void vlv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
+			 u32 bestm1, u32 bestm2, u32 bestp1, u32 bestp2,
+			 int port_clock, bool dp);
 
 int intel_gpu_freq(struct drm_i915_private *dev_priv, int val);
 int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 048a5bf..06d9b96 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7080,35 +7080,6 @@ static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
 	}
 }
 
-static void vlv_pllb_recal_opamp(struct drm_i915_private *dev_priv, enum pipe
-		pipe)
-{
-	u32 reg_val;
-
-	/*
-	 * PLLB opamp always calibrates to max value of 0x3f, force enable it
-	 * and set it to a reasonable value instead.
-	 */
-	reg_val = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW9(1));
-	reg_val &= 0xffffff00;
-	reg_val |= 0x00000030;
-	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW9(1), reg_val);
-
-	reg_val = vlv_dpio_read(dev_priv, pipe, VLV_REF_DW13);
-	reg_val &= 0x8cffffff;
-	reg_val = 0x8c000000;
-	vlv_dpio_write(dev_priv, pipe, VLV_REF_DW13, reg_val);
-
-	reg_val = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW9(1));
-	reg_val &= 0xffffff00;
-	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW9(1), reg_val);
-
-	reg_val = vlv_dpio_read(dev_priv, pipe, VLV_REF_DW13);
-	reg_val &= 0x00ffffff;
-	reg_val |= 0xb0000000;
-	vlv_dpio_write(dev_priv, pipe, VLV_REF_DW13, reg_val);
-}
-
 static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
 					 struct intel_link_m_n *m_n)
 {
@@ -7220,9 +7191,6 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum pipe pipe = crtc->pipe;
-	u32 mdiv;
-	u32 bestn, bestm1, bestm2, bestp1, bestp2;
-	u32 coreclk, reg_val;
 
 	/* Enable Refclk */
 	I915_WRITE(DPLL(pipe),
@@ -7233,85 +7201,12 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0)
 		return;
 
-	mutex_lock(&dev_priv->sb_lock);
-
-	bestn = pipe_config->dpll.n;
-	bestm1 = pipe_config->dpll.m1;
-	bestm2 = pipe_config->dpll.m2;
-	bestp1 = pipe_config->dpll.p1;
-	bestp2 = pipe_config->dpll.p2;
-
-	/* See eDP HDMI DPIO driver vbios notes doc */
-
-	/* PLL B needs special handling */
-	if (pipe == PIPE_B)
-		vlv_pllb_recal_opamp(dev_priv, pipe);
-
-	/* Set up Tx target for periodic Rcomp update */
-	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW9_BCAST, 0x0100000f);
+	vlv_phy_prepare_pll(crtc, pipe_config->dpll.n,
+			    pipe_config->dpll.m1, pipe_config->dpll.m2,
+			    pipe_config->dpll.p1, pipe_config->dpll.p2,
+			    pipe_config->port_clock,
+			    pipe_config->has_dp_encoder);
 
-	/* Disable target IRef on PLL */
-	reg_val = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW8(pipe));
-	reg_val &= 0x00ffffff;
-	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW8(pipe), reg_val);
-
-	/* Disable fast lock */
-	vlv_dpio_write(dev_priv, pipe, VLV_CMN_DW0, 0x610);
-
-	/* Set idtafcrecal before PLL is enabled */
-	mdiv = ((bestm1 << DPIO_M1DIV_SHIFT) | (bestm2 & DPIO_M2DIV_MASK));
-	mdiv |= ((bestp1 << DPIO_P1_SHIFT) | (bestp2 << DPIO_P2_SHIFT));
-	mdiv |= ((bestn << DPIO_N_SHIFT));
-	mdiv |= (1 << DPIO_K_SHIFT);
-
-	/*
-	 * Post divider depends on pixel clock rate, DAC vs digital (and LVDS,
-	 * but we don't support that).
-	 * Note: don't use the DAC post divider as it seems unstable.
-	 */
-	mdiv |= (DPIO_POST_DIV_HDMIDP << DPIO_POST_DIV_SHIFT);
-	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW3(pipe), mdiv);
-
-	mdiv |= DPIO_ENABLE_CALIBRATION;
-	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW3(pipe), mdiv);
-
-	/* Set HBR and RBR LPF coefficients */
-	if (pipe_config->port_clock == 162000 ||
-	    intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG) ||
-	    intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
-		vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW10(pipe),
-				 0x009f0003);
-	else
-		vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW10(pipe),
-				 0x00d0000f);
-
-	if (pipe_config->has_dp_encoder) {
-		/* Use SSC source */
-		if (pipe == PIPE_A)
-			vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW5(pipe),
-					 0x0df40000);
-		else
-			vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW5(pipe),
-					 0x0df70000);
-	} else { /* HDMI or VGA */
-		/* Use bend source */
-		if (pipe == PIPE_A)
-			vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW5(pipe),
-					 0x0df70000);
-		else
-			vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW5(pipe),
-					 0x0df40000);
-	}
-
-	coreclk = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW7(pipe));
-	coreclk = (coreclk & 0x0000ff00) | 0x01c00000;
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT) ||
-	    intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))
-		coreclk |= 0x01000000;
-	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW7(pipe), coreclk);
-
-	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW11(pipe), 0x87871000);
-	mutex_unlock(&dev_priv->sb_lock);
 }
 
 static void chv_prepare_pll(struct intel_crtc *crtc,
@@ -7320,11 +7215,6 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum pipe pipe = crtc->pipe;
-	enum dpio_channel port = vlv_pipe_to_channel(pipe);
-	u32 loopfilter, tribuf_calcntr;
-	u32 bestn, bestm1, bestm2, bestp1, bestp2, bestm2_frac;
-	u32 dpio_val;
-	int vco;
 
 	/* Enable Refclk and SSC */
 	I915_WRITE(DPLL(pipe),
@@ -7334,89 +7224,10 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
 	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0)
 		return;
 
-	bestn = pipe_config->dpll.n;
-	bestm2_frac = pipe_config->dpll.m2 & 0x3fffff;
-	bestm1 = pipe_config->dpll.m1;
-	bestm2 = pipe_config->dpll.m2 >> 22;
-	bestp1 = pipe_config->dpll.p1;
-	bestp2 = pipe_config->dpll.p2;
-	vco = pipe_config->dpll.vco;
-	dpio_val = 0;
-	loopfilter = 0;
-
-	mutex_lock(&dev_priv->sb_lock);
-
-	/* p1 and p2 divider */
-	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW13(port),
-			5 << DPIO_CHV_S1_DIV_SHIFT |
-			bestp1 << DPIO_CHV_P1_DIV_SHIFT |
-			bestp2 << DPIO_CHV_P2_DIV_SHIFT |
-			1 << DPIO_CHV_K_DIV_SHIFT);
-
-	/* Feedback post-divider - m2 */
-	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW0(port), bestm2);
-
-	/* Feedback refclk divider - n and m1 */
-	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW1(port),
-			DPIO_CHV_M1_DIV_BY_2 |
-			1 << DPIO_CHV_N_DIV_SHIFT);
-
-	/* M2 fraction division */
-	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW2(port), bestm2_frac);
-
-	/* M2 fraction division enable */
-	dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW3(port));
-	dpio_val &= ~(DPIO_CHV_FEEDFWD_GAIN_MASK | DPIO_CHV_FRAC_DIV_EN);
-	dpio_val |= (2 << DPIO_CHV_FEEDFWD_GAIN_SHIFT);
-	if (bestm2_frac)
-		dpio_val |= DPIO_CHV_FRAC_DIV_EN;
-	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW3(port), dpio_val);
-
-	/* Program digital lock detect threshold */
-	dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW9(port));
-	dpio_val &= ~(DPIO_CHV_INT_LOCK_THRESHOLD_MASK |
-					DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE);
-	dpio_val |= (0x5 << DPIO_CHV_INT_LOCK_THRESHOLD_SHIFT);
-	if (!bestm2_frac)
-		dpio_val |= DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE;
-	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW9(port), dpio_val);
-
-	/* Loop filter */
-	if (vco == 5400000) {
-		loopfilter |= (0x3 << DPIO_CHV_PROP_COEFF_SHIFT);
-		loopfilter |= (0x8 << DPIO_CHV_INT_COEFF_SHIFT);
-		loopfilter |= (0x1 << DPIO_CHV_GAIN_CTRL_SHIFT);
-		tribuf_calcntr = 0x9;
-	} else if (vco <= 6200000) {
-		loopfilter |= (0x5 << DPIO_CHV_PROP_COEFF_SHIFT);
-		loopfilter |= (0xB << DPIO_CHV_INT_COEFF_SHIFT);
-		loopfilter |= (0x3 << DPIO_CHV_GAIN_CTRL_SHIFT);
-		tribuf_calcntr = 0x9;
-	} else if (vco <= 6480000) {
-		loopfilter |= (0x4 << DPIO_CHV_PROP_COEFF_SHIFT);
-		loopfilter |= (0x9 << DPIO_CHV_INT_COEFF_SHIFT);
-		loopfilter |= (0x3 << DPIO_CHV_GAIN_CTRL_SHIFT);
-		tribuf_calcntr = 0x8;
-	} else {
-		/* Not supported. Apply the same limits as in the max case */
-		loopfilter |= (0x4 << DPIO_CHV_PROP_COEFF_SHIFT);
-		loopfilter |= (0x9 << DPIO_CHV_INT_COEFF_SHIFT);
-		loopfilter |= (0x3 << DPIO_CHV_GAIN_CTRL_SHIFT);
-		tribuf_calcntr = 0;
-	}
-	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW6(port), loopfilter);
-
-	dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW8(port));
-	dpio_val &= ~DPIO_CHV_TDC_TARGET_CNT_MASK;
-	dpio_val |= (tribuf_calcntr << DPIO_CHV_TDC_TARGET_CNT_SHIFT);
-	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW8(port), dpio_val);
-
-	/* AFC Recal */
-	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port),
-			vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port)) |
-			DPIO_AFC_RECAL);
-
-	mutex_unlock(&dev_priv->sb_lock);
+	chv_phy_prepare_pll(crtc, pipe_config->dpll.n,
+			    pipe_config->dpll.m1, pipe_config->dpll.m2,
+			    pipe_config->dpll.p1, pipe_config->dpll.p2,
+			    pipe_config->dpll.vco);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
index 288da35..fcadc92 100644
--- a/drivers/gpu/drm/i915/intel_dpio_phy.c
+++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
@@ -370,6 +370,97 @@ void chv_phy_post_pll_disable(struct intel_encoder *encoder)
 	chv_phy_powergate_lanes(encoder, false, 0x0);
 }
 
+void chv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
+			 u32 bestm1, u32 bestm2, u32 bestp1, u32 bestp2,
+			 int vco)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	enum dpio_channel port = vlv_pipe_to_channel(pipe);
+	u32 loopfilter, tribuf_calcntr;
+	u32 bestm2_frac;
+	u32 dpio_val;
+
+	bestm2_frac = bestm2 & 0x3fffff;
+	bestm2 = bestm2 >> 22;
+	dpio_val = 0;
+	loopfilter = 0;
+
+	mutex_lock(&dev_priv->sb_lock);
+
+	/* p1 and p2 divider */
+	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW13(port),
+			5 << DPIO_CHV_S1_DIV_SHIFT |
+			bestp1 << DPIO_CHV_P1_DIV_SHIFT |
+			bestp2 << DPIO_CHV_P2_DIV_SHIFT |
+			1 << DPIO_CHV_K_DIV_SHIFT);
+
+	/* Feedback post-divider - m2 */
+	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW0(port), bestm2);
+
+	/* Feedback refclk divider - n and m1 */
+	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW1(port),
+			DPIO_CHV_M1_DIV_BY_2 |
+			1 << DPIO_CHV_N_DIV_SHIFT);
+
+	/* M2 fraction division */
+	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW2(port), bestm2_frac);
+
+	/* M2 fraction division enable */
+	dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW3(port));
+	dpio_val &= ~(DPIO_CHV_FEEDFWD_GAIN_MASK | DPIO_CHV_FRAC_DIV_EN);
+	dpio_val |= (2 << DPIO_CHV_FEEDFWD_GAIN_SHIFT);
+	if (bestm2_frac)
+		dpio_val |= DPIO_CHV_FRAC_DIV_EN;
+	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW3(port), dpio_val);
+
+	/* Program digital lock detect threshold */
+	dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW9(port));
+	dpio_val &= ~(DPIO_CHV_INT_LOCK_THRESHOLD_MASK |
+					DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE);
+	dpio_val |= (0x5 << DPIO_CHV_INT_LOCK_THRESHOLD_SHIFT);
+	if (!bestm2_frac)
+		dpio_val |= DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE;
+	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW9(port), dpio_val);
+
+	/* Loop filter */
+	if (vco == 5400000) {
+		loopfilter |= (0x3 << DPIO_CHV_PROP_COEFF_SHIFT);
+		loopfilter |= (0x8 << DPIO_CHV_INT_COEFF_SHIFT);
+		loopfilter |= (0x1 << DPIO_CHV_GAIN_CTRL_SHIFT);
+		tribuf_calcntr = 0x9;
+	} else if (vco <= 6200000) {
+		loopfilter |= (0x5 << DPIO_CHV_PROP_COEFF_SHIFT);
+		loopfilter |= (0xB << DPIO_CHV_INT_COEFF_SHIFT);
+		loopfilter |= (0x3 << DPIO_CHV_GAIN_CTRL_SHIFT);
+		tribuf_calcntr = 0x9;
+	} else if (vco <= 6480000) {
+		loopfilter |= (0x4 << DPIO_CHV_PROP_COEFF_SHIFT);
+		loopfilter |= (0x9 << DPIO_CHV_INT_COEFF_SHIFT);
+		loopfilter |= (0x3 << DPIO_CHV_GAIN_CTRL_SHIFT);
+		tribuf_calcntr = 0x8;
+	} else {
+		/* Not supported. Apply the same limits as in the max case */
+		loopfilter |= (0x4 << DPIO_CHV_PROP_COEFF_SHIFT);
+		loopfilter |= (0x9 << DPIO_CHV_INT_COEFF_SHIFT);
+		loopfilter |= (0x3 << DPIO_CHV_GAIN_CTRL_SHIFT);
+		tribuf_calcntr = 0;
+	}
+	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW6(port), loopfilter);
+
+	dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW8(port));
+	dpio_val &= ~DPIO_CHV_TDC_TARGET_CNT_MASK;
+	dpio_val |= (tribuf_calcntr << DPIO_CHV_TDC_TARGET_CNT_SHIFT);
+	vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW8(port), dpio_val);
+
+	/* AFC Recal */
+	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port),
+			vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port)) |
+			DPIO_AFC_RECAL);
+
+	mutex_unlock(&dev_priv->sb_lock);
+}
+
 void vlv_set_phy_signal_level(struct intel_encoder *encoder,
 			      u32 demph_reg_value, u32 preemph_reg_value,
 			      u32 uniqtranscale_reg_value, u32 tx3_demph)
@@ -468,3 +559,114 @@ void vlv_phy_reset_lanes(struct intel_encoder *encoder)
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW1(port), 0x00e00060);
 	mutex_unlock(&dev_priv->sb_lock);
 }
+
+static void vlv_pllb_recal_opamp(struct drm_i915_private *dev_priv,
+				 enum pipe pipe)
+{
+	u32 reg_val;
+
+	/*
+	 * PLLB opamp always calibrates to max value of 0x3f, force enable it
+	 * and set it to a reasonable value instead.
+	 */
+	reg_val = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW9(1));
+	reg_val &= 0xffffff00;
+	reg_val |= 0x00000030;
+	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW9(1), reg_val);
+
+	reg_val = vlv_dpio_read(dev_priv, pipe, VLV_REF_DW13);
+	reg_val &= 0x8cffffff;
+	reg_val = 0x8c000000;
+	vlv_dpio_write(dev_priv, pipe, VLV_REF_DW13, reg_val);
+
+	reg_val = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW9(1));
+	reg_val &= 0xffffff00;
+	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW9(1), reg_val);
+
+	reg_val = vlv_dpio_read(dev_priv, pipe, VLV_REF_DW13);
+	reg_val &= 0x00ffffff;
+	reg_val |= 0xb0000000;
+	vlv_dpio_write(dev_priv, pipe, VLV_REF_DW13, reg_val);
+}
+
+void vlv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
+			 u32 bestm1, u32 bestm2, u32 bestp1, u32 bestp2,
+			 int port_clock, bool dp)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	int pipe = crtc->pipe;
+	u32 mdiv;
+	u32 coreclk, reg_val;
+
+	mutex_lock(&dev_priv->sb_lock);
+
+	/* See eDP HDMI DPIO driver vbios notes doc */
+
+	/* PLL B needs special handling */
+	if (pipe == PIPE_B)
+		vlv_pllb_recal_opamp(dev_priv, pipe);
+
+	/* Set up Tx target for periodic Rcomp update */
+	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW9_BCAST, 0x0100000f);
+
+	/* Disable target IRef on PLL */
+	reg_val = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW8(pipe));
+	reg_val &= 0x00ffffff;
+	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW8(pipe), reg_val);
+
+	/* Disable fast lock */
+	vlv_dpio_write(dev_priv, pipe, VLV_CMN_DW0, 0x610);
+
+	/* Set idtafcrecal before PLL is enabled */
+	mdiv = ((bestm1 << DPIO_M1DIV_SHIFT) | (bestm2 & DPIO_M2DIV_MASK));
+	mdiv |= ((bestp1 << DPIO_P1_SHIFT) | (bestp2 << DPIO_P2_SHIFT));
+	mdiv |= ((bestn << DPIO_N_SHIFT));
+	mdiv |= (1 << DPIO_K_SHIFT);
+
+	/*
+	 * Post divider depends on pixel clock rate, DAC vs digital (and LVDS,
+	 * but we don't support that).
+	 * Note: don't use the DAC post divider as it seems unstable.
+	 */
+	mdiv |= (DPIO_POST_DIV_HDMIDP << DPIO_POST_DIV_SHIFT);
+	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW3(pipe), mdiv);
+
+	mdiv |= DPIO_ENABLE_CALIBRATION;
+	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW3(pipe), mdiv);
+
+	/* Set HBR and RBR LPF coefficients */
+	if (port_clock == 162000 || !dp)
+		vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW10(pipe),
+				 0x009f0003);
+	else
+		vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW10(pipe),
+				 0x00d0000f);
+
+	if (dp) {
+		/* Use SSC source */
+		if (pipe == PIPE_A)
+			vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW5(pipe),
+					 0x0df40000);
+		else
+			vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW5(pipe),
+					 0x0df70000);
+	} else { /* HDMI or VGA */
+		/* Use bend source */
+		if (pipe == PIPE_A)
+			vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW5(pipe),
+					 0x0df70000);
+		else
+			vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW5(pipe),
+					 0x0df40000);
+	}
+
+	coreclk = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW7(pipe));
+	coreclk = (coreclk & 0x0000ff00) | 0x01c00000;
+	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT) ||
+	    intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))
+		coreclk |= 0x01000000;
+	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW7(pipe), coreclk);
+
+	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW11(pipe), 0x87871000);
+	mutex_unlock(&dev_priv->sb_lock);
+}
-- 
2.5.5

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

[PATCH 3/6] drm/i915: Merge vlv/chv _prepare_pll() with their enable counterpart

[Ander Conselvan de Oliveira]

With the bulk of the dpio code moved out of the vlv/chv prepare pll
functions into intel_dpio_phy.c, those functions became simple enough
that they can be merged with the pll enabling function, that always
succeeds the prepare call.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 82 ++++++++++++------------------------
 1 file changed, 26 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 06d9b96..3e494ec 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -104,10 +104,6 @@ static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
 static void ironlake_set_pipeconf(struct drm_crtc *crtc);
 static void haswell_set_pipeconf(struct drm_crtc *crtc);
 static void haswell_set_pipemisc(struct drm_crtc *crtc);
-static void vlv_prepare_pll(struct intel_crtc *crtc,
-			    const struct intel_crtc_state *pipe_config);
-static void chv_prepare_pll(struct intel_crtc *crtc,
-			    const struct intel_crtc_state *pipe_config);
 static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
 static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
 static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
@@ -1547,6 +1543,19 @@ static void vlv_enable_pll(struct intel_crtc *crtc,
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 
+	/* Enable Refclk */
+	I915_WRITE(DPLL(pipe),
+		   pipe_config->dpll_hw_state.dpll &
+		   ~(DPLL_VCO_ENABLE | DPLL_EXT_BUFFER_ENABLE_VLV));
+
+	/* No need to actually set up the DPLL with DSI */
+	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) != 0)
+		vlv_phy_prepare_pll(crtc, pipe_config->dpll.n,
+				    pipe_config->dpll.m1, pipe_config->dpll.m2,
+				    pipe_config->dpll.p1, pipe_config->dpll.p2,
+				    pipe_config->port_clock,
+				    pipe_config->has_dp_encoder);
+
 	assert_pipe_disabled(dev_priv, pipe);
 
 	/* PLL is protected by panel, make sure we can write it */
@@ -1596,6 +1605,17 @@ static void chv_enable_pll(struct intel_crtc *crtc,
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 
+	/* Enable Refclk and SSC */
+	I915_WRITE(DPLL(pipe),
+		   pipe_config->dpll_hw_state.dpll & ~DPLL_VCO_ENABLE);
+
+	/* No need to actually set up the DPLL with DSI */
+	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) != 0)
+		chv_phy_prepare_pll(crtc, pipe_config->dpll.n,
+				    pipe_config->dpll.m1, pipe_config->dpll.m2,
+				    pipe_config->dpll.p1, pipe_config->dpll.p2,
+				    pipe_config->dpll.vco);
+
 	assert_pipe_disabled(dev_priv, pipe);
 
 	/* PLL is protected by panel, make sure we can write it */
@@ -6086,13 +6106,10 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 		if (encoder->pre_pll_enable)
 			encoder->pre_pll_enable(encoder);
 
-	if (IS_CHERRYVIEW(dev)) {
-		chv_prepare_pll(intel_crtc, intel_crtc->config);
+	if (IS_CHERRYVIEW(dev))
 		chv_enable_pll(intel_crtc, intel_crtc->config);
-	} else {
-		vlv_prepare_pll(intel_crtc, intel_crtc->config);
+	else
 		vlv_enable_pll(intel_crtc, intel_crtc->config);
-	}
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		if (encoder->pre_enable)
@@ -7185,51 +7202,6 @@ static void chv_compute_dpll(struct intel_crtc *crtc,
 		(pipe_config->pixel_multiplier - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT;
 }
 
-static void vlv_prepare_pll(struct intel_crtc *crtc,
-			    const struct intel_crtc_state *pipe_config)
-{
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum pipe pipe = crtc->pipe;
-
-	/* Enable Refclk */
-	I915_WRITE(DPLL(pipe),
-		   pipe_config->dpll_hw_state.dpll &
-		   ~(DPLL_VCO_ENABLE | DPLL_EXT_BUFFER_ENABLE_VLV));
-
-	/* No need to actually set up the DPLL with DSI */
-	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0)
-		return;
-
-	vlv_phy_prepare_pll(crtc, pipe_config->dpll.n,
-			    pipe_config->dpll.m1, pipe_config->dpll.m2,
-			    pipe_config->dpll.p1, pipe_config->dpll.p2,
-			    pipe_config->port_clock,
-			    pipe_config->has_dp_encoder);
-
-}
-
-static void chv_prepare_pll(struct intel_crtc *crtc,
-			    const struct intel_crtc_state *pipe_config)
-{
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum pipe pipe = crtc->pipe;
-
-	/* Enable Refclk and SSC */
-	I915_WRITE(DPLL(pipe),
-		   pipe_config->dpll_hw_state.dpll & ~DPLL_VCO_ENABLE);
-
-	/* No need to actually set up the DPLL with DSI */
-	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0)
-		return;
-
-	chv_phy_prepare_pll(crtc, pipe_config->dpll.n,
-			    pipe_config->dpll.m1, pipe_config->dpll.m2,
-			    pipe_config->dpll.p1, pipe_config->dpll.p2,
-			    pipe_config->dpll.vco);
-}
-
 /**
  * vlv_force_pll_on - forcibly enable just the PLL
  * @dev_priv: i915 private structure
@@ -7257,11 +7229,9 @@ int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
 
 	if (IS_CHERRYVIEW(dev)) {
 		chv_compute_dpll(crtc, pipe_config);
-		chv_prepare_pll(crtc, pipe_config);
 		chv_enable_pll(crtc, pipe_config);
 	} else {
 		vlv_compute_dpll(crtc, pipe_config);
-		vlv_prepare_pll(crtc, pipe_config);
 		vlv_enable_pll(crtc, pipe_config);
 	}
 
-- 
2.5.5

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

[PATCH 4/6] drm/i915: Move VLV divider readout to intel_dpio_phy.c

[Ander Conselvan de Oliveira]

Reading the dividers depends on sideband messaging, so it fits well if
the other functions in intel_dpio_phy.c. The new function will also be
used in a future patch.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  2 ++
 drivers/gpu/drm/i915/intel_display.c  | 11 +----------
 drivers/gpu/drm/i915/intel_dpio_phy.c | 16 ++++++++++++++++
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 99dfacd..7dfa555 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3640,6 +3640,8 @@ void vlv_phy_reset_lanes(struct intel_encoder *encoder);
 void vlv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
 			 u32 bestm1, u32 bestm2, u32 bestp1, u32 bestp2,
 			 int port_clock, bool dp);
+void vlv_phy_read_dividers(struct drm_i915_private *dev_priv,
+			   enum pipe pipe, struct intel_dpll *clock);
 
 int intel_gpu_freq(struct drm_i915_private *dev_priv, int val);
 int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3e494ec..8d61263 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7806,22 +7806,13 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe = pipe_config->cpu_transcoder;
 	struct intel_dpll clock;
-	u32 mdiv;
 	int refclk = 100000;
 
 	/* In case of DSI, DPLL will not be used */
 	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0)
 		return;
 
-	mutex_lock(&dev_priv->sb_lock);
-	mdiv = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe));
-	mutex_unlock(&dev_priv->sb_lock);
-
-	clock.m1 = (mdiv >> DPIO_M1DIV_SHIFT) & 7;
-	clock.m2 = mdiv & DPIO_M2DIV_MASK;
-	clock.n = (mdiv >> DPIO_N_SHIFT) & 0xf;
-	clock.p1 = (mdiv >> DPIO_P1_SHIFT) & 7;
-	clock.p2 = (mdiv >> DPIO_P2_SHIFT) & 0x1f;
+	vlv_phy_read_dividers(dev_priv, pipe, &clock);
 
 	pipe_config->port_clock = vlv_calc_dpll_params(refclk, &clock);
 }
diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
index fcadc92..d28ef9f 100644
--- a/drivers/gpu/drm/i915/intel_dpio_phy.c
+++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
@@ -670,3 +670,19 @@ void vlv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
 	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW11(pipe), 0x87871000);
 	mutex_unlock(&dev_priv->sb_lock);
 }
+
+void vlv_phy_read_dividers(struct drm_i915_private *dev_priv,
+			   enum pipe pipe, struct intel_dpll *clock)
+{
+	u32 mdiv;
+
+	mutex_lock(&dev_priv->sb_lock);
+	mdiv = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe));
+	mutex_unlock(&dev_priv->sb_lock);
+
+	clock->m1 = (mdiv >> DPIO_M1DIV_SHIFT) & 7;
+	clock->m2 = mdiv & DPIO_M2DIV_MASK;
+	clock->n = (mdiv >> DPIO_N_SHIFT) & 0xf;
+	clock->p1 = (mdiv >> DPIO_P1_SHIFT) & 7;
+	clock->p2 = (mdiv >> DPIO_P2_SHIFT) & 0x1f;
+}
-- 
2.5.5

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

[PATCH 5/6] drm/i915: Move CHV divider readout to intel_dpio_phy.c

[Ander Conselvan de Oliveira]

Reading the dividers depends on sideband messaging, so it fits well if
the other functions in intel_dpio_phy.c. The new function will also be
used in a future patch.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  2 ++
 drivers/gpu/drm/i915/intel_display.c  | 18 +-----------------
 drivers/gpu/drm/i915/intel_dpio_phy.c | 24 ++++++++++++++++++++++++
 3 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7dfa555..149317c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3630,6 +3630,8 @@ void chv_phy_post_pll_disable(struct intel_encoder *encoder);
 void chv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
 			 u32 bestm1, u32 bestm2, u32 bestp1, u32 bestp2,
 			 int vco);
+void chv_phy_read_dividers(struct drm_i915_private *dev_priv,
+			   enum pipe pipe, struct intel_dpll *clock);
 
 void vlv_set_phy_signal_level(struct intel_encoder *encoder,
 			      u32 demph_reg_value, u32 preemph_reg_value,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8d61263..5ba000a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7892,30 +7892,14 @@ static void chv_crtc_clock_get(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe = pipe_config->cpu_transcoder;
-	enum dpio_channel port = vlv_pipe_to_channel(pipe);
 	struct intel_dpll clock;
-	u32 cmn_dw13, pll_dw0, pll_dw1, pll_dw2, pll_dw3;
 	int refclk = 100000;
 
 	/* In case of DSI, DPLL will not be used */
 	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0)
 		return;
 
-	mutex_lock(&dev_priv->sb_lock);
-	cmn_dw13 = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW13(port));
-	pll_dw0 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW0(port));
-	pll_dw1 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW1(port));
-	pll_dw2 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW2(port));
-	pll_dw3 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW3(port));
-	mutex_unlock(&dev_priv->sb_lock);
-
-	clock.m1 = (pll_dw1 & 0x7) == DPIO_CHV_M1_DIV_BY_2 ? 2 : 0;
-	clock.m2 = (pll_dw0 & 0xff) << 22;
-	if (pll_dw3 & DPIO_CHV_FRAC_DIV_EN)
-		clock.m2 |= pll_dw2 & 0x3fffff;
-	clock.n = (pll_dw1 >> DPIO_CHV_N_DIV_SHIFT) & 0xf;
-	clock.p1 = (cmn_dw13 >> DPIO_CHV_P1_DIV_SHIFT) & 0x7;
-	clock.p2 = (cmn_dw13 >> DPIO_CHV_P2_DIV_SHIFT) & 0x1f;
+	chv_phy_read_dividers(dev_priv, pipe, &clock);
 
 	pipe_config->port_clock = chv_calc_dpll_params(refclk, &clock);
 }
diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
index d28ef9f..2a5d333 100644
--- a/drivers/gpu/drm/i915/intel_dpio_phy.c
+++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
@@ -461,6 +461,30 @@ void chv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
 	mutex_unlock(&dev_priv->sb_lock);
 }
 
+void chv_phy_read_dividers(struct drm_i915_private *dev_priv,
+			   enum pipe pipe, struct intel_dpll *clock)
+{
+	enum dpio_channel port = vlv_pipe_to_channel(pipe);
+	u32 cmn_dw13, pll_dw0, pll_dw1, pll_dw2, pll_dw3;
+
+	mutex_lock(&dev_priv->sb_lock);
+	cmn_dw13 = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW13(port));
+	pll_dw0 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW0(port));
+	pll_dw1 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW1(port));
+	pll_dw2 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW2(port));
+	pll_dw3 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW3(port));
+	mutex_unlock(&dev_priv->sb_lock);
+
+	clock->m1 = (pll_dw1 & 0x7) == DPIO_CHV_M1_DIV_BY_2 ? 2 : 0;
+	clock->m2 = (pll_dw0 & 0xff) << 22;
+	if (pll_dw3 & DPIO_CHV_FRAC_DIV_EN)
+		clock->m2 |= pll_dw2 & 0x3fffff;
+	clock->n = (pll_dw1 >> DPIO_CHV_N_DIV_SHIFT) & 0xf;
+	clock->p1 = (cmn_dw13 >> DPIO_CHV_P1_DIV_SHIFT) & 0x7;
+	clock->p2 = (cmn_dw13 >> DPIO_CHV_P2_DIV_SHIFT) & 0x1f;
+}
+
+
 void vlv_set_phy_signal_level(struct intel_encoder *encoder,
 			      u32 demph_reg_value, u32 preemph_reg_value,
 			      u32 uniqtranscale_reg_value, u32 tx3_demph)
-- 
2.5.5

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

[PATCH 6/6] drm/i915: Move toggling of CHV DPIO_DCLKP_EN to intel_dpio_phy.c

[Ander Conselvan de Oliveira]

This simplifies the pll enable/disable a code a bit and hides the
sideband message neatly in intel_dpio_phy.c.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  2 ++
 drivers/gpu/drm/i915/intel_display.c  | 19 ++-----------------
 drivers/gpu/drm/i915/intel_dpio_phy.c | 18 ++++++++++++++++++
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 149317c..44f4b7a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3630,6 +3630,8 @@ void chv_phy_post_pll_disable(struct intel_encoder *encoder);
 void chv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
 			 u32 bestm1, u32 bestm2, u32 bestp1, u32 bestp2,
 			 int vco);
+void chv_phy_toggle_dclkp(struct drm_i915_private *dev_priv, enum pipe pipe,
+			  bool enable);
 void chv_phy_read_dividers(struct drm_i915_private *dev_priv,
 			   enum pipe pipe, struct intel_dpll *clock);
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5ba000a..d43bdff 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1574,17 +1574,9 @@ static void _chv_enable_pll(struct intel_crtc *crtc,
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
-	enum dpio_channel port = vlv_pipe_to_channel(pipe);
-	u32 tmp;
-
-	mutex_lock(&dev_priv->sb_lock);
 
 	/* Enable back the 10bit clock to display controller */
-	tmp = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
-	tmp |= DPIO_DCLKP_EN;
-	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), tmp);
-
-	mutex_unlock(&dev_priv->sb_lock);
+	chv_phy_toggle_dclkp(dev_priv, pipe, true);
 
 	/*
 	 * Need to wait > 100ns between dclkp clock enable bit and PLL enable.
@@ -1777,7 +1769,6 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
-	enum dpio_channel port = vlv_pipe_to_channel(pipe);
 	u32 val;
 
 	/* Make sure the pipe isn't still relying on us */
@@ -1791,14 +1782,8 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 	I915_WRITE(DPLL(pipe), val);
 	POSTING_READ(DPLL(pipe));
 
-	mutex_lock(&dev_priv->sb_lock);
-
 	/* Disable 10bit clock to display controller */
-	val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
-	val &= ~DPIO_DCLKP_EN;
-	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), val);
-
-	mutex_unlock(&dev_priv->sb_lock);
+	chv_phy_toggle_dclkp(dev_priv, pipe, false);
 }
 
 void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
index 2a5d333..64788e3 100644
--- a/drivers/gpu/drm/i915/intel_dpio_phy.c
+++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
@@ -461,6 +461,24 @@ void chv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
 	mutex_unlock(&dev_priv->sb_lock);
 }
 
+void chv_phy_toggle_dclkp(struct drm_i915_private *dev_priv, enum pipe pipe,
+			  bool enable)
+{
+	enum dpio_channel port = vlv_pipe_to_channel(pipe);
+	u32 dpio_val;
+
+	mutex_lock(&dev_priv->sb_lock);
+
+	dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
+	if (enable)
+		dpio_val |= DPIO_DCLKP_EN;
+	else
+		dpio_val &= ~DPIO_DCLKP_EN;
+	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), dpio_val);
+
+	mutex_unlock(&dev_priv->sb_lock);
+}
+
 void chv_phy_read_dividers(struct drm_i915_private *dev_priv,
 			   enum pipe pipe, struct intel_dpll *clock)
 {
-- 
2.5.5

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

Re: [PATCH 4/6] drm/i915: Move VLV divider readout to intel_dpio_phy.c

[Ville Syrjälä]

On Fri, May 13, 2016 at 05:15:01PM +0300, Ander Conselvan de Oliveira wrote:
> Reading the dividers depends on sideband messaging, so it fits well if
> the other functions in intel_dpio_phy.c. The new function will also be
> used in a future patch.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  2 ++
>  drivers/gpu/drm/i915/intel_display.c  | 11 +----------
>  drivers/gpu/drm/i915/intel_dpio_phy.c | 16 ++++++++++++++++
>  3 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 99dfacd..7dfa555 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3640,6 +3640,8 @@ void vlv_phy_reset_lanes(struct intel_encoder *encoder);
>  void vlv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
>  			 u32 bestm1, u32 bestm2, u32 bestp1, u32 bestp2,
>  			 int port_clock, bool dp);
> +void vlv_phy_read_dividers(struct drm_i915_private *dev_priv,
> +			   enum pipe pipe, struct intel_dpll *clock);

..._pll_dividers() ?

>  
>  int intel_gpu_freq(struct drm_i915_private *dev_priv, int val);
>  int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3e494ec..8d61263 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7806,22 +7806,13 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int pipe = pipe_config->cpu_transcoder;
>  	struct intel_dpll clock;
> -	u32 mdiv;
>  	int refclk = 100000;
>  
>  	/* In case of DSI, DPLL will not be used */
>  	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0)
>  		return;
>  
> -	mutex_lock(&dev_priv->sb_lock);
> -	mdiv = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe));
> -	mutex_unlock(&dev_priv->sb_lock);
> -
> -	clock.m1 = (mdiv >> DPIO_M1DIV_SHIFT) & 7;
> -	clock.m2 = mdiv & DPIO_M2DIV_MASK;
> -	clock.n = (mdiv >> DPIO_N_SHIFT) & 0xf;
> -	clock.p1 = (mdiv >> DPIO_P1_SHIFT) & 7;
> -	clock.p2 = (mdiv >> DPIO_P2_SHIFT) & 0x1f;
> +	vlv_phy_read_dividers(dev_priv, pipe, &clock);
>  
>  	pipe_config->port_clock = vlv_calc_dpll_params(refclk, &clock);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
> index fcadc92..d28ef9f 100644
> --- a/drivers/gpu/drm/i915/intel_dpio_phy.c
> +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
> @@ -670,3 +670,19 @@ void vlv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
>  	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW11(pipe), 0x87871000);
>  	mutex_unlock(&dev_priv->sb_lock);
>  }
> +
> +void vlv_phy_read_dividers(struct drm_i915_private *dev_priv,
> +			   enum pipe pipe, struct intel_dpll *clock)
> +{
> +	u32 mdiv;
> +
> +	mutex_lock(&dev_priv->sb_lock);
> +	mdiv = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe));
> +	mutex_unlock(&dev_priv->sb_lock);
> +
> +	clock->m1 = (mdiv >> DPIO_M1DIV_SHIFT) & 7;
> +	clock->m2 = mdiv & DPIO_M2DIV_MASK;
> +	clock->n = (mdiv >> DPIO_N_SHIFT) & 0xf;
> +	clock->p1 = (mdiv >> DPIO_P1_SHIFT) & 7;
> +	clock->p2 = (mdiv >> DPIO_P2_SHIFT) & 0x1f;
> +}
> -- 
> 2.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 6/6] drm/i915: Move toggling of CHV DPIO_DCLKP_EN to intel_dpio_phy.c

[Ville Syrjälä]

On Fri, May 13, 2016 at 05:15:03PM +0300, Ander Conselvan de Oliveira wrote:
> This simplifies the pll enable/disable a code a bit and hides the
> sideband message neatly in intel_dpio_phy.c.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  2 ++
>  drivers/gpu/drm/i915/intel_display.c  | 19 ++-----------------
>  drivers/gpu/drm/i915/intel_dpio_phy.c | 18 ++++++++++++++++++
>  3 files changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 149317c..44f4b7a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3630,6 +3630,8 @@ void chv_phy_post_pll_disable(struct intel_encoder *encoder);
>  void chv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
>  			 u32 bestm1, u32 bestm2, u32 bestp1, u32 bestp2,
>  			 int vco);
> +void chv_phy_toggle_dclkp(struct drm_i915_private *dev_priv, enum pipe pipe,
> +			  bool enable);

Toggle makes me think it just flips the bit. _enable_dclkp()
_set_dclkp() or something might be better?

>  void chv_phy_read_dividers(struct drm_i915_private *dev_priv,
>  			   enum pipe pipe, struct intel_dpll *clock);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5ba000a..d43bdff 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1574,17 +1574,9 @@ static void _chv_enable_pll(struct intel_crtc *crtc,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum pipe pipe = crtc->pipe;
> -	enum dpio_channel port = vlv_pipe_to_channel(pipe);
> -	u32 tmp;
> -
> -	mutex_lock(&dev_priv->sb_lock);
>  
>  	/* Enable back the 10bit clock to display controller */
> -	tmp = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
> -	tmp |= DPIO_DCLKP_EN;
> -	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), tmp);
> -
> -	mutex_unlock(&dev_priv->sb_lock);
> +	chv_phy_toggle_dclkp(dev_priv, pipe, true);
>  
>  	/*
>  	 * Need to wait > 100ns between dclkp clock enable bit and PLL enable.
> @@ -1777,7 +1769,6 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>  
>  static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
> -	enum dpio_channel port = vlv_pipe_to_channel(pipe);
>  	u32 val;
>  
>  	/* Make sure the pipe isn't still relying on us */
> @@ -1791,14 +1782,8 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>  	I915_WRITE(DPLL(pipe), val);
>  	POSTING_READ(DPLL(pipe));
>  
> -	mutex_lock(&dev_priv->sb_lock);
> -
>  	/* Disable 10bit clock to display controller */
> -	val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
> -	val &= ~DPIO_DCLKP_EN;
> -	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), val);
> -
> -	mutex_unlock(&dev_priv->sb_lock);
> +	chv_phy_toggle_dclkp(dev_priv, pipe, false);
>  }
>  
>  void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
> index 2a5d333..64788e3 100644
> --- a/drivers/gpu/drm/i915/intel_dpio_phy.c
> +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
> @@ -461,6 +461,24 @@ void chv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
>  	mutex_unlock(&dev_priv->sb_lock);
>  }
>  
> +void chv_phy_toggle_dclkp(struct drm_i915_private *dev_priv, enum pipe pipe,
> +			  bool enable)
> +{
> +	enum dpio_channel port = vlv_pipe_to_channel(pipe);
> +	u32 dpio_val;
> +
> +	mutex_lock(&dev_priv->sb_lock);
> +
> +	dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
> +	if (enable)
> +		dpio_val |= DPIO_DCLKP_EN;
> +	else
> +		dpio_val &= ~DPIO_DCLKP_EN;
> +	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), dpio_val);
> +
> +	mutex_unlock(&dev_priv->sb_lock);
> +}
> +
>  void chv_phy_read_dividers(struct drm_i915_private *dev_priv,
>  			   enum pipe pipe, struct intel_dpll *clock)
>  {
> -- 
> 2.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 5/6] drm/i915: Move CHV divider readout to intel_dpio_phy.c

[Ville Syrjälä]

On Fri, May 13, 2016 at 05:15:02PM +0300, Ander Conselvan de Oliveira wrote:
> Reading the dividers depends on sideband messaging, so it fits well if
> the other functions in intel_dpio_phy.c. The new function will also be
> used in a future patch.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  2 ++
>  drivers/gpu/drm/i915/intel_display.c  | 18 +-----------------
>  drivers/gpu/drm/i915/intel_dpio_phy.c | 24 ++++++++++++++++++++++++
>  3 files changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7dfa555..149317c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3630,6 +3630,8 @@ void chv_phy_post_pll_disable(struct intel_encoder *encoder);
>  void chv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
>  			 u32 bestm1, u32 bestm2, u32 bestp1, u32 bestp2,
>  			 int vco);
> +void chv_phy_read_dividers(struct drm_i915_private *dev_priv,
> +			   enum pipe pipe, struct intel_dpll *clock);

_pll_dividers() again?

>  
>  void vlv_set_phy_signal_level(struct intel_encoder *encoder,
>  			      u32 demph_reg_value, u32 preemph_reg_value,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8d61263..5ba000a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7892,30 +7892,14 @@ static void chv_crtc_clock_get(struct intel_crtc *crtc,
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int pipe = pipe_config->cpu_transcoder;
> -	enum dpio_channel port = vlv_pipe_to_channel(pipe);
>  	struct intel_dpll clock;
> -	u32 cmn_dw13, pll_dw0, pll_dw1, pll_dw2, pll_dw3;
>  	int refclk = 100000;
>  
>  	/* In case of DSI, DPLL will not be used */
>  	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0)
>  		return;
>  
> -	mutex_lock(&dev_priv->sb_lock);
> -	cmn_dw13 = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW13(port));
> -	pll_dw0 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW0(port));
> -	pll_dw1 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW1(port));
> -	pll_dw2 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW2(port));
> -	pll_dw3 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW3(port));
> -	mutex_unlock(&dev_priv->sb_lock);
> -
> -	clock.m1 = (pll_dw1 & 0x7) == DPIO_CHV_M1_DIV_BY_2 ? 2 : 0;
> -	clock.m2 = (pll_dw0 & 0xff) << 22;
> -	if (pll_dw3 & DPIO_CHV_FRAC_DIV_EN)
> -		clock.m2 |= pll_dw2 & 0x3fffff;
> -	clock.n = (pll_dw1 >> DPIO_CHV_N_DIV_SHIFT) & 0xf;
> -	clock.p1 = (cmn_dw13 >> DPIO_CHV_P1_DIV_SHIFT) & 0x7;
> -	clock.p2 = (cmn_dw13 >> DPIO_CHV_P2_DIV_SHIFT) & 0x1f;
> +	chv_phy_read_dividers(dev_priv, pipe, &clock);
>  
>  	pipe_config->port_clock = chv_calc_dpll_params(refclk, &clock);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
> index d28ef9f..2a5d333 100644
> --- a/drivers/gpu/drm/i915/intel_dpio_phy.c
> +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
> @@ -461,6 +461,30 @@ void chv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
>  	mutex_unlock(&dev_priv->sb_lock);
>  }
>  
> +void chv_phy_read_dividers(struct drm_i915_private *dev_priv,
> +			   enum pipe pipe, struct intel_dpll *clock)
> +{
> +	enum dpio_channel port = vlv_pipe_to_channel(pipe);
> +	u32 cmn_dw13, pll_dw0, pll_dw1, pll_dw2, pll_dw3;
> +
> +	mutex_lock(&dev_priv->sb_lock);
> +	cmn_dw13 = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW13(port));
> +	pll_dw0 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW0(port));
> +	pll_dw1 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW1(port));
> +	pll_dw2 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW2(port));
> +	pll_dw3 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW3(port));
> +	mutex_unlock(&dev_priv->sb_lock);
> +
> +	clock->m1 = (pll_dw1 & 0x7) == DPIO_CHV_M1_DIV_BY_2 ? 2 : 0;
> +	clock->m2 = (pll_dw0 & 0xff) << 22;
> +	if (pll_dw3 & DPIO_CHV_FRAC_DIV_EN)
> +		clock->m2 |= pll_dw2 & 0x3fffff;
> +	clock->n = (pll_dw1 >> DPIO_CHV_N_DIV_SHIFT) & 0xf;
> +	clock->p1 = (cmn_dw13 >> DPIO_CHV_P1_DIV_SHIFT) & 0x7;
> +	clock->p2 = (cmn_dw13 >> DPIO_CHV_P2_DIV_SHIFT) & 0x1f;
> +}
> +
> +
>  void vlv_set_phy_signal_level(struct intel_encoder *encoder,
>  			      u32 demph_reg_value, u32 preemph_reg_value,
>  			      u32 uniqtranscale_reg_value, u32 tx3_demph)
> -- 
> 2.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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/6] drm/i915: Rename struct dpll to struct intel_dpll

[Ville Syrjälä]

On Fri, May 13, 2016 at 05:14:58PM +0300, Ander Conselvan de Oliveira wrote:
> Prefix struct dpll with intel_ to follow the convention in the driver.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  2 +-
>  drivers/gpu/drm/i915/intel_ddi.c      |  2 +-
>  drivers/gpu/drm/i915/intel_display.c  | 76 +++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_dp.c       |  2 +-
>  drivers/gpu/drm/i915/intel_dpll_mgr.c |  2 +-
>  drivers/gpu/drm/i915/intel_drv.h      | 10 ++---
>  drivers/gpu/drm/i915/intel_sdvo.c     |  2 +-
>  7 files changed, 48 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7a0b513..5f9dda2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -577,7 +577,7 @@ struct intel_crtc_state;
>  struct intel_initial_plane_config;
>  struct intel_crtc;
>  struct intel_limit;
> -struct dpll;
> +struct intel_dpll;

Now that I see that, it kinda makes me think someone is bould to
confuse it to mean an actual DPLL instead of just the dividers and
whanot.

>  
>  struct drm_i915_display_funcs {
>  	int (*get_display_clock_speed)(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index c454744..1387acd 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -948,7 +948,7 @@ static int bxt_calc_pll_link(struct drm_i915_private *dev_priv,
>  {
>  	struct intel_shared_dpll *pll;
>  	struct intel_dpll_hw_state *state;
> -	struct dpll clock;
> +	struct intel_dpll clock;
>  
>  	/* For DDI ports we always use a shared PLL. */
>  	if (WARN_ON(dpll == DPLL_ID_PRIVATE))
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 05c7533..048a5bf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -578,7 +578,7 @@ static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
>   * divided-down version of it.
>   */
>  /* m1 is reserved as 0 in Pineview, n is a ring counter */
> -static int pnv_calc_dpll_params(int refclk, struct dpll *clock)
> +static int pnv_calc_dpll_params(int refclk, struct intel_dpll *clock)
>  {
>  	clock->m = clock->m2 + 2;
>  	clock->p = clock->p1 * clock->p2;
> @@ -590,12 +590,12 @@ static int pnv_calc_dpll_params(int refclk, struct dpll *clock)
>  	return clock->dot;
>  }
>  
> -static uint32_t i9xx_dpll_compute_m(struct dpll *dpll)
> +static uint32_t i9xx_dpll_compute_m(struct intel_dpll *dpll)
>  {
>  	return 5 * (dpll->m1 + 2) + (dpll->m2 + 2);
>  }
>  
> -static int i9xx_calc_dpll_params(int refclk, struct dpll *clock)
> +static int i9xx_calc_dpll_params(int refclk, struct intel_dpll *clock)
>  {
>  	clock->m = i9xx_dpll_compute_m(clock);
>  	clock->p = clock->p1 * clock->p2;
> @@ -607,7 +607,7 @@ static int i9xx_calc_dpll_params(int refclk, struct dpll *clock)
>  	return clock->dot;
>  }
>  
> -static int vlv_calc_dpll_params(int refclk, struct dpll *clock)
> +static int vlv_calc_dpll_params(int refclk, struct intel_dpll *clock)
>  {
>  	clock->m = clock->m1 * clock->m2;
>  	clock->p = clock->p1 * clock->p2;
> @@ -619,7 +619,7 @@ static int vlv_calc_dpll_params(int refclk, struct dpll *clock)
>  	return clock->dot / 5;
>  }
>  
> -int chv_calc_dpll_params(int refclk, struct dpll *clock)
> +int chv_calc_dpll_params(int refclk, struct intel_dpll *clock)
>  {
>  	clock->m = clock->m1 * clock->m2;
>  	clock->p = clock->p1 * clock->p2;
> @@ -640,7 +640,7 @@ int chv_calc_dpll_params(int refclk, struct dpll *clock)
>  
>  static bool intel_PLL_is_valid(struct drm_device *dev,
>  			       const struct intel_limit *limit,
> -			       const struct dpll *clock)
> +			       const struct intel_dpll *clock)
>  {
>  	if (clock->n   < limit->n.min   || limit->n.max   < clock->n)
>  		INTELPllInvalid("n out of range\n");
> @@ -712,11 +712,11 @@ i9xx_select_p2_div(const struct intel_limit *limit,
>  static bool
>  i9xx_find_best_dpll(const struct intel_limit *limit,
>  		    struct intel_crtc_state *crtc_state,
> -		    int target, int refclk, struct dpll *match_clock,
> -		    struct dpll *best_clock)
> +		    int target, int refclk, struct intel_dpll *match_clock,
> +		    struct intel_dpll *best_clock)
>  {
>  	struct drm_device *dev = crtc_state->base.crtc->dev;
> -	struct dpll clock;
> +	struct intel_dpll clock;
>  	int err = target;
>  
>  	memset(best_clock, 0, sizeof(*best_clock));
> @@ -769,11 +769,11 @@ i9xx_find_best_dpll(const struct intel_limit *limit,
>  static bool
>  pnv_find_best_dpll(const struct intel_limit *limit,
>  		   struct intel_crtc_state *crtc_state,
> -		   int target, int refclk, struct dpll *match_clock,
> -		   struct dpll *best_clock)
> +		   int target, int refclk, struct intel_dpll *match_clock,
> +		   struct intel_dpll *best_clock)
>  {
>  	struct drm_device *dev = crtc_state->base.crtc->dev;
> -	struct dpll clock;
> +	struct intel_dpll clock;
>  	int err = target;
>  
>  	memset(best_clock, 0, sizeof(*best_clock));
> @@ -824,11 +824,11 @@ pnv_find_best_dpll(const struct intel_limit *limit,
>  static bool
>  g4x_find_best_dpll(const struct intel_limit *limit,
>  		   struct intel_crtc_state *crtc_state,
> -		   int target, int refclk, struct dpll *match_clock,
> -		   struct dpll *best_clock)
> +		   int target, int refclk, struct intel_dpll *match_clock,
> +		   struct intel_dpll *best_clock)
>  {
>  	struct drm_device *dev = crtc_state->base.crtc->dev;
> -	struct dpll clock;
> +	struct intel_dpll clock;
>  	int max_n;
>  	bool found = false;
>  	/* approximately equals target * 0.00585 */
> @@ -874,8 +874,8 @@ g4x_find_best_dpll(const struct intel_limit *limit,
>   * best configuration and error found so far. Return the calculated error.
>   */
>  static bool vlv_PLL_is_optimal(struct drm_device *dev, int target_freq,
> -			       const struct dpll *calculated_clock,
> -			       const struct dpll *best_clock,
> +			       const struct intel_dpll *calculated_clock,
> +			       const struct intel_dpll *best_clock,
>  			       unsigned int best_error_ppm,
>  			       unsigned int *error_ppm)
>  {
> @@ -917,12 +917,12 @@ static bool vlv_PLL_is_optimal(struct drm_device *dev, int target_freq,
>  static bool
>  vlv_find_best_dpll(const struct intel_limit *limit,
>  		   struct intel_crtc_state *crtc_state,
> -		   int target, int refclk, struct dpll *match_clock,
> -		   struct dpll *best_clock)
> +		   int target, int refclk, struct intel_dpll *match_clock,
> +		   struct intel_dpll *best_clock)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
> -	struct dpll clock;
> +	struct intel_dpll clock;
>  	unsigned int bestppm = 1000000;
>  	/* min update 19.2 MHz */
>  	int max_n = min(limit->n.max, refclk / 19200);
> @@ -976,13 +976,13 @@ vlv_find_best_dpll(const struct intel_limit *limit,
>  static bool
>  chv_find_best_dpll(const struct intel_limit *limit,
>  		   struct intel_crtc_state *crtc_state,
> -		   int target, int refclk, struct dpll *match_clock,
> -		   struct dpll *best_clock)
> +		   int target, int refclk, struct intel_dpll *match_clock,
> +		   struct intel_dpll *best_clock)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	unsigned int best_error_ppm;
> -	struct dpll clock;
> +	struct intel_dpll clock;
>  	uint64_t m2;
>  	int found = false;
>  
> @@ -1032,7 +1032,7 @@ chv_find_best_dpll(const struct intel_limit *limit,
>  }
>  
>  bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
> -			struct dpll *best_clock)
> +			struct intel_dpll *best_clock)
>  {
>  	int refclk = 100000;
>  	const struct intel_limit *limit = &intel_limits_bxt;
> @@ -7041,19 +7041,19 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
>  		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
>  }
>  
> -static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
> +static uint32_t pnv_dpll_compute_fp(struct intel_dpll *dpll)
>  {
>  	return (1 << dpll->n) << 16 | dpll->m2;
>  }
>  
> -static uint32_t i9xx_dpll_compute_fp(struct dpll *dpll)
> +static uint32_t i9xx_dpll_compute_fp(struct intel_dpll *dpll)
>  {
>  	return dpll->n << 16 | dpll->m1 << 8 | dpll->m2;
>  }
>  
>  static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
>  				     struct intel_crtc_state *crtc_state,
> -				     struct dpll *reduced_clock)
> +				     struct intel_dpll *reduced_clock)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	u32 fp, fp2 = 0;
> @@ -7430,7 +7430,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>   * be enabled.
>   */
>  int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> -		     const struct dpll *dpll)
> +		     const struct intel_dpll *dpll)
>  {
>  	struct intel_crtc *crtc =
>  		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> @@ -7477,13 +7477,13 @@ void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe)
>  
>  static void i9xx_compute_dpll(struct intel_crtc *crtc,
>  			      struct intel_crtc_state *crtc_state,
> -			      struct dpll *reduced_clock)
> +			      struct intel_dpll *reduced_clock)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 dpll;
>  	bool is_sdvo;
> -	struct dpll *clock = &crtc_state->dpll;
> +	struct intel_dpll *clock = &crtc_state->dpll;
>  
>  	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
>  
> @@ -7553,12 +7553,12 @@ static void i9xx_compute_dpll(struct intel_crtc *crtc,
>  
>  static void i8xx_compute_dpll(struct intel_crtc *crtc,
>  			      struct intel_crtc_state *crtc_state,
> -			      struct dpll *reduced_clock)
> +			      struct intel_dpll *reduced_clock)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 dpll;
> -	struct dpll *clock = &crtc_state->dpll;
> +	struct intel_dpll *clock = &crtc_state->dpll;
>  
>  	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
>  
> @@ -8024,7 +8024,7 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int pipe = pipe_config->cpu_transcoder;
> -	struct dpll clock;
> +	struct intel_dpll clock;
>  	u32 mdiv;
>  	int refclk = 100000;
>  
> @@ -8121,7 +8121,7 @@ static void chv_crtc_clock_get(struct intel_crtc *crtc,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int pipe = pipe_config->cpu_transcoder;
>  	enum dpio_channel port = vlv_pipe_to_channel(pipe);
> -	struct dpll clock;
> +	struct intel_dpll clock;
>  	u32 cmn_dw13, pll_dw0, pll_dw1, pll_dw2, pll_dw3;
>  	int refclk = 100000;
>  
> @@ -8777,14 +8777,14 @@ int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp)
>  	return DIV_ROUND_UP(bps, link_bw * 8);
>  }
>  
> -static bool ironlake_needs_fb_cb_tune(struct dpll *dpll, int factor)
> +static bool ironlake_needs_fb_cb_tune(struct intel_dpll *dpll, int factor)
>  {
>  	return i9xx_dpll_compute_m(dpll) < factor * dpll->n;
>  }
>  
>  static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  				  struct intel_crtc_state *crtc_state,
> -				  struct dpll *reduced_clock)
> +				  struct intel_dpll *reduced_clock)
>  {
>  	struct drm_crtc *crtc = &intel_crtc->base;
>  	struct drm_device *dev = crtc->dev;
> @@ -8892,7 +8892,7 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct dpll reduced_clock;
> +	struct intel_dpll reduced_clock;
>  	bool has_reduced_clock = false;
>  	struct intel_shared_dpll *pll;
>  	const struct intel_limit *limit;
> @@ -10622,7 +10622,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  	int pipe = pipe_config->cpu_transcoder;
>  	u32 dpll = pipe_config->dpll_hw_state.dpll;
>  	u32 fp;
> -	struct dpll clock;
> +	struct intel_dpll clock;
>  	int port_clock;
>  	int refclk = i9xx_pll_refclk(dev, pipe_config);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3633002..e85711a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -49,7 +49,7 @@
>  
>  struct dp_link_dpll {
>  	int clock;
> -	struct dpll dpll;
> +	struct intel_dpll dpll;
>  };
>  
>  static const struct dp_link_dpll gen4_dpll[] = {
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index c283ba4..3499ed2 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -1508,7 +1508,7 @@ bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>  	int clock = crtc_state->port_clock;
>  
>  	if (encoder->type == INTEL_OUTPUT_HDMI) {
> -		struct dpll best_clock;
> +		struct intel_dpll best_clock;
>  
>  		/* Calculate HDMI div */
>  		/*
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8405ff7..2033ff1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -266,7 +266,7 @@ struct intel_connector {
>  	struct intel_dp *mst_port;
>  };
>  
> -struct dpll {
> +struct intel_dpll {
>  	/* given values */
>  	int n;
>  	int m1, m2;
> @@ -484,7 +484,7 @@ struct intel_crtc_state {
>  
>  	/* Settings for the intel dpll used on pretty much everything but
>  	 * haswell. */
> -	struct dpll dpll;
> +	struct intel_dpll dpll;
>  
>  	/* Selected dpll when shared or NULL. */
>  	struct intel_shared_dpll *shared_dpll;
> @@ -1206,7 +1206,7 @@ void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
>  				    enum pipe pipe);
>  
>  int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> -		     const struct dpll *dpll);
> +		     const struct intel_dpll *dpll);
>  void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe);
>  int lpt_get_iclkip(struct drm_i915_private *dev_priv);
>  
> @@ -1255,8 +1255,8 @@ void intel_dp_get_m_n(struct intel_crtc *crtc,
>  void intel_dp_set_m_n(struct intel_crtc *crtc, 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,
> -			struct dpll *best_clock);
> -int chv_calc_dpll_params(int refclk, struct dpll *pll_clock);
> +			struct intel_dpll *best_clock);
> +int chv_calc_dpll_params(int refclk, struct intel_dpll *pll_clock);
>  
>  bool intel_crtc_active(struct drm_crtc *crtc);
>  void hsw_enable_ips(struct intel_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 2128fae..96bedd0 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1101,7 +1101,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
>  static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state *pipe_config)
>  {
>  	unsigned dotclock = pipe_config->port_clock;
> -	struct dpll *clock = &pipe_config->dpll;
> +	struct intel_dpll *clock = &pipe_config->dpll;
>  
>  	/* SDVO TV has fixed PLL values depend on its clock range,
>  	   this mirrors vbios setting. */
> -- 
> 2.5.5

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

✗ Ro.CI.BAT: failure for Move dpio access out of intel_display.c

[Patchwork]

== Series Details ==

Series: Move dpio access out of intel_display.c
URL   : https://patchwork.freedesktop.org/series/7150/
State : failure

== Summary ==

Series 7150v1 Move dpio access out of intel_display.c
http://patchwork.freedesktop.org/api/1.0/series/7150/revisions/1/mbox

Test drv_hangman:
        Subgroup error-state-basic:
                fail       -> PASS       (ro-ilk1-i5-650)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (ro-ivb2-i7-3770)

ro-bdw-i5-5250u  total:219  pass:181  dwarn:0   dfail:0   fail:0   skip:38 
ro-bdw-i7-5557U  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-bsw-n3050     total:219  pass:175  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:218  pass:175  dwarn:0   dfail:0   fail:2   skip:41 
ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
ro-hsw-i7-4770r  total:219  pass:194  dwarn:0   dfail:0   fail:0   skip:25 
ro-ilk-i7-620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
ro-ilk1-i5-650   total:214  pass:152  dwarn:0   dfail:0   fail:1   skip:61 
ro-ivb-i7-3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
ro-ivb2-i7-3770  total:219  pass:186  dwarn:0   dfail:0   fail:1   skip:32 
ro-skl-i7-6700hq total:214  pass:189  dwarn:0   dfail:0   fail:0   skip:25 
ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 

Results at /archive/results/CI_IGT_test/RO_Patchwork_892/

accd824 drm-intel-nightly: 2016y-05m-13d-14h-38m-15s UTC integration manifest
e926ca9 drm/i915: Move toggling of CHV DPIO_DCLKP_EN to intel_dpio_phy.c
7d40ee1 drm/i915: Move CHV divider readout to intel_dpio_phy.c
4c6ddda drm/i915: Move VLV divider readout to intel_dpio_phy.c
2ba6b64 drm/i915: Merge vlv/chv _prepare_pll() with their enable counterpart
55b0906 drm/i915: Move dpio code of VLV/CHV dpll enabling to intel_dpio_phy.c
74a984c drm/i915: Rename struct dpll to struct intel_dpll

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

Re: [PATCH 1/6] drm/i915: Rename struct dpll to struct intel_dpll

[Ander Conselvan De Oliveira]

On Fri, 2016-05-13 at 17:29 +0300, Ville Syrjälä wrote:
> On Fri, May 13, 2016 at 05:14:58PM +0300, Ander Conselvan de Oliveira wrote:
> > 
> > Prefix struct dpll with intel_ to follow the convention in the driver.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@inte
> > l.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h       |  2 +-
> >  drivers/gpu/drm/i915/intel_ddi.c      |  2 +-
> >  drivers/gpu/drm/i915/intel_display.c  | 76 +++++++++++++++++---------------
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c       |  2 +-
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c |  2 +-
> >  drivers/gpu/drm/i915/intel_drv.h      | 10 ++---
> >  drivers/gpu/drm/i915/intel_sdvo.c     |  2 +-
> >  7 files changed, 48 insertions(+), 48 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 7a0b513..5f9dda2 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -577,7 +577,7 @@ struct intel_crtc_state;
> >  struct intel_initial_plane_config;
> >  struct intel_crtc;
> >  struct intel_limit;
> > -struct dpll;
> > +struct intel_dpll;
> Now that I see that, it kinda makes me think someone is bould to
> confuse it to mean an actual DPLL instead of just the dividers and
> whanot.

Yeah, its a bit of a confusing name. Perhaps the structure could have dividers
in the name, but it also has the derived values. Maybe, dpll_state as a counter
part to dpll_hw_state?

Anyway, I'll drop the patch for now. I could revisit this after I send some
pending patches, though.

Ander

> 
> > 
> >  
> >  struct drm_i915_display_funcs {
> >  	int (*get_display_clock_speed)(struct drm_device *dev);
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index c454744..1387acd 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -948,7 +948,7 @@ static int bxt_calc_pll_link(struct drm_i915_private
> > *dev_priv,
> >  {
> >  	struct intel_shared_dpll *pll;
> >  	struct intel_dpll_hw_state *state;
> > -	struct dpll clock;
> > +	struct intel_dpll clock;
> >  
> >  	/* For DDI ports we always use a shared PLL. */
> >  	if (WARN_ON(dpll == DPLL_ID_PRIVATE))
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 05c7533..048a5bf 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -578,7 +578,7 @@ static bool intel_pipe_will_have_type(const struct
> > intel_crtc_state *crtc_state,
> >   * divided-down version of it.
> >   */
> >  /* m1 is reserved as 0 in Pineview, n is a ring counter */
> > -static int pnv_calc_dpll_params(int refclk, struct dpll *clock)
> > +static int pnv_calc_dpll_params(int refclk, struct intel_dpll *clock)
> >  {
> >  	clock->m = clock->m2 + 2;
> >  	clock->p = clock->p1 * clock->p2;
> > @@ -590,12 +590,12 @@ static int pnv_calc_dpll_params(int refclk, struct
> > dpll *clock)
> >  	return clock->dot;
> >  }
> >  
> > -static uint32_t i9xx_dpll_compute_m(struct dpll *dpll)
> > +static uint32_t i9xx_dpll_compute_m(struct intel_dpll *dpll)
> >  {
> >  	return 5 * (dpll->m1 + 2) + (dpll->m2 + 2);
> >  }
> >  
> > -static int i9xx_calc_dpll_params(int refclk, struct dpll *clock)
> > +static int i9xx_calc_dpll_params(int refclk, struct intel_dpll *clock)
> >  {
> >  	clock->m = i9xx_dpll_compute_m(clock);
> >  	clock->p = clock->p1 * clock->p2;
> > @@ -607,7 +607,7 @@ static int i9xx_calc_dpll_params(int refclk, struct dpll
> > *clock)
> >  	return clock->dot;
> >  }
> >  
> > -static int vlv_calc_dpll_params(int refclk, struct dpll *clock)
> > +static int vlv_calc_dpll_params(int refclk, struct intel_dpll *clock)
> >  {
> >  	clock->m = clock->m1 * clock->m2;
> >  	clock->p = clock->p1 * clock->p2;
> > @@ -619,7 +619,7 @@ static int vlv_calc_dpll_params(int refclk, struct dpll
> > *clock)
> >  	return clock->dot / 5;
> >  }
> >  
> > -int chv_calc_dpll_params(int refclk, struct dpll *clock)
> > +int chv_calc_dpll_params(int refclk, struct intel_dpll *clock)
> >  {
> >  	clock->m = clock->m1 * clock->m2;
> >  	clock->p = clock->p1 * clock->p2;
> > @@ -640,7 +640,7 @@ int chv_calc_dpll_params(int refclk, struct dpll *clock)
> >  
> >  static bool intel_PLL_is_valid(struct drm_device *dev,
> >  			       const struct intel_limit *limit,
> > -			       const struct dpll *clock)
> > +			       const struct intel_dpll *clock)
> >  {
> >  	if (clock->n   < limit->n.min   || limit->n.max   < clock->n)
> >  		INTELPllInvalid("n out of range\n");
> > @@ -712,11 +712,11 @@ i9xx_select_p2_div(const struct intel_limit *limit,
> >  static bool
> >  i9xx_find_best_dpll(const struct intel_limit *limit,
> >  		    struct intel_crtc_state *crtc_state,
> > -		    int target, int refclk, struct dpll *match_clock,
> > -		    struct dpll *best_clock)
> > +		    int target, int refclk, struct intel_dpll *match_clock,
> > +		    struct intel_dpll *best_clock)
> >  {
> >  	struct drm_device *dev = crtc_state->base.crtc->dev;
> > -	struct dpll clock;
> > +	struct intel_dpll clock;
> >  	int err = target;
> >  
> >  	memset(best_clock, 0, sizeof(*best_clock));
> > @@ -769,11 +769,11 @@ i9xx_find_best_dpll(const struct intel_limit *limit,
> >  static bool
> >  pnv_find_best_dpll(const struct intel_limit *limit,
> >  		   struct intel_crtc_state *crtc_state,
> > -		   int target, int refclk, struct dpll *match_clock,
> > -		   struct dpll *best_clock)
> > +		   int target, int refclk, struct intel_dpll *match_clock,
> > +		   struct intel_dpll *best_clock)
> >  {
> >  	struct drm_device *dev = crtc_state->base.crtc->dev;
> > -	struct dpll clock;
> > +	struct intel_dpll clock;
> >  	int err = target;
> >  
> >  	memset(best_clock, 0, sizeof(*best_clock));
> > @@ -824,11 +824,11 @@ pnv_find_best_dpll(const struct intel_limit *limit,
> >  static bool
> >  g4x_find_best_dpll(const struct intel_limit *limit,
> >  		   struct intel_crtc_state *crtc_state,
> > -		   int target, int refclk, struct dpll *match_clock,
> > -		   struct dpll *best_clock)
> > +		   int target, int refclk, struct intel_dpll *match_clock,
> > +		   struct intel_dpll *best_clock)
> >  {
> >  	struct drm_device *dev = crtc_state->base.crtc->dev;
> > -	struct dpll clock;
> > +	struct intel_dpll clock;
> >  	int max_n;
> >  	bool found = false;
> >  	/* approximately equals target * 0.00585 */
> > @@ -874,8 +874,8 @@ g4x_find_best_dpll(const struct intel_limit *limit,
> >   * best configuration and error found so far. Return the calculated error.
> >   */
> >  static bool vlv_PLL_is_optimal(struct drm_device *dev, int target_freq,
> > -			       const struct dpll *calculated_clock,
> > -			       const struct dpll *best_clock,
> > +			       const struct intel_dpll *calculated_clock,
> > +			       const struct intel_dpll *best_clock,
> >  			       unsigned int best_error_ppm,
> >  			       unsigned int *error_ppm)
> >  {
> > @@ -917,12 +917,12 @@ static bool vlv_PLL_is_optimal(struct drm_device *dev,
> > int target_freq,
> >  static bool
> >  vlv_find_best_dpll(const struct intel_limit *limit,
> >  		   struct intel_crtc_state *crtc_state,
> > -		   int target, int refclk, struct dpll *match_clock,
> > -		   struct dpll *best_clock)
> > +		   int target, int refclk, struct intel_dpll *match_clock,
> > +		   struct intel_dpll *best_clock)
> >  {
> >  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> >  	struct drm_device *dev = crtc->base.dev;
> > -	struct dpll clock;
> > +	struct intel_dpll clock;
> >  	unsigned int bestppm = 1000000;
> >  	/* min update 19.2 MHz */
> >  	int max_n = min(limit->n.max, refclk / 19200);
> > @@ -976,13 +976,13 @@ vlv_find_best_dpll(const struct intel_limit *limit,
> >  static bool
> >  chv_find_best_dpll(const struct intel_limit *limit,
> >  		   struct intel_crtc_state *crtc_state,
> > -		   int target, int refclk, struct dpll *match_clock,
> > -		   struct dpll *best_clock)
> > +		   int target, int refclk, struct intel_dpll *match_clock,
> > +		   struct intel_dpll *best_clock)
> >  {
> >  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> >  	struct drm_device *dev = crtc->base.dev;
> >  	unsigned int best_error_ppm;
> > -	struct dpll clock;
> > +	struct intel_dpll clock;
> >  	uint64_t m2;
> >  	int found = false;
> >  
> > @@ -1032,7 +1032,7 @@ chv_find_best_dpll(const struct intel_limit *limit,
> >  }
> >  
> >  bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int
> > target_clock,
> > -			struct dpll *best_clock)
> > +			struct intel_dpll *best_clock)
> >  {
> >  	int refclk = 100000;
> >  	const struct intel_limit *limit = &intel_limits_bxt;
> > @@ -7041,19 +7041,19 @@ static inline bool intel_panel_use_ssc(struct
> > drm_i915_private *dev_priv)
> >  		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
> >  }
> >  
> > -static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
> > +static uint32_t pnv_dpll_compute_fp(struct intel_dpll *dpll)
> >  {
> >  	return (1 << dpll->n) << 16 | dpll->m2;
> >  }
> >  
> > -static uint32_t i9xx_dpll_compute_fp(struct dpll *dpll)
> > +static uint32_t i9xx_dpll_compute_fp(struct intel_dpll *dpll)
> >  {
> >  	return dpll->n << 16 | dpll->m1 << 8 | dpll->m2;
> >  }
> >  
> >  static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
> >  				     struct intel_crtc_state *crtc_state,
> > -				     struct dpll *reduced_clock)
> > +				     struct intel_dpll *reduced_clock)
> >  {
> >  	struct drm_device *dev = crtc->base.dev;
> >  	u32 fp, fp2 = 0;
> > @@ -7430,7 +7430,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
> >   * be enabled.
> >   */
> >  int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> > -		     const struct dpll *dpll)
> > +		     const struct intel_dpll *dpll)
> >  {
> >  	struct intel_crtc *crtc =
> >  		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> > @@ -7477,13 +7477,13 @@ void vlv_force_pll_off(struct drm_device *dev, enum
> > pipe pipe)
> >  
> >  static void i9xx_compute_dpll(struct intel_crtc *crtc,
> >  			      struct intel_crtc_state *crtc_state,
> > -			      struct dpll *reduced_clock)
> > +			      struct intel_dpll *reduced_clock)
> >  {
> >  	struct drm_device *dev = crtc->base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	u32 dpll;
> >  	bool is_sdvo;
> > -	struct dpll *clock = &crtc_state->dpll;
> > +	struct intel_dpll *clock = &crtc_state->dpll;
> >  
> >  	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
> >  
> > @@ -7553,12 +7553,12 @@ static void i9xx_compute_dpll(struct intel_crtc
> > *crtc,
> >  
> >  static void i8xx_compute_dpll(struct intel_crtc *crtc,
> >  			      struct intel_crtc_state *crtc_state,
> > -			      struct dpll *reduced_clock)
> > +			      struct intel_dpll *reduced_clock)
> >  {
> >  	struct drm_device *dev = crtc->base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	u32 dpll;
> > -	struct dpll *clock = &crtc_state->dpll;
> > +	struct intel_dpll *clock = &crtc_state->dpll;
> >  
> >  	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
> >  
> > @@ -8024,7 +8024,7 @@ static void vlv_crtc_clock_get(struct intel_crtc
> > *crtc,
> >  	struct drm_device *dev = crtc->base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	int pipe = pipe_config->cpu_transcoder;
> > -	struct dpll clock;
> > +	struct intel_dpll clock;
> >  	u32 mdiv;
> >  	int refclk = 100000;
> >  
> > @@ -8121,7 +8121,7 @@ static void chv_crtc_clock_get(struct intel_crtc
> > *crtc,
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	int pipe = pipe_config->cpu_transcoder;
> >  	enum dpio_channel port = vlv_pipe_to_channel(pipe);
> > -	struct dpll clock;
> > +	struct intel_dpll clock;
> >  	u32 cmn_dw13, pll_dw0, pll_dw1, pll_dw2, pll_dw3;
> >  	int refclk = 100000;
> >  
> > @@ -8777,14 +8777,14 @@ int ironlake_get_lanes_required(int target_clock,
> > int link_bw, int bpp)
> >  	return DIV_ROUND_UP(bps, link_bw * 8);
> >  }
> >  
> > -static bool ironlake_needs_fb_cb_tune(struct dpll *dpll, int factor)
> > +static bool ironlake_needs_fb_cb_tune(struct intel_dpll *dpll, int factor)
> >  {
> >  	return i9xx_dpll_compute_m(dpll) < factor * dpll->n;
> >  }
> >  
> >  static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
> >  				  struct intel_crtc_state *crtc_state,
> > -				  struct dpll *reduced_clock)
> > +				  struct intel_dpll *reduced_clock)
> >  {
> >  	struct drm_crtc *crtc = &intel_crtc->base;
> >  	struct drm_device *dev = crtc->dev;
> > @@ -8892,7 +8892,7 @@ static int ironlake_crtc_compute_clock(struct
> > intel_crtc *crtc,
> >  {
> >  	struct drm_device *dev = crtc->base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	struct dpll reduced_clock;
> > +	struct intel_dpll reduced_clock;
> >  	bool has_reduced_clock = false;
> >  	struct intel_shared_dpll *pll;
> >  	const struct intel_limit *limit;
> > @@ -10622,7 +10622,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc
> > *crtc,
> >  	int pipe = pipe_config->cpu_transcoder;
> >  	u32 dpll = pipe_config->dpll_hw_state.dpll;
> >  	u32 fp;
> > -	struct dpll clock;
> > +	struct intel_dpll clock;
> >  	int port_clock;
> >  	int refclk = i9xx_pll_refclk(dev, pipe_config);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 3633002..e85711a 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -49,7 +49,7 @@
> >  
> >  struct dp_link_dpll {
> >  	int clock;
> > -	struct dpll dpll;
> > +	struct intel_dpll dpll;
> >  };
> >  
> >  static const struct dp_link_dpll gen4_dpll[] = {
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > index c283ba4..3499ed2 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > @@ -1508,7 +1508,7 @@ bxt_get_dpll(struct intel_crtc *crtc, struct
> > intel_crtc_state *crtc_state,
> >  	int clock = crtc_state->port_clock;
> >  
> >  	if (encoder->type == INTEL_OUTPUT_HDMI) {
> > -		struct dpll best_clock;
> > +		struct intel_dpll best_clock;
> >  
> >  		/* Calculate HDMI div */
> >  		/*
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 8405ff7..2033ff1 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -266,7 +266,7 @@ struct intel_connector {
> >  	struct intel_dp *mst_port;
> >  };
> >  
> > -struct dpll {
> > +struct intel_dpll {
> >  	/* given values */
> >  	int n;
> >  	int m1, m2;
> > @@ -484,7 +484,7 @@ struct intel_crtc_state {
> >  
> >  	/* Settings for the intel dpll used on pretty much everything but
> >  	 * haswell. */
> > -	struct dpll dpll;
> > +	struct intel_dpll dpll;
> >  
> >  	/* Selected dpll when shared or NULL. */
> >  	struct intel_shared_dpll *shared_dpll;
> > @@ -1206,7 +1206,7 @@ void assert_pch_transcoder_disabled(struct
> > drm_i915_private *dev_priv,
> >  				    enum pipe pipe);
> >  
> >  int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> > -		     const struct dpll *dpll);
> > +		     const struct intel_dpll *dpll);
> >  void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe);
> >  int lpt_get_iclkip(struct drm_i915_private *dev_priv);
> >  
> > @@ -1255,8 +1255,8 @@ void intel_dp_get_m_n(struct intel_crtc *crtc,
> >  void intel_dp_set_m_n(struct intel_crtc *crtc, 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,
> > -			struct dpll *best_clock);
> > -int chv_calc_dpll_params(int refclk, struct dpll *pll_clock);
> > +			struct intel_dpll *best_clock);
> > +int chv_calc_dpll_params(int refclk, struct intel_dpll *pll_clock);
> >  
> >  bool intel_crtc_active(struct drm_crtc *crtc);
> >  void hsw_enable_ips(struct intel_crtc *crtc);
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> > b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 2128fae..96bedd0 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -1101,7 +1101,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo
> > *intel_sdvo,
> >  static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state *pipe_config)
> >  {
> >  	unsigned dotclock = pipe_config->port_clock;
> > -	struct dpll *clock = &pipe_config->dpll;
> > +	struct intel_dpll *clock = &pipe_config->dpll;
> >  
> >  	/* SDVO TV has fixed PLL values depend on its clock range,
> >  	   this mirrors vbios setting. */
> > -- 
> > 2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915: Move toggling of CHV DPIO_DCLKP_EN to intel_dpio_phy.c

[Ander Conselvan De Oliveira]

On Fri, 2016-05-13 at 17:25 +0300, Ville Syrjälä wrote:
> On Fri, May 13, 2016 at 05:15:03PM +0300, Ander Conselvan de Oliveira wrote:
> > 
> > This simplifies the pll enable/disable a code a bit and hides the
> > sideband message neatly in intel_dpio_phy.c.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@inte
> > l.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h       |  2 ++
> >  drivers/gpu/drm/i915/intel_display.c  | 19 ++-----------------
> >  drivers/gpu/drm/i915/intel_dpio_phy.c | 18 ++++++++++++++++++
> >  3 files changed, 22 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 149317c..44f4b7a 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3630,6 +3630,8 @@ void chv_phy_post_pll_disable(struct intel_encoder
> > *encoder);
> >  void chv_phy_prepare_pll(struct intel_crtc *crtc, u32 bestn,
> >  			 u32 bestm1, u32 bestm2, u32 bestp1, u32 bestp2,
> >  			 int vco);
> > +void chv_phy_toggle_dclkp(struct drm_i915_private *dev_priv, enum pipe
> > pipe,
> > +			  bool enable);
> Toggle makes me think it just flips the bit. _enable_dclkp()
> _set_dclkp() or something might be better?

I'll resend with _set. I find a bit weird to have a function called enable that
disables things.

Ander

> 
> > 
> >  void chv_phy_read_dividers(struct drm_i915_private *dev_priv,
> >  			   enum pipe pipe, struct intel_dpll *clock);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 5ba000a..d43bdff 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1574,17 +1574,9 @@ static void _chv_enable_pll(struct intel_crtc *crtc,
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	enum pipe pipe = crtc->pipe;
> > -	enum dpio_channel port = vlv_pipe_to_channel(pipe);
> > -	u32 tmp;
> > -
> > -	mutex_lock(&dev_priv->sb_lock);
> >  
> >  	/* Enable back the 10bit clock to display controller */
> > -	tmp = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
> > -	tmp |= DPIO_DCLKP_EN;
> > -	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), tmp);
> > -
> > -	mutex_unlock(&dev_priv->sb_lock);
> > +	chv_phy_toggle_dclkp(dev_priv, pipe, true);
> >  
> >  	/*
> >  	 * Need to wait > 100ns between dclkp clock enable bit and PLL
> > enable.
> > @@ -1777,7 +1769,6 @@ static void vlv_disable_pll(struct drm_i915_private
> > *dev_priv, enum pipe pipe)
> >  
> >  static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe
> > pipe)
> >  {
> > -	enum dpio_channel port = vlv_pipe_to_channel(pipe);
> >  	u32 val;
> >  
> >  	/* Make sure the pipe isn't still relying on us */
> > @@ -1791,14 +1782,8 @@ static void chv_disable_pll(struct drm_i915_private
> > *dev_priv, enum pipe pipe)
> >  	I915_WRITE(DPLL(pipe), val);
> >  	POSTING_READ(DPLL(pipe));
> >  
> > -	mutex_lock(&dev_priv->sb_lock);
> > -
> >  	/* Disable 10bit clock to display controller */
> > -	val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
> > -	val &= ~DPIO_DCLKP_EN;
> > -	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), val);
> > -
> > -	mutex_unlock(&dev_priv->sb_lock);
> > +	chv_phy_toggle_dclkp(dev_priv, pipe, false);
> >  }
> >  
> >  void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
> > diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c
> > b/drivers/gpu/drm/i915/intel_dpio_phy.c
> > index 2a5d333..64788e3 100644
> > --- a/drivers/gpu/drm/i915/intel_dpio_phy.c
> > +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
> > @@ -461,6 +461,24 @@ void chv_phy_prepare_pll(struct intel_crtc *crtc, u32
> > bestn,
> >  	mutex_unlock(&dev_priv->sb_lock);
> >  }
> >  
> > +void chv_phy_toggle_dclkp(struct drm_i915_private *dev_priv, enum pipe
> > pipe,
> > +			  bool enable)
> > +{
> > +	enum dpio_channel port = vlv_pipe_to_channel(pipe);
> > +	u32 dpio_val;
> > +
> > +	mutex_lock(&dev_priv->sb_lock);
> > +
> > +	dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
> > +	if (enable)
> > +		dpio_val |= DPIO_DCLKP_EN;
> > +	else
> > +		dpio_val &= ~DPIO_DCLKP_EN;
> > +	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), dpio_val);
> > +
> > +	mutex_unlock(&dev_priv->sb_lock);
> > +}
> > +
> >  void chv_phy_read_dividers(struct drm_i915_private *dev_priv,
> >  			   enum pipe pipe, struct intel_dpll *clock)
> >  {
> > -- 
> > 2.5.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/6] Move dpio access out of intel_display.c

[Daniel Vetter]

On Fri, May 13, 2016 at 05:14:57PM +0300, Ander Conselvan de Oliveira wrote:
> Hi,
> 
> This series moves all of the calls to vlv_dpio_{read,write} to
> intel_dpio_phy.c. I think it simplifies the surrounding code a bit.

You still owe us all the kerneldoc for the intel_dpll_mgr.c extraction. I
think better to complete one extraction before starting the next one,
resulting in an even bigger mess than what we had before.
-Daniel

> 
> Thanks,
> Ander
> 
> Ander Conselvan de Oliveira (6):
>   drm/i915: Rename struct dpll to struct intel_dpll
>   drm/i915: Move dpio code of VLV/CHV dpll enabling to intel_dpio_phy.c
>   drm/i915: Merge vlv/chv _prepare_pll() with their enable counterpart
>   drm/i915: Move VLV divider readout to intel_dpio_phy.c
>   drm/i915: Move CHV divider readout to intel_dpio_phy.c
>   drm/i915: Move toggling of CHV DPIO_DCLKP_EN to intel_dpio_phy.c
> 
>  drivers/gpu/drm/i915/i915_drv.h       |  14 +-
>  drivers/gpu/drm/i915/intel_ddi.c      |   2 +-
>  drivers/gpu/drm/i915/intel_display.c  | 395 ++++++----------------------------
>  drivers/gpu/drm/i915/intel_dp.c       |   2 +-
>  drivers/gpu/drm/i915/intel_dpio_phy.c | 260 ++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dpll_mgr.c |   2 +-
>  drivers/gpu/drm/i915/intel_drv.h      |  10 +-
>  drivers/gpu/drm/i915/intel_sdvo.c     |   2 +-
>  8 files changed, 350 insertions(+), 337 deletions(-)
> 
> -- 
> 2.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/6] Move dpio access out of intel_display.c

[Ander Conselvan De Oliveira]

On Tue, 2016-05-17 at 14:26 +0200, Daniel Vetter wrote:
> On Fri, May 13, 2016 at 05:14:57PM +0300, Ander Conselvan de Oliveira wrote:
> > 
> > Hi,
> > 
> > This series moves all of the calls to vlv_dpio_{read,write} to
> > intel_dpio_phy.c. I think it simplifies the surrounding code a bit.
> You still owe us all the kerneldoc for the intel_dpll_mgr.c extraction.

https://patchwork.freedesktop.org/series/7294/

>  I think better to complete one extraction before starting the next one,
> resulting in an even bigger mess than what we had before.

This is actually part of the same thing. These are prep patches for moving
VLV/CHV into the dpll infrastructure. But fair enough.


But I have to disagree this would create an even bigger mess. There is so much
code in intel_display.c that most static functions there are the equivalent of
an undocumented non-static function elsewhere. And since they are in the same
pile of 400+ functions, it is not obvious the documentation is missing. So I'd
claim splitting code out of intel_display.c, even if without documentation, is
an improvement.

With the current rules we transfer the burden of writing documentation from the
person that made intel_display.c longer to the one trying to make it smaller.
Maybe we should have an exception that everything in intel_display.c needs
kerneldoc?

Ander


> -Daniel
> 
> > 
> > 
> > Thanks,
> > Ander
> > 
> > Ander Conselvan de Oliveira (6):
> >   drm/i915: Rename struct dpll to struct intel_dpll
> >   drm/i915: Move dpio code of VLV/CHV dpll enabling to intel_dpio_phy.c
> >   drm/i915: Merge vlv/chv _prepare_pll() with their enable counterpart
> >   drm/i915: Move VLV divider readout to intel_dpio_phy.c
> >   drm/i915: Move CHV divider readout to intel_dpio_phy.c
> >   drm/i915: Move toggling of CHV DPIO_DCLKP_EN to intel_dpio_phy.c
> > 
> >  drivers/gpu/drm/i915/i915_drv.h       |  14 +-
> >  drivers/gpu/drm/i915/intel_ddi.c      |   2 +-
> >  drivers/gpu/drm/i915/intel_display.c  | 395 ++++++-------------------------
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c       |   2 +-
> >  drivers/gpu/drm/i915/intel_dpio_phy.c | 260 ++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c |   2 +-
> >  drivers/gpu/drm/i915/intel_drv.h      |  10 +-
> >  drivers/gpu/drm/i915/intel_sdvo.c     |   2 +-
> >  8 files changed, 350 insertions(+), 337 deletions(-)
> > 
> > -- 
> > 2.5.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/6] Move dpio access out of intel_display.c

[Daniel Vetter]

On Tue, May 17, 2016 at 04:30:41PM +0300, Ander Conselvan De Oliveira wrote:
> On Tue, 2016-05-17 at 14:26 +0200, Daniel Vetter wrote:
> > On Fri, May 13, 2016 at 05:14:57PM +0300, Ander Conselvan de Oliveira wrote:
> > > 
> > > Hi,
> > > 
> > > This series moves all of the calls to vlv_dpio_{read,write} to
> > > intel_dpio_phy.c. I think it simplifies the surrounding code a bit.
> > You still owe us all the kerneldoc for the intel_dpll_mgr.c extraction.
> 
> https://patchwork.freedesktop.org/series/7294/
> 
> >  I think better to complete one extraction before starting the next one,
> > resulting in an even bigger mess than what we had before.
> 
> This is actually part of the same thing. These are prep patches for moving
> VLV/CHV into the dpll infrastructure. But fair enough.
> 
> 
> But I have to disagree this would create an even bigger mess. There is so much
> code in intel_display.c that most static functions there are the equivalent of
> an undocumented non-static function elsewhere. And since they are in the same
> pile of 400+ functions, it is not obvious the documentation is missing. So I'd
> claim splitting code out of intel_display.c, even if without documentation, is
> an improvement.

The problem is that we've done this for some of the atomic work and
fumbled the job, so now there's also a bunch of non-static functions that
should be static but can't because they ended up split across .c files. It
is possible to make it worse ;-)

> With the current rules we transfer the burden of writing documentation from the
> person that made intel_display.c longer to the one trying to make it smaller.
> Maybe we should have an exception that everything in intel_display.c needs
> kerneldoc?

See above, I think writing docs is a crucial step of making things
actually more orthogonal, instead of just smearing it across more source
files. And from my quick review of the dpll doc patch I think we
can/should do better - at least if you don't look at kerneldoc as just a
typing exercise, but as an opportunity to really review everything.

And yes there's a problem with shifting the work, but I think the correct
fix for that is by volunteering the offenders who make intel_display.c
bigger to help out with cleaning up. Not by making it easier on those that
clean up, since that doesn't fix the source of the problem.

And I tried to help out in the past with a few ideas around extracting the
crtc platform support code, but those all died in bikesheds :(
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx