All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Get correct display clock on 945gm
@ 2017-01-27 14:44 Arthur Heymans
  2017-01-27 16:23 ` Ville Syrjälä
  2017-01-27 20:15 ` kbuild test robot
  0 siblings, 2 replies; 10+ messages in thread
From: Arthur Heymans @ 2017-01-27 14:44 UTC (permalink / raw)
  To: intel-gfx; +Cc: 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Get correct display clock on 945gm
  2017-01-27 14:44 [PATCH] drm/i915: Get correct display clock on 945gm Arthur Heymans
@ 2017-01-27 16:23 ` Ville Syrjälä
  2017-01-27 16:45   ` Arthur Heymans
  2017-01-27 20:15 ` kbuild test robot
  1 sibling, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2017-01-27 16:23 UTC (permalink / raw)
  To: Arthur Heymans; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] drm/i915: Get correct display clock on 945gm
  2017-01-27 16:23 ` Ville Syrjälä
@ 2017-01-27 16:45   ` Arthur Heymans
  2017-01-27 16:57     ` Ville Syrjälä
  0 siblings, 1 reply; 10+ messages in thread
From: Arthur Heymans @ 2017-01-27 16:45 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Get correct display clock on 945gm
  2017-01-27 16:45   ` Arthur Heymans
@ 2017-01-27 16:57     ` Ville Syrjälä
  2017-01-27 17:24       ` Arthur Heymans
  0 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2017-01-27 16:57 UTC (permalink / raw)
  To: Arthur Heymans; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Get correct display clock on 945gm
  2017-01-27 16:57     ` Ville Syrjälä
@ 2017-01-27 17:24       ` Arthur Heymans
  2017-01-27 19:51         ` Ville Syrjälä
  0 siblings, 1 reply; 10+ messages in thread
From: Arthur Heymans @ 2017-01-27 17:24 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Get correct display clock on 945gm
  2017-01-27 17:24       ` Arthur Heymans
@ 2017-01-27 19:51         ` Ville Syrjälä
  2017-01-30  9:31           ` Daniel Vetter
  0 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2017-01-27 19:51 UTC (permalink / raw)
  To: Arthur Heymans; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Get correct display clock on 945gm
  2017-01-27 14:44 [PATCH] drm/i915: Get correct display clock on 945gm Arthur Heymans
  2017-01-27 16:23 ` Ville Syrjälä
@ 2017-01-27 20:15 ` kbuild test robot
  1 sibling, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2017-01-27 20:15 UTC (permalink / raw)
  Cc: intel-gfx, kbuild-all, Arthur Heymans

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Get correct display clock on 945gm
  2017-01-27 19:51         ` Ville Syrjälä
@ 2017-01-30  9:31           ` Daniel Vetter
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2017-01-30  9:31 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Arthur Heymans

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Get correct display clock on 945gm
  2017-01-31 23:50 Arthur Heymans
@ 2017-02-07 18:04 ` Ville Syrjälä
  0 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2017-02-07 18:04 UTC (permalink / raw)
  To: Arthur Heymans; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] drm/i915: Get correct display clock on 945gm
@ 2017-01-31 23:50 Arthur Heymans
  2017-02-07 18:04 ` Ville Syrjälä
  0 siblings, 1 reply; 10+ messages in thread
From: Arthur Heymans @ 2017-01-31 23:50 UTC (permalink / raw)
  To: intel-gfx; +Cc: 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-02-07 18:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-27 14:44 [PATCH] drm/i915: Get correct display clock on 945gm Arthur Heymans
2017-01-27 16:23 ` Ville Syrjälä
2017-01-27 16:45   ` Arthur Heymans
2017-01-27 16:57     ` Ville Syrjälä
2017-01-27 17:24       ` Arthur Heymans
2017-01-27 19:51         ` Ville Syrjälä
2017-01-30  9:31           ` Daniel Vetter
2017-01-27 20:15 ` kbuild test robot
2017-01-31 23:50 Arthur Heymans
2017-02-07 18:04 ` Ville Syrjälä

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.