Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] sdvo tv clock improvements + some random stuff
@ 2013-04-30 12:01 Daniel Vetter
  2013-04-30 12:01 ` [PATCH 1/7] drm/i915: simplify DP/DDI port width macros Daniel Vetter
                   ` (6 more replies)
  0 siblings, 7 replies; 23+ messages in thread
From: Daniel Vetter @ 2013-04-30 12:01 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

Hi all,

So I'm still blazing ahead with converting everything over to pipe_config. This
is (mostly) a tiny detour into sdvo TV support, which accidentally ended up
fixing a regression going back to the introduction of multi-function sdvo
support in 2.6.35.

Quick update on this pipe_config endeavour: I have a few more smaller things
outstanding around pll limits and handling that, but the big thing I'm currently
working on is beating sanity into our different clocks: dotclock, adjusted
dotclock, port clock (i.e. what we program the dpll with), reduced clock
versions of all of the above. Next up is handling dpll computation and clock
sharing.

Cheers, Daniel

Daniel Vetter (7):
  drm/i915: simplify DP/DDI port width macros
  drm/i915: move sdvo TV clock computation to intel_sdvo.c
  drm/i915: drop TVclock special casing on ilk+
  drm/i915: rip out TV-out lore ...
  drm/i915: rip out now unused is_foo tracking from crtc code
  drm/i915: make SDVO TV-out work for multifunction devices
  drm/i915: rip out an unused lvds_reg variable

 drivers/gpu/drm/i915/i915_reg.h      | 11 +----
 drivers/gpu/drm/i915/intel_ddi.c     | 34 +-------------
 drivers/gpu/drm/i915/intel_display.c | 87 +++---------------------------------
 drivers/gpu/drm/i915/intel_dp.c      | 12 +----
 drivers/gpu/drm/i915/intel_drv.h     |  5 ++-
 drivers/gpu/drm/i915/intel_sdvo.c    | 38 +++++++++++++---
 6 files changed, 48 insertions(+), 139 deletions(-)

-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/7] drm/i915: simplify DP/DDI port width macros
  2013-04-30 12:01 [PATCH 0/7] sdvo tv clock improvements + some random stuff Daniel Vetter
@ 2013-04-30 12:01 ` Daniel Vetter
  2013-05-02 13:34   ` Paulo Zanoni
  2013-04-30 12:01 ` [PATCH 2/7] drm/i915: move sdvo TV clock computation to intel_sdvo.c Daniel Vetter
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2013-04-30 12:01 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, Paulo Zanoni

If we ever leak a non-DP compliant port width through here, we have a
pretty serious issue. So just rip out all these WARNs - if we need
them it's probably better to have them at a central place where we
compute the dp lane count.

Also use the new DDI width macro for FDI mode.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h  | 11 ++---------
 drivers/gpu/drm/i915/intel_ddi.c | 34 ++--------------------------------
 drivers/gpu/drm/i915/intel_dp.c  | 12 +-----------
 3 files changed, 5 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 95ae5cf..d84d694 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2664,9 +2664,7 @@
 #define   DP_PRE_EMPHASIS_SHIFT		22
 
 /* How many wires to use. I guess 3 was too hard */
-#define   DP_PORT_WIDTH_1		(0 << 19)
-#define   DP_PORT_WIDTH_2		(1 << 19)
-#define   DP_PORT_WIDTH_4		(3 << 19)
+#define   DP_PORT_WIDTH(width)		(((width) - 1) << 19)
 #define   DP_PORT_WIDTH_MASK		(7 << 19)
 
 /* Mystic DPCD version 1.1 special mode */
@@ -4751,9 +4749,6 @@
 #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5<<12)
 #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
 #define  TRANS_DDI_BFI_ENABLE		(1<<4)
-#define  TRANS_DDI_PORT_WIDTH_X1	(0<<1)
-#define  TRANS_DDI_PORT_WIDTH_X2	(1<<1)
-#define  TRANS_DDI_PORT_WIDTH_X4	(3<<1)
 
 /* DisplayPort Transport Control */
 #define DP_TP_CTL_A			0x64040
@@ -4797,9 +4792,7 @@
 #define  DDI_BUF_PORT_REVERSAL			(1<<16)
 #define  DDI_BUF_IS_IDLE			(1<<7)
 #define  DDI_A_4_LANES				(1<<4)
-#define  DDI_PORT_WIDTH_X1			(0<<1)
-#define  DDI_PORT_WIDTH_X2			(1<<1)
-#define  DDI_PORT_WIDTH_X4			(3<<1)
+#define  DDI_PORT_WIDTH(width)			(((width) - 1) << 1)
 #define  DDI_INIT_DISPLAY_DETECTED		(1<<0)
 
 /* DDI Buffer Translations */
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 72941f9..841c9a9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -687,22 +687,7 @@ static void intel_ddi_mode_set(struct drm_encoder *encoder,
 
 		intel_dp->DP = intel_dig_port->port_reversal |
 			       DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW;
-		switch (intel_dp->lane_count) {
-		case 1:
-			intel_dp->DP |= DDI_PORT_WIDTH_X1;
-			break;
-		case 2:
-			intel_dp->DP |= DDI_PORT_WIDTH_X2;
-			break;
-		case 4:
-			intel_dp->DP |= DDI_PORT_WIDTH_X4;
-			break;
-		default:
-			intel_dp->DP |= DDI_PORT_WIDTH_X4;
-			WARN(1, "Unexpected DP lane count %d\n",
-			     intel_dp->lane_count);
-			break;
-		}
+		intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
 
 		if (intel_dp->has_audio) {
 			DRM_DEBUG_DRIVER("DP audio on pipe %c on DDI\n",
@@ -1031,22 +1016,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
 
 		temp |= TRANS_DDI_MODE_SELECT_DP_SST;
 
-		switch (intel_dp->lane_count) {
-		case 1:
-			temp |= TRANS_DDI_PORT_WIDTH_X1;
-			break;
-		case 2:
-			temp |= TRANS_DDI_PORT_WIDTH_X2;
-			break;
-		case 4:
-			temp |= TRANS_DDI_PORT_WIDTH_X4;
-			break;
-		default:
-			temp |= TRANS_DDI_PORT_WIDTH_X4;
-			WARN(1, "Unsupported lane count %d\n",
-			     intel_dp->lane_count);
-		}
-
+		intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
 	} else {
 		WARN(1, "Invalid encoder type %d for pipe %c\n",
 		     intel_encoder->type, pipe_name(pipe));
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8759fb1..99f798a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -891,18 +891,8 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 
 	/* Handle DP bits in common between all three register formats */
 	intel_dp->DP |= DP_VOLTAGE_0_4 | DP_PRE_EMPHASIS_0;
+	intel_dp->DP |= DP_PORT_WIDTH(intel_dp->lane_count);
 
-	switch (intel_dp->lane_count) {
-	case 1:
-		intel_dp->DP |= DP_PORT_WIDTH_1;
-		break;
-	case 2:
-		intel_dp->DP |= DP_PORT_WIDTH_2;
-		break;
-	case 4:
-		intel_dp->DP |= DP_PORT_WIDTH_4;
-		break;
-	}
 	if (intel_dp->has_audio) {
 		DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
 				 pipe_name(intel_crtc->pipe));
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/7] drm/i915: move sdvo TV clock computation to intel_sdvo.c
  2013-04-30 12:01 [PATCH 0/7] sdvo tv clock improvements + some random stuff Daniel Vetter
  2013-04-30 12:01 ` [PATCH 1/7] drm/i915: simplify DP/DDI port width macros Daniel Vetter
