All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH 05/31] drm/i915: switch crtc->shared_dpll from a pointer to an enum
Date: Wed,  5 Jun 2013 13:34:07 +0200	[thread overview]
Message-ID: <1370432073-27634-6-git-send-email-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <1370432073-27634-1-git-send-email-daniel.vetter@ffwll.ch>

Dealing with discrete enum values is simpler for hw state readout and
pipe config computations than pointers - having neat names instead of
chasing pointers should look better in the code.

This isn't a that good reason for pch plls, but on haswell we actually
have 3 different types of plls: WRPLL, SPLL and the DP clocks. Having
explicit names should help there.

Since this also adds the intel_crtc_to_shared_dpll helper to further
abstract away the crtc -> dpll relationship this will also help to
make the next patch simpler, which moves the shared dpll into the pipe
configuration.

Also note that for uniformity we have two special dpll ids: NONE for
pipes which need a shared pll but don't have one (yet) and private for
when there's a non-shared pll (e.g. per-pipe or per-port pll).

I've thought whether we should also add a 2nd enum for the type of the
pll we want (for really generic pll selection code) but thrown that
idea out again - likely there's too much platform craziness going on
to be able to share the pll selection logic much.

Since this touched all the shared_pll functions a bit I've also done
an s/intel_crtc/crtc/ replacement on a few of them.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h      |  8 ++++
 drivers/gpu/drm/i915/intel_display.c | 92 ++++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 3 files changed, 60 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f41f42f..ca3cb3b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -140,6 +140,14 @@ struct intel_shared_dpll {
 	int fp0_reg;
 	int fp1_reg;
 };
