[PATCH 0/8] drm/i915: Some DPLL crtc->state/config cleanups etc.

[PATCH 0/8] drm/i915: Some DPLL crtc->state/config cleanups etc.

[Ville Syrjala]

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

This series nukes some crtc->config and crtc->state uses from the
DPLL code. And I tossed in a few other random DPLL things I had lying around.

Ville Syrjälä (8):
  drm/i915: Set output_types to EDP for vlv/chv DPLL forcing
  drm/i915: Parametrize CBR_DPLLBMD_PIPE defines
  drm/i915: Eliminate crtc->config from VLV/CHV DPLL functions
  drm/i915: Pass crtc state to i9xx_enable_pll()
  drm/i915: Nuke the bogus kernel doc for i9xx_disable_pll()
  drm/i915: Add for_each_oldnew_intel_crtc_in_state()
  drm/i915: Shrink active_crtcs and active_pipes_changed to u8
  drm/i915: Eliminate crtc->state usage from DVO pipe tracking

 drivers/gpu/drm/i915/i915_drv.h      | 12 ++++-
 drivers/gpu/drm/i915/i915_reg.h      |  3 +-
 drivers/gpu/drm/i915/intel_display.c | 88 ++++++++++++++++--------------------
 drivers/gpu/drm/i915/intel_drv.h     |  6 ++-
 4 files changed, 56 insertions(+), 53 deletions(-)

-- 
2.13.5

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

[PATCH 1/8] drm/i915: Set output_types to EDP for vlv/chv DPLL forcing

[Ville Syrjala]

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

When we enable the DPLL for the PPS kick, let's tell the DPLL code
we're dealing with an eDP output. This shouldn't really matter, but
it's more consistent with the way the DPLL is configured when we're
actually enabling the eDP port for real.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8599e425abb1..ac68b68dd268 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6899,6 +6899,7 @@ int vlv_force_pll_on(struct drm_i915_private *dev_priv, enum pipe pipe,
 	pipe_config->base.crtc = &crtc->base;
 	pipe_config->pixel_multiplier = 1;
 	pipe_config->dpll = *dpll;
+	pipe_config->output_types = BIT(INTEL_OUTPUT_EDP);
 
 	if (IS_CHERRYVIEW(dev_priv)) {
 		chv_compute_dpll(crtc, pipe_config);
-- 
2.13.5

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

[PATCH 2/8] drm/i915: Parametrize CBR_DPLLBMD_PIPE defines

[Ville Syrjala]

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

Apply a bit of polish by parametrizing the CBR_DPLLBMD_PIPE defines.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9f03cd063afe..06eaccf2c720 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5663,8 +5663,7 @@ enum {
 #define  CBR_PWM_CLOCK_MUX_SELECT	(1<<30)
 
 #define CBR4_VLV			_MMIO(VLV_DISPLAY_BASE + 0x70450)
-#define  CBR_DPLLBMD_PIPE_C		(1<<29)
-#define  CBR_DPLLBMD_PIPE_B		(1<<18)
+#define  CBR_DPLLBMD_PIPE(pipe)		(1<<(7+(pipe)*11)) /* pipes B and C */
 
 /* FIFO watermark sizes etc */
 #define G4X_FIFO_LINE_SIZE	64
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ac68b68dd268..ef836fe3eacd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1539,7 +1539,7 @@ static void chv_enable_pll(struct intel_crtc *crtc,
 		 * DPLLCMD is AWOL. Use chicken bits to propagate
 		 * the value from DPLLBMD to either pipe B or C.
 		 */
-		I915_WRITE(CBR4_VLV, pipe == PIPE_B ? CBR_DPLLBMD_PIPE_B : CBR_DPLLBMD_PIPE_C);
+		I915_WRITE(CBR4_VLV, CBR_DPLLBMD_PIPE(pipe));
 		I915_WRITE(DPLL_MD(PIPE_B), pipe_config->dpll_hw_state.dpll_md);
 		I915_WRITE(CBR4_VLV, 0);
 		dev_priv->chv_dpll_md[pipe] = pipe_config->dpll_hw_state.dpll_md;
-- 
2.13.5

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

[PATCH 3/8] drm/i915: Eliminate crtc->config from VLV/CHV DPLL functions

[Ville Syrjala]

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

Use the passed in crtc state rather than crtc->config when configuring
the DPLL on VLV/CHV.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ef836fe3eacd..23fead59b825 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5831,11 +5831,11 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
 	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
 
 	if (IS_CHERRYVIEW(dev_priv)) {
-		chv_prepare_pll(intel_crtc, intel_crtc->config);
-		chv_enable_pll(intel_crtc, intel_crtc->config);
+		chv_prepare_pll(intel_crtc, pipe_config);
+		chv_enable_pll(intel_crtc, pipe_config);
 	} else {
-		vlv_prepare_pll(intel_crtc, intel_crtc->config);
-		vlv_enable_pll(intel_crtc, intel_crtc->config);
+		vlv_prepare_pll(intel_crtc, pipe_config);
+		vlv_enable_pll(intel_crtc, pipe_config);
 	}
 
 	intel_encoders_pre_enable(crtc, pipe_config, old_state);
@@ -6735,8 +6735,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 
 	/* Set HBR and RBR LPF coefficients */
 	if (pipe_config->port_clock == 162000 ||
-	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_ANALOG) ||
-	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
+	    intel_crtc_has_type(pipe_config, INTEL_OUTPUT_ANALOG) ||
+	    intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
 		vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW10(pipe),
 				 0x009f0003);
 	else
@@ -6763,7 +6763,7 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 
 	coreclk = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW7(pipe));
 	coreclk = (coreclk & 0x0000ff00) | 0x01c00000;
-	if (intel_crtc_has_dp_encoder(crtc->config))
+	if (intel_crtc_has_dp_encoder(pipe_config))
 		coreclk |= 0x01000000;
 	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW7(pipe), coreclk);
 
-- 
2.13.5

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

[PATCH 4/8] drm/i915: Pass crtc state to i9xx_enable_pll()

[Ville Syrjala]

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

Pass the crtc state to i9xx_enable_pll() and use it rather than
crtc->config.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 23fead59b825..140649f5b018 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1568,11 +1568,12 @@ static int intel_num_dvo_pipes(struct drm_i915_private *dev_priv)
 	return count;
 }
 
