Re: [Intel-gfx] PCH reference clock cleanups

[Keith Packard]

On Wed, 28 Sep 2011 15:22:48 -0300, Paulo Zanoni przan...@gmail.com wrote:

 I also tested the patch you sent today 1 hour ago (inline in one of
 the emails) and things still work with it. I'll keep using these
 patches since they fix my laptop. Any problem will be reported.

Thanks. I think we're failing to reset the register on resume, leading
to a black screen for me. I may just break down and fix this the 'right'
way, enabling and disabling the clock sources during mode setting,
depending on which outputs are in use.

-- 
keith.pack...@intel.com


pgpxIjklGS005.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 9/9] drm/i915: Initialize PCH refclks at modeset init time

[Keith Packard]

The reference clock configuration must be done before any mode setting
can occur as all outputs must be disabled to change
anything. Initialize the clocks after turning everything off during
the initialization process.

Also, re-initialize the refclk at resume time.

Signed-off-by: Keith Packard kei...@keithp.com
---

v2 - re-initialize the refclk at resume time.

 drivers/gpu/drm/i915/i915_drv.c  |3 +++
 drivers/gpu/drm/i915/i915_drv.h  |1 +
 drivers/gpu/drm/i915/intel_display.c |   10 +++---
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 58480de..2b6c2d2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -471,6 +471,9 @@ static int i915_drm_thaw(struct drm_device *dev)
error = i915_gem_init_ringbuffer(dev);
mutex_unlock(dev-struct_mutex);
 
+   if (HAS_PCH_SPLIT(dev))
+   ironlake_init_pch_refclk(dev);
+
drm_mode_config_reset(dev);
drm_irq_install(dev);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 18df595..98f2e0b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1302,6 +1302,7 @@ extern int intel_modeset_vga_set_state(struct drm_device 
*dev, bool state);
 extern bool intel_fbc_enabled(struct drm_device *dev);
 extern void intel_disable_fbc(struct drm_device *dev);
 extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
+extern void ironlake_init_pch_refclk(struct drm_device *dev);
 extern void ironlake_enable_rc6(struct drm_device *dev);
 extern void gen6_set_rps(struct drm_device *dev, u8 val);
 extern void intel_detect_pch (struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index b072a35..91d7d5ed 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5109,7 +5109,10 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
return ret;
 }
 
-static void ironlake_update_pch_refclk(struct drm_device *dev)
+/*
+ * Initialize reference clocks when the driver loads
+ */
+void ironlake_init_pch_refclk(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_mode_config *mode_config = dev-mode_config;
@@ -5411,8 +5414,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
ironlake_compute_m_n(intel_crtc-bpp, lane, target_clock, link_bw,
 m_n);
 
-   ironlake_update_pch_refclk(dev);
-
fp = clock.n  16 | clock.m1  8 | clock.m2;
if (has_reduced_clock)
fp2 = reduced_clock.n  16 | reduced_clock.m1  8 |
@@ -7284,6 +7285,9 @@ static void intel_setup_outputs(struct drm_device *dev)
 
/* disable all the possible outputs/crtcs before entering KMS mode */
drm_helper_disable_unused_functions(dev);
+
+   if (HAS_PCH_SPLIT(dev))
+   ironlake_init_pch_refclk(dev);
 }
 
 static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 9/9] drm/i915: Initialize PCH refclks at modeset init time

[Keith Packard]

On Tue, 27 Sep 2011 17:56:39 +0100, Chris Wilson  
wrote:

> Ah, now I see why we moved from using the active configuration earlier. ;-)

My evil plan is revealed!

> Doesn't this prevent us from ever using SSC though, as virtually every
> single PCH machine has HDMI encoders that haven't been masked out through
> the chicken fuses or VBT?

That wasn't my intent -- the SSC source gets modulated whenever the VBT
table says it can, so when the panel uses the SSC source, it will get
SSC. Did I mess something up here? Or is it just some interaction with
the mode setting code that I didn't get right?

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PATCH 6/9] drm/i915: Fix PCH SSC reference clock settings

[Keith Packard]

On Tue, 27 Sep 2011 17:47:10 +0100, Chris Wilson  
wrote:
> On Mon, 26 Sep 2011 23:11:43 -0700, Keith Packard  
> wrote:
> > The PCH refclk settings are global, so we need to look at all of the
> > encoders, not just the current encoder when deciding how to configure
> > it. Also, handle systems with more than one panel (any combination of
> > PCH/non-PCH eDP and LVDS).
> 
> As I read it, this sets the refclk not on the active configuration, but
> on all the hardware detected for the system whether enabled or not.

Correct. We cannot randomly turn ref clocks on/off without also
disconnecting them from the PLLs that they drive.

What we could do is figure out which of the two clocks need to be
enabled and modify the mode set code to turn them on when needed before
setting the mode, and then turn them off after, when they aren't
needed. This would leave them off until needed, which might be nice?

This will make changing the driver to not disable the panel at startup
time harder; we'll need to switch the panel to the non-SSC reference,
turn the SSC reference off, reconfigure it and then switch the panel
back to the SSC reference. That's a project for a future change though.

> There are two basic changes here, the cleanup and improvement to the logic
> based on what type of output is connected and the second change to
> determine which outputs are active.

Right, the logic fixes ensure that the clocks are programmed in the
right sequence and that LVDS, eDP and pch-EDP all get SSC as necessary.

The change in dealing with the outputs means that the clocks are
programmed based not on which outputs are active, but on all possible
outputs, ensuring that the programming never changes in response to mode
setting requests.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20110927/1b2bebee/attachment.pgp>

PCH reference clock cleanups

[Keith Packard]

On Tue, 27 Sep 2011 10:01:33 +0100, Chris Wilson  
wrote:

> Oddly in the diagram SSC4 is given as a 100MHz clock that can be used for
> any output other than DP_A. However, the configuration register marks that
> as being a test-only mode.

Ok, it's all irrelevant -- the only configurations using anything other
than a fixed 120MHz were eDP and LVDS. LVDS used a value from the BIOS, which is
presumably always 120MHz. eDP ignored the refclk and used fixed PLL
values.

So, yes, we can always set the refclk to 120MHz; the cases which matter
were using that value already.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PATCH 9/9] drm/i915: Initialize PCH refclks at modeset init time

[Keith Packard]

The reference clock configuration must be done before any mode setting
can occur as all outputs must be disabled to change
anything. Initialize the clocks after turning everything off during
the initialization process.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_display.c |   10 +++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 919db79..1cc0962 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5109,7 +5109,10 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
return ret;
 }

-static void ironlake_update_pch_refclk(struct drm_device *dev)
+/*
+ * Initialize reference clocks when the driver loads
+ */
+static void ironlake_init_pch_refclk(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_mode_config *mode_config = >mode_config;
@@ -5401,8 +5404,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
ironlake_compute_m_n(intel_crtc->bpp, lane, target_clock, link_bw,
 _n);

-   ironlake_update_pch_refclk(dev);
-
fp = clock.n << 16 | clock.m1 << 8 | clock.m2;
if (has_reduced_clock)
fp2 = reduced_clock.n << 16 | reduced_clock.m1 << 8 |
@@ -7274,6 +7275,9 @@ static void intel_setup_outputs(struct drm_device *dev)

/* disable all the possible outputs/crtcs before entering KMS mode */
drm_helper_disable_unused_functions(dev);
+
+   if (HAS_PCH_SPLIT(dev))
+   ironlake_init_pch_refclk(dev);
 }

 static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
-- 
1.7.6.3

[PATCH 8/9] drm/i915: All PCH refclks are 120MHz

[Keith Packard]

I can't find any reference clocks which run at 96MHz as seems to be
indicated from the comments in this code.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_display.c |   14 --
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 66cd351..919db79 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5271,16 +5271,10 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
num_connectors++;
}

-   if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
-   refclk = dev_priv->lvds_ssc_freq * 1000;
-   DRM_DEBUG_KMS("using SSC reference clock of %d MHz\n",
- refclk / 1000);
-   } else {
-   refclk = 96000;
-   if (!has_edp_encoder ||
-   intel_encoder_is_pch_edp(_edp_encoder->base))
-   refclk = 12; /* 120Mhz refclk */
-   }
+   /*
+* Every reference clock in a PCH system is 120MHz
+*/
+   refclk = 12;

/*
 * Returns a set of divisors for the desired target clock with the given
-- 
1.7.6.3

[PATCH 7/9] drm/i915: Use CK505 as non-SSC source where available

[Keith Packard]

This eliminates VGA shimmer on some Ironlake machines which have a
CK505 clock source.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_display.c |   12 +---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f35..66cd351 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5137,8 +5137,10 @@ static void ironlake_update_pch_refclk(struct drm_device 
*dev)
break;
}
}
-   DRM_DEBUG_KMS("has_panel %d has_lvds %d has_pch_edp %d has_cpu_edp 
%d\n",
- has_panel, has_lvds, has_pch_edp, has_cpu_edp);
+
+   DRM_DEBUG_KMS("has_panel %d has_lvds %d has_pch_edp %d has_cpu_edp %d 
has_ck505 %d\n",
+ has_panel, has_lvds, has_pch_edp, has_cpu_edp,
+ dev_priv->display_clock_mode);  

/* Ironlake: try to setup display ref clock before DPLL
 * enabling. This is only under driver's control after
@@ -5148,7 +5150,11 @@ static void ironlake_update_pch_refclk(struct drm_device 
*dev)
temp = I915_READ(PCH_DREF_CONTROL);
/* Always enable nonspread source */
temp &= ~DREF_NONSPREAD_SOURCE_MASK;
-   temp |= DREF_NONSPREAD_SOURCE_ENABLE;
+
+   if (dev_priv->display_clock_mode)
+   temp |= DREF_NONSPREAD_CK505_ENABLE;
+   else
+   temp |= DREF_NONSPREAD_SOURCE_ENABLE;

if (has_panel) {
temp &= ~DREF_SSC_SOURCE_MASK;
-- 
1.7.6.3

[PATCH 6/9] drm/i915: Fix PCH SSC reference clock settings

[Keith Packard]

The PCH refclk settings are global, so we need to look at all of the
encoders, not just the current encoder when deciding how to configure
it. Also, handle systems with more than one panel (any combination of
PCH/non-PCH eDP and LVDS).

Disable SSC clocks when no panels are connected.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_display.c |   96 +-
 1 files changed, 59 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6039496..f35 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5113,31 +5113,32 @@ static void ironlake_update_pch_refclk(struct 
drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_mode_config *mode_config = >mode_config;
-   struct drm_crtc *crtc;
struct intel_encoder *encoder;
-   struct intel_encoder *has_edp_encoder = NULL;
u32 temp;
bool has_lvds = false;
+   bool has_cpu_edp = false;
+   bool has_pch_edp = false;
+   bool has_panel = false;

/* We need to take the global config into account */
-   list_for_each_entry(crtc, _config->crtc_list, head) {
-   if (!crtc->enabled)
-   continue;
-
-   list_for_each_entry(encoder, _config->encoder_list,
-   base.head) {
-   if (encoder->base.crtc != crtc)
-   continue;
-
-   switch (encoder->type) {
-   case INTEL_OUTPUT_LVDS:
-   has_lvds = true;
-   case INTEL_OUTPUT_EDP:
-   has_edp_encoder = encoder;
-   break;
-   }
+   list_for_each_entry(encoder, _config->encoder_list,
+   base.head) {
+   switch (encoder->type) {
+   case INTEL_OUTPUT_LVDS:
+   has_panel = true;
+   has_lvds = true;
+   break;
+   case INTEL_OUTPUT_EDP:
+   has_panel = true;
+   if (intel_encoder_is_pch_edp(>base))
+   has_pch_edp = true;
+   else
+   has_cpu_edp = true;
+   break;
}
}
+   DRM_DEBUG_KMS("has_panel %d has_lvds %d has_pch_edp %d has_cpu_edp 
%d\n",
+ has_panel, has_lvds, has_pch_edp, has_cpu_edp);

/* Ironlake: try to setup display ref clock before DPLL
 * enabling. This is only under driver's control after
@@ -5148,36 +5149,57 @@ static void ironlake_update_pch_refclk(struct 
drm_device *dev)
/* Always enable nonspread source */
temp &= ~DREF_NONSPREAD_SOURCE_MASK;
temp |= DREF_NONSPREAD_SOURCE_ENABLE;
-   temp &= ~DREF_SSC_SOURCE_MASK;
-   temp |= DREF_SSC_SOURCE_ENABLE;
-   I915_WRITE(PCH_DREF_CONTROL, temp);

-   POSTING_READ(PCH_DREF_CONTROL);
-   udelay(200);
+   if (has_panel) {
+   temp &= ~DREF_SSC_SOURCE_MASK;
+   temp |= DREF_SSC_SOURCE_ENABLE;

-   if (has_edp_encoder) {
+   /* SSC must be turned on before enabling the CPU output  */
if (intel_panel_use_ssc(dev_priv)) {
+   DRM_DEBUG_KMS("Using SSC on panel\n");
temp |= DREF_SSC1_ENABLE;
-   I915_WRITE(PCH_DREF_CONTROL, temp);
-
-   POSTING_READ(PCH_DREF_CONTROL);
-   udelay(200);
}
+
+   /* Get SSC going before enabling the outputs */
+   I915_WRITE(PCH_DREF_CONTROL, temp);
+   POSTING_READ(PCH_DREF_CONTROL);
+   udelay(200);
+
temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK;

/* Enable CPU source on CPU attached eDP */
-   if (!intel_encoder_is_pch_edp(_edp_encoder->base)) {
-   if (intel_panel_use_ssc(dev_priv))
+   if (has_cpu_edp) {
+   if (intel_panel_use_ssc(dev_priv)) {
+   DRM_DEBUG_KMS("Using SSC on eDP\n");
temp |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
+   }
else
temp |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
-   } else {
-   /* Enable SSC on PCH eDP if needed */
-   if (intel_panel_use_ssc(dev_priv)) {
-   DRM_ERROR("enabling SSC on PCH\n");
-   temp |= DREF_SUPERSPREAD_SOURCE_ENABLE;
-   }
- 

[PATCH 5/9] drm/i915: Allow SSC parameter to override VBT value

[Keith Packard]

Allow SSC to be enabled even when the BIOS disables it for testing SSC paths.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/i915_drv.c  |4 ++--
 drivers/gpu/drm/i915/intel_display.c |4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f07e425..58480de 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -79,11 +79,11 @@ MODULE_PARM_DESC(lvds_downclock,
"Use panel (LVDS/eDP) downclocking for power savings "
"(default: false)");

-unsigned int i915_panel_use_ssc __read_mostly = 1;
+unsigned int i915_panel_use_ssc __read_mostly = -1;
 module_param_named(lvds_use_ssc, i915_panel_use_ssc, int, 0600);
 MODULE_PARM_DESC(lvds_use_ssc,
"Use Spread Spectrum Clock with panels [LVDS/eDP] "
-   "(default: true)");
+   "(default: auto from VBT)");

 int i915_vbt_sdvo_panel_type __read_mostly = -1;
 module_param_named(vbt_sdvo_panel_type, i915_vbt_sdvo_panel_type, int, 0600);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 04411ad..6039496 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4584,7 +4584,9 @@ static void intel_update_watermarks(struct drm_device 
*dev)

 static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 {
-   return dev_priv->lvds_use_ssc && i915_panel_use_ssc
+   if (i915_panel_use_ssc >= 0)
+   return i915_panel_use_ssc != 0;
+   return dev_priv->lvds_use_ssc
&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
 }

-- 
1.7.6.3

[PATCH 4/9] drm/i915: Document a few more BDB_GENERAL_FEATURES bits from PCH BIOS

[Keith Packard]

This includes whether an eDP panel is present, and whether that should
use SSC (and at what frequency)

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_bios.h |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.h 
b/drivers/gpu/drm/i915/intel_bios.h
index 02b1b624..72fb500 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -135,7 +135,10 @@ struct bdb_general_features {
 /* bits 5 */
u8 int_crt_support:1;
u8 int_tv_support:1;
-   u8 rsvd11:6; /* finish byte */
+   u8 int_efp_support:1;
+   u8 dp_ssc_enb:1;/* PCH attached eDP supports SSC */
+   u8 dp_ssc_freq:1;   /* SSC freq for PCH attached eDP */
+   u8 rsvd11:3; /* finish byte */
 } __attribute__((packed));

 /* pre-915 */
-- 
1.7.6.3

[PATCH 3/9] drv/i915: Pull display_clock_mode out of VBT table

[Keith Packard]

This tells the driver whether a CK505 clock source is available on
pre-PCH hardware. If so, it should be used as the non-SSC source,
leaving the internal clock for use as the SSC source.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/i915_drv.h   |1 +
 drivers/gpu/drm/i915/intel_bios.c |6 --
 drivers/gpu/drm/i915/intel_bios.h |4 +++-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7916bd9..18df595 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -357,6 +357,7 @@ typedef struct drm_i915_private {
unsigned int lvds_vbt:1;
unsigned int int_crt_support:1;
unsigned int lvds_use_ssc:1;
+   unsigned int display_clock_mode:1;
int lvds_ssc_freq;
struct {
int rate;
diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index dcbc839..eb58784 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -309,11 +309,13 @@ parse_general_features(struct drm_i915_private *dev_priv,
dev_priv->lvds_use_ssc = general->enable_ssc;
dev_priv->lvds_ssc_freq =
intel_bios_ssc_frequency(dev, general->ssc_freq);
-   DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d 
int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d\n",
+   dev_priv->display_clock_mode = general->display_clock_mode;
+   DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d 
int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d display_clock_mode %d\n",
  dev_priv->int_tv_support,
  dev_priv->int_crt_support,
  dev_priv->lvds_use_ssc,
- dev_priv->lvds_ssc_freq);
+ dev_priv->lvds_ssc_freq,
+ dev_priv->display_clock_mode);
}
 }

diff --git a/drivers/gpu/drm/i915/intel_bios.h 
b/drivers/gpu/drm/i915/intel_bios.h
index 5f8e4ed..02b1b624 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -120,7 +120,9 @@ struct bdb_general_features {
u8 ssc_freq:1;
u8 enable_lfp_on_override:1;
u8 disable_ssc_ddt:1;
-   u8 rsvd8:3; /* finish byte */
+   u8 rsvd7:1;
+   u8 display_clock_mode:1;
+   u8 rsvd8:1; /* finish byte */

 /* bits 3 */
u8 disable_smooth_vision:1;
-- 
1.7.6.3

[PATCH 2/9] drm/i915: Use DRM_DEBUG_KMS for all messages in intel_bios.c

[Keith Packard]

These are all KMS related anyways, so don't hide them under other
debug levels.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_bios.c |9 +++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 4c530fa..dcbc839 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -309,6 +309,11 @@ parse_general_features(struct drm_i915_private *dev_priv,
dev_priv->lvds_use_ssc = general->enable_ssc;
dev_priv->lvds_ssc_freq =
intel_bios_ssc_frequency(dev, general->ssc_freq);
+   DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d 
int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d\n",
+ dev_priv->int_tv_support,
+ dev_priv->int_crt_support,
+ dev_priv->lvds_use_ssc,
+ dev_priv->lvds_ssc_freq);
}
 }

@@ -610,7 +615,7 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
/* Default to using SSC */
dev_priv->lvds_use_ssc = 1;
dev_priv->lvds_ssc_freq = intel_bios_ssc_frequency(dev, 1);
-   DRM_DEBUG("Set default to SSC at %dMHz\n", dev_priv->lvds_ssc_freq);
+   DRM_DEBUG_KMS("Set default to SSC at %dMHz\n", dev_priv->lvds_ssc_freq);

/* eDP data */
dev_priv->edp.bpp = 18;
@@ -639,7 +644,7 @@ intel_parse_bios(struct drm_device *dev)
if (dev_priv->opregion.vbt) {
struct vbt_header *vbt = dev_priv->opregion.vbt;
if (memcmp(vbt->signature, "$VBT", 4) == 0) {
-   DRM_DEBUG_DRIVER("Using VBT from OpRegion: %20s\n",
+   DRM_DEBUG_KMS("Using VBT from OpRegion: %20s\n",
 vbt->signature);
bdb = (struct bdb_header *)((char *)vbt + 
vbt->bdb_offset);
} else
-- 
1.7.6.3

[PATCH 1/9] drm/i915: broken copyright encoding in intel_bios.c

[Keith Packard]

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_bios.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 61abef8..4c530fa 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1,5 +1,5 @@
 /*
- * Copyright ? 2006 Intel Corporation
+ * Copyright ?? 2006 Intel Corporation
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
-- 
1.7.6.3

PCH reference clock cleanups

[Keith Packard]

Here's a patch sequence which cleans up a bunch of PCH refclk related
bits. There are a couple of questionable patches that I'd like to see
people look at:

 [PATCH 6/9] drm/i915: Fix PCH SSC reference clock settings
 [PATCH 9/9] drm/i915: Initialize PCH refclks at modeset init time

Here's the main patch -- this looks at the global set of encoders and
figures out what the refclk should be to make all of those work
correctly. Nothing is dependent on the active configuration, so we
aren't reprogramming this register during run-time. The last patch in
the sequence moves the setting of this register from modeset time to
init time.

 [PATCH 7/9] drm/i915: Use CK505 as non-SSC source where available

This is a small piece straight from Jesse's patch; just uses the VBT
configuration for CK505 clock sources.

 [PATCH 8/9] drm/i915: All PCH refclks are 120MHz

Ok, so I'd love to know where in any PCH reference matter someone has
found a place where the reference clock for any of the PLLs is
anything other than 120MHz. Can someone find a reference for other frequencies?

--
keith.packard at intel.com

PCH reference clock cleanups

[Keith Packard]

Here's a patch sequence which cleans up a bunch of PCH refclk related
bits. There are a couple of questionable patches that I'd like to see
people look at:

 [PATCH 6/9] drm/i915: Fix PCH SSC reference clock settings
 [PATCH 9/9] drm/i915: Initialize PCH refclks at modeset init time

Here's the main patch -- this looks at the global set of encoders and
figures out what the refclk should be to make all of those work
correctly. Nothing is dependent on the active configuration, so we
aren't reprogramming this register during run-time. The last patch in
the sequence moves the setting of this register from modeset time to
init time.

 [PATCH 7/9] drm/i915: Use CK505 as non-SSC source where available

This is a small piece straight from Jesse's patch; just uses the VBT
configuration for CK505 clock sources.

 [PATCH 8/9] drm/i915: All PCH refclks are 120MHz

Ok, so I'd love to know where in any PCH reference matter someone has
found a place where the reference clock for any of the PLLs is
anything other than 120MHz. Can someone find a reference for other frequencies?

--
keith.pack...@intel.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/9] drm/i915: broken copyright encoding in intel_bios.c

[Keith Packard]

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_bios.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 61abef8..4c530fa 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1,5 +1,5 @@
 /*
- * Copyright � 2006 Intel Corporation
+ * Copyright © 2006 Intel Corporation
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the Software),
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/9] drv/i915: Pull display_clock_mode out of VBT table

[Keith Packard]

This tells the driver whether a CK505 clock source is available on
pre-PCH hardware. If so, it should be used as the non-SSC source,
leaving the internal clock for use as the SSC source.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/i915_drv.h   |1 +
 drivers/gpu/drm/i915/intel_bios.c |6 --
 drivers/gpu/drm/i915/intel_bios.h |4 +++-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7916bd9..18df595 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -357,6 +357,7 @@ typedef struct drm_i915_private {
unsigned int lvds_vbt:1;
unsigned int int_crt_support:1;
unsigned int lvds_use_ssc:1;
+   unsigned int display_clock_mode:1;
int lvds_ssc_freq;
struct {
int rate;
diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index dcbc839..eb58784 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -309,11 +309,13 @@ parse_general_features(struct drm_i915_private *dev_priv,
dev_priv-lvds_use_ssc = general-enable_ssc;
dev_priv-lvds_ssc_freq =
intel_bios_ssc_frequency(dev, general-ssc_freq);
-   DRM_DEBUG_KMS(BDB_GENERAL_FEATURES int_tv_support %d 
int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d\n,
+   dev_priv-display_clock_mode = general-display_clock_mode;
+   DRM_DEBUG_KMS(BDB_GENERAL_FEATURES int_tv_support %d 
int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d display_clock_mode %d\n,
  dev_priv-int_tv_support,
  dev_priv-int_crt_support,
  dev_priv-lvds_use_ssc,
- dev_priv-lvds_ssc_freq);
+ dev_priv-lvds_ssc_freq,
+ dev_priv-display_clock_mode);
}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_bios.h 
b/drivers/gpu/drm/i915/intel_bios.h
index 5f8e4ed..02b1b624 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -120,7 +120,9 @@ struct bdb_general_features {
u8 ssc_freq:1;
u8 enable_lfp_on_override:1;
u8 disable_ssc_ddt:1;
-   u8 rsvd8:3; /* finish byte */
+   u8 rsvd7:1;
+   u8 display_clock_mode:1;
+   u8 rsvd8:1; /* finish byte */
 
 /* bits 3 */
u8 disable_smooth_vision:1;
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/9] drm/i915: Allow SSC parameter to override VBT value

[Keith Packard]

Allow SSC to be enabled even when the BIOS disables it for testing SSC paths.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/i915_drv.c  |4 ++--
 drivers/gpu/drm/i915/intel_display.c |4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f07e425..58480de 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -79,11 +79,11 @@ MODULE_PARM_DESC(lvds_downclock,
Use panel (LVDS/eDP) downclocking for power savings 
(default: false));
 
-unsigned int i915_panel_use_ssc __read_mostly = 1;
+unsigned int i915_panel_use_ssc __read_mostly = -1;
 module_param_named(lvds_use_ssc, i915_panel_use_ssc, int, 0600);
 MODULE_PARM_DESC(lvds_use_ssc,
Use Spread Spectrum Clock with panels [LVDS/eDP] 
-   (default: true));
+   (default: auto from VBT));
 
 int i915_vbt_sdvo_panel_type __read_mostly = -1;
 module_param_named(vbt_sdvo_panel_type, i915_vbt_sdvo_panel_type, int, 0600);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 04411ad..6039496 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4584,7 +4584,9 @@ static void intel_update_watermarks(struct drm_device 
*dev)
 
 static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 {
-   return dev_priv-lvds_use_ssc  i915_panel_use_ssc
+   if (i915_panel_use_ssc = 0)
+   return i915_panel_use_ssc != 0;
+   return dev_priv-lvds_use_ssc
 !(dev_priv-quirks  QUIRK_LVDS_SSC_DISABLE);
 }
 
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/9] drm/i915: Document a few more BDB_GENERAL_FEATURES bits from PCH BIOS

[Keith Packard]

This includes whether an eDP panel is present, and whether that should
use SSC (and at what frequency)

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_bios.h |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.h 
b/drivers/gpu/drm/i915/intel_bios.h
index 02b1b624..72fb500 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -135,7 +135,10 @@ struct bdb_general_features {
 /* bits 5 */
u8 int_crt_support:1;
u8 int_tv_support:1;
-   u8 rsvd11:6; /* finish byte */
+   u8 int_efp_support:1;
+   u8 dp_ssc_enb:1;/* PCH attached eDP supports SSC */
+   u8 dp_ssc_freq:1;   /* SSC freq for PCH attached eDP */
+   u8 rsvd11:3; /* finish byte */
 } __attribute__((packed));
 
 /* pre-915 */
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/9] drm/i915: Use DRM_DEBUG_KMS for all messages in intel_bios.c

[Keith Packard]

These are all KMS related anyways, so don't hide them under other
debug levels.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_bios.c |9 +++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 4c530fa..dcbc839 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -309,6 +309,11 @@ parse_general_features(struct drm_i915_private *dev_priv,
dev_priv-lvds_use_ssc = general-enable_ssc;
dev_priv-lvds_ssc_freq =
intel_bios_ssc_frequency(dev, general-ssc_freq);
+   DRM_DEBUG_KMS(BDB_GENERAL_FEATURES int_tv_support %d 
int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d\n,
+ dev_priv-int_tv_support,
+ dev_priv-int_crt_support,
+ dev_priv-lvds_use_ssc,
+ dev_priv-lvds_ssc_freq);
}
 }
 
@@ -610,7 +615,7 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
/* Default to using SSC */
dev_priv-lvds_use_ssc = 1;
dev_priv-lvds_ssc_freq = intel_bios_ssc_frequency(dev, 1);
-   DRM_DEBUG(Set default to SSC at %dMHz\n, dev_priv-lvds_ssc_freq);
+   DRM_DEBUG_KMS(Set default to SSC at %dMHz\n, dev_priv-lvds_ssc_freq);
 
/* eDP data */
dev_priv-edp.bpp = 18;
@@ -639,7 +644,7 @@ intel_parse_bios(struct drm_device *dev)
if (dev_priv-opregion.vbt) {
struct vbt_header *vbt = dev_priv-opregion.vbt;
if (memcmp(vbt-signature, $VBT, 4) == 0) {
-   DRM_DEBUG_DRIVER(Using VBT from OpRegion: %20s\n,
+   DRM_DEBUG_KMS(Using VBT from OpRegion: %20s\n,
 vbt-signature);
bdb = (struct bdb_header *)((char *)vbt + 
vbt-bdb_offset);
} else
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 9/9] drm/i915: Initialize PCH refclks at modeset init time

[Keith Packard]

The reference clock configuration must be done before any mode setting
can occur as all outputs must be disabled to change
anything. Initialize the clocks after turning everything off during
the initialization process.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_display.c |   10 +++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 919db79..1cc0962 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5109,7 +5109,10 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
return ret;
 }
 
-static void ironlake_update_pch_refclk(struct drm_device *dev)
+/*
+ * Initialize reference clocks when the driver loads
+ */
+static void ironlake_init_pch_refclk(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_mode_config *mode_config = dev-mode_config;
@@ -5401,8 +5404,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
ironlake_compute_m_n(intel_crtc-bpp, lane, target_clock, link_bw,
 m_n);
 
-   ironlake_update_pch_refclk(dev);
-
fp = clock.n  16 | clock.m1  8 | clock.m2;
if (has_reduced_clock)
fp2 = reduced_clock.n  16 | reduced_clock.m1  8 |
@@ -7274,6 +7275,9 @@ static void intel_setup_outputs(struct drm_device *dev)
 
/* disable all the possible outputs/crtcs before entering KMS mode */
drm_helper_disable_unused_functions(dev);
+
+   if (HAS_PCH_SPLIT(dev))
+   ironlake_init_pch_refclk(dev);
 }
 
 static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 8/9] drm/i915: All PCH refclks are 120MHz

[Keith Packard]

I can't find any reference clocks which run at 96MHz as seems to be
indicated from the comments in this code.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_display.c |   14 --
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 66cd351..919db79 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5271,16 +5271,10 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
num_connectors++;
}
 