+
+enum intel_dpll_id {
+	DPLL_ID_NONE = -2, /* no dpll assigned/used */
+	DPLL_ID_PRIVATE = -1, /* non-shared dpll in use */
+	/* real shared dpll ids must be >= 0 */
+	DPLL_ID_PCH_PLL_A,
+	DPLL_ID_PCH_PLL_B,
+};
 #define I915_NUM_PLLS 2
 
 /* Used by dp and fdi links */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index daf12b5..d95d813 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -891,6 +891,17 @@ static void assert_pll(struct drm_i915_private *dev_priv,
 #define assert_pll_enabled(d, p) assert_pll(d, p, true)
 #define assert_pll_disabled(d, p) assert_pll(d, p, false)
 
+static struct intel_shared_dpll *
+intel_crtc_to_shared_dpll(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+
+	if (crtc->shared_dpll < 0)
+		return NULL;
+
+	return &dev_priv->shared_dplls[crtc->shared_dpll];
+}
+
 /* For ILK+ */
 static void assert_shared_dpll(struct drm_i915_private *dev_priv,
 			       struct intel_shared_dpll *pll,
@@ -1374,16 +1385,15 @@ void vlv_wait_port_ready(struct drm_i915_private *dev_priv, int port)
  * The PCH PLL needs to be enabled before the PCH transcoder, since it
  * drives the transcoder clock.
  */
-static void ironlake_enable_shared_dpll(struct intel_crtc *intel_crtc)
+static void ironlake_enable_shared_dpll(struct intel_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = intel_crtc->base.dev->dev_private;
-	struct intel_shared_dpll *pll;
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
 	int reg;
 	u32 val;
 
 	/* PCH PLLs only available on ILK, SNB and IVB */
 	BUG_ON(dev_priv->info->gen < 5);
-	pll = intel_crtc->shared_dpll;
 	if (pll == NULL)
 		return;
 
@@ -1392,7 +1402,7 @@ static void ironlake_enable_shared_dpll(struct intel_crtc *intel_crtc)
 
 	DRM_DEBUG_KMS("enable PCH PLL %x (active %d, on? %d)for crtc %d\n",
 		      pll->pll_reg, pll->active, pll->on,
-		      intel_crtc->base.base.id);
+		      crtc->base.base.id);
 
 	/* PCH refclock must be enabled first */
 	assert_pch_refclk_enabled(dev_priv);
@@ -1415,10 +1425,10 @@ static void ironlake_enable_shared_dpll(struct intel_crtc *intel_crtc)
 	pll->on = true;
 }
 
-static void intel_disable_shared_dpll(struct intel_crtc *intel_crtc)
+static void intel_disable_shared_dpll(struct intel_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = intel_crtc->base.dev->dev_private;
-	struct intel_shared_dpll *pll = intel_crtc->shared_dpll;
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
 	int reg;
 	u32 val;
 
@@ -1432,7 +1442,7 @@ static void intel_disable_shared_dpll(struct intel_crtc *intel_crtc)
 
 	DRM_DEBUG_KMS("disable PCH PLL %x (active %d, on? %d) for crtc %d\n",
 		      pll->pll_reg, pll->active, pll->on,
-		      intel_crtc->base.base.id);
+		      crtc->base.base.id);
 
 	if (WARN_ON(pll->active == 0)) {
 		assert_shared_dpll_disabled(dev_priv, pll, NULL);
@@ -1447,7 +1457,7 @@ static void intel_disable_shared_dpll(struct intel_crtc *intel_crtc)
 	DRM_DEBUG_KMS("disabling PCH PLL %x\n", pll->pll_reg);
 
 	/* Make sure transcoder isn't still depending on us */
-	assert_pch_transcoder_disabled(dev_priv, intel_crtc->pipe);
+	assert_pch_transcoder_disabled(dev_priv, crtc->pipe);
 
 	reg = pll->pll_reg;
 	val = I915_READ(reg);
@@ -1464,6 +1474,7 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
 {
 	struct drm_device *dev = dev_priv->dev;
 	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	uint32_t reg, val, pipeconf_val;
 
 	/* PCH only available on ILK+ */
@@ -1471,8 +1482,8 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
 
 	/* Make sure PCH DPLL is enabled */
 	assert_shared_dpll_enabled(dev_priv,
-				   to_intel_crtc(crtc)->shared_dpll,
-				   to_intel_crtc(crtc));
+				   intel_crtc_to_shared_dpll(intel_crtc),
+				   intel_crtc);
 
 	/* FDI must be feeding us bits for PCH ports */
 	assert_fdi_tx_enabled(dev_priv, pipe);
@@ -2956,7 +2967,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
 			sel = TRANSC_DPLLB_SEL;
 			break;
 		}
-		if (intel_crtc->shared_dpll->pll_reg == _PCH_DPLL_B)
+		if (intel_crtc->shared_dpll == DPLL_ID_PCH_PLL_B)
 			temp |= sel;
 		else
 			temp &= ~sel;
@@ -3025,14 +3036,14 @@ static void lpt_pch_enable(struct drm_crtc *crtc)
 	lpt_enable_pch_transcoder(dev_priv, cpu_transcoder);
 }
 
-static void intel_put_shared_dpll(struct intel_crtc *intel_crtc)
+static void intel_put_shared_dpll(struct intel_crtc *crtc)
 {
-	struct intel_shared_dpll *pll = intel_crtc->shared_dpll;
+	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
 
 	if (pll == NULL)
 		return;
 
-	WARN_ON(!intel_crtc->config.has_pch_encoder);
+	WARN_ON(!crtc->config.has_pch_encoder);
 
 	if (pll->refcount == 0) {
 		WARN(1, "bad PCH PLL refcount\n");
@@ -3044,29 +3055,28 @@ static void intel_put_shared_dpll(struct intel_crtc *intel_crtc)
 		WARN_ON(pll->active);
 	}
 
-	intel_crtc->shared_dpll = NULL;
+	crtc->shared_dpll = DPLL_ID_NONE;
 }
 
-static struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *intel_crtc, u32 dpll, u32 fp)
+static struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, u32 dpll, u32 fp)
 {
-	struct drm_i915_private *dev_priv = intel_crtc->base.dev->dev_private;
-	struct intel_shared_dpll *pll;
-	int i;
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
+	enum intel_dpll_id i;
 
-	pll = intel_crtc->shared_dpll;
 	if (pll) {
 		DRM_DEBUG_KMS("CRTC:%d dropping existing PCH PLL %x\n",
-			      intel_crtc->base.base.id, pll->pll_reg);
-		intel_put_shared_dpll(intel_crtc);
+			      crtc->base.base.id, pll->pll_reg);
+		intel_put_shared_dpll(crtc);
 	}
 
 	if (HAS_PCH_IBX(dev_priv->dev)) {
 		/* Ironlake PCH has a fixed PLL->PCH pipe mapping. */
-		i = intel_crtc->pipe;
+		i = crtc->pipe;
 		pll = &dev_priv->shared_dplls[i];
 
 		DRM_DEBUG_KMS("CRTC:%d using pre-allocated PCH PLL %x\n",
-			      intel_crtc->base.base.id, pll->pll_reg);
+			      crtc->base.base.id, pll->pll_reg);
 
 		goto found;
 	}
@@ -3081,7 +3091,7 @@ static struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *intel_
 		if (dpll == (I915_READ(pll->pll_reg) & 0x7fffffff) &&
 		    fp == I915_READ(pll->fp0_reg)) {
 			DRM_DEBUG_KMS("CRTC:%d sharing existing PCH PLL %x (refcount %d, ative %d)\n",
-				      intel_crtc->base.base.id,
+				      crtc->base.base.id,
 				      pll->pll_reg, pll->refcount, pll->active);
 
 			goto found;
@@ -3093,7 +3103,7 @@ static struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *intel_
 		pll = &dev_priv->shared_dplls[i];
 		if (pll->refcount == 0) {
 			DRM_DEBUG_KMS("CRTC:%d allocated PCH PLL %x\n",
-				      intel_crtc->base.base.id, pll->pll_reg);
+				      crtc->base.base.id, pll->pll_reg);
 			goto found;
 		}
 	}
@@ -3101,8 +3111,8 @@ static struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *intel_
 	return NULL;
 
 found:
-	intel_crtc->shared_dpll = pll;
-	DRM_DEBUG_DRIVER("using pll %d for pipe %c\n", i, pipe_name(intel_crtc->pipe));
+	crtc->shared_dpll = i;
+	DRM_DEBUG_DRIVER("using pll %d for pipe %c\n", i, pipe_name(crtc->pipe));
 	if (pll->active == 0) {
 		DRM_DEBUG_DRIVER("setting up pll %d\n", i);
 		WARN_ON(pll->on);
@@ -5670,6 +5680,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	bool ok, has_reduced_clock = false;
 	bool is_lvds = false;
 	struct intel_encoder *encoder;
+	struct intel_shared_dpll *pll;
 	int ret;
 
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
@@ -5705,8 +5716,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	/* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
 	if (intel_crtc->config.has_pch_encoder) {
-		struct intel_shared_dpll *pll;
-
 		fp = i9xx_dpll_compute_fp(&intel_crtc->config.dpll);
 		if (has_reduced_clock)
 			fp2 = i9xx_dpll_compute_fp(&reduced_clock);
@@ -5731,11 +5740,15 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		if (encoder->pre_pll_enable)
 			encoder->pre_pll_enable(encoder);
 
-	if (intel_crtc->shared_dpll) {
-		I915_WRITE(intel_crtc->shared_dpll->pll_reg, dpll);
+	intel_crtc->lowfreq_avail = false;
+
+	if (intel_crtc->config.has_pch_encoder) {
+		pll = intel_crtc_to_shared_dpll(intel_crtc);
+
+		I915_WRITE(pll->pll_reg, dpll);
 
 		/* Wait for the clocks to stabilize. */
-		POSTING_READ(intel_crtc->shared_dpll->pll_reg);
+		POSTING_READ(pll->pll_reg);
 		udelay(150);
 
 		/* The pixel multiplier can only be updated once the
@@ -5743,16 +5756,13 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		 *
 		 * So write it again.
 		 */
-		I915_WRITE(intel_crtc->shared_dpll->pll_reg, dpll);
-	}
+		I915_WRITE(pll->pll_reg, dpll);
 