-static void i9xx_enable_pll(struct intel_crtc *crtc)
+static void i9xx_enable_pll(struct intel_crtc *crtc,
+			    const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	i915_reg_t reg = DPLL(crtc->pipe);
-	u32 dpll = crtc->config->dpll_hw_state.dpll;
+	u32 dpll = crtc_state->dpll_hw_state.dpll;
 	int i;
 
 	assert_pipe_disabled(dev_priv, crtc->pipe);
@@ -1609,7 +1610,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
 
 	if (INTEL_GEN(dev_priv) >= 4) {
 		I915_WRITE(DPLL_MD(crtc->pipe),
-			   crtc->config->dpll_hw_state.dpll_md);
+			   crtc_state->dpll_hw_state.dpll_md);
 	} else {
 		/* The pixel multiplier can only be updated once the
 		 * DPLL is enabled and the clocks are stable.
@@ -5894,7 +5895,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_encoders_pre_enable(crtc, pipe_config, old_state);
 
-	i9xx_enable_pll(intel_crtc);
+	i9xx_enable_pll(intel_crtc, pipe_config);
 
 	i9xx_pfit_enable(intel_crtc);
 
-- 
2.13.5

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

[PATCH 5/8] drm/i915: Nuke the bogus kernel doc for i9xx_disable_pll()

[Ville Syrjala]

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

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 140649f5b018..262f871c0fae 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1628,15 +1628,6 @@ static void i9xx_enable_pll(struct intel_crtc *crtc,
 	}
 }
 
-/**
- * i9xx_disable_pll - disable a PLL
- * @dev_priv: i915 private structure
- * @pipe: pipe PLL to disable
- *
- * Disable the PLL for @pipe, making sure the pipe is off first.
- *
- * Note!  This is for pre-ILK only.
- */
 static void i9xx_disable_pll(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-- 
2.13.5

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

[PATCH 6/8] drm/i915: Add for_each_oldnew_intel_crtc_in_state()

[Ville Syrjala]

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

Add for_each_oldnew_intel_crtc_in_state() and use it in
intel_modeset_checks() to let is play with the intel_ types rather than
the drm_ types.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  8 ++++++++
 drivers/gpu/drm/i915/intel_display.c | 12 ++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 068c35e49d99..d4ec44495c00 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -577,6 +577,14 @@ struct i915_hotplug {
 	     (__i)++) \
 		for_each_if (crtc)
 
+#define for_each_oldnew_intel_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \
+	for ((__i) = 0; \
+	     (__i) < (__state)->base.dev->mode_config.num_crtc && \
+		     ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \
+		      (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), \
+		      (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \
+	     (__i)++) \
+		for_each_if (crtc)
 
 #define for_each_oldnew_intel_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \
 	for ((__i) = 0; \
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 262f871c0fae..35fe8a98080e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11940,8 +11940,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 {
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+	struct intel_crtc *crtc;
+	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
 	int ret = 0, i;
 
 	if (!check_digital_port_conflicts(state)) {
@@ -11954,14 +11954,14 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 	intel_state->cdclk.logical = dev_priv->cdclk.logical;
 	intel_state->cdclk.actual = dev_priv->cdclk.actual;
 
-	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
-		if (new_crtc_state->active)
+	for_each_oldnew_intel_crtc_in_state(intel_state, crtc, old_crtc_state, new_crtc_state, i) {
+		if (new_crtc_state->base.active)
 			intel_state->active_crtcs |= 1 << i;
 		else
 			intel_state->active_crtcs &= ~(1 << i);
 
-		if (old_crtc_state->active != new_crtc_state->active)
-			intel_state->active_pipe_changes |= drm_crtc_mask(crtc);
+		if (old_crtc_state->base.active != new_crtc_state->base.active)
+			intel_state->active_pipe_changes |= drm_crtc_mask(&crtc->base);
 	}
 
 	/*
-- 
2.13.5

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

[PATCH 7/8] drm/i915: Shrink active_crtcs and active_pipes_changed to u8

[Ville Syrjala]

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

We only have three pipes, so 8 bits is more than sufficient to track
which is active. Also start using BIT() when populating them.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d4ec44495c00..f8d7599cff43 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2383,7 +2383,7 @@ struct drm_i915_private {
 	 */
 	struct mutex dpll_lock;
 
-	unsigned int active_crtcs;
+	u8 active_crtcs;
 	/* minimum acceptable cdclk for each pipe */
 	int min_cdclk[I915_MAX_PIPES];
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 35fe8a98080e..c33fce9d9824 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6032,7 +6032,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
 		intel_display_power_put(dev_priv, domain);
 	intel_crtc->enabled_power_domains = 0;
 
-	dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe);
+	dev_priv->active_crtcs &= ~BIT(intel_crtc->pipe);
 	dev_priv->min_cdclk[intel_crtc->pipe] = 0;
 }
 
@@ -11956,12 +11956,12 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 
 	for_each_oldnew_intel_crtc_in_state(intel_state, crtc, old_crtc_state, new_crtc_state, i) {
 		if (new_crtc_state->base.active)
-			intel_state->active_crtcs |= 1 << i;
+			intel_state->active_crtcs |= BIT(crtc->pipe);
 		else
-			intel_state->active_crtcs &= ~(1 << i);
+			intel_state->active_crtcs &= ~BIT(crtc->pipe);
 
 		if (old_crtc_state->base.active != new_crtc_state->base.active)
-			intel_state->active_pipe_changes |= drm_crtc_mask(&crtc->base);
+			intel_state->active_pipe_changes |= BIT(crtc->pipe);
 	}
 
 	/*
@@ -14951,7 +14951,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 		crtc->active = crtc_state->base.active;
 
 		if (crtc_state->base.active)
-			dev_priv->active_crtcs |= 1 << crtc->pipe;
+			dev_priv->active_crtcs |= BIT(crtc->pipe);
 
 		readout_plane_state(crtc);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 307807672896..96f683256d7e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -381,9 +381,9 @@ struct intel_atomic_state {
 	 * but the converse is not necessarily true; simply changing a mode may
 	 * not flip the final active status of any CRTC's
 	 */
-	unsigned int active_pipe_changes;
+	u8 active_pipe_changes;
 
-	unsigned int active_crtcs;
+	u8 active_crtcs;
 	/* minimum acceptable cdclk for each pipe */
 	int min_cdclk[I915_MAX_PIPES];
 
-- 
2.13.5

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

[PATCH 8/8] drm/i915: Eliminate crtc->state usage from DVO pipe tracking

[Ville Syrjala]

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

Change the DVO pipe tracking to maintain a bitmask in the top level
state, just as we do for active_crtcs. This gets rid of the ugly
intel_num_dvo_pipes() and its crtc->state and crtc->config usage.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f8d7599cff43..a352644ea5a5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2384,6 +2384,8 @@ struct drm_i915_private {
 	struct mutex dpll_lock;
 
 	u8 active_crtcs;
+	u8 dvo_pipes; /* bitmask of pipes driving DVO encoders */
+
 	/* minimum acceptable cdclk for each pipe */
 	int min_cdclk[I915_MAX_PIPES];
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c33fce9d9824..cb32f264a5d4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1555,19 +1555,6 @@ static void chv_enable_pll(struct intel_crtc *crtc,
 	}
 }
 
-static int intel_num_dvo_pipes(struct drm_i915_private *dev_priv)
-{
-	struct intel_crtc *crtc;
-	int count = 0;
-
-	for_each_intel_crtc(&dev_priv->drm, crtc) {
-		count += crtc->base.state->active &&
-			intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DVO);
-	}
-
-	return count;
-}
-
 static void i9xx_enable_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *crtc_state)
 {
@@ -1583,7 +1570,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc,
 		assert_panel_unlocked(dev_priv, crtc->pipe);
 
 	/* Enable DVO 2x clock on both PLLs if necessary */
-	if (IS_I830(dev_priv) && intel_num_dvo_pipes(dev_priv) > 0) {
+	if (IS_I830(dev_priv) && dev_priv->dvo_pipes != 0) {
 		/*
 		 * It appears to be important that we don't enable this
 		 * for the current pipe before otherwise configuring the
@@ -1634,9 +1621,7 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
 	enum pipe pipe = crtc->pipe;
 
 	/* Disable DVO 2x clock on both PLLs if necessary */
-	if (IS_I830(dev_priv) &&
-	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DVO) &&
-	    !intel_num_dvo_pipes(dev_priv)) {
+	if (IS_I830(dev_priv) && dev_priv->dvo_pipes == 0) {
 		I915_WRITE(DPLL(PIPE_B),
 			   I915_READ(DPLL(PIPE_B)) & ~DPLL_DVO_2X_MODE);
 		I915_WRITE(DPLL(PIPE_A),
@@ -6033,6 +6018,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
 	intel_crtc->enabled_power_domains = 0;
 
 	dev_priv->active_crtcs &= ~BIT(intel_crtc->pipe);
+	dev_priv->dvo_pipes &= ~BIT(intel_crtc->pipe);
 	dev_priv->min_cdclk[intel_crtc->pipe] = 0;
 }
 
@@ -11951,6 +11937,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 
 	intel_state->modeset = true;
 	intel_state->active_crtcs = dev_priv->active_crtcs;
+	intel_state->dvo_pipes = dev_priv->dvo_pipes;
 	intel_state->cdclk.logical = dev_priv->cdclk.logical;
 	intel_state->cdclk.actual = dev_priv->cdclk.actual;
 
@@ -11960,6 +11947,12 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 		else
 			intel_state->active_crtcs &= ~BIT(crtc->pipe);
 
+		if (new_crtc_state->base.active &&
+		    intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DVO))
+			intel_state->dvo_pipes |= BIT(crtc->pipe);
+		else
+			intel_state->dvo_pipes &= ~BIT(crtc->pipe);
+
 		if (old_crtc_state->base.active != new_crtc_state->base.active)
 			intel_state->active_pipe_changes |= BIT(crtc->pipe);
 	}
@@ -12560,6 +12553,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 		memcpy(dev_priv->min_cdclk, intel_state->min_cdclk,
 		       sizeof(intel_state->min_cdclk));
 		dev_priv->active_crtcs = intel_state->active_crtcs;
+		dev_priv->dvo_pipes = intel_state->dvo_pipes;
 		dev_priv->cdclk.logical = intel_state->cdclk.logical;
 		dev_priv->cdclk.actual = intel_state->cdclk.actual;
 	}
@@ -14935,6 +14929,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 	int i;
 
 	dev_priv->active_crtcs = 0;
+	dev_priv->dvo_pipes = 0;
 
 	for_each_intel_crtc(dev, crtc) {
 		struct intel_crtc_state *crtc_state =
@@ -14992,6 +14987,10 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			encoder->base.crtc = &crtc->base;
 			crtc_state->output_types |= 1 << encoder->type;
 			encoder->get_config(encoder, crtc_state);
+
+			if (crtc_state->base.active &&
+			    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DVO))
+				dev_priv->dvo_pipes |= BIT(crtc->pipe);
 		} else {
 			encoder->base.crtc = NULL;
 		}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 96f683256d7e..15ec6076e6a0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -384,6 +384,8 @@ struct intel_atomic_state {
 	u8 active_pipe_changes;
 
 	u8 active_crtcs;
+	u8 dvo_pipes; /* bitmask of pipes driving DVO encoders */
+
 	/* minimum acceptable cdclk for each pipe */
 	int min_cdclk[I915_MAX_PIPES];
 
-- 
2.13.5

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

✓ Fi.CI.BAT: success for drm/i915: Some DPLL crtc->state/config cleanups etc.

[Patchwork]

== Series Details ==

Series: drm/i915: Some DPLL crtc->state/config cleanups etc.
URL   : https://patchwork.freedesktop.org/series/30300/
State : success

== Summary ==

Series 30300v1 drm/i915: Some DPLL crtc->state/config cleanups etc.
https://patchwork.freedesktop.org/api/1.0/series/30300/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> FAIL       (fi-snb-2600) fdo#100215
        Subgroup basic-flip-before-cursor-atomic:
                incomplete -> PASS       (fi-bxt-j4205) fdo#102705
Test pm_rpm:
        Subgroup basic-rte:
                pass       -> DMESG-WARN (fi-cfl-s) fdo#102294

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#102705 https://bugs.freedesktop.org/show_bug.cgi?id=102705
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:441s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:457s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:377s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:537s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:268s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:509s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:504s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:499s
fi-cfl-s         total:289  pass:222  dwarn:35  dfail:0   fail:0   skip:32  time:557s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:454s
fi-glk-2a        total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:595s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:427s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:408s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:435s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:489s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:459s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:485s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:579s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:585s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:550s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:456s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:523s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:502s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:464s
fi-skl-x1585l    total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:482s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:569s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:1   skip:39  time:419s

76f9b11f445f4381eff873a62138ed0b00d08e80 drm-tip: 2017y-09m-13d-12h-28m-54s UTC integration manifest
afd5547161a3 drm/i915: Eliminate crtc->state usage from DVO pipe tracking
457e36d1e5b9 drm/i915: Shrink active_crtcs and active_pipes_changed to u8
4ecac6155ab9 drm/i915: Add for_each_oldnew_intel_crtc_in_state()
087dbb50c4d0 drm/i915: Nuke the bogus kernel doc for i9xx_disable_pll()
95792d411386 drm/i915: Pass crtc state to i9xx_enable_pll()
060b140e5840 drm/i915: Eliminate crtc->config from VLV/CHV DPLL functions
450555d732bd drm/i915: Parametrize CBR_DPLLBMD_PIPE defines
de34903a8af9 drm/i915: Set output_types to EDP for vlv/chv DPLL forcing

== Logs ==

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

Re: [PATCH 2/8] drm/i915: Parametrize CBR_DPLLBMD_PIPE defines

[Chris Wilson]

Quoting Ville Syrjala (2017-09-13 15:08:54)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Apply a bit of polish by parametrizing the CBR_DPLLBMD_PIPE defines.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 3 +--
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9f03cd063afe..06eaccf2c720 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5663,8 +5663,7 @@ enum {
>  #define  CBR_PWM_CLOCK_MUX_SELECT      (1<<30)
>  
>  #define CBR4_VLV                       _MMIO(VLV_DISPLAY_BASE + 0x70450)
> -#define  CBR_DPLLBMD_PIPE_C            (1<<29)
> -#define  CBR_DPLLBMD_PIPE_B            (1<<18)
> +#define  CBR_DPLLBMD_PIPE(pipe)                (1<<(7+(pipe)*11)) /* pipes B and C */

Hmm, all the prepared macros we have expect a 0-offset. (Otherwise you
get something like

(1 << (CBR_DPLLBMD_PIPE_B + (CBR_DPLLDMB_PIPE_C - CBR_DPLLDMB_PIPE_B)*((pipe) - PIPE_B)))

and at that point the numbers are much clearer and quicker to
reconstruct to match the docs.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/8] drm/i915: Eliminate crtc->config from VLV/CHV DPLL functions

[Chris Wilson]

Quoting Ville Syrjala (2017-09-13 15:08:55)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use the passed in crtc state rather than crtc->config when configuring
> the DPLL on VLV/CHV.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ef836fe3eacd..23fead59b825 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5831,11 +5831,11 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>         intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);

Missed intel_crtc_has_dp_encoder(intel_crtc->config) ?
>  
>         if (IS_CHERRYVIEW(dev_priv)) {
> -               chv_prepare_pll(intel_crtc, intel_crtc->config);
> -               chv_enable_pll(intel_crtc, intel_crtc->config);
> +               chv_prepare_pll(intel_crtc, pipe_config);
> +               chv_enable_pll(intel_crtc, pipe_config);
>         } else {
> -               vlv_prepare_pll(intel_crtc, intel_crtc->config);
> -               vlv_enable_pll(intel_crtc, intel_crtc->config);
> +               vlv_prepare_pll(intel_crtc, pipe_config);
> +               vlv_enable_pll(intel_crtc, pipe_config);
>         }
>  
>         intel_encoders_pre_enable(crtc, pipe_config, old_state);
> @@ -6735,8 +6735,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
>  
>         /* Set HBR and RBR LPF coefficients */
>         if (pipe_config->port_clock == 162000 ||
> -           intel_crtc_has_type(crtc->config, INTEL_OUTPUT_ANALOG) ||
> -           intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
> +           intel_crtc_has_type(pipe_config, INTEL_OUTPUT_ANALOG) ||
> +           intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
>                 vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW10(pipe),
>                                  0x009f0003);
>         else
> @@ -6763,7 +6763,7 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
>  
>         coreclk = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW7(pipe));
>         coreclk = (coreclk & 0x0000ff00) | 0x01c00000;
> -       if (intel_crtc_has_dp_encoder(crtc->config))
> +       if (intel_crtc_has_dp_encoder(pipe_config))
>                 coreclk |= 0x01000000;
>         vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW7(pipe), coreclk);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/8] drm/i915: Pass crtc state to i9xx_enable_pll()

