diff mbox

[01/18] drm/i915: Return more precise cdclk for gen2/3

Message ID 1416235432-16603-2-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Nov. 17, 2014, 2:43 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Fill out the lower three digits for gen2 and gen3 cdclk frqeuncy. It's
not clear if these are accurate frquencies or just in the ballpark, but
without docs this is the best we can do.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Comments

Daniel Vetter Nov. 17, 2014, 6:44 p.m. UTC | #1
On Mon, Nov 17, 2014 at 04:43:35PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Fill out the lower three digits for gen2 and gen3 cdclk frqeuncy. It's
> not clear if these are accurate frquencies or just in the ballpark, but
> without docs this is the best we can do.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Given that no one complained yet I'm not sure this is worth the trouble.
Otoh it's all below the 10% margin we have already anyway, so one big
wash ;-)
-Daniel
Ville Syrjälä Nov. 17, 2014, 7:02 p.m. UTC | #2
On Mon, Nov 17, 2014 at 07:44:30PM +0100, Daniel Vetter wrote:
> On Mon, Nov 17, 2014 at 04:43:35PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Fill out the lower three digits for gen2 and gen3 cdclk frqeuncy. It's
> > not clear if these are accurate frquencies or just in the ballpark, but
> > without docs this is the best we can do.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Given that no one complained yet I'm not sure this is worth the trouble.
> Otoh it's all below the 10% margin we have already anyway, so one big
> wash ;-)

Yeah <1Mhz is a fairly small error here. But I still prefer to make the
change, if only for consistency. Otherwise it'll keep bugging me and
I'll have to keep fighting the urge to change it every time I see those
numbers.
Daniel Vetter Nov. 17, 2014, 7:13 p.m. UTC | #3
On Mon, Nov 17, 2014 at 09:02:11PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 17, 2014 at 07:44:30PM +0100, Daniel Vetter wrote:
> > On Mon, Nov 17, 2014 at 04:43:35PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Fill out the lower three digits for gen2 and gen3 cdclk frqeuncy. It's
> > > not clear if these are accurate frquencies or just in the ballpark, but
> > > without docs this is the best we can do.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Given that no one complained yet I'm not sure this is worth the trouble.
> > Otoh it's all below the 10% margin we have already anyway, so one big
> > wash ;-)
> 
> Yeah <1Mhz is a fairly small error here. But I still prefer to make the
> change, if only for consistency. Otherwise it'll keep bugging me and
> I'll have to keep fighting the urge to change it every time I see those
> numbers.

I fully approve of OCD urges ;-) Series overall looks really nifty, I
think it would be best to sign up a senior person from the vpg display
team for to review the details in this - maybe they remember some of the
old lore ...

And one aside for atomic: I guess we need a clocks_lock ww mutex to
protect any dynamic cdclock state. Same for shared dpll state too. This
will be fun to integrate, but since all the code that checks cdclock can
fail with -EINVAL already wiring up -EDEADLK shouldn't cause any fuzz.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dda97b3..e364ca7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5605,7 +5605,7 @@  static int i945_get_display_clock_speed(struct drm_device *dev)
 
 static int i915_get_display_clock_speed(struct drm_device *dev)
 {
-	return 333000;
+	return 333333;
 }
 
 static int i9xx_misc_get_display_clock_speed(struct drm_device *dev)
@@ -5621,19 +5621,19 @@  static int pnv_get_display_clock_speed(struct drm_device *dev)
 
 	switch (gcfgc & GC_DISPLAY_CLOCK_MASK) {
 	case GC_DISPLAY_CLOCK_267_MHZ_PNV:
-		return 267000;
+		return 266667;
 	case GC_DISPLAY_CLOCK_333_MHZ_PNV:
-		return 333000;
+		return 333333;
 	case GC_DISPLAY_CLOCK_444_MHZ_PNV:
-		return 444000;
+		return 444444;
 	case GC_DISPLAY_CLOCK_200_MHZ_PNV:
 		return 200000;
 	default:
 		DRM_ERROR("Unknown pnv display core clock 0x%04x\n", gcfgc);
 	case GC_DISPLAY_CLOCK_133_MHZ_PNV:
-		return 133000;
+		return 133333;
 	case GC_DISPLAY_CLOCK_167_MHZ_PNV:
-		return 167000;
+		return 166667;
 	}
 }
 
@@ -5644,11 +5644,11 @@  static int i915gm_get_display_clock_speed(struct drm_device *dev)
 	pci_read_config_word(dev->pdev, GCFGC, &gcfgc);
 
 	if (gcfgc & GC_LOW_FREQUENCY_ENABLE)
-		return 133000;
+		return 133333;
 	else {
 		switch (gcfgc & GC_DISPLAY_CLOCK_MASK) {
 		case GC_DISPLAY_CLOCK_333_MHZ:
-			return 333000;
+			return 333333;
 		default:
 		case GC_DISPLAY_CLOCK_190_200_MHZ:
 			return 190000;
@@ -5658,7 +5658,7 @@  static int i915gm_get_display_clock_speed(struct drm_device *dev)
 
 static int i865_get_display_clock_speed(struct drm_device *dev)
 {
-	return 266000;
+	return 266667;
 }
 
 static int i855_get_display_clock_speed(struct drm_device *dev)
@@ -5674,7 +5674,7 @@  static int i855_get_display_clock_speed(struct drm_device *dev)
 	case GC_CLOCK_166_250:
 		return 250000;
 	case GC_CLOCK_100_133:
-		return 133000;
+		return 133333;
 	}
 
 	/* Shouldn't happen */
@@ -5683,7 +5683,7 @@  static int i855_get_display_clock_speed(struct drm_device *dev)
 
 static int i830_get_display_clock_speed(struct drm_device *dev)
 {
-	return 133000;
+	return 133333;
 }
 
 static void