Re: [PATCH 07/12] drm/i915: extend the fsb_freq initialization to more platforms

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

On Thu, 30 May 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> On Wed, 29 May 2024, Matt Roper <matthew.d.roper@intel.com> wrote:
>> On Tue, May 28, 2024 at 05:24:56PM +0300, Jani Nikula wrote:
>>> Initialize fsb frequency for more platforms to be able to use it for GT
>>> clock and rawclk frequency initialization.
>>> 
>>> Note: There's a discrepancy between existing pnv_fsb_freq() and
>>> i9xx_hrawclk() regarding CLKCFG interpretation. Presume all PNV is
>>> mobile.
>>
>> Do you just mean we assume PNV always treats CLKCFG the same way mobile
>> platforms do?  Because we have both mobile and non-mobile platforms
>> defined in the driver (pnv_m_info vs pnv_g_info) and that matches
>> https://ark.intel.com/content/www/us/en/ark/products/codename/32201/products-formerly-pineview.html
>> that lists both desktop and mobile.
>
> Yeah. The problem is, current code in intel_dram.c and intel_cdclk.c
> interpret the CLKCFG register differently for desktop PNV. At least one
> of them is wrong. Basically I just picked one, and secretly hoped Ville
> would tell me. ;)

Ville, do you have any idea about CLKCFG?

BR,
Jani.


>
> BR,
> Jani.
>
>
>>
>>
>> Matt
>>
>>> 
>>> FIXME: What should the default or failure mode be when the value is
>>> unknown?
>>> 
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/soc/intel_dram.c | 54 ++++++++++++++++++++-------
>>>  1 file changed, 40 insertions(+), 14 deletions(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
>>> index ace9372244a4..74b5b70e91f9 100644
>>> --- a/drivers/gpu/drm/i915/soc/intel_dram.c
>>> +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
>>> @@ -142,24 +142,50 @@ static void detect_mem_freq(struct drm_i915_private *i915)
>>>  		drm_dbg(&i915->drm, "DDR speed: %d kHz\n", i915->mem_freq);
>>>  }
>>>  
>>> -static unsigned int pnv_fsb_freq(struct drm_i915_private *i915)
>>> +static unsigned int i9xx_fsb_freq(struct drm_i915_private *i915)
>>>  {
>>>  	u32 fsb;
>>>  
>>>  	fsb = intel_uncore_read(&i915->uncore, CLKCFG) & CLKCFG_FSB_MASK;
>>>  
>>> -	switch (fsb) {
>>> -	case CLKCFG_FSB_400:
>>> -		return 400000;
>>> -	case CLKCFG_FSB_533:
>>> -		return 533333;
>>> -	case CLKCFG_FSB_667:
>>> -		return 666667;
>>> -	case CLKCFG_FSB_800:
>>> -		return 800000;
>>> +	if (IS_PINEVIEW(i915) || IS_MOBILE(i915)) {
>>> +		switch (fsb) {
>>> +		case CLKCFG_FSB_400:
>>> +			return 400000;
>>> +		case CLKCFG_FSB_533:
>>> +			return 533333;
>>> +		case CLKCFG_FSB_667:
>>> +			return 666667;
>>> +		case CLKCFG_FSB_800:
>>> +			return 800000;
>>> +		case CLKCFG_FSB_1067:
>>> +			return 1066667;
>>> +		case CLKCFG_FSB_1333:
>>> +			return 1333333;
>>> +		default:
>>> +			MISSING_CASE(fsb);
>>> +			return 1333333;
>>> +		}
>>> +	} else {
>>> +		switch (fsb) {
>>> +		case CLKCFG_FSB_400_ALT:
>>> +			return 400000;
>>> +		case CLKCFG_FSB_533:
>>> +			return 533333;
>>> +		case CLKCFG_FSB_667:
>>> +			return 666667;
>>> +		case CLKCFG_FSB_800:
>>> +			return 800000;
>>> +		case CLKCFG_FSB_1067_ALT:
>>> +			return 1066667;
>>> +		case CLKCFG_FSB_1333_ALT:
>>> +			return 1333333;
>>> +		case CLKCFG_FSB_1600_ALT:
>>> +			return 1600000;
>>> +		default:
>>> +			return 533333;
>>> +		}
>>>  	}
>>> -
>>> -	return 0;
>>>  }
>>>  
>>>  static unsigned int ilk_fsb_freq(struct drm_i915_private *dev_priv)
>>> @@ -193,8 +219,8 @@ static void detect_fsb_freq(struct drm_i915_private *i915)
>>>  {
>>>  	if (GRAPHICS_VER(i915) == 5)
>>>  		i915->fsb_freq = ilk_fsb_freq(i915);
>>> -	else if (IS_PINEVIEW(i915))
>>> -		i915->fsb_freq = pnv_fsb_freq(i915);
>>> +	else if (GRAPHICS_VER(i915) == 3 || GRAPHICS_VER(i915) == 4)
>>> +		i915->fsb_freq = i9xx_fsb_freq(i915);
>>>  
>>>  	if (i915->fsb_freq)
>>>  		drm_dbg(&i915->drm, "FSB frequency: %d kHz\n", i915->fsb_freq);
>>> -- 
>>> 2.39.2
>>> 

-- 
Jani Nikula, Intel