[Chris Wilson]

Quoting Ville Syrjala (2017-09-13 15:08:56)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pass the crtc state to i9xx_enable_pll() and use it rather than
> crtc->config.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

(mutters something about consistency in crtc_state vs pipe_config)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/8] drm/i915: Nuke the bogus kernel doc for i9xx_disable_pll()

[Chris Wilson]

Quoting Ville Syrjala (2017-09-13 15:08:57)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/8] drm/i915: Shrink active_crtcs and active_pipes_changed to u8

[Chris Wilson]

Quoting Ville Syrjala (2017-09-13 15:08:59)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We only have three pipes, so 8 bits is more than sufficient to track
> which is active. Also start using BIT() when populating them.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 10 +++++-----
>  drivers/gpu/drm/i915/intel_drv.h     |  4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d4ec44495c00..f8d7599cff43 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2383,7 +2383,7 @@ struct drm_i915_private {
>          */
>         struct mutex dpll_lock;
>  
> -       unsigned int active_crtcs;
> +       u8 active_crtcs;
>         /* minimum acceptable cdclk for each pipe */
>         int min_cdclk[I915_MAX_PIPES];
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 35fe8a98080e..c33fce9d9824 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6032,7 +6032,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
>                 intel_display_power_put(dev_priv, domain);
>         intel_crtc->enabled_power_domains = 0;
>  
> -       dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe);
> +       dev_priv->active_crtcs &= ~BIT(intel_crtc->pipe);
>         dev_priv->min_cdclk[intel_crtc->pipe] = 0;
>  }
>  
> @@ -11956,12 +11956,12 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>  
>         for_each_oldnew_intel_crtc_in_state(intel_state, crtc, old_crtc_state, new_crtc_state, i) {
>                 if (new_crtc_state->base.active)
> -                       intel_state->active_crtcs |= 1 << i;
> +                       intel_state->active_crtcs |= BIT(crtc->pipe);
>                 else
> -                       intel_state->active_crtcs &= ~(1 << i);
> +                       intel_state->active_crtcs &= ~BIT(crtc->pipe);

There's an hweight32(intel_state->active_crtcs & (drm_crtc_mask(for_crtc) - 1))

mixing drm_crtc_mask() and BIT(crtc->pipe) leaves a nasty taste. And
lots of hweight32() to cleanup.
>  
>                 if (old_crtc_state->base.active != new_crtc_state->base.active)
> -                       intel_state->active_pipe_changes |= drm_crtc_mask(&crtc->base);
> +                       intel_state->active_pipe_changes |= BIT(crtc->pipe);

Hmm. Looks very odd to be so isolated. It's treated as a bool which
explains why there's no corresponding clear_bit().
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915: Eliminate crtc->state usage from DVO pipe tracking

[Chris Wilson]

Quoting Ville Syrjala (2017-09-13 15:09:00)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Change the DVO pipe tracking to maintain a bitmask in the top level
> state, just as we do for active_crtcs. This gets rid of the ugly
> intel_num_dvo_pipes() and its crtc->state and crtc->config usage.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 ++
>  drivers/gpu/drm/i915/intel_display.c | 33 ++++++++++++++++-----------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  3 files changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f8d7599cff43..a352644ea5a5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2384,6 +2384,8 @@ struct drm_i915_private {
>         struct mutex dpll_lock;
>  
>         u8 active_crtcs;
> +       u8 dvo_pipes; /* bitmask of pipes driving DVO encoders */
> +
>         /* minimum acceptable cdclk for each pipe */
>         int min_cdclk[I915_MAX_PIPES];
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c33fce9d9824..cb32f264a5d4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1555,19 +1555,6 @@ static void chv_enable_pll(struct intel_crtc *crtc,
>         }
>  }
>  
> -static int intel_num_dvo_pipes(struct drm_i915_private *dev_priv)
> -{
> -       struct intel_crtc *crtc;
> -       int count = 0;
> -
> -       for_each_intel_crtc(&dev_priv->drm, crtc) {
> -               count += crtc->base.state->active &&
> -                       intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DVO);
> -       }
> -
> -       return count;
> -}
> -
>  static void i9xx_enable_pll(struct intel_crtc *crtc,
>                             const struct intel_crtc_state *crtc_state)
>  {
> @@ -1583,7 +1570,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc,
>                 assert_panel_unlocked(dev_priv, crtc->pipe);
>  
>         /* Enable DVO 2x clock on both PLLs if necessary */
> -       if (IS_I830(dev_priv) && intel_num_dvo_pipes(dev_priv) > 0) {
> +       if (IS_I830(dev_priv) && dev_priv->dvo_pipes != 0) {
>                 /*
>                  * It appears to be important that we don't enable this
>                  * for the current pipe before otherwise configuring the
> @@ -1634,9 +1621,7 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
>         enum pipe pipe = crtc->pipe;
>  
>         /* Disable DVO 2x clock on both PLLs if necessary */
> -       if (IS_I830(dev_priv) &&
> -           intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DVO) &&
> -           !intel_num_dvo_pipes(dev_priv)) {
> +       if (IS_I830(dev_priv) && dev_priv->dvo_pipes == 0) {
>                 I915_WRITE(DPLL(PIPE_B),
>                            I915_READ(DPLL(PIPE_B)) & ~DPLL_DVO_2X_MODE);
>                 I915_WRITE(DPLL(PIPE_A),
> @@ -6033,6 +6018,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
>         intel_crtc->enabled_power_domains = 0;
>  
>         dev_priv->active_crtcs &= ~BIT(intel_crtc->pipe);
> +       dev_priv->dvo_pipes &= ~BIT(intel_crtc->pipe);
>         dev_priv->min_cdclk[intel_crtc->pipe] = 0;
>  }
>  
> @@ -11951,6 +11937,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>  
>         intel_state->modeset = true;
>         intel_state->active_crtcs = dev_priv->active_crtcs;
> +       intel_state->dvo_pipes = dev_priv->dvo_pipes;
>         intel_state->cdclk.logical = dev_priv->cdclk.logical;
>         intel_state->cdclk.actual = dev_priv->cdclk.actual;
>  
> @@ -11960,6 +11947,12 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>                 else
>                         intel_state->active_crtcs &= ~BIT(crtc->pipe);
>  
> +               if (new_crtc_state->base.active &&
> +                   intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DVO))
> +                       intel_state->dvo_pipes |= BIT(crtc->pipe);
> +               else
> +                       intel_state->dvo_pipes &= ~BIT(crtc->pipe);
> +
>                 if (old_crtc_state->base.active != new_crtc_state->base.active)
>                         intel_state->active_pipe_changes |= BIT(crtc->pipe);
>         }
> @@ -12560,6 +12553,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>                 memcpy(dev_priv->min_cdclk, intel_state->min_cdclk,
>                        sizeof(intel_state->min_cdclk));
>                 dev_priv->active_crtcs = intel_state->active_crtcs;
> +               dev_priv->dvo_pipes = intel_state->dvo_pipes;
>                 dev_priv->cdclk.logical = intel_state->cdclk.logical;
>                 dev_priv->cdclk.actual = intel_state->cdclk.actual;
>         }
> @@ -14935,6 +14929,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>         int i;
>  
>         dev_priv->active_crtcs = 0;
> +       dev_priv->dvo_pipes = 0;
>  
>         for_each_intel_crtc(dev, crtc) {
>                 struct intel_crtc_state *crtc_state =
> @@ -14992,6 +14987,10 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>                         encoder->base.crtc = &crtc->base;
>                         crtc_state->output_types |= 1 << encoder->type;
>                         encoder->get_config(encoder, crtc_state);
> +
> +                       if (crtc_state->base.active &&
> +                           intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DVO))
> +                               dev_priv->dvo_pipes |= BIT(crtc->pipe);
>                 } else {
>                         encoder->base.crtc = NULL;
>                 }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 96f683256d7e..15ec6076e6a0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -384,6 +384,8 @@ struct intel_atomic_state {
>         u8 active_pipe_changes;
>  
>         u8 active_crtcs;
> +       u8 dvo_pipes; /* bitmask of pipes driving DVO encoders */

Easy enough for even me to follow.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/8] drm/i915: Eliminate crtc->config from VLV/CHV DPLL functions