-   if (is_lvds  intel_panel_use_ssc(dev_priv)  num_connectors  2) {
-   refclk = dev_priv-lvds_ssc_freq * 1000;
-   DRM_DEBUG_KMS(using SSC reference clock of %d MHz\n,
- refclk / 1000);
-   } else {
-   refclk = 96000;
-   if (!has_edp_encoder ||
-   intel_encoder_is_pch_edp(has_edp_encoder-base))
-   refclk = 12; /* 120Mhz refclk */
-   }
+   /*
+* Every reference clock in a PCH system is 120MHz
+*/
+   refclk = 12;
 
/*
 * Returns a set of divisors for the desired target clock with the given
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/9] drm/i915: Fix PCH SSC reference clock settings

[Keith Packard]

The PCH refclk settings are global, so we need to look at all of the
encoders, not just the current encoder when deciding how to configure
it. Also, handle systems with more than one panel (any combination of
PCH/non-PCH eDP and LVDS).

Disable SSC clocks when no panels are connected.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_display.c |   96 +-
 1 files changed, 59 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6039496..f35 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5113,31 +5113,32 @@ static void ironlake_update_pch_refclk(struct 
drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_mode_config *mode_config = dev-mode_config;
-   struct drm_crtc *crtc;
struct intel_encoder *encoder;
-   struct intel_encoder *has_edp_encoder = NULL;
u32 temp;
bool has_lvds = false;
+   bool has_cpu_edp = false;
+   bool has_pch_edp = false;
+   bool has_panel = false;
 
/* We need to take the global config into account */
-   list_for_each_entry(crtc, mode_config-crtc_list, head) {
-   if (!crtc-enabled)
-   continue;
-
-   list_for_each_entry(encoder, mode_config-encoder_list,
-   base.head) {
-   if (encoder-base.crtc != crtc)
-   continue;
-
-   switch (encoder-type) {
-   case INTEL_OUTPUT_LVDS:
-   has_lvds = true;
-   case INTEL_OUTPUT_EDP:
-   has_edp_encoder = encoder;
-   break;
-   }
+   list_for_each_entry(encoder, mode_config-encoder_list,
+   base.head) {
+   switch (encoder-type) {
+   case INTEL_OUTPUT_LVDS:
+   has_panel = true;
+   has_lvds = true;
+   break;
+   case INTEL_OUTPUT_EDP:
+   has_panel = true;
+   if (intel_encoder_is_pch_edp(encoder-base))
+   has_pch_edp = true;
+   else
+   has_cpu_edp = true;
+   break;
}
}
+   DRM_DEBUG_KMS(has_panel %d has_lvds %d has_pch_edp %d has_cpu_edp 
%d\n,
+ has_panel, has_lvds, has_pch_edp, has_cpu_edp);
 
/* Ironlake: try to setup display ref clock before DPLL
 * enabling. This is only under driver's control after
@@ -5148,36 +5149,57 @@ static void ironlake_update_pch_refclk(struct 
drm_device *dev)
/* Always enable nonspread source */
temp = ~DREF_NONSPREAD_SOURCE_MASK;
temp |= DREF_NONSPREAD_SOURCE_ENABLE;
-   temp = ~DREF_SSC_SOURCE_MASK;
-   temp |= DREF_SSC_SOURCE_ENABLE;
-   I915_WRITE(PCH_DREF_CONTROL, temp);
 
-   POSTING_READ(PCH_DREF_CONTROL);
-   udelay(200);
+   if (has_panel) {
+   temp = ~DREF_SSC_SOURCE_MASK;
+   temp |= DREF_SSC_SOURCE_ENABLE;
 
-   if (has_edp_encoder) {
+   /* SSC must be turned on before enabling the CPU output  */
if (intel_panel_use_ssc(dev_priv)) {
+   DRM_DEBUG_KMS(Using SSC on panel\n);
temp |= DREF_SSC1_ENABLE;
-   I915_WRITE(PCH_DREF_CONTROL, temp);
-
-   POSTING_READ(PCH_DREF_CONTROL);
-   udelay(200);
}
+
+   /* Get SSC going before enabling the outputs */
+   I915_WRITE(PCH_DREF_CONTROL, temp);
+   POSTING_READ(PCH_DREF_CONTROL);
+   udelay(200);
+
temp = ~DREF_CPU_SOURCE_OUTPUT_MASK;
 
/* Enable CPU source on CPU attached eDP */
-   if (!intel_encoder_is_pch_edp(has_edp_encoder-base)) {
-   if (intel_panel_use_ssc(dev_priv))
+   if (has_cpu_edp) {
+   if (intel_panel_use_ssc(dev_priv)) {
+   DRM_DEBUG_KMS(Using SSC on eDP\n);
temp |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
+   }
else
temp |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
-   } else {
-   /* Enable SSC on PCH eDP if needed */
-   if (intel_panel_use_ssc(dev_priv)) {
-   DRM_ERROR(enabling SSC on PCH\n);
-   temp |= DREF_SUPERSPREAD_SOURCE_ENABLE;
-   }
-   }
+   } else
+   temp

[PATCH 7/9] drm/i915: Use CK505 as non-SSC source where available

[Keith Packard]

This eliminates VGA shimmer on some Ironlake machines which have a
CK505 clock source.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_display.c |   12 +---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f35..66cd351 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5137,8 +5137,10 @@ static void ironlake_update_pch_refclk(struct drm_device 
*dev)
break;
}
}
-   DRM_DEBUG_KMS(has_panel %d has_lvds %d has_pch_edp %d has_cpu_edp 
%d\n,
- has_panel, has_lvds, has_pch_edp, has_cpu_edp);
+
+   DRM_DEBUG_KMS(has_panel %d has_lvds %d has_pch_edp %d has_cpu_edp %d 
has_ck505 %d\n,
+ has_panel, has_lvds, has_pch_edp, has_cpu_edp,
+ dev_priv-display_clock_mode);  
 
/* Ironlake: try to setup display ref clock before DPLL
 * enabling. This is only under driver's control after
@@ -5148,7 +5150,11 @@ static void ironlake_update_pch_refclk(struct drm_device 
*dev)
temp = I915_READ(PCH_DREF_CONTROL);
/* Always enable nonspread source */
temp = ~DREF_NONSPREAD_SOURCE_MASK;
-   temp |= DREF_NONSPREAD_SOURCE_ENABLE;
+
+   if (dev_priv-display_clock_mode)
+   temp |= DREF_NONSPREAD_CK505_ENABLE;
+   else
+   temp |= DREF_NONSPREAD_SOURCE_ENABLE;
 
if (has_panel) {
temp = ~DREF_SSC_SOURCE_MASK;
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: PCH reference clock cleanups

[Keith Packard]

On Tue, 27 Sep 2011 10:01:33 +0100, Chris Wilson ch...@chris-wilson.co.uk 
wrote:

 Oddly in the diagram SSC4 is given as a 100MHz clock that can be used for
 any output other than DP_A. However, the configuration register marks that
 as being a test-only mode.

Ok, it's all irrelevant -- the only configurations using anything other
than a fixed 120MHz were eDP and LVDS. LVDS used a value from the BIOS, which is
presumably always 120MHz. eDP ignored the refclk and used fixed PLL
values.

So, yes, we can always set the refclk to 120MHz; the cases which matter
were using that value already.

-- 
keith.pack...@intel.com


pgpITqGsLcwcf.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/9] drm/i915: Fix PCH SSC reference clock settings

[Keith Packard]

On Tue, 27 Sep 2011 17:47:10 +0100, Chris Wilson ch...@chris-wilson.co.uk 
wrote:
 On Mon, 26 Sep 2011 23:11:43 -0700, Keith Packard kei...@keithp.com wrote:
  The PCH refclk settings are global, so we need to look at all of the
  encoders, not just the current encoder when deciding how to configure
  it. Also, handle systems with more than one panel (any combination of
  PCH/non-PCH eDP and LVDS).
 
 As I read it, this sets the refclk not on the active configuration, but
 on all the hardware detected for the system whether enabled or not.

Correct. We cannot randomly turn ref clocks on/off without also
disconnecting them from the PLLs that they drive.

What we could do is figure out which of the two clocks need to be
enabled and modify the mode set code to turn them on when needed before
setting the mode, and then turn them off after, when they aren't
needed. This would leave them off until needed, which might be nice?

This will make changing the driver to not disable the panel at startup
time harder; we'll need to switch the panel to the non-SSC reference,
turn the SSC reference off, reconfigure it and then switch the panel
back to the SSC reference. That's a project for a future change though.

 There are two basic changes here, the cleanup and improvement to the logic
 based on what type of output is connected and the second change to
 determine which outputs are active.

Right, the logic fixes ensure that the clocks are programmed in the
right sequence and that LVDS, eDP and pch-EDP all get SSC as necessary.

The change in dealing with the outputs means that the clocks are
programmed based not on which outputs are active, but on all possible
outputs, ensuring that the programming never changes in response to mode
setting requests.

-- 
keith.pack...@intel.com


pgp00iVEdoBxV.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Intel-gfx] [PATCH 6/9] drm/i915: Make sure eDP power is on before using aux channel

[Keith Packard]

On Fri, 23 Sep 2011 08:25:13 +0530, Jesse Barnes  
wrote:

> Yeah that sounds good.  (2) and (3) are ok cleanups, but it would be
> best if they were a separate patch just in case the subtle timing
> change breaks the panel power sequencing state machine.

Ok, I'll split things up into tiny patches then; easier to review, and
easier to try different combinations.

> No I think:
>   1) VDD AUX override on
>   2) PPS on
>   3) (delay)
>   4) VDD AUX override off
> is safe, I'm just worried about the timing of step (3).

Ok, that seems like a nice plan to me -- just doing the PPS on sequence,
and then setting VDD AUX off after PPS completes. That seems easy, and
avoids leaving VDD AUX once the panel is completely running.

> To fix both PCH eDP and CPU eDP in the past, I did have a version that
> only used full PPS and not VDD AUX override.  So it is possible, but
> VDD AUX is a little cleaner since it allows us to keep the registers
> locked potentially (theoretically we only actually want to unlock to
> handle CPU eDP PLL enable/disable bugs and flicker-free panel fitter
> downscaling).

I really don't care about leaving registers locked; we're not running
DOS anymore. Just doesn't make any sense to me.

> But since we unlock unconditionally now, using full PPS would be ok.

> Though we will have to shut it down still; PPS on to get AUX data and
> EDID, then off while we program the mode and train DP, then PPS on
> again.  So I'm not sure it would save much.

VDD AUX on or PPS has to be done to run the training anyways.

> Yeah, I'm liking your delayed work much better now...  Bring up the
> panel early and then just modify the shutdown timeout at various points
> to make sure it stays up from module_init all the way through the final
> mode set (so an initial timeout of 2s or so would probably be
> sufficient).

The question is whether to do PPS or use VDD AUX on to start
with.

> Another potential optimization is to start trying AUX & i2c
> transactions right after we apply VDD AUX override.  The panel will
> come up much faster than the T* values imply most of the time (varies
> by panel).  And polling the bits can get us into the actual panel
> poking code much faster.

While that's possible, it wasn't true on the MBA; it took almost the
full T3 interval after VDD AUX was on before the EDID came back.

> But I think the bottom line is to fix the EDID read (make sure the
> panel is on) and the i2c stuff.  Everything else is just tasty
> gravy. :)

Right, that is what fixed the MBA for me.

> Also I think the change to prefer EDID over VBT is correct; afaik eDP
> panels are required to have an EDID, so trusting that data over some
> potentially untested VBT data is the right way to go.

Especially when VBT comes from a BIOS emulation mess which just lies.

I'll try to get a new sequence done tomorrow with these
suggestions. Hope your Indian adventures are going well.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Re: [Intel-gfx] [PATCH 6/9] drm/i915: Make sure eDP power is on before using aux channel

[Keith Packard]

On Fri, 23 Sep 2011 08:25:13 +0530, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 Yeah that sounds good.  (2) and (3) are ok cleanups, but it would be
 best if they were a separate patch just in case the subtle timing
 change breaks the panel power sequencing state machine.

Ok, I'll split things up into tiny patches then; easier to review, and
easier to try different combinations.

 No I think:
   1) VDD AUX override on
   2) PPS on
   3) (delay)
   4) VDD AUX override off
 is safe, I'm just worried about the timing of step (3).

Ok, that seems like a nice plan to me -- just doing the PPS on sequence,
and then setting VDD AUX off after PPS completes. That seems easy, and
avoids leaving VDD AUX once the panel is completely running.

 To fix both PCH eDP and CPU eDP in the past, I did have a version that
 only used full PPS and not VDD AUX override.  So it is possible, but
 VDD AUX is a little cleaner since it allows us to keep the registers
 locked potentially (theoretically we only actually want to unlock to
 handle CPU eDP PLL enable/disable bugs and flicker-free panel fitter
 downscaling).

