drm/i915: add missing "break"
Subsystems (2)
PATCH 1/1 drm/i915: add missing "break"
From: Paulo Zanoni <[email protected]>
It seems to be missing from this commit:
"drm/i915: split out PCH refclk update code"
--- drivers/gpu/drm/i915/intel_display.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/gpu/drm/i915/intel_display.c +1 -0
@@ -5120,6 +5120,7 @@ static void ironlake_update_pch_refclk(struct drm_device *dev)switch (encoder->type) {case INTEL_OUTPUT_LVDS : has_lvds = true; +break ;case INTEL_OUTPUT_EDP : has_edp_encoder = encoder;break ;
Signature · 1 line
1.7.4.1
Quoted 2 lines · On Thu, 22 Sep 2011 15:13:42 -0300, [email protected] wrote:
It seems to be missing from this commit:
"drm/i915: split out PCH refclk update code"
Oh, this code is missing far more than that. It doesn't deal with the LVDS case at all.
Here's a patch which turns on the right bits of SSC, in the right order, for any combination of CPU-eDP, PCH-eDP and LVDS.
Jesse: you wrote this stuff, can you review what I did? Also, my two SNB systems (X220 and MacBook Air) both have a BIOS table that disables SSC, is there any reason for us to believe the BIOS table on a PCH system? Can't we always use SSC?
I've tested this on LVDS and CPU-connected eDP with and without SSC and it seems to work for me.
From ea63d98df915f81c80cf4f91bf9d1bcaece7fce0 Mon Sep 17 00:00:00 2001
From: Keith Packard <[email protected]>
Date: Thu, 22 Sep 2011 12:01:57 -0700
Subject: [PATCH 1/3] drm/i915: Fix PCH SSC reference clock settings
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.
--- drivers/gpu/drm/i915/intel_display.c | 88 ++++++++++++++++++++++++--------- 1 files changed, 64 insertions(+), 24 deletions(-)
drivers/gpu/drm/i915/intel_display.c +64 -24
@@ -5113,9 +5113,11 @@ static void ironlake_update_pch_refclk(struct drm_device *dev)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) {
@@ -5124,18 +5126,23 @@ static void ironlake_update_pch_refclk(struct drm_device *dev)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_panel = true; has_lvds = true; +break ;case INTEL_OUTPUT_EDP : - has_edp_encoder = encoder; + 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
@@ -5146,36 +5153,69 @@ 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 ; + temp &= ~DREF_SUPERSPREAD_SOURCE_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 |=DREF_CPU_SOURCE_OUTPUT_DISABLE ; + +/* Enable SSC on PCH eDP or LVDS if needed */ +if ((has_pch_edp || has_lvds) &&intel_panel_use_ssc (dev_priv)) { +DRM_DEBUG_KMS ("Using SSC on PCH eDP or LVDS\n" ); + temp |=DREF_SUPERSPREAD_SOURCE_ENABLE ; + }else + temp |=DREF_SUPERSPREAD_SOURCE_DISABLE ; + +I915_WRITE (PCH_DREF_CONTROL , temp); +POSTING_READ (PCH_DREF_CONTROL ); +udelay (200 ); + }else { + temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK ; + temp &= ~DREF_SUPERSPREAD_SOURCE_MASK ; + +DRM_DEBUG_KMS ("Disabling SSC entirely\n" ); + +/* Turn off CPU output */ + temp |=DREF_CPU_SOURCE_OUTPUT_DISABLE ; + +/* Disable SSC on PCH */ + temp |=DREF_SUPERSPREAD_SOURCE_DISABLE ; + +I915_WRITE (PCH_DREF_CONTROL , temp); +POSTING_READ (PCH_DREF_CONTROL ); +udelay (200 ); + +/* Turn off the SSC source */ + temp &= ~DREF_SSC_SOURCE_MASK ; + temp |=DREF_SSC_SOURCE_DISABLE ; + +/* Turn off SSC1 */ + temp &= ~DREF_SSC1_ENABLE ; +I915_WRITE (PCH_DREF_CONTROL , temp);POSTING_READ (PCH_DREF_CONTROL );udelay (200 );
Signature · 7 lines
1.7.6.3 -- [email protected]
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 8 lines · On Thu, 22 Sep 2011 12:55:22 -0700 Keith Packard <[email protected]> wrote:
Quoted 2 lines · On Thu, 22 Sep 2011 15:13:42 -0300, [email protected] wrote:
It seems to be missing from this commit:
"drm/i915: split out PCH refclk update code"Oh, this code is missing far more than that. It doesn't deal with the LVDS case at all.
Quoted 7 lines
Here's a patch which turns on the right bits of SSC, in the right order, for any combination of CPU-eDP, PCH-eDP and LVDS.
Jesse: you wrote this stuff, can you review what I did? Also, my two SNB systems (X220 and MacBook Air) both have a BIOS table that disables SSC, is there any reason for us to believe the BIOS table on a PCH system? Can't we always use SSC?
I think it depends on the platform. On some, enabling SSC may actually create more noise than not for some components (not that I've run the EMF calculations...).
I don't have this code in my tree though... is this the patch I sent awhile back? I thought it broke external outputs too? The last time we touched this we broke the dual head case (a config change caused one head to go blank), did you test that?
What I don't understand about the refclk code is that we should be able to leave everything enabled and just select the right clock source in the DPLL_SEL bits. But that doesn't seem to help the wavy VGA bug, since in that case I think we're explicitly choosing the non-SSC clock and we still get waviness.
I *think* the code you changed is ok; just needs lots of testing and verification that the SSC bits are set like we expect as we change configurations. I like the "has_panel" cleanup too; previous versions of this code had is_lvds || is_edp && pch_edp etc sprinkled all over.
Jesse
Quoted 3 lines · Jesse Barnes
I think it depends on the platform. On some, enabling SSC may actually create more noise than not for some components (not that I've run the EMF calculations...).
So, why is the MBA EFI code enabling SSC while the VBT table says not to use it? It's all mysterious to me.
Quoted 4 lines
I don't have this code in my tree though... is this the patch I sent awhile back? I thought it broke external outputs too? The last time we touched this we broke the dual head case (a config change caused one head to go blank), did you test that?
You sent two pieces, I think; the second one tried to turn unused clocks off and that broke stuff. Which is understandable given that it was only looking at one CRTC when doing the global configuration.
Quoted 5 lines
What I don't understand about the refclk code is that we should be able to leave everything enabled and just select the right clock source in the
DPLL_SELbits. But that doesn't seem to help the wavy VGA bug, since in that case I think we're explicitly choosing the non-SSC clock and we still get waviness.
We don't have any hardware anywhere which exhibits this problem, do we? Getting hold of some would let us poke at it.
Quoted 4 lines
I *think* the code you changed is ok; just needs lots of testing and verification that the SSC bits are set like we expect as we change configurations. I like the "
has_panel" cleanup too; previous versions of this code hadis_lvds||is_edp&&pch_edpetc sprinkled all over.
Yeah, it's longer, but I think it's more readable now.
My big concern is that we're guessing which pipes will use SSC in this function and then computing which pipes actually use SSC separately. I think we should figure out which pipes want to use SSC and then go set the refclks. as needed.
Signature · 1 line
[email protected]
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Paulo Zanoni
Tested-byHi
2011/9/23 Keith Packard <[email protected]>:
Quoted 9 lines · Jesse Barnes
Quoted 5 lines
What I don't understand about the refclk code is that we should be able to leave everything enabled and just select the right clock source in the
DPLL_SELbits. But that doesn't seem to help the wavy VGA bug, since in that case I think we're explicitly choosing the non-SSC clock and we still get waviness.We don't have any hardware anywhere which exhibits this problem, do we? Getting hold of some would let us poke at it.
My personal laptop reproduces the "wavy output" problem.
If you look at the patch posted on comment #20 of bug #38750 [0], you'll see that it checks on the video bios for a field called "display_clock_mode" and then uses this field in the following code:
+ if (dev_priv->display_clock_mode)
+ temp |= DREF_NONSPREAD_CK505_ENABLE;
+ else
+ temp |= DREF_NONSPREAD_SOURCE_ENABLE;
This is what fixes the wavy output problem: setting DREF_NONSPREAD_CK505_ENABLE instead of the other bits.
And for Keith's patch:
[0]: https://bugs.freedesktop.org/attachment.cgi?id=49888
Signature · 1 line
Paulo Zanoni
Quoted 10 lines · Paulo Zanoni
If you look at the patch posted on comment #20 of bug #38750 [0], you'll see that it checks on the video bios for a field called "
display_clock_mode" and then uses this field in the following code:(quoted diff) +4
-0+ if (dev_priv->display_clock_mode) + temp |= DREF_NONSPREAD_CK505_ENABLE; + else + temp |= DREF_NONSPREAD_SOURCE_ENABLE;This is what fixes the wavy output problem: setting
DREF_NONSPREAD_CK505_ENABLEinstead of the other bits.
Ok, so I found out what CK505 is -- it's an external clock synthesizer for ancient CPUs (from 2006).
It seems like
Next, I went and looked at the Sandybridge VBIOS sources. That's quite interesting. I expected to see it looking at the display_clock_mode bit to select between the internal and CK505 sources for the non-spread clock, but it doesn't -- it always uses the CK505 source. In other news, it has a nice comment about the clock source not updating unless the DPLL is off.
I also found a few more interesting bits in the general features table:
@@ -133,7 +133,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 */
What I didn't find there was any mention of the display_clock_mode field; perhaps jbarnes has newer VBIOS sources or actual BDB documentation.
What I'm starting to suspect is that there are two clock sources, the internal one and the CK505 one and that if we need a non-SSC clock and can't use the CK505 one, then we can't turn on SSC for the internal one.
Quoted 3 lines
And for Keith's patch:
Tested-by Paulo Zanoni <[email protected]>
Can you explain what you tested?
Signature · 1 line
[email protected]
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
2011/9/23 Keith Packard <[email protected]>:
Quoted 2 lines
Can you explain what you tested?
Sure. It's an Ironlake laptop so I tested its LVDS, and I also plugged/unplugged the VGA output a few times, checked the value of the DREF_CONTROL register, tried to replace the SOURCE_ENABLE for CK505_ENABLE (to stop the wavy external output), rebooted a few times and used the machine for a few hours since yesterday. The patch is applied on top of your drm-intel-next tree. Nothing exploded so far.
Signature · 1 line
Paulo Zanoni
Quoted 3 lines · Keith Packard
What I didn't find there was any mention of the
display_clock_modefield; perhaps jbarnes has newer VBIOS sources or actual BDB documentation.
iirc display_clock_mode was found in the Capella VBIOS assembly, so indeed comparatively ancient.
I do enjoy how our best guides for programming the hardware comes from RE the bioses. :(
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
Quoted 7 lines · Chris Wilson
Quoted 3 lines · Keith Packard
What I didn't find there was any mention of the
display_clock_modefield; perhaps jbarnes has newer VBIOS sources or actual BDB documentation.iirc
display_clock_modewas found in the Capella VBIOS assembly, so indeed comparatively ancient.
Ok, so it appears to have disappeared in more recent hardware. I'm betting that with PCH hardware, there is always a CK505 source to use, so the BIOS just uses it unconditionally and may not actually be configured in the VBT.
Quoted 2 lines
I do enjoy how our best guides for programming the hardware comes from RE the bioses. :(
It's not the best, it's just yet another source of information :-)
So, what I think we should be doing is using display_clock_mode to select between ck505 and internal clock source on pre-PCH hardware. For PCH hardware, we unconditionally use the ck505 source for the non-SSC reference. When we need to drive two outputs and are *not* using ck505, then we disable SSC.
Seem reasonable?
Signature · 1 line
[email protected]
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 5 lines · Keith Packard
So, what I think we should be doing is using
display_clock_modeto select between ck505 and internal clock source on pre-PCH hardware. For PCH hardware, we unconditionally use the ck505 source for the non-SSC reference. When we need to drive two outputs and are *not* using ck505, then we disable SSC.
Ok, I'm not sure this is trivial -- without CK505, how do we switch from LVDS to LVDS+VGA (and back)? Seems like we'd have to turn the LVDS off and back on to play with SSC. Is it worth dealing with that case? Or do we just ignore SSC on non-CK505 hardware?
Signature · 1 line
[email protected]
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PCH reference clock cleanups
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?
PATCH 1/9 drm/i915: broken copyright encoding in intel_bios.c
--- drivers/gpu/drm/i915/intel_bios.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/gpu/drm/i915/intel_bios.c +1 -1
@@ -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"),
Signature · 1 line
1.7.6.3
PATCH 2/9 drm/i915: Use DRM_DEBUG_KMS for all messages in intel_bios.c
In reply to Keith PackardThese are all KMS related anyways, so don't hide them under other debug levels.
--- drivers/gpu/drm/i915/intel_bios.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_bios.c +7 -2
@@ -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
Signature · 1 line
1.7.6.3
Re: [PATCH 2/9] drm/i915: Use DRM_DEBUG_KMS for all messages in intel_bios.c
Quoted 4 lines · Keith Packard
These are all KMS related anyways, so don't hide them under other debug levels.
Signed-off-by Keith Packard <[email protected]>
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
PATCH 3/9 drv/i915: Pull display_clock_mode out of VBT table
In reply to Keith PackardThis 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.
--- 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(-)
drivers/gpu/drm/i915/i915_drv.h +1 -0
@@ -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;
drivers/gpu/drm/i915/intel_bios.c +4 -2
@@ -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); } }
drivers/gpu/drm/i915/intel_bios.h +3 -1
@@ -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;
Signature · 1 line
1.7.6.3
Re: [PATCH 3/9] drv/i915: Pull display_clock_mode out of VBT table
Quoted 5 lines · 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 <[email protected]>
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
PATCH 4/9 drm/i915: Document a few more BDB_GENERAL_FEATURES bits from PCH BIOS
In reply to Keith PackardThis includes whether an eDP panel is present, and whether that should use SSC (and at what frequency)
--- drivers/gpu/drm/i915/intel_bios.h | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
drivers/gpu/drm/i915/intel_bios.h +4 -1
@@ -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 */
Signature · 1 line
1.7.6.3
PATCH 5/9 drm/i915: Allow SSC parameter to override VBT value
In reply to Keith PackardAllow SSC to be enabled even when the BIOS disables it for testing SSC paths.
--- drivers/gpu/drm/i915/i915_drv.c | 4 ++-- drivers/gpu/drm/i915/intel_display.c | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/i915_drv.c +2 -2
@@ -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 );
drivers/gpu/drm/i915/intel_display.c +3 -1
@@ -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 ); }
Signature · 1 line
1.7.6.3
Re: [PATCH 5/9] drm/i915: Allow SSC parameter to override VBT value
Quoted 3 lines · Keith Packard
Allow SSC to be enabled even when the BIOS disables it for testing SSC paths.
Signed-off-by Keith Packard <[email protected]>
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
PATCH 6/9 drm/i915: Fix PCH SSC reference clock settings
In reply to Keith PackardThe 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.
--- drivers/gpu/drm/i915/intel_display.c | 96 +++++++++++++++++++++------------- 1 files changed, 59 insertions(+), 37 deletions(-)
drivers/gpu/drm/i915/intel_display.c +59 -37
@@ -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 |=DREF_CPU_SOURCE_OUTPUT_DISABLE ; + +I915_WRITE (PCH_DREF_CONTROL , temp); +POSTING_READ (PCH_DREF_CONTROL ); +udelay (200 ); + }else { +DRM_DEBUG_KMS ("Disabling SSC entirely\n" ); + + temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK ; + +/* Turn off CPU output */ + temp |=DREF_CPU_SOURCE_OUTPUT_DISABLE ; + +I915_WRITE (PCH_DREF_CONTROL , temp); +POSTING_READ (PCH_DREF_CONTROL ); +udelay (200 ); + +/* Turn off the SSC source */ + temp &= ~DREF_SSC_SOURCE_MASK ; + temp |=DREF_SSC_SOURCE_DISABLE ; + +/* Turn off SSC1 */ + temp &= ~DREF_SSC1_ENABLE ; +I915_WRITE (PCH_DREF_CONTROL , temp);POSTING_READ (PCH_DREF_CONTROL );udelay (200 );
Signature · 1 line
1.7.6.3
Re: [PATCH 6/9] drm/i915: Fix PCH SSC reference clock settings
Quoted 4 lines · 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).
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.
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.
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
Re: [PATCH 6/9] drm/i915: Fix PCH SSC reference clock settings
Quoted 8 lines · Chris Wilson
Quoted 4 lines · 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).
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.
Quoted 3 lines
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.
Signature · 1 line
[email protected]
Chris Wilson
Reviewed-byRe: [PATCH 6/9] drm/i915: Fix PCH SSC reference clock settings
On Tue, 27 Sep 2011 11:03:43 -0700, Keith Packard <[email protected]> wrote:
Non-text part: multipart/signed
Quoted 18 lines · Chris Wilson
Quoted 8 lines · Keith Packard
Quoted 4 lines
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?
Quoted 4 lines
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.
My understanding was that we could not enable SSC at all if we had a VGA, DVI/HDMI or TV output; DP may or may not work with SSC.
The patch says that we will want to enable SSC if we have an SSC capbable LVDS or eDP, which is certainly true. And that we can always do so if we remember to set a magic bit in refclk to prevent non-SSC capable outputs from being upset. I have not seen anything to support that last statement, but, then again, I have not seen anything that actually explains what CK505 is!
Having said that, this is an obvious improvement over the current situation in that we do choose correctly in more circumstances and we do not reprogram the refclk whilst active.
As an incremental improvement [in my understanding ;-]:
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
Re: [PATCH 6/9] drm/i915: Fix PCH SSC reference clock settings
Quoted 2 lines · Chris Wilson
My understanding was that we could not enable SSC at all if we had a VGA, DVI/HDMI or TV output; DP may or may not work with SSC.
Yeah, which makes no sense at all. If this were true, we'd have to turn off the LVDS/eDP panel whenever enabling one of the non-SSC outputs.
Quoted 6 lines
The patch says that we will want to enable SSC if we have an SSC capbable LVDS or eDP, which is certainly true. And that we can always do so if we remember to set a magic bit in refclk to prevent non-SSC capable outputs from being upset. I have not seen anything to support that last statement, but, then again, I have not seen anything that actually explains what CK505 is!
CK505 is an Intel specification for external clock synthesizers. Here's an older one made by Silego:
http://www.silego.com/uploads/Products/product_54/xSLG8SP585r101_10062009.pdf
There are newer ones which provide the (required?) 120MHz output specified in the bspec.
However, if you go read:
http://www.advantech.com.tw/embcore/promotions/Whitepaper/2nd_Gen_Intel_Core_i7_Processors.pdf
you'll see that Cougarpoint is reported to have a fully integrated clocking solution and not require an external CK505. Which makes the lack of the display_clock_mode bit in the VBIOS sources a lot more understandable.
So, I think we should not look for a CK505 on Cougar Point systems, only on Ibex Peak systems, and that we probably cannot use SSC on Ibex Peak unless we have a CK505.
Quoted 3 lines
Having said that, this is an obvious improvement over the current situation in that we do choose correctly in more circumstances and we do not reprogram the refclk whilst active.
I think we can reprogram the refclk after init time, but only if no-one is using it, which is something we can do later on.
Quoted 2 lines
As an incremental improvement [in my understanding ;-]:
Reviewed-by Chris Wilson <[email protected]>
Perhaps this patch on top of the existing patch?
drivers/gpu/drm/i915/intel_display.c +14 -4
@@ -5122,6 +5122,8 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)bool has_cpu_edp = false;bool has_pch_edp = false;bool has_panel = false; +bool has_ck505 = false; +bool can_ssc = false;/* We need to take the global config into account */ list_for_each_entry(encoder, &mode_config->encoder_list,
@@ -5141,9 +5143,17 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) } } +if (HAS_PCH_IBX (dev)) { + has_ck505 = dev_priv->display_clock_mode; + can_ssc = has_ck505; + }else { + has_ck505 = false; + can_ssc = true; + } +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); + has_ck505);/* Ironlake: try to setup display ref clock before DPLL * enabling. This is only under driver's control after
@@ -5154,7 +5164,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)/* Always enable nonspread source */ temp &= ~DREF_NONSPREAD_SOURCE_MASK ; -if (dev_priv->display_clock_mode) +if (has_ck505) temp |=DREF_NONSPREAD_CK505_ENABLE ;else temp |=DREF_NONSPREAD_SOURCE_ENABLE ;
@@ -5164,7 +5174,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) temp |=DREF_SSC_SOURCE_ENABLE ;/* SSC must be turned on before enabling the CPU output */ -if (intel_panel_use_ssc (dev_priv)) { +if (intel_panel_use_ssc (dev_priv) && can_ssc) {DRM_DEBUG_KMS ("Using SSC on panel\n" ); temp |=DREF_SSC1_ENABLE ; }
@@ -5178,7 +5188,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)/* Enable CPU source on CPU attached eDP */ if (has_cpu_edp) { -if (intel_panel_use_ssc (dev_priv)) { +if (intel_panel_use_ssc (dev_priv) && can_ssc) {DRM_DEBUG_KMS ("Using SSC on eDP\n" ); temp |=DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD ; }
Signature · 1 line
[email protected]
PATCH 7/9 drm/i915: Use CK505 as non-SSC source where available
In reply to Keith PackardThis eliminates VGA shimmer on some Ironlake machines which have a CK505 clock source.
--- drivers/gpu/drm/i915/intel_display.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/intel_display.c +9 -3
@@ -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 ;
Signature · 1 line
1.7.6.3
Re: [PATCH 7/9] drm/i915: Use CK505 as non-SSC source where available
Quoted 4 lines · Keith Packard
This eliminates VGA shimmer on some Ironlake machines which have a CK505 clock source.
Signed-off-by Keith Packard <[email protected]>
References: https://bugzilla.kernel.org/show_bug.cgi?id=21742
References: https://bugs.freedesktop.org/show_bug.cgi?id=38750
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
PATCH 8/9 drm/i915: All PCH refclks are 120MHz
In reply to Keith PackardI can't find any reference clocks which run at 96MHz as seems to be indicated from the comments in this code.
--- drivers/gpu/drm/i915/intel_display.c | 14 ++++---------- 1 files changed, 4 insertions(+), 10 deletions(-)
drivers/gpu/drm/i915/intel_display.c +4 -10
@@ -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 =120000 ;/* 120Mhz refclk */ - } +/* +* Every reference clock in a PCH system is 120MHz +*/ + refclk =120000 ;/* * Returns a set of divisors for the desired target clock with the given
Signature · 1 line
1.7.6.3
Re: [PATCH 8/9] drm/i915: All PCH refclks are 120MHz
Quoted 4 lines · 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 <[email protected]>
I think there exists a 100MHz test mode (certainly there is reference to such in the diagram and DPCTL). But as we never use that, this should be true.
I could find no reference at all to the 96MHz clock, so from that perspective,
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
PATCH 9/9 drm/i915: Initialize PCH refclks at modeset init time
In reply to Keith PackardThe 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.
--- drivers/gpu/drm/i915/intel_display.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/intel_display.c +7 -3
@@ -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)
Signature · 1 line
1.7.6.3
Re: [PATCH 9/9] drm/i915: Initialize PCH refclks at modeset init time
Quoted 4 lines · 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.
Ah, now I see why we moved from using the active configuration earlier. ;-)
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?
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
Re: [PATCH 9/9] drm/i915: Initialize PCH refclks at modeset init time
Quoted 1 line · Chris Wilson
Ah, now I see why we moved from using the active configuration earlier. ;-)
My evil plan is revealed!
Quoted 3 lines
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?
Signature · 1 line
[email protected]
Re: [Intel-gfx] [PATCH 9/9] drm/i915: Initialize PCH refclks at modeset init time
Quoted 6 lines · On Tue, 27 Sep 2011 11:11:57 -0700 Keith Packard <[email protected]> wrote:
Quoted 1 line · Chris Wilson
Ah, now I see why we moved from using the active configuration earlier. ;-)
My evil plan is revealed!
Quoted 8 lines
Quoted 3 lines
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?
Assuming we're selecting the proper reference clock in the PLL selection code anyway...
Doing it all up front seems nicer; did you get confirmation that the "wavy VGA" bug was fixed with this series? Overall seems like a good improvement over our old PCH refclk code...
Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
PATCH 9/9 drm/i915: Initialize PCH refclks at modeset init time
In reply to Keith PackardThe 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.
---
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(-)
drivers/gpu/drm/i915/i915_drv.c +3 -0
@@ -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);
drivers/gpu/drm/i915/i915_drv.h +1 -0
@@ -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);
drivers/gpu/drm/i915/intel_display.c +7 -3
@@ -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)
Signature · 1 line
1.7.6.3
Re: PCH reference clock cleanups
In reply to Keith PackardQuoted 3 lines · Keith Packard
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?
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.
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
Re: PCH reference clock cleanups
Quoted 3 lines · Chris Wilson
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.
Signature · 1 line
[email protected]
Re: [Intel-gfx] PCH reference clock cleanups
In reply to Keith Packard2011/9/27 Keith Packard <[email protected]>:
Quoted 2 lines
Here's a patch sequence which cleans up a bunch of PCH refclk related bits.
For the series: Tested-by: Paulo Zanoni <[email protected]>
Tested all the patches on Ironlake (LVDS + VGA). Fixes fd.o bug #38750 for me.
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.
Maybe my email client/server is ruining things, but I believe patch 7 includes whitespace errors.
Quoted 31 lines
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?
--
[email protected]
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 1 line
Paulo Zanoni
Re: [Intel-gfx] PCH reference clock cleanups
Quoted 3 lines · Paulo Zanoni
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.
Signature · 1 line
[email protected]
Re: [Intel-gfx] PCH reference clock cleanups
In reply to Paulo ZanoniQuoted 6 lines · On Wed, 28 Sep 2011 15:22:48 -0300 Paulo Zanoni <[email protected]> wrote:
2011/9/27 Keith Packard <[email protected]>:
Quoted 2 lines
Here's a patch sequence which cleans up a bunch of PCH refclk related bits.
For the series: Tested-by: Paulo Zanoni <[email protected]>
Quoted 8 lines
Tested all the patches on Ironlake (LVDS + VGA). Fixes fd.o bug #38750 for me.
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.
Maybe my email client/server is ruining things, but I believe patch 7 includes whitespace errors.
Yay excellent.
Now... is keeping the various refclks enabled costing us any power? IOW, should we be trying to disable them when everything has been DPMS'd off too?
Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
Re: [Intel-gfx] PCH reference clock cleanups
Quoted 3 lines · Jesse Barnes
Now... is keeping the various refclks enabled costing us any power? IOW, should we be trying to disable them when everything has been DPMS'd off too?
That's the same as tracking usage and enabling/disabling on the fly as modes are set. I think it's possible, but I'd like to have the 'simpler' fix present before we try a power saving move.
Signature · 1 line
[email protected]
Re: [Intel-gfx] PCH reference clock cleanups
Quoted 9 lines · On Mon, 03 Oct 2011 16:18:48 -0700 Keith Packard <[email protected]> wrote:
Quoted 3 lines · Jesse Barnes
Now... is keeping the various refclks enabled costing us any power? IOW, should we be trying to disable them when everything has been DPMS'd off too?
That's the same as tracking usage and enabling/disabling on the fly as modes are set. I think it's possible, but I'd like to have the 'simpler' fix present before we try a power saving move.
Agreed; fortunately shutting everything off when no outputs are active should be simpler than trying flip the bits on & off every mode set. :)
Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
Re: [Intel-gfx] PCH reference clock cleanups
Quoted 3 lines · Jesse Barnes
Agreed; fortunately shutting everything off when no outputs are active should be simpler than trying flip the bits on & off every mode set. :)
We'd have to track when outputs are shut off; just tracking per clock doesn't seem any harder. I'll expect a patch in the morning, ok?
Signature · 1 line
[email protected]