[Ville Syrjälä]

On Wed, Sep 13, 2017 at 03:40:40PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-09-13 15:08:55)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Use the passed in crtc state rather than crtc->config when configuring
> > the DPLL on VLV/CHV.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ef836fe3eacd..23fead59b825 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5831,11 +5831,11 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
> >         intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
> 
> Missed intel_crtc_has_dp_encoder(intel_crtc->config) ?

I was only going after the DPLL stuff. There's plenty more left.

> >  
> >         if (IS_CHERRYVIEW(dev_priv)) {
> > -               chv_prepare_pll(intel_crtc, intel_crtc->config);
> > -               chv_enable_pll(intel_crtc, intel_crtc->config);
> > +               chv_prepare_pll(intel_crtc, pipe_config);
> > +               chv_enable_pll(intel_crtc, pipe_config);
> >         } else {
> > -               vlv_prepare_pll(intel_crtc, intel_crtc->config);
> > -               vlv_enable_pll(intel_crtc, intel_crtc->config);
> > +               vlv_prepare_pll(intel_crtc, pipe_config);
> > +               vlv_enable_pll(intel_crtc, pipe_config);
> >         }
> >  
> >         intel_encoders_pre_enable(crtc, pipe_config, old_state);
> > @@ -6735,8 +6735,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
> >  
> >         /* Set HBR and RBR LPF coefficients */
> >         if (pipe_config->port_clock == 162000 ||
> > -           intel_crtc_has_type(crtc->config, INTEL_OUTPUT_ANALOG) ||
> > -           intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
> > +           intel_crtc_has_type(pipe_config, INTEL_OUTPUT_ANALOG) ||
> > +           intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
> >                 vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW10(pipe),
> >                                  0x009f0003);
> >         else
> > @@ -6763,7 +6763,7 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
> >  
> >         coreclk = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW7(pipe));
> >         coreclk = (coreclk & 0x0000ff00) | 0x01c00000;
> > -       if (intel_crtc_has_dp_encoder(crtc->config))
> > +       if (intel_crtc_has_dp_encoder(pipe_config))
> >                 coreclk |= 0x01000000;
> >         vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW7(pipe), coreclk);

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