I really don't care about leaving registers locked; we're not running
DOS anymore. Just doesn't make any sense to me.

 But since we unlock unconditionally now, using full PPS would be ok.

 Though we will have to shut it down still; PPS on to get AUX data and
 EDID, then off while we program the mode and train DP, then PPS on
 again.  So I'm not sure it would save much.

VDD AUX on or PPS has to be done to run the training anyways.

 Yeah, I'm liking your delayed work much better now...  Bring up the
 panel early and then just modify the shutdown timeout at various points
 to make sure it stays up from module_init all the way through the final
 mode set (so an initial timeout of 2s or so would probably be
 sufficient).

The question is whether to do PPS or use VDD AUX on to start
with.

 Another potential optimization is to start trying AUX  i2c
 transactions right after we apply VDD AUX override.  The panel will
 come up much faster than the T* values imply most of the time (varies
 by panel).  And polling the bits can get us into the actual panel
 poking code much faster.

While that's possible, it wasn't true on the MBA; it took almost the
full T3 interval after VDD AUX was on before the EDID came back.

 But I think the bottom line is to fix the EDID read (make sure the
 panel is on) and the i2c stuff.  Everything else is just tasty
 gravy. :)

Right, that is what fixed the MBA for me.

 Also I think the change to prefer EDID over VBT is correct; afaik eDP
 panels are required to have an EDID, so trusting that data over some
 potentially untested VBT data is the right way to go.

Especially when VBT comes from a BIOS emulation mess which just lies.

I'll try to get a new sequence done tomorrow with these
suggestions. Hope your Indian adventures are going well.

-- 
keith.pack...@intel.com


pgpZDbC7Q0YWK.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Whitespace cleanups in drm/i915

[Keith Packard]

On Wed, 21 Sep 2011 16:56:12 -0400, Akshay Joshi  wrote:

> Have we reached a consensus on this? Just curious.

Your patch was merged to Dave Airlie's drm-core-next branch.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Re: Whitespace cleanups in drm/i915

[Keith Packard]

On Wed, 21 Sep 2011 16:56:12 -0400, Akshay Joshi m...@akshayjoshi.com wrote:

 Have we reached a consensus on this? Just curious.

Your patch was merged to Dave Airlie's drm-core-next branch.

-- 
keith.pack...@intel.com


pgpDbfVeNrjh4.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Intel-gfx] [PATCH 9/9] drm/i915: Disable eDP VDD in a delayed work proc instead of synchronously

[Keith Packard]

On Wed, 21 Sep 2011 09:47:59 +0530, Jesse Barnes  
wrote:

> I'm worried this makes our PPS even more complex and hard to follow.
> I'd rather see VDD AUX applied only when we need it (dpms, mode set and
> detect; for hotplug we can assume the panel is alive) and that we
> carefully disable it after waiting for a time after a full PPS on
> sequence when doing a mode set or dpms on command.

You'll see the fairly long list of places where VDD AUX is needed in my
other reply; a couple of them could probably be fixed by moving the PPS
calls around.

> Having all the power stuff at the highest levels would be clearest I
> think.

Yes, making it cleaner would help a ton. There are some basic problems
with the DRM API that make this hard though -- intel_dp_prepare may not
ever be followed by a call to intel_dp_commit. That's why I had the VDD
AUX stuff get turned off by a delayed work proc instead.

Also, leaving VDD AUX high after EDID is fetched means that we can start
the mode setting immediately, rather than having to wait for the
power-off/power-on delay (which is really long).

What we could do is force VDD AUX off after the panel gets turned on;
that would ensure that turning the panel off would actually turn the
power off, rather than having VDD stay high for some time after that.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[Intel-gfx] [PATCH 6/9] drm/i915: Make sure eDP power is on before using aux channel

[Keith Packard]

On Wed, 21 Sep 2011 09:20:01 +0530, Jesse Barnes  
wrote:

> This one mixes up lots of cleanups plus the EDID read with the power
> changes.

I think the cleanups are;

 1) edp checks inside vdd_on and vdd_off to make the other code a bit
easier to read.

 2) Hold VDD on until the end of dp_commit. I think this isn't necessary
and could be removed -- once the panel is on, we don't need to hold
vdd up.

 3) Hold VDD up through the whole dpms sequence. Also probably
unnecessary.

 4) Move intel_dp_i2c_init past the computation of the power sequencing
delays. Necessary as i2c_init makes an i2c transaction, thus
powering up the panel.

I can remove the middle two and split the others out if you like.

> I'm worried about the VDD smashing as well; we have lots of
> bugs in the PPS hardware around VDD vs full PPS.

Are you worried that we should never have VDD up while running a panel
power sequence? As far as I can tell from the eDP specs, bringing VDD up
is part of the normal PPS, and the delay from VDD up to other panel
sequencing is shorter (T1+T2) than the delay to start using the aux channel
that I used in the later patch (T1+T2+T3).

One thing I learned for certain -- we don't want a synchronous delay
between turning the panel off and back on. The required interval between
these two operations is in units of 100ms; my machine spends 600ms doing
this; if we end up doing this in the middle of a mode set, it's gonna be
painful.

Can we replace any of the current VDD hacks with a full PPS? I can
easily imagine moving the call to ironlake_edp_panel_on to
intel_dp_prepare, except that if if the mode_set fails, dp_commit will
not be called (that looks like a potential source of failure at the DRM
level to me).

Getting the panel turned on complete as early as possible seems like a
good idea, instead of fussing around with the VDD force bits.

Alternatively, we can eliminate use of the VDD force hack and do a full
PPS and simply use the delayed work proc to turn that off when the
screen goes idle.

> We need to make sure appropriate delays are in place when
> transitioning from one to another.

As you can see, I stuck 1000ms delays in the vdd_on and vdd_off
functions to ensure that there was 'sufficient' delay in these cases. I
didn't touch the existing delays for regular panel_on/panel_off. I think
that the later patch, which computes 'correct' delays and applies those
uniformly should ensure that we're always waiting long enough.

The later patches track the jiffies value when the panel was turned off
and then wait the appropriate amount of time before turning it back
on.

> In what paths are we trying to do accesses without power applied?
> Looks like mainly edid?

(the previously broken cases are marked with '*')

* intel_dp_i2c_init

This probes the i2c bus to see if there's someone home. Failing
to have VDD up at this point causes a timeout, but no other
obvious problem.

As you can see, I moved the call to this function after the
computation of the power sequencing delay values.

* intel_dp_prepare

This wakes up the eDP panel out of a low power DPMS state. I
think I'd actually like to leave VDD high starting at this point
and ending after the panel is running.

intel_dp_commit

Calls intel_dp_start_link_train. before ironlake_edp_panel_on.
I think the call to ironlake_edp_panel_vdd_off could move back
where it was, right after ironlake_edp_panel_on.

intel_dp_dpms

*   In the DPMS_ON case, this calls intel_dp_sink_dpms and
intel_dp_start_link_train before calling
ironlake_edp_panel_on. I wonder if we shouldn't just turn the
panel on instead?

In the DPMS_OFF case, I don't think we need to mess with vdd at
all; the panel is definitely running until
ironlake_edp_panel_off is called, at which point we're done
using the aux channel.

* intel_dp_get_edid,
* intel_dp_get_edid_modes

These are called while the display is off (sometimes).

> I see the next patch handles the timing stuff, I assume it's ok.

I hope so; the actual timing values required by the eDP spec aren't
available, so I fudged by choosing ones that were at least as long.

Thanks for taking a look at the code.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PATCH] drm/i915: Enable digital port hotplug on PCH systems

[Keith Packard]

On Tue, 20 Sep 2011 17:24:21 +0100, Chris Wilson  
wrote:
> On Tue, 20 Sep 2011 08:47:21 -0700, Keith Packard  
> wrote:
> > We were relying on the BIOS to set these bits, which doesn't always
> > happen.
> 
> Do we need to clear IRQ bits on uninstall, for example to prevent
> "Interrupt 19: no one cared!" or is that due to a different cause?

Probably a good plan, but of course that's separate from turning on the
hotplug hardware in the PCH. Something like:

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c22823b..adeab2a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2044,6 +2044,10 @@ static void ironlake_irq_uninstall(struct drm_device 
*dev)
I915_WRITE(GTIMR, 0x);
I915_WRITE(GTIER, 0x0);
I915_WRITE(GTIIR, I915_READ(GTIIR));
+
+   I915_WRITE(SDEIMR, 0x);
+   I915_WRITE(SDEIER, 0x0);
+   I915_WRITE(SDEIIR, I915_READ(SDEIIR));
 }

 static void i915_driver_irq_uninstall(struct drm_device * dev)

> More of a general question really. The patch itself looks good and
> matches against the spec afaics.

I was amused to learn that the BIOS on my machine actually turns on the
hotplug hardware for no good reason, so things 'worked' without this
code on many machines. I wonder how many DP/DVI/HDMI machines aren't
actually doing hotplug today...

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20110920/a5773bbe/attachment.pgp>

Proposal for a low-level Linux display framework

[Keith Packard]

On Tue, 20 Sep 2011 10:29:23 +0200, Patrik Jakobsson  wrote:

> It would be nice to have a model that fits both DSI and SDVO, and the option
> to configure some of it from userspace.

> I thought the purpose of drm_encoder was to abstract hardware like this?

SDVO is entirely hidden by the drm_encoder interface; some of the
controls (like TV encoder parameters) are exposed through DRM
properties, others are used in the basic configuration of the device.

I'm not sure we need a new abstraction that subsumes both DSI and SDVO,
but we may need a DSI library that can be used by both DRM and other
parts of the kernel.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PATCH] drm/i915: Enable digital port hotplug on PCH systems

[Keith Packard]

We were relying on the BIOS to set these bits, which doesn't always
happen.

Signed-off-by: Keith Packard 
---

v2: set the hotplug bits in the irq post-install hook, just
like we do for pre-PCH hardware.

 drivers/gpu/drm/i915/i915_irq.c |   24 
 drivers/gpu/drm/i915/i915_reg.h |5 -
 2 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9cbb0cd..c22823b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1777,6 +1777,26 @@ static void ironlake_irq_preinstall(struct drm_device 
*dev)
POSTING_READ(SDEIER);
 }

+/*
+ * Enable digital hotplug on the PCH, and configure the DP short pulse
+ * duration to 2ms (which is the minimum in the Display Port spec)
+ *
+ * This register is the same on all known PCH chips.
+ */
+
+static void ironlake_enable_pch_hotplug(struct drm_device *dev)
+{
+   drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+   u32 hotplug;
+
+   hotplug = I915_READ(PCH_PORT_HOTPLUG);
+   hotplug &= 
~(PORTD_PULSE_DURATION_MASK|PORTC_PULSE_DURATION_MASK|PORTB_PULSE_DURATION_MASK);
+   hotplug |= PORTD_HOTPLUG_ENABLE | PORTD_PULSE_DURATION_2ms;
+   hotplug |= PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_2ms;
+   hotplug |= PORTB_HOTPLUG_ENABLE | PORTB_PULSE_DURATION_2ms;
+   I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
+}
+
 static int ironlake_irq_postinstall(struct drm_device *dev)
 {
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
@@ -1839,6 +1859,8 @@ static int ironlake_irq_postinstall(struct drm_device 
*dev)
I915_WRITE(SDEIER, hotplug_mask);
POSTING_READ(SDEIER);

+   ironlake_enable_pch_hotplug(dev);
+
if (IS_IRONLAKE_M(dev)) {
/* Clear & enable PCU event interrupts */
I915_WRITE(DEIIR, DE_PCU_EVENT);
@@ -1896,6 +1918,8 @@ static int ivybridge_irq_postinstall(struct drm_device 
*dev)
I915_WRITE(SDEIER, hotplug_mask);
POSTING_READ(SDEIER);

+   ironlake_enable_pch_hotplug(dev);
+
return 0;
 }

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 542453f..b7fbb74 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2903,12 +2903,13 @@
 #define SDEIER  0xc400c

 /* digital port hotplug */
-#define PCH_PORT_HOTPLUG0xc4030
+#define PCH_PORT_HOTPLUG0xc4030/* SHOTPLUG_CTL */
 #define PORTD_HOTPLUG_ENABLE(1 << 20)
 #define PORTD_PULSE_DURATION_2ms(0)
 #define PORTD_PULSE_DURATION_4_5ms  (1 << 18)
 #define PORTD_PULSE_DURATION_6ms(2 << 18)
 #define PORTD_PULSE_DURATION_100ms  (3 << 18)
+#define PORTD_PULSE_DURATION_MASK  (3 << 18)
 #define PORTD_HOTPLUG_NO_DETECT (0)
 #define PORTD_HOTPLUG_SHORT_DETECT  (1 << 16)
 #define PORTD_HOTPLUG_LONG_DETECT   (1 << 17)
@@ -2917,6 +2918,7 @@
 #define PORTC_PULSE_DURATION_4_5ms  (1 << 10)
 #define PORTC_PULSE_DURATION_6ms(2 << 10)
 #define PORTC_PULSE_DURATION_100ms  (3 << 10)
+#define PORTC_PULSE_DURATION_MASK  (3 << 10)
 #define PORTC_HOTPLUG_NO_DETECT (0)
 #define PORTC_HOTPLUG_SHORT_DETECT  (1 << 8)
 #define PORTC_HOTPLUG_LONG_DETECT   (1 << 9)
@@ -2925,6 +2927,7 @@
 #define PORTB_PULSE_DURATION_4_5ms  (1 << 2)
 #define PORTB_PULSE_DURATION_6ms(2 << 2)
 #define PORTB_PULSE_DURATION_100ms  (3 << 2)
+#define PORTB_PULSE_DURATION_MASK  (3 << 2)
 #define PORTB_HOTPLUG_NO_DETECT (0)
 #define PORTB_HOTPLUG_SHORT_DETECT  (1 << 0)
 #define PORTB_HOTPLUG_LONG_DETECT   (1 << 1)
-- 
1.7.6.3

[PATCH] drm/i915: Enable digital port hotplug on PCH systems

[Keith Packard]

We were relying on the BIOS to set these bits, which doesn't always
happen.

Signed-off-by: Keith Packard kei...@keithp.com
---

v2: set the hotplug bits in the irq post-install hook, just
like we do for pre-PCH hardware.

 drivers/gpu/drm/i915/i915_irq.c |   24 
 drivers/gpu/drm/i915/i915_reg.h |5 -
 2 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9cbb0cd..c22823b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1777,6 +1777,26 @@ static void ironlake_irq_preinstall(struct drm_device 
*dev)
POSTING_READ(SDEIER);
 }
 
+/*
+ * Enable digital hotplug on the PCH, and configure the DP short pulse
+ * duration to 2ms (which is the minimum in the Display Port spec)
+ *
+ * This register is the same on all known PCH chips.
+ */
+
+static void ironlake_enable_pch_hotplug(struct drm_device *dev)
+{
+   drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
+   u32 hotplug;
+
+   hotplug = I915_READ(PCH_PORT_HOTPLUG);
+   hotplug = 
~(PORTD_PULSE_DURATION_MASK|PORTC_PULSE_DURATION_MASK|PORTB_PULSE_DURATION_MASK);
+   hotplug |= PORTD_HOTPLUG_ENABLE | PORTD_PULSE_DURATION_2ms;
+   hotplug |= PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_2ms;
+   hotplug |= PORTB_HOTPLUG_ENABLE | PORTB_PULSE_DURATION_2ms;
+   I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
+}
+
 static int ironlake_irq_postinstall(struct drm_device *dev)
 {
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
@@ -1839,6 +1859,8 @@ static int ironlake_irq_postinstall(struct drm_device 
*dev)
I915_WRITE(SDEIER, hotplug_mask);
POSTING_READ(SDEIER);
 
+   ironlake_enable_pch_hotplug(dev);
+
if (IS_IRONLAKE_M(dev)) {
/* Clear  enable PCU event interrupts */
I915_WRITE(DEIIR, DE_PCU_EVENT);
@@ -1896,6 +1918,8 @@ static int ivybridge_irq_postinstall(struct drm_device 
*dev)
I915_WRITE(SDEIER, hotplug_mask);
POSTING_READ(SDEIER);
 
+   ironlake_enable_pch_hotplug(dev);
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 542453f..b7fbb74 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2903,12 +2903,13 @@
 #define SDEIER  0xc400c
 
 /* digital port hotplug */
-#define PCH_PORT_HOTPLUG0xc4030
+#define PCH_PORT_HOTPLUG0xc4030/* SHOTPLUG_CTL */
 #define PORTD_HOTPLUG_ENABLE(1  20)
 #define PORTD_PULSE_DURATION_2ms(0)
 #define PORTD_PULSE_DURATION_4_5ms  (1  18)
 #define PORTD_PULSE_DURATION_6ms(2  18)
 #define PORTD_PULSE_DURATION_100ms  (3  18)
+#define PORTD_PULSE_DURATION_MASK  (3  18)
 #define PORTD_HOTPLUG_NO_DETECT (0)
 #define PORTD_HOTPLUG_SHORT_DETECT  (1  16)
 #define PORTD_HOTPLUG_LONG_DETECT   (1  17)
@@ -2917,6 +2918,7 @@
 #define PORTC_PULSE_DURATION_4_5ms  (1  10)
 #define PORTC_PULSE_DURATION_6ms(2  10)
 #define PORTC_PULSE_DURATION_100ms  (3  10)
+#define PORTC_PULSE_DURATION_MASK  (3  10)
 #define PORTC_HOTPLUG_NO_DETECT (0)
 #define PORTC_HOTPLUG_SHORT_DETECT  (1  8)
 #define PORTC_HOTPLUG_LONG_DETECT   (1  9)
@@ -2925,6 +2927,7 @@
 #define PORTB_PULSE_DURATION_4_5ms  (1  2)
 #define PORTB_PULSE_DURATION_6ms(2  2)
 #define PORTB_PULSE_DURATION_100ms  (3  2)
+#define PORTB_PULSE_DURATION_MASK  (3  2)
 #define PORTB_HOTPLUG_NO_DETECT (0)
 #define PORTB_HOTPLUG_SHORT_DETECT  (1  0)
 #define PORTB_HOTPLUG_LONG_DETECT   (1  1)
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Proposal for a low-level Linux display framework

[Keith Packard]

On Tue, 20 Sep 2011 10:29:23 +0200, Patrik Jakobsson 
patrik.r.jakobs...@gmail.com wrote:

 It would be nice to have a model that fits both DSI and SDVO, and the option
 to configure some of it from userspace.

 I thought the purpose of drm_encoder was to abstract hardware like this?

SDVO is entirely hidden by the drm_encoder interface; some of the
controls (like TV encoder parameters) are exposed through DRM
properties, others are used in the basic configuration of the device.

I'm not sure we need a new abstraction that subsumes both DSI and SDVO,
but we may need a DSI library that can be used by both DRM and other
parts of the kernel.

-- 
keith.pack...@intel.com


pgpVNIHrOw0qF.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/i915: Enable digital port hotplug on PCH systems

[Keith Packard]

On Tue, 20 Sep 2011 17:24:21 +0100, Chris Wilson ch...@chris-wilson.co.uk 
wrote:
 On Tue, 20 Sep 2011 08:47:21 -0700, Keith Packard kei...@keithp.com wrote:
  We were relying on the BIOS to set these bits, which doesn't always
  happen.
 
 Do we need to clear IRQ bits on uninstall, for example to prevent
 Interrupt 19: no one cared! or is that due to a different cause?

Probably a good plan, but of course that's separate from turning on the
hotplug hardware in the PCH. Something like:

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c22823b..adeab2a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2044,6 +2044,10 @@ static void ironlake_irq_uninstall(struct drm_device 
*dev)
I915_WRITE(GTIMR, 0x);
I915_WRITE(GTIER, 0x0);
I915_WRITE(GTIIR, I915_READ(GTIIR));
+
+   I915_WRITE(SDEIMR, 0x);
+   I915_WRITE(SDEIER, 0x0);
+   I915_WRITE(SDEIIR, I915_READ(SDEIIR));
 }
 
 static void i915_driver_irq_uninstall(struct drm_device * dev)

 More of a general question really. The patch itself looks good and
 matches against the spec afaics.

I was amused to learn that the BIOS on my machine actually turns on the
hotplug hardware for no good reason, so things 'worked' without this
code on many machines. I wonder how many DP/DVI/HDMI machines aren't
actually doing hotplug today...

-- 
keith.pack...@intel.com


pgpPwhn4MT97d.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 6/9] drm/i915: Make sure eDP power is on before using aux channel

[Keith Packard]

On Wed, 21 Sep 2011 09:20:01 +0530, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 This one mixes up lots of cleanups plus the EDID read with the power
 changes.

I think the cleanups are;

 1) edp checks inside vdd_on and vdd_off to make the other code a bit
easier to read.

 2) Hold VDD on until the end of dp_commit. I think this isn't necessary
and could be removed -- once the panel is on, we don't need to hold
vdd up.

 3) Hold VDD up through the whole dpms sequence. Also probably
unnecessary.

 4) Move intel_dp_i2c_init past the computation of the power sequencing
delays. Necessary as i2c_init makes an i2c transaction, thus
powering up the panel.

I can remove the middle two and split the others out if you like.

 I'm worried about the VDD smashing as well; we have lots of
 bugs in the PPS hardware around VDD vs full PPS.

Are you worried that we should never have VDD up while running a panel
power sequence? As far as I can tell from the eDP specs, bringing VDD up
is part of the normal PPS, and the delay from VDD up to other panel
sequencing is shorter (T1+T2) than the delay to start using the aux channel
that I used in the later patch (T1+T2+T3).

One thing I learned for certain -- we don't want a synchronous delay
between turning the panel off and back on. The required interval between
these two operations is in units of 100ms; my machine spends 600ms doing
this; if we end up doing this in the middle of a mode set, it's gonna be
painful.

Can we replace any of the current VDD hacks with a full PPS? I can
easily imagine moving the call to ironlake_edp_panel_on to
intel_dp_prepare, except that if if the mode_set fails, dp_commit will
not be called (that looks like a potential source of failure at the DRM
level to me).

Getting the panel turned on complete as early as possible seems like a
good idea, instead of fussing around with the VDD force bits.

Alternatively, we can eliminate use of the VDD force hack and do a full
PPS and simply use the delayed work proc to turn that off when the
screen goes idle.

 We need to make sure appropriate delays are in place when
 transitioning from one to another.

As you can see, I stuck 1000ms delays in the vdd_on and vdd_off
functions to ensure that there was 'sufficient' delay in these cases. I
didn't touch the existing delays for regular panel_on/panel_off. I think
that the later patch, which computes 'correct' delays and applies those
uniformly should ensure that we're always waiting long enough.

The later patches track the jiffies value when the panel was turned off
and then wait the appropriate amount of time before turning it back
on.

 In what paths are we trying to do accesses without power applied?
 Looks like mainly edid?

(the previously broken cases are marked with '*')

* intel_dp_i2c_init

This probes the i2c bus to see if there's someone home. Failing
to have VDD up at this point causes a timeout, but no other
obvious problem.

As you can see, I moved the call to this function after the
computation of the power sequencing delay values.

* intel_dp_prepare

This wakes up the eDP panel out of a low power DPMS state. I
think I'd actually like to leave VDD high starting at this point
and ending after the panel is running.

intel_dp_commit

Calls intel_dp_start_link_train. before ironlake_edp_panel_on.
I think the call to ironlake_edp_panel_vdd_off could move back
where it was, right after ironlake_edp_panel_on.

intel_dp_dpms

*   In the DPMS_ON case, this calls intel_dp_sink_dpms and
intel_dp_start_link_train before calling
ironlake_edp_panel_on. I wonder if we shouldn't just turn the
panel on instead?

In the DPMS_OFF case, I don't think we need to mess with vdd at
all; the panel is definitely running until
ironlake_edp_panel_off is called, at which point we're done
using the aux channel.

* intel_dp_get_edid,
* intel_dp_get_edid_modes

These are called while the display is off (sometimes).

 I see the next patch handles the timing stuff, I assume it's ok.

I hope so; the actual timing values required by the eDP spec aren't
available, so I fudged by choosing ones that were at least as long.

Thanks for taking a look at the code.

-- 
keith.pack...@intel.com


pgpCGr2jh5E2W.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 9/9] drm/i915: Disable eDP VDD in a delayed work proc instead of synchronously

[Keith Packard]

On Wed, 21 Sep 2011 09:47:59 +0530, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 I'm worried this makes our PPS even more complex and hard to follow.
 I'd rather see VDD AUX applied only when we need it (dpms, mode set and
 detect; for hotplug we can assume the panel is alive) and that we
 carefully disable it after waiting for a time after a full PPS on
 sequence when doing a mode set or dpms on command.

You'll see the fairly long list of places where VDD AUX is needed in my
other reply; a couple of them could probably be fixed by moving the PPS
calls around.

 Having all the power stuff at the highest levels would be clearest I
 think.

Yes, making it cleaner would help a ton. There are some basic problems
with the DRM API that make this hard though -- intel_dp_prepare may not
ever be followed by a call to intel_dp_commit. That's why I had the VDD
AUX stuff get turned off by a delayed work proc instead.

Also, leaving VDD AUX high after EDID is fetched means that we can start
the mode setting immediately, rather than having to wait for the
power-off/power-on delay (which is really long).

What we could do is force VDD AUX off after the panel gets turned on;
that would ensure that turning the panel off would actually turn the
power off, rather than having VDD stay high for some time after that.

-- 
keith.pack...@intel.com


pgpFq0JraTovK.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/i915: FBC off for ironlake, otherwise on by default

[Keith Packard]

Make the default FBC behaviour chipset specific, allowing us to turn
it on by default for everything except Ironlake where it has been
seen to cause trouble with screen updates.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/i915_drv.c  |4 ++--
 drivers/gpu/drm/i915/intel_display.c |   10 +-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ce045a8..f07e425 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -67,11 +67,11 @@ module_param_named(i915_enable_rc6, i915_enable_rc6, int, 
0600);
 MODULE_PARM_DESC(i915_enable_rc6,
"Enable power-saving render C-state 6 (default: true)");

-unsigned int i915_enable_fbc __read_mostly = 1;
+unsigned int i915_enable_fbc __read_mostly = -1;
 module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600);
 MODULE_PARM_DESC(i915_enable_fbc,
"Enable frame buffer compression for power savings "
-   "(default: false)");
+   "(default: -1 (use per-chip default))");

 unsigned int i915_lvds_downclock __read_mostly = 0;
 module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9fb4a40..bc05deb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1799,6 +1799,7 @@ static void intel_update_fbc(struct drm_device *dev)
struct drm_framebuffer *fb;
struct intel_framebuffer *intel_fb;
struct drm_i915_gem_object *obj;
+   int enable_fbc;

DRM_DEBUG_KMS("\n");

@@ -1839,7 +1840,14 @@ static void intel_update_fbc(struct drm_device *dev)
intel_fb = to_intel_framebuffer(fb);
obj = intel_fb->obj;

-   if (!i915_enable_fbc) {
+   enable_fbc = i915_enable_fbc;
+   if (enable_fbc < 0) {
+   DRM_DEBUG_KMS("fbc set to per-chip default\n");
+   enable_fbc = 1;
+   if (INTEL_INFO(dev)->gen == 5)
+   enable_fbc = 0;
+   }
+   if (!enable_fbc) {
DRM_DEBUG_KMS("fbc disabled per module param (default off)\n");
dev_priv->no_fbc_reason = FBC_MODULE_PARAM;
goto out_disable;
-- 
1.7.6.3

drm/i915 git repository at freedesktop.org

[Keith Packard]

I've created a (temporary?) git repository for the drm/i915 driver at

git://people.freedesktop.org/~keithp/linux

This has the drm-intel-next and drm-intel-fixes branches, and may also
have occasional temporary branches with code which has not been merged yet.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PULL] drm-intel-next

[Keith Packard]

This is a single patch which cleans up almost all of the whitespace
errors in the i915 driver. It currently merges cleanly with your fdo
drm-core-next tree.

I've checked this patch quite carefully, examining the .o files with
objdump -s to make sure nothing significant changed. The only thing that
found was a couple of debug messages which had blank space before newlines.

The following changes since commit b6fd41e29dea9c6753b1843a77e50433e6123bcb:

  Linux 3.1-rc6 (2011-09-12 14:02:02 -0700)

are available in the git repository at:
  git://people.freedesktop.org/~keithp/linux drm-intel-next

Akshay Joshi (1):
  Drivers: i915: Fix all space related issues.

 drivers/gpu/drm/i915/dvo_ch7017.c   |2 +-
 drivers/gpu/drm/i915/dvo_ch7xxx.c   |4 +-
 drivers/gpu/drm/i915/dvo_ivch.c |6 +-
 drivers/gpu/drm/i915/dvo_sil164.c   |2 +-
 drivers/gpu/drm/i915/dvo_tfp410.c   |   14 +-
 drivers/gpu/drm/i915/i915_debugfs.c |   38 +-
 drivers/gpu/drm/i915/i915_dma.c |   44 ++--
 drivers/gpu/drm/i915/i915_drv.c |   16 +-
 drivers/gpu/drm/i915/i915_drv.h |   70 ++--
 drivers/gpu/drm/i915/i915_gem.c |   12 +-
 drivers/gpu/drm/i915/i915_gem_debug.c   |2 +-
 drivers/gpu/drm/i915/i915_gem_evict.c   |2 +-
 drivers/gpu/drm/i915/i915_irq.c |6 +-
 drivers/gpu/drm/i915/i915_mem.c |   14 +-
 drivers/gpu/drm/i915/i915_reg.h |8 +-
 drivers/gpu/drm/i915/i915_suspend.c |8 +-
 drivers/gpu/drm/i915/i915_trace.h   |   46 ++--
 drivers/gpu/drm/i915/intel_acpi.c   |2 +-
 drivers/gpu/drm/i915/intel_bios.c   |4 +-
 drivers/gpu/drm/i915/intel_bios.h   |2 +-
 drivers/gpu/drm/i915/intel_crt.c|2 +-
 drivers/gpu/drm/i915/intel_display.c|  222 ++--
 drivers/gpu/drm/i915/intel_dp.c |   26 +-
 drivers/gpu/drm/i915/intel_drv.h|   12 +-
 drivers/gpu/drm/i915/intel_opregion.c   |   90 +++---
 drivers/gpu/drm/i915/intel_overlay.c|  146 
 drivers/gpu/drm/i915/intel_panel.c  |6 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c |   76 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.h |8 +-
 drivers/gpu/drm/i915/intel_sdvo.c   |  228 +++---
 drivers/gpu/drm/i915/intel_sdvo_regs.h  |  558 +++---
 drivers/gpu/drm/i915/intel_tv.c |   58 ++--
 32 files changed, 867 insertions(+), 867 deletions(-)

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PATCH 9/9] drm/i915: Disable eDP VDD in a delayed work proc instead of synchronously

[Keith Packard]

There's no good reason to turn off the eDP force VDD bit synchronously
while probing devices; that just sticks a huge delay into all mode
setting paths. Instead, queue a delayed work proc to disable the VDD
force bit and then remember when that fires to ensure that the
appropriate delay is respected before trying to turn it back on.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_dp.c |   93 +-
 1 files changed, 80 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5bc30f9..8130e82 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -62,6 +62,9 @@ struct intel_dp {
int panel_power_up_delay;
int panel_power_down_delay;
struct drm_display_mode *panel_fixed_mode;  /* for eDP */
+   struct delayed_work panel_vdd_work;
+   bool panel_vdd_force;
+   unsigned long panel_off_jiffies;
 };

 /**
@@ -834,6 +837,23 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct 
drm_display_mode *mode,
}
 }

+static void ironlake_wait_panel_off(struct intel_dp *intel_dp)
+{
+   unsigned long   off_time;
+   unsigned long   delay;
+   DRM_DEBUG_KMS("Wait for panel power off time\n");
+   off_time = intel_dp->panel_off_jiffies + 
msecs_to_jiffies(intel_dp->panel_power_down_delay);
+   if (time_after(jiffies, off_time)) {
+   DRM_DEBUG_KMS("Time already passed");
+   return;
+   }
+   delay = jiffies_to_msecs(off_time - jiffies);
+   if (delay > intel_dp->panel_power_down_delay)
+   delay = intel_dp->panel_power_down_delay;
+   DRM_DEBUG_KMS("Waiting an additional %ld ms\n", delay);
+   msleep(delay);
+}
+
 static void ironlake_edp_panel_vdd_on(struct intel_dp *intel_dp)
 {
struct drm_device *dev = intel_dp->base.base.dev;
@@ -843,13 +863,18 @@ static void ironlake_edp_panel_vdd_on(struct intel_dp 
*intel_dp)
if (!is_edp(intel_dp))
return;
DRM_DEBUG_KMS("Turn eDP VDD on\n");
-   /*
-* If the panel wasn't on, make sure there's not a currently
-* active PP sequence before enabling AUX VDD.
-*/
-   pp_status = I915_READ(PCH_PP_STATUS);

+   WARN(intel_dp->panel_vdd_force,
+"eDP VDD already forced on\n");
+
+   intel_dp->panel_vdd_force = true;
pp = I915_READ(PCH_PP_CONTROL);
+   if (pp & EDP_FORCE_VDD) {
+   DRM_DEBUG_KMS("eDP VDD already on\n");
+   return;
+   }
+
+   ironlake_wait_panel_off(intel_dp);
pp &= ~PANEL_UNLOCK_MASK;
pp |= PANEL_UNLOCK_REGS;
pp |= EDP_FORCE_VDD;
@@ -857,21 +882,23 @@ static void ironlake_edp_panel_vdd_on(struct intel_dp 
*intel_dp)
POSTING_READ(PCH_PP_CONTROL);
DRM_DEBUG_KMS("PCH_PP_STATUS: 0x%08x PCH_PP_CONTROL: 0x%08x\n",
  I915_READ(PCH_PP_STATUS), I915_READ(PCH_PP_CONTROL));
+
+   /*
+* If the panel wasn't on, delay before accessing aux channel
+*/
+   pp_status = I915_READ(PCH_PP_STATUS);
if (!(pp_status & PP_ON)) {
+   DRM_DEBUG_KMS("eDP was not running\n");
msleep(intel_dp->panel_power_up_delay);
-   DRM_DEBUG_KMS("eDP VDD was not on\n");
}
 }

-static void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp)
+static void ironlake_panel_vdd_delayed_off(struct intel_dp *intel_dp)
 {
struct drm_device *dev = intel_dp->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
u32 pp;

-   if (!is_edp(intel_dp))
-   return;
-   DRM_DEBUG_KMS("Turn eDP VDD off\n");
pp = I915_READ(PCH_PP_CONTROL);
pp &= ~PANEL_UNLOCK_MASK;
pp |= PANEL_UNLOCK_REGS;
@@ -882,7 +909,37 @@ static void ironlake_edp_panel_vdd_off(struct intel_dp 
*intel_dp)
/* Make sure sequencer is idle before allowing subsequent activity */
DRM_DEBUG_KMS("PCH_PP_STATUS: 0x%08x PCH_PP_CONTROL: 0x%08x\n",
  I915_READ(PCH_PP_STATUS), I915_READ(PCH_PP_CONTROL));
-   msleep(intel_dp->panel_power_down_delay);
+   intel_dp->panel_off_jiffies = jiffies;
+}
+
+static void ironlake_panel_vdd_work(struct work_struct *__work)
+{
+   struct intel_dp *intel_dp = container_of(to_delayed_work(__work),
+struct intel_dp, 
panel_vdd_work);
+   struct drm_device *dev = intel_dp->base.base.dev;
+
+   mutex_lock(>struct_mutex);
+   if (!intel_dp->panel_vdd_force)
+   ironlake_panel_vdd_delayed_off(intel_dp);
+   mutex_unlock(>struct_mutex);
+}
+
+static void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp)
+{
+   if (!is

[PATCH 8/9] drm/i915: Move eDP panel fixed mode from dev_priv to intel_dp

[Keith Packard]

This value doesn't come directly from the VBT, and so is rather
specific to the particular DP output.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/i915_drv.h |1 -
 drivers/gpu/drm/i915/intel_dp.c |   35 ---
 2 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bcdf58b..e6dd19e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -347,7 +347,6 @@ typedef struct drm_i915_private {
/* LVDS info */
int backlight_level;  /* restore backlight to this value */
bool backlight_enabled;
-   struct drm_display_mode *panel_fixed_mode;
struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f1d6105..5bc30f9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -61,6 +61,7 @@ struct intel_dp {
uint8_t link_status[DP_LINK_STATUS_SIZE];
int panel_power_up_delay;
int panel_power_down_delay;
+   struct drm_display_mode *panel_fixed_mode;  /* for eDP */
 };

 /**
@@ -202,16 +203,14 @@ intel_dp_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
 {
struct intel_dp *intel_dp = intel_attached_dp(connector);
-   struct drm_device *dev = connector->dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
int max_link_clock = 
intel_dp_link_clock(intel_dp_max_link_bw(intel_dp));
int max_lanes = intel_dp_max_lane_count(intel_dp);

-   if (is_edp(intel_dp) && dev_priv->panel_fixed_mode) {
-   if (mode->hdisplay > dev_priv->panel_fixed_mode->hdisplay)
+   if (is_edp(intel_dp) && intel_dp->panel_fixed_mode) {
+   if (mode->hdisplay > intel_dp->panel_fixed_mode->hdisplay)
return MODE_PANEL;

-   if (mode->vdisplay > dev_priv->panel_fixed_mode->vdisplay)
+   if (mode->vdisplay > intel_dp->panel_fixed_mode->vdisplay)
return MODE_PANEL;
}

@@ -630,22 +629,21 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct 
drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
 {
struct drm_device *dev = encoder->dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
int lane_count, clock;
int max_lane_count = intel_dp_max_lane_count(intel_dp);
int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 
0;
static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };

-   if (is_edp(intel_dp) && dev_priv->panel_fixed_mode) {
-   intel_fixed_panel_mode(dev_priv->panel_fixed_mode, 
adjusted_mode);
+   if (is_edp(intel_dp) && intel_dp->panel_fixed_mode) {
+   intel_fixed_panel_mode(intel_dp->panel_fixed_mode, 
adjusted_mode);
intel_pch_panel_fitting(dev, DRM_MODE_SCALE_FULLSCREEN,
mode, adjusted_mode);
/*
 * the mode->clock is used to calculate the Data M/N
 * of the pipe. For the eDP the fixed clock should be used.
 */
-   mode->clock = dev_priv->panel_fixed_mode->clock;
+   mode->clock = intel_dp->panel_fixed_mode->clock;
}

for (lane_count = 1; lane_count <= max_lane_count; lane_count <<= 1) {
@@ -1812,35 +1810,34 @@ static int intel_dp_get_modes(struct drm_connector 
*connector)

ret = intel_dp_get_edid_modes(connector, _dp->adapter);
if (ret) {
-   if (is_edp(intel_dp) && !dev_priv->panel_fixed_mode) {
+   if (is_edp(intel_dp) && !intel_dp->panel_fixed_mode) {
struct drm_display_mode *newmode;
list_for_each_entry(newmode, >probed_modes,
head) {
-   if (newmode->type & DRM_MODE_TYPE_PREFERRED) {
-   dev_priv->panel_fixed_mode =
+   if ((newmode->type & DRM_MODE_TYPE_PREFERRED)) {
+   intel_dp->panel_fixed_mode =
drm_mode_duplicate(dev, 
newmode);
break;
}
}
}
-
return ret;
}

/* if eDP has no EDID, try to use fixed panel mode from VBT */
if (

[PATCH 7/9] drm/i915: Correct eDP panel power sequencing delay computations

[Keith Packard]

Store the panel power sequencing delays in the dp private structure,
rather than the global device structure. Who knows, maybe we'll get
more than one eDP device in the future.

Look at both the current hardware register settings and the VBT
specified panel power sequencing timings. Use the maximum of the two
delays, to make sure things work reliably. If there is no VBT data,
then those values will be initialized to zero, so we'll just use the
values as programmed in the hardware.

This patch computes power-up and power-down delays, rather than using
portions of the appropriate delay values as found in the hardware. The
eDP specified delay between raising VCC and communicating over the aux
channel includes both the power rise time (T1) and the aux channel
communication delay (T3). The eDP specified delay between powering
down the device and powering it back up includes both the power fall
time (T11) and the device idle time (T12).

[PATCH 6/9] drm/i915: Make sure eDP power is on before using aux channel

[Keith Packard]

The eDP panel may not be able to respond to aux channel communications
unless it has power supplied. During mode setting, power may be
cut-off during panel power sequencing. Make sure that any aux channel
communications will work by forcing vdd power active as needed.

This also delays after turning power on and off to ensure that the
panel is keeping up.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_dp.c |   84 +-
 1 files changed, 64 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a983d0f..41b1e05 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -595,10 +595,15 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
return -EREMOTEIO;
 }

+static void ironlake_edp_panel_vdd_on(struct intel_dp *intel_dp);
+static void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp);
+
 static int
 intel_dp_i2c_init(struct intel_dp *intel_dp,
  struct intel_connector *intel_connector, const char *name)
 {
+   int ret;
+
DRM_DEBUG_KMS("i2c_init %s\n", name);
intel_dp->algo.running = false;
intel_dp->algo.address = 0;
@@ -612,7 +617,10 @@ intel_dp_i2c_init(struct intel_dp *intel_dp,
intel_dp->adapter.algo_data = _dp->algo;
intel_dp->adapter.dev.parent = _connector->base.kdev;

-   return i2c_dp_aux_add_bus(_dp->adapter);
+   ironlake_edp_panel_vdd_on(intel_dp);
+   ret = i2c_dp_aux_add_bus(_dp->adapter);
+   ironlake_edp_panel_vdd_off(intel_dp);
+   return ret;
 }

 static bool
@@ -830,14 +838,20 @@ static void ironlake_edp_panel_vdd_on(struct intel_dp 
*intel_dp)
 {
struct drm_device *dev = intel_dp->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
-   u32 pp;
+   u32 pp, pp_status;

+   if (!is_edp(intel_dp))
+   return;
+   DRM_DEBUG_KMS("Turn eDP VDD on\n");
/*
 * If the panel wasn't on, make sure there's not a currently
 * active PP sequence before enabling AUX VDD.
 */
-   if (!(I915_READ(PCH_PP_STATUS) & PP_ON))
+   pp_status = I915_READ(PCH_PP_STATUS);
+   if (!(pp_status & PP_ON)) {
+   DRM_DEBUG_KMS("eDP VDD was not on\n");
msleep(dev_priv->panel_t3);
+   }

pp = I915_READ(PCH_PP_CONTROL);
pp &= ~PANEL_UNLOCK_MASK;
@@ -845,6 +859,9 @@ static void ironlake_edp_panel_vdd_on(struct intel_dp 
*intel_dp)
pp |= EDP_FORCE_VDD;
I915_WRITE(PCH_PP_CONTROL, pp);
POSTING_READ(PCH_PP_CONTROL);
+   DRM_DEBUG_KMS("PCH_PP_STATUS: 0x%08x PCH_PP_CONTROL: 0x%08x\n",
+ I915_READ(PCH_PP_STATUS), I915_READ(PCH_PP_CONTROL));
+   msleep(1000);
 }

 static void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp)
@@ -853,6 +870,9 @@ static void ironlake_edp_panel_vdd_off(struct intel_dp 
*intel_dp)
struct drm_i915_private *dev_priv = dev->dev_private;
u32 pp;

+   if (!is_edp(intel_dp))
+   return;
+   DRM_DEBUG_KMS("Turn eDP VDD off\n");
pp = I915_READ(PCH_PP_CONTROL);
pp &= ~PANEL_UNLOCK_MASK;
pp |= PANEL_UNLOCK_REGS;
@@ -862,6 +882,9 @@ static void ironlake_edp_panel_vdd_off(struct intel_dp 
*intel_dp)

/* Make sure sequencer is idle before allowing subsequent activity */
msleep(dev_priv->panel_t12);
+   DRM_DEBUG_KMS("PCH_PP_STATUS: 0x%08x PCH_PP_CONTROL: 0x%08x\n",
+ I915_READ(PCH_PP_STATUS), I915_READ(PCH_PP_CONTROL));
+   msleep(1000);
 }

 /* Returns true if the panel was already on when called */
@@ -1016,7 +1039,9 @@ static void intel_dp_prepare(struct drm_encoder *encoder)
struct drm_device *dev = encoder->dev;

/* Wake up the sink first */
+   ironlake_edp_panel_vdd_on(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
+   ironlake_edp_panel_vdd_off(intel_dp);

if (is_edp(intel_dp)) {
ironlake_edp_backlight_off(dev);
@@ -1034,21 +1059,17 @@ static void intel_dp_commit(struct drm_encoder *encoder)
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
struct drm_device *dev = encoder->dev;

-   if (is_edp(intel_dp))
-   ironlake_edp_panel_vdd_on(intel_dp);
-
+   ironlake_edp_panel_vdd_on(intel_dp);
intel_dp_start_link_train(intel_dp);

-   if (is_edp(intel_dp)) {
+   if (is_edp(intel_dp))
ironlake_edp_panel_on(intel_dp);
-   ironlake_edp_panel_vdd_off(intel_dp);
-   }

intel_dp_complete_link_train(intel_dp);

if (is_edp(intel_dp))
ironlake_edp_backlight_on(dev);
-
+   ironlake_edp_panel_vdd_off(intel_dp);
intel_dp->dpms_mode = DRM_MODE_DPMS_ON;
 }

[PATCH 5/9] drm/i915: Unlock PCH_PP_CONTROL always

[Keith Packard]

Avoid any question about locked registers by just writing the unlock
pattern with every write to the register.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/i915_reg.h |1 +
 drivers/gpu/drm/i915/intel_dp.c |   14 +-
 2 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b7fbb74..5596e8e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3311,6 +3311,7 @@
 #define PCH_PP_STATUS  0xc7200
 #define PCH_PP_CONTROL 0xc7204
 #define  PANEL_UNLOCK_REGS (0xabcd << 16)
+#define  PANEL_UNLOCK_MASK (0x << 16)
 #define  EDP_FORCE_VDD (1 << 3)
 #define  EDP_BLC_ENABLE(1 << 2)
 #define  PANEL_POWER_RESET (1 << 1)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3b29a6f..a983d0f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -840,6 +840,8 @@ static void ironlake_edp_panel_vdd_on(struct intel_dp 
*intel_dp)
msleep(dev_priv->panel_t3);

pp = I915_READ(PCH_PP_CONTROL);
+   pp &= ~PANEL_UNLOCK_MASK;
+   pp |= PANEL_UNLOCK_REGS;
pp |= EDP_FORCE_VDD;
I915_WRITE(PCH_PP_CONTROL, pp);
POSTING_READ(PCH_PP_CONTROL);
@@ -852,6 +854,8 @@ static void ironlake_edp_panel_vdd_off(struct intel_dp 
*intel_dp)
u32 pp;

pp = I915_READ(PCH_PP_CONTROL);
+   pp &= ~PANEL_UNLOCK_MASK;
+   pp |= PANEL_UNLOCK_REGS;
pp &= ~EDP_FORCE_VDD;
I915_WRITE(PCH_PP_CONTROL, pp);
POSTING_READ(PCH_PP_CONTROL);
@@ -871,13 +875,15 @@ static bool ironlake_edp_panel_on (struct intel_dp 
*intel_dp)
return true;

pp = I915_READ(PCH_PP_CONTROL);
+   pp &= ~PANEL_UNLOCK_MASK;
+   pp |= PANEL_UNLOCK_REGS;

/* ILK workaround: disable reset around power sequence */
pp &= ~PANEL_POWER_RESET;
I915_WRITE(PCH_PP_CONTROL, pp);
POSTING_READ(PCH_PP_CONTROL);

-   pp |= PANEL_UNLOCK_REGS | POWER_TARGET_ON;
+   pp |= POWER_TARGET_ON;
I915_WRITE(PCH_PP_CONTROL, pp);
POSTING_READ(PCH_PP_CONTROL);

@@ -900,6 +906,8 @@ static void ironlake_edp_panel_off (struct drm_device *dev)
PP_CYCLE_DELAY_ACTIVE | PP_SEQUENCE_STATE_MASK;

pp = I915_READ(PCH_PP_CONTROL);
+   pp &= ~PANEL_UNLOCK_MASK;
+   pp |= PANEL_UNLOCK_REGS;

/* ILK workaround: disable reset around power sequence */
pp &= ~PANEL_POWER_RESET;
@@ -926,6 +934,8 @@ static void ironlake_edp_backlight_on (struct drm_device 
*dev)

DRM_DEBUG_KMS("\n");
pp = I915_READ(PCH_PP_CONTROL);
+   pp &= ~PANEL_UNLOCK_MASK;
+   pp |= PANEL_UNLOCK_REGS;
pp |= EDP_BLC_ENABLE;
I915_WRITE(PCH_PP_CONTROL, pp);
 }
@@ -937,6 +947,8 @@ static void ironlake_edp_backlight_off (struct drm_device 
*dev)

DRM_DEBUG_KMS("\n");
pp = I915_READ(PCH_PP_CONTROL);
+   pp &= ~PANEL_UNLOCK_MASK;
+   pp |= PANEL_UNLOCK_REGS;
pp &= ~EDP_BLC_ENABLE;
I915_WRITE(PCH_PP_CONTROL, pp);
 }
-- 
1.7.6.3

[PATCH 4/9] drm/i915: Check eDP power when doing aux channel communications

[Keith Packard]

Verify that the eDP VDD is on, either with the panel being on or with
the VDD force-on bit being set.

This demonstrates that in many instances, VDD is not on when needed,
which leads to failed EDID communications.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_dp.c |   22 ++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8ab2a88..3b29a6f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -279,6 +279,24 @@ intel_hrawclk(struct drm_device *dev)
}
 }

+static void
+intel_dp_check_edp(struct intel_dp *intel_dp)
+{
+   struct drm_device *dev = intel_dp->base.base.dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   u32 pp_status, pp_control;
+   if (!is_edp(intel_dp))
+   return;
+   pp_status = I915_READ(PCH_PP_STATUS);
+   pp_control = I915_READ(PCH_PP_CONTROL);
+   if ((pp_status & PP_ON) == 0 && (pp_control & EDP_FORCE_VDD) == 0) {
+   WARN(1, "eDP powered off while attempting aux channel 
communication.\n");
+   DRM_DEBUG_KMS("Status 0x%08x Control 0x%08x\n",
+ pp_status,
+ I915_READ(PCH_PP_CONTROL));
+   }
+}
+
 static int
 intel_dp_aux_ch(struct intel_dp *intel_dp,
uint8_t *send, int send_bytes,
@@ -295,6 +313,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
uint32_t aux_clock_divider;
int try, precharge;

+   intel_dp_check_edp(intel_dp);
/* The clock divider is based off the hrawclk,
 * and would like to run at 2MHz. So, take the
 * hrawclk value and divide by 2 and use that
@@ -408,6 +427,7 @@ intel_dp_aux_native_write(struct intel_dp *intel_dp,
int msg_bytes;
uint8_t ack;

+   intel_dp_check_edp(intel_dp);
if (send_bytes > 16)
return -1;
msg[0] = AUX_NATIVE_WRITE << 4;
@@ -450,6 +470,7 @@ intel_dp_aux_native_read(struct intel_dp *intel_dp,
uint8_t ack;
int ret;

+   intel_dp_check_edp(intel_dp);
msg[0] = AUX_NATIVE_READ << 4;
msg[1] = address >> 8;
msg[2] = address & 0xff;
@@ -493,6 +514,7 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
int reply_bytes;
int ret;

+   intel_dp_check_edp(intel_dp);
/* Set up the command byte */
if (mode & MODE_I2C_READ)
msg[0] = AUX_I2C_READ << 4;
-- 
1.7.6.3

[PATCH 3/9] drm/i915: Only use VBT panel mode on eDP if no EDID is found

[Keith Packard]

We're going to assume that EDID is more reliable than the VBT tables
for eDP panels, which is notably true on MacBook machines where the
VBT contains completely bogus data.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_dp.c |   20 ++--
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f0cfb6b..8ab2a88 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1748,7 +1748,16 @@ static int intel_dp_get_modes(struct drm_connector 
*connector)

/* if eDP has no EDID, try to use fixed panel mode from VBT */
if (is_edp(intel_dp)) {
-   if (dev_priv->panel_fixed_mode != NULL) {
+   /* initialize panel mode from VBT if available for eDP */
+   if (dev_priv->panel_fixed_mode == NULL && 
dev_priv->lfp_lvds_vbt_mode != NULL) {
+   dev_priv->panel_fixed_mode =
+   drm_mode_duplicate(dev, 
dev_priv->lfp_lvds_vbt_mode);
+   if (dev_priv->panel_fixed_mode) {
+   dev_priv->panel_fixed_mode->type |=
+   DRM_MODE_TYPE_PREFERRED;
+   }
+   }
+   if (dev_priv->panel_fixed_mode) {
struct drm_display_mode *mode;
mode = drm_mode_duplicate(dev, 
dev_priv->panel_fixed_mode);
drm_mode_probed_add(connector, mode);
@@ -2061,15 +2070,6 @@ intel_dp_init(struct drm_device *dev, int output_reg)
intel_encoder->hot_plug = intel_dp_hot_plug;

if (is_edp(intel_dp)) {
-   /* initialize panel mode from VBT if available for eDP */
-   if (dev_priv->lfp_lvds_vbt_mode) {
-   dev_priv->panel_fixed_mode =
-   drm_mode_duplicate(dev, 
dev_priv->lfp_lvds_vbt_mode);
-   if (dev_priv->panel_fixed_mode) {
-   dev_priv->panel_fixed_mode->type |=
-   DRM_MODE_TYPE_PREFERRED;
-   }
-   }
dev_priv->int_edp_connector = connector;
intel_panel_setup_backlight(dev);
}
-- 
1.7.6.3

[PATCH 2/9] drm/i915: Remove extra 300ms delay during eDP mode setting

[Keith Packard]

There's no reason to enforce a 300ms delay during eDP mode setting.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/intel_dp.c |7 ---
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 44fef5e..f0cfb6b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -903,13 +903,6 @@ static void ironlake_edp_backlight_on (struct drm_device 
*dev)
u32 pp;

DRM_DEBUG_KMS("\n");
-   /*
-* If we enable the backlight right away following a panel power
-* on, we may see slight flicker as the panel syncs with the eDP
-* link.  So delay a bit to make sure the image is solid before
-* allowing it to appear.
-*/
-   msleep(300);
pp = I915_READ(PCH_PP_CONTROL);
pp |= EDP_BLC_ENABLE;
I915_WRITE(PCH_PP_CONTROL, pp);
-- 
1.7.6.3

[PATCH 1/9] drm/i915: Enable digital port hotplug on PCH systems

[Keith Packard]

We were relying on the BIOS to set these bits, which doesn't always
happen.

Signed-off-by: Keith Packard 
---
 drivers/gpu/drm/i915/i915_reg.h  |5 -
 drivers/gpu/drm/i915/intel_display.c |   12 
 2 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 542453f..b7fbb74 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2903,12 +2903,13 @@
 #define SDEIER  0xc400c

 /* digital port hotplug */
-#define PCH_PORT_HOTPLUG0xc4030
+#define PCH_PORT_HOTPLUG0xc4030/* SHOTPLUG_CTL */
 #define PORTD_HOTPLUG_ENABLE(1 << 20)
 #define PORTD_PULSE_DURATION_2ms(0)
 #define PORTD_PULSE_DURATION_4_5ms  (1 << 18)
 #define PORTD_PULSE_DURATION_6ms(2 << 18)
 #define PORTD_PULSE_DURATION_100ms  (3 << 18)
+#define PORTD_PULSE_DURATION_MASK  (3 << 18)
 #define PORTD_HOTPLUG_NO_DETECT (0)
 #define PORTD_HOTPLUG_SHORT_DETECT  (1 << 16)
 #define PORTD_HOTPLUG_LONG_DETECT   (1 << 17)
@@ -2917,6 +2918,7 @@
 #define PORTC_PULSE_DURATION_4_5ms  (1 << 10)
 #define PORTC_PULSE_DURATION_6ms(2 << 10)
 #define PORTC_PULSE_DURATION_100ms  (3 << 10)
+#define PORTC_PULSE_DURATION_MASK  (3 << 10)
 #define PORTC_HOTPLUG_NO_DETECT (0)
 #define PORTC_HOTPLUG_SHORT_DETECT  (1 << 8)
 #define PORTC_HOTPLUG_LONG_DETECT   (1 << 9)
@@ -2925,6 +2927,7 @@
 #define PORTB_PULSE_DURATION_4_5ms  (1 << 2)
 #define PORTB_PULSE_DURATION_6ms(2 << 2)
 #define PORTB_PULSE_DURATION_100ms  (3 << 2)
+#define PORTB_PULSE_DURATION_MASK  (3 << 2)
 #define PORTB_HOTPLUG_NO_DETECT (0)
 #define PORTB_HOTPLUG_SHORT_DETECT  (1 << 0)
 #define PORTB_HOTPLUG_LONG_DETECT   (1 << 1)
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9fb4a40..54403dd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7151,6 +7151,18 @@ static void intel_setup_outputs(struct drm_device *dev)
I915_WRITE(PFIT_CONTROL, 0);
}

+   /* Enable digital port hotplug detect on PCH hardware */
+   if (HAS_PCH_SPLIT(dev)) {
+   u32 hotplug;
+
+   hotplug = I915_READ(PCH_PORT_HOTPLUG);
+   hotplug &= 
~(PORTD_PULSE_DURATION_MASK|PORTC_PULSE_DURATION_MASK|PORTB_PULSE_DURATION_MASK);
+   hotplug |= PORTD_HOTPLUG_ENABLE | PORTD_PULSE_DURATION_2ms;
+   hotplug |= PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_2ms;
+   hotplug |= PORTB_HOTPLUG_ENABLE | PORTB_PULSE_DURATION_2ms;
+   I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
+   }
+
if (HAS_PCH_SPLIT(dev)) {
dpd_is_edp = intel_dpd_is_edp(dev);

-- 
1.7.6.3

drm/i915: eDP cleanup patch series -- fixes SNB MacBook Air

[Keith Packard]

Here's a sequence of eDP patches which are necessary to make the
driver work on the Sandybridge MacBook Air

The key trick was to make sure that the eDP power was applied before
trying to communicate with the device.

Proposal for a low-level Linux display framework

[Keith Packard]

On Mon, 19 Sep 2011 09:33:34 +0300, Tomi Valkeinen  
wrote:

> I think it's a bit more complex than that. True, there are MIPI
> standards, for the video there are DPI, DBI, DSI, and for the commands
> there is DCS. And, as you mentioned, many panels need custom
> initialization, or support only parts of the DCS, or have other
> quirks.

So DSI is more like i2c than the DisplayPort aux channel or DDC. That
seems fine; you can create a DSI infrastructure like the i2c
infrastructure and then just have your display drivers use it to talk to
the panel. We might eventually end up with some shared DRM code to deal
with common DSI functions for display devices, like the EDID code
today, but that doesn't need to happen before you can write your first
DSI-using display driver.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Re: Proposal for a low-level Linux display framework

[Keith Packard]

On Mon, 19 Sep 2011 09:33:34 +0300, Tomi Valkeinen tomi.valkei...@ti.com 
wrote:

 I think it's a bit more complex than that. True, there are MIPI
 standards, for the video there are DPI, DBI, DSI, and for the commands
 there is DCS. And, as you mentioned, many panels need custom
 initialization, or support only parts of the DCS, or have other
 quirks.

So DSI is more like i2c than the DisplayPort aux channel or DDC. That
seems fine; you can create a DSI infrastructure like the i2c
infrastructure and then just have your display drivers use it to talk to
the panel. We might eventually end up with some shared DRM code to deal
with common DSI functions for display devices, like the EDID code
today, but that doesn't need to happen before you can write your first
DSI-using display driver.

-- 
keith.pack...@intel.com


pgppSisS0Hgz6.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

drm/i915: eDP cleanup patch series -- fixes SNB MacBook Air

[Keith Packard]

Here's a sequence of eDP patches which are necessary to make the
driver work on the Sandybridge MacBook Air

The key trick was to make sure that the eDP power was applied before
trying to communicate with the device.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/9] drm/i915: Enable digital port hotplug on PCH systems

[Keith Packard]

We were relying on the BIOS to set these bits, which doesn't always
happen.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/i915_reg.h  |5 -
 drivers/gpu/drm/i915/intel_display.c |   12 
 2 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 542453f..b7fbb74 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2903,12 +2903,13 @@
 #define SDEIER  0xc400c
 
 /* digital port hotplug */
-#define PCH_PORT_HOTPLUG0xc4030
+#define PCH_PORT_HOTPLUG0xc4030/* SHOTPLUG_CTL */
 #define PORTD_HOTPLUG_ENABLE(1  20)
 #define PORTD_PULSE_DURATION_2ms(0)
 #define PORTD_PULSE_DURATION_4_5ms  (1  18)
 #define PORTD_PULSE_DURATION_6ms(2  18)
 #define PORTD_PULSE_DURATION_100ms  (3  18)
+#define PORTD_PULSE_DURATION_MASK  (3  18)
 #define PORTD_HOTPLUG_NO_DETECT (0)
 #define PORTD_HOTPLUG_SHORT_DETECT  (1  16)
 #define PORTD_HOTPLUG_LONG_DETECT   (1  17)
@@ -2917,6 +2918,7 @@
 #define PORTC_PULSE_DURATION_4_5ms  (1  10)
 #define PORTC_PULSE_DURATION_6ms(2  10)
 #define PORTC_PULSE_DURATION_100ms  (3  10)
+#define PORTC_PULSE_DURATION_MASK  (3  10)
 #define PORTC_HOTPLUG_NO_DETECT (0)
 #define PORTC_HOTPLUG_SHORT_DETECT  (1  8)
 #define PORTC_HOTPLUG_LONG_DETECT   (1  9)
@@ -2925,6 +2927,7 @@
 #define PORTB_PULSE_DURATION_4_5ms  (1  2)
 #define PORTB_PULSE_DURATION_6ms(2  2)
 #define PORTB_PULSE_DURATION_100ms  (3  2)
+#define PORTB_PULSE_DURATION_MASK  (3  2)
 #define PORTB_HOTPLUG_NO_DETECT (0)
 #define PORTB_HOTPLUG_SHORT_DETECT  (1  0)
 #define PORTB_HOTPLUG_LONG_DETECT   (1  1)
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9fb4a40..54403dd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7151,6 +7151,18 @@ static void intel_setup_outputs(struct drm_device *dev)
I915_WRITE(PFIT_CONTROL, 0);
}
 
+   /* Enable digital port hotplug detect on PCH hardware */
+   if (HAS_PCH_SPLIT(dev)) {
+   u32 hotplug;
+
+   hotplug = I915_READ(PCH_PORT_HOTPLUG);
+   hotplug = 
~(PORTD_PULSE_DURATION_MASK|PORTC_PULSE_DURATION_MASK|PORTB_PULSE_DURATION_MASK);
+   hotplug |= PORTD_HOTPLUG_ENABLE | PORTD_PULSE_DURATION_2ms;
+   hotplug |= PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_2ms;
+   hotplug |= PORTB_HOTPLUG_ENABLE | PORTB_PULSE_DURATION_2ms;
+   I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
+   }
+
if (HAS_PCH_SPLIT(dev)) {
dpd_is_edp = intel_dpd_is_edp(dev);
 
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/9] drm/i915: Remove extra 300ms delay during eDP mode setting

[Keith Packard]

There's no reason to enforce a 300ms delay during eDP mode setting.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_dp.c |7 ---
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 44fef5e..f0cfb6b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -903,13 +903,6 @@ static void ironlake_edp_backlight_on (struct drm_device 
*dev)
u32 pp;
 
DRM_DEBUG_KMS(\n);
-   /*
-* If we enable the backlight right away following a panel power
-* on, we may see slight flicker as the panel syncs with the eDP
-* link.  So delay a bit to make sure the image is solid before
-* allowing it to appear.
-*/
-   msleep(300);
pp = I915_READ(PCH_PP_CONTROL);
pp |= EDP_BLC_ENABLE;
I915_WRITE(PCH_PP_CONTROL, pp);
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/9] drm/i915: Only use VBT panel mode on eDP if no EDID is found

[Keith Packard]

We're going to assume that EDID is more reliable than the VBT tables
for eDP panels, which is notably true on MacBook machines where the
VBT contains completely bogus data.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_dp.c |   20 ++--
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f0cfb6b..8ab2a88 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1748,7 +1748,16 @@ static int intel_dp_get_modes(struct drm_connector 
*connector)
 
/* if eDP has no EDID, try to use fixed panel mode from VBT */
if (is_edp(intel_dp)) {
-   if (dev_priv-panel_fixed_mode != NULL) {
+   /* initialize panel mode from VBT if available for eDP */
+   if (dev_priv-panel_fixed_mode == NULL  
dev_priv-lfp_lvds_vbt_mode != NULL) {
+   dev_priv-panel_fixed_mode =
+   drm_mode_duplicate(dev, 
dev_priv-lfp_lvds_vbt_mode);
+   if (dev_priv-panel_fixed_mode) {
+   dev_priv-panel_fixed_mode-type |=
+   DRM_MODE_TYPE_PREFERRED;
+   }
+   }
+   if (dev_priv-panel_fixed_mode) {
struct drm_display_mode *mode;
mode = drm_mode_duplicate(dev, 
dev_priv-panel_fixed_mode);
drm_mode_probed_add(connector, mode);
@@ -2061,15 +2070,6 @@ intel_dp_init(struct drm_device *dev, int output_reg)
intel_encoder-hot_plug = intel_dp_hot_plug;
 
if (is_edp(intel_dp)) {
-   /* initialize panel mode from VBT if available for eDP */
-   if (dev_priv-lfp_lvds_vbt_mode) {
-   dev_priv-panel_fixed_mode =
-   drm_mode_duplicate(dev, 
dev_priv-lfp_lvds_vbt_mode);
-   if (dev_priv-panel_fixed_mode) {
-   dev_priv-panel_fixed_mode-type |=
-   DRM_MODE_TYPE_PREFERRED;
-   }
-   }
dev_priv-int_edp_connector = connector;
intel_panel_setup_backlight(dev);
}
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/9] drm/i915: Check eDP power when doing aux channel communications

[Keith Packard]

Verify that the eDP VDD is on, either with the panel being on or with
the VDD force-on bit being set.

This demonstrates that in many instances, VDD is not on when needed,
which leads to failed EDID communications.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_dp.c |   22 ++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8ab2a88..3b29a6f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -279,6 +279,24 @@ intel_hrawclk(struct drm_device *dev)
}
 }
 
+static void
+intel_dp_check_edp(struct intel_dp *intel_dp)
+{
+   struct drm_device *dev = intel_dp-base.base.dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   u32 pp_status, pp_control;
+   if (!is_edp(intel_dp))
+   return;
+   pp_status = I915_READ(PCH_PP_STATUS);
+   pp_control = I915_READ(PCH_PP_CONTROL);
+   if ((pp_status  PP_ON) == 0  (pp_control  EDP_FORCE_VDD) == 0) {
+   WARN(1, eDP powered off while attempting aux channel 
communication.\n);
+   DRM_DEBUG_KMS(Status 0x%08x Control 0x%08x\n,
+ pp_status,
+ I915_READ(PCH_PP_CONTROL));
+   }
+}
+
 static int
 intel_dp_aux_ch(struct intel_dp *intel_dp,
uint8_t *send, int send_bytes,
@@ -295,6 +313,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
uint32_t aux_clock_divider;
int try, precharge;
 
+   intel_dp_check_edp(intel_dp);
/* The clock divider is based off the hrawclk,
 * and would like to run at 2MHz. So, take the
 * hrawclk value and divide by 2 and use that
@@ -408,6 +427,7 @@ intel_dp_aux_native_write(struct intel_dp *intel_dp,
int msg_bytes;
uint8_t ack;
 
+   intel_dp_check_edp(intel_dp);
if (send_bytes  16)
return -1;
msg[0] = AUX_NATIVE_WRITE  4;
@@ -450,6 +470,7 @@ intel_dp_aux_native_read(struct intel_dp *intel_dp,
uint8_t ack;
int ret;
 
+   intel_dp_check_edp(intel_dp);
msg[0] = AUX_NATIVE_READ  4;
msg[1] = address  8;
msg[2] = address  0xff;
@@ -493,6 +514,7 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
int reply_bytes;
int ret;
 
+   intel_dp_check_edp(intel_dp);
/* Set up the command byte */
if (mode  MODE_I2C_READ)
msg[0] = AUX_I2C_READ  4;
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 8/9] drm/i915: Move eDP panel fixed mode from dev_priv to intel_dp

[Keith Packard]

This value doesn't come directly from the VBT, and so is rather
specific to the particular DP output.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/i915_drv.h |1 -
 drivers/gpu/drm/i915/intel_dp.c |   35 ---
 2 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bcdf58b..e6dd19e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -347,7 +347,6 @@ typedef struct drm_i915_private {
/* LVDS info */
int backlight_level;  /* restore backlight to this value */
bool backlight_enabled;
-   struct drm_display_mode *panel_fixed_mode;
struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f1d6105..5bc30f9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -61,6 +61,7 @@ struct intel_dp {
uint8_t link_status[DP_LINK_STATUS_SIZE];
int panel_power_up_delay;
int panel_power_down_delay;
+   struct drm_display_mode *panel_fixed_mode;  /* for eDP */
 };
 
 /**
@@ -202,16 +203,14 @@ intel_dp_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
 {
struct intel_dp *intel_dp = intel_attached_dp(connector);
-   struct drm_device *dev = connector-dev;
-   struct drm_i915_private *dev_priv = dev-dev_private;
int max_link_clock = 
intel_dp_link_clock(intel_dp_max_link_bw(intel_dp));
int max_lanes = intel_dp_max_lane_count(intel_dp);
 
-   if (is_edp(intel_dp)  dev_priv-panel_fixed_mode) {
-   if (mode-hdisplay  dev_priv-panel_fixed_mode-hdisplay)
+   if (is_edp(intel_dp)  intel_dp-panel_fixed_mode) {
+   if (mode-hdisplay  intel_dp-panel_fixed_mode-hdisplay)
return MODE_PANEL;
 
-   if (mode-vdisplay  dev_priv-panel_fixed_mode-vdisplay)
+   if (mode-vdisplay  intel_dp-panel_fixed_mode-vdisplay)
return MODE_PANEL;
}
 
@@ -630,22 +629,21 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct 
drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
 {
struct drm_device *dev = encoder-dev;
-   struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
int lane_count, clock;
int max_lane_count = intel_dp_max_lane_count(intel_dp);
int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 
0;
static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
 
-   if (is_edp(intel_dp)  dev_priv-panel_fixed_mode) {
-   intel_fixed_panel_mode(dev_priv-panel_fixed_mode, 
adjusted_mode);
+   if (is_edp(intel_dp)  intel_dp-panel_fixed_mode) {
+   intel_fixed_panel_mode(intel_dp-panel_fixed_mode, 
adjusted_mode);
intel_pch_panel_fitting(dev, DRM_MODE_SCALE_FULLSCREEN,
mode, adjusted_mode);
/*
 * the mode-clock is used to calculate the DataLink M/N
 * of the pipe. For the eDP the fixed clock should be used.
 */
-   mode-clock = dev_priv-panel_fixed_mode-clock;
+   mode-clock = intel_dp-panel_fixed_mode-clock;
}
 
for (lane_count = 1; lane_count = max_lane_count; lane_count = 1) {
@@ -1812,35 +1810,34 @@ static int intel_dp_get_modes(struct drm_connector 
*connector)
 
ret = intel_dp_get_edid_modes(connector, intel_dp-adapter);
if (ret) {
-   if (is_edp(intel_dp)  !dev_priv-panel_fixed_mode) {
+   if (is_edp(intel_dp)  !intel_dp-panel_fixed_mode) {
struct drm_display_mode *newmode;
list_for_each_entry(newmode, connector-probed_modes,
head) {
-   if (newmode-type  DRM_MODE_TYPE_PREFERRED) {
-   dev_priv-panel_fixed_mode =
+   if ((newmode-type  DRM_MODE_TYPE_PREFERRED)) {
+   intel_dp-panel_fixed_mode =
drm_mode_duplicate(dev, 
newmode);
break;
}
}
}
-
return ret;
}
 
/* if eDP has no EDID, try to use fixed panel mode from VBT */
if (is_edp(intel_dp)) {
/* initialize panel mode from VBT if available for eDP */
-   if (dev_priv-panel_fixed_mode == NULL  
dev_priv-lfp_lvds_vbt_mode != NULL) {
-   dev_priv

[PATCH 6/9] drm/i915: Make sure eDP power is on before using aux channel

[Keith Packard]

The eDP panel may not be able to respond to aux channel communications
unless it has power supplied. During mode setting, power may be
cut-off during panel power sequencing. Make sure that any aux channel
communications will work by forcing vdd power active as needed.

This also delays after turning power on and off to ensure that the
panel is keeping up.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_dp.c |   84 +-
 1 files changed, 64 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a983d0f..41b1e05 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -595,10 +595,15 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
return -EREMOTEIO;
 }
 
+static void ironlake_edp_panel_vdd_on(struct intel_dp *intel_dp);
+static void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp);
+
 static int
 intel_dp_i2c_init(struct intel_dp *intel_dp,
  struct intel_connector *intel_connector, const char *name)
 {
+   int ret;
+
DRM_DEBUG_KMS(i2c_init %s\n, name);
intel_dp-algo.running = false;
intel_dp-algo.address = 0;
@@ -612,7 +617,10 @@ intel_dp_i2c_init(struct intel_dp *intel_dp,
intel_dp-adapter.algo_data = intel_dp-algo;
intel_dp-adapter.dev.parent = intel_connector-base.kdev;
 
-   return i2c_dp_aux_add_bus(intel_dp-adapter);
+   ironlake_edp_panel_vdd_on(intel_dp);
+   ret = i2c_dp_aux_add_bus(intel_dp-adapter);
+   ironlake_edp_panel_vdd_off(intel_dp);
+   return ret;
 }
 
 static bool
@@ -830,14 +838,20 @@ static void ironlake_edp_panel_vdd_on(struct intel_dp 
*intel_dp)
 {
struct drm_device *dev = intel_dp-base.base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
-   u32 pp;
+   u32 pp, pp_status;
 
+   if (!is_edp(intel_dp))
+   return;
+   DRM_DEBUG_KMS(Turn eDP VDD on\n);
/*
 * If the panel wasn't on, make sure there's not a currently
 * active PP sequence before enabling AUX VDD.
 */
-   if (!(I915_READ(PCH_PP_STATUS)  PP_ON))
+   pp_status = I915_READ(PCH_PP_STATUS);
+   if (!(pp_status  PP_ON)) {
+   DRM_DEBUG_KMS(eDP VDD was not on\n);
msleep(dev_priv-panel_t3);
+   }
 
pp = I915_READ(PCH_PP_CONTROL);
pp = ~PANEL_UNLOCK_MASK;
@@ -845,6 +859,9 @@ static void ironlake_edp_panel_vdd_on(struct intel_dp 
*intel_dp)
pp |= EDP_FORCE_VDD;
I915_WRITE(PCH_PP_CONTROL, pp);
POSTING_READ(PCH_PP_CONTROL);
+   DRM_DEBUG_KMS(PCH_PP_STATUS: 0x%08x PCH_PP_CONTROL: 0x%08x\n,
+ I915_READ(PCH_PP_STATUS), I915_READ(PCH_PP_CONTROL));
+   msleep(1000);
 }
 
 static void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp)
@@ -853,6 +870,9 @@ static void ironlake_edp_panel_vdd_off(struct intel_dp 
*intel_dp)
struct drm_i915_private *dev_priv = dev-dev_private;
u32 pp;
 
+   if (!is_edp(intel_dp))
+   return;
+   DRM_DEBUG_KMS(Turn eDP VDD off\n);
pp = I915_READ(PCH_PP_CONTROL);
pp = ~PANEL_UNLOCK_MASK;
pp |= PANEL_UNLOCK_REGS;
@@ -862,6 +882,9 @@ static void ironlake_edp_panel_vdd_off(struct intel_dp 
*intel_dp)
 
/* Make sure sequencer is idle before allowing subsequent activity */
msleep(dev_priv-panel_t12);
+   DRM_DEBUG_KMS(PCH_PP_STATUS: 0x%08x PCH_PP_CONTROL: 0x%08x\n,
+ I915_READ(PCH_PP_STATUS), I915_READ(PCH_PP_CONTROL));
+   msleep(1000);
 }
 
 /* Returns true if the panel was already on when called */
@@ -1016,7 +1039,9 @@ static void intel_dp_prepare(struct drm_encoder *encoder)
struct drm_device *dev = encoder-dev;
 
/* Wake up the sink first */
+   ironlake_edp_panel_vdd_on(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
+   ironlake_edp_panel_vdd_off(intel_dp);
 
if (is_edp(intel_dp)) {
ironlake_edp_backlight_off(dev);
@@ -1034,21 +1059,17 @@ static void intel_dp_commit(struct drm_encoder *encoder)
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
struct drm_device *dev = encoder-dev;
 
-   if (is_edp(intel_dp))
-   ironlake_edp_panel_vdd_on(intel_dp);
-
+   ironlake_edp_panel_vdd_on(intel_dp);
intel_dp_start_link_train(intel_dp);
 
-   if (is_edp(intel_dp)) {
+   if (is_edp(intel_dp))
ironlake_edp_panel_on(intel_dp);
-   ironlake_edp_panel_vdd_off(intel_dp);
-   }
 
intel_dp_complete_link_train(intel_dp);
 
if (is_edp(intel_dp))
ironlake_edp_backlight_on(dev);
-
+   ironlake_edp_panel_vdd_off(intel_dp);
intel_dp-dpms_mode = DRM_MODE_DPMS_ON;
 }
 
@@ -1060,6 +1081,7 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode)
struct drm_i915_private

[PATCH 7/9] drm/i915: Correct eDP panel power sequencing delay computations

[Keith Packard]

Store the panel power sequencing delays in the dp private structure,
rather than the global device structure. Who knows, maybe we'll get
more than one eDP device in the future.

Look at both the current hardware register settings and the VBT
specified panel power sequencing timings. Use the maximum of the two
delays, to make sure things work reliably. If there is no VBT data,
then those values will be initialized to zero, so we'll just use the
values as programmed in the hardware.

This patch computes power-up and power-down delays, rather than using
portions of the appropriate delay values as found in the hardware. The
eDP specified delay between raising VCC and communicating over the aux
channel includes both the power rise time (T1) and the aux channel
communication delay (T3). The eDP specified delay between powering
down the device and powering it back up includes both the power fall
time (T11) and the device idle time (T12).

From the hardware, I'm taking the T3 value from the PP_OFF_DELAYS
Power_Down_delay value, which is actually documented to be the 'T3
time sequence' value used 'during power up'. There aren't separate T1
and T2 values, but there is a combined T1+T2 value in the PP_ON_DELAYS
register, so I use that instead.

VBT doesn't provide any values for T1 or T2, so we'll always just use
the hardware value for that.

The panel power up delay is thus T1 + T2 + T3, which should be
sufficient in all cases.

The panel power down delay is T1 + T2 + T12, using T1+T2 as a proxy
for T11, which isn't available anywhere.

On the macbook air I'm testing with, this yields a power-up delay of
over 200ms and a power-down delay of over 600ms. It all works now, but
we're frobbing these power controls several times during mode setting,
making the whole process take an awfully long time.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/i915_drv.h |1 -
 drivers/gpu/drm/i915/intel_dp.c |   56 --
 2 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7916bd9..bcdf58b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -672,7 +672,6 @@ typedef struct drm_i915_private {
unsigned int lvds_border_bits;
/* Panel fitter placement and size for Ironlake+ */
u32 pch_pf_pos, pch_pf_size;
-   int panel_t3, panel_t12;
 
struct drm_crtc *plane_to_crtc_mapping[2];
struct drm_crtc *pipe_to_crtc_mapping[2];
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 41b1e05..f1d6105 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -59,6 +59,8 @@ struct intel_dp {
bool is_pch_edp;
uint8_t train_set[4];
uint8_t link_status[DP_LINK_STATUS_SIZE];
+   int panel_power_up_delay;
+   int panel_power_down_delay;
 };
 
 /**
@@ -848,10 +850,6 @@ static void ironlake_edp_panel_vdd_on(struct intel_dp 
*intel_dp)
 * active PP sequence before enabling AUX VDD.
 */
pp_status = I915_READ(PCH_PP_STATUS);
-   if (!(pp_status  PP_ON)) {
-   DRM_DEBUG_KMS(eDP VDD was not on\n);
-   msleep(dev_priv-panel_t3);
-   }
 
pp = I915_READ(PCH_PP_CONTROL);
pp = ~PANEL_UNLOCK_MASK;
@@ -861,7 +859,10 @@ static void ironlake_edp_panel_vdd_on(struct intel_dp 
*intel_dp)
POSTING_READ(PCH_PP_CONTROL);
DRM_DEBUG_KMS(PCH_PP_STATUS: 0x%08x PCH_PP_CONTROL: 0x%08x\n,
  I915_READ(PCH_PP_STATUS), I915_READ(PCH_PP_CONTROL));
-   msleep(1000);
+   if (!(pp_status  PP_ON)) {
+   msleep(intel_dp-panel_power_up_delay);
+   DRM_DEBUG_KMS(eDP VDD was not on\n);
+   }
 }
 
 static void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp)
@@ -881,10 +882,9 @@ static void ironlake_edp_panel_vdd_off(struct intel_dp 
*intel_dp)
POSTING_READ(PCH_PP_CONTROL);
 
/* Make sure sequencer is idle before allowing subsequent activity */
-   msleep(dev_priv-panel_t12);
DRM_DEBUG_KMS(PCH_PP_STATUS: 0x%08x PCH_PP_CONTROL: 0x%08x\n,
  I915_READ(PCH_PP_STATUS), I915_READ(PCH_PP_CONTROL));
-   msleep(1000);
+   msleep(intel_dp-panel_power_down_delay);
 }
 
 /* Returns true if the panel was already on when called */
@@ -922,8 +922,10 @@ static bool ironlake_edp_panel_on (struct intel_dp 
*intel_dp)
return false;
 }
 
-static void ironlake_edp_panel_off (struct drm_device *dev)
+static void ironlake_edp_panel_off(struct drm_encoder *encoder)
 {
+   struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+   struct drm_device *dev = encoder-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
u32 pp, idle_off_mask = PP_ON | PP_SEQUENCE_MASK |
PP_CYCLE_DELAY_ACTIVE | PP_SEQUENCE_STATE_MASK;
@@ -940,6 +942,7 @@ static void

[PATCH 9/9] drm/i915: Disable eDP VDD in a delayed work proc instead of synchronously

[Keith Packard]

There's no good reason to turn off the eDP force VDD bit synchronously
while probing devices; that just sticks a huge delay into all mode
setting paths. Instead, queue a delayed work proc to disable the VDD
force bit and then remember when that fires to ensure that the
appropriate delay is respected before trying to turn it back on.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_dp.c |   93 +-
 1 files changed, 80 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5bc30f9..8130e82 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -62,6 +62,9 @@ struct intel_dp {
int panel_power_up_delay;
int panel_power_down_delay;
struct drm_display_mode *panel_fixed_mode;  /* for eDP */
+   struct delayed_work panel_vdd_work;
+   bool panel_vdd_force;
+   unsigned long panel_off_jiffies;
 };
 
 /**
@@ -834,6 +837,23 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct 
drm_display_mode *mode,
}
 }
 
+static void ironlake_wait_panel_off(struct intel_dp *intel_dp)
+{
+   unsigned long   off_time;
+   unsigned long   delay;
+   DRM_DEBUG_KMS(Wait for panel power off time\n);
+   off_time = intel_dp-panel_off_jiffies + 
msecs_to_jiffies(intel_dp-panel_power_down_delay);
+   if (time_after(jiffies, off_time)) {
+   DRM_DEBUG_KMS(Time already passed);
+   return;
+   }
+   delay = jiffies_to_msecs(off_time - jiffies);
+   if (delay  intel_dp-panel_power_down_delay)
+   delay = intel_dp-panel_power_down_delay;
+   DRM_DEBUG_KMS(Waiting an additional %ld ms\n, delay);
+   msleep(delay);
+}
+
 static void ironlake_edp_panel_vdd_on(struct intel_dp *intel_dp)
 {
struct drm_device *dev = intel_dp-base.base.dev;
@@ -843,13 +863,18 @@ static void ironlake_edp_panel_vdd_on(struct intel_dp 
*intel_dp)
if (!is_edp(intel_dp))
return;
DRM_DEBUG_KMS(Turn eDP VDD on\n);
-   /*
-* If the panel wasn't on, make sure there's not a currently
-* active PP sequence before enabling AUX VDD.
-*/
-   pp_status = I915_READ(PCH_PP_STATUS);
 
+   WARN(intel_dp-panel_vdd_force,
+eDP VDD already forced on\n);
+
+   intel_dp-panel_vdd_force = true;
pp = I915_READ(PCH_PP_CONTROL);
+   if (pp  EDP_FORCE_VDD) {
+   DRM_DEBUG_KMS(eDP VDD already on\n);
+   return;
+   }
+
+   ironlake_wait_panel_off(intel_dp);
pp = ~PANEL_UNLOCK_MASK;
pp |= PANEL_UNLOCK_REGS;
pp |= EDP_FORCE_VDD;
@@ -857,21 +882,23 @@ static void ironlake_edp_panel_vdd_on(struct intel_dp 
*intel_dp)
POSTING_READ(PCH_PP_CONTROL);
DRM_DEBUG_KMS(PCH_PP_STATUS: 0x%08x PCH_PP_CONTROL: 0x%08x\n,
  I915_READ(PCH_PP_STATUS), I915_READ(PCH_PP_CONTROL));
+
+   /*
+* If the panel wasn't on, delay before accessing aux channel
+*/
+   pp_status = I915_READ(PCH_PP_STATUS);
if (!(pp_status  PP_ON)) {
+   DRM_DEBUG_KMS(eDP was not running\n);
msleep(intel_dp-panel_power_up_delay);
-   DRM_DEBUG_KMS(eDP VDD was not on\n);
}
 }
 
-static void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp)
+static void ironlake_panel_vdd_delayed_off(struct intel_dp *intel_dp)
 {
struct drm_device *dev = intel_dp-base.base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
u32 pp;
 
-   if (!is_edp(intel_dp))
-   return;
-   DRM_DEBUG_KMS(Turn eDP VDD off\n);
pp = I915_READ(PCH_PP_CONTROL);
pp = ~PANEL_UNLOCK_MASK;
pp |= PANEL_UNLOCK_REGS;
@@ -882,7 +909,37 @@ static void ironlake_edp_panel_vdd_off(struct intel_dp 
*intel_dp)
/* Make sure sequencer is idle before allowing subsequent activity */
DRM_DEBUG_KMS(PCH_PP_STATUS: 0x%08x PCH_PP_CONTROL: 0x%08x\n,
  I915_READ(PCH_PP_STATUS), I915_READ(PCH_PP_CONTROL));
-   msleep(intel_dp-panel_power_down_delay);
+   intel_dp-panel_off_jiffies = jiffies;
+}
+
+static void ironlake_panel_vdd_work(struct work_struct *__work)
+{
+   struct intel_dp *intel_dp = container_of(to_delayed_work(__work),
+struct intel_dp, 
panel_vdd_work);
+   struct drm_device *dev = intel_dp-base.base.dev;
+
+   mutex_lock(dev-struct_mutex);
+   if (!intel_dp-panel_vdd_force)
+   ironlake_panel_vdd_delayed_off(intel_dp);
+   mutex_unlock(dev-struct_mutex);
+}
+
+static void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp)
+{
+   if (!is_edp(intel_dp))
+   return;
+
+   DRM_DEBUG_KMS(Turn eDP VDD off %d\n, intel_dp-panel_vdd_force);
+   WARN(!intel_dp-panel_vdd_force, eDP VDD not forced on);
+   
+   intel_dp

[PULL] drm-intel-next

[Keith Packard]

This is a single patch which cleans up almost all of the whitespace
errors in the i915 driver. It currently merges cleanly with your fdo
drm-core-next tree.

I've checked this patch quite carefully, examining the .o files with
objdump -s to make sure nothing significant changed. The only thing that
found was a couple of debug messages which had blank space before newlines.

The following changes since commit b6fd41e29dea9c6753b1843a77e50433e6123bcb:

  Linux 3.1-rc6 (2011-09-12 14:02:02 -0700)

are available in the git repository at:
  git://people.freedesktop.org/~keithp/linux drm-intel-next

Akshay Joshi (1):
  Drivers: i915: Fix all space related issues.

 drivers/gpu/drm/i915/dvo_ch7017.c   |2 +-
 drivers/gpu/drm/i915/dvo_ch7xxx.c   |4 +-
 drivers/gpu/drm/i915/dvo_ivch.c |6 +-
 drivers/gpu/drm/i915/dvo_sil164.c   |2 +-
 drivers/gpu/drm/i915/dvo_tfp410.c   |   14 +-
 drivers/gpu/drm/i915/i915_debugfs.c |   38 +-
 drivers/gpu/drm/i915/i915_dma.c |   44 ++--
 drivers/gpu/drm/i915/i915_drv.c |   16 +-
 drivers/gpu/drm/i915/i915_drv.h |   70 ++--
 drivers/gpu/drm/i915/i915_gem.c |   12 +-
 drivers/gpu/drm/i915/i915_gem_debug.c   |2 +-
 drivers/gpu/drm/i915/i915_gem_evict.c   |2 +-
 drivers/gpu/drm/i915/i915_irq.c |6 +-
 drivers/gpu/drm/i915/i915_mem.c |   14 +-
 drivers/gpu/drm/i915/i915_reg.h |8 +-
 drivers/gpu/drm/i915/i915_suspend.c |8 +-
 drivers/gpu/drm/i915/i915_trace.h   |   46 ++--
 drivers/gpu/drm/i915/intel_acpi.c   |2 +-
 drivers/gpu/drm/i915/intel_bios.c   |4 +-
 drivers/gpu/drm/i915/intel_bios.h   |2 +-
 drivers/gpu/drm/i915/intel_crt.c|2 +-
 drivers/gpu/drm/i915/intel_display.c|  222 ++--
 drivers/gpu/drm/i915/intel_dp.c |   26 +-
 drivers/gpu/drm/i915/intel_drv.h|   12 +-
 drivers/gpu/drm/i915/intel_opregion.c   |   90 +++---
 drivers/gpu/drm/i915/intel_overlay.c|  146 
 drivers/gpu/drm/i915/intel_panel.c  |6 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c |   76 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.h |8 +-
 drivers/gpu/drm/i915/intel_sdvo.c   |  228 +++---
 drivers/gpu/drm/i915/intel_sdvo_regs.h  |  558 +++---
 drivers/gpu/drm/i915/intel_tv.c |   58 ++--
 32 files changed, 867 insertions(+), 867 deletions(-)

-- 
keith.pack...@intel.com


pgplgiKN0Lhl5.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

drm/i915 git repository at freedesktop.org

[Keith Packard]

I've created a (temporary?) git repository for the drm/i915 driver at

git://people.freedesktop.org/~keithp/linux

This has the drm-intel-next and drm-intel-fixes branches, and may also
have occasional temporary branches with code which has not been merged yet.

-- 
keith.pack...@intel.com


pgp1wNikipeHi.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/i915: FBC off for ironlake, otherwise on by default

[Keith Packard]

Make the default FBC behaviour chipset specific, allowing us to turn
it on by default for everything except Ironlake where it has been
seen to cause trouble with screen updates.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/i915_drv.c  |4 ++--
 drivers/gpu/drm/i915/intel_display.c |   10 +-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ce045a8..f07e425 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -67,11 +67,11 @@ module_param_named(i915_enable_rc6, i915_enable_rc6, int, 
0600);
 MODULE_PARM_DESC(i915_enable_rc6,
Enable power-saving render C-state 6 (default: true));
 
-unsigned int i915_enable_fbc __read_mostly = 1;
+unsigned int i915_enable_fbc __read_mostly = -1;
 module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600);
 MODULE_PARM_DESC(i915_enable_fbc,
Enable frame buffer compression for power savings 
-   (default: false));
+   (default: -1 (use per-chip default)));
 
 unsigned int i915_lvds_downclock __read_mostly = 0;
 module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9fb4a40..bc05deb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1799,6 +1799,7 @@ static void intel_update_fbc(struct drm_device *dev)
