Re: [PATCH 4/9] drm/i915: fixup g4x pll limits

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

On Wed, Jun 12, 2013 at 12:07:39AM +0100, Damien Lespiau wrote:
> On Tue, May 21, 2013 at 09:54:54PM +0200, Daniel Vetter wrote:
> > Again the same confusion that our code expects m1/m2 in register values.
> > This time around with the added fun that many of the existing values
> > have been all off by a bit in different directions. Hence extract a
> > common #define.
> > 
> > Note that n limits differ between lvds and other outputs. Strangely they've
> > all been correct already.
> > 
> > v2: Rebased on top of the DP pll rework, which makes it even more
> > obvious that we can do this ...
> 
> I'm a bit confused by this one:
> 
>   - I don't seem to find the special LVDS limits for n

Me neither.

>   - Those limits are gated by a IS_G4X(). IS_G4X() is true for eaglelake
>     and cantiga. The docs single out Cantiga and we seem to need a
>     different set of limits for that platform (compared to the rest of
>     gen4) platforms?

I've noticed that the docs often mention just CTG leaving out ELK, even
though the stuff it's describing applies to both.

>   - The limits for n are 3-6 or 3-5 in this code but I read 3-8 and 5-6
>     (cantiga)

I see 3-8 and 4-6.

>   - m doesn't seem to match what I have, it seems like it could be the
>     cantiga values (except that m is in "formula space" so we don't
>     substract 2)

105-140 is what the spec tells me, so a bit off either way.

There's more inconsistency w/ the p/p1 limits too, and the single
channel lvds dotclock limit doesn't agree w/ the spec either.

>   - m1 and m2 seem to match what I have for cantiga, are we supposed to
>     have those limits for eaglelake as well?
> 
> Well, the only conclusion is that I must be reading the wrong docs, or?
> 
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++------------
> >  1 file changed, 8 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ea8eb0c..cb54131 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -136,14 +136,16 @@ static const intel_limit_t intel_limits_i9xx_lvds = {
> >  		.p2_slow = 14, .p2_fast = 7 },
> >  };
> >  
> > +#define G4X_DPLL_M_LIMITS \
> > +	.m = { .min = 104, .max = 138 },\
> > +	.m1 = { .min = 15, .max = 21 },\
> > +	.m2 = { .min = 3, .max = 11 },
> >  
> >  static const intel_limit_t intel_limits_g4x_sdvo = {
> >  	.dot = { .min = 25000, .max = 270000 },
> >  	.vco = { .min = 1750000, .max = 3500000},
> >  	.n = { .min = 1, .max = 4 },
> > -	.m = { .min = 104, .max = 138 },
> > -	.m1 = { .min = 17, .max = 23 },
> > -	.m2 = { .min = 5, .max = 11 },
> > +	G4X_DPLL_M_LIMITS
> >  	.p = { .min = 10, .max = 30 },
> >  	.p1 = { .min = 1, .max = 3},
> >  	.p2 = { .dot_limit = 270000,
> > @@ -156,9 +158,7 @@ static const intel_limit_t intel_limits_g4x_hdmi = {
> >  	.dot = { .min = 22000, .max = 400000 },
> >  	.vco = { .min = 1750000, .max = 3500000},
> >  	.n = { .min = 1, .max = 4 },
> > -	.m = { .min = 104, .max = 138 },
> > -	.m1 = { .min = 16, .max = 23 },
> > -	.m2 = { .min = 5, .max = 11 },
> > +	G4X_DPLL_M_LIMITS
> >  	.p = { .min = 5, .max = 80 },
> >  	.p1 = { .min = 1, .max = 8},
> >  	.p2 = { .dot_limit = 165000,
> > @@ -169,9 +169,7 @@ static const intel_limit_t intel_limits_g4x_single_channel_lvds = {
> >  	.dot = { .min = 20000, .max = 115000 },
> >  	.vco = { .min = 1750000, .max = 3500000 },
> >  	.n = { .min = 1, .max = 3 },
> > -	.m = { .min = 104, .max = 138 },
> > -	.m1 = { .min = 17, .max = 23 },
> > -	.m2 = { .min = 5, .max = 11 },
> > +	G4X_DPLL_M_LIMITS
> >  	.p = { .min = 28, .max = 112 },
> >  	.p1 = { .min = 2, .max = 8 },
> >  	.p2 = { .dot_limit = 0,
> > @@ -183,9 +181,7 @@ static const intel_limit_t intel_limits_g4x_dual_channel_lvds = {
> >  	.dot = { .min = 80000, .max = 224000 },
> >  	.vco = { .min = 1750000, .max = 3500000 },
> >  	.n = { .min = 1, .max = 3 },
> > -	.m = { .min = 104, .max = 138 },
> > -	.m1 = { .min = 17, .max = 23 },
> > -	.m2 = { .min = 5, .max = 11 },
> > +	G4X_DPLL_M_LIMITS
> >  	.p = { .min = 14, .max = 42 },
> >  	.p1 = { .min = 2, .max = 6 },
> >  	.p2 = { .dot_limit = 0,
> > -- 
> > 1.7.11.7
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC