Re: [PATCH 1/2] drm/i915: move rawclk init to intel_cdclk_init()

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 27 May 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> On Mon, 08 Apr 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> 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