struct drm_framebuffer *fb;
struct intel_framebuffer *intel_fb;
struct drm_i915_gem_object *obj;
+   int enable_fbc;
 
DRM_DEBUG_KMS(\n);
 
@@ -1839,7 +1840,14 @@ static void intel_update_fbc(struct drm_device *dev)
intel_fb = to_intel_framebuffer(fb);
obj = intel_fb-obj;
 
-   if (!i915_enable_fbc) {
+   enable_fbc = i915_enable_fbc;
+   if (enable_fbc  0) {
+   DRM_DEBUG_KMS(fbc set to per-chip default\n);
+   enable_fbc = 1;
+   if (INTEL_INFO(dev)-gen == 5)
+   enable_fbc = 0;
+   }
+   if (!enable_fbc) {
DRM_DEBUG_KMS(fbc disabled per module param (default off)\n);
dev_priv-no_fbc_reason = FBC_MODULE_PARAM;
goto out_disable;
-- 
1.7.6.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Proposal for a low-level Linux display framework

[Keith Packard]

On Thu, 15 Sep 2011 18:39:21 +, Florian Tobias Schandinat 
 wrote:

> Well, I'm not against sharing the code and not against taking DRM's current
> implementation as a base but the steps required to make it generally 
> acceptable
> would be to split it of, probably as a standalone module and strip all DRM
> specific things off. Than all things that require EDID can use it, DRM can add
> DRM-specific things on top and fb can add fb-specific things.

The rendering portions of the DRM drivers are all device-specific. The
core DRM ioctls are largely about providing some sharing control over
the device, mapping memory around and mode setting.

> One of my biggest problems with KMS is that it has (naturally) a lot more
> complexity than the fb API which leads to instability.

The mode setting portions are of necessity the same. The KMS API exposes
more functionality for mode setting, but doesn't actually require any
additional hardware-specific knowledge. You still have to be able to
bring the hardware up from power on and light up every connected
monitor.

However, if you want acceleration, you're going to run into bugs that
crash the machine. It's a sad reality that graphics hardware just isn't
able to recover cleanly in all cases from programmer errors, and that
includes errors that come from user mode.

Hardware is improving in this area, and reset is getting more reliable
than it used to be. But, until we can context switch the graphics
hardware at arbitrary points during execution, we're kinda stuck with
using the really big reset hammer when programs go awry.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Proposal for a low-level Linux display framework

[Keith Packard]

On Thu, 15 Sep 2011 20:21:15 +0300, Tomi Valkeinen  
wrote:

> 2) panel drivers, handles panel specific things. Each panel may support
> custom commands and features, for which we need a dedicated driver. And
> this driver is not platform specific, but should work with any platform
> which has the output used with the panel.

Right, we've got DDC ports (which are just i2c) and DisplayPort aux
channel stuff.

The DDC stuff is abstracted out and shared across the drivers, but the
DisplayPort aux channel code is not -- it's duplicated in every output
driver. 

> DSI bus is a half-duplex serial bus, and while it's designed for
> displays you could use it easily for any communication between the SoC
> and the peripheral.

Yeah, HDMI uses DDC for all kinds of crazy stuff in the CE world.

> The point is that we cannot have standard "MIPI DSI command mode panel
> driver" which would work for all DSI cmd mode panels, but we need (in
> the worst case) separate driver for each panel.

