Re: [PATCH] drm/i915: add missing "break"

From: Keith Packard <keithp@keithp.com>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: add missing "break"
Date: Fri, 23 Sep 2011 09:15:05 -0700	[thread overview]
Message-ID: <yuny5xfz14m.fsf@aiko.keithp.com> (raw)
In-Reply-To: <CA+gsUGTuH7H_4TsQFwoD+rrFT0jtcRNeMYtrT3GrHQkujroHjg@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 2017 bytes --]

On Fri, 23 Sep 2011 09:06:31 -0300, Paulo Zanoni <przanoni@gmail.com> wrote:

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

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.

> And for Keith's patch:
> 
> Tested-By: Paulo Zanoni <paulo.r.zanoni@intel.com>

Can you explain what you tested?

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