-	intel_crtc->lowfreq_avail = false;
-	if (intel_crtc->shared_dpll) {
 		if (is_lvds && has_reduced_clock && i915_powersave) {
-			I915_WRITE(intel_crtc->shared_dpll->fp1_reg, fp2);
+			I915_WRITE(pll->fp1_reg, fp2);
 			intel_crtc->lowfreq_avail = true;
 		} else {
-			I915_WRITE(intel_crtc->shared_dpll->fp1_reg, fp);
+			I915_WRITE(pll->fp1_reg, fp);
 		}
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 163bee9..422b2ad 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -306,7 +306,7 @@ struct intel_crtc {
 	struct intel_crtc_config config;
 
 	/* We can share PLLs across outputs if the timings match */
-	struct intel_shared_dpll *shared_dpll;
+	enum intel_dpll_id shared_dpll;
 	uint32_t ddi_pll_sel;
 
 	/* reset counter value when the last flip was submitted */
-- 
1.7.11.7

  parent reply	other threads:[~2013-06-05 11:34 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-05 11:34 [PATCH 00/31] shared pch display pll rework Daniel Vetter
2013-06-05 11:34 ` [PATCH 01/31] drm/i915: fix up pch pll handling in ->mode_set Daniel Vetter
2013-06-05 11:34 ` [PATCH 02/31] drm/i915: conditionally disable pch resources in ilk_crtc_disable Daniel Vetter
2013-06-05 11:34 ` [PATCH 03/31] drm/i915: lock down pch pll accouting some more Daniel Vetter
2013-06-07 16:32   ` Ville Syrjälä
2013-06-07 20:03     ` Daniel Vetter
2013-06-07 20:46       ` Ville Syrjälä
2013-06-07 21:13         ` Daniel Vetter
2013-06-10 10:11           ` Ville Syrjälä
2013-06-10 14:34             ` Daniel Vetter
2013-06-10 14:47               ` Ville Syrjälä
2013-06-10 15:28                 ` [PATCH] " Daniel Vetter
2013-06-07 21:09     ` Daniel Vetter
2013-06-05 11:34 ` [PATCH 04/31] drm/i915: s/pch_pll/shared_dpll/ Daniel Vetter
2013-06-05 11:34 ` Daniel Vetter [this message]
2013-06-07 16:48   ` [PATCH 05/31] drm/i915: switch crtc->shared_dpll from a pointer to an enum Ville Syrjälä
2013-06-07 21:10     ` [PATCH] " Daniel Vetter
2013-06-05 11:34 ` [PATCH 06/31] drm/i915: move shared_dpll into the pipe config Daniel Vetter
2013-06-07 17:03   ` Ville Syrjälä
2013-06-07 21:10     ` [PATCH] " Daniel Vetter
2013-06-05 11:34 ` [PATCH 07/31] drm/i915: refactor PCH_DPLL_SEL #defines Daniel Vetter
2013-06-05 11:34 ` [PATCH 08/31] drm/i915: hw state readout for shared pch plls Daniel Vetter
2013-06-07 17:23   ` Ville Syrjälä
2013-06-07 20:11     ` Daniel Vetter
2013-06-07 21:11     ` [PATCH] " Daniel Vetter
2013-06-05 11:34 ` [PATCH 09/31] drm/i915: consolidate ->num_shared_dplls assignement Daniel Vetter
2013-06-05 11:34 ` [PATCH 10/31] drm/i915: metadata for shared dplls Daniel Vetter
2013-06-05 11:34 ` [PATCH 11/31] drm/i915: scrap register address storage Daniel Vetter
2013-06-05 11:34 ` [PATCH 12/31] drm/i915: enable/disable hooks for shared dplls Daniel Vetter
2013-06-05 11:34 ` [PATCH 13/31] drm/i915: drop crtc checking from assert_shared_dpll Daniel Vetter
2013-06-05 11:34 ` [PATCH 14/31] drm/i915: display pll hw state readout and checking Daniel Vetter
2013-06-12 13:31   ` Damien Lespiau
2013-06-12 13:39     ` Ville Syrjälä
2013-06-12 13:49       ` Damien Lespiau
2013-06-05 11:34 ` [PATCH 15/31] drm/i915: extract readout_hw_state from setup_hw_state Daniel Vetter
2013-06-12 13:32   ` Damien Lespiau
2013-06-12 14:26   ` Daniel Vetter
2013-06-05 11:34 ` [PATCH 16/31] drm/i915: split up intel_modeset_check_state Daniel Vetter
2013-06-12 13:33   ` Damien Lespiau
2013-06-05 11:34 ` [PATCH 17/31] drm/i915: WARN on lack of shared dpll Daniel Vetter
2013-06-12 13:38   ` Damien Lespiau
2013-06-05 11:34 ` [PATCH 18/31] drm/i915: hw state readout and cross-checking for shared dplls Daniel Vetter
2013-06-12 15:04   ` Damien Lespiau
2013-06-05 11:34 ` [PATCH 19/31] drm/i915: fix up pch pll enabling for pixel multipliers Daniel Vetter
2013-06-12 15:12   ` Damien Lespiau
2013-06-12 19:34     ` Daniel Vetter
2013-06-05 11:34 ` [PATCH 20/31] drm/i915: simplify the reduced clock handling for pch plls Daniel Vetter
2013-06-13 11:26   ` Damien Lespiau
2013-06-13 11:35     ` Daniel Vetter
2013-06-13 12:32       ` Damien Lespiau
2013-06-13 14:33         ` Daniel Vetter
2013-06-05 11:34 ` [PATCH 21/31] drm/i915: consolidate pch pll enable sequence Daniel Vetter
2013-06-24 14:30   ` Damien Lespiau
2013-06-05 11:34 ` [PATCH 22/31] drm/i915: use sw tracked state to select shared dplls Daniel Vetter
2013-06-12 15:20   ` Damien Lespiau
2013-06-05 11:34 ` [PATCH 23/31] drm/i915: duplicate intel_enable_pll into i9xx and vlv versions Daniel Vetter
2013-06-05 15:12   ` Jani Nikula
2013-06-05 22:52     ` [PATCH] " Daniel Vetter
2013-06-05 11:34 ` [PATCH 24/31] drm/i915: asserts for lvds pre_enable Daniel Vetter
2013-06-13 20:26   ` Imre Deak
2013-06-13 20:46     ` Daniel Vetter
2013-06-14 10:45       ` Imre Deak
2013-06-16 19:42     ` [PATCH] " Daniel Vetter
2013-06-05 11:34 ` [PATCH 25/31] drm/i915: move encoder pre enable hooks togther on ilk+ Daniel Vetter
2013-06-05 11:34 ` [PATCH 26/31] drm/i915: hw state readout for i9xx dplls Daniel Vetter
2013-06-05 11:34 ` [PATCH 27/31] drm/i915: move i9xx dpll enabling into crtc enable function Daniel Vetter
2013-06-05 15:13   ` Jani Nikula
2013-06-06  8:20     ` [PATCH] " Daniel Vetter
2013-06-14 16:02   ` [PATCH 27/31] " Imre Deak
2013-06-16 19:15     ` Daniel Vetter
2013-06-16 19:24     ` [PATCH] " Daniel Vetter
2013-06-05 11:34 ` [PATCH 28/31] drm/i915: s/pre_pll/pre/ on the lvds port " Daniel Vetter
2013-06-15  8:32   ` Imre Deak
2013-06-26 10:02     ` Daniel Vetter
2013-06-05 11:34 ` [PATCH 29/31] drm/i915: clean up vlv ->pre_pll_enable and pll enable sequence Daniel Vetter
2013-06-06  8:22   ` [PATCH] " Daniel Vetter
2013-07-11 14:11     ` Imre Deak
2013-07-11 20:13       ` Daniel Vetter
2013-07-12 16:27       ` Daniel Vetter
2013-06-05 11:34 ` [PATCH 30/31] drm/i915: Fix up cpt pixel multiplier " Daniel Vetter
2013-06-05 11:34 ` [PATCH 31/31] drm/i915: clear DPLL reg when disabling i9xx dplls Daniel Vetter
2013-06-07 17:46 ` [PATCH 00/31] shared pch display pll rework Ville Syrjälä
2013-06-10 15:57   ` Ville Syrjälä
2013-06-10 18:16     ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1370432073-27634-6-git-send-email-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.