Re: [PATCH 4/8] drm/i915: Pass crtc state to i9xx_enable_pll()

[Ville Syrjälä]

On Wed, Sep 13, 2017 at 03:41:46PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-09-13 15:08:56)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Pass the crtc state to i9xx_enable_pll() and use it rather than
> > crtc->config.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> (mutters something about consistency in crtc_state vs pipe_config)

Yeah, that's a PITA. I'm thinking we'll try to take care of that
with coccinelle or sed once the non-trivial cases are sorted.

Also looks like I missed i9xx_set_pll_dividers(). I'll have to fix that
one up as well to get the gmch DPLL code cleaned up.

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

Re: [PATCH 7/8] drm/i915: Shrink active_crtcs and active_pipes_changed to u8

[Ville Syrjälä]

On Wed, Sep 13, 2017 at 03:49:10PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-09-13 15:08:59)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We only have three pipes, so 8 bits is more than sufficient to track
> > which is active. Also start using BIT() when populating them.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |  2 +-
> >  drivers/gpu/drm/i915/intel_display.c | 10 +++++-----
> >  drivers/gpu/drm/i915/intel_drv.h     |  4 ++--
> >  3 files changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index d4ec44495c00..f8d7599cff43 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2383,7 +2383,7 @@ struct drm_i915_private {
> >          */
> >         struct mutex dpll_lock;
> >  
> > -       unsigned int active_crtcs;
> > +       u8 active_crtcs;
> >         /* minimum acceptable cdclk for each pipe */
> >         int min_cdclk[I915_MAX_PIPES];
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 35fe8a98080e..c33fce9d9824 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6032,7 +6032,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
> >                 intel_display_power_put(dev_priv, domain);
> >         intel_crtc->enabled_power_domains = 0;
> >  
> > -       dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe);
> > +       dev_priv->active_crtcs &= ~BIT(intel_crtc->pipe);
> >         dev_priv->min_cdclk[intel_crtc->pipe] = 0;
> >  }
> >  
> > @@ -11956,12 +11956,12 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
> >  
> >         for_each_oldnew_intel_crtc_in_state(intel_state, crtc, old_crtc_state, new_crtc_state, i) {
> >                 if (new_crtc_state->base.active)
> > -                       intel_state->active_crtcs |= 1 << i;
> > +                       intel_state->active_crtcs |= BIT(crtc->pipe);
> >                 else
> > -                       intel_state->active_crtcs &= ~(1 << i);
> > +                       intel_state->active_crtcs &= ~BIT(crtc->pipe);
> 
> There's an hweight32(intel_state->active_crtcs & (drm_crtc_mask(for_crtc) - 1))
> 
> mixing drm_crtc_mask() and BIT(crtc->pipe) leaves a nasty taste.

Hmm. Looks like that mixup goes even deeper in the SKL+ wm code. Not
sure I want to try untangling it all.

> And
> lots of hweight32() to cleanup.
> >  
> >                 if (old_crtc_state->base.active != new_crtc_state->base.active)
> > -                       intel_state->active_pipe_changes |= drm_crtc_mask(&crtc->base);
> > +                       intel_state->active_pipe_changes |= BIT(crtc->pipe);
> 
> Hmm. Looks very odd to be so isolated. It's treated as a bool which
> explains why there's no corresponding clear_bit().
> -Chris

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

✗ Fi.CI.IGT: failure for drm/i915: Some DPLL crtc->state/config cleanups etc.

[Patchwork]

== Series Details ==

Series: drm/i915: Some DPLL crtc->state/config cleanups etc.
URL   : https://patchwork.freedesktop.org/series/30300/
State : failure

== Summary ==

Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test drv_missed_irq:
                pass       -> FAIL       (shard-hsw)

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2313 pass:1243 dwarn:0   dfail:0   fail:15  skip:1055 time:10013s

== Logs ==

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

Re: [PATCH 0/8] drm/i915: Some DPLL crtc->state/config cleanups etc.

[Ville Syrjälä]

On Wed, Sep 13, 2017 at 05:08:52PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> This series nukes some crtc->config and crtc->state uses from the
> DPLL code. And I tossed in a few other random DPLL things I had lying around.
> 
> Ville Syrjälä (8):
>   drm/i915: Parametrize CBR_DPLLBMD_PIPE defines
>   drm/i915: Pass crtc state to i9xx_enable_pll()
>   drm/i915: Nuke the bogus kernel doc for i9xx_disable_pll()

Pushed those three to dinq. Thanks for the review.

>   drm/i915: Set output_types to EDP for vlv/chv DPLL forcing
>   drm/i915: Eliminate crtc->config from VLV/CHV DPLL functions
>   drm/i915: Add for_each_oldnew_intel_crtc_in_state()
>   drm/i915: Shrink active_crtcs and active_pipes_changed to u8
>   drm/i915: Eliminate crtc->state usage from DVO pipe tracking
> 
>  drivers/gpu/drm/i915/i915_drv.h      | 12 ++++-
>  drivers/gpu/drm/i915/i915_reg.h      |  3 +-
>  drivers/gpu/drm/i915/intel_display.c | 88 ++++++++++++++++--------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  6 ++-
>  4 files changed, 56 insertions(+), 53 deletions(-)
> 
> -- 
> 2.13.5

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