* [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set
@ 2012-11-29 14:59 Daniel Vetter
2012-11-29 14:59 ` [PATCH 02/10] drm/i915: move set_pll_edp to intel_dp.c Daniel Vetter
` (9 more replies)
0 siblings, 10 replies; 18+ messages in thread
From: Daniel Vetter @ 2012-11-29 14:59 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
Especially getting rid of all things lvds is ... great!
v2: Drop the two additional pre-hsw hunks noticed by Paulo Zanoni.
v3:
- handle DP ports correctly (spoted by Paulo)
- don't leave {} behind for a single-line block (again spotted by
Paulo)
- kill another if (IBX || CPT) block
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 131 +----------------------------------
1 file changed, 2 insertions(+), 129 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3b6e4b0..fecaabc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5343,9 +5343,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
num_connectors++;
}
- WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)),
- "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev));
-
ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
&has_reduced_clock, &reduced_clock);
if (!ok) {
@@ -5464,20 +5461,13 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
int pipe = intel_crtc->pipe;
int plane = intel_crtc->plane;
int num_connectors = 0;
- intel_clock_t clock, reduced_clock;
- u32 dpll = 0, fp = 0, fp2 = 0;
- bool ok, has_reduced_clock = false;
- bool is_lvds = false, is_dp = false, is_cpu_edp = false;
+ bool is_dp = false, is_cpu_edp = false;
struct intel_encoder *encoder;
- u32 temp;
int ret;
bool dither;
for_each_encoder_on_crtc(dev, crtc, encoder) {
switch (encoder->type) {
- case INTEL_OUTPUT_LVDS:
- is_lvds = true;
- break;
case INTEL_OUTPUT_DISPLAYPORT:
is_dp = true;
break;
@@ -5511,143 +5501,26 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
if (!intel_ddi_pll_mode_set(crtc, adjusted_mode->clock))
return -EINVAL;
- if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
- ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
- &has_reduced_clock,
- &reduced_clock);
- if (!ok) {
- DRM_ERROR("Couldn't find PLL settings for mode!\n");
- return -EINVAL;
- }
- }
-
/* Ensure that the cursor is valid for the new mode before changing... */
intel_crtc_update_cursor(crtc, true);
/* determine panel color depth */
dither = intel_choose_pipe_bpp_dither(crtc, fb, &intel_crtc->bpp,
adjusted_mode);
- if (is_lvds && dev_priv->lvds_dither)
- dither = true;
DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
drm_mode_debug_printmodeline(mode);
- if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
- fp = clock.n << 16 | clock.m1 << 8 | clock.m2;
- if (has_reduced_clock)
- fp2 = reduced_clock.n << 16 | reduced_clock.m1 << 8 |
- reduced_clock.m2;
-
- dpll = ironlake_compute_dpll(intel_crtc, adjusted_mode, &clock,
- fp);
-
- /* CPU eDP is the only output that doesn't need a PCH PLL of its
- * own on pre-Haswell/LPT generation */
- if (!is_cpu_edp) {
- struct intel_pch_pll *pll;
-
- pll = intel_get_pch_pll(intel_crtc, dpll, fp);
- if (pll == NULL) {
- DRM_DEBUG_DRIVER("failed to find PLL for pipe %d\n",
- pipe);
- return -EINVAL;
- }
- } else
- intel_put_pch_pll(intel_crtc);
-
- /* The LVDS pin pair needs to be on before the DPLLs are
- * enabled. This is an exception to the general rule that
- * mode_set doesn't turn things on.
- */
- if (is_lvds) {
- temp = I915_READ(PCH_LVDS);
- temp |= LVDS_PORT_EN | LVDS_A0A2_CLKA_POWER_UP;
- if (HAS_PCH_CPT(dev)) {
- temp &= ~PORT_TRANS_SEL_MASK;
- temp |= PORT_TRANS_SEL_CPT(pipe);
- } else {
- if (pipe == 1)
- temp |= LVDS_PIPEB_SELECT;
- else
- temp &= ~LVDS_PIPEB_SELECT;
- }
-
- /* set the corresponsding LVDS_BORDER bit */
- temp |= dev_priv->lvds_border_bits;
- /* Set the B0-B3 data pairs corresponding to whether
- * we're going to set the DPLLs for dual-channel mode or
- * not.
- */
- if (clock.p2 == 7)
- temp |= LVDS_B0B3_POWER_UP | LVDS_CLKB_POWER_UP;
- else
- temp &= ~(LVDS_B0B3_POWER_UP |
- LVDS_CLKB_POWER_UP);
-
- /* It would be nice to set 24 vs 18-bit mode
- * (LVDS_A3_POWER_UP) appropriately here, but we need to
- * look more thoroughly into how panels behave in the
- * two modes.
- */
- temp &= ~(LVDS_HSYNC_POLARITY | LVDS_VSYNC_POLARITY);
- if (adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC)
- temp |= LVDS_HSYNC_POLARITY;
- if (adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC)
- temp |= LVDS_VSYNC_POLARITY;
- I915_WRITE(PCH_LVDS, temp);
- }
- }
-
- if (is_dp && !is_cpu_edp) {
+ if (is_dp && !is_cpu_edp)
intel_dp_set_m_n(crtc, mode, adjusted_mode);
- } else {
- if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
- /* For non-DP output, clear any trans DP clock recovery
- * setting.*/
- I915_WRITE(TRANSDATA_M1(pipe), 0);
- I915_WRITE(TRANSDATA_N1(pipe), 0);
- I915_WRITE(TRANSDPLINK_M1(pipe), 0);
- I915_WRITE(TRANSDPLINK_N1(pipe), 0);
- }
- }
intel_crtc->lowfreq_avail = false;
- if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
- if (intel_crtc->pch_pll) {
- I915_WRITE(intel_crtc->pch_pll->pll_reg, dpll);
-
- /* Wait for the clocks to stabilize. */
- POSTING_READ(intel_crtc->pch_pll->pll_reg);
- udelay(150);
-
- /* The pixel multiplier can only be updated once the
- * DPLL is enabled and the clocks are stable.
- *
- * So write it again.
- */
- I915_WRITE(intel_crtc->pch_pll->pll_reg, dpll);
- }
-
- if (intel_crtc->pch_pll) {
- if (is_lvds && has_reduced_clock && i915_powersave) {
- I915_WRITE(intel_crtc->pch_pll->fp1_reg, fp2);
- intel_crtc->lowfreq_avail = true;
- } else {
- I915_WRITE(intel_crtc->pch_pll->fp1_reg, fp);
- }
- }
- }
intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
if (!is_dp || is_cpu_edp)
ironlake_set_m_n(crtc, mode, adjusted_mode);
- if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
- if (is_cpu_edp)
- ironlake_set_pll_edp(crtc, adjusted_mode->clock);
-
haswell_set_pipeconf(crtc, adjusted_mode, dither);
/* Set up the display plane register */
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 02/10] drm/i915: move set_pll_edp to intel_dp.c
2012-11-29 14:59 [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Daniel Vetter
@ 2012-11-29 14:59 ` Daniel Vetter
2012-11-29 14:59 ` [PATCH 03/10] drm/i915: rip out pre-production ilk cpu edp w/a Daniel Vetter
` (8 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2012-11-29 14:59 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
Now that we enable the cpu edp pll in intel_dp->pre_enable and no
longer in crtc_mode_set, we can also move the modeset part to the
intel_dp->mode_set callback. Previously this was not possible because
the encoder ->mode_set callbacks are called after the crtc mode set
callback.
v2: Rebase on top of copy&pasted hsw crtc_mode_set.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 40 ------------------------------------
drivers/gpu/drm/i915/intel_dp.c | 40 ++++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fecaabc..0c44421 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2293,43 +2293,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
return 0;
}
-static void ironlake_set_pll_edp(struct drm_crtc *crtc, int clock)
-{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- u32 dpa_ctl;
-
- DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", clock);
- dpa_ctl = I915_READ(DP_A);
- dpa_ctl &= ~DP_PLL_FREQ_MASK;
-
- if (clock < 200000) {
- u32 temp;
- dpa_ctl |= DP_PLL_FREQ_160MHZ;
- /* workaround for 160Mhz:
- 1) program 0x4600c bits 15:0 = 0x8124
- 2) program 0x46010 bit 0 = 1
- 3) program 0x46034 bit 24 = 1
- 4) program 0x64000 bit 14 = 1
- */
- temp = I915_READ(0x4600c);
- temp &= 0xffff0000;
- I915_WRITE(0x4600c, temp | 0x8124);
-
- temp = I915_READ(0x46010);
- I915_WRITE(0x46010, temp | 1);
-
- temp = I915_READ(0x46034);
- I915_WRITE(0x46034, temp | (1 << 24));
- } else {
- dpa_ctl |= DP_PLL_FREQ_270MHZ;
- }
- I915_WRITE(DP_A, dpa_ctl);
-
- POSTING_READ(DP_A);
- udelay(500);
-}
-
static void intel_fdi_normal_train(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
@@ -5429,9 +5392,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
- if (is_cpu_edp)
- ironlake_set_pll_edp(crtc, adjusted_mode->clock);
-
ironlake_set_pipeconf(crtc, adjusted_mode, dither);
intel_wait_for_vblank(dev, pipe);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d76258d..dc019af 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -851,6 +851,43 @@ void intel_dp_init_link_config(struct intel_dp *intel_dp)
}
}
+static void ironlake_set_pll_edp(struct drm_crtc *crtc, int clock)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 dpa_ctl;
+
+ DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", clock);
+ dpa_ctl = I915_READ(DP_A);
+ dpa_ctl &= ~DP_PLL_FREQ_MASK;
+
+ if (clock < 200000) {
+ u32 temp;
+ dpa_ctl |= DP_PLL_FREQ_160MHZ;
+ /* workaround for 160Mhz:
+ 1) program 0x4600c bits 15:0 = 0x8124
+ 2) program 0x46010 bit 0 = 1
+ 3) program 0x46034 bit 24 = 1
+ 4) program 0x64000 bit 14 = 1
+ */
+ temp = I915_READ(0x4600c);
+ temp &= 0xffff0000;
+ I915_WRITE(0x4600c, temp | 0x8124);
+
+ temp = I915_READ(0x46010);
+ I915_WRITE(0x46010, temp | 1);
+
+ temp = I915_READ(0x46034);
+ I915_WRITE(0x46034, temp | (1 << 24));
+ } else {
+ dpa_ctl |= DP_PLL_FREQ_270MHZ;
+ }
+ I915_WRITE(DP_A, dpa_ctl);
+
+ POSTING_READ(DP_A);
+ udelay(500);
+}
+
static void
intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
@@ -950,6 +987,9 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
} else {
intel_dp->DP |= DP_LINK_TRAIN_OFF_CPT;
}
+
+ if (is_cpu_edp(intel_dp))
+ ironlake_set_pll_edp(crtc, adjusted_mode->clock);
}
#define IDLE_ON_MASK (PP_ON | 0 | PP_SEQUENCE_MASK | 0 | PP_SEQUENCE_STATE_MASK)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 03/10] drm/i915: rip out pre-production ilk cpu edp w/a
2012-11-29 14:59 [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Daniel Vetter
2012-11-29 14:59 ` [PATCH 02/10] drm/i915: move set_pll_edp to intel_dp.c Daniel Vetter
@ 2012-11-29 14:59 ` Daniel Vetter
2012-11-29 14:59 ` [PATCH 04/10] drm/i915: use wait_for_vblank instead of msleep(17) Daniel Vetter
` (7 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2012-11-29 14:59 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
While reading docs I've noticed that this special workaround to select
the 1.6 GHz DP clock only applies to pre-production ilk machines.
Since the registers we're touching here are rather undocumented and
might be harmful on later chips, rip it out.
For the Bspec reference of this w/a look in "vol4g CPU Display
Registers [DevILK]", Section 4.1.7.1 "DP_A—DisplayPort A
Control Register", "DP_PLL_Frequency_Select".
v2: Keep a debug message as a hint in case something regresses.
Requested by Chris Wilson.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_dp.c | 21 +++++----------------
1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dc019af..ee03a45 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -862,26 +862,15 @@ static void ironlake_set_pll_edp(struct drm_crtc *crtc, int clock)
dpa_ctl &= ~DP_PLL_FREQ_MASK;
if (clock < 200000) {
- u32 temp;
+ /* For a long time we've carried around a ILK-DevA w/a for the
+ * 160MHz clock. If we're really unlucky, it's still required.
+ */
+ DRM_DEBUG_KMS("160MHz cpu eDP clock, might need ilk devA w/a\n");
dpa_ctl |= DP_PLL_FREQ_160MHZ;
- /* workaround for 160Mhz:
- 1) program 0x4600c bits 15:0 = 0x8124
- 2) program 0x46010 bit 0 = 1
- 3) program 0x46034 bit 24 = 1
- 4) program 0x64000 bit 14 = 1
- */
- temp = I915_READ(0x4600c);
- temp &= 0xffff0000;
- I915_WRITE(0x4600c, temp | 0x8124);
-
- temp = I915_READ(0x46010);
- I915_WRITE(0x46010, temp | 1);
-
- temp = I915_READ(0x46034);
- I915_WRITE(0x46034, temp | (1 << 24));
} else {
dpa_ctl |= DP_PLL_FREQ_270MHZ;
}
+
I915_WRITE(DP_A, dpa_ctl);
POSTING_READ(DP_A);
--
1.7.11.7
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 04/10] drm/i915: use wait_for_vblank instead of msleep(17)
2012-11-29 14:59 [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Daniel Vetter
2012-11-29 14:59 ` [PATCH 02/10] drm/i915: move set_pll_edp to intel_dp.c Daniel Vetter
2012-11-29 14:59 ` [PATCH 03/10] drm/i915: rip out pre-production ilk cpu edp w/a Daniel Vetter
@ 2012-11-29 14:59 ` Daniel Vetter
2012-11-29 14:59 ` [PATCH 05/10] drm/i915: WARN on !crtc in intel_dp_link_down Daniel Vetter
` (6 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2012-11-29 14:59 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
17 ms is eerily close to 60 Hz ^-1
Unfortunately this goes back to the original DP enabling for ilk, and
unfortunately does not come with a reason for it's existance attached.
Some closer inspection of the code and DP specs shows that we set the
idle link pattern before we disable the port. And it seems like that
the DP spec (or at least our hw) only switch to the idle pattern on
the next vblank. Hence a vblank wait at this spot makes _much_ more
sense than a really long wait.
v2: Rebase fixup.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_dp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ee03a45..b3754b0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1993,6 +1993,8 @@ intel_dp_link_down(struct intel_dp *intel_dp)
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc =
+ to_intel_crtc(intel_dig_port->base.base.crtc);
uint32_t DP = intel_dp->DP;
/*
@@ -2027,7 +2029,7 @@ intel_dp_link_down(struct intel_dp *intel_dp)
}
POSTING_READ(intel_dp->output_reg);
- msleep(17);
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
if (HAS_PCH_IBX(dev) &&
I915_READ(intel_dp->output_reg) & DP_PIPEB_SELECT) {
@@ -2059,7 +2061,7 @@ intel_dp_link_down(struct intel_dp *intel_dp)
POSTING_READ(intel_dp->output_reg);
msleep(50);
} else
- intel_wait_for_vblank(dev, to_intel_crtc(crtc)->pipe);
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
}
DP &= ~DP_AUDIO_OUTPUT_ENABLE;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 05/10] drm/i915: WARN on !crtc in intel_dp_link_down
2012-11-29 14:59 [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Daniel Vetter
` (2 preceding siblings ...)
2012-11-29 14:59 ` [PATCH 04/10] drm/i915: use wait_for_vblank instead of msleep(17) Daniel Vetter
@ 2012-11-29 14:59 ` Daniel Vetter
2012-11-29 14:59 ` [PATCH 06/10] drm/i915: drop unnecessary clearing of pch dp transcoder timings Daniel Vetter
` (5 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2012-11-29 14:59 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
This could have happened with the old crtc helper based modeset code,
but can't happen any longer with the new code.
Hence put in a WARN and adjust the comment. If no one hits this, we
can eventually remove it (like a few other such cases across our
code).
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_dp.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b3754b0..c308550 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2049,15 +2049,10 @@ intel_dp_link_down(struct intel_dp *intel_dp)
/* Changes to enable or select take place the vblank
* after being written.
*/
- if (crtc == NULL) {
- /* We can arrive here never having been attached
- * to a CRTC, for instance, due to inheriting
- * random state from the BIOS.
- *
- * If the pipe is not running, play safe and
- * wait for the clocks to stabilise before
- * continuing.
- */
+ if (WARN_ON(crtc == NULL)) {
+ /* We should never try to disable a port without a crtc
+ * attached. For paranoia keep the code around for a
+ * bit. */
POSTING_READ(intel_dp->output_reg);
msleep(50);
} else
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 06/10] drm/i915: drop unnecessary clearing of pch dp transcoder timings
2012-11-29 14:59 [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Daniel Vetter
` (3 preceding siblings ...)
2012-11-29 14:59 ` [PATCH 05/10] drm/i915: WARN on !crtc in intel_dp_link_down Daniel Vetter
@ 2012-11-29 14:59 ` Daniel Vetter
2012-12-03 15:45 ` Paulo Zanoni
2012-11-29 14:59 ` [PATCH 07/10] drm/i915: extract common link_m_n helpers Daniel Vetter
` (4 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2012-11-29 14:59 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
This has originally been added in
commit 8db9d77b1b14fd730561f64beea8c00e4478d7c5
Author: Zhenyu Wang <zhenyuw@linux.intel.com>
Date: Wed Apr 7 16:15:54 2010 +0800
drm/i915: Support for Cougarpoint PCH display pipeline
probably to combat issues with hw state left behind by the BIOS. And
indeed, I've checked out that specific revision, and there is no DP
support yet. So the pch dp transcoder won't be correctly disabled, and
that's important since it requires a rether special disable dance:
Just writing 0 to TRANS_DP_CTL won't cut it, since we need to select
the NONE port when disabling, too.
And indeed, things seem to still work, so let's just remove this.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0c44421..456acd6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5345,15 +5345,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
} else
intel_put_pch_pll(intel_crtc);
- if (is_dp && !is_cpu_edp) {
+ if (is_dp && !is_cpu_edp)
intel_dp_set_m_n(crtc, mode, adjusted_mode);
- } else {
- /* For non-DP output, clear any trans DP clock recovery setting.*/
- I915_WRITE(TRANSDATA_M1(pipe), 0);
- I915_WRITE(TRANSDATA_N1(pipe), 0);
- I915_WRITE(TRANSDPLINK_M1(pipe), 0);
- I915_WRITE(TRANSDPLINK_N1(pipe), 0);
- }
for_each_encoder_on_crtc(dev, crtc, encoder)
if (encoder->pre_pll_enable)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 07/10] drm/i915: extract common link_m_n helpers
2012-11-29 14:59 [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Daniel Vetter
` (4 preceding siblings ...)
2012-11-29 14:59 ` [PATCH 06/10] drm/i915: drop unnecessary clearing of pch dp transcoder timings Daniel Vetter
@ 2012-11-29 14:59 ` Daniel Vetter
2012-12-03 15:33 ` Paulo Zanoni
2012-11-29 14:59 ` [PATCH 08/10] drm/i915: don't call update_watermark in crtc_mode_set Daniel Vetter
` (3 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2012-11-29 14:59 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
Both the dp and fdi code use the exact same computations (ignore minor
differences in conversion between bits and bytes).
This makes it even more apparent that we have a _massive_ mess between
cpu transcoder/fdi link/pch transcoder and pch link settings. And also
that we have hilarious amounts of confusion between edp and dp
(despite that they're identical at a link level).
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_drv.h | 13 ++++++++++++
drivers/gpu/drm/i915/intel_display.c | 31 +++++++++-------------------
drivers/gpu/drm/i915/intel_dp.c | 39 +++---------------------------------
3 files changed, 26 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9be7efd..a98cff0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -101,6 +101,19 @@ struct intel_pch_pll {
};
#define I915_NUM_PLLS 2
+/* Used by dp and fdi links */
+struct intel_link_m_n {
+ uint32_t tu;
+ uint32_t gmch_m;
+ uint32_t gmch_n;
+ uint32_t link_m;
+ uint32_t link_n;
+};
+
+void intel_link_compute_m_n(int bpp, int nlanes,
+ int pixel_clock, int link_clock,
+ struct intel_link_m_n *m_n);
+
struct intel_ddi_plls {
int spll_refcount;
int wrpll1_refcount;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 456acd6..9e13636 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3954,16 +3954,8 @@ static int i830_get_display_clock_speed(struct drm_device *dev)
return 133000;
}
-struct fdi_m_n {
- u32 tu;
- u32 gmch_m;
- u32 gmch_n;
- u32 link_m;
- u32 link_n;
-};
-
static void
-fdi_reduce_ratio(u32 *num, u32 *den)
+intel_reduce_ratio(uint32_t *num, uint32_t *den)
{
while (*num > 0xffffff || *den > 0xffffff) {
*num >>= 1;
@@ -3971,20 +3963,18 @@ fdi_reduce_ratio(u32 *num, u32 *den)
}
}
-static void
-ironlake_compute_m_n(int bits_per_pixel, int nlanes, int pixel_clock,
- int link_clock, struct fdi_m_n *m_n)
+void
+intel_link_compute_m_n(int bits_per_pixel, int nlanes,
+ int pixel_clock, int link_clock,
+ struct intel_link_m_n *m_n)
{
- m_n->tu = 64; /* default size */
-
- /* BUG_ON(pixel_clock > INT_MAX / 36); */
+ m_n->tu = 64;
m_n->gmch_m = bits_per_pixel * pixel_clock;
m_n->gmch_n = link_clock * nlanes * 8;
- fdi_reduce_ratio(&m_n->gmch_m, &m_n->gmch_n);
-
+ intel_reduce_ratio(&m_n->gmch_m, &m_n->gmch_n);
m_n->link_m = pixel_clock;
m_n->link_n = link_clock;
- fdi_reduce_ratio(&m_n->link_m, &m_n->link_n);
+ intel_reduce_ratio(&m_n->link_m, &m_n->link_n);
}
static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
@@ -5098,7 +5088,7 @@ static void ironlake_set_m_n(struct drm_crtc *crtc,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
struct intel_encoder *intel_encoder, *edp_encoder = NULL;
- struct fdi_m_n m_n = {0};
+ struct intel_link_m_n m_n = {0};
int target_clock, pixel_multiplier, lane, link_bw;
bool is_dp = false, is_cpu_edp = false;
@@ -5156,8 +5146,7 @@ static void ironlake_set_m_n(struct drm_crtc *crtc,
if (pixel_multiplier > 1)
link_bw *= pixel_multiplier;
- ironlake_compute_m_n(intel_crtc->bpp, lane, target_clock, link_bw,
- &m_n);
+ intel_link_compute_m_n(intel_crtc->bpp, lane, target_clock, link_bw, &m_n);
I915_WRITE(PIPE_DATA_M1(cpu_transcoder), TU_SIZE(m_n.tu) | m_n.gmch_m);
I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c308550..c5bd555 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -742,39 +742,6 @@ intel_dp_mode_fixup(struct drm_encoder *encoder,
return false;
}
-struct intel_dp_m_n {
- uint32_t tu;
- uint32_t gmch_m;
- uint32_t gmch_n;
- uint32_t link_m;
- uint32_t link_n;
-};
-
-static void
-intel_reduce_ratio(uint32_t *num, uint32_t *den)
-{
- while (*num > 0xffffff || *den > 0xffffff) {
- *num >>= 1;
- *den >>= 1;
- }
-}
-
-static void
-intel_dp_compute_m_n(int bpp,
- int nlanes,
- int pixel_clock,
- int link_clock,
- struct intel_dp_m_n *m_n)
-{
- m_n->tu = 64;
- m_n->gmch_m = (pixel_clock * bpp) >> 3;
- m_n->gmch_n = link_clock * nlanes;
- intel_reduce_ratio(&m_n->gmch_m, &m_n->gmch_n);
- m_n->link_m = pixel_clock;
- m_n->link_n = link_clock;
- intel_reduce_ratio(&m_n->link_m, &m_n->link_n);
-}
-
void
intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
@@ -785,7 +752,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int lane_count = 4;
- struct intel_dp_m_n m_n;
+ struct intel_link_m_n m_n;
int pipe = intel_crtc->pipe;
enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
@@ -808,8 +775,8 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
* the number of bytes_per_pixel post-LUT, which we always
* set up for 8-bits of R/G/B, or 3 bytes total.
*/
- intel_dp_compute_m_n(intel_crtc->bpp, lane_count,
- mode->clock, adjusted_mode->clock, &m_n);
+ intel_link_compute_m_n(intel_crtc->bpp, lane_count,
+ mode->clock, adjusted_mode->clock, &m_n);
if (IS_HASWELL(dev)) {
I915_WRITE(PIPE_DATA_M1(cpu_transcoder),
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 08/10] drm/i915: don't call update_watermark in crtc_mode_set
2012-11-29 14:59 [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Daniel Vetter
` (5 preceding siblings ...)
2012-11-29 14:59 ` [PATCH 07/10] drm/i915: extract common link_m_n helpers Daniel Vetter
@ 2012-11-29 14:59 ` Daniel Vetter
2012-11-29 14:59 ` [PATCH 09/10] drm/i915: don't call update_watermarks from haswell enable/disable code Daniel Vetter
` (2 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2012-11-29 14:59 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
We already update watermarks at all the right places in the
crtc_enable/disable callbacks. And since ->mode_set doesn't change the
active state, nothing will have changed ...
v2: Rebase on top of latest haswell copy&pasta.
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 9e13636..bb42102 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4713,8 +4713,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
ret = intel_pipe_set_base(crtc, x, y, fb);
- intel_update_watermarks(dev);
-
return ret;
}
@@ -5384,8 +5382,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
ret = intel_pipe_set_base(crtc, x, y, fb);
- intel_update_watermarks(dev);
-
intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
return fdi_config_ok ? ret : -EINVAL;
@@ -5471,8 +5467,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
ret = intel_pipe_set_base(crtc, x, y, fb);
- intel_update_watermarks(dev);
-
intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
return ret;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 09/10] drm/i915: don't call update_watermarks from haswell enable/disable code
2012-11-29 14:59 [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Daniel Vetter
` (6 preceding siblings ...)
2012-11-29 14:59 ` [PATCH 08/10] drm/i915: don't call update_watermark in crtc_mode_set Daniel Vetter
@ 2012-11-29 14:59 ` Daniel Vetter
2012-11-29 14:59 ` [PATCH 10/10] drm/i915: rip out update_linetime_wm abstraction Daniel Vetter
2012-12-03 15:03 ` [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Paulo Zanoni
9 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2012-11-29 14:59 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
We don't use these on haswell, since haswell as per-pipe watermarks
and nothing global any more.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bb42102..0a45d07 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3357,7 +3357,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
return;
intel_crtc->active = true;
- intel_update_watermarks(dev);
is_pch_port = haswell_crtc_driving_pch(crtc);
@@ -3546,7 +3545,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
}
intel_crtc->active = false;
- intel_update_watermarks(dev);
mutex_lock(&dev->struct_mutex);
intel_update_fbc(dev);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 10/10] drm/i915: rip out update_linetime_wm abstraction
2012-11-29 14:59 [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Daniel Vetter
` (7 preceding siblings ...)
2012-11-29 14:59 ` [PATCH 09/10] drm/i915: don't call update_watermarks from haswell enable/disable code Daniel Vetter
@ 2012-11-29 14:59 ` Daniel Vetter
2012-12-04 17:34 ` Paulo Zanoni
2012-12-03 15:03 ` [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Paulo Zanoni
9 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2012-11-29 14:59 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
I like abstraction and vfuncs, but only if they actually abstract anything.
In this case here they just obfuscate, so let's rip this stuff out.
Aside: We really should move all the haswell stuff into it's own file ...
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_drv.h | 2 --
drivers/gpu/drm/i915/intel_display.c | 31 +++++++++++++++++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 2 --
drivers/gpu/drm/i915/intel_pm.c | 37 ------------------------------------
4 files changed, 28 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a98cff0..ac52c75 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -269,8 +269,6 @@ struct drm_i915_display_funcs {
void (*update_wm)(struct drm_device *dev);
void (*update_sprite_wm)(struct drm_device *dev, int pipe,
uint32_t sprite_width, int pixel_size);
- void (*update_linetime_wm)(struct drm_device *dev, int pipe,
- struct drm_display_mode *mode);
void (*modeset_global_resources)(struct drm_device *dev);
int (*crtc_mode_set)(struct drm_crtc *crtc,
struct drm_display_mode *mode,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0a45d07..0c7e88d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5380,11 +5380,36 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
ret = intel_pipe_set_base(crtc, x, y, fb);
- intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
-
return fdi_config_ok ? ret : -EINVAL;
}
+static void
+haswell_update_linetime_wm(struct drm_device *dev, int pipe,
+ struct drm_display_mode *mode)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 temp;
+
+ temp = I915_READ(PIPE_WM_LINETIME(pipe));
+ temp &= ~PIPE_WM_LINETIME_MASK;
+
+ /* The WM are computed with base on how long it takes to fill a single
+ * row at the given clock rate, multiplied by 8.
+ * */
+ temp |= PIPE_WM_LINETIME_TIME(
+ ((mode->crtc_hdisplay * 1000) / mode->clock) * 8);
+
+ /* IPS watermarks are only used by pipe A, and are ignored by
+ * pipes B and C. They are calculated similarly to the common
+ * linetime values, except that we are using CD clock frequency
+ * in MHz instead of pixel rate for the division.
+ *
+ * This is a placeholder for the IPS watermark calculation code.
+ */
+
+ I915_WRITE(PIPE_WM_LINETIME(pipe), temp);
+}
+
static int haswell_crtc_mode_set(struct drm_crtc *crtc,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode,
@@ -5465,7 +5490,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
ret = intel_pipe_set_base(crtc, x, y, fb);
- intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
+ haswell_update_linetime_wm(dev, pipe, adjusted_mode);
return ret;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7ca7772..824efde 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -622,8 +622,6 @@ extern void intel_update_watermarks(struct drm_device *dev);
extern void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
uint32_t sprite_width,
int pixel_size);
-extern void intel_update_linetime_watermarks(struct drm_device *dev, int pipe,
- struct drm_display_mode *mode);
extern unsigned long intel_gen4_compute_offset_xtiled(int *x, int *y,
unsigned int bpp,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f595b8d..a487d15 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1890,33 +1890,6 @@ static void sandybridge_update_wm(struct drm_device *dev)
cursor_wm);
}
-static void
-haswell_update_linetime_wm(struct drm_device *dev, int pipe,
- struct drm_display_mode *mode)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- u32 temp;
-
- temp = I915_READ(PIPE_WM_LINETIME(pipe));
- temp &= ~PIPE_WM_LINETIME_MASK;
-
- /* The WM are computed with base on how long it takes to fill a single
- * row at the given clock rate, multiplied by 8.
- * */
- temp |= PIPE_WM_LINETIME_TIME(
- ((mode->crtc_hdisplay * 1000) / mode->clock) * 8);
-
- /* IPS watermarks are only used by pipe A, and are ignored by
- * pipes B and C. They are calculated similarly to the common
- * linetime values, except that we are using CD clock frequency
- * in MHz instead of pixel rate for the division.
- *
- * This is a placeholder for the IPS watermark calculation code.
- */
-
- I915_WRITE(PIPE_WM_LINETIME(pipe), temp);
-}
-
static bool
sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
uint32_t sprite_width, int pixel_size,
@@ -2112,15 +2085,6 @@ void intel_update_watermarks(struct drm_device *dev)
dev_priv->display.update_wm(dev);
}
-void intel_update_linetime_watermarks(struct drm_device *dev,
- int pipe, struct drm_display_mode *mode)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- if (dev_priv->display.update_linetime_wm)
- dev_priv->display.update_linetime_wm(dev, pipe, mode);
-}
-
void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
uint32_t sprite_width, int pixel_size)
{
@@ -4011,7 +3975,6 @@ void intel_init_pm(struct drm_device *dev)
if (SNB_READ_WM0_LATENCY()) {
dev_priv->display.update_wm = sandybridge_update_wm;
dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
- dev_priv->display.update_linetime_wm = haswell_update_linetime_wm;
} else {
DRM_DEBUG_KMS("Failed to read display plane latency. "
"Disable CxSR\n");
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set
2012-11-29 14:59 [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Daniel Vetter
` (8 preceding siblings ...)
2012-11-29 14:59 ` [PATCH 10/10] drm/i915: rip out update_linetime_wm abstraction Daniel Vetter
@ 2012-12-03 15:03 ` Paulo Zanoni
2012-12-03 16:42 ` Daniel Vetter
9 siblings, 1 reply; 18+ messages in thread
From: Paulo Zanoni @ 2012-12-03 15:03 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development
Hi
2012/11/29 Daniel Vetter <daniel.vetter@ffwll.ch>:
> Especially getting rid of all things lvds is ... great!
>
> v2: Drop the two additional pre-hsw hunks noticed by Paulo Zanoni.
>
> v3:
> - handle DP ports correctly (spoted by Paulo)
> - don't leave {} behind for a single-line block (again spotted by
> Paulo)
> - kill another if (IBX || CPT) block
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 131 +----------------------------------
> 1 file changed, 2 insertions(+), 129 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3b6e4b0..fecaabc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5343,9 +5343,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> num_connectors++;
> }
>
> - WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)),
> - "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev));
> -
This is patching ironlake_crtc_mode_set, instead of
haswell_crtc_mode_set, which still includes the following lines:
/* We are not sure yet this won't happen. */
WARN(!HAS_PCH_LPT(dev), "Unexpected PCH type %d\n",
INTEL_PCH_TYPE(dev));
Patch still looks correct, so with or without that fixed:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
> &has_reduced_clock, &reduced_clock);
> if (!ok) {
> @@ -5464,20 +5461,13 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> int pipe = intel_crtc->pipe;
> int plane = intel_crtc->plane;
> int num_connectors = 0;
> - intel_clock_t clock, reduced_clock;
> - u32 dpll = 0, fp = 0, fp2 = 0;
> - bool ok, has_reduced_clock = false;
> - bool is_lvds = false, is_dp = false, is_cpu_edp = false;
> + bool is_dp = false, is_cpu_edp = false;
> struct intel_encoder *encoder;
> - u32 temp;
> int ret;
> bool dither;
>
> for_each_encoder_on_crtc(dev, crtc, encoder) {
> switch (encoder->type) {
> - case INTEL_OUTPUT_LVDS:
> - is_lvds = true;
> - break;
> case INTEL_OUTPUT_DISPLAYPORT:
> is_dp = true;
> break;
> @@ -5511,143 +5501,26 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> if (!intel_ddi_pll_mode_set(crtc, adjusted_mode->clock))
> return -EINVAL;
>
> - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
> - ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
> - &has_reduced_clock,
> - &reduced_clock);
> - if (!ok) {
> - DRM_ERROR("Couldn't find PLL settings for mode!\n");
> - return -EINVAL;
> - }
> - }
> -
> /* Ensure that the cursor is valid for the new mode before changing... */
> intel_crtc_update_cursor(crtc, true);
>
> /* determine panel color depth */
> dither = intel_choose_pipe_bpp_dither(crtc, fb, &intel_crtc->bpp,
> adjusted_mode);
> - if (is_lvds && dev_priv->lvds_dither)
> - dither = true;
>
> DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
> drm_mode_debug_printmodeline(mode);
>
> - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
> - fp = clock.n << 16 | clock.m1 << 8 | clock.m2;
> - if (has_reduced_clock)
> - fp2 = reduced_clock.n << 16 | reduced_clock.m1 << 8 |
> - reduced_clock.m2;
> -
> - dpll = ironlake_compute_dpll(intel_crtc, adjusted_mode, &clock,
> - fp);
> -
> - /* CPU eDP is the only output that doesn't need a PCH PLL of its
> - * own on pre-Haswell/LPT generation */
> - if (!is_cpu_edp) {
> - struct intel_pch_pll *pll;
> -
> - pll = intel_get_pch_pll(intel_crtc, dpll, fp);
> - if (pll == NULL) {
> - DRM_DEBUG_DRIVER("failed to find PLL for pipe %d\n",
> - pipe);
> - return -EINVAL;
> - }
> - } else
> - intel_put_pch_pll(intel_crtc);
> -
> - /* The LVDS pin pair needs to be on before the DPLLs are
> - * enabled. This is an exception to the general rule that
> - * mode_set doesn't turn things on.
> - */
> - if (is_lvds) {
> - temp = I915_READ(PCH_LVDS);
> - temp |= LVDS_PORT_EN | LVDS_A0A2_CLKA_POWER_UP;
> - if (HAS_PCH_CPT(dev)) {
> - temp &= ~PORT_TRANS_SEL_MASK;
> - temp |= PORT_TRANS_SEL_CPT(pipe);
> - } else {
> - if (pipe == 1)
> - temp |= LVDS_PIPEB_SELECT;
> - else
> - temp &= ~LVDS_PIPEB_SELECT;
> - }
> -
> - /* set the corresponsding LVDS_BORDER bit */
> - temp |= dev_priv->lvds_border_bits;
> - /* Set the B0-B3 data pairs corresponding to whether
> - * we're going to set the DPLLs for dual-channel mode or
> - * not.
> - */
> - if (clock.p2 == 7)
> - temp |= LVDS_B0B3_POWER_UP | LVDS_CLKB_POWER_UP;
> - else
> - temp &= ~(LVDS_B0B3_POWER_UP |
> - LVDS_CLKB_POWER_UP);
> -
> - /* It would be nice to set 24 vs 18-bit mode
> - * (LVDS_A3_POWER_UP) appropriately here, but we need to
> - * look more thoroughly into how panels behave in the
> - * two modes.
> - */
> - temp &= ~(LVDS_HSYNC_POLARITY | LVDS_VSYNC_POLARITY);
> - if (adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC)
> - temp |= LVDS_HSYNC_POLARITY;
> - if (adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC)
> - temp |= LVDS_VSYNC_POLARITY;
> - I915_WRITE(PCH_LVDS, temp);
> - }
> - }
> -
> - if (is_dp && !is_cpu_edp) {
> + if (is_dp && !is_cpu_edp)
> intel_dp_set_m_n(crtc, mode, adjusted_mode);
> - } else {
> - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
> - /* For non-DP output, clear any trans DP clock recovery
> - * setting.*/
> - I915_WRITE(TRANSDATA_M1(pipe), 0);
> - I915_WRITE(TRANSDATA_N1(pipe), 0);
> - I915_WRITE(TRANSDPLINK_M1(pipe), 0);
> - I915_WRITE(TRANSDPLINK_N1(pipe), 0);
> - }
> - }
>
> intel_crtc->lowfreq_avail = false;
> - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
> - if (intel_crtc->pch_pll) {
> - I915_WRITE(intel_crtc->pch_pll->pll_reg, dpll);
> -
> - /* Wait for the clocks to stabilize. */
> - POSTING_READ(intel_crtc->pch_pll->pll_reg);
> - udelay(150);
> -
> - /* The pixel multiplier can only be updated once the
> - * DPLL is enabled and the clocks are stable.
> - *
> - * So write it again.
> - */
> - I915_WRITE(intel_crtc->pch_pll->pll_reg, dpll);
> - }
> -
> - if (intel_crtc->pch_pll) {
> - if (is_lvds && has_reduced_clock && i915_powersave) {
> - I915_WRITE(intel_crtc->pch_pll->fp1_reg, fp2);
> - intel_crtc->lowfreq_avail = true;
> - } else {
> - I915_WRITE(intel_crtc->pch_pll->fp1_reg, fp);
> - }
> - }
> - }
>
> intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
>
> if (!is_dp || is_cpu_edp)
> ironlake_set_m_n(crtc, mode, adjusted_mode);
>
> - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
> - if (is_cpu_edp)
> - ironlake_set_pll_edp(crtc, adjusted_mode->clock);
> -
> haswell_set_pipeconf(crtc, adjusted_mode, dither);
>
> /* Set up the display plane register */
> --
> 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] 18+ messages in thread
* Re: [PATCH 07/10] drm/i915: extract common link_m_n helpers
2012-11-29 14:59 ` [PATCH 07/10] drm/i915: extract common link_m_n helpers Daniel Vetter
@ 2012-12-03 15:33 ` Paulo Zanoni
2012-12-06 13:33 ` Daniel Vetter
0 siblings, 1 reply; 18+ messages in thread
From: Paulo Zanoni @ 2012-12-03 15:33 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development
2012/11/29 Daniel Vetter <daniel.vetter@ffwll.ch>:
> Both the dp and fdi code use the exact same computations (ignore minor
> differences in conversion between bits and bytes).
>
> This makes it even more apparent that we have a _massive_ mess between
> cpu transcoder/fdi link/pch transcoder and pch link settings. And also
> that we have hilarious amounts of confusion between edp and dp
> (despite that they're identical at a link level).
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 13 ++++++++++++
> drivers/gpu/drm/i915/intel_display.c | 31 +++++++++-------------------
> drivers/gpu/drm/i915/intel_dp.c | 39 +++---------------------------------
> 3 files changed, 26 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9be7efd..a98cff0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -101,6 +101,19 @@ struct intel_pch_pll {
> };
> #define I915_NUM_PLLS 2
>
> +/* Used by dp and fdi links */
> +struct intel_link_m_n {
> + uint32_t tu;
> + uint32_t gmch_m;
> + uint32_t gmch_n;
> + uint32_t link_m;
> + uint32_t link_n;
> +};
> +
> +void intel_link_compute_m_n(int bpp, int nlanes,
> + int pixel_clock, int link_clock,
> + struct intel_link_m_n *m_n);
> +
> struct intel_ddi_plls {
> int spll_refcount;
> int wrpll1_refcount;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 456acd6..9e13636 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3954,16 +3954,8 @@ static int i830_get_display_clock_speed(struct drm_device *dev)
> return 133000;
> }
>
> -struct fdi_m_n {
> - u32 tu;
> - u32 gmch_m;
> - u32 gmch_n;
> - u32 link_m;
> - u32 link_n;
> -};
> -
> static void
> -fdi_reduce_ratio(u32 *num, u32 *den)
> +intel_reduce_ratio(uint32_t *num, uint32_t *den)
> {
> while (*num > 0xffffff || *den > 0xffffff) {
> *num >>= 1;
> @@ -3971,20 +3963,18 @@ fdi_reduce_ratio(u32 *num, u32 *den)
> }
> }
>
> -static void
> -ironlake_compute_m_n(int bits_per_pixel, int nlanes, int pixel_clock,
> - int link_clock, struct fdi_m_n *m_n)
> +void
> +intel_link_compute_m_n(int bits_per_pixel, int nlanes,
> + int pixel_clock, int link_clock,
> + struct intel_link_m_n *m_n)
> {
> - m_n->tu = 64; /* default size */
> -
> - /* BUG_ON(pixel_clock > INT_MAX / 36); */
> + m_n->tu = 64;
> m_n->gmch_m = bits_per_pixel * pixel_clock;
> m_n->gmch_n = link_clock * nlanes * 8;
> - fdi_reduce_ratio(&m_n->gmch_m, &m_n->gmch_n);
> -
> + intel_reduce_ratio(&m_n->gmch_m, &m_n->gmch_n);
> m_n->link_m = pixel_clock;
> m_n->link_n = link_clock;
> - fdi_reduce_ratio(&m_n->link_m, &m_n->link_n);
> + intel_reduce_ratio(&m_n->link_m, &m_n->link_n);
> }
>
> static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
> @@ -5098,7 +5088,7 @@ static void ironlake_set_m_n(struct drm_crtc *crtc,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> struct intel_encoder *intel_encoder, *edp_encoder = NULL;
> - struct fdi_m_n m_n = {0};
> + struct intel_link_m_n m_n = {0};
> int target_clock, pixel_multiplier, lane, link_bw;
> bool is_dp = false, is_cpu_edp = false;
>
> @@ -5156,8 +5146,7 @@ static void ironlake_set_m_n(struct drm_crtc *crtc,
>
> if (pixel_multiplier > 1)
> link_bw *= pixel_multiplier;
> - ironlake_compute_m_n(intel_crtc->bpp, lane, target_clock, link_bw,
> - &m_n);
> + intel_link_compute_m_n(intel_crtc->bpp, lane, target_clock, link_bw, &m_n);
>
> I915_WRITE(PIPE_DATA_M1(cpu_transcoder), TU_SIZE(m_n.tu) | m_n.gmch_m);
> I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c308550..c5bd555 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -742,39 +742,6 @@ intel_dp_mode_fixup(struct drm_encoder *encoder,
> return false;
> }
>
> -struct intel_dp_m_n {
> - uint32_t tu;
> - uint32_t gmch_m;
> - uint32_t gmch_n;
> - uint32_t link_m;
> - uint32_t link_n;
> -};
> -
> -static void
> -intel_reduce_ratio(uint32_t *num, uint32_t *den)
> -{
> - while (*num > 0xffffff || *den > 0xffffff) {
> - *num >>= 1;
> - *den >>= 1;
> - }
> -}
> -
> -static void
> -intel_dp_compute_m_n(int bpp,
> - int nlanes,
> - int pixel_clock,
> - int link_clock,
> - struct intel_dp_m_n *m_n)
> -{
> - m_n->tu = 64;
> - m_n->gmch_m = (pixel_clock * bpp) >> 3;
> - m_n->gmch_n = link_clock * nlanes;
> - intel_reduce_ratio(&m_n->gmch_m, &m_n->gmch_n);
> - m_n->link_m = pixel_clock;
> - m_n->link_n = link_clock;
> - intel_reduce_ratio(&m_n->link_m, &m_n->link_n);
> -}
> -
> void
> intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
> struct drm_display_mode *adjusted_mode)
> @@ -785,7 +752,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> int lane_count = 4;
> - struct intel_dp_m_n m_n;
> + struct intel_link_m_n m_n;
> int pipe = intel_crtc->pipe;
> enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>
> @@ -808,8 +775,8 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
> * the number of bytes_per_pixel post-LUT, which we always
> * set up for 8-bits of R/G/B, or 3 bytes total.
> */
> - intel_dp_compute_m_n(intel_crtc->bpp, lane_count,
> - mode->clock, adjusted_mode->clock, &m_n);
> + intel_link_compute_m_n(intel_crtc->bpp, lane_count,
> + mode->clock, adjusted_mode->clock, &m_n);
>
> if (IS_HASWELL(dev)) {
> I915_WRITE(PIPE_DATA_M1(cpu_transcoder),
> --
> 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] 18+ messages in thread
* Re: [PATCH 06/10] drm/i915: drop unnecessary clearing of pch dp transcoder timings
2012-11-29 14:59 ` [PATCH 06/10] drm/i915: drop unnecessary clearing of pch dp transcoder timings Daniel Vetter
@ 2012-12-03 15:45 ` Paulo Zanoni
0 siblings, 0 replies; 18+ messages in thread
From: Paulo Zanoni @ 2012-12-03 15:45 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development
Hi
2012/11/29 Daniel Vetter <daniel.vetter@ffwll.ch>:
> This has originally been added in
>
> commit 8db9d77b1b14fd730561f64beea8c00e4478d7c5
> Author: Zhenyu Wang <zhenyuw@linux.intel.com>
> Date: Wed Apr 7 16:15:54 2010 +0800
>
> drm/i915: Support for Cougarpoint PCH display pipeline
>
> probably to combat issues with hw state left behind by the BIOS. And
> indeed, I've checked out that specific revision, and there is no DP
> support yet. So the pch dp transcoder won't be correctly disabled, and
> that's important since it requires a rether special disable dance:
> Just writing 0 to TRANS_DP_CTL won't cut it, since we need to select
> the NONE port when disabling, too.
>
> And indeed, things seem to still work, so let's just remove this.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Briefly tested on SNB:
- Connected DP, disconnected
- Connected HDMI, disconnected
- Connected VGA, disconnected
They all worked.
Briefly-tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0c44421..456acd6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5345,15 +5345,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> } else
> intel_put_pch_pll(intel_crtc);
>
> - if (is_dp && !is_cpu_edp) {
> + if (is_dp && !is_cpu_edp)
> intel_dp_set_m_n(crtc, mode, adjusted_mode);
> - } else {
> - /* For non-DP output, clear any trans DP clock recovery setting.*/
> - I915_WRITE(TRANSDATA_M1(pipe), 0);
> - I915_WRITE(TRANSDATA_N1(pipe), 0);
> - I915_WRITE(TRANSDPLINK_M1(pipe), 0);
> - I915_WRITE(TRANSDPLINK_N1(pipe), 0);
> - }
>
> for_each_encoder_on_crtc(dev, crtc, encoder)
> if (encoder->pre_pll_enable)
> --
> 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] 18+ messages in thread
* Re: [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set
2012-12-03 15:03 ` [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Paulo Zanoni
@ 2012-12-03 16:42 ` Daniel Vetter
2012-12-06 13:25 ` Daniel Vetter
0 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2012-12-03 16:42 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Daniel Vetter, Intel Graphics Development
On Mon, Dec 03, 2012 at 01:03:14PM -0200, Paulo Zanoni wrote:
> Hi
>
> 2012/11/29 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > Especially getting rid of all things lvds is ... great!
> >
> > v2: Drop the two additional pre-hsw hunks noticed by Paulo Zanoni.
> >
> > v3:
> > - handle DP ports correctly (spoted by Paulo)
> > - don't leave {} behind for a single-line block (again spotted by
> > Paulo)
> > - kill another if (IBX || CPT) block
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 131 +----------------------------------
> > 1 file changed, 2 insertions(+), 129 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 3b6e4b0..fecaabc 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5343,9 +5343,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> > num_connectors++;
> > }
> >
> > - WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)),
> > - "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev));
> > -
>
> This is patching ironlake_crtc_mode_set, instead of
> haswell_crtc_mode_set, which still includes the following lines:
>
> /* We are not sure yet this won't happen. */
> WARN(!HAS_PCH_LPT(dev), "Unexpected PCH type %d\n",
> INTEL_PCH_TYPE(dev));
Hm, can I blame git rebase on this one? Probably not ;-) I'll fix this
when applying, and we really need to split this stuff up a bit - the
current code is way too confuse, with too many lookalikes all around.
Thanks for catching this.
-Daniel
>
>
> Patch still looks correct, so with or without that fixed:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> > ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
> > &has_reduced_clock, &reduced_clock);
> > if (!ok) {
> > @@ -5464,20 +5461,13 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> > int pipe = intel_crtc->pipe;
> > int plane = intel_crtc->plane;
> > int num_connectors = 0;
> > - intel_clock_t clock, reduced_clock;
> > - u32 dpll = 0, fp = 0, fp2 = 0;
> > - bool ok, has_reduced_clock = false;
> > - bool is_lvds = false, is_dp = false, is_cpu_edp = false;
> > + bool is_dp = false, is_cpu_edp = false;
> > struct intel_encoder *encoder;
> > - u32 temp;
> > int ret;
> > bool dither;
> >
> > for_each_encoder_on_crtc(dev, crtc, encoder) {
> > switch (encoder->type) {
> > - case INTEL_OUTPUT_LVDS:
> > - is_lvds = true;
> > - break;
> > case INTEL_OUTPUT_DISPLAYPORT:
> > is_dp = true;
> > break;
> > @@ -5511,143 +5501,26 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> > if (!intel_ddi_pll_mode_set(crtc, adjusted_mode->clock))
> > return -EINVAL;
> >
> > - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
> > - ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
> > - &has_reduced_clock,
> > - &reduced_clock);
> > - if (!ok) {
> > - DRM_ERROR("Couldn't find PLL settings for mode!\n");
> > - return -EINVAL;
> > - }
> > - }
> > -
> > /* Ensure that the cursor is valid for the new mode before changing... */
> > intel_crtc_update_cursor(crtc, true);
> >
> > /* determine panel color depth */
> > dither = intel_choose_pipe_bpp_dither(crtc, fb, &intel_crtc->bpp,
> > adjusted_mode);
> > - if (is_lvds && dev_priv->lvds_dither)
> > - dither = true;
> >
> > DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
> > drm_mode_debug_printmodeline(mode);
> >
> > - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
> > - fp = clock.n << 16 | clock.m1 << 8 | clock.m2;
> > - if (has_reduced_clock)
> > - fp2 = reduced_clock.n << 16 | reduced_clock.m1 << 8 |
> > - reduced_clock.m2;
> > -
> > - dpll = ironlake_compute_dpll(intel_crtc, adjusted_mode, &clock,
> > - fp);
> > -
> > - /* CPU eDP is the only output that doesn't need a PCH PLL of its
> > - * own on pre-Haswell/LPT generation */
> > - if (!is_cpu_edp) {
> > - struct intel_pch_pll *pll;
> > -
> > - pll = intel_get_pch_pll(intel_crtc, dpll, fp);
> > - if (pll == NULL) {
> > - DRM_DEBUG_DRIVER("failed to find PLL for pipe %d\n",
> > - pipe);
> > - return -EINVAL;
> > - }
> > - } else
> > - intel_put_pch_pll(intel_crtc);
> > -
> > - /* The LVDS pin pair needs to be on before the DPLLs are
> > - * enabled. This is an exception to the general rule that
> > - * mode_set doesn't turn things on.
> > - */
> > - if (is_lvds) {
> > - temp = I915_READ(PCH_LVDS);
> > - temp |= LVDS_PORT_EN | LVDS_A0A2_CLKA_POWER_UP;
> > - if (HAS_PCH_CPT(dev)) {
> > - temp &= ~PORT_TRANS_SEL_MASK;
> > - temp |= PORT_TRANS_SEL_CPT(pipe);
> > - } else {
> > - if (pipe == 1)
> > - temp |= LVDS_PIPEB_SELECT;
> > - else
> > - temp &= ~LVDS_PIPEB_SELECT;
> > - }
> > -
> > - /* set the corresponsding LVDS_BORDER bit */
> > - temp |= dev_priv->lvds_border_bits;
> > - /* Set the B0-B3 data pairs corresponding to whether
> > - * we're going to set the DPLLs for dual-channel mode or
> > - * not.
> > - */
> > - if (clock.p2 == 7)
> > - temp |= LVDS_B0B3_POWER_UP | LVDS_CLKB_POWER_UP;
> > - else
> > - temp &= ~(LVDS_B0B3_POWER_UP |
> > - LVDS_CLKB_POWER_UP);
> > -
> > - /* It would be nice to set 24 vs 18-bit mode
> > - * (LVDS_A3_POWER_UP) appropriately here, but we need to
> > - * look more thoroughly into how panels behave in the
> > - * two modes.
> > - */
> > - temp &= ~(LVDS_HSYNC_POLARITY | LVDS_VSYNC_POLARITY);
> > - if (adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC)
> > - temp |= LVDS_HSYNC_POLARITY;
> > - if (adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC)
> > - temp |= LVDS_VSYNC_POLARITY;
> > - I915_WRITE(PCH_LVDS, temp);
> > - }
> > - }
> > -
> > - if (is_dp && !is_cpu_edp) {
> > + if (is_dp && !is_cpu_edp)
> > intel_dp_set_m_n(crtc, mode, adjusted_mode);
> > - } else {
> > - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
> > - /* For non-DP output, clear any trans DP clock recovery
> > - * setting.*/
> > - I915_WRITE(TRANSDATA_M1(pipe), 0);
> > - I915_WRITE(TRANSDATA_N1(pipe), 0);
> > - I915_WRITE(TRANSDPLINK_M1(pipe), 0);
> > - I915_WRITE(TRANSDPLINK_N1(pipe), 0);
> > - }
> > - }
> >
> > intel_crtc->lowfreq_avail = false;
> > - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
> > - if (intel_crtc->pch_pll) {
> > - I915_WRITE(intel_crtc->pch_pll->pll_reg, dpll);
> > -
> > - /* Wait for the clocks to stabilize. */
> > - POSTING_READ(intel_crtc->pch_pll->pll_reg);
> > - udelay(150);
> > -
> > - /* The pixel multiplier can only be updated once the
> > - * DPLL is enabled and the clocks are stable.
> > - *
> > - * So write it again.
> > - */
> > - I915_WRITE(intel_crtc->pch_pll->pll_reg, dpll);
> > - }
> > -
> > - if (intel_crtc->pch_pll) {
> > - if (is_lvds && has_reduced_clock && i915_powersave) {
> > - I915_WRITE(intel_crtc->pch_pll->fp1_reg, fp2);
> > - intel_crtc->lowfreq_avail = true;
> > - } else {
> > - I915_WRITE(intel_crtc->pch_pll->fp1_reg, fp);
> > - }
> > - }
> > - }
> >
> > intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
> >
> > if (!is_dp || is_cpu_edp)
> > ironlake_set_m_n(crtc, mode, adjusted_mode);
> >
> > - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
> > - if (is_cpu_edp)
> > - ironlake_set_pll_edp(crtc, adjusted_mode->clock);
> > -
> > haswell_set_pipeconf(crtc, adjusted_mode, dither);
> >
> > /* Set up the display plane register */
> > --
> > 1.7.11.7
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 10/10] drm/i915: rip out update_linetime_wm abstraction
2012-11-29 14:59 ` [PATCH 10/10] drm/i915: rip out update_linetime_wm abstraction Daniel Vetter
@ 2012-12-04 17:34 ` Paulo Zanoni
2012-12-05 11:03 ` Daniel Vetter
0 siblings, 1 reply; 18+ messages in thread
From: Paulo Zanoni @ 2012-12-04 17:34 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development
Hi
2012/11/29 Daniel Vetter <daniel.vetter@ffwll.ch>:
> I like abstraction and vfuncs, but only if they actually abstract anything.
> In this case here they just obfuscate, so let's rip this stuff out.
>
> Aside: We really should move all the haswell stuff into it's own file ...
Well, a few Kernels ago we did a nice effort to move a lot of code
from intel_display.c to intel_pm.c. Now we're bringing it back... I'd
like to understand what are the current plans regarding this.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 --
> drivers/gpu/drm/i915/intel_display.c | 31 +++++++++++++++++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 2 --
> drivers/gpu/drm/i915/intel_pm.c | 37 ------------------------------------
> 4 files changed, 28 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a98cff0..ac52c75 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -269,8 +269,6 @@ struct drm_i915_display_funcs {
> void (*update_wm)(struct drm_device *dev);
> void (*update_sprite_wm)(struct drm_device *dev, int pipe,
> uint32_t sprite_width, int pixel_size);
> - void (*update_linetime_wm)(struct drm_device *dev, int pipe,
> - struct drm_display_mode *mode);
> void (*modeset_global_resources)(struct drm_device *dev);
> int (*crtc_mode_set)(struct drm_crtc *crtc,
> struct drm_display_mode *mode,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0a45d07..0c7e88d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5380,11 +5380,36 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>
> ret = intel_pipe_set_base(crtc, x, y, fb);
>
> - intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
> -
> return fdi_config_ok ? ret : -EINVAL;
> }
>
> +static void
> +haswell_update_linetime_wm(struct drm_device *dev, int pipe,
> + struct drm_display_mode *mode)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 temp;
> +
> + temp = I915_READ(PIPE_WM_LINETIME(pipe));
> + temp &= ~PIPE_WM_LINETIME_MASK;
> +
> + /* The WM are computed with base on how long it takes to fill a single
> + * row at the given clock rate, multiplied by 8.
> + * */
> + temp |= PIPE_WM_LINETIME_TIME(
> + ((mode->crtc_hdisplay * 1000) / mode->clock) * 8);
> +
> + /* IPS watermarks are only used by pipe A, and are ignored by
> + * pipes B and C. They are calculated similarly to the common
> + * linetime values, except that we are using CD clock frequency
> + * in MHz instead of pixel rate for the division.
> + *
> + * This is a placeholder for the IPS watermark calculation code.
> + */
> +
> + I915_WRITE(PIPE_WM_LINETIME(pipe), temp);
> +}
> +
> static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> struct drm_display_mode *mode,
> struct drm_display_mode *adjusted_mode,
> @@ -5465,7 +5490,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>
> ret = intel_pipe_set_base(crtc, x, y, fb);
>
> - intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
> + haswell_update_linetime_wm(dev, pipe, adjusted_mode);
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 7ca7772..824efde 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -622,8 +622,6 @@ extern void intel_update_watermarks(struct drm_device *dev);
> extern void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
> uint32_t sprite_width,
> int pixel_size);
> -extern void intel_update_linetime_watermarks(struct drm_device *dev, int pipe,
> - struct drm_display_mode *mode);
>
> extern unsigned long intel_gen4_compute_offset_xtiled(int *x, int *y,
> unsigned int bpp,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f595b8d..a487d15 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1890,33 +1890,6 @@ static void sandybridge_update_wm(struct drm_device *dev)
> cursor_wm);
> }
>
> -static void
> -haswell_update_linetime_wm(struct drm_device *dev, int pipe,
> - struct drm_display_mode *mode)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 temp;
> -
> - temp = I915_READ(PIPE_WM_LINETIME(pipe));
> - temp &= ~PIPE_WM_LINETIME_MASK;
> -
> - /* The WM are computed with base on how long it takes to fill a single
> - * row at the given clock rate, multiplied by 8.
> - * */
> - temp |= PIPE_WM_LINETIME_TIME(
> - ((mode->crtc_hdisplay * 1000) / mode->clock) * 8);
> -
> - /* IPS watermarks are only used by pipe A, and are ignored by
> - * pipes B and C. They are calculated similarly to the common
> - * linetime values, except that we are using CD clock frequency
> - * in MHz instead of pixel rate for the division.
> - *
> - * This is a placeholder for the IPS watermark calculation code.
> - */
> -
> - I915_WRITE(PIPE_WM_LINETIME(pipe), temp);
> -}
> -
> static bool
> sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
> uint32_t sprite_width, int pixel_size,
> @@ -2112,15 +2085,6 @@ void intel_update_watermarks(struct drm_device *dev)
> dev_priv->display.update_wm(dev);
> }
>
> -void intel_update_linetime_watermarks(struct drm_device *dev,
> - int pipe, struct drm_display_mode *mode)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> - if (dev_priv->display.update_linetime_wm)
> - dev_priv->display.update_linetime_wm(dev, pipe, mode);
> -}
> -
> void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
> uint32_t sprite_width, int pixel_size)
> {
> @@ -4011,7 +3975,6 @@ void intel_init_pm(struct drm_device *dev)
> if (SNB_READ_WM0_LATENCY()) {
> dev_priv->display.update_wm = sandybridge_update_wm;
> dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
> - dev_priv->display.update_linetime_wm = haswell_update_linetime_wm;
> } else {
> DRM_DEBUG_KMS("Failed to read display plane latency. "
> "Disable CxSR\n");
> --
> 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] 18+ messages in thread
* Re: [PATCH 10/10] drm/i915: rip out update_linetime_wm abstraction
2012-12-04 17:34 ` Paulo Zanoni
@ 2012-12-05 11:03 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2012-12-05 11:03 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Daniel Vetter, Intel Graphics Development
On Tue, Dec 04, 2012 at 03:34:09PM -0200, Paulo Zanoni wrote:
> Hi
>
> 2012/11/29 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > I like abstraction and vfuncs, but only if they actually abstract anything.
> > In this case here they just obfuscate, so let's rip this stuff out.
> >
> > Aside: We really should move all the haswell stuff into it's own file ...
>
> Well, a few Kernels ago we did a nice effort to move a lot of code
> from intel_display.c to intel_pm.c. Now we're bringing it back... I'd
> like to understand what are the current plans regarding this.
I guess we should move all the haswell stuff into intel_ddi.c, once it
settles. But I'll drop the watermark patches here anyway, not thought
through these here ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set
2012-12-03 16:42 ` Daniel Vetter
@ 2012-12-06 13:25 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2012-12-06 13:25 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Daniel Vetter, Intel Graphics Development
On Mon, Dec 03, 2012 at 05:42:40PM +0100, Daniel Vetter wrote:
> On Mon, Dec 03, 2012 at 01:03:14PM -0200, Paulo Zanoni wrote:
> > Hi
> >
> > 2012/11/29 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > > Especially getting rid of all things lvds is ... great!
> > >
> > > v2: Drop the two additional pre-hsw hunks noticed by Paulo Zanoni.
> > >
> > > v3:
> > > - handle DP ports correctly (spoted by Paulo)
> > > - don't leave {} behind for a single-line block (again spotted by
> > > Paulo)
> > > - kill another if (IBX || CPT) block
> > >
> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 131 +----------------------------------
> > > 1 file changed, 2 insertions(+), 129 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 3b6e4b0..fecaabc 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5343,9 +5343,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> > > num_connectors++;
> > > }
> > >
> > > - WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)),
> > > - "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev));
> > > -
> >
> > This is patching ironlake_crtc_mode_set, instead of
> > haswell_crtc_mode_set, which still includes the following lines:
> >
> > /* We are not sure yet this won't happen. */
> > WARN(!HAS_PCH_LPT(dev), "Unexpected PCH type %d\n",
> > INTEL_PCH_TYPE(dev));
>
> Hm, can I blame git rebase on this one? Probably not ;-) I'll fix this
> when applying, and we really need to split this stuff up a bit - the
> current code is way too confuse, with too many lookalikes all around.
>
> Thanks for catching this.
Actually I've just left this bit of code in there and deleted this hunk
from the patch - a bit of paranoid WARN to check that the respective
mode_set functions are only called on the right cpu/pch combinations can't
hurt.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 07/10] drm/i915: extract common link_m_n helpers
2012-12-03 15:33 ` Paulo Zanoni
@ 2012-12-06 13:33 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2012-12-06 13:33 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Daniel Vetter, Intel Graphics Development
On Mon, Dec 03, 2012 at 01:33:29PM -0200, Paulo Zanoni wrote:
> 2012/11/29 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > Both the dp and fdi code use the exact same computations (ignore minor
> > differences in conversion between bits and bytes).
> >
> > This makes it even more apparent that we have a _massive_ mess between
> > cpu transcoder/fdi link/pch transcoder and pch link settings. And also
> > that we have hilarious amounts of confusion between edp and dp
> > (despite that they're identical at a link level).
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Slurped in the DP part of this patch-bomb, the watermark stuff isn't
really thought through. 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] 18+ messages in thread
end of thread, other threads:[~2012-12-06 13:32 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-29 14:59 [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Daniel Vetter
2012-11-29 14:59 ` [PATCH 02/10] drm/i915: move set_pll_edp to intel_dp.c Daniel Vetter
2012-11-29 14:59 ` [PATCH 03/10] drm/i915: rip out pre-production ilk cpu edp w/a Daniel Vetter
2012-11-29 14:59 ` [PATCH 04/10] drm/i915: use wait_for_vblank instead of msleep(17) Daniel Vetter
2012-11-29 14:59 ` [PATCH 05/10] drm/i915: WARN on !crtc in intel_dp_link_down Daniel Vetter
2012-11-29 14:59 ` [PATCH 06/10] drm/i915: drop unnecessary clearing of pch dp transcoder timings Daniel Vetter
2012-12-03 15:45 ` Paulo Zanoni
2012-11-29 14:59 ` [PATCH 07/10] drm/i915: extract common link_m_n helpers Daniel Vetter
2012-12-03 15:33 ` Paulo Zanoni
2012-12-06 13:33 ` Daniel Vetter
2012-11-29 14:59 ` [PATCH 08/10] drm/i915: don't call update_watermark in crtc_mode_set Daniel Vetter
2012-11-29 14:59 ` [PATCH 09/10] drm/i915: don't call update_watermarks from haswell enable/disable code Daniel Vetter
2012-11-29 14:59 ` [PATCH 10/10] drm/i915: rip out update_linetime_wm abstraction Daniel Vetter
2012-12-04 17:34 ` Paulo Zanoni
2012-12-05 11:03 ` Daniel Vetter
2012-12-03 15:03 ` [PATCH 01/10] drm/i915: rip out pre-DDI stuff from haswell_crtc_mode_set Paulo Zanoni
2012-12-03 16:42 ` Daniel Vetter
2012-12-06 13:25 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).