Re: [PATCH 06/12] drm/i915: convert fsb_freq and mem_freq to kHz

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

On Wed, 29 May 2024, Matt Roper <matthew.d.roper@intel.com> wrote:
> On Tue, May 28, 2024 at 05:24:55PM +0300, Jani Nikula wrote:
>> We'll want to use fsb frequency for deriving GT clock and rawclk
>> frequencies in the future. Increase the accuracy by converting to
>> kHz. Do the same for mem freq to be aligned.
>> 
>> Round the frequencies ending in 666 to 667.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Would it be worth adding a "_khz" suffix to the structure fields to help
> clarify the units?

Thought about it, but decided kHz is pretty much the norm, and it's
everything else that should have a suffix to clarify the units!

> Either way,
>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

Thanks,
Jani.

>
>> ---
>>  drivers/gpu/drm/i915/display/i9xx_wm.c |  6 ++--
>>  drivers/gpu/drm/i915/gt/intel_rps.c    |  4 +--
>>  drivers/gpu/drm/i915/soc/intel_dram.c  | 50 +++++++++++++-------------
>>  3 files changed, 30 insertions(+), 30 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
>> index 8b8a0f305c3a..08c5d122af8f 100644
>> --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
>> +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
>> @@ -83,14 +83,14 @@ static const struct cxsr_latency *pnv_get_cxsr_latency(struct drm_i915_private *
>>  
>>  		if (is_desktop == latency->is_desktop &&
>>  		    i915->is_ddr3 == latency->is_ddr3 &&
>> -		    i915->fsb_freq == latency->fsb_freq &&
>> -		    i915->mem_freq == latency->mem_freq)
>> +		    DIV_ROUND_CLOSEST(i915->fsb_freq, 1000) == latency->fsb_freq &&
>> +		    DIV_ROUND_CLOSEST(i915->mem_freq, 1000) == latency->mem_freq)
>>  			return latency;
>>  	}
>>  
>>  err:
>>  	drm_dbg_kms(&i915->drm,
>> -		    "Could not find CxSR latency for DDR%s, FSB %u MHz, MEM %u MHz\n",
>> +		    "Could not find CxSR latency for DDR%s, FSB %u kHz, MEM %u kHz\n",
>>  		    i915->is_ddr3 ? "3" : "2", i915->fsb_freq, i915->mem_freq);
>>  
>>  	return NULL;
>> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
>> index c9cb2a391942..5d3de1cddcf6 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
>> @@ -280,9 +280,9 @@ static void gen5_rps_init(struct intel_rps *rps)
>>  	u32 rgvmodectl;
>>  	int c_m, i;
>>  
>> -	if (i915->fsb_freq <= 3200)
>> +	if (i915->fsb_freq <= 3200000)
>>  		c_m = 0;
>> -	else if (i915->fsb_freq <= 4800)
>> +	else if (i915->fsb_freq <= 4800000)
>>  		c_m = 1;
>>  	else
>>  		c_m = 2;
>> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
>> index 266ed6cfa485..ace9372244a4 100644
>> --- a/drivers/gpu/drm/i915/soc/intel_dram.c
>> +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
>> @@ -56,11 +56,11 @@ static unsigned int pnv_mem_freq(struct drm_i915_private *dev_priv)
>>  
>>  	switch (tmp & CLKCFG_MEM_MASK) {
>>  	case CLKCFG_MEM_533:
>> -		return 533;
>> +		return 533333;
>>  	case CLKCFG_MEM_667:
>> -		return 667;
>> +		return 666667;
>>  	case CLKCFG_MEM_800:
>> -		return 800;
>> +		return 800000;
>>  	}
>>  
>>  	return 0;
>> @@ -73,13 +73,13 @@ static unsigned int ilk_mem_freq(struct drm_i915_private *dev_priv)
>>  	ddrpll = intel_uncore_read16(&dev_priv->uncore, DDRMPLL1);
>>  	switch (ddrpll & 0xff) {
>>  	case 0xc:
>> -		return 800;
>> +		return 800000;
>>  	case 0x10:
>> -		return 1066;
>> +		return 1066667;
>>  	case 0x14:
>> -		return 1333;
>> +		return 1333333;
>>  	case 0x18:
>> -		return 1600;
>> +		return 1600000;
>>  	default:
>>  		drm_dbg(&dev_priv->drm, "unknown memory frequency 0x%02x\n",
>>  			ddrpll & 0xff);
>> @@ -97,9 +97,9 @@ static unsigned int chv_mem_freq(struct drm_i915_private *i915)
>>  
>>  	switch ((val >> 2) & 0x7) {
>>  	case 3:
>> -		return 2000;
>> +		return 2000000;
>>  	default:
>> -		return 1600;
>> +		return 1600000;
>>  	}
>>  }
>>  
>> @@ -114,11 +114,11 @@ static unsigned int vlv_mem_freq(struct drm_i915_private *i915)
>>  	switch ((val >> 6) & 3) {
>>  	case 0:
>>  	case 1:
>> -		return 800;
>> +		return 800000;
>>  	case 2:
>> -		return 1066;
>> +		return 1066667;
>>  	case 3:
>> -		return 1333;
>> +		return 1333333;
>>  	}
>>  
>>  	return 0;
>> @@ -139,7 +139,7 @@ static void detect_mem_freq(struct drm_i915_private *i915)
>>  		i915->is_ddr3 = pnv_is_ddr3(i915);
>>  
>>  	if (i915->mem_freq)
>> -		drm_dbg(&i915->drm, "DDR speed: %d MHz\n", i915->mem_freq);
>> +		drm_dbg(&i915->drm, "DDR speed: %d kHz\n", i915->mem_freq);
>>  }
>>  
>>  static unsigned int pnv_fsb_freq(struct drm_i915_private *i915)
>> @@ -150,13 +150,13 @@ static unsigned int pnv_fsb_freq(struct drm_i915_private *i915)
>>  
>>  	switch (fsb) {
>>  	case CLKCFG_FSB_400:
>> -		return 400;
>> +		return 400000;
>>  	case CLKCFG_FSB_533:
>> -		return 533;
>> +		return 533333;
>>  	case CLKCFG_FSB_667:
>> -		return 667;
>> +		return 666667;
>>  	case CLKCFG_FSB_800:
>> -		return 800;
>> +		return 800000;
>>  	}
>>  
>>  	return 0;
>> @@ -170,19 +170,19 @@ static unsigned int ilk_fsb_freq(struct drm_i915_private *dev_priv)
>>  
>>  	switch (fsb) {
>>  	case 0x00c:
>> -		return 3200;
>> +		return 3200000;
>>  	case 0x00e:
>> -		return 3733;
>> +		return 3733333;
>>  	case 0x010:
>> -		return 4266;
>> +		return 4266667;
>>  	case 0x012:
>> -		return 4800;
>> +		return 4800000;
>>  	case 0x014:
>> -		return 5333;
>> +		return 5333333;
>>  	case 0x016:
>> -		return 5866;
>> +		return 5866667;
>>  	case 0x018:
>> -		return 6400;
>> +		return 6400000;
>>  	default:
>>  		drm_dbg(&dev_priv->drm, "unknown fsb frequency 0x%04x\n", fsb);
>>  		return 0;
>> @@ -197,7 +197,7 @@ static void detect_fsb_freq(struct drm_i915_private *i915)
>>  		i915->fsb_freq = pnv_fsb_freq(i915);
>>  
>>  	if (i915->fsb_freq)
>> -		drm_dbg(&i915->drm, "FSB frequency: %d MHz\n", i915->fsb_freq);
>> +		drm_dbg(&i915->drm, "FSB frequency: %d kHz\n", i915->fsb_freq);
>>  }
>>  
>>  static int intel_dimm_num_devices(const struct dram_dimm_info *dimm)
>> -- 
>> 2.39.2
>> 

-- 
Jani Nikula, Intel