@ 2013-04-30 12:01 ` Daniel Vetter
  2013-05-10 11:13   ` Jani Nikula
  2013-04-30 12:01 ` [PATCH 3/7] drm/i915: drop TVclock special casing on ilk+ Daniel Vetter
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2013-04-30 12:01 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

We have a very nice infrastructure for this now!

Note that the multifunction sdvo support is pretty neatly broken: We
completely ignore userspace's request for which connector to wire up
with the encoder and just use whatever the last detect callback has
seen.

Not something I'll fix in this patch, but unfortunately something
which is also broken in the DDI code ...

v2: Don't call sdvo_tv_clock twice.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 30 ------------------------------
 drivers/gpu/drm/i915/intel_sdvo.c    | 30 ++++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1128f83..f10f094 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4286,30 +4286,6 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
 	return refclk;
 }
 
-static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc *crtc)
-{
-	unsigned dotclock = crtc->config.adjusted_mode.clock;
-	struct dpll *clock = &crtc->config.dpll;
-
-	/* SDVO TV has fixed PLL values depend on its clock range,
-	   this mirrors vbios setting. */
-	if (dotclock >= 100000 && dotclock < 140500) {
-		clock->p1 = 2;
-		clock->p2 = 10;
-		clock->n = 3;
-		clock->m1 = 16;
-		clock->m2 = 8;
-	} else if (dotclock >= 140500 && dotclock <= 200000) {
-		clock->p1 = 1;
-		clock->p2 = 10;
-		clock->n = 6;
-		clock->m1 = 12;
-		clock->m2 = 8;
-	}
-
-	crtc->config.clock_set = true;
-}
-
 static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
 {
 	return (1 << dpll->n) << 16 | dpll->m1 << 8 | dpll->m2;
@@ -4926,9 +4902,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 		intel_crtc->config.dpll.p2 = clock.p2;
 	}
 
-	if (is_sdvo && is_tv)
-		i9xx_adjust_sdvo_tv_clock(intel_crtc);
-
 	if (IS_GEN2(dev))
 		i8xx_update_pll(intel_crtc, adjusted_mode,
 				has_reduced_clock ? &reduced_clock : NULL,
@@ -5538,9 +5511,6 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 						     reduced_clock);
 	}
 
-	if (is_sdvo && is_tv)
-		i9xx_adjust_sdvo_tv_clock(to_intel_crtc(crtc));
-
 	return true;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index d154284..f6bf9fc 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1041,6 +1041,32 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
 	return true;
 }
 
+static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_config *pipe_config)
+{
+	unsigned dotclock = pipe_config->adjusted_mode.clock;
+	struct dpll *clock = &pipe_config->dpll;
+
+	/* SDVO TV has fixed PLL values depend on its clock range,
+	   this mirrors vbios setting. */
+	if (dotclock >= 100000 && dotclock < 140500) {
+		clock->p1 = 2;
+		clock->p2 = 10;
+		clock->n = 3;
+		clock->m1 = 16;
+		clock->m2 = 8;
+	} else if (dotclock >= 140500 && dotclock <= 200000) {
+		clock->p1 = 1;
+		clock->p2 = 10;
+		clock->n = 6;
+		clock->m1 = 12;
+		clock->m2 = 8;
+	} else {
+		WARN(1, "SDVO TV clock out of range: %i\n", dotclock);
+	}
+
+	pipe_config->clock_set = true;
+}
+
 static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
 				      struct intel_crtc_config *pipe_config)
 {
@@ -1097,6 +1123,10 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
 	if (intel_sdvo->color_range)
 		pipe_config->limited_color_range = true;
 
+	/* Clock computation needs to happen after pixel multiplier. */
+	if (intel_sdvo->is_tv)
+		i9xx_adjust_sdvo_tv_clock(pipe_config);
+
 	return true;
 }
 
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/7] drm/i915: drop TVclock special casing on ilk+
  2013-04-30 12:01 [PATCH 0/7] sdvo tv clock improvements + some random stuff Daniel Vetter
  2013-04-30 12:01 ` [PATCH 1/7] drm/i915: simplify DP/DDI port width macros Daniel Vetter
  2013-04-30 12:01 ` [PATCH 2/7] drm/i915: move sdvo TV clock computation to intel_sdvo.c Daniel Vetter
@ 2013-04-30 12:01 ` Daniel Vetter
  2013-05-10 11:18   ` Jani Nikula
  2013-04-30 12:01 ` [PATCH 4/7] drm/i915: rip out TV-out lore Daniel Vetter
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2013-04-30 12:01 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

TV-out uses the same reference clock as everyone else. The only
difference seems to be in the slightly different CB tuning limit.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f10f094..317e055 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5629,9 +5629,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 			if (intel_encoder->needs_tv_clock)
 				is_tv = true;
 			break;
-		case INTEL_OUTPUT_TVOUT:
-			is_tv = true;
-			break;
 		}
 
 		num_connectors++;
@@ -5690,13 +5687,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 		break;
 	}
 
-	if (is_sdvo && is_tv)
-		dpll |= PLL_REF_INPUT_TVCLKINBC;
-	else if (is_tv)
-		/* XXX: just matching BIOS for now */
-		/*	dpll |= PLL_REF_INPUT_TVCLKINBC; */
-		dpll |= 3;
-	else if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2)
+	if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2)
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
 		dpll |= PLL_REF_INPUT_DREFCLK;
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 4/7] drm/i915: rip out TV-out lore ...
  2013-04-30 12:01 [PATCH 0/7] sdvo tv clock improvements + some random stuff Daniel Vetter
                   ` (2 preceding siblings ...)
  2013-04-30 12:01 ` [PATCH 3/7] drm/i915: drop TVclock special casing on ilk+ Daniel Vetter