It sounds like we do want to share code for those bits, much like we
have DDC split out now. And, we should do something about the
DisplayPort aux channel stuff to avoid duplicating it everywhere.

I'm not sure a common interface to all of these different
channels makes sense, but surely a DSI library and an aux channel
library would fit nicely alongside the existing DDC library.

I suspect helper functions would be a good model to follow, rather than
trying to create a whole new device infrastructure; some of the
communication paths aren't easily separable from the underlying output
devices.

Oh, I think you're also trying to get at how we expose some of these
controls outside of the display driver -- right now, they're mostly
exposed as properties on the output device. Things like backlight
brightness, a million analog TV output values, dithering control and
other more esoteric controls.

DRM properties include booleans, enumerations, integer ranges and chunks
of binary data. Some are read-only (like EDID data), some are writable
(like overscan values).

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Whitespace cleanups in drm/i915

[Keith Packard]

I've got this nice patch from Akshay Joshi that removes almost all of
the checkpatch.pl warnings from drm/i915. If I don't merge it now, it's
going to go stale and be useless; if I merge it only to drm-intel-next,
it will be the source of endless conflicts.

However, it's a huge patch (yes, the code was rather sloppy), and
doesn't exactly fit into the "critical patches only please" mode of the
current stage of 3.1 development.

I've checked the patch very carefully, using the obvious git diff -b to
make sure it really doesn't touch anything but whitespace, but also
using objdump -s to compare the output of the compiler. There were no
differences found with git-diff -b. The only differences found by
objdump are two whitespace changes in some debug output messages in
intel_bios.c.

I think I have three choices:

 1) merge the patch and expect complaints from upstream

 2) thank Akshay for his good intentions, discard the patch and hope
that he feels motivated enough to do it all over again in time for
the 3.2 merge window.

 3) thank Akshay for his good intentions and leave the code as-is,
forever to ease back-porting of fixes to older kernel versions.

Frankly, if we're ever going to merge whitespace fixups, this would be a
pretty darn good time; drm-intel-fixes and drm-intel-next are in-sync as
I haven't started pulling 3.2 code into -next.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Proposal for a low-level Linux display framework

[Keith Packard]

On Thu, 15 Sep 2011 17:12:43 +, Florian Tobias Schandinat 
 wrote:

> Interesting that this comes from the people that pushed the latest mode 
> setting
> code into the kernel. But I don't think that this will happen, the exposed 
> user
> interfaces will be around for decades and the infrastructure code could be
> shared, in theory.

We moved mode setting code from user space to kernel space -- the DRM
stuff comes directly from X, which has a fairly long history of
complicated display environments.

The DRM code does expose fb interfaces to both kernel and user mode,
figuring out how to integrate v4l2 and drm seems like the remaining
challenge.

> For fb and V4L2 I think we'll develop some level of interoperability, share
> concepts and maybe even some code. The FOURCC pixel formats and overlays are
> such examples. As Laurent is really interested in it I think we can get some
> nice progress here.

Jesse's design for the DRM overlay code will expose the pixel formats as
FOURCC codes so that DRM and v4l2 can interoperate -- we've got a lot of
hardware that has both video decode and 3D acceleration, so those are
going to get integrated somehow. And, we have to figure out how to share
buffers between these APIs to avoid copying data with the CPU.

DRM provides fb interfaces, so you don't need to change fb at all --
hardware that requires the capabilities provided by DRM will use that
and not use any of the other fb code in the kernel.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Proposal for a low-level Linux display framework

[Keith Packard]

On Thu, 15 Sep 2011 18:29:54 +0300, Tomi Valkeinen  
wrote:

> 1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
> the plan is to make DRM the core Linux display framework, upon which
> everything else is built, and fb and v4l2 are changed to use DRM.

I'd like to think we could make DRM the underlying display framework;
it already exposes an fb interface, and with overlays, a bit more of the
v4l2 stuff is done as well. Certainly eliminating three copies of mode
setting infrastructure would be nice...

> But even if it was done like that, I see that it's combining two
> separate things: 1) the lower level HW control, and 2) the upper level
> buffer management, policies and userspace interfaces.

Those are split between the DRM layer and the underlying device driver,
which provides both kernel (via fb) and user space interfaces.

> 2) It's missing the panel driver part. This is rather important on
> embedded systems, as the panels often are not "dummy" panels, but they
> need things like custom initialization, sending commands to adjust
> backlight, etc.

We integrate the panel (and other video output) drivers into the device
drivers. With desktop chips, they're not easily separable. None of the
desktop output drivers are simple; things like DisplayPort require link
training, and everyone needs EDID. We share some of that code in the DRM
layer today, and it would be nice to share even more.

We should figure out if the DRM interfaces are sufficient for your
needs; they're pretty flexible at this point.

Of course, backlight remains a mess in the desktop world; with many
custom backlight drivers along with generic ACPI and then
per-video-device drivers as well.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Proposal for a low-level Linux display framework

[Keith Packard]

On Thu, 15 Sep 2011 15:07:05 +0300, Tomi Valkeinen  
wrote:

> This was a very rough and quite short proposal, but I'm happy to improve
> and extend it if it's not totally shot down.

Jesse Barnes has put together a proposal much like this to work within
the existing DRM environment. This is pretty much the last piece of
missing mode-setting functionality that we know of, making DRM capable
of fully supporting existing (and planned) devices.

Here's a link to some older discussion on the issue, things have changed
a bit since then and we had a long talk about this during the X
Developers' Conference this week in Chicago. Expect an update to his
proposal in the coming weeks.

http://lists.freedesktop.org/archives/dri-devel/2011-April/010559.html

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[REGRESSION] i915 KMS dual head broken on DELL latitude E6420 in 3.1-rc*

[Keith Packard]

On Thu, 15 Sep 2011 10:12:59 +0200, Samuel Thibault  wrote:

> At home only. At work, with a different VGA screen, I'm still getting
> the issue.

You're still having a problem with the LVDS screen at work with FBC
disabled? Can you send along a kernel log with drm.debug=5?

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[REGRESSION] i915 KMS dual head broken on DELL latitude E6420 in 3.1-rc*

[Keith Packard]

On Thu, 15 Sep 2011 01:27:17 +0200, Samuel Thibault  wrote:
Non-text part: multipart/mixed
> Hello,
> 
> I'm trying to upgrade from 3.0 to 3.1-rcsomething on a DELL latitude
> E6420, but dual head is broken. Here is the scenario:
> 
> - Turn computer on with VGA1 connected. Both LVDS1 and VGA1 show the
>   text console fine.
> - Start X. VGA1 shows X fine, but LVDS1 (1600x900 resolution) shows an
>   odd screen: completely black on the left part (about 1060x900), sort
>   of gray on the right part (about 540x900). Playing with xrandr doesn't
>   help, I can at best make it completely black with --off, and not get
>   it back again with --auto or anything else.

Can you try disabling frame buffer compression?

i915.i915_enable-fbc=0

I'm about to send a patch to disable this by default; I've gotten two
people saying that this helps them already.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Re: [REGRESSION] i915 KMS dual head broken on DELL latitude E6420 in 3.1-rc*

[Keith Packard]

On Thu, 15 Sep 2011 10:12:59 +0200, Samuel Thibault 
samuel.thiba...@ens-lyon.org wrote:

 At home only. At work, with a different VGA screen, I'm still getting
 the issue.

You're still having a problem with the LVDS screen at work with FBC
disabled? Can you send along a kernel log with drm.debug=5?

-- 
keith.pack...@intel.com


pgpYyPB3g6uXx.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Proposal for a low-level Linux display framework

[Keith Packard]

On Thu, 15 Sep 2011 15:07:05 +0300, Tomi Valkeinen tomi.valkei...@ti.com 
wrote:

 This was a very rough and quite short proposal, but I'm happy to improve
 and extend it if it's not totally shot down.

Jesse Barnes has put together a proposal much like this to work within
the existing DRM environment. This is pretty much the last piece of
missing mode-setting functionality that we know of, making DRM capable
of fully supporting existing (and planned) devices.

Here's a link to some older discussion on the issue, things have changed
a bit since then and we had a long talk about this during the X
Developers' Conference this week in Chicago. Expect an update to his
proposal in the coming weeks.

http://lists.freedesktop.org/archives/dri-devel/2011-April/010559.html

-- 
keith.pack...@intel.com


pgpYTn677eyFT.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Proposal for a low-level Linux display framework

[Keith Packard]

On Thu, 15 Sep 2011 18:29:54 +0300, Tomi Valkeinen tomi.valkei...@ti.com 
wrote:

 1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
 the plan is to make DRM the core Linux display framework, upon which
 everything else is built, and fb and v4l2 are changed to use DRM.

I'd like to think we could make DRM the underlying display framework;
it already exposes an fb interface, and with overlays, a bit more of the
v4l2 stuff is done as well. Certainly eliminating three copies of mode
setting infrastructure would be nice...

 But even if it was done like that, I see that it's combining two
 separate things: 1) the lower level HW control, and 2) the upper level
 buffer management, policies and userspace interfaces.

Those are split between the DRM layer and the underlying device driver,
which provides both kernel (via fb) and user space interfaces.

 2) It's missing the panel driver part. This is rather important on
 embedded systems, as the panels often are not dummy panels, but they
 need things like custom initialization, sending commands to adjust
 backlight, etc.

We integrate the panel (and other video output) drivers into the device
drivers. With desktop chips, they're not easily separable. None of the
desktop output drivers are simple; things like DisplayPort require link
training, and everyone needs EDID. We share some of that code in the DRM
layer today, and it would be nice to share even more.

We should figure out if the DRM interfaces are sufficient for your
needs; they're pretty flexible at this point.

Of course, backlight remains a mess in the desktop world; with many
custom backlight drivers along with generic ACPI and then
per-video-device drivers as well.

-- 
keith.pack...@intel.com


pgpBbHEiWm7w2.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Proposal for a low-level Linux display framework

[Keith Packard]

On Thu, 15 Sep 2011 17:12:43 +, Florian Tobias Schandinat 
florianschandi...@gmx.de wrote:

 Interesting that this comes from the people that pushed the latest mode 
 setting
 code into the kernel. But I don't think that this will happen, the exposed 
 user
 interfaces will be around for decades and the infrastructure code could be
 shared, in theory.

We moved mode setting code from user space to kernel space -- the DRM
stuff comes directly from X, which has a fairly long history of
complicated display environments.

The DRM code does expose fb interfaces to both kernel and user mode,
figuring out how to integrate v4l2 and drm seems like the remaining
challenge.

 For fb and V4L2 I think we'll develop some level of interoperability, share
 concepts and maybe even some code. The FOURCC pixel formats and overlays are
 such examples. As Laurent is really interested in it I think we can get some
 nice progress here.

Jesse's design for the DRM overlay code will expose the pixel formats as
FOURCC codes so that DRM and v4l2 can interoperate -- we've got a lot of
hardware that has both video decode and 3D acceleration, so those are
going to get integrated somehow. And, we have to figure out how to share
buffers between these APIs to avoid copying data with the CPU.

DRM provides fb interfaces, so you don't need to change fb at all --
hardware that requires the capabilities provided by DRM will use that
and not use any of the other fb code in the kernel.

-- 
keith.pack...@intel.com


pgp67vDMCuCZU.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Whitespace cleanups in drm/i915

[Keith Packard]

I've got this nice patch from Akshay Joshi that removes almost all of
the checkpatch.pl warnings from drm/i915. If I don't merge it now, it's
going to go stale and be useless; if I merge it only to drm-intel-next,
it will be the source of endless conflicts.

However, it's a huge patch (yes, the code was rather sloppy), and
doesn't exactly fit into the critical patches only please mode of the
current stage of 3.1 development.

I've checked the patch very carefully, using the obvious git diff -b to
make sure it really doesn't touch anything but whitespace, but also
using objdump -s to compare the output of the compiler. There were no
differences found with git-diff -b. The only differences found by
objdump are two whitespace changes in some debug output messages in
intel_bios.c.

I think I have three choices:

 1) merge the patch and expect complaints from upstream

 2) thank Akshay for his good intentions, discard the patch and hope
that he feels motivated enough to do it all over again in time for
the 3.2 merge window.

 3) thank Akshay for his good intentions and leave the code as-is,
forever to ease back-porting of fixes to older kernel versions.

Frankly, if we're ever going to merge whitespace fixups, this would be a
pretty darn good time; drm-intel-fixes and drm-intel-next are in-sync as
I haven't started pulling 3.2 code into -next.

-- 
keith.pack...@intel.com


pgp00OWQJWleX.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Proposal for a low-level Linux display framework

[Keith Packard]

On Thu, 15 Sep 2011 20:21:15 +0300, Tomi Valkeinen tomi.valkei...@ti.com 
wrote:

 2) panel drivers, handles panel specific things. Each panel may support
 custom commands and features, for which we need a dedicated driver. And
 this driver is not platform specific, but should work with any platform
 which has the output used with the panel.

Right, we've got DDC ports (which are just i2c) and DisplayPort aux
channel stuff.

The DDC stuff is abstracted out and shared across the drivers, but the
DisplayPort aux channel code is not -- it's duplicated in every output
driver. 

 DSI bus is a half-duplex serial bus, and while it's designed for
 displays you could use it easily for any communication between the SoC
 and the peripheral.

Yeah, HDMI uses DDC for all kinds of crazy stuff in the CE world.

 The point is that we cannot have standard MIPI DSI command mode panel
 driver which would work for all DSI cmd mode panels, but we need (in
 the worst case) separate driver for each panel.

It sounds like we do want to share code for those bits, much like we
have DDC split out now. And, we should do something about the
DisplayPort aux channel stuff to avoid duplicating it everywhere.

I'm not sure a common interface to all of these different
channels makes sense, but surely a DSI library and an aux channel
library would fit nicely alongside the existing DDC library.

I suspect helper functions would be a good model to follow, rather than
trying to create a whole new device infrastructure; some of the
communication paths aren't easily separable from the underlying output
devices.

Oh, I think you're also trying to get at how we expose some of these
controls outside of the display driver -- right now, they're mostly
exposed as properties on the output device. Things like backlight
brightness, a million analog TV output values, dithering control and
other more esoteric controls.

DRM properties include booleans, enumerations, integer ranges and chunks
of binary data. Some are read-only (like EDID data), some are writable
(like overscan values).

-- 
keith.pack...@intel.com


pgpcYNnms8Jlj.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Proposal for a low-level Linux display framework

[Keith Packard]

On Thu, 15 Sep 2011 18:39:21 +, Florian Tobias Schandinat 
florianschandi...@gmx.de wrote:

 Well, I'm not against sharing the code and not against taking DRM's current
 implementation as a base but the steps required to make it generally 
 acceptable
 would be to split it of, probably as a standalone module and strip all DRM
 specific things off. Than all things that require EDID can use it, DRM can add
 DRM-specific things on top and fb can add fb-specific things.

The rendering portions of the DRM drivers are all device-specific. The
core DRM ioctls are largely about providing some sharing control over
the device, mapping memory around and mode setting.

 One of my biggest problems with KMS is that it has (naturally) a lot more
 complexity than the fb API which leads to instability.

The mode setting portions are of necessity the same. The KMS API exposes
more functionality for mode setting, but doesn't actually require any
additional hardware-specific knowledge. You still have to be able to
bring the hardware up from power on and light up every connected
monitor.

However, if you want acceleration, you're going to run into bugs that
crash the machine. It's a sad reality that graphics hardware just isn't
able to recover cleanly in all cases from programmer errors, and that
includes errors that come from user mode.

Hardware is improving in this area, and reset is getting more reliable
than it used to be. But, until we can context switch the graphics
hardware at arbitrary points during execution, we're kinda stuck with
using the really big reset hammer when programs go awry.

-- 
keith.pack...@intel.com


pgpYfZZld1zo9.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [REGRESSION] i915 KMS dual head broken on DELL latitude E6420 in 3.1-rc*

[Keith Packard]

On Thu, 15 Sep 2011 01:27:17 +0200, Samuel Thibault 
samuel.thiba...@ens-lyon.org wrote:
Non-text part: multipart/mixed
 Hello,
 
 I'm trying to upgrade from 3.0 to 3.1-rcsomething on a DELL latitude
 E6420, but dual head is broken. Here is the scenario:
 
 - Turn computer on with VGA1 connected. Both LVDS1 and VGA1 show the
   text console fine.
 - Start X. VGA1 shows X fine, but LVDS1 (1600x900 resolution) shows an
   odd screen: completely black on the left part (about 1060x900), sort
   of gray on the right part (about 540x900). Playing with xrandr doesn't
   help, I can at best make it completely black with --off, and not get
   it back again with --auto or anything else.

Can you try disabling frame buffer compression?

i915.i915_enable-fbc=0

I'm about to send a patch to disable this by default; I've gotten two
people saying that this helps them already.

-- 
keith.pack...@intel.com


pgprv0XZ4J9DB.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/i915: Enable dither whenever display bpc < frame buffer bpc

[Keith Packard]

We want to enable dithering on any pipe where the frame buffer has
more color resolution than the output device.

The previous code was incorrectly clamping the frame buffer bpc to the
display bpc, effectively disabling dithering all of the time as the
computed frame buffer bpc would never be larger than the display bpc.

Signed-off-by: Keith Packard 
Reported-by: Oliver Hartkopp 
Tested-by: Oliver Hartkopp 
---
 drivers/gpu/drm/i915/intel_display.c |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 56a8554..9fb4a40 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4687,13 +4687,13 @@ static bool intel_choose_pipe_bpp_dither(struct 
drm_crtc *crtc,
bpc = 6; /* min is 18bpp */
break;
case 24:
-   bpc = min((unsigned int)8, display_bpc);
+   bpc = 8;
break;
case 30:
-   bpc = min((unsigned int)10, display_bpc);
+   bpc = 10;
break;
case 48:
-   bpc = min((unsigned int)12, display_bpc);
+   bpc = 12;
break;
default:
DRM_DEBUG("unsupported depth, assuming 24 bits\n");
@@ -4701,10 +4701,12 @@ static bool intel_choose_pipe_bpp_dither(struct 
drm_crtc *crtc,
break;
}

+   display_bpc = min(display_bpc, bpc);
+
DRM_DEBUG_DRIVER("setting pipe bpc to %d (max display bpc %d)\n",
 bpc, display_bpc);

-   *pipe_bpp = bpc * 3;
+   *pipe_bpp = display_bpc * 3;

return display_bpc != bpc;
 }
-- 
1.7.5.4

[PULL] drm-intel-fixes (drm/i915 driver)

[Keith Packard]

This is all I've seen since rc3; a couple of tiny fixes, but one has
seen several complaints on the list, so I figured I'd send them in now.

The following changes since commit fcb8ce5cfe30ca9ca5c9a79cdfe26d1993e65e0c:

  Linux 3.1-rc3 (2011-08-22 11:42:53 -0700)

are available in the git repository at:
  ssh://master.kernel.org/pub/scm/linux/kernel/git/keithp/linux-2.6.git 
drm-intel-fixes

Kamal Mostafa (1):
  i915: do not setup intel_backlight twice

Thomas Jarosch (1):
  drm/i915: Fix wrong initializer for "locked" variable in 
assert_panel_unlocked

 drivers/gpu/drm/i915/intel_display.c |4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PULL] drm-intel-fixes (drm/i915 driver)

[Keith Packard]

This is all I've seen since rc3; a couple of tiny fixes, but one has
seen several complaints on the list, so I figured I'd send them in now.

The following changes since commit fcb8ce5cfe30ca9ca5c9a79cdfe26d1993e65e0c:

  Linux 3.1-rc3 (2011-08-22 11:42:53 -0700)

are available in the git repository at:
  ssh://master.kernel.org/pub/scm/linux/kernel/git/keithp/linux-2.6.git 
drm-intel-fixes

Kamal Mostafa (1):
  i915: do not setup intel_backlight twice

Thomas Jarosch (1):
  drm/i915: Fix wrong initializer for locked variable in 
assert_panel_unlocked

 drivers/gpu/drm/i915/intel_display.c |4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

-- 
keith.pack...@intel.com


pgp0LTuTtroa4.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

i915: -next: have to switch terms after display goes to sleep

[Keith Packard]

On Mon, 22 Aug 2011 12:47:49 +0300, Dan Carpenter  wrote:
> In linux-next 3.1.0-rc2-next-20110819+ when the screen goes to sleep
> then the screen doesn't refresh properly until I switch to a
> non-graphical TTY and then come back.  It does this every time.

> The gnome tool bar at the top of my screen partly shows up, and the
> window outlines are there.  I can click on the terminal icon on my
> gnome tool bar and the terminals start, but I just can't see them.

Can you describe your environment a bit more completely? A kernel log
with drm.debug=0x4, the output of 'xrandr' and which gnome environment
and operating system you're running.

What is the most recent revision which did work? Can you bisect to find
out which commit caused this change?

If you'd file all of this as a bug on freedesktop.org, we'll make sure
to track it and get it fixed.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Re: i915: -next: have to switch terms after display goes to sleep

[Keith Packard]

On Mon, 22 Aug 2011 12:47:49 +0300, Dan Carpenter erro...@gmail.com wrote:
 In linux-next 3.1.0-rc2-next-20110819+ when the screen goes to sleep
 then the screen doesn't refresh properly until I switch to a
 non-graphical TTY and then come back.  It does this every time.

 The gnome tool bar at the top of my screen partly shows up, and the
 window outlines are there.  I can click on the terminal icon on my
 gnome tool bar and the terminals start, but I just can't see them.

Can you describe your environment a bit more completely? A kernel log
with drm.debug=0x4, the output of 'xrandr' and which gnome environment
and operating system you're running.

What is the most recent revision which did work? Can you bisect to find
out which commit caused this change?

If you'd file all of this as a bug on freedesktop.org, we'll make sure
to track it and get it fixed.

-- 
keith.pack...@intel.com


pgp2l0D40979V.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PULL] drm-intel-fixes (drm/i915 driver)

[Keith Packard]

Two bug fixes, a new platform backlight driver and a bit of debug output
for IVB interrupts:

 * Dual-display mode setting manipulations on SNB machines would sometimes
   accidentally turn off all outputs.

 * Attempts to use UMS would segfault in the kernel and generate a pile
   of spurious kernel diagnostics. Detecting a few more KMS-only
   features in the general paths fixes this.

 * Expose a native backlight driver for machines that don't bother to
   have suitable platform or ACPI drivers.

 * Share an SNB interrupt debug function for IVB.

The following changes since commit 322a8b034003c0d46d39af85bf24fee27b902f48:

  Linux 3.1-rc1 (2011-08-07 18:23:30 -0700)

are available in the git repository at:
  ssh://master.kernel.org/pub/scm/linux/kernel/git/keithp/linux-2.6.git 
drm-intel-fixes

Jesse Barnes (2):
  drm/i915: show interrupt info on IVB
  drm/i915: split out PCH refclk update code

Keith Packard (6):
  drm/i915: Wait for LVDS panel power sequence
  drm/i915: Leave LVDS registers unlocked
  drm/i915: Fix PCH port pipe select in CPT disable paths
  drm/i915: Remove unused 'reg' argument to dp_pipe_enabled
  drm/i915: Can't do accurate vblank timestamps with UMS
  drm/i915: Cannot set clock gating under UMS

Matthew Garrett (1):
  Not all systems expose a firmware or platform mechanism for changing the 
backlight intensity on i915, so add native driver support.

 drivers/gpu/drm/i915/i915_debugfs.c   |2 +-
 drivers/gpu/drm/i915/i915_drv.h   |4 +
 drivers/gpu/drm/i915/i915_irq.c   |6 +-
 drivers/gpu/drm/i915/i915_reg.h   |   13 +--
 drivers/gpu/drm/i915/i915_suspend.c   |3 +-
 drivers/gpu/drm/i915/intel_display.c  |  187 +++-
 drivers/gpu/drm/i915/intel_dp.c   |7 ++
 drivers/gpu/drm/i915/intel_drv.h  |3 +-
 drivers/gpu/drm/i915/intel_lvds.c |   82 ++
 drivers/gpu/drm/i915/intel_opregion.c |1 -
 drivers/gpu/drm/i915/intel_panel.c|   72 +-
 11 files changed, 264 insertions(+), 116 deletions(-)

-- 
keith.pack...@intel.com


pgpuLW5ZPPfA7.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Reverting rc6 by default

[Keith Packard]

On Thu, 14 Jul 2011 19:00:26 +0200, Francesco Allertsen  wrote:

> I have tried to boot with the latest git with the commit
> 05bd42688dbc066d4e2689b6f73c0470601f788b reverted (so I have the 'Idling
> fix' and the rc6 enabled), but I have the same freeze.

Can you send me your kernel .config file? I'm still not having any luck
reproducing your problems, using an X201s with the 1.22 BIOS version.

I'm wondering if the trouble is caused by the the selection of kernel config
parameters

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PULL] drm-intel-next

[Keith Packard]

On Wed, 10 Aug 2011 12:20:14 -0400, Andy Lutomirski  wrote:

> Can you ack at least this one:
> 
> >Revert and fix "drm/i915/dp: remove DPMS mode tracking from DP"
> (i.e. d2b996ac698aebb28557355857927b8b934bb4f9)
> 
> for -stable?  It fixes an annoying regression in 3.0.

I'm working on a set of patches for -stable; we've found a number of
small patches that fix some fairly serious regressions.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Re: [PULL] drm-intel-next

[Keith Packard]

On Wed, 10 Aug 2011 12:20:14 -0400, Andy Lutomirski l...@mit.edu wrote:

 Can you ack at least this one:
 
 Revert and fix drm/i915/dp: remove DPMS mode tracking from DP
 (i.e. d2b996ac698aebb28557355857927b8b934bb4f9)
 
 for -stable?  It fixes an annoying regression in 3.0.

I'm working on a set of patches for -stable; we've found a number of
small patches that fix some fairly serious regressions.

-- 
keith.pack...@intel.com


pgpGyMjuOrpj5.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Keith Packard]

On Mon, 8 Aug 2011 13:01:28 -0700, Jesse Barnes  
wrote:
> On Mon, 08 Aug 2011 12:53:31 -0700
> Keith Packard  wrote:
> 
> > On Mon, 8 Aug 2011 11:49:54 -0700, Jesse Barnes  > virtuousgeek.org> wrote:
> > 
> > > Yep, it's safe and possible to do on pre-PCH as well.  For panel
> > > fitting we do need to do an actual power cycle when going from
> > > non-native back to native iirc, but we can still leave them unlocked so
> > > we don't have to worry about the lock/unlock sequence everywhere.
> > 
> > Hidden in the unlock patch was a call to intel_lvds_disable from
> > intel_lvds_prepare -- that *always* turns off the LVDS for mode
> > setting. Do we care enough about LVDS mode setting performance that we
> > should try leave the optimization in place that doesn't turn off the
> > backlight when switching between modes?
> 
> We hate flicker right?  But generally yes it's safer to just turn it
> off all the time.

I'll leave the optimization in place then; it's been there for a while
so at least it shouldn't cause any regressions.

How about this? Has the advantage of not lying in the commit message
anymore.

[Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Keith Packard]

On Mon, 8 Aug 2011 11:49:54 -0700, Jesse Barnes  
wrote:

> Yep, it's safe and possible to do on pre-PCH as well.  For panel
> fitting we do need to do an actual power cycle when going from
> non-native back to native iirc, but we can still leave them unlocked so
> we don't have to worry about the lock/unlock sequence everywhere.

Hidden in the unlock patch was a call to intel_lvds_disable from
intel_lvds_prepare -- that *always* turns off the LVDS for mode
setting. Do we care enough about LVDS mode setting performance that we
should try leave the optimization in place that doesn't turn off the
backlight when switching between modes?

Here's a replacement which unlocks the regs at init time for all
generations. This also includes the unconditional call to
intel_lvds_disable in the _prepare function. I could back that out if
you like.

[Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Keith Packard]

On Mon, 8 Aug 2011 09:30:10 -0700, Jesse Barnes  
wrote:

> Yep, looks fine.  The only think we might want to sprinkle about are
> checks for panel off so we can avoid visible corruption if we whack
> timing or fb stuff while the panel is on.

So, I'd like to know if we could unlock the panel registers on pre-PCH
hardware as well at init time; that way I could remove the unlock code

[Intel-gfx] [PATCH 1/4] drm/i915: Wait for LVDS panel power sequence

[Keith Packard]

On Mon, 8 Aug 2011 09:27:19 -0700, Jesse Barnes  
wrote:

> ...to catch places like this where the wrong register gets used. :)

Ouch! There are only two places we *should* have these loops, one when
turning it off, another when turning it on. There's a couple of loops
which just need to be removed.

Here's a replacement patch:

[Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Keith Packard]

On Mon, 8 Aug 2011 09:30:10 -0700, Jesse Barnes  
wrote:

> Yep, looks fine.  The only think we might want to sprinkle about are
> checks for panel off so we can avoid visible corruption if we whack
> timing or fb stuff while the panel is on.

Yeah, could do. Would be nice to somehow get the LVDS code ripped out of
the middle of intel_display.c; everything in intel_lvds.c is nicely
bracketed by prepare/commit, which turn the panel off and back on.

-- 
keith.packard at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Leave LVDS registers unlocked

[Keith Packard]

On Mon, 8 Aug 2011 09:30:10 -0700, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 Yep, looks fine.  The only think we might want to sprinkle about are
 checks for panel off so we can avoid visible corruption if we whack
 timing or fb stuff while the panel is on.

Yeah, could do. Would be nice to somehow get the LVDS code ripped out of
the middle of intel_display.c; everything in intel_lvds.c is nicely
bracketed by prepare/commit, which turn the panel off and back on.

-- 
keith.pack...@intel.com


pgp726no3kU84.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Wait for LVDS panel power sequence

[Keith Packard]

On Mon, 8 Aug 2011 09:27:19 -0700, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 ...to catch places like this where the wrong register gets used. :)

Ouch! There are only two places we *should* have these loops, one when
turning it off, another when turning it on. There's a couple of loops
which just need to be removed.

Here's a replacement patch:

From 436f2b19cf17c43e4d5ad55b47aeb3660c2af9b9 Mon Sep 17 00:00:00 2001
From: Keith Packard kei...@keithp.com
Date: Sat, 6 Aug 2011 10:30:45 -0700
Subject: [PATCH] drm/i915: Wait for LVDS panel power sequence

During mode setting, check to make sure the panel power sequencing has
completed before doing further operations on the device. This
uncovered errors with DPMS not turning the device off as it was left locked.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/intel_lvds.c |   26 ++
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
b/drivers/gpu/drm/i915/intel_lvds.c
index 2e8ddfc..6318828 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -72,14 +72,16 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 {
struct drm_device *dev = intel_lvds-base.base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
-   u32 ctl_reg, lvds_reg;
+   u32 ctl_reg, lvds_reg, stat_reg;
 
if (HAS_PCH_SPLIT(dev)) {
ctl_reg = PCH_PP_CONTROL;
lvds_reg = PCH_LVDS;
+   stat_reg = PCH_PP_STATUS;
} else {
ctl_reg = PP_CONTROL;
lvds_reg = LVDS;
+   stat_reg = PP_STATUS;
}
 
I915_WRITE(lvds_reg, I915_READ(lvds_reg) | LVDS_PORT_EN);
@@ -94,17 +96,16 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
DRM_DEBUG_KMS(applying panel-fitter: %x, %x\n,
  intel_lvds-pfit_control,
  intel_lvds-pfit_pgm_ratios);
-   if (wait_for((I915_READ(PP_STATUS)  PP_ON) == 0, 1000)) {
-   DRM_ERROR(timed out waiting for panel to power off\n);
-   } else {
-   I915_WRITE(PFIT_PGM_RATIOS, 
intel_lvds-pfit_pgm_ratios);
-   I915_WRITE(PFIT_CONTROL, intel_lvds-pfit_control);
-   intel_lvds-pfit_dirty = false;
-   }
+
+   I915_WRITE(PFIT_PGM_RATIOS, intel_lvds-pfit_pgm_ratios);
+   I915_WRITE(PFIT_CONTROL, intel_lvds-pfit_control);
+   intel_lvds-pfit_dirty = false;
}
 
I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
POSTING_READ(lvds_reg);
+   if (wait_for((I915_READ(stat_reg)  PP_ON) != 0, 1000))
+   DRM_ERROR(timed out waiting for panel to power on\n);
 
intel_panel_enable_backlight(dev);
 }
@@ -113,24 +114,25 @@ static void intel_lvds_disable(struct intel_lvds 
*intel_lvds)
 {
struct drm_device *dev = intel_lvds-base.base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
-   u32 ctl_reg, lvds_reg;
+   u32 ctl_reg, lvds_reg, stat_reg;
 
if (HAS_PCH_SPLIT(dev)) {
ctl_reg = PCH_PP_CONTROL;
lvds_reg = PCH_LVDS;
+   stat_reg = PCH_PP_STATUS;
} else {
ctl_reg = PP_CONTROL;
lvds_reg = LVDS;
+   stat_reg = PP_STATUS;
}
 
intel_panel_disable_backlight(dev);
 
I915_WRITE(ctl_reg, I915_READ(ctl_reg)  ~POWER_TARGET_ON);
+   if (wait_for((I915_READ(stat_reg)  PP_ON) == 0, 1000))
+   DRM_ERROR(timed out waiting for panel to power off\n);
 
if (intel_lvds-pfit_control) {
-   if (wait_for((I915_READ(PP_STATUS)  PP_ON) == 0, 1000))
-   DRM_ERROR(timed out waiting for panel to power off\n);
-
I915_WRITE(PFIT_CONTROL, 0);
intel_lvds-pfit_dirty = true;
}
-- 
1.7.5.4

-- 
keith.pack...@intel.com


pgpSwVs9ixH0m.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel