On Wed, 28 Sep 2011 10:09:13 +0100, Chris Wilson <ch...@chris-wilson.co.uk> 
wrote:

> 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.

> 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.

> 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.

> As an incremental improvement [in my understanding ;-]:
> Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk>

Perhaps this patch on top of the existing patch?

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 1cc0962..4bf49eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -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;
                        }

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

Attachment: pgpgzyVssPKt9.pgp
Description: PGP signature

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

Reply via email to