All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 01/10] drm/i915: move memory frequency detection to intel_dram.c
Date: Wed, 8 Feb 2023 15:24:26 +0200	[thread overview]
Message-ID: <Y+OiilaLU1NtfAEp@intel.com> (raw)
In-Reply-To: <687d94454a784738aed104d715516a6dc065593a.1675849634.git.jani.nikula@intel.com>

On Wed, Feb 08, 2023 at 11:48:39AM +0200, Jani Nikula wrote:
> The memory frequency detection is a bit spread out here and
> there. Consolidate to intel_dram.c.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_rps.c   |  29 -----
>  drivers/gpu/drm/i915/intel_pm.c       | 101 ----------------
>  drivers/gpu/drm/i915/soc/intel_dram.c | 158 +++++++++++++++++++++++++-
>  3 files changed, 157 insertions(+), 131 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index f5d7b5126433..4d0dc9de23f9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -1677,7 +1677,6 @@ static void vlv_init_gpll_ref_freq(struct intel_rps *rps)
>  static void vlv_rps_init(struct intel_rps *rps)
>  {
>  	struct drm_i915_private *i915 = rps_to_i915(rps);
> -	u32 val;
>  
>  	vlv_iosf_sb_get(i915,
>  			BIT(VLV_IOSF_SB_PUNIT) |
> @@ -1686,21 +1685,6 @@ static void vlv_rps_init(struct intel_rps *rps)
>  
>  	vlv_init_gpll_ref_freq(rps);
>  
> -	val = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
> -	switch ((val >> 6) & 3) {
> -	case 0:
> -	case 1:
> -		i915->mem_freq = 800;
> -		break;
> -	case 2:
> -		i915->mem_freq = 1066;
> -		break;
> -	case 3:
> -		i915->mem_freq = 1333;
> -		break;
> -	}
> -	drm_dbg(&i915->drm, "DDR speed: %d MHz\n", i915->mem_freq);
> -
>  	rps->max_freq = vlv_rps_max_freq(rps);
>  	rps->rp0_freq = rps->max_freq;
>  	drm_dbg(&i915->drm, "max GPU freq: %d MHz (%u)\n",
> @@ -1727,7 +1711,6 @@ static void vlv_rps_init(struct intel_rps *rps)
>  static void chv_rps_init(struct intel_rps *rps)
>  {
>  	struct drm_i915_private *i915 = rps_to_i915(rps);
> -	u32 val;
>  
>  	vlv_iosf_sb_get(i915,
>  			BIT(VLV_IOSF_SB_PUNIT) |
> @@ -1736,18 +1719,6 @@ static void chv_rps_init(struct intel_rps *rps)
>  
>  	vlv_init_gpll_ref_freq(rps);
>  
> -	val = vlv_cck_read(i915, CCK_FUSE_REG);
> -
> -	switch ((val >> 2) & 0x7) {
> -	case 3:
> -		i915->mem_freq = 2000;
> -		break;
> -	default:
> -		i915->mem_freq = 1600;
> -		break;
> -	}
> -	drm_dbg(&i915->drm, "DDR speed: %d MHz\n", i915->mem_freq);
> -
>  	rps->max_freq = chv_rps_max_freq(rps);
>  	rps->rp0_freq = rps->max_freq;
>  	drm_dbg(&i915->drm, "max GPU freq: %d MHz (%u)\n",
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e0364c4141b8..ee8f8d2d2a66 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -131,101 +131,6 @@ static void glk_init_clock_gating(struct drm_i915_private *dev_priv)
>  		   PWM1_GATING_DIS | PWM2_GATING_DIS);
>  }
>  
> -static void pnv_get_mem_freq(struct drm_i915_private *dev_priv)
> -{
> -	u32 tmp;
> -
> -	tmp = intel_uncore_read(&dev_priv->uncore, CLKCFG);
> -
> -	switch (tmp & CLKCFG_FSB_MASK) {
> -	case CLKCFG_FSB_533:
> -		dev_priv->fsb_freq = 533; /* 133*4 */
> -		break;
> -	case CLKCFG_FSB_800:
> -		dev_priv->fsb_freq = 800; /* 200*4 */
> -		break;
> -	case CLKCFG_FSB_667:
> -		dev_priv->fsb_freq =  667; /* 167*4 */
> -		break;
> -	case CLKCFG_FSB_400:
> -		dev_priv->fsb_freq = 400; /* 100*4 */
> -		break;
> -	}
> -
> -	switch (tmp & CLKCFG_MEM_MASK) {
> -	case CLKCFG_MEM_533:
> -		dev_priv->mem_freq = 533;
> -		break;
> -	case CLKCFG_MEM_667:
> -		dev_priv->mem_freq = 667;
> -		break;
> -	case CLKCFG_MEM_800:
> -		dev_priv->mem_freq = 800;
> -		break;
> -	}
> -
> -	/* detect pineview DDR3 setting */
> -	tmp = intel_uncore_read(&dev_priv->uncore, CSHRDDR3CTL);
> -	dev_priv->is_ddr3 = (tmp & CSHRDDR3CTL_DDR3) ? 1 : 0;
> -}
> -
> -static void ilk_get_mem_freq(struct drm_i915_private *dev_priv)
> -{
> -	u16 ddrpll, csipll;
> -
> -	ddrpll = intel_uncore_read16(&dev_priv->uncore, DDRMPLL1);
> -	csipll = intel_uncore_read16(&dev_priv->uncore, CSIPLL0);
> -
> -	switch (ddrpll & 0xff) {
> -	case 0xc:
> -		dev_priv->mem_freq = 800;
> -		break;
> -	case 0x10:
> -		dev_priv->mem_freq = 1066;
> -		break;
> -	case 0x14:
> -		dev_priv->mem_freq = 1333;
> -		break;
> -	case 0x18:
> -		dev_priv->mem_freq = 1600;
> -		break;
> -	default:
> -		drm_dbg(&dev_priv->drm, "unknown memory frequency 0x%02x\n",
> -			ddrpll & 0xff);
> -		dev_priv->mem_freq = 0;
> -		break;
> -	}
> -
> -	switch (csipll & 0x3ff) {
> -	case 0x00c:
> -		dev_priv->fsb_freq = 3200;
> -		break;
> -	case 0x00e:
> -		dev_priv->fsb_freq = 3733;
> -		break;
> -	case 0x010:
> -		dev_priv->fsb_freq = 4266;
> -		break;
> -	case 0x012:
> -		dev_priv->fsb_freq = 4800;
> -		break;
> -	case 0x014:
> -		dev_priv->fsb_freq = 5333;
> -		break;
> -	case 0x016:
> -		dev_priv->fsb_freq = 5866;
> -		break;
> -	case 0x018:
> -		dev_priv->fsb_freq = 6400;
> -		break;
> -	default:
> -		drm_dbg(&dev_priv->drm, "unknown fsb frequency 0x%04x\n",
> -			csipll & 0x3ff);
> -		dev_priv->fsb_freq = 0;
> -		break;
> -	}
> -}
> -
>  static const struct cxsr_latency cxsr_latency_table[] = {
>  	{1, 0, 800, 400, 3382, 33382, 3983, 33983},    /* DDR2-400 SC */
>  	{1, 0, 800, 667, 3354, 33354, 3807, 33807},    /* DDR2-667 SC */
> @@ -4909,12 +4814,6 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
>  		return;
>  	}
>  
> -	/* For cxsr */
> -	if (IS_PINEVIEW(dev_priv))
> -		pnv_get_mem_freq(dev_priv);
> -	else if (GRAPHICS_VER(dev_priv) == 5)
> -		ilk_get_mem_freq(dev_priv);
> -
>  	/* For FIFO watermark updates */
>  	if (HAS_PCH_SPLIT(dev_priv)) {
>  		ilk_setup_wm_latency(dev_priv);
> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
> index bba8cb6e8ae4..ad767b677486 100644
> --- a/drivers/gpu/drm/i915/soc/intel_dram.c
> +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
> @@ -10,6 +10,7 @@
>  #include "intel_dram.h"
>  #include "intel_mchbar_regs.h"
>  #include "intel_pcode.h"
> +#include "vlv_sideband.h"
>  
>  struct dram_dimm_info {
>  	u16 size;
> @@ -42,6 +43,156 @@ static const char *intel_dram_type_str(enum intel_dram_type type)
>  
>  #undef DRAM_TYPE_STR
>  
> +static void pnv_detect_mem_freq(struct drm_i915_private *dev_priv)
> +{
> +	u32 tmp;
> +
> +	tmp = intel_uncore_read(&dev_priv->uncore, CLKCFG);
> +
> +	switch (tmp & CLKCFG_FSB_MASK) {
> +	case CLKCFG_FSB_533:
> +		dev_priv->fsb_freq = 533; /* 133*4 */
> +		break;
> +	case CLKCFG_FSB_800:
> +		dev_priv->fsb_freq = 800; /* 200*4 */
> +		break;
> +	case CLKCFG_FSB_667:
> +		dev_priv->fsb_freq =  667; /* 167*4 */
> +		break;
> +	case CLKCFG_FSB_400:
> +		dev_priv->fsb_freq = 400; /* 100*4 */
> +		break;
> +	}
> +
> +	switch (tmp & CLKCFG_MEM_MASK) {
> +	case CLKCFG_MEM_533:
> +		dev_priv->mem_freq = 533;
> +		break;
> +	case CLKCFG_MEM_667:
> +		dev_priv->mem_freq = 667;
> +		break;
> +	case CLKCFG_MEM_800:
> +		dev_priv->mem_freq = 800;
> +		break;
> +	}
> +
> +	/* detect pineview DDR3 setting */
> +	tmp = intel_uncore_read(&dev_priv->uncore, CSHRDDR3CTL);
> +	dev_priv->is_ddr3 = (tmp & CSHRDDR3CTL_DDR3) ? 1 : 0;
> +}
> +
> +static void ilk_detect_mem_freq(struct drm_i915_private *dev_priv)
> +{
> +	u16 ddrpll, csipll;
> +
> +	ddrpll = intel_uncore_read16(&dev_priv->uncore, DDRMPLL1);
> +	switch (ddrpll & 0xff) {
> +	case 0xc:
> +		dev_priv->mem_freq = 800;
> +		break;
> +	case 0x10:
> +		dev_priv->mem_freq = 1066;
> +		break;
> +	case 0x14:
> +		dev_priv->mem_freq = 1333;
> +		break;
> +	case 0x18:
> +		dev_priv->mem_freq = 1600;
> +		break;
> +	default:
> +		drm_dbg(&dev_priv->drm, "unknown memory frequency 0x%02x\n",
> +			ddrpll & 0xff);
> +		dev_priv->mem_freq = 0;
> +		break;
> +	}
> +
> +	csipll = intel_uncore_read16(&dev_priv->uncore, CSIPLL0);
> +	switch (csipll & 0x3ff) {
> +	case 0x00c:
> +		dev_priv->fsb_freq = 3200;
> +		break;
> +	case 0x00e:
> +		dev_priv->fsb_freq = 3733;
> +		break;
> +	case 0x010:
> +		dev_priv->fsb_freq = 4266;
> +		break;
> +	case 0x012:
> +		dev_priv->fsb_freq = 4800;
> +		break;
> +	case 0x014:
> +		dev_priv->fsb_freq = 5333;
> +		break;
> +	case 0x016:
> +		dev_priv->fsb_freq = 5866;
> +		break;
> +	case 0x018:
> +		dev_priv->fsb_freq = 6400;
> +		break;
> +	default:
> +		drm_dbg(&dev_priv->drm, "unknown fsb frequency 0x%04x\n",
> +			csipll & 0x3ff);
> +		dev_priv->fsb_freq = 0;
> +		break;
> +	}
> +}
> +
> +static void chv_detect_mem_freq(struct drm_i915_private *i915)
> +{
> +	u32 val;
> +
> +	vlv_iosf_sb_get(i915, BIT(VLV_IOSF_SB_CCK));
> +	val = vlv_cck_read(i915, CCK_FUSE_REG);
> +	vlv_iosf_sb_put(i915, BIT(VLV_IOSF_SB_CCK));
> +
> +	switch ((val >> 2) & 0x7) {
> +	case 3:
> +		i915->mem_freq = 2000;
> +		break;
> +	default:
> +		i915->mem_freq = 1600;
> +		break;
> +	}
> +}
> +
> +static void vlv_detect_mem_freq(struct drm_i915_private *i915)
> +{
> +	u32 val;
> +
> +	vlv_iosf_sb_get(i915, BIT(VLV_IOSF_SB_PUNIT));
> +	val = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
> +	vlv_iosf_sb_put(i915, BIT(VLV_IOSF_SB_PUNIT));
> +
> +	switch ((val >> 6) & 3) {
> +	case 0:
> +	case 1:
> +		i915->mem_freq = 800;
> +		break;
> +	case 2:
> +		i915->mem_freq = 1066;
> +		break;
> +	case 3:
> +		i915->mem_freq = 1333;
> +		break;
> +	}
> +}
> +
> +static void detect_mem_freq(struct drm_i915_private *i915)
> +{
> +	/* For cxsr */

Not true for all the platforms...

> +	if (IS_PINEVIEW(i915))
> +		pnv_detect_mem_freq(i915);
> +	else if (GRAPHICS_VER(i915) == 5)
> +		ilk_detect_mem_freq(i915);
> +	else if (IS_CHERRYVIEW(i915))
> +		chv_detect_mem_freq(i915);
> +	else if (IS_VALLEYVIEW(i915))
> +		vlv_detect_mem_freq(i915);
> +
> +	if (i915->mem_freq)
> +		drm_dbg(&i915->drm, "DDR speed: %d MHz\n", i915->mem_freq);
> +}
> +
>  static int intel_dimm_num_devices(const struct dram_dimm_info *dimm)
>  {
>  	return dimm->ranks * 64 / (dimm->width ?: 1);
> @@ -507,8 +658,13 @@ void intel_dram_detect(struct drm_i915_private *i915)
>  	struct dram_info *dram_info = &i915->dram_info;
>  	int ret;
>  
> -	if (GRAPHICS_VER(i915) < 9 || IS_DG2(i915) || !HAS_DISPLAY(i915))
> +	if (!HAS_DISPLAY(i915) || IS_DG2(i915))
> +		return;

...which also means doing the HAS_DISPLAY check befoae 
detect_mem_freq() is kinda wrong.

I suppose we don't have any !HAS_DISPLAY stuff on these
platforms, but would seem better to be consistent anyway.

> +
> +	if (GRAPHICS_VER(i915) < 9) {
> +		detect_mem_freq(i915);
>  		return;
> +	}
>  
>  	/*
>  	 * Assume level 0 watermark latency adjustment is needed until proven
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

  reply	other threads:[~2023-02-08 13:24 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-08  9:48 [Intel-gfx] [PATCH 00/10] drm/i915/wm: legacy watermark code shuffling Jani Nikula
2023-02-08  9:48 ` [Intel-gfx] [PATCH 01/10] drm/i915: move memory frequency detection to intel_dram.c Jani Nikula
2023-02-08 13:24   ` Ville Syrjälä [this message]
2023-02-09  9:45     ` Jani Nikula
2023-02-08  9:48 ` [Intel-gfx] [PATCH 02/10] drm/i915/wm: move remaining watermark code out of intel_pm.c Jani Nikula
2023-02-08 13:55   ` Ville Syrjälä
2023-02-09 19:07     ` Jani Nikula
2023-02-08  9:48 ` [Intel-gfx] [PATCH 03/10] drm/i915/wm: move functions to call watermark hooks to intel_wm.[ch] Jani Nikula
2023-02-08  9:48 ` [Intel-gfx] [PATCH 04/10] drm/i915/wm: add .get_hw_state to watermark funcs Jani Nikula
2023-02-08 12:37   ` kernel test robot
2023-02-08 12:37     ` kernel test robot
2023-02-08 12:37   ` kernel test robot
2023-02-08 12:37     ` kernel test robot
2023-02-08 12:37   ` kernel test robot
2023-02-08 12:37     ` kernel test robot
2023-02-08 13:12   ` Ville Syrjälä
2023-02-09 19:28     ` Jani Nikula
2023-02-08  9:48 ` [Intel-gfx] [PATCH 05/10] drm/i915/wm: move watermark sanitization to intel_wm.[ch] Jani Nikula
2023-02-08  9:48 ` [Intel-gfx] [PATCH 06/10] drm/i915/wm: s/intel_wm_num_levels/g4x_wm_num_levels/ Jani Nikula
2023-02-08  9:48 ` [Intel-gfx] [PATCH 07/10] drm/i915/wm: add *_wm_num_levels() functions Jani Nikula
2023-02-08 19:56   ` Ville Syrjälä
2023-02-08  9:48 ` [Intel-gfx] [PATCH 08/10] drm/i915/wm: convert from ilk_wm_max_level() to {intel, skl}_wm_num_levels() Jani Nikula
2023-02-08  9:48 ` [Intel-gfx] [PATCH 09/10] drm/i915/wm: move watermark debugfs to intel_wm.c Jani Nikula
2023-02-08  9:48 ` [Intel-gfx] [PATCH 10/10] drm/i915: rename intel_pm_types.h -> display/intel_wm_types.h Jani Nikula
2023-02-08 12:59 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/wm: legacy watermark code shuffling Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y+OiilaLU1NtfAEp@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.