[PATCH] drm/i915: Set i9xx lvds clock limits according to specifications

[PATCH] drm/i915: Set i9xx lvds clock limits according to specifications

[Patrik Jakobsson]

The Intel PRM says the M1 and M2 divisors must be in the range of 10-20 and 5-9.
Since we do all calculations based on them being register values (which are
subtracted by 2) we need to specify them accordingly.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/i915/intel_display.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0dfecaf..4f6c594 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -168,8 +168,8 @@ static const intel_limit_t intel_limits_i9xx_lvds = {
 	.vco = { .min = 1400000, .max = 2800000 },
 	.n = { .min = 1, .max = 6 },
 	.m = { .min = 70, .max = 120 },
-	.m1 = { .min = 10, .max = 22 },
-	.m2 = { .min = 5, .max = 9 },
+	.m1 = { .min = 8, .max = 18 },
+	.m2 = { .min = 3, .max = 7 },
 	.p = { .min = 7, .max = 98 },
 	.p1 = { .min = 1, .max = 8 },
 	.p2 = { .dot_limit = 112000,
-- 
1.7.10.4

[PATCH] drm/i915: Set i9xx sdvo clock limits according to specifications

[Patrik Jakobsson]

The Intel PRM says the M1 and M2 divisors must be in the range of 10-20 and 5-9.
Since we do all calculations based on them being register values (which are
subtracted by 2) we need to specify them accordingly.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/i915/intel_display.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4f6c594..458dfd9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -154,8 +154,8 @@ static const intel_limit_t intel_limits_i9xx_sdvo = {
 	.vco = { .min = 1400000, .max = 2800000 },
 	.n = { .min = 1, .max = 6 },
 	.m = { .min = 70, .max = 120 },
-	.m1 = { .min = 10, .max = 22 },
-	.m2 = { .min = 5, .max = 9 },
+	.m1 = { .min = 8, .max = 18 },
+	.m2 = { .min = 3, .max = 7 },
 	.p = { .min = 5, .max = 80 },
 	.p1 = { .min = 1, .max = 8 },
 	.p2 = { .dot_limit = 200000,
-- 
1.7.10.4

Re: [PATCH] drm/i915: Set i9xx lvds clock limits according to specifications

[Chris Wilson]

On Wed, Feb 13, 2013 at 10:20:21PM +0100, Patrik Jakobsson wrote:
> The Intel PRM says the M1 and M2 divisors must be in the range of 10-20 and 5-9.
> Since we do all calculations based on them being register values (which are
> subtracted by 2) we need to specify them accordingly.
> 
> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Set i9xx sdvo clock limits according to specifications

[Chris Wilson]

On Wed, Feb 13, 2013 at 10:20:22PM +0100, Patrik Jakobsson wrote:
> The Intel PRM says the M1 and M2 divisors must be in the range of 10-20 and 5-9.
> Since we do all calculations based on them being register values (which are
> subtracted by 2) we need to specify them accordingly.
> 
> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Set i9xx sdvo clock limits according to specifications

[Daniel Vetter]

On Thu, Feb 14, 2013 at 08:50:25PM +0000, Chris Wilson wrote:
> On Wed, Feb 13, 2013 at 10:20:22PM +0100, Patrik Jakobsson wrote:
> > The Intel PRM says the M1 and M2 divisors must be in the range of 10-20 and 5-9.
> > Since we do all calculations based on them being register values (which are
> > subtracted by 2) we need to specify them accordingly.
> > 
> > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Queued both for -next with a cc: stable tag to increase the odds of
detecting bugs (or fixing some). Thanks for the patches.
-Daniel
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Set i9xx sdvo clock limits according to specifications

[Dave Airlie]

On Fri, Feb 15, 2013 at 6:59 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Feb 14, 2013 at 08:50:25PM +0000, Chris Wilson wrote:
>> On Wed, Feb 13, 2013 at 10:20:22PM +0100, Patrik Jakobsson wrote:
>> > The Intel PRM says the M1 and M2 divisors must be in the range of 10-20 and 5-9.
>> > Since we do all calculations based on them being register values (which are
>> > subtracted by 2) we need to specify them accordingly.
>> >
>> > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> Queued both for -next with a cc: stable tag to increase the odds of
> detecting bugs (or fixing some). Thanks for the patches.
> -Daniel

yeah no stable for this sorta thing, this could go horribly wrong, if
it fxies something stable it later.

stable isn't meant for testing patches that could break things.

Dave.

Re: [PATCH] drm/i915: Set i9xx sdvo clock limits according to specifications

[Daniel Vetter]

On Thu, Feb 14, 2013 at 10:45 PM, Dave Airlie <airlied@gmail.com> wrote:
> On Fri, Feb 15, 2013 at 6:59 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Thu, Feb 14, 2013 at 08:50:25PM +0000, Chris Wilson wrote:
>>> On Wed, Feb 13, 2013 at 10:20:22PM +0100, Patrik Jakobsson wrote:
>>> > The Intel PRM says the M1 and M2 divisors must be in the range of 10-20 and 5-9.
>>> > Since we do all calculations based on them being register values (which are
>>> > subtracted by 2) we need to specify them accordingly.
>>> >
>>> > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>>>
>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>
>> Queued both for -next with a cc: stable tag to increase the odds of
>> detecting bugs (or fixing some). Thanks for the patches.
>> -Daniel
>
> yeah no stable for this sorta thing, this could go horribly wrong, if
> it fxies something stable it later.
>
> stable isn't meant for testing patches that could break things.

Ok, unstabled again. But wouldn't really have been the first time we'd
throw a revert to stable ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Set i9xx lvds clock limits according to specifications

[Chris Wilson]

On Wed, Feb 13, 2013 at 10:20:21PM +0100, Patrik Jakobsson wrote:
> The Intel PRM says the M1 and M2 divisors must be in the range of 10-20 and 5-9.
> Since we do all calculations based on them being register values (which are
> subtracted by 2) we need to specify them accordingly.

One thing I've just noticed is that intel_limits_i9xx_sdvo is reused by
g4x, so I'll double check that in the morning unless someone beats me to
it.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [Intel-gfx] [PATCH] drm/i915: Set i9xx lvds clock limits according to specifications

[Chris Wilson]

On Fri, Feb 15, 2013 at 12:18:49AM +0000, Chris Wilson wrote:
> On Wed, Feb 13, 2013 at 10:20:21PM +0100, Patrik Jakobsson wrote:
> > The Intel PRM says the M1 and M2 divisors must be in the range of 10-20 and 5-9.
> > Since we do all calculations based on them being register values (which are
> > subtracted by 2) we need to specify them accordingly.
> 
> One thing I've just noticed is that intel_limits_i9xx_sdvo is reused by
> g4x, so I'll double check that in the morning unless someone beats me to
> it.

Okay, so gen4 share the same values for sdvo as gen3, so we are okay in
fixing those up. However, the same offset-by-2 exists for the g4x values
of m1,m2. And one begins to suspect all the m values.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [Intel-gfx] [PATCH] drm/i915: Set i9xx lvds clock limits according to specifications

[Patrik Jakobsson]

On Fri, Feb 15, 2013 at 1:51 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Fri, Feb 15, 2013 at 12:18:49AM +0000, Chris Wilson wrote:
>> On Wed, Feb 13, 2013 at 10:20:21PM +0100, Patrik Jakobsson wrote:
>> > The Intel PRM says the M1 and M2 divisors must be in the range of 10-20 and 5-9.
>> > Since we do all calculations based on them being register values (which are
>> > subtracted by 2) we need to specify them accordingly.
>>
>> One thing I've just noticed is that intel_limits_i9xx_sdvo is reused by
>> g4x, so I'll double check that in the morning unless someone beats me to
>> it.
>
> Okay, so gen4 share the same values for sdvo as gen3, so we are okay in
> fixing those up. However, the same offset-by-2 exists for the g4x values
> of m1,m2. And one begins to suspect all the m values.
> -Chris

Seems to be all M values. As we discussed on IRC this is confusing and it might
be worth treating all values as according to specification and fix them up at
register read/write time. Makes it easier to read, but then again, the specs
play a trick on us by assuming that m1 and m2 are what we read from the regs
when calculating M.

-Patrik

Re: [PATCH] drm/i915: Set i9xx lvds clock limits according to specifications

[Patrik Jakobsson]

On Fri, Feb 15, 2013 at 2:30 PM, Patrik Jakobsson
<patrik.r.jakobsson@gmail.com> wrote:
> On Fri, Feb 15, 2013 at 1:51 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> On Fri, Feb 15, 2013 at 12:18:49AM +0000, Chris Wilson wrote:
>>> On Wed, Feb 13, 2013 at 10:20:21PM +0100, Patrik Jakobsson wrote:
>>> > The Intel PRM says the M1 and M2 divisors must be in the range of 10-20 and 5-9.
>>> > Since we do all calculations based on them being register values (which are
>>> > subtracted by 2) we need to specify them accordingly.
>>>
>>> One thing I've just noticed is that intel_limits_i9xx_sdvo is reused by
>>> g4x, so I'll double check that in the morning unless someone beats me to
>>> it.
>>
>> Okay, so gen4 share the same values for sdvo as gen3, so we are okay in
>> fixing those up. However, the same offset-by-2 exists for the g4x values
>> of m1,m2. And one begins to suspect all the m values.
>> -Chris
>
> Seems to be all M values. As we discussed on IRC this is confusing and it might
> be worth treating all values as according to specification and fix them up at
> register read/write time. Makes it easier to read, but then again, the specs
> play a trick on us by assuming that m1 and m2 are what we read from the regs
> when calculating M.
>
> -Patrik

Spotted one more thing. Dot clock min and max are based on all display modes
combined. E.g. i9xx_sdvo is set to 20-400 MHz but should be 100-270 MHz and
i9xx_lvds is set to 20-400 MHz but should be 20-112 MHz (single channel) and
80-224 MHz (dual channel).

-Patrik