On Mon, 27 May 2024, Jani Nikula <jani.nikula@xxxxxxxxx> wrote: > On Mon, 08 Apr 2024, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: >> On Mon, Apr 08, 2024 at 08:28:27PM +0300, Ville Syrjälä wrote: >>> On Mon, Apr 08, 2024 at 08:23:14PM +0300, Jani Nikula wrote: >>> > The rawclk initialization is a bit out of place in >>> > intel_device_info_runtime_init(). Move it to intel_cdclk_init(), with a >>> > bit of refactoring on intel_read_rawclk(). >>> >>> rawclk is used outside of display. >> >> The correct solution would likely be to extract a >> i9xx_fsb_freq(), and use that to populate both rawclk_freq >> and fsb_freq (and switch over to fsb_freq in the >> non-display code). > > I circled back to this, and PNV seems to be the problem case for making > this happen. > > pnv_detect_mem_freq() in intel_dram.c and i9xx_hrawclk() in > intel_cdclk.c interpret the CLKCFG register slightly differently. > > I'm presuming PNV only supports a subset of the values covered by > i9xx_hrawclk(). For IS_MOBILE() they all match, but for !IS_MOBILE() > there's a different value for 400 MHz FSB. > > So how should desktop PNV interpret the register, I wonder? I can't find > any specs on that anymore. My guess would be this: index b78154c82a71..19ca3ed5212a 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -3545,7 +3545,7 @@ static int i9xx_hrawclk(struct drm_i915_private *dev_priv) */ clkcfg = intel_de_read(dev_priv, CLKCFG) & CLKCFG_FSB_MASK; - if (IS_MOBILE(dev_priv)) { + if (IS_PINEVIEW(dev_priv) || IS_MOBILE(dev_priv)) { switch (clkcfg) { case CLKCFG_FSB_400: return 100000; -- Jani Nikula, Intel