@ 2013-04-30 12:01 ` Daniel Vetter
  2013-05-10 11:34   ` Jani Nikula
  2013-04-30 12:01 ` [PATCH 5/7] drm/i915: rip out now unused is_foo tracking from crtc code Daniel Vetter
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2013-04-30 12:01 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

This seems to be an impressive piece of copy&pasta lore. I've
checked all docs and on most platforms these bits are all MBZ, with
the exception of the SDVO pixel multiplier on gen3. On gen4 that
moved to a special DPLL_MD registers.

No indication whatsoever that we actually need this for native
TV-out support. I suspect this started as a hack when we didn't
yet have proper pixel multiplier support in place for SDVO TV, but
then got stuck in a life of its own.

Just rip it out.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 317e055..e68d26a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4548,10 +4548,6 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
 
 	if (is_sdvo && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
 		dpll |= PLL_REF_INPUT_TVCLKINBC;
-	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
-		/* XXX: just matching BIOS for now */
-		/*	dpll |= PLL_REF_INPUT_TVCLKINBC; */
-		dpll |= 3;
 	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
 		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 5/7] drm/i915: rip out now unused is_foo tracking from crtc code
  2013-04-30 12:01 [PATCH 0/7] sdvo tv clock improvements + some random stuff Daniel Vetter
                   ` (3 preceding siblings ...)
  2013-04-30 12:01 ` [PATCH 4/7] drm/i915: rip out TV-out lore Daniel Vetter
@ 2013-04-30 12:01 ` Daniel Vetter
  2013-05-10 11:39   ` Jani Nikula
  2013-04-30 12:01 ` [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices Daniel Vetter
  2013-04-30 12:01 ` [PATCH 7/7] drm/i915: rip out an unused lvds_reg variable Daniel Vetter
  6 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2013-04-30 12:01 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

More ugly stuff gone for good! The big special case left now is
lvds (which is indeed really special).

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 28 +++-------------------------
 1 file changed, 3 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e68d26a..44bcfae 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4833,8 +4833,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	int refclk, num_connectors = 0;
 	intel_clock_t clock, reduced_clock;
 	u32 dspcntr;
-	bool ok, has_reduced_clock = false, is_sdvo = false;
-	bool is_lvds = false, is_tv = false;
+	bool ok, has_reduced_clock = false;
+	bool is_lvds = false;
 	struct intel_encoder *encoder;
 	const intel_limit_t *limit;
 	int ret;
@@ -4844,15 +4844,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 		case INTEL_OUTPUT_LVDS:
 			is_lvds = true;
 			break;
-		case INTEL_OUTPUT_SDVO:
-		case INTEL_OUTPUT_HDMI:
-			is_sdvo = true;
-			if (encoder->needs_tv_clock)
-				is_tv = true;
-			break;
-		case INTEL_OUTPUT_TVOUT:
-			is_tv = true;
-			break;
 		}
 
 		num_connectors++;
@@ -5291,7 +5282,6 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_encoder *encoder;
-	struct intel_encoder *edp_encoder = NULL;
 	int num_connectors = 0;
 	bool is_lvds = false;
 
@@ -5300,9 +5290,6 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
 		case INTEL_OUTPUT_LVDS:
 			is_lvds = true;
 			break;
-		case INTEL_OUTPUT_EDP:
-			edp_encoder = encoder;
-			break;
 		}
 		num_connectors++;
 	}
@@ -5461,22 +5448,13 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 	struct intel_encoder *intel_encoder;
 	int refclk;
 	const intel_limit_t *limit;
-	bool ret, is_sdvo = false, is_tv = false, is_lvds = false;
+	bool ret, is_lvds = false;
 
 	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
 		switch (intel_encoder->type) {
 		case INTEL_OUTPUT_LVDS:
 			is_lvds = true;
 			break;
-		case INTEL_OUTPUT_SDVO:
-		case INTEL_OUTPUT_HDMI:
-			is_sdvo = true;
-			if (intel_encoder->needs_tv_clock)
-				is_tv = true;
-			break;
-		case INTEL_OUTPUT_TVOUT:
-			is_tv = true;
-			break;
 		}
 	}
 
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices
  2013-04-30 12:01 [PATCH 0/7] sdvo tv clock improvements + some random stuff Daniel Vetter
                   ` (4 preceding siblings ...)
  2013-04-30 12:01 ` [PATCH 5/7] drm/i915: rip out now unused is_foo tracking from crtc code Daniel Vetter
@ 2013-04-30 12:01 ` Daniel Vetter
  2013-04-30 12:49   ` Chris Wilson
                     ` (2 more replies)
  2013-04-30 12:01 ` [PATCH 7/7] drm/i915: rip out an unused lvds_reg variable Daniel Vetter
  6 siblings, 3 replies; 23+ messages in thread
From: Daniel Vetter @ 2013-04-30 12:01 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

We need to track this correctly. While at it shovel the boolean
to track whether the sdvo is in tv mode or not into pipe_config.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36997
Tested-by: Pierre Assal <pierre.assal@verint.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36997
Tested-by: cancan,feng <cancan.feng@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 8 +++-----
 drivers/gpu/drm/i915/intel_drv.h     | 5 ++++-
 drivers/gpu/drm/i915/intel_sdvo.c    | 8 ++------
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 44bcfae..ef0d27b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4546,7 +4546,7 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
 	if (INTEL_INFO(dev)->gen >= 4)
 		dpll |= (6 << PLL_LOAD_PULSE_PHASE_SHIFT);
 
-	if (is_sdvo && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
+	if (crtc->config.sdvo_tv_clock)
 		dpll |= PLL_REF_INPUT_TVCLKINBC;
 	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
 		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
@@ -5590,7 +5590,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	struct intel_encoder *intel_encoder;
 	uint32_t dpll;
 	int factor, num_connectors = 0;
-	bool is_lvds = false, is_sdvo = false, is_tv = false;
+	bool is_lvds = false, is_sdvo = false;
 
 	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
 		switch (intel_encoder->type) {
@@ -5600,8 +5600,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 		case INTEL_OUTPUT_SDVO:
 		case INTEL_OUTPUT_HDMI:
 			is_sdvo = true;
-			if (intel_encoder->needs_tv_clock)
-				is_tv = true;
 			break;
 		}
 
@@ -5615,7 +5613,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 		     dev_priv->lvds_ssc_freq == 100) ||
 		    (HAS_PCH_IBX(dev) && intel_is_dual_link_lvds(dev)))
 			factor = 25;
-	} else if (is_sdvo && is_tv)
+	} else if (intel_crtc->config.sdvo_tv_clock)
 		factor = 20;
 
 	if (ironlake_needs_fb_cb_tune(&intel_crtc->config.dpll, factor))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 766afcf..be196ff 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -120,7 +120,6 @@ struct intel_encoder {
 	struct intel_crtc *new_crtc;
 
 	int type;
-	bool needs_tv_clock;
 	/*
 	 * Intel hw has only one MUX where encoders could be clone, hence a
 	 * simple flag is enough to compute the possible_clones mask.
@@ -223,6 +222,10 @@ struct intel_crtc_config {
 	/* Controls for the clock computation, to override various stages. */
 	bool clock_set;
 
+	/* SDVO TV has a bunch of special case. To make multifunction encoders
+	 * work correctly, we need to track this at runtime.*/
+	bool sdvo_tv_clock;
+
 	/*
 	 * crtc bandwidth limit, don't increase pipe bpp or clock if not really
 	 * required. This is set in the 2nd loop of calling encoder's
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index f6bf9fc..3a1d710 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1092,6 +1092,7 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
 		(void) intel_sdvo_get_preferred_input_mode(intel_sdvo,
 							   mode,
 							   adjusted_mode);
+		pipe_config->sdvo_tv_clock = true;
 	} else if (intel_sdvo->is_lvds) {
 		if (!intel_sdvo_set_output_timings_from_mode(intel_sdvo,
 							     intel_sdvo->sdvo_lvds_fixed_mode))
@@ -1655,12 +1656,9 @@ intel_sdvo_detect(struct drm_connector *connector, bool force)
 	if (ret == connector_status_connected) {
 		intel_sdvo->is_tv = false;
 		intel_sdvo->is_lvds = false;
-		intel_sdvo->base.needs_tv_clock = false;
 
-		if (response & SDVO_TV_MASK) {
+		if (response & SDVO_TV_MASK)
 			intel_sdvo->is_tv = true;
-			intel_sdvo->base.needs_tv_clock = true;
-		}
 		if (response & SDVO_LVDS_MASK)
 			intel_sdvo->is_lvds = intel_sdvo->sdvo_lvds_fixed_mode != NULL;
 	}
@@ -2357,7 +2355,6 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
 	intel_sdvo_connector->output_flag = type;
 
 	intel_sdvo->is_tv = true;
-	intel_sdvo->base.needs_tv_clock = true;
 
 	intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo);
 
@@ -2445,7 +2442,6 @@ static bool
 intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, uint16_t flags)
 {
 	intel_sdvo->is_tv = false;
-	intel_sdvo->base.needs_tv_clock = false;
 	intel_sdvo->is_lvds = false;
 
 	/* SDVO requires XXX1 function may not exist unless it has XXX0 function.*/
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 7/7] drm/i915: rip out an unused lvds_reg variable
  2013-04-30 12:01 [PATCH 0/7] sdvo tv clock improvements + some random stuff Daniel Vetter
                   ` (5 preceding siblings ...)
  2013-04-30 12:01 ` [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices Daniel Vetter
@ 2013-04-30 12:01 ` Daniel Vetter
  2013-05-10 11:42   ` Jani Nikula
  6 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2013-04-30 12:01 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

Somehow this has been forgotten in

commit 1974cad0ee4ce84e5cb792e49c4f0d9421e0312c
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Mon Nov 26 17:22:09 2012 +0100

    drm/i915: move is_dual_link_lvds to intel_lvds.c

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ef0d27b..30f452e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -651,12 +651,6 @@ intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 	found = false;
 
 	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
-		int lvds_reg;
-
-		if (HAS_PCH_SPLIT(dev))
-			lvds_reg = PCH_LVDS;
-		else
-			lvds_reg = LVDS;
 		if (intel_is_dual_link_lvds(dev))
 			clock.p2 = limit->p2.p2_fast;
 		else
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices
  2013-04-30 12:01 ` [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices Daniel Vetter
@ 2013-04-30 12:49   ` Chris Wilson
  2013-04-30 13:10     ` Daniel Vetter
  2013-05-06 13:36   ` Jani Nikula
  2013-05-10 11:47   ` Jani Nikula
  2 siblings, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2013-04-30 12:49 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development

On Tue, Apr 30, 2013 at 02:01:45PM +0200, Daniel Vetter wrote:
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -223,6 +222,10 @@ struct intel_crtc_config {
>  	/* Controls for the clock computation, to override various stages. */
>  	bool clock_set;
>  
> +	/* SDVO TV has a bunch of special case. To make multifunction encoders
> +	 * work correctly, we need to track this at runtime.*/
> +	bool sdvo_tv_clock;
> +
>  	/*
>  	 * crtc bandwidth limit, don't increase pipe bpp or clock if not really
>  	 * required. This is set in the 2nd loop of calling encoder's

What is becoming less clear over time is what is derived state internal
to the modesetting sequence, and what is the state used to select a mode
e.g. to decide if two configs are compatible.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices
  2013-04-30 12:49   ` Chris Wilson
@ 2013-04-30 13:10     ` Daniel Vetter
  0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2013-04-30 13:10 UTC (permalink / raw)
  To: Chris Wilson, Daniel Vetter, Intel Graphics Development

On Tue, Apr 30, 2013 at 2:49 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue, Apr 30, 2013 at 02:01:45PM +0200, Daniel Vetter wrote:
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -223,6 +222,10 @@ struct intel_crtc_config {
>>       /* Controls for the clock computation, to override various stages. */
>>       bool clock_set;
>>
>> +     /* SDVO TV has a bunch of special case. To make multifunction encoders
>> +      * work correctly, we need to track this at runtime.*/
>> +     bool sdvo_tv_clock;
>> +
>>       /*
>>        * crtc bandwidth limit, don't increase pipe bpp or clock if not really
>>        * required. This is set in the 2nd loop of calling encoder's
>
> What is becoming less clear over time is what is derived state internal
> to the modesetting sequence, and what is the state used to select a mode
> e.g. to decide if two configs are compatible.

Atm everything is derived state and we currently have don't take the
pipe config into account for comparing state. My plan is that we
eventually add flags to the pipe_config compare function to decide
whether we need a full-blown modeset or can go with an "update pfit
state" fastpath.

So atm I'm just moving state around, since pipe_config will also be
used for atomic modeset. And checking the hw config upfront e.g. for
pll sharing has similar, but not completely overlapping needs to the
state comparison fastboot needs to do.

Another thing I don't yet have an opinion about is what we should do
if the mode matches, but the configuration details are a bit
suboptimal (like wasteful watermark settings). I guess we need to
rework the code so that we can fix this all up without stopping the
crtc. And there's tons of little bits of state like that (audio
config, infoframes, clock trees, watermarks, fdi config, ...). Added
fun is that often it is highly platform specific whether a given piece
of state must match for fastboot or not ...

My current plan is to just chip away with pipe_config conversions,
with more a focus towards atomic modeset (i.e. upfront checking),
leaving some of the harder fastboot issues for Jesse ;-)

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/7] drm/i915: simplify DP/DDI port width macros
  2013-04-30 12:01 ` [PATCH 1/7] drm/i915: simplify DP/DDI port width macros Daniel Vetter
@ 2013-05-02 13:34   ` Paulo Zanoni
  2013-05-02 14:51     ` Daniel Vetter
  0 siblings, 1 reply; 23+ messages in thread
From: Paulo Zanoni @ 2013-05-02 13:34 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development, Paulo Zanoni

2013/4/30 Daniel Vetter <daniel.vetter@ffwll.ch>:
> If we ever leak a non-DP compliant port width through here, we have a
> pretty serious issue. So just rip out all these WARNs - if we need
> them it's probably better to have them at a central place where we
> compute the dp lane count.
>
> Also use the new DDI width macro for FDI mode.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Nice one.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 11 ++---------
>  drivers/gpu/drm/i915/intel_ddi.c | 34 ++--------------------------------
>  drivers/gpu/drm/i915/intel_dp.c  | 12 +-----------
>  3 files changed, 5 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 95ae5cf..d84d694 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2664,9 +2664,7 @@
>  #define   DP_PRE_EMPHASIS_SHIFT                22
>
>  /* How many wires to use. I guess 3 was too hard */
> -#define   DP_PORT_WIDTH_1              (0 << 19)
> -#define   DP_PORT_WIDTH_2              (1 << 19)
> -#define   DP_PORT_WIDTH_4              (3 << 19)
> +#define   DP_PORT_WIDTH(width)         (((width) - 1) << 19)
>  #define   DP_PORT_WIDTH_MASK           (7 << 19)
>
>  /* Mystic DPCD version 1.1 special mode */
> @@ -4751,9 +4749,6 @@
>  #define  TRANS_DDI_EDP_INPUT_B_ONOFF   (5<<12)
>  #define  TRANS_DDI_EDP_INPUT_C_ONOFF   (6<<12)
>  #define  TRANS_DDI_BFI_ENABLE          (1<<4)
> -#define  TRANS_DDI_PORT_WIDTH_X1       (0<<1)
> -#define  TRANS_DDI_PORT_WIDTH_X2       (1<<1)
> -#define  TRANS_DDI_PORT_WIDTH_X4       (3<<1)
>
>  /* DisplayPort Transport Control */
>  #define DP_TP_CTL_A                    0x64040
> @@ -4797,9 +4792,7 @@
>  #define  DDI_BUF_PORT_REVERSAL                 (1<<16)
>  #define  DDI_BUF_IS_IDLE                       (1<<7)
>  #define  DDI_A_4_LANES                         (1<<4)
> -#define  DDI_PORT_WIDTH_X1                     (0<<1)
> -#define  DDI_PORT_WIDTH_X2                     (1<<1)
> -#define  DDI_PORT_WIDTH_X4                     (3<<1)
> +#define  DDI_PORT_WIDTH(width)                 (((width) - 1) << 1)
>  #define  DDI_INIT_DISPLAY_DETECTED             (1<<0)
>
>  /* DDI Buffer Translations */
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 72941f9..841c9a9 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -687,22 +687,7 @@ static void intel_ddi_mode_set(struct drm_encoder *encoder,
>
>                 intel_dp->DP = intel_dig_port->port_reversal |
>                                DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW;
> -               switch (intel_dp->lane_count) {
> -               case 1:
> -                       intel_dp->DP |= DDI_PORT_WIDTH_X1;
> -                       break;
> -               case 2:
> -                       intel_dp->DP |= DDI_PORT_WIDTH_X2;
> -                       break;
> -               case 4:
> -                       intel_dp->DP |= DDI_PORT_WIDTH_X4;
> -                       break;
> -               default:
> -                       intel_dp->DP |= DDI_PORT_WIDTH_X4;
> -                       WARN(1, "Unexpected DP lane count %d\n",
> -                            intel_dp->lane_count);
> -                       break;
> -               }
> +               intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
>
>                 if (intel_dp->has_audio) {
>                         DRM_DEBUG_DRIVER("DP audio on pipe %c on DDI\n",
> @@ -1031,22 +1016,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
>
>                 temp |= TRANS_DDI_MODE_SELECT_DP_SST;
>
> -               switch (intel_dp->lane_count) {
> -               case 1:
> -                       temp |= TRANS_DDI_PORT_WIDTH_X1;
> -                       break;
> -               case 2:
> -                       temp |= TRANS_DDI_PORT_WIDTH_X2;
> -                       break;
> -               case 4:
> -                       temp |= TRANS_DDI_PORT_WIDTH_X4;
> -                       break;
> -               default:
> -                       temp |= TRANS_DDI_PORT_WIDTH_X4;
> -                       WARN(1, "Unsupported lane count %d\n",
> -                            intel_dp->lane_count);
> -               }
> -
> +               intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
>         } else {
>                 WARN(1, "Invalid encoder type %d for pipe %c\n",
>                      intel_encoder->type, pipe_name(pipe));
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8759fb1..99f798a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -891,18 +891,8 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
>
>         /* Handle DP bits in common between all three register formats */
>         intel_dp->DP |= DP_VOLTAGE_0_4 | DP_PRE_EMPHASIS_0;
> +       intel_dp->DP |= DP_PORT_WIDTH(intel_dp->lane_count);
>
> -       switch (intel_dp->lane_count) {
> -       case 1:
> -               intel_dp->DP |= DP_PORT_WIDTH_1;
> -               break;
> -       case 2:
> -               intel_dp->DP |= DP_PORT_WIDTH_2;
> -               break;
> -       case 4:
> -               intel_dp->DP |= DP_PORT_WIDTH_4;
> -               break;
> -       }
>         if (intel_dp->has_audio) {
>                 DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
>                                  pipe_name(intel_crtc->pipe));
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/7] drm/i915: simplify DP/DDI port width macros
  2013-05-02 13:34   ` Paulo Zanoni
@ 2013-05-02 14:51     ` Daniel Vetter
  2013-05-02 17:38       ` Paulo Zanoni
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2013-05-02 14:51 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Daniel Vetter, Intel Graphics Development, Paulo Zanoni

On Thu, May 02, 2013 at 10:34:23AM -0300, Paulo Zanoni wrote:
> 2013/4/30 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > If we ever leak a non-DP compliant port width through here, we have a
> > pretty serious issue. So just rip out all these WARNs - if we need
> > them it's probably better to have them at a central place where we
> > compute the dp lane count.
> >
> > Also use the new DDI width macro for FDI mode.
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Nice one.
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Queued for -next, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/7] drm/i915: simplify DP/DDI port width macros
  2013-05-02 14:51     ` Daniel Vetter
@ 2013-05-02 17:38       ` Paulo Zanoni
  2013-05-02 19:29         ` Daniel Vetter
  0 siblings, 1 reply; 23+ messages in thread
From: Paulo Zanoni @ 2013-05-02 17:38 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, Paulo Zanoni

2013/5/2 Daniel Vetter <daniel@ffwll.ch>:
> On Thu, May 02, 2013 at 10:34:23AM -0300, Paulo Zanoni wrote:
>> 2013/4/30 Daniel Vetter <daniel.vetter@ffwll.ch>:
>> > If we ever leak a non-DP compliant port width through here, we have a
>> > pretty serious issue. So just rip out all these WARNs - if we need
>> > them it's probably better to have them at a central place where we
>> > compute the dp lane count.
>> >
>> > Also use the new DDI width macro for FDI mode.
>> >
>> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> Nice one.
>>
>> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Queued for -next, thanks for the review.

And now that I've actually booted a Kernel with the patch I see that
the chunk inside intel_ddi_enable_transcoder_func is wrong. We should
assign to the "temp" variable, not "intel_dp->DP". This gives me a
nice black screen on DP.

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/7] drm/i915: simplify DP/DDI port width macros
  2013-05-02 17:38       ` Paulo Zanoni
@ 2013-05-02 19:29         ` Daniel Vetter
  0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2013-05-02 19:29 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Daniel Vetter, Intel Graphics Development, Paulo Zanoni

On Thu, May 02, 2013 at 02:38:09PM -0300, Paulo Zanoni wrote:
> 2013/5/2 Daniel Vetter <daniel@ffwll.ch>:
> > On Thu, May 02, 2013 at 10:34:23AM -0300, Paulo Zanoni wrote:
> >> 2013/4/30 Daniel Vetter <daniel.vetter@ffwll.ch>:
> >> > If we ever leak a non-DP compliant port width through here, we have a
> >> > pretty serious issue. So just rip out all these WARNs - if we need
> >> > them it's probably better to have them at a central place where we
> >> > compute the dp lane count.
> >> >
> >> > Also use the new DDI width macro for FDI mode.
> >> >
> >> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>
> >> Nice one.
> >>
> >> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Queued for -next, thanks for the review.
> 
> And now that I've actually booted a Kernel with the patch I see that
> the chunk inside intel_ddi_enable_transcoder_func is wrong. We should
> assign to the "temp" variable, not "intel_dp->DP". This gives me a
> nice black screen on DP.

Oops. Ok, I've pushed an updated version, that one better?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices
  2013-04-30 12:01 ` [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices Daniel Vetter
  2013-04-30 12:49   ` Chris Wilson
@ 2013-05-06 13:36   ` Jani Nikula
  2013-05-06 14:15     ` Daniel Vetter
  2013-05-10 11:47   ` Jani Nikula
  2 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2013-05-06 13:36 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

On Tue, 30 Apr 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We need to track this correctly. While at it shovel the boolean
> to track whether the sdvo is in tv mode or not into pipe_config.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36997

Bug scrubbing, should this be:
https://bugs.freedesktop.org/show_bug.cgi?id=63609 ?

Jani.


> Tested-by: Pierre Assal <pierre.assal@verint.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36997
> Tested-by: cancan,feng <cancan.feng@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 +++-----
>  drivers/gpu/drm/i915/intel_drv.h     | 5 ++++-
>  drivers/gpu/drm/i915/intel_sdvo.c    | 8 ++------
>  3 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 44bcfae..ef0d27b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4546,7 +4546,7 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
>  	if (INTEL_INFO(dev)->gen >= 4)
>  		dpll |= (6 << PLL_LOAD_PULSE_PHASE_SHIFT);
>  
> -	if (is_sdvo && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
> +	if (crtc->config.sdvo_tv_clock)
>  		dpll |= PLL_REF_INPUT_TVCLKINBC;
>  	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> @@ -5590,7 +5590,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  	struct intel_encoder *intel_encoder;
>  	uint32_t dpll;
>  	int factor, num_connectors = 0;
> -	bool is_lvds = false, is_sdvo = false, is_tv = false;
> +	bool is_lvds = false, is_sdvo = false;
>  
>  	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
>  		switch (intel_encoder->type) {
> @@ -5600,8 +5600,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  		case INTEL_OUTPUT_SDVO:
>  		case INTEL_OUTPUT_HDMI:
>  			is_sdvo = true;
> -			if (intel_encoder->needs_tv_clock)
> -				is_tv = true;
>  			break;
>  		}
>  
> @@ -5615,7 +5613,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  		     dev_priv->lvds_ssc_freq == 100) ||
>  		    (HAS_PCH_IBX(dev) && intel_is_dual_link_lvds(dev)))
>  			factor = 25;
> -	} else if (is_sdvo && is_tv)
> +	} else if (intel_crtc->config.sdvo_tv_clock)
>  		factor = 20;
>  
>  	if (ironlake_needs_fb_cb_tune(&intel_crtc->config.dpll, factor))
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 766afcf..be196ff 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -120,7 +120,6 @@ struct intel_encoder {
>  	struct intel_crtc *new_crtc;
>  
>  	int type;
> -	bool needs_tv_clock;
>  	/*
>  	 * Intel hw has only one MUX where encoders could be clone, hence a
>  	 * simple flag is enough to compute the possible_clones mask.
> @@ -223,6 +222,10 @@ struct intel_crtc_config {
>  	/* Controls for the clock computation, to override various stages. */
>  	bool clock_set;
>  
> +	/* SDVO TV has a bunch of special case. To make multifunction encoders
> +	 * work correctly, we need to track this at runtime.*/
> +	bool sdvo_tv_clock;
> +
>  	/*
>  	 * crtc bandwidth limit, don't increase pipe bpp or clock if not really
>  	 * required. This is set in the 2nd loop of calling encoder's
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index f6bf9fc..3a1d710 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1092,6 +1092,7 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  		(void) intel_sdvo_get_preferred_input_mode(intel_sdvo,
>  							   mode,
>  							   adjusted_mode);
> +		pipe_config->sdvo_tv_clock = true;
>  	} else if (intel_sdvo->is_lvds) {
>  		if (!intel_sdvo_set_output_timings_from_mode(intel_sdvo,
>  							     intel_sdvo->sdvo_lvds_fixed_mode))
> @@ -1655,12 +1656,9 @@ intel_sdvo_detect(struct drm_connector *connector, bool force)
>  	if (ret == connector_status_connected) {
>  		intel_sdvo->is_tv = false;
>  		intel_sdvo->is_lvds = false;
> -		intel_sdvo->base.needs_tv_clock = false;
>  
> -		if (response & SDVO_TV_MASK) {
> +		if (response & SDVO_TV_MASK)
>  			intel_sdvo->is_tv = true;
> -			intel_sdvo->base.needs_tv_clock = true;
> -		}
>  		if (response & SDVO_LVDS_MASK)
>  			intel_sdvo->is_lvds = intel_sdvo->sdvo_lvds_fixed_mode != NULL;
>  	}
> @@ -2357,7 +2355,6 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
>  	intel_sdvo_connector->output_flag = type;
>  
>  	intel_sdvo->is_tv = true;
> -	intel_sdvo->base.needs_tv_clock = true;
>  
>  	intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo);
>  
> @@ -2445,7 +2442,6 @@ static bool
>  intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, uint16_t flags)
>  {
>  	intel_sdvo->is_tv = false;
> -	intel_sdvo->base.needs_tv_clock = false;
>  	intel_sdvo->is_lvds = false;
>  
>  	/* SDVO requires XXX1 function may not exist unless it has XXX0 function.*/
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices
  2013-05-06 13:36   ` Jani Nikula
@ 2013-05-06 14:15     ` Daniel Vetter
  0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2013-05-06 14:15 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Intel Graphics Development

On Mon, May 6, 2013 at 3:36 PM, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Tue, 30 Apr 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> We need to track this correctly. While at it shovel the boolean
>> to track whether the sdvo is in tv mode or not into pipe_config.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36997
>
> Bug scrubbing, should this be:
> https://bugs.freedesktop.org/show_bug.cgi?id=63609 ?


Yeah, I've copy&pasted the wrong bugzilla link, that's the 2nd one.
Thanks for spotting.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/7] drm/i915: move sdvo TV clock computation to intel_sdvo.c
  2013-04-30 12:01 ` [PATCH 2/7] drm/i915: move sdvo TV clock computation to intel_sdvo.c Daniel Vetter
@ 2013-05-10 11:13   ` Jani Nikula
  0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2013-05-10 11:13 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

On Tue, 30 Apr 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We have a very nice infrastructure for this now!
>
> Note that the multifunction sdvo support is pretty neatly broken: We
> completely ignore userspace's request for which connector to wire up
> with the encoder and just use whatever the last detect callback has
> seen.
>
> Not something I'll fix in this patch, but unfortunately something
> which is also broken in the DDI code ...
>
> v2: Don't call sdvo_tv_clock twice.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 30 ------------------------------
>  drivers/gpu/drm/i915/intel_sdvo.c    | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1128f83..f10f094 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4286,30 +4286,6 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
>  	return refclk;
>  }
>  
> -static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc *crtc)
> -{
> -	unsigned dotclock = crtc->config.adjusted_mode.clock;
> -	struct dpll *clock = &crtc->config.dpll;
> -
> -	/* SDVO TV has fixed PLL values depend on its clock range,
> -	   this mirrors vbios setting. */
> -	if (dotclock >= 100000 && dotclock < 140500) {
> -		clock->p1 = 2;
> -		clock->p2 = 10;
> -		clock->n = 3;
> -		clock->m1 = 16;
> -		clock->m2 = 8;
> -	} else if (dotclock >= 140500 && dotclock <= 200000) {
> -		clock->p1 = 1;
> -		clock->p2 = 10;
> -		clock->n = 6;
> -		clock->m1 = 12;
> -		clock->m2 = 8;
> -	}
> -
> -	crtc->config.clock_set = true;
> -}
> -
>  static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
>  {
>  	return (1 << dpll->n) << 16 | dpll->m1 << 8 | dpll->m2;
> @@ -4926,9 +4902,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  		intel_crtc->config.dpll.p2 = clock.p2;
>  	}
>  
> -	if (is_sdvo && is_tv)
> -		i9xx_adjust_sdvo_tv_clock(intel_crtc);
> -
>  	if (IS_GEN2(dev))
>  		i8xx_update_pll(intel_crtc, adjusted_mode,
>  				has_reduced_clock ? &reduced_clock : NULL,
> @@ -5538,9 +5511,6 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>  						     reduced_clock);
>  	}
>  
> -	if (is_sdvo && is_tv)
> -		i9xx_adjust_sdvo_tv_clock(to_intel_crtc(crtc));
> -

With this change, is_sdvo and is_tv are no longer used, and could be
dropped from the function. All the same,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>  	return true;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index d154284..f6bf9fc 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1041,6 +1041,32 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
>  	return true;
>  }
>  
> +static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_config *pipe_config)
> +{
> +	unsigned dotclock = pipe_config->adjusted_mode.clock;
> +	struct dpll *clock = &pipe_config->dpll;
> +
> +	/* SDVO TV has fixed PLL values depend on its clock range,
> +	   this mirrors vbios setting. */
> +	if (dotclock >= 100000 && dotclock < 140500) {
> +		clock->p1 = 2;
> +		clock->p2 = 10;
> +		clock->n = 3;
> +		clock->m1 = 16;
> +		clock->m2 = 8;
> +	} else if (dotclock >= 140500 && dotclock <= 200000) {
> +		clock->p1 = 1;
> +		clock->p2 = 10;
> +		clock->n = 6;
> +		clock->m1 = 12;
> +		clock->m2 = 8;
> +	} else {
> +		WARN(1, "SDVO TV clock out of range: %i\n", dotclock);
> +	}
> +
> +	pipe_config->clock_set = true;
> +}
> +
>  static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  				      struct intel_crtc_config *pipe_config)
>  {
> @@ -1097,6 +1123,10 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  	if (intel_sdvo->color_range)
>  		pipe_config->limited_color_range = true;
>  
> +	/* Clock computation needs to happen after pixel multiplier. */
> +	if (intel_sdvo->is_tv)
> +		i9xx_adjust_sdvo_tv_clock(pipe_config);
> +
>  	return true;
>  }
>  
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/7] drm/i915: drop TVclock special casing on ilk+
  2013-04-30 12:01 ` [PATCH 3/7] drm/i915: drop TVclock special casing on ilk+ Daniel Vetter
@ 2013-05-10 11:18   ` Jani Nikula
  0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2013-05-10 11:18 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

On Tue, 30 Apr 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> TV-out uses the same reference clock as everyone else. The only
> difference seems to be in the slightly different CB tuning limit.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f10f094..317e055 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5629,9 +5629,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  			if (intel_encoder->needs_tv_clock)
>  				is_tv = true;
>  			break;
> -		case INTEL_OUTPUT_TVOUT:
> -			is_tv = true;
> -			break;

An observation, this code path is actually dead code for ilk+, as is the
else if (is_tv) branch below. Good riddance.

>  		}
>  
>  		num_connectors++;
> @@ -5690,13 +5687,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  		break;
>  	}
>  
> -	if (is_sdvo && is_tv)
> -		dpll |= PLL_REF_INPUT_TVCLKINBC;

The commit message could reflect that this is actually a reserved value
in the register spec. With that bikeshed,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> -	else if (is_tv)
> -		/* XXX: just matching BIOS for now */
> -		/*	dpll |= PLL_REF_INPUT_TVCLKINBC; */
> -		dpll |= 3;
> -	else if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> +	if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>  	else
>  		dpll |= PLL_REF_INPUT_DREFCLK;
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/7] drm/i915: rip out TV-out lore ...
  2013-04-30 12:01 ` [PATCH 4/7] drm/i915: rip out TV-out lore Daniel Vetter
@ 2013-05-10 11:34   ` Jani Nikula
  0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2013-05-10 11:34 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

On Tue, 30 Apr 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> This seems to be an impressive piece of copy&pasta lore. I've
> checked all docs and on most platforms these bits are all MBZ, with
> the exception of the SDVO pixel multiplier on gen3. On gen4 that
> moved to a special DPLL_MD registers.
>
> No indication whatsoever that we actually need this for native
> TV-out support. I suspect this started as a hack when we didn't
> yet have proper pixel multiplier support in place for SDVO TV, but
> then got stuck in a life of its own.
>
> Just rip it out.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 317e055..e68d26a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4548,10 +4548,6 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
>  
>  	if (is_sdvo && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
>  		dpll |= PLL_REF_INPUT_TVCLKINBC;
> -	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
> -		/* XXX: just matching BIOS for now */
> -		/*	dpll |= PLL_REF_INPUT_TVCLKINBC; */
> -		dpll |= 3;

I found a g45 spec saying this about bits 7:0 on CTG:

"FPB1 P1 Post Divisor: Writes to this byte cause the staging register
 contents to be written into the active register when in the VGA mode of
 operation. This will also occur when the VGA MSR register is written."

Additionally 3 is an illegal value...

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>  	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 5/7] drm/i915: rip out now unused is_foo tracking from crtc code
  2013-04-30 12:01 ` [PATCH 5/7] drm/i915: rip out now unused is_foo tracking from crtc code Daniel Vetter
@ 2013-05-10 11:39   ` Jani Nikula
  0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2013-05-10 11:39 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

On Tue, 30 Apr 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> More ugly stuff gone for good! The big special case left now is
> lvds (which is indeed really special).

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

A follow-up patch could switch the switches to ifs for the OCD folks. ;)

> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 28 +++-------------------------
>  1 file changed, 3 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e68d26a..44bcfae 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4833,8 +4833,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	int refclk, num_connectors = 0;
>  	intel_clock_t clock, reduced_clock;
>  	u32 dspcntr;
> -	bool ok, has_reduced_clock = false, is_sdvo = false;
> -	bool is_lvds = false, is_tv = false;
> +	bool ok, has_reduced_clock = false;
> +	bool is_lvds = false;
>  	struct intel_encoder *encoder;
>  	const intel_limit_t *limit;
>  	int ret;
> @@ -4844,15 +4844,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  		case INTEL_OUTPUT_LVDS:
>  			is_lvds = true;
>  			break;
> -		case INTEL_OUTPUT_SDVO:
> -		case INTEL_OUTPUT_HDMI:
> -			is_sdvo = true;
> -			if (encoder->needs_tv_clock)
> -				is_tv = true;
> -			break;
> -		case INTEL_OUTPUT_TVOUT:
> -			is_tv = true;
> -			break;
>  		}
>  
>  		num_connectors++;
> @@ -5291,7 +5282,6 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_encoder *encoder;
> -	struct intel_encoder *edp_encoder = NULL;
>  	int num_connectors = 0;
>  	bool is_lvds = false;
>  
> @@ -5300,9 +5290,6 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
>  		case INTEL_OUTPUT_LVDS:
>  			is_lvds = true;
>  			break;
> -		case INTEL_OUTPUT_EDP:
> -			edp_encoder = encoder;
> -			break;
>  		}
>  		num_connectors++;
>  	}
> @@ -5461,22 +5448,13 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>  	struct intel_encoder *intel_encoder;
>  	int refclk;
>  	const intel_limit_t *limit;
> -	bool ret, is_sdvo = false, is_tv = false, is_lvds = false;
> +	bool ret, is_lvds = false;
>  
>  	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
>  		switch (intel_encoder->type) {
>  		case INTEL_OUTPUT_LVDS:
>  			is_lvds = true;
>  			break;
> -		case INTEL_OUTPUT_SDVO:
> -		case INTEL_OUTPUT_HDMI:
> -			is_sdvo = true;
> -			if (intel_encoder->needs_tv_clock)
> -				is_tv = true;
> -			break;
> -		case INTEL_OUTPUT_TVOUT:
> -			is_tv = true;
> -			break;
>  		}
>  	}
>  
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/7] drm/i915: rip out an unused lvds_reg variable
  2013-04-30 12:01 ` [PATCH 7/7] drm/i915: rip out an unused lvds_reg variable Daniel Vetter
@ 2013-05-10 11:42   ` Jani Nikula
  2013-05-10 14:03     ` Daniel Vetter
  0 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2013-05-10 11:42 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

On Tue, 30 Apr 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Somehow this has been forgotten in
>
> commit 1974cad0ee4ce84e5cb792e49c4f0d9421e0312c
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Mon Nov 26 17:22:09 2012 +0100
>
>     drm/i915: move is_dual_link_lvds to intel_lvds.c
>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ef0d27b..30f452e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -651,12 +651,6 @@ intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
>  	found = false;
>  
>  	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> -		int lvds_reg;
> -
> -		if (HAS_PCH_SPLIT(dev))
> -			lvds_reg = PCH_LVDS;
> -		else
> -			lvds_reg = LVDS;
>  		if (intel_is_dual_link_lvds(dev))
>  			clock.p2 = limit->p2.p2_fast;
>  		else
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices
  2013-04-30 12:01 ` [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices Daniel Vetter
  2013-04-30 12:49   ` Chris Wilson
  2013-05-06 13:36   ` Jani Nikula
@ 2013-05-10 11:47   ` Jani Nikula
  2 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2013-05-10 11:47 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

On Tue, 30 Apr 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We need to track this correctly. While at it shovel the boolean
> to track whether the sdvo is in tv mode or not into pipe_config.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36997
> Tested-by: Pierre Assal <pierre.assal@verint.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36997
> Tested-by: cancan,feng <cancan.feng@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 +++-----
>  drivers/gpu/drm/i915/intel_drv.h     | 5 ++++-
>  drivers/gpu/drm/i915/intel_sdvo.c    | 8 ++------
>  3 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 44bcfae..ef0d27b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4546,7 +4546,7 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
>  	if (INTEL_INFO(dev)->gen >= 4)
>  		dpll |= (6 << PLL_LOAD_PULSE_PHASE_SHIFT);
>  
> -	if (is_sdvo && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
> +	if (crtc->config.sdvo_tv_clock)
>  		dpll |= PLL_REF_INPUT_TVCLKINBC;
>  	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> @@ -5590,7 +5590,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  	struct intel_encoder *intel_encoder;
>  	uint32_t dpll;
>  	int factor, num_connectors = 0;
> -	bool is_lvds = false, is_sdvo = false, is_tv = false;
> +	bool is_lvds = false, is_sdvo = false;
>  
>  	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
>  		switch (intel_encoder->type) {
> @@ -5600,8 +5600,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  		case INTEL_OUTPUT_SDVO:
>  		case INTEL_OUTPUT_HDMI:
>  			is_sdvo = true;
> -			if (intel_encoder->needs_tv_clock)
> -				is_tv = true;
>  			break;
>  		}
>  
> @@ -5615,7 +5613,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  		     dev_priv->lvds_ssc_freq == 100) ||
>  		    (HAS_PCH_IBX(dev) && intel_is_dual_link_lvds(dev)))
>  			factor = 25;
> -	} else if (is_sdvo && is_tv)
> +	} else if (intel_crtc->config.sdvo_tv_clock)
>  		factor = 20;
>  
>  	if (ironlake_needs_fb_cb_tune(&intel_crtc->config.dpll, factor))
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 766afcf..be196ff 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -120,7 +120,6 @@ struct intel_encoder {
>  	struct intel_crtc *new_crtc;
>  
>  	int type;
> -	bool needs_tv_clock;
>  	/*
>  	 * Intel hw has only one MUX where encoders could be clone, hence a
>  	 * simple flag is enough to compute the possible_clones mask.
> @@ -223,6 +222,10 @@ struct intel_crtc_config {
>  	/* Controls for the clock computation, to override various stages. */
>  	bool clock_set;
>  
> +	/* SDVO TV has a bunch of special case. To make multifunction encoders
> +	 * work correctly, we need to track this at runtime.*/
> +	bool sdvo_tv_clock;
> +
>  	/*
>  	 * crtc bandwidth limit, don't increase pipe bpp or clock if not really
>  	 * required. This is set in the 2nd loop of calling encoder's
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index f6bf9fc..3a1d710 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1092,6 +1092,7 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  		(void) intel_sdvo_get_preferred_input_mode(intel_sdvo,
>  							   mode,
>  							   adjusted_mode);
> +		pipe_config->sdvo_tv_clock = true;
>  	} else if (intel_sdvo->is_lvds) {
>  		if (!intel_sdvo_set_output_timings_from_mode(intel_sdvo,
>  							     intel_sdvo->sdvo_lvds_fixed_mode))
> @@ -1655,12 +1656,9 @@ intel_sdvo_detect(struct drm_connector *connector, bool force)
>  	if (ret == connector_status_connected) {
>  		intel_sdvo->is_tv = false;
>  		intel_sdvo->is_lvds = false;
> -		intel_sdvo->base.needs_tv_clock = false;
>  
> -		if (response & SDVO_TV_MASK) {
> +		if (response & SDVO_TV_MASK)
>  			intel_sdvo->is_tv = true;
> -			intel_sdvo->base.needs_tv_clock = true;
> -		}
>  		if (response & SDVO_LVDS_MASK)
>  			intel_sdvo->is_lvds = intel_sdvo->sdvo_lvds_fixed_mode != NULL;
>  	}
> @@ -2357,7 +2355,6 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
>  	intel_sdvo_connector->output_flag = type;
>  
>  	intel_sdvo->is_tv = true;
> -	intel_sdvo->base.needs_tv_clock = true;
>  
>  	intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo);
>  
> @@ -2445,7 +2442,6 @@ static bool
>  intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, uint16_t flags)
>  {
>  	intel_sdvo->is_tv = false;
> -	intel_sdvo->base.needs_tv_clock = false;
>  	intel_sdvo->is_lvds = false;
>  
>  	/* SDVO requires XXX1 function may not exist unless it has XXX0 function.*/
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/7] drm/i915: rip out an unused lvds_reg variable
  2013-05-10 11:42   ` Jani Nikula
@ 2013-05-10 14:03     ` Daniel Vetter
  0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2013-05-10 14:03 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Daniel Vetter, Intel Graphics Development

On Fri, May 10, 2013 at 02:42:03PM +0300, Jani Nikula wrote:
> On Tue, 30 Apr 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Somehow this has been forgotten in
> >
> > commit 1974cad0ee4ce84e5cb792e49c4f0d9421e0312c
> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Date:   Mon Nov 26 17:22:09 2012 +0100
> >
> >     drm/i915: move is_dual_link_lvds to intel_lvds.c
> >
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Oops, missed one. Queued for -next, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2013-05-10 14:00 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-30 12:01 [PATCH 0/7] sdvo tv clock improvements + some random stuff Daniel Vetter
2013-04-30 12:01 ` [PATCH 1/7] drm/i915: simplify DP/DDI port width macros Daniel Vetter
2013-05-02 13:34   ` Paulo Zanoni
2013-05-02 14:51     ` Daniel Vetter
2013-05-02 17:38       ` Paulo Zanoni
2013-05-02 19:29         ` Daniel Vetter
2013-04-30 12:01 ` [PATCH 2/7] drm/i915: move sdvo TV clock computation to intel_sdvo.c Daniel Vetter
2013-05-10 11:13   ` Jani Nikula
2013-04-30 12:01 ` [PATCH 3/7] drm/i915: drop TVclock special casing on ilk+ Daniel Vetter
2013-05-10 11:18   ` Jani Nikula
2013-04-30 12:01 ` [PATCH 4/7] drm/i915: rip out TV-out lore Daniel Vetter
2013-05-10 11:34   ` Jani Nikula
2013-04-30 12:01 ` [PATCH 5/7] drm/i915: rip out now unused is_foo tracking from crtc code Daniel Vetter
2013-05-10 11:39   ` Jani Nikula
2013-04-30 12:01 ` [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices Daniel Vetter
2013-04-30 12:49   ` Chris Wilson
2013-04-30 13:10     ` Daniel Vetter
2013-05-06 13:36   ` Jani Nikula
2013-05-06 14:15     ` Daniel Vetter
2013-05-10 11:47   ` Jani Nikula
2013-04-30 12:01 ` [PATCH 7/7] drm/i915: rip out an unused lvds_reg variable Daniel Vetter
2013-05-10 11:42   ` Jani Nikula
2013-05-10 14:03     ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox