Re: [PATCH] drm/i915: Get correct display clock on 945gm

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Jan 27, 2017 at 06:24:25PM +0100, Arthur Heymans wrote:
> Ville Syrjälä <ville.syrjala@linux.intel.com> writes:
> 
> >> 
> >> > Now if someone could figure out where to dig up the DDR and FSB clocks
> >> > we could also fix up the 190 vs. 200 MHz case...
> >> >
> >> >> +		default:
> >> >> +		case GC_DISPLAY_CLOCK_190_200_MHZ:
> >> >> +			return 200000;
> >> >> +		}
> >> >> +	}
> >> >> +}
> >> >> +
> >> 
> >> Hmm that seems to be 915gm specific (always 200 on 945gm). According to
> >> "Mobile Intel® 915/910 Express Chipset: Datasheet", the only fsb/dram
> >> combo that has 190MHz is FSB: 533MHz, DDR333. All the other supported
> >> combos have 200Mhz set by that configuration.
> >
> > Yeah. Unfortunately I couldn't find a solid source of FSB/DDR clock
> > information in the spec, apart from the actual strap pins but I can't
> > see any register that would reflect those. So I guess we'll just leave
> > it the way it is.
> 
> Hmm "Mobile Intel® 915GM/PM/GME/GMS and 910GML/GMLE Express Chipset
> Specification Update" seems to sketch an even more complicated scenario,
> where it even depends on the GMCH subtype...

Oh, I didn't even have that particular document here. Have it now, and
indeed it shows even more variation.

> 
> FSB and also DRAM freq can be probably be fetched from respectively
> [2:0] and [6:4] of CLKCFG, MCHBAR(0xC00) but not documented.

I managed to dig up one more doc where the 6:4 bits are listed as
1 = 333
2 = 400
3 = 533

so those seem to agree with the 945gm version, with the 333 case
added. The FSB speed I didn't manage to find in any 915 doc
unfortunately. I suppose we could assume it matches 945gm with
the extra 0=400 case added.

Apparently there's also CLKCFG bit 16 on 915gm which seems to be
telling me something about the ddr speed and voltage:
0 = 333,400 (533 @ 1.5v)
1 = 533 @ 1.05V

so that could potentially solve part the voltage detection. Sadly only
for DDR 533, if it's even correct.

So I see a couple of options we could do here:
a) ignore all of it and just go with your original change of for the
   320 vs. 333 case
b) ignore most of these details and just pick either the 190/213 or
   200/222 as our presumed clocks
c) just use the fsb/ddr info and again just assume one or the other
   for the cases where the voltage matters
d) just assume the 945 DFT_STRAP1 detection of the voltage works on 915
   and deal with every case. We wouldn't that far off the mark even if
   we got it wrong so I don't see a huge problem with just guessing
e) something I didn't think of?

I'll let you figure it out ;)

> On 945GM I think the voltage strap is on DFT_STRAP1 bit20, MCHBAR(0xE08)
> (also not really documented but this is how it is done in Coreboot).
> It might be the same on 915GM...
> 
> Given that getting the 915GM display clock correctly probably ends up
> being quite different from 945GM it might make sense to have 2 separate
> functions for that?
> 
> Kind regards
> -- 
> Arthur Heymans

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx