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

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

[Arthur Heymans]

This is according to Mobile Intel® 945 Express Chipset
Family datasheet.

Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
---
 drivers/gpu/drm/i915/i915_reg.h      |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++--
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 02a65ddae3a3..f0b7849ace17 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -119,7 +119,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define GCFGC	0xf0 /* 915+ only */
 #define   GC_LOW_FREQUENCY_ENABLE	(1 << 7)
 #define   GC_DISPLAY_CLOCK_190_200_MHZ	(0 << 4)
-#define   GC_DISPLAY_CLOCK_333_MHZ	(4 << 4)
+#define   GC_DISPLAY_CLOCK_333_320_MHZ	(4 << 4)
 #define   GC_DISPLAY_CLOCK_267_MHZ_PNV	(0 << 4)
 #define   GC_DISPLAY_CLOCK_333_MHZ_PNV	(1 << 4)
 #define   GC_DISPLAY_CLOCK_444_MHZ_PNV	(2 << 4)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ac25706b7d4d..998920ab3ec8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7407,6 +7407,26 @@ static int i945_get_display_clock_speed(struct drm_i915_private *dev_priv)
 	return 400000;
 }
 
+static int i945gm_get_display_clock_speed(struct drm_i915_private *dev_priv)
+{
+	struct pci_dev *pdev = dev_priv->drm.pdev;
+	u16 gcfgc = 0;
+
+	pci_read_config_word(pdev, GCFGC, &gcfgc);
+
+	if (gcfgc & GC_LOW_FREQUENCY_ENABLE)
+		return 133333;
+	else {
+		switch (gcfgc & GC_DISPLAY_CLOCK_MASK) {
+		case GC_DISPLAY_CLOCK_333_320_MHZ:
+			return 320000;
+		default:
+		case GC_DISPLAY_CLOCK_190_200_MHZ:
+			return 200000;
+		}
+	}
+}
+
 static int i915_get_display_clock_speed(struct drm_i915_private *dev_priv)
 {
 	return 333333;
@@ -7453,7 +7473,7 @@ static int i915gm_get_display_clock_speed(struct drm_i915_private *dev_priv)
 		return 133333;
 	else {
 		switch (gcfgc & GC_DISPLAY_CLOCK_MASK) {
-		case GC_DISPLAY_CLOCK_333_MHZ:
+		case GC_DISPLAY_CLOCK_333_320_MHZ:
 			return 333333;
 		default:
 		case GC_DISPLAY_CLOCK_190_200_MHZ:
@@ -16244,9 +16264,12 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 	else if (IS_I915G(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i915_get_display_clock_speed;
-	else if (IS_I945GM(dev_priv) || IS_I845G(dev_priv))
+	else if (IS_I845G(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i9xx_misc_get_display_clock_speed;
+	else if (IS_I945GM(dev_priv))
+		dev_priv->display.get_display_clock_speed =
+			i945gm_get_display_clock_speed;
 	else if (IS_I915GM(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i915gm_get_display_clock_speed;
-- 
2.11.0

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

✓ Fi.CI.BAT: success for drm/i915: Get correct display clock on 945gm (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Get correct display clock on 945gm (rev3)
URL   : https://patchwork.freedesktop.org/series/18693/
State : success

== Summary ==

Series 18693v3 drm/i915: Get correct display clock on 945gm
https://patchwork.freedesktop.org/api/1.0/series/18693/revisions/3/mbox/


fi-bdw-5557u     total:247  pass:233  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:247  pass:225  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:78   pass:65   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:247  pass:224  dwarn:0   dfail:0   fail:2   skip:21 
fi-skl-6260u     total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:247  pass:227  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:247  pass:222  dwarn:4   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:247  pass:215  dwarn:0   dfail:0   fail:0   skip:32 

83f390eaec64a1b0566f5f747d20741c077fcff5 drm-tip: 2017y-01m-31d-20h-19m-59s UTC integration manifest
a784911 drm/i915: Get correct display clock on 945gm

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3658/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Wed, Feb 01, 2017 at 12:50:26AM +0100, Arthur Heymans wrote:
> This is according to Mobile Intel® 945 Express Chipset
> Family datasheet.
> 
> Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

and pushed to dinq. Thanks for the patch.

> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++--
>  2 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 02a65ddae3a3..f0b7849ace17 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -119,7 +119,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define GCFGC	0xf0 /* 915+ only */
>  #define   GC_LOW_FREQUENCY_ENABLE	(1 << 7)
>  #define   GC_DISPLAY_CLOCK_190_200_MHZ	(0 << 4)
> -#define   GC_DISPLAY_CLOCK_333_MHZ	(4 << 4)
> +#define   GC_DISPLAY_CLOCK_333_320_MHZ	(4 << 4)
>  #define   GC_DISPLAY_CLOCK_267_MHZ_PNV	(0 << 4)
>  #define   GC_DISPLAY_CLOCK_333_MHZ_PNV	(1 << 4)
>  #define   GC_DISPLAY_CLOCK_444_MHZ_PNV	(2 << 4)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ac25706b7d4d..998920ab3ec8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7407,6 +7407,26 @@ static int i945_get_display_clock_speed(struct drm_i915_private *dev_priv)
>  	return 400000;
>  }
>  
> +static int i945gm_get_display_clock_speed(struct drm_i915_private *dev_priv)
> +{
> +	struct pci_dev *pdev = dev_priv->drm.pdev;
> +	u16 gcfgc = 0;
> +
> +	pci_read_config_word(pdev, GCFGC, &gcfgc);
> +
> +	if (gcfgc & GC_LOW_FREQUENCY_ENABLE)
> +		return 133333;
> +	else {
> +		switch (gcfgc & GC_DISPLAY_CLOCK_MASK) {
> +		case GC_DISPLAY_CLOCK_333_320_MHZ:
> +			return 320000;
> +		default:
> +		case GC_DISPLAY_CLOCK_190_200_MHZ:
> +			return 200000;
> +		}
> +	}
> +}
> +
>  static int i915_get_display_clock_speed(struct drm_i915_private *dev_priv)
>  {
>  	return 333333;
> @@ -7453,7 +7473,7 @@ static int i915gm_get_display_clock_speed(struct drm_i915_private *dev_priv)
>  		return 133333;
>  	else {
>  		switch (gcfgc & GC_DISPLAY_CLOCK_MASK) {
> -		case GC_DISPLAY_CLOCK_333_MHZ:
> +		case GC_DISPLAY_CLOCK_333_320_MHZ:
>  			return 333333;
>  		default:
>  		case GC_DISPLAY_CLOCK_190_200_MHZ:
> @@ -16244,9 +16264,12 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  	else if (IS_I915G(dev_priv))
>  		dev_priv->display.get_display_clock_speed =
>  			i915_get_display_clock_speed;
> -	else if (IS_I945GM(dev_priv) || IS_I845G(dev_priv))
> +	else if (IS_I845G(dev_priv))
>  		dev_priv->display.get_display_clock_speed =
>  			i9xx_misc_get_display_clock_speed;
> +	else if (IS_I945GM(dev_priv))
> +		dev_priv->display.get_display_clock_speed =
> +			i945gm_get_display_clock_speed;
>  	else if (IS_I915GM(dev_priv))
>  		dev_priv->display.get_display_clock_speed =
>  			i915gm_get_display_clock_speed;
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

[Arthur Heymans]

This is according to Mobile Intel® 945 Express Chipset
Family datasheet.

Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
---
 drivers/gpu/drm/i915/i915_reg.h      |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++--
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 70d96162def6..c3141e40d938 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -114,7 +114,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define GCFGC	0xf0 /* 915+ only */
 #define   GC_LOW_FREQUENCY_ENABLE	(1 << 7)
 #define   GC_DISPLAY_CLOCK_190_200_MHZ	(0 << 4)
-#define   GC_DISPLAY_CLOCK_333_MHZ	(4 << 4)
+#define   GC_DISPLAY_CLOCK_333_320_MHZ	(4 << 4)
 #define   GC_DISPLAY_CLOCK_267_MHZ_PNV	(0 << 4)
 #define   GC_DISPLAY_CLOCK_333_MHZ_PNV	(1 << 4)
 #define   GC_DISPLAY_CLOCK_444_MHZ_PNV	(2 << 4)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 81c11499bcf0..307fc62e7c70 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7389,6 +7389,26 @@ static int i945_get_display_clock_speed(struct drm_device *dev)
 	return 400000;
 }
 
+static int 945gm_get_display_clock_speed(struct drm_device *dev)
+{
+	struct pci_dev *pdev = dev->pdev;
+	u16 gcfgc = 0;
+
+	pci_read_config_word(pdev, GCFGC, &gcfgc);
+
+	if (gcfgc & GC_LOW_FREQUENCY_ENABLE)
+		return 133333;
+	else {
+		switch (gcfgc & GC_DISPLAY_CLOCK_MASK) {
+		case GC_DISPLAY_CLOCK_333_320_MHZ:
+			return 320000;
+		default:
+		case GC_DISPLAY_CLOCK_190_200_MHZ:
+			return 200000;
+		}
+	}
+}
+
 static int i915_get_display_clock_speed(struct drm_device *dev)
 {
 	return 333333;
@@ -7435,7 +7455,7 @@ static int i915gm_get_display_clock_speed(struct drm_device *dev)
 		return 133333;
 	else {
 		switch (gcfgc & GC_DISPLAY_CLOCK_MASK) {
-		case GC_DISPLAY_CLOCK_333_MHZ:
+		case GC_DISPLAY_CLOCK_333_320_MHZ:
 			return 333333;
 		default:
 		case GC_DISPLAY_CLOCK_190_200_MHZ:
@@ -15991,9 +16011,12 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 	else if (IS_I915G(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i915_get_display_clock_speed;
-	else if (IS_I945GM(dev_priv) || IS_845G(dev_priv))
+	else if (IS_845G(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i9xx_misc_get_display_clock_speed;
+	else if (IS_I945GM(dev_priv))
+		dev_priv->display.get_display_clock_speed =
+			954gm_get_display_clock_speed;
 	else if (IS_I915GM(dev_priv))
 		dev_priv->display.get_display_clock_speed =
 			i915gm_get_display_clock_speed;
-- 
2.11.0

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

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

[Ville Syrjälä]

On Fri, Jan 27, 2017 at 03:44:27PM +0100, Arthur Heymans wrote:
> This is according to Mobile Intel® 945 Express Chipset
> Family datasheet.
> 
> Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++--
>  2 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 70d96162def6..c3141e40d938 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -114,7 +114,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define GCFGC	0xf0 /* 915+ only */
>  #define   GC_LOW_FREQUENCY_ENABLE	(1 << 7)
>  #define   GC_DISPLAY_CLOCK_190_200_MHZ	(0 << 4)
> -#define   GC_DISPLAY_CLOCK_333_MHZ	(4 << 4)
> +#define   GC_DISPLAY_CLOCK_333_320_MHZ	(4 << 4)
>  #define   GC_DISPLAY_CLOCK_267_MHZ_PNV	(0 << 4)
>  #define   GC_DISPLAY_CLOCK_333_MHZ_PNV	(1 << 4)
>  #define   GC_DISPLAY_CLOCK_444_MHZ_PNV	(2 << 4)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 81c11499bcf0..307fc62e7c70 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7389,6 +7389,26 @@ static int i945_get_display_clock_speed(struct drm_device *dev)
>  	return 400000;
>  }
>  
> +static int 945gm_get_display_clock_speed(struct drm_device *dev)
> +{
> +	struct pci_dev *pdev = dev->pdev;
> +	u16 gcfgc = 0;
> +
> +	pci_read_config_word(pdev, GCFGC, &gcfgc);
> +
> +	if (gcfgc & GC_LOW_FREQUENCY_ENABLE)
> +		return 133333;
> +	else {
> +		switch (gcfgc & GC_DISPLAY_CLOCK_MASK) {
> +		case GC_DISPLAY_CLOCK_333_320_MHZ:
> +			return 320000;

That indeed is what the docs say.

The code is tantalizingly close to the 915gm code now, so maybe
we could share it with a simple

if (IS_915GM(dev_priv))
	return 320000;
else
	return 333333;

?

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;
> +		}
> +	}
> +}
> +
>  static int i915_get_display_clock_speed(struct drm_device *dev)
>  {
>  	return 333333;
> @@ -7435,7 +7455,7 @@ static int i915gm_get_display_clock_speed(struct drm_device *dev)
>  		return 133333;
>  	else {
>  		switch (gcfgc & GC_DISPLAY_CLOCK_MASK) {
> -		case GC_DISPLAY_CLOCK_333_MHZ:
> +		case GC_DISPLAY_CLOCK_333_320_MHZ:
>  			return 333333;
>  		default:
>  		case GC_DISPLAY_CLOCK_190_200_MHZ:
> @@ -15991,9 +16011,12 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  	else if (IS_I915G(dev_priv))
>  		dev_priv->display.get_display_clock_speed =
>  			i915_get_display_clock_speed;
> -	else if (IS_I945GM(dev_priv) || IS_845G(dev_priv))
> +	else if (IS_845G(dev_priv))
>  		dev_priv->display.get_display_clock_speed =
>  			i9xx_misc_get_display_clock_speed;
> +	else if (IS_I945GM(dev_priv))
> +		dev_priv->display.get_display_clock_speed =
> +			954gm_get_display_clock_speed;
>  	else if (IS_I915GM(dev_priv))
>  		dev_priv->display.get_display_clock_speed =
>  			i915gm_get_display_clock_speed;
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

[Arthur Heymans]

Ville Syrjälä <ville.syrjala@linux.intel.com> writes:

> On Fri, Jan 27, 2017 at 03:44:27PM +0100, Arthur Heymans wrote:
>
> That indeed is what the docs say.
>
> The code is tantalizingly close to the 915gm code now, so maybe
> we could share it with a simple
>
> if (IS_915GM(dev_priv))
> 	return 320000;
> else
> 	return 333333;
>

Agreed but it's the other way around ;)

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

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

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

[Ville Syrjälä]

On Fri, Jan 27, 2017 at 05:45:06PM +0100, Arthur Heymans wrote:
> Ville Syrjälä <ville.syrjala@linux.intel.com> writes:
> 
> > On Fri, Jan 27, 2017 at 03:44:27PM +0100, Arthur Heymans wrote:
> >
> > That indeed is what the docs say.
> >
> > The code is tantalizingly close to the 915gm code now, so maybe
> > we could share it with a simple
> >
> > if (IS_915GM(dev_priv))
> > 	return 320000;
> > else
> > 	return 333333;
> >
> 
> Agreed but it's the other way around ;)

Indeed. Apparently my brain can't reconcile the fact that the older part
is faster.

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

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

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

[Arthur Heymans]

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

FSB and also DRAM freq can be probably be fetched from respectively
[2:0] and [6:4] of CLKCFG, MCHBAR(0xC00) but not documented.
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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

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

[Daniel Vetter]

On Fri, Jan 27, 2017 at 09:51:50PM +0200, Ville Syrjälä wrote:
> 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 ;)

Isn't the defensive thing to go with the lower clock? We might get a
regression report, but chances are slim, and if we do get it we'll have a
guinea-pig to test stuff on :-) So option a)
-Daniel

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1696 bytes --]

Hi Arthur,

[auto build test ERROR on v4.9-rc8]
[cannot apply to drm-intel/for-linux-next next-20170125]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Arthur-Heymans/drm-i915-Get-correct-display-clock-on-945gm/20170128-022541
config: i386-randconfig-x004-201704 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/i915/intel_display.c:7392:12: error: invalid suffix "gm_get_display_clock_speed" on integer constant
    static int 945gm_get_display_clock_speed(struct drm_device *dev)
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/intel_display.c:7392:12: error: expected identifier or '(' before numeric constant
   drivers/gpu/drm/i915/intel_display.c: In function 'intel_init_display_hooks':
   drivers/gpu/drm/i915/intel_display.c:16020:4: error: invalid suffix "gm_get_display_clock_speed" on integer constant
       954gm_get_display_clock_speed;
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/gm_get_display_clock_speed +7392 drivers/gpu/drm/i915/intel_display.c

  7386	
  7387	static int i945_get_display_clock_speed(struct drm_device *dev)
  7388	{
  7389		return 400000;
  7390	}
  7391	
> 7392	static int 945gm_get_display_clock_speed(struct drm_device *dev)
  7393	{
  7394		struct pci_dev *pdev = dev->pdev;
  7395		u16 gcfgc = 0;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29825 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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